Repository navigation
chore: sunset the completer reviewer and verifier - #81
Conversation
All lab repos now run the agent-session path (reviewer + verifier), so the one-shot completer is deleted — the "never two implementations of one role" cut. Deleted whole: advisory-review.yml, verify.yml, review_cli.py, verifier_cli.py, llm.py (AnthropicCompleter), test_clis.py, test_llm.py, and the anthropic dependency (+ the `review` extra; workflows drop `--extra review`; uv.lock regenerated, -anthropic and transitive deps). Removed the completer-only surface from shared modules, keeping everything the agent path reuses: review.Completer + review.review() gone (build_prompt, result_from_data, format_review, SYSTEM_PROMPT stay); verifier.verify() gone (build_verify_prompt, verify_result_from_data, gather_thread, format_verify_comment stay). Tests: the shared rendering/parsing/sanitization tests keep coverage via a small test-local shim for the deleted completer entry (its non-skip path was just result_from_data over the model payload); the completer-CLI-only tests (gather_ruler/_ruler_paths, the vcli.main integration) are deleted, with a direct verifier.gather_thread test kept. 511 green, ruff + mypy + lock clean; agent CLIs import on base deps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Round 1 — reviewed head a7e7b693.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 4 advisory notes.
Advisory (non-blocking):
- Install docs still point at the deleted advisory-review.yml reusable workflow (
docs/install.md:36; high) - Deleting test_clis.py wholesale also removes the only contract_cli tests (
tests/test_clis.py; high) - Comments and README still describe the removed completer as present (
.github/workflows/review.yml:4; high) - pick_context_files and MAX_CONTEXT_FILES no longer have a production caller (
src/autoresearch/review.py:203; medium)
The test shims added to tests/test_review.py and tests/test_verifier.py pass the real SYSTEM_PROMPT/schema constants, and the agent path keeps its own skip tests (tests/test_review_agent.py:149, tests/test_verify_agent.py:116), so I found no skip-gate coverage lost there. The three deleted verifier_cli tests (_ruler_paths, gather_ruler, module-budget) covered code deleted in the same diff. Dropping --extra review looks safe: no module under src/ imports anthropic or httpx. I cannot verify the claims about the other lab repos' migration state, the retired org secrets, or the "511 tests green" run from the provided context.
Update the completer-era sections to history: the model seam is the Harness (not a Completer interface), both judges are agent sessions, and the Security section now describes the actual single-step containment (same-repo gate + read-only + claude-only auto for the proc reason) with the tokenless split named as the not-yet-built fork-PR-phase work. Sequencing items marked done/pending/superseded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Advisory findings (all real, caught by the agent reviewer): - recover the contract_cli tests (the deleted test_clis held them too) into tests/test_contract_cli.py - remove pick_context_files + MAX_CONTEXT_* (no caller after the completer went) and their now-orphaned tests; context_files stays (build_prompt still renders it, the fence-forging tests keep coverage) - install.md, README, review.yml comment: describe the agent path, not the removed completer Docs simplified per feedback: reviewer-infra.md states the current design without the migration trace (no strikethroughs, no completer-history parentheticals); same trim on architecture/consolidation/public-surface. Legacy secret references updated to ANTHROPIC_* (the org + yolo-jepa REVIEWER_API_KEY / VERIFIER_API_KEY secrets were deleted). 511 green, ruff + mypy + lock clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both addressed in
(Also folded in the 4 advisory findings from the round-1 review: recovered the |
Hermes runs on OpenRouter (an aggregator), so 'third-party aggregators deferred' and 'pilot on one subscription harness / Anthropic as the pilot' were stale. State the current set: Claude Code, Codex, hermes-agent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Round 2 — reviewed head c9ee1556.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 4 advisory notes.
1 finding attached to the lines below.
Advisory (non-blocking):
- Deleting the reusable workflows red-lines any caller still pinned to @main (
.github/workflows/advisory-review.yml; medium) - The explicit re-request override for bot PRs loses its only production wiring (
src/autoresearch/review_agent_cli.py:92; medium) - Roadmap still names the retired VERIFIER_API_KEY as the verifier deploy step (
docs/roadmap.md:154; low)
I checked the whole tree for dangling references to the deleted modules: nothing in src/ imports review_cli, verifier_cli, autoresearch.llm, pick_context_files or MAX_CONTEXT_FILES, no source or test imports anthropic/httpx, and uv.lock has no leftover references to the ten packages it drops, so the delete looks internally consistent. The test-local review()/verify() shims keep the parsing/rendering assertions pointed at result_from_data/verify_result_from_data plus the real schema and system-prompt constants, and roles.py + test_role_runner still pin those constants to the agent path, so I found no coverage that only passes because the shim exists. install.md's surviving line "A comment appears within a minute or two" now describes an agent session with a 70-minute timeout; that is wording, so I did not raise it. Whether other repos or forks still call the deleted reusable workflows at @main is not verifiable from this checkout.
Removing the completer left two things dead that nothing populated or wired anymore; remove them and their tests in the same PR: - `PullRequest.context_files` + its prompt rendering in `build_prompt` and `build_verify_prompt`. The completer's context-gathering was the only producer; the agent path reads the checkout directly. - The explicit re-request override for bot PRs. Only the completer workflow set `REVIEW_EXPLICIT_REQUEST`, so `review_agent_cli`'s read of it, `run_agent_review(explicit=...)`, `skip_reason`'s `explicit_request` param, and the bot-PR issue-comment branch were all dead. The advisory reviewer now never reviews bot-authored PRs. Docstrings/comments that described those paths updated to state the current design. 505 tests green, mypy and ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Round-2 notes addressed in
Not in this PR (tracked separately):
505 tests green, mypy and ruff clean. |
Round-3 self-review: the sunset deleted the completer code but left
~15 docstrings and comments across the tree still narrating "the
completer path" as a live parallel path — three of them now false
("both the completer path and the agent path...", "the completer path
enforces the schema"). Reword them to state what the code IS now, one
agent path, dropping the migration trace:
- review.py / verifier.py: result_from_data and verify_result_from_data
docstrings (no "both paths"); AGENT_INVESTIGATION / AGENT_VERIFY
lead-ins; build_agent_brief.
- verify_agent.py + the two agent CLIs: module docstrings.
- posting.py / roles.py / test_posting.py / test_verifier.py: dropped
"the completer path" contrasts and the "sunset completer" shim notes.
- workflows: "not the completer's effort knob" -> "not a per-call effort
knob"; verify-agent constitution no longer contrasts the completer;
review.yml/install.md label comment states the manual-re-review intent.
verify-agent.yml's re-request gate is left as is: the verifier's
re-request label is a live, separate mechanism (re-verify a bot PR,
applied by a non-author), not the reviewer's removed override.
Design docs that describe the broader kernel consolidation
(agent-substrate.md, consolidation.md) are left for the consolidation-doc
pass. No behavior change; 505 tests green, mypy and ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The one-cut completer sunset. All four lab repos now run the agent-session reviewer and verifier (jepa-agent #7, egolearn #5 just merged; autoresearch + yolo-jepa already there), so the completer path has zero callers and comes out clean — the "never two implementations of one role" cut.
Deleted whole
.github/workflows/advisory-review.yml,.github/workflows/verify.yml(completer reusables)src/autoresearch/review_cli.py,src/autoresearch/verifier_cli.py(completer entry points)src/autoresearch/llm.py(AnthropicCompleter)tests/test_clis.py,tests/test_llm.pyanthropicdependency + thereviewoptional extra; the three agent workflows drop--extra review;uv.lockregeneratedRemoved from shared modules (kept what the agent path reuses)
review.py:Completerprotocol +review()gone. Kept:build_prompt,result_from_data,format_review/format_comment,SYSTEM_PROMPT, the PR/finding/result types.verifier.py:verify()gone. Kept:build_verify_prompt,verify_result_from_data,gather_thread,format_verify_comment.Tests
The many rendering/parsing/sanitization tests that drove behavior through the completer entry keep their coverage via a small test-local shim for the deleted
review()/verify()— their non-skip path was justresult_from_data/verify_result_from_dataover the model payload, so the shared assertions are unchanged. The genuinely completer-CLI-only tests (gather_ruler/_ruler_paths, thevcli.mainthread-gate integration) are deleted; a directverifier.gather_threadtest is kept for that coverage.511 tests green, ruff + mypy + lock clean; agent CLIs import on base deps.
Note:
reviewer-infra.mdhad its current-state claims corrected (both judges are agents; completer removed), but its deeper design sections still read as completer-era history — a fuller pass could follow. Retiring the legacy org secretsREVIEWER_API_KEY/VERIFIER_API_KEYis a separate op (not code).🤖 Generated with Claude Code