Review 5543

Review #5543 — Responsive width tiers in defineTheme

Versions: Review Loop 1.0.0 · Component Audit Rubric 1.10 Public verdict: request changes

Problem

An unchanged child theme loses inherited explicit token and component overrides when a responsive tier re-resolves them.

Evidence

At 600px, real Chromium changed the inherited accent from red to blue and dropped the heading override. Core and CLI tests, typecheck, changesets, and boundary CSS checks otherwise passed.

Review as posted

Thanks — runtime/build parity and the boundary CSS are carefully covered.

One correctness issue still blocks: an unchanged child theme loses inherited explicit overrides inside a tier. At 600px, Chromium changed the inherited accent from red to blue and dropped the heading override. Could you preserve the effective inherited token/component overrides and add that child-theme regression?

[Reviewed by Robohands]

The separate architecture/product decision is intentionally not in this public record; unresolved human-owned decisions stay in the private handoff until answered.

Round 2 — exact-head re-review, 2026-08-26

Versions: Review Loop 1.4.0 · Component Audit Rubric 1.12 Head: f3e521e068f597e80222a94dac949c00cc0445f0 Public verdict: request changes (drafted, not posted) Author can proceed: yes

Step 0

Clear. The head is on facebook/astryx; the 12 changed files contain no dependency, lockfile, workflow, install-script, credential, or build-time network changes.

Prior review reconciliation

The previous block is fixed and deleted from this round's findings. The inherited explicit token and component overrides now survive tier resolution, and new regression cases cover token, component, mirror, chain, built-base, and stale-base paths.

The prior architecture/product decision remains outside this public record, as in round 1.

Problem

A theme author can now vary a theme by viewport width without swapping themes or hand-writing parallel media queries. No shipped theme opts in yet, so the end-user impact begins only when a theme adopts tiers.

Solution

The PR adds four named width tiers, sparse tier partitioning, tier inheritance, pointer refinements, serialized generative axes for built-theme inheritance, and public tier helpers/types. Runtime and built paths use the same tier generator.

Impact

Existing tier-less theme CSS remains unchanged.
Opted-in themes change token/component values at viewport-width boundaries, not container boundaries.
Every built theme module grows by 780 raw bytes / 141 gzip bytes so future tiered children can inherit its axes.

API

DefineThemeInput.mobile/tablet/desktop/wide?: ThemeTier
ThemeTier.maxWidth?: number
ThemeTier.extends?: WidthTier
Nested '@media (pointer: coarse)' | '@media (pointer: fine)'
Public tier generator, constants, and input types from the core theme barrel

No component props or theme targets change.

Theming

One new correctness block remains. A token explicitly pinned to the value its axis already generates is treated as generated rather than explicit. When a tier regenerates that axis to a new value, the pin is lost.

At a 14px base, explicit --font-size-base: 0.875rem, and a mobile tier with base 16, the emitted tier CSS changes the explicit pin to 1rem. A distinct 2rem pin survives. This was confirmed in the resolver output, generated CSS, and Chromium at 600px with an 800px control.

Breaking

API: no; the surface is additive.
Visual: no existing rendered theme changes because no shipped theme declares a tier.
Theme: existing tier-less themes keep their CSS; the new tier capability mishandles the equal-value explicit-pin case above.

Performance and resources

No effects, listeners, observers, layout reads, or runtime JavaScript width checks were added. The browser handles the media queries. The measured built-theme module delta is +780 raw / +141 gzip bytes for every theme, including themes without tiers.

Visual evidence

Manual Chromium frames were required because the acceptance claim is rendered width-dependent behavior and no build/test/visual workflow ran on this conflicting head.

756px → mobile tier value; 757px → next partition value.
Equal-value explicit pin: 14px outside tiers at 800px, incorrectly regenerated to 16px inside the mobile tier at 600px.
Distinct 2rem pin remains 32px in both controls.

Receipt table

Sensor	Boundary 756	Boundary 757	Pin 600	Pin 800
Build	`f3e521e068f597e80222a94dac949c00cc0445f0`	same	same	same
Receipt	`JSON`	`JSON`	`JSON`	`JSON`
Viewport / DPR	756×300 @1	757×300 @1	600×220 @1	800×220 @1
Theme / mode / direction	`tiered` / light / LTR	same	`pin-differs` / light / LTR	same
Media	forced-colors off; reduced-motion off; fine pointer; hover	same	same	same
Targets	1	1	2	2
State	accent red	accent green	distinct pin 32px; equal-value pin 16px; pin kept false	distinct pin 32px; equal-value pin 14px; pin kept true
Settled / errors	fonts loaded; 0 animations; no page error	same	same	same
Exact arm diff	`empty` — same head, viewport only	same pair	`empty` — same head, viewport only	same pair

