fix(axes): enforce every guarantee the axis layer states (#20)

[fsecada01]

Closes #20. Phase 1 of epic #19.

Four findings from the reviews of #5 and #7. They were consolidated into one phase because they touch the same two files — axes.py and cf_ui_tailwind_plugin.mjs — and the cross-language parity test binds them together; splitting them meant three rounds of conflicts in the same functions.

They are also the same mistake four times: the axis layer stated properties it did not enforce.

---

§1 — Validate axis token values (was #11, security)

_validate_tokens checked that a token name started with -- and nothing about the value. _block emitted f"{name}: {value};" verbatim, and style_element() wraps that in a <style> element injected into the page.

• Reject values containing ;, {, }, /*, */, or < in _validate_tokens
• Mirror the plugin's UNSAFE_VALUE so the two generators agree, with a test asserting they reject the same inputs
• Confirm no shipped value trips the rule — --cf-font-display carries commas and quotes and survives
• CHANGELOG.md notes it as a breaking change

Two things beyond the bare regex, both needed for "the two generators agree" to be true rather than approximate:

• < was added to the plugin too. The original JS rule was /[;{}]|\/\*|\*\//. < only matters on the Python side (</style> escapes the element), but the parity test compares rejection sets, so both carry the identical rule.
• Non-string values are rejected, matching the plugin, which already did.

The p3 block now goes through the same gate — it reaches the same stylesheet and was previously unvalidated on the Python side.

§2 — Drop the --spacing alias (was #12)

Decided in the epic: dropped, not made theme-aware.

• Removed "--cf-spacing": "--spacing" from _ALIASES
• Regenerated both artifacts; the plugin reads aliases from the JSON, so it followed automatically and the parity test agrees
• Documented the one-line opt-in in docs/theming.md
• CHANGELOG.md breaking note with the migration

--color-primary is a name cf-ui effectively owns in a Tailwind context. --spacing is the root of Tailwind v4's entire spacing scale, so data-density="compact" was rescaling every p-4, gap-2, and m-8 in the consuming app by ±20% — and a Bulma consumer who also ran Tailwind got that with no cf-ui component using it.

test_density_drives_tailwind_spacing_base_unit encoded the old behaviour and is inverted, not deleted — the axis token itself must still be emitted.

§3 — Enforce the p3 lightness invariant (was #13)

Decided in the epic: assert the lightness invariant rather than run full WCAG over oklch.

• oklab_lightness() / oklch_lightness() in Python, oklabLightness() / oklchLightness() in the plugin
• p3_lightness_failures() / p3LightnessFailures() assert each override matches its base's L within tolerance
• Fails CI (test_shipped_p3_overrides_hold_the_lightness_invariant)
• Mirrored in the plugin; warnContrast now has a warnP3 companion under contrastReport: true
• docs/theming.md updated to state what is enforced
• Mutation test: shifting a shipped override's L flags it

The tolerance is set from data, not guessed. I measured every shipped override against its base before writing the test:

accent/azure/light/--cf-accent:         base=49.998%  p3=50.000%  d=0.0018pp
accent/azure/light/--cf-accent-strong:  base=44.338%  p3=44.300%  d=0.0376pp
accent/azure/dark/--cf-accent:          base=75.351%  p3=75.400%  d=0.0487pp
accent/azure/dark/--cf-accent-strong:   base=82.759%  p3=82.800%  d=0.0414pp
accent/jade/light/--cf-accent:          base=50.813%  p3=50.800%  d=0.0127pp
accent/jade/light/--cf-accent-strong:   base=43.180%  p3=43.200%  d=0.0200pp
accent/jade/dark/--cf-accent:           base=77.294%  p3=77.300%  d=0.0056pp
accent/jade/dark/--cf-accent-strong:    base=84.519%  p3=84.500%  d=0.0186pp

WORST: 0.0487pp

The worst deviation is pure rounding from one-decimal authoring. P3_LIGHTNESS_TOLERANCE = 0.005 (0.5pp) is ~10× that — loose enough not to be brittle, tight enough that a meaningful error (which moves several points) cannot pass. A test asserts the rounding case stays legal so the gate does not get tightened into flakiness later.

Failures are collected, not raised, for four distinct causes: drift beyond tolerance, an override of a token the base never declared, a p3 value that is not oklch(), and a base that is not sRGB hex. The last two are flagged rather than skipped — silently skipping unparseable values is how the gate went blind in the first place.

§4 — buildAxisBase / buildAxisCss validate (was #16)

Took option 1 from the original ticket: validate by default, opt out explicitly.

• buildAxisBase(valueSets, definition, { validate: false })
• The parity test passes the opt-out, and still compares the same rule set
• Every export documented in docs/tailwind-plugin.md, not just the plugin factory
• Tests that the direct-call path rejects bad names, unsafe values, and invalid value names unless opted out

On the parity test specifically: passing { validate: false } there is deliberate. Comparing only validated input would quietly narrow the test from "the two generators agree" to "they agree on input the validator already approved" — and the unvalidated path is exactly the one a consumer reaches through the exported function.

§5 — Generated artifacts stop re-dirtying the tree

• .gitattributes pins cf_ui_axes.css, cf_ui_axes.json, and the plugin to LF
• Verified: python -m cf_ui.axes on the committed tree leaves git status clean

---

Verification

Gate	Result
pytest tests/unit tests/integration	453 passed
node --test tests/js/**/*.test.mjs	65 passed, 0 failed
pytest tests/e2e --browser chromium	42 passed, 2 skipped
ruff check src tests	passed
ruff format --check src tests	passed
prek run --all-files	passed, twice (idempotent)
python -m cf_ui.axes on a clean tree	no diff

The JS suite grew 39 → 65. Cross-language parity now covers three properties, not one: identical emitted rules, identical rejection of unsafe values (and identical acceptance of legitimate ones, so the rule cannot degenerate into rejecting everything), and byte-identical p3 failure messages.

Follow-up, not in this diff

Running ruff format . unscoped reformats Python snippets inside markdown, and one such rewrite is wrong — it turns an INSTALLED_APPS line in prompts/migrate-django.md into a tuple:

-"cf_ui.django.CfUiConfig",   # correct
+("cf_ui.django.CfUiConfig",)  # wrong

Neither CI (ruff format --check src tests) nor just format nor the prek hooks touch those files, so nothing is broken today — it is a trap for anyone who runs ruff unscoped. I hit it during this phase and reverted the three affected files rather than folding them in. Worth a docs/prompts exclusion in [tool.ruff], as its own ticket.

[fsecada01]

Adversarial review — PR #26

Scope: 11 files, +924/−27. Four consolidated findings (#11, #12, #13, #16) plus a .gitattributes fix.

Overview

The PR does what it says. The four items really are one mistake repeated, and grouping them was the right call — they touch the same two functions in axes.py and the parity test genuinely couples them.

What is good here, specifically:

• The tolerance is derived, not guessed. Measuring all eight shipped overrides (worst 0.0487pp) before picking 0.005 is the difference between a gate and a number someone will loosen later. The companion test asserting the rounding case stays legal is what stops that.
• The p3 gate flags rather than skips on unparseable values. Silently skipping is precisely how the original gate went blind.
• The parity test passing { validate: false } is a subtle and correct call. Validating first would have narrowed it to "they agree on approved input."
• Rejecting rather than escaping, and adding < to both generators so the parity assertion is real rather than approximate.
• test_density_drives_tailwind_spacing_base_unit inverted rather than deleted, still asserting --cf-spacing is emitted.

Three findings below. None is a blocker on its own; the first is the one I would want addressed.

---

1. The Python generators still emit unvalidated CSS — the exact asymmetry §4 just closed on the JS side

Confirmed by execution, on this branch:

>>> from cf_ui.axes import style_element
>>> style_element({'form': {'evil': {'--cf-radius': '0; } body { display: none'}}})
'<style>\n... [data-form="evil"] {\n  --cf-radius: 0; } body { display: none;\n}\n</style>'

§4's whole argument is that an exported function which generates CSS with the build error switched off contradicts the guarantee the package ships on. That argument was applied to buildAxisBase / buildAxisCss and not to their Python counterparts. style_element, custom_axis_css, and render_axis_css are all in __all__, and style_element is the function the new _UNSAFE_VALUE comment itself names as the sink:

Token values are interpolated straight into generated CSS, which style_element injects into the page.

So after this PR the two generators disagree about the thing the PR is about: JS validates at the generator by default, Python validates only at merge_value_sets.

Severity is bounded, and the PR is not wrong to have shipped: every documented path merges first. I traced them — axis_value_sets() (django.py:26) returns merge_value_sets(custom, mode=mode), and build_axis_globals (axes.py:477) merges whenever value_sets is supplied. There is no reachable injection through Django settings or the Jinja installers. This is a latent sharp edge on a public API, not a live vulnerability.

Suggestion: give the Python side the same treatment — custom_axis_css(value_sets, banner=False, validate=True) and style_element(value_sets, validate=True), with render_axis_css left alone since _regenerate() calls it on the shipped constants. If that is deferred, say so explicitly in docs/theming.md rather than leaving the JS and Python docs implying the same contract.

2. CHANGELOG.md contradicts itself inside [Unreleased]

Line 125, in the #5 technical-notes block, still reads:

Density drives Tailwind v4's --spacing; accent aliases to --color-primary*

Line 21, in the new Changed block, says the alias is gone. Both are under ## [Unreleased] — ## [0.1.1] does not start until line 127 — so the next release ships notes that announce and retract the same behaviour with nothing telling the reader the first half never reached a tagged release.

Suggestion: amend line 125 to Accent aliases to --color-primary* and let the Changed entry carry the --spacing story. The alias never shipped in 0.1.0 or 0.1.1, so there is no history to preserve.

3. test_regenerating_the_artifacts_is_a_no_op cannot fail for the reason it documents

The test's docstring frames it as guarding §5 (regeneration leaves no diff), but it compares Path.read_text() output, and read_text() applies universal-newline translation:

>>> p.write_bytes(b'a {\r\n  --x: 1;\r\n}\r\n')
>>> p.read_text(encoding='utf-8')
'a {\n  --x: 1;\n}\n'

A CRLF-on-disk artifact — the exact state .gitattributes exists to prevent — still compares equal. So the test asserts content equality, which test_shipped_axis_css_matches_generator_output already covered, and not the line-ending property it is named for.

This is the "proxy test" pattern the epic's own body calls out: "Where a test can assert real behaviour — computed style, a real build, actual focus location — it should." §5 is not unprotected (test_generated_artifacts_are_pinned_to_lf checks the real mechanism), but this second test is weaker than it reads.

Suggestion: assert b"\r\n" not in AXIS_CSS_PATH.read_bytes(), which fails for the actual condition. Or drop the test as redundant and let the .gitattributes assertion stand alone.

---

Minor notes, no action needed

• Cross-language float formatting. The p3 parity test compares strings built with Python f"{x:.2f}" and JS toFixed(2). These differ on exact binary ties — f"{0.125:.2f}" is '0.12', (0.125).toFixed(2) is '0.13'. Reaching a tie requires a drift value exactly representable at the rounding boundary, which the OKLab cube roots make effectively impossible. Worth knowing if that test ever flakes; not worth pre-emptive work.
• validateSets duplicates the axis/shape checks in mergeValueSets. Deliberate-looking (it must work without merging), and extracting a shared helper would probably cost more clarity than it saves.
• Double validation in createPlugin — mergeValueSets validates the custom sets, then buildAxisBase validates the merged result including shipped defaults. Negligible cost, and the defensive default is right.
• oklab_lightness's cone ** (1/3) would return a complex number for a negative input rather than raising. Unreachable: the OKLab M1 coefficients are all positive and linearised channels are non-negative. Noting it only because it is the kind of thing that becomes reachable if someone later generalises the function.

Verification reproduced

ruff check src tests and ruff format --check src tests (CI's exact scope) pass; prek run --all-files passes twice, idempotent. 453 pytest + 65 node + 42 E2E as claimed. python -m cf_ui.axes on the committed tree leaves git status clean, so §5's mechanism does work — finding 3 is about the test, not the fix.

The follow-up the PR body flags — ruff format . unscoped rewriting an INSTALLED_APPS snippet into a tuple in prompts/migrate-django.md — is real and correctly kept out of this diff.

[fsecada01]

All three findings fixed — 05658e5

1. Python generators now validate by default

render_axis_css, custom_axis_css, and style_element take validate=True by default with an explicit validate=False opt-out, matching buildAxisBase/buildAxisCss. Verified the concrete regression is closed:

>>> style_element({'form': {'evil': {'--cf-radius': '0; } body { display: none'}}})
AxisConfigError: axis value form/evil declares --cf-radius with an unsafe value: ...
>>> style_element(evil, validate=False)   # escape hatch intact
'<style>... display: none ...</style>'

The per-axis checks merge_value_sets did inline are extracted to _validate_value_sets, mirroring validateSets in the plugin — one implementation instead of two that can drift.

The parity test now passes validate=False on both sides. Previously only the JS half skipped the gate; leaving Python validating would have made the comparison asymmetric in exactly the way the original opt-out was meant to avoid.

Six new tests, parametrised across all three generators: rejects by default, opt-out works, opt-out produces byte-identical output for valid input, and unknown axes are rejected.

2. CHANGELOG no longer contradicts itself

The #5 note is amended to drop the --spacing claim and say plainly that the alias never reached a tagged release, so the Changed block carries the whole story.

3. The no-op test can now actually fail

Switched to read_bytes() with an explicit CRLF assertion — and mutation-tested it rather than assuming:

$ python -c "p.write_bytes(orig.replace(b'\n', b'\r\n'))"
$ pytest ...::test_regenerating_the_artifacts_is_a_no_op
FAILED - AssertionError: cf_ui_axes.css is CRLF on disk; the generator writes LF
$ git checkout -- src/cf_ui/static/cf_ui/cf_ui_axes.css
$ pytest ...::test_regenerating_the_artifacts_is_a_no_op
1 passed

Before the change that mutation passed, which is what made the test a proxy.

Verification

Gate	Result
pytest tests/unit tests/integration	462 passed (was 453)
node --test tests/js/**/*.test.mjs	65 passed, 0 failed
pytest tests/e2e --browser chromium	42 passed, 2 skipped
ruff check src tests / format --check	passed (CI's exact scope)
prek run --all-files	passed twice, idempotent
python -m cf_ui.axes on a clean tree	no diff

The minor notes from the review (cross-language float formatting, validateSets duplication, double validation in createPlugin, the unreachable negative-cube-root branch) were left alone as flagged — none is actionable without costing more clarity than it buys.