Skip to content

feat(providers): show each agent the share of a picked set it can run - #531

Open
xiaoland wants to merge 3 commits into
masterfrom
yihong/code-620
Open

feat(providers): show each agent the share of a picked set it can run#531
xiaoland wants to merge 3 commits into
masterfrom
yihong/code-620

Conversation

@xiaoland

Copy link
Copy Markdown
Member

Closes CODE-620.

What was already true

The capability question is answered on the wire and in the pickers already: the gateway serves
GET /v1/models?protocol=…, LinkCode Gateway's openai-responses and anthropic variants read
their own filtered lists, the daemon's probe tags each model with the protocols whose list named it
(AccountModel.protocols), and enabledAccountModels keeps a model out of an agent's picker when
that agent binds a protocol the model does not answer. So a Codex user can no longer pick a model
that is certain to answer 404 model_not_found.

What was missing is the part CODE-620 left open — Settings never said so. An enabled agent whose
picked set is mostly, or entirely, unreachable looked identical to one that offers everything.

This change

Each agent row in an account's dialog now carries the reach of the account's picked set:

  • a shortfall reads 2 of 3 selected models work with this agent — only when the agent can run
    some but not all of them;
  • zero reachable is a stated reason, not an empty menu: None of the selected models speaks the protocol this agent uses;
  • a full set, or a set with no protocols tags at all (probed before tagging existed), says
    nothing — silence stays the "nothing to report" state.

accountModelReach in @linkcode/providers counts it, next to the filter it mirrors, so the row
and the picker can never disagree about what an agent can run.

Also fixes a stale claim in packages/foundation/providers/AGENTS.md: model lists have been
per-variant for LinkCode Gateway since the Responses work, and the file still said the list is
always service-level.

Verification

pnpm check:ci and pnpm test (3044 passed). New coverage: accountModelReach per agent,
providerAccountDetailViewModel's row for both the shortfall and the empty case, and a jsdom
render of AccountDetail asserting which note each state shows.

Not observed in the running app: the note only appears for models carrying protocol tags, which
requires a real probe against a LinkCode Gateway credential — a hand-typed or unprobed model set
is untagged and deliberately silent.

Settings named a model set the agent's own protocol could not fully reach, so a
gateway account with responses-only coverage left codex with a silently short —
or empty — picker.
Copilot AI lite review requested due to automatic review settings September 10, 2026 07:35
@linear-code

linear-code Bot commented Sep 10, 2026

Copy link
Copy Markdown

CODE-620

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

ℹ️ Minor suggestions only — the reach computation itself checks out.

Reviewed changes — the new accountModelReach helper, its two view-model consumers, the agent-row rendering, both locale files, the AGENTS.md correction, and all three new test cases (run locally: 17 pass).

  • accountModelReach counts what enabledAccountModels filters — same modelReachable predicate, same boundProtocol(resolveBinding(…)) input, so an untagged set and a binding with no protocol both count as fully reachable. I traced every binding shape that produces no protocol (oauth, grok-build native, pre-catalog bare key, pinned endpoint) and none of them can manufacture a false shortfall; unavailable tiers early-return before the count is taken.
  • offerStatus splits "off" from "on but offering nothing"disabled still wins, and no-reachable-model only fires for an enabled, available row whose picked set is non-empty and entirely unreachable.
  • The UI shows the count only in the shortfall band 0 < reachable < picked, and note moving from string | undefined to an always-string .join(' · ') is safe — the render guard at account-detail.tsx:369 is a truthiness check, so '' still renders nothing.
  • AGENTS.md now records that ServiceVariant.models overrides the service-level list, which matches the catalog: linkcode-gateway is the only entry with per-variant models, and model-probe.ts tags a model with every protocol whose list named it.
  • Locales — two keys added to both files; the zh-cn strings follow the CJK typography rule (spaces at every CJK↔digit boundary, no half-width punctuation next to CJK).

ℹ️ The reach signal stops at the account dialog

