Consolidate remaining CSS only after module extraction #26

New issue

Closed

opened 2026-08-30 21:31:06 +02:00 by kristofferopsahl · 2 comments

kristofferopsahl commented

2026-08-30 21:31:06 +02:00

Collaborator

Child of #7 and intentionally last.

Consolidate remaining repeated base selectors while preserving deliberate adjacent breakpoint overrides.

Acceptance: test_css_characterization.py passes against the existing baseline at every covered tab and width; do not re-baseline merely to make the refactor pass.

Child of #7 and intentionally last. Consolidate remaining repeated base selectors while preserving deliberate adjacent breakpoint overrides. Acceptance: `test_css_characterization.py` passes against the existing baseline at every covered tab and width; do not re-baseline merely to make the refactor pass.

kristofferopsahl referenced this issue

2026-08-30 21:31:07 +02:00

Split oversized modules and consolidate cascading CSS overrides #7

solvreven commented

2026-09-20 01:27:48 +02:00

Owner

Worth knowing before this one is picked up: the acceptance criterion above rested on a gate that could not fail. Repaired on feat/captaincy-board tonight (e32b153, wiki in 05c484e).

What was wrong. test_css_characterization.py hashes every property getComputedStyle exposes for each element, compares the hashes, and re-fetches the differing paths in full so the message can name the property. But getComputedStyle enumerates custom properties in an order the document decides, and the two pages disagree about it: the live page listed --page-max before --fx-5-bg, the served baseline the other way round, with all 539 names and values otherwise equal. Hashing in enumeration order therefore made every element differ, on every tab, at every width. _differences re-fetches the first 25 paths per (tab, width), none of those had a property to show, and the report came back empty.

It surfaced because a real change passed the gate cleanly: a new @media rule folding .builder-layout to one column under 640px, which fixes Squad → Edit squad scrolling sideways by 41px at 390px (fr tracks do not shrink below their content, and that layout had no breakpoint where .xi-layout has one).

The repair.

The hash sorts the property names, so it hashes the properties and not their order.
A hash that differs with no property behind it is now reported as an instrument fault instead of being skipped in silence. That silence is what emptied the report.
test_the_comparison_can_see_a_change now runs the gate's own pair — the live page against served text — instead of two served pages. Two served pages agree about the enumeration order, which is exactly why the proof never saw this.
A new test, test_the_live_stylesheet_and_the_same_bytes_served_agree, asks the gate's question with nothing changed between the pages.

Proved by cutting each piece:

cut	result
`names.sort()` removed	the new test and the gate go red
sort removed and hash-only differences skipped	everything green while `.builder-layout` really differs — the original defect, reproduced
mutation proof back on two served pages	green: that pair catches a 77px padding change too, so the old proof was not wrong, only narrower than the gate's use

With the repair in place the gate found the .builder-layout rule by itself, at 420px on all four tabs. The baseline was then re-based with the same rule, deliberately and with the reason in a comment.

The acceptance criterion here now means what it says. A consolidation checked before tonight was checked by an instrument that could not answer.

🤖 Generated with Claude Code

Worth knowing before this one is picked up: the acceptance criterion above rested on a gate that could not fail. Repaired on `feat/captaincy-board` tonight (`e32b153`, wiki in `05c484e`). **What was wrong.** `test_css_characterization.py` hashes every property `getComputedStyle` exposes for each element, compares the hashes, and re-fetches the differing paths in full so the message can name the property. But `getComputedStyle` enumerates **custom properties in an order the document decides**, and the two pages disagree about it: the live page listed `--page-max` before `--fx-5-bg`, the served baseline the other way round, with all 539 names and values otherwise equal. Hashing in enumeration order therefore made every element differ, on every tab, at every width. `_differences` re-fetches the first 25 paths per (tab, width), none of those had a property to show, and the report came back empty. It surfaced because a real change passed the gate cleanly: a new `@media` rule folding `.builder-layout` to one column under 640px, which fixes Squad → Edit squad scrolling sideways by 41px at 390px (`fr` tracks do not shrink below their content, and that layout had no breakpoint where `.xi-layout` has one). **The repair.** - The hash sorts the property names, so it hashes the properties and not their order. - A hash that differs with no property behind it is now reported as an instrument fault instead of being skipped in silence. That silence is what emptied the report. - `test_the_comparison_can_see_a_change` now runs the gate's own pair — the live page against served text — instead of two served pages. Two served pages agree about the enumeration order, which is exactly why the proof never saw this. - A new test, `test_the_live_stylesheet_and_the_same_bytes_served_agree`, asks the gate's question with nothing changed between the pages. Proved by cutting each piece: | cut | result | |---|---| | `names.sort()` removed | the new test and the gate go red | | sort removed **and** hash-only differences skipped | everything green while `.builder-layout` really differs — the original defect, reproduced | | mutation proof back on two served pages | green: that pair catches a 77px padding change too, so the old proof was not wrong, only narrower than the gate's use | With the repair in place the gate found the `.builder-layout` rule by itself, at 420px on all four tabs. The baseline was then re-based with the same rule, deliberately and with the reason in a comment. The acceptance criterion here now means what it says. A consolidation checked before tonight was checked by an instrument that could not answer. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

