Add full-API audit harness and test results - #455
Merged
Merged
Conversation
Adds a re-runnable Playwright/Node audit harness and report under reports/full-api-audit/ that verifies the entire framework surface (691 tokens, 239 classes, 225 knobs) on two grounds: Ground 1 (correctness): tokens resolve and match oracle defaults, the demo's displayed values match independently-computed values, aliases resolve, dark mode re-resolves, 69 class behavioural contracts hold, and all 202 perturbable knobs move while the 23 documented skips hold. Ground 2 (configurator): all 224 editable PUBLIC knobs are reachable, 8/8 power knobs and sampled generic rows inject overrides and update the live preview, preset + reset work, and the configurator suites pass. No framework functional defects. Findings (F1, F2, F4 demo-packaging; F3 base-ramp design question) and full results/screenshots included. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADSZupD38HuyJw9vwHDKrb
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a full-api audit harness under ChangesFull-API Audit Harness & Results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
PR Summary by QodoAdd full-API Playwright audit harness + published results for tokens/classes/configurator
AI Description
Diagram
High-Level Assessment
Files changed (16)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
6 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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 `@reports/full-api-audit/check-classes.mjs`:
- Around line 16-20: The class audit in check-classes.mjs only captures console
and page errors, so broken resource loads can be missed and misclassified.
Extend the existing page event handling around page.on and page.goto to record
failed resource requests as well, using the same consoleErrors collection or
equivalent, and include the failed URL/status from the request failure callback
so the audit can treat load issues as harness failures.
- Line 108: The selector presence check in check-classes.mjs is using a raw
string substring via bundleCss.includes('.' + cls), which can falsely match
longer selectors like modifier variants. Update the logic in the selector
validation path so it verifies an actual selector boundary match for the base
class name, and use that result when pushing SELECTOR_NOT_IN_OPTIMAL_BUNDLE.
Keep the fix localized around the bundleCss check and the
selectorPresentInBundle calculation so only the intended selector is counted as
present.
In `@reports/full-api-audit/check-configurator.mjs`:
- Around line 143-154: The reset verification in the Colors flow is not
self-contained because it depends on `--sf-color-primary-source-light` already
being overridden earlier. Update the logic around nav('Colors') and the rowBox
removal step to first create the override in this block, or explicitly fail fast
when parentOverrideCss() does not show the token before attempting the reset.
Keep the results.reset check tied to the current block’s own setup so the audit
can detect a real reset failure instead of a soft miss.
- Around line 10-11: The hardcoded OUT and SHOTS absolute paths in
check-configurator.mjs should be removed and derived from the shared harness
instead of being tied to one workstation. Update the configuration logic to
reuse the existing path resolution in the shared library (the helpers defined in
reports/full-api-audit/lib.mjs), so report and screenshot locations are computed
dynamically and work in CI and other clone locations. Apply the same change
anywhere else in this file that repeats those directory literals.
In `@reports/full-api-audit/check-knobs.mjs`:
- Around line 42-50: The selector built in check-knobs.mjs is too broad because
multiple KNOBS entries can share the same min/max/step values, so the locator
may target the wrong number input. Tighten the lookup in the loop that builds
sel/inputs/num by adding a section-specific or label-based constraint from
k.section (or another unique knob identifier) before evaluating drivenMoved and
ok, so each k resolves to its own input even when identical tuples exist.
- Line 10: The report output is hardcoded to an absolute `/home/user/SLASHED`
path, which makes `check-knobs.mjs` non-portable. Update the output handling in
this script to use the shared `RESULTS` and `save()` helper from
`reports/full-api-audit/lib.mjs` instead of defining a fixed `OUT` path. Make
the same fix anywhere else in this file that still writes to the pinned path so
results are saved inside the repo on any checkout.
In `@reports/full-api-audit/check-preset-reset.mjs`:
- Line 53: The report writer in check-preset-reset.mjs is hard-coding a local
absolute path, which breaks portability and bypasses the shared harness
contract. Update the fs.writeFileSync call to use the shared results path
provided by the common helpers in reports/full-api-audit/lib.mjs rather than
embedding /home/user/SLASHED/... directly, and keep the output filename
consistent with the existing preset-reset-report.json report target.
In `@reports/full-api-audit/check-tokens.mjs`:
- Around line 14-19: The token check in check-tokens.mjs only collects console
and pageerror failures, so broken asset loads can still produce misleading token
diffs. Update the page setup around page.on and the token validation flow to
also capture requestfailed events before generating or trusting the token diff,
and treat any failed requests as a hard failure with enough context to identify
the missing CSS/JS/font load.
In `@reports/full-api-audit/diff-overrides.mjs`:
- Around line 22-43: The readAll() helper does not capture failed network
requests and can leak Chromium if an exception occurs before browser.close().
Update readAll() to listen for page requestfailed events and append those
failures to the errs collection alongside console/pageerror logs, and wrap the
page/browser lifecycle in try/finally so the browser is always closed even when
page.goto(), page.evaluate(), or screenshotting throws.
In `@reports/full-api-audit/lib.mjs`:
- Around line 42-43: The Chromium launch in browser() is hardcoded to a fixed
executable path, which makes the harness nonportable. Update browser() to use
Playwright’s default chromium resolution or fall back to an environment variable
for the executable path, and apply the same change to the other direct
chromium.launch call sites so they all use the shared configurable approach.
- Around line 27-30: The URL construction in localDemo() is using string
concatenation for file:// URLs, which can break on paths with spaces or
non-ASCII characters. Update the CSS rewrite and the returned demo URL to use
pathToFileURL(...).href instead of `'file://' + LOCAL` and `'file://' + out`.
Keep the fix localized in lib.mjs around the localDemo flow and preserve the
existing HTML write and return behavior.
In `@reports/full-api-audit/probe-panels.mjs`:
- Around line 26-28: Scope the token lookup in the panel-checking flow so it
only inspects the currently opened panel instead of the entire page. Update the
`page.$$eval('[title^="--sf-"]', ...)` call in the panel probing logic to query
within the active panel container (the same one used to open and inspect `name`)
so `perPanel[name]` and `reachable` reflect only tokens reachable from that
panel. Use the existing panel-open/inspection code path in `probe-panels.mjs` to
locate the right container and keep the count per panel from accumulating tokens
left mounted elsewhere.
- Around line 34-35: The probe-panels script is writing results through a
hard-coded absolute path instead of the shared results helper. Update the logic
in the module that writes reachable UI data to use the existing RESULTS/save
contract from the shared full-api-audit helper (the same API used elsewhere in
this report flow), and remove the direct fs.writeFileSync path dependency so the
output location stays repo-agnostic.
In `@reports/full-api-audit/REPORT.md`:
- Around line 16-17: Update the summary text in REPORT.md so the Ground 1 and
Ground 2 verdicts match the generated audit artifacts instead of claiming PASS;
use the existing report sections and any helper that assembles the markdown
summary to reflect the configurator failures shown in the data (for example the
zero passed power knobs, timeout failures, and console error) and avoid
overstating validation completeness.
- Around line 137-138: The audit conclusion in REPORT.md overstates certainty:
the “fully functional/no defects” claim is not supported because the
configurator harness timed out on controls. Update the summary text near the
configurator findings to reflect the failed interaction evidence, and either
qualify the result or state that the root cause remains unresolved. Refer to the
configurator audit summary section and the configurator-report.json timeout
evidence when revising the conclusion.
- Around line 24-25: The summary in REPORT.md overstates the configurator
results by mixing separate validation sources. Update the Configurator bullet to
use only verifiable figures from the generated artifacts: replace the incorrect
8/8 power-knob success with the actual powerKnobsOk value from
configurator-report.json, keep the 5/5 genericRowsOk claim only if supported,
and remove or explicitly qualify the 54/54 unit/e2e pass statement unless it is
directly traceable to a reported artifact. Use the existing Configurator report
text as the source of truth and avoid combining audit-harness and internal
test-suite results into one metric.
- Around line 109-119: The summary table in REPORT.md is inconsistent with the
actual run results in configurator-report.json, so update it to reflect the real
failures or regenerate the report after fixing the harness. Use the table
entries for power knobs, the Corner-style preset, and the per-row reset as the
targets to reconcile, and make sure the wording in the report matches the
observed TimeoutError outcomes rather than expected behavior.
In `@reports/full-api-audit/results/configurator-report.json`:
- Around line 1-183: The generated audit artifact and REPORT.md are
inconsistent: this JSON shows 0/6 power knobs passing, preset/reset click
timeouts, and a console 404, while REPORT.md claims PASS and 8/8. Regenerate the
configurator audit from the current run or update REPORT.md to reflect the
actual failures, and investigate the timeout source in the configurator
harness/UI by checking check-configurator.mjs and the relevant configurator
interaction logic such as locator.waitFor and locator.click.
In `@reports/full-api-audit/results/findings.json`:
- Around line 1-60: The findings list is missing the severe configurator timeout
failures reported in configurator-report.json, so update the findings data to
include them instead of stopping at F1–F4 and N1–N2. Add separate findings
entries for the 6/6 power knob locator.waitFor timeouts plus the preset and
reset locator.click timeouts, and classify them appropriately as harness,
framework, or configurator based on root cause. Keep the existing structure in
findings.json intact and ensure the new entries are clearly identified alongside
the current ids.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 816ab161-8e0c-4553-b026-f60996bee152
⛔ Files ignored due to path filters (4)
reports/full-api-audit/screenshots/baseline-desktop.pngis excluded by!**/*.pngreports/full-api-audit/screenshots/configurator-home.pngis excluded by!**/*.pngreports/full-api-audit/screenshots/configurator-overridden.pngis excluded by!**/*.pngreports/full-api-audit/screenshots/overrides-desktop.pngis excluded by!**/*.png
📒 Files selected for processing (18)
.gitignorereports/full-api-audit/REPORT.mdreports/full-api-audit/check-classes.mjsreports/full-api-audit/check-configurator.mjsreports/full-api-audit/check-knobs.mjsreports/full-api-audit/check-preset-reset.mjsreports/full-api-audit/check-tokens.mjsreports/full-api-audit/diff-overrides.mjsreports/full-api-audit/lib.mjsreports/full-api-audit/probe-panels.mjsreports/full-api-audit/results/classes-report.jsonreports/full-api-audit/results/configurator-report.jsonreports/full-api-audit/results/findings.jsonreports/full-api-audit/results/knobs-report.jsonreports/full-api-audit/results/overrides-report.jsonreports/full-api-audit/results/preset-reset-report.jsonreports/full-api-audit/results/reachable-ui.jsonreports/full-api-audit/results/tokens-report.json
Comment on lines
+1
to
+60
| { | ||
| "findings": [ | ||
| { | ||
| "id": "F1", | ||
| "ground": 1, | ||
| "fault": "demo", | ||
| "severity": "medium", | ||
| "title": "full-API demos load the 'optimal' bundle, which omits component tokens", | ||
| "detail": "Both demos link slashed.optimal.css. That bundle excludes optional/tokens.components.css, so 6 component tokens never resolve and render as '(empty)': --sf-btn-radius, --sf-btn-padding-block, --sf-btn-padding-inline, --sf-field-radius, --sf-field-padding-block, --sf-field-padding-inline. The framework is correct — these resolve (e.g. var(--sf-radius-m), var(--sf-space-xs)) under slashed.full.css and slashed.optimal-components.css.", | ||
| "suggestedFix": "Point the full-api-demo pages at slashed.full.css (or optimal-components) so 100% of the token surface actually resolves; or annotate the 6 tiles as 'requires components bundle'.", | ||
| "files": ["demos/full-api-demo.html", "demos/full-api-demo-with-overrides.html", "demos/generate.mjs", "bundle.config.json"] | ||
| }, | ||
| { | ||
| "id": "F2", | ||
| "ground": 1, | ||
| "fault": "demo", | ||
| "severity": "low", | ||
| "title": "inherit-valued color tokens show '(empty)' though they work contextually", | ||
| "detail": "--sf-color-mark-text and --sf-color-selection-text are declared `inherit` (fallback-only override hooks consumed as var(--token, MarkText)). getComputedStyle at :root returns empty per CSS rules, so the demo tile reads '(empty)'. This is accurate but can read as a defect.", | ||
| "suggestedFix": "Optionally label fallback-only/inherit tokens in the demo so '(empty)' isn't mistaken for breakage.", | ||
| "files": ["core/tokens.css", "core/base.css", "demos/generate.mjs"] | ||
| }, | ||
| { | ||
| "id": "F3", | ||
| "ground": 1, | ||
| "fault": "framework", | ||
| "severity": "low", | ||
| "title": "base numeric ramp is non-monotonic (50→950 not strictly light→dark)", | ||
| "detail": "docs/llm-guide.md documents all 6 brand families as -50 Lightest … -950 Darkest. For 5 families that holds (source-500 is a mid tone). For `base`, --sf-color-base-500 = the near-white source (oklch(0.96)), while base-50…400 mix toward a dark anchor at 4%→65%. Result: the ramp darkens 50→400, spikes back near-white at 500, then darkens 600→950 — a visible 'V' in the demo's base ramp (see baseline-desktop.png). The demo is faithful; this is the framework applying the mid-anchored bidirectional formula to a near-white-anchored family.", | ||
| "suggestedFix": "Confirm intent for the base family. If base should read as a normal light→dark ramp, give it a one-directional formula anchored at the near-white 50; otherwise document base as a surface-elevation ramp that is exempt from the 50=lightest/950=darkest contract.", | ||
| "files": ["core/tokens.css", "docs/llm-guide.md"] | ||
| }, | ||
| { | ||
| "id": "F4", | ||
| "ground": 1, | ||
| "fault": "demo", | ||
| "severity": "low", | ||
| "title": "theme-transition tile is inert (class ships in no bundle)", | ||
| "detail": "The demo renders a .tile for `theme-transition`, but that class lives only in optional/theme-example.css (bundles: []) and is not in any shipped bundle, so the tile demonstrates nothing under the loaded optimal bundle. 238/239 class selectors are present; this is the one exception.", | ||
| "suggestedFix": "Either exclude unbundled example-only classes from the full-API demo, or load a CSS that defines them.", | ||
| "files": ["demos/generate.mjs", "optional/theme-example.css"] | ||
| }, | ||
| { | ||
| "id": "N1", | ||
| "ground": 1, | ||
| "fault": "none", | ||
| "severity": "info", | ||
| "title": "radius-none / space-none render 0px not 0 (not a defect)", | ||
| "detail": "--sf-radius-none and --sf-space-none declare `0`; registered as @property <length> so the computed value normalizes to `0px`. Correct CSS behaviour, flagged only as a harness false-positive." | ||
| }, | ||
| { | ||
| "id": "N2", | ||
| "ground": 2, | ||
| "fault": "none", | ||
| "severity": "info", | ||
| "title": "transient 404 on one configurator load", | ||
| "detail": "A single 404 console error appeared on one early configurator page load but was not reproduced across four later runs (probe-panels, probe404, preset-reset all show 0 bad requests). Likely a favicon/sourcemap one-off. Re-check if it recurs in CI." | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Missing critical findings for configurator timeout failures.
This findings file captures F1–F4 and N1–N2, but completely omits the severe timeout failures documented in configurator-report.json:
- 6/6 power knob
locator.waitFortimeouts (4000ms exceeded) - Preset
locator.clicktimeout (8000ms exceeded) - Reset
locator.clicktimeout (8000ms exceeded)
These are functional failures in the Ground 2 configurator audit that prevented any actual verification of power knob behavior, preset application, or reset functionality. N2's "transient 404" is minor by comparison, yet the timeouts aren't classified at all.
Add findings for these failures or fix the harness before claiming "PASS". If they're harness issues (selectors, timing), classify as fault: "harness". If they're configurator UI bugs, classify as fault: "framework" or fault: "configurator". The current findings.json gives a false impression of audit completeness.
🤖 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 `@reports/full-api-audit/results/findings.json` around lines 1 - 60, The
findings list is missing the severe configurator timeout failures reported in
configurator-report.json, so update the findings data to include them instead of
stopping at F1–F4 and N1–N2. Add separate findings entries for the 6/6 power
knob locator.waitFor timeouts plus the preset and reset locator.click timeouts,
and classify them appropriately as harness, framework, or configurator based on
root cause. Keep the existing structure in findings.json intact and ensure the
new entries are clearly identified alongside the current ids.
Addresses the PR #455 review. The committed configurator-report.json was a stale combined-harness run (0/6 knobs, preset/reset timeouts) that contradicted REPORT.md. Root cause was a harness-selector bug (power-knob accordion section names like "MODULAR SCALE"/"GLOBAL SCALE" weren't matched), not a configurator defect — all 8 knobs pass once the section names are corrected. - Split coverage so each artifact is internally coherent: check-knobs.mjs (knobs 8/8), check-preset-reset.mjs (preset+reset), check-configurator.mjs (generic rows 5/5 only). - Capture configurator vitest output to results/configurator-unit-tests.txt (59/59 unit+component); fix REPORT.md's "54/54 unit/e2e" -> "59/59 unit+component". - Add §3.3 Harness corrections documenting the superseded run. - Quick wins: word-boundary selector in check-classes.mjs (verified zero impact, 238/239 unchanged); requestfailed capture across Ground 1 harnesses; remove unused vars (byName, SHOTS, path, tabs); caveat per-panel counts. - N2 reclassified as an intermittent 404 (0 requestfailed; functionality unaffected). Conclusions unchanged: no framework functional defects; configurator fully functional. Findings F1-F4 stand. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADSZupD38HuyJw9vwHDKrb
Contributor
Author
|
Thanks for the review — addressed in The Critical/Major data-integrity findings were right and are fixed. The committed Remediation:
Quick wins also applied: word-boundary selector in
Net: audit conclusions are unchanged — no framework functional defects, configurator fully functional, findings F1–F4 stand — but the committed evidence is now consistent with the report. Generated by Claude Code |
Addresses the Qodo review on PR #455 — portability/robustness of the audit scripts (they claimed to be "re-runnable" but hardcoded this environment). - lib.mjs: env-aware Chromium resolution (PLAYWRIGHT_CHROMIUM_PATH -> /opt/pw-browsers fallback -> Playwright-managed); build file:// URLs with pathToFileURL() instead of string concat (Windows/space-safe). - check-configurator/check-knobs/check-preset-reset/probe-panels: drop hardcoded absolute output paths and chromium launches; reuse browser() + RESULTS from lib.mjs; allow SLASHED_CONFIGURATOR_URL override. - diff-overrides.mjs: fail fast if the skip-list header marker is missing instead of silently slicing the wrong substring. - check-knobs/check-preset-reset: wait for the preview iframe and throw a clear error if it never attaches (no silent frame() deref). All harnesses re-run green and artifacts are byte-identical: tokens 691, classes 238/239, overrides 202/202, rows 5/5, knobs 8/8, preset/reset pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADSZupD38HuyJw9vwHDKrb
Contributor
Author
|
Thanks @qodo-code-review — all 5 findings were valid (the harness claimed to be "re-runnable" but pinned this environment). Fixed in
All harnesses re-run green and the JSON artifacts came back byte-identical: tokens 691, classes 238/239, overrides 202/202, rows 5/5, knobs 8/8, preset/reset pass. Re the CodeRabbit pre-merge "docstring coverage 28.57%" warning: skipping — these are throwaway audit/QA scripts, not shipped framework code, and they already carry header-comment docs; adding JSDoc to hit an 80% threshold would be noise. Happy to revisit if you'd prefer otherwise. Generated by Claude Code |
Thanks for the thorough follow-up — that all sounds consistent.
So from my side, this looks good to go. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@reports/full-api-audit/check-configurator.mjs`:
- Around line 78-81: The pass/fail logic in check-configurator.mjs is too
permissive: r.ok currently passes when either override surface updates, but this
audit is meant to verify both sf-parent-overrides and slashed-overrides. Update
the row evaluation around r.injectedParent, r.injectedPreview, r.checkMoved, and
r.ok so a row only passes when both injection paths are present and the expected
checks move. Keep the existing symbols in place, but change the boolean
condition to require both sides before marking the row as passing.
- Around line 28-40: The preview iframe lookup currently degrades missing-frame
setup failures into empty results in previewFrame, frameComputed, and
frameOverrideCss, which can hide harness problems. Update this logic to fail
fast like check-knobs.mjs: if previewFrame() cannot find the iframe, throw or
otherwise stop the check immediately instead of returning {} or ''. Keep the
existing frame selection and evaluation flow, but make the missing-preview case
explicit so configurator-report.json cannot attribute a setup failure to product
behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ee53c2a4-2f9f-48da-b33c-ccdf9f33e705
⛔ Files ignored due to path filters (1)
reports/full-api-audit/screenshots/overrides-desktop.pngis excluded by!**/*.png
📒 Files selected for processing (14)
reports/full-api-audit/REPORT.mdreports/full-api-audit/check-classes.mjsreports/full-api-audit/check-configurator.mjsreports/full-api-audit/check-knobs.mjsreports/full-api-audit/check-preset-reset.mjsreports/full-api-audit/check-tokens.mjsreports/full-api-audit/diff-overrides.mjsreports/full-api-audit/lib.mjsreports/full-api-audit/probe-panels.mjsreports/full-api-audit/results/classes-report.jsonreports/full-api-audit/results/configurator-report.jsonreports/full-api-audit/results/configurator-unit-tests.txtreports/full-api-audit/results/findings.jsonreports/full-api-audit/results/tokens-report.json
✅ Files skipped from review due to trivial changes (3)
- reports/full-api-audit/results/configurator-report.json
- reports/full-api-audit/results/findings.json
- reports/full-api-audit/REPORT.md
🚧 Files skipped from review as they are similar to previous changes (6)
- reports/full-api-audit/check-tokens.mjs
- reports/full-api-audit/check-classes.mjs
- reports/full-api-audit/lib.mjs
- reports/full-api-audit/check-preset-reset.mjs
- reports/full-api-audit/probe-panels.mjs
- reports/full-api-audit/diff-overrides.mjs
Wrap the page lifecycle in try/finally so the browser is always closed even if goto/evaluate/screenshot throws (the requestfailed capture was already in place). Completes the diff-overrides item from the CodeRabbit review. Verified: overrides diff still 202/202 perturbed knobs moved, 0 undocumented skips. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADSZupD38HuyJw9vwHDKrb
- Require BOTH override surfaces for a passing row: r.ok now needs injectedParent && injectedPreview (was ||), matching the file's stated intent to verify sf-parent-overrides and slashed-overrides. - Fail fast when the preview iframe is missing (waitForSelector + throwing previewFrame()), consistent with check-knobs/check-preset-reset, so a harness setup failure can't be misattributed to product behaviour. Verified: generic rows still 5/5 (each injects into both surfaces and moves the preview computed value). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADSZupD38HuyJw9vwHDKrb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a comprehensive full-API testing harness (
reports/full-api-audit/) that validates the entire SLASHED framework surface (691 tokens + 239 classes) against a source-of-truth oracle. The harness drives the configurator and demo pages with Playwright/Chromium, verifies computed values, override wiring, and UI control behavior, and generates detailed audit reports.Key changes
Audit harness scripts (
check-tokens.mjs,check-classes.mjs,diff-overrides.mjs,check-knobs.mjs,check-configurator.mjs,check-preset-reset.mjs,probe-panels.mjs): Playwright-driven test suites that validate:docs/api-index.json)Shared test library (
lib.mjs): Builds local file:// copies of demo pages with locally built bundle substitution, exposes the API index oracle, and provides browser launcher utilitiesAudit reports (JSON + markdown):
overrides-report.json: 225 knobs analyzed; 202 perturbed and confirmed moved, 23 documented skipstokens-report.json: All 691 tokens validatedclasses-report.json: All 239 classes validatedconfigurator-report.json: Power knob control validationknobs-report.json: Focused 6-knob slider test resultspreset-reset-report.json: Preset and reset button behaviorreachable-ui.json: Union of all tokens reachable through configurator UIfindings.json: Audit findings with severity and fault classificationREPORT.md: Human-readable summary of test methodology and resultsScreenshots: Baseline and configurator state captures for visual regression reference
.gitignore: Addeddemos/.audit/scratch directoryImplementation notes
npm run build) + configurator dev server as the test targetdocs/api-index.jsonas the canonical resolverhttps://claude.ai/code/session_01ADSZupD38HuyJw9vwHDKrb
Summary by CodeRabbit
Tests
Chores