Skip to content

fix: remove host model warnings from profile selectors - #3521

Merged
carlosflorencio merged 2 commits into
mainfrom
feature/investigate-misleadi-70f
Sep 9, 2026
Merged

fix: remove host model warnings from profile selectors#3521
carlosflorencio merged 2 commits into
mainfrom
feature/investigate-misleadi-70f

Conversation

@carlosflorencio

@carlosflorencio carlosflorencio commented Sep 8, 2026

Copy link
Copy Markdown
Member

Tip

PR walkthrough: Open the visual walkthrough

Host model catalogs do not prove which models an executor can use, so profile selectors no longer show host-model advisories. Agent health warnings, profile-editor diagnostics, and persisted runtime fallback warnings remain available.

Validation

  • Targeted Vitest suites: 55 passed across profile options, profile editing, model variations, and chat status messages.
  • Managed Playwright runner: six Chromium tests and four mobile Chrome tests passed. Coverage includes keyboard/touch selection, different host and executor catalogs, model and effort persistence, and actual fallback after reload.
  • New absence assertions failed against the old component and production browser build before implementation.
  • Web typecheck, targeted ESLint, i18n:check, and i18n:ratchet passed.
  • Specification lint passed. Public-documentation validation passed: 61 validator tests and 46 pages.
  • Normal pre-commit and commit-message hooks passed. Fresh desktop/mobile screenshots show both selector states.

Exact commands and results are recorded in docs/plans/profile-selector-model-warnings/task-01-remove-host-model-advisories.md.

