Feat: Add PR review impact metrics producer

[rubambiza]

Summary

Adds pr-review-impact.sh, a read-only metrics producer that measures the clawgenti PR reviewer's impact on time-to-merge. It writes reports/pr-review/impact.json, consumed by the automation health dashboard (kagenti/agent-skills#16).

Per repo, the bot's activation date is derived at runtime from the earliest PR carrying a clawgenti marked review (<!-- reviewed: SHA --> in the review body) — no hardcoded split date. Merged PRs are then segmented:

• before vs. after that repo's own activation (the headline impact measure), and
• reviewed vs. unreviewed (secondary; selection-biased since ready-for-ai-review marks substantive PRs).

Median time-to-merge is reported in hours (PRs here merge in hours, so day granularity would round to 0).

Detection notes

• Reviews are fetched via the reviews API (/pulls/N/reviews), not the PR description. A reviewed-by:clawgenti search prefilter plus a required marker excludes dep-bump comment-only activity on Dependabot PRs.
• get_repos() isolates the repo set behind a function with a documented seam to swap in a tier=core query once the repository-tiers proposal (#1811) lands.

Testing

• tests/test-pr-review-impact.sh covers per-repo activation, hours-TTM medians, and Dependabot exclusion.
• Verified live against the org: 4 activations derived, non-zero hour medians.

Fixes #19

Assisted by Claude Code

[clawgenti]

Shell script is well-structured with thorough test coverage, clean error handling, and solid documentation of the activation-detection logic.

• ORG variable defined but only referenced in a commented-out code block — dead code.
• TMPDIR is overwritten with $PROGRAM_TMPDIR (shadowing the shell env var); not harmful given setup_workspace controls the value, but worth noting the inconsistency with the test file which uses its own mktemp -d.

---

Reviewed by clawgenti using github:pr-review

[rubambiza]

Thanks for the review. Both nits addressed in e597626:

• ORG dead code: removed the standalone ORG="kagenti" assignment and inlined the literal kagenti into the future-seam comment, so the seam snippet stays self-documenting without an unused top-level variable.
• TMPDIR shadowing: renamed to WORK_DIR throughout, matching the PROGRAM_TMPDIR naming convention and avoiding the shadow of the standard shell temp-dir variable.

Test still passes and the live dry-run is unchanged (4 activations, reviewed count 12, non-zero hour medians).

[clawgenti]

Two nits found in otherwise well-structured new scripts; logic, detection, and test coverage look solid.

• nit tests/test-pr-review-impact.sh line 7: TMPDIR=$(mktemp -d) shadows the system $TMPDIR variable used by mktemp and other tools — rename to TEST_TMPDIR (and update all references in the test).
• nit Commit subjects (feat:, refactor:) and PR title (Feat:) use conventional-commit text prefixes instead of emoji prefixes per git:commit conventions.

---

Reviewed by clawgenti using github:pr-review

[clawgenti]

nit: TMPDIR=$(mktemp -d) overrides the system $TMPDIR environment variable, which mktemp and other tools use to determine the temp directory. If any subcommand inside the test relies on $TMPDIR, it now points into your test dir instead of the system temp. Rename to TEST_TMPDIR and update the trap and all $TMPDIR references in this file.

[rubambiza]

Thanks. Addressed the actionable nit in 0b764aa:

• TMPDIR shadowing (tests/test-pr-review-impact.sh): renamed to TEST_TMPDIR and updated the assignment, the trap, and all references. Same fix we applied to WORK_DIR in the script earlier. Test still passes (PASS: segmentation + hours-TTM + dependabot exclusion).

• Commit/PR title prefixes: leaving as-is. This repo follows the capitalized conventional-prefix convention enforced by pr-verifier (e.g. Test:, Fix:), not the emoji style the git:commit skill still documents — that skill discrepancy is being corrected separately (Fix: Correct PR-review title/commit convention to capitalized prefixes kagenti#2021).

[clawgenti]

New script pr-review-impact.sh is well-structured — logic, test coverage, and Dependabot exclusion design are solid. Two nits and a commit-convention flag below.

• Commit conventions (nit): All 3 commits lack an emoji prefix required by Kagenti's git:commit convention (feat:, refactor:, Test: should be 🚀 feat:, ♻️ refactor:, 🧪 test: etc.). Commit 3 also uses uppercase Test: — should be lowercase.
• PR title (nit): Feat: Add PR review impact metrics producer is missing an emoji prefix per convention.

---

Reviewed by clawgenti using github:pr-review

[clawgenti]

nit: The heredoc is single-quoted ('USAGE'), so $REPORTS_DIR is printed literally in the help output — the reader sees the unexpanded variable name, not a value. Consider replacing with just ./reports/pr-review (the actual default) to avoid the misleading text.

[clawgenti]

suggestion: LOOKBACK_LIMIT=200 is a meaningful cap — if a repo has more than 200 merged PRs it silently truncates the baseline. Worth a short note in the --help output (e.g. --reports-dir DIR) or a --lookback flag so operators know the window.

[rubambiza]

Addressed both --help nits in 286b5b6:

• Misleading --reports-dir default: the single-quoted heredoc printed $REPORTS_DIR literally. Reworded so the help shows the real default (./reports/pr-review) and describes the env-var as an explicit fallback ("or $REPORTS_DIR if set in the environment") rather than presenting an unexpanded variable as the default.
• Silent lookback truncation: added a NOTES section documenting that each repo's baseline is capped at the most recent 200 merged PRs (LOOKBACK_LIMIT), so operators know the window. Kept it as a documented constant rather than a new --lookback flag to avoid expanding the CLI surface for what is currently a fixed operational window — happy to add the flag if you would prefer it configurable.

Help output verified and the test still passes (PASS: segmentation + hours-TTM + dependabot exclusion).

On the commit/PR-title emoji prefixes: leaving as-is — this repo follows the capitalized text-prefix convention that pr-verifier enforces, not the emoji style the git:commit skill still documents (that skill discrepancy was corrected in kagenti/kagenti#2021).

[clawgenti]

Clean implementation of the impact metrics producer — detection logic, activation derivation, and test coverage all look sound.

All checks pass. Ready for human review.

---

Reviewed by clawgenti using github:pr-review

[esnible]

Clean, well-documented metrics producer. The script correctly reuses program-lib.sh infrastructure (setup_workspace, gh_with_backoff, write_report_latest — all signatures verified), follows the established scanner conventions (--dry-run, --verbose, $REPORTS_DIR override), and is read-only against GitHub.

Verification performed:

• shellcheck passes at the strictest (style) level on both files
• the bundled test passes locally
• edge cases (empty buckets, empty marked-set) produce valid JSON for the downstream --argjson assembly
• the test's median_ttm is byte-identical to the script's

The two-layer detection (reviewed-by:clawgenti search prefilter + required <!-- reviewed: marker) sensibly excludes Dependabot comment-only activity, and the test explicitly exercises that exclusion. The per-repo runtime-derived activation (no hardcoded date) and the documented tier=core future seam (#1811) are nice touches.

Two non-blocking notes inline; both optional.