Verify findings against full file before posting #82

New issue

Closed

opened 2026-10-15 19:45:25 +00:00 by agent-pi · 1 comment

agent-pi commented

2026-10-15 19:45:25 +00:00

Collaborator

Reviewers see only diff hunks, so findings regularly contain false premises — claims contradicted by code or docs outside the reviewed hunks.

Evidence from #75: "count says six but only four backends documented" (the missing sections sat outside the diff); "no positive test exists" (it did, outside the hunk); "base_url/model are not validated" (they were, in unchanged code).

A verification pass — checking each candidate finding against the full file (or wider repo context) before it is posted — would drop findings that do not survive contact and spare the author round-trips.

Reviewers see only diff hunks, so findings regularly contain false premises — claims contradicted by code or docs outside the reviewed hunks. Evidence from #75: "count says six but only four backends documented" (the missing sections sat outside the diff); "no positive test exists" (it did, outside the hunk); "base_url/model are not validated" (they were, in unchanged code). A verification pass — checking each candidate finding against the full file (or wider repo context) before it is posted — would drop findings that do not survive contact and spare the author round-trips.

agent-pi added this to the Review quality & convergence milestone

2026-10-15 19:45:25 +00:00

agent-pi added the

Kind/Enhancement

Priority

Low

labels

2026-10-15 19:45:25 +00:00

agent-pi commented

2026-10-16 08:38:56 +00:00

Author

Collaborator

Plan of attack (design agreed in session; spec: wave-1 review quality)

Exploration findings:

Backend.complete() already takes a per-call model (all three backends honor it, defaulting when None) — so the verifier can use the producing agent model with no new knob (decided: no WUMING_VERIFY_MODEL).
429/5xx retry with backoff already lives inside every backend via with_retry — verification inherits rate-limit resilience for free; the pass only needs to bound its own fan-out.
ForgejoClient.get_file_content() returns None on 404; FileDiff.full_content already carries attached file contents that can seed the verification cache.

Approach: new src/wuming/verify.py:

parse_verdict() — strict parse of {"verdict": "confirmed"|"rejected", "reason": str}; anything else → keep.
verify_finding() — one call per finding, fail-open: only an affirmative rejected drops; garbage/exception/timeout keep the finding with a logged warning.
fetch_file_contents() — lazy per-file fetch, cached, seeded from attached full_content, oversize guard (2000-line module constant) skips verification for huge files.
build_model_map() — agent → model from WUMING_AGENTS (last-wins; unknown → backend default).
verify_comments() — bounded concurrency (semaphore, 4) so a 50–200-finding PR cannot trigger a rate-limit storm; fail-open otherwise turns the pass into a silent no-op exactly when needed.
WUMING_VERIFY config flag, default on, fail-closed parser (typo → ValueError).
Wired into _main() after position resolution — free filters always run before LLM spend. Scope note: the verifier checks claims against the changed file only; claims about other files are out of scope by design.

Branch: feat/issue-82-verify-findings
Commits: :sparkles: verdict parsing → :sparkles: fail-open verify call → :sparkles: file sourcing → :sparkles: model map → :sparkles: orchestration → :sparkles: config flag → :sparkles: pipeline wiring → :memo: docs
PR: targets main, Closes #82

**Plan of attack** (design agreed in session; spec: wave-1 review quality) **Exploration findings:** - `Backend.complete()` already takes a per-call `model` (all three backends honor it, defaulting when `None`) — so the verifier can use the producing agent model with no new knob (decided: no `WUMING_VERIFY_MODEL`). - 429/5xx retry with backoff already lives inside every backend via `with_retry` — verification inherits rate-limit resilience for free; the pass only needs to bound its own fan-out. - `ForgejoClient.get_file_content()` returns `None` on 404; `FileDiff.full_content` already carries attached file contents that can seed the verification cache. **Approach:** new `src/wuming/verify.py`: 1. `parse_verdict()` — strict parse of `{"verdict": "confirmed"|"rejected", "reason": str}`; anything else → keep. 2. `verify_finding()` — one call per finding, **fail-open**: only an affirmative `rejected` drops; garbage/exception/timeout keep the finding with a logged warning. 3. `fetch_file_contents()` — lazy per-file fetch, cached, seeded from attached `full_content`, oversize guard (2000-line module constant) skips verification for huge files. 4. `build_model_map()` — agent → model from `WUMING_AGENTS` (last-wins; unknown → backend default). 5. `verify_comments()` — bounded concurrency (semaphore, 4) so a 50–200-finding PR cannot trigger a rate-limit storm; fail-open otherwise turns the pass into a silent no-op exactly when needed. 6. `WUMING_VERIFY` config flag, default **on**, fail-closed parser (typo → `ValueError`). 7. Wired into `_main()` after position resolution — free filters always run before LLM spend. Scope note: the verifier checks claims against the **changed file only**; claims about other files are out of scope by design. **Branch:** `feat/issue-82-verify-findings` **Commits:** `:sparkles:` verdict parsing → `:sparkles:` fail-open verify call → `:sparkles:` file sourcing → `:sparkles:` model map → `:sparkles:` orchestration → `:sparkles:` config flag → `:sparkles:` pipeline wiring → `:memo:` docs **PR:** targets `main`, `Closes #82`

agent-pi referenced this issue from a pull request that will close it,

2026-10-16 09:24:31 +00:00

Verify findings against full file content before posting #102

agent-pi closed this issue

2026-10-16 20:59:57 +00:00

agent-pi referenced this issue

2026-10-16 21:03:38 +00:00

Deferred verification minors: oversize guard, model pin, config tests #103

agent-pi referenced this issue

2026-10-16 21:20:42 +00:00

Add WUMING_MIN_SEVERITY severity floor #104