Skip to content

fix(gui): select Advisor model from the routed catalog - #7114

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:codex/advisor-model-picker-20261015
Oct 15, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:codex/advisor-model-picker-20261015

Conversation

@luvs01

@luvs01 luvs01 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Replace the Advisor expert-model text box with the shared catalog selector. Picking a model saves its exact namespaced identifier, preventing display-name or case typos from becoming later consultation failures. Native, provider, account-qualified, custom, and combo selectors use the existing catalog contract.
  • Keep an existing unlisted identifier visible without silently changing or saving it. Disabled and initial-selection-pending rows are excluded from new choices; clearing remains supported. Loading, empty, malformed, and failed catalog responses have explicit states and retry, and catalog completion does not reset other draft edits.
  • Preserve the existing API/CLI model-string contract, consent, effort, policy, and consultation behavior. Catalog presence is not a guarantee that a provider/model can complete a text consultation. Update all GUI locales and the directly affected Advisor documentation.

Verification

  • Windows, pinned Bun 1.4.3: bun test --isolate ./gui/tests/advisor-save-feedback.test.tsx — 10 passed / 0 failed, 76 assertions. Covers exact identifier PUT payloads, legacy value retention, disabled/pending rows, clear, load failure/retry, malformed/empty catalog, and edits while the catalog is pending.
  • GUI: the exact lint:i18n oxlint command arguments passed; tsc -b --force and Vite build passed. Force typechecking avoids relying on shared incremental build metadata. Dependency lockfiles matched an already installed isolated worktree and were reused without changing dependencies.
  • bun scripts/structure-ssot.ts and git diff --check passed.
  • Documentation bun run build passed: 577 pages and 81,200 internal links checked. The dependency installation was reused from an identical frozen lockfile after its first build completed.
  • Independent source review found no blocker. Follow-up feedback was addressed: a failed catalog read no longer falsely claims the selected model is absent, and pending-load draft preservation is directly asserted.
  • Headless Chromium rendered the actual page with a synthetic catalog and opened the dropdown, with no page errors or horizontal overflow. No model consultation or live settings write was used by this UI test.
  • Full repository/GUI suites were not run locally because this is a scoped selector change with active independent worktrees. The focused behavior tests and GUI/documentation builds are complete; broader coverage remains with exact-head CI. No deployment is included.

Advisor model selector with a synthetic catalog

Advisor lane verification (2026-10-15)

Head: f260787527234f849b09bb2c81f0990f3bdf2338; baseline: 7640a942b35044c974e534b84d5a8049f4a8ae9a.

  • bun test tests/advisor/ tests/server/advisor-routes.test.ts: 148 passed / 0 failed.
  • (cd gui && bun test --isolate tests/advisor-save-feedback.test.tsx): 10 passed / 0 failed.
  • bun run typecheck, bun run build:gui, (cd gui && bun run lint:i18n), bun run structure:check, bun run privacy:scan, bun scripts/file-size-ratchet.ts, git diff --check: exit 0.
  • Full cross-platform CI: https://github.com/lidge-jun/opencodex/actions/runs/38712593627 (exact head on ci/pr-7114).

All lane commands used fresh OPENCODEX_HOME=$(mktemp -d) CODEX_HOME=$(mktemp -d) sandboxes. Focused local coverage was used because independent worktrees are testing concurrently; the standalone full suite was not run on this PR head. Live-provider consultations were not run. Cross-platform CI below is queued, not passing evidence.

Real GUI builds served by sandboxed proxies on ports 18741/18742/18743. Screenshot capture used node /tmp/shot-advisor/capture.mjs with scratch-installed playwright-core. The installed /Applications/Google Chrome.app timed out before browser initialization, so capture used Chromium headless shell 154. Synthetic model-catalog rows are used for the dropdown; no model calls or settings saves were made. No browser page errors or horizontal overflow at 1440×1200.

BEFORE — origin/dev 7640a942b

Before: Advisor expert model text input

AFTER — PR head f26078752

After: Advisor routed model catalog selector

Local interaction test: start at origin/dev (7640a942b), merge #7114, then #7134, then #7124. The first two merge cleanly. Adding #7124 requires preserving both changes in gui/src/pages/Advisor.tsx, gui/tests/advisor-save-feedback.test.tsx, structure/advisor.md, structure/gui-and-management-api.md, and tests/advisor/advisor-responses-wiring.test.ts. The locally resolved union passes 248 backend/layout tests, 12 GUI tests, root typechecking and the GUI build. The earlier resolved union also completed bun run test:changed: 15,454 passed / 35 skipped / 0 failed across 628 files; the later refusal-reporting fixes were validated with focused tests. Recommended integration order: #7134 and #7114 first (either order), then resolve #7124 against the landed tree and rerun its exact-head CI. No PR was merged.

