feat(output): promote externalization percentage to headline#54
Conversation
The externalization percentage is the unit of progress the v0.1 launch asks every adopter to share back, so it now leads both the text and JSON reports instead of trailing them. Text: emit `Externalization: N% (X externalized / Y enforcement points)` as the first line whenever there is any enforcement point — including the 0% case, which the previous `if enforcement_points > 0` guard silently dropped. The 100%-externalized case (no findings, some enforcement points) now prints the headline and exits cleanly instead of reporting "No authorization patterns found." Removed the redundant trailing externalization line. JSON: add a top-level `headline` object alongside `findings` and `summary` so `jq .headline scan.json` is the one-liner for sharing the number back. Same numbers stay inside `summary` for existing consumers.
Address review feedback on the externalization headline: - Rename `Headline.enforcement_points` → `externalized` so the headline schema is internally consistent: `externalized + embedded_findings = total_enforcement_points`. Pairs symmetrically with `embedded_findings` and frees "enforcement points" to mean its literal sense (the sum). `Summary.enforcement_points` keeps its older, narrower meaning for back-compat. - Extract `output::externalized_pct` so the text and JSON formatters share one rounding rule. - Collapse a stray double space in the text headline. - Add 13 unit tests covering the helper, the text formatter (no-signal, 0%, 100% regression, mixed), and the JSON headline shape (incl. that `summary.enforcement_points`/`externalized_pct` still carry the same counts as the headline).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes add a unified "Externalization" metrics headline to security scan reports. A shared percentage computation helper is extracted to ChangesExternalization Metrics Headline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
The implementation correctly promotes the externalization percentage to the report headline across both text and JSON formats. The refactoring properly extracts the percentage calculation into a shared helper function, ensuring consistent rounding behavior. The bug fix for the 100% externalization case (which previously printed nothing) is now properly handled. All edge cases are covered by comprehensive unit tests including 0%, 100%, and mixed percentage scenarios. The code is ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Summary
Promote the externalization percentage to the top of every scan report, so adopters can see — and share back — the one number that captures progress on moving authorization out of source code.
Externalization: N% (X externalized / Y enforcement points)for every non-empty scan, including the 0% and 100% cases (the latter previously fell throughfindings.is_empty()and printed nothing — now fixed).headlineobject alongside the existingsummary, with self-consistent fields:externalized + embedded_findings = total_enforcement_points.summary.enforcement_pointsandsummary.externalized_pctkeep their existing names for back-compat.Changes
src/output/text.rs— headline leads the report, emitted unconditionally whentotal > 0; 100%-externalized case now reports instead of silently bailing.src/output/json.rs— addedHeadline { externalized_pct, externalized, embedded_findings, total_enforcement_points }as a top-level field; doc comments spell out which fields overlap withsummaryand which are headline-only.src/output/mod.rs— extractedexternalized_pct(externalized, embedded) -> usizehelper so both formatters share one rounding rule.Test plan
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo test(368 passed)cargo run -- scan <repo>shows the headline firstcargo run -- scan <repo> --format json | jq .headlinereturns the four fieldsSummary by CodeRabbit