Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 134
fix: plan-mode refusals on altimate-default + warning false-positive (#887)#888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
d21f9b16bbcfbfb07da4ea88f40677ffd9264f72f2098a646File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // altimate_change start — shared vendor classifier for `model.family` values. | ||
| // | ||
| // `model.family` is a free-form string in the Model schema; the model registry | ||
| // (`packages/opencode/test/tool/fixtures/models-api.json`) uses *specific* | ||
| // values like `claude-sonnet`, `claude-haiku`, `gemini-pro`, `gemini-flash`, | ||
| // `gpt-codex`, etc. — never the coarse `anthropic`/`gemini`/`openai` literals. | ||
| // Routing code that exact-matches against the coarse names silently misses | ||
| // real models, recreating the GH #887 misrouting class on any altimate-backend | ||
| // gateway path that exposes Claude or Gemini models. Use this helper to map a | ||
| // specific family value to its vendor bucket. | ||
| // | ||
| // Reviewer-driven addition (PR #888 review thread, J1 from the multi-LLM | ||
| // panel). Keep this the single source of truth so the prompt-routing and the | ||
| // hoist-decision both interpret family the same way. | ||
| export type Vendor = "anthropic" | "gemini" | "openai" | ||
| export function familyVendor(family: string | undefined): Vendor | undefined { | ||
| if (!family) return undefined | ||
| const f = family.toLowerCase() | ||
| if (f === "anthropic" || f === "claude" || f.startsWith("claude-")) return "anthropic" | ||
| if (f === "gemini" || f.startsWith("gemini-")) return "gemini" | ||
| if (f === "openai" || f === "openai-compatible" || f === "gpt" || f.startsWith("gpt-")) return "openai" | ||
| return undefined | ||
| } | ||
| // altimate_change end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -303,7 +303,7 @@ export namespace SessionProcessor { | ||
| }) | ||
| break | ||
| case "finish-step": | ||
| case "finish-step": { | ||
| const usage = Session.getUsage({ | ||
| model: input.model, | ||
| usage: value.usage, | ||
| @@ -348,10 +348,27 @@ export namespace SessionProcessor { | ||
| // Users read the text, see no progress, and abandon. Surface a | ||
| // warning + telemetry so the pattern is measurable and the user | ||
| // knows to try a different model. | ||
| // | ||
| // sessionToolCallsMade tracks tool calls in the CURRENT step only | ||
| // — SessionProcessor.create() is called per-step by loop() (see | ||
| // prompt.ts), so the closure variable resets each step. A multi- | ||
| // step plan-mode session (read → grep → read → … → final text) | ||
| // would then false-positive on the final text-only step. Also | ||
| // scan streamInput.messages for any prior assistant tool-call | ||
| // content; if found, the session has used tools and the warning | ||
| // should be suppressed. | ||
| const sessionHasPriorToolCalls = | ||
| sessionToolCallsMade > 0 || | ||
| streamInput.messages.some( | ||
| (m) => | ||
| m.role === "assistant" && | ||
| Array.isArray(m.content) && | ||
| m.content.some((p) => p.type === "tool-call"), | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if ( | ||
| input.assistantMessage.agent === "plan" && | ||
| value.finishReason === "stop" && | ||
| sessionToolCallsMade === 0 && | ||
| !sessionHasPriorToolCalls && | ||
| !planNoToolWarningEmitted | ||
| ) { | ||
| planNoToolWarningEmitted = true | ||
| @@ -381,9 +398,13 @@ export namespace SessionProcessor { | ||
| type: "text", | ||
| synthetic: true, | ||
| text: | ||
| `⚠️ altimate-code: the \`plan\` agent is running on \`${input.model.providerID}/${input.model.id}\`, ` + | ||
| `which returned text without calling any tools. If you expected the plan agent to explore the ` + | ||
| `codebase, try switching to a model with stronger tool-use via \`/model\`.`, | ||
| `⚠️ altimate-code: the \`plan\` agent on \`${input.model.providerID}/${input.model.id}\` ` + | ||
| `stopped without calling any tools — it neither read, searched, nor explored the codebase. ` + | ||
| `Common causes: (a) the model wrote a plan from prompt context alone, (b) the model declined ` + | ||
| `to engage with the request (content-policy refusal), or (c) the request was too thin to act on. ` + | ||
| `To recover, try one of: reply asking it to investigate first (\`read\`/\`grep\`/\`glob\`/\`explore\`); ` + | ||
| `rephrase the request more concretely; or \`/model\` to a tier that's more eager to explore ` + | ||
| `(e.g. Claude Sonnet/Opus).`, | ||
| time: { start: Date.now(), end: Date.now() }, | ||
| }) | ||
| } | ||
| @@ -424,6 +445,7 @@ export namespace SessionProcessor { | ||
| needsCompaction = true | ||
| } | ||
| break | ||
| } | ||
| case "text-start": | ||
| currentText = { | ||
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH · code-reviewer] The fix for the false-positive warning checks for prior tool calls in messages, but does not account for tool calls that occurred in earlier steps but were not preserved in the current streamInput.messages.
💡 Suggestion: Consider tracking tool usage in a session-scoped state variable (e.g., on the session object) rather than relying solely on message inspection, which may be incomplete due to step isolation.
Confidence: 85/100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified false against a live trace, not deferring to inspection. In a 4-step plan-mode session captured under
~/.local/share/altimate-code/log/plan-debug/, the final-stepstreamInput.messagescontained 7 messages including 3role: "assistant"messages withtool-callcontent parts (and the matchingrole: "tool"results). The path is:MessageV2.toModelMessages(message-v2.ts:707-769) emits every completed tool call as atool-${name}-typed UIMessage part, AI SDK'sconvertToModelMessageslowers those intotool-callcontent on the ModelMessage, and our predicatep.type === "tool-call"matches. Empirically a multi-step plan session DOES populatestreamInput.messageswith prior tool calls, so the check correctly suppresses the false positive without needing a session-scoped state field.