#5955 is already integrated as 13212e6ef; #6617 as e2b6eef5c. These are the foundation/admission fixes, not open duplicate PRs.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features
    • Added an Advisor model selector populated from the model catalog. It saves exact model identifiers and lets you clear your selection.
    • Disabled models and models awaiting initial selection are not offered as new choices.
    • If a saved model is unavailable, it remains selected with a warning. Catalog load failures and empty catalogs include retry options.
  • Documentation
    • Clarified that catalog availability does not guarantee compatibility with Advisor consultations; CLI and API model-string support is unchanged.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: ff07cc3b-8a96-49a3-8d99-c6360ca5ac5a
📥 Commits

Reviewing files that changed from the base of the PR and between 7640a94 and f260787.

📒 Files selected for processing: 25
  • docs-site/src/content/docs/fr/reference/configuration/advisor.md
  • docs-site/src/content/docs/ja/reference/configuration/advisor.md
  • docs-site/src/content/docs/ko/reference/configuration/advisor.md
  • docs-site/src/content/docs/reference/configuration/advisor.md
  • docs-site/src/content/docs/ru/reference/configuration/advisor.md
  • docs-site/src/content/docs/tr/reference/configuration/advisor.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/advisor.md
  • docs-site/src/content/docs/zh-tw/reference/configuration/advisor.md
  • gui/src/advisor-models.ts
  • gui/src/components/AdvisorModelSelect.tsx
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/fr.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/pt.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/tr.ts
  • gui/src/i18n/vi.ts
  • gui/src/i18n/zh-TW.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Advisor.tsx
  • gui/tests/advisor-save-feedback.test.tsx
  • structure/advisor.md
  • structure/gui-and-management-api.md

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

📝 Walkthrough

The Advisor dashboard now selects models from the model catalog instead of using a free-text field. The selector filters unavailable choices, preserves saved identifiers that are not listed, and supports retry after catalog load failures. Tests, translations, and documentation cover the selector behavior.

Changes: Advisor model selection

Layer / File(s) Summary
Catalog filtering and selector behavior
gui/src/advisor-models.ts, gui/src/components/AdvisorModelSelect.tsx
The selector loads model rows and filters disabled or initial-selection-pending entries. It preserves a selected identifier missing from the catalog and shows loading, failure, empty, and unavailable states.
Dashboard integration and validation
gui/src/pages/Advisor.tsx, gui/src/i18n/*.ts, gui/tests/advisor-save-feedback.test.tsx
The Advisor page uses the selector to update the draft model. Translations and tests cover exact identifiers, clearing, unavailable selections, catalog filtering, load failures, retries, and malformed responses.
Selection behavior documentation
structure/advisor.md, structure/gui-and-management-api.md, docs-site/src/content/docs/reference/configuration/advisor.md, docs-site/src/content/docs/*/reference/configuration/advisor.md
The documentation describes catalog selection, unavailable saved identifiers, load failures, compatibility limits, and unchanged CLI and API model-string support.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant AdvisorPage as Advisor page
  participant ModelSelect as AdvisorModelSelect
  participant ModelsAPI as /api/models
  AdvisorPage->>ModelSelect: Pass API base and draft model
  ModelSelect->>ModelsAPI: Fetch model catalog
  ModelsAPI-->>ModelSelect: Return catalog rows
  ModelSelect-->>AdvisorPage: Send selected identifier through onChange
Loading

Suggested reviewers: flowershangfromthebranches

Merge Risk: ⚪ Minimal · up to f2607

No actionable issue is established from the available evidence; the change is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 15 files. (10 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
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.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: replacing the Advisor model text input with selection from the routed catalog. It is directly related to the implementation and PR objective.
Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 15 files. (10 skipped: 10 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • 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

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 15, 2026
@luvs01
luvs01 marked this pull request as ready for review October 15, 2026 02:20
@lidge-jun
lidge-jun merged commit 60bf4c7 into lidge-jun:dev Oct 15, 2026
71 of 74 checks passed
lidge-jun pushed a commit to Flowershangfromthebranches/opencodex that referenced this pull request Oct 15, 2026
… evidence

GUI files follow the reviewed union resolution 97d21b763 (lidge-jun#7114's selector inside the adaptive editor rows); structure docs keep both the adaptive budget contract and the dashboard model-selection section.
lidge-jun added a commit that referenced this pull request Oct 15, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Thanks @luvs01. This is being reverted in #7192: the maintainer wants Advisor model selection inside the existing subagent tab on the right side, with no new tab. A follow-up PR will rework it there and keep the catalog-backed exact model IDs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants