Skip to content

fix: preserve valid reviewer coverage during output repair - #587

Merged
zzwong merged 1 commit into
mainfrom
zzwong/fix-reviewer-coverage-repair
Sep 9, 2026
Merged

fix: preserve valid reviewer coverage during output repair#587
zzwong merged 1 commit into
mainfrom
zzwong/fix-reviewer-coverage-repair

Conversation

@zzwong

@zzwong zzwong commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve valid review results when coverage lists repeat a file, and make assignment-scope errors easier to correct. In the observed failure, 73 entries represented 71 assigned files. Two duplicate pairs caused the result to be discarded; the retry then lost coverage and introduced an out-of-assignment claim.

Changes

  • Treat repeated inspected or skipped paths as one coverage claim without adding coverage or suppressing findings.
  • Report invalid coverage positions using zero-based array indexes without echoing rejected path text into instructions. Preserve the retry-summary length cap.
  • Keep out-of-assignment claims, conflicting inspected/skipped claims, missing coverage, and incomplete tool evidence blocking approval.

Verification

  • Decoder regression tests failed before the repair and passed afterward.
  • Structured-retry, missing-coverage, approval-clamp, and tool-evidence checks passed.
  • Recorded-output replay normalized 73 entries to exactly 71 assigned claims; the invalid retry remained rejected.
  • Verified the actual reviewer assignment after post-selection coverage expansion against the per-reviewer artifacts.
  • Lint and build passed with Go 1.26.
  • Full local tests passed except the installed-runtime preflight test, which timed out invoking its help command and reproduced on targeted rerun. No adapter behavior changes here.
  • All 10 CI checks passed. The first test job hit a helper-startup deadline; a failed-job rerun passed without code changes. Three targeted local runs also passed; the timeout's root cause is not established.

Live outcome

A scoped local binary completed a live review with valid output from all six reviewers. The repository reviewer reported 71 inspected files, zero skipped files, and zero findings. The replay isolates the decoder repair; the live run demonstrates workflow completion rather than sole causation or guaranteed validation of future output. Application-specific identifiers and evidence are retained privately.

For reproduction, compare the recorded prompt's assignment.files with the corresponding per-reviewer slices and initial structured output. Post-selection expansion precedes reviewer validation-scope construction.

Closes #586.

Treat repeated coverage claims as one claim and report out-of-assignment positions safely so correction attempts remain actionable. Keep missing, conflicting, and invalid coverage checks intact.

@monit-reviewer monit-reviewer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated PR Review

Reviewed commit: 16f7916eb4c1
Profile: open-cli-collective-review - Posting as: monit-reviewer

Summary

Reviewer Findings
go:implementation-tests 0
structure:repo-health 0

Approval Withheld

No blocking or major findings were reported. Approval is withheld because this run did not cover the change:

  • unassigned⚠️ unassigned: changed files were not assigned to a selected reviewer
  • 1 file inspected by no reviewer:
    • docs/llm-task-artifacts.md

Re-running the same review reproduces this: a reviewer that declined a file declines it again. Closing the gap means bringing these paths into the remit of a reviewer that will read them, or establishing that they need no review.

Reviewer Coverage

  • go:implementation-tests — complete (broad); skipped: none; constraints: none
  • structure:repo-health — complete (broad); inspected 2 assigned files (4 inspected across reviewers): internal/llm/contracts.go, internal/pipeline/pipeline_test.go; skipped: none; constraints: Review scoped to the two assigned changed files.
  • unassigned⚠️ unassigned; skipped: docs/llm-task-artifacts.md; changed files were not assigned to a selected reviewer
