Skip reviews of snapshots with no diff
This commit is contained in:
+1
-1
@@ -238,7 +238,7 @@ The fork's session guidance gets one more line (`forkGuidance` in `internal/agen
|
|||||||
|
|
||||||
## Reviews
|
## Reviews
|
||||||
|
|
||||||
Every new snapshot of a task gets a review: a separate agent session on the vendor the task's agent doesn't use reads the change and reports on it (`internal/agents/review.go`, `internal/store/review.go`). A turn that changes no files makes no snapshot and so no review. Nothing else starts a review, and nothing waits for one: reviews ride along for the human to look at when they want.
|
Every new snapshot of a task gets a review: a separate agent session on the vendor the task's agent doesn't use reads the change and reports on it (`internal/agents/review.go`, `internal/store/review.go`). A turn that changes no files makes no snapshot and so no review, and a snapshot whose change (the diff below) has no files, such as a task's first snapshot taken before its agent changed anything, gets no review either. Nothing else starts a review, and nothing waits for one: reviews ride along for the human to look at when they want.
|
||||||
|
|
||||||
- **Starting.** After `takeSnapshot` creates a snapshot, `afterSnapshot` runs `startReviewer`: it picks the vendor opposite the task's latest implementer session (`reviewVendor`) and that vendor's model in `reviewModels`, checks the snapshot's commit out detached at `worktrees/<project-id>/review-<review-id>`, creates an `agent_sessions` row with `role` `reviewer` on the task (created by the task's creator), a `reviews` row (`running`, one per snapshot), and sends `reviewPrompt` as its first turn (actor `byre`): the change as `git diff` from the previous snapshot (the snapshot commit's parent for the first) with its changed files, the merge-base diff of all the task's work for context, and the task's first message to its agent.
|
- **Starting.** After `takeSnapshot` creates a snapshot, `afterSnapshot` runs `startReviewer`: it picks the vendor opposite the task's latest implementer session (`reviewVendor`) and that vendor's model in `reviewModels`, checks the snapshot's commit out detached at `worktrees/<project-id>/review-<review-id>`, creates an `agent_sessions` row with `role` `reviewer` on the task (created by the task's creator), a `reviews` row (`running`, one per snapshot), and sends `reviewPrompt` as its first turn (actor `byre`): the change as `git diff` from the previous snapshot (the snapshot commit's parent for the first) with its changed files, the merge-base diff of all the task's work for context, and the task's first message to its agent.
|
||||||
- **Running.** Reviewer sessions get `reviewGuidance` and only the `submit_review` tool, whose arguments (`reviewSchema`: a Markdown `summary` and `findings`, each with `severity`, `category`, `title`, `detail`, `path` and `line`) become the review's `result` and mark it `done`. They run without the Stop-hook bounce, mid-turn delivery, end-of-turn commit, snapshots or reports, and their `user.message` events carry no `head`. When the reviewer's turn ends byre completes its session, kills its process and removes the worktree; a review still `running` then fails with why (no submission, with the final message; an error; an interrupt). A process that exits mid-turn fails the review the same way. Merging or discarding the task fails its running reviews ("the task was closed") and ends their reviewers. On startup, a running reviewer resumes like any session, and a running review whose reviewer isn't `running` fails.
|
- **Running.** Reviewer sessions get `reviewGuidance` and only the `submit_review` tool, whose arguments (`reviewSchema`: a Markdown `summary` and `findings`, each with `severity`, `category`, `title`, `detail`, `path` and `line`) become the review's `result` and mark it `done`. They run without the Stop-hook bounce, mid-turn delivery, end-of-turn commit, snapshots or reports, and their `user.message` events carry no `head`. When the reviewer's turn ends byre completes its session, kills its process and removes the worktree; a review still `running` then fails with why (no submission, with the final message; an error; an interrupt). A process that exits mid-turn fails the review the same way. Merging or discarding the task fails its running reviews ("the task was closed") and ends their reviewers. On startup, a running reviewer resumes like any session, and a running review whose reviewer isn't `running` fails.
|
||||||
|
|||||||
@@ -108,6 +108,10 @@ func (m *Manager) startReviewer(ctx context.Context, task api.Task, snap api.Sna
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
if prompt == "" {
|
||||||
|
slog.Info("review skipped, snapshot has no diff", "task", task.Id, "snapshot", snap.N)
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
reviewID := store.NewID()
|
reviewID := store.NewID()
|
||||||
dir := m.reviewDir(project.Id, reviewID)
|
dir := m.reviewDir(project.Id, reviewID)
|
||||||
@@ -177,6 +181,9 @@ func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.P
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
if len(changes.Files) == 0 {
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
whole, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
|
whole, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
|
|||||||
Reference in New Issue
Block a user