From 02605b06e643ae2286fb6efdaddc85cf6d0bc2ce Mon Sep 17 00:00:00 2001 From: James Dabbs Date: Mon, 6 Jul 2026 18:52:31 -0700 Subject: [PATCH 1/3] fix(viewer): recompute related traits on client-side navigation Related.svelte derives its rows via `$: all = related($traits)`, which only re-runs when the `related` prop or the traits store changes identity. Both callers passed hoisted function declarations that closed over their space/property prop, so navigating between two spaces (or properties) reused the mounted component and kept showing the previous entity's traits table. Declare the closures reactively so a new function is passed whenever the entity changes. Also make the `rel` tab-link prop reactive on the space page, matching the property and theorem pages. Adds a fixture-only Cypress regression spec driving the exact flow from the issue: an in-description space link (unpadded id), asserting the traits table recomputes, including on browser back. S000001's fixture description now links to {S4} to support it; "Indiscrete" is asserted false for S000001 and true for S000004, so the value icon flips iff the table recomputes (deduction can add traits but never contradict asserted ones). Verified red against the unfixed code, green with the fix. Closes #255 Co-Authored-By: Claude Fable 5 --- packages/viewer/cypress/e2e/space.spec.ts | 32 +++++++++++++++++++ .../viewer/cypress/fixtures/main.min.json | 2 +- .../src/components/Properties/Spaces.svelte | 6 ++-- .../src/components/Spaces/Properties.svelte | 6 ++-- .../src/routes/(app)/spaces/[id]/+page.svelte | 3 +- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/packages/viewer/cypress/e2e/space.spec.ts b/packages/viewer/cypress/e2e/space.spec.ts index 62727b2b..a65de846 100644 --- a/packages/viewer/cypress/e2e/space.spec.ts +++ b/packages/viewer/cypress/e2e/space.spec.ts @@ -2,6 +2,10 @@ import { deduce, setup } from '../support' beforeEach(setup) +// Marks specs that depend on exact fixture-pinned content and only run in +// fixture mode (see doc/testing.md). +const fixtureIt = Cypress.env('mode') === 'live' ? it.skip : it + function clickTraitFor(name: string) { cy.get('.related-traits') .contains(name) @@ -51,3 +55,31 @@ it('displays references', () => { cy.get('ul.references').should('exist') }) + +// Navigating between space pages reuses the mounted component, and the traits +// table must recompute for the new space instead of continuing to show the +// previous space's data. See https://github.com/pi-base/web/issues/255. +// +// Fixture-only: relies on S000001's fixture description linking to {S4} (live +// descriptions differ) and on "Indiscrete" being asserted false for S000001 +// and true for S000004, so the value icon flips iff the table recomputes. +fixtureIt('recomputes traits when navigating between spaces', () => { + function indiscreteRow() { + return cy.get('.related-traits').contains('tr', 'Indiscrete') + } + + cy.visit('spaces/S000001') + + cy.contains('h1', 'Discrete topology on') + indiscreteRow().find('.bi-x').should('exist') + + // Client-side navigation via the in-description link (note the unpadded id) + cy.get('.description').contains('a', 'Indiscrete topology on').click() + cy.location('pathname').should('eq', '/spaces/S4') + cy.contains('h1', 'Indiscrete topology on') + indiscreteRow().find('.bi-check').should('exist') + + cy.go('back') + cy.location('pathname').should('eq', '/spaces/S000001') + indiscreteRow().find('.bi-x').should('exist') +}) diff --git a/packages/viewer/cypress/fixtures/main.min.json b/packages/viewer/cypress/fixtures/main.min.json index 9c0c3bb8..e41c21f0 100644 --- a/packages/viewer/cypress/fixtures/main.min.json +++ b/packages/viewer/cypress/fixtures/main.min.json @@ -945,7 +945,7 @@ "uid": "S000001", "counterexamples_id": 1, "name": "Discrete topology on $\\{0,1\\}$", - "description": "-", + "description": "Compare with the indiscrete topology {S4} on the same set.", "aliases": ["Discrete topology on a two-point set", "Finite discrete topology"], "refs": [] }, diff --git a/packages/viewer/src/components/Properties/Spaces.svelte b/packages/viewer/src/components/Properties/Spaces.svelte index 4badbdbd..c58fc5a2 100644 --- a/packages/viewer/src/components/Properties/Spaces.svelte +++ b/packages/viewer/src/components/Properties/Spaces.svelte @@ -5,9 +5,9 @@ export let property: Property - function related(traits: Traits): [Space, Property, Trait | undefined][] { - return traits.forPropertyAll(property).map(([s, t]) => [s, property, t]) - } + // Reactive so that Related recomputes when navigating between properties + $: related = (traits: Traits): [Space, Property, Trait | undefined][] => + traits.forPropertyAll(property).map(([s, t]) => [s, property, t]) diff --git a/packages/viewer/src/components/Spaces/Properties.svelte b/packages/viewer/src/components/Spaces/Properties.svelte index 2cfbc062..7edb9cd1 100644 --- a/packages/viewer/src/components/Spaces/Properties.svelte +++ b/packages/viewer/src/components/Spaces/Properties.svelte @@ -5,9 +5,9 @@ export let space: Space - function related(traits: Traits): [Space, Property, Trait | undefined][] { - return traits.forSpaceAll(space).map(([p, t]) => [space, p, t]) - } + // Reactive so that Related recomputes when navigating between spaces + $: related = (traits: Traits): [Space, Property, Trait | undefined][] => + traits.forSpaceAll(space).map(([p, t]) => [space, p, t]) diff --git a/packages/viewer/src/routes/(app)/spaces/[id]/+page.svelte b/packages/viewer/src/routes/(app)/spaces/[id]/+page.svelte index c1671a62..367c81f5 100644 --- a/packages/viewer/src/routes/(app)/spaces/[id]/+page.svelte +++ b/packages/viewer/src/routes/(app)/spaces/[id]/+page.svelte @@ -5,11 +5,10 @@ import { page } from '$app/stores' export let data: PageData - let rel = $page.url.pathname $: title = `S${data.space.id}: ${data.space.name}` -<Show space={data.space} tab="properties" {rel} /> +<Show space={data.space} tab="properties" rel={$page.url.pathname} /> From ad5a1dcfe9780b883a40cc94bbbfe4c27336f7c7 Mon Sep 17 00:00:00 2001 From: James Dabbs <james.dabbs@gmail.com> Date: Mon, 6 Jul 2026 18:52:49 -0700 Subject: [PATCH 2/3] docs(testing): fixture-only specs and fixture-mode build gotchas Document the fixtureIt convention for specs that depend on fixture-pinned content, and two traps when running fixture mode against a production-style build (also what CI runs): the bundle is baked in and SSR-injected so cy.intercept on main.json is inert, and the preview server must be restarted after every rebuild to avoid serving HTML that references the previous build's hashed chunks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --- doc/testing.md | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/doc/testing.md b/doc/testing.md index a02bff9f..fc86b621 100644 --- a/doc/testing.md +++ b/doc/testing.md @@ -70,18 +70,21 @@ against: exercise the actual deployment end-to-end and confirm the sites stay consistent. -| `CYPRESS_ENV` | URL | Mode | -| -------------------- | ------------------------------------ | ------- | -| `local` (default) | `http://localhost:5173` | fixture | -| `pages` | `https://topology.pi-base.org` | live | -| `workers` | `topology.pi-base.workers.dev` | live | -| `graphs` | `graphs.pi-base.workers.dev` | live | -| `preview` | `$PREVIEW_URL` | live | - -The whole suite runs in both modes; assertions are written to hold against the +| `CYPRESS_ENV` | URL | Mode | +| ----------------- | ------------------------------ | ------- | +| `local` (default) | `http://localhost:5173` | fixture | +| `pages` | `https://topology.pi-base.org` | live | +| `workers` | `topology.pi-base.workers.dev` | live | +| `graphs` | `graphs.pi-base.workers.dev` | live | +| `preview` | `$PREVIEW_URL` | live | + +The suite runs in both modes; assertions are written to hold against the real data (stable IDs, math-free name prefixes, behavioral checks) rather than -exact HTML snapshots. Set `CYPRESS_MODE=fixture` to force the deterministic -fixture against a deployed URL while debugging. +exact HTML snapshots. Specs that can only hold against fixture-pinned content +(e.g. the space-to-space navigation regression, which follows a description +link that only exists in the fixture) are declared with a `fixtureIt` helper +and show as pending in live runs. Set `CYPRESS_MODE=fixture` to force the +deterministic fixture against a deployed URL while debugging. The fixture (`cypress/fixtures/main.min.json`) is a hand-curated subset; keep its tested entities (e.g. `S000001`, `S000004`, `P000001`) in sync with live data. @@ -89,6 +92,20 @@ Note it predates a pi-base property-ID reorganization, so its property `uid`s ar **not** interchangeable with the current bundle's — refresh display fields per entity, don't bulk-remap by `uid`. +Two things to know when running fixture mode against a production-style build +(`VITE_BUNDLE_HOST=http://localhost:4173 pnpm run build` + `pnpm run preview`): + +- The bundle is baked in at build time: with a localhost `VITE_BUNDLE_HOST`, + `+layout.server.ts` imports `public/refs/heads/main.json` (a symlink to the + Cypress fixture) and injects it during SSR, so the browser never fetches + `main.json`. The `cy.intercept` in `cypress/support/commands.ts` is inert in + this mode (it matters against `pnpm dev` and deployed targets, where the + client fetches the bundle). To vary fixture data here, edit the fixture and + rebuild — intercepting won't work. +- Restart the preview server after every rebuild. A still-running server can + serve HTML referencing the previous build's hashed chunks, which fails with + "Failed to fetch dynamically imported module". + ## Remote End-to-End Testing The `./bin/e2e` script runs the suite against the deployed targets (live data), From ce5ec72a5dfe4326346033b22d7dece82c55ed46 Mon Sep 17 00:00:00 2001 From: James Dabbs <james.dabbs@gmail.com> Date: Mon, 6 Jul 2026 19:02:35 -0700 Subject: [PATCH 3/3] refactor(viewer): pass the Related anchor entity as data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related took a `(traits) => rows` closure prop, leaving its real data dependency (the space or property being viewed) invisible to Svelte's reactivity — the trap behind #255, avoidable only by every caller remembering to re-create the closure reactively. Take the anchor entity itself as a discriminated-union prop and compute the rows inside Related: the object literal at the call sites is re-evaluated whenever the entity changes, so recomputation falls out of ordinary prop flow and the stale-closure mistake is no longer expressible. This also subsumes the separate `mode` prop. Covered end-to-end by the navigation regression spec (17/17 fixture suite passing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --- .../src/components/Properties/Spaces.svelte | 8 ++---- .../src/components/Spaces/Properties.svelte | 8 ++---- .../src/components/Traits/Related.svelte | 28 +++++++++++++++---- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/packages/viewer/src/components/Properties/Spaces.svelte b/packages/viewer/src/components/Properties/Spaces.svelte index c58fc5a2..73a8d109 100644 --- a/packages/viewer/src/components/Properties/Spaces.svelte +++ b/packages/viewer/src/components/Properties/Spaces.svelte @@ -1,16 +1,12 @@ <script lang="ts"> import { Link } from '../Shared' import { Related } from '../Traits' - import type { Property, Space, Trait, Traits } from '@/models' + import type { Property } from '@/models' export let property: Property - - // Reactive so that Related recomputes when navigating between properties - $: related = (traits: Traits): [Space, Property, Trait | undefined][] => - traits.forPropertyAll(property).map(([s, t]) => [s, property, t]) </script> -<Related mode="spaces" {related}> +<Related anchor={{ mode: 'spaces', property }}> <Link.Space slot="id" let:space {space} content="id" /> <Link.Space slot="name" let:space {space} /> diff --git a/packages/viewer/src/components/Spaces/Properties.svelte b/packages/viewer/src/components/Spaces/Properties.svelte index 7edb9cd1..1d23848a 100644 --- a/packages/viewer/src/components/Spaces/Properties.svelte +++ b/packages/viewer/src/components/Spaces/Properties.svelte @@ -1,16 +1,12 @@ <script lang="ts"> import { Link } from '../Shared' import { Related } from '../Traits' - import type { Property, Space, Trait, Traits } from '@/models' + import type { Space } from '@/models' export let space: Space - - // Reactive so that Related recomputes when navigating between spaces - $: related = (traits: Traits): [Space, Property, Trait | undefined][] => - traits.forSpaceAll(space).map(([p, t]) => [space, p, t]) </script> -<Related mode="properties" {related}> +<Related anchor={{ mode: 'properties', space }}> <Link.Property slot="id" let:property {property} content="id" /> <Link.Property slot="name" let:property {property} /> diff --git a/packages/viewer/src/components/Traits/Related.svelte b/packages/viewer/src/components/Traits/Related.svelte index 9df2165e..37279cba 100644 --- a/packages/viewer/src/components/Traits/Related.svelte +++ b/packages/viewer/src/components/Traits/Related.svelte @@ -10,11 +10,28 @@ import urlSearchParam from '@/stores/urlSearchParam' import { checkIfRedundant } from '@/stores/deduction' - export let related: (traits: Traits) => [Space, Property, Trait | undefined][] - export let mode: 'spaces' | 'properties' + type Row = [Space, Property, Trait | undefined] + + // The entity whose related traits are listed. Passing the entity itself + // (rather than a closure over it) keeps the data dependency visible to + // Svelte's reactivity, so the table recomputes when navigating between + // entities. See https://github.com/pi-base/web/issues/255. + export let anchor: + | { mode: 'properties'; space: Space } + | { mode: 'spaces'; property: Property } const { theorems, traits } = context() + function rows(a: typeof anchor, traits: Traits): Row[] { + if (a.mode === 'properties') { + const { space } = a + return traits.forSpaceAll(space).map(([p, t]) => [space, p, t]) + } else { + const { property } = a + return traits.forPropertyAll(property).map(([s, t]) => [s, property, t]) + } + } + const filter = writable('') urlSearchParam('filter', filter) @@ -39,11 +56,12 @@ } } - $: all = related($traits) - // all has type [Space, Property, Trait][] + $: all = rows(anchor, $traits) // we need to index names in different positions depending on which kind we // are displaying - $: index = new Fuse(all, { keys: [`${mode === 'spaces' ? 0 : 1}.name`] }) + $: index = new Fuse(all, { + keys: [`${anchor.mode === 'spaces' ? 0 : 1}.name`], + }) $: searched = $filter ? index.search($filter).map(r => r.item) : all $: filtered = searched.filter(([_space, _property, t]) => matchesFilter(filterMode, t),