fix(ci): decouple dashboard/report smoke from stand-only sts_audit - #32
Conversation
The dashboard smoke ('SARIF export + dashboard smoke') failed after the
DATA_DIR default flipped to the gitignored, CI-empty 'artifacts/'
(FileNotFoundError on artifacts/findings.json). Both CI steps also read the
stand-only, proprietary sts_audit corpus, which is slated for removal.
- add viz/fixtures/{findings.json,health-report.md}: a small SYNTHETIC golden
set (invented names) exercising every tool, tier, source family, category and
OWN leak-shape, so the smoke keeps catching template regressions without
depending on sts_audit or leaking real data.
- point both CI steps at the fixture (report.cli --findings, build_dashboard
positional DATA_DIR).
- build_dashboard: skip cleanly (exit 0) when findings.json/health-report.md are
absent, so an empty default artifacts/ is 'no work', not a hard failure.
- gitignore the generated viz/arch-dashboard.html (sibling of sts-dashboard.html).
Verified locally: report.cli + build_dashboard render from the fixture; empty and
missing dirs skip with exit 0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFSpD9yXgXePrFFX62XZ53
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds committed synthetic fixtures (findings.json, health-report.md) for CI smoke testing, updates viz/build_dashboard.py to make data_dir optional with skip-on-missing-file behavior versus error-on-explicit-missing, updates ci.yml to reference the fixtures directory, and ignores the generated dashboard HTML. ChangesFixture-driven CI dashboard smoke test
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Report as report.cli
participant Dashboard as build_dashboard.py
participant Fixtures as viz/fixtures
CI->>Report: run with --findings viz/fixtures/findings.json
Report->>Fixtures: read findings.json
CI->>Dashboard: run with viz/fixtures argument
Dashboard->>Fixtures: read findings.json, health-report.md
Dashboard-->>CI: generate dashboard HTML or skip if missing
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a9218373f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if missing: | ||
| print(f"build_dashboard: {STS} has no {', '.join(missing)} — nothing to render, skipping.") | ||
| return |
There was a problem hiding this comment.
Make fixture smoke fail when inputs are missing
When the caller supplies the new fixture path, treating missing inputs as success makes the dashboard smoke a no-op: report.cli only requires findings.json, so if viz/fixtures/health-report.md is deleted/renamed or the CI path points at an empty directory, the workflow's python3 viz/build_dashboard.py viz/fixtures still exits 0 here without rendering the dashboard. Limit the clean skip to the default empty artifacts/ case, or make CI assert the fixture files exist before relying on the smoke.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 78132ca (your first option). The clean skip now applies only to the implicit default artifacts/ (empty until a stand run populates it). An explicitly-supplied dir missing findings.json/health-report.md is now a hard error (argparse exit 2), so CI's build_dashboard.py viz/fixtures fails loudly if the fixture is ever deleted/renamed — no more silent no-op.
Verified: explicit valid → render (exit 0); default empty artifacts/ → skip (exit 0); explicit dir missing a file / nonexistent → exit 2.
Generated by Claude Code
… explicit Addresses Codex review on #32: the clean skip made the fixture smoke a no-op — an explicit `build_dashboard.py viz/fixtures` would still exit 0 if a fixture file were deleted/renamed, silently passing CI. Now the skip applies ONLY to the implicit default `artifacts/` (empty until a stand run); an explicitly-supplied dir missing findings.json/health-report.md is a hard error (exit 2), so CI fails loudly on a broken fixture. Verified: explicit valid -> render; default empty -> skip exit 0; explicit missing/nonexistent -> exit 2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFSpD9yXgXePrFFX62XZ53
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
What & why
The
SARIF export + dashboard smokeCI job is red onmain(regression that merged with #31). #31 flippedbuild_dashboard.py's defaultDATA_DIRfromsts_auditto the gitignored, CI-emptyartifacts/, and the smoke invokes it with no argument:Both smoke steps also read the stand-only, proprietary
sts_audit/corpus — which is slated for removal — so pointing CI back at it would only defer the break (and keeps real project data in the repo). Instead this decouples CI fromsts_auditentirely, using a committed synthetic golden fixture.Changes
viz/fixtures/findings.json,viz/fixtures/health-report.mdown-check/roslyn/codeql/infersharp), all four fix tiers (T1–T4), every source family, multiple categories, and all four OWN leak-shapes, so the smoke still catches Python/JS-template regressions..github/workflows/ci.ymlreport.cli --findings viz/fixtures/findings.jsonandbuild_dashboard.py viz/fixtures.viz/build_dashboard.pyfindings.json/health-report.mdare absent — an empty defaultartifacts/is "no work", not a hard failure..gitignoreviz/arch-dashboard.html(sibling of the already-ignoredsts-dashboard.html).Once
sts_audit/is deleted, nothing in CI breaks: neither step references it any more.How verified
Locally, reproducing the CI job:
report.cli --min-level warning --findings viz/fixtures/findings.json→ 14 findings → 11 SARIF results,report/out/written.build_dashboard.py viz/fixtures→ renders (14 findings, 4 modules).build_arch_dashboard.py→ empty-state (unchanged).build_dashboard.py artifacts(empty) and a nonexistent dir → both skip, exit 0.🤖 Generated with Claude Code
https://claude.ai/code/session_01DFSpD9yXgXePrFFX62XZ53
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes