fix(workspace): one composer shell — field on top, controls inside, model picker beside Send (#2662) - #2665
Conversation
…odel picker beside Send (#2662) ent#403 shipped the model picker as a `BaseSelect` on its default recipe, on its own row above the composer, at `w-full max-w-sm`: a settings form field bolted onto a chat. The operator's read was right, and the cause was never the control — it was that the composer had nowhere to put it. In the single-row layout every 44px button came out of the field's width, so at 375px the textarea measured 143px of a 351px form and wrapped its placeholder over four lines; adding a picker to that row left 34px, which is why ent#403 gave it a row of its own, and why that row looked parked. The composer is now ONE shell: the field on top, the controls in a row inside it, the model picker right-aligned beside Send. Border, fill and focus ring move off the textarea onto the shell (`focus-within:`); the textarea goes transparent and borderless and must never regain `rounded-2xl` or a background, or there are two nested boxes. Stacked, the field takes the whole shell at every width — 1030px of 1048 at 1440, 333px of 351 at 375 — and the picker yields, truncating itself at narrow widths while Send stays 44px. Both portal composers take the shape, PortalRoom.vue included: the two are the same markup in two files (the #2211 lesson recorded in portalComposerAlignment.spec.js), so a shell landing in one leaves the room composer visibly diverged from the chat it sits beside. The room's control row holds Send alone — the picker is per-agent, and a room has several. BaseSelect gains `variant="ghost"` — the borderless, content-width recipe, on the primitive rather than hand-rolled, which design-system-contract.md asks for. The native <select> is kept deliberately: keyboard operation, focus management and the platform picker on touch come free. A trigger + popover would have inherited the agent picker's known gap instead — PortalConversation.vue records that it closes on outside-click only, so Escape does not dismiss it. ent#547's rule holds with a second inert region: the field's wrapper and the control row's wrapper each carry the pair, because `opacity` needs a real box on each row, and the call toggle still precedes the wrapper it must stay outside. ent#392's `composerWrap` keeps its ref name — the outside-click close reads it — and becomes the shell's first row rather than a `flex-1` item. Specs follow the new contract rather than the old geometry. The alignment spec retires `items-end` on the form — asserted absent, not merely dropped, because reintroducing it is a silent revert to the layout that cost the field its width — and pins the shell. The e2e case is rendered against a live page, per the #2659 standard: the picker inside the shell on Send's row and right of every icon button, no border colour and no fill of its own, the field at ≥90% of the form (41% before, at 375px), every button still 44px. Five of five green against the sibling stack at 375, 768 and 1280. Refs #2662
…e ghost recipe (#2662) Its own commit, per the #2605 guard's instruction and that issue's AC: an increase is dispositioned deliberately with the increase named, not absorbed silently into a feature diff. src/frontend/src/components/base/fieldClasses.js — raw_gray 11 → 15 The four are `FIELD_GHOST_CLASS`'s resting ink and hover ground, light and dark. Gray is the neutral ink ladder and has no semantic token to move to. The recipe was trimmed from six classes to four by dropping the hover ink change, matching the Workspace agent picker — the sibling "New chat" button spends two more raw-gray classes on that nuance and this does not need it. PortalConversation.vue and PortalRoom.vue move chrome between elements without adding any: both stay at their entries (90, 47). A `placeholder:` pair the original textarea never set was added and removed before this landed. Hand-edited rather than regenerated: `scan-raw-colors.mjs --baseline` rewrites the whole file, which drops #2605's `refrozen` provenance block and nulls `commit`. No other entry moves. Refs #2662
… on its own ground (#2662) dark:hover:bg-gray-800 on a gray-800 shell produced no change on hover. gray-750 is the contract's dark chrome shade, one step off the surface, and the light/dark pair now mirrors chrome-on-surface in both themes.
…y other field recipe (#2662) 13px sat between the six-size scale and the primitives' 13.5; BaseSelect's field variant and BaseButton md are both 13.5.
…y in dark mode (#2662) Voice, attach and mic carried dark:hover:bg-gray-800 from the days they sat on the gray-900 page. Inside the gray-800 shell that tint was invisible — the same defect FINDING-002 fixed on the picker, on the three buttons beside it. gray-750 is the contract's dark chrome shade. The header's picker and New chat keep gray-800: they sit on the gray-900 header, where it is a visible step.
…the field primitive's recipe (#2662) focus-within lit the shell for ANY descendant: tabbing onto an icon button lit the whole composer, and tabbing onto the picker drew two concentric rings — the shell's 2px and the select's own 3px. has-[textarea:focus] (Tailwind 3.4) scopes the ring to the field. The recipe is now FIELD_VALID_CLASS's — 3px ring, border action-primary-600 light / 500 dark, ring 500/40 light / 400/40 dark — so the composer no longer shows two field-focus recipes side by side. Both twins. The alignment spec's shell assertion moves with it.
…n sit on the 4px grid (#2662) py-1.5 (6px) was this branch's value on the textarea; py-2 (8px) puts the field row at 40px (24 line + 16) on the grid. The ghost chevron's right-[6px] becomes right-2. Both composers. Also squares the room composer's indentation, which the restructure left uneven.
…label (#2662) FINDING-003 took the ghost select from 13 to 13.5px; at the 16rem cap the default label — the longest option — then overflowed and the native select clipped its closing parenthesis. A select does not report that overflow through scrollWidth, so the cap is measured from the label's TEXT width in the select's own font plus its 36px of padding: 262px → 17rem. At 375px the row still shrinks it (min-w-0 on the cluster) — the cap only decides the wide case. The e2e ceiling moves with it.
…2662) Both surfaces still placed the picker where ent#403 put it and described the composer it no longer is. feature-flows/workspace-model-choice.md - Entry Points and the Frontend Layer table said "the composer row, between the mic button and the textarea". It is now inside the shell, right-aligned on the control row beside Send, on `BaseSelect variant="ghost"` — with ent#403's 34px squeeze recorded as the reason the own-row placement existed at all. - The Testing section claimed "nothing rendered is machine-verified ... the select's width against the textarea, its alignment, both themes and the narrow viewport are human checks". Half of that is now false and understating coverage is how a future change ships without running the guard that would have caught it: e2e/workspace-model-choice.spec.js measures real boxes at three widths. The honest residue — both themes, the hover/focus tints — stays named as a human check, because the e2e reads geometry and computed border/background, not the whole colour ladder. architecture/workspace.md - The composer paragraph listed a four-button row and ONE inert region. The row now carries the model picker, and stacking made the inert pair TWO regions: one without the other leaves half the composer live during a call. - Records what the shape is for. It is not cosmetic: while the buttons shared a row with a growing field, every 44px box came out of the field's width (143px of a 351px form at 375px), which is the mechanism behind both #2259's wrap and ent#403's 34px. Stacked, the picker is the element that yields. - Names `has-[textarea:focus]` and why `focus-within` was rejected, and that both composers carry the shape (the #2211 lesson) with the room's row holding Send alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZpX5EoY6J6Kx4XTwhCF6H
…record (#2662) The variant shipped in this branch without reaching design-system.md, which is the system of record for the primitives catalog. That gap has a specific cost: the contract's first rule is primitives over hand-rolled lookalikes, and a variant nobody can find is a variant the next contributor re-implements inline — which is exactly the defect #2662 exists to close, one surface along. - design-system.md §BaseSelect now documents both recipes rather than one: `field` (unchanged) and `ghost` (borderless, transparent, content-width, h-11 to match the 44px icon buttons, 13.5 ink, hover tints the ground only — gray-750 in dark because the composer shell IS gray-800). Records that both are a native <select> on purpose, and that one `recipe` computed resolves every variant-dependent class so a third cannot be half-added. - The primitives table's BaseSelect row cites both adoptions, so the catalog points at a live example of each. - design-system-contract.md gains the BaseSelect variant line beside BaseButton's, with the rule for choosing: `ghost` where a select is a preference beside content, `field` where it is a field in a form. - Notes the naming collision honestly rather than leaving it for someone to rediscover: this `ghost` is gray-inked chrome, BaseButton's is accent-inked. #2662 carries the open question of renaming it `bare`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZpX5EoY6J6Kx4XTwhCF6H
…ally uses (#2662) The block above the composer shell said the ring moves onto it via `focus-within:`. The shipped class is `has-[textarea:focus]:`, and portalComposerAlignment.spec.js asserts the ABSENCE of `focus-within:` on this element — so the comment pointed a reader at the one approach FINDING-008 removed, in the file where they would act on it. `focus-within` lit the whole shell when an icon button was merely tabbed onto, and drew a second ring concentric with the model picker's own. The comment now says that, and says the spec pins it, so the next reader knows it is a constraint rather than a preference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZpX5EoY6J6Kx4XTwhCF6H
… buttons (#2662) FINDING-001 gave the model picker a 44px box; nothing held it there. The existing guard loops over `<button>` tags, and the picker is a `<BaseSelect>`, so a regression to the 30px default recipe was invisible to the suite — a 30px tap target on a phone, which is the defect that finding was raised for. Two halves, because the height is split across two files: - At the call site: any `<BaseSelect>` inside a composer form must carry `variant="ghost"`. Written as a conditional over whatever selects are present rather than asserting one exists, so it also covers PortalRoom (no picker today — per-agent, and a room has several) if that ever changes. - In the recipe: FIELD_GHOST_CLASS must carry `h-11`, and must NOT carry `w-full` — content-width is the property keeping the picker out of the field's width budget, and `w-full` would re-create the ent#403 squeeze one level down. The recipe half asserts the IMPORTED VALUE, not the source text. The first version read fieldClasses.js as a string and matched /\bh-11\b/ — and passed with `h-11` deleted from the class, because the comment explaining the choice contains the literal `h-11`. A source-text guard over a documented constant tests the prose. Both halves were verified by breaking them: dropping `variant="ghost"`, dropping `h-11`, and swapping `w-auto` for `w-full` each fail the suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZpX5EoY6J6Kx4XTwhCF6H
…hipped (#2662) `recipe`'s comment still read "28px + right-6px for the smaller ghost chevron". FINDING-009 (e422295) moved that value to `right-2` — 8px, on the 4px grid — and left the sentence above it describing the value it replaced. design-system.md already says "12px chevron at right 8px with a 28px gutter", so the doc was right and only the code comment was wrong. That is the worse half to leave: the comment is the copy a contributor edits from, and it exists precisely to state that the gutter and the offset have to agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
…lass (#2662) FINDING-001's claim is that the ghost select is a 44px box like the buttons beside it — a 30px select next to 44px buttons is a 30px tap target on a phone. Nothing rendered was checking it. The three assertions that look like they might: - the button loop walks `<button>` only, and the picker is a `<select>`; - the centre-alignment check (|picker.mid - send.mid| <= 2) is satisfied by a short picker exactly as by a tall one, because the row is `items-center`; - `portalComposerAlignment.spec.js` asserts `h-11` on the recipe STRING, which is a source assertion in a file this branch's own PR body says should be rendered ones (#2659). Falsified before it was kept: with `h-8` on FIELD_GHOST_CLASS the new line fails `Received: 32` at 375px while every pre-existing assertion in the file still passes — the gap, exactly. Verified against the live sibling stack (:3081): 5 passed at 375 / 768 / 1280. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
FINDING-009 (e422295) said it "squares the room composer's indentation, which the restructure left uneven"; this one tag was missed. Its closing `>` sits two columns right of `<button`, where every other tag in both composers closes at the opening column. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
… call (#2662) ent#547's rule is that the chat goes inert around the live call toggle, and this branch kept it — two inert regions, one per row. What it missed is that the shell it introduced is the PARENT of both and is what now carries the border and fill. So a call rendered a full-contrast frame around opacity-60 contents; before this branch that chrome lived on the textarea and dimmed with it. The shell cannot simply join the regions: the call toggle is inside it and must stay bright, and `opacity` on a parent is not something a child can undo. So the chrome is removed for the call's duration rather than dimmed — the composer recedes to the page ground, the one live control stays legible. Removed and not muted because a muted pair is four more raw-gray classes in a file this issue's AC5 says must not grow; PortalConversation.vue stays at 90, verified by scan. Both arms are bound and the static class carries no chrome colour, which is the second bug rather than a style choice. The first cut left `border-transparent bg-transparent` static and bound only the resting pair, and rendered the LIGHT composer with no border: Tailwind emits `.border-transparent` AFTER `.border-gray-300` (transparent wins) but `.bg-transparent` BEFORE `.bg-white` (white wins), so two utilities of the same shape disagree about which of an equal-specificity pair survives. Dark was correct throughout, because every `dark:` variant is emitted after both — which is exactly how it would have shipped. Caught by reading computed styles off a live render, not by eye. Resting chrome, measured after the fix (:3081, both themes): light border rgb(209,213,219) = gray-300 bg #ffffff dark border rgb(55,65,81) = gray-700 bg rgb(31,41,55) = gray-800 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
#2662) The chrome moved off the textarea onto the shell, so the box you see is no longer the box you type in: its 8px padding band and the control row's ground read as "the input" and did nothing. Before this branch the visible rounded box WAS the textarea and a click anywhere in it put the caret in. Measured on a live render before the fix — `document.activeElement` came back `BODY`, both themes. Guarded rather than unconditional. A click that already reached a control keeps its own effect, and the typeahead is excluded by role: it picks on `mousedown` and `.prevent`s the default, so the click that follows would otherwise arrive here and pull focus straight back out of the row the user just chose. Verified live: after clicking attach, `activeElement` is that button; after clicking the picker, it is the select. Both composers, because both grew the same box. The e2e assertion is a rendered one, and its coordinate is load-bearing: the shell is `rounded-2xl` and hit-testing respects border-radius, so a point 4px in from a corner is OUTSIDE the 16px arc and the click falls through — the first version of this line read BODY against a working handler. Mid-width, in the top padding band. Falsified with the handler removed: BODY at 1280px. npm run test:unit ............ 113 files / 2536 tests pass playwright vs :3081 .......... 5/5 at 375 / 768 / 1280 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
… the fallout (#2662) The class the #2662 review hit twice-removed: two utilities written as one visual pair on one element resolved in opposite directions, because `.border-transparent` is emitted after `.border-gray-300` while `.bg-transparent` is emitted before `.bg-white`. Light lost its border; dark was correct throughout, since every `dark:` variant is emitted after both base utilities — so the defect lived entirely in the theme that gets looked at least. Written for /autoplan, which reads this file: the rule is mutually exclusive binding arms rather than a static "off" plus a bound "on", and the instrument is getComputedStyle off a live render in BOTH themes. Every class-string check in the repo — unit specs, check:tokens, the raw-colour ratchet — was green while the border was missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
10cc6a0 to
e026f0a
Compare
…ex carries the change (#2662) The architecture rewrite in 018ce31 described the shell as it stood then. Two later fixes changed exactly the two mechanisms that section names, and neither touched a doc: 032f523f3 made the shell's chrome CONDITIONAL on the call — it is the parent of both inert regions and holds the one control that must stay bright, so it sheds border and fill rather than dimming. The section still read as though the chrome were unconditional. e7fd83167 added `focusComposerFromShell`, because moving the chrome off the textarea made the visible box larger than the field and a click on the padding band landed on <body>. Its second guard is the model picker's: the typeahead commits on mousedown with the default prevented, so an unguarded handler would pull focus back out of the row just chosen. Both are now in `architecture/workspace.md`, next to the mechanisms they modify, with the Tailwind ordering trap stated as the reason both arms are bound — a static `border-transparent bg-transparent` renders a borderless LIGHT composer and dark hides it. Also: `feature-flows.md` had no Recent Updates row for #2662, though every sibling Workspace change (#2579, #2582 + ent#548, ent#547 + #2580) added one — and the flow's Testing table named neither the picker's measured 44px box nor the click-to-focus assertion added to the e2e, nor `portalVoiceMode.spec.js`, which this branch changed and which now pins the call-state chrome. npm run test:unit — 116 files / 2596 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHgiPs5QbdDxPjP4JGg7V1
…2662) Nine dropdowns across the app already flip their chevron on open — ChatHistoryDropdown, OverflowTabs, InfoPanel, and the disclosure rows in LoopsPanel / TasksPanel / PortalAgentDetails / PortalDeliverables / ChannelDisclosure. Every one of them owns its open state in JS. A native <select>'s picker is drawn by the platform and reports nothing to the page, which is why selects were the one control left out of the idiom rather than an oversight. `:open` is the first hook that closes that gap (Baseline newly-available 2026-05: Chrome 133, Firefox 136, Safari 26.5). On an older engine the chevron stays static, so nothing depends on it. The rule is a sibling selector on the select, not `:has()` on the wrapper: the chevron is the select's next sibling, so no group class is needed on the parent. It rides on the shared <select> rather than on `recipe`, so `field` and `ghost` behave identically — a per-variant chevron would be exactly the "third variant half-added" case the recipe computed exists to prevent. Verified in a real Chrome 152 against the generated stylesheet, not from the class string: `CSS.supports('selector(select:open)')` is true, Tailwind emits `.[&:open~svg]:rotate-180:open~svg` (the `~` survives the extractor), and a render captured immediately after a real click shows the chevron mid-flip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuaY5y2AeB3erMyNHHE5RN
…s on (#2662) Comments are stripped before matching. This branch already shipped a guard that passed on the prose above the constant it was meant to pin, and BaseSelect's block comment now names `:open` in exactly the way that would fake a pass. The adjacency assertion is the one that earns its keep: the rule is a sibling combinator, so wrapping the chevron or moving it above the select leaves every class in place and silently stops the flip — no other assertion in the suite would notice. Proven by breaking it four ways: deleting the flip class, inserting an element between select and chevron, dropping the reduced-motion guard, and moving the rule into the per-variant recipe each fail exactly one test, control green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuaY5y2AeB3erMyNHHE5RN
…rd (#2662) Same reasoning as the ghost recipe earlier in this branch: a behaviour nobody can find in the docs is one the next contributor re-implements inline. The contract carries the rule and the adjacency constraint; the full doc carries the mechanism and why a native select needed `:open` at all; the flow index row records it alongside the rest of the composer work. No new doc files — all three were already in this PR's diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuaY5y2AeB3erMyNHHE5RN
The flip shipped on the shared <select>, so it also changed `field` — the recipe Settings and ResourceModal render. #2662 is a Workspace composer bug and does not own those surfaces. The reason given for widening it was primitive consistency, which is a design-taste argument, not a scope one; this branch had already deferred the icon buttons' radius and the room Send's disabled state as out of scope, so the discipline was applied to other people's defects and not to this one. The flip and its transition move into the one `recipe` computed, on the ghost arm. `field` gains `flip: ''`, which `normalizeClass` drops, so its class string is byte-identical to before — verified, not assumed. Bringing every select into the idiom is still the right end state. It is a change to surfaces with their own reviewers, so it wants its own issue. The guard now pins the boundary in the direction it can drift: widening the flip back onto the shared element fails, as does leaking the transition onto the field chevron. Four mutations, one failing test each, control green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuaY5y2AeB3erMyNHHE5RN
`js/incomplete-multi-character-sanitization` (high) on the chain of `String.replace` regexes that removed comments before matching. The rule was right about the shape: a non-greedy `<!--[\s\S]*?-->` does not reach a fixed point on malformed or overlapping delimiters, which is the sanitiser failure mode it exists to catch. The alert was on this branch's own new file, so it is fixed rather than dismissed. Replaced with a single linear scan — every byte is either inside a comment run or copied out, exactly once, so there is no residue case to reason about. Own-line `//` only, so a `https://` inside a class string cannot truncate the code being asserted on. Adds the control that was missing: a stripper that returned its input unchanged would have passed all four existing assertions, since the block comment names `:open`, `ghost` and `field` in the shapes being matched. The new test pins a prose-only phrase as absent and two code tokens as present, and fails when the stripper is replaced by the identity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuaY5y2AeB3erMyNHHE5RN
…ase (#2662) The previous freeze was computed before this branch was rebased onto dev, so it recorded totals from a tree that predated `PortalJumpToLatest.vue` (created on dev by #2652, and never added to the baseline there). Committed 8722/3997/197 against an actual 8723/4020/198. Cosmetic, not a failing gate — `rawColorRatchet.spec.js` reads per-file entries and never `totals`, and the new file carries 0 `raw_nongray` so the "new code starts at zero" rule passes it. But the file's entire job is to be exact, and a stale total is the thing a later reader trusts. Nothing on this branch moved: fieldClasses.js stays 15, PortalConversation.vue 90, PortalRoom.vue 47 — verified by scanning dev's tree and this one separately rather than by reading the summary, which prints top offenders only. Own commit with the absorbed entry named, per the contract; same shape as the merge-train note already in the file. One trap worth knowing: `scan-raw-colors.mjs --baseline` REWRITES the file, it does not merge. A plain regeneration deletes the whole `refrozen` block — the #2605 provenance, the named increases, the merge-train note — and the diff for that deletion is easy to skim past because the entry counts look right. Restored here by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
…jacent one (#2662) Both docs described a stricter selector than the code ships. The contract said "keep the chevron the select's immediate next sibling or the sibling rule silently stops matching"; design-system.md said "the structural precondition is that nothing may come between the two". The class is `[&:open~svg]:rotate-180` — `~`, the general sibling combinator — so an element between the select and the svg is harmless. Measured in the browser the app runs in rather than read off the spec: `select ~ svg` matches across an intervening element, `select + svg` does not (Chrome 151). The two edits that DO kill the flip silently are wrapping the svg and moving it above the select, so the docs now name those instead. The spec keeps asserting the stricter adjacency — that is a fine thing to hold, and it catches both real breakages — but its comment gave the inaccurate reason, which is how a wrong mechanism outlives the sentence that stated it. Same class as the stale `right-6px` comment fixed earlier on this branch: the doc that is wrong about WHY is the one a contributor reasons from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
…ow says so (#2662) The open-state chevron reached the design system and the contract but not this flow, whose Testing section is where the honest coverage line lives — and it is the one behaviour in the branch that no test can verify as rendered. `:open` exists precisely because the platform draws the picker, so neither suite can open it and read the rotation back; `baseSelectChevron.spec.js` pins the class, the sibling adjacency, the reduced-motion guard and the ghost-only scope, and the visible turn was a hand check in a real Chrome 152. Stated because it degrades SILENTLY: below Baseline 2026-05 the chevron stays put, which is both correct behaviour and indistinguishable from the sibling rule having been broken by an intervening element. This is the same understatement the branch already corrected once, in the opposite direction — that block used to claim nothing rendered was machine- verified while the e2e measured boxes. A doc that overstates its coverage earns the same fix as one that understates it. npm run test:unit — 117 files / 2601 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHgiPs5QbdDxPjP4JGg7V1
…2662) The same trap this branch already recorded in `learnings.md`, one level down from the shell and twice. Both were found by reading `getComputedStyle` off a real render in BOTH themes; both were green under `npm run test:unit`, `check:tokens` and the raw-colour ratchet, because every one of those reads the class string rather than the cascade. 1. A DISABLED ghost select still lit up under the cursor in DARK mode. `disabled:hover:bg-transparent` outranks `hover:bg-gray-100` on specificity (3 vs 2) so light was correct, but it only TIES `dark:hover:bg-gray-750` — which compiles to `:hover:is(.dark *)`, also 3 — and Tailwind emits the dark variant later, so the tint won. The picker is `:disabled` during a voice call and whenever the server capability gate is off, so this is the live path, not a hypothetical. Measured before: rgb(42,48,60) on hover while disabled; after: rgba(0,0,0,0), light and dark. Fixed with the dark arm of the same reset. 2. A ghost select carrying an `error` rendered with NO danger border. `border-transparent` sat in the base string, and `.border-transparent` is emitted AFTER `.border-status-danger-500` at equal specificity, so the resting keyword silently beat the error colour — while `field`, whose border colour has always lived on the valid/invalid arms, was correct. Measured: ghost error border rgba(0,0,0,0) against field's rgb(239,68,68). Latent today (the composer passes no `error`) but this is a SHARED primitive recipe, and the next consumer to pass one would get a control that announces itself invalid through `aria-invalid` and looks untouched. Fixed by moving the resting colour onto the valid arm, which is where `FIELD_CLASS` has always kept it — so the two variants now fail and succeed identically. The guard is a SHAPE guard and says so: a class-string assertion is structurally blind to this class of defect, so it pins the arrangement that makes the cascade safe (colour on the arms, never in the base; every dark hover tint paired with a dark disabled reset) rather than pretending to check the cascade. Both halves falsified — reverting either one fails it. Costs nothing in the raw-colour budget: `transparent` is a keyword, not a palette shade, so the scanner counts neither class. Re-scanned — zero per-file movement, totals unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
…py (#2662) `tests/unit/helpers/stripComments.js` exists, and its own docstring says why: "Extracted here (#2161) because it was about to gain a third copy, and the copies carried a real defect." This file arrived as the fourth, and then `fdefeb9a6` re-derived by hand the exact CodeQL fix (js/incomplete-multi-character-sanitization — a non-greedy `<!--[\s\S]*?-->` leaving residue) that the shared helper had already solved, documented, and proven does not yield to the obvious "just loop it" patch either. Verified equivalent rather than assumed: the suite passes unchanged on the shared helper, self-proving control included — `'Deliberately NOT on \`field\`'` is still present in SRC and absent from CODE, so the stripper is still not the identity function. Not widened to `portalVoiceMode.spec.js`, which carries the same duplication in its regex-loop form. That copy predates this branch and its assertions are what this PR touched, not its helper; it belongs in the sweep, not in a composer fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
#2662) Three occurrences of one cascade trap in one diff — the shell's light-mode border, the ghost select's error border, the ghost select's dark disabled hover — means the lesson belongs where it gets read before code is written, not only in the learnings ledger after the fact. One bullet under Color in `design-system-contract.md`: never put two utilities of one CSS property in a class string and rely on emission order; state colour goes on mutually exclusive arms; a variant chain buys specificity, so a reset that must hold in both themes is spelled in both arms. The rest is the doc catching up with the fix: - `design-system.md` — "No resting border to recolour, so the valid state is focus-only" was true of the shipped string and is the sentence that produced the error-border bug. Replaced with what ghost actually does now (width in the base, colour on the arms, resets in both theme arms) plus the ordering rule in its recipe form. - `architecture/workspace.md` — the shell's cascade paragraph now names the two recurrences one level down, so a reader who fixes the shell does not conclude the class is closed. - `feature-flows/workspace-model-choice.md` — three corrections. The test table still described the chevron guard as "nothing may come between select and chevron", the claim `7ca9bb226` retired in the other two docs (`~` is the GENERAL sibling; the spec asserts adjacency on purpose, which is a different statement). Chrome 152 → 151, matching the other two citations and the Chromium the measurements were actually taken in (151.0.7922.34). And the Verification-honesty block now says what the e2e does NOT drive — hover, `error`, `disabled` — because that gap is precisely where both defects lived. - `learnings.md` — a recurrence note on the existing entry rather than a second one: same lesson, and the recurrences break its own corollary (1), since neither of these is light-mode-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrKTzf82bGXuxjckouLNFN
Review — one composer shell (#2662)Verification run: built the branch's Tailwind CSS and the full Vite bundle (both the Findings1. 2. 3. Not findings, checked and cleared
🤖 Generated with Claude Code |
…gain (#2662) The probe located the inert region with a `form.children` walk. This branch wrapped the composer in a shell, so the region moved two levels down (`form` -> `div.rounded-2xl` -> `div.mt-1.flex` -> `div.flex-1.min-w-0`) and the walk matched nothing: `inert` came back `undefined`, both assertions evaluated to `null`, and `expect(...).toBe(false)` failed wherever the voice-call button renders. The spec is `@interactive` and `frontend-e2e.yml` runs `@smoke` only, so CI could not have said so — and this is the one spec pinning "the call toggle sits outside the inert region", the rule the shell restructure had to preserve. A descendant query instead. Verified against the branch's own template: `.flex-1.min-w-0` matches exactly ONE element inside the composer `<form>` — the control row's wrapper, the one carrying `opacity-60 pointer-events-none` during a call — with the voice-call button before it (outside, as required) and the attach button inside it. `inertRegionFound` is asserted separately so the next nesting change reports its own cause instead of a confusing `null !== false` two lines further down. Reported by dolho in review of #2665. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9kHAsm3mH7trwQ9R4u3eN
…2662) Moving the picker inside the `<form>` gave it a behaviour it did not have on `dev`, where it was a sibling ABOVE the form: Chrome and Firefox route Enter on a focused `<select>` to the form's default button. So a user who tabs to the picker, arrows to another model and presses Enter to commit the choice sends their unfinished draft instead — Enter there previously did nothing. Harmless during a live turn (`canCancelTurn` swaps in a `type="button"` Stop and `send()` early-returns on `sending`); the exposure is idle-with-a-draft, which is the ordinary case for changing model before sending. `@keydown.enter.prevent` costs nothing: on every engine whose picker is drawn by the platform, the open dropdown never dispatches to the page, so the only page-level effect of Enter on this control was the submit. The guard is pinned in `portalComposerAlignment.spec.js` in the same "if there is a select, it wears this" shape as the ghost-recipe test beside it, so it covers PortalRoom too if that composer ever gains a picker. Falsified by deleting the handler: one failing test, control green. The comment naming the handler does not rescue it — `composerForm` strips comments before matching. Reported by dolho in review of #2665. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9kHAsm3mH7trwQ9R4u3eN
…#2662) "Each is a real flex row" was factually wrong for the field's wrapper: it ships as `<div ref="composerWrap" class="relative">` — a block. The substantive requirement (it generates a box, so `opacity` applies) holds, but the sentence invited a future contributor to "correct" `composerWrap` into a flex container, which would make the `block w-full` textarea a flex item and undo the #2259 line box fix this same file documents two paragraphs earlier. Reworded to what the rule actually is — each generates a real box — with the reason `composerWrap` specifically must NOT become one stated inline. Reported by dolho in review of #2665. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L9kHAsm3mH7trwQ9R4u3eN
All three review findings addressed —
|
vybe
left a comment
There was a problem hiding this comment.
Approving. /validate-pr Lane B — clean on every mechanical gate (security greps, docs across all six named files, Closes #2662 with status-in-progress set so it auto-promotes, ui label present so e2e actually ran), and all three of dolho's review findings are now fixed and pinned in d92404484.
24 checks green on the new head, including the six-job pytest matrix, build (which is what runs check:tokens + test:unit, i.e. the raw-colour and loading-gate ratchets) and e2e.
The two rulings the PR body asks for — the "Agent's default ((" truncation at 375px, and ghost vs bare as the BaseSelect variant name — are copy/vocabulary decisions that want their own issues; neither blocks the shape this PR is fixing.
Closes #2662
What
The Workspace composer becomes one box: the field on top, the controls in a row inside it, the model picker right-aligned beside Send. The reference is the shape every current chat UI uses.
ent#403 shipped the picker as a
BaseSelecton its default recipe — the same bordered field BaseInput uses on the Settings page — on its own row above the composer atw-full max-w-sm. Operator feedback ondev: "not a great location … looks very much out of place." The cause was never the control; the single-row composer had nowhere to put it. Every 44px button came out of the field's width, so at 375px the textarea measured 143px of a 351px form and wrapped its placeholder over four lines; adding a picker to that row is what left 34px in ent#403, hence its own row, hence the complaint.Stacked, the field takes the whole box at every width — 333px at 375px — and the picker is the element that yields, truncating itself while Send stays 44px.
How
BaseSelectgainsvariant="ghost"(fieldClasses.js→FIELD_GHOST_CLASS): borderless, transparent, content-width, 44px tall, hover-tinted like the header's agent picker. A variant on the primitive rather than a hand-rolled lookalike (design contract). The native<select>is kept on purpose: keyboard,:focus-visiblering and the platform picker on touch come free; a trigger + popover would have inherited the agent picker's known gap (closes on outside-click only, Escape does nothing).has-[textarea:focus]in the field primitive's recipe; the textarea goes transparent and borderless.focus-withinwas tried first and rejected: it lit the whole box when a button was tabbed onto, and drew two concentric rings on the picker.PortalRoom.vueincluded: the two are the same markup in two files (the bug: Workspace chat readability — tight line spacing, narrow message column, scrollbar in the empty composer #2211 lesson pinned inportalComposerAlignment.spec.js), so a shell landing in one leaves the room visibly diverged from the chat beside it. The room's row holds Send alone — the picker is per-agent, a room has several.composerWrapkeeps its ref name.Design-review pass (
/design-review, 8 one-class commits after the restructure)Plain terms: the picker was shorter than the buttons beside it (30 vs 44px — a small tap target on a phone); in dark mode hovering it, and the attach/mic/voice buttons, did nothing visible (their hover tint equalled the new box's ground — the buttons used to sit on the darker page); the box glowed when you tabbed onto a button and double-glowed on the picker; the picker's type was 13 where every sibling recipe is 13.5, and once corrected its longest label clipped its closing bracket at the width cap; two 6px values were off the 4px grid. All fixed, each verified live in both themes.
Open-state chevron (follow-on, operator-requested)
The picker's chevron now points up while its dropdown is open, animated over 150ms and static under
prefers-reduced-motion.This is the idiom the app already had everywhere else —
ChatHistoryDropdown,OverflowTabs,InfoPanel, and the disclosure rows inLoopsPanel/TasksPanel/PortalAgentDetails/PortalDeliverables/ChannelDisclosureall flip on open. Selects were the one control excluded from it, and not by oversight: all nine of those own their open state in JS, while a native<select>'s picker is drawn by the platform and reports nothing to the page.:openis the first hook that closes that gap (Baseline newly-available 2026-05 — Chrome 133, Firefox 136, Safari 26.5); on an older engine the chevron stays static, so nothing depends on it.Scoped to
ghost, deliberately. It first landed on the shared<select>, which also changedfield— the recipe Settings andResourceModalrender. #2662 does not own those surfaces, and "a primitive whose variants differ is a defect vector" is a design-taste argument, not a scope one. It now resolves through the onerecipecomputed;fieldgainsflip: '', whichnormalizeClassdrops, so its class string is byte-identical to before (checked through@vue/shared, not asserted). Widening the idiom to every select is the right end state and wants its own issue.Verified against a real browser, not the class string — this branch already shipped classes that were written and did not survive to the box (see the
learnings.mdentry):CSS.supports('selector(select:open)')is true in Chrome 152; Tailwind genuinely emits.\[\&\:open\~svg\]\:rotate-180:open~svg, so the~survives the extractor; and a render captured immediately after a real click shows the chevron mid-flip. Not verified: whether macOS's native menu sits over the control while open, which would hide the flip on that platform — a one-second human check, and the reason to look before merging.tests/unit/baseSelectChevron.spec.js(5 tests) strips comments before matching — theh-11prose trap from this branch's own review — and pins the sibling adjacency the rule depends on, since wrapping the chevron leaves every class in place and silently kills the flip. Proven by four mutations (widen onto the shared element, leak the transition tofield, break adjacency, drop the reduced-motion guard): one failing test each, control green. It uses the shared stripper (tests/unit/helpers/stripComments.js, #2161) rather than a fifth local copy, and the fifth test is a control proving that stripper is not the identity —BaseSelect.vue's block comment names:open,ghostandfieldin exactly the shapes being asserted on, so an identity stripper would pass every assertion above it.Two cascade races in the ghost recipe (adversarial review,
fc3e0f83d)The same trap the shell hit, one level down — both measured on a live render in both themes, not reasoned:
errorrendered with no danger border.border-transparentsat inFIELD_GHOST_CLASS's base string, and.border-transparentis emitted after.border-status-danger-500at equal specificity, so the resting keyword silently beat the error colour.The base string now carries
borderwith no colour (exactly likeFIELD_CLASS— the 1px is reservedso the focus border costs no layout shift); resting-transparent moved to the valid arm, leaving the
invalid arm's colour the only border colour in play. Latent on this branch, but
BaseSelectis a sharedprimitive, so it was a defect for any future consumer passing
error.disabled:hover:bg-transparentoutrankshover:bg-gray-100(3 vs 2), butdark:hover:bg-gray-750compiles to
:hover:is(.dark *)— also 3 — and is emitted later, so it won. The reset is now spelled inboth theme arms. Reachable today: the picker is
:disabledfor the whole of a voice call.portalComposerAlignment.spec.jsgains the cascade-shape guard (colour on the arms, never in the basestring; every dark hover tint paired with a dark disabled reset). Both fixes were falsified by reverting
each: one failing test apiece. Raw-colour count is unchanged by this commit (
fieldClasses.jsstays 15).Verification
e2e/workspace-model-choice.spec.jsrewritten to the new contract and run against a live sibling stack (not mocked geometry): picker inside the shell on Send's row and right of every icon button, no border/fill of its own, field ≥ 90% of the form (was 41% at 375px), every button 44px — 5/5 at 375 / 768 / 1280. Rendered assertions per Review standard: a source-text assertion is not behaviour coverage #2659, not source greps.portalComposerAlignment(both surfaces),portalComposerTypeahead,portalModelChoice,portalVoiceModeupdated to the stacked contract and green; full frontend suite green apart from five specs that read repo-root paths outside the container mount.fieldClasses.js11 → 15 for the ghost recipe's ink + hover pair, re-frozen in its own commit with the increase named, as that issue's AC requires.PortalConversation.vueandPortalRoom.vuemove chrome between elements without adding any (90, 47 unchanged).@vue/compiler-sfc.Docs (landed —
/validate-prpass)Every doc file in this diff, named.
/validate-prflaggeddesign-system.mdas an unnamed doc change on the first pass; the list below is now the complete set of six, so a reviewer can check the diff off against it rather than discover a file.docs/memory/feature-flows/workspace-model-choice.mddocs/memory/feature-flows.mddocs/memory/architecture/workspace.mddocs/memory/design-system.mdBaseSelect variant="ghost") entering the system of recorddocs/memory/design-system-contract.mddocs/memory/learnings.mdfeature-flows/workspace-model-choice.md— Entry Points and the Frontend Layer table now place the picker inside the shell on the control row. Also corrected: the Testing section claimed "nothing rendered is machine-verified … the select's width against the textarea, its alignment … are human checks", which this branch made false. It now names the live e2e and keeps the honest residue (both themes, the hover/focus tints) as a human check.architecture/workspace.md— the composer paragraph listed a four-button row and one inert region; there are now five controls and two regions, and one without the other leaves half the composer live during a call. It also records why the shape exists (the width mechanism behind bug(workspace): composer buttons align to the typeahead wrapper, not to the input — Send hangs 6px low #2259's wrap and ent#403's 34px), thehas-[textarea:focus]scope and whyfocus-withinwas rejected, and that both composers carry it.design-system.md+design-system-contract.md— not previously listed, and the one with teeth.variant="ghost"was a new primitive recipe that never reached the system of record. The contract's first rule is primitives over hand-rolled lookalikes; a variant nobody can find is one the next contributor re-implements inline — bug(workspace): the model dropdown is a settings-form field parked above the composer — wrong primitive, wrong container, wrong weight #2662's own defect, one surface along. §BaseSelect now documents both recipes, the primitives table cites an adoption of each, and the contract carries the choosing rule. Theghostnaming collision with BaseButton is stated there rather than left to be rediscovered.feature-flows.md(the index) — the flow index gains its2026-09-09 | #2662row. Named separately because it is the line that gets forgotten: a flow doc edited without its index row leaves the change invisible to anyone reading the index, and this is a repeat miss for me. The row carries the width mechanism, thehas-[textarea:focus]scope, the shell's two obligations during a call, and the ghost recipe.learnings.md— a new pitfall entry: two Tailwind utilities of the same shape can disagree about which of an equal-specificity pair wins (.border-transparentis emitted after.border-gray-300, but.bg-transparentbefore.bg-white), soborderandbgwritten as one visual pair resolved in opposite directions — and everydark:variant is emitted after both base utilities, so the defect was light-mode-only and invisible to a dark-mode reviewer. It shipped past source review, the unit specs,check:tokensand the raw-colour ratchet, because all of them read the class string and none reads the cascade. Recorded because the instrument that caught it (getComputedStyleoff a live render, in both themes) is the generalisable part.PortalComposerAlignment.spec.js— the 44px rule now covers the select. Two halves, because the height is split:variant="ghost"at the call site (conditional, so it also covers PortalRoom if that ever gains a picker), andFIELD_GHOST_CLASScarryingh-11and notw-fullin the recipe. Proven by breaking each: all three mutations fail the suite.PortalConversation.vue:524— the shell comment said the ring arrives viafocus-within:; the code shipshas-[textarea:focus]:and the spec assertsfocus-within's absence. The comment pointed a reader at the exact approach FINDING-008 removed.One note on method, since it nearly shipped: the recipe guard's first version read
fieldClasses.jsas source text and matched/\bh-11\b/. It passed withh-11deleted from the class — the comment above the constant explains the choice and contains the literalh-11. A source-text guard over a documented constant tests the prose. It asserts the imported value now.Needs a ruling (operator)
variant="ghost"on BaseSelect is gray-inked; the system's only documented ghost (BaseButton) is accent-inked. Two recipes under one name —variant="bare"for the gray chrome idiom is the cleaner vocabulary, if agreed.Deferred as pre-existing and outside this branch's scope: the six icon buttons' 12px radius (contract says 6, nesting rule says 8) and the room Send's missing
disabled:hoverguard /.40/.50disabled opacities.🤖 Generated with Claude Code