mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-12 18:15:33 +00:00
fix(v1): address review feedback (multimodal provider gate, save default, leak scope, continuation sampling)
- Drop multimodal content for non-OpenAI-family providers (Google/Anthropic raised on OpenAI image_url parts) so multimodal requests degrade to text instead of returning 500. - Stateless continuations (no conversation_id) default save_conversation to False, avoiding an orphan conversation with an empty question on every tool round (stateful continuations still default True). - Only strip leaked reasoning from content for structured requests (response_format / json_schema / json_object); legitimate answers that mention the marker text are no longer corrupted. - Forward sampling params (temperature, max_tokens, ...) on continuation turns. - An explicit json_object request clears an agent-configured json_schema so it isn't silently overridden. - Drop max_tokens when max_completion_tokens is also sent (OpenAI rejects both). - Don't build a chatcmpl-None completion id from the placeholder "None" id event.
This commit is contained in:
1 parent
9eb7878ad3
commit
37417f99c8
3 files changed
+57
-10
No files matched your search
@@ -715,6 +715,9 @@ class StreamProcessor:
|
||||
self.agent_config["json_schema_strict"] = self.data.get("json_schema_strict")
|
||||
if self.data.get("json_object"):
|
||||
self.agent_config["json_object"] = True
|
||||
# An explicit json_object request beats an agent-configured schema
|
||||
# (otherwise the configured json_schema would silently override it).
|
||||
self.agent_config["json_schema"] = None
|
||||
|
||||
def _configure_retriever(self):
|
||||
"""Assemble retriever config; agent's values are authoritative when bound."""
|
||||
@@ -1309,6 +1312,18 @@ class StreamProcessor:
|
||||
if client_tools:
|
||||
tool_executor.client_tools = client_tools
|
||||
|
||||
# OpenAI-style image_url content parts are only understood by the
|
||||
# OpenAI-family providers; drop multimodal content for others (Google,
|
||||
# Anthropic, ...) so a multimodal request degrades to text rather than
|
||||
# erroring upstream.
|
||||
from application.llm.openai import OpenAILLM
|
||||
|
||||
request_multimodal = (
|
||||
self.data.get("multimodal_content")
|
||||
if isinstance(llm, OpenAILLM)
|
||||
else None
|
||||
)
|
||||
|
||||
agent_kwargs = {
|
||||
"endpoint": "stream",
|
||||
"llm_name": provider or settings.LLM_PROVIDER,
|
||||
@@ -1326,7 +1341,7 @@ class StreamProcessor:
|
||||
"json_schema_strict": self.agent_config.get("json_schema_strict", True),
|
||||
"json_object": self.agent_config.get("json_object", False),
|
||||
"llm_params": self.data.get("llm_params") or {},
|
||||
"multimodal_content": self.data.get("multimodal_content"),
|
||||
"multimodal_content": request_multimodal,
|
||||
"compressed_summary": self.compressed_summary,
|
||||
"llm": llm,
|
||||
"llm_handler": llm_handler,
|
||||
|
||||
@@ -158,6 +158,12 @@ def chat_completions():
|
||||
return usage_error
|
||||
|
||||
should_save_conversation = bool(internal_data.get("save_conversation", False))
|
||||
# Only strip leaked reasoning from content for structured requests -- the
|
||||
# only path where models echo reasoning into content -- so legitimate
|
||||
# answers that mention the marker text are never corrupted.
|
||||
strip_reasoning_leak = bool(
|
||||
internal_data.get("json_schema") or internal_data.get("json_object")
|
||||
)
|
||||
|
||||
if is_stream:
|
||||
return Response(
|
||||
@@ -169,6 +175,7 @@ def chat_completions():
|
||||
model_name,
|
||||
continuation,
|
||||
should_save_conversation,
|
||||
strip_reasoning_leak,
|
||||
),
|
||||
mimetype="text/event-stream",
|
||||
headers={
|
||||
@@ -185,6 +192,7 @@ def chat_completions():
|
||||
model_name,
|
||||
continuation,
|
||||
should_save_conversation,
|
||||
strip_reasoning_leak,
|
||||
)
|
||||
|
||||
except ValueError as e:
|
||||
@@ -215,6 +223,7 @@ def _stream_response(
|
||||
model_name: str,
|
||||
continuation: Optional[Dict],
|
||||
should_save_conversation: bool,
|
||||
strip_reasoning_leak: bool = False,
|
||||
) -> Generator[str, None, None]:
|
||||
"""Generate translated SSE chunks for streaming response."""
|
||||
completion_id = f"chatcmpl-{int(time.time())}"
|
||||
@@ -258,11 +267,13 @@ def _stream_response(
|
||||
# Update completion_id when we get the conversation id
|
||||
if event_data.get("type") == "id":
|
||||
conv_id = event_data.get("id", "")
|
||||
if conv_id:
|
||||
if conv_id and conv_id != "None":
|
||||
completion_id = f"chatcmpl-{conv_id}"
|
||||
|
||||
# Translate to standard format
|
||||
translated = translate_stream_event(event_data, completion_id, model_name)
|
||||
translated = translate_stream_event(
|
||||
event_data, completion_id, model_name, strip_reasoning_leak
|
||||
)
|
||||
for chunk in translated:
|
||||
yield chunk
|
||||
|
||||
@@ -275,6 +286,7 @@ def _non_stream_response(
|
||||
model_name: str,
|
||||
continuation: Optional[Dict],
|
||||
should_save_conversation: bool,
|
||||
strip_reasoning_leak: bool = False,
|
||||
) -> Response:
|
||||
"""Collect full response and return as single JSON."""
|
||||
stream = helper.complete_stream(
|
||||
@@ -309,6 +321,7 @@ def _non_stream_response(
|
||||
thought=result["thought"] or "",
|
||||
model_name=model_name,
|
||||
pending_tool_calls=pending,
|
||||
strip_reasoning_leak=strip_reasoning_leak,
|
||||
)
|
||||
return make_response(jsonify(response), 200)
|
||||
|
||||
|
||||
@@ -243,6 +243,10 @@ def translate_request(
|
||||
):
|
||||
if data.get(_k) is not None:
|
||||
sampling_params[_k] = data[_k]
|
||||
# OpenAI rejects sending both; the provider maps max_tokens ->
|
||||
# max_completion_tokens, so drop the alias when the canonical key is present.
|
||||
if "max_completion_tokens" in sampling_params:
|
||||
sampling_params.pop("max_tokens", None)
|
||||
|
||||
# Check for continuation (tool results after assistant tool_calls)
|
||||
if is_continuation(messages):
|
||||
@@ -259,11 +263,13 @@ def translate_request(
|
||||
# rebuilt from the resent messages instead of server-side state.
|
||||
"messages": messages,
|
||||
}
|
||||
# A continuation only exists if turn 1 was saved, so default to True —
|
||||
# otherwise the final turn and its WAL row are never persisted. An
|
||||
# explicit override is honoured if the client resends it.
|
||||
# Stateful continuations (with a conversation_id) default to True so the
|
||||
# final turn / WAL row is persisted. Stateless continuations (no
|
||||
# conversation_id, e.g. opencode) default to False — otherwise every
|
||||
# tool round persists an orphan conversation with an empty question. An
|
||||
# explicit ``docsgpt.save_conversation`` override is honoured either way.
|
||||
result["save_conversation"] = bool(
|
||||
data.get("docsgpt", {}).get("save_conversation", True)
|
||||
data.get("docsgpt", {}).get("save_conversation", bool(conversation_id))
|
||||
)
|
||||
# Carry tools forward for next iteration
|
||||
if data.get("tools"):
|
||||
@@ -273,6 +279,8 @@ def translate_request(
|
||||
result["json_schema_strict"] = json_schema_strict
|
||||
if json_object_mode:
|
||||
result["json_object"] = True
|
||||
if sampling_params:
|
||||
result["llm_params"] = sampling_params
|
||||
return result
|
||||
|
||||
# Normal request — extract the question (text) from the last user message,
|
||||
@@ -337,6 +345,7 @@ def translate_response(
|
||||
thought: str,
|
||||
model_name: str,
|
||||
pending_tool_calls: Optional[List[Dict]] = None,
|
||||
strip_reasoning_leak: bool = False,
|
||||
) -> Dict[str, Any]:
|
||||
"""Translate DocsGPT response to chat completions format.
|
||||
|
||||
@@ -378,7 +387,10 @@ def translate_response(
|
||||
]
|
||||
finish_reason = "tool_calls"
|
||||
else:
|
||||
clean_answer, leaked_reasoning = _split_leaked_reasoning(answer)
|
||||
if strip_reasoning_leak:
|
||||
clean_answer, leaked_reasoning = _split_leaked_reasoning(answer)
|
||||
else:
|
||||
clean_answer, leaked_reasoning = answer, ""
|
||||
message["content"] = clean_answer
|
||||
combined_reasoning = (thought or "") + leaked_reasoning
|
||||
if combined_reasoning:
|
||||
@@ -470,6 +482,7 @@ def translate_stream_event(
|
||||
event_data: Dict[str, Any],
|
||||
completion_id: str,
|
||||
model_name: str,
|
||||
strip_reasoning_leak: bool = False,
|
||||
) -> List[str]:
|
||||
"""Translate a DocsGPT SSE event dict to standard streaming chunks.
|
||||
|
||||
@@ -490,7 +503,10 @@ def translate_stream_event(
|
||||
chunks: List[str] = []
|
||||
|
||||
if event_type == "answer":
|
||||
clean, leaked = _split_leaked_reasoning(event_data.get("answer", ""))
|
||||
raw = event_data.get("answer", "")
|
||||
clean, leaked = (
|
||||
_split_leaked_reasoning(raw) if strip_reasoning_leak else (raw, "")
|
||||
)
|
||||
if leaked:
|
||||
chunks.append(
|
||||
_make_chunk(completion_id, model_name, {"reasoning_content": leaked})
|
||||
@@ -589,7 +605,10 @@ def translate_stream_event(
|
||||
chunks.append(f"data: {json.dumps(error_data)}\n\n")
|
||||
|
||||
elif event_type == "structured_answer":
|
||||
clean, leaked = _split_leaked_reasoning(event_data.get("answer", ""))
|
||||
raw = event_data.get("answer", "")
|
||||
clean, leaked = (
|
||||
_split_leaked_reasoning(raw) if strip_reasoning_leak else (raw, "")
|
||||
)
|
||||
if leaked:
|
||||
chunks.append(
|
||||
_make_chunk(completion_id, model_name, {"reasoning_content": leaked})
|
||||
|
||||
Reference in new issue
Block a user