fix: normalize DeepSeek tool protocol responses

[qinxuye]

Summary

• move DeepSeek tool-protocol validation out of ReAct and into a provider-owned codec
• apply the codec to both direct DeepSeek calls and DeepSeek models selected through OpenRouter
• reject serialized DSML in assistant content, nested DSML inside structured tool arguments, unavailable tools, malformed arguments, and schema-forbidden arguments
• preserve progressive valid tool-call streaming while withholding incomplete or confirmed DSML marker tails
• surface provider failures as a typed protocol error; ReAct only owns the generic retry and the existing tool-authorization boundary
• keep ordinary DeepSeek chat unchanged when the request does not include tools

Root causes

Task 722 reached a repeated-tool forced-final turn. DeepSeek V4 Flash emitted a fetch_web_content call using its internal DSML grammar as ordinary assistant text. Because it was text rather than a native tool call, the response reached the final-answer stream.

Task 724 came through Auto Router -> OpenRouter -> DeepSeek and exposed a second shape. DeepSeek returned a native final_answer call, but embedded a serialized write_file call in answer and put the script in an unexpected content argument. The outer tool call looked valid enough for ReAct to finish, so neither write_file nor TTS ran.

These are provider response-normalization failures, not ReAct syntax. The provider codec validates non-streaming responses and progressively forwards safe accumulated tool-call snapshots. It withholds a possible split marker tail until it can be classified, performs authoritative validation on the complete response, and emits a generic protocol error without exposing DSML to the agent pattern or user stream.

Difference from #790

#790 parses leaked DSML inside the local forced-final path, then gives that parser every base_tool_schema even though the actual provider request intentionally exposes no work tools. A recovered fetch_web_content call can therefore be executed, clearing force_final_answer_next and bypassing the repeated-tool finalization decision. The visible leak becomes another real work-tool call and can restart the loop.

Its parser also has an unmatched-marker case where an incomplete DSML prefix before a later complete tool-call block causes the complete block to be discarded.

This PR does not reconstruct and execute unauthorized calls. The provider reports an invalid response; ReAct retries with exactly the tools allowed for that turn. Forced-final retries expose only final_answer, while normal turns such as task 724 retain their existing work tools.

Review follow-up

• restored progressive forwarding for valid DeepSeek tool-call snapshots
• detect serialized markers anywhere in a response, including after same-line prose
• withhold markers split across streaming chunks in both text and structured tool arguments
• mark discarded protocol responses as unsuccessful trace events while retaining their real usage accounting
• add dedicated coverage for all six protocol violation codes
• remove the unused recoverable field

Validation

• pytest -q tests/core/agent/test_react.py tests/core/agent/test_dag.py tests/core/agent/test_execution_adapter.py tests/core/agent/test_auto.py tests/core/agent/test_runtime.py tests/core/agent/test_react_segment_loop.py tests/core/model/chat/basic/test_deepseek.py tests/core/model/chat/basic/test_openrouter.py tests/core/model/chat/basic/test_deepseek_tool_protocol.py (282 passed)
• direct DeepSeek and OpenRouter DeepSeek integration regressions
• task 722 raw-content DSML regression
• task 724 nested write_file plus unexpected content regression
• same-line and split-marker streaming regressions
• progressive valid accumulated tool-call regression
• pre-commit hooks: ruff, format, mypy, whitespace, isort, codespell, and alembic check

[gemini-code-assist]

Code Review

This pull request enhances the ReAct pattern's final answer generation by explicitly requiring the final_answer tool and introducing a retry mechanism if the model outputs an invalid serialized tool-call protocol instead of a user-facing final answer. The unit tests have been updated to cover these changes. The feedback suggests ensuring that any active streaming sessions (answer_streamer) are cleanly closed/failed if validation fails on either the initial attempt or the retry attempt, preventing dangling streams on the runtime.

