Require proof-backed PR review handoff claims #395

Closed
opened 2026-07-07 09:19:35 -05:00 by jcwalker3 · 2 comments
Owner

Problem:
Reviewer final reports can claim workflow proof that is not visibly supported by the action transcript. This makes it difficult for the controller/operator to distinguish actual validation from narrative overclaim.

Observed example:
A PR #383 review/merge report claimed:

  • inventory was complete because 13 results were less than page size 50,
  • earlier PRs were skipped with conflict proof,
  • a baseline master worktree was created and validated,
  • master was merged into the review worktree,
  • baseline and review worktrees were cleaned.

The merge outcome appears safe, but the visible command transcript did not clearly show every claimed proof step. Future reports should require explicit proof entries.

Required behavior:
A reviewer final report must not claim proof-sensitive workflow steps unless each claim is backed by explicit command/tool evidence in the transcript or structured MCP result metadata.

Acceptance criteria:

  1. Inventory proof must include explicit pagination metadata:

    • has_more=false or equivalent,
    • is_final_page=true or equivalent,
    • inventory_complete=true or equivalent,
    • total count if available.
  2. Earlier PR skip proof must include either:

    • explicit command/tool proof for each skipped PR,
    • or structured inventory fields proving non-mergeable / prior-blocked state.
  3. Baseline validation claims must include:

    • baseline worktree path,
    • baseline target SHA,
    • baseline dirty-before status,
    • exact baseline validation command,
    • exact baseline result,
    • baseline dirty-after status.
  4. Review worktree master-integration claims must include the exact merge/rebase/simulation command and result.

  5. Cleanup claims must include final git worktree list or equivalent proof that session-owned worktrees were removed.

  6. Reports must distinguish:

    • command actually run,
    • result inferred from MCP metadata,
    • result inherited from previous blocker state,
    • result not checked.
  7. Tests cover:

    • report claims baseline validation without baseline command,
    • report claims skipped-PR conflict proof without evidence,
    • report claims inventory complete from page size assumption only,
    • report claims cleanup without final worktree proof,
    • fully proof-backed report passes.
  8. Documentation updates the review-merge handoff template to require proof-backed claims.

Non-goals:

  • Do not block valid merges that have complete proof.
  • Do not allow author self-review.
  • Do not weaken exact review_pr or merge_pr capability checks.
Problem: Reviewer final reports can claim workflow proof that is not visibly supported by the action transcript. This makes it difficult for the controller/operator to distinguish actual validation from narrative overclaim. Observed example: A PR #383 review/merge report claimed: * inventory was complete because 13 results were less than page size 50, * earlier PRs were skipped with conflict proof, * a baseline master worktree was created and validated, * master was merged into the review worktree, * baseline and review worktrees were cleaned. The merge outcome appears safe, but the visible command transcript did not clearly show every claimed proof step. Future reports should require explicit proof entries. Required behavior: A reviewer final report must not claim proof-sensitive workflow steps unless each claim is backed by explicit command/tool evidence in the transcript or structured MCP result metadata. Acceptance criteria: 1. Inventory proof must include explicit pagination metadata: * `has_more=false` or equivalent, * `is_final_page=true` or equivalent, * `inventory_complete=true` or equivalent, * total count if available. 2. Earlier PR skip proof must include either: * explicit command/tool proof for each skipped PR, * or structured inventory fields proving non-mergeable / prior-blocked state. 3. Baseline validation claims must include: * baseline worktree path, * baseline target SHA, * baseline dirty-before status, * exact baseline validation command, * exact baseline result, * baseline dirty-after status. 4. Review worktree master-integration claims must include the exact merge/rebase/simulation command and result. 5. Cleanup claims must include final `git worktree list` or equivalent proof that session-owned worktrees were removed. 6. Reports must distinguish: * command actually run, * result inferred from MCP metadata, * result inherited from previous blocker state, * result not checked. 7. Tests cover: * report claims baseline validation without baseline command, * report claims skipped-PR conflict proof without evidence, * report claims inventory complete from page size assumption only, * report claims cleanup without final worktree proof, * fully proof-backed report passes. 8. Documentation updates the review-merge handoff template to require proof-backed claims. Non-goals: * Do not block valid merges that have complete proof. * Do not allow author self-review. * Do not weaken exact `review_pr` or `merge_pr` capability checks.
jcwalker3 added the status:in-progress label 2026-07-07 09:21:33 -05:00
Author
Owner

Duplicate-implementation reconciliation (author session prgs-author, 2026-07-07)

Two implementations of this issue exist because a work-issue session hit the duplicate-PR gate after pushing its branch:

  • PR #397 (canonical candidate): branch feat/issue-395-proof-backed-review-handoff, head c1d85b6. Module reviewer_proof_backed_handoff.py with structured session cross-checks (action_log, inventory_session, baseline_session, skip_session), page-size-assumption rejection, proof-source labeling enforcement (command / MCP metadata / prior blocker / not checked — rejects unlabeled "live proof"), schema update to schemas/review-merge-final-report.md (the actual handoff template, per criterion 8), build_final_report downgrade wiring (hint-gated), 10 tests including all five required scenarios.
  • Duplicate branch (no PR, per gate): feat/issue-395-proof-backed-handoff-claims, commit e1e76f0. Module reviewer_proof_backed_claims.py (text-only), 13 tests, full suite green on its base (1501 passed).

Recommendation: keep PR #397 as-is and absorb four small deltas from e1e76f0 that #397 lacks:

  1. An exported PROOF_PROVENANCE_CLASSES constant (typed source of truth for the four provenance classes; #397 encodes them only inside a regex).
  2. proof_backed_claims_proven / proof_backed_claims_violations entries in the build_final_report checks dict (machine-readable surface; #397 adds only downgrade reasons).
  3. Workflow-doc rule section (review-merge-pr.md §30A) cross-referencing the schema requirements, plus its contract-test marker in test_llm_workflow_split.py#397 updates the schema template but not the workflow rules doc.
  4. Test angles #397 lacks: inventory-complete claimed with no pagination metadata at all (distinct from the page-size-assumption case), baseline claim missing dirty-before/after specifically, and skipped-PR failure reasons carrying the PR number.

No second PR was or will be created for this issue; neither branch should be deleted until #397's author absorbs or declines the deltas. Cherry-pick source: e1e76f0 on feat/issue-395-proof-backed-handoff-claims.

## Duplicate-implementation reconciliation (author session `prgs-author`, 2026-07-07) Two implementations of this issue exist because a work-issue session hit the duplicate-PR gate after pushing its branch: - **PR #397** (canonical candidate): branch `feat/issue-395-proof-backed-review-handoff`, head `c1d85b6`. Module `reviewer_proof_backed_handoff.py` with structured session cross-checks (`action_log`, `inventory_session`, `baseline_session`, `skip_session`), page-size-assumption rejection, proof-source labeling enforcement (`command` / `MCP metadata` / `prior blocker` / `not checked` — rejects unlabeled "live proof"), schema update to `schemas/review-merge-final-report.md` (the actual handoff template, per criterion 8), `build_final_report` downgrade wiring (hint-gated), 10 tests including all five required scenarios. - **Duplicate branch** (no PR, per gate): `feat/issue-395-proof-backed-handoff-claims`, commit `e1e76f0`. Module `reviewer_proof_backed_claims.py` (text-only), 13 tests, full suite green on its base (1501 passed). **Recommendation: keep PR #397 as-is and absorb four small deltas from `e1e76f0` that #397 lacks:** 1. An exported `PROOF_PROVENANCE_CLASSES` constant (typed source of truth for the four provenance classes; #397 encodes them only inside a regex). 2. `proof_backed_claims_proven` / `proof_backed_claims_violations` entries in the `build_final_report` checks dict (machine-readable surface; #397 adds only downgrade reasons). 3. Workflow-doc rule section (`review-merge-pr.md` §30A) cross-referencing the schema requirements, plus its contract-test marker in `test_llm_workflow_split.py` — #397 updates the schema template but not the workflow rules doc. 4. Test angles #397 lacks: inventory-complete claimed with *no* pagination metadata at all (distinct from the page-size-assumption case), baseline claim missing dirty-before/after specifically, and skipped-PR failure reasons carrying the PR number. No second PR was or will be created for this issue; neither branch should be deleted until #397's author absorbs or declines the deltas. Cherry-pick source: `e1e76f0` on `feat/issue-395-proof-backed-handoff-claims`.
Author
Owner

Evidence update — PR #374 shows good pagination fields but final reports must preserve raw proof.

The PR #374 report included stronger inventory fields than prior runs: page=1, per_page=50, returned_count=10, has_more=false, is_final_page=true, and inventory_complete=true.

This is the kind of proof the canonical workflow needs. The verifier should require these explicit fields and reject older wording such as “returned fewer than page size, so inventory complete” when has_more, is_final_page, or total metadata is absent.

Evidence update — PR #374 shows good pagination fields but final reports must preserve raw proof. The PR #374 report included stronger inventory fields than prior runs: `page=1`, `per_page=50`, `returned_count=10`, `has_more=false`, `is_final_page=true`, and `inventory_complete=true`. This is the kind of proof the canonical workflow needs. The verifier should require these explicit fields and reject older wording such as “returned fewer than page size, so inventory complete” when `has_more`, `is_final_page`, or total metadata is absent.
sysadmin removed the status:in-progress label 2026-07-07 10:34:37 -05:00
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#395