CORE-2731: Add the CSS color audit engine by OpenStaxClaude · Pull Request #3142 · openstax/rex-web

OpenStaxClaude · 2026-09-11T15:20:38Z

1 of 2 — the split Roy asked for in review on #3133, following the same shape as ui-components#149 → #150. main → this → #3133.

What this is

Pure CSS parsing that answers one question: given stylesheet text, which color literals are in it, and which declaration was each one written in. It knows nothing about REX, the theme, the token file or the baseline — that layer, and everything CI enforces with it, is #3133.

The file has no imports at all, which is what makes it reviewable on its own: you can read it without knowing anything about theme tokens.

stripNoise blanks comments, string contents and url() payloads. It blanks to spaces rather than deleting, so the result is the same length as the input and every character keeps its index. That is load-bearing: declarations reads structure from the copy where strings are gone (so a ; or { inside a string cannot split a declaration) and slices context from a second copy where strings survive (so [data-loading="true"] and [data-loading="false"] stay distinguishable). One index addresses both only because the lengths match.
A brace/semicolon scan pulls {context, property, value} at any nesting depth, so @media blocks are covered while selectors and @keyframes percentages are not read as values.
Each value is then walked for colors in any syntax — hex, the functional notations, and bare names against the full CSS named-color table.

Three rules worth a reviewer's attention:

Functions that merely contain colors are descended into (var(), color-mix(), the gradients) rather than treated as literals, so building a value out of tokens stays clean and a hardcoded gradient stop is still found.
A bare identifier is only read as a color where one can legally go — a custom property, a color-named property, or one of the color-bearing shorthands, vendor prefixes stripped first. So animation-name: red, font-family: black and grid-area: navy are not findings. Hex and the color functions are unambiguous and stay in scope everywhere; a case asserts animation-name: #fff is still flagged, so the narrowing cannot quietly grow into "skip these properties".
Anything that cannot be resolved to channels comes back as rgba: null rather than being dropped — hsl(), oklch(), color(), rgb() over a var() channel list. The caller decides, and in CORE-2731: Generate a global CSS token file from the theme #3133 it fails, so the escape hatch stays explicit.

Why it is worth a separate review

It is about half of what #3133 added and it is the half with least to do with theme tokens. The 114 cases in cssColors.spec.ts are the contract — each says either "this must be flagged" or "this must be left alone" — and they are the reason "CI enforces the palette" is a tested guarantee rather than an assertion.

Two of them exist because the parser got these wrong in review on #3133, and they are the ones to read first if you are spot-checking:

rgb(50%, 50%, 50%) keyed as 127,127,127 while #808080 keyed as 128,128,128, because scaling by the decimal 2.55 is not representable in binary. The two spellings of one grey could never match.
#ggg parsed as a color: only the expanded length was checked, not the digits, so it handed back an Rgba of NaNs that read as resolved.

Nothing consumes it yet

On main alone this is test infrastructure with a spec and no caller; src/test/themeColors.ts and src/app/theme.spec.ts, which the header comment points at, arrive in #3133. That is the cost of the split, and it is the same trade ui-components#149 made. The alternative is reviewing the parser and the token design in one 2,000-line diff.

src/test/ is outside jest's collectCoverageFrom, so this does not have to reach the repo's 100% threshold on src/{app,helpers,gateways} — it is test infrastructure, and its spec covers it regardless.

Verification

Checked out on top of main with nothing else applied: cssColors.spec.ts 114 tests pass and tsc is clean, so it genuinely stands alone.
src/test is on .eslintrc's ignorePatterns (pre-existing), so eslint src does not cover this file; tsc does.
On the CORE-2731: Generate a global CSS token file from the theme #3133 branch, splitting this out regenerates theme.baseline.json byte-identical at 206 duplicates / 37 unrecognised, which is the check that the extraction changed no behaviour.

Review round (`c25e419`, `9878d19`)

Copilot found six parsing edge cases and all six were real. Each already had a fix on
ui-components#149, so these are
ports of those rather than independently derived — the two copies converged, which is
what #3137 has to reconcile.

device-cmyk() added to the terminal color functions; it was descended into and
produced no finding at all.
url( now requires an ident boundary, so myurl(#fff) is no longer read as a url
token with its payload blanked.
The url() scan is quote- and escape-aware. This was the one with real blast radius:
url("asset).svg") closed at the paren in the filename, leaving the trailing quote to
open an unterminated string that blanked the rest of the stylesheet.
Selector whitespace collapses only outside strings, so [data-value="a b"] and
[data-value="a b"] stay distinct — the reason the context copy keeps strings at all.
Channels and alpha share the CSS <number> grammar. [\d.]+ matched . and 1..2,
which parseFloat turned into NaN and a truncated 1 and which came back as
resolved — the same failure the hex grammar check exists to prevent. It also
rejected the legal +255 and 2.55e2, so it was wrong in both directions.
The rgb()/rgba() grammar is dotall, so a literal wrapped across lines resolves.

18 new cases, 13 of which fail against the previous parser. 96 → 114.

None of these six constructs appear in REX's 106 stylesheets, so theme.baseline.json
on #3133 is unaffected; regenerating it there remains the definitive check.

Second review round — no code change

Copilot returned 13 more findings (3 inline, 10 suppressed). All were assessed; none
were fixed here, deliberately:

None of the 13 can fire on REX's CSS. Seven need a CSS \ escape, and the 110
stylesheets contain exactly one backslash — content: "\2022", inside a string that is
blanked regardless. The rest need constructs REX does not have: vendor-prefixed
gradients, list-style-image/mask-image/border-image-source, color-scheme,
uppercase custom properties, brace-valued custom properties, #fff-foo hash tokens, or
a comment used as a compound-selector joiner. So theme.baseline.json on CORE-2731: Generate a global CSS token file from the theme #3133 is
unaffected either way.
Nine of the 13 are already fixed in ui-components#149, the parser that replaces this
file — ESCAPE/identSpan/decodeEscapes, COLOR_CONTAINERS, the NON_COLOR
exclusions, list-style removed, custom-property names kept as written, and
component-value brace tracking.
The four that are not fixed upstream were carried to
ui-components#149, where the code
survives: escape decoding missing for the url(/function-name/property-name identifier
positions, the \b in the hex pattern letting #fff-foo through, and comments padded to
spaces merging .a/**/.b with .a .b.

Rationale in this comment.
Still 114/114 with tsc clean; all 8 checks green on 9878d19f.

Follow-up

CORE-2737 (#3137) replaces this copy with the same engine published from ui-components, so REX stops carrying its own. That is why this file is a plausible thing to have in its own commit: it is the thing that gets swapped out.

🤖 Generated with Claude Code

Pure CSS parsing that answers one question: given stylesheet text, which color literals are in it and which declaration each one was written in. It knows nothing about REX, the theme or the token file -- that layer, and everything that enforces it, is the stacked PR. - stripNoise blanks comments, string contents and url() payloads without changing length, so a second differently-blanked copy can be indexed in step with the first: structure is read where strings are gone, context is sliced where they survive. - A brace/semicolon scan pulls {context, property, value} at any nesting depth, so @media is covered while selectors and @Keyframes percentages are not read as values. - Each value is walked for colors in any syntax. Functions that merely contain colors (var(), color-mix(), the gradients) are descended into; the color functions are terminal. A bare identifier is only read as a color where one can legally go, so animation-name: red is not a finding while hex and the color functions stay in scope everywhere. Anything that cannot be resolved to channels comes back as rgba: null rather than passing silently. 96 cases pin the contract in both directions -- what it must flag and what it must leave alone.

Each was reported by Copilot on #3142, and each already had a fix on ui-components#149, so these are ports of those rather than new inventions -- the two copies converge ahead of CORE-2737 swapping this file for the published engine. - device-cmyk() is a terminal color function. Absent from the list it was descended into, its numeric arguments matched nothing, and a hardcoded color passed the audit unreported. - `url(` now needs an ident boundary. `myurl(#fff)` was read as a url token and its payload blanked, losing the color. - The url scan is quote- and escape-aware. `url("asset).svg")` closed at the paren in the filename, leaving the trailing quote to open an unterminated string that blanked the rest of the stylesheet. - Selector whitespace collapses only outside strings, so `[data-value="a b"]` and `[data-value="a b"]` stay distinct -- which is the reason the context copy keeps its strings at all. - Channels and alpha use the CSS <number> grammar. `[\d.]+` also matched `.` and `1..2`, which parseFloat turned into NaN and a truncated 1 and which were handed back as resolved -- the failure the hex grammar check already prevents. It also rejected the legal `+255` and `2.55e2`. - The rgb()/rgba() grammar crosses newlines. Upstream uses the `s` flag; REX targets es2017, where that is a compile error, so this uses [\s\S]. 18 spec cases, 13 of which fail against the previous parser. None of these constructs appear in REX's stylesheets, so theme.baseline.json on #3133 is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

I claimed in review that REX's es2017 target made the `s` flag a compile error and used [\s\S] instead. That was wrong: flag validation landed in TypeScript 5.5 and REX is on 4.9.5, and the flag is a runtime feature that the target does not gate anyway. So this matches ui-components#149 verbatim, which is the point -- the copy is deleted by CORE-2737 and every needless difference is friction for that swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Unresolved moderate parser correctness issues remain before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (10)

Previously missed (7) — in code that hasn't changed since the last review.

src/test/cssColors.ts:147

An escaped quote outside a string is valid CSS, but this scanner treats it as the start of a string. For example, .foo\"bar { color: #fff; } causes the quote scan to run to end-of-file and the declaration disappears from both blanked copies. Handle an escape before testing for a quote.
src/test/cssColors.ts:290
Custom property names are case-sensitive in CSS, but this lowercases every property. --Brand and --brand therefore return the same property and can collide when the caller keys occurrences by declaration, even though they are distinct names. Preserve the spelling of --* properties while normalizing ordinary properties.
src/test/cssColors.ts:300
The structural scan does not honor CSS escapes, so escaped delimiters are treated as syntax. A valid selector such as .foo\{bar { color: #fff; } } opens an extra block, and an escaped semicolon in a custom-property value splits one declaration into several. Skip the escaped character before counting parentheses, braces, or semicolons.
src/test/cssColors.ts:310
Balanced blocks are valid component values in custom properties, but every { is currently treated as a nested rule. For :root { --x: { red }; }, the --x declaration is not retained and red is never scanned, even though custom properties are explicitly intended to accept arbitrary values. Track component-value brace depth after a custom-property declaration starts before applying rule/declaration structure handling.
src/test/cssColors.ts:332
list-style does not accept a color; its first identifier can be an author-defined counter style, so list-style: red is not a hardcoded color. Including it here makes takesColor report a false palette violation, contrary to the bare-identifier property gate. Remove list-style from this list (and handle colors inside image functions separately).
src/test/cssColors.ts:341
includes('color') treats color-scheme as a color-valued property, but its values are custom identifiers; color-scheme: red is a scheme name, not a color literal. This causes a false finding, so exclude this non-color property before applying the name heuristic.
src/test/cssColors.ts:475
Named identifiers are compared only in their literal ASCII spelling, so valid escaped color names are missed. For example, color: \72 ed is CSS's escaped spelling of red, but the scanner sees ed and reports no color, allowing a hardcoded value to bypass the audit. Decode CSS escapes before the named-color lookup while retaining the source span as literal.

src/test/cssColors.ts:468

The \b check allows a hyphen after the matched hex digits, so #fff-foo is read as #fff even though CSS tokenizes the whole value as one non-color hash token. This produces false findings in grammar-free custom-property values; require the next character not to continue a hash name.

    const hex = /^#[0-9a-fA-F]{3,8}\b/.exec(rest);

src/test/cssColors.ts:447

The function-name regex accepts only literal names beginning with [a-z], so valid vendor-prefixed or escaped CSS functions are not recognized. For example, -webkit-linear-gradient(red, blue) and linear-gr\61 dient(red, blue) are scanned as loose words; in a property such as list-style-image, their named stops are then gated off, and an escaped terminal rgb spelling would not be reported at all. Parse and decode a CSS identifier before classifying the function.

    const call = /^([a-z][\w-]*)\(/i.exec(rest);
    if (call) {

src/test/cssColors.ts:291

CSS escapes are not decoded before the property gate is applied. A declaration such as c\6f lor: red is parsed by CSS as color: red, but the raw spelling returned here does not contain color, so takesColor returns false and the named color is silently omitted. Decode ordinary property identifiers before vendor-prefix and color-property checks.

      const value = segment.slice(separator + 1).trim();
      const property = segment.slice(0, separator).trim().toLowerCase();
      if (value) { found.push({context: stack.join(' '), property, value}); }

Files reviewed: 2/2 changed files
Comments generated: 3
Review effort level: Lite

OpenStaxClaude · 2026-09-11T16:14:40Z

+    if (rest.startsWith('/*')) {
+      const end = css.indexOf('*/', index + 2);
+      const stop = end === -1 ? css.length : end + 2;
+      out += pad(stop - index);
+      index = stop;


Correct in principle, and not fixed upstream either — I've carried it to ui-components#149.

Not changing it here, per the merge discussion below. It also cannot fire on REX's CSS: the collision needs the comment to act as a compound joiner (.a/**/.b), and the only comment inside a selector anywhere in the tree is

> a /*> .number*/ {

which has whitespace on both sides, so padding it to spaces already yields the correct > a. I checked for the adjacent form (/* preceded, or */ followed, by a selector character) across all 110 stylesheets — none.

OpenStaxClaude · 2026-09-11T16:14:41Z

+    const boundary = index === 0 || !/[\w-]/.test(css[index - 1]);
+    const url = boundary ? /^url\(/i.exec(rest) : null;


Real, and not fixed upstream — carried to ui-components#149 as part of a single coherent gap: escapes are decoded for named-color lookup there, but not for the url(, function-name or property-name identifier positions. Your thread 3 sibling and the suppressed c\6f lor comment are the same root cause.

Not fixing here, per the merge discussion below. It needs a CSS escape to trigger, and REX's 110 stylesheets contain exactly one backslash — content: "\2022" — inside a string, which is blanked regardless of this code path.

OpenStaxClaude · 2026-09-11T16:14:43Z

+      } else {
+        found.push(...findColors(args, named));
+      }


This is the best of the three — it is the "silent miss" class rather than a false finding, so it would weaken the guarantee rather than just cause noise.

Already fixed upstream: ui-components#149 introduces COLOR_CONTAINERS and passes named || holdsColor(fn) down, which is your suggested split exactly — containers open the gate for their own arguments, while var() is deliberately excluded so a fallback keeps the enclosing property's gate. It also removes list-style from the shorthands for the reason in your suppressed comment.

Not porting it here, per the merge discussion below: REX has no list-style-image, mask-image or border-image-source declarations at all, and every list-style: is none (a COLOR_KEYWORD, never flagged), so there is nothing for the gate hole to bite.

RoyEJohnson

Copilot has some more comments.
As I understand it, this PR has no impact on any runtime code. It is all about testing, and so it is very low-risk to have imperfections such as Copilot is finding.
I am inclined to merge this PR and let the further fixes happen when we move to one parser. Is that safe and prudent? I don't want to spend a lot of resources debugging throwaway code if the benefit is marginal.

OpenStaxClaude · 2026-09-11T16:14:24Z

Is that safe and prudent?

Yes. I checked all 13 of this round's findings (3 inline + 10 suppressed) against two questions, and both answers point the same way.

1. None of the 13 can fire on REX's CSS

Not "unlikely" — they have no trigger present. Across 106 .css and 4 .less files:

Finding class	Trigger needed	Present in REX?
Escaped quote/brace/semicolon, escaped property name, escaped named color, escaped `url(`/function spelling (7 of 13)	any CSS `\` escape	One backslash total: `content: "\2022"` — inside a string, which is blanked regardless
Vendor-prefixed gradient not read as a call	`-webkit-linear-gradient(` etc.	none
Gradient stops lose the named gate	`list-style-image` / `mask-image` / `border-image-source`	none exist; every `list-style:` is `none`, a `COLOR_KEYWORD` that is never flagged
`list-style: red` false finding	named color as a counter-style	none
`color-scheme: red` false finding	`color-scheme` declaration	none
`#fff-foo` read as `#fff`	hash token continuing into an ident	none
`--Brand` vs `--brand` collide	uppercase custom property	none — all 42 are lower-case
`{}` in a custom-property value	brace-valued custom property	none
`.a/**/.b` context collision	comment as a compound-selector joiner	the only selector comment is `> a /> .number/ {`, which has whitespace on both sides, so padding it to spaces is already correct

So theme.baseline.json on #3133 is unaffected by every one of them, and the CI gate behaves identically fixed or not.

2. Nine of the thirteen are already fixed in the parser that replaces this one

ui-components#149 already has ESCAPE/identSpan/decodeEscapes, COLOR_CONTAINERS, a NON_COLOR exclusion list, list-style deliberately removed, custom-property names kept as written, and component-value brace tracking — several with comments naming these exact cases. Patching them here would be re-deriving work that already exists in the file that deletes this one. That is precisely the cost you are trying not to pay.

One refinement to the premise, which doesn't change the conclusion

"No impact on runtime code" is exactly right — nothing outside src/test/ imports this, and I re-verified that. It is worth being precise that in #3133 it does gate CI, so imperfections split in two:

False finding → CI fails on legitimate CSS. Loud, immediate, self-announcing. A safe failure mode.
Missed finding → a hardcoded color slips through silently. This is the class worth caring about, because it quietly weakens the guarantee the PR claims.

Even so, the worst case of a missed finding is a hardcoded color stays in a stylesheet — a code-quality regression, not a user-visible defect, and one the audit will catch the moment the parser improves. So your risk read holds for the bad class too, not just the benign one.

What I'd do instead of debugging here

Four findings are not fixed upstream, and they are worth more there than here, since that code survives:

Escape decoding is applied to named colors but not to the three other identifier positions — url(, function names, and property names. One coherent gap (covers threads 2 and 3's escaped halves, plus the suppressed c\6f lor one).
#[0-9a-fA-F]{3,8}\b still lets #fff-foo read as #fff — the \b is satisfied by the hyphen.
Comments padded to spaces still merge .a/**/.b with .a .b in the context.

I've carried these to #149 so they land where the engine lives. Nothing to do on this PR for them.

State

All 8 checks green on 9878d19f; tsc clean and 114/114 locally. mergeStateStatus is BLOCKED purely on this review being CHANGES_REQUESTED — so it needs your approval to go in. I haven't merged it; that's your call.

OpenStaxClaude requested a review from a team as a code owner September 11, 2026 15:20

OpenStaxClaude requested review from RoyEJohnson and TomWoodward September 11, 2026 15:20

TomWoodward temporarily deployed to rex-web-core-2731-css-c-6ccqzn September 11, 2026 15:21 Inactive

This was referenced Sep 11, 2026

CORE-2731: Generate a global CSS token file from the theme #3133

Open

CORE-2737: Use the published ui-components CSS colour audit in REX #3137

Draft

RoyEJohnson requested a balanced review from Copilot and removed request for TomWoodward September 11, 2026 15:28

RoyEJohnson added this pull request to stack #3143 September 11, 2026 15:29

Copilot started reviewing on behalf of RoyEJohnson September 11, 2026 15:29 View session

This comment was marked as resolved.

Sign in to view

TomWoodward temporarily deployed to rex-web-core-2731-css-c-6ccqzn September 11, 2026 15:48 Inactive

This comment was marked as resolved.

Sign in to view

TomWoodward temporarily deployed to rex-web-core-2731-css-c-6ccqzn September 11, 2026 15:51 Inactive

RoyEJohnson requested a lite review from Copilot September 11, 2026 15:52

Copilot started reviewing on behalf of RoyEJohnson September 11, 2026 15:53 View session

OpenStaxClaude requested a review from RoyEJohnson September 11, 2026 16:01

Copilot AI reviewed Sep 11, 2026

View reviewed changes

RoyEJohnson requested changes Sep 11, 2026

View reviewed changes

OpenStaxClaude mentioned this pull request Sep 11, 2026

CORE-2736: Add the CSS colour audit engine openstax/ui-components#149

Open

OpenStaxClaude requested a review from RoyEJohnson September 11, 2026 16:15

RoyEJohnson approved these changes Sep 11, 2026

View reviewed changes

RoyEJohnson merged commit c8f2b73 into main Sep 11, 2026
8 checks passed

RoyEJohnson deleted the CORE-2731-css-color-audit-engine branch September 11, 2026 16:22

staxbot added the in-release-2900 label Sep 11, 2026

Navigation Menu

Uh oh!

CORE-2731: Add the CSS color audit engine - #3142

CORE-2731: Add the CSS color audit engine#3142
RoyEJohnson merged 3 commits into
mainfrom
CORE-2731-css-color-audit-engine

OpenStaxClaude commented Sep 11, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Copilot AI left a comment

Uh oh!

OpenStaxClaude Sep 11, 2026

Uh oh!

OpenStaxClaude Sep 11, 2026

Uh oh!

OpenStaxClaude Sep 11, 2026

Uh oh!

RoyEJohnson left a comment

Uh oh!

OpenStaxClaude commented Sep 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		const boundary = index === 0 \|\| !/[\w-]/.test(css[index - 1]);
		const url = boundary ? /^url\(/i.exec(rest) : null;

Uh oh!

Conversation

OpenStaxClaude commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this is

Why it is worth a separate review

Nothing consumes it yet

Verification

Review round (c25e419, 9878d19)

Second review round — no code change

Follow-up

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (10)

Uh oh!

OpenStaxClaude Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

OpenStaxClaude Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

OpenStaxClaude Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

RoyEJohnson left a comment

Choose a reason for hiding this comment

Uh oh!

OpenStaxClaude commented Sep 11, 2026

1. None of the 13 can fire on REX's CSS

2. Nine of the thirteen are already fixed in the parser that replaces this one

One refinement to the premise, which doesn't change the conclusion

What I'd do instead of debugging here

State

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

OpenStaxClaude commented Sep 11, 2026 •

edited

Loading

Review round (`c25e419`, `9878d19`)