[qinxuye]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a retry mechanism in the ReAct agent pattern to handle cases where a forced final answer returns an invalid serialized tool-call protocol instead of a user-facing final answer. It ensures the final_answer tool schema is provided and required when forcing a final answer, updates LLM instructions, and adds comprehensive unit tests. Feedback on the changes suggests a robustness improvement in _forced_final_response_requires_retry to prevent a potential TypeError if the tool_calls key is explicitly set to None.

[rogercloud]

Summary

This PR fixes DeepSeek's tendency to violate the tool-call protocol by moving detection and rejection out of the ReAct agent and into a provider-owned codec (deepseek_tool_protocol.py, new), applied to both direct DeepSeek calls and DeepSeek-via-OpenRouter. The codec detects and rejects DSML markup leaked as plain assistant text (task 722), nested serialized tool calls embedded inside another call's structured arguments (task 724), calls to unavailable tools, malformed arguments, and schema-forbidden arguments — surfacing each as a typed ToolProtocolViolation. ReAct is left owning only a single generic retry (re-issuing with the same allowed tools) on top of its pre-existing tool-authorization boundary. This supersedes sibling PR #790, which reconstructed and executed recovered tool calls from parsed DSML and thereby risked bypassing forced-final-answer state.

This PR previously received several rounds of review that raised 3 inline findings; all 3 are now confirmed fixed (see "Previously reported issues" below). The branch has since been rebased/squashed into a single commit (8467be77 "fix: keep ReAct tool calls structured") on top of base 01a456691cfc5bdf8a1349d573c407f8f93a145c — the fix commits referenced in the author's earlier replies (ee1f5d6a, 5f267d67) are no longer reachable in the current history. There is therefore no separate "diff since last review" to isolate: this review re-evaluates the PR fresh in its entirety (a full Round 0/1/2 pass over the current diff, with every Major finding independently re-verified a second time), which already reflects the 3 prior fixes and every other change made since.

Previously reported issues

All 3 prior inline findings (from an earlier review of src/xagent/core/agent/pattern/react/react.py) were re-verified against the current code and are FIXED:

• A — Dangling stream on first invalid attempt (was high). FIXED. When _response_requires_tool_protocol_retry(...) is true (react.py:432-437), await answer_streamer.fail("invalid tool protocol, retrying") now runs before _retry_tool_protocol_response(...). The retry mechanism was generalized/renamed from _forced_final_response_requires_retry/_retry_serialized_final_answer into _response_requires_tool_protocol_retry/_retry_tool_protocol_response as part of the broader DeepSeek-protocol work, but the "fail the first streamer before retrying" guarantee carried forward intact.
• B — Dangling stream when the retry also fails (was high). FIXED. When the retry's response also violates protocol (react.py:448-460), await answer_streamer.fail("invalid tool protocol after retry") (line 452-453) runs before return PatternResult(success=False, ...). Confirmed by directly running the regression test test_react_closes_invalid_retry_final_answer_stream_before_failure (tests/core/agent/test_react.py:1087) — passes, asserting success is False, status == "invalid_tool_protocol", and the stream ending with final_answer_error rather than dangling.
• C — None tool_calls TypeError (was medium). FIXED at the reported site. _response_requires_tool_protocol_retry now uses normalized.get("tool_calls") or [] (react.py:712). Confirmed by running test_react_accepts_null_tool_calls_in_forced_final_response (tests/core/agent/test_react.py:1044) directly — passes.
  • Follow-up note (out of scope, not blocking): two other call sites in the same file have the same unguarded .get("tool_calls", []) pattern (react.py:947 in _normalize_tool_calls, react.py:1891 in _parse_react_decision). Both are pre-existing (identical in the base commit, untouched by this PR), so they are out of scope here — noting only for a possible separate follow-up.

Verdict: acceptable-with-reservations

