Repository navigation
StyleX: collapsed stack — tokens, enforcement, story gate, pilot, and the main merge - #1191
Merged
schickling merged 46 commits intoSep 2, 2026
Merged
Conversation
Both are catalog-only: nothing declares them yet, so no generated file or lockfile entry moves. Consumers opt in per package, which is the point for the converter — it must never reach a shared component package. @stylexjs/eslint-plugin rides at 0.19.0 rather than its own latest so it stays in lockstep with the compiler pins; its dependency closure pins @stylexjs/shared 0.19.0, so a split would be silently incoherent. The converter is pinned to the published 0.1.0-alpha.1 rather than a git rev of the maintainer's fork. The fork is where the useful work is, but its build output is gitignored and it declares no `prepare` script, so a git-rev install resolves to a package with no lib/ at all and every import dies with ERR_MODULE_NOT_FOUND. That is true even with lifecycle scripts fully enabled, so it is not a policy question. alpha.1 carries gitHead a150f9e, the exact fork revision that was wanted, and ships the built output, so we get the intended code through an integrity-pinned npm version instead of an unbuildable git reference. The three @babel/* pins are load-bearing, not incidental. Babel resolves plugin names relative to the calling package, so the consumer must declare the syntax plugins even though the converter lists one of them as its own dependency; without them every invocation fails. Babel 7 because @babel/plugin-syntax-typescript@7 peers @babel/core@^7 and Babel 8 trips strictPeerDependencies.
The gate needs light and dark covered as separate Vitest projects. That
is `storybookTest({ initialGlobals })`, added in 10.5.0 (#35226) and
absent from 10.4.6 — verified by diffing the shipped plugin option types,
not by reading release notes: 10.4.6's UserOptions has no such field,
10.5.10's documents it as "define one Vitest project per theme". Without
the bump the gate cannot express the requirement at all.
10.5.10 is current stable across the whole cohort. Checked upstream
MIGRATION.md for 10.4->10.5: one entry, the deprecation of ExternalDocs
and ExternalDocsContainer, which this repo does not use. No breaking
change to absorb.
@storybook/addon-vitest peers `storybook@^10.5.10`, so the cohort has to
move together rather than one package at a time. addon-a11y is here
because parameters.a11y.test is inert unless the addon is registered, and
it defaults to warn-only.
genie/internal.ts had the framework version restated as a literal in
packageExtensions, which would have left a second @storybook/react-vite
resolving after this bump. It now derives from the catalog so it cannot
drift again. The unplugin duplicate exception was re-checked rather than
assumed: @storybook/csf-plugin@10.5.10 still declares unplugin ^2.3.5, so
the exception is still required. Its reason now names the constraint
instead of a consumer package, because the consumer is moving.
The pnpm-lock regeneration invalidated eight pnpm-deps FODs; those hashes
are the measured values, refreshed in one batch.
Proof: storybook:build green for one DOM package
(effect-schema-form-aria, 39 stories) and one TUI package (tui-stories,
11 stories), both resolving 10.5.10, since the two share a factory but
not a code path. check:quick green.
…ection Two things were tangled behind one name. `@overeng/stylex-preset` shipped browser-side design tokens *and* a Node-side bundler plugin, and the plugin was what dragged the package into 18 workspace closures while only two packages actually consumed a token. Renaming it to `@overeng/stylex-tokens` says what it durably is — the Tailwind framing is gone with Tailwind — and moving the plugin into `@overeng/utils` beside the Storybook and Playwright factories drops it out of 16 of those closures rather than merely relocating it, because utils needed the token package only to wire the story factory. The integration is rewritten rather than moved. Upstream decides which emitted CSS asset receives the compiled rules by matching filenames: a caller predicate, then an unhashed `index.css`, then `style.css`, then — silently, at exit code zero — the first CSS asset in the bundle. A predicate that matches nothing degrades to exactly the fallback it was meant to prevent, and on a route-chunked build the first CSS asset can be a lazily-loaded chunk, which strands every rule while the build succeeds. That is not a bug to configure around; it is the wrong seam. Every peer compile-to-CSS plugin routes generated CSS through the module graph instead, and StyleX itself already does so in dev. So compiled CSS now enters as a virtual CSS module that each named entry imports, and the bundler owns placement, hashing, minification and manifest membership. The eager guarantee reduces to "the entry imports it", which a build can check. Content is not known until every module has been transformed, so the module loads a placeholder and its real content is swapped in from `renderChunk` via the `vite:css-post` transform handler — the UnoCSS technique. Per-module virtual CSS (the parcel-macros technique) was rejected: StyleX atomic rules must be globally sorted by priority and de-duplicated, which only one collected blob can express. Going through the pipeline also picks up minification, which the appended-asset path bypassed. The import is appended rather than prepended to the entry, so unlayered StyleX rules sort after the entry's own stylesheets — the precedence the migration relies on while a utility framework is still present. The entry stays checked JavaScript for the reason #1167 established: Vite loads config through Node, which refuses TypeScript stripping under `node_modules`. Also drops the Storybook factory's `stylex` flag. It was redundant wherever an app's Vite config already registers the plugin, and measurably harmless rather than a defect, so it goes as noise. Compiled class names move. A `defineConsts` value inlines its literal, but the atomic class hash still incorporates the constant's package-qualified identity, so renaming the package rehashes every constant-valued declaration while leaving the declaration itself byte-identical. Emitted rules are unchanged as a multiset; the markup baselines were regenerated for the names.
BLOCKED, and deliberately committed red: this does not type-check until `genie/external.ts` gains the five catalog pins `CatalogPins` owns — @storybook/addon-vitest 10.5.10, @storybook/addon-a11y 10.5.10, @vitest/browser 4.1.9, @vitest/browser-playwright 4.1.9, playwright 1.61.0 — and those pins are declared in `packages/@overeng/utils/package.json.genie.ts`. Nothing else is missing; see the PR description for the exact remaining wiring. Three settings here are non-default, and each ecosystem default fails silently, which is why the gate is a shared helper rather than per-package config: The comparator does not compare exactly even with the allowed mismatch at zero. pixelmatch defaults to a perceptual threshold of 0.1 and to ignoring anti-aliased pixels — verified by reading the shipped defaults. A one-pixel radius change with a six-of-255 colour shift passes under those defaults. So the project pins threshold 0 with antialiasing included. Per-story accessibility is a no-op by default: `parameters.a11y.test` defaults to 'todo', which the addon maps to a warning rather than a failure. The gate raises it to 'error'. That parameter is also inert unless `@storybook/addon-a11y` is registered, so `createDomStorybookConfig` grew an `a11y` flag rather than leaving a second thing to remember. Baselines are derived from a git ref and never committed. Captures depend on the host's installed fonts wherever a generic family is used, which moves one to three thousand pixels per story — one to two orders of magnitude more than a real regression — so a committed baseline is only valid on the machine that made it. `runStoryGate` materializes a worktree at the ref, borrows the installed dependencies when the lockfile has not moved, captures there, then compares on the same host. Applying the annotations is done from `beforeAll` rather than at module scope because `setProjectAnnotations` replaces rather than merges and the addon calls it from its own setup file; setup-file ordering is decided by Vite's config merge and is not ours to control, but every setup module is evaluated before any hook runs, so a `beforeAll` is deterministically last. Story add/remove is the one failure the test results cannot show, because a story that no longer exists produces no test. `resolveScreenshotPath` is called once per assertion, so the run records the baselines the current tree asked for and the runner reports any baseline nobody claimed. Dimension mismatch turned out to be already loud in this stack — the comparator refuses to compare differently-sized images and says so — unlike the difference composite in the earlier prototype, which under-reported silently.
Declares the gate stack, exports `@overeng/utils/node/storybook/gate` and its browser-side setup entry, and gives the already-migrated Aria package a `vitest.gate.config.ts` so the gate has something real to run against. The five gate packages are devDependencies of `@overeng/utils`, not dependencies, for the same reason `storybook` and `@storybook/react-vite` already are: anything that runs the gate necessarily owns its own Storybook install, and making them real dependencies would put Storybook back in the closure of everything that depends on utils — the exact shape this branch just spent a commit removing. `a11y: true` on the Aria Storybook config is load-bearing rather than cosmetic: `parameters.a11y.test` is inert unless `@storybook/addon-a11y` is registered, so without it the gate's accessibility check passes everything silently.
Vitest treats `vitest.gate.config.ts` as the Vite config for the run, so the package's `vite.config.ts` is never loaded — unlike a Storybook build, where the builder merges it. Without the compiler transform every story throws `Unexpected 'stylex.keyframes' call at runtime` before a single assertion runs.
A baseline capture that exits non-zero is not a broken capture. Under --update the screenshots are written regardless, and the reference ref can legitimately carry failing stories: this package has real accessibility violations today, and aborting on them would make the gate unusable on exactly the packages that need it most. So the capture is kept when it produced a report, its failures are recorded, and the compare phase subtracts them. What the gate answers is 'did this change make it worse', which is what R08 asks for. Pre-existing failures are still reported, under their own key, so subtracting them cannot quietly become ignoring them.
The baseline half of a run happens inside a git worktree that borrows the main tree's node_modules by symlink, so every workspace source — including the gate's own setup file — resolves to a path outside the served root and Vite refuses it. The whole run failed at import with nothing but "Failed to fetch dynamically imported module". Strict fs roots buy nothing on a throwaway test server and cost the derived-baseline mechanism entirely. Also ignores `.vitest-attachments`. Those are the diff and actual images written when a screenshot assertion fails: evidence for one run, never an artifact. With baselines derived from a git ref into node_modules/.cache, nothing the gate produces is committable, which is the point of R09.
Eight fixed-output derivations take the root lockfile as an input, and renaming the token package plus adding the gate stack to @overeng/utils moved it. Seven are CLI-shaped and were measured by `evergreen fod refresh --linux-system x86_64-linux`; `nix/oxc-config-plugin.nix` is hand-written, so evergreen cannot reach it and its value comes from the `got:` line of a local `nix build .#oxlint-npm`, then re-verified by building again clean. Note for whoever merges this alongside #1172: that eighth hash is contested by construction, not by accident. Its inputs include the root lockfile, so it moves independently for any branch that touches dependencies — #1172 adds a devDep to the very package it covers. Both values are correct for their own tree and neither is correct for the merged one. Resolve the conflict by re-measuring on the merged tree, never by picking the surviving side.
Mechanical: `!existsSync(x)` -> `existsSync(x) === false` and friends across the new gate runner, the StyleX plugin factory and the Storybook factory. No behaviour change; `check:quick` is green and the gate still reports zero on an unchanged tree.
…eze motion Two defects found from the consumer side, both in the shared layer. The gate subtracts failures present at both refs so it reports regressions rather than absolute health — necessary, or it is unusable on any package carrying drift. But with no floor the degenerate case inverts it: when every story fails at the baseline they all land in `preExisting`, the regression list is empty by construction, and the gate reports success over a total loss of styling. Measured at 212/212 failed on one app and 708/942 on a component library. The proximate line was ordering — the pre-existing skip ran before the missing-reference branch, so a story with no baseline image had its compare-side failure swallowed as debt. Coverage is now derived from the filesystem rather than inferred from message text: a baseline the compare run asked for that does not exist is `uncovered` and fails, and cannot be absorbed into `preExisting` by any wording. Zero passes at the baseline is a hard failure. Deliberately no fraction threshold — a quarter passing is degraded but still informative, and a threshold set wrong rejects that along with the genuinely broken case, so the machine refuses only the unambiguous case and the counts go in front of a human. The verdict is extracted as `isStoryGateOk` and unit-tested against both measured shapes, so the guard is provable without a capture cycle. The stability timeouts underneath were transitions firing once on mount, not continuous motion: every Button variant failed while Avatar and Badge passed, and Button at rest has only `transition-colors duration-150`. An A/B with the accessibility addon off changed nothing, which rules out axe triggering them, so disabling transitions before first paint is sufficient and not merely necessary. A freeze cannot reach canvas or JS-driven animation, so those stories declare `parameters.storyGate.unstable` and are reported as excluded — otherwise they fail at baseline and quietly poison the same subtraction this commit just fixed. Also gives the gate a runnable entry point. It was a library function with no caller, so every target had to write its own driver, and a check nobody can run is not the working visual check R08 asks for.
The wrapper existed to make `@overeng/oxc-config` resolve, but it wrote the whole `jsPlugins` list, so any plugin declared beside ours became unresolvable. That is why two consumers already carry local workarounds with comments describing the clobbering. Substituting only our entry point makes adopting a third-party plugin — `@stylexjs/eslint-plugin` is the immediate one — possible at all. The match is the entry point, not the package directory, because we now ship a second plugin entry from the same directory and swapping that for ours would reintroduce the same silent failure one level down. Two more defects fixed in the same pass, both of which cost real debugging time. `((i++))` evaluates to the OLD value of `i`, so it exits 1 when `i` is 0, and `writeShellApplication`'s `set -o errexit` turned that into a silent abort: with `--config` as the first argument the wrapper exited 1 and printed nothing, which in a pipeline is indistinguishable from a lint failure. And because the injected plugin is a Nix build-time snapshot, editing a rule had no effect and adding one reported "not found in plugin 'overeng'" — rule development was impossible through the wrapper. `OVERENG_OXC_CONFIG_PLUGIN` now overrides the path, so pointing it at the plugin's TypeScript entry lints against live source (the host runtime is Bun, which imports .ts directly). The regression test drives the real built wrapper against the real oxlint rather than a stub, because all three were failures of plugin RESOLUTION, which a stub cannot model. It prefers the already-realised wrapper on PATH so it costs nothing inside the devenv shell.
Our token architecture makes raw colours available and semantic tokens merely conventional, and a raw scale step is an inlined constant: a component reading one compiles cleanly and silently ignores dark mode. Types cannot close that gap, because they constrain what callers pass in, not what an author writes inside their own style object. So it has to be a failing check. Both layers ship because they are provably complementary, not redundant. Upstream per-property value limits are strictly better than anything we would hand-roll for pure colour properties — they distinguish a literal from a token reference — but a limit is keyed on one property and structurally cannot see a colour embedded in a composite value. Measured: upstream reports nothing about the colour inside `boxShadow: '0 10px 15px -3px rgb(0 0 0 / 0.1)'`, which the first-party rule catches. Banning composite properties outright is not viable; they need literal offsets. The overlap on plain colour properties is deliberate, because an array `limit` drops its custom `reason` and only our message names the remedy. Upstream loads through a first-party namespace shim rather than the bare package specifier. Upstream ships no `meta.name`, so oxlint would derive the namespace from the specifier — and a bare specifier only resolves from the ROOT `node_modules`, which this repo's root manifest cannot own because it is a pure genie workspace aggregate. The shim supplies `meta.name` explicitly, so the dependency sits on the package that owns lint config and rule names still read exactly as upstream documents them. Colour properties are enumerated rather than globbed: measured, `*olor*` also matches `colorScheme`, `colorAdjust`, `forcedColorAdjust`, `printColorAdjust` and `colorInterpolation`, which take keywords and would be wrongly banned. The second rule partitions the focus ring. StyleX orders conditions by kind, not by authoring position, so a selected-state shadow beat a focus-visible shadow and an element that was both selected and keyboard-focused silently lost its focus ring. Reserving `outline*` for focus-visible makes that collision impossible rather than managed, and it is the half of the invariant a linter can see. `stylex-no-raw-color` treats `color-mix()` and relative-colour syntax as derivations, not constructors: a derivation over a token still follows the colour scheme, so flagging it would contradict the rule's own purpose. Raw arguments inside them are still caught, because the patterns are not anchored. These rules use TSESTree types instead of the `any` the sixteen older rules use. The no-any rule is binding and postdates that convention, and `@typescript-eslint/utils` is already a devDependency, so the type-only import is erased before bundling. The older rules are left alone. The pilot package fails the enforcement derived from it, in 17 places: 13 sites still using the deprecated top-level pseudo-class syntax, and 4 raw colours. That is the rules working — they found real drift in the one place everyone would have assumed was clean. Migrating it is out of scope here: nesting a pseudo-class changes which condition wins, and that package has no visual gate, so changing it now would be exactly the unverified visual change our requirements forbid; the colours need semantic tokens that do not exist yet. Each site therefore carries a per-line disable with a reason instead of a file-scoped override, so new code there stays gated. Tracked in #1171 alongside that package's existing open divergence, which needs the same visual gate.
…honestly
Drops `caret-color` and `scroll-behavior` from the injected rule. Caret blink is
already handled by the matcher's own `caret: 'hide'` screenshot option and the
scroll rule was speculative; an unevidenced `!important` against every element
is a liability rather than insurance. Animations are made to finish rather than
pause, because pausing freezes each one at whatever frame it had reached when
the style applied, which depends on mount timing.
Neither change fixes the flake, and the flake is real. Measured on
effect-schema-form-aria, 39 stories, both sides captured under the freeze in one
invocation:
pre-freeze 0 changed x3 runs, plus an independent control at the
pre-freeze commit that also saw 0 and no timeouts
freeze, pause 1, 3, 1
freeze, finish 1, 0, 0, 1
freeze, finish+narrow 2, 0, 0, 0
So the freeze intermittently produces one to three spurious diffs on a package
that has no animations at all and only 150ms transitions. Cause unresolved.
Shipping it anyway, deliberately, because the asymmetry matters: this is a false
ALARM, not a false pass. It never lets a regression through, and the two stacks
blocked on the freeze are blocked by stability timeouts that make their gate
report nothing at all. A gate that occasionally cries wolf is strictly better
than one that cannot see. It is not yet good enough to gate a merge unattended,
and the PR says so rather than implying the gate is clean.
Adding `@stylexjs/eslint-plugin` to `@overeng/oxc-config` moves the root lockfile, which is an input to every pnpm-deps fixed-output derivation — so three hashes had to be re-measured, not just the one covering that package. `nix/oxc-config-plugin.nix`'s hash is hand-written; `evergreen fod refresh` cannot address it, so the value is the one `nix build` reported. The other two were refreshed by evergreen (it writes the correct value even though its post-write verification step fails to evaluate these attrs). That hash is contested by construction rather than by accident: its inputs include the root lockfile, so it moves for any branch that touches dependencies. The value here is correct for THIS tree and will not be correct for the merged tree. Whoever merges second must re-measure rather than keep either side.
… findings Three separate checks each found real drift in this package, none of them looking for it: the state-idiom review behind decision 0004, the StyleX lint rules the package's own patterns produced, and the first run of the visual and accessibility gate. All three were deferred on the same prerequisite — a working visual gate for this package — which the gate now supplies, so they are settled together rather than three times over. Raw colours become semantic tokens. `on-primary` is the foreground for anything drawn on a `primary` background; `shadow-raised` is the popover elevation and takes its default from the `shadows.lg` scale step rather than restating the rgba stack. Both defaults reference a named scale value, so the palette choice stays in one reviewable file. `primary` moves from `blue500` to `blue600`, and that is a legibility fact rather than a preference. The accessibility gate failed ten of this package's thirty-nine stories on exactly one thing: white body text on the selected segment measured 3.76:1 where AA requires 4.5:1. `blue600` measures 5.25:1. Renaming the literal to `on-primary` would have preserved the violation behind a nicer name. `accent` follows `primary` so the brand blue stays one colour. The thirteen deprecated top-level pseudo-class sites move into condition objects. This is the change the deferral was actually about, because nesting a pseudo-class changes which condition wins, so every site was checked for a competing state on the same property rather than translated mechanically. Eleven of the thirteen set a property nothing else touches and carry no precedence risk at all. The two that do compete — the segmented control's and the list option's background under hover versus selection — are resolved by application order instead: hover lives in the base style, selection is a later `stylex.props` argument, and it restates the hover value because a later unconditional `backgroundColor` does not replace an earlier `backgroundColor` under a `:hover` key. That is the R06 shape, and it means the outcome no longer depends on attribute conditions outranking pseudo-class ones. Focus moves from `:focus` to the accessible-component library's own focus-visible state where the element is one of its components, and to the native `:focus-visible` on the one plain `<input>` that is not. A pointer click no longer paints a keyboard focus ring. Note what this is NOT: the ring is still drawn with `boxShadow` rather than converted to `outline`. The partitioning invariant already holds here — nothing else on these elements sets `outline*`, and the lint rule confirms it — and no story exercises a focused element, so repainting the ring would be a visual change the gate structurally cannot adjudicate. Recorded as follow-up rather than done blind. State resolution stops re-deriving what the component already knows. The checkbox derived its box style from its own `value` prop; selection lives on the CheckboxButton, which is an ancestor of the box, so the box cannot read it as one of its own conditions and React Aria's render prop is the sanctioned mechanism for that case. The segmented control and the list options branched between two mutually exclusive style objects, hover rule included; they now apply one additive override in argument order. The accessibility gate's eleventh failure was structural, not colour. React Aria's `Header` renders `<header>`, and a `<header>` outside sectioning content is a `banner` landmark, so two nested field groups produced two banners and tripped `landmark-no-duplicate-banner` and `landmark-unique`. The label is now a plain element and the group carries the accessible name it was missing. The seventeen per-line `oxlint-disable` suppressions are gone; the package is gated by all eight rules again.
…s render `@overeng/utils` reaches most gate consumers as a `link:` into a sibling megarepo checkout that carries its own `node_modules`, and Node resolves from a link's real path. So `@storybook/react-dom-shim` — the thing that renders every story — resolved `react`/`react-dom` inside the effect-utils tree while the consuming package's stories and its `react-aria-components` resolved them inside theirs. Two React copies, same version, one optimise pass, different absolute files. The renderer installs the hook dispatcher on its own copy's internals, so any component reaching a hook through the other copy read a null dispatcher and threw, React unwound and retried, the DOM never settled, and the screenshot matcher reported a stability timeout naming none of the above. Measured on one consumer's `Button.stories`: 40 `Invalid hook call` warnings, 20 null-dispatcher errors, 10/10 stories failed in 67.6s. `resolve.dedupe` does not fix this — it picks one copy of one package, but esbuild shares a chunk per module and these are two files on disk. `optimizeDeps.include`/`exclude` move modules between graphs without changing which file `react` resolves to. Only a resolution-level override collapses them. So the gate now injects a `resolve.alias` that pins every React specifier to the CONSUMING package's copy, resolved from Vite's own project root rather than from this module's location — resolving from `import.meta.url` here would pin every consumer to the effect-utils copy, i.e. the wrong side of the duplicate. For a consumer that lives inside the effect-utils tree the two resolutions are the same file, so it degrades to an identity mapping. Every `find` is anchored: a bare `'react'` is a prefix match and would swallow `react-dom` and `react-aria-components`. The five measured entry points map to resolved files; two anchored subpath rules follow them so specifiers nobody enumerated — `react-dom/test-utils` is already live in one prebundle — still land in the consuming tree by construction.
…lind `threshold: 0` failed on anti-aliasing rather than on regressions. A border at a fractional y renders as ~70% coverage on one row with the remainder bleeding into the next, and the split is not stable across capture sessions. Measured on a captured artifact: identical dimensions, one 1px-tall row, 689 pixels, each off by 1-2/255 against white. The tolerance looked dangerous and is not, which is the part worth recording. The noise is an INTENSITY delta; the smallest regression the gate must catch is a STRUCTURAL one across a whole edge. Swept against the real comparator: threshold AA fringe 1px border shift 0 689 1868 0.005 689 1868 0.0075 0 1868 0.02 0 ~1868 0.05 0 1860 0.1 0 0 0.02 sits ~2.7x above where the noise dies and ~5x below where a 1px shift starts eroding at all. `allowedMismatchedPixels` stays 0 deliberately. Absorbing the fringe with a pixel-count budget instead would have cost the property that a single structurally-different pixel still fails; keeping the count at 0 preserves it. `includeAA: true` is unchanged.
… them Unlayered output was never a preference. It is what lets converted code beat a utility framework's layered utilities without any ordering work, which is the property a migrating target depends on. So `useCSSLayers` is a per-target option that defaults OFF, and this turns it on for `effect-schema-form-aria` only. Nothing outside this repo changes, and the other repos must not take this flip while they still carry Tailwind. The audit, in full, because it did not come back the way "effect-utils is Tailwind-free" suggests. No package in the repo declares a utility framework. Exactly one package uses StyleX, plus the token package itself. Five CSS files exist: three belong to `notion-react`, which does not use StyleX and never shares a document with it here; one is a re-export; and the fifth is `@overeng/stylex-tokens/preflight.css`. That fifth file is the finding. It was unlayered and it sets `box-sizing`, `margin`, `padding` and `border` on `*`. Layered CSS loses to ANY unlayered CSS, so flipping layers on without touching it hands those four properties to the reset on every component in the package — the exact silent regression the migration exists to prevent, arriving through the change meant to clean the cascade up. Measured rather than argued. With the reset left unlayered, the gate fails sixteen stories with DIMENSION mismatches — every component loses its padding and borders and collapses. With the reset declared in `overeng.reset` and named in the compiler's `before` list, the gate is 39 compared, 39/39 passed at baseline, 0 changed: the layered output renders identically to the unlayered output it replaces. The ordering is fixed by declaration, not by luck. Naming the reset in `before` makes the compiler emit `@layer overeng.reset, priority1, ...;` ahead of its rules, so the reset sits below every StyleX priority regardless of which stylesheet the browser parses first — the same rule the token layer already follows, and the reason this is not another bundler-injection-order dependency.
…aried A Storybook theme toolbar global was keyed on `:root[data-theme=…]` while the decorator set the attribute on its own wrapper div. Both gate theme projects therefore captured the same light palette, compared them, and reported green double-coverage. Measured: light and dark both computed rgb(255,255,255). This is a different failure from the four before it. Those were a pass signal defined as the absence of a failure marker. This one is a matrix dimension that does not vary being indistinguishable from one that does — the gate ran, both projects produced captures, every comparison succeeded, and the theme axis was imaginary the whole time. No care at the call site detects that; only a count does. So the runner now counts, over the baseline captures, how many stories render differently across theme projects, and refuses to pass at zero. Three details, each of which the naive form gets wrong: - The count is taken over the COMPARABLE set only. The baseline tree is now captured twice and the second capture is kept; any story whose two captures of one unchanged tree disagree is excluded and named in `selfInconsistent`. Without that, a nondeterministic story differs across projects for the wrong reason and satisfies the assertion vacuously. Measured shapes: 9 of 70 and 17 of 212 captures on two surfaces, both traced to a single element appearing in one capture and not the next. - A count, not a boolean. A boolean still passes at one differing story, so it misses a real axis that a change silently flattened; the number makes that visible and is free, since both captures already exist. - `themeVaries: false` (`--single-scheme`) is how a target that legitimately ships one colour scheme declares itself. Measured counterexample: a site pinning one scheme has 0 of 57 probes differing, correctly. Failing it would punish a correct target for a property of the target, and the guard would get switched off. The second baseline capture also removes a cold/warm asymmetry the design built in: the baseline was captured in a freshly derived worktree while the compare side ran warm, and the first capture into a cold cache was measured to differ from every later one deterministically, reproducing to the pixel at 689 and 693 pixels on two stories. Keeping the second capture makes both sides warm, so the comparator's tolerance is spent on real host variance instead of on an artefact we introduced. What this does NOT establish, and the comment says so: that the axis varies CORRECTLY. The count is unchanged if light and dark are swapped wholesale. Comparing rendered values against expected ones is a separate check.
`animations.*` were `animation` SHORTHAND strings. Measured against
@stylexjs/babel-plugin@0.19.0, compiling
stylex.create({ loader: { animation: `${spin} 1s linear infinite` } })
emits the @Keyframes rule and then `{ loader: { $$css: true } }` — an empty
style object. No `animation` declaration, no `animation-name`, no warning, no
error. tsc is happy, the linter is happy, review is happy, and the animation
never runs. A pulse in one consuming app had silently stopped and only a
computed-style diff caught it.
Replaced with four longhand constant groups. The same input compiles to
animation-name, animation-duration, animation-timing-function and
animation-iteration-count, with the keyframes name resolved — verified by
compiling this file through the plugin and reading the emitted constants.
The shorthand export is REMOVED rather than deprecated, because removal is the
only thing that turns this into a type error: `animations.spin` no longer
resolves, so every call site fails tsc instead of silently rendering nothing.
There is no way to make a value of type `string` reject a shorthand at the
StyleX API boundary. No in-repo callers existed.
`bounce` carried no timing function upstream and keeps the CSS initial `ease`
rather than acquiring one here.
The gate's readiness signal was "two consecutive frames are identical", which is a PROXY for "the DOM has settled" and fails in both directions: it accidentally passes on a tree that is merely quiet between two async arrivals, and it accidentally works when an unrelated delay happens to straddle a transition. Replaced with a signal that measures the thing the proxy stood for. Measured cause, on an application whose harness waited a fixed 400ms. A maplibre AttributionControl mounts hidden and empty, then populates from source metadata on its `sourcedata` event: the links appeared at +1873ms, +8573ms, and NOT WITHIN 3300ms across three runs of one story. The same mechanism produced a marker div arriving at ~4s elsewhere in the same app. The result was elements present in one capture and absent in the next -- nine stories differing from themselves, a structural difference no comparator tolerance can or should absorb. The predicate polls the captured subtree's shape -- element count plus markup length -- until it repeats three times running, bounded at 20s. Deliberately NOT computed-style or pixel based: it decides *when* to read, so if it depended on *what* is read, the readiness check and the measurement could agree with each other while both were wrong. Rooted at `canvasElement`, which is exactly what `toMatchScreenshot` captures, so the predicate and the capture cannot disagree about scope. Recording the outcome is the load-bearing half, more than the waiting. Every story emits a record: settled with its cost, or never-settled with a reason and the observed shape history. A story that never settles is excluded from the comparison, NAMED, and counted -- an exclusion rather than a disappearance. It returns without asserting, so it resolves no baseline path and enters no manifest, which is what keeps it out of `preExisting` where a baseline-side failure would silently subtract its own compare-side failure. Declared and observed exclusions stay distinguishable. `excluded` is `parameters.storyGate.unstable`, a reviewed decision recorded in source, and passes. `unsettled` is one the harness discovered and nobody reviewed, and fails -- because passing would report green over a story the gate did not compare, which is the defect its other guards exist to prevent. The remedy is visible: fix the story, or declare it and put the decision on the record. Three further things this had to fix to work at all: `--reporter json` ALONE was discarding the channel. It replaces the console reporter, and the JSON report has no field for a browser-side `console.info` -- so the story-gate markers, the only channel carrying "why this story was not compared", never reached the runner. Now both reporters run and the JSON one is named in `--outputFile.json`. Capture liveness is asserted BEFORE any pass/fail number, with a hard "NOTHING SETTLED" when the settled count is zero. The signature that has burned this project repeatedly is zero-work-plus-zero-errors: a harness that never launches a browser emits no failures and reads as perfect. The settled count is emitted positively, per story, by the browser itself, so it cannot be faked by silence. Tree identity is recorded and asserted across each capture set. If source changes land mid-pair, HMR recompiles and BOTH captures span both trees, so they agree with each other while both are wrong -- a self-consistency check is structurally blind to contamination common to both samples. Scoped to tracked content plus untracked files under the package's source and story roots, because hashing every untracked file would refuse runs over a scratch script and a guard that cries wolf gets switched off. The refusal prints what it hashed and which entry moved. Comparator threshold stays at 0.02, untouched. This is additive: sub-pixel intensity noise dies by 0.0075 while a one-pixel border shift still scores ~1860 mismatches at 0.05, and widening the tolerance on the strength of this change would undo a measured result. Also states in the report what each verdict rests on: one invocation captures the baseline tree twice and the compare tree ONCE, so a run cannot speak to the compare side's self-consistency. Separating capture from comparison is the real repair for that and is deliberately left alone here -- one semantic change at a time is what keeps a measurement attributable. The settle-signal-as-primitive idea is SchicklingDevStack's; PtgRest built and measured it (212/212 captures settled, 0 errored, against nine self-inconsistent stories plus capture timeouts before); this commit lifts it into the shared layer.
…putes `selfInconsistent` was computed by the baseline probe, printed in the summary, and then used in exactly one place: the theme-variation count. It was never subtracted from `changed`, which subtracted only `preExisting`. So the gate identified the nondeterministic stories, reported how many there were, and then reported them as regressions anyway. Measured on an IDENTICAL tree with nothing changed: 21 stories reported as `changed`, of which 19 were already named self-inconsistent by the gate's own probe. Every conversion PR on every target therefore opened with a changed-list that was almost entirely noise the machinery already knew how to filter. The reason it was never wired is that the two sides speak different namespaces, and joining them wrongly is worse than not joining them. A capture key ends in the story ID (`.../NumberField.stories.tsx/components-numberfield--with-hint.png`) while a Vitest assertion carries the BARE story name (`With Hint`) -- measured, not assumed. Keying on the name alone would conflate every `Default` in the library, letting one file's nondeterminism silently absorb a real regression to a same-named story in another file. That is a false green, which is the failure class this gate exists to prevent, so the join is scoped by story file and matched on Storybook's own id slug. `parseAssertions` now carries each assertion's file for that purpose. Excluded stories move to their own `nondeterministic` list rather than being dropped, so the report can state how much of the difference was noise rather than quietly shrinking. A story there is not evidence of a regression and not evidence of its absence; it needs its nondeterminism fixed before the gate can say anything about it at all. Kept separate from the settle-signal commit deliberately: two semantic changes measured together are one measurement you cannot attribute. The 21-of-which-19 figure is GeistComponents'; it found the unused exclusion set while measuring the component library's baseline.
The predicate had a hole that reported as a clean result, and it was
found by reading the code rather than by measuring it -- which is the
wrong way round for the check that decides when every capture is taken.
THE HOLE. The floor is three quiet polls at 200ms, so a story settles at
~600ms. But the defect the settle signal was built for is a map
attribution control that mounts hidden and EMPTY and populates from
network metadata at a measured +1873ms, +8573ms, or not within 3300ms.
Between mount and population the DOM is perfectly quiet. So the
shape-only predicate was satisfied at its floor and captured the empty
state.
That is worse than the flake it replaced. Every capture would report as
settled AND self-consistent, because a reliably-empty capture is
genuinely deterministic: the original nondeterminism disappears by never
capturing the content at all, a real change to that content becomes
invisible, and any story whose population happened to land inside 600ms
would flip-flop exactly as before. `212/212 settled, 0 never-settled` is
fully consistent with this -- which is why it looked like success.
MEASURED, against the shipped loop, with the +1873ms figure:
- network reported idle -> settled at 600ms, shapes ['3:120'].
One shape, the empty one. It never saw the content.
- resource timeline hot until arrival -> settled at >=1873ms,
shapes ['3:120','42:2400']. Both states recorded, capture after
arrival.
THE FIX. Quiet is accepted only once nothing known-pending remains: no
network activity within `workQuietMs` (600ms, matched to the DOM quiet
budget so both conditions demand the same evidence) and no incomplete
images inside the captured root. `work-never-drained` is a distinct
reason from `shape-never-quiet`, because "the DOM kept moving" and "the
network never drained" have different fixes and one reason covering both
sends whoever reads it to the wrong one.
Network activity is read off the RESOURCE TIMELINE rather than by
patching `fetch`/`XMLHttpRequest`: the gate runs inside Vitest's own
browser client, and monkey-patching the transport it communicates over
risks breaking every run in order to fix a subset of them. An in-flight
entry reports `responseEnd: 0`, so `startTime` is used as the lower
bound -- the conservative reading, which also covers the window before a
request completes. Images are included because an image completing
changes pixels while leaving element count and markup length untouched,
blind in exactly the way the shape predicate is blind to a font swap.
NOT COVERED, stated in the module doc rather than left to be
rediscovered: a mutation driven by a pure timer with no network or image
activity behind it. Nothing short of the full bound catches that, and
the bound is the backstop.
The predicate moves to `gate/settle.ts` with its clock and platform
injected. That is what made the hole answerable at all: a 20-second
bound and a +1873ms arrival are now exercised in microseconds against
the real loop, instead of inferred backwards from flake counts after a
full suite. The hole existed because the loop was only reachable through
a browser.
`pollIntervalMs` is deliberately unchanged. A shorter interval makes
premature settling MORE likely, so cutting the cost while widening the
hole would be the wrong trade.
Cost of the added gate, measured on effect-schema-form-aria, 39 stories,
host load 24.6-30.8 on 32 cores: mean per-story settle 601ms -> 737ms,
total settling 23.4s -> 28.8s. 13 of 39 stories now wait beyond the
600ms floor, up to 1022ms -- those are stories where the shape predicate
saw nothing move while resources were still landing, so the extra wait
is the fix doing its job rather than overhead. Still 39/39 settled,
28/39 passed at baseline, 39 captures: no interference from reading the
resource timeline, which was the real risk.
The hole was spotted by SchicklingDevStack from the floor arithmetic
against the +1873ms measurement.
A validated detector left in a temp directory is a detector nobody runs, and one left in a docs folder is the same thing with better filing. This moves it into the shared layer with an export entry and a runnable entry point, so it can be invoked by hand or wired into CI against a production stylesheet. WHAT IT DETECTS. `@stylexjs/shared@0.19.0` spells `:focusVisible` and `:focusWithin` in camelCase in its priority table but looks them up by literal CSS text, so both fall through to the unknown-pseudo default of 40 while `:hover` sits at 130. Hover therefore outranks focus-visible -- the inverse of Tailwind, which emits focus-visible after hover at equal specificity. A conversion that looks correct silently inverts shipped behaviour, with nothing for a compiler, linter or type checker to see. WHY IT READS THE ARTEFACT. A source-level audit looks for one property object carrying both keys. That misses a pair split across two style objects or across ordered `stylex.props` arguments, which still compete as atomic rules in the cascade. Reading the emitted stylesheet also forces the right granularity: CSS has no style keys, only leaf properties, so it cannot manufacture the false collisions that matching style-key objects produces -- a real audit did exactly that and reported 12 of them. Four properties preserved from the original because each was deliberate, and one of them matters more than the detection: 1. It pairs only rules that TIE on one property. A pair at differing specificity is decided by the cascade and is not exposed, so reporting it would be noise -- and a detector that cries wolf gets switched off, which costs more than the case it was guarding. 2. IT REFUSES TO REPORT ON A ZERO PARSE, with a distinct exit code 2. An agent independently produced exactly the failure this prevents: a scan printing zero hover rules on a page carrying an entire utility framework, because `cssRules` throws on a cross-origin sheet and the loop silently continued. It printed identically to a clean result and was caught only because the number was implausible. "I did not look" and "I looked and it is clean" must not share an exit code. That refusal is worth more than the specificity model, which a reader cannot independently check. 3. It prints both fixes with the reason to prefer them, because a verdict channel that cannot carry its own cause gets bypassed. 4. It refuses to conclude the app is broken. Order-exposed means any element carrying both classes is wrong, not that such an element exists; that is a per-element question only the producers and composition sites can answer, and it says so rather than guessing. `vacuous` is kept distinct from `clean` for the same reason as the zero parse: a build with no native focus rule at all passes for a different reason than one whose ordering was checked and found sound. A test asserting otherwise was wrong and was corrected rather than the code. Rationale note, because the wrong explanation propagates further than the right code: the fixes are NOT "specificity instead of priority". The compiler materialises priority AS specificity by repeating `:not(#\#)`, so those are one axis. What makes both fixes sound is that their correctness is INVARIANT UNDER THE TABLE'S VALUES -- a compound key sums positive parts, and a `default`-condition rule is lowest by construction. Anything depending on two specific numbers being in a particular order is correct today by accident of the defect and flips back on a routine dependency bump. Scope is stated in the module doc: right for compiled atomic output, not for hand-written CSS, and on a dev page pair count is not severity because the whole utility layer and tool chrome get parsed too. Written and validated by SmallAppSurfaces.MiscSurface against two independent production builds, both of which came out order-exposed; PtgSurfaces reproduced it on a third. This commit adapts it, adds the tests, and makes it runnable.
…symlink Measured while setting up a megarepo consumer's gate run: the process exited 0, printed nothing, and did no work. One second, no output, no error, success. Cause. A megarepo materialises `repos/effect-utils` as a SYMLINK into the store, so `process.argv[1]` is the symlinked path while `import.meta.url` is the resolved one. The direct-invocation guard compared them literally, so it never matched, the module loaded, and the entry point simply did not run. This is the same silent-failure signature the gate exists to remove, sitting in the gate's own entry point: zero work reported as success. Worse than a crash, because a caller and any CI step reading the exit code cannot tell it from a clean pass. And it affects EVERY megarepo consumer, since they all reach this file through such a symlink -- which is the normal way this gate is invoked, not an edge case. Fixed by also comparing through `realpath`. Verified both directions: a symlinked invocation now prints its usage, where before it produced nothing at all.
The self-consistency probe captured the baseline tree twice and treated agreement as stability. That has measured false negatives: `Avatar > All Sizes` passed the pair check and then differed on a third capture of the identical tree, so a pair's "agreed" silently contained two populations. This is not a readiness problem and no better settle predicate reaches it. Readiness asks whether the DOM stopped changing; reproducibility asks whether the render repeats. A surface whose DOM is quiet while its compositor is not — a 3D transform, an image downscale under a rounded clip — satisfies the first and fails the second. Capture three times by default, keep the last (still warm, so the cold/warm reasoning is unchanged), and report the outcome three ways: reproduced across all three, differed on the second, differed only on the third. The third class is the one worth naming because it was invisible before. Sized honestly, at GeistComponents' insistence: this is a better detector, not an eliminator. A story alternating between two frames at ~50/50 agrees across N captures at 2^-(N-1), so three captures still miss it 25% of the time. The report prints that residual next to the count, because a probe trusted as certain while being 75% reliable is worse than one known to be partial. `--skip-third-capture` opts out for a fast local loop. The default is the thorough value and the report always states which was used; a run that silently captured less than it claims would be a fifth way this gate reports success while doing less work than it says. `differedOnThird` is reported as NOT MEASURED rather than as zero under the flag, and a cached baseline taken with fewer captures is refused rather than reused, so one fast run cannot poison later ones. Settle signal and the 0.02 comparator threshold are untouched. Both are measured and neither is what this addresses. Credit: the false negative and its ~50/50 two-frame cause were measured by GeistComponents, which also insisted the residual be stated rather than the change sold on Avatar being fixed.
…ng conversion needs Additive coverage, and a prerequisite rather than a nicety. Before this file no story in the package rendered a focused control, so the focus ring sat outside the visual gate entirely -- every other property of these components is gated, and the ring was the one thing a conversion could silently repaint with a full pass. That is the same blind spot that let a real ring defect ship unnoticed in the design system these components borrow their conventions from: 8 of 11 ring-carrying files inheriting a white offset default, painting a solid halo on a near-black backdrop, invisible for years because nothing rendered a focused control. Three properties make these captures usable as baselines: - The gate's setup freezes transitions and sets caret-color transparent, so a focused text input has no blinking caret. Without the caret rule a focused-input story could not have a stable baseline at all. - Focus is taken with userEvent.tab() rather than element.focus(). React Aria decides data-focus-visible from its own input-modality tracking, which flips to keyboard only on a key event, so a bare .focus() focuses the element and paints no ring. - Each play asserts the focus state actually landed. A story that captured an unfocused element would still produce a stable baseline and a green gate, so the assertion is what makes the capture evidence rather than a picture. Authored by a delegated agent that parked before committing; recovered and verified here. VERIFIED: in-package tsc --noEmit clean. NOT YET VERIFIED: story execution -- run reported separately.
schickling-assistant
added
origin:agent
This was referenced Sep 2, 2026
Closed
Closed
schickling
marked this pull request as ready for review
September 2, 2026 14:25
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e054106c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The collapse renamed `@overeng/stylex-preset` to `@overeng/stylex-tokens` (CHANGELOG documents it, and `genie/buck2/typescript-admissions.ts` already carries the `stylexTokens` admission), but this test still imported `packages/@overeng/stylex-preset/BUCK.genie.ts`. Bun could not resolve the module, so the whole FILE failed to load: error: Cannot find module '../../packages/@overeng/stylex-preset/BUCK.genie.ts' Measured effect of the missing module, `devenv tasks run genie:buck2:test`: before 11 pass, 1 fail, 1 error (12 tests across 3 files) after 17 pass, 0 fail (17 tests across 3 files) The test count is the point: the five extra tests are not new, they are the ones in this file that never ran because the import aborted module load. A whole test file silently not executing is exactly the pass-as-absence failure mode, and it reached CI green on `typecheck` because `keyof typeof buck2TypeScriptAdmissions` is only checked where the file compiles at all. Reproduced on a WARM tree; this is not a cold-`dist` artefact.
`oxlint-plugin-injection.test.sh` is new in this stack and gates three wrapper defects. Only bug 1's fix (the jq in-place substitution) survived the collapse. Diffing the wrapper against its own ancestor 48104c1 shows the merge took `main`'s side of the hunk wholesale and reverted the other two: - bug 2 `i=$((i + 1))` reverted to `((i++))` - bug 3 `pluginPath="${OVERENG_OXC_CONFIG_PLUGIN:-...}"` reverted to the bare Nix snapshot path, plus its header docs Both are restored here verbatim from 48104c1, re-applied on top of main's newer persistent-cache design (flock + staged `mv`), which is kept. Bug 2 mechanism, measured in isolation rather than inferred: $ bash -c 'set -euo pipefail; args=(--config x.json f.ts) for ((i=0;i<${#args[@]};i++)); do case "${args[$i]}" in -c|--config) echo "matched at i=$i"; ((i++)); echo survived;; esac; done' matched at i=0 EXIT=1 `writeShellApplication` runs under `set -o errexit`, and `((i++))` exits 1 when the post-increment's result is 0 -- i.e. exactly when the config flag is the FIRST argument. The wrapper died mid-rewrite with empty output and exit 1, indistinguishable from a real lint failure. Confirmed against the same wrapper store path CI used (xl9blnqkfk90v2b4dwdyyn97835nmvay). TEST CHANGED DELIBERATELY -- Test 4 encoded superseded behaviour: Test 4 asserted `.oxlint-with-plugins.*` is "cleaned up on exit". That was true of ancestor 48104c1, where tmpconfig was an `mktemp` file removed by an EXIT trap. `main` then replaced it with a PERSISTENT, git-ignored root cache: `.gitignore:77-78` exists specifically for it, and the wrapper documents the measured reason ("keeping the complete file avoids a hash-crawler stat/open race with an EXIT-time deletion"). Test 4 was therefore asserting the opposite of the shipped design, and never failed only because the bug-2 abort meant it was never reached. It now asserts the real contract: exactly the one deterministic cache name is published (so it stays inside the gitignore pattern), and no randomly-suffixed STAGED file leaks into the tree. Proven live, not rubber-stamped: adding `"$tmpconfig"` to the EXIT trap makes it fail with `expected: ./.oxlint-with-plugins.json / actual: (empty)`. Also drops the stale "Cleaned up on EXIT via trap." sentence that contradicted the very next paragraph and is what the old assertion was written from. before Test 3 aborts (bugs 2 and 3 unfixed, Tests 4 and 5 unreachable) after all 5 tests, 13 assertions, exit 0
… hashes
SNAPSHOT UPDATED TO MATCH NEW BEHAVIOUR -- stating it explicitly, because a
snapshot edit is otherwise indistinguishable from hiding a regression.
This stack converts the focus ring from `boxShadow` to `outline`. StyleX keys
a class on property + value + condition, so the ring's classes necessarily
changed. Four inline snapshots still carried the pre-conversion hashes.
The diff is exactly two substitutions and nothing else -- verified
mechanically by applying only these two renames to every removed line and
checking it reproduces the added line byte-for-byte (4 lines, 4 hunks):
x1ju6hqh -> xoceq77
x15uj1cd -> x1nz3mli
Both are the focus ring, confirmed by reading the compiled CSS rather than
inferring from position:
.xoceq77[data-focus-visible]{outline:1px solid var(--x1aboq7h)}
.x1nz3mli:focus-visible{outline:1px solid var(--x1aboq7h)}
one gated on React Aria's data attribute, one on the native pseudo-class. The
stale hashes appear nowhere in a freshly built `dist/styles.css`; the new ones
appear in it.
The regenerated values are CI's, not merely this machine's: run 5210 reported
`Received` hashes `xoceq77` and `x1nz3mli` on BOTH the linux-x86-64 and
macos-arm64 runners, matching what regeneration produced here exactly.
Reproduced on a WARM tree -- `check:quick` simply does not cover this suite,
so this is not a cold-`dist` artefact.
Gate proven live rather than assumed: perturbing the ring to `2px solid` fails
2 of these snapshots (exit 1); reverting returns 7 passed (exit 0). So the
re-recorded bytes still assert the ring.
NOT tuned: the ring's known 5.216% pixel delta (3856/73920 px, max channel
delta 83, self-comparison control 0) is untouched and remains open for the
human decision recorded in the PR body. This commit only records the class
hashes the converted ring already produces; it changes no style value and adds
no `outlineOffset`.
This was referenced Sep 2, 2026
Addresses the Codex review on 5e05410. All four were pass-as-absence defects in this stack's own new code, and each is now defended by a test that was proven to fail when the fix is reverted. P1 — the verdict ignored the one term a dead harness cannot fake. `isStoryGateOk` required `baseline.passed > 0` but never read `settle.settledStories`. Every other term is a LIST, and a list is empty both when nothing is wrong and when nothing was examined: if the gate annotations stop firing while ordinary Storybook assertions still pass, added/removed/ changed/uncovered/unsettled are empty by construction and `baseline.passed` stays positive. `constants.ts` already documented `settledStories` as the positive per-story measurement "a harness that never launched cannot fake", and `cli.ts` already printed "NOTHING SETTLED — every number below is meaningless" — and then the process exited 0 underneath that banner. The verdict now agrees with the banner. proof: neutering the term fails exactly `refuses to pass when nothing settled, even with every other term clean`; 16 others still pass. P1 — `preExisting` was the one join still keyed on the bare story name. This file DEFINES `storyKey({file, slug})` for this exact hazard, documents that "`fullName` is unique only within a story file", uses it for `selfInconsistentKeys` — and then built `preExisting` from `fullName` alone. So a baseline failure called `Default` in one story file subtracted the compare-side failure of `Default` in every other file, skipping a real regression as somebody else's debt. Extracted as exported `assertionStoryKey` so both sides join on one identity and the keying is testable at all. proof: forcing the file component to a constant fails 2 tests. P1 — a capture missing from an intermediate probe was silently dropped. `classifyStability` skipped any key not present in every capture, justified in a comment by "`added`/`removed`/`uncovered` already carry it". That is true only of the KEPT capture: those three compare the retained baseline directory with the compare run, and the probe directories are deleted before that comparison. A story that captured in probes 1 and 3 and in the comparison but not in probe 2 was watched failing to reproduce on an identical tree and then reported by nothing. Now named in `stability.inconsistentPresence`, folded into `selfInconsistent`, and printed by the CLI. Iteration also moved to the UNION of capture keys — keying off capture 1 could not see a story that appeared only later, the same blind spot mirrored. TEST CHANGED DELIBERATELY: `leaves a capture missing from any set unclassified rather than counting it unstable` asserted the OLD behaviour on the reasoning above. Rewritten to assert the corrected contract, with the reason the old rationale was wrong recorded in the test. proof: reverting the collection fails 2 tests. P2 — the baseline cache was keyed by commit alone. `baselineDir` was `<cacheRoot>/<sha>` under a cache root shared by the whole repository, so the first package to write `<sha>/.complete` handed its own report, screenshots, settle records and theme matrix to every other package's gate at that ref. The completeness check cannot catch it: the entry IS complete, it is just a baseline of a different package. Now keyed by package path, config file, source roots and capture count via exported `baselineCacheKey`. The shared WORKTREE is deliberately left unscoped — it is a checkout of the whole repo at that commit and identical for every package. proof: forcing the package component to a constant fails `gives two packages at the same commit different baselines`. P2 — the raw-colour rule permitted the literal its message bans. `stylex-no-raw-color` matched hex and functional syntax only, so `boxShadow: '0 0 4px red'` passed: a named colour is as fixed and as scheme-insensitive as `#f00`, and `boxShadow` is composite so upstream `propLimits` structurally cannot reach it — which is the case the rule's own header names as its reason to exist. Added the 148 CSS named colours, word-bounded on both sides with `-` treated as a boundary so `Tangerine`, `border-color` and `var(--brand-red)` stay silent. `transparent` and `currentColor` are excluded on purpose: neither pins a colour, and `currentColor` follows the scheme a raw literal ignores. Blast radius MEASURED before adopting, not assumed: 0 new diagnostics across 1567 files. Because a zero is also what a rule that never ran reports, it was then proven to fire — `red` in `boxShadow` and `MidnightBlue` in `border` both report, while `transparent`, `currentColor`, `Tangerine, serif` and `border-color 200ms` stay clean. Verified: ts:check:strict BUILD SUCCEEDED; lint:check:format and lint:check:oxlint exit 0; test:utils and test:oxc-config exit 0; gate suite 22 passed (14 before), raw-colour suite 50 passed (40 before). Not addressed here, and named rather than quietly skipped: the ring's 5.216% focus-ring pixel delta remains open for the human decision recorded in the PR body, and R08 is still not claimed.
schickling
deleted the
schickling-assistant/2026-09-02-stylex-collapsed
branch
September 2, 2026 17:13
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
2 participants
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.
Collapses the eight-PR StyleX stack onto
main. Supersedes #1170, #1172, #1173, #1175, #1176, #1183, #1184, #1185, whose bodies stay readable for the per-change evidence they carry.Why one PR
Propagating
mainthrough eight stacked branches meant re-reconciling the lockfile, the fixed-output hashes and the Buck projection once per branch. Branch 2 alone produced 11 conflicts, a semantically broken auto-merged lockfile, aBUCKadmission rename and a circular tooling block. On one tree each of those happens once — and did.What is in it
stylex-preset→stylex-tokenseffect-schema-form-ariamigrated, cascade layers on the one target that can take themboxShadowtooutlineThe
mainmerge, and what it costEvery conflict is recorded at its site. Three are worth surfacing:
The auto-merged lockfile was broken and pnpm's advice did not apply. pnpm reported
no entry for vitest@4.1.9(…)and advised--no-frozen-lockfile; that and--forceboth no-op in ~1s with zero drift, because the lock is consistent for the full 39-project workspace. The inconsistency lives only in the pruned workspace the Nix deps build projects. Removing the lock and re-resolving ran a real pass (28.4s, 667 packages) and turned the failure into a plain hash mismatch — the expected next layer. This is the procedure663e07b0edocuments for itself.One conflict had a real fix on both sides.
mainaddedflockserialisation and atomic publish to the oxlint wrapper; this stack fixed which plugins survive injection. Neither could be taken — resolved by keepingmain's locking structure and substituting this stack'sjqprogram. Provenance verified rather than assumed:mainhas zero matches for the substitution.The Buck projection rejected a bin collision it should tolerate. Detailed below.
The Buck bin collision, researched rather than patched
Generating
buck2/dependencies/BUCKfailed onimporter bin playwright is ambiguous. Rather than pick a tie-break, the idiomatic model was established first:implemented_rulesinprelude/js/js.bzlis exactlyjs_bundle,js_bundle_genrule,js_library) — a negative result, not support for either design.rules_jsand legacyrules_nodejsboth address bins package-qualified (a per-package generatedbinstruct bound to that package's store target;@npm//<package>/bin:<bin>), making collisions unrepresentable..binlayer and deliberately picks a winner: owner-of-name → lexicographic package name → greater version, debug-logged, never failing.The decisive finding is that the package-qualified half already exists here. Every package target carries its own bins via
PnpmPackageInfo, and_nested_binskeys nested links<owner>\tname. Only the importer root map was flat — and a flat directory holding one file per name cannot be made collision-free by requalifying a key. So a winner policy was the missing layer, not an alternative. This is the two halves agreeing, not a divergence.The VRS ratifies parity with the pnpm tree and says nothing about collision rejection, so failing where pnpm succeeds was violating the requirement that is stated.
Verified: exactly one colliding
(importer, bin)pair across 110 read manifests. pnpm's installed tree links bareplaywrightthere, read from the shim; the comparator selects the same. Generated output: theutilsimporter projectsplaywright@1.61.0, and all 15 single-provider importers are unchanged. Comparator unit-checked on 9 cases covering all three pnpm tiers, order independence, peer-suffixed keys and scope restoration, with a negative control that fails when inverted.One decision left to a human
#1185's ring conversion is not pixel-equivalent and is deliberately not tuned to be. Measured with a self-comparison control at 0 and identical canvas dimensions, so paint and not layout: 3856/73920 px (5.216%) at max channel delta 83 on three full-width inputs, 351/61440 on a compact one, with the bounding box covering the whole ring perimeter rather than the corners.
Accepting the
outlineconvention means accepting that change on every field in the package. Reservingoutlineis right on the merits — StyleX writes one value per property, and a ring plus a shadow are twoboxShadowwriters that collide with no compile error — but "right on the merits" and "invisible" are different claims and only the first is established.Verified
check:quickexits 0, zero error and zero warning lines.git merge-base --is-ancestor— fix(story-gate): thread consumer plugins into every project, not the root #1184 was found missing this way and merged in.genie:checkclean; it demonstrably fails onbuck2/dependencies/BUCK, which is how that defect surfaced.Not claimed: R08. The ptg story gate is a different repo; the two blockers behind it are identified and fixed separately.
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile