feat(review): inject authoritative scope + diff into reviewer and judge prompts · Entire

feat(review): inject authoritative scope + diff into reviewer and judge prompts

a598a11 · peyton-alt · 1w ago · 11 files · +577 added / -7 removed

The parent computes the review scope (commits, files, uncommitted, diff) for its banner, then discards it and tells each child agent only how to re-derive it. That cost minutes of setup per reviewer and let one agent diff a behind-main branch in the wrong direction, reporting mainline evolution as branch regressions — two of four high findings in a real run were such phantoms, and the judge consolidated them unchecked.

ComposeReviewPrompt now renders a parent-computed ScopeContext: commit list (oldest first), three-dot name-status file list, porcelain uncommitted lines, and the diff itself when it fits a 48KiB inline budget (beyond that, the exact three-dot command). The judge prompt gets the authoritative changed-file list with an instruction to discard findings outside it. Lists are capped with truncation flags so agents never mistake a bounded enumeration for an exhaustive one.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

ca5fa66e3b98 View transcript

Changes

11

    1039 unmodified lines
    1040
    1041
    1042
    1043
    1044
    1045
    1046
    1 unmodified line
    1048
    1049
    1050
    1051
    1052
    1053
    1054
    118 unmodified lines
    1173
    1174
    1175
    1176
    1177
    1178
    1179
    24 unmodified lines
    1204
    1205
    1206
    1207
    1208
    1209
    1210
    14 unmodified lines
    1225
    1226
    1227
    1228
    1229
    1230
    1231
    64 unmodified lines
    1296
    1297
    1298
    1299
    1300
    1301
    1302
    1303
    1304
    1305
    1306
    1307
    1308
    1309
    1310
    1311
    1312
    1313
    1314
    1315
    1316
    22 unmodified lines
    1339
    1340
    1341
    1342
    1343
    1344
    1345
    1346
    1347
    24 unmodified lines
    1372
    1373
    1374
    1375
    1376
    1377
    1378
    23 unmodified lines
    1402
    1403
    1404
    1405
    1406
    1407
    1408
    
    1039 unmodified lines

if deps.ReviewCheckpointContext != nil {
        checkpointContext = deps.ReviewCheckpointContext(ctx, worktreeRoot, scopeBaseRef)
    }
    scopeCtx := buildScopeContextBestEffort(ctx, worktreeRoot, scopeBaseRef)

runCfg := reviewtypes.RunConfig{
        ProfileName:       profileName,
        PerRunPrompt:      perRunPrompt,
        ScopeBaseRef:      scopeBaseRef,
        CheckpointContext: checkpointContext,
        ScopeContext:      scopeCtx,
        StartingSHA:       headSHA,
        ReviewerTimeout:   timeout,
    }
    118 unmodified lines

if deps.ReviewCheckpointContext != nil {
        checkpointContext = deps.ReviewCheckpointContext(ctx, worktreeRoot, scopeBaseRef)
    }
    scopeCtx := buildScopeContextBestEffort(ctx, worktreeRoot, scopeBaseRef)
    reviewers := make([]reviewtypes.AgentReviewer, 0, len(launchableEligible))
    for _, choice := range launchableEligible {
        workerName := choice.Name
        24 unmodified lines

PerRunPrompt:      perRunPrompt,
                ScopeBaseRef:      scopeBaseRef,
                CheckpointContext: checkpointContext,
                ScopeContext:      scopeCtx,
                StartingSHA:       headSHA,
            }, agentCfg),
        })
    }

var synthProvider SynthesisProvider = AgentSynthesisProvider{AgentName: judge.agent, Model: judge.model}
    masterLabel := judgeLabel(judge)
    sinks := composeMultiAgentSinks(multiAgentSinkInputs{
        scope:             scopeCtx,
        out:               out,
        isTTY:             interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively(),
        agentNames:        agentNames,
    })

return silentErr(pickErr)
    // buildScopeContextBestEffort wraps BuildScopeContext for the launch paths:
    // scope enumeration is prompt enrichment, so a failure (odd repo state, git
    // error) degrades to the descriptive scope clause instead of failing the run.
    func buildScopeContextBestEffort(ctx context.Context, worktreeRoot, scopeBaseRef string) reviewtypes.ScopeContext {
        if scopeBaseRef == "" {
            return reviewtypes.ScopeContext{}
        }
        sc, err := BuildScopeContext(ctx, worktreeRoot, scopeBaseRef)
        if err != nil {
            logging.Debug(ctx, "review scope context unavailable", slog.String("error", err.Error()))
            return reviewtypes.ScopeContext{}
        }
        return sc
    }

// multiAgentSinkInputs collects the parameters composeMultiAgentSinks needs.
    // It exists so tests can drive the helper with an explicit isTTY value
    // instead of monkey-patching interactive helpers at run time.
    22 unmodified lines

judgeTimeout      time.Duration
    onSynthesisResult func(result string)
    onSynthesisError  func(err error)
    // scope is the parent-computed authoritative scope, threaded to the
    // judge so consolidation can discard out-of-scope findings.
    scope reviewtypes.ScopeContext
}

