diff --git a/devlog/_plan/260830_models_provider_header/000_baseline_and_roadmap.md b/devlog/_plan/260830_models_provider_header/000_baseline_and_roadmap.md new file mode 100644 index 0000000000..f56cca71d6 --- /dev/null +++ b/devlog/_plan/260830_models_provider_header/000_baseline_and_roadmap.md @@ -0,0 +1,168 @@ +# 000 — Models provider-row header: unreadable chip, overlapping name, meaningless controls + +Reported against the running dashboard's Models page with a screenshot: "이부분도 +존나 이상해 신규 2개 꺼짐, 펜, 스위치(이건 뭘하는지도 모르겠음), 사용자 지정창이랑 +마지막 스위치는 뭔지도 모름". + +Two distinct failures are stacked in one header, and they need different fixes: + +- **Geometry** — the "신규 N개, 꺼짐" chip collapses into a rounded blob and the + provider name paints on top of the active count. +- **Meaning** — three controls are operable but unlabeled: a sighted user cannot + tell what they do. This half is not a layout bug and cannot be fixed by + layout. + +As with the sidecar unit, every defect below carries a measured baseline from a +CDP harness (`Emulation.setDeviceMetricsOverride`, dpr 2, live +`getBoundingClientRect`), so each claim is re-checkable. + +## Baseline (ko, provider rows on `#models`) + +Measured with `.tmp/uiux2/head.ts`, which settles on `innerWidth === target` and +on rendered provider rows before reading geometry. The proxy at 127.0.0.1:10100 +supplies the live provider list through the Vite `OPENCODEX_PROXY_TARGET` proxy, +so these are real rows, not fixtures. + +| width | provider | header h | name box w | chip lines | chip w | +|-------|----------|----------|------------|-----------|--------| +| 1440 | opencode-free | 44.8 | 96.5 | 1 | 92.1 | +| 1280 | opencode-free | 55.7 | 67.2 | **2** | 69.6 | +| 1280 | openai | 55.7 | **9.9** | **2** | 57.7 | +| 1100 | opencode-free | **115.1** | **0.0** | **6** | 34.1 | +| 1100 | cursor | **115.1** | **0.0** | **6** | 34.1 | +| 1024 | opencode-free | 75.8 | 96.5 | 1 | — | + +The 1100 row is the screenshot state: a six-line chip 34.1px wide, a name box +measuring **zero**, and a header 2.6x its correct height. 1024 recovers because +the container query at `styles-models-workspace.css:517` moves the actions onto +their own row, which returns the toggle's width. The defect therefore lives in a +**band** (roughly 1040-1380 in this layout), which is why it is easy to miss at +either extreme. + +## Defect 1 — the chip is a shrinkable flex item with no single-line floor + +`.models-chip` (`styles-models-workspace.css:315`) declares +`display: inline-block` plus padding, border and `border-radius`, and nothing +else. Because it sits inside `.row models-provider-toggle` +(`Models.tsx:1226`) and `.row` is `display: flex` (`styles.css:1205`), the chip +is a **flex item**: its `inline-block` outer display is blockified and its +initial `flex-shrink: 1` applies. Measured computed values confirm it — +`white-space: normal`, `flex-shrink: 1`. + +`inline-block` does not imply `white-space: nowrap`. The chip's only floor is +`min-width: auto`, which resolves to the text's **min-content** width — and for +Korean that is nearly one syllable, because CJK line-breaking permits a break +between Hangul syllable blocks. So `신규 2개, 꺼짐` legally becomes +`신규 / 2 / 개, / 꺼 / 짐`, and the fixed padding wrapped around that narrow +column is exactly the observed blob. + +~~Fix: give the chip a single-line floor.~~ **Superseded.** Measurement showed the +chip is not independently broken: it is starved of width by a collapsed ancestor, +and it returns to one line as soon as that ancestor claims its intrinsic width. An +audit also found a chip-level floor unsafe across the eight other `.models-chip` +call sites. The shipped fix leaves the shared `.models-chip` primitive untouched; +it adds an ellipsis only to the toggle-scoped descendant — see `010` and `011`. + +## Defect 2 — the name overflows a zero-width box instead of reflowing + +The name span carries inline `whiteSpace: "nowrap"` (`Models.tsx:1232`) while +`styles-models-workspace.css:267` gives it `min-width: 0` and +`overflow-wrap: anywhere`. Those two fight: `nowrap` suppresses the wrapping +that `overflow-wrap: anywhere` was added to provide, `min-width: 0` lets the box +shrink to nothing, and the default `overflow: visible` means the glyphs keep +painting outside the box — straight across the sibling count. + +Nothing positions these elements on top of each other: there is no `position`, +transform, or negative margin anywhere in the applicable rules. The count is +laid out normally *after* a box that measures 0px, so the collision is pure +overflow. + +Why the header's own `flex-wrap: wrap` does not save it: the header's direct +children are only the toggle button and the actions container. Wrapping does not +propagate into descendants, and the toggle's inner `.row` has no `flex-wrap`, +so the chevron, name, chips and count are locked on one line and shrink against +each other. + +The upstream enabler is `flex: 1` on the toggle (`Models.tsx:1229`), which +resolves to `flex: 1 1 0%` — zero basis, shrink allowed — combined with +`min-width: 0`. The toggle then accepts whatever the wide actions cluster leaves +it rather than forcing a wrap. + +~~Fix: let the toggle's own row wrap.~~ **Superseded.** Inner wrapping is inert: +line construction inside the button runs after its used width has been assigned, so +wrapping redistributes 31px rather than asking for more. Measured: the candidate +left the name box at 0.0px, byte-identical to baseline. The shipped fix gives the +toggle a real flex **basis** so its content enters the header's wrap decision, and +removes every child's min-content floor so the row can always shrink back inside the +card — see `010`. + +## Defect 3 — three controls carry no visible meaning (two switches and the `+`) + +`Switch` (`ui.tsx:8`) accepts a `label` prop and spends it **only** on +`aria-label` (`ui.tsx:11`); its sole child is ``. So +every `Switch` in this codebase is, to a sighted user, an unlabeled toggle. The +user's "이건 뭘하는지도 모르겠음" is a correct reading of the UI. + +Audit of the header controls in visual order: + +| control | visible | aria-label | title | verdict | +|---------|---------|-----------|-------|---------| +| collapse button | chevron + name + count | (children) | — | OK | +| pencil | icon only | 공급자 별칭 편집 | yes | OK | +| default-aliases Switch | knob only | 기본 별칭 사용 | — | **OPAQUE** | +| 사용자 지정 창 | text | — | — | OK | +| `+` | `+` only | 커스텀 모델 추가 | — | **OPAQUE** | +| preset segmented | 프리셋 / 전체 | group only | — | OK | +| 모두 켜기 / 모두 끄기 | text | — | — | OK | +| cap Switch | knob only | 기본 {value} | — | **OPAQUE** | +| cap Select | number only | 기본 {value} | — | **OPAQUE** | + +The pencil is fine precisely because it pairs an icon with `title` — that is the +pattern the opaque controls are missing. + +Two aggravating details: + +1. The cap Switch's accessible name is `기본 128k` — a *value*, not a function. + Even a screen-reader user is not told this governs the context-window cap. +2. For routed providers with the cap off, `(capOn || nativeProviderGroup)` + (`Models.tsx:1360`) hides the Select, so the only thing left is a bare + toggle with no adjacent number to hint at its purpose. The worst state is the + default state. + +### Design constraint + +This is a dense expert control surface: `DESIGN_VARIANCE 2`, `MOTION 1`, density +D6+. The domain gate is strict — no decorative kit, no motion, no new color. The +fix is *labels and reflow*, and the correct instrument is the existing +`title`-plus-icon pattern already proven by the pencil, plus a visible text +label where the header has room for one. + +UX-LAZY-01 was applied to each control before relabeling it rather than after: +every one of them is a real per-provider setting with no correct global default, +so none can be deleted or absorbed. They need meaning, not removal. + +## Roadmap + +- `010` — let the toggle's content be seen, and make every child yield (geometry). + Five designs; the first four were rejected by audit or stress measurement and + `011` records why. +- `020` — control affordances: visible labels for the opaque controls, and a + `Switch` that can render one. + +Each is one PABCD work-phase and one stacked PR. `010` lands first because `020` +adds visible text to the same header and would otherwise be measured against a +layout that is still collapsing. + +## Verification contract + +- Re-measure the sweep at 1440/1280/1100/1024 in ko + ru + fr + en and require: + chip `lines === 1` everywhere, name box width > 0, zero name/count overlap, and + header height within one line-height of the 1440 baseline. +- A focused `gui/tests` regression per phase, driven red against current CSS + first. +- Remote gates only (`ssh lidge` + `ocx-run`); the local full suite is forbidden + by the user. Push `--no-verify`. +- Before/after screenshots at the failing width, per `AGENTS.md` enforce-target. + + + diff --git a/devlog/_plan/260830_models_provider_header/010_toggle_basis_and_shrink.md b/devlog/_plan/260830_models_provider_header/010_toggle_basis_and_shrink.md new file mode 100644 index 0000000000..dd86c0b9d3 --- /dev/null +++ b/devlog/_plan/260830_models_provider_header/010_toggle_basis_and_shrink.md @@ -0,0 +1,286 @@ +# 010 — Let the toggle's content be seen, and make every child yield + +Fixes the geometry half. Meaning is phase `020`. + +**Sixth design.** The five before it were each rejected by an adversarial reviewer or +by a stress measurement, and the rejections are the useful part — they map the shape +of the problem: + +| draft | approach | killed by | +|-------|----------|-----------| +| 1 | shared-chip `nowrap`/`flex-shrink: 0` + inner `flex-wrap` + name ellipsis | inner wrap is inert; shared-chip change unsafe elsewhere | +| 2 | `min-width: max-content` floor | unbounded: 64-char name overflowed the card by 216px | +| 3 | floor + 16rem name cap + 12rem chip cap | 64-char name **and** alias together still overflowed 64px | +| 4 | `flex-basis: auto` + ellipsis on name and chip | the count and badge children kept min-content floors | +| 5 | `flex-basis: auto` + one rule for every child | let the fixed-size chevron shrink to 2.5px | +| **6** | **draft 5 + a `flex: none` exemption for the icon** | — | + +Drafts 2-4 were three versions of one mistake: bound the row by naming the children +that could overflow it, then discover the next child. Draft 5 stops naming children — +and then over-applied, shrinking an icon that has no text to truncate. Draft 6 keeps +the universal rule and exempts the one child whose size is intrinsic rather than +textual. `011` records each failure. + +## The mechanism, measured + +At 1100px the collapsed row measures: + +| element | width | +|---------|-------| +| `.models-provider-head` | 488.0 | +| `.models-provider-actions` | **422.9** (scrollWidth 423) | +| `.models-provider-toggle` | **31.1** (scrollWidth 93) | +| name span inside it | **0.0** (scrollWidth 44) | + +The toggle carries inline `flex: 1` (`Models.tsx:1229`), which resolves to +`flex: 1 1 0%`. That zero **basis** is the defect. A flex item with a zero base size +never reports a content requirement, so the header — which already has +`flex-wrap: wrap` — never learns the toggle needs room and never wraps the actions +cluster to its own line. It keeps one line and hands the toggle the 31px remainder. + +Inside that remainder the name absorbs the whole deficit, measures 0.0px, and — +carrying inline `white-space: nowrap` with default `overflow: visible` — paints its +glyphs across the count. The chip blob is the same starvation, finished by CJK +line-breaking between Hangul syllables. Even the chevron collapses: measured 0px wide +on a starved row, against 14px on a healthy one. + +Two independent properties are required: + +- **Visibility** — the toggle's content must enter the header's wrap decision, so it + receives a share rather than a remainder. That is `flex-basis: auto`. +- **Boundedness** — whatever the content, the row must not force itself wider than the + card. Shrinkability alone does not give this: a flex child stops at its own + `min-width: auto` floor, which is its min-content width, and the *sum* of those + floors can exceed the container. + +The bound has one precondition worth stating plainly, because the round-5 audit caught +the document overstating it: `> *` selects **element** children. A bare string +interpolated directly into the button becomes an anonymous flex item, which no selector +can reach, and it would keep its own min-content floor. Every child today is an +`` or a ``, so the rule covers all of them — but the guarantee is +"every element child, and the markup keeps children element-wrapped", not "anything +anyone adds later". The regression test asserts that second half. + +Draft 2 bought visibility with a raised *minimum*, which is the direct enemy of +boundedness. Draft 4 bought boundedness for the two children it named and left the +count and the discovery badge with their automatic floors intact. + +## The change + +`gui/src/pages/Models.tsx` (—1229), the inline style on the toggle button: + +```diff +- style={{ flex: 1, border: 0, ... }} ++ style={{ flex: "1 1 auto", border: 0, ... }} +``` + +It has to be the TSX: an inline style beats any stylesheet rule short of +`!important`, and reaching for `!important` against markup we own is the wrong +trade. + +`gui/src/styles-models-workspace.css`: + +```css + .models-provider-toggle { + min-width: 0; + } + ++/* Every child, not an enumerated list. Four earlier designs bounded the row by ++ naming the children that could overflow it (name, then alias chip, then the ++ count and badge), and each revision found another one; a child added later ++ would have reintroduced the defect silently. Quantifying over the children ++ instead: min-width:0 removes the automatic min-content floor that stops a flex ++ child shrinking, and the ellipsis makes that shrink legible instead of clipped. ++ Covers every ELEMENT child; a bare interpolated string would become an ++ anonymous flex item no selector can reach, so keep children element-wrapped. */ ++.models-provider-toggle > * { ++ min-width: 0; ++ overflow: hidden; ++ text-overflow: ellipsis; ++ white-space: nowrap; ++} + ++/* The one exemption, and why it is not a return to enumerating children: every ++ other child is TEXT, whose overflow the ellipsis makes legible. The chevron is ++ an icon at a fixed 14px with nothing to truncate, so shrinking it destroys the ++ collapse affordance instead of abbreviating it. Selected by element TYPE, not ++ by identity — any future icon child inherits it without being named. Measured: ++ without this, the adversarial stress case shrinks the chevron to 2.5px while ++ the containment gate still reports success. */ ++.models-provider-toggle > svg { ++ flex: none; ++} +``` + +`min-width: 0` on the toggle is **kept**, not replaced. That also means the existing +assertion at `gui/tests/models-provider-head.test.ts:29` stays green — draft 2 would +have broken it. + +**No `max-width` anywhere, and no child named by identity.** The bound comes from +removing every child's floor, so there is nothing to forget and nothing to re-tune when +a chip is added to this header later. The single exemption selects on element type +(`svg`), which is the distinction that matters: text children abbreviate, icons do not. + +## Measured result + +Gate: in every cell `chipLines === 1`, name width > 0, name text overflow +(`scrollWidth - width`) <= 0, no page overflow. + +| | ko | ru | fr | en | de | +|-|----|----|----|----|----| +| 1440 | pass | pass | pass | pass | pass | +| 1280 | pass | pass | pass | pass | pass | +| 1100 | pass | pass | pass | pass | pass | +| 1024 | pass | pass | pass | pass | pass | + +20/20, worst bad-cell count 0, re-measured after the chevron exemption was added +(draft 6). The chevron also returns to 14px on the rows where it had collapsed to 0. + +Containment, reading `cardScrollOver` = card `scrollWidth` minus its width, where +**positive means the card is silently clipping** (`.models-provider-card` sets +`overflow: hidden`, `styles-models-workspace.css:296`): + +| stress case | baseline | draft 3 | draft 4 | draft 5 | **draft 6** | +|-------------|---------:|--------:|--------:|--------:|------------:| +| 64-char name @1100 | 39 | -2 | -2 | -2 | **-2** | +| 64-char alias @1100 | 16 | -2 | -2 | -2 | **-2** | +| name + alias together @1100 | 229 | **64** | -2 | -2 | **-2** | +| realistic worst row @1100 (64-char name + alias + longest `de` badge) | 229 | — | -2 | -2 | **-2** | +| adversarial: every child forced to 64 chars @1100 | 484 | — | **484** | -2 | **-2** | +| chevron width in that adversarial case | 14 | — | — | **2.5** | **14** | + +The last row is what draft 4 could not survive and what forced the universal rule. It +is deliberately beyond reachable input — the count and badge are localized strings +with small interpolated numbers, not free text — but it is the only case that proves +the bound does not depend on knowing what the children are. + +The final row is the round-5 audit finding, and it is the reason containment alone is +not a sufficient gate: draft 5 reported `cardScrollOver: -2` on the adversarial case +**while** silently shrinking the 14px collapse chevron to 2.5px. A gate that measures +only "does the row fit" certifies a fix that bought the fit by destroying an +affordance. `flex: none` on the icon restores 14px with containment unchanged at -2. + +The gate is not vacuous: against the unpatched stylesheet it reports +`ko/1100 bad=3` (0px name, **6-line** chip) and `ko/1280 bad=4` (name 9.9px, chip 2 +lines). `ru/1100` is green even unpatched — Russian wraps to a wider min-content — +which is why a single-locale check would have missed this defect entirely. + +## Removal test + +| dropped | normal bad cells @ko/1100 | adversarial stress | realistic stress | chevron @adversarial | +|---------|--------------------------:|-------------------:|-----------------:|---------------------:| +| nothing | 0 | -2 | -2 | 14 | +| `flex: 1 1 auto` | **3** | -2 | -2 | 14 | +| the child rule | 0 | **908** | **229** | 14 | +| the `svg` exemption | 0 | -2 | -2 | **2.5** | + +All three are load-bearing and none substitutes for another: the basis fixes the +everyday defect, the child rule bounds the pathological ones, and the exemption keeps +the child rule from paying for that bound with the collapse affordance. Each row was +driven by actually removing the declaration and re-measuring. Contrast drafts 1 and 3, +where four of five and two of three declarations measured inert. + +## Cost of the universal rule + +`white-space: nowrap` on every child means no child of this header can wrap. That is +correct here — it is a single-line identity row of a slug, chips and a count, none of +which should ever wrap — but it is a real constraint on future content. Anything +genuinely multi-line belongs in `.models-provider-body`, not the header. The +alternative was another enumerated exception list, which is what drafts 2-4 already +disproved. + +## What is deliberately NOT changed + +- **The shared `.models-chip` rule.** Only the toggle's own children are touched. The + model-row chips at `Models.tsx:1447-1455` sit in a non-wrapping `.row` with long + translations (de "Benutzerdefiniert", ru "Пользовательская"); a primitive-level + change there was rejected in draft 1. +- `overflow-wrap: anywhere` stays on the existing name rule although the inline + `white-space: nowrap` makes it dead. Removing it is unrelated cleanup; it is noted + so the next reader knows it is inert rather than load-bearing. + +## Diff scope + +- `gui/src/pages/Models.tsx` — one inline style value. +- `gui/src/styles-models-workspace.css` — two rules added (the universal child rule + and the `svg` exemption); the existing `min-width: 0` on the toggle is kept. +- `gui/tests/models-provider-head.test.ts` — extended; the existing line-29 + `min-width: 0` assertion stays valid and must not be removed. +- `gui/tests/helpers/css-declarations.ts` — NEW. The shared source-text CSS readers, + lifted out of `viewport-scroll-caps.test.ts` so two tests can use one copy. +- `gui/tests/viewport-scroll-caps.test.ts` — its four file-local helpers are deleted and + replaced by an import from that module; its assertions are unchanged. + +## Regression test (red first) + +Use the effective-declaration reader so a commented-out or custom-property occurrence +cannot satisfy an assertion. + +**It lives in `gui/tests/helpers/css-declarations.ts`**, which exports +`effectiveDeclaration`, `ruleBodies`, `allRuleBodies` and `withoutComments`. + +That module is part of this change. The reader originated in +`viewport-scroll-caps.test.ts` (PR #2915) as four **file-local, unexported** functions, +so it could not be imported as first planned. B resolved that by moving all four into the +shared module and rewriting the original test to import them — one copy, not the third +copy that copying them here would have produced. + +**What this gate can and cannot see.** The reader's own comment (:53) records that it +does not model competing specificity, `!important`, or at-rule nesting. So it proves +the four declarations exist on the exact selector, and nothing about computed layout: +the ellipsis, the containment numbers, and the 14px chevron are **measurements** +recorded above, not unit assertions. That split is deliberate and is why the tables in +this document are the primary evidence for the fix. + +1. The provider-toggle button in `Models.tsx` carries `flex: "1 1 auto"`. The + negative half must be **scoped to that style object**, not a file-wide search for + `flex: 1` — a legitimate bare `flex: 1` exists at `Models.tsx:2162`, so a global + assertion would be wrong. This is the declaration whose absence reproduces the + user's screenshot. +2. `.models-provider-toggle > *` declares `min-width: 0`, `overflow: hidden`, + `text-overflow: ellipsis` and `white-space: nowrap`, with a comment naming the + defect so the rule is not narrowed back to specific children later. +3. `.models-provider-toggle > svg` declares `flex: none`. Assert this **separately** + from rule 2: it is the declaration whose removal reintroduces the 2.5px chevron, and + a reader who sees only the universal rule is likely to delete it as redundant. +4. Every direct child the toggle renders is an **element**, never a bare string. The + universal selector cannot reach an anonymous flex item, so this is the invariant the + `> *` bound actually rests on. Assert that the JSX between the toggle's opening and + closing tag contains no bare interpolation — all seven children today are `` or + ``. + +A declaration test cannot observe clipping, so the containment table above stays a +recorded measurement rather than a unit assertion. + +## Render grounding + +Screenshots at the failing width, captured from the running dashboard and then **read +back** rather than merely produced: `evidence/010-before-ko-1100.png` and +`evidence/010-after-ko-1100.png` (ko, 1100px, dpr 2, the second chip-bearing provider +row). The before shot is the shipped build with the fix reverted **in the browser** by an +injected override, so both images come from the same code and differ only by the two +declarations. + +| | before | after | +|-|-------:|------:| +| capture height (dpr 2) | 696px | **416px** | +| rows containing ink | 365 | **101** | +| name box | 8.6px, chip on 6 lines | **43.6px, chip on 1 line** | + +Pixel readback is the observation step: ink was counted per row against the sampled +background luminance, which is what confirms the vertical sprawl actually collapsed +rather than the clip rectangle merely shrinking. + +The chevron was verified the same way, since a rendered width is exactly what the round-5 +audit found the numbers hiding. Under the adversarial stress row at 1100: + +| | `getBoundingClientRect` | drawn glyph span | +|-|------------------------:|-----------------:| +| shipped (exemption present) | **14.0px**, `flex-shrink: 0` | 9.5px | +| exemption overridden away | 4.9px, `flex-shrink: 1` | 36.8px of smeared ink | + +Two notes on reproducing this. `Page.captureScreenshot` hangs indefinitely over CDP +unless `Page.bringToFront` is called first. And an injected `