inspect: resolve the judge once and pass it into runMultiAgentPath

837549d→main·

dipree·1mo ago·1 file·+8 added/-11 removed

Addresses a Trail review note about runMultiAgentPath re-deriving the judge and assuming success. The described crash couldn't occur (the provider assignment was guarded by the ok check, so it stayed nil, and composeMultiAgentSinks skips synthesis on a nil provider), but the validate-then-re-derive-and-assume pattern was fragile.

runReview now resolves the judge once, errors if none resolves, and passes the validated judgeSpec into runMultiAgentPath, which uses it directly. Single source of truth; no second derivation, no nil case.

Sessions

5ad95833bb48View transcript

?\Checkout the hand off doc that I just added.Pi·Opus 4.8·1 step

Changes

1

• cmd/entire/cli/review

  • Mcmd.go+8/-11

787 unmodified lines

788
789
790
791
791
792
793
794
795
796
797
797
798
799
800
801
221 unmodified lines

1023
1024
1025
1026
1027
1028
1029
72 unmodified lines

1102
1103
1104
1103
1104
1105
1106
1107
1108
1109
1110
1111
1105
1106
1107
1108
1109
1110
1111

787 unmodified lines

// can't actually write a verdict (no text generation) is tolerated here and
        // handled at synthesis time, where it fails gracefully ("final report
        // unavailable").
        if _, ok := resolveJudge(ctx, profile); !ok {
            judge, ok := resolveJudge(ctx, profile)
            if !ok {
                cmd.SilenceUsage = true
                err := fmt.Errorf("review profile %q has multiple inspectors but no judge that can write a verdict; set review_profiles.%s.judge", profileName, profileName)
                fmt.Fprintln(cmd.ErrOrStderr(), err.Error())
                return silentErr(err)
            }
            return runMultiAgentPath(ctx, cmd, profileName, profile, launchableEligible, baseOverride, perRunPrompt, deps, out)
            return runMultiAgentPath(ctx, cmd, profileName, profile, launchableEligible, judge, baseOverride, perRunPrompt, deps, out)
        }
    }
}

221 unmodified lines

profileName string,
    profile settings.ReviewProfileConfig,
    launchableEligible []AgentChoice,
    judge judgeSpec,
    baseOverride string,
    perRunPrompt string,
    deps Deps,
72 unmodified lines

}
    aggregateOutput := ""

// Resolve the single consolidating judge (explicit or auto-selected) that
    // turns the inspectors' reports into the final verdict. Validation upstream
    // guarantees one resolves; a nil provider would simply skip synthesis.
    var synthProvider SynthesisProvider
    masterLabel := ""
    if judge, ok := resolveJudge(ctx, profile); ok {
        synthProvider = AgentSynthesisProvider{AgentName: judge.agent, Model: judge.model}
        masterLabel = judgeLabel(judge)
    }
    // The single consolidating judge (resolved and validated by the caller)
    // turns the inspectors' reports into the final verdict.
    var synthProvider SynthesisProvider = AgentSynthesisProvider{AgentName: judge.agent, Model: judge.model}
    masterLabel := judgeLabel(judge)
    sinks := composeMultiAgentSinks(multiAgentSinkInputs{
        out:               out,
        isTTY:             interactive.IsTerminalWriter(out) && interactive.CanPromptInteractively(),