UI library: add exhaustive page coverage contract - #619
Conversation
WalkthroughThis PR adds repository-wide page-coverage discovery, plan validation, deterministic JSON/Markdown ledger generation, checked-in coverage artifacts, comprehensive tests, documentation, and a CI contract check. ChangesPage coverage contract
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant generatePageCoverageLedger
participant discoverRepository
participant validatePlan
participant LedgerArtifacts
CI->>generatePageCoverageLedger: run check:page-coverage
generatePageCoverageLedger->>discoverRepository: discover repository surfaces
generatePageCoverageLedger->>validatePlan: validate checked-in plan
validatePlan-->>generatePageCoverageLedger: return validated records
generatePageCoverageLedger->>LedgerArtifacts: compare rendered JSON and Markdown
LedgerArtifacts-->>CI: report current or stale outputs
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@apps/wodsmith-start/scripts/page-coverage/discovery.mjs`:
- Around line 250-264: The parseFrontmatter() implementation should parse the
frontmatter block as YAML instead of splitting lines and coercing only boolean
scalars. Replace the manual parsing logic with the repository’s available
YAML/frontmatter parser, preserving quoted strings and structured values such as
arrays or objects while retaining the existing empty/invalid-frontmatter
fallback behavior.
In `@apps/wodsmith-start/scripts/page-coverage/validation.mjs`:
- Around line 243-246: Update the scenario validation near the scenario ID
uniqueness check to reject any scenario whose id is missing, non-string, or
blank after trimming, before emitting the ledger; retain the duplicate-ID check
for valid identities and reference the scenario mapping in the validation logic.
- Around line 122-146: The repository-boundary check in validateEvidence must
use portable path semantics instead of startsWith. Resolve the repository root
and evidence path, compute relative(repoRoot, absolutePath), and reject the path
when isAbsolute(relativePath) or it starts with .. plus a path separator; retain
the existing missing-file and hash validation.
In `@apps/wodsmith-start/test/scripts/page-coverage-ledger.test.ts`:
- Around line 181-233: Add exactly one `// `@lat`:` specification reference next
to every `it()` test in `page-coverage-ledger.test.ts` that currently lacks one,
including the tests covering route-source validation, app registration,
Docusaurus drift, Hono/worker/cron decisions, dynamic fixtures, unknown axes,
dispositions, layout-only records, and deterministic rendering. Use the existing
nearby tests’ reference format and ensure each test has a unique, relevant spec
reference with no duplicates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f15c34d1-048e-41bf-b556-4649de0efeca
📒 Files selected for processing (13)
.github/workflows/ci.yamlapps/wodsmith-start/package.jsonapps/wodsmith-start/scripts/generate-page-coverage-ledger.mjsapps/wodsmith-start/scripts/page-coverage/config.mjsapps/wodsmith-start/scripts/page-coverage/discovery.mjsapps/wodsmith-start/scripts/page-coverage/render.mjsapps/wodsmith-start/scripts/page-coverage/validation.mjsapps/wodsmith-start/test/scripts/page-coverage-ledger.test.tsdocs/ui-library/page-coverage-ledger.jsondocs/ui-library/page-coverage-ledger.mddocs/ui-library/page-coverage.plan.jsonlat.md/ui-library.mdpackage.json
zacjones93
left a comment
zacjones93
left a comment
There was a problem hiding this comment.
Independent stacked review — changes requested
Risk: MEDIUM. The checked-in baseline is internally consistent, but the “exhaustive” drift gate still has three silent false-negative/false-positive paths that should be closed before this layer becomes the source of truth.
-
[P1] Docusaurus MDX pages bypass discovery.
discoverDocsonly accepts.mdand_category_.json(discovery.mjs:309-311). Docusaurus treats.mdxas a docs page, but a temp fixture containing onlydocs/hidden.mdxproduced 0 records and no error. A future MDX page would therefore ship without a plan entry whilecheck:page-coveragestays green. Include both.mdand.mdxin discovery/frontmatter handling and add a fixture asserting the MDX route is inventoried (plus a missing-plan failure). -
[P1] Service markers prove listed decisions still exist, not that the service inventory is complete.
discoverServicesiterates the hard-codedSERVICE_DECISIONSand checks marker presence (discovery.mjs:360-381), while the registry only requires at least one decision per service app (lines 447-458). Adding a new Honoapp.get/post/routeor Worker path branch leaves every existing marker intact, so CI does not detect the new surface despite the LAT claim that repository-surface drift fails generation. Add source-derived route/mount reconciliation, or another deterministic routing manifest/hash gate that fails when routing code changes without an explicit decision; test an added unlisted Hono endpoint and Worker branch. -
[P1] Route classification reads comments as executable route options.
classifyTanstackRouteregexes raw option text forcomponent:,server.handlers, andredirect((discovery.mjs:98-103). Direct proof:{ // component: RetiredPage\n beforeLoad: guard }is classified as a page. The brace scanner also does not lex comments/regex literals, so commented braces can truncate/extend the option slice. Use a TypeScript AST or a comment-aware lexer before classification, and add fixtures for line/block comments and regex literals so commented properties cannot create visual/API/redirect records. -
[P2] Scenario state-machine validation accepts malformed identities and contradictory pending blockers. A one-scenario plan with
scenario.iddeleted passes validation; likewise astatus: "pending"scenario with a populated blocker passes. The validator only checks duplicate IDs and validates blocker shape forblocked(validation.mjs:207-254). Require a non-empty string ID for every scenario, and enforce blockers only forblocked(or document/test a different explicit rule). Add rejection tests for missing/blank IDs and pending scenarios carrying blockers.
Evidence checked
- Exact stack:
b76ab29b6...397af8aef; 13-file PR diff. - Independent tree/source denominator: generated-tree entries + root = Start 212, Crew 37, CRM 17, Ledger 5, Gameday 4, matching source declaration counts and ledger app counts.
- Ledger: 328 total = 239 page/page-layout + 89 non-page; 17 redirects; 239 pending / 0 blocked / 0 verified. All pending scenarios currently have empty evidence and blockers, so the PR does not fabricate verification.
- Current Docs inventory reconciles 31 Markdown docs + 10 categories, including 1 draft and 6 navigation-only categories; current routeBasePath/category outputs are consistent.
node apps/wodsmith-start/scripts/generate-page-coverage-ledger.mjs --checkpassed;lat checkpassed; worktree remained clean.- All 14 GitHub checks are green. No inline review threads or submitted reviews exist; CodeRabbit remains in its in-progress summary state.
- Local pnpm/Vitest and Docs-build reruns were blocked by this worktree’s pnpm store mismatch/non-TTY reinstall guard; CI’s Start test/build/typecheck and E2E checks provide the full-suite signal.
- GitNexus compare output was stale to an earlier stacked worktree (it reported 174 changed files rather than this PR’s 13), so it was not used as correctness evidence.
|
Follow-up review fixes are pushed in 675338f. Addressed:
Validation:
All four CodeRabbit inline threads have evidence replies and are resolved. Requesting a delta review of 675338f. @coderabbitai review |
|
✅ Action performedReview finished.
|
Delta review of 397af8a..675338f — one remaining finding[P1] The service drift gate still silently misses valid routing syntax. The new reconciliation is source-derived only for narrow regex shapes:
This leaves the original completeness problem open whenever routing uses legitimate alternate syntax or a stale call survives in a comment. Make service parsing AST-based/fail-closed too: walk chained Hono call expressions and mounted receivers; enumerate/normalize supported Worker path predicates while rejecting unrecognized All prior findings are otherwise resolved: MDX/YAML, TanStack AST classification, scenario ID/pending-blocker validation, and portable evidence refs. Validation: focused contract 18/18, direct ledger freshness passed, |
|
Addressed the remaining P1 from the delta review in fae7b83. Tests-first evidence:
Implementation:
Validation:
Requesting another delta review of 675338f..fae7b83. The parent remains open pending reviewer clearance. @coderabbitai review |
Final delta review of 675338f..fae7b83 — remaining fail-closed gapsThe exact prior bypasses are fixed, current repository reconciliation is clean, and the 22 focused tests pass. Two closely related AST edge cases still let real service behavior bypass the “exhaustive/fail-closed” contract:
Evidence
Recommendation: changes requested until these source forms fail closed. |
|
✅ Action performedReview finished.
|
|
Addressed the final delta findings from issuecomment-4938897198 in 13d3a89. Tests-first evidence:
Correction:
Validation:
Requesting the final delta review of fae7b83..13d3a89. The parent remains open pending reviewer clearance. @coderabbitai review |
|
✅ Action performedReview finished.
|
Final delta review of
|
|
Addressed the final binding-identity P1 from issuecomment-4939003365 in 0570c05. Tests-first evidence:
Correction:
Validation:
Requesting the final delta review of 13d3a89..0570c05. The parent remains open pending reviewer clearance. @coderabbitai review |
|
✅ Action performedReview finished.
|
Final-final delta review of
|
d6316c1
into
codex/ui-semantic-contrast
1 participant
Summary
routeTree.gen.tsfiles and reconcile it one-to-one with source route declarationsThis is contract/infrastructure only. It does not change page/component/domain behavior and does not add browser evidence.
Reconciled baseline
The 239 visual records plus 17 redirect-only records reconcile to 256 browser-addressable patterns. The earlier ~203 estimate treated many trailing-slash index pages as layout-only; the generated/source identity contract correctly keeps those records as pages.
All 239 visual records are explicitly
unassessedwith pending placeholders. There are zero fabricated verified scenarios.Validation
pnpm check:page-coveragelat checkgit diff --checkmainSummary by cubic
Adds an exhaustive page coverage contract that reconciles all registered
TanStackroutes, docs, and service surfaces with a human-owned plan. Generates deterministic JSON/MD ledgers, enforces the contract in tests/CI, proves service delegation paths, and binds proxy coverage to a canonical import; no runtime or UI changes.docs/ui-library/page-coverage.plan.json; generatedocs/ui-library/page-coverage-ledger.jsonand.md.page-coverage:scaffold,page-coverage:generate,check:page-coverage; CI runspnpm check:page-coverage.Written for commit 0570c05. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests