feat: dashboard interactivity + Own.NET Auditor roadmap + SARIF exporter (phase 1) - #7
Conversation
…s-run trend Reworks the dashboard from static server-rendered charts into an interactive view: - Filter chips for tool and category; every live chart (category, source, tool, rules, tiers) and the findings table re-aggregate client-side. Per-finding rows are embedded with string interning (paths/rules/tools/cats -> int indices) so filtering over 72k findings stays cheap and the file stays ~6.8MB. - Drill-down: click a module treemap tile to filter to its findings (longest segment-aligned module prefix), shown in a sortable-looking findings table with file:line, rule, tier badge, category, tool. - Fix-tier chart (T1 auto / T2 review / T3 unfixable / T4 bespoke), tiers taken from the real fixarm.tiers.tier_of so the dashboard and the fix arm never drift. - Trend across runs: each build appends a dated snapshot (total + per-tier) to viz/history.jsonl; the trend chart plots the series (one point until the next run, with a note). Verified headless: 8 charts, no console errors; own-check filter -> 380 (all T4), Broker drill-down -> 8,986 findings, tier totals T1 3935 / T2 58403 / T3 9851 / T4 380. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
…roducts) Strategy note from the "we have 72k findings — what other value can we extract" discussion. Covers: the FP-vs-true-but-wontfix reframe, why FP rate is unmeasurable without ground truth and the per-rule-homogeneity trick that makes labeling cheap, a metric catalog (free-now / needs-labeling / needs-trend), and ranked byproduct options (rule-verdict sheet, churn hotspots, codemod mining; AI corpus parked, public dataset advised against). Grounded in real numbers from the current run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
…eline Turns the "make Own.NET an auditor like NDepend + Joern + WPF inspector" vision into a phased plan over the EXISTING OwnAudit instead of a greenfield CLI. Maps vision -> what already exists (findings.json, the audit orchestrator, fix-arm diff_findings, history.jsonl, the dashboard) -> the real gap (architecture graph + SARIF/baseline output). Three guardrails: don't rebuild Roslyn's semantic layer (graph = projection over symbols), justify the graph by cross-cutting architecture not re-derived metrics, sequence the runtime track as Windows-only. Phases: SARIF -> baseline/diff -> architecture-pass -> drift report -> runtime correlation; DSL and the 40-metric zoo deferred. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
…r phase 1) First slice of the auditor roadmap: a pure transform over the existing sts_audit/findings.json into SARIF — no .NET, testable in CI. - report/sarif.py: one run per tool (own-check/CodeQL/Infer#/Roslyn), deduped rule set per driver, category->level mapping (leaks=error, correctness/arch= warning, style=note), startLine clamped, line-independent partialFingerprints so GitHub correlates an alert across edits, suppression passthrough, tier+ category in result properties. Reuses fixarm.tiers.tier_of as the single source of truth for T1..T4. - min_level / max_results_per_run so you export by severity instead of dumping 72k alerts: the full log is 47MB (over GitHub's gzip limit), --min-level warning brings it to 6,115 results / ~4MB. - report/cli.py: writes ownnet-audit.sarif + metrics.json + report.md. - 9 tests (shape, rule dedup+index, level mapping, region clamp, fingerprint stability/line-independence, suppression, min_level, max_results, tier), green under python3 and python3 -O. report/out/ gitignored as a build artifact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
|
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds two planning documents, a SARIF export and CLI report generator, and a filterable dashboard with compact embedded data and history snapshots. ChangesPlanning documents
SARIF export pipeline
Interactive dashboard
Sequence Diagram(s)SARIF export flow sequenceDiagram
participant Main as report.cli.main
participant Sarif as report.sarif.to_sarif
participant Metrics as report.cli._metrics
participant Render as report.cli._report_md
Main->>Sarif: convert findings.json with min_level/max_results_per_run
Sarif-->>Main: SARIF log
Main->>Metrics: aggregate totals by tool, category, level, and tier
Metrics-->>Main: metrics dict
Main->>Render: render markdown summary tables
Render-->>Main: report.md text
Main->>Main: write ownnet-audit.sarif, metrics.json, report.md
Dashboard filter/update flow sequenceDiagram
participant Collect as collect()
participant Update as update()
participant Filtered as filteredRows()
participant Treemap as drawTreemap()
participant Tiers as drawTiers()
participant Table as drawTable()
participant Trend as drawTrend()
Collect-->>Update: D.rows and D.history
Update->>Filtered: select rows by chips and module drilldown
Filtered-->>Update: filtered rows
Update->>Treemap: redraw filtered treemap
Update->>Tiers: redraw tier bars from filtered rows
Update->>Table: render filtered findings table
Update->>Trend: redraw trend from D.history
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 392b59cca0
ℹ️ 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".
| ap.add_argument("--min-level", choices=("note", "warning", "error"), default=None, | ||
| help="drop results below this SARIF level (GitHub-friendly export)") | ||
| ap.add_argument("--max-results", type=int, default=None, | ||
| help="cap results per run (highest severity kept first)") |
There was a problem hiding this comment.
With the current sts_audit/findings.json, the documented default python3 -m report.cli leaves both filters unset and emits 62,338 Roslyn results in one run; GitHub's SARIF support caps uploads at 25,000 results per run (https://docs.github.com/en/enterprise-cloud@latest/code-security/reference/code-scanning/sarif-files/sarif-support#file-compatibility), so the default ownnet-audit.sarif advertised as GitHub-ready is rejected unless callers know to pass --min-level/--max-results. Please make the default output GitHub-safe or fail before writing an unuploadable artifact.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex Fixed in 799e018. --max-results now defaults to GitHub's per-run limit (GITHUB_MAX_RESULTS_PER_RUN = 25000), so the default python3 -m report.cli is uploadable out of the box — Roslyn's 62,338 results cap to 25,000 (highest severity first), and the drop is reported on stderr and in metrics.json's export block. --max-results 0 opts back into the full (non-GitHub-safe) set. Test: test_default_cap_is_github_limit + the CLI integration test.
Generated by Claude Code
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
| def _fingerprint(f) -> str: | ||
| """Stable, line-INDEPENDENT id (rule + path + normalized message) so an alert is the | ||
| same across commits even if the line moved.""" | ||
| basis = "\n".join([f.get("rule") or "", f.get("path") or "", _norm(f.get("message"))]) |
There was a problem hiding this comment.
On files that contain multiple instances of the same rule with the same message at different lines, this basis gives every instance the same fingerprint (the current data has thousands of such groups, including repeated OWN001 hits in a single XAML code-behind). Since SARIF consumers use partialFingerprints as result identity, these distinct alerts can be merged or correlated to the wrong occurrence; use a per-occurrence signal such as a source-line hash/duplicate counter instead of only rule+path+message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex Fixed in 799e018. Added a per-occurrence disambiguator: within a run I keep a counter of identical base fingerprints, so the first hit keeps the stable line-independent base and each subsequent identical rule+path+message gets base/1, base/2, … — distinct alert identities, while the first occurrence still survives edits. Test test_duplicate_findings_get_distinct_fingerprints (two OWN001 at different lines in one file → 2 distinct fingerprints, first == base).
Generated by Claude Code
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
docs/audit-data-leverage.md (1)
35-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLabel the plain-text fences.
These blocks are not code, so the unlabeled fences trip markdownlint (
MD040). Mark them astextfor clarity and to keep the doc lint-clean.Proposed fix
-``` +```text 1. INPC020 5607 7.7% (INotifyPropertyChanged correctness) 2. MA0006 5198 7.2% (Meziantou) 3. MA0003 3991 5.5% 4. INPC003 3472 4.8% 5. WPF0041 3351 4.6% (WpfAnalyzers) 6. MA0011 3194 4.4% 7. INPC017 2595 3.6% 8. INPC013 2529 3.5% 9. cs/useless-assignment-to-local 1954 2.7% (CodeQL dead store) 10. INPC002 1567 2.2% -``` +``` -``` +```text general-quality 32801 45.2% <- главный кандидат на wontfix / severity-тюнинг inpc-correctness 17122 23.6% architecture 15301 21.1% wpf-freezable 3902 5.4% idisposable-leak 2440 3.4% uncategorized 670 0.9% subscription-leak 326 0.4% region-escape 7 0.0% -``` +``` -``` +```text T1 (auto) 3935 5.4% T2 (review) 58403 80.5% T3 (unfixable) 9851 13.6% T4 (bespoke) 380 0.5% -``` +```Also applies to: 49-58, 61-66
🤖 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 `@docs/audit-data-leverage.md` around lines 35 - 46, The markdown fence blocks in the audit data tables are plain text, so they should be labeled with text to satisfy markdownlint MD040. Update the fenced blocks in the documented tables near the referenced sections by using text-labeled fences consistently, including the blocks containing the category counts and T1/T2/T3/T4 breakdown, so the doc remains lint-clean.Source: Linters/SAST tools
docs/own-net-auditor.md (1)
31-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTag the illustrative fences as
text.These are prose snippets/diagrams, not code blocks. The unlabeled fences trigger markdownlint (
MD040) and are slightly harder to read without a language hint.Proposed fix
-``` +```text Own.NET Auditor = semantic code graph + architecture rules + WPF/runtime-specific probes + baseline/diff + SARIF/GitHub reports + AI explanation layer -``` +``` -``` +```text PR `#1234` increased coupling in Broker.Documents by 18%. New dependencies: Broker.Documents -> DevExpress.Xpf.Grid, -> System.Data.SqlClient New cycle: Broker.Documents -> Broker.Services -> Broker.Documents Risk: High — domain-ish module now depends on UI and SQL infrastructure. -``` +``` -``` +```text static: subscribes to DocumentStore.Changed, no matching unsubscribe runtime: 132 retained instances, held by static DocumentStore.Changed delegate => MEM-WPF-014: event leak confirmed, ~84 MB retained after closing window 10× confidence: high -``` +```Also applies to: 73-77, 121-126, 130-135
🤖 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 `@docs/own-net-auditor.md` around lines 31 - 39, The markdown examples in the docs are plain prose/diagrams, not code, so the unlabeled fenced blocks should be tagged as text to satisfy markdownlint MD040. Update each relevant fence in the documentation snippet to use the text language hint, including the illustrative blocks around the Own.NET Auditor summary, the dependency/cycle example, and the memory leak probe example, so the content remains readable without being treated as code.Source: Linters/SAST tools
🤖 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 `@report/cli.py`:
- Around line 64-70: The CLI currently writes metrics and the markdown report
from the unfiltered findings while SARIF is generated from the filtered/exported
subset. Update the logic in cli.py around to_sarif() and _metrics() so
metrics.json and report.md are derived from the same filtered result set, or
explicitly include both raw and exported counts in the metrics/report output.
Use the existing args.min_level and args.max_results flow to keep all emitted
artifacts consistent.
In `@report/sarif.py`:
- Around line 74-79: Update to_sarif to validate its inputs before building the
log: reject any min_level value not recognized by _LEVEL_RANK instead of
defaulting to no filtering, and reject negative max_results_per_run values in
the to_sarif flow. Use the existing to_sarif function and _LEVEL_RANK lookup as
the entry point for this validation, and keep the capped-results and
dropped-count logic unchanged for valid inputs.
In `@viz/build_dashboard.py`:
- Line 78: Ruff is flagging the non-ASCII multiplication character used in the
docstring and matching user-facing text. Update the wording in the affected
`build_dashboard.py` strings to use plain ASCII terms instead of `×`, and make
the same replacement anywhere else the same phrase appears so the
`build_dashboard` copy stays lint-clean.
- Line 391: The dashboard rendering in the module row builder is injecting
audit-derived module names directly into innerHTML, which can allow HTML
injection. Update the code in the module-label rendering path (the b.innerHTML
assignment and the related path/rule/category/tool rendering logic referenced by
this audit comment) to escape or sanitize all audit-derived strings before
inserting them into innerHTML, or switch to text-safe DOM APIs for the dynamic
parts. Make sure the same treatment is applied to the other affected render
blocks noted in the comment so every path, rule, category, tool, and module name
is handled consistently.
- Around line 164-198: The history update logic in _update_history currently
deduplicates by date, which causes multiple same-day dashboard builds to
overwrite earlier audit points; change it so each snapshot is appended as a
separate entry instead of filtering out entries with the same snapshot["date"].
Keep the existing load-and-sort flow around HISTORY, but remove the same-date
replacement behavior so every run is preserved in the dated series used by the
trend chart.
---
Nitpick comments:
In `@docs/audit-data-leverage.md`:
- Around line 35-46: The markdown fence blocks in the audit data tables are
plain text, so they should be labeled with text to satisfy markdownlint MD040.
Update the fenced blocks in the documented tables near the referenced sections
by using text-labeled fences consistently, including the blocks containing the
category counts and T1/T2/T3/T4 breakdown, so the doc remains lint-clean.
In `@docs/own-net-auditor.md`:
- Around line 31-39: The markdown examples in the docs are plain prose/diagrams,
not code, so the unlabeled fenced blocks should be tagged as text to satisfy
markdownlint MD040. Update each relevant fence in the documentation snippet to
use the text language hint, including the illustrative blocks around the Own.NET
Auditor summary, the dependency/cycle example, and the memory leak probe
example, so the content remains readable without being treated as code.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 14cf3e6d-71e9-4d83-a34b-dc9df1cf2b81
📒 Files selected for processing (10)
.gitignoredocs/audit-data-leverage.mddocs/own-net-auditor.mdreport/__init__.pyreport/cli.pyreport/sarif.pyreport/tests/__init__.pyreport/tests/test_sarif.pyviz/build_dashboard.pyviz/history.jsonl
… lint) SARIF (report/): - default export caps each run at GitHub's 25,000-results limit so the artifact is uploadable by default; --max-results 0 opts back into the full set, and the drop is reported (Codex P1). - partialFingerprints disambiguate per occurrence (base/N) so identical rule+path+message hits in one file don't collapse to one alert, while the first occurrence keeps the stable line-independent base (Codex P2). - to_sarif validates min_level / max_results_per_run instead of silently disabling the filter or off-by-one dropping (CodeRabbit). - metrics.json/report.md now carry an "export" block (min_level, cap, emitted, dropped) so the three artifacts never disagree about what was exported. Dashboard (viz/): - escape audit-derived strings (path/rule/category/tool/module) before innerHTML in the table and module chip — no HTML injection from a crafted name (CodeRabbit). - history keyed by UTC timestamp; distinct audits (even same-day) are preserved, only a no-op rebuild with identical counts is collapsed (CodeRabbit). - drop the ambiguous U+00D7 in docstring/copy; [*mod_names, ...] over concat (Ruff). Docs: label the plain-text fences as ```text (markdownlint MD040). Tests: 13/13 SARIF (added arg-validation, per-occurrence fingerprint, default cap, CLI export-consistency), green under -O. Dashboard re-verified headless: 8 charts, esc() neutralizes <img onerror>, drill-down intact, no console errors. SARIF default run caps Roslyn 62,338 -> 25,000. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
viz/build_dashboard.py (2)
313-319: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape
%DATA%for the script context too.
esc()protects laterinnerHTMLwrites, but audit-derived strings are already inside the<script>atconst D = %DATA%;. A path/rule/module containing</script>can break out beforeesc()ever runs. Serialize the payload with<,>, and&escaped, e.g.\u003c, where%DATA%is substituted.🛡️ Proposed direction
+def _json_for_script(data: dict) -> str: + return (json.dumps(data) + .replace("&", "\\u0026") + .replace("<", "\\u003c") + .replace(">", "\\u003e"))Use
_json_for_script(data)instead of rawjson.dumps(data)when replacing%DATA%.🤖 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 `@viz/build_dashboard.py` around lines 313 - 319, The dashboard template currently inserts raw `%DATA%` into the `<script>` block, so `esc()` only protects later DOM writes and does not prevent script-breaking payloads. Update the `%DATA%` substitution in `build_dashboard.py` to use a script-safe JSON serializer such as `_json_for_script(data)` instead of plain `json.dumps(data)`, so characters like `<`, `>`, and `&` are escaped before reaching `const D = %DATA%;`.
426-435: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMake the treemap reflect active filters.
update()recomputes the category/tool/rule/source/tier/table views fromfilteredRows(), butdrawTreemap()always renders globalD.modules. After selecting a tool/category chip, the module sizes and hover counts remain unfiltered, which makes the dashboard state misleading.Also applies to: 493-512
🤖 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 `@viz/build_dashboard.py` around lines 426 - 435, The treemap is still using the global D.modules data instead of the currently active filtered dataset, so it does not match the rest of the dashboard. Update drawTreemap() to build labels, values, colors, customdata, and hover text from the same filteredRows()/derived filtered aggregation that update() uses for the other views, so selecting tool/category chips changes the treemap module sizes and counts consistently.
🤖 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 `@report/cli.py`:
- Around line 62-71: Reject negative values for the --max-results argument
during CLI parsing in cli.py so invalid input fails as a normal usage error
instead of reaching to_sarif() and crashing. Update the argument handling around
ap.add_argument and args.max_results to validate that the value is either 0 or a
positive integer before computing cap, and report the error through argparse’s
standard validation path in the same flow that builds sarif via to_sarif().
In `@report/tests/test_sarif.py`:
- Around line 16-20: The test module import root is set too shallow in the
direct-execution setup, so `from report import cli` can resolve against the
wrong package path. Update the `HERE`/`ROOT` bootstrap in `test_sarif.py` so the
inserted path points to the project root that contains the `report` package, and
keep the `report` import unchanged so the test can run correctly under the bare
Python runner.
In `@viz/build_dashboard.py`:
- Around line 198-201: The trend-building logic in the snapshot append block is
deduping runs by aggregate totals and tier counts, which can merge distinct
audit runs. Update the series handling so every audit run is appended in this
path, or replace the equality check with a true run/input identity in the code
that builds the dashboard series, using the snapshot/series logic around the
unchanged check and series.append.
---
Outside diff comments:
In `@viz/build_dashboard.py`:
- Around line 313-319: The dashboard template currently inserts raw `%DATA%`
into the `<script>` block, so `esc()` only protects later DOM writes and does
not prevent script-breaking payloads. Update the `%DATA%` substitution in
`build_dashboard.py` to use a script-safe JSON serializer such as
`_json_for_script(data)` instead of plain `json.dumps(data)`, so characters like
`<`, `>`, and `&` are escaped before reaching `const D = %DATA%;`.
- Around line 426-435: The treemap is still using the global D.modules data
instead of the currently active filtered dataset, so it does not match the rest
of the dashboard. Update drawTreemap() to build labels, values, colors,
customdata, and hover text from the same filteredRows()/derived filtered
aggregation that update() uses for the other views, so selecting tool/category
chips changes the treemap module sizes and counts consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a141cc35-9b84-4ac7-8a8f-31c6789f354c
📒 Files selected for processing (7)
docs/audit-data-leverage.mddocs/own-net-auditor.mdreport/cli.pyreport/sarif.pyreport/tests/test_sarif.pyviz/build_dashboard.pyviz/history.jsonl
✅ Files skipped from review due to trivial changes (2)
- viz/history.jsonl
- docs/own-net-auditor.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/audit-data-leverage.md
- report/sarif.py
…validation - viz: _json_for_script escapes <, >, & to \uXXXX before %DATA% lands in the <script> block, so an audit-derived </script> (etc.) can't break out of the script context. Verified it neutralizes </script> and JSON round-trips (CR). - viz: history dedup now keys on the findings.json content digest, not aggregate counts — distinct audits are always appended (even with coincidentally equal total/tiers), only a byte-identical re-render is collapsed (CR). - report/cli: --max-results is validated by argparse (_non_negative_int), so a negative value is a usage error, not a to_sarif() traceback (CR). Test added. Not changed: the test_sarif.py ROOT bootstrap — ROOT already resolves to the repo root (dirname(dirname(report/tests))), verified by running the suite standalone from /tmp with no PYTHONPATH (14/14). The suggested extra dirname would point above the repo and break the import. Tests 14/14 (normal + -O); dashboard re-verified headless (8 charts, drill-down, no console errors); SARIF default export unchanged (Roslyn 62,338 -> 25,000). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
|
@coderabbitai re: the two outside-diff findings from the last review:
Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
(ᵔᴥᵔ) ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
viz/build_dashboard.py (1)
179-180: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle zero-finding audits before dividing.
If
findings.jsoncontains an emptyfindingsarray,collect()crashes here withZeroDivisionErrorinstead of producing an empty dashboard.Proposed fix
- "fixable_pct": round(100 * sum(c for s, c in by_source.items() - if sources[s]["codefix"]) / len(findings)), + "fixable_pct": (round(100 * sum(c for s, c in by_source.items() + if sources[s]["codefix"]) / len(findings)) + if findings else 0),🤖 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 `@viz/build_dashboard.py` around lines 179 - 180, The collect() summary calculation in build_dashboard.py divides by len(findings) without guarding against an empty findings list, which can raise ZeroDivisionError. Update the fixable_pct computation to handle zero findings safely by returning 0 (or an equivalent empty-dashboard value) when findings is empty, and keep the existing aggregation logic 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.
Inline comments:
In `@viz/build_dashboard.py`:
- Line 125: The trend snapshot identity in build_dashboard.py is still using
SHA-1 via the digest assignment, which triggers the security lint and leaves
collision risk. Update the digest generation in the snapshot identity path to
use SHA-256 instead, keeping the existing truncation behavior if needed, and
ensure the change is applied where the raw snapshot string is hashed for the run
identity.
---
Outside diff comments:
In `@viz/build_dashboard.py`:
- Around line 179-180: The collect() summary calculation in build_dashboard.py
divides by len(findings) without guarding against an empty findings list, which
can raise ZeroDivisionError. Update the fixable_pct computation to handle zero
findings safely by returning 0 (or an equivalent empty-dashboard value) when
findings is empty, and keep the existing aggregation logic otherwise.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: de4533d0-74da-40ed-9c24-6a5b60ad7330
📒 Files selected for processing (4)
report/cli.pyreport/tests/test_sarif.pyviz/build_dashboard.pyviz/history.jsonl
✅ Files skipped from review due to trivial changes (1)
- viz/history.jsonl
🚧 Files skipped from review as they are similar to previous changes (2)
- report/cli.py
- report/tests/test_sarif.py
- viz: trend run identity uses sha256 (was sha1) — clears Ruff S324 and drops the collision edge case; digest stays 16 hex chars (CR). - viz: fixable_pct guards against an empty findings array (returns 0 instead of ZeroDivisionError), so a zero-finding audit produces an empty dashboard (CR). Verified: 14/14 SARIF tests; empty-findings collect() -> fixable_pct 0, no crash; dashboard rebuild clean (1 run, sha256 digest). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLHz4cevQyXHKZnjzCqhoa
|
@coderabbitai re: the outside-diff finding on Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
|
2 participants
Что внутри
Три связанных трека, поверх смерженного #6.
1. Интерактивность дашборда (
viz/build_dashboard.py)Переработка статичных графиков в интерактивный вид — те четыре фичи, что обсуждались:
file:line, правилом, бейджем тира, категорией, тулом.T1 auto / T2 review / T3 unfixable / T4 bespoke; тиры из настоящегоfixarm.tiers.tier_of, чтобы дашборд и fix-arm не разъезжались. Фильтр-зависимая.viz/history.jsonl; график рисует ряд (пока одна точка + подпись).Проверено headless (Chromium): 8 графиков, ноль ошибок; фильтр
own-check→ 380 (все T4), drill-downBroker→ 8 986 находок.2. Стратегические доки (
docs/)docs/audit-data-leverage.md— разбор «72k находок, какую ещё пользу извлечь»: переформулировка FP ≠ true-but-wontfix, почему FP-rate неизмерим без ground truth и трюк «размечать по правилу, а не случайно», каталог метрик, ранжированные побочные продукты. С реальными числами текущего прогона.docs/own-net-auditor.md— дорожная карта «сделать из Own.NET аудитор уровня NDepend + Joern + WPF-инспектор», приземлённая на существующий OwnAudit: таблица «вижн → что уже есть → реальный гэп», три предупреждения (не переписывать семантику Roslyn; граф оправдывать архитектурой; runtime — Windows-only трек), фазовый план.3. SARIF-экспортер (
report/) — Фаза 1 дорожной картыЧистый трансформ
sts_audit/findings.json → SARIF 2.1.0для GitHub code scanning, без .NET, тестируется в CI.error, correctness/arch →warning, style →note);properties; тиры изfixarm.tiers;--min-level/--max-results— экспорт по severity, а не вываливание 72k (полный лог 47MB > gzip-лимита GitHub;--min-level warning→ 6 115 результатов / ~4MB);ownnet-audit.sarif+metrics.json+report.md.Test plan
Генерируемые артефакты (
viz/sts-dashboard.html,report/out/) в.gitignore;viz/history.jsonlкоммитится как трендовая база.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit