Skip to content

fix: mobile models page #1027

Open
aschkanAH wants to merge 6 commits into
doublewordai:mainfrom
aschkanAH:feat-mobile-models
Open

fix: mobile models page #1027
aschkanAH wants to merge 6 commits into
doublewordai:mainfrom
aschkanAH:feat-mobile-models

Conversation

@aschkanAH
Copy link
Copy Markdown
Collaborator

Adds a mobile-first React view for the Models catalog that renders alongside the existing desktop table via CSS-only breakpoint switching (block md:hidden / hidden md:block). No new runtime dependencies; everything is built on components/ui/* (Sheet, Input, Button, Card, Badge, Skeleton, Select).

cursoragent and others added 5 commits April 27, 2026 09:41
Promote a small surface of constants and helpers out of ModelCatalog.tsx
so that both the desktop table and an upcoming mobile catalog can share
the same provenance for category/capability/cutoff/pricing logic.

The promoted module also introduces aggregateFamilies, which collapses a
flat list of composite models into family rows keyed by display_name
(falling back to a canonicalised model_name for variants without a
display name). Each family carries roll-ups (intelligenceMax, priceFrom,
contextMax, releasedAt, capabilities) used by family-oriented views.

Includes a nullsLast comparator used by sort pipelines, and a
deterministic computeNewCutoff helper for testability.

Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
Adds MobileModelCatalog, a vertical card list that consumes the same
useModels({ is_composite: true, include: 'pricing' }) endpoint as the
desktop table and runs aggregateFamilies + nullsLast sorting entirely
client-side against the resulting families.

State lives in the URL via useSearchParams: search (debounced 250ms),
category, providers/capabilities/groups (repeated params, alphabetised),
sort + dir (dir omitted when it matches the field's default), and
modelId for the detail drawer. Filter toggles use replace semantics so
they don't pollute history; opening the detail drawer pushes a new
entry, and tapping back closes it. Variant switching inside the drawer
is replace-only so the back button always closes rather than cycling
through variants.

The mobile pricing context (async vs. batch) is persisted in
localStorage under catalog-pricing-context and feeds back into the
family aggregation so price roll-ups reflect the active context without
triggering loading skeletons.

Desktop and mobile both render under /models with CSS-only switching
(hidden md:block and block md:hidden) to avoid SSR/initial-paint
flicker. The previous experimental MobileModelsView swimlane is removed
since its surface is fully subsumed by the new card view.

Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
Two bugs in the mobile/desktop dual-mount setup:

1. Both views call useModels with non-overlapping params (limit:500 vs
   limit:100, plus different sort), which produces distinct React Query
   cache keys. Mounting both views unconditionally meant every page load
   issued two list requests regardless of viewport — one of them always
   wasted. Pass an isMobile flag from the wrapper into both children and
   gate each useModels call with { enabled: ... } so only the active
   view fetches. Layout switching remains CSS-only, so SSR/initial paint
   is preserved.

2. closeModel pushed a fresh history entry on top of the openModel push.
   Result: after open+close the back button rolled into the still-in-
   history modelId entry and reopened the drawer. Track whether we own
   the topmost entry via a ref; pop history (navigate(-1)) on close when
   we do, otherwise fall back to replace. The ref is reset whenever
   modelId leaves the URL by any path other than closeModel (e.g. nav
   away then back) so a stale 'we own it' flag can't navigate too far.

Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
…erflow

Two issues raised in review:

1. PricingContext was redeclared in MobileModelCatalog.tsx as
   '"async" | "batch"' even though shared.ts already exports the same
   type (and modelFamily.ts already imports it from there). Drop the
   local copy and import the shared type so the two definitions can't
   drift.

2. computeNewCutoff used 'cutoff.setMonth(getMonth() - months)' which
   silently overflows when the source day exceeds the target month's
   length. May 31 minus 3 months sets month=February but day-31 wraps
   forward to March 3, pushing the cutoff later than intended and
   incorrectly demoting borderline-new models. Clamp the source day to
   the last valid day of the target month before constructing the date,
   so May 31 → Feb 28 (or Feb 29 in a leap year), Mar 31 minus 4 months
   → Nov 30 of the previous year, etc. Also stop building the ISO
   string via toISOString() (which is UTC and disagreed with the local-
   time getMonth/getDate elsewhere in the function); format it directly
   from the local-time getters.

Adds tests for: standard month subtraction, day clamping (incl. leap
year), year boundary crossing.

Co-authored-by: aschkanAH <aschkanAH@users.noreply.github.com>
feat(catalog): mobile-optimised model catalog view
@pjb157 pjb157 requested a review from Copilot April 27, 2026 10:41
@pjb157 pjb157 changed the title Feat - mobile models page feat: mobile models page Apr 27, 2026
@pjb157 pjb157 changed the title feat: mobile models page fix: mobile models page Apr 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new mobile-first Models catalog UI in the dashboard and refactors shared catalog utilities so mobile and desktop views can coexist with CSS breakpoint switching while avoiding duplicate model-list queries.

Changes:

  • Introduces a new mobile catalog experience (MobileModelCatalog) with URL-backed sort/filter state and model-family aggregation.
  • Extracts shared catalog helpers/constants (pricing, capabilities, release-date formatting, “new” cutoff) into catalog/shared.ts.
  • Updates useModels to support an enabled flag, and refactors ModelCatalog to mount both mobile/desktop views while gating their list queries.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
dashboard/src/components/features/models/manage/MobileModelsView.tsx Removes the prior mobile view implementation (replaced by the new catalog mobile view).
dashboard/src/components/features/models/catalog/shared.ts Adds shared catalog constants and helper functions used by both desktop and mobile.
dashboard/src/components/features/models/catalog/modelFamily.ts Adds model-family aggregation + sorting helpers and “new cutoff” computation utilities.
dashboard/src/components/features/models/catalog/modelFamily.test.ts Adds unit tests for family aggregation, cutoff computation, and nulls-last sorting.
dashboard/src/components/features/models/catalog/mobileUrlState.ts Adds serialization/deserialization for mobile catalog URL state and filter-counting.
dashboard/src/components/features/models/catalog/mobileUrlState.test.ts Adds unit tests for mobile URL state round-tripping and defaults.
dashboard/src/components/features/models/catalog/ModelCatalog.tsx Refactors desktop catalog into a child component and mounts mobile+desktop with query gating.
dashboard/src/components/features/models/catalog/MobileModelCatalog.tsx Adds the new mobile-first catalog UI (filters, sorting, cards, detail drawer).
dashboard/src/api/control-layer/hooks.ts Extends useModels to accept an enabled option and prevents enabled from affecting query keys.

Comment on lines +807 to +811
const groupFilter =
canManageGroups &&
!(urlState.groups.length === 1 && urlState.groups[0] === EVERYONE_GROUP_ID)
? urlState.groups.join(",")
: undefined;
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

groupFilter omits the default Everyone group (it becomes undefined when state.groups is [EVERYONE_GROUP_ID]), which makes the mobile models query behave differently from the desktop catalog (desktop always sends the selected groups, defaulting to Everyone). This can change which models are returned for users with manage-groups. Align behavior by always passing group: urlState.groups.join(",") when canManageGroups (or otherwise make the desktop view also treat Everyone as omitted, but keep both consistent).

Suggested change
const groupFilter =
canManageGroups &&
!(urlState.groups.length === 1 && urlState.groups[0] === EVERYONE_GROUP_ID)
? urlState.groups.join(",")
: undefined;
const groupFilter = canManageGroups ? urlState.groups.join(",") : undefined;

Copilot uses AI. Check for mistakes.
Comment on lines +899 to +917
const filteredFamilies = useMemo(() => {
let result = allFamilies;

if (urlState.category !== "all") {
result = result.filter((f) => f.category === urlState.category);
}

if (urlState.providers.length > 0) {
const providerSet = new Set(urlState.providers);
result = result.filter((f) => providerSet.has(f.providerKey));
}

if (urlState.capabilities.length > 0) {
result = result.filter((f) =>
urlState.capabilities.every((cap) => f.capabilities.includes(cap)),
);
}

return result;
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catalog helper getCatalogTabForModel() is documented to return null for models that should be hidden, and the desktop view effectively hides those by only rendering known sections. In the mobile pipeline, families with category === null will still appear when the category tab is "All". Filter these out (e.g., drop category == null families before applying the rest of the filters) so mobile and desktop visibility rules match.

Copilot uses AI. Check for mistakes.
Comment on lines +151 to +252
<Card
role="button"
tabIndex={0}
onClick={() => onOpen(primary.id)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
onOpen(primary.id);
}
}}
className="cursor-pointer p-4 gap-3 transition-colors hover:bg-doubleword-neutral-50 active:scale-[0.99]"
>
<div className="flex items-start justify-between gap-3">
<div className="min-w-0">
<div className="flex items-center gap-2 flex-wrap mb-1">
<h3 className="font-semibold text-doubleword-neutral-900 text-base truncate">
{family.label}
</h3>
{family.isNew && (
<Badge className="bg-blue-100 text-blue-800 border-transparent text-[10px] uppercase tracking-wide">
New
</Badge>
)}
{primary.metadata?.quantization && (
<Badge
variant="secondary"
className="text-[10px] uppercase tracking-wide"
>
{primary.metadata.quantization}
</Badge>
)}
</div>
<div className="text-xs text-muted-foreground font-medium">
{family.providerLabel}
</div>
</div>
<CapabilityRow caps={family.capabilities} />
</div>

<div className="grid grid-cols-3 gap-2 rounded-xl border bg-doubleword-neutral-50 px-3 py-2.5 text-center">
<div>
<div className="text-[10px] uppercase tracking-wide text-muted-foreground mb-0.5">
Intel
</div>
<div className="text-sm font-semibold text-doubleword-neutral-900 tabular-nums">
{family.intelligenceMax ?? "—"}
</div>
</div>
<div className="border-x">
<div className="text-[10px] uppercase tracking-wide text-muted-foreground mb-0.5">
Cost
</div>
<div className="text-sm font-semibold text-doubleword-neutral-900 tabular-nums">
{priceFrom != null ? (
<>
{formatTariffPrice(priceFrom)}
<span className="text-muted-foreground text-xs font-normal">
{" "}/M
</span>
</>
) : (
"——"
)}
</div>
</div>
<div>
<div className="text-[10px] uppercase tracking-wide text-muted-foreground mb-0.5">
Context
</div>
<div className="text-sm font-semibold text-doubleword-neutral-900 tabular-nums">
{family.contextMax != null
? formatContextLength(family.contextMax)
: "—"}
</div>
</div>
</div>

<div className="flex items-center justify-between">
<button
type="button"
aria-label="API examples"
className="inline-flex items-center gap-1 text-xs text-muted-foreground hover:text-doubleword-neutral-900 transition-colors"
onClick={(e) => {
e.stopPropagation();
onOpen(primary.id);
}}
>
<Code className="h-3.5 w-3.5" /> API Docs
</button>
<Button
size="sm"
variant="outline"
disabled={!playgroundAvailable}
onClick={(e) => {
e.stopPropagation();
if (!playgroundAvailable) return;
navigate(`/playground?model=${encodeURIComponent(primary.id)}`);
}}
>
Try it →
</Button>
</div>
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FamilyCard makes the entire Card act like a button (role="button", tabIndex, Enter/Space handlers) while also containing interactive controls (the "API Docs" <button> and the "Try it" <Button>). Nested interactive elements inside a button-like container is an accessibility anti-pattern and can confuse screen readers/keyboard users. Consider removing the card-level button semantics and using an explicit "View details" control (or otherwise restructuring so there is only one interactive root element per card).

Copilot uses AI. Check for mistakes.
Comment on lines +789 to +790
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [debouncedSearch]);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effect suppresses react-hooks/exhaustive-deps, but the missing dependencies (urlState.search, updateUrl) can be safely included because the early-return guard prevents loops. Prefer removing the eslint disable and listing the full dependency set to avoid stale-closure bugs and keep hook behavior easier to reason about.

Suggested change
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [debouncedSearch]);
}, [debouncedSearch, updateUrl, urlState.search]);

Copilot uses AI. Check for mistakes.
const sidebar = useSidebar();
const { data: currentUser } = useUser("current");
const { data: balance, isLoading: balanceLoading } = useUserBalance(
currentUser?.id ?? "",
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useUserBalance(currentUser?.id ?? "") will issue a user fetch with an empty id during the initial render (before useUser("current") resolves), which can produce a failing /users/ request and unnecessary query churn. Gate the balance query until the id is known (e.g. add an enabled option to useUserBalance / useUser, or call it with a stable id like "current" if supported).

Suggested change
currentUser?.id ?? "",
currentUser?.id ?? "current",

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants