feat(dashboard): retry a failed review call, review the result, then accept or discard it #116

Merged

rcsheets merged 2 commits from feat/dashboard-retry into main 2026-10-03 05:35:04 +00:00

Conversation 13 Commits 2 Files changed 24 +2830 -44

rcsheets commented 2026-10-02 06:19:06 +00:00

Owner

Copy link

An admin who finds a failed review on the dashboard can re-run the call that failed, read what comes back, and then choose whether it replaces the failure. Nothing changes on the record or the PR until the result has been seen and accepted.

What an admin sees

• Failed review page (/reviews/{id}): a Retry card with a "Retry this call" button. The comparison page's failed columns link to it.
• While it runs: the page reloads every 3s and offers "Stop waiting and discard".
• When it finishes: verdict, tokens, duration and the full body, marked "nothing has been changed yet", with "Replace the failure with this result" and "Discard it".
• PR checkbox: "Also rewrite the PR comment with this review", checked by default. Unchecking changes only the dashboard record.
• After accepting: the page says the result came from a retry and whether the PR comment was rewritten; the original failure is kept behind a disclosure.

How it works

• One model call is retried, not the review. The retry re-sends the exact prompt stored on the failed row (reviewer.ReplayPrompt) to the same model config, read as it stands at retry time.
• The dashboard only queues. review_retries (migration 000020) is a queue between the binaries; runner.RetryWorker in the webhook service makes the call and, on accept, writes the PR comment. The dashboard still holds no LLM or forge credentials.
• Accepting is one transaction (tracker.AcceptRetry): the failed row becomes completed, its review_passes row follows, and the retry row keeps the failure it replaced.
• The PR write goes through openReviewComment, so the one-comment rules apply unchanged.

Behavior worth knowing

• A failure with no stored prompt (liveness probe, diff fetch) can't be retried here; the control is shown disabled, labeled "No stored prompt".
• The PR checkbox is not offered if the PR has been reviewed since. The worker re-checks before writing and reports "skipped" with the reason rather than overwriting.
• Shadow reviews can be retried and accepted, but never touch the PR.
• One undecided retry per review; a double click lands on the open one.
• Work claimed by a service that then died is given up on by age, and a queued or running retry can be discarded.

Deploying

Both binaries need the new version. A retry requested while the webhook service is on the old one sits queued; the page says so after 30s and stops reloading after 10 minutes.

Testing

• go test -race ./... passes with Postgres attached and without.
• New Postgres-backed tests cover the store SQL (internal/tracker), the worker through its real loop against a fake forge and stub backend (internal/runner), and the dashboard routes (internal/dashboard, which gets its own database). The CI integration job now runs internal/dashboard too.
• Each behavior was broken in turn to confirm a test fails for it.
• Not exercised: the two binaries running together against a real forge and model.

Language

The second commit records en-US as the project's language in a new AGENTS.md (imported from CLAUDE.md) and respells this change to match.

🤖 Generated with Claude Code

An admin who finds a failed review on the dashboard can re-run the call that failed, read what comes back, and then choose whether it replaces the failure. Nothing changes on the record or the PR until the result has been seen and accepted. ## What an admin sees - **Failed review page** (`/reviews/{id}`): a Retry card with a "Retry this call" button. The comparison page's failed columns link to it. - **While it runs**: the page reloads every 3s and offers "Stop waiting and discard". - **When it finishes**: verdict, tokens, duration and the full body, marked "nothing has been changed yet", with "Replace the failure with this result" and "Discard it". - **PR checkbox**: "Also rewrite the PR comment with this review", checked by default. Unchecking changes only the dashboard record. - **After accepting**: the page says the result came from a retry and whether the PR comment was rewritten; the original failure is kept behind a disclosure. ## How it works - **One model call is retried, not the review.** The retry re-sends the exact prompt stored on the failed row (`reviewer.ReplayPrompt`) to the same model config, read as it stands at retry time. - **The dashboard only queues.** `review_retries` (migration `000020`) is a queue between the binaries; `runner.RetryWorker` in the webhook service makes the call and, on accept, writes the PR comment. The dashboard still holds no LLM or forge credentials. - **Accepting is one transaction** (`tracker.AcceptRetry`): the failed row becomes `completed`, its `review_passes` row follows, and the retry row keeps the failure it replaced. - **The PR write goes through `openReviewComment`**, so the one-comment rules apply unchanged. ## Behavior worth knowing - A failure with no stored prompt (liveness probe, diff fetch) can't be retried here; the control is shown disabled, labeled "No stored prompt". - The PR checkbox is not offered if the PR has been reviewed since. The worker re-checks before writing and reports "skipped" with the reason rather than overwriting. - Shadow reviews can be retried and accepted, but never touch the PR. - One undecided retry per review; a double click lands on the open one. - Work claimed by a service that then died is given up on by age, and a queued or running retry can be discarded. ## Deploying Both binaries need the new version. A retry requested while the webhook service is on the old one sits queued; the page says so after 30s and stops reloading after 10 minutes. ## Testing - `go test -race ./...` passes with Postgres attached and without. - New Postgres-backed tests cover the store SQL (`internal/tracker`), the worker through its real loop against a fake forge and stub backend (`internal/runner`), and the dashboard routes (`internal/dashboard`, which gets its own database). The CI `integration` job now runs `internal/dashboard` too. - Each behavior was broken in turn to confirm a test fails for it. - Not exercised: the two binaries running together against a real forge and model. ## Language The second commit records en-US as the project's language in a new `AGENTS.md` (imported from `CLAUDE.md`) and respells this change to match. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

rcsheets added 1 commit 2026-10-02 06:19:06 +00:00

feat(dashboard): retry a failed review call, review the result, then accept or discard it ... 35c284fc53

An admin who finds a failed review on the dashboard can now re-run the call
that failed, read what comes back, and choose whether it replaces the
failure. The three steps are separate: nothing changes on the record or the
PR until the result has been seen and accepted.

What is retried is one model call, not the review. A failed review_events
row keeps the exact prompt its call was given, so the retry re-sends that
(reviewer.ReplayPrompt) to the same model config and nothing else runs
again. A review that failed before a prompt existed has nothing to re-send;
the page shows the retry control disabled, labelled "No stored prompt". The
config is read as it stands at retry time, since a retry is usually pressed
after fixing it.

The dashboard still holds no LLM or forge credentials. review_retries
(migration 000020) is a queue between the two binaries: the dashboard
inserts a row, and runner.RetryWorker in the webhook service claims it,
makes the call, and writes the result back. A partial unique index allows
one undecided retry per review.

Accepting is one transaction: the review_events row takes the result and
becomes completed, the review call's review_passes row follows it, and the
retry keeps the failure it displaced. Discarding touches nothing.

Rewriting the PR comment is a separate step on the accept form, ticked by
default. It goes through openReviewComment, so the one-comment rules apply
unchanged. It is offered, and honoured, only for a primary review whose PR
has not been reviewed since (tracker.NewerReviewExists) — checked by the
dashboard before offering and again by the worker before writing. The
write holds the PR's in-flight slot through claimInflight, which refuses a
taken slot rather than displacing a running review.

Work claimed by a service that then died is given up on by age, and an
admin can discard a retry that is still queued or running; without both, a
retry left open would block every later one for that review.

internal/dashboard gains Postgres-backed tests for the retry routes, with
its own database, and the CI integration job now runs that package.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

pr-reviewer-bot commented 2026-10-02 06:19:07 +00:00

Collaborator

Copy link

The latest review failed.

Jump to the current review

Review history

• 2026-10-02 06:19 UTC · safety check · couldn't run · r-bf4cdb-4b2ea8
• 2026-10-02 06:22 UTC · safety check · couldn't run · r-bf4d91-9090aa
• 2026-10-02 06:28 UTC · safety check · failed · r-bf4ef3-fe6d15
• 2026-10-02 06:32 UTC · safety check · No verdict returned · r-bf501b-3b9715
• 2026-10-02 08:23 UTC · safety check · No verdict returned · r-bf6a13-2f5546
• 2026-10-02 09:33 UTC · safety check · failed · r-bf7a60-33bfc7
• 2026-10-02 22:54 UTC · safety check · LGTM · r-c03640-2c0107
• 2026-10-02 23:27 UTC · safety check · failed · r-c03df3-48a7aa · current

