Keep ACP model descriptions and disambiguate same-name picker rows - #2226
Open
SawyerHood wants to merge 1 commit into
Open
Keep ACP model descriptions and disambiguate same-name picker rows#2226SawyerHood wants to merge 1 commit into
SawyerHood wants to merge 1 commit into
Conversation
The ACP bridge's configOptions model-catalog path hard-coded description: "" and the wire schema did not type the per-option description, so agents like omp that send description: "provider/model" on every select option lost it. The desktop picker renders only the display name, so omp's colliding names (github-copilot/gpt-5.1 vs openai-codex/gpt-5.1 and 14 more) were indistinguishable. Bridge: type `description` on the select-option schema and carry it through buildModelCatalogFromConfigOptions, mirroring the session-models path. No protocol bump: AvailableModel.description was already a required string on the wire. Desktop: ModelPickerOption's route-provider field becomes a general `qualifier`, built once in toModelPickerOptions. Only rows whose formatted label collides within a list get a qualifier (the agent's description when it is short, else the raw model id); Pi's route provider is kept as before. Unique-label rows (Claude Code, Codex) are unchanged, and row height does not change. Fixes #2062 Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
buildModelCatalogFromConfigOptionsin the ACP bridge hard-codeddescription: ""for every model select option, andacpConfigOptionSelectOptionSchemadid not type the option'sdescriptionat all (it only survived via.passthrough()). Agents such as omp (oh-my-pi) senddescription: "provider/modelId"on every option, and advertise the same display name under several route prefixes (on this machine: 15 collisions across 72 models, e.g.github-copilot/gpt-5.1vsopenai-codex/gpt-5.1). Because the desktop model picker renders only the display name, those rows were identical in text, tooltip and accessible name. Issue: #2062. Report: https://get-bb.github.io/reports/issues/2062.htmlWhat changed
Bridge (
plugins/provider-acp):src/wire.ts:description: acpOptionalStringon the select-option schema (string or null, normalized toundefinedlikename).src/bridge/model-catalog.ts:description: option.description ?? "", mirroring the siblingbuildModelCatalogFromSessionModelspath.HOST_DAEMON_PROTOCOL_VERSIONbump:AvailableModel.descriptionis already a required string on the wire; only its value changes.Desktop (
apps/app):ModelPickerOption.routeProviderIdbecomes a generalqualifier, and a newtoModelPickerOptions(models, formatLabel)inmodel-picker-option.tsbuilds rows foruseThreadCreationOptions(modelOptions/moreModelOptions) and both preview mappings inModelReasoningPicker.routeProviderIdis kept as the qualifier exactly as before.MenuRowButton'squalifier, already used for Pi) and in the row'stitletooltip, so row height does not change and providers with unique names (Claude Code, Codex) render exactly as before. The qualifier also feeds the model search text, so typingopenai-codexfinds the right row.Relation to #2063 (external, reviewed REQUEST CHANGES): its bridge hunks are the same fix and are credited here. This PR differs on the desktop side: #2063 forwarded
descriptionfor every provider into a new two-lineMenuRowButtonlayout, which centered every picker row (the wrapper lost the button's default alignment) and turned every Claude Code / Codex row into a second line of truncated marketing copy. This PR instead fills the existing qualifier slot, and only for colliding rows.How you verified
Tests (fail before, pass after):
plugins/provider-acp/src/bridge/model-catalog.test.ts: "keeps the per-option description the agent sends (ACP bridge drops model descriptions, so the model picker shows indistinguishable duplicate rows #2062)" (fails on main withdescription: "").plugins/provider-acp/src/wire.test.ts: the schema now returns the parsed description and normalizesnulltoundefined(fails on main because the untyped field is not present in the inferred type).apps/app/src/components/pickers/ModelReasoningPicker.test.tsx: "tells apart previewed models that share a display name (ACP bridge drops model descriptions, so the model picker shows indistinguishable duplicate rows #2062)": two equal-label options render distinct row text (GPT-5.1github-copilot/gpt-5.1vsGPT-5.1openai-codex/gpt-5.1), a unique-label sibling stays a plain single-segment row, and clicking the second row emitsopenai-codex/gpt-5.1.apps/app/src/components/pickers/model-picker-option.test.ts: unique labels keep no qualifier regardless of description; only colliding rows are qualified; long/missing descriptions fall back to the raw model id; collisions are detected on the formatted label androuteProviderIdwins.Commands:
pnpm exec turbo run typecheck lint --filter=@bb/app --filter=bb-plugin-provider-acpandpnpm exec turbo run test --filter=@bb/app --filter=bb-plugin-provider-acpall green.Manual, own dev instance with the real
omp16.3.10 on PATH (auto-registered asacp-omp):GET /api/v1/system/execution-options?providerId=acp-ompnow returns 72 models with 72 non-empty descriptions (15 duplicate display names). In the picker, the oh-my-pi tab searched forGPT-5.1showsGPT-5.1 github-copilot/gpt-5.1andGPT-5.1 openai-codex/gpt-5.1(and the Codex / Codex Max / Codex mini pairs) as distinct rows with distinct tooltips; every row, including Codex and Claude Code rows, measured 26px tall, and the Codex and Claude pickers are byte-for-byte the same row text as before.Fixes #2062