Inspected files (4)
  • internal/llm/adapter_test.go
  • internal/llm/contracts.go
  • internal/llm/contracts_test.go
  • internal/pipeline/pipeline_test.go

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 1m 43s | $0.04 | openai-codex/gpt-5.6-terra | cr 0.10.299
Field Value
Model openai-codex/gpt-5.6-terra
Reviewers go:implementation-tests, structure:repo-health
Engine pi_rpc · openai-codex/gpt-5.6-terra
Reviewed by cr · monit-reviewer
Duration 1m 43s wall · 2m 41s compute
Cost $0.04
Tokens 10.7k in / 1.1k out

Per-workstream usage

  • orchestrator-selection — openai-codex/gpt-5.6-terra
    • In: 6.2k
    • Out: 311
    • Cache read: 0
    • Cache create: 0
    • Cost: $0.02
    • Duration: 8s
  • go:implementation-tests — openai-codex/gpt-5.6-terra
    • In: 2.0k
    • Out: 442
    • Cache read: 27.1k
    • Cache create: 0
    • Cost: $0.01
    • Duration: 1m 21s
  • structure:repo-health — openai-codex/gpt-5.6-terra
    • In: 1.2k
    • Out: 297
    • Cache read: 14.8k
    • Cache create: 0
    • Cost: $0.01
    • Duration: 1m 07s
  • orchestrator-rollup — openai-codex/gpt-5.6-terra
    • In: 1.2k
    • Out: 38
    • Cache read: 2.8k
    • Cache create: 0
    • Cost: $0.00
    • Duration: 4s

@monit-reviewer monit-reviewer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated PR Review

Reviewed commit: 16f7916eb4c1
Profile: open-cli-collective-review - Posting as: monit-reviewer

Summary

Reviewer Findings
go:implementation-tests 0
structure:repo-health 0

Reviewer Coverage

  • go:implementation-tests — complete (constrained); inspected 4 assigned files (5 inspected across reviewers): internal/llm/adapter_test.go, internal/llm/contracts.go, internal/llm/contracts_test.go, internal/pipeline/pipeline_test.go; skipped: none; constraints: Review limited to the assigned changed Go implementation and test files; tests were inspected but not executed.
  • structure:repo-health — complete (constrained); inspected 3 assigned files (5 inspected across reviewers): docs/llm-task-artifacts.md, internal/llm/contracts.go, internal/pipeline/pipeline_test.go; skipped: none; constraints: none
Inspected files (5)
  • docs/llm-task-artifacts.md
  • internal/llm/adapter_test.go
  • internal/llm/contracts.go
  • internal/llm/contracts_test.go
  • internal/pipeline/pipeline_test.go

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 1m 51s | $0.03 | openai-codex/gpt-5.6-terra | cr 0.10.299
Field Value
Model openai-codex/gpt-5.6-terra
Reviewers go:implementation-tests, structure:repo-health
Engine pi_rpc · openai-codex/gpt-5.6-terra
Reviewed by cr · monit-reviewer
Duration 1m 51s wall · 3m 06s compute
Cost $0.03
Tokens 7.5k in / 781 out

Per-workstream usage

  • go:implementation-tests — openai-codex/gpt-5.6-terra
    • In: 1.9k
    • Out: 402
    • Cache read: 23.0k
    • Cache create: 0
    • Cost: $0.01
    • Duration: 1m 27s
  • structure:repo-health — openai-codex/gpt-5.6-terra
    • In: 1.6k
    • Out: 342
    • Cache read: 28.2k
    • Cache create: 0
    • Cost: $0.01
    • Duration: 1m 37s
  • orchestrator-rollup — openai-codex/gpt-5.6-terra
    • In: 4.0k
    • Out: 37
    • Cache read: 0
    • Cache create: 0
    • Cost: $0.01
    • Duration: 2s

@zzwong
zzwong marked this pull request as ready for review September 9, 2026 22:52
@zzwong
zzwong merged commit fb9759a into main Sep 9, 2026
19 of 20 checks passed
@zzwong
zzwong deleted the zzwong/fix-reviewer-coverage-repair branch September 9, 2026 22:57
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.

Make reviewer coverage validation repair actionable without weakening approval gates

2 participants