Skip to content

feat(reviewer-rigor): call-tree inspection + ephemeral-ID lint + BP audit + release YAML fix (4.2.0) - #41

Merged
lklimek merged 8 commits into
mainfrom
feat/reviewer-rigor
Jun 3, 2026
Merged

feat(reviewer-rigor): call-tree inspection + ephemeral-ID lint + BP audit + release YAML fix (4.2.0)#41
lklimek merged 8 commits into
mainfrom
feat/reviewer-rigor

Conversation

@lklimek

@lklimek lklimek commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Bundles four pending reviewer-rigor items into one focused PR.

  1. Call-tree inspection for modified functions — grumpy-review, review-pr, and check-pr-comments now run a deep transitive in-repo caller walk for every function the diff touches. Tool-agnostic: the reviewer probes the environment (which ctags global rg tree-sitter, inspects compile_commands.json / tsconfig.json / language hints) and picks the deepest tool available, gracefully falling back to grep-based caller extraction when no language-aware tool is installed. Reviewer ranks modified functions by risk (public API > private; trait/interface impl > leaf; signature-changed > body-only) and walks the top 10. Per-function caps: depth 5, callers 200, 60s wall-clock. Findings emit under new call_tree category with CALL- ID prefix, with Walked via: <tool> recorded in every finding's description for auditability.

  2. Ephemeral-ID coding convention — new Cross-Cutting Rule in coding-best-practices: source code, comments, and committed docs MUST NOT reference transient review-finding IDs (CMT-001, SEC-014, RUST-123, CALL-005, etc.). Allow-list documented for permanent IDs (ADR-NNN, RFC-NNN, CWE-NNN, CVE-YYYY-NNNN, OWASP-*, GHSA-*, GitHub issue/PR refs, TODO/FIXME, committed test-spec IDs). Enforced two ways: Bilby preloads BP (write-side); grumpy-review/review-pr run new scripts/lint_ephemeral_ids.py then dismiss in-skill example matches (review-side, dumb lint + LLM context filter).

  3. BP-load auditcoding-best-practices now loaded by every reviewer/coder agent. Six agents missing it gained it (architect-nagatha, claudius, qa-engineer-marvin, security-engineer-smythe, technical-writer-trillian, ux-designer-diziet). Two orchestrator skills (grumpy-review, check-pr-comments) carry an explicit BP-preload directive for spawned agents. tests/test_skill_frontmatter.py is the regression guard against PyYAML parse breakage anywhere in skills/agents; tests/test_bp_load_audit.py is parameterised over the curated agent set so future agents fail closed.

  4. Release SKILL YAML fixskills/release/SKILL.md frontmatter description: value quoted to fix the long-standing PyYAML parse error that surfaced on every claude plugin validate .. Validation now exits 0 (modulo one unrelated CLAUDE.md-at-root warning).

Where the changes land

