review: RunMulti falls back to cfg.Model like Run

ae3846e→main·

dipree·1mo ago·2 files·+34 added/-1 removed

Run() falls back to cfg.Model when a reviewer carries no model metadata, but RunMulti only read reviewerModelName(r), leaving AgentRun.Model empty for reviewers that don't implement reviewerRunMetadata — which would drop them from session-to-manifest matching. Mirror Run's fallback in RunMulti.

In the current wiring the multi reviewers are perAgentConfiguredReviewer (which implements the metadata interface), so this is a defensive consistency fix rather than a live break, but it makes the two orchestrators behave identically. Adds a test via stubReviewer (no model metadata) asserting the fallback to cfg.Model.

Sessions

262d2ea09704View transcript

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

Changes

2

• cmd/entire/cli/review

• Mrun_multi.go+8/-1

• Mrun_multi_test.go+26

109 unmodified lines

110
111
112
113
114
115
116
117
118
119
120
121
122
116
123
124
125
126

109 unmodified lines

states := make([]*perAgentState, len(reviewers))
    for i, r := range reviewers {
        // Mirror Run's fallback: when a reviewer carries no model metadata, use
        // the run config's model so session-to-manifest matching still sees the
        // model that was actually requested.
        model := reviewerModelName(r)
        if model == "" {
            model = cfg.Model
        }
        states[i] = &perAgentState{
            name:      r.Name(),
            agentName: reviewerActualAgentName(r),
            model:     reviewerModelName(r),
            model:     model,
            startedAt: time.Now(),
        }
    }

Mcmd/entire/cli/review/run_multi.go+8/-1

597 unmodified lines

598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626

597 unmodified lines

func (s *liveTokenSink) RunFinished(reviewtypes.RunSummary) {
    s.finished <- struct{}{}
}

// TestRunMulti_FallsBackToConfigModel verifies that when a reviewer carries no
// model metadata (does not implement reviewerRunMetadata), RunMulti falls back
to the run config's model — mirroring Run — so session-to-manifest matching
// still sees the model that was actually requested.
func TestRunMulti_FallsBackToConfigModel(t *testing.T) {

t.Parallel()
    ra := &stubReviewer{name: "agent-a", events: []reviewtypes.Event{
        reviewtypes.Started{},
        reviewtypes.AssistantText{Text: "a"},
        reviewtypes.Finished{Success: true},
    }}
    rec := &stubSinkRecorder{}
    cfg := reviewtypes.RunConfig{Model: "claude-sonnet-4-5"}

summary, err := RunMulti(context.Background(), []reviewtypes.AgentReviewer{ra}, cfg, []reviewtypes.Sink{rec})
    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if len(summary.AgentRuns) != 1 {
        t.Fatalf("expected 1 AgentRun, got %d", len(summary.AgentRuns))
    }
    if got := summary.AgentRuns[0].Model; got != "claude-sonnet-4-5" {
        t.Errorf("AgentRun.Model = %q, want fallback to cfg.Model %q", got, "claude-sonnet-4-5")
    }
}

Mcmd/entire/cli/review/run_multi_test.go+26