Skip to content

feat(render): v0.5 collapsible PR comment + severity-sort + footer (Phase C)#3

Merged
Metbcy merged 1 commit intomainfrom
feat/v0.5-comment-ux
Apr 29, 2026
Merged

feat(render): v0.5 collapsible PR comment + severity-sort + footer (Phase C)#3
Metbcy merged 1 commit intomainfrom
feat/v0.5-comment-ux

Conversation

@Metbcy
Copy link
Copy Markdown
Owner

@Metbcy Metbcy commented Apr 29, 2026

Summary

Phase C of the v0.5 OSS-adoption milestone (~/.claude/plans/bomdrift-v0.5-adoption.md). UX polish on the markdown comment that the action posts:

  • Each per-category section wrapped in <details><summary>Show details ...</summary> so big diffs are skim-friendly. ### Header (count) stays visible above the wrapper.
  • Vulnerability rows now sorted by max-severity DESC across components — Critical / High cluster at the top of the table.
  • New <summary> teasers surface the most-actionable item in each finding section (top severity: CRITICAL (...), top similarity: 0.95 (...)) so reviewers know whether to expand without clicking.
  • Per-section "Why this matters" links reuse the SARIF helpUris from v0.4.2 — same docs URL across both output formats.
  • New --repo-url flag (also reads BOMDRIFT_REPO_URL env var) renders an action-affordance footer with a pre-filled "Report this finding" issue link, the /bomdrift suppress <id> hint (Phase D's mechanism), and a Docs link. Footer omitted entirely when unset so forks don't render dead links.

Test plan

  • cargo test --release — 236 lib tests + 7 real-world + 24 bin/integration pass (8 new tests in markdown.rs)
  • cargo clippy --all-targets --all-features -- -D warnings clean
  • cargo fmt --all --check clean
  • bash examples/run-all.sh — all 4 scenarios pass with regenerated expected-output.md files
  • Visual review of the rendered comment on a real PR (best done after merge — none of the v0.4 callers pass --repo-url, so footer-rendering needs a deliberate wire-up in the action's entrypoint, deferred to Phase D's release polish)

Pinned decisions

  • Collapsible by default, not opt-in. Plan said "skimmable for big diffs" — that's the goal. A pre-merge round-trip on a small diff (say PR chore(deps): add humantime as v0.5 wiring placeholder + dogfood demo #1's humantime add) shows the collapsed form working without any consumer config.
  • Header outside the details block. Both ### Added (5)\n<details> and <details><summary>### Added (5)</summary> were considered. The latter renders ### Added (5) as literal text inside <summary> (GFM doesn't render markdown inside <summary>). Headers stay outside → table-of-contents and visual scan still work.
  • Footer omission when repo_url == None. Forks and the JSON / SARIF outputs already do not render the markdown footer, but a fork that does use markdown doesn't want bomdrift's repo as the "Report it" target. Opt-in URL keeps the surface honest.

🤖 Generated with Claude Code

Metbcy added a commit that referenced this pull request Apr 29, 2026
The workflow's "Checkout PR head" step had this name string:

    - name: Checkout PR head (so `uses: ./` resolves)

The second `:` (in `uses: ./` inside the parenthetical) made the line
invalid YAML — a YAML scanner sees `name: ...` as a key:value pair
and then chokes on the second colon mid-value with
`mapping values are not allowed here`.

GitHub Actions silently skips malformed workflow files (no UI surface
for the parse error in PR Checks), which is why the previous agent's
attempt to verify the v0.5 zero-config flow on this PR didn't fire
the dogfood action even though sbom-diff.yml is in main's path-filter
list. The handoff note tracked it as "needs investigation"; the
investigation was a YAML syntax check.

Fix: drop the parenthetical from the step name and move the
explanatory text into a comment above the `uses:` line. No behavior
change — the comment captures the same intent (the checkout step
exists only so `uses: ./` has something to resolve against; downstream
consumers copying this workflow as `uses: Metbcy/bomdrift@v1` don't
need the checkout step because v0.5's zero-config flow handles
before/after checkouts internally).

