enh: optimize routed agent context usage

[qinxuye]

Summary

• resolve xrouter virtual models before context compaction, derive the threshold from the selected model's context window, and reuse the same route for the LLM call
• record the selected model and context window in LLM trace metadata
• retry OpenRouter DeepSeek requests once after removing unsupported assistant text prefixes from historical tool-call messages
• reduce provider-facing tool overhead by compacting presentation-only schema metadata, lowering the default user-file listing page size, and making browser session diagnostics opt-in

Why

Auto-routed models did not expose the selected model's context length before compaction, so long-running tasks could compact at the fallback threshold even when xrouter selected a much larger-context model. Large default tool schemas and file listings also consumed avoidable context on every call.

This keeps routing and compaction aligned with the concrete downstream model while reducing low-value tool context. It also recovers from DeepSeek's deterministic rejection of assistant prefixes on historical function-call messages.

Validation

• 706 agent and chat-model tests
• 21 workspace file-operation tests
• 13 workspace durable-registration tests
• Ruff check and format
• repository pre-commit hooks, including mypy

[gemini-code-assist]

Code Review

This pull request introduces optimizations and robustness improvements to the agent's LLM routing and context management. Key changes include resolving virtual models before context compaction to prevent redundant routing, compacting tool schemas to reduce token usage, making browser debug tools opt-in, and lowering the default file listing limit from 1000 to 50. Additionally, retry logic was added for OpenRouter DeepSeek models when they reject assistant prefixes in tool-call history. The feedback points out a potential AttributeError in RouterLLM where self.context_window is accessed directly, and suggests using getattr to safely retrieve it.

[qinxuye]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces several optimizations and fixes, including a mechanism to resolve virtual models before context compaction to avoid double routing, automatic retries for OpenRouter DeepSeek calls when rejected due to assistant tool-call prefixes, a reduction in the default workspace file listing limit from 1000 to 50, and making browser debug tools opt-in. The review feedback points out that the new _ResolvedRouterLLM class lacks model_id and timeout properties, which could lead to AttributeErrors in downstream components, and suggests adding them for full compatibility with the BaseLLM interface.

[rogercloud]

Summary

This PR restructures how routed agents (xrouter virtual models) resolve their downstream model and derive context-compaction thresholds. Instead of routing twice, it resolves the route once (prepare_for_call / _ResolvedRouterLLM) and reuses it for the LLM call, deriving the compaction threshold from the selected model's context_window and recording the selected model + context window in trace metadata. It also adds a one-shot OpenRouter/DeepSeek retry that strips unsupported assistant text prefixes from historical tool-call messages, and trims provider-facing tool context (compacted tool-description/JSON-schema whitespace, default file-listing page size 1000→50, and making the browser session-diagnostics tool opt-in).

Reviewed at commit ba748c81 (includes the fix: expose resolved router metadata commit pushed during this review).

Follow-up / re-review status

Two prior gemini-code-assist reviews (both COMMENTED, no formal verdict) raised two findings:

• Prior finding A — RouterLLM context_window direct access (AttributeError risk): FIXED. prepare_for_call now reads getattr(self, "context_window", None) (router.py:430) with a regression test covering the missing-attribute case.
• Prior finding B — _ResolvedRouterLLM missing model_id / timeout: FIXED (as of ba748c81). _ResolvedRouterLLM now defines proper model_id (delegating to self._downstream.model_id) and timeout (delegating to self._router.timeout) properties, with a test asserting both. Confirmed RouterLLM.timeout is unconditionally set in __init__ (never None), so the delegation is safe.

Design verdict (Round 0)

Sound / acceptable-with-minor-reservations. Collapsing "route twice" into "route once, reuse" is a clean fit, and the new threshold derivation in runtime.prepare_llm_for_context mirrors the existing runner._resolve_compact_threshold formula, complementing the existing mechanism rather than fighting it. Two design-level notes (not line bugs):