The layering is right: normalization lives at the provider/model boundary, ReAct keeps only a generic retry, and the codec is shared cleanly between deepseek.py and openrouter.py. Critically, unlike #790 this fix does not reconstruct or execute unauthorized recovered calls — so it avoids #790's loop-restart hazard, where executing a recovered leaked tool call could clear force_final_answer_next and bypass the repeated-tool finalization decision. This is a root-cause fix at the correct architectural altitude and a genuine improvement over #790. The two Major findings below (a streaming UX regression and a detection gap that lets the exact leak this PR targets slip through permanently) are worth fixing before merge; both were independently re-verified twice, including a live reproduction of the M2 leak against the current code.

Findings

Major

M1 — Valid DeepSeek final answers no longer stream progressively. src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:67-69,100-112. adapt_deepseek_stream buffers every is_tool_call() chunk and only yields the last accumulated one at stream end. Before this PR, OpenAICompatibleLLM._parse_stream_chunk (openai.py:1184-1257) emitted an incremental accumulated-so-far TOOL_CALL chunk on every delta — genuinely progressive, and ToolCallStringFieldStreamer.handle_chunk (final_answer_stream.py:115-145) is built to consume that incremental sequence and stream the answer field to the user token-by-token. Because this buffering collapses that sequence down to one final chunk, valid final answers now appear in one shot at the end instead of streaming — a PR-introduced UX regression on the common, valid path. test_deepseek_stream_keeps_latest_accumulated_valid_tool_call itself confirms this by asserting only 1 chunk survives from 2 fed in.

Suggested fix (two coordinated changes):