providerAccountListItem still describes an account with boundAgents alone, so the accounts list renders an agent chip identically whether that agent can run the whole picked set or none of it. The state CODE-620 is about — an enabled agent that offers nothing — is now explained, but only once you open that account's dialog. Whether the list should carry the signal too is a product call, not a defect in this diff; worth deciding explicitly rather than by omission.

Technical details
# Reach is invisible on the accounts list

## Affected sites
- `packages/client/workbench/src/settings/providers/view.ts:184``providerAccountListItem` returns
  `boundAgents` with no reach information, so `ProviderAccountListItem` cannot distinguish a bound
  agent that runs 3 of 3 from one that runs 0 of 3.

## Required outcome
- A decision, recorded either in the PR description or in `packages/foundation/providers/AGENTS.md`:
  either the accounts list deliberately stays reach-free (the dialog is the one place that answers
  the question), or a follow-up issue carries the signal up to the list.

## Open questions for the human
- Is the dialog intended to be the only surface that answers "can this agent actually run anything
  from this account?", or is the list expected to flag it eventually?

ℹ️ Nitpicks

  • ProviderAgentViewModel.models holds a count pair, but the sibling ProviderAccountDetailViewModel.accountModels holds an actual list — agent.models reads like a list at the call site. modelReach would echo the helper it comes from (accountModelReach) and remove the ambiguity.
  • The AGENTS.md reflow left a ragged one-clause line (wrong wherever variants) mid-paragraph — worth re-wrapping to the file's fill column.
  • That bullet's bolded lead-in still reads models is service-level and spelled out, never derived, which is the claim the body now qualifies with "for every service but one" and the new sub-bullet overrides outright. Since the skim-path is the bold text, something like "models is service-level by default" would carry the correction to a reader who does not read past it.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/client/workbench/src/settings/providers/view.ts Outdated
| { kind: 'unavailable-oauth'; agent: AgentKind }
| { kind: 'unavailable-endpoint-incomplete' }
| { kind: 'unavailable-protocol' }
| { kind: 'no-reachable-model' }

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.

This variant falsifies the doc comment on status three lines below: "Only a reason the row cannot be, or is not, on. Absent means enabled and available." no-reachable-model is only ever set on a row that is on and is available, so status now means "a reason this row offers nothing", not "a reason it is off". Worth widening that sentence in the same commit that widens the union — this PR is already correcting one stale claim in AGENTS.md.

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.

The rewrite in 3bf358b fixes the false "Absent means enabled and available" claim and adds the shortfall, but its enumeration ("a reason it cannot be, or is not, on, or a picked set it can run only part of") still omits no-reachable-model — a row that is on, available, and can run none of the set — so leaving this open.

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

unavailableProtocol: 'The endpoint protocol is incompatible with this agent',
unavailableEndpointIncomplete: 'Endpoint details are incomplete — finish the account setup',
noReachableModel: 'None of the selected models speaks the protocol this agent uses',
modelsReachable: '{reachable} of {picked} selected models work with this agent',

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.

The display gate is reachable > 0 && reachable < picked, so reachable === 1 is the common shortfall and the string renders "1 of 2 selected models work with this agent". This file already uses ICU plurals in ~18 places, so the agreement is worth fixing; rewording sidesteps the plural form entirely, or keep the count-first phrasing with {reachable, plural, one {works} other {work}}.

Suggested change
modelsReachable: '{reachable} of {picked} selected models work with this agent',
modelsReachable: 'Works with {reachable} of {picked} selected models',

…once

The rule for when a share is worth showing sat half in the view model and half in
the row that renders it, so the two could drift apart.
Copilot AI review requested due to automatic review settings September 11, 2026 02:58

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

ℹ️ No new issues in this commit — it is a clean, behavior-preserving refactor. Not an approval only because the three inline threads from the previous review are still open and unchanged.

