Repository navigation
Allowlist visual regression files in Frontend Unused File Guard - #3546
PetrefiedThunder wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 50d4915b-05d2-4f40-a11f-f7a11b05712f) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description covers the purpose and the change. It explains why the visual regression files are flagged, lists the two Knip entry points added in frontend/package.json, and notes that lib/capabilit… | Restructure the description to follow the template. Add the Summary as 1-3 bullet points. Check the applicable Type of Change option, which is likely CI/CD or infrastructure. Complete the Test Plan by stating how the change was verified, fo… |
-
Check name Status Explanation Title check The title "Allowlist visual regression files in Frontend Unused File Guard" matches the change. The PR adds Knip entry points in frontend/package.json for playwright.visual.config.ts and tests/visual/… Docstring Coverage No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "Allowlist visual regression files in Frontend Unused File Guard" matches the change. The PR adds Knip entry points in frontend/package.json for playwright.visual.config.ts and tests/visual/*.spec.ts so the Frontend Unused File Guard stops flagging them. The title is specific and describes the main change. It is 9 words and 63 characters, which is within a reasonable length for a title.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Full details: Description check
-
Explanation
The description covers the purpose and the change. It explains why the visual regression files are flagged, lists the two Knip entry points added in frontend/package.json, and notes that lib/capabilities.ts is intentionally excluded. It does not follow the repository template. The Summary section is a prose paragraph rather than 1-3 bullets, and the Type of Change, Test Plan, Checklist, and Related Issues sections are missing. The Test Plan gap matters most, because the description does not say how the change was tested. The description also contains a non-technical note ("Reviewer: VS Code Codex. Do not merge while the merge freeze is on.") that does not belong in the PR body.
Resolution
Restructure the description to follow the template. Add the Summary as 1-3 bullet points. Check the applicable Type of Change option, which is likely CI/CD or infrastructure. Complete the Test Plan by stating how the change was verified, for example by running the Knip check locally or in CI. Complete the Checklist items. Add any related issue links, such as
#3513, under Related Issues. Move the merge-freeze note out of the description or into a PR comment or label.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
PR Blocker ChecklistHead SHA: Status: Ready
|
PetrefiedThunder
left a comment
PetrefiedThunder
left a comment
There was a problem hiding this comment.
REVIEW: PASS codex 547ed16
- Diff is exactly the claimed change: frontend/package.json knip.entry gains
playwright.visual.config.tsandtests/visual/*.spec.ts(plus trailing comma on the prior entry). No other files; lib/capabilities.ts not added. - Both entries match files on this head: frontend/playwright.visual.config.ts and 7 specs under frontend/tests/visual/ (helpers.ts should be reached via spec imports).
- package.json parses as valid JSON. No data added.
- Tests/knip not run locally (no node_modules, installs not allowed); CI is the gate. Frontend Unused File Guard and Legacy Pattern Guard were pending at review time.
github-actions
Bot
added
dependencies
…3546 #3535) (#3552) * fix(epcis-export): return honest empty state instead of sample data Remove SAMPLE_EPCIS_EVENTS and the hardcoded FDA sample CSV. Tenants with no rows now get an empty eventList / header-only CSV, count 0, dataSource 'empty' and a 'No data yet' message. Tests are data-free. Drop the now-stale detect-secrets baseline entries for the sample hashes. Signed-off-by: Christopher Sellers <1@christophersellers.com> * fix(frontend): restore main build after SEL-410/SEL-414 ErrorState collision #3475 and #3479 merged back to back and both defined ErrorState. The surviving component lacked requestId/retrying used by #3479 callers, a test mock had a duplicate useQAHealth key, and suppliers/page.tsx used NO_DATA_YET without importing it. Vercel production builds for 03903ee and e7ee4a7 failed type check; prod is still on c7874cb. - ErrorState: optional requestId line, retrying disables Retry and shows Retrying..., support link, className kept on root - drop duplicate useQAHealth mock key - import NO_DATA_YET in suppliers page * test(epcis): drop SAMPLE_EPCIS_EVENTS patches; data-free empty-state and header-only FDA CSV assertions * ci: fix required gates on #3524 (pin copy-lint actions, inventory new workflows, allowlist false-positive secret, declare visual/capabilities knip entries) * chore(secrets): refresh .secrets.baseline (detect-secrets 1.5.0) * test(ingestion): make #2868 error-logging tests data-free to fix collection test_ingest_error_logging_2868.py imported _FakePersistence, _FakeSession, _client and _make_shipping_event from test_webhook_router_v2_1342.py, which the real-data-only cleanup removed, so collection failed on main. Drop the ingest-batch route cases that needed a fabricated event, test _failure_diagnostics directly with no payload, and keep the chain-verify case using MagicMock and a local app builder. Remove the stale detect-secrets baseline entry. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> * fix(ci): scope suppliers no-data test; match uses: as YAML key in supply-chain checker * test(ingestion): restore data-free ingest_batch_failed route coverage for #2868 Mint TENANT_ID and KEY_ID with uuid4 at runtime. Re-add a POST /api/v1/webhooks/ingest failure test that uses caplog to check the traceback, tenant_id, event_count, source, error_type and correlation_id, and that the uuid4 canary lot code, product text and location never reach the log. A MagicMock persistence store raises to reach the handler's except block. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> * fix(frontend): show QA-signal loading row on dashboard; align qa-health test with ErrorState copy * fix(tenant): update route manifest expectations for two new tenant-scoped routes * ci: list 9 missing workflows in inventory; stop action-pin matching 'statuses:' permissions * ci(frontend): allowlist visual regression files in Knip unused file guard * test(ingestion): fix-forward email_ingest principal stub and sel409 admin DB env Signed-off-by: Christopher Sellers <1@christophersellers.com> * fix(deps): bump pypdf to 6.21.2 for CVE-2026-115572/115585/115580 * test(ingestion): fix suite A collection (2868 persistence import) and drop SAMPLE_EPCIS_EVENTS expectations Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> * fix(ci): stop Head SHA Check Guard failing main on check_suite/workflow_run events Those events run in the default-branch context, so failed PR verdicts were attributed to main. Keep the failure commit status on the PR head SHA; only fail the job for pull_request events. * ci: clear detect-secrets false positive in vercel-preview-guard * test(ingestion): make 944/945 envelope test data-free after #3500 fixture removal * ci: refresh .secrets.baseline line numbers (detect-secrets 1.5.0, no new findings) * test(ingestion): capture ingest_batch_failed via dedicated logger; bump factory-local pypdf floor * ci: refresh .secrets.baseline after test change (drop stale entry) * fix: address CodeRabbit review on #3552 (checkout creds, retry a11y name, EDI envelope literals, duplicate test name) --------- Signed-off-by: Christopher Sellers <1@christophersellers.com> Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com>
Merge Conflict with mainBranch Main conflicts with this branch, and a person must resolve it. Please resolve the conflict manually and push an updated branch: git checkout muse/unused-guard-visual-allowlist
git fetch origin main
git merge origin/main
# resolve conflicts and commit
git push origin muse/unused-guard-visual-allowlistLast attempted sync: 2026-10-23T22:46:28.982Z |
The Frontend Unused File Guard (Knip,
.github/workflows/legacy-code-scan.yml) flags the visual regression files that came in with #3513. They are not imported by app code:frontend/playwright.visual.config.tsis passed to Playwright with--configby the visual workflows, andfrontend/tests/visual/*.spec.tsare run by that config (helpers.ts and the JSON files are imported by those specs).This adds two Knip entry points in
frontend/package.json:playwright.visual.config.tstests/visual/*.spec.tsNothing else changes;
lib/capabilities.tsis intentionally left out.Reviewer: VS Code Codex. Do not merge while the merge freeze is on.
Note
Low Risk
Config-only Knip allowlist for CI dead-code scanning; no application or test execution logic changes.
Overview
Updates Knip configuration in
frontend/package.jsonso the Frontend Unused File Guard stops flagging visual regression assets that are only reached via Playwright, not app imports.Two new
knip.entrypaths are allowlisted:playwright.visual.config.ts(loaded with--configin visual workflows) andtests/visual/*.spec.ts(the specs those runs execute). No runtime or test behavior changes—only CI/static unused-file detection.Reviewed by Cursor Bugbot for commit 547ed16. Bugbot is set up for automated code reviews on this repo. Configure here.