Skip to content

ci: dismantle the opus-5 advisory reviewer, keep terra - #130

Merged
renmengye merged 1 commit into
mainfrom
ci/dismantle-opus5-reviewer
Aug 23, 2026
Merged

renmengye merged 1 commit into
mainfrom
ci/dismantle-opus5-reviewer

Conversation

@renmengye

Copy link
Copy Markdown
Member

Removes the claude-opus-5 advisory reviewer job from review.yml; terra (hermes/openai-direct, gpt-5.6-terra) becomes the sole CI reviewer.

Why: on #129 the opus-5 reviewer never converged — unbounded nitpick rounds, re-raising already-resolved items — while terra caught the real bugs early and settled after a couple of rounds. Chasing opus-5 to "quiet" was a token sink and a disservice.

The advisory-review-agent reusable still supports the claude backend for other callers; only this repo's use of it is removed. No required checks reference the review jobs (only ci is required), so the job rename (advisory/second-opinion → review) breaks no gate.

Self-reviewed: YAML valid, no dangling refs, docs are generic examples (not stale mirrors).

The opus-5 advisory reviewer did not converge — on #129 it produced unbounded
nitpick rounds and re-raised already-resolved items (a token sink), while terra
(the second opinion) caught the real issues early and settled. Remove the
claude-opus-5 job; terra (hermes/openai-direct, gpt-5.6-terra) becomes the sole
CI reviewer. The advisory-review-agent reusable still supports the claude backend
for other callers — only this repo's use of it is removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

The advisory review (second opinion — terra) round could not run — the model API refused the request (RuntimeError: invalid structured output: final message is not valid JSON: Extra data: line 1 column 44 (char 43)). Treat this as an outage, not a clean read; re-add the review label to re-request once the API recovers.

@github-actions github-actions Bot 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.

Round 1 — reviewed head e1b5acca — reviewer claude/claude-opus-5.

Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.

Verdict: nothing blocking — 3 advisory notes.

1 finding attached to the lines below.

Advisory (non-blocking):

  • The claude branch of the reusable is no longer exercised by this repo's own PRs (.github/workflows/review.yml:19; low)
  • Sole remaining reviewer runs the backend the docs call not token-safe (.github/workflows/review.yml:22; low)

The workflow YAML parses as valid and the job rename does not affect the reusable: artifact name and concurrency group key on opinion_id || backend (advisory-review-agent.yml lines 115, 236, 276), not the caller's job id. opinion_label is display-only (review_post_cli.py lines 86-105), so shortening it to "terra" changes no logic. Whether any branch protection required check names the old advisory/second-opinion jobs cannot be verified from the checkout; that claim in the description rests on repo settings I cannot see.

Comment thread .github/workflows/review.yml

@github-actions github-actions Bot 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.

Round 1 — reviewed head e1b5acca — reviewer hermes/gpt-5.6-terra.

second opinion — terra
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.

Verdict: no defects found.

No defects found in the diff.

@github-actions github-actions Bot 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.

Round 2 (re-run on the same head) — reviewed head e1b5acca — reviewer claude/claude-opus-5.

Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.

Verdict: nothing blocking — 3 advisory notes.

1 finding attached to the lines below.

Advisory (non-blocking):

  • The remaining reviewer is now a single point of failure (.github/workflows/review.yml:15; medium)
  • Design doc still says the advisory reviewer runs on claude-opus-5 (docs/design/scaling.md:257; low)

I verified the PR's claims against the checked-out tree: review.yml parses as one job with valid inputs for advisory-review-agent.yml; opinion_label is display-only (review_post_cli.py:100-105) and round counting keys off reviewed_by, not the label (posting.py:79-84), so shortening the label to "terra" changes no logic; artifact name and concurrency group both key off opinion_id || backend = "hermes", unchanged, and the removed claude job's artifact name no longer collides; no test, script, or source file references the advisory/second-opinion job names; scripts/setup_branch_protection.sh requires only the ci check, matching the claim that no gate breaks; docs/install.md is a generic template, not a mirror of this file; ANTHROPIC_REVIEWER_KEY is still referenced by review-agent.yml, so no dangling secret cleanup is implied. The actual branch-protection settings on GitHub cannot be read from this checkout, so the "no required checks reference the review jobs" claim is verified only against the script that sets them. I found no correctness, security, or resource defect in the diff.

Comment thread .github/workflows/review.yml
@renmengye
renmengye merged commit 60b676e into main Aug 23, 2026
9 checks passed
@renmengye
renmengye deleted the ci/dismantle-opus5-reviewer branch August 23, 2026 23:51
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.

1 participant