• Threshold formula duplicated. runner._resolve_compact_threshold (runner.py:585-598, pre-existing) and the new runtime.prepare_llm_for_context (runtime.py:31-61) both compute max(1, int(context_window * get_compact_threshold_ratio())) with no shared helper. The recomputation-after-routing is deliberate, but a shared compute_compact_threshold(context_window) helper would remove the double-maintenance burden.
• Tool-description whitespace collapse is broad and blunt. _compact_tool_description / _compact_tool_json_schema run " ".join(desc.split()) on every tool description and every nested schema description, for every provider. This destroys intentional structure (numbered steps, bullet lists) in tool docs, silently trading instruction-following fidelity for token savings across the entire tool surface. Judgment call, not a bug — but worth confirming this is intended and measuring the fidelity impact.

Findings

Inline comments below cover the MAJOR and MINOR line-level findings. Summary:

• MAJOR — react.py tool-protocol retry path uses the unresolved llm instead of the resolved call_llm, so it can re-route to a different model and loses trace metadata (see inline comment).
• MINOR — runtime.py compaction-threshold overwrite can silently break the documented "resumed tasks keep their checkpointed threshold" guarantee (see inline comment).
• MINOR — openrouter.py strips the message history twice per DeepSeek retry (see inline comment).
• MINOR — router.py's _resolve is now dead production code, kept alive only by a test (see inline comment).
• MINOR — react.py's schema compaction can skip stripping title when a tool parameter's name collides with a JSON-Schema structural keyword (properties/$defs/etc.) — purely cosmetic, no functional impact (see inline comment).

Test coverage gaps

1. test_runtime.py — only the router (prepare_for_call) path is tested for prepare_llm_for_context/resolved_llm_metadata. Add tests for the plain-LLM-with-context_window branch and the missing/None-context_window branch (runtime.py:44-59).
2. test_router.py — _profile_context_window's exception branch (router.py:482-494) is never hit; both tests monkeypatch the method itself rather than triggering its internal try/except.
3. test_openrouter.py — add a test for "sanitized retry also fails → error propagates, no second retry", and one asserting whitespace-only content → changed=False → no retry.
4. test_react.py — add a test exercising _retry_tool_protocol_response with a router-style LLM (prepare_for_call) to lock down the route-reuse/metadata behavior in the MAJOR finding above end-to-end.
5. test_workspace_file_operations.py — the pagination test uses a workspace id that never resolves to a task_id, so it skips the real DB-backed pagination path (workspace.py:1293-1300). Add a test with a resolving workspace, >50 records, and assertions that two pages return different files plus correct total_count truncation signaling.

Documentation

• File-listing default page size dropped 1000→50 (DEFAULT_USER_FILE_LIST_LIMIT, workspace.py) — discoverable via total_count/limit/offset, but a changelog/doc note explaining the smaller default would help.
• browser_list_sessions diagnostics now opt-in — no production caller passes include_debug_tools=True, so this tool is effectively unavailable to agents by default. Intended, but worth documenting.

Simplification opportunities

• src/xagent/core/model/chat/basic/openrouter.py (chat(), initial call ~L114-124 vs. sanitized retry ~L134-144): DUPLICATION — the two super().chat(...) calls repeat the same 7 kwargs + **kwargs verbatim, differing only in messages. Build a shared kwargs dict once and reuse.
• src/xagent/core/model/chat/basic/openrouter.py (_stream_chat_with_prefix_retry, ~L164-174 vs. ~L189-199): same duplicated 7-kwarg pattern for super().stream_chat(...). Build once, reuse.

net: -10 lines possible

Test results

All 155 tests across the changed test files (test_react.py, test_runtime.py, test_browser_tools.py, test_openrouter.py, test_router.py, test_workspace_file_operations.py) pass in the review worktree.

[qinxuye]

Addressed the re-review in 67eb1fb: all five inline findings are fixed, and the five listed coverage gaps now have regression tests (plain/checkpoint thresholds, profile lookup failure, DeepSeek retry failure and whitespace handling, routed protocol-retry reuse, and real DB-backed file pagination). Tool-description compaction also now preserves paragraph/list structure. I left the shared-threshold-helper extraction, duplicated kwargs cleanup, and documentation notes as non-functional follow-ups to keep this fix focused. Validation: 192 review-related tests plus Ruff/format and the full pre-commit hook set, including mypy.

[rogercloud]

Re-review (Round 2) — commit ba748c81 → 67eb1fb2