solvreven referenced this issue

2026-09-20 11:55:21 +02:00

Squad rail becomes a league-wide captaincy board, star-rated #31

solvreven commented

2026-09-20 16:02:15 +02:00

Owner

Follow-up on the gate itself, since this issue's acceptance criterion rests on it.

The repair in e32b153 holds — but an audit of the running setup the next day found the gate barely runs. 434 of 2124 tests need a browser, and CI installs only the [test] extra, so they skip there. That was deliberate (05530f7: 13-17 minutes of Chromium per push on the shared runner), and both the workflow comment and README.md say to run them locally before pushing frontend changes.

That instruction is the only thing standing between this issue and an unnoticed regression, and nothing enforces it. A frontend-only push with a green CI badge looks identical whether the browser suite was run or not. For a consolidation of this size — where the whole argument is "the computed styles did not move" — that seems worth closing before the work starts, not after.

Two options that keep the runner cost where you put it:

a job that runs the browser suite only when the push touches frontend/** or tests/web/**, so ordinary backend pushes stay at the current cost;
a pre-push hook, which is cheaper still but per-checkout, and CLAUDE.md already argues against guards whose correctness depends on every contributor's local config (that was the .gitattributes reasoning in the CRLF digest case).

I lean towards the path filter: it is one file, it cannot be forgotten, and it costs Chromium only on the pushes that can break the gate.

For the record, the full suite in the live checkout is 2124 passed, 0 skipped as of 7b317df — that number is only reachable on a machine with Playwright and the gitignored data, which is the point above.

🤖 Generated with Claude Code

Follow-up on the gate itself, since this issue's acceptance criterion rests on it. The repair in `e32b153` holds — but an audit of the running setup the next day found the gate barely runs. **434 of 2124 tests need a browser, and CI installs only the `[test]` extra**, so they skip there. That was deliberate (`05530f7`: 13-17 minutes of Chromium per push on the shared runner), and both the workflow comment and README.md say to run them locally before pushing frontend changes. That instruction is the only thing standing between this issue and an unnoticed regression, and nothing enforces it. A frontend-only push with a green CI badge looks identical whether the browser suite was run or not. For a consolidation of this size — where the whole argument is "the computed styles did not move" — that seems worth closing before the work starts, not after. Two options that keep the runner cost where you put it: - a job that runs the browser suite only when the push touches `frontend/**` or `tests/web/**`, so ordinary backend pushes stay at the current cost; - a pre-push hook, which is cheaper still but per-checkout, and CLAUDE.md already argues against guards whose correctness depends on every contributor's local config (that was the `.gitattributes` reasoning in the CRLF digest case). I lean towards the path filter: it is one file, it cannot be forgotten, and it costs Chromium only on the pushes that can break the gate. For the record, the full suite in the live checkout is **2124 passed, 0 skipped** as of `7b317df` — that number is only reachable on a machine with Playwright and the gitignored data, which is the point above. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

solvreven referenced this issue

2026-09-20 16:03:13 +02:00

Small findings from 2026-09-13, each verified on the live page or in code #34

kristofferopsahl referenced this issue from a pull request that will close it,

2026-10-15 21:41:27 +02:00

Split application domains and cache player delivery #43

kristofferopsahl closed this issue

2026-10-15 22:01:06 +02:00