fix(review): enable targeted validator inspection

[Alan-TheGentleman]

🔗 Linked Issue

Closes #2945

---

🏷️ PR Type

• type:bug — Bug fix
• type:feature — New feature
• type:docs — Documentation only
• type:refactor — Code refactoring
• type:chore — Build, CI, or tooling changes
• type:breaking-change — Breaking change

---

📝 Summary

Integration tracker for D2945's provider-bound corrected-tree inspection:

• Slice 1, PR fix(review): resolve immutable corrected inspection #2991: immutable corrected-inspection resolver and fail-closed unit matrix
• Slice 2, PR fix(review): admit targeted corrected inspection #3002: public inspect-candidate admission, STATUS binding, j95, docs, and temporary baseline removal
• Review correction, PR fix(review): harden targeted inspection evidence #3011: evidence classifications, operation contracts, and provider-owned argument/path derivation

All slices are integrated, the tracker is synchronized with current main, the temporary deadcode entry is removed, and exact-head local verification is green. Merge after tracker CI/review completes.

---

Chain Context

Field	Value
Strategy	Feature Branch Chain
Tracker	📍 This PR
Base	main@e612ef33
Integrated	#2991 resolver; #3002 public admission + j95; #3011 review corrections
Pending	Exact-head tracker CI/review
Review boundary	Every child <=400 lines; aggregate tracker uses size:exception only as delivery boundary

main
 └── 📍 tracker: targeted validator inspection
      ├── ✅ #2991 immutable resolver
      ├── ✅ #3002 public admission + j95
      └── ✅ #3011 review corrections

---

🧪 Test Plan

Child evidence:

• Focused resolver RED/GREEN/decoys
• Root tests, vet, gofmtcheck, deadcode
• Required child CI and platform lanes

Final tracker evidence:

• STATUS emits corrected context, purpose, and request hash
• Targeted inspector reads immutable corrected tree after live drift
• Forged/stale/lens/order decoys fail closed
• Existing FINALIZE remains unchanged and succeeds after restored live state
• Driven j95 and full 91-core corpus pass
• Exact-head tracker required CI and review pass

---

✅ Contributor Checklist

• Linked to approved issue bug(review 2.2.4): targeted_validation_required transition supplies no repository_context, so the correction validator can never inspect #2945
• Exactly one type:* label will be applied
• Child PRs remain within 400 lines
• Aggregate tracker carries documented size:exception
• All slices integrated and independently reviewed
• Final candidate synchronized with main@e612ef33 and locally verified
• Conventional commits with no Co-Authored-By

---

💬 Notes for Reviewers

The aggregate tracker is not the semantic review unit. The resolver was reviewed in #2991, public admission/j95 in #3002, and exact-head findings in #3011. The exact tracker head is 4b972750; local root, bench-module, deadcode, 91-core driven, and source-coupled checks are green.

Summary by CodeRabbit

• New Features
  • Added targeted validation for corrected candidates using provider-issued inspection details.
  • Inspections now read the immutable corrected snapshot, even when the live workspace changes.
  • Added validation for repository context, lineage, revision, target, purpose, and request hash.
• Bug Fixes
  • Rejected forged, stale, mismatched, or drifted inspection requests.
  • Improved restoration and finalization after workspace changes.
• Documentation
  • Added integration guidance for targeted validation and corrected-candidate inspection.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Targeted corrected-candidate inspection

Layer / File(s)	Summary
Targeted inspection binding contracts bench/journeys_wave1.go, internal/cli/review_next_transition.go, internal/cli/review_status_contract.go, internal/cli/review_submission_descriptor_test.go, internal/reviewtransaction/repository_locator.go, bench/journeys_capture_evidence_v5.go, docs/review-integration.md	Targeted-validation descriptors now include six provider-bound arguments. Repository context resolution separates opaque locator validation from live authority validation. Documentation defines the required inspection inputs and restrictions.
Immutable corrected snapshot resolution internal/reviewtransaction/verification_evidence.go, internal/reviewtransaction/corrected_inspection.go	Captured verification evidence is loaded and validated by identity. Corrected inspection validates authority, evidence, request identity, and snapshot metadata before returning the immutable corrected snapshot.
Targeted CLI inspection flow internal/cli/review_inspect_candidate.go	review inspect-candidate adds targeted-validation purpose and request-hash handling. Targeted mode rejects lens and order flags and inspects the resolved corrected snapshot without repository discovery.
Correction workflow validation bench/journeys_capture_evidence_v5.go, internal/cli/review_inspect_candidate_test.go, internal/reviewtransaction/targeted_validation_request_test.go, internal/reviewtransaction/verification_evidence_test.go, bench/journeys_sdd_test.go	Correction evidence and completion helpers accept lineage. Tests and journey j95 cover immutable reads during worktree drift, rejected drifted finalization, restoration, evidence classification, and invalid bindings. j95 increases the registered journey count to 91.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Validator
  participant InspectCandidate
  participant ReviewTransaction
  participant EvidenceStore
  Validator->>InspectCandidate: invoke targeted inspection with provider-bound arguments
  InspectCandidate->>ReviewTransaction: resolve corrected inspection binding
  ReviewTransaction->>EvidenceStore: load captured verification evidence
  EvidenceStore-->>ReviewTransaction: return validated evidence
  ReviewTransaction-->>InspectCandidate: return immutable corrected snapshot
  InspectCandidate-->>Validator: return inspection payload

