fix(ui): draw the reasoning chevron from the Astryx icon registry - #2228
Merged
Conversation
The ejected lab ChatReasoning hand-wrote its own chevron (12 viewBox, strokeWidth 1.5) while tool rows use Astryx `Icon icon="chevronDown"` (24 viewBox, theme stroke 1.75). chat-message.css forces both to 10x10, so the same-looking glyph rendered with 1.25px of stroke on the Thinking row against 0.73px on tool rows. Route the reasoning chevron through the registry so the app has one chevron authority, extend the 10x10 rule to the registry icon's wrapper span on both rows, and pin the shared source with a markup contract.
The markup contract compared one filtered pool of registry icons across the whole turn, so the reasoning row — whose pre-fix chevron was not a registry icon at all — fell out of the pool and left the tool rows agreeing with themselves. Only the negative viewBox assertion caught the regression the test was named for. Slice the turn at the two row roots and compare each row's own chevron instead: the pre-fix component and a swap to a different registry icon now both fail on the assertion that states the invariant. Add the CSS half at the css-test-helpers seam. The markup test cannot see chat-message.css, so nothing stopped a later edit from dropping a row (or the wrapper arm) out of the 10x10 rule and desyncing the two chevrons again.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
The disclosure chevron on the Thinking row rendered with a visibly heavier stroke than the identical chevron on tool rows.
Root cause: two different chevron sources. Tool rows use Astryx
<Icon icon="chevronDown" size="xsm">— a 24-viewBox glyph carrying the theme'sstroke-width: 1.75.ChatReasoning, ejected from the Astryx lab package, hand-wrote its own chevron at 12 viewBox /strokeWidth 1.5. Both are then forced to 10x10 bychat-message.css, so the effective stroke was 1.25px vs 0.73px — the same-looking glyph, ~70% heavier on one row. Path geometry was otherwise identical.The reasoning chevron now goes through the registry (
<Icon icon="chevronDown" size="xsm" color="inherit" />), so the app has one chevron authority and one place to retune it. The registry's string mode wraps its svg in anastryx-iconspan, so the 10x10 rule now covers the wrapper (> *) on both rows, not just on tool rows — the two selectors collapse into one:is()pair.The file header already documents the product-dialect deviations layered on the ejected lab DOM (cursor, hover wash, chevron size); this deviation is recorded there too.
Verification
Measured in Storybook (
Product/Shell Official AppShell → NativeConversation) against the live DOM, not eyeballed —getComputedStyle(path).strokeWidth × path.getScreenCTM().afor the trailing chevron of each disclosure:0 0 12 120 0 24 240 0 24 240 0 24 24Layout and motion are unchanged: both chevron wrappers still measure 14x14 with a 10x10 svg,
margin-inline-start: autointact, and expanding still rotates the chevron 180°.(One measuring trap worth recording: read the expanded chevron's
transformin a throttled background tab and you get an identity matrix, because theCSSTransitionsits atcurrentTime: 0forever. Force a paint first, orgetAnimations().forEach(a => a.finish()).)Also run:
npm run format/npm run format:check/npm run lint— cleannpm run typecheck(all workspaces) — cleannode --test "packages/ui/dist/**/*.test.js"— 342 pass, 0 fail, including the new chevron contractapps/desktopmain tests — the 12 failures inproject-root-controller/ project-management reproduce on a cleanmaincheckout (tmpdir symlink resolution) and are untouched by this changeTwo contracts guard the invariant, one per half, each verified to fail on the state it claims to catch:
processing-block.test.tsxslices the turn at the two row roots and compares each row's own chevron markup. Restoring the pre-fix component fails on "the reasoning row must draw exactly one registry icon"; swapping the row tochevronRightfails on "both rows must render the same chevron markup".chat-disclosure-chevron-contract.test.ts(new, at the existingcss-test-helpersseam) asserts each row declares both its chevron svg and its wrapper 10x10. Dropping the reasoning arm from the:is()fails; dropping the wrapper arm fails. It pins the outcome, not the wording — resplitting or reordering the selector list stays green.