Skip to content

fix(pi): keep model-only messages deliverable - #31863

Closed
enwaiax wants to merge 1 commit into
stablyai:mainfrom
enwaiax:fix/pi-model-default-effort
Closed

enwaiax wants to merge 1 commit into
stablyai:mainfrom
enwaiax:fix/pi-model-default-effort

Conversation

@enwaiax

@enwaiax enwaiax commented Oct 22, 2026

Copy link
Copy Markdown

ELI5

Choosing a different Pi model without also choosing an effort made Orca reject the message before Pi ever saw it. This change lets Pi keep the effort it selects for the new model, so the prompt is delivered normally.

What Changed

Before: writing model also recorded an effort write even though Orca never sent set_thinking_level. The following effort pass treated that synthetic write as proof that an unknown explicit effort was required and rejected the message.

After: a model write still invalidates the cached current effort, but it no longer claims that Orca wrote an effort. If the message has no explicit effort, Orca leaves Pi's model-selected thinking level alone and proceeds to the prompt. Explicit model + effort messages retain their existing set_model → set_thinking_level order.

Why

set_model belongs to Pi, and Pi chooses and reports the applicable thinking level for that model. Marking effort as written without sending an effort command creates false proof and blocks valid model-only messages. Removing only that false write marker preserves the concurrency guard for real option writes while keeping the cached effort invalidated after a model change.

Linked Issue

Fixes #31849

Visual Proof

N/A — no visual or interaction layout changes. The failure and fix are covered at the Pi RPC dispatch boundary.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Verified locally on Linux:

  • pnpm exec vitest run --config config/vitest.config.ts src/main/pi/rpc-session-adapter.test.ts src/main/pi/rpc-options.test.ts — 23/23 passed
  • Added a regression that changes model without effort and proves only set_model is sent before the prompt
  • Sabotage check: restoring the false effort write marker makes the new regression fail with The default effort for this model is unknown
  • pnpm run check:code-quality:changed — passed with 0 findings
  • Node typecheck: pnpm run build:packages && node config/scripts/run-typecheck-projects-in-parallel.mjs --project=config/tsconfig.node.json --compiler=native — passed
  • oxfmt --check, focused oxlint --deny-warnings, and git diff --check — passed

AI Disclosure

OpenAI GPT-5.6-sol assisted with root-cause validation, implementation, tests, and review.

Review

The change removes one synthetic option-proof write and adds one dispatch regression. It does not alter credentials, process launch, RPC wire formats, persistence, UI, mobile, or remote/SSH behavior. Explicit effort writes and their supersession checks are unchanged. Performance impact is none.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Cross-platform: provider-neutral TypeScript logic; tested on Linux.
  • Remote/SSH: no transport or host-routing changes.
  • Mobile: mobile uses the same host dispatch contract and receives the corrected behavior without protocol changes.
  • Backwards compatibility: no schema or wire changes; explicit model + effort behavior remains covered by the existing test.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (targeted tests, changed-code gate, and native Node typecheck pass; full repository gates remain for CI)

Author X handle: @enwaiax

Signed-off-by: enwaiax <114924709+enwaiax@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

When applying a model option, the code no longer records an effort write. A new test checks that a message with a model selection and no effort selection is admitted, sends only a set_model request, and prompts Pi with the message text.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to 66d82

The model-only regression test does not verify the selected model or that it is applied before the prompt. This is a bounded coverage gap, and the change remains mergeable with follow-up.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: keeping Pi messages deliverable when only the model changes.
Description check Passed The description follows the required template and explains the user impact, mechanism, rationale, linked issue, testing, compatibility considerations, and checklist status. It clearly notes that full …
Linked Issues check Passed Issue #31849 requires model-only Pi messages to reach Pi without an explicit effort. The PR removes the synthetic proof.writes entry for effort while retaining proof.current.delete('effort') aft…
Out of Scope Changes check Passed The diff contains one focused runtime change in src/main/pi/rpc-options.ts and one regression test in src/main/pi/rpc-session-adapter.test.ts. Both changes support issue #31849. No unrelated produ…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Full details: Description check

Explanation

The description follows the required template and explains the user impact, mechanism, rationale, linked issue, testing, compatibility considerations, and checklist status. It clearly notes that full repository checks remain for CI.

Full details: Linked Issues check

Explanation

Issue #31849 requires model-only Pi messages to reach Pi without an explicit effort. The PR removes the synthetic proof.writes entry for effort while retaining proof.current.delete('effort') after set_model. The new regression test confirms that a model-only dispatch is admitted, sends only set_model, and sends the prompt. The existing test confirms that explicit model plus effort still sends set_model before set_thinking_level. These changes address the reported rejection path.

Full details: Out of Scope Changes check

Explanation

The diff contains one focused runtime change in src/main/pi/rpc-options.ts and one regression test in src/main/pi/rpc-session-adapter.test.ts. Both changes support issue #31849. No unrelated product, protocol, persistence, UI, transport, or configuration changes are present in the reviewed diff.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments: 1
src/main/pi/rpc-session-adapter.test.ts: 1

213-216: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the selected model and dispatch order.

This test checks request names and prompt delivery in separate arrays. It can pass if set_model receives the wrong model or if Pi receives the prompt before set_model. Record the request payload and both operations in one ordered sequence. Assert that anthropic/model-2 is selected before Pi receives "chosen".


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 450d92e7-7a00-471a-85be-f8aa2b7fc4cd
📥 Commits

Reviewing files that changed from the base of the PR and between 1769476 and 66d82b5.

📒 Files selected for processing: 2
  • src/main/pi/rpc-options.ts
  • src/main/pi/rpc-session-adapter.test.ts
💤 Files with no reviewable changes: 1
  • src/main/pi/rpc-options.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@enwaiax

enwaiax commented Oct 23, 2026

Copy link
Copy Markdown
Author

Closing as superseded by #32004, which has now merged a broader Pi + ACP fix for model-only dispatch and includes the away/back/away regression coverage. Thanks.

@enwaiax enwaiax closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Pi native chat rejects every message when Orca's Pi model preference has no effort for it

2 participants