feat(desktop): declare one thinking level across a relay's models at once - #3448
Conversation
…once A relay usually fronts one model family that accepts the same `reasoning_effort` values, but the capability section only offered a per-model menu: declaring `high` on eight models was eight menus × one tick, for a single fact about the relay. The section now opens with one control that writes into every enabled model's row. It is per level, not per level set — ticking `high` adds it everywhere and unticking removes it everywhere, leaving every other level on every row exactly as the user left it. Replacing the whole set would have been one fewer concept and would silently discard what a row already declared, which is the destructive reading of a control whose only job is to save clicks. The box ticks only at full coverage, so a level that 3 of 5 models declare reads as unticked with its count beside it: the next click then means "give it to everyone" rather than "take it from the rows that have it", and the count is the only thing separating partial coverage from none. Edits land in the same local draft the rows use and commit through the same 保存能力声明. Also names the context-window field per model. It was the one control in the section without a model in its accessible name, so every row exposed the same `上下文窗口(tokens)` spinbutton — the AX audit flags it as soon as a story renders a relay with more than one enabled model, which is the ordinary case and had no story until this one. Generated-by: Claude Opus 5
| is the sole place partial coverage is legible — | ||
| without it "3 of 5 declare high" and "none do" | ||
| present as the same empty box. */ | ||
| description={copy.thinkingBulkCoverage(state.declaredCount, state.total)} |
There was a problem hiding this comment.
[P2] Expose partial model coverage to assistive technology
Thanks for showing the partial n/total state visually—it makes the batch behavior much easier to understand. Because this item also supplies an aria-label, however, the screen reader does not receive that visible description and cannot distinguish "none of the models declare this level" from "some models already declare it."
Could we expose thinkingBulkCoverage(...) through aria-description or an aria-describedby relationship? A focused accessibility assertion for the partial state would be enough; the existing pure-function coverage does not need to be expanded further.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for making the batch operation reuse the existing relay-profile draft and save path. The per-level add/remove semantics are clear, partial coverage is preserved, other thinking levels are not overwritten, and the focused tests cover the important state transitions and edge cases.
I'm approving this exact head with one non-blocking accessibility P2 below.
AI-assisted review disclosure: OpenAI Codex coordinated two independent exact-head review passes. I verified the form-state and persistence path, batch semantics, accessibility behavior, tests, exact head, and CI state, and I independently made the approval decision.
Review was right that the visible `n/total` never reached a screen reader.
The menu item supplies its own `aria-label`, which replaces the accessible
name the visible description would otherwise have joined, and the component
does not wire `description` to `aria-describedby`. Read straight off the
rendered story before the fix, `low` — declared by one of four models — and
`minimal` — declared by none — were indistinguishable:
low aria-label="批量设置思考档位 low" aria-checked=false
minimal aria-label="批量设置思考档位 minimal" aria-checked=false
Both unchecked, both identically named. Coverage is the only thing that
separates them, and it was the one part that did not survive.
The item now carries `aria-description`. Confirmed against the real
Chromium accessibility tree through CDP, not only the DOM attribute:
批量设置思考档位 low description="1/4 个模型"
批量设置思考档位 minimal description="全部未声明"
The assertion lives in the story's own `play`, which the storybook smoke
already runs before reading the AX tree, so it is checked on every CI run
that builds the catalog. Removing `aria-description` fails that story.
Generated-by: Claude Opus 5
|
Addressed in Read straight off the rendered story before the fix:
The item now carries The assertion. It lives in the story's own await expect(partial).toHaveAttribute('aria-checked', 'false');
await expect(none).toHaveAttribute('aria-checked', 'false');
await expect(partial).toHaveAttribute('aria-description', '1/4 个模型');
await expect(none).toHaveAttribute('aria-description', '全部未声明');Both unchecked is the point: coverage is the only thing separating them. Removing Per your note, the pure-function coverage is unchanged. Verification
|
Summary
设置 → 模型 → a custom relay → 能力 declared thinking levels one model at a time. A relay normally fronts one model family sharing a
reasoning_effortvocabulary, sohighacross eight enabled models was eight menus opened and eight ticks — for one fact about the relay.The section now opens with one control that writes into every enabled model's row.
Per level, not per level set. Ticking
highadds it everywhere; unticking removes it everywhere; every other level on every row is left exactly as the user left it. Replacing the whole set would have been one fewer concept and would silently discard what a row already declared — the destructive reading of a control whose only job is to save clicks.The box ticks only at full coverage. A level 3 of 5 models declare shows unticked with
3/5 个模型beside it, so the next click means "give it to everyone" rather than "take it from the rows that have it". The count is the only thing separating partial coverage from none, since a checkbox has no third state here.The control shares the 思考档位 row's relay gate —
reasoning_effortis a wire field only the OpenAI-compatible relays accept — and appears only from two rows up, where there is repetition to save. Edits land in the same local draft the rows use and commit through the same 保存能力声明. Vision, context window, and fast mode stay per-row: those are per-model facts.Fixes #3447
Review focus
The bulk menu offers only the declarable vocabulary.
offis a disable wire (reasoning_effort: 'none'), not an intensity tier, and no generic relay is presumed to honor it. It cannot reach a draft anyway — the seed sanitizes throughnormalizeRelayModelProfiles— but spreading one across a whole table is the specific thing worth not being able to do by accident.One state update, not a loop of per-model setters. A bulk tick is a single gesture; committing it in N steps would let a re-render land mid-way and paint a half-applied table. The rule for what an emptied declaration collapses to now lives in one place (
relayProfileWithThinkingLevels) so the row setter and the bulk path cannot drift on it.Also in this PR: an accessibility fix the new story exposed
上下文窗口(tokens)was the one capability control without a model in its accessible name — vision, thinking, and fast mode all carry— ${modelId}. Every row therefore exposed an identically-named spinbutton, and the AX audit'sambiguous_actionablerule fires on it:This is a defect on
main, reachable by anyone with a relay and two or more enabled models — the ordinary case. It had no story: the only existing multi-model capability story is a non-relay whose rows are filtered down to one. Fixed by giving the field the same per-model name as its three siblings.Verification
Behaviour, on the new story (
product-settings-providers--relay-connection-detail, a relay with four enabled models, one of which already declareslow+high):low 1/4 个模型,high 1/4 个模型, rest全部未声明highhigh→4/4 个模型, ticked;lowstill1/4; rows read 已选择 2/1/1/1 个high 4/4from the saved tablehighhigh→全部未声明everywhere;deepseek-r2drops to 已选择 1 个, keeping itslowThe last row is the non-destructive claim: the model that had two levels kept the one the bulk edit was not about.
Checks run locally:
@maka/desktoptypecheck (preload + main + renderer + storybook) — cleanbiome lint apps/desktop/src apps/desktop/stories— 719 files, clean@maka/desktopsuite — 1048/1048 pass, including 14 newbuild-storybook+smoke:storybook— 161 stories pass, AX audit included. It failed before the context-window fix, on exactly the story this PR adds.Not run: the packaged-artifact and release checks, and the other workspaces' suites — nothing outside
apps/desktopis touched.AI use
Select exactly one:
Tool(s) and scope: Claude Opus 5 (Claude Code) — wrote the implementation, tests, and story; ran the checks above. Reviewed and directed by me.
Generated-bytrailer is on the commit.Checklist
Does this PR entail a change in behavior?