1. In adapt_deepseek_stream, forward each tool-call chunk as it arrives instead of only buffering it (yield chunk in addition to tracking buffered_tool_chunk), and stop re-emitting buffered_tool_chunk a second time at stream end (the downstream snapshot-merge in _merge_tool_call_chunks/merge_streamed_tool_call_arguments already dedupes by accumulated snapshot, so re-forwarding won't duplicate content). Keep the authoritative end-of-stream _response_violation check and still emit PROTOCOL_ERROR if a violation is found.
2. In ToolCallStringFieldStreamer.handle_chunk (final_answer_stream.py), apply the same two-step guard already used for the TOKEN path to the decoded answer field value before emit_prefix: hold back a not-yet-safe trailing prefix (reusing _safe_streaming_text_length-style logic) and stop emitting entirely once _serialized_tool_call_start-style detection finds a complete leaked marker, deferring to the existing final_answer_error retraction signal (runtime.fail_final_answer_stream, already wired at react.py:437) rather than a new mechanism.

This restores progressive streaming for the common/valid path while still catching a leak before it's irreversibly shown, at the cost of ~15-25 lines plus a small shared helper; the only user-visible cost is a brief hold on lines that legitimately start with <, matching existing TOKEN-path behavior.

M2 — Marker detection is anchored to line-start, defeating the leak it guards against — and this leak is permanent, not just a transient streaming artifact. src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:221-231 (_serialized_tool_call_start) and :234-240 (_safe_streaming_text_length). Both only detect a DSML/serialized-tool-call marker when it is the first non-whitespace content on its line (candidate.lstrip().startswith("<")); the startswith check short-circuits before the substring checks run. Critically, _response_violation — the authoritative end-of-stream/non-streaming fallback check — routes through the same line-anchored _serialized_tool_call_start, so there is no marker-anywhere fallback anywhere in the module. A marker preceded by same-line prose (e.g. Sure: <｜｜DSML｜｜tool_calls>) is never detected, in streaming or non-streaming, and the raw markup reaches the user permanently, not just transiently mid-stream. This was independently reproduced against the live code: feeding 'Sure: <｜｜DSML｜｜tool_calls>\n<｜｜DSML｜｜invoke name="x">' through get_tool_protocol_error returns None (no violation detected) both as a plain response and as a streamed one, and a marker split across two chunks ("Answer: <｜" + "｜DSML｜｜tool_calls>") leaks its partial prefix <｜ before the second chunk even arrives. All 7 existing tests pass because none of them cover a same-line or split-prefix marker.

Suggested fix (concrete, both functions in deepseek_tool_protocol.py):

import re

# A complete serialized-tool-call opener anywhere in the text/line: "<", then
# delimiter/label chars with no ">" or newline in between, containing "dsml"
# followed by "tool_calls". Matches anywhere, not only at line-start.
_SERIALIZED_TOOL_CALL_RE = re.compile(
    r"<[^>\n]*dsml[^>\n]*tool_calls",
    re.IGNORECASE,
)

# A possibly-incomplete marker prefix anchored to end-of-string, used to withhold
# a trailing partial marker while streaming without holding back ordinary text
# like "< 6" or "<div>".
_PARTIAL_MARKER_RE = re.compile(
    r"<[^>\w\s]*(?:d(?:s(?:m(?:l[^>\w\s]*"
    r"(?:t(?:o(?:o(?:l(?:_(?:c(?:a(?:l(?:l(?:s)?)?)?)?)?)?)?)?)?)?)?"
    r")?)?)?)?\Z",
    re.IGNORECASE,
)

def _serialized_tool_call_start(content: Any) -> int | None:
    if not isinstance(content, str):
        return None
    match = _SERIALIZED_TOOL_CALL_RE.search(content)
    return match.start() if match else None

def _safe_streaming_text_length(content: str) -> int:
    start = _serialized_tool_call_start(content)
    if start is not None:
        return start
    search_from = 0
    while True:
        idx = content.find("<", search_from)
        if idx == -1:
            return len(content)
        if _PARTIAL_MARKER_RE.match(content, idx):
            return idx
        search_from = idx + 1

This searches the full content/line for the marker instead of anchoring to line-start, while [^>\n]* keeps the original precision of requiring the marker to sit within one <...>-delimited, single-line span (so it isn't meaningfully more prone to false positives on ordinary prose than the current line-based check). Add regression tests for: a same-line marker preceded by prose (non-streaming and streaming), and a marker split across two chunks.

Minor

N1 — Discarded first response is traced as a successful call on protocol-retry (re-scoped from an earlier "double-counting" framing). src/xagent/core/agent/pattern/react/react.py:424-428 calls runtime.on_llm_end on the raw first response before the protocol-violation check at :431-445; on a retry, _retry_tool_protocol_response makes its own second on_llm_start/on_llm_end call (:671/:697). On independent re-verification, recording usage for both calls is correct, not a bug — both are real API round-trips that really consumed tokens, so context.record_llm_usage appending a record for each is honest cost accounting, not double-counting. The genuine (and narrower) issue is only that the discarded first response's trace event carries "success": True" with no marker that it was rejected, which is misleading for anyone reading traces. Suggested fix: compute normalized/_response_requires_tool_protocol_retry(...) before calling on_llm_end on the first response, then pass metadata={"success": False, "phase": "discarded_invalid_protocol"} when a retry is needed — on_llm_end's metadata is merged in after the hardcoded "success": True", so this cleanly overrides it with no signature change and no effect on usage accounting.

N2 — No captured production payload checked in as a DSML fixture. src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:221-231. DSML detection is a narrow literal-substring match; test fixtures use a hand-authored <｜｜DSML｜｜tool_calls> string rather than a real task-722/724 payload. The ｜｜DSML｜｜ markup shape has real provenance — a prior 10+-commit series iterated on this exact shape over ~2 weeks of fix/revert cycles, so it is not an invented guess. Still, checking in a raw captured payload as a fixture would future-proof against format drift.

N3 — 4 of 6 violation codes have no dedicated test coverage. src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:145-208. Only serialized_tool_call_content and nested_serialized_tool_call are exercised; malformed_tool_call, malformed_tool_arguments, unavailable_tool_call, and unexpected_tool_arguments have none. In particular the task-724 "unexpected content argument" scenario is masked in both tests modeling it: the nested-DSML check runs before the schema/additionalProperties check, so the unexpected_tool_arguments branch never fires even though the test payload carries an extra content key. Suggestion: add a test per untested code, including an unexpected_tool_arguments case that omits the nested-DSML trigger so that branch actually runs.

N4 — No handling for a provider that hard-rejects tool_choice="required". src/xagent/core/agent/pattern/react/react.py:354-358,388-398. Forced-final turns require a final_answer tool call (tool_choice="required") for every provider (note: self.tool_choice already defaulted to "required" for ordinary ReAct tool-calling turns across all providers before this PR — this is not new). The graceful fallback for a provider that ignores the constraint and returns plain text (react.py:471) is real and tested with a non-DeepSeek mock. The residual gap: a provider that hard-rejects the request (e.g. a 400 because it cannot honor tool_choice="required") has no test or handling — the exception just propagates uncaught. Low-priority follow-up, not blocking.

N5 — Typed protocol error isn't propagated to DAG/Auto's existing retry paths. src/xagent/core/agent/runtime.py:190-199. The new typed signal (get_tool_protocol_error) is consumed only by ReAct. This is not a silent-failure regression: DAG's per-step execution reuses ReActPattern internally (so it already gets the new retry); DAG's completion-assessment call fails loudly via _fail(...) with a surfaced failure_reason; and Auto's decision-routing call has its own generic empty-tool-call retry loop (RequiredToolCallError → corrective feedback). This PR changes nothing about DAG/Auto behavior. Pre-existing gap, worth a follow-up, not a blocker.

Nit

T1 — Return bool instead of an offset. src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:221-231. _serialized_tool_call_start returns a precise int byte offset, but both call sites only test is None/is not None; no caller uses the numeric value. (Note: the M2 fix above repurposes this offset as the marker start position for _safe_streaming_text_length, so this nit is naturally resolved if M2's fix is applied.)

Simplification opportunities

• src/xagent/core/model/chat/tool_protocol.py:14: yagni recoverable: bool = True on ToolProtocolViolation is never set to False and never read anywhere in production, tests, or serialization consumers (full-repo grep). Remove it.
• src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:135-208: yagni ToolProtocolViolation.code carries 6 distinct string values, but no production code (react.py:710, runtime.py:190) branches on the value — both only check truthiness/presence. Several tests do assert specific code strings, so collapsing to a bare bool/None would require updating those assertions — not zero-risk, but the production case for keeping the taxonomy is weak.
• src/xagent/core/model/chat/basic/openrouter.py:63-118: dedup OpenRouterLLM.chat/stream_chat's ~25-30 lines of parameter-forwarding and codec-wrap boilerplate against DeepSeekLLM.chat/stream_chat (both ultimately call the same OpenAICompatibleLLM signature). A shared mixin could extract the common forwarding, but DeepSeek's extra pre-processing would need template-method hooks, so the net reduction is modest and adds indirection — worth doing only if it reads cleaner.

net: ~30-40 lines possible.

Testing

270 tests passed (pytest -q over test_react.py, test_dag.py, test_execution_adapter.py, test_auto.py, test_runtime.py, test_react_segment_loop.py, test_deepseek.py, test_openrouter.py, test_deepseek_tool_protocol.py), independently re-run in a fresh worktree — matches the PR's own stated validation. The 3 previously reported findings were additionally confirmed fixed by running their specific regression tests directly. Both Major findings above (M1, M2) were independently re-verified a second time by a separate reviewer pass, including a live reproduction of the M2 leak against the current code — none of the 270 passing tests cover the scenarios M1/M2 describe.

[qinxuye]

Addressed the new review in bf0edf2. M1/M2 are fixed entirely in the DeepSeek provider codec: valid accumulated tool-call snapshots stream progressively again, while same-line and split-chunk DSML markers are withheld and rejected before reaching the user. Also fixed discarded-response trace status, covered all six violation codes, and removed the unused recoverable field. N2 was not added because we should not check a raw production trace into the repo; N4/N5 are pre-existing non-blocking follow-ups. Local related suite: 281 passed; pre-commit: passed.

[qinxuye]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a robust tool protocol validation and retry mechanism specifically designed for DeepSeek models (including those routed via OpenRouter). It intercepts invalid tool protocol responses—such as serialized tool-call markup in plain text or nested serialized tool calls—and attempts a repair turn before failing. The feedback highlights two key areas for improvement: first, wrapping the retry call in a try...except LLMCallInterrupted block to ensure correct interrupt handling and metadata labeling; second, capping the string slice length in _safe_streaming_text_length to prevent potential $O(N^2)$ string copying overhead during streaming.

[rogercloud]

Re-review: commit 5a90f399 (follow-up to review at 8467be77)

Update summary

Since the last review the author pushed a single commit ("keep ReAct tool calls structured") touching deepseek_tool_protocol.py, react.py, runtime.py, and the two test files. It reworks DSML tool-call detection from line-anchored (lstrip().startswith('<')) to a regex .search() that matches the marker anywhere in the text, and restores progressive per-chunk forwarding of valid tool-call snapshots in adapt_deepseek_stream (no longer buffered until stream end). It also bounds the partial-marker tail scan to a 64-char window, wraps the protocol-retry LLM call in try/except LLMCallInterrupted so interrupts get the during_llm checkpoint label, reorders the first response's on_llm_end so a discarded/retried response is traced success=False, phase="discarded_invalid_tool_protocol" (usage/cost still recorded), adds test coverage for all six violation codes, and drops the unused recoverable field. Net effect: both prior Majors and both Gemini findings are addressed.

Prior findings status

All statuses below were independently re-verified against the current code (not taken from the author's self-report).

FIXED

Item	Origin	Evidence
M1 streaming buffering regression (Major)	prior review	adapt_deepseek_stream yields each chunk via _safe_streaming_tool_chunk; new test_deepseek_stream_forwards_accumulated_valid_tool_calls (2 chunks survive)
M2 line-anchored detection gap (Major)	prior review	_SERIALIZED_TOOL_CALL_RE.search(content); new same-line + split-marker tests, deepseek_tool_protocol.py:57-68/:286-299
N1 discarded response traced as success	prior review	retry check runs before on_llm_end; metadata success=False, phase="discarded_invalid_tool_protocol"
N3 4/6 violation codes untested	prior review	all 6 codes now tested, incl. non-masked unexpected_tool_arguments
T1 unused byte offset (Nit)	prior review	_safe_streaming_text_length now consumes the offset as a real value
G4 uncaught LLMCallInterrupted on retry (High)	gemini	retry wrapped in try/except, routes to _interrupt_if_requested(label="during_llm"); new test_react_pattern_pause_during_tool_protocol_retry_is_during_llm
G5 O(N²) tail slicing (Medium)	gemini	slice bounded to _PARTIAL_MARKER_SCAN_LIMIT = 64 at the slice point
"recoverable" field simplification	prior review	unused recoverable: bool = True removed from ToolProtocolViolation
A/B/C (dangling stream ×2, None tool_calls TypeError)	earliest round	unchanged, still fixed (carried forward for completeness)

NOT FIXED (still open, low-priority / non-blocking — as originally assessed)

• N2 — no captured production DSML fixture; fixtures remain hand-authored literals. Nice-to-have.
• N4 — no handling/test for a provider hard-rejecting tool_choice="required". Untouched.

WAIVED (re-confirmed non-issue)

• N5 — typed protocol error not propagated to DAG/Auto: DAG reuses ReActPattern internally; DAG completion-assessment and Auto decision-routing both already fail/retry generically against the synthetic protocol-error response. Nothing regressed.

Verdict

The two prior Major findings (M1, M2) and both Gemini findings (G4, G5) are fixed, and this round's fresh independent Round 0/1 pass surfaced no new Major or Critical issues. Design is sound: provider isolation is clean (no DeepSeek/DSML knowledge leaked into react.py/runtime.py), the ToolProtocolViolation / get_tool_protocol_error() seam is provider-agnostic, and ChunkType.PROTOCOL_ERROR threads uniformly through streaming and non-streaming paths. This PR no longer has any blocking Major findings and is acceptable to merge, modulo the two still-open low-priority Minors above and the two new Minors below.

New findings

New-Minor-1 — protocol-error response drops usage on the non-streaming path
src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:35-36, src/xagent/core/model/chat/tool_protocol.py:19-32 — Severity: Minor.
normalize_deepseek_response builds its error response via tool_protocol_error_response(violation, raw=raw), which does not carry usage forward. When hit, runtime.on_llm_end's _extract_token_usage finds no usage key and context.record_llm_usage is skipped for that turn — a narrow loss of context-level cost/usage accounting, contradicting the "retain real usage accounting" intent. Not reachable through ReAct's normal loop today (ReAct always uses run_streaming_llm_call, whose consume_stream() builds its own error dict that merges usage); it only matters for the streaming-unavailable fallback or a future non-streaming .chat() caller with tools. Suggested fix: have tool_protocol_error_response copy usage from the raw response when present.

New-Minor-2 (perf) — full-text rescan on every TOKEN chunk
src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:57-68, :286-299 — Severity: Minor.
_serialized_content_violation / _safe_streaming_text_length run on the FULL accumulated text on every TOKEN chunk, each doing a regex .search() / re.finditer("<", ...) over the whole string so far — O(N²) over the length of a long streamed answer. G5's O(N²) slicing is fixed, but this enclosing full-text rescan remains. Negligible for typical few-KB responses (C-level regex) but grows for very long completions. Follow-up: scan only from the previously-scanned position forward, or track a rolling window.

Design-level notes (informational, not blocking): the DSML marker grammar (｜｜DSML｜｜tool_calls) is hard-coded to the one observed variant — worth a one-line comment noting it tracks an observed provider quirk. And adapt_deepseek_stream implicitly assumes each tool-call chunk is a cumulative snapshot (verified correct against openai.py's _process_stream_chunk, but an unguarded cross-file invariant).

Simplification opportunities

• src/xagent/core/agent/pattern/react/react.py:662: yagni: _retry_tool_protocol_response recomputes tools = [self._final_answer_tool_schema()] if force_final_answer else tool_schemas, but its sole caller already sets tool_schemas to exactly that when force_final_answer is true. Replace with tools = tool_schemas.
• src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:325: yagni: _function_payload's getattr(tool_call, "function", None) and flat-dict fallbacks handle shapes that never occur — every caller produces a dict with a nested function. Simplify to tool_call.get("function", {}) if isinstance(tool_call, dict) else {}.
• src/xagent/core/agent/runtime.py:154-160: shrink: the protocol-error chunk check is inlined, duplicating the pattern already factored into _chunk_text_delta / _chunk_tool_calls / _chunk_usage. Extract a matching _chunk_protocol_error(chunk) helper.
• (carry-forward, unchanged) src/xagent/core/model/chat/basic/deepseek_tool_protocol.py:135-208: yagni: ToolProtocolViolation.code's 6-value taxonomy is still only checked for truthiness in production; collapsing to a bool/None is possible but modest given test assertions on code strings.
• (carry-forward, unchanged) src/xagent/core/model/chat/basic/openrouter.py:63-118 vs deepseek.py: ~25-30 lines of duplicated parameter-forwarding/codec-wrap boilerplate; a shared mixin needs template-method hooks for DeepSeek's pre-processing, so net reduction stays modest.

net: -20 to -30 lines possible (3 new confirmed this round, plus 2 open carry-forward items).

Testing

pytest -q passes on test_react.py (66 passed) and test_deepseek_tool_protocol.py (14 passed). The new regression tests for each fix were run directly and pass: progressive streaming, same-line and split-marker detection, interrupt-during-retry checkpoint label, discarded-response trace metadata, and all six violation codes. No regressions found in this update.