Area Change
schemas/review-report.schema.json Additive: CALL added to finding.id regex; call_tree added to finding_section.category enum and to severity_category_matrix row schema.
scripts/consolidate_reports.py CATEGORY_PREFIX maps call_tree → CALL-.
scripts/generate_review_report.py CATEGORY_LABELS + section_labels extended; HTML/Triage filter chips and JS catLabels cover call_tree. Part numbering downstream of Call-Tree bumped.
scripts/lint_ephemeral_ids.py (NEW) Dumb lint imports CATEGORY_PREFIX from consolidator to prevent drift. --diff mode for unified-diff + lines, `--format json
skills/grumpy-review/references/call-tree-walk.md (NEW, 190 LOC) Authoritative walk methodology: outcome contract, when-to-run trigger, ranking + top-10, tool-probe order, walk caps, per-caller judgement rubric, finding emission shape.
skills/grumpy-review/SKILL.md New "Call-tree inspection" subsection delegating to the reference doc; lint step wired; allowed-tools extended; BP-preload directive for spawned reviewer agents.
skills/review-pr/SKILL.md Inheritance note + lint step; allowed-tools extended.
skills/check-pr-comments/SKILL.md Walk trigger when a resolved comment references a function modified in resolution commits; BP-rules directive for resolution verification; allowed-tools extended.
skills/coding-best-practices/SKILL.md New Cross-Cutting Rule "No ephemeral review IDs in committed artifacts".
skills/report-format/SKILL.md CALL- row in ID Prefix table + call_tree in category enum.
skills/release/SKILL.md description: quoted in frontmatter.
agents/{architect-nagatha,claudius,qa-engineer-marvin,security-engineer-smythe,technical-writer-trillian,ux-designer-diziet}.md coding-best-practices added to skills: frontmatter.
tests/test_lint_ephemeral_ids.py (NEW) 42 tests: positives across all 13 consolidator-owned prefixes, negatives across allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/etc.), word-boundary edge cases, --diff mode, prefix-source-of-truth assertion.
tests/test_skill_frontmatter.py (NEW) Parses every skills/*/SKILL.md and agents/*.md frontmatter via PyYAML; one parametrized test per file.
tests/test_bp_load_audit.py (NEW) Asserts curated agent set loads coding-best-practices.
tests/test_schema_v3_strict.py, tests/test_consolidate_reports.py, tests/test_render_v3.py Extended for call_tree happy path, unknown-prefix rejection, sequential ID assignment, matrix population, filter chip / column / JS-label coverage.
tests/fixtures/reports/v3-call-tree.json (NEW) Minimal valid call_tree fixture.
.claude-plugin/plugin.json 4.1.7 → 4.2.0.
CHANGELOG.md [4.2.0] entry covering all four items.

Design decisions (locked, see in-PR rationale)

  • Lint is dumb; LLM filters context. The script blindly reports every consolidator-prefix match. Reviewer skill instructs the LLM to dismiss matches inside markdown code fences / example blocks in skill files themselves. No inline # noqa markers, no allow-list of files — simplicity is the design.
  • Walk top 10 ranked, not all. Skill ranks modified functions by risk and walks only the top 10. Truncation surfaced as an INFO finding. Bounds turn count predictably on large PRs.
  • No walk script. Methodology lives in the reference doc; the runtime LLM dispatches whichever tool is available. A scripts/walk_call_tree.sh would add a layer of indirection without the corresponding allow-list simplification.
  • Schema is additive; no schema-version bump. All existing v3 fixtures still validate.
  • call_tree is its own category, not folded into code_quality. Lets reviewers filter; has distinct severity floor.

Stream split (for archaeology)

PR built as 3 parallel streams in pre-created worktrees:

  • ae2bdda — Stream A: schema + pipeline plumbing (+7 tests)
  • de76536 — Stream C: BP audit + YAML fix + 4.2.0 + CHANGELOG (+43 tests)
  • 22bdcae — Merge C into integration branch
  • e6975ee — Stream B: walk methodology + ephemeral-ID lint (+42 tests)

Stream B built on top of A+C to avoid late-stage merge conflicts on the SKILL.md files; final integration was a clean fast-forward.

Known issue surfaced (not blocking)

The schema regex ^(SEC|QA|PROJ|CODE|RUST|...)-\d{3}$ accepts QA- IDs but the consolidator's CATEGORY_PREFIX has no qa_* category mapping. Stream B's lint script handled this cleanly by listing QA- under an _EXTRA_PREFIXES set with a comment. Worth a follow-up to either drop QA- from the schema OR add a qa category — separate concern, will file a TODO.

Test plan

  • pytest tests/ — 343/343 (251 baseline + 92 new across A/B/C)
  • claude plugin validate . — exit 0 (release/SKILL.md YAML fix removes the long-standing error; one unrelated CLAUDE.md-at-root warning remains, pre-existing)
  • scripts/validate_report.py tests/fixtures/reports/v3-call-tree.json — Valid
  • scripts/validate_report.py tests/fixtures/reports/v3-{minimal,full}.json — both still Valid (schema additivity confirmed)
  • End-to-end consolidator smoke: producer-shape call_tree report → sequential CALL-001/CALL-002 assigned, OWASP-derived severity 4/3, matrix populated, code_quality column stays zero (categories not conflated)
  • scripts/lint_ephemeral_ids.py --help clean; --diff parses unified-diff + lines correctly; prefix set imported live from consolidate_reports (drift-free)
  • First real-PR dogfood: invoke /claudius:review-pr against a PR with modified functions, observe call-tree findings; iterate on the methodology if the walk produces noise or misses

🤖 Co-authored by Claudius the Magnificent AI Agent

claude added 4 commits May 28, 2026 12:08
Stream C of reviewer-rigor PR. Bundles three housekeeping items:
- skills/release/SKILL.md: quote frontmatter description to fix PyYAML parse error (claude plugin validate now clean)
- coding-best-practices added to skills frontmatter on 6 agents missing it (architect, claudius, qa, security, writer, ux)
- orchestrator skills (grumpy-review, check-pr-comments) note BP preload requirement for spawned agents
- tests/test_skill_frontmatter.py: regression guard parses every frontmatter via PyYAML
- tests/test_bp_load_audit.py: asserts curated agent set loads BP
- .claude-plugin/plugin.json: 4.1.7 -> 4.2.0
- CHANGELOG.md: [4.2.0] entry covering the full PR (Streams A + B + C)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…all-tree inspection

Pipeline plumbing for the upcoming reviewer call-tree walk (Stream A of reviewer-rigor PR).
- schemas/review-report.schema.json: CALL added to id pattern; call_tree added to category enum and matrix
- scripts/consolidate_reports.py: CATEGORY_PREFIX maps call_tree -> CALL-
- scripts/generate_review_report.py: CATEGORY_LABELS includes call_tree
- skills/report-format/SKILL.md: ID prefix table + category list extended
- tests: schema happy/reject paths, consolidator id assignment + matrix population, renderer column/chip coverage
- fixture: tests/fixtures/reports/v3-call-tree.json

No skill-body changes (Stream B), no agent BP edits (Stream C), no version bump (Stream C does final bump after all streams land).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… lint

Stream B of reviewer-rigor PR. Two reviewer-rigor threads:

Call-tree inspection:
- skills/grumpy-review/references/call-tree-walk.md: NEW authoritative methodology — outcome contract, when-to-run trigger, rank-and-walk-top-10 algorithm, tool-probe order, walk caps (depth 5, callers 200, 60s/function), per-caller judgement rubric, finding emission shape with mandatory `Walked via: <tool>` line.
- skills/grumpy-review/SKILL.md: new "Call-tree inspection" subsection delegating to the reference doc.
- skills/review-pr/SKILL.md: inheritance note (delegated via grumpy-review).
- skills/check-pr-comments/SKILL.md: trigger walk when a resolved comment references a function modified in resolution commits.

Ephemeral-ID enforcement:
- skills/coding-best-practices/SKILL.md: new Cross-Cutting Rule banning transient review-finding IDs in committed artifacts; documented allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/GH refs/TODO/test-spec IDs); rule-of-thumb (regenerated JSON IDs = forbidden, committed-doc IDs = fine).
- scripts/lint_ephemeral_ids.py: NEW deterministic lint. Imports CATEGORY_PREFIX from consolidate_reports to prevent drift. JSON or text output, --diff mode for unified-diff + lines, always exit 0.
- skills/grumpy-review/SKILL.md + skills/review-pr/SKILL.md: wire the lint step, instruct LLM to dismiss in-skill example matches (no markers per design decision).
- allowed-tools updates for the walk + lint binaries on the three reviewer skills.

Tests:
- tests/test_lint_ephemeral_ids.py: positives across all 13 consolidator prefixes, negatives across allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/etc.), word-boundary edge cases, --diff mode parsing, prefix-source-of-truth assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds reviewer rigor around call-tree inspection and ephemeral review-ID linting, extends report plumbing for a new call_tree category, audits agent BP loading, and bumps the plugin to 4.2.0.

Changes:

  • Adds CALL- / call_tree schema, consolidation, rendering, fixtures, and tests.
  • Introduces scripts/lint_ephemeral_ids.py plus review-skill guidance for detecting transient review IDs in committed artifacts.
  • Updates skills/agents to preload coding best practices, fixes release skill YAML frontmatter, and records the 4.2.0 release.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.claude-plugin/plugin.json Bumps plugin version to 4.2.0.
CHANGELOG.md Adds 4.2.0 release notes.
agents/architect-nagatha.md Adds coding-best-practices.
agents/claudius.md Adds coding-best-practices.
agents/qa-engineer-marvin.md Adds coding-best-practices.
agents/security-engineer-smythe.md Adds coding-best-practices.
agents/technical-writer-trillian.md Adds coding-best-practices.
agents/ux-designer-diziet.md Adds coding-best-practices.
schemas/review-report.schema.json Adds CALL- and call_tree schema support.
scripts/consolidate_reports.py Maps call_tree to CALL-.
scripts/generate_review_report.py Renders call_tree in Markdown, HTML, triage, and labels.
scripts/lint_ephemeral_ids.py Adds advisory lint for transient review-finding IDs.
skills/check-pr-comments/SKILL.md Adds BP and call-tree verification guidance.
skills/coding-best-practices/SKILL.md Adds ephemeral review-ID convention.
skills/grumpy-review/SKILL.md Adds BP preload, call-tree, and lint workflow guidance.
skills/grumpy-review/references/call-tree-walk.md Adds call-tree walk methodology.
skills/release/SKILL.md Quotes YAML description.
skills/report-format/SKILL.md Documents call_tree and CALL-.
skills/review-pr/SKILL.md Inherits call-tree and lint steps from grumpy review.
tests/fixtures/reports/v3-call-tree.json Adds call-tree report fixture.
tests/test_bp_load_audit.py Adds BP preload regression guard.
tests/test_consolidate_reports.py Tests CALL- assignment and matrix counts.
tests/test_lint_ephemeral_ids.py Tests ephemeral-ID lint behavior.
tests/test_render_v3.py Tests call_tree renderer coverage.
tests/test_schema_v3_strict.py Tests schema support for CALL- / call_tree.
tests/test_skill_frontmatter.py Adds PyYAML frontmatter parsing guard.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/lint_ephemeral_ids.py
Comment thread skills/grumpy-review/SKILL.md
Comment thread skills/grumpy-review/references/call-tree-walk.md Outdated
- lint_ephemeral_ids: skip '\ No newline at end of file' diff marker
  so it no longer advances new_line (off-by-one on added matches after
  a no-newline final line); add regression test
- grumpy-review SKILL §3: add call_tree category + CALL- prefix to the
  generic finding-format block for consistency with the call-tree section
- call-tree-walk: fix ranking example to emit producer risk/impact/scope
  floats instead of coordinator-derived severity (producer-contract compliance)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lklimek
lklimek marked this pull request as ready for review May 29, 2026 09:43
@lklimek
lklimek requested a review from Copilot May 29, 2026 09:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.

Comment thread skills/review-pr/SKILL.md Outdated
Comment thread skills/grumpy-review/SKILL.md Outdated
Comment thread skills/check-pr-comments/SKILL.md Outdated
Comment thread tests/test_skill_frontmatter.py
Comment thread tests/test_skill_frontmatter.py
Comment thread skills/review-pr/SKILL.md Outdated
Six defects in PR #41's own new code/config:

- review-pr: ephemeral-ID lint used $BASE; switch to $BASE_BRANCH so the
  diff scans the PR range (matches grumpy-review's convention).
- call-tree-walk: same $BASE -> $BASE_BRANCH fix in the trigger condition;
  swap test -f/ps -e tool detection for which-based probes (the only
  detection primitive on every skill's allow-list).
- grumpy-review/review-pr/check-pr-comments: add Bash(gtags *) and
  Bash(tree-sitter *) to allowed-tools so the call-tree walk's executed
  tools aren't sandbox-blocked; all three lists now consistent with the
  methodology and the rg/which fallback path.
- CI: run the full pytest suite (tests/ -q) instead of one file, and
  widen push/pull_request paths to skills/agents/tests/schemas/scripts
  plus the workflow itself so the new guards actually fire.
- scripts/requirements.txt: declare PyYAML (>= to match existing pins);
  CI installs this file, so the frontmatter guard can import yaml.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Comment thread CHANGELOG.md Outdated
Comment thread skills/grumpy-review/references/call-tree-walk.md Outdated
Two doc defects in PR #41's own new content, plus a self-consistency scrub:

- call-tree-walk: remove the LSP "Best" walk tier (rust-analyzer/gopls/
  pyright/tsserver). It was the only tier with no allow-listed invocation
  -- no skill frontmatter permits driving an LSP client and the sandbox
  can't from a single Bash call, so the documented "best path" was
  unreachable and could lead a reviewer to record `Walked via: LSP` they
  couldn't perform. ctags/gtags remain the two "Best" tiers; the which
  probe drops the LSP binaries; the line-67 reachability hand-wave is gone.
- CHANGELOG 4.2.0: the ephemeral-ID examples were committed in concrete
  form (CMT-001/SEC-014/RUST-123/CALL-005), which the new lint this PR
  ships would flag in a committed doc; switch to NNN placeholder form,
  matching the permanent-ID examples on the same line. Drop "any installed
  LSP" from the call-tree feature summary to track the methodology change.
- CHANGELOG hygiene: strip four pre-existing transient finding-ID backrefs
  ((CQ-001)/(DOC-001)/(QA-004)/(QA-005)) from historical 3.14.x entries --
  the dead-reference anti-pattern the new convention bans. lint_ephemeral_ids
  now reports CHANGELOG.md clean ([]). Full suite: 344 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Comment thread skills/coding-best-practices/SKILL.md
Comment thread schemas/review-report.schema.json Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lklimek
lklimek merged commit 7fcc279 into main Jun 3, 2026
2 checks passed
@lklimek
lklimek deleted the feat/reviewer-rigor branch June 3, 2026 10:53
lklimek pushed a commit that referenced this pull request Jun 3, 2026
Bring the squash-merged PR #41 (commit 7fcc279, final reviewer-rigor incl.
2nd+3rd Copilot passes) into the severity-pipeline branch.

#42 was cut from #41 at b76bf72 (first pass only), so its branch carried a
stale copy of #41. Resolution rule: pure-#41 content -> take main (final,
thrice-reviewed); #42 severity-pipeline additions -> keep ours; mixed files
-> combine. 8 conflicts resolved:

- .claude-plugin/plugin.json: ours (4.3.0)
- CHANGELOG.md: combined (ours 4.3.0 above main's final 4.2.0; NNN
  placeholders, no LSP, lint-clean)
- schemas/review-report.schema.json: ours 3.1.0 superset + main's CALL-/
  call_tree id-prefix description
- skills/check-pr-comments,grumpy-review,review-pr SKILL.md: combined
  (main's gtags/tree-sitter allowed-tools + $BASE_BRANCH; ours @{u} permalink)
- skills/grumpy-review/references/call-tree-walk.md: main (LSP tier removed)
- tests/test_schema_v3_strict.py: ours (union, #42 TestV31AdditiveFields)

consolidate_reports.py + generate_review_report.py merged byte-identical to
ours (call_tree rendering and on-the-fly severity never overlapped). Also
normalizes black formatting on 6 pre-existing not-black-clean test files
(pure style, no semantic change). 379 tests pass; plugin validate clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lklimek added a commit that referenced this pull request Jun 3, 2026
…0) (#42)

* chore(reviewer-rigor): BP audit + release SKILL YAML fix + version 4.2.0

Stream C of reviewer-rigor PR. Bundles three housekeeping items:
- skills/release/SKILL.md: quote frontmatter description to fix PyYAML parse error (claude plugin validate now clean)
- coding-best-practices added to skills frontmatter on 6 agents missing it (architect, claudius, qa, security, writer, ux)
- orchestrator skills (grumpy-review, check-pr-comments) note BP preload requirement for spawned agents
- tests/test_skill_frontmatter.py: regression guard parses every frontmatter via PyYAML
- tests/test_bp_load_audit.py: asserts curated agent set loads BP
- .claude-plugin/plugin.json: 4.1.7 -> 4.2.0
- CHANGELOG.md: [4.2.0] entry covering the full PR (Streams A + B + C)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(schema): add call_tree category + CALL- id prefix for reviewer call-tree inspection

Pipeline plumbing for the upcoming reviewer call-tree walk (Stream A of reviewer-rigor PR).
- schemas/review-report.schema.json: CALL added to id pattern; call_tree added to category enum and matrix
- scripts/consolidate_reports.py: CATEGORY_PREFIX maps call_tree -> CALL-
- scripts/generate_review_report.py: CATEGORY_LABELS includes call_tree
- skills/report-format/SKILL.md: ID prefix table + category list extended
- tests: schema happy/reject paths, consolidator id assignment + matrix population, renderer column/chip coverage
- fixture: tests/fixtures/reports/v3-call-tree.json

No skill-body changes (Stream B), no agent BP edits (Stream C), no version bump (Stream C does final bump after all streams land).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reviewer-rigor): call-tree inspection methodology + ephemeral-ID lint

Stream B of reviewer-rigor PR. Two reviewer-rigor threads:

Call-tree inspection:
- skills/grumpy-review/references/call-tree-walk.md: NEW authoritative methodology — outcome contract, when-to-run trigger, rank-and-walk-top-10 algorithm, tool-probe order, walk caps (depth 5, callers 200, 60s/function), per-caller judgement rubric, finding emission shape with mandatory `Walked via: <tool>` line.
- skills/grumpy-review/SKILL.md: new "Call-tree inspection" subsection delegating to the reference doc.
- skills/review-pr/SKILL.md: inheritance note (delegated via grumpy-review).
- skills/check-pr-comments/SKILL.md: trigger walk when a resolved comment references a function modified in resolution commits.

Ephemeral-ID enforcement:
- skills/coding-best-practices/SKILL.md: new Cross-Cutting Rule banning transient review-finding IDs in committed artifacts; documented allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/GH refs/TODO/test-spec IDs); rule-of-thumb (regenerated JSON IDs = forbidden, committed-doc IDs = fine).
- scripts/lint_ephemeral_ids.py: NEW deterministic lint. Imports CATEGORY_PREFIX from consolidate_reports to prevent drift. JSON or text output, --diff mode for unified-diff + lines, always exit 0.
- skills/grumpy-review/SKILL.md + skills/review-pr/SKILL.md: wire the lint step, instruct LLM to dismiss in-skill example matches (no markers per design decision).
- allowed-tools updates for the walk + lint binaries on the three reviewer skills.

Tests:
- tests/test_lint_ephemeral_ids.py: positives across all 13 consolidator prefixes, negatives across allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/etc.), word-boundary edge cases, --diff mode parsing, prefix-source-of-truth assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reviewer-rigor): address Copilot review on PR #41

- lint_ephemeral_ids: skip '\ No newline at end of file' diff marker
  so it no longer advances new_line (off-by-one on added matches after
  a no-newline final line); add regression test
- grumpy-review SKILL §3: add call_tree category + CALL- prefix to the
  generic finding-format block for consistency with the call-tree section
- call-tree-walk: fix ranking example to emit producer risk/impact/scope
  floats instead of coordinator-derived severity (producer-contract compliance)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): derive severity on-the-fly + schema 3.1.0 additive bits

Producers (esp. check-pr-comments) emit per-finding risk/impact/scope floats
but no integer severity and never route through the coordinator, so findings
rendered INFO and severity_counts/severity_category_matrix stayed zero. Fix it
where it bites — at render time — and tidy up the schema while we're in here.

Fix 1 (HIGH): new scripts/severity_util.py is the single source of truth for
the OWASP band table, per-finding severity derivation, and the stats/matrix
builder. consolidate_reports imports from it and keeps the legacy private
names (_derive_overall/_derive_severity_int) as re-export aliases. The renderer
gains a normalization pass (markdown/html/triage/pdf): it derives each finding's
integer severity from the floats when absent and recomputes severity_counts +
severity_category_matrix when the supplied counts are missing or all-zero —
never clobbering non-zero supplied counts. triage inherits the fix via
_build_html_context.

Fix 2 (MED): schema accepts optional finding author_type (bot|human) so
check-pr-comments output validates.

Fix 3 (MED): permalink commit now derives from `git rev-parse @{u}` with a
HEAD fallback (check-pr-comments, report-format, grumpy-review) so links resolve
on GitHub instead of 404-ing on an unpushed HEAD.

Fix 4: schema 3.1.0 additive bits — schema_version accepts 3.0.0 and 3.1.0,
metadata.report_type adds pr_audit, finding_section gains optional verdict
(PASS|FAIL|NEEDS_REVIEW). severity stays optional on findings.

Schema -> 3.1.0, plugin -> 4.3.0. New tests: test_severity_util.py, schema
additive cases, render regression (floats-only report renders HIGH + non-zero
counts), re-export parity, and an end-to-end producer->validate->render check
in the shell pipeline. 357 passed; black/ruff clean; plugin validates.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): renderer on-the-fly severity + permalink @{u} + tests

Completes PR-A. The earlier commit shipped schema 3.1.0, severity_util, and the
consolidate_reports re-export; this lands the renderer-side fix plus the
remaining producer/test/doc changes.

- generate_review_report.py: add _normalize_report() — derive each finding's
  integer severity from risk/impact/scope when absent, and recompute
  severity_counts + severity_category_matrix only when supplied counts are
  missing/all-zero (never clobbering non-zero ones). Invoked from
  render_markdown, _build_html_context (html + triage), and render_pdf.
  triage_server.py inherits via the triage format, no change needed.
- check-pr-comments / report-format / grumpy-review: permalink commit now uses
  git rev-parse @{u} with a git rev-parse HEAD fallback so links resolve on
  GitHub; check-pr-comments schema_version bumped to 3.1.0 (3.0.0 still accepted).
- consolidate_reports.py: drop unused build_severity_stats import.
- tests: test_severity_util.py (band mapping via derive_severity_int for exact
  boundaries — the float mean rounds at edges), test_render_v3.py regression
  (floats-only report renders HIGH + non-zero counts in md/html, supplied counts
  preserved), test_report_pipeline.sh end-to-end producer->validate->render,
  test_consolidate_reports asserts against cr.SCHEMA_VERSION not a frozen literal.
- CHANGELOG: 4.3.0 section.

384 tests pass; black/ruff clean; plugin validates; shell pipeline 26/0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(consolidate): drop unused build_severity_stats import (ruff F401)

The coordinator has its own compute_statistics; only the renderer uses the
shared build_severity_stats. Removing the dead import clears the last ruff F401.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): address Copilot review on PR #42

Three valid catches in the severity-pipeline code:

- generate_review_report: _normalize_summary_statistics rebuilt
  severity_counts + severity_category_matrix but left total_findings
  stale, so the HTML KPI could disagree with the rebuilt counts. Realign
  total_findings from the rebuilt stats (same all-zero/missing branch, so
  a hand-supplied non-zero stats block is still never touched).
- generate_review_report: corrected the _normalize_summary_statistics
  docstring — the real trigger is "counts missing/all-zero AND >=1 finding
  exists" (floatless findings count as INFO), not "a finding carries a
  derivable severity".
- consolidate_reports: ACCEPTED_SCHEMA_VERSIONS now derives from the
  schema's schema_version enum (single read shared with SCHEMA_VERSION)
  instead of a hard-coded set, so it can't drift on the next schema bump.
  _read_schema_version() preserved as a thin newest-entry wrapper.

Regression guards: anti-drift assert (ACCEPTED_SCHEMA_VERSIONS == set of
schema enum) and a total_findings-realignment test that fails without the
fix. 382 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): address second Copilot pass on PR #42

Three follow-on findings on the on-the-fly severity normalization:

- generate_review_report: _normalize_finding_severities now computes and
  stores overall_severity (mean of risk/impact/scope) when missing/invalid,
  then derives the integer band from that value — instead of filling only
  the integer severity. Producer-shape reports now populate the Markdown
  (overall=...) suffix and the HTML data-overall "Sort: overall" key. A
  finding with a valid int severity but no overall_severity still gets
  overall_severity filled; both already-valid values are left untouched.
- generate_review_report: import derive_overall/derive_severity_int from
  severity_util (derive_finding_severity stays in severity_util, still used
  by _effective_severity; the renderer no longer imports it).
- consolidate_reports: remove the now-dead _read_schema_version() wrapper —
  SCHEMA_VERSION reads _SCHEMA_VERSIONS[-1] directly since ACCEPTED_SCHEMA_
  VERSIONS became schema-enum-derived.

Two regression tests pin the overall_severity fill (floats-only finding +
int-severity-without-overall). 384 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claudius Agent <noreply@anthropic.com>
lklimek added a commit that referenced this pull request Jun 3, 2026
#43)

* chore(reviewer-rigor): BP audit + release SKILL YAML fix + version 4.2.0

Stream C of reviewer-rigor PR. Bundles three housekeeping items:
- skills/release/SKILL.md: quote frontmatter description to fix PyYAML parse error (claude plugin validate now clean)
- coding-best-practices added to skills frontmatter on 6 agents missing it (architect, claudius, qa, security, writer, ux)
- orchestrator skills (grumpy-review, check-pr-comments) note BP preload requirement for spawned agents
- tests/test_skill_frontmatter.py: regression guard parses every frontmatter via PyYAML
- tests/test_bp_load_audit.py: asserts curated agent set loads BP
- .claude-plugin/plugin.json: 4.1.7 -> 4.2.0
- CHANGELOG.md: [4.2.0] entry covering the full PR (Streams A + B + C)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(schema): add call_tree category + CALL- id prefix for reviewer call-tree inspection

Pipeline plumbing for the upcoming reviewer call-tree walk (Stream A of reviewer-rigor PR).
- schemas/review-report.schema.json: CALL added to id pattern; call_tree added to category enum and matrix
- scripts/consolidate_reports.py: CATEGORY_PREFIX maps call_tree -> CALL-
- scripts/generate_review_report.py: CATEGORY_LABELS includes call_tree
- skills/report-format/SKILL.md: ID prefix table + category list extended
- tests: schema happy/reject paths, consolidator id assignment + matrix population, renderer column/chip coverage
- fixture: tests/fixtures/reports/v3-call-tree.json

No skill-body changes (Stream B), no agent BP edits (Stream C), no version bump (Stream C does final bump after all streams land).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reviewer-rigor): call-tree inspection methodology + ephemeral-ID lint

Stream B of reviewer-rigor PR. Two reviewer-rigor threads:

Call-tree inspection:
- skills/grumpy-review/references/call-tree-walk.md: NEW authoritative methodology — outcome contract, when-to-run trigger, rank-and-walk-top-10 algorithm, tool-probe order, walk caps (depth 5, callers 200, 60s/function), per-caller judgement rubric, finding emission shape with mandatory `Walked via: <tool>` line.
- skills/grumpy-review/SKILL.md: new "Call-tree inspection" subsection delegating to the reference doc.
- skills/review-pr/SKILL.md: inheritance note (delegated via grumpy-review).
- skills/check-pr-comments/SKILL.md: trigger walk when a resolved comment references a function modified in resolution commits.

Ephemeral-ID enforcement:
- skills/coding-best-practices/SKILL.md: new Cross-Cutting Rule banning transient review-finding IDs in committed artifacts; documented allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/GH refs/TODO/test-spec IDs); rule-of-thumb (regenerated JSON IDs = forbidden, committed-doc IDs = fine).
- scripts/lint_ephemeral_ids.py: NEW deterministic lint. Imports CATEGORY_PREFIX from consolidate_reports to prevent drift. JSON or text output, --diff mode for unified-diff + lines, always exit 0.
- skills/grumpy-review/SKILL.md + skills/review-pr/SKILL.md: wire the lint step, instruct LLM to dismiss in-skill example matches (no markers per design decision).
- allowed-tools updates for the walk + lint binaries on the three reviewer skills.

Tests:
- tests/test_lint_ephemeral_ids.py: positives across all 13 consolidator prefixes, negatives across allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/etc.), word-boundary edge cases, --diff mode parsing, prefix-source-of-truth assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reviewer-rigor): address Copilot review on PR #41

- lint_ephemeral_ids: skip '\ No newline at end of file' diff marker
  so it no longer advances new_line (off-by-one on added matches after
  a no-newline final line); add regression test
- grumpy-review SKILL §3: add call_tree category + CALL- prefix to the
  generic finding-format block for consistency with the call-tree section
- call-tree-walk: fix ranking example to emit producer risk/impact/scope
  floats instead of coordinator-derived severity (producer-contract compliance)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(git-and-github): PR bodies lead with "Why this PR exists" rationale

PR descriptions now open with a "Why this PR exists" section (problem,
reproduction/threat scenario, blocking relationship) ahead of What/Testing/
Breaking/Checklist/Attribution. The skeleton lives in one place —
git-and-github §Creating a PR — and push delegates to it (no duplication).
Pinned by tests/test_pr_body_template.py. Bumps plugin to 4.4.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): derive severity on-the-fly + schema 3.1.0 additive bits

Producers (esp. check-pr-comments) emit per-finding risk/impact/scope floats
but no integer severity and never route through the coordinator, so findings
rendered INFO and severity_counts/severity_category_matrix stayed zero. Fix it
where it bites — at render time — and tidy up the schema while we're in here.

Fix 1 (HIGH): new scripts/severity_util.py is the single source of truth for
the OWASP band table, per-finding severity derivation, and the stats/matrix
builder. consolidate_reports imports from it and keeps the legacy private
names (_derive_overall/_derive_severity_int) as re-export aliases. The renderer
gains a normalization pass (markdown/html/triage/pdf): it derives each finding's
integer severity from the floats when absent and recomputes severity_counts +
severity_category_matrix when the supplied counts are missing or all-zero —
never clobbering non-zero supplied counts. triage inherits the fix via
_build_html_context.

Fix 2 (MED): schema accepts optional finding author_type (bot|human) so
check-pr-comments output validates.

Fix 3 (MED): permalink commit now derives from `git rev-parse @{u}` with a
HEAD fallback (check-pr-comments, report-format, grumpy-review) so links resolve
on GitHub instead of 404-ing on an unpushed HEAD.

Fix 4: schema 3.1.0 additive bits — schema_version accepts 3.0.0 and 3.1.0,
metadata.report_type adds pr_audit, finding_section gains optional verdict
(PASS|FAIL|NEEDS_REVIEW). severity stays optional on findings.

Schema -> 3.1.0, plugin -> 4.3.0. New tests: test_severity_util.py, schema
additive cases, render regression (floats-only report renders HIGH + non-zero
counts), re-export parity, and an end-to-end producer->validate->render check
in the shell pipeline. 357 passed; black/ruff clean; plugin validates.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): renderer on-the-fly severity + permalink @{u} + tests

Completes PR-A. The earlier commit shipped schema 3.1.0, severity_util, and the
consolidate_reports re-export; this lands the renderer-side fix plus the
remaining producer/test/doc changes.

- generate_review_report.py: add _normalize_report() — derive each finding's
  integer severity from risk/impact/scope when absent, and recompute
  severity_counts + severity_category_matrix only when supplied counts are
  missing/all-zero (never clobbering non-zero ones). Invoked from
  render_markdown, _build_html_context (html + triage), and render_pdf.
  triage_server.py inherits via the triage format, no change needed.
- check-pr-comments / report-format / grumpy-review: permalink commit now uses
  git rev-parse @{u} with a git rev-parse HEAD fallback so links resolve on
  GitHub; check-pr-comments schema_version bumped to 3.1.0 (3.0.0 still accepted).
- consolidate_reports.py: drop unused build_severity_stats import.
- tests: test_severity_util.py (band mapping via derive_severity_int for exact
  boundaries — the float mean rounds at edges), test_render_v3.py regression
  (floats-only report renders HIGH + non-zero counts in md/html, supplied counts
  preserved), test_report_pipeline.sh end-to-end producer->validate->render,
  test_consolidate_reports asserts against cr.SCHEMA_VERSION not a frozen literal.
- CHANGELOG: 4.3.0 section.

384 tests pass; black/ruff clean; plugin validates; shell pipeline 26/0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(consolidate): drop unused build_severity_stats import (ruff F401)

The coordinator has its own compute_statistics; only the renderer uses the
shared build_severity_stats. Removing the dead import clears the last ruff F401.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): address Copilot review on PR #42

Three valid catches in the severity-pipeline code:

- generate_review_report: _normalize_summary_statistics rebuilt
  severity_counts + severity_category_matrix but left total_findings
  stale, so the HTML KPI could disagree with the rebuilt counts. Realign
  total_findings from the rebuilt stats (same all-zero/missing branch, so
  a hand-supplied non-zero stats block is still never touched).
- generate_review_report: corrected the _normalize_summary_statistics
  docstring — the real trigger is "counts missing/all-zero AND >=1 finding
  exists" (floatless findings count as INFO), not "a finding carries a
  derivable severity".
- consolidate_reports: ACCEPTED_SCHEMA_VERSIONS now derives from the
  schema's schema_version enum (single read shared with SCHEMA_VERSION)
  instead of a hard-coded set, so it can't drift on the next schema bump.
  _read_schema_version() preserved as a thin newest-entry wrapper.

Regression guards: anti-drift assert (ACCEPTED_SCHEMA_VERSIONS == set of
schema enum) and a total_findings-realignment test that fails without the
fix. 382 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): address second Copilot pass on PR #42

Three follow-on findings on the on-the-fly severity normalization:

- generate_review_report: _normalize_finding_severities now computes and
  stores overall_severity (mean of risk/impact/scope) when missing/invalid,
  then derives the integer band from that value — instead of filling only
  the integer severity. Producer-shape reports now populate the Markdown
  (overall=...) suffix and the HTML data-overall "Sort: overall" key. A
  finding with a valid int severity but no overall_severity still gets
  overall_severity filled; both already-valid values are left untouched.
- generate_review_report: import derive_overall/derive_severity_int from
  severity_util (derive_finding_severity stays in severity_util, still used
  by _effective_severity; the renderer no longer imports it).
- consolidate_reports: remove the now-dead _read_schema_version() wrapper —
  SCHEMA_VERSION reads _SCHEMA_VERSIONS[-1] directly since ACCEPTED_SCHEMA_
  VERSIONS became schema-enum-derived.

Two regression tests pin the overall_severity fill (floats-only finding +
int-severity-without-overall). 384 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(test): black-format test_pr_body_template.py

PR-C's own test file (from f10308d) failed black --check in this
environment — assert line-wrapping only, no logic change. Surfaced
during the reviewer-rigor/severity-pipeline merges; brings the
git diff main...HEAD set fully clean under black.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claudius Agent <noreply@anthropic.com>
lklimek added a commit that referenced this pull request Jun 3, 2026
…s (4.5.0) (#44)

* chore(reviewer-rigor): BP audit + release SKILL YAML fix + version 4.2.0

Stream C of reviewer-rigor PR. Bundles three housekeeping items:
- skills/release/SKILL.md: quote frontmatter description to fix PyYAML parse error (claude plugin validate now clean)
- coding-best-practices added to skills frontmatter on 6 agents missing it (architect, claudius, qa, security, writer, ux)
- orchestrator skills (grumpy-review, check-pr-comments) note BP preload requirement for spawned agents
- tests/test_skill_frontmatter.py: regression guard parses every frontmatter via PyYAML
- tests/test_bp_load_audit.py: asserts curated agent set loads BP
- .claude-plugin/plugin.json: 4.1.7 -> 4.2.0
- CHANGELOG.md: [4.2.0] entry covering the full PR (Streams A + B + C)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(schema): add call_tree category + CALL- id prefix for reviewer call-tree inspection

Pipeline plumbing for the upcoming reviewer call-tree walk (Stream A of reviewer-rigor PR).
- schemas/review-report.schema.json: CALL added to id pattern; call_tree added to category enum and matrix
- scripts/consolidate_reports.py: CATEGORY_PREFIX maps call_tree -> CALL-
- scripts/generate_review_report.py: CATEGORY_LABELS includes call_tree
- skills/report-format/SKILL.md: ID prefix table + category list extended
- tests: schema happy/reject paths, consolidator id assignment + matrix population, renderer column/chip coverage
- fixture: tests/fixtures/reports/v3-call-tree.json

No skill-body changes (Stream B), no agent BP edits (Stream C), no version bump (Stream C does final bump after all streams land).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(reviewer-rigor): call-tree inspection methodology + ephemeral-ID lint

Stream B of reviewer-rigor PR. Two reviewer-rigor threads:

Call-tree inspection:
- skills/grumpy-review/references/call-tree-walk.md: NEW authoritative methodology — outcome contract, when-to-run trigger, rank-and-walk-top-10 algorithm, tool-probe order, walk caps (depth 5, callers 200, 60s/function), per-caller judgement rubric, finding emission shape with mandatory `Walked via: <tool>` line.
- skills/grumpy-review/SKILL.md: new "Call-tree inspection" subsection delegating to the reference doc.
- skills/review-pr/SKILL.md: inheritance note (delegated via grumpy-review).
- skills/check-pr-comments/SKILL.md: trigger walk when a resolved comment references a function modified in resolution commits.

Ephemeral-ID enforcement:
- skills/coding-best-practices/SKILL.md: new Cross-Cutting Rule banning transient review-finding IDs in committed artifacts; documented allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/GH refs/TODO/test-spec IDs); rule-of-thumb (regenerated JSON IDs = forbidden, committed-doc IDs = fine).
- scripts/lint_ephemeral_ids.py: NEW deterministic lint. Imports CATEGORY_PREFIX from consolidate_reports to prevent drift. JSON or text output, --diff mode for unified-diff + lines, always exit 0.
- skills/grumpy-review/SKILL.md + skills/review-pr/SKILL.md: wire the lint step, instruct LLM to dismiss in-skill example matches (no markers per design decision).
- allowed-tools updates for the walk + lint binaries on the three reviewer skills.

Tests:
- tests/test_lint_ephemeral_ids.py: positives across all 13 consolidator prefixes, negatives across allow-list (ADR/RFC/CWE/CVE/OWASP/GHSA/etc.), word-boundary edge cases, --diff mode parsing, prefix-source-of-truth assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reviewer-rigor): address Copilot review on PR #41

- lint_ephemeral_ids: skip '\ No newline at end of file' diff marker
  so it no longer advances new_line (off-by-one on added matches after
  a no-newline final line); add regression test
- grumpy-review SKILL §3: add call_tree category + CALL- prefix to the
  generic finding-format block for consistency with the call-tree section
- call-tree-walk: fix ranking example to emit producer risk/impact/scope
  floats instead of coordinator-derived severity (producer-contract compliance)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(git-and-github): PR bodies lead with "Why this PR exists" rationale

PR descriptions now open with a "Why this PR exists" section (problem,
reproduction/threat scenario, blocking relationship) ahead of What/Testing/
Breaking/Checklist/Attribution. The skeleton lives in one place —
git-and-github §Creating a PR — and push delegates to it (no duplication).
Pinned by tests/test_pr_body_template.py. Bumps plugin to 4.4.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): derive severity on-the-fly + schema 3.1.0 additive bits

Producers (esp. check-pr-comments) emit per-finding risk/impact/scope floats
but no integer severity and never route through the coordinator, so findings
rendered INFO and severity_counts/severity_category_matrix stayed zero. Fix it
where it bites — at render time — and tidy up the schema while we're in here.

Fix 1 (HIGH): new scripts/severity_util.py is the single source of truth for
the OWASP band table, per-finding severity derivation, and the stats/matrix
builder. consolidate_reports imports from it and keeps the legacy private
names (_derive_overall/_derive_severity_int) as re-export aliases. The renderer
gains a normalization pass (markdown/html/triage/pdf): it derives each finding's
integer severity from the floats when absent and recomputes severity_counts +
severity_category_matrix when the supplied counts are missing or all-zero —
never clobbering non-zero supplied counts. triage inherits the fix via
_build_html_context.

Fix 2 (MED): schema accepts optional finding author_type (bot|human) so
check-pr-comments output validates.

Fix 3 (MED): permalink commit now derives from `git rev-parse @{u}` with a
HEAD fallback (check-pr-comments, report-format, grumpy-review) so links resolve
on GitHub instead of 404-ing on an unpushed HEAD.

Fix 4: schema 3.1.0 additive bits — schema_version accepts 3.0.0 and 3.1.0,
metadata.report_type adds pr_audit, finding_section gains optional verdict
(PASS|FAIL|NEEDS_REVIEW). severity stays optional on findings.

Schema -> 3.1.0, plugin -> 4.3.0. New tests: test_severity_util.py, schema
additive cases, render regression (floats-only report renders HIGH + non-zero
counts), re-export parity, and an end-to-end producer->validate->render check
in the shell pipeline. 357 passed; black/ruff clean; plugin validates.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): renderer on-the-fly severity + permalink @{u} + tests

Completes PR-A. The earlier commit shipped schema 3.1.0, severity_util, and the
consolidate_reports re-export; this lands the renderer-side fix plus the
remaining producer/test/doc changes.

- generate_review_report.py: add _normalize_report() — derive each finding's
  integer severity from risk/impact/scope when absent, and recompute
  severity_counts + severity_category_matrix only when supplied counts are
  missing/all-zero (never clobbering non-zero ones). Invoked from
  render_markdown, _build_html_context (html + triage), and render_pdf.
  triage_server.py inherits via the triage format, no change needed.
- check-pr-comments / report-format / grumpy-review: permalink commit now uses
  git rev-parse @{u} with a git rev-parse HEAD fallback so links resolve on
  GitHub; check-pr-comments schema_version bumped to 3.1.0 (3.0.0 still accepted).
- consolidate_reports.py: drop unused build_severity_stats import.
- tests: test_severity_util.py (band mapping via derive_severity_int for exact
  boundaries — the float mean rounds at edges), test_render_v3.py regression
  (floats-only report renders HIGH + non-zero counts in md/html, supplied counts
  preserved), test_report_pipeline.sh end-to-end producer->validate->render,
  test_consolidate_reports asserts against cr.SCHEMA_VERSION not a frozen literal.
- CHANGELOG: 4.3.0 section.

384 tests pass; black/ruff clean; plugin validates; shell pipeline 26/0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(consolidate): drop unused build_severity_stats import (ruff F401)

The coordinator has its own compute_statistics; only the renderer uses the
shared build_severity_stats. Removing the dead import clears the last ruff F401.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(review-pr): Pass C v1.1 doc heuristics + regression tests (4.5.0)

Tighten review-pr Pass C (promise verification) per TODOs 8c17d019 and
4463333d (F-1..F-5, F-7). DOC-ONLY in skills/review-pr/SKILL.md plus tests;
no schema changes (PR-A landed pr_audit + finding_section.verdict already).

- Fenced-body unwrap: strip+dedent a wholly-fenced PR body before the
  column-0 Summary/Out-of-scope regexes; emit one INFO "PR body unparseable"
  finding instead of silently skipping when nothing parses.
- Clean-pass shape: findings:[] plus one INFO "PR self-description verified".
- Compound titles: split on commas/em-dashes, verify per-topic, majority-hits.
- Undocumented-change: keep >=50 LOC trigger, define "mentioned" precisely
  (keyword overlap with a Summary bullet or field-ownership-table row).
- Summary-heading precedence: ## Summary > ### Summary > ## What changed.
- Optional finding_section.verdict (PASS/FAIL/NEEDS_REVIEW) + report_type.
- code_snippets language: cross-ref report-format §code_snippets, not hard-coded.

Tests: new tests/test_review_pr_passc.py (grep-assert the documented rules +
a parser-mirror of the fenced-body dedent that exposes ## Summary) and
tests/fixtures/pr-promises/synthetic-fenced.md (self-describing, carries the
standard <!-- expected --> annotation). Full suite: 358 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): address Copilot review on PR #42

Three valid catches in the severity-pipeline code:

- generate_review_report: _normalize_summary_statistics rebuilt
  severity_counts + severity_category_matrix but left total_findings
  stale, so the HTML KPI could disagree with the rebuilt counts. Realign
  total_findings from the rebuilt stats (same all-zero/missing branch, so
  a hand-supplied non-zero stats block is still never touched).
- generate_review_report: corrected the _normalize_summary_statistics
  docstring — the real trigger is "counts missing/all-zero AND >=1 finding
  exists" (floatless findings count as INFO), not "a finding carries a
  derivable severity".
- consolidate_reports: ACCEPTED_SCHEMA_VERSIONS now derives from the
  schema's schema_version enum (single read shared with SCHEMA_VERSION)
  instead of a hard-coded set, so it can't drift on the next schema bump.
  _read_schema_version() preserved as a thin newest-entry wrapper.

Regression guards: anti-drift assert (ACCEPTED_SCHEMA_VERSIONS == set of
schema enum) and a total_findings-realignment test that fails without the
fix. 382 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(report-pipeline): address second Copilot pass on PR #42

Three follow-on findings on the on-the-fly severity normalization:

- generate_review_report: _normalize_finding_severities now computes and
  stores overall_severity (mean of risk/impact/scope) when missing/invalid,
  then derives the integer band from that value — instead of filling only
  the integer severity. Producer-shape reports now populate the Markdown
  (overall=...) suffix and the HTML data-overall "Sort: overall" key. A
  finding with a valid int severity but no overall_severity still gets
  overall_severity filled; both already-valid values are left untouched.
- generate_review_report: import derive_overall/derive_severity_int from
  severity_util (derive_finding_severity stays in severity_util, still used
  by _effective_severity; the renderer no longer imports it).
- consolidate_reports: remove the now-dead _read_schema_version() wrapper —
  SCHEMA_VERSION reads _SCHEMA_VERSIONS[-1] directly since ACCEPTED_SCHEMA_
  VERSIONS became schema-enum-derived.

Two regression tests pin the overall_severity fill (floats-only finding +
int-severity-without-overall). 384 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(test): black-format test_pr_body_template.py

PR-C's own test file (from f10308d) failed black --check in this
environment — assert line-wrapping only, no logic change. Surfaced
during the reviewer-rigor/severity-pipeline merges; brings the
git diff main...HEAD set fully clean under black.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(test): ruff E741 + black on test_review_pr_passc.py

Rename ambiguous loop var l->ln and apply black to #44's new Pass C
regression test, carried in with the feat/pr-why-template merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(review-pr): correct Pass C informational-finding severity floats + tighten fence parser

Three valid Copilot findings on PR #44:

- Pass C's "PR self-description verified" (clean pass) and "PR body
  unparseable" markers were labeled INFO but specified scope=1.0. With
  scope pinned at 1.0 the mean (risk+impact+scope)/3 floors at 1/3, so the
  band table can never reach INFO (needs <0.1) — both derived to MEDIUM. Fix
  per the check-pr-comments RESOLVED convention: informational Pass C
  findings use scope=0.0 (no actionable diff work). "verified" -> 0.1/0.1/0.0
  -> INFO (band 1); "unparseable" relabeled LOW -> 0.2/0.2/0.0 -> LOW (band
  2). Documented the Pass C scope=0.0 exception (scope=1.0 applies only to
  axis 1-3 mismatches).
- Tightened the fenced-body unwrap's closing-fence check (SKILL spec + the
  test's parser-mirror): the closer must be the same fence character, length
  >= the opener, and contain only fence chars — previously a 3-char closer
  unwrapped a 4-char opener and "```python" trailing text was accepted.