Loading

Possibly related PRs

• Gentleman-Programming/gentle-ai#2011: Earlier review inspect-candidate functionality extended by this targeted-validation flow.
• Gentleman-Programming/gentle-ai#1796: Related provider-bound review context and captured verification evidence.
• Gentleman-Programming/gentle-ai#2640: Earlier correction evidence capture flow extended by this change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.15% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: enabling targeted validator inspection.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/2945-targeted-validator-inspection

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR completes the #2945 fix by making the targeted_validation_required transition provider-bind the corrected-tree inspection inputs (including repository context + request hash) and by enabling review inspect-candidate to inspect the immutable corrected candidate (not the live worktree) for targeted validation.

Changes:

• Add an immutable corrected-candidate inspection resolver/binding path for targeted validation.
• Extend STATUS/next-transition descriptors to include repository-context, purpose=targeted-validation, and request-hash, and validate them fail-closed.
• Add unit + bench journey coverage (j95) proving corrected-tree inspection remains stable across live drift.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/reviewtransaction/verification_evidence.go	Adds evidence lookup by target identity to support corrected-tree reconstruction.
internal/reviewtransaction/targeted_validation_request_test.go	Adds fail-closed and live-drift coverage for corrected inspection resolution.
internal/reviewtransaction/repository_locator.go	Splits “opaque locator resolution” from “live validation” to support immutable inspection flows.
internal/reviewtransaction/corrected_inspection.go	Implements corrected-candidate inspection resolution and binding verification.
internal/cli/review_submission_descriptor_test.go	Asserts targeted-validation descriptor now carries the corrected inspection binding args.
internal/cli/review_status_contract.go	Validates targeted-validation descriptors include the full corrected inspection binding (6 args).
internal/cli/review_next_transition.go	Emits the new targeted-validation arguments for contract v2.
internal/cli/review_inspect_candidate.go	Adds purpose/request-hash mode to inspect immutable corrected candidates for targeted validation.
internal/cli/review_inspect_candidate_test.go	Proves corrected immutable inspection succeeds after drift and rejects decoy bindings.
docs/review-integration.md	Documents the targeted-validation inspect-candidate closed form and constraints.
bench/journeys_wave1.go	Extends bench status decoding to capture transition arguments.
bench/journeys_sdd_test.go	Updates core journey count and documents new j95.
bench/journeys_capture_evidence_v5.go	Adds j95 journey proving provider-bound corrected-tree inspection through drift.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coderabbitai]

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bench/journeys_capture_evidence_v5.go`:
- Around line 21-22: Update targetedInspectionCapability.Flags to declare the
--operation, --path-index, and --side flags alongside the existing
inspect-candidate flags. Ensure inspectJ95CorrectedCandidate uses this
capability declaration so the runner validates all required flags before
execution.

In `@docs/review-integration.md`:
- Line 263: Update the targeted validation guidance around review
inspect-candidate to state that canonical path index applies only to path and
object operations, while object also requires --side; specify that global
name-status and numstat operations do not accept --path-index.

In `@internal/cli/review_inspect_candidate_test.go`:
- Line 296: Update the test helper’s returned path index to resolve candidate.go
from the corrected target’s path manifest instead of returning literal 0. Use
the manifest associated with the corrected tree, and make the test fail
explicitly when candidate.go is absent so new paths cannot silently select the
wrong file.
- Around line 295-296: Update the helper returning the review command arguments
to resolve required ReviewTransitionInput.Arguments entries by their Name fields
instead of fixed indexes. Retrieve the values for repository-context,
expected-revision, lineage, target, purpose, and request-hash by name, and fail
when any required argument is absent.

In `@internal/cli/review_inspect_candidate.go`:
- Around line 109-131: Refactor the review-inspection flow so targeted
validation and the alternate mode each resolve `builder` and `snapshot`, then
share one common `deps.inspect` error-wrapping, context-check, and
payload-return tail. Preserve each mode’s distinct resolution and preflight
behavior, while also consolidating the duplicated repository-context, revision,
lineage, and target argument validation where practical.
- Around line 37-43: Update the resolveCorrected callback to distinguish
authority-state errors from context-resolution errors returned by
ResolveCorrectedCandidateInspectionBinding. Wrap only genuine context failures
with reviewRepositoryContextResolutionFailure; preserve spent correction
authority, missing passed evidence, and stale request-hash errors so they are
reported with their specific remedies and authority-state classifications.

In `@internal/cli/review_next_transition.go`:
- Around line 878-885: Define a shared package-level constant for the
“targeted-validation” purpose and update reviewTargetedValidationArguments to
use it. Replace the matching literals in the validator, CLI mode selector, and
review_submission_descriptor_test.go so every producer, consumer, and test
references the same constant.

In `@internal/cli/review_submission_descriptor_test.go`:
- Around line 94-101: Extend the forge-value loop in the targeted submission
descriptor validation test to include decoy values for the purpose and
request-hash arguments, in addition to the existing lineage, revision, target,
and repository-context values. Use the corresponding validation argument
ordering and preserve the existing rejection assertion and value restoration for
both new cases.

In `@internal/reviewtransaction/corrected_inspection.go`:
- Around line 112-121: Add a concise comment around the two resolution passes in
the corrected candidate inspection flow, referencing
targetedValidationRequestForCorrection and ResolveCorrectedCandidateInspection.
Document that the first pass bootstraps correction from identity-selected
evidence, while the second validates Git evidence and confirms
binding.TargetIdentity matches request.CorrectionTargetIdentity; also note that
targetedValidationRequestHash covers CorrectionTargetIdentity. Preserve both
passes unchanged.

In `@internal/reviewtransaction/targeted_validation_request_test.go`:
- Around line 360-379: Add an assertion immediately after SnapshotBuilder.Build
and before publishing the repository context that compares correction’s target
identity with request.CorrectionTargetIdentity. Fail the fixture with testing.T
if they differ, ensuring every fixture user detects identity divergence before
evidence publication.

In `@internal/reviewtransaction/verification_evidence.go`:
- Around line 284-309: Extract the duplicated hardened directory/artifact read
and parse sequence from ReadCapturedVerificationEvidence and
ReadCapturedVerificationEvidenceByIdentity into an unexported
readCompactEvidencePair helper. Have the helper retain all directory checks,
both file reads, record parsing and payload validation, plus the post-read
Lstat/os.SameFile check, while returning the underlying error needed by callers.
Keep each exported reader’s existing os.ErrNotExist sentinel mapping and binding
predicate or direct field comparisons outside the helper.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ea5fda9a-18a8-462d-8c08-f0298b91b5cc

📥 Commits

Reviewing files that changed from the base of the PR and between e612ef3 and c1ff7dc.

📒 Files selected for processing (13)

• bench/journeys_capture_evidence_v5.go
• bench/journeys_sdd_test.go
• bench/journeys_wave1.go
• docs/review-integration.md
• internal/cli/review_inspect_candidate.go
• internal/cli/review_inspect_candidate_test.go
• internal/cli/review_next_transition.go
• internal/cli/review_status_contract.go
• internal/cli/review_submission_descriptor_test.go
• internal/reviewtransaction/corrected_inspection.go
• internal/reviewtransaction/repository_locator.go
• internal/reviewtransaction/targeted_validation_request_test.go
• internal/reviewtransaction/verification_evidence.go

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/reviewtransaction/verification_evidence.go:251

• The doc comment says this function reads a passed evidence record, but the implementation does not check captured.Record.Outcome (callers do that separately). This is misleading and can cause incorrect assumptions when using the helper.

// ReadCapturedVerificationEvidenceByIdentity reads a passed evidence record
// before its immutable tree can be reconstructed from the record itself.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/cli/review_inspect_candidate_test.go:352

• removeInspectionArg can panic if the flag is missing (slices.Index returns -1) or if it appears without a following value. Adding bounds checks avoids an invalid-slice panic if future test cases attempt to remove an argument that isn't present.

func removeInspectionArg(args []string, name string) []string {
	result := slices.Clone(args)
	index := slices.Index(result, name)
	return append(result[:index], result[index+2:]...)
}