fix(compare): never request a model pair the current period does not list (#1721)

Switching period or provider kept the previous model pair in state, so the
report query fired once with the new period and the old models before the
facets effect reset them, and the CLI exited 1 with "model not found". Both
Cohorts and Classic now request a report only for models listed in the
current period's facets.
This commit is contained in:
Resham Joshi 2026-10-10 07:05:54 +02:00 • committed by GitHub
parent 95651189e2
commit c2cfbd18d8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 39 additions and 2 deletions

View file

@ -3,6 +3,7 @@ import { render, screen, waitFor, within } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { __resetPolledMemo } from '../hooks/usePolled'
import type { CohortComparisonReport, CompareJsonReport, ModelStats } from '../lib/types'
import { Compare } from './Compare'
@ -162,6 +163,21 @@ describe('Compare', () => {
expect(mocks.getCompareModels).toHaveBeenCalledWith('30days', 'all')
})
it('never requests the previous pair against a period whose models do not include it', async () => {
__resetPolledMemo()
const haiku = { ...modelA, model: 'Haiku 5' }
const gpt = { ...modelB, model: 'GPT-6' }
mocks.getCompareModels.mockImplementation(async period => period === 'today' ? [haiku, gpt] : [modelA, modelB])
mocks.getCompare.mockResolvedValue(report)
const { rerender } = render(<Compare period="30days" provider="all" />)
await waitFor(() => expect(mocks.getCompare).toHaveBeenCalledWith('30days', 'all', 'Opus 4.8', 'Sonnet 5'))
rerender(<Compare period="today" provider="all" />)
await waitFor(() => expect(mocks.getCompare).toHaveBeenCalledWith('today', 'all', 'Haiku 5', 'GPT-6'))
const stale = mocks.getCompare.mock.calls.filter(([period, , a, b]) => period === 'today' && (a === 'Opus 4.8' || b === 'Sonnet 5'))
expect(stale).toEqual([])
})
it('renders the need-two-models note without requesting a report', async () => {
mocks.getCompareModels.mockResolvedValue([modelA])
render(<Compare period="week" provider="all" />)
@ -334,6 +350,23 @@ describe('Compare cohorts mode', () => {
})
})
it('never requests the previous pair against a period whose models do not include it', async () => {
__resetPolledMemo()
const haiku = { ...modelA, model: 'Haiku 5' }
const gpt = { ...modelB, model: 'GPT-6' }
mocks.getCompareCohortModels.mockImplementation(async period =>
period === 'today' ? { ...facets, models: [haiku, gpt] } : facets)
const user = userEvent.setup()
const { rerender } = render(<Compare period="30days" provider="all" />)
await user.click(await screen.findByRole('tab', { name: 'Cohorts' }))
await waitFor(() => expect(mocks.getCompareCohort).toHaveBeenCalledWith('30days', 'all', 'Opus 4.8', 'Sonnet 5', undefined, undefined, undefined))
rerender(<Compare period="today" provider="all" />)
await waitFor(() => expect(mocks.getCompareCohort).toHaveBeenCalledWith('today', 'all', 'Haiku 5', 'GPT-6', undefined, undefined, undefined))
const stale = mocks.getCompareCohort.mock.calls.filter(([period, , a, b]) => period === 'today' && (a === 'Opus 4.8' || b === 'Sonnet 5'))
expect(stale).toEqual([])
})
it('returns to the classic comparison unchanged', async () => {
const user = await openCohorts()
mocks.getCompareModels.mockResolvedValue([modelA, modelB])

View file

@ -194,7 +194,7 @@ function ClassicCompare({
}}
/>
</div>
{modelA && modelB && modelA !== modelB && (
{modelA && modelB && modelA !== modelB && modelRows.some(row => row.model === modelA) && modelRows.some(row => row.model === modelB) && (
<CompareReport
period={period}
provider={provider}
@ -432,11 +432,15 @@ function CohortCompare({
})
}, [facets.data])
// facets.data is masked to the current period/provider, so this keeps the
// previous selection from being requested before the effect above resets it
// (the CLI exits 1 on a model it cannot find).
const listed = (model: string | null) => !!model && !!facets.data?.models.some(row => row.model === model)
const report = usePolled<CohortComparisonReport>(
() => codeburn.getCompareCohort(period, provider, modelA ?? '', modelB ?? '', range ?? undefined, project ? [project] : undefined, category || undefined),
[period, provider, modelA, modelB, range?.from, range?.to, project, category, refreshToken],
{
enabled: ready && !!modelA && !!modelB && modelA !== modelB,
enabled: ready && listed(modelA) && listed(modelB) && modelA !== modelB,
memoKey: reportMemoKey('cohort-v2', period, provider, range, JSON.stringify([modelA, modelB, project, category])),
},
)