Update summary. Since the last review, the author pushed 67eb1fb2 ("fix: preserve routed call invariants"), replying to all five inline findings. I independently re-verified each fix against the diff and its regression tests: all five are correctly resolved, each backed by a passing regression test, and no regressions were introduced by the fix commit. The change is tightly scoped — every edit maps directly to one of the five findings with no extraneous complexity.

Prior findings status

#	Severity	Finding	Status	Fix + proving test
1	MAJOR	react.py tool-protocol retry re-routes	✅ FIXED	Call site passes the resolved call_llm (not llm) into _retry_tool_protocol_response; retry trace metadata now uses resolved_llm_metadata(call_llm). Proven by test_react_reuses_resolved_route_for_tool_protocol_retry — a real RouterLLM across 2 iterations asserts selected_models == ["test/model-1", "test/model-2"], confirming the iteration-2 retry reuses model-2 rather than re-routing.
2	MINOR	runtime.py threshold overwrite on resume	✅ FIXED	New prepared is not llm guard gates the threshold-overwrite block, so plain/fixed models preserve their runner-initialized or checkpoint-restored threshold. Proven by test_prepare_llm_for_context_preserves_plain_llm_threshold (parametrized over context_window ∈ {None, 128_000}), which would fail under the old code.
3	MINOR	openrouter.py double-strip on DeepSeek retry	✅ FIXED	_should_retry_deepseek_function_prefix replaced by _deepseek_function_prefix_retry_messages, which strips once and returns the sanitized list; both sync and streaming paths call it exactly once. Covered by new tests for sanitized-retry failure propagation and whitespace-only skip.
4	MINOR	router.py _resolve dead code	✅ FIXED	_resolve removed entirely; the three tests that called it directly were rewritten against prepare_for_call with no loss of coverage. No orphaned ._resolve( references remain in src/ or tests/.
5	MINOR	react.py schema-compaction title bug on name collision	✅ FIXED	Name-based preserve_mapping_keys replaced by structural named_schema_mapping, decoupling "structural JSON-Schema keyword" from "field named like one." New regression test builds a schema with fields literally named properties and title and confirms correct per-field title stripping. Bonus: _compact_tool_description now preserves paragraph/list structure via per-line whitespace collapse.

New findings this round

Test coverage gap (recommendation, non-blocking). src/xagent/core/agent/pattern/auto/auto.py's _decide (≈ lines 762-798) mirrors the react.py pattern — it calls prepare_llm_for_context, builds resolved_llm_metadata-based trace metadata, and reuses the resolved call_llm — but there is no test in tests/core/agent/test_auto.py exercising this path with a router-style LLM. The existing fakes implement only chat/stream_chat and never prepare_for_call, so resolved_llm_metadata is always asserted empty (test_auto.py:983, 1047-1048) and the router-resolution branch of _decide is never exercised. Recommend adding an auto.py test mirroring test_react_reuses_resolved_route_for_tool_protocol_retry. This is a nice-to-have, not a blocker — the underlying route-reuse logic is proven by the shared runtime-layer tests (finding #2) and the end-to-end react.py test (finding #1).

For the record, two additional candidates were investigated this round and confirmed to be non-issues: (a) _compact_tool_description's per-line whitespace collapse cannot damage indentation-semantic content — the behavior is intentional, tested, and no YAML/aligned-table tool descriptions exist in the codebase; (b) the **metadata spread in _run_repeated_tool_decision cannot let a caller override resolved_llm_metadata keys — that metadata is built internally from _repeated_tool_call_metadata with a fixed key set that structurally cannot collide.

Design note

Per-iteration route re-evaluation means the compaction threshold can still fluctuate across iterations if auto-routing selects models with very different context windows on consecutive calls. This is inherent to and intended by the per-call routing design — noted for awareness, not a new issue.

Test results

225 tests pass across all changed/affected test files (test_react.py: 77, test_runtime.py: 35, test_openrouter.py: 21, test_router_provider_config.py: 22, plus test_browser_tools.py, test_auto.py, test_router.py, test_workspace_file_operations.py) at commit 67eb1fb2.

Overall verdict

All five prior findings are resolved with passing regression coverage, and the simplification lens over the ba748c81..67eb1fb2 delta surfaced nothing new. This PR is ready to merge from a review perspective. The only outstanding item is the optional auto.py router-path test, which is a nice-to-have rather than a merge blocker.