type singleAgentSinkInputs struct {
    24 unmodified lines

sinks = append(sinks, DumpSink{W: postRunOut})
            sinks = append(sinks, SynthesisSink{
                Provider:        in.synthesisProvider,
                Scope:           in.scope,
                Writer:          postRunOut,
                RenderWriter:    in.out,
                PerRunPrompt:    in.perRunPrompt,
                ProfileName:     in.profileName,
            })
    }
    
    Mcmd/entire/cli/review/cmd.go +25
    
    ```
    3 unmodified lines

4
5
6
7
8
9
10
1209 unmodified lines

1220
1221
1222
1223
1224
1225
1226
1227
1228
1229
1230
1231
1232
1233
1234
1235
1236
1237
1238
1239
1240
1241
1242
1243
1244
1245
1246
1247
1248
1249
1250
1251
1252
1253
1254
1255
1256
1257
1258
1259
1260
1261
1262
1263
1264
1265
1266
1267
1268
1269
1270
1271
1272
1273
1274
1275
1276
1277
1278
1279
1280
1281
1282
1283
1284
1285
1286
1287
1288
1289
1290
1291
1292
1293
1294
1295
1296
1297
1298
1299
1300
1301
1302
1303
1304
1305
1306
1307
1308
1309
1310
1311
1312

3 unmodified lines

"bytes"
    "context"
    "errors"
    "os"
    "strings"
    "testing"
    "time"
1209 unmodified lines

t.Fatal("auto synthesis should notify the TUI when the final judge starts/completes")
}

// TestRunReview_InjectsScopeContext verifies the single-agent path hands the
// spawned reviewer the parent-computed scope enumeration (commits, files,
// uncommitted, inline diff) so the child never re-derives review scope.
func TestRunReview_InjectsScopeContext(t *testing.T) {
    setupCmdTestRepo(t)
    cwd, err := os.Getwd()
    if err != nil {
        t.Fatal(err)
    }
    testutil.GitCheckoutNewBranch(t, cwd, "feat/scope-ctx")
    testutil.WriteFile(t, cwd, "changed.go", "package changed")
    testutil.GitAdd(t, cwd, "changed.go")
    testutil.GitCommit(t, cwd, "add changed.go")
    testutil.WriteFile(t, cwd, "dirty.txt", "wip")

if err := seedReviewConfig(context.Background(), map[string]settings.ReviewConfig{
        testAgentName: {Skills: []string{"/review"}},
    }); err != nil {
        t.Fatal(err)
    }

reviewer := &captureRunConfigReviewer{name: testAgentName}
    deps := review.Deps{
        GetAgentsWithHooksInstalled: func(_ context.Context) []types.AgentName {
            return []types.AgentName{testAgentName}
        },
        NewSilentError: func(err error) error { return err },
        HeadHasReviewCheckpoint: func(_ context.Context) (bool, string) {
            return false, ""
        },
        ReviewerFor: func(agentName string) reviewtypes.AgentReviewer {
            if agentName == testAgentName {
                return reviewer
            }
            return nil
        },
    }

out := &bytes.Buffer{}
    cmd := review.NewCommand(deps)
    cmd.SetOut(out)
    cmd.SetErr(&bytes.Buffer{})
    cmd.SetArgs([]string{"general"})

if err := cmd.Execute(); err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if !reviewer.called {
        t.Fatal("reviewer was not started")
    }

sc := reviewer.got.ScopeContext
    if len(sc.Commits) != 1 || !strings.HasSuffix(sc.Commits[0], "add changed.go") {
        t.Errorf("ScopeContext.Commits = %v, want the branch commit", sc.Commits)
    }
    if len(sc.Files) != 1 || sc.Files[0] != "A\tchanged.go" {
        t.Errorf("ScopeContext.Files = %v, want [A\tchanged.go]", sc.Files)
    }
    if len(sc.Uncommitted) != 1 || !strings.Contains(sc.Uncommitted[0], "dirty.txt") {
        t.Errorf("ScopeContext.Uncommitted = %v, want dirty.txt porcelain line", sc.Uncommitted)
    }
    if !strings.Contains(sc.Diff, "+package changed") {
        t.Errorf("ScopeContext.Diff missing committed content:\n%s", sc.Diff)
    }
}

// TestComposeMultiAgentSinks_ThreadsScopeToSynthesisSink verifies the judge
// sink receives the scope so out-of-scope findings can be discarded during
// consolidation.
func TestComposeMultiAgentSinks_ThreadsScopeToSynthesisSink(t *testing.T) {
    t.Parallel()
    scope := reviewtypes.ScopeContext{Files: []string{"M\tcmd/foo.go"}}
    sinks := review.ExposedComposeMultiAgentSinks(review.SinkComposeInputs{
        Out:               &bytes.Buffer{},
        IsTTY:             false,
        AgentNames:        []string{"a", "b"},
        SynthesisProvider: &stubSynthesisProvider{},
        Scope:             scope,
    })
    for _, s := range sinks {
        if syn, ok := s.(review.SynthesisSink); ok {
            if len(syn.Scope.Files) != 1 || syn.Scope.Files[0] != "M\tcmd/foo.go" {
                t.Errorf("SynthesisSink.Scope = %+v, want threaded scope", syn.Scope)
            }
            return
        }
    }
    t.Fatal("no SynthesisSink composed")
}