<!-- pr-reviewer:history --> **The latest review failed.** [Jump to the current review](#issuecomment-4839) <details><summary>Review history</summary> - 2026-10-02 06:19 UTC · safety check · couldn't run · `r-bf4cdb-4b2ea8` - 2026-10-02 06:22 UTC · safety check · couldn't run · `r-bf4d91-9090aa` - 2026-10-02 06:28 UTC · safety check · failed · `r-bf4ef3-fe6d15` - 2026-10-02 06:32 UTC · safety check · No verdict returned · `r-bf501b-3b9715` - 2026-10-02 08:23 UTC · safety check · No verdict returned · `r-bf6a13-2f5546` - 2026-10-02 09:33 UTC · safety check · failed · `r-bf7a60-33bfc7` - 2026-10-02 22:54 UTC · safety check · LGTM · `r-c03640-2c0107` - 2026-10-02 23:27 UTC · safety check · failed · `r-c03df3-48a7aa` · **current** </details>

rcsheets added 1 commit 2026-10-02 06:22:05 +00:00

doc: record en-US as the project's language, and respell the retry change to match ... 8fbcad3b49

The dashboard-retry change was written in British English: "honoured",
"cancelled", "uncancellable", "afterwards", and "ticked"/"untick" for the
checkbox, one of which is text an admin reads on the accept form. Those are
now en-US. No behavior changes; one test is renamed to match.

AGENTS.md is new and states the rule, covering usage as well as spelling.
CLAUDE.md imports it so the rule is loaded alongside the rest of the
project guidance.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

rcsheets commented 2026-10-02 06:25:15 +00:00

Author

Owner

Copy link

@pr-reviewer-bot retry

@pr-reviewer-bot retry

pr-reviewer-bot commented 2026-10-02 06:25:15 +00:00

Collaborator

Copy link

pr-reviewer v0.52.3 | tracking id r-bf4e4b-0b0331

There's nothing to retry — the most recent review attempt on this PR didn't fail. Try @pr-reviewer-bot review to request a fresh review.

*[pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.52.3 | tracking id `r-bf4e4b-0b0331`* There's nothing to retry — the most recent review attempt on this PR didn't fail. Try `@pr-reviewer-bot review` to request a fresh review.

rcsheets commented 2026-10-02 06:28:02 +00:00

Author

Owner

Copy link

@pr-reviewer-bot review

@pr-reviewer-bot review

pr-reviewer-bot commented 2026-10-02 06:28:04 +00:00

Collaborator

Copy link

Superseded — a newer review is further down this thread. This pass is kept on the dashboard as r-bf501b-3b9715.

<!-- pr-reviewer:superseded --> *Superseded — a newer review is further down this thread. This pass is kept on the dashboard as `r-bf501b-3b9715`.*

rcsheets referenced this pull request from a commit 2026-10-02 06:29:31 +00:00

fix(webhook): `retry` accepts a review that couldn't run, not only one that failed

rcsheets referenced this pull request 2026-10-02 06:29:44 +00:00

fix(webhook): retry accepts a review that couldn't run, not only one that failed #117

rcsheets commented 2026-10-02 06:32:57 +00:00

Author

Owner

Copy link

@pr-reviewer-bot review

@pr-reviewer-bot review

pr-reviewer-bot commented 2026-10-02 06:32:59 +00:00

Collaborator

Copy link

Superseded — a newer review is further down this thread. This pass is kept on the dashboard as r-bf6a13-2f5546.

<!-- pr-reviewer:superseded --> *Superseded — a newer review is further down this thread. This pass is kept on the dashboard as `r-bf6a13-2f5546`.*

rcsheets commented 2026-10-02 08:23:46 +00:00

Author

Owner

Copy link

@pr-reviewer-bot review

@pr-reviewer-bot review

pr-reviewer-bot commented 2026-10-02 08:23:48 +00:00

Collaborator

Copy link

Superseded — a newer review is further down this thread. This pass is kept on the dashboard as r-bf7a60-33bfc7.

<!-- pr-reviewer:superseded --> *Superseded — a newer review is further down this thread. This pass is kept on the dashboard as `r-bf7a60-33bfc7`.*

rcsheets commented 2026-10-02 09:33:19 +00:00

Author

Owner

Copy link

@pr-reviewer-bot review

@pr-reviewer-bot review

pr-reviewer-bot commented 2026-10-02 09:33:21 +00:00

Collaborator

Copy link

Superseded — a newer review is further down this thread. This pass is kept on the dashboard as r-c03640-2c0107.

<!-- pr-reviewer:superseded --> *Superseded — a newer review is further down this thread. This pass is kept on the dashboard as `r-c03640-2c0107`.*

rcsheets referenced this pull request 2026-10-02 19:49:42 +00:00

feat: per-config thinking switch and stored reasoning for gllm #118

rcsheets commented 2026-10-02 22:54:55 +00:00

Author

Owner

Copy link

@pr-reviewer-bot review

@pr-reviewer-bot review

pr-reviewer-bot commented 2026-10-02 22:54:57 +00:00

Collaborator

Copy link

Automated review by pr-reviewer v0.53.0 | Safety Check | Nemotron 3 Nano | tracking id r-c03df3-48a7aa
This is an AI-generated review and may contain mistakes.

Status: ❌ Failed

---

Review failed. Tracking id r-c03df3-48a7aa — see logs for details.

Comment @pr-reviewer-bot retry to try again.

<!-- pr-reviewer:review --> *Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.53.0 | Safety Check | Nemotron 3 Nano | tracking id `r-c03df3-48a7aa`* *This is an AI-generated review and may contain mistakes.* **Status:** ❌ Failed --- Review failed. Tracking id `r-c03df3-48a7aa` — see logs for details. Comment `@pr-reviewer-bot retry` to try again.

rcsheets force-pushed feat/dashboard-retry from 8fbcad3b49 to 22b156e1ad 2026-10-02 23:27:44 +00:00 Compare

rcsheets merged commit 1de6c69195 into main 2026-10-03 05:35:04 +00:00

rcsheets deleted branch feat/dashboard-retry 2026-10-03 05:35:04 +00:00

rcsheets referenced this pull request from a commit 2026-10-03 05:35:05 +00:00

Merge pull request 'feat(dashboard): retry a failed review call, review the result, then accept or discard it' (#116) from feat/dashboard-retry into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

brooktrails/pr-reviewer!116

No description provided.