Uh oh!
There was an error while loading. Please reload this page.
feat(ui): migrate ToolActivity card shell onto @maka/ui chat primitives (#332 PR3b) - #350
Merged
Merged
Conversation
…ity card (#332 PR3b) Add a `toolVariants` cva literalize table to packages/ui/src/primitives/chat.tsx covering the `ToolActivity` card shell parts (container / count / item / header / dot / name / meta / duration / status-label / body / intent / args). Every value is a literal arbitrary utility compiling 1:1 to the bespoke `.maka-tool*` it will replace, mirroring `markerVariants` / `streamVariants`. Kept OFF the package barrel (relative import only) so it stays an internal, removable styling detail. The running dot's `[animation:maka-tool-pulse…]` rides in the `dot` part; the mount entrance + `<summary>` marker reset are left for the CSS residue (next commit) since they escape the leaf-literal proof. `waiting_permission` uses a `String.raw` `\_` escape so the scanned source and cva's runtime class agree on a single backslash — a bare `_` would be read by Tailwind as a space and never match. No consumer yet; the table is wired up in the following commit.
…-tool* CSS (#332 PR3b) Rewire `ToolActivity` to apply the `toolVariants` parts (+ `data-slot="tool"` on the card), and delete the bespoke shell it replaces: `.maka-tool` / `.maka-tool-header` / `-name` / `-meta` / `-duration` / `-status-label` / `-status-dot` / `-body` / `-intent` / `-count` and the `[data-status]` border swaps (maka-tokens.css), plus `.toolInline` / `.toolItem` / `.toolArgs` (tool-output.css). The args `<pre>` keeps the shared `.maka-code` base. What stays is the irreducible residue that escapes the computed-style proof, re-keyed off the retired `.maka-tool` class onto the governed `[data-slot="tool"]` hook: the mount entrance (transition + `@starting-style`) and the native `<summary>` marker reset. The running dot's `@keyframes maka-tool-pulse` ring is kept too (a keyframe is a global rule, not an element property). Consumer + CSS removal land together so the renderer orphan-selector contract stays green. Out of scope: `ToolErrorBanner` + `.maka-tool-error*` (on `Alert`; PR3c) and the result-preview renderers (`.maka-tool-diff*` / `.maka-tool-terminal*`).
…f harness (#332 PR3b) Pin the two halves of "zero visual change" the way PR2 / PR3a did: - Static shell -> computed-style diff: extend check-chat-marker-computed-style.mjs with a tool-card tree (section + count, all five `[data-status]` cards, the summary header grid, four static dot colors, name/meta/duration/status-label/ body/intent/args). 20 new rows, 0 delta (running dot excluded — animated). - Escapes -> source / keyframe contract: add chat-tool-card-cascade-contract.test.ts asserting the retired selectors are gone (without touching `.maka-tool-error*`, `.maka-tool-diff*`, `.maka-tool-terminal*`, `.maka-code`), the `@keyframes maka-tool-pulse` ring frames stay, the `[data-slot="tool"]` entrance + marker residue stays, and the running-dot literals hold. Plus a runtime guard in chat-primitives.test.ts that cva preserves the single-backslash `waiting_permission` escape and stays literal. The diff harness caught a real bug while building this: `waiting_permission`'s underscore compiled to a Tailwind space and the tint silently fell back to base — fixed in the primitive (String.raw `\_`), now locked by both tests.
…oof (#332 PR3b) Review follow-up (P2-B). The computed-style diff forced every status card `open`, so it never exercised the real `isOpenByDefault` default — and the most common historical card is a settled, COLLAPSED `completed` tool. Rebuild the fixture to render each card at its true state (pending/waiting/running/errored open, completed/interrupted collapsed), move the rich inner parts onto the open `errored` card, and add a collapsed `completed` card. The non-vacuous collapsed signal is the summary divider: 1px (open) vs 0px (collapsed, the `[open]` gate) — verified distinct, identical across main/head. 23 tool-card rows, 0 delta (59 rows total). Also pin the `[open]>summary` divider literal in the cascade contract as the automated guard (the diff harness is manual / no CI). (The body is hidden via Chromium's `::details-content`, so its child `display` stays `block`; the collapsed body row still diffs box / typography parity. The earlier comment claiming `display:none` is corrected.)
… why (#332 PR3b) Review follow-up (P3-A). Shed the PR-narrative lines from the `WP_*` escape comment (12 -> 5), keeping the Tailwind `_`-as-space / `String.raw` rationale. Density now matches the sibling `streamVariants` / `markerVariants` docstrings.
…#332 PR3b) The computed-style fixture rendered five of the six production tool statuses but omitted `pending` — a default-expanded state (isOpenByDefault) whose card therefore had no diffed row, so a future pending-specific tint drift or mis-literalization (the class of bug `waiting_permission`'s `_`-escape hit) would pass unseen. The script's own comment + openByDefault already claimed pending was covered, so the fixture was internally inconsistent. - render a `pending` card (STAT) + diff its item/dot (IDS); it has no `data-[status=pending]` branch, so it proves the base border + gray-dot fallback. Computed-diff: 61 rows, tool-item/dot-pending 49/49 identical, 0 delta. - guard the gap shut: the cascade contract now derives the status set from components.tsx's `STATUS_LABEL` (the `ToolActivityItem['status']` Record) and asserts the harness STAT renders every one — a new status can no longer escape the proof silently.
The fixture's header count was hardcoded `'5'` while STAT had grown to six
statuses — a self-description that no longer matched the rendered cards. The
count never entered the computed diff (it reads CSS props, not textContent), so
this was a credibility gap, not a missed regression — but the cards, count, and
diffed IDS were three hand-kept lists that could drift apart.
Collapse them onto the single module-level STAT: cards `STAT.map`, count
`String(STAT.length)`, and the IDS tool rows derive `tool-item-${s}` for all and
`tool-dot-${s}` for all but running (its animated ring stays keyframe-pinned).
This also closes a residual hole — a new status added to STAT now auto-gets a
diffed row instead of rendering an un-measured card. Diff unchanged: 61 rows, 0 delta.
This was referenced Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves the
ToolActivitycard shell — the inline section + count pill, the<details>card, its<summary>header row + status dot, the body / intent, and the args<pre>override — onto the@maka/uichat substrate, retiring the bespoke.maka-tool*/.toolInline/.toolItem/.toolArgsCSS with zero visual change. This is PR3b of the #332 conversation-flow governance pass, following PR1 (#334, bubble/row shell), PR2 (#337, turn markers), and PR3a (#348, tool live-output stream).The static shell literalizes onto a new internal
toolVariantscva exactly as PR2/PR3a did. Three pieces escape the leaf-literal proof and stay a small named residue re-keyed off the retired.maka-toolclass onto the governed[data-slot="tool"]hook: the card mount entrance (transition+@starting-style, which only paints on the first frame), the native<summary>marker reset (pseudo-elements), and the running dot's@keyframes maka-tool-pulsering (a keyframe is a global rule, not an element property). These are pinned by a cascade contract + the diff harness instead.Why
Closes part of #332 (PR3b). The conversation-flow display is being migrated onto one
@maka/uisubstrate + a test net, one island at a time, with zero visual change. The tool card was the one remaining bespoke island between already-governed neighbours (bubble / marker / stream); lifting it now keeps the chat surface coherent and location-independent (status is adata-*vocabulary, not a descendant-selector coupling). The native<details>card and the.maka-turn-thinkingblock can later converge onto one Base UI Disclosure primitive — PR3b deliberately leaves both in the same shape (native details + named residue) so that structural pass is a clean follow-up, not a prerequisite.Scope
Changed (three atomic commits):
feat(ui): add internal toolVariants chat primitive—packages/ui/src/primitives/chat.tsx(the cva table; every value a LITERAL arbitrary utility compiling 1:1 to the retired declaration),packages/ui/src/index.ts(kept OFF the barrel likemarkerVariants/streamVariants— single consumer, relative import).refactor(ui): migrate ToolActivity onto toolVariants and retire .maka-tool* CSS—packages/ui/src/components.tsx(rewire +data-slot="tool"),apps/desktop/src/renderer/maka-tokens.css+styles/tool-output.css(delete the migrated rules; keep the[data-slot="tool"]entrance/marker residue + the@keyframes maka-tool-pulsering). Consumer + CSS removal land together so the renderer orphan-selector contract stays green.test(ui): lock the migration with cascade contract + diff harness— see Verification.Not included:
ToolErrorBanner+.maka-tool-error*— already on the@maka/uiAlertprimitive; its dead grid/icon/body/title classes and live copy-feedback tints are coupled tovisible-copy-hygiene-contract, a different review lens. Separate PR (PR3c)..maka-tool-diff*/.maka-tool-terminal*/.maka-office-document*/.maka-explore-agent*/.maka-load-tool-*/.maka-overlay-*) — separate components rendered inside the tool body, a different concern..maka-codeinline-code base (Markdown / artifact previews use it too) — untouched; the args<pre>keeps the class and only its.toolArgsoverride moves..maka-turn-thinking— a later structural pass.Verification
This repo has no CI; everything below was run locally.
npm run -w @maka/ui test— 13/13 (adds atoolVariantsliteral/scale-drift + single-backslashwaiting_permissionruntime guard).npm run -w @maka/desktop test— 1613/1613 (addschat-tool-card-cascade-contract.test.ts: retired selectors absent without touching.maka-tool-error*/.maka-tool-diff*/.maka-tool-terminal*/.maka-code;@keyframes maka-tool-pulsering frames pinned; the[data-slot="tool"]entrance + marker residue pinned; the[open]>summarydivider + running-dot literals +String.rawescape pinned; and a guard deriving the status set fromSTATUS_LABELso every production status keeps a diffed row).npm run typecheck— clean across all workspaces.npm run check:chat-visual -- <pre-PR2 baseline.css> <head.css>(the computed-style diff, extended with a tool-card tree): against a pre-PR2 renderer CSS baseline (e033a8c4~1, which still carries the bespoke marker + stream + tool CSS), TOTAL DIFFS: 0 across 61 element/state rows + 2::beforemiddots — including all 25 tool-card rows (every one of the six production statuses renders a card —pendingincluded, proving the base border + gray-dot fallback it has nodata-[status]branch for). Each card is rendered at its realisOpenByDefaultstate (pending/waiting/running/errored open, completed/interrupted collapsed), so the most common historical card — a settled, collapsedcompletedtool — is covered too: the collapsed summary's divider reads 0px vs the open card's 1px (the non-vacuous proof the collapsed branch is exercised), identical across main/head. Non-vacuous: the baseline and head bundles differ in source, yet the migrated chrome reads byte-identical computed style on both sides.getComputedStyle); pinned by the@keyframes maka-tool-pulseframe contract + thechat.tsxliteral.User-facing impact
None — zero visual change by design, locked by the contract + diff above.
Reviewer notes
waiting_permissioncarries a literal underscore, and Tailwind turns a bare_in an arbitrary value into a SPACE — sodata-[status=waiting_permission]compiled to[data-status="waiting permission"]and the card/dot tint silently fell back to the base color (3 diff rows). Fixed with aString.raw\_escape so the scanned source text and cva's runtime class agree on a single backslash; both contract tests now lock it (the four underscore-free statuses never had the problem).toolVariantsis deliberately off the barrel (likemarkerVariants/streamVariants): one consumer, applied by relative import, so it stays an internal, freely-removable styling detail rather than public API.toolVariantspart, not a shared primitive likeLiveIndicator: it is tool-specific (six states, a box-shadow ring pulse distinct from themaka-pulseopacity/scale breath), so there is no second consumer to justify promotion.*rules (maka-tokens.css / base.css), so — unlikeLiveIndicator, a reusable primitive carrying its ownmotion-reduce:guards — the dot and card need no per-element motion utilities; the same global rules cover them as before.