Skip to content

Commit b112d29

Browse files
NickTitleGoose
andcommitted
fix(chat): hide starred models the catalog no longer serves
A starred model a provider stopped serving kept rendering as starred through the synthesized row for the current selection: rows otherwise come only from the available models, so dead stars were already invisible everywhere else. RecommendedModelList now takes the raw catalog (catalogModels) and honors starred state only for models present in it. The dropped current model stays visible and selectable but renders unstarred, without a star toggle (a dead model cannot be favorited). Stored entries are kept, so a star returns if the provider serves the model again; hard pruning was rejected because loading/partial-catalog states could wipe stars. Tests: a starred current model dropped by its provider renders unstarred with no toggle or divider (verified red without the fix), and a star for a model absent from the list renders no row while its entry survives. Co-authored-by: Goose <opensource@block.xyz>
1 parent 34d27b3 commit b112d29

3 files changed

Lines changed: 161 additions & 33 deletions

File tree

src/features/chat/ui/AgentModelPicker.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,7 @@ export function AgentModelPicker({
703703
key={selectedAgentId}
704704
ref={modelListRef}
705705
models={displayedModels}
706+
catalogModels={availableModels}
706707
currentModelId={currentModelId}
707708
currentModelProviderId={currentModelProviderId}
708709
selectedAgentId={selectedAgentId}

src/features/chat/ui/AgentModelPickerLists.tsx

Lines changed: 75 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,14 @@ function sortModels(
134134

135135
interface ModelListProps {
136136
models: ModelOption[];
137+
/**
138+
* The authoritative catalog the rows were built from, without any
139+
* synthesized rows for the current selection. Starred state is only
140+
* honored for models present here, so favorited models a provider no
141+
* longer serves stop rendering as starred. Omit to treat every row as
142+
* existing.
143+
*/
144+
catalogModels?: ModelOption[];
137145
currentModelId: string | null;
138146
currentModelProviderId: string | null;
139147
selectedAgentId: string;
@@ -157,6 +165,7 @@ export const RecommendedModelList = forwardRef<
157165
>(function RecommendedModelList(
158166
{
159167
models,
168+
catalogModels,
160169
currentModelId,
161170
currentModelProviderId,
162171
selectedAgentId,
@@ -166,7 +175,33 @@ export const RecommendedModelList = forwardRef<
166175
},
167176
ref,
168177
) {
169-
const { isStarred, toggleStar, starredKeys } = useStarredModels();
178+
const { toggleStar, starredKeys } = useStarredModels();
179+
// Rows include a synthesized entry for the current selection when the
180+
// catalog no longer serves it. Honoring starred state only for catalog
181+
// models keeps a favorited model a provider dropped from rendering as
182+
// starred; the stored entry survives so the star returns if the model does.
183+
const existingModelKeys = useMemo(() => {
184+
if (!catalogModels) {
185+
return null;
186+
}
187+
return new Set(
188+
catalogModels.map((model) =>
189+
modelStarKey(model.providerId ?? selectedAgentId, model.id),
190+
),
191+
);
192+
}, [catalogModels, selectedAgentId]);
193+
const liveStarredKeys = useMemo(() => {
194+
if (!existingModelKeys) {
195+
return starredKeys;
196+
}
197+
const live = new Set<string>();
198+
for (const key of starredKeys) {
199+
if (existingModelKeys.has(key)) {
200+
live.add(key);
201+
}
202+
}
203+
return live;
204+
}, [existingModelKeys, starredKeys]);
170205
const [searchOpen, setSearchOpen] = useState(false);
171206
const [showAll, setShowAll] = useState(false);
172207
const [query, setQuery] = useState("");
@@ -191,7 +226,7 @@ export const RecommendedModelList = forwardRef<
191226
const recencyMap = useModelRecency();
192227
const recommended = useMemo(() => {
193228
const starred = models.filter((model) =>
194-
starredKeys.has(
229+
liveStarredKeys.has(
195230
modelStarKey(model.providerId ?? selectedAgentId, model.id),
196231
),
197232
);
@@ -208,7 +243,7 @@ export const RecommendedModelList = forwardRef<
208243
currentModelId,
209244
currentModelProviderId,
210245
) &&
211-
!starredKeys.has(
246+
!liveStarredKeys.has(
212247
modelStarKey(
213248
entry.model.providerId ?? selectedAgentId,
214249
entry.model.id,
@@ -229,7 +264,9 @@ export const RecommendedModelList = forwardRef<
229264
.filter(
230265
(m) =>
231266
!recent.some((r) => r.id === m.id && r.providerId === m.providerId) &&
232-
!starredKeys.has(modelStarKey(m.providerId ?? selectedAgentId, m.id)),
267+
!liveStarredKeys.has(
268+
modelStarKey(m.providerId ?? selectedAgentId, m.id),
269+
),
233270
);
234271
const shortlist = [...recent, ...rec];
235272
if (
@@ -251,7 +288,7 @@ export const RecommendedModelList = forwardRef<
251288
}
252289
const unstarredFallback = models.filter(
253290
(model) =>
254-
!starredKeys.has(
291+
!liveStarredKeys.has(
255292
modelStarKey(model.providerId ?? selectedAgentId, model.id),
256293
),
257294
);
@@ -265,7 +302,7 @@ export const RecommendedModelList = forwardRef<
265302
currentModelProviderId,
266303
recencyMap,
267304
selectedAgentId,
268-
starredKeys,
305+
liveStarredKeys,
269306
]);
270307

271308
useEffect(() => {
@@ -313,7 +350,7 @@ export const RecommendedModelList = forwardRef<
313350
const unstarred: ModelOption[] = [];
314351
for (const model of visibleModels) {
315352
const scopeId = model.providerId ?? selectedAgentId;
316-
(starredKeys.has(modelStarKey(scopeId, model.id))
353+
(liveStarredKeys.has(modelStarKey(scopeId, model.id))
317354
? starred
318355
: unstarred
319356
).push(model);
@@ -334,7 +371,7 @@ export const RecommendedModelList = forwardRef<
334371
currentModelProviderId,
335372
recencyMap,
336373
selectedAgentId,
337-
starredKeys,
374+
liveStarredKeys,
338375
]);
339376
const sorted = [...grouped.starred, ...grouped.unstarred];
340377

@@ -449,15 +486,18 @@ export const RecommendedModelList = forwardRef<
449486
currentModelProviderId,
450487
);
451488
const scopeId = model.providerId ?? selectedAgentId;
452-
const starred = isStarred(scopeId, model.id);
489+
const modelKey = modelStarKey(scopeId, model.id);
490+
const starred = liveStarredKeys.has(modelKey);
491+
const existsInCatalog =
492+
!existingModelKeys || existingModelKeys.has(modelKey);
453493
const showStarredDivider =
454494
index === grouped.starred.length - 1 &&
455495
grouped.unstarred.length > 0;
456496
return (
457-
<div key={modelStarKey(scopeId, model.id)}>
497+
<div key={modelKey}>
458498
<div
459499
className="group flex min-w-0 items-center gap-1"
460-
data-model-key={modelStarKey(scopeId, model.id)}
500+
data-model-key={modelKey}
461501
data-starred={starred || undefined}
462502
>
463503
<PickerItem
@@ -485,28 +525,30 @@ export const RecommendedModelList = forwardRef<
485525
<IconCheck className="size-4 shrink-0 text-muted-foreground" />
486526
) : null}
487527
</PickerItem>
488-
<Button
489-
variant="ghost"
490-
size="icon-xs"
491-
selected={starred}
492-
onClick={() => toggleStar(scopeId, model.id)}
493-
// Hover-reveal keeps rows calm; keyboard users still
494-
// reach the control through row focus
495-
// (group-focus-within) or direct focus. The idle
496-
// (unstarred) star rests on the ghost icon contract's
497-
// muted-foreground — ≈5.7:1 light / ≈6.1:1 dark against
498-
// the popover, above the 3:1 WCAG non-text bar
499-
// (enforced in globals.test.ts) — and favorited rows
500-
// soften to foreground/80 via the selected flag.
501-
className="shrink-0 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100"
502-
aria-label={t(
503-
starred ? "toolbar.unstarModel" : "toolbar.starModel",
504-
{ model: getModelDisplayName(model) },
505-
)}
506-
aria-pressed={starred}
507-
>
508-
{starred ? <IconStarFilled /> : <IconStar />}
509-
</Button>
528+
{existsInCatalog ? (
529+
<Button
530+
variant="ghost"
531+
size="icon-xs"
532+
selected={starred}
533+
onClick={() => toggleStar(scopeId, model.id)}
534+
// Hover-reveal keeps rows calm; keyboard users still
535+
// reach the control through row focus
536+
// (group-focus-within) or direct focus. The idle
537+
// (unstarred) star rests on the ghost icon contract's
538+
// muted-foreground — ≈5.7:1 light / ≈6.1:1 dark against
539+
// the popover, above the 3:1 WCAG non-text bar
540+
// (enforced in globals.test.ts) — and favorited rows
541+
// soften to foreground/80 via the selected flag.
542+
className="shrink-0 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100"
543+
aria-label={t(
544+
starred ? "toolbar.unstarModel" : "toolbar.starModel",
545+
{ model: getModelDisplayName(model) },
546+
)}
547+
aria-pressed={starred}
548+
>
549+
{starred ? <IconStarFilled /> : <IconStar />}
550+
</Button>
551+
) : null}
510552
</div>
511553
{showStarredDivider ? (
512554
<Separator

src/features/chat/ui/__tests__/AgentModelPicker.test.tsx

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2108,4 +2108,89 @@ describe("AgentModelPicker starred models", () => {
21082108
removeItemSpy.mockRestore();
21092109
}
21102110
});
2111+
it("renders a starred current model unstarred once its provider drops it", async () => {
2112+
seedStar("prov-a", "ghost");
2113+
__resetStarredModelsCacheForTests();
2114+
const user = userEvent.setup();
2115+
render(
2116+
<AgentModelPicker
2117+
agents={AGENTS}
2118+
selectedAgentId="goose"
2119+
onAgentChange={vi.fn()}
2120+
currentModelId="ghost"
2121+
currentModelName="Ghost"
2122+
currentModelProviderId="prov-a"
2123+
availableModels={[
2124+
{
2125+
id: "preferred",
2126+
name: "Preferred",
2127+
recommended: true,
2128+
providerId: "prov-a",
2129+
},
2130+
{ id: "other", name: "Other", providerId: "prov-a" },
2131+
]}
2132+
onModelChange={vi.fn()}
2133+
/>,
2134+
);
2135+
2136+
await user.click(
2137+
screen.getByRole("button", { name: /choose agent and model/i }),
2138+
);
2139+
screen.getByRole("dialog");
2140+
2141+
// The dropped selection stays visible so the user can see what is in use...
2142+
const ghostRow = document.querySelector(
2143+
'[data-model-key=\'["prov-a","ghost"]\']',
2144+
);
2145+
expect(ghostRow).toBeInTheDocument();
2146+
// ...but it is no longer a favorite: no star state, no toggle, no divider.
2147+
expect(ghostRow).not.toHaveAttribute("data-starred");
2148+
expect(
2149+
within(ghostRow as HTMLElement).queryByRole("button", {
2150+
name: /star ghost/i,
2151+
}),
2152+
).not.toBeInTheDocument();
2153+
expect(
2154+
screen.queryByTestId("starred-models-divider"),
2155+
).not.toBeInTheDocument();
2156+
// The stored entry survives so the star returns if the model does.
2157+
expect(
2158+
localStorage.getItem(
2159+
starredModelStorageKey(modelStarKey("prov-a", "ghost")),
2160+
),
2161+
).toBe("1");
2162+
});
2163+
2164+
it("hides a starred model that is no longer in the available list", async () => {
2165+
seedStar("goose", "ghost");
2166+
__resetStarredModelsCacheForTests();
2167+
const user = userEvent.setup();
2168+
render(
2169+
<AgentModelPicker
2170+
agents={AGENTS}
2171+
selectedAgentId="goose"
2172+
onAgentChange={vi.fn()}
2173+
currentModelId="preferred"
2174+
currentModelName="Preferred"
2175+
availableModels={[
2176+
{ id: "preferred", name: "Preferred", recommended: true },
2177+
{ id: "other", name: "Other" },
2178+
]}
2179+
onModelChange={vi.fn()}
2180+
/>,
2181+
);
2182+
2183+
await user.click(
2184+
screen.getByRole("button", { name: /choose agent and model/i }),
2185+
);
2186+
2187+
expect(
2188+
document.querySelector('[data-model-key=\'["goose","ghost"]\']'),
2189+
).not.toBeInTheDocument();
2190+
expect(
2191+
localStorage.getItem(
2192+
starredModelStorageKey(modelStarKey("goose", "ghost")),
2193+
),
2194+
).toBe("1");
2195+
});
21112196
});

0 commit comments

Comments
 (0)