diff --git a/devlog/_plan/260904_codex_set_head_and_logo/040_wp4_regression_coverage.md b/devlog/_plan/260904_codex_set_head_and_logo/040_wp4_regression_coverage.md new file mode 100644 index 00000000000..5b0cfc975a9 --- /dev/null +++ b/devlog/_plan/260904_codex_set_head_and_logo/040_wp4_regression_coverage.md @@ -0,0 +1,123 @@ +# 040 — Regression coverage for the two chrome fixes + +## Why this exists + +Both fixes in #3465 shipped with live proof and zero committed guard. Two +independent reviewers flagged the same hole from opposite directions: + +- The plan auditor found `gui/tests/sidebar-codex-set.test.ts:28` pinning + `Icon: IconKey` and required it be unpinned. It was. +- CodeRabbit then observed that unpinning removed the *only* thing tying that + nav row to any icon at all, and asked for focused coverage of the new mapping. + +Both are right, and they are not in conflict — the mistake would be to answer +the second by undoing the first. The comment already in that file says why: +pinning "the shape of one line" made the test fail "the moment an entry gained a +field — a change it was never written to catch". Re-pinning the whole literal +with a new icon name recreates exactly that. + +The CSS fix has no guard either. `rg -l codex-auth-page-head gui/tests` returns +one file, about toast tone, not layout. A future `flex-wrap` removal would +restore the clip silently. + +## What to assert, and where + +A sibling file, `gui/tests/codex-set-chrome.test.ts`, not an extension of the +sidebar test. That file's subject is routing identity — "always present, never +filtered by view mode", plus the legacy `#codex-auth` bookmark. Icon identity +and page-head layout are a different contract, and `anthropic-pool-card-layout.test.ts` +is the precedent for giving a visual contract its own file with its own +explanation of why each value is what it is. + +Instrument: source-text assertions, matching this suite's convention for +anything layout-shaped. `anthropic-pool-card-layout.test.ts` states the reason +outright — happy-dom performs no layout, so `getBoundingClientRect()` returns +zeros and would prove nothing. The rendered proof for this unit lives in `030` +as real-browser measurements; these tests guard the source that produced it. + +Borrow that file's two safeguards: + +- `withoutComments()` before matching CSS, so an assertion can never pass on + prose quoting the value. This unit's fix ships with a long explanatory comment + that literally contains the words `flex-wrap`, so without stripping, the CSS + assertions would be self-satisfying. This is the single most important detail + here. +- `ruleBody()` anchored at line start, so a selector that also appears indented + inside `@media` is not read off the wrong rule. + +### wp4-a/b — the row wears the mark, asserted through a render + +The first draft of this doc proposed `toContain('Icon: IconCodex')`. A reviewer +rejected it, correctly: that is the *same* fragility the sidebar test documents +removing, one rename later. Renaming the symbol is not a regression; the string +breaks anyway. What must not change is the glyph. + +So the name is read from the NAV row only to *resolve* the component, and every +assertion lands on rendered output: + +1. Strip comments from `App.tsx`, slice the `NAV` table, and capture the + `Icon:` identifier for the `codex-set` row. Source text is forced here — + `NAV` is not exported, so no other instrument can observe the mapping. +2. Resolve that name against the `icons` module namespace and fail with a + readable message if it is not exported. +3. `renderToStaticMarkup` it and assert the geometry. + +Rendering is the right instrument for the second half because the JSX is +multi-line and attribute-ordered — `toContain('strokeWidth={2.484}')` would +break on a reformat. `renderToStaticMarkup` normalizes to +`stroke-width="2.484"` regardless of authoring style. It is also the suite's +*lighter* render instrument: happy-dom is installed but needs five global swaps +plus teardown, and no layout, event, or lifecycle behavior is under test here. + +Geometry is asserted as tokens from the `d` attribute — the ring's radius and +its extreme points, the underscore, the chevron — with whitespace collapsed +first, so a decimal-preserving reflow passes while a redraw fails. + +The specific silent-revert this guards, which the first draft failed to name: +folding the mark back through `S()`. That helper hardcodes a 24-unit box and +stroke 2, which rescales a path drawn for 32 units and still renders something +icon-shaped — review does not catch it, and `stroke-width="2.484"` does not +contain `stroke-width="2"`. Hence the negative assertions on +`viewBox="0 0 24 24"` and `stroke-width="2"`. + +File: `gui/tests/sidebar-codex-mark.test.tsx`, a sibling rather than an +extension. `sidebar-codex-set.test.ts`'s declared subject is the row surviving +the removed viewMode filter; mark identity is a different contract, and this +repository already keeps mark-identity tests in their own files +(`provider-icons`, `client-marks-assets`, `integration-marks`). + +### wp4-c — the page head still wraps + +```ts +expect(ruleBody(css, ".codex-auth-page-head")).toContain("flex-wrap: wrap"); +expect(ruleBody(css, ".codex-auth-page-head__actions")).toContain("flex-wrap: wrap"); +``` + +on comment-stripped CSS. + +## What this does NOT catch + +Worth stating so nobody reads more into a green run than is there: + +- It does not catch upstream drift. Nothing re-fetches `success.html`; this pins + "matches what was copied on 2026-09-04", not "matches upstream today". +- It does not catch a mangled path that happens to retain the asserted tokens. + Closing that needs a full normalized-`d` equality, at real formatting cost — + a deliberate trade. +- It does not prove the sidebar renders the icon at all: the row is read as + source text, so deleting `` from the nav JSX would leave this green. +- It does not prove the head *renders* unclipped. No layout engine runs. A + change to `.page-head`, `.btn`, or the container width could reintroduce a clip + with both `flex-wrap` declarations still present. Only `030`'s browser + measurements prove the rendered outcome, and only for the moment they were taken. +- It does not cover 17px sizing, `currentColor` inheritance, or theme contrast. +- It does not cover the `embedded` variant, which never had the defect. +- It does not cover the other eight nav rows, whose glyphs stay freely swappable. + +## Verification + +`bun test --isolate tests/sidebar-codex-mark.test.tsx tests/codex-set-chrome.test.ts` +plus the existing `tests/sidebar-codex-set.test.ts`, and both new files driven +red once — the mark test by pointing the row at a neighbouring glyph, the CSS +test by removing a `flex-wrap` — because an assertion never seen to fail is not +known to be load-bearing. No repository-wide suite. diff --git a/gui/tests/codex-set-page-head-wrap.test.ts b/gui/tests/codex-set-page-head-wrap.test.ts new file mode 100644 index 00000000000..8a6afd22554 --- /dev/null +++ b/gui/tests/codex-set-page-head-wrap.test.ts @@ -0,0 +1,50 @@ +import { expect, test } from "bun:test"; + +/** + * The Codex Set page head must be able to wrap. + * + * Its action cluster is four nowrap items — the Spark switch, two labelled + * buttons, and the feedback slot. While the head was a single nowrap flex row, + * a narrow viewport pushed the trailing button past its own container, and + * `overflow-x: hidden` on html/body turned that into a clip rather than a + * scrollbar: measured at 850px, "Refresh quotas" ran to x=944 against a + * container ending at 804. + * + * Source-text assertions, not measurements: happy-dom performs no layout, so a + * getBoundingClientRect() here returns zeros and would prove nothing. The + * rendered proof was captured in a real browser and lives in + * devlog/_plan/260904_codex_set_head_and_logo/030_live_verification_record.md. + * What this file guards is the declaration that produced it. + */ +const cssUrl = new URL("../src/styles.css", import.meta.url); + +/** Strip comments so no assertion can pass on prose that quotes a value. */ +function withoutComments(css: string): string { + return css.replace(/\/\*[\s\S]*?\*\//g, ""); +} + +/** + * Body of a top-level rule. Anchored with no leading whitespace on purpose, so a + * selector that also appears indented inside an `@media` block is not read off + * the wrong rule. + */ +function ruleBody(css: string, selector: string): string { + const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const match = css.match(new RegExp(`(^|\\n)${escaped}\\s*\\{([^}]*)\\}`)); + if (!match) throw new Error(`rule not found: ${selector}`); + return match[2]!; +} + +test("the Codex Set page head and its action cluster both wrap", async () => { + const css = withoutComments(await Bun.file(cssUrl).text()); + + // The head wraps so the cluster can drop below the title instead of competing + // with it for one line. + expect(ruleBody(css, ".codex-auth-page-head")).toContain("flex-wrap: wrap"); + + // The cluster wraps so it can break internally when even a full row is not + // enough, staying right-aligned as it does at wide widths. + const actions = ruleBody(css, ".codex-auth-page-head__actions"); + expect(actions).toContain("flex-wrap: wrap"); + expect(actions).toContain("justify-content: flex-end"); +}); diff --git a/gui/tests/sidebar-codex-mark.test.tsx b/gui/tests/sidebar-codex-mark.test.tsx new file mode 100644 index 00000000000..3c7d61bbc4b --- /dev/null +++ b/gui/tests/sidebar-codex-mark.test.tsx @@ -0,0 +1,70 @@ +import { expect, test } from "bun:test"; +import { renderToStaticMarkup } from "react-dom/server"; +import { createElement, type FC, type SVGProps } from "react"; +import * as icons from "../src/icons"; + +/** + * The codex-set nav row wears the Codex mark. + * + * Deliberately not in `sidebar-codex-set.test.ts`: that file's subject is the row + * surviving the removed viewMode filter, and it dropped its own `Icon: IconKey` + * pin for failing on a change it was never written to catch. This file's subject + * IS the glyph, so it is supposed to fail when the glyph changes. + * + * It still does not pin the symbol NAME. A rename is not a regression; wearing a + * key again is. The name is read from the row only to resolve the component, and + * every assertion lands on rendered geometry. + */ +function iconNameForNavRow(src: string, id: string): string { + // Comments naming the icon are prose, not evidence: icons.tsx carries a long + // block comment naming this mark, and App.tsx has comment prose inside