Checklist

  • If I do not have repository write access and this is a large architectural change, I discussed the direction in a linked issue before opening this PR.
  • This PR contains one logical change; unrelated work is split into separate PRs.
  • I have performed a self-review of my code.
  • I have manually tested my changes and they work as expected.
  • My changes have tests that cover the new functionality and edge cases.
  • If my change touches UI files (apps/web/), I have added or updated Playwright e2e tests in apps/web/e2e/ and verified them with make test-e2e.
  • I checked whether this affects public docs in docs/public/** and updated them or noted why no docs change is needed.

Review in cubic

Screenshots

Desktop profile options

Desktop selected profile

Mobile profile options

Mobile selected profile

@carlosflorencio
carlosflorencio temporarily deployed to opencode-review-trusted September 8, 2026 22:14 — with GitHub Actions Inactive
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T22:17:48.417743Z e6eb665 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Advanced

Run ID: caf552c9-9460-4223-83a2-17d78c196373

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Advanced

Run ID: 128b2b5b-3f5f-4810-9725-43eecd67b0fa

📥 Commits

Reviewing files that changed from the base of the PR and between 401947f and e6eb665.

📒 Files selected for processing (19)
  • apps/web/components/task-create-dialog-options.test.tsx
  • apps/web/components/task-create-dialog-options.tsx
  • apps/web/e2e/tests/session/model-mismatch-warning-helpers.ts
  • apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts
  • apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts
  • apps/web/e2e/tests/settings/profile-model-selection-helpers.ts
  • apps/web/src/locales/en/settings.json
  • apps/web/src/locales/pseudo/settings.json
  • apps/web/src/locales/pt-pt/settings.json
  • apps/web/src/locales/zh-cn/settings.json
  • apps/web/src/locales/zh-hk/settings.json
  • apps/web/src/locales/zh-tw/settings.json
  • docs/plans/profile-selector-model-warnings/plan.md
  • docs/plans/profile-selector-model-warnings/task-01-remove-host-model-advisories.md
  • docs/public/agents-and-profiles.md
  • docs/specs/agents/README.md
  • docs/specs/agents/requirements/no-silent-model-fallback.md
  • docs/specs/agents/system-design/no-silent-model-fallback-01.md
  • docs/specs/agents/system-design/no-silent-model-fallback-02.md
💤 Files with no reviewable changes (6)
  • apps/web/src/locales/zh-hk/settings.json
  • apps/web/src/locales/zh-cn/settings.json
  • apps/web/src/locales/pseudo/settings.json
  • apps/web/src/locales/zh-tw/settings.json
  • apps/web/src/locales/pt-pt/settings.json
  • apps/web/src/locales/en/settings.json

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Summary

Summary by CodeRabbit

  • User Experience

    • Profile selectors no longer display host-model availability warnings or warning icons.
    • Saved profiles remain selectable, including when models are missing, ambiguous, or unavailable in the host catalog.
    • Authentication, installation, and probe-failure indicators remain visible.
    • Selected models and fallback settings are preserved when launching tasks.
  • Documentation

    • Updated guidance and requirements to clarify where model warnings appear and how host model discovery is presented.

Walkthrough

The change removes host-catalog model advisories from saved-profile selectors. Profiles remain selectable, health indicators remain visible, and executor model selection remains authoritative. Unit tests, desktop/mobile E2E tests, specifications, translations, and documentation reflect the new behavior.

Changes

Profile selector advisory removal

Layer / File(s) Summary
Selector warning placement contract
docs/specs/agents/...
Requirements and design specifications define warning-free selectors while preserving health, editor, and executor warnings.
Selector rendering and unit coverage
apps/web/components/task-create-dialog-options.tsx, apps/web/components/task-create-dialog-options.test.tsx, apps/web/src/locales/*/settings.json
The selector no longer reads host catalogs or renders model advisories. Tests cover stable labels, health indicators, and saved profile settings.
Executor-only selection flow
apps/web/e2e/tests/settings/*, apps/web/e2e/tests/session/*
Desktop and mobile tests verify warning-free selection and execution on the requested model.
Documentation and implementation records
docs/plans/..., docs/public/agents-and-profiles.md
Plans, verification records, and public documentation describe the completed selector behavior.

Priority: ⬇️ Low

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

Merge Risk: ⚪ Minimal · up to e6eb6

Profile selectors now omit host-model advisories while preserving health indicators and executor model behavior. The covered selection and execution flows indicate no remaining merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant ProfileSelector
  participant KanbanTaskDialog
  participant MockAgentSession
  ProfileSelector->>KanbanTaskDialog: Select saved executor-only profile
  KanbanTaskDialog->>MockAgentSession: Submit task with saved model
  MockAgentSession-->>KanbanTaskDialog: Complete session with requested model
  KanbanTaskDialog-->>ProfileSelector: Show selected profile without host advisory
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 9 functions across 6 files. (7 skipped: 7 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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.
Title check ✅ Passed The title clearly and concisely describes the main change: removing host model warnings from profile selectors.
Description check ✅ Passed The description explains the problem and outcome, lists comprehensive validation results, preserves the required checklist, and includes relevant screenshots and preview details.
Full details: Docstring Coverage

Explanation

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 9 functions across 6 files. (7 skipped: 7 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/investigate-misleadi-70f

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

A rabbit hops past warnings bright
The profile keeps its saved delight
Health signs still remain in view
The executor model stays true
Select, launch, and verify
No host hint can make it shy

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

@claude

claude Bot commented Sep 8, 2026

Copy link
Copy Markdown

Claude finished @carlosflorencio's task in 8m 50s —— View job


Code Review Complete

  • Read code-review skill
  • Read changed files and gather context
  • Analyze changes for correctness, quality, and consistency
  • Post inline findings (2 suggestions)
  • Post findings summary

Verdict: Ready to merge. No blockers found. Two minor suggestions posted as inline comments.

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown

Greptile Summary

The PR removes host-catalog model advisories from the shared saved-profile selector, retains capability-health rendering, adds desktop/mobile regression coverage, removes obsolete translations, and updates the corresponding requirements and documentation.

  • The presentation change is narrow and consistently reflected in unit tests, E2E tests, locale catalogs, specifications, and public documentation.
  • Capability polling was unintentionally coupled to the removed host-catalog hook, so selector surfaces without another poll can retain stale health state.

Confidence Score: 4/5

The PR should not merge until capability revalidation remains available to every shared profile-selector surface.

The visual removal itself matches the amended requirements, but removing the useAvailableAgents subscription also removes the only explicit pending-capability poll from Office setup and other selector-only paths, allowing health state and eligibility to remain stale until reload.

Files Needing Attention: apps/web/components/task-create-dialog-options.tsx

Important Files Changed

Filename Overview
apps/web/components/task-create-dialog-options.tsx Removes host-model warning rendering and catalog subscription, but also drops capability revalidation needed by some shared selector consumers.
apps/web/components/task-create-dialog-options.test.tsx Adds broad absence assertions and verifies rendering of already-present capability warnings, but does not cover asynchronous capability settlement after a pending probe.
apps/web/e2e/tests/settings/profile-model-selection-helpers.ts Adds a well-guarded shared flow proving an executor-only model launches successfully and persists across reload.
apps/web/e2e/tests/settings/no-silent-model-fallback.spec.ts Replaces desktop advisory checks with warning-free keyboard selection and successful-launch coverage.
apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts Replaces touch advisory interaction with direct row selection and adds mobile launch and layout coverage.
docs/specs/agents/requirements/no-silent-model-fallback.md Defines selector warning placement and explicitly preserves health indicators, editor diagnostics, runtime warnings, and saved configuration.
docs/public/agents-and-profiles.md Documents that host catalogs are editing hints and profile selectors no longer display model-difference warnings.

Reviews (1): Last reviewed commit: "fix: remove host model warnings from pro..." | Re-trigger Greptile

Comment thread apps/web/components/task-create-dialog-options.tsx Outdated
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Cloudflare Pages docs preview

Open the docs preview

Built from docs commit 06e75fd.

Stable PR alias: https://docs-pr-3521.landing-87j.pages.dev/docs

Comment thread apps/web/e2e/tests/settings/profile-model-selection-helpers.ts Outdated
Comment thread apps/web/components/task-create-dialog-options.test.tsx
@claude

claude Bot commented Sep 8, 2026

Copy link
Copy Markdown

Findings

Suggestion (recommended, doesn't block)

  1. Hardcoded model display name in E2E helperapps/web/e2e/tests/settings/profile-model-selection-helpers.ts:89

    • Issue: "Opus (270k)" is the mock fixture's display name for opus[270k]. If the fixture catalog renames the entry this assertion silently tests the wrong thing.
    • Fix: derive the expected display name from the same apiClient.listAvailableAgents() call already made just above (lines 22-29 read the executor catalog); map model.name for opus[270k] instead of inlining the string.
  2. renderTriggerLabel positive anchor missing in it.eachapps/web/components/task-create-dialog-options.test.tsx:212

    • Issue: the it.each block asserts the absence of warning indicators in both option-label and selected-label, but only checks toContain("hybrid") for the option label. An empty renderTriggerLabel would still pass the absence checks.
    • Fix: move the positive .toContain("hybrid") inside the for (const label of [...]) loop so both labels are confirmed to carry content.

Summary

Severity Count
Blocker 0
Suggestion 2

Verdict: Ready to merge

The frontend changes are well-scoped and correct. ModelProbeWarning, ModelProbeWarningIndicator, advertisedModelIDs, and the useAvailableAgents dependency are all cleanly removed from useAgentProfileOptions. The useMemo dependency array is correctly narrowed. Both renderLabel and renderTriggerLabel sharing the same function reference is intentional and fine. Unused imports (useState, IconAlertTriangle, Drawer*, Tooltip*, AvailableAgent, findUniqueModelVariation, useTouchDrawer) are all removed without orphaning any consumers — findUniqueModelVariation and useAvailableAgents remain in use elsewhere.

The two new test suites cover the right axes: parameterized absence assertions across every catalog state (missing, exact, unique variation, multiple variations, empty, pending), preserved capability health indicators, profile value immutability, and label stability when the host catalog changes. The new launchExecutorOnlyModelProfile E2E helper is a solid end-to-end regression that verifies the executor's model authority independently of the host catalog. The createExecutorOnlyModelProfile precondition assertion (hostModels.length > 0 and !hostModels.includes(profile.model)) ensures the fixture actually exercises the divergent-catalog scenario rather than accidentally passing on an exact match.

Spec and documentation updates are accurate and consistent with the implementation. The plan scope (frontend-only, no Go changes) is respected — the backend divergence visible in a broad git diff origin/main..HEAD is pre-existing main history, not part of this commit.

@carlosflorencio
carlosflorencio temporarily deployed to opencode-review-trusted September 8, 2026 22:37 — with GitHub Actions Inactive
@carlosflorencio
carlosflorencio merged commit 867b2da into main Sep 9, 2026
81 checks passed
@carlosflorencio
carlosflorencio deleted the feature/investigate-misleadi-70f branch September 9, 2026 06:48
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