Every capture was SHA-pinned and recorded viewport, DPR, theme, color mode, direction, media, target count, semantic state, geometry, font load, and page errors. The complete portable receipt and image hashes are in the asset README.

Boundary pair

756px — mobile tier	757px — next partition

Explicit-pin pair

600px — inside mobile tier	800px — outside tiers, control

A11y and i18n

No role, ARIA, focus, keyboard, user-facing string, catalog, or direction-sensitive CSS path is reachable. No accessibility baseline changes.

Integration consequence

The branch conflicts with current main in packages/core/src/theme/generateThemeRules.ts and packages/cli/api/theme/build/build.mjs. Three hunks are mechanical additions; one is semantic.

Main's #5566 / #5562 path detects light-dark() from the theme's own values so data-token defaults do not force color-scheme on every theme. This branch detects it from generated tier CSS so tier-only tuples still get color-scheme. The rebase must preserve both properties; taking either side verbatim is wrong. The existing tier-only guard test is the acceptance check.

Anchors opened at f3e521e0: build.mjs:1251 is const needsColorScheme =; theme/index.ts:52 is DEFAULT_TIER_MAX_WIDTH,; build-theme.tiers.test.mjs:175 names the tier-only light-dark() guard; theme.template.ts:358 is typography: {scale: {base: 16}},.

Evidence and tests

Core build: passed.
TypeScript --noEmit: passed.
Focused theme/CLI suite: 873 tests across 20 files passed, including all 13 CLI tier tests. Exact repo-root invocation: npx --yes pnpm@11.10.0 exec vitest run packages/core/src/theme packages/cli/clients/cli/commands/build-theme.tiers.test.mjs 2>&1 | tail -32.
Changesets: [feat] / patch for both packages; no breaking marker.
Exact head remained pinned and the worktree was clean after measurement.
GitHub reports CONFLICTING / DIRTY; only two pull_request_target workflows ran on this head, so no build, test, lint, or visual CI job has validated it.

Judgement

Request changes. Two findings block this head:

An equal-value explicit token pin is silently discarded when a tier regenerates the axis (packages/core/src/theme/themeTiers.ts:514).
The branch does not merge; the light-dark() conflict requires a semantic resolution (packages/cli/api/theme/build/build.mjs:1251).

The author can proceed without waiting: add the equal-value pin regression, preserve explicit-pin precedence, and resolve the merge so the color-scheme predicate reads theme-owned values plus tier-owned values without counting generated data-token defaults.

The review, drafted — not posted

Thanks — the inherited-override fix is right, and deriving it from the axes rather than carrying declarations is the right call.

It doesn't merge, and so CI has run nothing on this head: no build, no tests, no lint. The light-dark() guard is the one conflict that isn't mechanical — #5566 moved it off the generated CSS, yours widened it to read the tier CSS, and merged it needs both the theme's own values and each tier layer's. Your test at build-theme.tiers.test.mjs:175 is the check.

The second is the shape you just fixed. Deriving pinned-ness from values can't see a pin equal to what the axis generates, so it loses inside a tier: '--font-size-base': '0.875rem' on a 14/1.2 scale emits 1rem in mobile; 2rem holds. themeTiers.ts:478 calls that harmless — true at the theme level, not in a tier.

Is there a cheap way to keep that case without the 10.5 KB?

Full review

[Reviewed by Robohands]

Prose: 149 words (request-changes cap: 150).

Proposed inlines:

packages/core/src/theme/themeTiers.ts:478 — A pin equal to its generated value isn't harmless in a tier: the tier regenerates it and the pin loses.
packages/cli/assets/theme.template.ts:358 — typography: {scale: {base: 16}} lifts the whole ladder; inputs already floor at 1rem on coarse pointers.
packages/core/src/theme/index.ts:52 — DEFAULT_TIER_MAX_WIDTH, tierWidthQuery and ThemeGenerativeAxes have no caller. Might be worth keeping these internal.

Critic

Seven critic passes ran. Gate 4's first pass fixed the causal anchor, added frames for the new rendered block, and added independent confirmations. Later blind passes removed an unmeasured page-level consequence and unsupported test counts, added exact source-line and command receipts, and clarified the condensed API excerpt. Gate 6's final pass returned clean.

What changed before posting

Nothing was posted to the PR. This run made no GitHub review, comment, code push, merge, auto-merge, or workflow approval. The four decisive frames, portable sensor receipts, and exact empty arm diffs were published only to this fork-wiki record under assets/pr-5543/round-2/.

Round 3 — R1e exact-head re-review, 2026-08-27

Versions: Review Loop 1.5.0 · Component Audit Rubric 1.13 Head: ec5a4d623a16e497c210e170ed41f420a1ffff7d Public verdict: withheld; no review posted Author can proceed: no — the implementation is clean, but the separate maintainer-owned API/architecture decision remains outside this public record