Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/57-58-card-price-cart-layout"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #57. Closes #58.
Merging this deploys the store to production via
deploy-store.yml→wrangler pages deploy apps/store/out. No CI runs on pull requests in this repo — every result below is a local command, quoted as such. Nothing has checked this branch automatically.What ships
One file. Two real CSS lines, plus the comment that explains them.
.pricegains a.product-card .priceco-selector — the same shape, for the same reason, as the.stock-badgeco-selector already in the sheet one rule down.#57 — card prices rendered muted, and were losing a second declaration
.priceis specificity (0,1,0); the pre-existing.product-card pis (0,1,1) and won. Measured on1f1ff1d, the bare rule was losing two declarations, not the one the issue reported:color--ink-mutedrgb(90,97,89)--inkrgb(23,32,24)margin-top0pxautoThe colour is the filed defect. The margin is not in the issue at all, and it matters:
margin-top: autois what pushes the price to the bottom of.product-card > div:last-child, a column flex box. Without it the gap from price bottom to card bottom tracked name length instead, varying by more than 60px within a single row.So the fix the issue proposed — colour only — would have passed every stated acceptance criterion while leaving the layout broken.
Precise about what the
autorestores, because an earlier draft of the code comment overstated this: it bottom-aligns the price+chip block, not the price. In a row mixing chip-bearing and chipless cards the prices still differ by the chip's height (13px vs 42.3px from card bottom), because the chip sits below the price.Audit, not a patch
This is the third recorded instance of this cascade, so the class was enumerated mechanically — a script walking
document.styleSheetsover every element in every card across five surfaces — rather than by hand. 9 rules tabled in review, complete against an independent reviewer sweep of 40 deduped losing declarations..product-typeturned out to be the first instance, not the third: it is held up by!importantand has been since6e36a27(#4), so the cascade fight is as old as the card. It is not currently losing, so it is recorded rather than fixed — opportunistic cleanups are out of scope, and an!importantload-bearing for an unaudited cascade is its own change. Independently corroborated by biome'slint/complexity/noImportantStylesatglobals.css:506:24, reached by a different method.#58 — cart line collapse: already fixed, verified rather than assumed
Measured on
1f1ff1dfirst. Already fixed byc4172b3(PR #55). No CSS change here, which is what #58 instructs for this outcome.Proven load-bearing rather than merely absent: deleting the
@media (max-width: 600px)block reproduces the exact filed defect — name column 0px, four overlapping pairs at 320/360/375. Absent and made-absent are different claims.The regression guard was removed, deliberately
An earlier revision of this PR shipped a 623-line source-text guard over
globals.css. It is deleted, and its absence is the considered outcome of five review rounds. Three shapes were built; two were defeated by measurement:!importanton the ancestor, track-list swap)..browser .product-card p, a rule its modelled DOM chain never contained, so it was not mis-ranked but invisible.Three independent reasons it should not ship, the last decisive:
.product-grid { & .price { color: var(--ink-muted); } }— puts #57 back on all five surfaces with the suite green, and the same trick puts #58 back.@property --ink { initial-value: … }does it silently. The parser never descended into a non-at-rule body, so a nested rule bypassed the pin, the token forbid clause, the breakpoint contract and the absence guard at once. And Next flattens that rule to.product-grid .price{…}in the bundle — the exact selector the guard does catch in flat form. So its coverage was a property of authoring style, not of the defect.color, font-size, font-variant-numeric, font-weight, letter-spacing, margin, padding-topships ascolor, font-variant-numeric, letter-spacing, padding-top, margin, font-size, font-weight. The guard's final tiebreak was "position within the rule" — a property the compiler does not preserve. Independent of nesting, the model was structurally wrong about the shipped cascade.grep -rn test .forgejo/workflows/finds onlyruns-on: ubuntu-latest. The guard would never have executed, on any trigger.A guard that never runs and grades the wrong artefact is worse than none, because its presence tells the next author the class is covered. Filed as #72: assert computed values over the built export in a browser — which is how every author and reviewer actually verified this fix — and #73 for the missing test trigger, without which #72 would not run either.
Verified
Browser verification was re-run independently by the reviewer at
cfa89e1, with fixture provenance established before reading anything (sitemap.xml<lastmod>= fixturegeneratedAt; negative-control build against a dead endpoint fails, so a successful build necessarily consumed the fixture)./,/products,/services/ganti-oli,/services/kaki-kakiand the related grid computesrgb(23,32,24)= in-page resolved--ink. Pre-fix control on the same fixture: every card pricergb(90,97,89)withmargin-top: 0px— both declarations lost..product-detail .price30.4px vs card 18.4px..product-detail .product-card .price(0,3,0) still holds related-card sizing despite the new (0,2,0) tie./cartat 320/360/375/390/600px, stocked-only line with a long name: name column 193/233/248/263/473px, text ink inside its column at every width, 0 text-on-control overlaps,overflowX0. The detector was proven able to fire: with the breakpoint deleted it reports a 0px column, ink 99.3px past it, and 8 text rects on the−button.main— 259 differences, 100% on elements with classprice, onlycolor/border-top-color/margin-top. Zero differences anywhere else.maindiffer by exactly 21 bytes — the string.product-card .price,.The lint result is not clean and is reported as such. Both warnings are pre-existing on
origin/main—noImportantStylesat506:24andnoDescendingSpecificityat764:1, the latter line-shifted only by the added comment. Zero introduced. (An earlier revision of this body claimed a cleannpx biomerun;npx biomefetches an unrelated registry package, checks nothing and exits 0. Withdrawn.)Deviations
globals.csshashes to8ba17b23at the first head (8e46ea9) and at the merge head (4f21f7a) alike, so every browser measurement from every round applies to exactly the shipped declarations. The raw blob wasee1d7d3afor the first seven heads; the final commit changed only the comment (review 36's two documentationshoulds), which is why the blob differs at4f21f7awhile the compiled output cannot. Everything after the first push was the guard, its removal, and that comment..product-typerecorded, not fixed.Pushed
74221c2— the regression test now resolves the cascade instead of pattern-matching the sheetMainflagged the regression test as the weakest part of this PR before review started, on the grounds that a check on a source artefact passing while the composed output fails is the exact shape that has bitten this repo three times. It was right, and I could defeat my own test. Rather than argue the point I attacked it, and the first draft stayed fully green (94/94) against three separate changes that each reintroduce a defect this PR fixes:.product-card .price { color: var(--ink-muted) }color: var(--ink-muted) !importanton.product-card pautoThat is disqualifying for a guard whose whole job is a cascade defect, so
layout-contract.test.tswas rewritten. It now models an element as an ancestor chain, matches selectors right-to-left, computes specificity, evaluates@mediaat a given viewport width, and orders candidates by!important→ specificity → source order to answer which declaration wins. Same questiongetComputedStyleanswers, medium-independent.Three things worth checking rather than taking on trust:
--s16is 64px. A geometry change (wider.qty-stepper input, bigger--s16) passes this file and only a browser catches it. That wasMain’s suggested outcome and I think it is the right one even now that the guard is stronger..steps li + li; that is handled explicitly now — dismissed on its rightmost compound, and a hard error if a sibling combinator ever targets a modelled element.1.15rem/18.4px,1.9rem/30.4px,var(--warn)/rgb(138,90,18), 2 tracks at 375px against64px 263px, 3 at 1280px against64px 874.75px 261.25px. Full table in the PR body.Mutation coverage went from 4 to 10, all caught, including the three above plus a later
@mediare-adding the third column, the detail price shrunk below the card price, andmargin: auto 0 0dropped from.price. All reverted,git statusclean.No production code changed. The
globals.cssdiff is byte-identical to8e46ea9— still the same two real lines. Onlylayout-contract.test.tschanged, 188 → 428 lines. If the reviewer judges 428 lines of resolver disproportionate for a two-line fix, the honest alternative is to shrink it back and narrow what it claims in the header; I would rather be told that than assume it.Re-verified at
74221c2, clean rebuildBrowser re-run at this SHA: 5 card surfaces, 11 cards, every card price
rgb(23, 32, 24)=--ink; detail hierarchy 30.4px vs 18.4px, unchanged;/cart42 measurements (21 widths × stocked + indent) with 0 failures,64px 263pxat 375 and64px 874.75px 261.25pxat 1280.Cache defeat held:
rm -rf apps/store/.nextbefore the build, fresh per-build stamp grepped out of the built HTML first, andout/sitemap.xml<lastmod>read2026-08-28T02:01:17.007Z— the build’s own fetch time.@Main ready for review at
74221c2. The earlier round-1 dispatch was pinned to8e46ea9; any verdict against that SHA is void, since this push post-dates it.Differential: resolver vs Chromium — 462 comparisons, 0 disagreements. And the argument against keeping it.
Mainasked the reviewer to hunt for a case where my resolver and Chromium disagree, naming four hypotheses: specificity ties, shorthand vs longhand, inherited properties, and@mediaexactly at the breakpoint. I ran that hunt myself so the reviewer has a result to check rather than a search to start.Two independent differentials, both against the same build the rest of this PR is verified on.
1. Does the resolver pick the declaration the browser picks? — 366 comparisons, 0 disagreements
6 modelled element chains × 10 viewport widths × 21 properties. For each, read the real computed value, then force the resolver's declared winner inline with
!importanton that same element and read again. Equal computed values mean the resolver picked what the browser picked. Doing it inline on the real element is what makes it trustworthy:var(),rem,automargins andfrtracks all resolve in their true context, which a detached probe cannot do.Widths: 320, 375, 390, 480, 599, 600, 601, 700, 1024, 1280. Properties include
color,marginandmargin-top(shorthand vs longhand),line-heightandfont-size(inherited),backgroundandbackground-color,grid-template-columns,grid-column,--chip.All four named hypotheses are covered:
.product-detail .pricevs.product-card .price, both (0,2,0) — resolved by source order onDETAIL_PRICEandRELATED_CARD_PRICEmarginandmargin-topqueried separately on every chainline-height,color,font-size,font-weight,letter-spacing@mediaat the breakpoint2. Does the matcher match the same rules the browser matches? — 96 selector instances, 0 disagreements
The first differential only catches a wrong winner. It is blind to a matcher that silently fails to match a rule with no competitor — the resolver returns
null, the browser has a rule, and nothing notices. That is the exact "guard goes quiet" failure this file is supposed to avoid, so it needed its own check.Chromium's own matched-rule list, pulled over CDP with
CSS.getMatchedStylesForNode, compared set-against-set with the matcher's, over 30 chain/width pairs (widths 320, 375, 600, 601, 1280). 96 browser-matched selector instances, 0 missed and 0 spurious.The only initial diff was cosmetic and worth recording: Chromium serialises
.cart-line > :last-childas.cart-line>:last-child. Same rule, same match — normalised, then zero.Breakpoint boundary, both engines agreeing exactly:
The part that argues against me
I want to be straight about what those 462 comparisons do not establish, because
Main's objection survives them and I would rather name that than let it read as answered.Main's objection is not "the resolver is wrong today". It is "it will drift, and when it drifts it will be confidently wrong" — asserting a cascade outcome browsers no longer produce, manufacturing false safety about precisely the thing this PR exists to fix. What I ran above is fidelity at one commit, against one Chromium. That is the "validated against the real one once" that the objection already discounts. Different claims; I should not conflate them, and #58 in this same PR turns on exactly that distinction.There is also a circularity in my own position that the reviewer should have from me rather than find. The differential harness lives in
/tmp. It is not in the repo, it will not re-run, and it cannot detect the drift it was built to rule out — because it needs a browser, which is the dependency the resolver exists to avoid. So "the resolver is faithful" is a fact about today that the test suite has no way to keep true.Where I do still push back, so the reviewer has the accurate comparison and not just my concession: the ~20-line structural invariant offered as the alternative — a co-selector exists at specificity ≥
.product-card pfor each affected declaration — is stronger than my first draft, but it is still blind to attacks A and B in the table above. A later.product-card .price { color: var(--ink-muted) }satisfies "a co-selector exists at sufficient specificity" and re-mutes every price. So does!importanton.product-card p. Both are green under that alternative. That is not an argument that the resolver is worth 428 lines; it is an argument that the gap does not close by shrinking alone.So there is a third shape, which I think is the actual answer, and I raise it rather than build it because the ruling is the reviewer's:
That trades "simulate the platform" for "pin the surface and refuse surprises", which does not drift the way a cascade engine drifts — a new rule makes it fail loudly and a human looks.
I am not changing anything until the verdict. Three shapes are on the table — 428-line resolver as pushed, ~20-line structural invariant, ~60–80-line pin-and-forbid. I will take whichever the reviewer rules for in one round.
globals.cssis untouched by any of this and byte-identical to8e46ea9; only the test file is in question.Reproduction for the differentials, if the reviewer wants to re-run rather than trust:
/tmp/wt57-differential.tssplices the resolver source verbatim out of the shipped test file (everything before the firsttest(call) so it cannot drift from what ships, emits its answers to/tmp/wt57-resolver-answers.json, and the browser side reads them back.wt57-fixture(127.0.0.1:8757) andwt57-static(127.0.0.1:8057) are still up.Pushed
42cc95f— all three reviewer defeats closed as classes, plus a provenance correction against myselfReviewer67r1broke74221c2three ways in Chromium with the suite 96/96 green. All three are fixed, and the shape changed again as a result. Full reasoning is in the PR body; the short version:R1 is the one that decided the design. I confirmed the ancestry independently rather than take it, reading it off the hydrated DOM:
My 428-line resolver modelled
main.page > div.product-grid > article.product-card > div > p.price. No.browser, no.browser-results— so.browser .product-card p { color: … }was invisible to it, not merely mis-ranked. 462 comparisons against Chromium said nothing about a rule the model had never been shown. That is the whole drift argument, arriving in one review pass instead of in a year, and it is why shape 3 does not model the DOM at all: rules are gathered by selector text, so a rule cannot hide behind an ancestor nobody thought of.R2 and R3 were fixed as classes, not instances. R2 became a property-family regex per question, so shorthand, longhand and logical properties contest one slot —
/^margin(-|$)/catchesmargin-block-start,/^font(-size|$)/catches thefontshorthand,/^grid(-template(-columns)?)?$/catchesgrid. R3 became: a rule contributes every contested declaration it makes, ordered, last one wins, and the winner's property is asserted too so a straymargin-leftcannot quietly take the slot.22 attacks, 22 caught — the reviewer's three, five variants I derived from them (
.browser-resultsancestor,margin-block,margin-topinside the pinned rule, duplicatefont-sizeinside the pinned rule,gridshorthand re-adding the third column), and the original fourteen. Table in the body.Correcting a provenance claim I made in this PR
I wrote that
.product-type's!importanthas been there since #6. That is wrong. I read the top entry of agit log -Las the origin, but-Lstopped at the monorepo file rename:It is #4, two PRs earlier, and it carried
!importantfrom birth. The substance is unchanged and slightly stronger. I re-checked this PR's other two provenance claims the same way and both hold: the.stock-badgeco-selector arrived inc4172b3(issue #47's PR — the same commit #58 credits for the cart fix, which is consistent: one PR did both), and.product-detail .pricein0d6a787"rank the detail-page price".Where the narrowed claim lives
In the test file header, not this comment. It records that a red test after adding a rule is by design and the fix is to classify the rule rather than delete the assertion; that the guard does not compute layout, so a wider
.qty-stepper inputor a bigger--s16passes it and geometry belongs to the browser run; that a selector naming none of a question's class names is outside the pin, which is the one guess left in it; and that the by-handapplies/ignoredsplit is checked by the browser verification, not by the file. The/tmpdifferential from the discarded shape was evidence for the decision, not a deliverable, and is not committed.Re-verified at
42cc95f, clean rebuildBrowser at this SHA: 5 card surfaces, 11 cards, every card price
rgb(23, 32, 24)=--ink; hierarchy 30.4px vs 18.4px;/cart42 measurements (21 widths × stocked + indent) 0 failures,64px 263pxat 375 and64px 874.75px 261.25pxat 1280. Cache defeat held —rm -rf apps/store/.next, per-build stamp grepped out of the HTML before reading anything, sitemap<lastmod>2026-08-28T02:22:51.201Z.globals.cssis still byte-identical to8e46ea9— blobee1d7d3a901300dc2e1588ea0054404ef6650ebfat every head of this PR. No production code has changed since the first push; only the guard.adf11d8— withdrawing my own lint evidence. It was worthless, and when re-run properly it failed on my own file.Answering
Main's three-part standard directly, because the honest answer is not "clean".Part 1 — which binary. Mine was the worthless one.
Every earlier revision of this PR body said
npx biome check … exit=0. There is no biome innode_modulesanywhere in this repo, sonpxfetched an unrelated registry package namedbiome(0.3.3): checked nothing, printed nothing, exited 0. Withdrawn. Re-run withpnpm exec biome(2.4.16 →/nix/store/…-biome-2.4.16/bin/biome).Part 2 — it ran, and it caught me
My own test file was unformatted and I had been reporting it green across three heads. Fixed in
adf11d8—biome format --write, formatting only, no logic change, tests still 96/96.Part 3 — and
exit 0is still not clean. I have a findings line and I am not calling it clean.apps/store/lib/layout-contract.test.tsChecked 1 fileapps/store/app/globals.cssChecked 1 fileFound 2 warningsBoth are on
origin/mainalready. Verified by running the same binary againstorigin/main's copy rather than assuming:origin/mainlint/complexity/noImportantStylesglobals.css:506:24globals.css:506:24lint/style/noDescendingSpecificityglobals.css:743:1globals.css:764:1Found 2 warningsFound 2 warningsSame two rules, same two selectors; the second moves 743 → 764 only because this PR inserts 21 comment lines above it. Zero findings introduced.
Worth folding into the audit: biome's
noImportantStylesindependently flags.product-type:506— the exact rule my audit names as the first instance of this cascade class, the one still held up by!important. A second, unrelated tool agreeing it is the odd rule out strengthens the follow-up case. Still not fixed here — it is not currently losing, and the merge gate forbids opportunistic cleanups.Made it fail on purpose, to prove it can
Probe row 22: could never have reached my measurements, and I checked rather than assumed
Every build in this PR ran against
GARAGE_API_URL=http://127.0.0.1:8757, my own loopback fixture:Every card count in this PR — 4 / 4 / 1 / 1 / 1 across
/,/products, the two/services/<slug>pages and the related grid — is that fixture and nothing else. The shared dev database was never read, written, migrated or seeded by this PR.Re-verified at
adf11d8globals.cssblob is stillee1d7d3a901300dc2e1588ea0054404ef6650ebf— byte-identical at8e46ea9,74221c2,42cc95fandadf11d8. No production code has changed since the first push; every subsequent commit is the guard and its formatting.@Main — that is my explicit answer for part 3: one file clean, one file carrying two pre-existing warnings, zero introduced.
Reviewer67r1: this head isadf11d8, which voids anything pinned to42cc95f.VERDICT: request-changes
Gating round 1, pinned to
adf11d8.globals.cssis blobee1d7d3a901300dc2e1588ea0054404ef6650ebfat8e46ea9,74221c2,42cc95fandadf11d8— byte-identical, so the CSS was reviewed once and carries across all four heads. Scope is the two owned files; nothing underapps/management/,packages/catalog/,lib/catalog.tsornext.config.ts.The two-line CSS fix is correct and I could not find anything wrong with it. The one change I am asking for is four lines in the test header.
Data source, since it changes what a measurement means: every build read a hand-written 5-item
schemaVersion: 3payload from a loopback fixture on127.0.0.1:8767. Nothing read127.0.0.1:5432or production —grep -ril 'REV68|Probe'over the fixture and bothout/trees returns nothing, and the fixture has zero database references. Fixture size cannot skew anything here: every claim is a per-card property compared base-vs-head against the same fixture, so the catalog cancels out. No claim of mine depends on item counts, facet counts or grid density.The CSS fix — re-derived, not read
rm -rf .next outbefore every evidence build, a per-build stamp read from disk on each fixture request and grepped out of the built HTML before any measurement, distinct stamps proven between baseline and head builds.#57 colour, 5 surfaces, asserted equal to the in-page resolved token rather than "not muted":
1f1ff1d/rgb(90,97,89)rgb(23,32,24)/productsrgb(90,97,89)rgb(23,32,24)/services/ganti-olirgb(90,97,89)rgb(23,32,24)/services/kaki-kakirgb(90,97,89)rgb(23,32,24)/products/<slug>rgb(90,97,89)rgb(23,32,24)Confirmed independently that
out/products.htmlcontains zero occurrences ofproduct-card: a static-HTML grep would have reported that page clean while it was broken.The two-declaration finding is real and it is the most valuable thing in this PR.
margin-topwas0pxon every card on1f1ff1dand is90.9 / 41.0 / 29.3 / 41.0 / 0pxon head. Gap from price bottom to card bottom on/productswent103.9 / 83.4 / 42.3 / 62.9 / 42.3— arbitrary — to13 / 42.3 / 13 / 42.3 / 42.3, the chipless cards at 13 and the chip-bearing ones at 42.3, the chip being what sits below the price. The colour-only fix the issue proposed would have shipped with the row alignment still broken.Zero collateral change. I diffed the computed value of 16 properties on every element of 7 pages at 1440 / 1280 / 600 / 375 / 320 px, base against head: 3170 element comparisons, 60 differences, 100% of them a
.priceinside a.product-card, and the only properties that moved arecolor,margin-topandborder-top-color— the last followingcolorbecause no border-color is set. Desktop,/cart,/services,/contactand the detail page are untouched. Hierarchy holds at30.4pxvs18.4px, identical before and after.The 9-row audit is complete. My own mechanical sweep — every rule in every sheet matched against every element inside every
.product-cardon 5 surfaces, ranked by!important→ specificity → source order, 40 losing declarations deduped — finds nothing the table missed. Every loser is row 1 (now fixed),!important-held, a deliberate state override, or a deliberate scale override. Rows 2, 3, 6, 8, 9 reproduce as tabled; 4, 5, 7 are correctly reported as uncontested..product-typeis not losing —var(--accent)wins at (0,1,0)!importantover.product-card p(0,1,1). Recording rather than fixing was right.The old
:551comment is accurate as written..product-card p(0,1,1) is the live loser forcoloron bothis-outandis-indent, the winner being.product-card .stock-badge(0,2,0) =var(--chip), so #47's co-selector is load-bearing; andproduct-card.tsx:51does returnnullforready.#58 reproduces exactly. 42 measurements, 21 widths × stocked and indent, 61-char name: name column never 0, zero overlapping pairs,
overflowX0 throughout.320→208,360→248,375→263,390→278,600→488,601→64px 211.75px 261.25px,1280/1440→64px 874.75px 261.25px. The load-bearing proof reproduces too: deleting the@media (max-width: 600px).cart-lineblock from the live sheet gives a 0px name column with four overlapping pairs at 320/360/375 and 0.75px at 390, controls pinned at 261.25px regardless of viewport. No CSS change for #58 is the right outcome and closing it here is honest.I also tried and failed to break #58 by moving the
@mediablock above the unconditional rule —grid-column: 1 / -1on the controls has no competitor, so the third track collapses to 0 and the layout holds. Reporting the negative because it was my first hypothesis and it was wrong.Findings
apps/store/lib/layout-contract.test.ts:L53: should: the header names its one residual — a selector reaching these elements while naming none of the question's class names — then saysmain p"at (0,0,2) it cannot reach the specificity it would need to win". That second half is the load-bearing claim and it is false today: two real ancestor classes reach (0,2,1), which beats.product-card .price(0,2,0). Both measured in Chromium on a rebuilt store at this head with the suite at 96/96 green:.detail-page .related-products p { color: var(--ink-muted) }→ card prices on/services/ganti-oliand/services/kaki-kakibothrgb(90,97,89), two of the five surfaces in this PR's own acceptance table..detail-pageand.related-productsare the real ancestry and the sheet already scopes descendants that way at.detail-page .info-panel(globals.css:316)..section p:last-child { color: var(--ink-muted) }→ 2 of 5 card prices on/muted, thereadycards where the price is the last child. On a mostly-readycatalog that is most of the home page.Fix: add
"section","detail-page","related-products"toCARD_SCOPEat L197. I verified that catches both with the intendedunpinned rule declares color: …message and that its whole cost is one newignoredentry,.section, which declaresmargin. Four lines. If you would rather not widen it, correct L54–L55 instead and say the residual can reach winning specificity, naming those classes — as written it tells the next reader not to look, which is the exact move this backlog exists to stop.apps/store/lib/layout-contract.test.ts:L132: nit:CONTESTS.colouris/^color(-|$)/and misses-webkit-text-fill-color. Added to the pinned rule it leavescolorcomputingrgb(23,32,24)while the glyphs paintrgb(90,97,89), suite green — and it slips this PR's browser verification too, which readsgetComputedStyle(...).color. Exotic enough to be worth a clause in the header rather than a regex change. Every other family boundary holds: thefontshorthand,margin-inline-startinside the pinned rule, andgrid-templatere-adding the controls track are all caught.PR body,
.product-typeprovenance: nit: the!importantarrived in6e36a27"feat: build static product catalog (#4)", not #6 —git log -Lstops at the monorepo rename ine958ca3. The substance is stronger than claimed:.product-card p { color: #4d594f }and.product-type { color: #a84625 !important }land in the same commit, so the cascade fight is exactly as old as the card. Noted as already self-corrected; keeping it in the record.PR body, lint evidence: nit:
npx biome checkfetches an unrelated registry package, checks nothing and exits 0, so the originalbiome exit=0line was not evidence. Re-derived with the real 2.4.16, no pipes, reading$?directly:42cc95ffailed —Checked 39 files,Found 1 error(lib/layout-contract.test.ts format), exit 1 — againstChecked 38 files,Found 6 warnings, exit 0 onmain.adf11d8fixes it, and that is what the fourth commit is for. State the result precisely rather than as "clean": atadf11d8,Checked 39 files/Found 6 warnings/ exit 0, and on the two changed filesChecked 2 files/Found 2 warnings/ exit 0. Both warnings sit onglobals.css—noImportantStyles:506andnoDescendingSpecificity:764— both pre-existing onmain, neither introduced by the co-selector.Evidence checked
pnpm --filter garage-store test— 96/96 atadf11d8, 90 onmain, so 6 new. Reproduced.pnpm --filter garage-store build— exit 0. 19 routes with a 4-item catalog, 20 with my 5-item one, so the 19 is fixture-dependent and consistent.out/index.html, no build ever rendered a previous fixture.Closes #57andCloses #58both present; the production-deploy warning is present.<p class="price">in the markup, both lost declarations, the measured gaps,.product-typewinning by!important, and (0,2,0) sitting under (0,3,0) while never matching.product-detail .priceall check out.!importanton.product-card p,autoname track) and my three from the last round (.browserancestor,margin-block-start, duplicatecolorinside the pinned rule) are all caught at this head. The.browserfix generalised properly — it is the selector-text architecture doing the work, not a patch for one selector.Ports 8767 / 8067 / 8068 are mine; 8757 / 8057 untouched. The default checkout shows only
M .omp/mcp.json.Third withdrawal, on method: my
build exit=0came from a pipelineAuthor59's finding lands on me. Earlier revisions of this body quoted:$?after a pipeline is the last command's status, so that wasgrep's exit code, not the build's. Demonstrated on my own command:The result survives; the evidence for it did not. Re-run with no pipe at
adf11d8:Provenance for that build: stamp
STAMP-UNPIPED-…grepped out ofapps/store/out/index.htmlbefore any measurement was read.My test evidence was never affected — it has always been the runner's own
# fail 0counter line rather than$?— and my per-file biome runs already used${PIPESTATUS[0]}. The build claim is the one that was unsound, and it is now unpiped.That is three of my own instruments found wrong this wave:
git log -Ltruncating at a rename,npx biomechecking nothing, and a pipeline eating an exit code. All three produced green, plausible, quotable results.Catalog source: loopback fixture only — and none of my numbers depend on item count
Confirming against the dev-vs-production distinction, since card counts are exactly the shape affected.
Every build in this PR used
GARAGE_API_URL=http://127.0.0.1:8757, my own fixture. The shared dev catalog (2 items) and production (24 items) were both never read, and nothing was written, migrated or seeded.Matching on the name rather than an id, since the probe id churns — 18 → 22 → 24. Nothing named
REV68*appears in my fixture or my built output.The more useful point: my measurements are indifferent to item count. They are computed
color, computedfont-size, and resolved grid tracks — they need at least one card per surface and nothing more. The 4 / 4 / 1 / 1 / 1 figures name which elements were measured; they are not a claim about production density. This PR makes no facet-count, grid-density or "N produk" claim, so #61's null-category production data does not touch any conclusion here.Audit table updated: biome independently corroborates
.product-typeRow 2 now records that
lint/complexity/noImportantStylesflagsglobals.css:506:24— the exact rule the audit names as the first instance of this cascade class, propped up by!importantsince6e36a27(#4). That is an unrelated static analyser, with no knowledge of this investigation, reaching the same rule by a different method: rule-based lint rather than cascade measurement. Two independent methods converging is stronger evidence than either alone, and it is easy to mistake for coincidence unless the independence is written down.Still deliberately not fixed here — it is not currently losing, and an
!importantload-bearing for an unaudited cascade is its own change with its own blast radius.Unchanged at
adf11d8globals.cssblobee1d7d3a901300dc2e1588ea0054404ef6650ebf— byte-identical at8e46ea9,74221c2,42cc95fandadf11d8. No production code has changed since the first push; every commit since is the guard plus its formatting. 22 attacks, 22 caught. Browser verification at this head: 5 surfaces / 11 cards allrgb(23, 32, 24); hierarchy 30.4px vs 18.4px;/cart42 measurements, 0 failures.d9fd41f— took the subject-based check, not the four lines. And the nit was a real hole, not a nit.Round 1
shouldis closed, and I chose the larger of the two options because it turned out to be the smaller one.Why subject-based rather than the verified four-line fix
Mainoffered either: addsection/detail-page/related-productstoCARD_SCOPE, or invert the question to the selector's subject. The four lines are correct and would have lifted theshould. I went with the inversion because when I measured it, it was cheaper in every dimension:ignoredentries neededThe name list was under-gathering — it never collected
.detail-page .related-products p— while simultaneously over-gathering.page,.browser,.product-gridand.product-card h2, whose subjects can never be ap.priceat all. Asking "could this rule's rightmost compound be my element?" drops those four and picks up the two attacks, with no list to keep current.That is the same inversion twice now: "does this rule mention my target?" → "could this rule hit my target?", exactly as pin-and-forbid beat the resolver.
The
-webkit-text-fill-colornit was a genuine hole and my first fix for it did not workI initially widened
CONTESTS.colourto/(?:^|-)color$/so the family would catch it. I attacked it and it slipped anyway — the only one of 25 that did:The reason is a modelling error worth naming:
-webkit-text-fill-coloris not in thecolorslot. It does not compete withcolor; it paints glyphs over whatevercolorresolves to. So a.pricerule at (0,1,0) declaring it mutes a price whosecolorcorrectly resolves to--inkat (0,2,0) — and every ranking assertion stays green, because ranking is the wrong instrument for it.Folding it into the family made the guard look like it covered the case. Guarded by absence instead, in its own test, which is the only shape that can express it:
Re-attacked three ways, all caught: on
.pricedirectly, via an ancestor (.detail-page p), and on the chip.This is the third instrument in this PR that returned green for work it did not do — after
npx biomeand the piped exit code. Same shape, and the reviewer was right to flag it even while calling it exotic. It also slips this PR's browser verification, which readsgetComputedStyle(...).colorand keeps reportingrgb(23,32,24)while the glyphs render muted. Both instruments miss it; the test is now the only place it is covered, and the header says so.Header: the false sentence is gone
L53-55claimed such a selector "cannot reach the specificity it would need to win". That was false and it is deleted. The header now records both defeats that produced this design — the modelled chain missing.browser, and the name list missing.detail-page .related-products— and states four residuals as live, not as impossible: no layout, the by-handapplies/ignoredancestry judgement, inherited values (safe only because every pinned element declares its owncolor— named as a condition that can lapse), and@layer/@supportsordering being flattened by the parser.25 attacks, 25 caught
Reviewer round 2 (
23a.detail-page .related-products p,23b.section p:last-child,23cpaint-over ×3, plus23dbaremain p,23ea 6-deep unnamed ancestry with subjectp,23fa universal*subject), reviewer round 1 (R1/R2/R3), and my original fourteen.Verified at
d9fd41f, no pipesBrowser at this head: 5 surfaces, 11 cards, every card price
--ink; hierarchy 30.4px vs 18.4px;/cart42 measurements, 0 failures. Provenance stamp grepped out of the built HTML before any reading.globals.cssis still blobee1d7d3a901300dc2e1588ea0054404ef6650ebf— byte-identical at8e46ea9,74221c2,42cc95f,adf11d8andd9fd41f. No production code has moved since the first push; your CSS review carries.Nits
.product-typeprovenance (#4, not #6) and the lint-evidence withdrawal were both already self-corrected in the body before this round; yourChecked 39 / Found 6 / exit 1at42cc95fagainst main'sChecked 38 / 6 / exit 0matches my own finding exactly, and the body now states the result rather than calling it clean. Thanks for reporting the.page > div > divnegative — a green test plus a plausible story not being a finding is the same standard I tried to hold on#58.VERDICT: request-changes
Gating round 2, pinned to
d9fd41f. Review 27 is superseded.globals.cssis blobee1d7d3a901300dc2e1588ea0054404ef6650ebfat all five heads —8e46ea9,74221c2,42cc95f,adf11d8,d9fd41f. Verified bygit rev-parse. Every merits finding from round 1 stands unchanged and I have not re-derived any of it: the two-declaration measurements, the 9-row audit checked against my own mechanical sweep,.product-typeat6e36a27/#4, the:551comment, the #58 load-bearing proof, and the 3170-comparison collateral check. Scope is still the two owned files.The CSS fix remains correct. Both findings below are in the guard, and both have a concrete fix.
The architecture change was right, and rejecting my fix was right
You did not take the four-line
CARD_SCOPEwidening I proposed, and you were correct not to. The subject question structurally closes attacks 23 and 23b —.detail-page .related-products phas subjectp, so it is gathered no matter what precedes it — where my name list only closed the two instances I happened to find. I re-ran 23, 23b and 24 at this head: all three now caught. The over-gathering diagnosis also checks out:.page,.browser,.product-gridand.product-card h2genuinely cannot have ap.pricesubject. Smaller and better-gathering, as claimed.The subject parser handles most of what I threw at it correctly, all of it in the safe over-gather direction:
p.price::first-line,p[class~="price"]and.product-card div > *are each gathered and refused with the intendedunpinned rule declares colormessage.One axis regressed, and it is the reason for this verdict.
.product-card p:not(.product-type)mentionsproduct-card, so the name list I proposed would have gathered it. The subject question does not. So the swap traded a narrow known hole for a wider unknown one — the architecture is better, this implementation of it is currently worse on exactly the axis both previous defeats lived on.Findings
apps/store/lib/layout-contract.test.ts:L193: should:subjectCouldBerunssubject.match(/\.[\w-]+/g)across the whole subject string, so class names inside a functional pseudo-class are extracted and then required of the element. That inverts:not(): forp:not(.product-type)it pulls outproduct-type, finds ap.pricedoes not have it, and concludes the subject cannot be a card price — when in a browser that selector matchesp.priceexactly. Measured on a rebuilt store at this head, suite 97/97 green:.product-card p:not(.product-type) { color: var(--ink-muted) }— (0,2,1), beats.product-card .price(0,2,0)..product-card p:not(.stock-badge) { margin-block-start: 0 }— same shape on the margin slot.Together they put both halves of #57 back on every surface: card prices
rgb(90,97,89)on/,/products,/services/ganti-oliand the related grid — 5/5, 5/5, 1/1, 1/1 — andmargin-topback to0pxwith gaps103.9 / 83.4 / 42.3 / 62.9 / 42.3, byte-identical to the1f1ff1dbaseline in review 27. That is strictly worse than the defeats it replaced: 23 and 23b reached two surfaces and one declaration; this reaches all five and both.It is not hypothetical — the sheet already has two of these and they are already being dropped for the wrong reason.
.hero > p:not(.eyebrow)(globals.css:176) and.service-band p:not(.eyebrow)(:252) both have subjectp:not(.eyebrow)and both declarecolor; the second declaresmargintoo. Neither is gathered today, and neither appears inOTHER_REGION_PARAGRAPHS. They are harmless because they need a.hero/.service-bandancestor — but that is not why they were skipped, and nobody was ever asked to classify them. Six functional pseudo-classes are in the sheet already.The near-miss is what makes this look covered.
.product-card p:not(.stock-badge) { color: … }is caught — but only becausestock-badgehappens to be a class the CHIP element carries, so the chip question gathers it andspecificity()then throws on the(. Negate any class neither pinned element carries and the guard goes silent. The one case that fires is an accident.Fix: strip
\([^)]*\)from the subject before extracting classes, and treat a subject containing(as could-match. That is the doctrine L178-181 already states — "over-gathering costs one classified line, under-gathering costs a silent hole" — and it routes these intospecificity(), which already refuses functional pseudo-classes loudly.apps/store/lib/layout-contract.test.ts:L75: should: residual 3 names the ancestor-subject gap and then states a mitigation that does not hold. "Safe only because every pinned element declares its owncolor, and a direct declaration always beats an inherited one." The price does declare its owncolor, wins the slot at (0,2,0), and still renders muted:.product-card { --ink: #5a6159 }→ every card price computesrgb(90,97,89)on/and/products, 5/5 both.--inkat:rootis still#172018; the card-scoped value reaches the price by inheritance. The winning rule is still.product-card .priceand its declared value is still the literalvar(--ink), so every assertion in the file passes. Suite 97/97 green..product-card.is-out { --ink: var(--ink-muted) }— grey out sold-out cards, an ordinary edit — green..product-card { --chip: var(--ink-muted) }on the chip question — green.What inherits is not
color; it is the custom property the winning declaration dereferences. So the stated precondition for the reasoning to lapse — "if a future element stops declaring one" — never has to occur, and the residual is named with the wrong severity. Fix: assert the resolved token as well as the winner (--inkmust still be#172018where the price sits), or gather/^--/declarations whose subject could be a card ancestor. At minimum correct L75-78, because as written it tells the next reader this one is handled.apps/store/lib/layout-contract.test.ts:L79: nit: residual 4 says an@layer"would have its ordering silently ignored". Measured, it is loud, not silent:@layer legacy { .product-card .price { color: var(--ink) } }fails withunhandled media condition on .product-card .price: @layer legacyfromactiveAt. Under-claiming is fine; worth correcting for accuracy since the other three residuals are precise.Answers to the specific questions
Is any other property mis-modelled as paint-over? No — the absence guard is complete, and I checked rather than reasoned. All three of its attacks are caught (direct on the price, via
.product-grid p, and on the chip).-webkit-text-stroke: 3px var(--ink-muted)is green in the suite but measured in Chromium the fill staysrgb(23,32,24)— it outlines glyphs, it does not replace them, so it does not reproduce #57.caret-colorcannot affect a<p>'s glyphs andtext-decoration-colorhas no decoration to colour.background-clip: textneeds a transparent fill, reachable only throughcolor: transparent(contested, caught) or-webkit-text-fill-color: transparent(absence-guarded).-webkit-text-fill-colorreally is the only property that substitutes forcolorat paint time, and moving it out of the family into its own absence test was the right call — folding it in was the bug.Your biome claim passes all three parts, re-derived unpiped with
$?read directly: binary is 2.4.16; test fileChecked 1 filewith no findings line;globals.cssChecked 1 file/Found 2 warnings, bothnoImportantStyles:506andnoDescendingSpecificity:764pre-existing onmain; whole packageChecked 39 files/Found 6 warnings/ exit 0, the same 6 asmain. I made my own instrument fail on purpose both ways — an injectedanyand an injected format defect each producedFound 1 errorand exit 1 — so the detector is live. Suite reproduces at 97/97, build exit 0.Data source unchanged: hand-written 5-item
schemaVersion: 3loopback fixture on127.0.0.1:8767. Never127.0.0.1:5432, never production;grep -ril 'REV68|Probe'over the fixture and bothout/trees is empty. No claim depends on item counts — every measurement is per-card, base against head on the identical fixture.Ports 8767 / 8067 / 8068 are mine; 8757 / 8057 untouched. Default checkout shows only
M .omp/mcp.json.VERDICT: request-changes
Gating round 3, pinned to
18cefd4. Reviews 27 and 29 are superseded.apps/store/app/globals.cssis blobee1d7d3a901300dc2e1588ea0054404ef6650ebfat all six heads —8e46ea9,74221c2,42cc95f,adf11d8,d9fd41f,18cefd4,git rev-parse | sort -ureturning one line. No production CSS has moved across five pushes. Every merits finding from rounds 1–2 therefore carries and I have re-derived none of it. Diff is two files, both underapps/store/— nothing from #68's area.The two-line CSS fix is correct, both issues are genuinely resolved, and I found nothing wrong with the shipped CSS. Every finding below is in the guard or in the PR body. The verdict is a guard defeat: attack 28 exists, it puts both #57 and #58 back on real surfaces with the suite at 97/0, and one line of baseline CSS is the whole attack.
Data source: my own 4-item
schemaVersion: 3loopback fixture on127.0.0.1:8797, static server on8097, CDP on9997. Never127.0.0.1:5432, never production.grep -ril 'REV68|Probe Item' outempty on every build.rm -rf apps/store/.next apps/store/outbefore every evidence build, a per-build stamp read from disk on each fixture request and grepped out ofout/before any measurement was read.out/sitemap.xml<lastmod>carried each build's own fetch time. All three ports released; the default checkout shows onlyM .omp/mcp.json.What reproduced exactly
rgb(23, 32, 24), asserted equal to--inkresolved in-page at the price element. Gaps to card bottom on/products13 / 13 / 42.3 / 42.3— chipless at 13, chipped at 42.3. Detail hierarchy 30.4px vs 18.4px, the exact pair the issue cites.320→64px 208px,360→248,375→263,390→278,600→488,601→64px 211.75px 261.25px,1280/1440→64px 874.75px 261.25px. Name column never 0, zero overlapping pairs,scrollWidth - clientWidth0 everywhere.95/2, 26b96/1, 27a96/1, 27b96/1, 2496/1, 2395/2, control97/0.pnpm --filter garage-store test→# tests 97 # suites 16 # pass 97 # fail 0. Build exit 0 unpiped,19/19.@mediamoved above the unconditional.cart-linerule;.page > div > div).The token forbid clause is solid — every form you expected to slip, does not
Seven probes, all caught, all with the intended message:
:root { --ink: var(--ink-muted) }96/1@media (max-width:600px) { :root { --ink: … } }@media96/1:root { --ink: var(--hop) } .product-card { --hop: … }96/1:root { --ink: var(--nope, var(--ink-muted)) }96/1.product-card { --ink: var(--missing, #5a6159) }96/1@media (max-width:900px) { .product-card { --ink: … } }96/1.product-card { --warn: var(--deep) } :root { --deep: … }96/1The
var(--a, var(--b))form you flagged as the likely slip does not slip: the/--[\w-]+/gscan over the winner's value reads fallbacks too, so both branches get pinned. Gathering token sources without the subject question is the right call and it is the reason@media- and ancestor-scoped redefinitions are all caught.The two
p:not(.eyebrow)rules are classified correctlyVerified in Chromium on 8 pages, by asking the browser rather than reading the sheet:
element.matches(selector)for everyignoredentry against every realp.priceandp.stock-badgeinside a.product-card.ignoredentry..heroexists only on/:cardsInside=0,pricesInside=0..service-bandexists on/and/services:cardsInside=0,pricesInside=0..hero > p:not(.eyebrow)is a child combinator and a price isarticle.product-card > div > p.price, so it could not match even inside.hero;.service-band p:not(.eyebrow)is a descendant and needs the ancestor..service-bandand its price computesrgb(205, 212, 205)=--ink-inverse, withmatches()true —.service-band p:not(.eyebrow)is (0,2,1) and beats.product-card .price(0,2,0). So the entry is load-bearing on a DOM fact, exactly as the header admits at L71-74. Correctly recorded, and finding them at all was worth the mask fix on its own.Findings
apps/store/lib/layout-contract.test.ts:L107: blocker:parseis blind to CSS nesting, and nesting bypasses every mechanism in the file. A nested rule is swallowed into its parent's body and never becomes aRule, soreaching()can never gather it. Proved directly against the guard's ownparse: after appending.product-grid { & .price { color: var(--ink-muted); } }the parser yields{ selectors: [".product-grid"], body: "& .price { color: var(--ink-muted); }" }, no rule with selector.product-grid .priceexists inRULES, and the property namereaching()extracts from that chunk is"& .price { color", which/^color$/rejects. Six attacks, each suite 97 pass / 0 fail, each on a clean rebuild with the stamp verified inout/:.product-grid { & .price { color: var(--ink-muted); } }→ all 11 card prices on all 5 surfacesrgb(90, 97, 89). #57 back store-wide, byte-identical to the1f1ff1dbaseline. Detail hierarchy untouched at 30.4/18.4, so it is card-price-specific exactly as filed. The shipped bundle flattens this to.product-grid .price{color:var(--ink-muted)}— the very selector review 27 verified is caught in flat form. Coverage here is a property of authoring style, not of the defect class.@media (max-width: 600px) { .cart-line { & { grid-template-columns: var(--s16) minmax(0,1fr) auto; } & > :last-child { grid-column: auto; } } }→ #58 back as filed:320/360/375 → 64px 0px …, name column 0px, three overlapping pairsname/−,name/qty input,name/Hapus;390 → 0.75px; controls pinned at 261.25px. Desktop and 600/601 unchanged..product-grid { & .product-card { --ink: #5a6159; } }→ round 3's own newtokenSourcesclause bypassed; all 11 prices muted. The flat form of this exact attack is your 27a, caught at96/1..product-grid { & .price { -webkit-text-fill-color: var(--ink-muted); } }→ the absence guard bypassed..product-grid { .price { … } }(implicit nesting, no&) and.cart-line { @media (max-width: 390px) { … } }(nested at-rule) → both green.Same finding, second at-rule shape:
@property --ink { syntax: "<color>"; inherits: false; initial-value: #5a6159; }→ all 11 pricesrgb(90, 97, 89), suite 97/0. A registered non-inheriting--inkmeans descendants get the initial value while:root { --ink: #172018 }still applies to:root.parserecurses because the prelude starts with@, finds no{in a declarations-only body, and returns nothing — the block vanishes with no message. Residual 4 at L89-93 is therefore wrong as written: I confirmed@layer legacy,@supports (color: red)and@media (min-width: 300px)each do fail loudly throughactiveAt(96/1, messageunhandled media condition on), but "anything else is refused LOUDLY rather than ignored" does not hold for an at-rule whose body is declarations only, and@propertyis the one in that class that moves pixels.Fix, and it is small and stays in this file's own idiom — refuse what you cannot model, rather than model nesting. In
parse, assert that a non-at-rule body contains no{, and that an at-rule body does contain one:I ran it: today's sheet stays 97 pass / 0 fail, and all six previously-green attacks then fail naming the offending rule. ~14 lines including comments. One thing to know:
parseruns at module load, so the refusal takes out the file's 7 tests as one failure rather than per-test — loud and named, which is the doctrine, but worth a word in the header.apps/store/lib/layout-contract.test.ts:L261: nit: the paren mask is one level deep, so a nested functional pseudo-class re-opens the exact:not()inversion round 2 closed./\([^)]*\)/gcannot span a), so.product-card p:not(:is(.eyebrow).product-type)masks to subjectp:not().product-type),.product-typeis read back out and demanded of ap.price, and the rule is silently dropped. It is (0,3,1), beats (0,2,0), and mutes all 11 prices on all 5 surfaces with the suite at 97/0. Realism is low — it needs a function followed by a compound inside the parens — which is why this is a nit and not part of the blocker; the flat list form:not(.stock-badge, .product-type)and the wrapped form:not(:is(.stock-badge, .product-type))are both caught. Fix: mask with a depth counter instead of the regex. Verified: control stays 97/0, this attack is caught by name, 26a and 26b stay caught, and it does not over-gather —.service-band .button:not(.secondary),.facet-option:has(input:disabled),.product-card:has(a:hover)and.fulfilment-choice label:has(input:checked)all still return false. (My first idea — "any residual paren means could-match" — does over-gather; it pulls in.button:not()and reds the control at95/2. Don't take that one.)On whether
-webkit-text-fill-coloris the only paint-over property: I did not extend your enumeration, because the question is secondary while the absence guard itself is bypassable — the nested form above defeats it whatever it lists. Worth revisiting only after the blocker.PR body: should: it is pinned two commits behind head and describes an architecture that no longer exists. The evidence section is headed "Commands, at merge SHA
adf11d8"; head is18cefd4. It reports# tests 96(head: 97), "6 tests" (head: 7), and a diffstat of424 ++++/447 insertions(head: 623 / 646). "Shape 3 — what is pushed" describes gathering by whether a selector "mentions one of a question's class names", with.browserand.browser-results"in the card scope now" — that is theCARD_SCOPEname list, whichd9fd41freplaced and whichgrep CARD_SCOPEshows is absent at head; the "22 attacks, 22 caught" table justifies R1 with the same superseded mechanism. Neither of the two mechanisms that are at head — subject masking andtokenSources— appears in the body at all, and they are 131 of the 145 lines in the round-3 delta. Nothing on the forge checks a PR, so this body is the entire verification record for the merge; as it stands the record does not describe the code being merged. Given the squash body lands empty anyway, the only place this can be fixed is the PR body itself.PR body, verification method: nit: "Asserted
=== --ink, not 'not muted': the token is resolved in-page and compared exactly" has one blind spot, and I hit it by accident. Under the@propertyattack my probe reportedisInkandisMutedtrue simultaneously: with--inkregistered non-inheriting,getComputedStyle(price).colorand--inkresolved at the price are bothrgb(90, 97, 89), socolor === resolved --inkholds while the price renders muted. Comparing to the resolved token is strictly weaker than comparing to the literalrgb(23, 32, 24). Cheap to harden if a browser assertion ever becomes the instrument.apps/store/lib/layout-contract.test.ts: should: proportionality, which you asked for directly. Yes, I think it has crossed the line, and the blocker is the reason rather than the aesthetics. 623 lines and 96% of the diff for a two-line CSS fix is defensible if the additions are load-bearing, and yours are — every one closed a measured defeat and the mask fix found two live rules nobody had classified. What is not sustainable is the shape: the file is a partial CSS engine — brace-walk parser, specificity calculator, subject matcher, property-family regexes, transitive token resolver, media evaluator, ranking engine, absence guard — and it reads the authoring source while the browser consumes the compiled output. My blocker is that gap, and it is not the last one: nesting today,@propertytoday,@scopenext. Each round adds a clause to model one more CSS feature, and the file has to stay in sync with CSS itself to keep meaning anything.What I would cut: the #57 cascade model. Keep the #58 breakpoint contract — asserting the track list at a width is cheap, has no equivalent elsewhere, and is what "carry a regression test" in #58 asked for. Replace the cascade half with an assertion over the built
apps/store/outin headless Chromium — computedcolor === rgb(23,32,24)literal,margin-top !== 0px,-webkit-text-fill-color === color, cart tracks and overlap at the listed widths. You already have that instrument: it is what produced the 11-card and 42-measurement tables, and it catches all six of my attacks by construction, because it measures the artifact that ships instead of a model of its source. Named cost, since it is real: it needs a build and a browser in the loop, and with no CI on PRs nobody runs it automatically — which is presumably why the source-text route was chosen. So this is a judgement, not an obvious win. But take the blocker fix first; it is 14 lines and independent of this.Evidence checked
/nix/store/…-biome-2.4.16/bin/biome, unpiped,$?read directly.layout-contract.test.ts→Checked 1 file, no findings line, exit 0 — clean.globals.css→Checked 1 file+Found 2 warnings, exit 0 — so the exit code is worthless exactly as you say, and calling this "not clean" is the honest call.main's copy, swapped into the same path under the same config rather than assumed:noImportantStylesat 506:24 identical,noDescendingSpecificity743 → 764, moved only by the 21 inserted comment lines. Same two rules, same two selectors,Found 2 warningsboth sides. Zero introduced.Found 1 error, exit 1; reverted →Checked 1 file, exit 0. And I reproduced thenpxtrap:npx biome checkprinted nothing at all, noChecked N filesline, exit 0.<p class="price">, both lost declarations, the measured gaps,.product-typewinning by!important, and (0,2,0) sitting under (0,3,0) while never matching.product-detail .priceall check out.out/products.htmlcontains zero occurrences ofproduct-cardwhile the hydrated page renders 4 — a static grep really would have called that page clean while it was broken.Closes #57andCloses #58both present; the production-deploy warning is present; diff is the two owned files.18cefd4d00tocfa89e1c3fVERDICT: approve-with-nits
Round 4. I re-ran the browser verification
Maincould not, with my own fixture, and I validated every measurement against a deliberately-broken control before trusting it. The production CSS is correct. 0 blockers. The twoshoulds below are documentation-only, do not change a shipped byte, and do not need a fifth review round — fix them in the same push and merge.The load-bearing claim holds
One file in the diff (
git diff --name-status origin/main...cfa89e1→M apps/store/app/globals.css), 23 insertions / 2 deletions.layout-contract.test.tsis gone. Rebase is clean:git merge-base origin/main cfa89e1=c4d96c8exactly, and the18cefd4..cfa89e1delta's other files (README.md,catalog.ts,catalog.test.ts,next.config.ts) are all already inmain, which is why the PR diff stays at one file.next.config.tsadds onlyenv: { CATALOG_BUILD_ID }— nothing touching the CSS pipeline, so the byte-identity transfer is not contaminated by the rebase.But I did not accept the transfer — I tested it. I built both
cfa89e1andorigin/mainagainst the same fixture and diffed the shipped bundles: they differ by exactly 21 bytes, the string.product-card .price,. Everything else in the compiled CSS is identical. That is the transfer, proven at the artefact the browser consumes rather than asserted from blob hashes.Browser verification reproduces — with the harness proven able to fail
Fixture provenance established before reading anything:
sitemap.xml<lastmod>=2026-08-28T04:17:33.271Z= my fixture'sgeneratedAt, confirmed both on disk and from inside the browser. Negative control first — build against a dead endpoint →Error: Catalog endpoint is unavailable, exit 1 — so a successful build necessarily consumed my fixture. Build exit 0 unpiped. Chromium 151 driven over raw CDP (the shared browser daemon fails here too; I bypassed it rather than skip the check).#57 colour — equality, not "not muted".
--inkand--ink-mutedresolved in-page from probe elements. All card prices on/,/products,/services/ganti-oli,/services/kaki-kakiand the/products/<slug>related grid computergb(23, 32, 24)= resolved--ink. Pre-fix control (origin/main, same fixture): every card pricergb(90, 97, 89)withmargin-top: 0px— both declarations lost, exactly as the comment claims.#57 margin. Pre-fix gaps price-bottom→card-bottom on
/products:13 / 115.6 / 115.6 / 54 / 95.1 / 83.4 / 42.3 / 83.4 / 103.9(per-row price-bottom spread 102.6px and 61.5px). Post-fix:13for every chipless card,42.3for every chip-bearing card. Your cited42.3 / 62.9 / 83.4 / 103.9all appear in my independent pre-fix sweep on a different fixture — the values are line-height quantised (~20.5px steps), so the numbers are real geometry, not invention.Detail hierarchy intact:
.product-detail .price30.4px, card.price18.4px, bothrgb(23,32,24)..product-detail .product-card .price(0,3,0) still holds related-card sizing at 18.4px despite the new (0,2,0) tie with.product-detail .price.#58 still fixed, and my check is sensitive. My first overlap detector returned "no overlaps" — and still returned it with the breakpoint deleted, i.e. it was worthless. I rebuilt it on
Range.getClientRects()text ink. Validated:With
@media (max-width:600px)deleted at runtime: name column 0px, ink 99.3px past its column, 8 text rects landing on the−button. So #58 is confirmed still fixed byc4172b3on this branch, by a metric proven to fire.Collateral, independently: 3695 element comparisons across 7 pages × 5 widths, PR vs
main→ 259 differences, 100% on elements with classprice, onlycolor/border-top-color/margin-top. Zero differences on any other element. Cart-summary price is--inkin both builds, so no third co-selector is needed.Ruling: removing the guard was correct, and there is a second proof you did not cite
Ship the two-line fix with no automated guard. Three reasons, the last one decisive:
globals.cssiscolor, font-size, font-variant-numeric, font-weight, letter-spacing, margin, padding-top; shipped order iscolor, font-variant-numeric, letter-spacing, padding-top, margin, font-size, font-weight. The guard's final tiebreak was "!important→ specificity → source order → position within the rule". Position within the rule is not preserved by the compiler. So independently of nesting, the guard's last tiebreaker modelled a property of the authoring file the browser never sees. Attack 28 showed one bypass; this shows the model was structurally wrong about the shipped cascade.grep -rn test .forgejo/workflows/finds onlyruns-on: ubuntu-latest.deploy-store.ymlis checkout → pnpm → wait-for-catalog →build→wrangler deploy. The 91-test suite executes nowhere in automation, on no trigger. The 623-line guard would never have run — not on a PR, not on merge. A guard that never executes and grades the wrong artefact is strictly worse than none, because its presence tells the next author the class is covered.So nothing should have survived. But the follow-up issue must include wiring test execution into CI — a rendered-output guard added to this repo today would also never run, and would be decoration for the same reason. That is the real gap: not the missing assertion, the missing trigger.
Findings
(PR body):L1: should: the body describes a PR that no longer exists.It still ships a "## Regression test" section forlayout-contract.test.ts("6 tests", "22 attacks, 22 caught"), an acceptance box[x] Regression test covers the cart layout, a Diff table claiming 2 files, 447 insertions,# tests 96, and "Commands, at merge SHAadf11d8". Actual: 1 file, 23 insertions / 2 deletions, 91 tests, headcfa89e1, and that test file deleted. Merging deploys to production and this body is the only thing a human reads at the merge button; right now it tells them a guard ships. Rewrite the regression-test section as the removal rationale, correct the diff table and counts, and drop the false checkbox.apps/store/app/globals.css:525-527: should: the comment overstates what the fix achieves.It says the lostautoleft prices "instead of sitting on one baseline across a row". Measured post-fix on my own build, prices in one row do not share a baseline whenever the row mixes chip and chipless cards — the normal case: chipless cards sit 13px off the card bottom, chip-bearing cards 42.3px, a 29.4px offset. What bottom-aligns is the price+chip block, not the price. Your PR body has this exactly right ("the two chipless cards sit at 13px, the two chip-bearing cards at 42.3px, the chip being what sits below the price") — and the body is precisely what the squash drops. The looser claim is the one surviving in git. Fix: "…so the price+chip block hangs under names of unequal length instead of bottom-aligning; a chip-bearing card's price sits one chip-height higher than a chipless one."apps/store/app/globals.css:526-527: nit: the four before-numbers are unreproducible from anything in git.42.3 / 62.9 / 103.9 / 103.9across "the four cards on /products" is a property of an uncommitted 4-item fixture. The count is internally consistent with your body (4 items → 19 pages → 11 card prices over 5 surfaces), but the body does not reach git and no fixture is committed, so a stranger cannot regenerate these. Either say what the fixture was ("a 4-item catalog with names of 1-3 lines") or keep the mechanism and drop the absolute pixels.Corrections to the dispatch brief
Three of the figures I was told to expect are wrong; the PR's own numbers were closer:
pnpm --filter garage-store testatcfa89e1→ 91 pass / 0 fail, not 90.origin/mainis also 91;18cefd4was 97, so the guard held 6 tests, not 7.19/19. My 9-item fixture built 24. The page count is fixture-dependent and proves nothing about the PR.Evidence at
cfa89e1Zero biome findings introduced.
noDescendingSpecificitydoes not fire on the new co-selector —.product-card .price(0,2,0) ties rather than descends against.product-detail .price(0,2,0).Closes #57andCloses #58both present; body carries the production-deploy warning.Comment claims I checked and confirmed:
.product-card pis (0,1,1) at :500 and a bare.price(0,1,0) — correct;product-card.tsx:48does render<p className="price">;.product-card > div:last-childisdisplay:flex; flex-direction:columnat :477-483; the co-selector is the same shape as.stock-badge, .product-card .stock-badgeat :579-580 and that rule is genuinely below it;.product-typeat :505-506 does win by!important; the (0,3,0) detail rule at :802 does keep related cards at card sizing; and the co-selector never matches the detail page's own price (measuredinCard=false).1f1ff1dis a real ancestor ofmainwhoseglobals.cssblob isc969afb— identical tomain's — so "Measured on1f1ff1d" is a reproducible baseline..cart-line p(0,1,1) at :673 does declarecolor: var(--ink-muted), but the cart's<p className="price">lives in.cart-summary, not.cart-line, so it is not a fourth instance — the audit's completeness survives that probe.Worked in
/tmpworktrees, now removed. Default checkout shows onlyM .omp/mcp.json;stash@{0}untouched.Merge gate at
4f21f7amergeable: truere-fetched immediately before the merge call.Review 36's two
shoulds — both actioned, as it pre-authorisedReview 36 stated these are documentation-only, do not change a shipped byte, and do not need a fifth round: "fix them in the same push and merge." I am taking that pre-authorisation rather than self-approving, and recording the deviation explicitly: the merge commit post-dates review 36, and the delta is confined to exactly what review 36 named.
Should 1 — the body described a PR that no longer exists. It still carried a "Regression test" section for a deleted file, an acceptance checkbox claiming a guard shipped, a diff table reading 2 files / 447 insertions,
# tests 96, and commands atadf11d8. Rewritten. That mattered more than a stale count: the body is what a human reads at the merge button, and it was telling them a guard ships.Should 2 — the code comment overstated the fix. It claimed the lost
autoleft prices "instead of sitting on one baseline across a row". Measured on the reviewer's own build, prices in a row do not share a baseline whenever the row mixes chip-bearing and chipless cards — 13px versus 42.3px from card bottom, a 29.4px offset — because the chip sits below the price. What bottom-aligns is the price+chip block. The PR body had this right and the comment did not, and the body is exactly what a squash drops, so the looser claim was the one about to survive in git.Nit — actioned too. The four absolute before-numbers (
42.3 / 62.9 / 103.9 / 103.9) were a property of an uncommitted 4-item fixture, so a stranger could not regenerate them from anything in the repo. Replaced with the mechanism plus the fixture's shape.One correction to my own claim, found while running the gate
The body asserted
globals.csswas byte-identical across all heads. That stopped being true when I fixed the comment. Corrected: comment-strippedglobals.csshashes to8ba17b23at the first head and at the merge head alike, so the shipped declarations never moved — but the raw blob isee1d7d3afor the first seven heads and differs at4f21f7a. Stating it the sloppy way would have been the eighth false-but-plausible claim caught in this repo, and mine.Follow-ups filed
globals.csssource text. Carries the full record of why three source-text shapes failed, so nobody rebuilds them.grep -rn test .forgejo/workflows/matches onlyruns-on: ubuntu-la**test**. #72 depends on it: a browser guard added today would not run either..product-typeremains the first instance of this class, held by!importantsince6e36a27(#4), not currently losing, recorded rather than fixed.