Skip to content

feat: Add claims check and opportunistic head-vs-base probes to github-pr-review - #25

Closed
jimcadden wants to merge 1 commit into
rossoctl:mainfrom
jimcadden:feat/pr-review-behavioral-checks
Closed

jimcadden wants to merge 1 commit into
rossoctl:mainfrom
jimcadden:feat/pr-review-behavioral-checks

Conversation

@jimcadden

Copy link
Copy Markdown
Contributor

Summary

Mirrors rossoctl/agent-skills#53 into this repo, where the deployed pr-review program lives. The misses that motivated the change (rossoctl/cortex#1658, #1659) came from this copy, so the fix has to land here to take effect; the agent-skills PR keeps the standalone plugin in sync.

Adds two behavioral checks to skills/github-pr-review/SKILL.md:

  • §3.11 Claims Check — list every fix the PR claims (title, body, commits, new test names) and verify each in its exact target case and sibling variants. The new test must assert what the user sees, not only an intermediate value.
  • §3.12 Behavioral Probes (head vs base) — a concrete-case rule (every finding names an input/situation and the wrong result it leads to; no case, no finding) plus opportunistic head-vs-base probes: run only with tooling already on PATH, never install or substitute tools mid-review. The probe recipe is self-cleaning (trap + worktree remove/prune) and states its $LOG_DIR precondition.
  • Severity: must-fix is defined positively — the user-visible behavior is wrong — so a claimed fix that does not hold (§3.11) and input that base handled but head rejects (§3.12) are must-fix even when main was equally broken.
  • Summary template: the free-text Verification line becomes an auditable Not checked: bullet list naming what was skipped and why (e.g. missing toolchain, which §3.12 forbids installing).

Follows the maintainer review on agent-skills#53 and the tracking issue rossoctl/agent-skills#54.

Assisted-By: Claude Code

…b-pr-review

Mirror of rossoctl/agent-skills#53 for the deployed pr-review program,
which is what missed the two must-fix bugs on rossoctl/cortex#1658 and
#1659 that motivated the change:

- §3.11 Claims Check: verify every fix a PR claims in its exact target
  case and sibling variants; the new test must assert what the user sees.
- §3.12 Behavioral Probes: concrete-case rule (no case, no finding) and
  opportunistic head-vs-base probes that run only with tooling already
  on PATH — never installed mid-review. The recipe is self-cleaning
  (worktree trap) and states its $LOG_DIR precondition.
- must-fix is defined positively: the user-visible behavior is wrong.
  'Not a regression vs main' does not downgrade it.
- The review summary carries an auditable 'Not checked:' list naming
  what was skipped and why.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Jim Cadden <jcadden@ibm.com>
@jimcadden

Copy link
Copy Markdown
Contributor Author

This is a mistake. The github-pr-review skill will be brought over at the vendoring for 0.1.2

@jimcadden jimcadden closed this Nov 10, 2026
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