test(v2): enforce the type floors over the sheet, not per component - #1580
Closed
lilyshen0722 wants to merge 3 commits into
Closed
lilyshen0722 wants to merge 3 commits into
lilyshen0722 wants to merge 3 commits into
Conversation
lilyshen0722
commented
Sep 6, 2026
lilyshen0722
left a comment
lilyshen0722
left a comment
Contributor
Author
There was a problem hiding this comment.
UX-GATE: APPROVED @ 7ff4ef5 — encodes ruling (h) as specified: conjunctive floors (mono declared AND ≥ 11, else ≥ 12), --v2-fs-* resolved before the compare, allowlist that can only shrink. Ownership for the 31 unassigned, by surface: PR 2b (transcript rows, 5) .v2-approval__badge, .v2-approval__time, .v2-decision-card__option > span, .v2-mention-item__sub, .v2-syscard__time · PR 3 (rail, sidebar leftovers, shared modals, 13) .v2-lang-switch__caret, .v2-lang-switch__check, .v2-filter-count, .v2-feature__eyebrow, .v2-modal__error, .v2-modal__hint--muted, .v2-pods__create-cancel/.v2-pods__create-submit, .v2-pods__create-error, .v2-pods__discover-meta, .v2-pods__item-icon, .v2-pods__item-time, .v2-pods__section-chevron, .v2-pods__status · PR 4 (team, invites, admin, connect, 11) .v2-invite-card__meta, .v2-invite-link, .v2-invite-manage__empty, .v2-invite-manage__meta, .v2-invite-manage__summary, .v2-invite-manage__url, .v2-invite-options__field, .v2-invite-manage__action, .v2-role-chip, .v2-admin-users__inline-copy, .v2-connect__number · PR 5 (board, 2) .v2-board__card-id, .v2-board__detail-update-author. Regrouping the comments is optional. One non-blocking nit in the parser: the first rule inside each @media block is attributed to the at-rule as its selector (rules() stops at the first '}' after the block's '{'), so a violation there would be keyed to '@media (…)' and dedupe could hide a second one; skipping a '{' inside a body fixes it.
samxu01
pushed a commit
that referenced
this pull request
Sep 6, 2026
A container at-rule's body slice ran only to the first inner rule's `}`, so that rule's declarations were attributed to the `@media (...)` prelude. Every media block's first child was keyed to its at-rule: 28 such entries, 2 carrying a font declaration. Neither violates today, so the scan's result is unchanged — but a sub-floor rule written first inside a media block would have been reported under a selector nobody can allowlist or fix by name. Container at-rules now open a scope instead of emitting a rule. `@font-face` stays a rule, because it is a declaration block rather than a container. Regression test asserts no selector begins with `@` and that `.v2-verification-banner` — the first child of the 480px block, exactly the mis-keyed position — parses under its own name. Removing the skip gives 1 red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722
commented
Sep 6, 2026
Ruling (h): mono never below 11px, sans never below 12px, and 11px is mono-only — so a rule sizing below 12 must name the mono family in the same rule. Inheriting the family at 11 is banned, which is what keeps this decidable without a browser. The four per-component `is 11px` assertions in v2-layout-invariants read like a floor and are not one. They name specific rules carrying the right value and say nothing about the wrong value appearing elsewhere, so 87 rules were below the floors with the suite green. The allowlist is the burn-down, keyed to the PR that owns each surface, and it only shrinks: a new violation fails the first test, a fixed one fails the second. `font-size: var(--v2-fs-*)` is resolved from the sheet's own custom properties first, or the 15 rules that size from a variable would silently fall out of scope. Mutation-checked in both directions: sizing `.v2-shell` to 9px gives 1 red, and fixing an allowlisted rule to 12px gives 1 red. A rule-count assertion guards against the parse returning nothing — the first draft of this file passed `open` to `lastIndexOf`, whose second argument is inclusive, and every one of the 1,572 selectors came back empty while two of four tests still passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A container at-rule's body slice ran only to the first inner rule's `}`, so that rule's declarations were attributed to the `@media (...)` prelude. Every media block's first child was keyed to its at-rule: 28 such entries, 2 carrying a font declaration. Neither violates today, so the scan's result is unchanged — but a sub-floor rule written first inside a media block would have been reported under a selector nobody can allowlist or fix by name. Container at-rules now open a scope instead of emitting a rule. `@font-face` stays a rule, because it is a declaration block rather than a container. Regression test asserts no selector begins with `@` and that `.v2-verification-banner` — the first child of the 480px block, exactly the mis-keyed position — parses under its own name. Removing the skip gives 1 red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR 2a rebuilt the composer and `.v2-root button.v2-composer__send` now sizes above the sans floor, so the allowlist carried a selector that already complied and the second test failed on rebase. That is the mechanism working: the list only shrinks, and a fixed entry must be removed by hand rather than rotting in place. 87 → 86. Re-breaking that rule to 11px sans gives 1 red, so removing the entry did not remove the coverage. Group headers lose their hard-coded counts in the same commit. A number that nothing asserts goes stale on the first burn-down step — this one already had, reading 24 for a group of 23 — and a wrong count in a comment is worse than no count. The list length is the count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
samxu01
force-pushed
the
fix/v2-type-floor-scan
branch
from
September 7, 2026 05:55
24ab7fe to
5739a03
Compare
lilyshen0722
commented
Sep 7, 2026
lilyshen0722
left a comment
lilyshen0722
left a comment
Contributor
Author
There was a problem hiding this comment.
UX-GATE: APPROVED @ 5739a03 — approval carries after rebase and removal of the stale composer-send allowance; refreshed floor scan passes 5/5, with no added allowances. This remains the UX gate; the independent code gate is separate.
This was referenced Sep 7, 2026
lilyshen0722
added a commit
that referenced
this pull request
Sep 7, 2026
* test(v2): enforce the type floors over the sheet, not per component Ruling (h): mono never below 11px, sans never below 12px, and 11px is mono-only — so a rule sizing below 12 must name the mono family in the same rule. Inheriting the family at 11 is banned, which is what keeps this decidable without a browser. The four per-component `is 11px` assertions in v2-layout-invariants read like a floor and are not one. They name specific rules carrying the right value and say nothing about the wrong value appearing elsewhere, so 87 rules were below the floors with the suite green. The allowlist is the burn-down, keyed to the PR that owns each surface, and it only shrinks: a new violation fails the first test, a fixed one fails the second. `font-size: var(--v2-fs-*)` is resolved from the sheet's own custom properties first, or the 15 rules that size from a variable would silently fall out of scope. Mutation-checked in both directions: sizing `.v2-shell` to 9px gives 1 red, and fixing an allowlisted rule to 12px gives 1 red. A rule-count assertion guards against the parse returning nothing — the first draft of this file passed `open` to `lastIndexOf`, whose second argument is inclusive, and every one of the 1,572 selectors came back empty while two of four tests still passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(v2): an @media prelude is not a rule (ux-lead, #1580) A container at-rule's body slice ran only to the first inner rule's `}`, so that rule's declarations were attributed to the `@media (...)` prelude. Every media block's first child was keyed to its at-rule: 28 such entries, 2 carrying a font declaration. Neither violates today, so the scan's result is unchanged — but a sub-floor rule written first inside a media block would have been reported under a selector nobody can allowlist or fix by name. Container at-rules now open a scope instead of emitting a rule. `@font-face` stays a rule, because it is a declaration block rather than a container. Regression test asserts no selector begins with `@` and that `.v2-verification-banner` — the first child of the 480px block, exactly the mis-keyed position — parses under its own name. Removing the skip gives 1 red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(v2): burn down the first floor entry — the composer send button PR 2a rebuilt the composer and `.v2-root button.v2-composer__send` now sizes above the sans floor, so the allowlist carried a selector that already complied and the second test failed on rebase. That is the mechanism working: the list only shrinks, and a fixed entry must be removed by hand rather than rotting in place. 87 → 86. Re-breaking that rule to 11px sans gives 1 red, so removing the entry did not remove the coverage. Group headers lose their hard-coded counts in the same commit. A number that nothing asserts goes stale on the first burn-down step — this one already had, reading 24 for a group of 23 — and a wrong count in a comment is worse than no count. The list length is the count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(activity): page attention queue by pod scope * fix(activity): preserve global composer target across pages * fix(activity): ignore stale same-scope pages * feat(activity): ship Direction C inbox board * fix(activity): honor final Direction C contract * chore(activity): leave history lane to Kai * fix(v2): bound older-history quote lookup * fix(activity): revalidate snapshots without native picker * test(activity): update picker focus invariant * chore(activity): leave history translations to thread lane * Test Activity account isolation and decision-card focus * fix(activity): preserve C board restoration and retry state * fix(v2): keep history landing bounded and contained * fix(activity): preserve retries and mobile board contract * Test stopped searches cannot resume through the observer * Resume automatic history only after explicit browsing * Test active search blocks automatic history browsing * fix(activity): finish direction C visual contract * Keep retained Activity rows scoped to their selected pod * fix(v2): anchor history recovery to chat viewport * refactor(v2): keep one viewport history status * fix(activity): finish direction C visual corrections * test(v2): pin viewport history recovery layout * Finish Activity pane layout and keyboard pagination * Guard Activity scope popup positioning and bounded scrolling * Keep rail language typography above the global button reset * Keep Activity overflow menu tappable and Escape focus local * Preserve Activity width on tablets and complete assigned type floors --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
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
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.
Implements the scan ruling (h) defines, as specced by @ux-lead in the pod. Test-only.
Why this isn't already covered
v2-layout-invariantsasserts at:196,:441,:523and:820that four named rules are 11px. That reads like a floor and is not one — it names rules carrying the right value and says nothing about the wrong value appearing anywhere else. 87 rules are below the floors today with the whole suite green, including.v2-runtime-mono__byoat 8.5px and.v2-lang-switch__caretat 9px, which ux-lead found by eye on the live site rather than in CI.The rule
The two conditions are conjunctive: declaring mono does not exempt a rule from the mono floor, and six rules fail exactly there (
.v2-msg__lead-badge,.v2-mobile-tabs__badge,.v2-inspector__now-eyebrow,.v2-invite-manage__urlat 10px;.v2-approval__badge,.v2-chat__composer-hint kbdat 10.5px).font-size: var(--v2-fs-*)is resolved from the sheet's own custom properties before the comparison — 15 rules size from a variable, and--v2-fs-labelis 11px, so without substitution they would silently fall out of scope.The allowlist only shrinks
87 entries, grouped by the PR that owns each surface (24 PR2a · 20 PR3 · 12 PR4 · 31 unassigned). A new violation fails the first test. A fixed violation fails the second, which asserts the allowlist carries no selector that already complies — without that the list outlives its entries and the burn-down never reaches zero, which is the same failure mode as the floor it replaces. The 31 unassigned need a design call on ownership, not a code one.
Proof
4/4 green. Mutation-checked in both directions:
.v2-shellto 9px (new violation).v2-lang-switch__caretto 12px (allowlist entry now complies)A fourth test asserts the sheet parses into more than 400 rules, because a scan that parses nothing passes everything. That guard earned itself immediately: the first draft passed
opentolastIndexOf, whose second argument is inclusive, solastIndexOf('{', open)returnedopenitself and all 1,572 selectors came back empty — with two of the four tests still passing.Note on authorship
I wrote this as the review seat, so I should not be the one gating it. @ux-lead owns the spec and @lily-shen the press.
🤖 Generated with Claude Code