test(e2e): stop ancillary LLM calls draining the mock forced-response queue (#4517)#4519
Conversation
… queue (tinyhumansai#4517) The desktop chat agent is the orchestrator archetype, which always advertises delegate_* tools (analyze_image, delegate_*) whose schema requires `prompt` + `citation_requirement`. The E2E mock's `llmForcedResponses` is a process-global one-shot FIFO consumed one entry per /chat/completions. Fire-and-forget thread- title generation (`chat_with_system`, tools: None) is dispatched on user send and races the visible turn — draining a scripted response, desyncing the queue, so the turn falls through to the dynamic fallback which fabricates a call to declared[0] (= analyze_image) with {url} args → "arguments.prompt is required" and the scripted canary never renders. Fix (mock-side, no core change): - Only the primary *interactive* turn drains the forced/stream queue. Guard both consumption sites (streaming + non-stream) with isPrimaryTurn(parsedBody) = request advertises tools. Ancillary no-tools completions (title/summary) now fall through to dynamic and leave the queue intact. - Harden the dynamic fallback: inferToolCalls only fabricates calls for simple worker tools (no required `prompt`); delegate-only turns return benign text instead of a schema-error tool call, so any residual desync is legible. Validated against the running mock: a no-tools title-gen call leaves the queue intact; consecutive tool-bearing turns receive web_fetch then the canary in order; a delegate-only drained turn returns text, not a broken analyze_image call, while simple worker tools still fabricate. Resolves the single-turn forced-queue specs (user-journey-full-task, chat-tool-call-flow, chat-thread-todo-strip). Genuinely multi-agent specs (chat-harness-subagent, chat-multi-tool-round) additionally need their forced arrays to cover the sub-agent's own model calls — tracked in tinyhumansai#4517. Refs tinyhumansai#4517 Claude-Session: https://claude.ai/code/session_01UAetDHQCyFtWnyP7RwGdJU
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 783867c13f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // desync and the turn falls through to the dynamic fallback | ||
| // (tinyhumansai/openhuman#4517). | ||
| function isPrimaryTurn(parsedBody) { | ||
| return Array.isArray(parsedBody?.tools) && parsedBody.tools.length > 0; |
There was a problem hiding this comment.
When the mocked provider is used with prompt-guided/local providers, the main agent request intentionally has no tools array (tinyagents/model.rs only passes tools when supports_native_tools() is true and specs are non-empty; Ollama/LM Studio explicitly opt out of native tools). Those requests are still primary user turns that tests can script with llmForcedResponses, but this predicate now treats them as ancillary and leaves the FIFO untouched, so the response falls through to the dynamic/default text instead of the configured script. Please distinguish the title/summary calls by a more specific signal than tools.length so prompt-guided primary turns can still consume forced responses.
Useful? React with 👍 / 👎.
1 participant
Summary
analyze_imagecall.Problem
The desktop chat agent is the orchestrator archetype, which always advertises
delegate_*tools (incl.analyze_image, whose schema requiresprompt+citation_requirement). The E2E mock'sllmForcedResponsesis a process-global one-shot FIFO consumed one entry per/chat/completions. Fire-and-forget thread-title generation (chat_with_system,tools: None, dispatched on user send inapp/src/store/threadSlice.ts) races the visible turn and drains a scripted response. The queue desyncs → the turn falls through to the dynamic fallback, which fabricates a call todeclared[0](=analyze_image) with{url}args →arguments.prompt is required, and the scripted canary never renders.Full root-cause trace in #4517.
Solution
scripts/mock-api/routes/llm.mjs:isPrimaryTurn(parsedBody)= the request advertises tools. Only the primary interactive turn drains the forced/stream queue (both the streaming and non-stream consumption sites). Ancillary no-tools completions (title/summary) fall through to the dynamic responder and leave the queue intact.scripts/mock-api/routes/llm/dynamic.mjs:inferToolCalls: only fabricate tool calls for simple worker tools (no requiredprompt). Delegate-only turns return benign text instead of a schema-error tool call, so any residual desync is legible rather than a confusinganalyze_imagebubble.Validated against the running mock: a no-tools title-gen call leaves the queue intact; consecutive tool-bearing turns receive
web_fetchthen the canary in order; a delegate-only drained turn returns text (not a brokenanalyze_imagecall) while simple worker tools still fabricate.Submission Checklist
srclines).Impact
user-journey-full-task,chat-tool-call-flow,chat-thread-todo-strip) and likelychat-multi-tool-round(local-tool, no sub-agent). No runtime/product behavior change.Related
chat-harness-subagent(orchestrator→researcher) additionally needs itsFORCED_RESPONSESto cover the sub-agent's own model calls (or convert tollmKeywordRules, which are content-addressed and never shifted); tracked there.onboarding-modeswizard (disabled-Whisper-option / vault-step), both needing app-level/harness work.https://claude.ai/code/session_01UAetDHQCyFtWnyP7RwGdJU