Reviewed changes74be5458, the one commit since the prior review, which renames the reach field and moves the "is this shortfall worth showing" decision from the component up into the view model.

  • Named the reach typeAccountModelReach is now an exported interface next to accountModelReach in packages/foundation/providers/src/enabled-models.ts, replacing the inline { picked; reachable } return type, and its jsdoc gained an explicit warning that the count is only meaningful for a binding that resolves.
  • Moved the shortfall band into the view modelagentStatus emits modelShortfall only when 0 < reachable < picked (view.ts:126), and AgentRow renders it whenever present instead of re-deriving the same band.
  • Renamed ProviderAgentViewModel.models to modelShortfall — the doc comment now states that absent means "nothing to say", and that zero reachable arrives as a status rather than a ratio.
  • Sharpened the jsdom translation mockpassthrough emits key:name=value pairs, so the shortfall assertion pins each number to its own placeholder instead of to argument order; a swapped {reachable}/{picked} would now fail.
  • Rebalanced coverage for the moved decision — the full-reach silence case migrated from the component test to view.test.ts:192, and the component test keeps only the rule it still owns (an absent shortfall renders nothing).

I traced the note text across every reachable (tier, enabled, picked, reachable, status) combination — including translate with no status, and the all-absent case where note is now '' rather than undefined — and the rendered output is identical before and after. The rename is complete (no stale agent.models survives anywhere), the barrel export is correct, tsc --build --noEmit is clean for the four touched packages, and all 17 tests in the three touched files pass.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

Two optional fields for facts that never stack let a row claim to be both off and
short of models; the union now admits one answer at a time.
Copilot AI review requested due to automatic review settings September 11, 2026 03:09

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

ℹ️ The fold is correct and the note it produces is unchanged everywhere except the one case it was meant to change. Not an approval only because two threads from the first review are still open — one untouched, one addressed most of the way.

Reviewed changes3bf358b, the one commit since the prior review, which replaces the separate modelShortfall field with a variant of the status union the row already had.

  • Folded the shortfall into ProviderAgentStatus — the union gained { kind: 'model-shortfall'; picked; reachable } next to no-reachable-model, and ProviderAgentViewModel.modelShortfall is gone. agentStatusLabel picks up the new case and keeps its satisfies never exhaustiveness.
  • offerStatus is now the whole decision!enableddisabled, a full or empty set → nothing, reachable === 0no-reachable-model, otherwise the ratio. Because disabled returns first, a row the user switched off can no longer state the reach of a set its picker never receives; that is the previous review's view.ts thread, now unrepresentable rather than merely unset, so I resolved it.
  • AgentRow went back to its pre-PR note shapeagent.tier === 'translate' ? [t('translateNote'), status].filter(Boolean).join(' · ') : status, with no third term to join and no band to re-derive, so note is string | undefined again rather than an always-string.
  • Moved the test assertions onto statusview.test.ts pins the exact variant for all three bands, and the jsdom test's shortfall case now arrives as a status; the "says nothing" case was renamed to match what it actually asserts.
  • Widened the status doc comment — it no longer claims "Absent means enabled and available", and now names the shortfall alongside the off/unavailable reasons.

I traced the rendered note across every (tier, enabled, picked, reachable) combination. The only difference from 74be5458 is the intended one: an off row in the shortfall band used to read "Hidden from this agent's model menu · 2 of 3 selected models work with this agent" and now reads only the first clause. translate + shortfall, off + zero-reachable, and the all-absent case (undefined now, '' before — both falsy at the note ? guard) all render identically. ProviderAgentStatus and ProviderAgentViewModel have exactly two consumers repo-wide, so the added variants break no exhaustive switch elsewhere; tsc --build --noEmit is clean for the four touched packages and all 17 tests in the three touched files pass.

ℹ️ Nitpicks

  • offerStatus' models.picked === 0 clause is subsumed by the models.reachable === models.picked beside it — reachable counts over models, so an empty set always satisfies the second.
  • The two new doc blocks (offerStatus' five-line jsdoc, status' four-line one) are design rationale rather than a constraint or trap; AGENTS.md caps inline comments at 1–2 lines and sends anything longer to the owning AGENTS.md. The "these never stack" invariant is worth keeping — it just fits in a sentence.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

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.

2 participants