Skip to content

fix(model-selection): unify catalog and session identity writes - #2117

Open
beruro wants to merge 3 commits into
developfrom
junyu/model-selection-consistency
Open

beruro wants to merge 3 commits into
developfrom
junyu/model-selection-consistency

Conversation

@beruro

@beruro beruro commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Model choice has multiple competing interpretations across KeyVault, the desktop picker, mobile catalog, session persistence, and the running provider. A refresh can overwrite newer account preferences; a model/account change can be acknowledged while a stale runtime remains installed; rapid selections and late save replies can mix or roll back the pair. Ordinary KeyVault saves also merge a UI snapshot instead of the latest persisted record, so concurrent edits can erase unrelated fields.

Solution

Make KeyVault the authoritative account catalog and apply discovery and ordinary account edits against its latest locked record. Keep discovered defaults separate from explicit user overrides, preserve an intentionally empty enabled inventory, and project the same selectable models to desktop and mobile. Route desktop and mobile session identity changes through one serialized persistence/runtime boundary, capture the admitted provider and lease for an in-flight turn, and queue optimistic frontend pair/default saves so only confirmed current intent updates recent/default choices. Existing selection controls and interaction steps remain intact.

Potential risks

credentials.json gains serde-defaulted catalog generation and discovered-default fields. Old files load without migration; legacy default_variants are conservatively treated as explicit user choices. The new optional RPC catalog and override fields remain backward compatible, but an older app version cannot enforce these new concurrency guarantees. To roll back, revert this commit and retain a backup of the credentials file before any older build rewrites it; no automatic historical cleanup is performed. Per-session and per-account queues add bounded memory and waiting time, with no new polling. Cross-process writes to the same credentials file remain outside the in-process lock. Real provider discovery, signed mobile UI, and live CPU/RSS were not measured. Six agent-core files also overlap the open agent finalization PR #2105; GitHub may require conflict resolution when that PR advances.

Verification

  • CI follow-up: reproduced the sole failing ModelVariantInlineCard assertion locally, then corrected its inferred o4-mini fixture and added explicit catalog-key compatibility plus mini/nano resolver coverage. Runtime behavior is unchanged; the component edit only corrects a stale comment.

  • pnpm test src/modules/MainApp/Integrations/KeyVault/shared/ModelTable src/util/__tests__/modelVariants.test.ts src/util/__tests__/selectableModelVariants.test.ts src/util/__tests__/defaultModelVariant.test.ts src/util/__tests__/variantEditOptions.test.ts src/hooks/keyVault/defaultVariantSaveCoordinator.test.ts src/hooks/models/accountModelCatalog.test.ts: 8 files, 58 tests passed after the CI fixture correction.

  • pnpm exec eslint --max-warnings 0 src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/ModelVariantInlineCard.test.ts src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/ModelVariantInlineCard.tsx src/util/__tests__/modelVariants.test.ts and NODE_OPTIONS=--max-old-space-size=8192 pnpm typecheck:fast: passed after the fixture correction. The complete frontend suite was not rerun locally; the updated branch will run CI again.

  • Fetched current origin/develop; git merge-tree --write-tree HEAD origin/develop found no conflicts. No published history was rewritten.

  • pnpm test with the model hooks, KeyVault, palette, session queue, variant and context suites: 34 files, 180 tests passed in the isolated branch.

  • NODE_OPTIONS='--max-old-space-size=8192' pnpm typecheck:fast: passed after rebasing onto the latest develop.

  • ESLint with --max-warnings 0 on every changed TS/TSX file: passed. The six files flagged by CI were also checked with the typed ESLint configuration after explicit async completion handling.

  • cargo test -p key_vault --lib --quiet: 494 passed, 1 ignored.

  • cargo test -p agent_core --lib -- state::session_identity --test-threads=1: 2 passed.

  • cargo test -p agent_core --lib -- entry::runtime_tests --test-threads=1: 2 passed.

  • cargo test -p org2 --lib -- agent_sessions::session_directory::patch::tests --test-threads=1: 9 passed.

  • cargo test -p org2 --lib -- api::mobile_bridge::adapters::model::tests --test-threads=1: 6 passed.

  • After the async-lint correction, 19 focused frontend files / 87 tests and pnpm typecheck:fast passed. NODE_OPTIONS=--max-old-space-size=6144 pnpm check:typed-lint found 0 new or increased findings.

  • git diff --check origin/develop...HEAD: passed. Architecture, UI and performance reviews are included under docs/; UI audit verdict: 0 fix, 6 keep with reason, 0 abstract.

  • Full workspace Clippy, real account/provider requests, mobile visual QA, and dual-instance runtime checks were not run. No new screenshots were captured; the existing control layout and flow were preserved, while the underlying state transitions changed.

This branch has not been deployed

No deployments
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