From 1482fc44b907d0799e0d92b924d18d20b9f9cb51 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 23:36:42 +0900 Subject: [PATCH 1/5] fix(gui): correct the record and lock the test constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An independent final-gate review of the merged unit returned NEAR-PASS with five findings. Four were correct; all four are fixed here. Each was re-verified against origin/dev before being accepted. **The 030 citation fix I made in #2911 introduced an error.** Replacing the drifted line numbers, I wrote that ".action-toast (749) and .notice (1215) cap toast width with calc(100vw - Npx)". .notice does not use a viewport unit at all — it caps with var(--prose-measure), which is precisely why it wins the cascade and why the compound selector is needed. The second calc(100vw - Npx) belongs to .toast-notice at 1229. Naming the wrong rule undercut the explanation two paragraphs later. **A stylesheet comment overstated what the test does.** It said the three-line floor is something "the regression test asserts by measuring rendered line counts". The committed test is source-text only and says so in its own header; the line count came from browser measurement recorded in devlog 013. The comment now distinguishes the two. **000 blamed the wrong property.** Its root-cause paragraph attributed the mis-distributed wrapped lines to align-items: center. align-items centres each item within its line; the lines themselves are align-content, which defaults to stretch. That is the whole reason align-content: start ships and align-items stays. Corrected in place with a note, since the original wording is part of how the investigation went wrong. **000 still carried the drifted citations** that #2911 fixed in 030 — styles.css:2003, :755, :1222, :2198, none of which point at the cited rule any more. Now selector-named, with the reason stated. The fifth finding was about test strength, and the reviewer was right that > 0 accepts anything: expect(Number(cap[1])).toBeGreaterThan(0) // 1px design width passes expect(Number(cap[2])).toBeGreaterThan(0) // 1px inset passes Both toast constants and the log-cap subtrahend are now asserted exactly (480, 48, 260). Proved non-vacuous by mutation: rewriting the stylesheet to 479px / 47px / 261px turns both tests red, where the previous assertions passed. The stylesheet was restored byte-identical to origin/dev afterwards (verified by sha256), and the suites are 10 pass / 0 fail. Not changed: the reviewer also flagged that several secondary sidecar assertions can pass on an empty match (no @container rule reaching the inner loop). That is a real weakness but a pre-existing one, unrelated to this unit's fixes, and tightening it means deciding what those queries must contain — a separate change rather than a correction to this record. --- .../000_baseline_and_roadmap.md | 28 +++++++++++++------ .../030_dynamic_viewport_units.md | 10 ++++--- gui/src/styles-dashboard-workspace.css | 8 ++++-- gui/tests/viewport-scroll-caps.test.ts | 13 +++++++-- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md b/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md index d31c31da47..e79960d74b 100644 --- a/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md +++ b/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md @@ -51,10 +51,16 @@ stretches them to equal height. Below ~`36rem` of *card* width the container query gives copy and controls `flex-basis: 100%`, so each card becomes two wrapped flex lines. The two cards then have **equal outer height but different content height** — vision's control column is taller (select row + advanced -disclosure). Flexbox distributes the leftover space of each card independently -and `align-items: center` centres each line within its own leftover, so the -shorter card's control row sinks by half the difference. Nothing ties one card's -second line to the other's. +disclosure). Flexbox distributes the leftover space of each card independently, +so the shorter card's control row sinks. Nothing ties one card's second line to +the other's. + +> **Corrected during implementation.** This paragraph originally blamed +> `align-items: center`. That is the wrong property: `align-items` centres each item +> *within* its line, while the mis-distributed thing is the **lines**, which is +> `align-content` — defaulting to `stretch` on a multi-line flex container. The fix +> is `align-content: start`; `align-items: center` stays and is what keeps the +> single-line (one-column) regime centred. See `013`. The shipped mitigation is a hard-coded reserved band: @@ -89,12 +95,18 @@ third card is added to either grid. ## Defect 3 — static viewport units in scroll surfaces -`gui/src/styles.css:2003` caps `.logs-table-wrap` with +**As measured before the fix.** `.logs-table-wrap` capped with `max-height: calc(100vh - 260px)`. Static `vh` resolves against the *large* viewport, ignoring mobile browser chrome, while the rest of the shell already -uses `100dvh` (styles.css:244, 247, 411, 412, 2198). The log table is therefore -sized for a viewport the user cannot see. `styles.css:755` and `1222` cap toast -width with `calc(100vw - Npx)`, which ignores classic scrollbar width. +used `100dvh` (`.app`, the sidebar, `.main-inner--combos`, the mobile drawer). The +log table was therefore sized for a viewport the user cannot see. `.action-toast` +and `.toast-notice` cap toast width with `calc(100vw - Npx)`, which ignores classic +scrollbar width. + +Rules are named by selector rather than line number on purpose: the fix itself +inserted lines above them, so every original citation (`styles.css:2003`, +`:755`, `:1222`, `:2198`) now lands on unrelated CSS. Current locations are in +`030` and the Outcome section below. The probe measures this behaviourally — comparing each scroll container's computed cap against `visualViewport.height` — rather than grepping for the diff --git a/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md b/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md index 30a6b5891c..6ec1b1ef54 100644 --- a/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md +++ b/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md @@ -23,10 +23,12 @@ rows under the browser UI. The rest of the shell already moved to `100dvh` — `.app` (244), the sidebar (247), `.main-inner--combos` (411-412) and the mobile drawer (2213) — so this line is an outlier, not a convention. -`.action-toast` (749) and `.notice` (1215) cap toast width with `calc(100vw - Npx)`. Per CSS -Values and Units 4, `100vw` includes the classic scrollbar gutter, so a -scrollbar-reserving platform can in principle render a cap wider than the visible -area. +`.action-toast` (749) and `.toast-notice` (1229) cap toast width with +`calc(100vw - Npx)`. Per CSS Values and Units 4, `100vw` includes the classic +scrollbar gutter, so a scrollbar-reserving platform can in principle render a cap +wider than the visible area. (`.notice` at 1215 is a different rule: it caps with +`var(--prose-measure)`, which is what makes it win the cascade below — it does not +use a viewport unit at all.) A separate, *reproduced* toast defect turned up while measuring that one: the cap on `.action-toast` never applied at all. Every toast also carries `.notice`, diff --git a/gui/src/styles-dashboard-workspace.css b/gui/src/styles-dashboard-workspace.css index 9d1d20504b..5cce0e3e6e 100644 --- a/gui/src/styles-dashboard-workspace.css +++ b/gui/src/styles-dashboard-workspace.css @@ -231,8 +231,12 @@ boxes of the hint's own computed line-height. It covers the longest shipped hint, so the shorter hint reserves the same three lines and both control rows start together. Because it scales with font metrics rather than a hard-coded 19.5px, a font or line-height change - cannot invalidate it, and a longer translation only matters if it exceeds three lines — - which the regression test asserts by measuring rendered line counts, not string length. + cannot invalidate it, and a longer translation only matters if it exceeds three lines. + + The committed regression is source-text only (happy-dom performs no layout): it asserts + the floor is expressed in `lh` with at least 3 lines and that the old `rem` band is gone. + The three-line number itself came from rendered measurement in a real browser, which is + recorded in devlog/_plan/260829_gui_dashboard_slop/013, not re-derived by the test. Verified across all eight shipped locales at every two-up width (1600-740): worst offset 0.0px, no hint truncated, no card collapsed. */ diff --git a/gui/tests/viewport-scroll-caps.test.ts b/gui/tests/viewport-scroll-caps.test.ts index a8996cadf5..f95a59017e 100644 --- a/gui/tests/viewport-scroll-caps.test.ts +++ b/gui/tests/viewport-scroll-caps.test.ts @@ -33,7 +33,11 @@ test("the log table caps its scroll height against the dynamic viewport", async // last rows sit underneath the address bar. The rest of the shell (.app, .sidebar, // .main-inner--combos, the mobile drawer) already uses 100dvh, so this rule was the // outlier rather than the convention. - expect(wrap).toMatch(/max-height:\s*calc\(\s*100dvh\s*-/); + // The subtrahend is locked, not just the unit: a `calc(100dvh - )` would + // satisfy a unit-only assertion while silently resizing the table. + const cap = wrap.match(/max-height:\s*calc\(\s*100dvh\s*-\s*([\d.]+)px\s*\)/); + expect(cap).not.toBeNull(); + expect(Number(cap![1])).toBe(260); expect(wrap).not.toMatch(/max-height:\s*calc\(\s*100vh\s*-/); }); @@ -51,9 +55,12 @@ test("the toast width cap outranks the later .notice rule", async () => { // Both halves are asserted on purpose. An earlier revision kept only the design width, // which dropped the viewport term and let the toast reach the screen edge at narrow // widths (measured left = 0 at 430px, losing the 24px inset the right side keeps). + // Exact values, not merely positive ones: a 1px design width or a 1px inset would pass + // a `> 0` check while destroying the layout. 480px is the design width and 48px is the + // 24px inset doubled, both measured on the rendered toast. expect(cap).not.toBeNull(); - expect(Number(cap![1])).toBeGreaterThan(0); - expect(Number(cap![2])).toBeGreaterThan(0); + expect(Number(cap![1])).toBe(480); + expect(Number(cap![2])).toBe(48); // Guard the ordering premise itself: if `.notice` ever moved ABOVE this rule, a // single-class cap would start working and someone could "simplify" the compound From c1f918ef8e268b1ae4ef4c81d64baae799bac736 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 23:41:30 +0900 Subject: [PATCH 2/5] fix(gui): assert the effective declaration, not the first match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second independent final-gate round returned NEAR-PASS: five claims verified, two partial. Both partials were correct. **The overstated claim was mine.** Fixing the drifted citations, I wrote that "every original citation now lands on unrelated CSS". Three of four do — styles.css:2003 is now min-width: 220px, :1222 is a z-index, :2198 is a background — but :755 still lands exactly on .action-toast's max-width. One of four survived by coincidence, and nothing marked which, so the note now says that instead of claiming all four rotted. Same correction in 000 and 030. **The test gap was real and worse than the one I had just fixed.** Locking the constants closed the 479px case but not this one: allRuleBodies() concatenates every body for a selector and .match() takes the FIRST hit, so a second rule for the same selector added later with a wrong value wins the cascade while the original correct declaration still satisfies the assertion. The test would stay green while the toast rendered at whatever the duplicate said. Both assertions now read the effective declaration — the last one across all bodies of the selector — and anchor the regex to the whole value: effectiveDeclaration(css, ".action-toast.notice", "max-width") effectiveDeclaration(css, ".logs-table-wrap", "max-height") Proved against the reviewer's exact scenario. Appending .action-toast.notice { max-width: min(200px, calc(100vw - 8px)); } .logs-table-wrap { max-height: calc(100dvh - 900px); } turns both tests red (Expected: 480, Received: 200) where the previous first-match logic passed. The earlier 479/47/261 value mutation is still caught. gui/src/styles.css was restored byte-identical to origin/dev afterwards, verified by sha256, and the suites are 10 pass / 0 fail. --- .../000_baseline_and_roadmap.md | 9 +++-- .../030_dynamic_viewport_units.md | 7 +++- gui/tests/viewport-scroll-caps.test.ts | 39 ++++++++++++++++--- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md b/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md index e79960d74b..732f0f7399 100644 --- a/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md +++ b/devlog/_plan/260829_gui_dashboard_slop/000_baseline_and_roadmap.md @@ -104,9 +104,12 @@ and `.toast-notice` cap toast width with `calc(100vw - Npx)`, which ignores clas scrollbar width. Rules are named by selector rather than line number on purpose: the fix itself -inserted lines above them, so every original citation (`styles.css:2003`, -`:755`, `:1222`, `:2198`) now lands on unrelated CSS. Current locations are in -`030` and the Outcome section below. +inserted lines above them, so most of the original citations no longer describe +what they pointed at. `:2003` is now `min-width: 220px`, `:1222` is the toast +host's `z-index`, and `:2198` is `background: var(--glass-rail)`. `:755` still +happens to land on `.action-toast`'s `max-width`, which is the point rather than a +reprieve: one of four survived by coincidence, and nothing marks which. Current +locations are in `030` and the Outcome section below. The probe measures this behaviourally — comparing each scroll container's computed cap against `visualViewport.height` — rather than grepping for the diff --git a/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md b/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md index 6ec1b1ef54..24abe812e7 100644 --- a/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md +++ b/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md @@ -6,8 +6,11 @@ > pass found both. > > Rules below are named by selector, not line number: the fix this document -> describes inserted lines above the very rules it cites, so the original -> citations (`styles.css:2003`, `:755`, `:1222`) now land on unrelated CSS. +> describes inserted lines above the very rules it cites, so most of its original +> citations stopped describing what they pointed at — `styles.css:2003` is now +> `min-width: 220px` and `:1222` is a `z-index`. `:755` still lands on +> `.action-toast`'s `max-width` by coincidence, which is why the selector is the +> reference and the line is only a hint. ## Defect diff --git a/gui/tests/viewport-scroll-caps.test.ts b/gui/tests/viewport-scroll-caps.test.ts index f95a59017e..b3d8d38f46 100644 --- a/gui/tests/viewport-scroll-caps.test.ts +++ b/gui/tests/viewport-scroll-caps.test.ts @@ -18,10 +18,34 @@ function withoutComments(css: string): string { /** All bodies for a selector, which may be declared more than once. */ function allRuleBodies(css: string, selector: string): string { + return ruleBodies(css, selector).join("\n"); +} + +/** Every body for a selector, in source order. */ +function ruleBodies(css: string, selector: string): string[] { const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const matches = [...css.matchAll(new RegExp("(^|\\n)\\s*" + escaped + "\\s*\\{([^}]*)\\}", "g"))]; if (matches.length === 0) throw new Error("rule not found: " + selector); - return matches.map((m) => m[2]).join("\n"); + return matches.map((m) => m[2]); +} + +/** + * The declaration that actually wins for a property: the LAST one across all bodies of + * the selector. + * + * Concatenating bodies and taking the first match is what a reviewer correctly called a + * false negative: a second rule for the same selector, added later with a wrong value, + * wins the cascade while the first (correct) declaration still satisfies the assertion. + * Reading the last occurrence is what makes these tests describe the rendered result. + */ +function effectiveDeclaration(css: string, selector: string, property: string): string { + const pattern = new RegExp(property + "\\s*:\\s*([^;}]+)", "g"); + let winner: string | null = null; + for (const body of ruleBodies(css, selector)) { + for (const m of body.matchAll(pattern)) winner = m[1].trim(); + } + if (winner === null) throw new Error("property not found: " + selector + " { " + property + " }"); + return winner; } test("the log table caps its scroll height against the dynamic viewport", async () => { @@ -34,8 +58,10 @@ test("the log table caps its scroll height against the dynamic viewport", async // .main-inner--combos, the mobile drawer) already uses 100dvh, so this rule was the // outlier rather than the convention. // The subtrahend is locked, not just the unit: a `calc(100dvh - )` would - // satisfy a unit-only assertion while silently resizing the table. - const cap = wrap.match(/max-height:\s*calc\(\s*100dvh\s*-\s*([\d.]+)px\s*\)/); + // satisfy a unit-only assertion while silently resizing the table. Read from the + // EFFECTIVE declaration so a later duplicate rule cannot hide behind this one. + const effective = effectiveDeclaration(css, ".logs-table-wrap", "max-height"); + const cap = effective.match(/^calc\(\s*100dvh\s*-\s*([\d.]+)px\s*\)$/); expect(cap).not.toBeNull(); expect(Number(cap![1])).toBe(260); expect(wrap).not.toMatch(/max-height:\s*calc\(\s*100vh\s*-/); @@ -49,8 +75,11 @@ test("the toast width cap outranks the later .notice rule", async () => { // order therefore won and a single-class `.action-toast` cap never applied - the toast // rendered 542px instead of its design width. Two classes is what wins the cascade, so // the cap must stay on the compound selector. - const compound = allRuleBodies(css, ".action-toast.notice"); - const cap = compound.match(/max-width:\s*min\(\s*([\d.]+)px\s*,\s*calc\(\s*100vw\s*-\s*([\d.]+)px\s*\)\s*\)/); + // Again the EFFECTIVE declaration, for the same reason: a second `.action-toast.notice` + // rule added later with a wrong width would win the cascade while the first one still + // matched a first-occurrence assertion. + const effective = effectiveDeclaration(css, ".action-toast.notice", "max-width"); + const cap = effective.match(/^min\(\s*([\d.]+)px\s*,\s*calc\(\s*100vw\s*-\s*([\d.]+)px\s*\)\s*\)$/); // Both halves are asserted on purpose. An earlier revision kept only the design width, // which dropped the viewport term and let the toast reach the screen edge at narrow From bafca0db73dd659031bba78a9ec1852907904887 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 23:47:43 +0900 Subject: [PATCH 3/5] test(gui): anchor the cap reader and fixture both false negatives Third independent round refuted my previous fix. It was right, and the bug was mine: reading a property with an unanchored name matches inside a CUSTOM PROPERTY, so .logs-table-wrap { max-height: calc(100dvh - 261px); --max-height: calc(100dvh - 260px); } passed while the rendered cap was wrong. Same for --max-width against the toast. Reproduced against the real stylesheet before fixing: 10 pass / 0 fail with both effective declarations wrong. The property must now start the body or follow ; or a newline, so a name prefixed with -- cannot satisfy it. Verified by re-running the reviewer's exact adversarial stylesheet: both tests go red, then green again once it is removed. gui/src/styles.css restored byte-identical to origin/dev (sha256 39425844a41cba50). Also fixtured, so neither false negative can come back quietly. The new test feeds inline CSS to the helper directly: - a custom property whose name contains the read property must NOT be reported; - a later duplicate rule for the same selector MUST win; - a genuinely absent property must throw rather than report a neighbour. Driven red first: reverting only the regex to its unanchored form fails the fixture with Received: calc(100dvh - 260px), the masked custom-property value. The helper's doc comment also overclaimed, and the reviewer was right to flag the prose as well as the code. It said the helper returns the declaration that "actually wins" and describes "the rendered result". It resolves source order within one exact selector string; it does not model !important, competing specificity, or @-rule nesting. The comment now says that, and says why it is sufficient for these two rules: each is declared once, and the cascade question that actually matters -- .notice beating a single-class .action-toast -- is asserted separately in the same file. --- gui/tests/viewport-scroll-caps.test.ts | 55 ++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/gui/tests/viewport-scroll-caps.test.ts b/gui/tests/viewport-scroll-caps.test.ts index b3d8d38f46..ba96cbde38 100644 --- a/gui/tests/viewport-scroll-caps.test.ts +++ b/gui/tests/viewport-scroll-caps.test.ts @@ -30,16 +30,27 @@ function ruleBodies(css: string, selector: string): string[] { } /** - * The declaration that actually wins for a property: the LAST one across all bodies of - * the selector. + * The last declaration of a property across all bodies of an exact selector. * - * Concatenating bodies and taking the first match is what a reviewer correctly called a - * false negative: a second rule for the same selector, added later with a wrong value, - * wins the cascade while the first (correct) declaration still satisfies the assertion. - * Reading the last occurrence is what makes these tests describe the rendered result. + * Two false negatives, both found by review and both reproduced before being closed: + * + * 1. Concatenating bodies and taking the FIRST match let a second rule for the same + * selector, added later with a wrong value, win the cascade while the earlier correct + * declaration still satisfied the assertion. Hence reading the last occurrence. + * 2. An unanchored property name matched inside a CUSTOM PROPERTY, so + * `max-height: calc(100dvh - 261px); --max-height: calc(100dvh - 260px)` passed with + * the rendered cap wrong. Hence the boundary below: the property must start the body + * or follow `;`/newline, and must not be preceded by `-`. + * + * Scope, stated because the previous comment overclaimed: this is source-order within one + * exact selector string. It does not model `!important`, competing selectors of different + * specificity, or @-rule nesting. For these two rules that is enough - each is declared + * once, and the cascade question that matters (`.notice` beating `.action-toast`) is + * asserted separately below. */ function effectiveDeclaration(css: string, selector: string, property: string): string { - const pattern = new RegExp(property + "\\s*:\\s*([^;}]+)", "g"); + const escaped = property.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const pattern = new RegExp("(?:^|[;{\\n])\\s*" + escaped + "\\s*:\\s*([^;}]+)", "g"); let winner: string | null = null; for (const body of ruleBodies(css, selector)) { for (const m of body.matchAll(pattern)) winner = m[1].trim(); @@ -99,3 +110,33 @@ test("the toast width cap outranks the later .notice rule", async () => { expect(compoundIndex).toBeGreaterThanOrEqual(0); expect(noticeIndex).toBeGreaterThan(compoundIndex); }); + +test("the cap reader is not fooled by a custom property or an earlier duplicate", () => { + // Both of these are regressions, not hypotheticals: each passed a previous revision of + // this file while the rendered cap was wrong, and each was reproduced against the real + // stylesheet before being closed. The fixture is inline so the guard is testable without + // touching gui/src/styles.css. + + // A custom property whose NAME contains the property being read. Reading `max-height` + // without a declaration boundary matched `--max-height` and reported the good value + // while the real declaration was 261px. + const masked = [ + ".logs-table-wrap {", + " max-height: calc(100dvh - 261px);", + " --max-height: calc(100dvh - 260px);", + "}", + ].join("\n"); + expect(effectiveDeclaration(masked, ".logs-table-wrap", "max-height")).toBe("calc(100dvh - 261px)"); + + // A later duplicate rule for the same selector wins the cascade. Taking the FIRST match + // reported the earlier correct value. + const duplicated = [ + ".action-toast.notice { max-width: min(480px, calc(100vw - 48px)); }", + ".action-toast.notice { max-width: min(200px, calc(100vw - 8px)); }", + ].join("\n"); + expect(effectiveDeclaration(duplicated, ".action-toast.notice", "max-width")).toBe("min(200px, calc(100vw - 8px))"); + + // A property that genuinely is not there must throw rather than silently report a + // neighbouring declaration. + expect(() => effectiveDeclaration(".logs-table-wrap { overflow-y: auto; }", ".logs-table-wrap", "max-height")).toThrow(); +}); From e1e048810a91ecaa2a95dc57c0eead6a70fffe0e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 23:54:44 +0900 Subject: [PATCH 4/5] test(gui): match property names as CSS does, and fix a misleading heading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth independent round. It confirmed the custom-property hole is closed and that both fixtures genuinely guard their regressions, then found two more things. **CSS property names are case-insensitive, and mine was not.** `MAX-HEIGHT` is a real declaration that wins the cascade, but a case-sensitive literal match walked past it and reported the earlier lowercase value. The reviewer verified in a browser that Chrome honours both `MAX-HEIGHT` and the escaped `max\2d width`. Matching is now case-insensitive. Escaped identifiers are refused rather than skipped. `max\2d height` IS `max-height`, and resolving it properly means CSS identifier unescaping — more machinery than a source-text oracle should pretend to have. Nothing in this stylesheet writes one, so its appearance means the reader no longer understands the file, and failing loudly is the honest response. Silently ignoring it is what produced the false green. Both cases are fixtured, and both were driven red against the real stylesheet first: appending an uppercase duplicate fails 2 tests, appending an escaped one fails 1. Previously each passed 3/0. **The 030 heading was misleading.** It read "(line 2011 as shipped)" directly above the pre-fix `100vh` snippet, so the shipped code appeared to be the defective code. Line 2011 is where the rule sits now, and it carries `100dvh`. Now says so explicitly. Also narrowed the helper's own description: it finds the last TEXTUAL declaration matched on the canonical spelling, which is what it does, rather than "the last declaration" as though it resolved CSS semantics. Suites 12 pass / 0 fail (34 expects). gui/src/styles.css byte-identical to origin/dev after every probe, verified by sha256. --- .../030_dynamic_viewport_units.md | 3 +- gui/tests/viewport-scroll-caps.test.ts | 46 ++++++++++++++++--- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md b/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md index 24abe812e7..67b3461b96 100644 --- a/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md +++ b/devlog/_plan/260829_gui_dashboard_slop/030_dynamic_viewport_units.md @@ -14,7 +14,8 @@ ## Defect -`.logs-table-wrap` in `gui/src/styles.css` (line 2011 as shipped): +`.logs-table-wrap` in `gui/src/styles.css` — **as it was before the fix**; the +rule now sits at line 2011 and carries `100dvh`: ```css .logs-table-wrap { max-height: calc(100vh - 260px); } diff --git a/gui/tests/viewport-scroll-caps.test.ts b/gui/tests/viewport-scroll-caps.test.ts index ba96cbde38..7e991ecf96 100644 --- a/gui/tests/viewport-scroll-caps.test.ts +++ b/gui/tests/viewport-scroll-caps.test.ts @@ -30,7 +30,8 @@ function ruleBodies(css: string, selector: string): string[] { } /** - * The last declaration of a property across all bodies of an exact selector. + * The last *textual* declaration of a property across all bodies of an exact selector, + * matched on the property's canonical lowercase spelling. * * Two false negatives, both found by review and both reproduced before being closed: * @@ -42,17 +43,29 @@ function ruleBodies(css: string, selector: string): string[] { * the rendered cap wrong. Hence the boundary below: the property must start the body * or follow `;`/newline, and must not be preceded by `-`. * - * Scope, stated because the previous comment overclaimed: this is source-order within one - * exact selector string. It does not model `!important`, competing selectors of different - * specificity, or @-rule nesting. For these two rules that is enough - each is declared - * once, and the cascade question that matters (`.notice` beating `.action-toast`) is - * asserted separately below. + * CSS property names are case-insensitive, and an identifier may be written with escapes + * (`max\\2d height` is `max-height`). A case-sensitive literal match therefore reported the + * wrong winner when the real declaration used `MAX-HEIGHT`. Matching is now case-insensitive, + * and an escape in the property name is rejected outright rather than silently skipped: + * nothing in this stylesheet writes one, so its appearance means the oracle no longer + * understands the file and should fail loudly instead of guessing. + * + * Scope, stated because an earlier version of this comment overclaimed: this is source + * order within one exact selector string. It does not model `!important`, competing + * selectors of different specificity, or @-rule nesting. For these two rules that is + * enough - each is declared once, and the cascade question that matters (`.notice` beating + * a single-class `.action-toast`) is asserted separately below. */ function effectiveDeclaration(css: string, selector: string, property: string): string { const escaped = property.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const pattern = new RegExp("(?:^|[;{\\n])\\s*" + escaped + "\\s*:\\s*([^;}]+)", "g"); + const pattern = new RegExp("(?:^|[;{\\n])\\s*" + escaped + "\\s*:\\s*([^;}]+)", "gi"); let winner: string | null = null; for (const body of ruleBodies(css, selector)) { + // An escaped identifier would need CSS unescaping to compare; refuse rather than + // report a value this reader cannot prove is the winner. + if (/\\[0-9a-fA-F]/.test(body)) { + throw new Error("escaped property identifier in " + selector + "; this reader cannot resolve it"); + } for (const m of body.matchAll(pattern)) winner = m[1].trim(); } if (winner === null) throw new Error("property not found: " + selector + " { " + property + " }"); @@ -140,3 +153,22 @@ test("the cap reader is not fooled by a custom property or an earlier duplicate" // neighbouring declaration. expect(() => effectiveDeclaration(".logs-table-wrap { overflow-y: auto; }", ".logs-table-wrap", "max-height")).toThrow(); }); + +test("the cap reader survives case variants and refuses escaped identifiers", () => { + // CSS property names are case-insensitive, so `MAX-HEIGHT` is a real declaration and won + // the cascade while a case-sensitive reader reported the earlier lowercase value. Both of + // these were demonstrated in a browser before being closed here. + const shouted = [ + ".logs-table-wrap {", + " max-height: calc(100dvh - 260px);", + " MAX-HEIGHT: calc(100dvh - 261px);", + "}", + ].join("\n"); + expect(effectiveDeclaration(shouted, ".logs-table-wrap", "max-height")).toBe("calc(100dvh - 261px)"); + + // An escaped identifier (`max\\2d height` is `max-height`) would need CSS unescaping to + // compare. Rather than skip it and report a value it cannot prove is the winner, the + // reader fails loudly - nothing in this stylesheet writes one. + const escapedIdent = ".logs-table-wrap { max-height: calc(100dvh - 260px); max\\2d height: calc(100dvh - 261px); }"; + expect(() => effectiveDeclaration(escapedIdent, ".logs-table-wrap", "max-height")).toThrow(/escaped/); +}); From 9387ad84490961392d1400944fbc70ee4f03f33d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 30 Aug 2026 00:00:01 +0900 Subject: [PATCH 5/5] test(gui): scope the escape guard to property-name position MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 5 passed the branch and noted one loose end I had left: the escape guard scanned the whole rule body, so it would also have rejected an escape in a VALUE. `content: "\2014"` is ordinary CSS and says nothing about which declaration wins — a false failure is as broken an oracle as a false pass, and this one would have fired on a stylesheet that was perfectly fine. The guard is now anchored the same way the matcher is: body start or after `;`/newline, then a name containing a hex escape, then a colon. A value escape passes through; a property-name escape still throws. Both directions fixtured and driven red first. With the broad guard restored, the new value-escape test fails; with the anchored guard, appending `max\2d height` to the real stylesheet still fails as it should. Suites 13 pass / 0 fail (35 expects). gui/src/styles.css byte-identical to origin/dev throughout. --- gui/tests/viewport-scroll-caps.test.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/gui/tests/viewport-scroll-caps.test.ts b/gui/tests/viewport-scroll-caps.test.ts index 7e991ecf96..e760b000c3 100644 --- a/gui/tests/viewport-scroll-caps.test.ts +++ b/gui/tests/viewport-scroll-caps.test.ts @@ -60,10 +60,15 @@ function effectiveDeclaration(css: string, selector: string, property: string): const escaped = property.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const pattern = new RegExp("(?:^|[;{\\n])\\s*" + escaped + "\\s*:\\s*([^;}]+)", "gi"); let winner: string | null = null; + // Only an escape in PROPERTY-NAME position defeats this reader. Scanning the whole body + // would also reject an escape in a value - `content: "\\2014"` is ordinary CSS - so the + // guard is anchored the same way the matcher is: body start or after `;`/newline, then a + // name containing a hex escape, then a colon. + const escapedName = /(?:^|[;{\n])\s*[-\w]*\\[0-9a-fA-F]/; for (const body of ruleBodies(css, selector)) { - // An escaped identifier would need CSS unescaping to compare; refuse rather than - // report a value this reader cannot prove is the winner. - if (/\\[0-9a-fA-F]/.test(body)) { + // An escaped identifier would need CSS unescaping to compare; refuse rather than report + // a value this reader cannot prove is the winner. + if (escapedName.test(body)) { throw new Error("escaped property identifier in " + selector + "; this reader cannot resolve it"); } for (const m of body.matchAll(pattern)) winner = m[1].trim(); @@ -172,3 +177,16 @@ test("the cap reader survives case variants and refuses escaped identifiers", () const escapedIdent = ".logs-table-wrap { max-height: calc(100dvh - 260px); max\\2d height: calc(100dvh - 261px); }"; expect(() => effectiveDeclaration(escapedIdent, ".logs-table-wrap", "max-height")).toThrow(/escaped/); }); + +test("an escape in a VALUE is ordinary CSS and must not trip the guard", () => { + // The escape guard exists for property NAMES. Scanning the whole rule body would also + // reject `content: "\\2014"`, which is ordinary CSS and says nothing about which + // declaration wins - a false failure is as much a broken oracle as a false pass. + const valueEscape = [ + ".logs-table-wrap {", + " content: \"\\\\2014\";", + " max-height: calc(100dvh - 260px);", + "}", + ].join("\n"); + expect(effectiveDeclaration(valueEscape, ".logs-table-wrap", "max-height")).toBe("calc(100dvh - 260px)"); +});