Wire macro catalog check into CI and fix documentation

[jackgranatowski]

Summary

This PR integrates the macro catalog verification into the CI pipeline and fixes related documentation issues. The check:macros script is now a CI gate to catch macro/documentation drift automatically, and several documentation inconsistencies are corrected.

Key changes

• CI integration: Added npm run check:macros to the CI workflow (.github/workflows/ci.yml) so macro catalog mismatches are caught automatically rather than requiring manual verification
• Test script consolidation: Simplified npm run test:unit to use a glob pattern (tests/*.test.js) instead of listing test files individually, making it easier to add new tests without updating package.json
• Macro catalog skip list: Added .sf-is-active to the SKIP_IN_DOCS set in scripts/check-macro-catalog.js with a comment explaining it's defined in core/states.css and only cross-referenced in the docs
• Documentation fixes:
  • Updated CLAUDE.md to reflect that check:macros is now a CI gate (was previously "not currently wired into CI")
  • Fixed docs/macros.md to use full class names (.sf-drop-shadow-s, .sf-drop-shadow-m, etc.) instead of shorthand abbreviations in the section heading

Implementation details

The test script change from explicit file listing to glob pattern makes the test suite more maintainable — new test files in tests/ will automatically be included without requiring package.json edits. The pretest hook now delegates to npm run test:unit, keeping the logic in one place.

https://claude.ai/code/session_01BNdrFatwnUKuBcdLcJs6DX

Summary by CodeRabbit

• New Features
  • Added an extra CI verification step for generated artifacts, including macro catalog checks.
• Bug Fixes
  • Improved macro documentation consistency by treating a shared active-state class as an expected exception during drift checks.
• Documentation
  • Updated macro-related docs to reflect current CI enforcement and clearer class naming.
• Tests
  • Simplified test commands so unit tests run across all matching test files, with build verification before tests.

[coderabbitai]

Warning

Review limit reached

@jackgranatowski, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cd3d9fb4-4cfb-4178-b29e-b08d665f8f74

📥 Commits

Reviewing files that changed from the base of the PR and between 4a19503 and e25967f.

📒 Files selected for processing (1)

• tests/README.md

📝 Walkthrough

Walkthrough

Adds a CI step running scripts/check-macro-catalog.js, adds .sf-is-active to that script's docs/CSS drift-skip list, updates CLAUDE.md and docs/macros.md text, and changes package.json pretest/test:unit scripts to run all tests/*.test.js via glob instead of a hardcoded file list.

Changes

Macro Catalog CI Gate

Layer / File(s)	Summary
Script exception and CI wiring scripts/check-macro-catalog.js, .github/workflows/ci.yml	Adds .sf-is-active to SKIP_IN_DOCS to avoid a false drift error, and adds a CI step running the macro catalog check script in the artifact-verification job.
Documentation updates CLAUDE.md, docs/macros.md	Updates CLAUDE.md text to state the macro check is now a CI gate, and expands drop-shadow class name list in macros.md.

Test Script Consolidation

Layer / File(s)	Summary
Unit test glob update package.json	Changes pretest to run npm run build then test:unit, and changes test:unit to run node --test tests/*.test.js instead of a hardcoded file list.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• codeslash-dev/SLASHED#273: Both PRs modify .github/workflows/ci.yml around the artifact-verification checks.
• codeslash-dev/SLASHED#519: Both PRs modify scripts/check-macro-catalog.js to add skip exceptions for specific macro classes.

Suggested labels: codex

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main changes: wiring the macro catalog check into CI and updating related documentation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/repo-audit-trnajc

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Add macro-catalog verification to CI; fix macros docs and unit-test script

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🧪 Tests 🕐 20-40 Minutes

AI Description

• Run macro catalog verification in CI to prevent CSS/docs drift landing unnoticed
• Fix macro documentation headings so the catalog checker can correctly match classes
• Simplify unit test invocation to a glob-based runner to reduce future maintenance

Diagram

graph TD
  A["GitHub Actions CI"] --> B["artifacts-freshness job"] --> C["check-macro-catalog (node)"] --> D["Macro CSS sources"] --> E["docs/macros.md"]
  B --> F["npm test:unit (node --test)"] --> G["tests/*.test.js"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Run via npm script in CI (npm run check:macros)

• ➕ Keeps CI invocation consistent with documented/local developer workflow
• ➕ Avoids CI drifting if the script path/name changes
• ➖ Slightly more indirection than calling node directly
• ➖ Depends on package.json being present/valid for that job step

2. Dedicated CI job for macro-catalog check

• ➕ Clearer failure attribution and easier reruns (job-level)
• ➕ Can run in parallel with other freshness checks
• ➖ More workflow verbosity and job overhead for a lightweight check
• ➖ May require additional job dependencies/artifact setup if reused

Recommendation: Current approach (adding the node script to the existing freshness checks) is solid and keeps drift prevention close to other repository integrity gates. If consistency with docs/local commands is a priority, consider switching the CI step to npm run check:macros; otherwise, the direct node invocation is acceptable.

Files changed (5) +8 / -4

Bug fix (1) +3 / -0

check-macro-catalog.jsAllowlist .sf-is-active as docs-only cross-reference +3/-0

Allowlist .sf-is-active as docs-only cross-reference

• Adds '.sf-is-active' to the SKIP_IN_DOCS set with context that it is defined outside macro sources and only referenced indirectly in the docs.

scripts/check-macro-catalog.js

Documentation (2) +2 / -2

CLAUDE.mdDocument check:macros as a CI gate +1/-1

Document check:macros as a CI gate

• Updates the developer command table to reflect that 'npm run check:macros' is now enforced by CI.

CLAUDE.md

macros.mdFix drop-shadow heading to use full macro class names +1/-1

Fix drop-shadow heading to use full macro class names

• Expands the drop-shadow section heading to list the full '.sf-drop-shadow-*' class names so the macro catalog checker can match them reliably.

docs/macros.md

Other (2) +3 / -2

ci.ymlGate CI on macro catalog verification +1/-0

Gate CI on macro catalog verification

• Adds 'node scripts/check-macro-catalog.js' to the existing CI checks so macro/documentation drift fails the pipeline instead of being detected manually.

.github/workflows/ci.yml

package.jsonSimplify unit test scripts using a glob runner +2/-2

Simplify unit test scripts using a glob runner

• Changes 'test:unit' to run 'node --test tests/*.test.js' and updates 'pretest' to delegate to 'test:unit', removing duplicated per-file lists.

package.json

[coderabbitai]

🧹 Nitpick comments (1)

package.json (1)

57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Quote the glob pattern for cross-shell portability.

Glob patterns follow the behavior of glob(7). The glob patterns should be enclosed in double quotes on the command line to prevent shell expansion, which can reduce portability across systems. Leaving tests/*.test.js unquoted works today on POSIX shells (CI), but it's fragile for local/Windows dev shells where either no expansion happens or expansion behaves inconsistently across shells.

♻️ Proposed fix

-    "test:unit": "node --test tests/*.test.js",
+    "test:unit": "node --test \"tests/*.test.js\"",

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` at line 57, The test:unit script currently passes the
tests/*.test.js glob unquoted, which can break or behave inconsistently across
shells. Update the package.json script to quote the glob pattern in the node
--test command so it is preserved for Node to expand consistently, and verify
the test:unit entry remains the same otherwise.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@package.json`:
- Line 57: The test:unit script currently passes the tests/*.test.js glob
unquoted, which can break or behave inconsistently across shells. Update the
package.json script to quote the glob pattern in the node --test command so it
is preserved for Node to expand consistently, and verify the test:unit entry
remains the same otherwise.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e371ef6-3b80-4e3d-81f6-e285371aa8c7

📥 Commits

Reviewing files that changed from the base of the PR and between e83b1ff and 4a19503.

📒 Files selected for processing (5)

• .github/workflows/ci.yml
• CLAUDE.md
• docs/macros.md
• package.json
• scripts/check-macro-catalog.js

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 12 rules

1. Unit-test docs outdated ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

tests/README.md still says new *.test.js files must be manually wired into pretest/test:unit,
but package.json now discovers unit tests via the tests/*.test.js glob and pretest delegates
to test:unit. This mismatch will confuse contributors and can lead to unnecessary edits to
package.json.

Code

package.json[R55-57]

+    "pretest": "npm run build && npm run test:unit",
    "test": "playwright test",
-    "test:unit": "node --test tests/tier1-p2-coverage.test.js tests/tier1-p7-oldengine.test.js tests/tier1-p10-contrast.test.js tests/api-index-sync.test.js tests/color-docs.test.js tests/docs-artifacts-sync.test.js tests/check-version-sync.test.js tests/coverage.test.js tests/parse-lib.test.js",
+    "test:unit": "node --test tests/*.test.js",

Relevance

⭐⭐⭐ High

Team has accepted fixing docs/audits when they drift from code (e.g., checklist/doc corrections in
PR #48, #474).

PR-#48
PR-#474

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

package.json now runs unit tests via a glob (node --test tests/*.test.js) and pretest
delegates to test:unit, but tests/README.md still instructs contributors to manually wire new
unit test files into those scripts.

package.json[34-62]
tests/README.md[1-13]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tests/README.md` describes the old workflow where new `*.test.js` files had to be manually added to `pretest`/`test:unit`. The PR changed `test:unit` to use a glob (`tests/*.test.js`) and made `pretest` call `test:unit`, so the README is now inaccurate.

## Issue Context
The repo now automatically includes any `*.test.js` file placed directly under `tests/` when running `npm run test:unit`.

## Fix Focus Areas
- tests/README.md[1-13]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---