Skip to content

fix(native-chat): new chats' model picker opens with the full list instead of a partial one - #36646

Merged
brennanb2025 merged 9 commits into
mainfrom
brennanb2025/model-picker-partial-list
Oct 31, 2026
Merged

brennanb2025 merged 9 commits into
mainfrom
brennanb2025/model-picker-partial-list

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1045 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1045
Prod 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​194 $\color{#cf222e}{\Huge{\mathbf{−}}}$​70 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​124

Problem

Opening the model picker in a new Codex or Claude chat could briefly show only part of the list, often just the remembered model, before the rest appeared. Each folder has its own list. A new folder had none, and startup discovery only populated the home-folder entry, leaving the existing account fallback empty.

Change

Orca saves the account list learned during startup and uses it while a new folder’s own list loads. The folder’s result then replaces it, including any project-specific models and defaults.

The provisional list declares neither the folder’s default model nor its default effort. Ordinary reads stay immediate when the account is not known yet. Account changes and shutdown invalidate obsolete results. An account request that reuses or joins an existing listing still publishes the account list. Provisional answers remain folder-scoped, and visiting many folders cannot discard the known account identity. If a later account lookup fails or times out, an existing valid list and its defaults remain available until a successful replacement arrives.

There are no new saved fields, wire fields or RPCs. Older desktop and phone clients use the reads they already make; older hosts retain their existing behavior.

Differences from the common pattern

  • Intended: Folder lists stay authoritative because project configuration can change membership: Codex model_provider, model_providers and model_catalog_json, and Claude availableModels. The account list is provisional, so Orca does not duplicate the CLIs’ configuration rules.
  • Intended: Only the account’s own requested listing supplies the account list, including reuse of that same home listing. An arbitrary project’s restricted list never becomes another project’s fallback.
  • Intended: With no account list yet, or before the current account is known, the picker keeps its existing behavior. A saved folder list also remains authoritative while refreshed. This change improves the answer available to a new folder; it does not promise a full list before the first host response.

Verification

Tests cover both Claude and Codex: immediate provisional membership, the folder’s final list and defaults, preservation of explicit and unlisted choices, account retirement and shutdown, passive reads, shared listing/cache reuse, and a real catalog-file save and reload.

A production-path operation-count contract uses 61/610 models and 10/100 unrelated folders/readers. It checks one account publication plus the existing exact publication, no extra probes or writes from passive reads, coalesced persistence, unchanged-data suppression, and delivery of genuinely changed membership. This is an operation-count check, not a latency claim.

Focused catalog, RPC, shared option-display and renderer tests passed. The final repair passed 61 tests across the account service, renderer-cache integration and terminal enrichment files; subsequent checks of the two modified files passed 42 and 4 tests. Regressions cover failed and timed-out refreshes, late completion, subsequent success, account changes and newer publications, plus the actual structured shown default and terminal catalog consumption. All five provisional producer-to-cache checks remain covered in the existing cross-surface test project. The receiving-account regression file also passed 14/14, including proof that an unavailable receiving account never borrows the original account’s fallback list and that resolved account inventory stays correctly scoped. No app run was required for these catalog projection and persistence flows. Local phone-hook tests could not collect because react-test-renderer is absent; shared production option-display tests cover the changed input for both agents.

…own listing runs

A new Claude or Codex chat in a folder with no saved model list showed only its remembered model (or nothing) until that folder's model probe finished, then the full list appeared 1-2 s later. Every new worktree and every chat after a restart hit this, because these agents keep one list per folder and the account-level list reads already fall back to was never written.

The account's own listing (prewarm names no folder) now also saves that list, and a read whose folder isn't listed yet answers from it, including before its scope resolves. The stand-in names no default as the folder's; the folder's own listing still replaces it.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 31, 2026 08:54
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cdbddd74-d769-40af-b927-823fdd4e0951
Commits

Reviewing files that changed from the base of the PR and between ae3db80 and adc9527.

Files selected for processing: 1
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-list.test.ts

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

Walkthrough

The service identifies eligible account-level catalog listings and resolves catalog fingerprints for session records. It records completed folder listings at a base fingerprint without copying the folder’s configured default. For qualifying reads, it can use an account observation when the later snapshot remains unknown and preserve the listing-in-progress indicator. Snapshot projection suppresses default selection under specified conditions. Tests cover pending listings, folder isolation, lifecycle handling, persistence, option state, and scaling.

Merge Risk: Low · up to adc95

A scope-resolution failure can temporarily hide folder-specific models or defaults, but the impact is localized and the existing published finding identifies a small, straightforward fix.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description clearly explains the problem, change, scope, compatibility, and verification. It is incomplete against the repository template because it omits the required Linked Issue, Visual Proof … Add an actual issue link under Linked Issue, provide before/after visual proof or write N/A with a reason, complete the Testing section and platform coverage, complete the Review, Notes, Agent skill upstream boundary, and Checklist sections…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary user-facing change: new chats open the model picker with the full list instead of a partial list.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 8 functions across 7 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The description clearly explains the problem, change, scope, compatibility, and verification. It is incomplete against the repository template because it omits the required Linked Issue, Visual Proof or an explicit N/A explanation, testing checkboxes and platform details, review notes, and the checklist.

Resolution

Add an actual issue link under Linked Issue, provide before/after visual proof or write N/A with a reason, complete the Testing section and platform coverage, complete the Review, Notes, Agent skill upstream boundary, and Checklist sections, and add AI Disclosure if the contributor is external.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
  • 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

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

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 63df04c3-ea4b-48ed-a82f-896f9f7a0e09
Commits

Reviewing files that changed from the base of the PR and between 55507b8 and 1b80900.

Files selected for processing: 6
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-list.test.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-list.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-fingerprint.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-snapshot.ts

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

@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/native-chat/agent-model-catalog/agent-model-catalog-account-list.test.ts: 1

204-205: Trivial · Maintainability & Code Quality · Quick win

Let the late account resolution settle before you assert that no probe ran.

home.resolve(HOME) only schedules the continuations that wait on home.promise. The next statement, expect(h.runs).toEqual([]), runs synchronously in the same turn. No continuation has run at that point, so h.runs is always empty, even if the service ignores its deadline and probes /cold.

As written, this check cannot catch a regression where a late account resolution still starts a listing. Yield to the event loop before the assertion. Alternatively, wait for a short interval and also check that h.resolve was never called.

💚 Proposed fix
       home.resolve(HOME)
+      await new Promise((resolve) => setTimeout(resolve, 0))
+      expect(h.resolve).not.toHaveBeenCalled()
       expect(h.runs).toEqual([])

Based on learnings, the repository asks for tests "that would actually catch a regression, not shallow coverage that only exercises the happy path."

Source: Learnings


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a85d0928-0b5c-4a31-ad8e-80a5ed31d521
Commits

Reviewing files that changed from the base of the PR and between 1b80900 and 8362e5f.

Files selected for processing: 4
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-list.test.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-list.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-snapshot.ts

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

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

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1d7bf938-dd29-4384-93a2-009ca9a10c65
Commits

Reviewing files that changed from the base of the PR and between 8362e5f and b0f8d3e.

Files selected for processing: 4
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-list.test.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-snapshot.ts
  • src/renderer/src/runtime/host-model-catalog-snapshots.test.ts

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

New Claude and Codex chats could briefly show only a remembered model or a partial list while their folder’s inventory loaded. Startup discovery populated a different entry, leaving the existing account fallback empty.

This PR makes the known account list available while each folder’s own list loads. The provisional answer declares no folder default model or effort, remains folder-scoped, and never controls whether a user’s literal choice is accepted. A valid same-account result also survives failed refreshes until a successful replacement arrives; real account changes, retirement and shutdown still invalidate it.

Five review rounds and the unanimous design-panel ruling are complete. Review fixed configured-default leakage, obsolete account replies, missed cached/joined publication, passive-read waiting, provisional scope metadata, account-proof eviction and failed-refresh demotion. Fresh correctness, consumer, architecture, performance, readiness and whole-mechanism reference checks accepted the final mechanism; the continued reviewer agreed. The addressed automated thread is resolved.

Verified for both Claude and Codex through the real service/store, persistence save/reload, shared shown-model/effort view, renderer cache and terminal discovery. The focused run passed 61 tests across three files, with final modified-file checks passing 42 and 4 tests. Six new regressions failed before the repair. Operation-count contracts cover 61/610 models and 260/2,600 requests, with no extra passive probes/writes and bounded canonical retention. The main-merge receiving-account regression file passed 14/14. CI is green on the merged head, including actually executed compiler/type checks, the cross-surface type ratchet, all twelve unit shards, wire compatibility, persistence, packaging and WSL jobs.

QA is tests-only: no app-only behavior, layout or interaction change was identified. No app, real-agent, emulator or screenshot run is claimed. Local phone-hook tests did not collect because react-test-renderer is absent; shared production response-to-shown-output contracts cover the changed input for both agents. The change does not promise a full list before the first host response; an account without a known list keeps its existing first-use behavior.

Two unproved concerns remain deferred: freshness bookkeeping when an old complete list is reused after a failed refresh, and precision of one effort-display assertion. Neither has a demonstrated user-visible failure.

For Brennan’s review:

Review and tests-row evidence are on 5747e32. The conflict-exception main merge eaafb55 incorporates main 002e975. Its narrow three-way review preserves main’s extracted lookup and receiving-account checks together with the approved publication rule; a demonstrated fallback interaction was corrected using those existing ownership facts and the targeted 14-test check. Evidence carries across that merge under the main-merge rule; final CI covers eaafb55.

@brennanb2025
brennanb2025 merged commit a2235ad into main Oct 31, 2026
48 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/model-picker-partial-list branch October 31, 2026 17:12
LuweiLiao added a commit to LuweiLiao/orca that referenced this pull request Oct 31, 2026
Adapt upstream stablyai#36646 to the LLW catalog service. Keep temporary rows free of project defaults and retain account isolation.

(cherry picked from commit a2235ad)
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.

1 participant