diff --git a/devlog/_fin/260807_models_workspace_tabs/000_plan.md b/devlog/_fin/260807_models_workspace_tabs/000_plan.md new file mode 100644 index 000000000..0a5bca5c2 --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/000_plan.md @@ -0,0 +1,143 @@ +# 260807 — Models workspace tabs (Models / Combos / Routing) + +## Objective + +Fold three sidebar destinations into one tabbed page. The Models page becomes a +three-tab workspace — **Models** (catalog), **Combos**, **Routing (beta)** — and the +sidebar drops from eleven rows to nine. + +The three tabs are not three unrelated screens sharing a container. They are the same +question asked at three depths, and the answer to all three is a model id the client +can call: + +| Tab | Question | What the client sees | +|-----|----------|----------------------| +| Models | what is visible | `anthropic/claude-opus-5` | +| Combos | who answers, in the order I chose | `combo/` | +| Routing | who answers, chosen by score | `policy/` | + +A combo and a routing profile are both virtual models that resolve to a real one; one +is manual (ordered failover / round-robin), the other automatic (hard requirements plus +a score). Grouping them under Models makes the page title honest rather than merely +shorter. + +## Why the sidebar loses two rows + +`Routing (beta)` moves into the strip. `Claude` goes away because it was never a page: +it is a shortcut into a tab of Integrations, and paying for it is `isNavEntryActive()` +in `gui/src/App.tsx` — a function whose entire job is stopping the sidebar from +claiming the user is in two places at once. Remove the duplicate row and the +correction disappears with it. + +Combos is a special case worth stating plainly: **it is already not in the sidebar.** +The NAV array has no `combos` entry, and the only route to `#combos` today is a +`Set up` link on a card inside the Models page. So for Combos this change is not one +level deeper — it is one level shallower. A card link that swaps the whole page becomes +a sibling tab. + +## Constraints + +- Hash is the source of truth. Refresh, bookmark, and Back/Forward keep the tab. + Precedent: `#logs` / `#logs/debug` in `gui/src/pages/Logs.tsx`. +- A hidden panel must not do work. The poll is in **Models itself** — `pollMs: 10_000` + on the catalog resource plus a second 10-second V2 interval. Routing and Combos do not + poll; they fetch once on mount. Gating covers all three, and cancellation matters as + much as suppression: a load already in flight must be aborted, not merely ignored. +- Combos holds unsaved editor drafts. Panels mount lazily and then stay mounted so a + half-typed combo survives a tab hop. Gate the network, never the tree. +- No `src/` runtime change. This is a GUI navigation refactor; the proxy, the routing + engine, and every management API contract stay exactly as they are. + +## External evidence + +Three findings changed or confirmed decisions here. All were verified by opening the +source, not from search snippets. + +**Primer, [UnderlineNav guidelines](https://primer.style/product/components/underline-nav/guidelines/) +and [navigation patterns](https://primer.style/product/ui-patterns/navigation/)** — do not +stack multiple underline tab rows directly on top of each other; and a tab that changes +the URL is `UnderlineNav`, while a tab that only swaps visible content without touching +the URL is `UnderlinePanels`. This is the direct warrant for two decisions: every page +tab here gets its own hash, and the Combos detail panel's inner `Config` / `About` +underline row must stop being an underline row (phase 3). + +**Carbon, [tabs usage](https://carbondesignsystem.com/components/tabs/usage/)** — at most +six tabs, and tab variants "should never be nested within each other." Three is +comfortable. Integrations already runs eleven and reads as a second navigation bar +rather than one page's facets; that is the shape being avoided, not copied. + +**W3C, [WAI-ARIA `tab` role](https://www.w3.org/TR/wai-aria/#tab) and the +[APG tabs pattern](https://www.w3.org/WAI/ARIA/apg/patterns/tabs/)** — `tab` elements MUST +be contained in a `tablist`; roving tabindex puts `0` on the active tab and `-1` on the +rest; Left/Right wrap, Home/End jump; an inactive panel SHOULD be hidden, and the APG +examples use the native `hidden` attribute, which is what the existing Logs code already +does. + +Worth recording honestly: **the accessibility specs do not forbid nested tabs.** No +opened W3C/APG page prohibits a `tablist` inside a `tabpanel`, provided the inner set is +an independently labelled composite with its own roving-tabindex scope. So demoting the +Combos inner tabs is a *visual* decision backed by Primer and Carbon, not an +accessibility fix. The plan should not claim otherwise. + +One lane produced weaker evidence and is recorded as such. A survey of comparable +products (Portkey, OpenRouter, Cloudflare AI Gateway, Kong, Vercel AI Gateway) found +that most keep the model catalog documented separately from routing/fallback config; +only Vercel nests fallbacks under models-and-providers, and that page could not be +opened (`candidate — unverified`). This is documentation structure, not UI navigation, +so it is not treated as evidence for or against this design. + +## Work-phase map + +Dependency-ordered. Each phase is one full PABCD cycle and one commit series. + +| Phase | Doc | Deliverable | Depends on | +|-------|-----|-------------|------------| +| wp01 | `010_routing_layer.md` | Additive hash contract + `models-tab.ts`, tests | — | +| wp02a | `020_models_shell.md` | Nested workspace **alongside** the legacy pages: tab i18n, strip, panels, per-panel boundaries, active-aware CSS, catalog gating | wp01 | +| wp02b | `020_models_shell.md` | Route cutover: union removal, redirects, three links, Routing NAV row + `IconRoute` | wp02a | +| wp03 | `030_combos_embed.md` | Combos panel: `retainedData` state path, abort signal, inner tabs demoted, count callback | wp02b | +| wp04 | `040_routing_embed_and_sidebar.md` | Routing panel: shared abort controller, heading removal + its test, Claude row, subtitles, render grounding | wp02b | + +wp03 and wp04 both depend on wp02b but not on each other; they run in order because they +touch the same panel block. + +**Why wp02 is two halves.** The first draft spread this work across three phases and +produced commits that could not compile (audit round 1). The correction over-swung: one +atomic phase that was atomic in the sense of *unreviewable* (audit round 2). The split +the second audit proposed is better than either: wp02a builds the nested workspace while +`#combos` and `#routing` keep working, so both routes render and every existing Routing +test stays valid; wp02b then deletes the old form only once the new one is proven in the +same tree. + +## Out of scope + +`src/` runtime, `src/routing/` engine behaviour, management API contracts, docs-site, +release, and promotion to `main`/`preview`. No push and no PR without explicit +approval. + +## Verification + +Every phase ends green on **five** commands: + +```bash +bun run typecheck +bun run test # root tests/ ONLY +cd gui && bun test tests # the 116-file GUI suite — a SEPARATE run +bun run lint:gui +bun run build:gui +``` + +`bun run test` does **not** reach `gui/tests/`: `scripts/test.ts:122` defaults to +`["./tests/"]`. The first draft missed that directory entirely and concluded no test +covered the affected routes; the second draft knew it existed and still asserted the root +command ran it. Both were wrong, and the second kind of wrong is worse — an assumption +stated as fact inside the document that defines what "green" means. + +`gui/tests/` holds the mounted happy-dom tests for page loading, the sidebar, and the +Routing page. Those are the oracle. `expect(src).toContain(...)` checks are supplements +that pass while the UI is broken. + +The final phase additionally requires live browser observation +(C-RENDER-GROUNDING-01): drive all three tabs, refresh on each, Back/Forward, and +arrow-key traversal against the running dashboard, read the screenshots back, and fix +what observation reveals. Static gates passing is not the same as the thing working. diff --git a/devlog/_fin/260807_models_workspace_tabs/001_audit_round1.md b/devlog/_fin/260807_models_workspace_tabs/001_audit_round1.md new file mode 100644 index 000000000..a7fd6c0e9 --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/001_audit_round1.md @@ -0,0 +1,116 @@ +# Audit round 1 — VERDICT: FAIL + +An independent reviewer audited the roadmap against the actual tree and returned FAIL +with eight blockers. Every one was re-verified here before acceptance. All eight are +accepted; none is rebutted. The roadmap is amended in place and re-audited. + +## The root mistake + +**There are two test directories.** `tests/` at the repository root, and `gui/tests/` +with 116 files. The roadmap looked only at the first and concluded "no existing test +covers the combos route." That is false: + +- `gui/tests/page-loading-contract.test.tsx:136` boots a happy-dom window at + `#combos` and asserts against `.combos-workspace-shell-body`. +- `gui/tests/sidebar-claude-entry.test.ts:18` requires the exact Claude row and its + `activeHashes` — the row phase 4 deletes. +- `gui/tests/routing-profiles.test.tsx:175` requires the literal string + "Routing Intelligence (beta)" and `[data-page="routing"]`. + +These are mounted behavioural tests, which is precisely the kind the roadmap proposed +to *invent* while the repository already had them. Worse, the plan's own test proposals +were mostly `expect(src).toContain(...)` string matches — assertions that pass while the +UI is broken. The reviewer's judgement stands: static source checks may supplement, but +they cannot be the oracle. + +## Blockers, verified + +**B1 — phase 2 cannot typecheck.** `NavEntry.id` is typed `Page` +(`App.tsx:53`) and NAV holds `{ id: "routing" }` (`App.tsx:71`). Removing `"routing"` +from the union in phase 2 while deferring NAV cleanup to phase 4 is a type error. +Same class of problem for i18n: `TKey` derives from `en`, so the tab keys must exist in +the phase that renders the strip. +→ Routing NAV row, `IconRoute`, its tests, and all tab-shell i18n keys move into +phase 2. Only the Claude row stays in phase 4. + +**B2 — legacy hashes lose their destination on cold load.** `replaceHash` deliberately +emits no `hashchange` (`hash-routing.ts:8`) and the redirect runs in an effect +(`use-app-route-state.ts:87`). So a cold load at `#combos` rewrites the URL to +`#models/combos` while the tab state — initialized from the *original* hash — is already +`catalog`. The URL says Combos, the screen shows the catalog. The three +`href="#combos"` links (`Models.tsx:1104,1132,1143`) hit this on every click, and the +roadmap never scheduled changing them. +→ `readModelsTab` must recognize `combos`, `combos/*`, `routing`, `routing/*` as well +as the nested forms, so the pre-redirect hash resolves to the right tab. All three links +point at `#models/combos`. Covered by a mounted cold-load test, not a resolver assertion. + +**B3 — the `active` gating destroys the drafts it was meant to protect.** A disabled +`useDataSurface` yields `data: undefined` (`data-surface.ts:59`), and the roadmap's +answer was to render the skeleton. But the skeleton *replaces* `ComboWorkspace` +(`Combos.tsx:223`), unmounting the editor and its draft. Keeping the page mounted while +swapping its subtree preserves nothing. +→ The disabled path must retain the last rendered data and keep the workspace subtree +alive. Gate the *network*, never the tree. Proven by a type → switch → switch back test. + +**B4 — the hidden-work analysis gated the wrong component.** Routing and Combos do not +poll; that correction was right. But **Models does**: `pollMs: 10_000` on the catalog +resource (`Models.tsx:271`) and a second 10-second `setInterval` for V2 +(`Models.tsx:302`). So the catalog keeps hitting `/api/models` and `/api/v2` while the +user reads Combos — the exact leak the plan claimed to prevent, in the one panel it never +examined. Also, `if (!active) return` does not cancel a load already in flight: +`RoutingProfiles` fetches take no signal and hiding never bumps `loadGenerationRef`. +→ Gate the catalog resource, the combo-summary resource, and the shadow/V2 effect and +interval on the catalog tab. Give Routing real cancellation, not just scheduling +suppression. + +**B5 — the CSS fix is right but lands a phase late and is incomplete.** The direct-child +break at `styles.css:399` is real and the fill-panel chain is sound. But phase 2 inserts +the wrapper and phase 3 repairs it, so phase 2 knowingly ships a broken layout while +claiming all three tabs paint. Two omissions: the per-tab `.page-sub` also needs the +restored padding and `flex-shrink: 0`, and `.main-inner:has(.models-workspace-shell)` +(`styles-models-workspace.css:8`) still matches a *hidden* catalog panel — so Routing +renders at 980px on a direct visit and 1200px after the catalog has mounted once. A +history-dependent width is a bug, not a cosmetic detail. +→ All wrapper CSS moves to phase 2. The 1200px selector becomes active-panel-aware. + +**B6 — `ErrorBoundary key={page}` stops resetting.** The boundary is keyed on `page` +(`App.tsx:328`) and all three tabs are now one page, so an error in Combos persists +after switching to Routing. Keying on the tab instead is worse: it remounts the whole +workspace on every switch and destroys drafts — the same trap as B3. +→ Per-panel boundaries, or a reset that clears an existing error without remounting. +Regression test: error, switch, expect a clean panel. + +**B7 — the tab counts cannot work as specified.** Models' combo summary uses a different +resource key than the Combos workspace (`Models.tsx:143` vs `Combos.tsx:157`), and combo +mutations refresh only their own (`Combos.tsx:186`) — so the count goes stale right after +a create or delete. Routing has no channel at all to report `profiles.length`, which +makes the promised discoverability mitigation undeliverable as written. +→ Child-to-shell count callbacks or one shared resource owner, tested after a mutation. +A count that lies is worse than no count. + +**B8 — test adequacy.** Covered above. + +## Non-blocking, accepted + +- The Routing header instruction was incoherent ("an `h3` carrying only the action + buttons" — a heading cannot carry buttons). Decision: `routing.title` is dropped from + the panel entirely and its actions move into a toolbar row; the Models page header is + the only title. `gui/tests/routing-profiles.test.tsx:175` asserts that string, so the + test moves with the decision rather than the decision bending to the test. +- The ≤939px stacked layout keeps a 220px rail minimum; adding a header and strip leaves + very little detail height on short landscape viewports. Added to browser coverage. +- `nav.combos`, `nav.routing`, and `nav.claude` all keep non-sidebar consumers. Do not + delete them. + +## Revised phase map + +| Phase | Scope change | +|-------|--------------| +| wp01 | unchanged — additive, green | +| wp02 | **+** Routing NAV row + `IconRoute`, **+** all tab-shell i18n keys, **+** the complete wrapper CSS, **+** catalog poll gating, **+** per-panel error boundaries | +| wp03 | **−** CSS (moved up); **+** retained-data path for drafts; **+** count callback | +| wp04 | **−** Routing NAV (moved up); keeps Claude row, remaining i18n, render grounding | + +wp02 becomes the largest phase. That is correct: "remove a page, add the tab that +replaces it, keep the tree compiling and the layout intact" is one atomic change, and +splitting it was what produced four of these eight blockers. diff --git a/devlog/_fin/260807_models_workspace_tabs/002_audit_round2.md b/devlog/_fin/260807_models_workspace_tabs/002_audit_round2.md new file mode 100644 index 000000000..7ecdf6625 --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/002_audit_round2.md @@ -0,0 +1,113 @@ +# Audit round 2 — VERDICT: FAIL + +Round 1's eight blockers came back as three resolved, four partially resolved, and one +resolved-with-a-caveat, plus five new findings. Accepted in full again. The pattern is +consistent and worth naming: round 1 caught *missing* work, round 2 caught **rules +written where mechanisms were required.** "Gate the network, never the tree" is a +correct invariant and not an implementation. + +## The finding that invalidates the verification plan + +**`bun run test` does not run `gui/tests/`.** `scripts/test.ts:122` defaults to +`["./tests/"]`, so the 116-file GUI suite needs `cd gui && bun test tests`. Confirmed by +running it: `gui/tests/routing-profiles.test.tsx` passes 6/6 under the GUI command and +is never reached by the root one. + +Round 1 taught me the directory existed. I then wrote into `000_plan.md` that the root +command covers both — an assumption, stated as fact, in the document that defines what +"green" means. Every phase gate now names both commands explicitly. + +## Blockers + +**B3 (drafts) — the mechanism is unsafe as written.** I specified a ref read during +render. This repository avoids exactly that under React Compiler / `react-hooks/refs` +(`client-resource.ts:353`), so it can fail lint and is unsound under concurrent +rendering. The reviewer supplied the correct shape and I am adopting it verbatim: +`retainedData` in **state**, seeded from the session cache, updated when `loadCombos` +produces a coherent payload, rendered as `state.data ?? retainedData`, cold skeleton +only when both are absent, and never replacing a rendered `ComboWorkspace` because +`active` went false. + +**B4 (cancellation) — ownership was never assigned.** `load` in `RoutingProfiles` has +four entry points: the initial effect (`:243`), Retry (`:426`), post-save (`:291`), +post-delete (`:321`). An effect-local controller cancels only the first; a Retry or +mutation reload keeps running after the tab hides. Generation invalidation blocks the +*write* but not the *work*. +→ `load` owns a component-level `loadAbortRef`: each call aborts and replaces the +previous controller, every fetch takes that signal, deactivation aborts and bumps the +generation, and the ref is cleared only by the request that still owns it. + +And Models is worse than I recorded: `fetchCatalog` **accepts** a signal and passes it to +none of its four requests (`Models.tsx:212`). Disabling the resource stops the state +write, not the network. Phase 2 must thread it and gate all four workers — catalog +resource, combo-summary resource, shadow-call load, V2 load *and* interval. My phase-2 +text named only two. + +**B5 (`.page-sub`) — I wrote two incompatible designs.** Phase 2 moves the subtitle +*inside* each panel; phase 3's CSS targets `.main-inner--combos > .page-sub`, a direct +child. Those cannot both be true, and the selector would simply never match. +→ Locking the reviewer's recommendation: **one subtitle for the active tab, rendered as +a direct sibling between the strip and the panels.** The documented selector then works +and the fill panel gets the remaining height. A subtitle per panel buys nothing when +only one panel is visible. + +**B8 (test scheduling) — one edit lands two phases early.** Phase 2 scheduled changing +`gui/tests/routing-profiles.test.tsx`, but the heading it asserts is removed in phase 4. +Editing it early means either a red wp02 or coverage deleted two phases before the +behaviour changes. +→ That mounted test is untouched through wp03 and changes atomically with the heading in +wp04. Also fixing the stale "no existing test references `#combos`" line still sitting in +`030` — round 1 disproved it and I corrected the claim in one document but not the other. + +**NEW — `Models 0/0` on a cold direct load.** Phase 2 stops catalog work while the +catalog is hidden, but the header and tab meta read `effectiveVisibleCount` / +`models.length`, which start empty. Land directly on `#models/combos` and the strip +confidently reports `Models 0/273` → `0/0`. That is the exact failure my own rule warns +about: "a wrong count is worse than none." +→ Track catalog-count readiness explicitly and omit the meta until a session seed or a +successful response exists. + +## The split I should have found myself + +The reviewer's judgement that wp02 is now too large to verify as one unit is correct, and +the proposed split is better than anything I had, because it never creates a broken +intermediate: + +**wp02a — additive.** Build the whole nested workspace *while the legacy pages keep +working*: tab i18n, tab shell, nested panels, per-panel boundaries, active-aware CSS, +catalog gating. `Page` keeps `combos` and `routing`; their App branches and the Routing +NAV row stay. The full-bleed modifier accepts either condition: +`page === "combos" || (page === "models" && modelsTab === "combos")`. Both routes render. +Every existing Routing test stays valid. + +**wp02b — the cutover.** Remove the union members, the standalone branches and imports, +the Routing NAV row and `IconRoute`; add the legacy redirects; repoint the three +`href="#combos"` links; simplify the modifier; update the root routing tests. + +The old form dies only once the new form is proven in the same tree. That is strictly +better than my "atomic big phase," which was atomic in the sense of *unreviewable*. + +## Document drift, fixed + +- `000_plan.md` still said Routing polls analytics — disproved in round 1, corrected in + `040` only. +- `000_plan.md` still credited wp01 with the `Page` union removal, which moved to wp02. +- `040` still repeated the tab-key table that now ships in wp02. + +Three separate cases of correcting a claim in one document and leaving it standing in +another. The roadmap is the artifact the build phase executes from, so a contradiction +between its pages is a defect in the deliverable, not an editing slip. + +## Revised phase map + +| Phase | Scope | +|-------|-------| +| wp01 | Additive hash contract + `models-tab.ts` (unchanged) | +| wp02a | Nested workspace alongside the legacy pages; both routes render | +| wp02b | Route cutover: union removal, redirects, links, Routing NAV row | +| wp03 | Combos panel: `retainedData` state path, abort signal, inner tabs, count callback | +| wp04 | Routing panel: shared abort controller, heading removal + its test, Claude row, subtitles, render grounding | + +Five implementation phases plus this roadmap cycle. Gate for every one: +`bun run typecheck` **and** `bun run test` **and** `cd gui && bun test tests` **and** +`bun run lint:gui` **and** `bun run build:gui`. diff --git a/devlog/_fin/260807_models_workspace_tabs/003_audit_round3.md b/devlog/_fin/260807_models_workspace_tabs/003_audit_round3.md new file mode 100644 index 000000000..6474da791 --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/003_audit_round3.md @@ -0,0 +1,67 @@ +# Audit round 3 — VERDICT: NEAR-PASS + +"The plan is ready for B." Three rounds, two FAILs, thirteen blockers, all accepted and +none rebutted. + +## Disposition + +All five round-2 blockers resolved. The two that mattered — the ones where I had written +a rule where a mechanism was required — are now judged implementable as written: + +- **`retainedData`** performs no render-time ref access and no render-time state update. + `setRetainedData` goes immediately after the coherent payload is assembled and before + it is returned. Expected to satisfy React Compiler and `react-hooks/refs`. +- **`loadAbortRef`** touches the ref only in callbacks and effects, never render. Owner- + checked clearing belongs in `finally`; the existing generation check already stops a + superseded aborted request from publishing an error. + +Every phase boundary derives green. No assertion is forced to fail and no union, import, +or prop mismatch is created by the ordering. + +## The duplicate-mount question, answered + +I flagged wp02a's dual routes as a risk: during that phase both `#combos` and +`#models/combos` render Combos. The answer is that they never coexist — App renders one +page at a time, so `page === "combos"` and `page === "models"` are mutually exclusive +branches. No duplicate dialogs, DOM ids, or subscribers. + +Better still, the churn is already handled: `client-resource.ts:277` delays +zero-subscriber eviction by a macrotask precisely to survive an unmount/remount gap. The +shared cache key helps here instead of colliding. The additive phase is safe for a reason +that predates this work. + +## Residual risks accepted + +Four, all bounded and observable inside a normal build → test → browser loop: + +1. **`fetchSelectedModels` takes `fetchImpl`, not a signal** (`model-visibility.ts:27`). + The fourth catalog request crosses a helper boundary to become cancellable. Caught by + typecheck and the hidden-request test. +2. **Routing needs unmount cleanup too**, not only inactive-tab cleanup — leaving Models + entirely should not strand a request. A mounted unmount test covers it. +3. **Shadow/V2 cancellation shape** — one shared controller or per-effect controllers is + a local choice. Request-count tests expose a wrong one. +4. **Full-height CSS and native modal behaviour are browser truths.** Short landscape + height, independent rail scrolling, and top-layer dialogs cannot be settled by more + planning. They are in the render-grounding checklist. + +## Closing the roadmap cycle + +The value here was not the documents; it was that three of the thirteen blockers would +have produced commits that could not compile, one would have shipped a knowingly broken +layout, one would have silently destroyed the drafts the design existed to protect, and +one invalidated the definition of "green" itself. None was visible from reading my own +plan. + +Two lessons worth carrying forward rather than filing: + +**Verify the verification.** The claim "`bun run test` covers both suites" sat inside the +document that defines what done means. I asserted it after learning the second directory +existed — an assumption upgraded to fact without a single command run. One `sed` of +`scripts/test.ts` would have caught it. + +**A rule is not a mechanism.** "Gate the network, never the tree" is correct and +unimplementable. Both times I stated an invariant and moved on, the reviewer had to +supply the design. Diff-level means the diff, not the principle behind it. + +wp00 closes. wp01 begins. diff --git a/devlog/_fin/260807_models_workspace_tabs/010_phase1_routing_layer.md b/devlog/_fin/260807_models_workspace_tabs/010_phase1_routing_layer.md new file mode 100644 index 000000000..da508ecde --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/010_phase1_routing_layer.md @@ -0,0 +1,117 @@ +# Phase 1 — Routing layer + +Owns the hash contract. Nothing renders differently after this phase; the point is +that the router can already describe the destination before any component exists to +fill it. Same order the `#debug` → `#logs/debug` move used. + +**This phase is purely additive and stays green.** The `Page` union keeps `"combos"` +and `"routing"` until phase 2. The first draft of this plan removed them here, which +would have made every `page === "combos"` comparison in `App.tsx` a type error and +left one commit knowingly red — a red commit is not a checkpoint, it is a broken +bisect point. Removing a page and adding the tab that replaces it is one atomic +change, so both belong to phase 2. + +## Target contract + +| Hash | Page | Tab | +|------|------|-----| +| `models` | models | Models (catalog) | +| `models/combos` | models | Combos | +| `models/routing` | models | Routing | +| `combos` | models | → replace to `models/combos` | +| `routing` | models | → replace to `models/routing` | + +Redirects are passive (`replaceState`), so Back is never trapped on a URL the router +immediately corrects. That is the existing `resolveAppHashChange` contract, not a new +rule. + +## MODIFY `gui/src/app-routing.ts` + +### 1. Add the tab hash list + +Placed next to `DASHBOARD_TAB_HASHES`, same shape: + +```ts +/** + * Models owns three tabs. Catalog is the bare `#models`, so it has no suffix entry + * here — same convention as Dashboard's Overview. + */ +export const MODELS_TAB_HASHES = ["models/combos", "models/routing"] as const; +``` + +### 2. Teach `hashBelongsToPage` the nested hashes + +```diff + return rawHash === page + || (page === "logs" && rawHash === "logs/debug") ++ || (page === "models" && (MODELS_TAB_HASHES as readonly string[]).includes(rawHash)) + || (page === "dashboard" && ... +``` + +### 3. Nothing else changes here + +`readPageFromHash` already answers `models` for `models/combos` and `models/routing`, +because it reads the first `/`-separated segment. The legacy `#combos` / `#routing` +redirects and the `Page` union removal are phase 2, where a destination exists to +redirect to. + +## NEW `gui/src/pages/models-tab.ts` + +Mirrors `gui/src/pages/logs-tab-keydown.ts`. Kept out of `Models.tsx` because that +file is already 1432 lines and this is the part the tests want to import directly. + +```ts +import { navigateHash, normalizeHashPath } from "../hash-routing"; + +export type ModelsTab = "catalog" | "combos" | "routing"; + +export const MODELS_TABS: readonly ModelsTab[] = ["catalog", "combos", "routing"]; + +export function modelsTabHash(tab: ModelsTab): string { + return tab === "catalog" ? "models" : `models/${tab}`; +} + +export function readModelsTab(hash = window.location.hash): ModelsTab { + const raw = normalizeHashPath(hash); + // Legacy top-level hashes resolve here too. The redirect that rewrites `#combos` to + // `#models/combos` runs via replaceState and emits NO hashchange, so tab state is + // initialized from the ORIGINAL hash. Recognising only the nested form would land a + // cold load at `#combos` on the catalog with the URL claiming Combos (audit B2). + if (raw === "models/combos" || raw === "combos" || raw.startsWith("combos/")) return "combos"; + if (raw === "models/routing" || raw === "routing" || raw.startsWith("routing/")) return "routing"; + return "catalog"; +} + +export function selectModelsTab(next: ModelsTab): void { + navigateHash(modelsTabHash(next)); +} + +export function modelsTabDomId(tab: ModelsTab): string { return `models-tab-${tab}`; } +export function modelsPanelDomId(tab: ModelsTab): string { return `models-panel-${tab}`; } +``` + +`catalog` is the internal id; the visible label is `Models` (user's call — the page is +"models" and the first tab is the plain list of them). The id stays distinct so the +code never has to disambiguate `models` the page from `models` the tab. + +## NEW `tests/models-workspace-tabs.test.ts` + +Phase-1 half (routing only — component assertions land in later phases): + +- `readModelsTab` maps all three hashes and defaults unknown input to `catalog`. +- `readModelsTab` also maps the legacy `combos`, `combos/x`, `routing`, `routing/x` + forms — the cold-load case from audit B2. +- `modelsTabHash` round-trips every tab through `readModelsTab`. +- `hashBelongsToPage("models/combos", "models")` and `("models/routing", "models")` + are both true. +- `hashBelongsToPage` rejects an invented `models/nope`, so normalization strips it. +- `readPageFromHash("models/combos")` is `models` — the first segment wins. + +No existing test changes in this phase. `tests/routing-intelligence-ui.test.ts` still +describes Routing as a top-level page and still passes, because the union is untouched. + +## Verification + +All four gates stay green: `bun run typecheck`, `bun run test`, `bun run lint:gui`, +`bun run build:gui`. Nothing in this phase can break a render path, because nothing +reads the new module yet. diff --git a/devlog/_fin/260807_models_workspace_tabs/020_phase2_models_shell.md b/devlog/_fin/260807_models_workspace_tabs/020_phase2_models_shell.md new file mode 100644 index 000000000..7e60d4392 --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/020_phase2_models_shell.md @@ -0,0 +1,273 @@ +# Phase 2 — Models shell + +The atomic phase: the `Page` union loses `combos` and `routing`, the tab strip appears, +and the panels that replace those pages mount. Splitting any of it out would leave a +commit where a page has been deleted but its replacement does not exist. + +**Scope grew after audit round 1** (`001_audit_round1.md`). Four things that were +deferred to later phases cannot be: the Routing NAV row (typed `Page`, so the union +removal breaks it), the tab-shell i18n keys (`TKey` derives from `en`), the full-bleed +CSS (phase 2 inserts the wrapper that breaks the selector), and the catalog poll gating. +Deferring them meant shipping a commit that does not compile or knowingly renders a +broken layout. This is the big phase, and that is correct. + +## MODIFY `gui/src/app-routing.ts` — remove the two pages + +```diff + export type Page = + ... + | "models" +- | "combos" + | "subagents" + ... +- | "integrations" +- | "routing"; ++ | "integrations"; +``` + +Same two entries out of `VALID_PAGES`. Then the legacy ids in `readPageFromHash`, +beside the existing `debug` line: + +```ts +// Legacy: Combos and Routing used to be standalone pages; both are Models tabs now. +if (pageId === ("combos" as Page) || pageId === ("routing" as Page)) return "models"; +``` + +and the redirects in `resolveAppHashChange`, directly after the `debug` branch: + +```ts +if (rawHash === "combos" || rawHash.startsWith("combos/")) { + return { page: "models", replaceTo: "models/combos" }; +} +if (rawHash === "routing" || rawHash.startsWith("routing/")) { + return { page: "models", replaceTo: "models/routing" }; +} +``` + +The `startsWith` arm is not decoration: `#routing/foo` from an old bookmark must reach +the Routing tab rather than be normalized to a bare page that drops the destination — +the exact failure the file's `#api` comment already documents. + +## MODIFY `gui/src/App.tsx` + +`PAGE_TKEY` loses its `combos` and `routing` keys (the compiler demands it — the record +is keyed by `Page`). + +Render block: + +```diff +- {page === "models" && } +- {page === "combos" && } ++ {page === "models" && } + ... +- {page === "routing" && } +``` + +`Combos` and `RoutingProfiles` imports move out of `App.tsx` into `Models.tsx`. + +The full-bleed modifier stops asking about the page and starts asking about the tab: + +```diff +-
++
+``` + +where `modelsTab` comes from a `readModelsTab()` state synced on `hashchange` / +`popstate`, the same listener pair `useAppRouteState` already installs. + +> This is the one piece of tab knowledge that has to live in App rather than in +> Models: the `.main-inner` element is App's, and phase 3 explains why the modifier +> cannot simply move inside the page. + +### NAV: the Routing row goes now + +```diff +- { id: "routing", tkey: "nav.routing", Icon: IconRoute }, +``` + +plus the `IconRoute` import if unused elsewhere. Not optional and not deferrable: +`NavEntry.id` is typed `Page` (`App.tsx:53`), so a NAV entry naming a removed page is a +type error the moment the union shrinks. + +The duplicate **Claude** row and `isNavEntryActive` stay until phase 4 — they are a +separate concern (Integrations, not Models) and they still typecheck. + +### Per-panel error boundaries + +`ErrorBoundary` is keyed on `page` (`App.tsx:328`). With three tabs on one page, an +error thrown in Combos survives a switch to Routing, because the key never changes. +Adding the tab to the key is worse: every ordinary switch remounts the workspace and +destroys drafts. + +So each tabpanel gets its own boundary inside `Models.tsx`, and App's page-level +boundary stays as the outer net. A failing panel then shows its error in its own panel +and the other two keep working. + +### Full-bleed CSS moves here + +The wrapper this phase introduces is what breaks +`.main-inner--combos > .combos-workspace-shell`, so the repair ships in the same +commit. Full detail in `030`; the rules land here. + +Including the one the first draft missed: `.main-inner:has(.models-workspace-shell)` +(`styles-models-workspace.css:8`) widens the column to 1200px, and a lazily-mounted +hidden catalog still matches it. Left alone, Routing renders at 980px on a direct visit +and 1200px once the catalog has been opened — width that depends on history. The +selector must match only a **visible** catalog panel. + +### Catalog work stops when the catalog is hidden + +`Models` polls: `pollMs: 10_000` on the catalog resource (`Models.tsx:271`) and a +separate 10-second `setInterval` for V2 (`Models.tsx:302`). Both keep running while the +user is on Combos or Routing unless gated on `tab === "catalog"` — the leak the plan +claimed to prevent while overlooking the only panel that actually had one. + +## MODIFY `gui/src/pages/Models.tsx` + +### Tab state + +```tsx +const [tab, setTab] = useState(readModelsTab); +const [mounted, setMounted] = useState>(() => new Set([readModelsTab()])); + +const activateTab = (next: ModelsTab) => { + setTab(next); + setMounted(current => (current.has(next) ? current : new Set([...current, next]))); +}; +``` + +Copied deliberately from `Integrations.tsx`: panels mount lazily and then stay mounted +so a half-typed combo draft survives a tab hop, and the accumulation happens in the +event handler rather than an effect so a switch costs one render, not two. + +`hashchange` + `popstate` listeners call `activateTab(readModelsTab())`. + +### Strip markup + +`.page-tabs` / `.page-tab` / `.page-tab--active`, `role="tablist"`, roving tabindex, +`aria-selected`, `aria-controls`, and Arrow/Home/End — the wiring the APG requires and +that `Integrations.tsx` already implements. Each label carries a `.section-tab-meta` +count: `Models 35/273`, `Combos 3`, `Routing 2`. The class and its +`page-tab--active > .section-tab-meta` rule already exist in `styles.css`. + +Counts come from data the page already holds — `effectiveVisibleCount` / `models.length` +for the catalog and `combos.length` from the existing `combosResource`. Routing's count +needs a profile list, which the Routing panel owns; until it reports one the meta is +omitted rather than rendered as `0`, because a wrong count is worse than none. + +### Body split + +Everything currently returned by the component — rail, controls, provider list, modals +— becomes the catalog panel body. The three panels are siblings, each `hidden` when +inactive (`hidden` per the APG examples, matching the existing Logs code). + +The page header (`h2` + count) and the strip live above all three panels and stay +visible on every tab. The `page-sub` is ONE element rendered between the strip and the +panels, carrying the active tab's copy — see the subtitle note above. + +## wp02a tests — new, in `gui/tests/` (mounted, happy-dom) + +The oracle. Source-string checks are supplements that pass while the UI is broken. + +- Cold load at `#models/combos` renders the Combos panel; `#models/routing` renders + Routing. +- Clicking each tab updates both the rendered panel and the hash. +- Arrow Left/Right/Home/End move focus and selection together. +- A panel that throws shows its error while the other two still render. +- With Combos visible, no `/api/models`, `/api/v2`, `/api/provider-context-caps`, or + `/api/providers` request fires after the poll interval elapses. +- A cold load at `#models/combos` renders no `0/0` count. + +Nothing existing changes in this half — `#combos` and `#routing` still work, so +`page-loading-contract` and `routing-profiles` stay green untouched. That is precisely +what splitting here buys. + +--- + +# wp02b — route cutover + +The old form dies in one commit, with the new form already proven beside it. + +## MODIFY `gui/src/app-routing.ts` + +Remove `"combos"` and `"routing"` from the `Page` union and `VALID_PAGES`. Then the +legacy ids in `readPageFromHash`, beside the existing `debug` line: + +```ts +// Legacy: Combos and Routing used to be standalone pages; both are Models tabs now. +if (pageId === ("combos" as Page) || pageId === ("routing" as Page)) return "models"; +``` + +and the redirects in `resolveAppHashChange`, directly after the `debug` branch: + +```ts +if (rawHash === "combos" || rawHash.startsWith("combos/")) { + return { page: "models", replaceTo: "models/combos" }; +} +if (rawHash === "routing" || rawHash.startsWith("routing/")) { + return { page: "models", replaceTo: "models/routing" }; +} +``` + +The `startsWith` arm is not decoration: `#routing/foo` from an old bookmark must reach +the Routing tab rather than be normalized to a bare page that drops the destination — +the exact failure the file's `#api` comment documents. + +## MODIFY `gui/src/App.tsx` + +- `PAGE_TKEY` loses both keys (the record is keyed by `Page`; the compiler demands it). +- Delete the `page === "combos"` / `page === "routing"` render branches and their imports. +- Delete the Routing NAV row, and `IconRoute` if now unused. `NavEntry.id` is typed + `Page` (`App.tsx:53`), so the union change forces this rather than it being a choice. +- Simplify the modifier to `page === "models" && modelsTab === "combos"`. + +The duplicate **Claude** row and `isNavEntryActive` stay until wp04 — separate concern, +still typechecks. + +## MODIFY `gui/src/pages/Models.tsx` + +The three `href="#combos"` links (`:1104`, `:1132`, `:1143`) point at `#models/combos`. +Missing these was audit round 1's B2: the redirect fires, rewrites the URL, and leaves +the tab on the catalog because `replaceHash` emits no `hashchange`. + +## wp02b tests + +`tests/models-workspace-tabs.test.ts`: `VALID_PAGES` holds neither id; +`resolveAppHashChange` maps `combos`, `combos/x`, `routing`, `routing/x`. + +`gui/tests/page-loading-contract.test.tsx`: boots at `#combos` (`:136`) and asserts +`.combos-workspace-shell-body` (`:183`). The URL becomes `#models/combos`; the shell +assertions stay valid because the workspace markup does not change. + +**`gui/tests/routing-profiles.test.tsx` is NOT touched here.** It asserts +`[data-page="routing"]` and the literal "Routing Intelligence (beta)" (`:175`), and the +heading it depends on is removed in wp04. Editing it now means either a red wp02b or +coverage deleted two phases before the behaviour changes. + +## MODIFY `tests/routing-intelligence-ui.test.ts` + +Now genuinely stale, and the compiler cannot catch a string assertion: + +```diff +- expect(VALID_PAGES.has("routing")).toBe(true); +- expect(readPageFromHash("routing")).toBe("routing"); +- expect(hashBelongsToPage("routing", "routing")).toBe(true); +- expect(resolveAppHashChange("routing").replaceTo).toBeNull(); ++ expect(readPageFromHash("models/routing")).toBe("models"); ++ expect(hashBelongsToPage("models/routing", "models")).toBe(true); ++ expect(resolveAppHashChange("models/routing").replaceTo).toBeNull(); ++ expect(resolveAppHashChange("routing")).toEqual({ page: "models", replaceTo: "models/routing" }); +``` + +and `expect(app).toContain('page === "routing"')` becomes an assertion that +`Models.tsx` mounts `RoutingProfiles`. + +## Verification (both halves) + +All five commands green, including the separate `cd gui && bun test tests` — the root +`bun run test` does not reach the GUI suite (`scripts/test.ts:122`). + +Browser observation starts here rather than waiting for wp04, because this is where a +mistake shows up as a blank page or a collapsed workspace: load `#models`, +`#models/combos`, `#models/routing`, confirm each paints, and confirm the Combos +workspace fills the viewport under the header and strip. diff --git a/devlog/_fin/260807_models_workspace_tabs/030_phase3_combos_embed.md b/devlog/_fin/260807_models_workspace_tabs/030_phase3_combos_embed.md new file mode 100644 index 000000000..c366eb841 --- /dev/null +++ b/devlog/_fin/260807_models_workspace_tabs/030_phase3_combos_embed.md @@ -0,0 +1,252 @@ +# Phase 3 — Combos as a panel + +Combos is the only surface in the GUI that opts out of the normal 980px scrolling +column: it is a full-bleed `100dvh` workspace whose rail and detail pane scroll +independently. Making it a tab means reconciling that with a page header and a tab +strip that must stay visible above it. + +**Scope changed after audit round 1.** The CSS below **ships in phase 2**, in the same +commit that inserts the wrapper — repairing it a phase later would mean phase 2 +knowingly ships a broken layout. It stays documented here because this is where the +reasoning belongs. What remains phase-3 work: the draft-preserving `active` path, the +abort signal, the inner-tab demotion, and the count callback. + +## The selector that actually breaks + +```css +.main-inner.main-inner--combos > .combos-workspace-shell { flex: 1 1 auto; min-height: 0; height: 100%; ... } +``` + +`gui/src/styles.css:399`. It is a **direct-child** selector. Today `Combos` returns +`.combos-workspace-shell` as `.main-inner`'s immediate child, so it matches. + +As a tab, the shell sits inside a panel wrapper: + +``` +.main-inner--combos +├─ .page-head (header, stays visible) +├─ .page-tabs (strip, stays visible) +└─ #models-panel-combos ← new wrapper + └─ .combos-workspace-shell ← no longer a direct child +``` + +The rule stops matching, the shell loses `flex: 1 1 auto` and `min-height: 0`, and the +workspace collapses to content height inside a clipped `100dvh` parent — rail and +detail scrolling both die. + +An investigation pass reported that inserting siblings keeps the selector intact. That +is true for *siblings*, and false for the structure this phase actually builds, because +the panel wrapper adds a level. Verified by reading `gui/src/styles.css:399-405` +directly. Recording it because the wrong version of this claim would have shipped a +broken layout that typecheck and tests cannot see. + +### Fix + +Make the panel wrapper the flex child and let the shell fill it: + +```diff +-.main-inner.main-inner--combos > .combos-workspace-shell { ++.main-inner.main-inner--combos > .models-tab-panel--fill, ++.main-inner.main-inner--combos .models-tab-panel--fill > .combos-workspace-shell { + flex: 1 1 auto; + min-height: 0; + height: 100%; + display: flex; + flex-direction: column; + } +``` + +The header and strip need horizontal padding back, since `.main-inner--combos` zeroes +the container's: + +```css +.main-inner--combos > .page-head, +.main-inner--combos > .page-tabs, +.main-inner--combos > .page-sub { padding-inline: 36px; flex-shrink: 0; } +@media (max-width: 760px) { + .main-inner--combos > .page-head, + .main-inner--combos > .page-tabs, + .main-inner--combos > .page-sub { padding-inline: 18px; } +} +``` + +`flex-shrink: 0` matters: without it the header is a flex item in a fixed-height column +and gets squeezed when the workspace wants room. + +`.page-sub` is in that list because phase 2 moves the subtitle per tab. The first draft +padded only the header and strip, which would have left the Combos subtitle flush +against the viewport edge (audit B5). + +### The 1200px selector + +`.main-inner:has(.models-workspace-shell)` (`styles-models-workspace.css:8`) widens the +column, and a lazily-mounted **hidden** catalog panel still satisfies `:has()`. So +Routing would render at 980px on a direct visit and 1200px after the catalog had been +opened once — a width that depends on browsing history. The selector must require a +visible catalog panel (`:has(.models-tab-panel:not([hidden]) .models-workspace-shell)` +or equivalent). Ships in phase 2 with the rest of the CSS. + +The two mobile rules (`gui/src/styles.css:1983`, `2020`) need no change — they set the +container height and padding, and both still apply. + +## Why the modifier stays in App + +`.main-inner` belongs to `App.tsx`; a page cannot add a class to its own container +without a callback or a portal. So App keeps the modifier and reads the tab (phase 2), +which is the smallest coupling available. The alternative — Models rendering its own +full-height wrapper inside the 980px column — does not work, because `.main-inner` has +`max-width: 980px` and normal padding until the modifier removes them. + +## Inactive panels + +The other two panels are `hidden`, which is `display: none` in the UA stylesheet, so +they occupy no flex space. No extra rule needed. + +## MODIFY `gui/src/pages/Combos.tsx` + +### Props + +```diff +-export default function Combos({ apiBase }: { apiBase: string }) { ++export default function Combos({ apiBase, active = true }: { apiBase: string; active?: boolean }) { +``` + +Default `true` keeps every existing call site and test honest. + +### Gate the fetch + +`Combos` fires three parallel fetches (`/api/combos`, `/api/config`, `/api/models`) on +subscription. It does **not** poll — no `pollMs` — so the risk of a permanently mounted +panel is a wasted cold load, not a background traffic leak. Still worth gating: + +```diff + const resource = useDataSurface( + `combos-workspace:${apiBase}`, + [apiBase], + loadCombos, +- { ... }, ++ { ..., enabled: active }, + ); +``` + +### The trap the first draft walked into + +A disabled resource yields `data: undefined` with no skeleton and no error +(`data-surface.ts:59`), and the existing fallback arrays would make `ComboWorkspace` +paint as a first-run empty state. The first draft's answer was "render the skeleton +instead" — which is wrong in a way that defeats the whole point: the skeleton +*replaces* `ComboWorkspace` (`Combos.tsx:223`), unmounting the editor and destroying the +unsaved draft this design exists to protect (audit B3). + +The rule is: **gate the network, never the tree.** + +An earlier draft said "hold the last payload in a ref and read it during render." Audit +round 2 rejected that mechanism, correctly: this repository avoids render-time ref reads +under React Compiler / `react-hooks/refs` (`client-resource.ts:353`), so it can fail lint +and is unsound under concurrent rendering. A rule is not a mechanism, and the one I wrote +would not have survived the linter. + +The concrete design: + +```tsx +const [retainedData, setRetainedData] = useState(() => seed); + +// loadCombos already assembles one coherent payload from three responses; retain there. +const data = resource.state.data ?? retainedData; +``` + +- `retainedData` lives in **state**, seeded from the session cache. +- It is written where `loadCombos` produces its coherent payload — one place, never a + render side effect. +- Render `state.data ?? retainedData`. +- The cold skeleton appears only when **both** are absent. +- `active` going false never replaces an already-rendered `ComboWorkspace`. + +Proven by a mounted test: open a combo, type into the draft, switch to Models, switch +back, expect the typed value still there. Not by a source-string assertion. + +### Pre-existing defect found while reading + +`loadCombos` takes no `AbortSignal` and none of its three `fetch` calls pass one, so +resource cleanup cannot cancel them. Harmless today because the page only unmounts on +navigation; more visible once the panel mounts lazily. Threading the signal through is +a two-line change and belongs here rather than in a separate unit — it is the same code +being touched, and leaving a known un-cancellable fetch behind while explicitly adding +lifecycle control would be incoherent. + +### Dialogs + +Add, Remove, and Unsaved use native `showModal()`. A dialog in the browser's top layer +is not clipped by an ancestor's `hidden`. Whether an open dialog can survive a tab +switch depends on whether `hidden` on an ancestor closes it — **this must be checked in +the browser, not reasoned about.** If a modal does survive, the fix is to close open +dialogs when `active` goes false. + +## MODIFY `gui/src/components/combo-workspace-detail-panel.tsx` — inner tabs + +Currently `combos-workspace-tabs` / `combos-workspace-tab` with `role="tablist"` and +`aria-selected`. Not `.page-tabs`, but visually the same underline vocabulary, so under +the page strip it reads as two stacked underline rows — the pattern Primer names +directly. + +Demote to a pill, following `.segmented.models-segmented` at `Models.tsx:924`: + +```diff +-
+- - + {/* + Pills, not an underline row. Combos is a tab of the Models page now, so an + underline strip here would sit directly under the page strip — two rows of the + same visual language stacked, which reads as two navigation levels rather than + one page's facets. + + The roles stay `tablist`/`tab`/`aria-selected`: these control a real tabpanel + below, so this is a tab set wearing pill styling, not a filter. The + `radiogroup` shape used by `.models-segmented` would misdescribe the widget. + */} +
+ {DETAIL_TABS.map((candidate, index) => ( + + ))}
-
- {tab === "config" ? ( + {/* + Both panels stay in the tree, the inactive one `hidden`. A single panel whose id + followed the active tab left the OTHER tab's `aria-controls` pointing at an + element that did not exist — a broken IDREF on whichever tab was not selected. + */} + + + {/* + `tabIndex={0}` because this panel holds no focusable descendants: without it, + Tab out of the tablist would skip the content the tab just revealed. + */} +
); } diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 7703f6c09..bc23cf5a2 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -10,7 +10,6 @@ export const de: Record = { "nav.providers": "Anbieter", "nav.models": "Modelle", "nav.combos": "Combos", - "nav.routing": "Routing (beta)", "nav.subagents": "Sub-Agenten", // routing intelligence @@ -427,6 +426,12 @@ export const de: Record = { "prov.manageCodexAccounts": "Codex-Konten verwalten", "prov.openaiApiMissing": "API-Schlüssel erforderlich", "prov.openaiApiSetup": "API-Schlüssel einrichten", + "models.tab.catalog": "Modelle", + "models.tab.combos": "Combos", + "models.tab.routing": "Routing (beta)", + "models.tabsLabel": "Modell-Oberflächen", + "models.subtitle.combos": "Geordnete Modellgruppen, die unter einer id antworten. Failover probiert Ziele der Reihe nach, Round-Robin verteilt die Last.", + "models.subtitle.routing": "Policy-Profile, Dry-Run-Auswertung und quellenbasierte Routing-Analysen.", "models.subtitle": "Steuere, welche Modelle Codex sieht — natives GPT-Passthrough und geroutete Anbieter, nach Anbieter gruppiert (Kopfzeile zum Einklappen anklicken). Ausgeblendete Modelle fehlen in Katalog und Auswahl, bleiben aber per genauer ID aufrufbar. Änderungen gelten bei der nächsten Codex-Runde — opencodex invalidiert Codex 5-Minuten-Modell-Cache, kein Neustart nötig.", "models.nativeGroupLabel": "OpenAI nativ", "models.nativeHint": "Passthrough-Modelle verwenden die unter Anbieter gewählte Pool- oder Direkt-Option. Ausblenden entfernt sie aus der Codex-Auswahl (Katalogeintrag bleibt, Reaktivierung stellt exakt wieder her).", @@ -434,10 +439,6 @@ export const de: Record = { "models.workspace.providers": "Anbieter", "models.workspace.allProviders": "Alle Anbieter", "models.workspace.mainAria": "Modelldetails", - "models.combosEmpty": "Noch keine Kombos konfiguriert", - "models.combosSetup": "Einrichten", - "models.combosAdd": "Kombo hinzufügen", - "models.combosActive": "{count} aktiv", "models.allOn": "Alle an", "models.allOff": "Alle aus", "models.cap350k": "Limit 350k", @@ -1177,7 +1178,6 @@ export const de: Record = { "api.attribution.ambiguous": "Zwei Schlüssel teilen sich diese ID, daher lässt sich die Nutzung keinem davon zuordnen. Vergib in der Konfigurationsdatei je Schlüssel eine eindeutige ID.", "api.attribution.railAmbiguous": "doppelte ID", // Claude Code inbound - "nav.claude": "Claude", "claude.subtitle": "GPT, Gemini und andere Modelle in Claude Code verwenden.", "claude.enabledLabel": "Claude-Verbindung", "claude.enabledHint": "Wenn aus, kann Claude Code diesen Proxy nicht verwenden.", @@ -1715,6 +1715,7 @@ export const de: Record = { "cws.allCombos": "Alle Combos", "cws.copyModel": "ID kopieren", "cws.copied": "Kopiert", + "cws.tabsLabel": "Combo-Detailbereiche", "cws.tab.config": "Konfiguration", "cws.tab.about": "Info", "cws.strategy": "Strategie", diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index 9ae886124..c63a33495 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -12,7 +12,6 @@ export const en = { "nav.providers": "Providers", "nav.models": "Models", "nav.combos": "Combos", - "nav.routing": "Routing (beta)", "nav.subagents": "Subagents", "nav.logs": "Logs & Debug", "nav.usage": "Usage", @@ -446,6 +445,12 @@ export const en = { "prov.openaiApiSetup": "Set up API key", // models + "models.tab.catalog": "Models", + "models.tab.combos": "Combos", + "models.tab.routing": "Routing (beta)", + "models.tabsLabel": "Model surfaces", + "models.subtitle.combos": "Ordered groups of models that answer as one id. Failover tries targets in order; round-robin spreads the load.", + "models.subtitle.routing": "Policy profiles, dry-run evaluation, and source-backed routing analytics.", "models.subtitle": "Toggle which models Codex sees — native GPT passthrough and routed providers, grouped by provider (click a header to collapse). Hidden models stay off the catalog + model picker but remain directly callable by exact id. Changes apply on the next Codex turn — opencodex invalidates Codex's 5-min model cache so no restart is needed.", "models.nativeGroupLabel": "OpenAI native", "models.nativeHint": "Passthrough models use the Pool or Direct account option selected on Providers. Toggling one off hides it from the Codex picker (the catalog entry is kept, so re-enabling restores it exactly).", @@ -453,10 +458,6 @@ export const en = { "models.workspace.providers": "Providers", "models.workspace.allProviders": "All providers", "models.workspace.mainAria": "Model details", - "models.combosEmpty": "No combos configured yet", - "models.combosSetup": "Set up", - "models.combosAdd": "Add combo", - "models.combosActive": "{count} active", "models.allOn": "All on", "models.allOff": "All off", "models.cap350k": "Cap 350k", @@ -1640,7 +1641,6 @@ export const en = { "api.attribution.ambiguous": "Two keys share this ID, so usage cannot be attributed to one of them. Give each key a unique ID in the config file.", "api.attribution.railAmbiguous": "duplicate ID", // Claude Code inbound - "nav.claude": "Claude", "claude.subtitle": "Use GPT, Gemini, and other models inside Claude Code.", "claude.pageTitle": "Claude Code", "claude.workspace.settings": "Settings", @@ -1749,6 +1749,7 @@ export const en = { "cws.allCombos": "All combos", "cws.copyModel": "Copy id", "cws.copied": "Copied", + "cws.tabsLabel": "Combo detail sections", "cws.tab.config": "Config", "cws.tab.about": "About", "cws.strategy": "Strategy", diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index d5f7a915b..8bef10acc 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -10,7 +10,6 @@ export const ja: Record = { "nav.providers": "プロバイダー", "nav.models": "モデル", "nav.combos": "コンボ", - "nav.routing": "ルーティング (beta)", "nav.subagents": "サブエージェント", // routing intelligence @@ -435,6 +434,12 @@ export const ja: Record = { "prov.openaiApiSetup": "API キーを設定", // models + "models.tab.catalog": "モデル", + "models.tab.combos": "コンボ", + "models.tab.routing": "ルーティング (beta)", + "models.tabsLabel": "モデルサーフェス", + "models.subtitle.combos": "複数のモデルを 1 つの id にまとめ、順に応答させます。failover は順番に試し、round-robin は負荷を分散します。", + "models.subtitle.routing": "ポリシープロファイル、dry-run 評価、そして根拠の残るルーティング分析です。", "models.subtitle": "Codex に表示するモデルを切り替えます — ネイティブ GPT パススルーとルーティングプロバイダー、プロバイダー別(ヘッダーをクリックで折りたたみ)。非表示モデルはカタログとピッカーから外れますが、正確な id での直接呼び出しは可能です。変更は次回の Codex ターンで適用 — opencodex は Codex の 5 分間モデルキャッシュを無効化するので再起動は不要です。", "models.nativeGroupLabel": "OpenAI ネイティブ", "models.nativeHint": "パススルーモデルはプロバイダーで選択したプールまたはダイレクトアカウントオプションを使用します。一つオフにすると Codex ピッカーから隠します(カタログエントリは保持されるので、再有効化で正確に復元されます)。", @@ -442,10 +447,6 @@ export const ja: Record = { "models.workspace.providers": "プロバイダー", "models.workspace.allProviders": "すべてのプロバイダー", "models.workspace.mainAria": "モデルの詳細", - "models.combosEmpty": "まだコンボが設定されていません", - "models.combosSetup": "セットアップ", - "models.combosAdd": "コンボを追加", - "models.combosActive": "{count} アクティブ", "models.allOn": "すべてオン", "models.allOff": "すべてオフ", "models.cap350k": "350k 上限", @@ -1585,7 +1586,6 @@ export const ja: Record = { "api.attribution.ambiguous": "2 つのキーが同じ ID を共有しているため、どちらの使用状況か判別できません。設定ファイルでキーごとに一意の ID を指定してください。", "api.attribution.railAmbiguous": "ID 重複", // Claude Code inbound - "nav.claude": "Claude", "claude.subtitle": "Claude Code 内で GPT、Gemini などのモデルを使用します。", "claude.pageTitle": "Claude Code", "claude.workspace.settings": "設定", @@ -1783,6 +1783,7 @@ export const ja: Record = { "cws.copyModel": "ID をコピー", "cws.copied": "コピーしました", "cws.renamed": "{from} を {to} に変更しました。", + "cws.tabsLabel": "コンボ詳細セクション", "cws.tab.config": "設定", "cws.tab.about": "概要", "cws.strategy": "ストラテジー", diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index e0ee1af6a..4112cbd79 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -10,7 +10,6 @@ export const ko: Record = { "nav.providers": "프로바이더", "nav.models": "모델", "nav.combos": "콤보", - "nav.routing": "라우팅 (beta)", "nav.subagents": "서브에이전트", // routing intelligence @@ -438,6 +437,12 @@ export const ko: Record = { "prov.openaiApiSetup": "API 키 설정", // models + "models.tab.catalog": "모델", + "models.tab.combos": "콤보", + "models.tab.routing": "라우팅 (beta)", + "models.tabsLabel": "모델 표면", + "models.subtitle.combos": "여러 모델을 하나의 id로 묶어 순서대로 응답하게 합니다. failover는 순서대로 시도하고, round-robin은 부하를 나눕니다.", + "models.subtitle.routing": "정책 프로필, dry-run 평가, 그리고 근거가 남는 라우팅 분석입니다.", "models.subtitle": "Codex가 보는 모델을 켜고 끕니다 — 네이티브 GPT passthrough와 라우팅된 모델을 프로바이더별로 묶어 보여줍니다(헤더를 클릭하면 접힘). 숨긴 모델은 카탈로그와 선택기에서 빠지지만 정확한 id로 직접 호출할 수 있습니다. 변경 사항은 다음 Codex 턴에 적용됩니다 — opencodex가 Codex의 5분 모델 캐시를 무효화하므로 재시작이 필요 없습니다.", "models.nativeGroupLabel": "OpenAI 네이티브", "models.nativeHint": "프로바이더에서 선택한 풀 또는 직접 계정 옵션으로 서빙되는 passthrough 모델입니다. 끄면 Codex 선택기에서 숨겨지고, 카탈로그 항목은 유지되므로 다시 켜면 그대로 복원됩니다.", @@ -445,10 +450,6 @@ export const ko: Record = { "models.workspace.providers": "프로바이더", "models.workspace.allProviders": "모든 프로바이더", "models.workspace.mainAria": "모델 세부정보", - "models.combosEmpty": "아직 설정된 콤보가 없습니다", - "models.combosSetup": "설정하기", - "models.combosAdd": "콤보 추가하기", - "models.combosActive": "{count}개 활성", "models.allOn": "모두 켜기", "models.allOff": "모두 끄기", "models.cap350k": "350k 제한", @@ -1204,7 +1205,6 @@ export const ko: Record = { "api.attribution.ambiguous": "두 키가 같은 ID를 쓰고 있어 어느 쪽 사용량인지 가릴 수 없습니다. 설정 파일에서 키마다 다른 ID를 주세요.", "api.attribution.railAmbiguous": "ID 중복", // Claude Code inbound - "nav.claude": "Claude", "claude.subtitle": "Claude Code에서 GPT, Gemini 등 다른 모델도 쓸 수 있게 해줍니다.", "claude.enabledLabel": "Claude 연결", "claude.enabledHint": "끄면 Claude Code가 이 프록시를 사용할 수 없습니다.", @@ -1742,6 +1742,7 @@ export const ko: Record = { "cws.allCombos": "모든 콤보", "cws.copyModel": "ID 복사", "cws.copied": "복사됨", + "cws.tabsLabel": "콤보 상세 섹션", "cws.tab.config": "설정", "cws.tab.about": "정보", "cws.strategy": "전략", diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index f707bb5f5..d38a236bc 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -10,7 +10,6 @@ export const ru: Record = { "nav.providers": "Провайдеры", "nav.models": "Модели", "nav.combos": "Комбо", - "nav.routing": "Маршрутизация (beta)", "nav.subagents": "Подагенты", // routing intelligence @@ -440,6 +439,12 @@ export const ru: Record = { "prov.openaiApiSetup": "Настроить API-ключ", // models + "models.tab.catalog": "Модели", + "models.tab.combos": "Комбо", + "models.tab.routing": "Маршрутизация (beta)", + "models.tabsLabel": "Поверхности моделей", + "models.subtitle.combos": "Упорядоченные группы моделей, отвечающие под одним id. Failover пробует цели по порядку, round-robin распределяет нагрузку.", + "models.subtitle.routing": "Профили политик, оценка в режиме dry-run и аналитика маршрутизации с подтверждением источников.", "models.subtitle": "Управляйте тем, какие модели видит Codex — нативные GPT (сквозной проброс) и модели маршрутизируемых провайдеров, сгруппированные по провайдеру (нажмите на заголовок, чтобы свернуть группу). Скрытые модели исчезают из каталога и селектора, но остаются вызываемыми по точному id. Изменения применяются на следующем ходе Codex — opencodex сбрасывает 5-минутный кэш моделей Codex, поэтому перезапуск не требуется.", "models.nativeGroupLabel": "Нативные OpenAI", "models.nativeHint": "Модели сквозного проброса используют режим аккаунта (пул или прямое подключение), выбранный на странице «Провайдеры». Отключение модели скрывает её из селектора Codex (запись в каталоге сохраняется, поэтому при повторном включении она восстанавливается в точности).", @@ -447,10 +452,6 @@ export const ru: Record = { "models.workspace.providers": "Провайдеры", "models.workspace.allProviders": "Все провайдеры", "models.workspace.mainAria": "Сведения о моделях", - "models.combosEmpty": "Комбо ещё не настроены", - "models.combosSetup": "Настроить", - "models.combosAdd": "Добавить комбо", - "models.combosActive": "Активно: {count}", "models.allOn": "Все вкл.", "models.allOff": "Все выкл.", "models.cap350k": "Лимит 350k", @@ -1627,7 +1628,6 @@ export const ru: Record = { "api.attribution.ambiguous": "Два ключа используют один и тот же ID, поэтому нельзя определить, чьё это использование. Задайте каждому ключу уникальный ID в файле конфигурации.", "api.attribution.railAmbiguous": "дубль ID", // Claude Code inbound - "nav.claude": "Claude", "claude.subtitle": "Используйте GPT, Gemini и другие модели внутри Claude Code.", "claude.pageTitle": "Claude Code", "claude.workspace.settings": "Настройки", @@ -1825,6 +1825,7 @@ export const ru: Record = { "cws.allCombos": "Все комбо", "cws.copyModel": "Копировать id", "cws.copied": "Скопировано", + "cws.tabsLabel": "Разделы деталей комбо", "cws.tab.config": "Конфигурация", "cws.tab.about": "О комбо", "cws.strategy": "Стратегия", diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index 14113c611..ba9ca825e 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -10,7 +10,6 @@ export const zh: Record = { "nav.providers": "提供方", "nav.models": "模型", "nav.combos": "组合", - "nav.routing": "路由 (beta)", "nav.subagents": "子代理", // routing intelligence @@ -435,6 +434,12 @@ export const zh: Record = { "prov.openaiApiSetup": "设置 API 密钥", // models + "models.tab.catalog": "模型", + "models.tab.combos": "组合", + "models.tab.routing": "路由 (beta)", + "models.tabsLabel": "模型界面", + "models.subtitle.combos": "把多个模型合成一个 id 依次应答。failover 按顺序尝试，round-robin 分摊负载。", + "models.subtitle.routing": "策略配置、dry-run 评估，以及有据可查的路由分析。", "models.subtitle": "开关 Codex 可见的模型 — 原生 GPT passthrough 与已路由模型按提供方分组（点击标题可折叠）。隐藏的模型不会出现在目录和模型选择器中，但仍可按精确 id 直接调用。更改在下一个 Codex 回合生效 — opencodex 会使 Codex 的 5 分钟模型缓存失效，因此无需重启。", "models.nativeGroupLabel": "OpenAI 原生", "models.nativeHint": "Passthrough 模型使用在提供方页面选择的账户池或直连选项。关闭后会从 Codex 选择器中隐藏（目录条目保留，重新开启即可完整恢复）。", @@ -442,10 +447,6 @@ export const zh: Record = { "models.workspace.providers": "提供方", "models.workspace.allProviders": "所有提供方", "models.workspace.mainAria": "模型详情", - "models.combosEmpty": "尚未配置组合", - "models.combosSetup": "设置", - "models.combosAdd": "添加组合", - "models.combosActive": "{count} 个已启用", "models.allOn": "全部开启", "models.allOff": "全部关闭", "models.cap350k": "限制 350k", @@ -1197,7 +1198,6 @@ export const zh: Record = { "api.attribution.ambiguous": "两个密钥共用同一个 ID，无法判断用量属于哪一个。请在配置文件中为每个密钥设置唯一 ID。", "api.attribution.railAmbiguous": "ID 重复", // Claude Code inbound - "nav.claude": "Claude", "claude.subtitle": "在 Claude Code 中使用 GPT、Gemini 等其他模型。", "claude.enabledLabel": "Claude 连接", "claude.enabledHint": "关闭后 Claude Code 无法使用此代理。", @@ -1735,6 +1735,7 @@ export const zh: Record = { "cws.allCombos": "全部组合", "cws.copyModel": "复制 ID", "cws.copied": "已复制", + "cws.tabsLabel": "组合详情分区", "cws.tab.config": "配置", "cws.tab.about": "关于", "cws.strategy": "策略", diff --git a/gui/src/model-visibility.ts b/gui/src/model-visibility.ts index e422edc40..5007d979d 100644 --- a/gui/src/model-visibility.ts +++ b/gui/src/model-visibility.ts @@ -27,8 +27,9 @@ export function parseSelectedModels(value: unknown): ProviderModelMap { export async function fetchSelectedModels( apiBase: string, fetchImpl: typeof fetch = fetch, + signal?: AbortSignal, ): Promise { - const response = await fetchImpl(`${apiBase}/api/selected-models`); + const response = await fetchImpl(`${apiBase}/api/selected-models`, signal ? { signal } : undefined); if (!response.ok) throw new Error(`selected models HTTP ${response.status}`); return parseSelectedModels(await response.json()); } diff --git a/gui/src/pages/Combos.tsx b/gui/src/pages/Combos.tsx index 64f671d76..7b4fc86d6 100644 --- a/gui/src/pages/Combos.tsx +++ b/gui/src/pages/Combos.tsx @@ -52,10 +52,38 @@ function seedCombos(cacheKey: string): CachedCombosPage | null { return readSessionListCache(cacheKey); } -export default function Combos({ apiBase }: { apiBase: string }) { +export default function Combos({ + apiBase, + active = true, + onCountChange, +}: { + apiBase: string; + /** + * False while this panel is mounted but hidden behind another Models tab. It gates + * the NETWORK only — the rendered tree stays put so unsaved editor drafts survive a + * tab hop. Defaults true so the standalone page keeps its existing behaviour. + */ + active?: boolean; + /** Reports the combo count up to the tab strip. */ + onCountChange?: (count: number) => void; +}) { const t = useT(); const cacheKey = `ocx.combos.workspace.v1:${apiBase}`; const cached = useMemo(() => seedCombos(cacheKey), [cacheKey]); + + /* + * The last coherent payload, kept so a hidden panel can keep rendering. + * + * While `active` is false the resource is disabled and reports `data: undefined` with + * no skeleton and no error. Falling back to empty arrays there swaps the whole + * ComboWorkspace for a first-run empty state and takes every unsaved draft with it — + * proven in a browser: type into a combo, switch tabs, come back, field blank. + * + * State rather than a ref: this repo avoids render-time ref reads under React + * Compiler, and a ref would not re-render when the retained payload changes. Written + * on the load success path, never during render and never from an effect. + */ + const [retainedData, setRetainedData] = useState(cached ?? null); const [status, setStatus] = useState(""); const [statusOk, setStatusOk] = useState(false); const [adding, setAdding] = useState(false); @@ -75,12 +103,13 @@ export default function Combos({ apiBase }: { apiBase: string }) { return () => window.clearTimeout(timer); }, [status, statusOk]); - const loadCombos = useCallback(async (): Promise => { + const loadCombos = useCallback(async (signal?: AbortSignal): Promise => { // Keep all three requests parallel: this workspace is only coherent once every input arrives. const [combosRes, configRes, modelsRes] = await Promise.all([ - fetch(`${apiBase}/api/combos`), - fetch(`${apiBase}/api/config`), - fetch(`${apiBase}/api/models`), + // Signals were missing entirely, so resource cleanup could not cancel these. + fetch(`${apiBase}/api/combos`, { signal }), + fetch(`${apiBase}/api/config`, { signal }), + fetch(`${apiBase}/api/models`, { signal }), ]); if (!combosRes.ok || !configRes.ok || !modelsRes.ok) { throw new Error("combo workspace load failed"); @@ -151,6 +180,9 @@ export default function Combos({ apiBase }: { apiBase: string }) { const next = { combos, providers, models, cataloguedComboIds: [...catalogued] } satisfies CachedCombosPage; writeSessionListCache(cacheKey, next); + // Retain the coherent payload here — one place, on the success path, never during + // render. See the `retainedData` note below. + setRetainedData(next); return next; }, [apiBase, cacheKey]); @@ -158,11 +190,27 @@ export default function Combos({ apiBase }: { apiBase: string }) { cacheKey, [apiBase], loadCombos, - { isEmpty: () => false, initialData: cached ?? undefined }, + /* + * Gate the network, never the tree. A hidden panel must not fetch, but the rendered + * workspace has to stay mounted so an unsaved editor draft survives a tab hop. + * Disabling reports `data: undefined`, so `retainedData` below keeps the last good + * payload and the subtree never unmounts. + */ + { isEmpty: () => false, initialData: cached ?? undefined, enabled: active }, ); const { state } = resource; - const data = state.data; + + const data = state.data ?? retainedData ?? undefined; const combos = data?.combos ?? []; + + /* + * Report the count up to the tab strip from an effect keyed on the list length, not + * during render, so a parent re-render cannot refire it. + */ + useEffect(() => { + if (!data) return; + onCountChange?.(combos.length); + }, [combos.length, data, onCountChange]); const providers = data?.providers ?? []; const models = data?.models ?? []; const cataloguedComboIds = new Set(data?.cataloguedComboIds ?? []); @@ -225,7 +273,14 @@ export default function Combos({ apiBase }: { apiBase: string }) { return ; } - if (state.kind === "failed-cold") { + /* + * `!data` matters. Disabling the only subscriber schedules store eviction, so a + * reactivation whose fetch fails is classified `failed-cold` even when this component + * still holds a coherent retained payload — and replacing the workspace there would + * unmount the editor and destroy the very draft retention exists to protect. With + * retained data the workspace stays up and the failure shows in the stale banner below. + */ + if (state.kind === "failed-cold" && !data) { const reason = state.error instanceof Error ? state.error.message : t("cws.loadFailed"); return ( <> diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index e584c2ba3..3c94b66a8 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -1,16 +1,26 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Switch, Notice, EmptyState, Select, Tooltip } from "../ui"; -import { IconChevron, IconBoxes, IconInfo, IconShuffle, IconCheck, IconAlert } from "../icons"; +import { IconChevron, IconBoxes, IconInfo, IconCheck, IconAlert } from "../icons"; import { useT } from "../i18n/shared"; import type { TFn, TKey } from "../i18n/shared"; import { modelLabel } from "../model-display"; import { formatNamespacedModelId, formatProviderDisplayName, providerDisplaySlug } from "../provider-icons"; -import { type ComboItem, parseComboList } from "../combo-workspace-data"; import { readJsonIfOk, readJsonOrThrow } from "../fetch-json"; import { readSessionListCache, writeSessionListCache } from "../session-list-cache"; import { setClientResourceData } from "../client-resource"; import { useDataSurface } from "../data-surface"; import { DataSurfaceSkeleton } from "../components/data-surface"; +import ErrorBoundary from "../components/ErrorBoundary"; +import Combos from "./Combos"; +import RoutingProfiles from "./RoutingProfiles"; +import { ModelsTabStrip } from "./models-tab-strip"; +import { + modelsPanelDomId, + modelsTabDomId, + readModelsTab, + selectModelsTab, + type ModelsTab, +} from "./models-tab"; import { buildProviderModelGroups, type ConfiguredProviderSummary, @@ -34,11 +44,9 @@ import { fmtK, PAGE, readCollapsedProviders, - readCombosOpen, THREAD_OPTION_SET, THREAD_OPTIONS, writeCollapsedProviders, - writeCombosOpen, discoveryFailureLabel, type ModelRow, type ProviderContextCapsResponse, @@ -58,13 +66,53 @@ type CachedModelsPage = { contextCapValue: number; }; -/** Session JSON is untrusted — only seed rows that survive parseComboList (targets always arrays). */ -function readCachedCombos(value: unknown): ComboItem[] | null { - if (!Array.isArray(value)) return null; - return parseComboList({ combos: value }); -} +/** One subtitle per tab: only one panel is visible, so only one description applies. */ +const SUBTITLE_TKEY: Record = { + catalog: "models.subtitle", + combos: "models.subtitle.combos", + routing: "models.subtitle.routing", +}; export default function Models({ apiBase }: { apiBase: string }) { + /* + * Tab state. The hash is the source of truth, so refresh, bookmark, and + * Back/Forward keep the choice — same contract as `#logs` / `#logs/debug`. + * + * Panels mount lazily and then STAY mounted, hidden, so a half-typed combo draft + * survives a tab hop. The mounted set accumulates in the handler rather than an + * effect: an effect would cost a second render pass on every switch for a value both + * callers already know. + */ + const [tab, setTab] = useState(readModelsTab); + const [mounted, setMounted] = useState>(() => new Set([readModelsTab()])); + + const activateTab = useCallback((next: ModelsTab) => { + setTab(next); + setMounted(current => (current.has(next) ? current : new Set([...current, next]))); + }, []); + + useEffect(() => { + const syncFromHash = () => activateTab(readModelsTab()); + window.addEventListener("hashchange", syncFromHash); + window.addEventListener("popstate", syncFromHash); + return () => { + window.removeEventListener("hashchange", syncFromHash); + window.removeEventListener("popstate", syncFromHash); + }; + }, [activateTab]); + + const selectTab = useCallback((next: ModelsTab) => { + // Deliberate navigation: push a history entry so Back/Forward restore the tab. + selectModelsTab(next); + activateTab(next); + }, [activateTab]); + + const catalogActive = tab === "catalog"; + + /** Counts reported up by the panels that own the underlying lists. */ + const [comboCount, setComboCount] = useState(null); + const [routingCount, setRoutingCount] = useState(null); + const t: TFn = useT(); const cacheKey = `ocx.models.catalog.v1:${apiBase}`; const cached = useMemo(() => readSessionListCache(cacheKey), [cacheKey]); @@ -130,42 +178,9 @@ export default function Models({ apiBase }: { apiBase: string }) { const hoverTimerRef = useRef | null>(null); const [shadowCall, setShadowCall] = useState(null); const [shadowCallSaving, setShadowCallSaving] = useState(false); - // Combo summary section. null = cold load with no seed (pending strut). Failed reads stay - // null + combosError so an API error never masquerades as "no combos configured". - const combosCacheKey = `ocx.models.combos.v1:${apiBase}`; - const seededCombos = useMemo(() => { - const own = readCachedCombos(readSessionListCache(combosCacheKey)); - if (own !== null) return own; - // Reuse the Combos workspace session snapshot when Models opens first in the session. - const workspace = readSessionListCache<{ combos?: unknown }>(`ocx.combos.workspace.v1:${apiBase}`); - return readCachedCombos(workspace?.combos); - }, [apiBase, combosCacheKey]); - const combosResource = useDataSurface( - `models-combos:${apiBase}`, - [apiBase], - async (signal) => { - const r = await fetch(`${apiBase}/api/combos`, { signal }); - const j = await readJsonOrThrow(r); - const next = parseComboList(j); - writeSessionListCache(combosCacheKey, next); - return next; - }, - { isEmpty: () => false, initialData: seededCombos ?? undefined }, - ); - const combosState = combosResource.state; - // Keep a previously painted card on a later failure so the catalog does not yank down. - const combos = combosState.data ?? seededCombos; - // Announce failures even when stale/seeded rows remain (layout kept; freshness not faked). - const combosError = combosState.showError; - const [combosOpen, setCombosOpen] = useState(readCombosOpen); // App owns the in-session view mode; fallback to persisted mode for isolated renders/tests. const [selectedProvider, setSelectedProvider] = useState(null); - const toggleCombosOpen = () => { - const next = !combosOpen; - writeCombosOpen(next); - setCombosOpen(next); - }; useEffect(() => () => { if (hoverTimerRef.current) clearTimeout(hoverTimerRef.current); @@ -211,10 +226,12 @@ export default function Models({ apiBase }: { apiBase: string }) { const fetchCatalog = useCallback(async (signal: AbortSignal): Promise => { const [modelsRes, capsRes, providersRes, selectionData] = await Promise.all([ - fetch(`${apiBase}/api/models`), - fetch(`${apiBase}/api/provider-context-caps`), - fetch(`${apiBase}/api/providers`), - fetchSelectedModels(apiBase), + // Every request carries the resource signal, so leaving the catalog tab cancels + // the work rather than only discarding its result. + fetch(`${apiBase}/api/models`, { signal }), + fetch(`${apiBase}/api/provider-context-caps`, { signal }), + fetch(`${apiBase}/api/providers`, { signal }), + fetchSelectedModels(apiBase, fetch, signal), ]); const [data, capsData, providerData] = await Promise.all([ readJsonOrThrow(modelsRes), @@ -268,7 +285,9 @@ export default function Models({ apiBase }: { apiBase: string }) { applyCatalog(next); return next; }, - { isEmpty: () => false, pollMs: 10_000, initialData: cached ?? undefined }, + // Gated on the catalog tab: a 10-second poll that keeps running while the user + // reads Combos or Routing is exactly the hidden work this workspace avoids. + { isEmpty: () => false, pollMs: 10_000, initialData: cached ?? undefined, enabled: catalogActive }, ); const catalogState = catalogResource.state; @@ -295,6 +314,9 @@ export default function Models({ apiBase }: { apiBase: string }) { // Shadow/v2 controls must not wait on the models catalog (live discovery can be slow). useEffect(() => { + // Both belong to the catalog tab; a hidden panel polling /api/v2 every ten seconds + // is the same leak as the catalog poll above. + if (!catalogActive) return; const timeout = window.setTimeout(() => { void loadShadowCall(); void loadV2(); @@ -306,13 +328,21 @@ export default function Models({ apiBase }: { apiBase: string }) { window.clearTimeout(timeout); window.clearInterval(timer); }; - }, [loadShadowCall, loadV2]); + }, [catalogActive, loadShadowCall, loadV2]); const groups = useMemo( () => buildProviderModelGroups(models, providers), [models, providers], ); + /* + * The catalog count is only honest once a seed or a real response has landed. With + * the catalog gated, a cold load straight to `#models/combos` never fetches it, and + * rendering "0/0" would present unknown as fact. + */ + const catalogCountReady = models.length > 0 || catalogState.data !== undefined; + + // One-shot default collapse. It stays an effect on `groups` so CACHED groups collapse // immediately on first paint, even when revalidation is slow or fails; moving it into // the load() success path would render cached providers expanded and leave them @@ -338,6 +368,19 @@ export default function Models({ apiBase }: { apiBase: string }) { )).length; }, [disabled, models, selectedModels]); + /* + * Quiet per-tab counts. A count is omitted, never zeroed, while it is unknown: the + * panels report theirs up once mounted, and a tab that has never been opened has + * nothing truthful to say. + */ + const tabMeta = useMemo(() => ({ + catalog: catalogCountReady + ? t("models.active", { active: effectiveVisibleCount, total: models.length }) + : undefined, + combos: comboCount === null ? undefined : String(comboCount), + routing: routingCount === null ? undefined : String(routingCount), + }), [catalogCountReady, comboCount, effectiveVisibleCount, models.length, routingCount, t]); + const applyVisibility = async ( scope: ModelVisibilityScope, provider: string, @@ -658,22 +701,19 @@ export default function Models({ apiBase }: { apiBase: string }) { const catalog = catalogState.data ?? cached; - // A session seed keeps the workspace usable during the first shared-resource revalidation. - // Without a catalog, the skeleton owns the only live region for this transition. - if (catalogState.showSkeleton && !catalog) { - return ( - - ); - } - if (catalogState.kind === "failed-cold") { - const reason = catalogState.error instanceof Error ? catalogState.error.message : t("models.loadFail"); - return ( - <> - {reason} - - - ); - } + /* + * Catalog loading and cold failure belong to the CATALOG PANEL, not the page. + * + * These used to be component-level early returns, which is correct for a page that is + * only a catalog and wrong for a page that owns three tabs: a slow or failed catalog + * would unmount the whole workspace, tab strip included, taking every sibling panel + * and any unsaved combo draft with it — and on a cold failure the user could not even + * reach Combos or Routing. Rendered below inside the catalog panel instead. + */ + const catalogColdFailure = catalogState.kind === "failed-cold" + ? (catalogState.error instanceof Error ? catalogState.error.message : t("models.loadFail")) + : null; + const catalogCold = catalogState.showSkeleton && !catalog; const selectedModelMap = selectedModels ?? {}; @@ -1059,97 +1099,6 @@ export default function Models({ apiBase }: { apiBase: string }) { ); - const combosBlock = ( - <> - {/* Silent height strut: reserves the empty-card slot so a late /api/combos - cannot insert a row, without a bordered "Combos · Loading…" placeholder. */} - {combos === null && !combosError && ( -
- - {t("common.loading")} - -
- )} - {combos === null && combosError && ( -
-
-
-
- -
-
- )} - {combos !== null && combos.length === 0 && ( -
-
-
-
- {combosError ? ( - - ) : ( - {t("models.combosSetup")} - )} -
-
- )} - {combos !== null && combos.length > 0 && ( -
-
- - {combosError ? ( - - ) : ( - {t("models.combosSetup")} - )} -
- {combosOpen && ( -
- {combos.map(c => ( -
- {c.model} - {c.strategy} · {c.targets.length} -
- ))} - - + {t("models.combosAdd")} - -
- )} -
- )} - - ); - const collapseControls = (
+ + ) + : catalogPanel} + +
+ + {/* + The panel SHELL is always present; only its contents mount lazily. A conditional + wrapper left the tab's `aria-controls` pointing at an element that did not exist + until the tab had been visited once. + */} + + + + + ); + } diff --git a/gui/src/pages/RoutingProfiles.tsx b/gui/src/pages/RoutingProfiles.tsx index e8832d23c..56543c891 100644 --- a/gui/src/pages/RoutingProfiles.tsx +++ b/gui/src/pages/RoutingProfiles.tsx @@ -141,7 +141,20 @@ function selectedAfterLoad( return profiles[0] ?? null; } -export default function RoutingProfiles({ apiBase }: { apiBase: string }) { +export default function RoutingProfiles({ + apiBase, + active = true, + onCountChange, +}: { + apiBase: string; + /** + * False while this panel is mounted but hidden behind another Models tab. Defaults + * true so a direct render (tests) behaves like a visible panel. + */ + active?: boolean; + /** Reports the profile count up to the tab strip. */ + onCountChange?: (count: number) => void; +}) { const t = useT(); const unavailable = t("routing.unavailable"); const [profiles, setProfiles] = useState([]); @@ -163,6 +176,31 @@ export default function RoutingProfiles({ apiBase }: { apiBase: string }) { const [running, setRunning] = useState(false); const selectedRef = useRef(null); const loadGenerationRef = useRef(0); + /** Owned by `load` so every entry point — mount, Retry, save, delete — is cancellable. */ + const loadAbortRef = useRef(null); + /* + * Cancelling in-flight work is not enough on its own. A save or delete can resolve + * AFTER the panel is hidden or unmounted and then call `load()`, which would open a + * fresh controller and four requests that the deactivation effect has already run + * past — and whose generation is current, so its writes would land in a panel nobody + * is looking at. `load` checks this before it starts anything. + */ + const loadEnabledRef = useRef(true); + + /* + * Stop loading and cancel whatever is running. + * + * A stable callback rather than inline cleanup: reading the refs at cleanup time is + * the point — whatever load is in flight NOW is what must be cancelled, and the + * generation has to move past the value that load captured. Inline, that reads as a + * stale-ref mistake to both the linter and the next reader. Naming it says the + * latest-value read is deliberate, and it works for deactivation and unmount alike. + */ + const cancelActiveLoad = useCallback(() => { + loadEnabledRef.current = false; + loadAbortRef.current?.abort(); + loadGenerationRef.current++; + }, []); const dryRunGenerationRef = useRef(0); const notify = useCallback((message: string, ok: boolean) => { @@ -191,14 +229,27 @@ export default function RoutingProfiles({ apiBase }: { apiBase: string }) { }, [clearDryRun]); const load = useCallback(async (preferredId?: string) => { + if (!loadEnabledRef.current) return; + /* + * `load` owns the controller, not the effect that happens to call it. + * + * There are four entry points — the mount effect, Retry, post-save, and + * post-delete — so an effect-local controller would cancel only the first and let + * a Retry or a mutation reload keep running after the tab hides. Generation + * invalidation stops the state write but not the network work. + */ + loadAbortRef.current?.abort(); + const controller = new AbortController(); + loadAbortRef.current = controller; + const { signal } = controller; const generation = ++loadGenerationRef.current; setLoadError(""); try { const [profilesRes, analyticsRes, configRes, modelsRes] = await Promise.all([ - fetch(`${apiBase}/api/routing-profiles`), - fetch(`${apiBase}/api/routing-analytics`), - fetch(`${apiBase}/api/config`), - fetch(`${apiBase}/api/models`), + fetch(`${apiBase}/api/routing-profiles`, { signal }), + fetch(`${apiBase}/api/routing-analytics`, { signal }), + fetch(`${apiBase}/api/config`, { signal }), + fetch(`${apiBase}/api/models`, { signal }), ]); if (!profilesRes.ok) throw new Error(`load-${profilesRes.status}`); const [profilesJson, analyticsJson, configJson, modelsJson] = await Promise.all([ @@ -236,14 +287,38 @@ export default function RoutingProfiles({ apiBase }: { apiBase: string }) { } } catch (error) { if (generation !== loadGenerationRef.current) return; + // An aborted supersede or deactivate is not a failure worth showing. + if (signal.aborted) return; setLoadError(error instanceof Error ? error.message : String(error)); + } finally { + // Clear only if this request still owns the ref; a newer load may have replaced it. + if (loadAbortRef.current === controller) loadAbortRef.current = null; } }, [apiBase, clearDryRun]); useEffect(() => { + if (!active) { + // Hidden: stop new loads, cancel work in flight, and invalidate its generation so + // a late resolve cannot write into a panel nobody is looking at. + cancelActiveLoad(); + return; + } + loadEnabledRef.current = true; const timer = window.setTimeout(() => void load(), 0); - return () => window.clearTimeout(timer); - }, [load]); + // Unmounting counts too — leaving Models entirely must not strand a request. + return () => { + window.clearTimeout(timer); + cancelActiveLoad(); + }; + }, [active, cancelActiveLoad, load]); + + /* + * Report the count up to the tab strip from an effect keyed on the list length, not + * during render. + */ + useEffect(() => { + onCountChange?.(profiles.length); + }, [onCountChange, profiles.length]); const firstProvider = providerNames[0] ?? ""; const firstModel = providerDefaults[firstProvider] @@ -417,16 +492,19 @@ export default function RoutingProfiles({ apiBase }: { apiBase: string }) { return (
-
-

{t("routing.title")}

-
- - -
+ {/* + Embedded as a Models tab, so the page title and subtitle belong to the shell. + Rendering them here too put "Routing Intelligence (beta)" and its description on + screen twice — visible the moment the panel was opened in a browser, invisible to + every static gate. The actions stay; a heading cannot carry buttons, so they sit + in a plain toolbar row. + */} +
+ +
-

{t("routing.subtitle")}

{loadError ? {t("routing.loadFailed")}: {loadError} : null} {status ? {status.message} : null} diff --git a/gui/src/pages/models-shared.ts b/gui/src/pages/models-shared.ts index 75b1f71c3..ab845a13c 100644 --- a/gui/src/pages/models-shared.ts +++ b/gui/src/pages/models-shared.ts @@ -68,8 +68,6 @@ export const THREAD_OPTION_SET = new Set(THREAD_OPTIONS); export const PAGE = 60; // rows rendered per provider before a "show more" export const COLLAPSED_KEY_V2 = "ocx-models-collapsed:v2"; -export const COMBOS_OPEN_KEY_V1 = "ocx-models-combos-open:v1"; -export const COMBOS_OPEN_KEY_LEGACY = "ocx-models-combos-open"; /** Compact token display (350k) — unit is technical, not prose. */ export function fmtK(n: number): string { @@ -125,19 +123,4 @@ export function writeCollapsedProviders(collapsed: Set, storage: Storage } } -export function readCombosOpen(storage: StorageLike = localStorage): boolean { - try { - const saved = storage.getItem(COMBOS_OPEN_KEY_V1) ?? storage.getItem(COMBOS_OPEN_KEY_LEGACY); - return saved === "1"; - } catch { - return false; - } -} -export function writeCombosOpen(open: boolean, storage: StorageLike = localStorage): void { - try { - storage.setItem(COMBOS_OPEN_KEY_V1, open ? "1" : "0"); - } catch { - /* quota / private-mode */ - } -} diff --git a/gui/src/pages/models-tab-strip.tsx b/gui/src/pages/models-tab-strip.tsx new file mode 100644 index 000000000..c60f1958b --- /dev/null +++ b/gui/src/pages/models-tab-strip.tsx @@ -0,0 +1,92 @@ +/** + * The Models page tab strip. + * + * Underline page tabs, the same vocabulary Logs, Dashboard, and Integrations use. ARIA + * wiring follows the APG tabs pattern: `tab` elements inside a `tablist`, roving + * tabindex (0 on the active tab, -1 on the rest), `aria-controls` to the panel, and + * Arrow/Home/End traversal. + */ +import type { KeyboardEvent } from "react"; +import { useRef } from "react"; +import { useT, type TKey } from "../i18n/shared"; +import { + MODELS_TABS, + modelsPanelDomId, + modelsTabDomId, + type ModelsTab, +} from "./models-tab"; + +const TAB_LABEL: Record = { + catalog: "models.tab.catalog", + combos: "models.tab.combos", + routing: "models.tab.routing", +}; + +export function ModelsTabStrip({ + tab, + onSelect, + meta, +}: { + tab: ModelsTab; + onSelect: (next: ModelsTab) => void; + /** + * Quiet per-tab counts. A tab whose count is not yet known is omitted rather than + * rendered as zero — an unknown catalog would otherwise claim "0/0" on a cold load + * that never fetched it, and a wrong count is worse than none. + */ + meta?: Partial>; +}) { + const t = useT(); + const refs = useRef | null>(null); + if (refs.current === null) refs.current = new Map(); + + const move = (next: ModelsTab) => { + onSelect(next); + // Focus follows selection, so keyboard traversal lands where the eye does. + window.requestAnimationFrame(() => { + refs.current!.get(next)?.focus({ preventScroll: true }); + }); + }; + + const onKeyDown = (event: KeyboardEvent) => { + const index = MODELS_TABS.indexOf(tab); + let nextIndex: number | null = null; + if (event.key === "ArrowLeft") nextIndex = (index - 1 + MODELS_TABS.length) % MODELS_TABS.length; + else if (event.key === "ArrowRight") nextIndex = (index + 1) % MODELS_TABS.length; + else if (event.key === "Home") nextIndex = 0; + else if (event.key === "End") nextIndex = MODELS_TABS.length - 1; + if (nextIndex === null) return; + event.preventDefault(); + move(MODELS_TABS[nextIndex]!); + }; + + return ( +
+ {MODELS_TABS.map(candidate => { + const active = candidate === tab; + const count = meta?.[candidate]; + return ( + + ); + })} +
+ ); +} diff --git a/gui/src/pages/models-tab.ts b/gui/src/pages/models-tab.ts new file mode 100644 index 000000000..28bc30f46 --- /dev/null +++ b/gui/src/pages/models-tab.ts @@ -0,0 +1,51 @@ +/** + * Models tab identity and hash mapping. + * + * Mirrors `logs-tab-keydown.ts`: the hash is the source of truth, so refresh, bookmark, + * and Back/Forward all keep the tab choice. Kept out of `Models.tsx` because that file + * is already large and because the tests want to import this directly. + */ + +import { navigateHash, normalizeHashPath } from "../hash-routing"; + +/** + * `catalog` rather than `models` for the first tab: the page is Models and its first + * tab shows the plain model list, so a distinct id keeps "the page" and "the tab" from + * ever having to be disambiguated in code. The visible label is still "Models". + */ +export type ModelsTab = "catalog" | "combos" | "routing"; + +export const MODELS_TABS: readonly ModelsTab[] = ["catalog", "combos", "routing"]; + +export function modelsTabHash(tab: ModelsTab): string { + return tab === "catalog" ? "models" : `models/${tab}`; +} + +/** + * Legacy top-level hashes resolve here too, and that is not redundancy with the + * resolver's redirect. + * + * The redirect rewrites `#combos` to `#models/combos` with replaceState, which + * deliberately emits no `hashchange`. Tab state is therefore initialized from the + * ORIGINAL hash: recognising only the nested form would land a cold load at `#combos` + * on the catalog while the URL claimed Combos. + */ +export function readModelsTab(hash = window.location.hash): ModelsTab { + const raw = normalizeHashPath(hash); + if (raw === "models/combos" || raw === "combos" || raw.startsWith("combos/")) return "combos"; + if (raw === "models/routing" || raw === "routing" || raw.startsWith("routing/")) return "routing"; + return "catalog"; +} + +/** Deliberate navigation: pushes a history entry so Back/Forward restore the tab. */ +export function selectModelsTab(next: ModelsTab): void { + navigateHash(modelsTabHash(next)); +} + +export function modelsTabDomId(tab: ModelsTab): string { + return `models-tab-${tab}`; +} + +export function modelsPanelDomId(tab: ModelsTab): string { + return `models-panel-${tab}`; +} diff --git a/gui/src/styles-combos-workspace.css b/gui/src/styles-combos-workspace.css index f73aa3b8a..58279c128 100644 --- a/gui/src/styles-combos-workspace.css +++ b/gui/src/styles-combos-workspace.css @@ -217,37 +217,41 @@ flex-wrap: wrap; } -.combos-workspace-tabs { - display: flex; - gap: 4px; +/* + Pill group for the detail panel's Config/About switch. Mirrors `.models-segmented`; + `.segmented` has no standalone declaration in this codebase, so every use pairs it + with a concrete class. It replaced an underline row that would have stacked under the + Models page tab strip. +*/ +.combos-workspace-segmented { + display: inline-flex; + border: 1px solid var(--border); + border-radius: var(--radius-pill); + background: var(--surface); + padding: 2px; + gap: 2px; margin-bottom: 16px; - border-bottom: 1px solid var(--border-soft); } -.combos-workspace-tab { - appearance: none; +.combos-workspace-segmented .btn { + border-radius: var(--radius-pill); + min-width: 0; + min-height: 0; + padding: 4px 12px; border: none; - background: none; - font: inherit; - font-size: var(--text-control); - font-weight: 500; - color: var(--muted); - padding: 8px 12px; - cursor: pointer; - border-bottom: 2px solid transparent; - margin-bottom: -1px; -} - -.combos-workspace-tab:hover { - color: var(--text); + font-size: var(--text-label); + line-height: inherit; } -.combos-workspace-tab.combos-workspace-tab--active { - color: var(--text); - border-bottom-color: var(--accent); -} +/* + `:not([hidden])`, not a bare `display: flex`. -.combos-workspace-tab-content { + Both panels stay in the tree so each tab's `aria-controls` resolves, and the inactive + one carries `hidden`. But author CSS beats the UA's `[hidden] { display: none }`, so a + plain `display: flex` here left BOTH panels on screen at once — Config and About + stacked, one of them marked hidden and rendering anyway. +*/ +.combos-workspace-tab-content:not([hidden]) { display: flex; flex-direction: column; gap: 16px; diff --git a/gui/src/styles-models-workspace.css b/gui/src/styles-models-workspace.css index 0504ac31e..881bbd904 100644 --- a/gui/src/styles-models-workspace.css +++ b/gui/src/styles-models-workspace.css @@ -5,7 +5,17 @@ Uses only design tokens from styles.css. No gradients. ============================================================================ */ -.main-inner:has(.models-workspace-shell) { +/* + The catalog wants a wider column than the 980px default. + + Scoped to a VISIBLE catalog panel, not merely a present one: panels mount lazily and + then stay mounted so drafts survive a tab hop, so a bare `:has(.models-workspace-shell)` + keeps matching after the catalog has been opened once. Routing would then render at + 980px on a direct visit and 1200px afterwards — a width that depends on browsing + history. No surface renders the shell outside a tabpanel any more, so the old + direct-child arm is gone with the standalone pages it served. +*/ +.main-inner:has(#models-panel-catalog:not([hidden]) .models-workspace-shell) { max-width: 1200px; } @@ -16,6 +26,14 @@ container-name: models-workspace; } +/* + Tab panels. Inactive panels carry `hidden`, which the UA stylesheet renders as + display:none, so they take no space and leave the focus order — no rule needed for + that. `--fill` marks the panel that owns a full-height workspace; the height chain + that feeds it lives with the combos rules in styles.css. +*/ +.models-tab-panel { min-width: 0; } + .models-workspace-root { display: grid; grid-template-columns: minmax(240px, 280px) minmax(0, 1fr); diff --git a/gui/src/styles.css b/gui/src/styles.css index cb41c986d..576122d8c 100644 --- a/gui/src/styles.css +++ b/gui/src/styles.css @@ -396,7 +396,21 @@ input[type="checkbox"], input[type="radio"] { accent-color: var(--accent); } display: flex; flex-direction: column; } -.main-inner.main-inner--combos > .combos-workspace-shell { +/* + The combos workspace is a full-bleed 100dvh shell, so whatever owns the remaining + height has to be a flexible, shrinkable column. + + The shell now reaches this container inside its tabpanel, one level down. That extra + level is exactly what breaks a plain direct-child rule, so the panel becomes the flex + item and the shell fills it. +*/ +/* + `:not([hidden])` on the panel: panel shells stay mounted so every tab's + `aria-controls` resolves, and author `display: flex` would otherwise beat the UA's + `[hidden] { display: none }` and paint a hidden panel anyway. +*/ +.main-inner.main-inner--combos > .models-tab-panel--fill:not([hidden]), +.main-inner.main-inner--combos > .models-tab-panel--fill:not([hidden]) > .combos-workspace-shell { flex: 1 1 auto; min-height: 0; height: 100%; @@ -404,6 +418,19 @@ input[type="checkbox"], input[type="radio"] { accent-color: var(--accent); } flex-direction: column; } +/* + `.main-inner--combos` zeroes the container padding, so the page chrome above the + workspace has to bring its own back. `flex-shrink: 0` keeps the header, tab strip, + and subtitle from being squeezed when the workspace wants the room. +*/ +.main-inner.main-inner--combos > .page-head, +.main-inner.main-inner--combos > .page-tabs, +.main-inner.main-inner--combos > .page-sub { + flex-shrink: 0; + padding-inline: 36px; +} +.main-inner.main-inner--combos > .page-sub { margin-bottom: 10px; } + /* ---- page header ---- */ .page-head { display: flex; align-items: center; justify-content: space-between; gap: 16px; margin-bottom: 6px; } .page-head h2 { font-size: var(--text-title); } @@ -2020,6 +2047,9 @@ button.prov-account-row.active { cursor: default; } .main-inner { padding: 22px 18px 48px; } /* The mobile app grid already reserves the top-bar row; fill only its remaining main row. */ .main-inner.main-inner--combos { padding: 0; min-height: 0; height: 100%; overflow: hidden; } + .main-inner.main-inner--combos > .page-head, + .main-inner.main-inner--combos > .page-tabs, + .main-inner.main-inner--combos > .page-sub { padding-inline: 18px; } /* settings rows: copy takes the full width, controls drop underneath */ .setting-row { flex-wrap: wrap; } .setting-row .setting-copy { flex: 1 1 100% !important; } diff --git a/gui/tests/combos-detail-segmented.test.ts b/gui/tests/combos-detail-segmented.test.ts new file mode 100644 index 000000000..39be280fb --- /dev/null +++ b/gui/tests/combos-detail-segmented.test.ts @@ -0,0 +1,43 @@ +/** + * The Combos detail panel's Config/About switch. + * + * Combos is a tab of the Models page now, so an underline row here would sit directly + * beneath the page tab strip — two rows of the same visual language stacked, which + * reads as two levels of navigation rather than one page's facets. Primer names this + * directly in its UnderlineNav guidance. + * + * The roles stay tab semantics because they control a real tabpanel; only the styling + * changed. That distinction is what these assertions protect: a future "cleanup" that + * converts them to a radiogroup would misdescribe the widget. + */ +import { expect, test } from "bun:test"; + +const panel = await Bun.file( + new URL("../src/components/combo-workspace-detail-panel.tsx", import.meta.url), +).text(); +const css = await Bun.file( + new URL("../src/styles-combos-workspace.css", import.meta.url), +).text(); + +test("the detail switch renders as a segmented pill, not an underline row", () => { + expect(panel).toContain('className="segmented combos-workspace-segmented"'); + // The old underline classes are gone from both the markup and the stylesheet. + expect(panel).not.toContain("combos-workspace-tab--active"); + expect(css).not.toContain(".combos-workspace-tab {"); + expect(css).not.toContain(".combos-workspace-tabs {"); +}); + +test("it keeps tab semantics because it controls a real tabpanel", () => { + expect(panel).toContain('role="tablist"'); + expect(panel).toContain('role="tab"'); + expect(panel).toContain("aria-selected={tab ==="); + expect(panel).toContain('role="tabpanel"'); + // A filter shape would be wrong here: these switch a panel, they do not filter rows. + expect(panel).not.toContain('role="radiogroup"'); +}); + +test("the pill group has its own concrete styling, since .segmented alone has none", () => { + expect(css).toContain(".combos-workspace-segmented {"); + expect(css).toContain(".combos-workspace-segmented .btn {"); + expect(css).toContain("border-radius: var(--radius-pill)"); +}); diff --git a/gui/tests/combos-detail-tabs-dom.test.tsx b/gui/tests/combos-detail-tabs-dom.test.tsx new file mode 100644 index 000000000..94f6e8790 --- /dev/null +++ b/gui/tests/combos-detail-tabs-dom.test.tsx @@ -0,0 +1,141 @@ +/** + * The combo detail tablist, mounted. + * + * `combos-detail-segmented.test.ts` pins the markup and stylesheet as text, which is + * proportionate for a styling change but blind to two things that actually broke here: + * a tab whose `aria-controls` pointed at an element that did not exist, and an author + * `display: flex` overriding the UA's `[hidden] { display: none }` so a hidden panel + * rendered anyway. Both need a DOM. + */ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import { DetailPanel } from "../src/components/combo-workspace-detail-panel"; +import { LanguageProvider } from "../src/i18n/provider"; +import { emptyDraft } from "../src/combo-workspace-data"; + +const globals = ["document", "window", "navigator", "localStorage", "sessionStorage"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; + +beforeEach(() => { + previousGlobals = Object.fromEntries(globals.map(k => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/#models/combos" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow.window }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + sessionStorage: { configurable: true, value: testWindow.sessionStorage }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +}); + +afterEach(() => { + testWindow.close(); + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +async function mountDetail(): Promise<{ container: HTMLElement; root: Root }> { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render( + + ({ ok: true })} + onDirtyChange={() => {}} + /> + , + ); + }); + /* + * DetailPanel resets its tab to `config` from a zero-delay timer keyed on the + * baseline. Let that settle before the test drives anything, or the assertion races + * a reset it did not ask for. + */ + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + return { container, root }; +} + +const tabs = (c: HTMLElement) => [...c.querySelectorAll('[role="tab"]')] as HTMLButtonElement[]; +const panels = (c: HTMLElement) => [...c.querySelectorAll('[role="tabpanel"]')] as HTMLElement[]; + +test("both tabs control an element that exists", () => { + return mountDetail().then(async ({ container, root }) => { + try { + const controls = tabs(container).map(t => t.getAttribute("aria-controls")!); + expect(controls).toHaveLength(2); + for (const id of controls) expect(container.querySelector(`#${id}`)).toBeTruthy(); + // Each panel names the tab that owns it, not just whichever is active. + for (const p of panels(container)) { + expect(container.querySelector(`#${p.getAttribute("aria-labelledby")}`)).toBeTruthy(); + } + } finally { + await act(async () => root.unmount()); + } + }); +}); + +test("exactly one panel is exposed at a time", async () => { + const { container, root } = await mountDetail(); + try { + const visible = () => panels(container).filter(p => !p.hasAttribute("hidden")); + expect(visible()).toHaveLength(1); + expect(visible()[0]!.id).toBe("cws-detail-panel-config"); + + await act(async () => { (container.querySelector("#cws-detail-tab-about") as HTMLButtonElement).click(); }); + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + expect(visible()).toHaveLength(1); + expect(visible()[0]!.id).toBe("cws-detail-panel-about"); + } finally { + await act(async () => root.unmount()); + } +}); + +test("roving tabindex keeps the tablist to one tab stop", async () => { + const { container, root } = await mountDetail(); + try { + expect(tabs(container).filter(t => t.tabIndex === 0)).toHaveLength(1); + await act(async () => { (container.querySelector("#cws-detail-tab-about") as HTMLButtonElement).click(); }); + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + const inOrder = tabs(container).filter(t => t.tabIndex === 0); + expect(inOrder).toHaveLength(1); + expect(inOrder[0]!.id).toBe("cws-detail-tab-about"); + } finally { + await act(async () => root.unmount()); + } +}); + +test("the About panel is focusable, since it holds nothing focusable itself", async () => { + const { container, root } = await mountDetail(); + try { + const about = container.querySelector("#cws-detail-panel-about") as HTMLElement; + expect(about.tabIndex).toBe(0); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * The cascade bug: author `display: flex` beat `[hidden] { display: none }`, so both + * panels painted at once. The stylesheet is the only place this contract lives. + */ +test("the panel rule is scoped so a hidden panel cannot paint", async () => { + const css = await Bun.file(new URL("../src/styles-combos-workspace.css", import.meta.url)).text(); + expect(css).toContain(".combos-workspace-tab-content:not([hidden])"); + expect(css).not.toMatch(/\.combos-workspace-tab-content\s*\{/); +}); diff --git a/gui/tests/models-workspace-panels.test.tsx b/gui/tests/models-workspace-panels.test.tsx new file mode 100644 index 000000000..93ec55893 --- /dev/null +++ b/gui/tests/models-workspace-panels.test.tsx @@ -0,0 +1,405 @@ +/** + * Models tab workspace — mounted behaviour. + * + * The routing helpers are unit-tested at `tests/models-workspace-tabs.test.ts`. This file + * exists because those assertions cannot see the failures that actually happened here: + * a component-level early return that unmounted the whole tab tree while the catalog + * loaded, and a disabled resource that swapped the combo editor for an empty state and + * destroyed an unsaved draft. Both passed typecheck, lint, and every source-string + * assertion. Only mounting the thing catches them. + */ +import { afterEach, beforeEach, expect, jest, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import Models from "../src/pages/Models"; +import { LanguageProvider } from "../src/i18n/provider"; +import { clearClientResourceStoresForTests } from "../src/client-resource"; + +const globals = ["document", "window", "navigator", "localStorage", "sessionStorage"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; +const originalFetch = globalThis.fetch; + +const API_BASE = "http://localhost"; + +/** Every catalog/combos/routing endpoint the workspace can reach, with counted hits. */ +function installFetch(): { hits: Map } { + const hits = new Map(); + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = String(input); + const key = url.replace(API_BASE, "").split("?")[0]!; + hits.set(key, (hits.get(key) ?? 0) + 1); + if (url.includes("/api/models")) { + return Response.json([ + { provider: "openai", id: "gpt-5", namespaced: "openai/gpt-5", native: true }, + { provider: "anthropic", id: "claude", namespaced: "anthropic/claude" }, + ]); + } + if (url.includes("/api/provider-context-caps")) return Response.json({ providers: {} }); + if (url.includes("/api/providers")) return Response.json([{ name: "openai", disabled: false }]); + if (url.includes("/api/selected-models")) return Response.json({}); + if (url.includes("/api/combos")) return Response.json([]); + if (url.includes("/api/config")) return Response.json({ providers: { openai: { defaultModel: "gpt-5" } } }); + if (url.includes("/api/routing-profiles")) return Response.json([]); + if (url.includes("/api/routing-analytics")) return Response.json(null); + if (url.includes("/api/shadow-call-settings")) return Response.json({ enabled: false }); + if (url.includes("/api/v2")) return new Response(null, { status: 404 }); + return new Response(null, { status: 404 }); + }) as typeof fetch; + return { hits }; +} + +beforeEach(() => { + clearClientResourceStoresForTests(); + previousGlobals = Object.fromEntries(globals.map(k => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/#models" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow.window }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + sessionStorage: { configurable: true, value: testWindow.sessionStorage }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + clearClientResourceStoresForTests(); + testWindow.close(); + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +async function mountModels(): Promise<{ container: HTMLElement; root: Root }> { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render( + + + , + ); + }); + await act(async () => { await Promise.resolve(); }); + return { container, root }; +} + +const tabs = (container: HTMLElement) => [...container.querySelectorAll('[role="tab"]')] as HTMLButtonElement[]; +const panel = (container: HTMLElement, id: string) => container.querySelector(`#models-panel-${id}`); + +test("the strip renders all three tabs with the catalog selected on the bare hash", async () => { + installFetch(); + const { container, root } = await mountModels(); + try { + expect(tabs(container).map(t => t.id)).toEqual([ + "models-tab-catalog", "models-tab-combos", "models-tab-routing", + ]); + const selected = tabs(container).filter(t => t.getAttribute("aria-selected") === "true"); + expect(selected).toHaveLength(1); + expect(selected[0]!.id).toBe("models-tab-catalog"); + // Roving tabindex: exactly one tab is in the tab order. + expect(tabs(container).filter(t => t.tabIndex === 0)).toHaveLength(1); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * The regression that shipped and had to be fixed: catalog loading and cold failure were + * component-level early returns, so a slow catalog replaced the entire workspace — strip + * included — and a cold failure left Combos and Routing unreachable. + */ +test("a cold catalog never removes the tab strip", async () => { + let releaseCatalog!: () => void; + const gate = new Promise(resolve => { releaseCatalog = resolve; }); + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/api/models")) { await gate; return Response.json([]); } + if (url.includes("/api/provider-context-caps")) return Response.json({ providers: {} }); + if (url.includes("/api/providers")) return Response.json([]); + if (url.includes("/api/selected-models")) return Response.json({}); + return new Response(null, { status: 404 }); + }) as typeof fetch; + + const { container, root } = await mountModels(); + try { + // Still cold here: the catalog fetch is parked on the gate. + expect(tabs(container)).toHaveLength(3); + expect(panel(container, "catalog")).toBeTruthy(); + releaseCatalog(); + await act(async () => { await Promise.resolve(); }); + expect(tabs(container)).toHaveLength(3); + } finally { + await act(async () => root.unmount()); + } +}); + +test("a cold catalog failure still lets the user reach another tab", async () => { + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/api/models")) throw new Error("catalog down"); + if (url.includes("/api/provider-context-caps")) return Response.json({ providers: {} }); + if (url.includes("/api/providers")) return Response.json([]); + if (url.includes("/api/selected-models")) return Response.json({}); + if (url.includes("/api/routing-profiles")) return Response.json([]); + if (url.includes("/api/routing-analytics")) return Response.json(null); + if (url.includes("/api/config")) return Response.json({ providers: {} }); + return new Response(null, { status: 404 }); + }) as typeof fetch; + + const { container, root } = await mountModels(); + try { + await act(async () => { await Promise.resolve(); }); + expect(tabs(container)).toHaveLength(3); + + // "Reachable" has to mean the click works and the panel actually appears — asserting + // the button merely exists would pass with a dead tab. + await act(async () => { + (container.querySelector("#models-tab-routing") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + expect(panel(container, "routing")).toBeTruthy(); + expect(panel(container, "routing")?.hasAttribute("hidden")).toBe(false); + } finally { + await act(async () => root.unmount()); + } +}); + +test("panel shells are always present; contents mount lazily and then stay", async () => { + installFetch(); + const { container, root } = await mountModels(); + try { + /* + * The SHELL exists from the first render so every tab's `aria-controls` resolves; + * a conditional wrapper left the unvisited tab pointing at nothing. The shell is + * empty until visited, which is what keeps the lazy part lazy. + */ + expect(panel(container, "combos")).toBeTruthy(); + expect(panel(container, "combos")?.children).toHaveLength(0); + expect(panel(container, "routing")?.children).toHaveLength(0); + + await act(async () => { + (container.querySelector("#models-tab-combos") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + expect(panel(container, "combos")!.children.length).toBeGreaterThan(0); + expect(panel(container, "catalog")?.hasAttribute("hidden")).toBe(true); + + await act(async () => { + (container.querySelector("#models-tab-catalog") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + // Still mounted, just hidden — this is what lets an unsaved draft survive. + expect(panel(container, "combos")!.children.length).toBeGreaterThan(0); + expect(panel(container, "combos")?.hasAttribute("hidden")).toBe(true); + expect(panel(container, "catalog")?.hasAttribute("hidden")).toBe(false); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * Every tab's `aria-controls` must resolve from the first render, including for tabs + * that have never been opened. A conditional panel wrapper broke this silently. + */ +test("every tab controls an element that exists before it is visited", async () => { + installFetch(); + const { container, root } = await mountModels(); + try { + for (const tabEl of tabs(container)) { + const target = tabEl.getAttribute("aria-controls")!; + expect(container.querySelector(`#${target}`)).toBeTruthy(); + } + } finally { + await act(async () => root.unmount()); + } +}); + +test("every rendered panel is wired to its tab", async () => { + installFetch(); + const { container, root } = await mountModels(); + try { + for (const id of ["combos", "routing"]) { + await act(async () => { + (container.querySelector(`#models-tab-${id}`) as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + } + for (const id of ["catalog", "combos", "routing"]) { + const p = panel(container, id)!; + expect(p.getAttribute("role")).toBe("tabpanel"); + expect(p.getAttribute("aria-labelledby")).toBe(`models-tab-${id}`); + const tab = container.querySelector(`#models-tab-${id}`)!; + expect(tab.getAttribute("aria-controls")).toBe(`models-panel-${id}`); + } + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * The ARIA test above pins wiring, not isolation — it would pass with every boundary + * deleted. This one makes a panel actually throw. App's boundary is keyed by page, and + * all three tabs are now one page, so without per-panel boundaries one broken tab would + * take the whole workspace down and stay broken across a switch. + */ +test("a panel load failure does not take its siblings with it", async () => { + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/api/models")) return Response.json([]); + if (url.includes("/api/provider-context-caps")) return Response.json({ providers: {} }); + if (url.includes("/api/providers")) return Response.json([]); + if (url.includes("/api/selected-models")) return Response.json({}); + // A shape the combos loader cannot parse into a coherent page. + if (url.includes("/api/combos")) return Response.json({ combos: { not: "an array" } }); + if (url.includes("/api/config")) return Response.json(null); + if (url.includes("/api/routing-profiles")) return Response.json([]); + if (url.includes("/api/routing-analytics")) return Response.json(null); + return new Response(null, { status: 404 }); + }) as typeof fetch; + + const { container, root } = await mountModels(); + try { + await act(async () => { + (container.querySelector("#models-tab-combos") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + + // Whatever Combos did, the strip and the other tabs must still be usable. This is a + // failed LOAD, not a render throw — the boundary mechanism itself is covered by + // error-boundary.test.tsx; what matters here is that one panel's failure is + // contained. + expect(tabs(container)).toHaveLength(3); + await act(async () => { + (container.querySelector("#models-tab-routing") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + expect(panel(container, "routing")?.hasAttribute("hidden")).toBe(false); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * The whole point of gating: a hidden catalog must stop polling. + * + * The first version waited 60ms against a 10-SECOND poll interval, so it passed whether + * or not the poll was gated — a green assertion proving nothing. Fake timers advance + * past a full period without spending it, following `logs-auto-refresh.test.tsx`. The + * counted endpoint is catalog-exclusive; `/api/models` is requested by several panels. + */ +test("a hidden catalog stops polling across a full interval", async () => { + const { hits } = installFetch(); + jest.useFakeTimers({ now: 1_700_000_000_000 }); + const { container, root } = await mountModels(); + try { + await act(async () => { await Promise.resolve(); }); + expect(hits.get("/api/provider-context-caps") ?? 0).toBeGreaterThan(0); + + await act(async () => { + (container.querySelector("#models-tab-routing") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + const afterSwitch = hits.get("/api/provider-context-caps") ?? 0; + + // Past one full poll period. A shorter advance cannot tell a gated poll from an + // ungated one, which is exactly how the first version of this test lied. + await act(async () => { jest.advanceTimersByTime(11_000); }); + await act(async () => { await Promise.resolve(); }); + expect(hits.get("/api/provider-context-caps") ?? 0).toBe(afterSwitch); + } finally { + await act(async () => root.unmount()); + jest.useRealTimers(); + } +}); + +/* + * The failure that actually shipped: an unsaved draft vanished on a tab switch. The + * lazy-mount test above cannot catch it — the panel wrapper stayed mounted the whole + * time while the editor subtree underneath was replaced. + */ +test("an unsaved combo draft survives a tab switch", async () => { + installFetch(); + const { container, root } = await mountModels(); + try { + await act(async () => { + (container.querySelector("#models-tab-combos") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + + const field = () => panel(container, "combos")?.querySelector("input") as HTMLInputElement | null; + const input = field(); + expect(input).toBeTruthy(); + + await act(async () => { + const setter = Object.getOwnPropertyDescriptor( + testWindow.HTMLInputElement.prototype, "value", + )?.set; + setter?.call(input, "draft-probe"); + input!.dispatchEvent(new testWindow.Event("input", { bubbles: true })); + }); + await act(async () => { await Promise.resolve(); }); + expect(field()?.value).toBe("draft-probe"); + + await act(async () => { + (container.querySelector("#models-tab-catalog") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + await act(async () => { + (container.querySelector("#models-tab-combos") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + + expect(field()?.value).toBe("draft-probe"); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * A reactivation whose fetch fails is classified `failed-cold` once the store has been + * evicted, and replacing the workspace there would destroy the retained draft. + */ +test("a failed combos reload keeps the workspace instead of replacing it", async () => { + let failNext = false; + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = String(input); + if (failNext && url.includes("/api/combos")) throw new Error("combos down"); + if (url.includes("/api/models")) return Response.json([]); + if (url.includes("/api/provider-context-caps")) return Response.json({ providers: {} }); + if (url.includes("/api/providers")) return Response.json([]); + if (url.includes("/api/selected-models")) return Response.json({}); + if (url.includes("/api/combos")) return Response.json([]); + if (url.includes("/api/config")) return Response.json({ providers: {} }); + return new Response(null, { status: 404 }); + }) as typeof fetch; + + const { container, root } = await mountModels(); + try { + await act(async () => { + (container.querySelector("#models-tab-combos") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + expect(panel(container, "combos")?.querySelector(".combos-workspace-root")).toBeTruthy(); + + failNext = true; + await act(async () => { + (container.querySelector("#models-tab-catalog") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + await act(async () => { + (container.querySelector("#models-tab-combos") as HTMLButtonElement).click(); + }); + await act(async () => { await Promise.resolve(); }); + + expect(panel(container, "combos")?.querySelector(".combos-workspace-root")).toBeTruthy(); + } finally { + await act(async () => root.unmount()); + } +}); diff --git a/gui/tests/page-loading-contract.test.tsx b/gui/tests/page-loading-contract.test.tsx index f59f4a28a..a8ceffa6f 100644 --- a/gui/tests/page-loading-contract.test.tsx +++ b/gui/tests/page-loading-contract.test.tsx @@ -133,7 +133,7 @@ const CACHED_PAGE = { beforeEach(() => { clearClientResourceStoresForTests(); previousGlobals = Object.fromEntries(globals.map(key => [key, Reflect.get(globalThis, key)])) as typeof previousGlobals; - testWindow = new Window({ url: "http://localhost/#combos" }); + testWindow = new Window({ url: "http://localhost/#models/combos" }); Object.defineProperties(globalThis, { document: { configurable: true, value: testWindow.document }, window: { configurable: true, value: testWindow.window }, diff --git a/gui/tests/routing-panel-lifecycle.test.tsx b/gui/tests/routing-panel-lifecycle.test.tsx new file mode 100644 index 000000000..1405f57d5 --- /dev/null +++ b/gui/tests/routing-panel-lifecycle.test.tsx @@ -0,0 +1,234 @@ +/** + * RoutingProfiles as a hidden Models tab. + * + * Review found a path that abort and generation-invalidation both miss: a save or + * delete resolving AFTER the panel is hidden calls `load()`, which opened a fresh + * controller and four requests the deactivation effect had already run past — with a + * current generation, so its writes would land in a panel nobody is looking at. + */ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import RoutingProfiles from "../src/pages/RoutingProfiles"; +import { LanguageProvider } from "../src/i18n/provider"; +import { clearClientResourceStoresForTests } from "../src/client-resource"; + +const globals = ["document", "window", "navigator", "localStorage", "sessionStorage"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; +const originalFetch = globalThis.fetch; +const API_BASE = "http://localhost"; + +beforeEach(() => { + clearClientResourceStoresForTests(); + previousGlobals = Object.fromEntries(globals.map(k => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/#models/routing" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, + window: { configurable: true, value: testWindow.window }, + navigator: { configurable: true, value: testWindow.navigator }, + localStorage: { configurable: true, value: testWindow.localStorage }, + sessionStorage: { configurable: true, value: testWindow.sessionStorage }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + clearClientResourceStoresForTests(); + testWindow.close(); + for (const key of globals) { + Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); + } +}); + +type Counts = { profiles: number; aborted: number }; + +function installFetch(counts: Counts, gate?: Promise) { + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + if (url.includes("/api/routing-profiles")) { + counts.profiles++; + if (gate) await gate; + // Report abort the way a real fetch would, so the caller's guard is exercised. + if (init?.signal?.aborted) { counts.aborted++; throw new Error("aborted"); } + return Response.json([]); + } + if (url.includes("/api/routing-analytics")) return Response.json(null); + if (url.includes("/api/config")) return Response.json({ providers: {} }); + if (url.includes("/api/models")) return Response.json([]); + return new Response(null, { status: 404 }); + }) as typeof fetch; +} + +async function mount(active: boolean): Promise<{ + root: Root; + container: HTMLElement; + rerender: (a: boolean) => Promise; +}> { + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + let root!: Root; + const render = (a: boolean) => ( + + + + ); + await act(async () => { + root = createRoot(container); + root.render(render(active)); + }); + // The load is scheduled through a zero-delay timeout, so a microtask flush is not + // enough to observe it. + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + return { + root, + container, + rerender: async (a: boolean) => { + await act(async () => { root.render(render(a)); }); + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + }, + }; +} + +test("a hidden panel never starts its initial load", async () => { + const counts: Counts = { profiles: 0, aborted: 0 }; + installFetch(counts); + const { root } = await mount(false); + try { + expect(counts.profiles).toBe(0); + } finally { + await act(async () => root.unmount()); + } +}); + +test("an in-flight load is aborted when the panel is hidden", async () => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const counts: Counts = { profiles: 0, aborted: 0 }; + installFetch(counts, gate); + + const { root, rerender } = await mount(true); + try { + expect(counts.profiles).toBe(1); + await rerender(false); + release(); + await act(async () => { await Promise.resolve(); }); + expect(counts.aborted).toBe(1); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * The blocker, reproduced through the real path: Retry calls `load()` directly, exactly + * as the post-save and post-delete handlers do. Cancelling what is already running does + * not stop a call that arrives AFTER the panel is hidden from opening a whole new load. + * + * Driven by clicking Retry while hidden — the same entry point a mutation resolving + * late would use, and one a test can reach without faking a save. + */ +test("a load requested while hidden never reaches the network", async () => { + const counts: Counts = { profiles: 0, aborted: 0 }; + installFetch(counts); + + const { root, rerender, container } = await mount(true); + try { + const afterMount = counts.profiles; + expect(afterMount).toBeGreaterThan(0); + + const retry = [...container.querySelectorAll("button")] + .find(b => b.textContent?.includes("Retry")) as HTMLButtonElement | undefined; + expect(retry).toBeTruthy(); + + await rerender(false); + // The panel is hidden; the handler still exists and still calls load(). + await act(async () => { retry!.click(); }); + await act(async () => { await new Promise(r => setTimeout(r, 30)); }); + + expect(counts.profiles).toBe(afterMount); + } finally { + await act(async () => root.unmount()); + } +}); + +test("becoming visible again loads", async () => { + const counts: Counts = { profiles: 0, aborted: 0 }; + installFetch(counts); + const { root, rerender } = await mount(false); + try { + expect(counts.profiles).toBe(0); + await rerender(true); + expect(counts.profiles).toBeGreaterThan(0); + } finally { + await act(async () => root.unmount()); + } +}); + +/* + * The tab meta is only useful if it follows the list. `onCountChange` fires from an + * effect keyed on `profiles.length`, so a reload that changes the list has to report. + */ +test("the profile count is reported and follows a reload", async () => { + const seen: number[] = []; + /* + * The real response shape: a `profiles` wrapper, and every nested object present. + * `parseProfiles` drops anything that omits them, so a looser fixture silently + * yields an empty list and the test would measure nothing. + */ + const PROFILE = { + id: "balanced", + model: "policy/balanced", + revision: "rev-abc", + candidates: [{ provider: "openai", model: "gpt-5" }], + require: {}, + optimize: {}, + limits: {}, + unknownEvidence: {}, + }; + let profiles: unknown[] = []; + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/api/routing-profiles")) return Response.json({ profiles }); + if (url.includes("/api/routing-analytics")) return Response.json(null); + if (url.includes("/api/config")) return Response.json({ providers: {} }); + if (url.includes("/api/models")) return Response.json([]); + return new Response(null, { status: 404 }); + }) as typeof fetch; + + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + document.body.append(container); + let root!: Root; + await act(async () => { + root = createRoot(container); + root.render( + + seen.push(n)} /> + , + ); + }); + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + + try { + expect(seen.at(-1)).toBe(0); + + // Stand in for a create: the list grows, then Retry reloads it. + profiles = [PROFILE]; + const retry = [...container.querySelectorAll("button")] + .find(b => b.textContent?.includes("Retry")) as HTMLButtonElement; + await act(async () => { retry.click(); }); + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + expect(seen.at(-1)).toBe(1); + + // And a delete: back to empty. + profiles = []; + await act(async () => { retry.click(); }); + await act(async () => { await new Promise(r => setTimeout(r, 10)); }); + expect(seen.at(-1)).toBe(0); + } finally { + await act(async () => root.unmount()); + } +}); diff --git a/gui/tests/routing-profiles.test.tsx b/gui/tests/routing-profiles.test.tsx index 28e195204..9262c937f 100644 --- a/gui/tests/routing-profiles.test.tsx +++ b/gui/tests/routing-profiles.test.tsx @@ -173,7 +173,13 @@ test("routing page loads profiles, analytics, and marks the dry-run selection", const { container, root } = await mountPage(); try { expect(container.querySelector('[data-page="routing"]')).toBeTruthy(); - expect(container.textContent).toContain("Routing Intelligence (beta)"); + /* + * The panel no longer renders its own title: it is a Models tab now, and the shell + * above it owns the page heading and subtitle. Rendering them here put both on + * screen twice. The string still exists in the product — as the tab label. + */ + expect(container.textContent).not.toContain("Routing Intelligence (beta)"); + expect(container.textContent).toContain("Create profile"); expect(container.textContent).toContain("balanced"); expect(container.textContent).toContain("policy/balanced"); expect(container.textContent).toContain("rev-abc"); diff --git a/gui/tests/sidebar-claude-entry.test.ts b/gui/tests/sidebar-claude-entry.test.ts deleted file mode 100644 index 446add149..000000000 --- a/gui/tests/sidebar-claude-entry.test.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { expect, test } from "bun:test"; - -/** - * The Claude row. - * - * It was removed from the sidebar together with the connection switch it used - * to carry (a56a4aea6). Removing the switch was right — a nav row owning a - * mutation is a trap — but the entry went with it, and Claude Code is the - * deepest surface in the app. - * - * Two things have to stay true: the row navigates and nothing else, and it does - * not light up at the same time as Integrations, since both resolve to the same - * page and only the hash tells them apart. - */ - -const src = await Bun.file(new URL("../src/App.tsx", import.meta.url)).text(); - -test("the Claude row targets the Claude tab and carries no mutation", () => { - expect(src).toContain('tkey: "nav.claude"'); - expect(src).toContain('subPath: "claude"'); - expect(src).toContain('activeHashes: ["integrations/claude"]'); - - /* - * The sidebar is navigation only. A Switch here is the exact regression the - * collapse removed. Strip comments before asserting: the block explains the - * removed mutation in prose, and matching that prose is not evidence about - * the code — an earlier version of this test failed on its own explanation. - */ - const navBlock = src.slice(src.indexOf("")); - const navCode = navBlock.replace(/\{?\/\*[\s\S]*?\*\/\}?/g, "").replace(/\/\/.*$/gm, ""); - expect(navCode).not.toContain("Switch"); - expect(navCode).not.toContain("/api/claude"); -}); - -test("the orphaned sidebar switch styles are gone", async () => { - const css = await Bun.file(new URL("../src/styles.css", import.meta.url)).text(); - expect(css).not.toContain(".nav-entry-claude .switch"); -}); - -/** - * Mirrors `isNavEntryActive`. Kept as a local re-implementation rather than an - * export because App does not otherwise expose its nav internals; the rule is - * small and the two hash cases below are what actually matter. - */ -function activeRow(rawHash: string): "claude" | "integrations" | null { - const claimed = rawHash === "integrations/claude" || rawHash.startsWith("integrations/claude/"); - if (claimed) return "claude"; - if (rawHash === "integrations" || rawHash.startsWith("integrations/")) return "integrations"; - return null; -} - -test("exactly one row is current for any integrations hash", () => { - expect(activeRow("integrations")).toBe("integrations"); - expect(activeRow("integrations/keys")).toBe("integrations"); - expect(activeRow("integrations/grok")).toBe("integrations"); - // Claude wins its own tab, and the nested Desktop route too — that is what - // the prefix match buys instead of a second nav entry. - expect(activeRow("integrations/claude")).toBe("claude"); - expect(activeRow("integrations/claude/desktop")).toBe("claude"); -}); diff --git a/gui/tests/sidebar-rows.test.ts b/gui/tests/sidebar-rows.test.ts new file mode 100644 index 000000000..b711c5421 --- /dev/null +++ b/gui/tests/sidebar-rows.test.ts @@ -0,0 +1,58 @@ +/** + * The sidebar's row contract. + * + * Replaces `sidebar-claude-entry.test.ts`, which asserted the exact Claude shortcut row + * that has now been removed. Two of its rules outlived it and are kept here: the + * sidebar carries navigation and nothing else, and no orphaned switch styles are left + * behind. The third — that exactly one of two rows resolving to the same page lights up + * — cannot be violated any more, because every row maps one-to-one onto a page again. + */ +import { expect, test } from "bun:test"; + +const raw = await Bun.file(new URL("../src/App.tsx", import.meta.url)).text(); + +/* + * Comments explain the removed Claude row by name, and matching that prose is not + * evidence about the code — the predecessor of this file learned that the hard way, and + * so did this one on its first run. + */ +const src = raw.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, ""); + +test("every row maps one-to-one onto a page", () => { + // The duplicate-row machinery is gone with the row that needed it. + expect(src).not.toContain("activeHashes"); + expect(src).not.toContain("isNavEntryActive"); + expect(src).not.toContain('tkey: "nav.claude"'); + + const navBlock = src.slice(src.indexOf("const NAV: NavEntry[] = ["), src.indexOf("];", src.indexOf("const NAV: NavEntry[] = ["))); + const ids = [...navBlock.matchAll(/\{ id: "([^"]+)"/g)].map(m => m[1]); + + // The exact nine, in order. A count alone would pass if a row were swapped for + // another, and Routing folding into Models is precisely that kind of change. + expect(ids).toEqual([ + "dashboard", "codex-auth", "providers", "models", "subagents", + "logs", "usage", "storage", "integrations", + ]); + // No two rows share a page id, which is what made the correction helper necessary. + expect(new Set(ids).size).toBe(ids.length); +}); + +test("the sidebar is navigation only", () => { + // A nav row owning a mutation is the exact regression that removed the Claude + // connection switch. + const navCode = src.slice(src.indexOf("")); + expect(navCode).not.toContain("Switch"); + expect(navCode).not.toContain("/api/claude"); +}); + +test("the orphaned sidebar switch styles are gone", async () => { + const css = await Bun.file(new URL("../src/styles.css", import.meta.url)).text(); + expect(css).not.toContain(".nav-entry-claude .switch"); +}); + +test("Claude Code is still reachable, just not as a duplicate row", async () => { + // Removing the shortcut must not remove the destination. + const routing = await Bun.file(new URL("../src/app-routing.ts", import.meta.url)).text(); + expect(routing).toContain('"integrations/claude"'); + expect(routing).toContain('"integrations/claude/desktop"'); +}); diff --git a/tests/models-workspace-tabs.test.ts b/tests/models-workspace-tabs.test.ts new file mode 100644 index 000000000..a30b17526 --- /dev/null +++ b/tests/models-workspace-tabs.test.ts @@ -0,0 +1,137 @@ +/** + * Models workspace tabs — routing contract. + * + * `#combos` and `#routing` are no longer pages. Both resolve to the Models page and + * redirect to their nested tab hashes, and `readModelsTab` recognises the pre-redirect + * form so a cold load lands on the right tab. + */ +import { expect, test, describe } from "bun:test"; +import { + MODELS_TAB_HASHES, + hashBelongsToPage, + readPageFromHash, + resolveAppHashChange, +} from "../gui/src/app-routing"; +import { + MODELS_TABS, + modelsPanelDomId, + modelsTabDomId, + modelsTabHash, + readModelsTab, + type ModelsTab, +} from "../gui/src/pages/models-tab"; + +describe("nested Models hashes", () => { + test("both tab hashes are registered and belong to the models page", () => { + expect([...MODELS_TAB_HASHES]).toEqual(["models/combos", "models/routing"]); + for (const hash of MODELS_TAB_HASHES) { + expect(hashBelongsToPage(hash, "models")).toBe(true); + // The first segment already answers the page; the suffix only picks the tab. + expect(readPageFromHash(hash)).toBe("models"); + } + }); + + test("the bare page hash still belongs to models", () => { + expect(hashBelongsToPage("models", "models")).toBe(true); + expect(resolveAppHashChange("models").replaceTo).toBeNull(); + }); + + test("an unregistered sub-hash is normalised away instead of rendering a blank tab", () => { + expect(hashBelongsToPage("models/nope", "models")).toBe(false); + expect(resolveAppHashChange("models/nope")).toEqual({ page: "models", replaceTo: "models" }); + }); + + test("a registered tab hash survives resolution untouched", () => { + for (const hash of MODELS_TAB_HASHES) { + expect(resolveAppHashChange(hash)).toEqual({ page: "models", replaceTo: null }); + } + }); +}); + +describe("readModelsTab", () => { + test("maps every nested hash to its tab", () => { + expect(readModelsTab("#models")).toBe("catalog"); + expect(readModelsTab("#models/combos")).toBe("combos"); + expect(readModelsTab("#models/routing")).toBe("routing"); + }); + + test("accepts the bare and slash-prefixed hash forms", () => { + expect(readModelsTab("models/combos")).toBe("combos"); + expect(readModelsTab("#/models/routing")).toBe("routing"); + }); + + /* + * The redirect that rewrites `#combos` uses replaceState and emits no `hashchange`, + * so tab state is read from the ORIGINAL hash. Recognising only the nested form + * would land a cold load on the catalog with the URL claiming Combos. + */ + test("resolves legacy top-level hashes so a cold load lands on the right tab", () => { + expect(readModelsTab("#combos")).toBe("combos"); + expect(readModelsTab("#routing")).toBe("routing"); + expect(readModelsTab("#combos/anything")).toBe("combos"); + expect(readModelsTab("#routing/anything")).toBe("routing"); + }); + + test("anything unrecognised falls back to the catalog", () => { + expect(readModelsTab("#dashboard")).toBe("catalog"); + expect(readModelsTab("#")).toBe("catalog"); + expect(readModelsTab("")).toBe("catalog"); + }); + + /* + * The legacy arms match on a `/` delimiter, not a bare prefix. Without these a + * later "simplification" to `startsWith("routing")` would hijack any future page + * whose id merely begins with one of these words. + */ + test("legacy matching is delimiter-aware, not prefix-aware", () => { + expect(readModelsTab("#combosomething")).toBe("catalog"); + expect(readModelsTab("#routings")).toBe("catalog"); + expect(readModelsTab("#routingthing")).toBe("catalog"); + expect(readModelsTab("#combos-legacy")).toBe("catalog"); + }); + + /* + * `models/combos/extra` is not a registered hash: the resolver normalises it away, + * so the tab reader must agree rather than treating it as the Combos tab. + */ + test("a deeper nested hash is not mistaken for a tab", () => { + expect(readModelsTab("#models/combos/extra")).toBe("catalog"); + expect(readModelsTab("#models/routing/extra")).toBe("catalog"); + }); +}); + +describe("modelsTabHash", () => { + test("round-trips every tab", () => { + for (const tab of MODELS_TABS) { + expect(readModelsTab(`#${modelsTabHash(tab)}`)).toBe(tab); + } + }); + + test("the catalog owns the bare page hash", () => { + expect(modelsTabHash("catalog")).toBe("models"); + expect(modelsTabHash("combos")).toBe("models/combos"); + expect(modelsTabHash("routing")).toBe("models/routing"); + }); + + test("every non-catalog hash is registered for normalization", () => { + const nested = MODELS_TABS.filter((tab): tab is Exclude => tab !== "catalog"); + expect(nested.map(modelsTabHash).sort()).toEqual([...MODELS_TAB_HASHES].sort()); + }); +}); + +test("tab and panel dom ids are distinct per tab, so aria-controls cannot collide", () => { + const ids = MODELS_TABS.flatMap(tab => [modelsTabDomId(tab), modelsPanelDomId(tab)]); + expect(new Set(ids).size).toBe(ids.length); + // Pin the exact shape: the rendered aria-controls/aria-labelledby pair is asserted + // against these ids once the strip exists, so a silent rename would desync them. + expect(modelsTabDomId("combos")).toBe("models-tab-combos"); + expect(modelsPanelDomId("combos")).toBe("models-panel-combos"); +}); + +test("an unregistered deep hash normalises to the bare page, matching readModelsTab", () => { + for (const stray of ["models/combos/extra", "models/routing/extra"]) { + expect(hashBelongsToPage(stray, "models")).toBe(false); + expect(resolveAppHashChange(stray)).toEqual({ page: "models", replaceTo: "models" }); + expect(readModelsTab(`#${stray}`)).toBe("catalog"); + } +}); diff --git a/tests/routing-intelligence-ui.test.ts b/tests/routing-intelligence-ui.test.ts index 10376fc78..c557d3725 100644 --- a/tests/routing-intelligence-ui.test.ts +++ b/tests/routing-intelligence-ui.test.ts @@ -10,11 +10,19 @@ import { const guiRoot = join(import.meta.dir, "..", "gui", "src"); -test("routing is a first-class dashboard page with a registered hash", () => { - expect(VALID_PAGES.has("routing")).toBe(true); - expect(readPageFromHash("routing")).toBe("routing"); - expect(hashBelongsToPage("routing", "routing")).toBe(true); - expect(resolveAppHashChange("routing").replaceTo).toBeNull(); +test("routing is a Models tab with a registered nested hash", () => { + // It used to be a first-class page. Both ids are gone from the union now, and the + // old top-level hashes keep working through a passive redirect. + expect(VALID_PAGES.has("routing" as never)).toBe(false); + expect(VALID_PAGES.has("combos" as never)).toBe(false); + + expect(readPageFromHash("models/routing")).toBe("models"); + expect(hashBelongsToPage("models/routing", "models")).toBe(true); + expect(resolveAppHashChange("models/routing").replaceTo).toBeNull(); + + expect(resolveAppHashChange("routing")).toEqual({ page: "models", replaceTo: "models/routing" }); + expect(resolveAppHashChange("routing/anything")).toEqual({ page: "models", replaceTo: "models/routing" }); + expect(resolveAppHashChange("combos")).toEqual({ page: "models", replaceTo: "models/combos" }); }); test("Routing page wires profile CRUD, dry-run, and analytics against management APIs", () => { @@ -84,10 +92,13 @@ test("sanitizeLogEntryRouteDecision drops invalid routeDecision and keeps valid expect(sanitizeLogEntryRouteDecision(baseLog).routeDecision).toBeUndefined(); }); -test("App mounts RoutingProfiles from the sidebar NAV entry", () => { +test("Models mounts RoutingProfiles as a tab, and App no longer owns a Routing row", () => { + const models = readFileSync(join(guiRoot, "pages", "Models.tsx"), "utf8"); + expect(models).toContain("RoutingProfiles"); + expect(models).toContain('modelsPanelDomId("routing")'); + const app = readFileSync(join(guiRoot, "App.tsx"), "utf8"); - expect(app).toContain('id: "routing"'); - expect(app).toContain("RoutingProfiles"); - expect(app).toContain('page === "routing"'); - expect(app).toContain("IconRoute"); + expect(app).not.toContain('id: "routing"'); + expect(app).not.toContain('page === "routing"'); + expect(app).not.toContain("IconRoute"); });