Validated locally with `python3 -c "import yaml; yaml.safe_load(...)"`
on the corrected file. After this commit pushes to the PR branch,
expect the SBOM-diff workflow to actually run.

Note for the next session: the entrypoint.sh `--repo-url` wire-up
(the other handoff action item) was deliberately NOT included in
this commit. `--repo-url` is a v0.5 binary flag (added by Phase C /
PR #3) and the action's `download_bomdrift` resolves `@v1` which
still points at v0.4.4. Passing an unrecognized flag would break the
action against the currently-published binary. The wire-up needs to
land AFTER:
  1. Phase C / PR #3 merges (adds --repo-url to the binary)
  2. v0.5.0 is tagged
  3. @v1 is bumped to v0.5.0

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Phase C of the v0.5 OSS-adoption milestone. Polishes the markdown
output for skim-readability without changing what the action posts
on a no-finding diff.

src/render/markdown.rs:
- Each populated per-category section now wraps its body table in a
  `<details><summary>Show details ...</summary>` block. The
  `### Section (count)` markdown header stays outside the details
  wrapper so it remains visible (and TOC-eligible) when the section
  is collapsed; the table is hidden by default for skim-readability
  on big diffs. The blank-line padding inside the block follows
  GitHub-Flavored Markdown's rule for letting markdown tables render
  as markdown rather than as raw HTML.
- New `vuln_components_sorted` orders vulnerable components by their
  highest-severity advisory (DESC), with alphabetical tie-breaking
  on ecosystem+name. Per-component advisories are still severity-
  then-id sorted in `write_one_vuln_row` (unchanged from v0.3).
  Critical / High findings now cluster at the top of the table —
  the load-bearing rows that justify a reviewer's attention.
- New `vuln_teaser` and `typosquat_teaser` populate the `<summary>`
  line with the most-actionable item in the section (e.g.
  `top severity: CRITICAL (CVE-2025-foo)` or `top similarity: 0.95
  (plain-crypto-js → crypto-js)`). The reviewer knows whether to
  expand without clicking.
- Each finding section gains a "Why this matters" link pointing at
  the SARIF helpUri introduced in v0.4.2. Same docs URL across the
  two output formats so the per-rule explanation is discoverable
  from either path.
- New `write_footer` renders an action-affordance footer when
  `Options::repo_url` is set: a pre-filled "Report this finding"
  issue URL, the `/bomdrift suppress <id>` suppress-comment hint
  (Phase D wires the actual mechanism), and a Docs link. Wrapped in
  `<sub>` so it doesn't compete visually with section bodies. When
  `repo_url` is `None`, the footer is omitted entirely — forks /
  standalone CLI use don't render dead links to bomdrift's own
  issue tracker.
- `Options` loses `Copy` (it now carries an `Option<String>`); both
  call sites pass it by value once so the cost is nil.

src/cli.rs:
- New `--repo-url` flag on `bomdrift diff`. clap's `env =`
  attribute requires a feature flag we don't enable, so the env-var
  fallback (`BOMDRIFT_REPO_URL`) is wired manually in `lib.rs::run_diff`.
  Empty strings are treated as unset so shell-script callers can
  pass `BOMDRIFT_REPO_URL=` to clear without juggling `unset`.

examples/*/expected-output.md:
- Regenerated against the new markdown output. Visible diff: each
  populated section gains a `### Header (count)\n<details><summary>
  Show details</summary>\n\n... table ...\n\n</details>` wrapper.
  Existing tests for `### Added`, `### Vulnerabilities`, etc. all
  continue to pass since the headers are kept above the details
  wrapper.

Eight new tests cover: collapsible-block emission, count + teaser
in summary lines, cross-component severity ordering within the
Vulnerabilities table, footer presence + URL synthesis with /
without trailing slash, and the SARIF-helpUri reuse. 236 lib tests
+ 7 real-world tests + 24 binary/integration tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Metbcy Metbcy force-pushed the feat/v0.5-comment-ux branch from 36fdd51 to c13d57b Compare April 29, 2026 05:48
@Metbcy Metbcy merged commit db4fe2d into main Apr 29, 2026
7 checks passed
@Metbcy Metbcy deleted the feat/v0.5-comment-ux branch April 29, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant