Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title names the main change: models other than the current one now have their effort menus marked unknown, so new chats can seed an effort. It uses the fix(acp) convention and stays specific to th… Description check The description follows the repository template closely. It has ELI5, What Changed, Why, Linked Issue (Fixes #31995), Visual Proof marked N/A with a reason, Testing with commands and results, AI Discl…Linked Issues check Issue #31995requires a saved model without a saved effort to remain deliverable while the agent retains its own thinking level.AcpStructuredOptions.read()now marks non-current models as `effortsK…Out of Scope Changes check The pull request changes ACP model-effort catalog facts and updates the related regression test. These changes directly support issue #31995by distinguishing an unknown effort menu from a confirmed e…Full details: Title check
-
Explanation
The title names the main change: models other than the current one now have their effort menus marked unknown, so new chats can seed an effort. It uses the fix(acp) convention and stays specific to the ACP options code. It is a single sentence, a bit long at 85 characters, but it is clear.
Full details: Description check
-
Explanation
The description follows the repository template closely. It has ELI5, What Changed, Why, Linked Issue (Fixes
#31995), Visual Proof marked N/A with a reason, Testing with commands and results, AI Disclosure, the upstream boundary checkbox, Notes, and the checklist. It explains the before and after behavior and the alternative that was rejected. The Testing section shows the author ran targeted tests, lint, and format checks. The full pnpm lint, typecheck, test, and build checks were left to CI, which the author states openly. The 'Review' heading is empty, which is a minor gap.
Full details: Linked Issues check
-
Explanation
Issue
#31995requires a saved model without a saved effort to remain deliverable while the agent retains its own thinking level.AcpStructuredOptions.read()now marks non-current models aseffortsKnown: falsewhen the agent reports an effort option but does not advertise a per-model menu. This prevents an explicit empty effort list from rejecting the seeded effort. The current model still uses the session menu, and dialect-provided per-model menus remain supported. The updated test verifies that the non-current model has an unknown, not empty, effort menu.
Full details: Out of Scope Changes check
-
Explanation
The pull request changes ACP model-effort catalog facts and updates the related regression test. These changes directly support issue
#31995by distinguishing an unknown effort menu from a confirmed empty menu. No unrelated implementation or test changes appear in the whole-PR diff.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…n seed an effort An ACP agent without per-model effort metadata (OMP) only reports the running session's thought_level menu. Every other model was recorded with efforts: [], which admission reads as 'offers no effort', so a new chat seeded with that model plus an effort was refused with optionRejected. Mark those rows effortsKnown: false instead.
6ec0c15 to
d59d1eb
Compare
pandudpn
changed the title
…ort in new chats (#33646) OMP native chats keep a remembered or explicit effort on models other than OMP's current one: for dialects that clamp unlisted efforts themselves (OMP only, via clampsUnlistedEffort), non-current models' efforts are recorded as unknown instead of "admits none"; catalog merges keep an older explicit empty list. OpenCode keeps main's behaviour. Fixes #31995 Supersedes #31998 Co-authored-by: Pandu dwi Putra <218730385+pandudpn@users.noreply.github.com>
ELI5
When you open a new OMP chat on a model other than OMP's starting model, Orca sends that model plus your thinking level. Orca's saved model list said that model had no thinking levels at all, so it refused to create the chat and the desktop showed "Chat connection could not be confirmed." OMP never said that; it only lists thinking levels for the model a session is running. Orca now records "unknown" for the other models and lets the agent decide.
What Changed
Before:
AcpStructuredOptions.read()gave every model except the session's current oneefforts: []when the dialect has no per-model menu (OMP has nomodelEfforts). The catalog stored those rows, andassertAgentSessionOptionsAdmittedreads an explicit empty menu as "admits none" (agentSessionEffortLevels). A new chat seeded with{model: 'omniroute/coding-max', effort: 'xhigh'}was refused withagent_session_operation_invalid/optionRejected, before any session record was written.After: when the agent reports a
thought_leveloption but no per-model menu, models other than the current one get{ efforts: [], effortsKnown: false }.agentSessionEffortLevelsreturnsnullfor those rows, so admission lets the effort through. Catalog persistence already keepseffortsKnown: false(agent-model-catalog-persistence.ts).Unchanged:
thought_leveloption keepsefforts: [], so an effort is still refused there.This PR first carried the send-path guard fix. #32004 shipped that fix upstream, so I rebased onto
mainand replaced the old commit with this one.Why
#32004 fixed sending a message after a model-only change. The same report shows up again on 1.4.239 one step earlier, at chat creation, because the composer seeds an effort (OMP's
defaultThinkingLevel) and the catalog row for the seeded model claims no efforts.Alternative considered: copying the session's effort menu onto every model. Rejected, because Orca would then claim menus OMP never reported for those models.
Linked Issue
Fixes #31995
Visual Proof
N/A. No interface changes. The difference is that the create request is accepted instead of failing behind the "Chat connection could not be confirmed." banner.
Testing
Throwaway check (not committed): an OMP-shaped session (
model+thought_levelconfig options, current modelomniroute/chat) fed intoassertAgentSessionOptionsAdmitted('omp', models, { model: 'omniroute/coding-max', effort: 'xhigh' }). It throws onmainand passes on this branch. An unlisted effort on the current model is still refused.tsc -p config/tsconfig.node.jsonreports two errors insrc/main/git/command-runner/exec-file-capture.ts. They come frommainand have nothing to do with this change.AI Disclosure
The patch and tests were drafted with an AI coding agent (OMP, running an OmniRoute-hosted model), then reviewed and verified on macOS by the reporter.
Review
Agent skill upstream boundary
Notes
Cross-platform: main-process TypeScript only, no paths or platform APIs.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred). Targeted vitest, oxlint and oxfmt ran locally; the rest is left to CI.