Guards added: a band-derivation test pinning the documented floats to the
severity_util bands (INFO/LOW), and fence cases for the 4-vs-3 mismatch and
trailing-text-after-fence. 408 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(changelog): correct PR-body-unparseable finding band INFO -> LOW

Copilot follow-on on PR #44: the 4.5.0 CHANGELOG entry still described the
"PR body unparseable" Pass C fallback as INFO, but the SKILL fix relabeled
it LOW (scope=0.0, risk≈impact≈0.2 -> mean ≈0.133 -> band 2). Align the
changelog. The "PR self-description verified" finding correctly stays INFO.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(review-pr): correct band-math wording in Pass C scope note + test comment

Copilot's 4th pass on PR #44 flagged that the scope=1.0 rationale overstated
"MEDIUM". Verified ground truth: at scope=1.0, {0.1,0.1,1.0} derives to LOW(2)
(IEEE-754 puts (0.1+0.1+1.0)/3 = 0.39999... just under the 0.4 cutoff), and
{0.2,0.2,1.0} to MEDIUM(3). The explanatory text in both the SKILL exception
note and the test comment wrongly said both inflate to MEDIUM.

- SKILL: reword the exception note to state the real means/bands (LOW + MEDIUM).
- test: rename test_scope_one_would_inflate_to_medium ->
  test_scope_one_lifts_above_documented_bands; correct the comment and document
  the IEEE-754 boundary so the intentional ==2 isn't mistaken for a typo.

Assertions unchanged (both verified correct). The Copilot complaint that the
==2 assertion "will fail" was a false positive — it's the genuine float result.
408 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claudius Agent <noreply@anthropic.com>
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.

3 participants