Fix #348: re-measure the bubble graph when its container resizes

[pieer]

Closes #348

What was wrong

The bubble graph sizes its <svg> from the measured container. There was a
ResizeObserver on the container, but its bookkeeping could not tell a real
resize from a redelivery:

• it stored the clamped layout width (Math.min(w, 1100)) as "the last width
  seen" and compared the next raw measurement against it — so on any
  container wider than ~1100px every single delivery counted as a resize and
  re-ran layoutAndRender() + resetView() (throwing away the user's pan/zoom)
  for a resize that never happened;
• the mount path adopted the raw width while the observer path adopted the
  clamped one, so the first frame was drawn at a width no later frame would use;
• nothing re-measured on the paths where the observer cannot deliver: a resize
  (or the mount itself) that happens while the tab is backgrounded/throttled is
  coalesced away, and the table→bubble switch mounts the component while its
  section is still the hidden 0-height placeholder, so the canvas keeps the
  DEFAULT_CONTAINER_HEIGHT (800px) placeholder.

What changed (JS only)

• New web_src/js/components/graph/graph-viewport.ts — the pure functions
  that turn a measurement into canvas numbers (canvasHeightFor,
  layoutWidthFor, isMeasurable, sizeChanged) plus the constants that used
  to be inline in the component (MIN_SVG_HEIGHT, MAX_LAYOUT_WIDTH,
  DEFAULT_CONTAINER_*). Unit-tested in graph-viewport.test.ts (8 tests),
  including the "a redelivery of the same box is not a change, above the width
  cap too" case that is the bug above.
• FishboneGraph.vue — one re-measure path (measureContainer() +
  scheduleRemeasure(), rAF-throttled). It keeps the raw box it last
  measured, so the bail-out compares like with like; a container that is not
  rendered (0×0) is refused rather than adopted; the observer stays on the
  container, never on the <svg> it sizes, so a taller canvas cannot
  enlarge what is observed; the observer is disconnected and any pending rAF
  cancelled on unmount, along with the new listeners.
• Extra triggers that all funnel into the same bail-out (so they cost one
  getBoundingClientRect when nothing moved): window resize /
  orientationchange, document visibilitychange (the throttled-tab case),
  and a new repo:bubble-visible event.
• repo-history.ts — dispatches repo:bubble-visible after the switch to
  bubble view, once the section is visible (the component's listener is
  registered by then: its mount awaits nextTick() before this call does).

Overlap with PR #343 (fix/149-consistent-footer, not yet merged)

This branch is off master, so it does not contain #343's CSS. The two touch
the same feature from opposite sides and should rebase cleanly:

• Fix #149: keep the footer consistent on the bubble view #343 (CSS only): removes height: calc(100vh - 25rem) from
  #bubble-view-root and .f-fishbone-graph and replaces it with flexbox
  (flex: 1 1 0, min-height: 320px) in web_src/css/features/bubble-graph.css.
• This PR (JS only): does not touch the CSS sizing strategy at all —
  .f-fishbone-graph's scoped height: calc(100vh - 25rem) is left exactly as
  it is on master for Fix #149: keep the footer consistent on the bubble view #343 to remove.

They are complementary: #343 makes the container the right size, this PR makes
the canvas follow the container whenever that size changes. Whichever lands
second needs no manual conflict resolution beyond the usual rebase. One thing
to re-check after both land: under #343's flex: 1 1 0 the container's height
must still come from the page, not from the SVG inside it, or the re-measure
would have a feedback path — the guard here (observe the container, bail when
unchanged) already stops the loop, but the sizing intent is worth a visual
confirmation.

Verification

• npx vitest run — 33 files / 185 tests pass (8 of them new).
• npx eslint … on the four touched files — clean. npx vue-tsc --noEmit — clean.
• Browser (running dev instance at localhost:3000, unpatched build): both
  symptoms reproduced against a real page (/subject/Deep Sea Mining Governance), driving the viewport through a same-origin iframe because the
  Chrome window could not be resized:
  • viewport 896px → 596px: container 546→246 while the canvas kept its old
    484px box until the observer got a frame in;
  • ?view=table → click Bubble view: container 546px, <svg> stuck at
    height: 800px (the placeholder) until rendering resumed.
• Not verified in the browser: the patched build. The running instance
  serves assets compiled into the developer's own checkout, and I did not
  modify that checkout to swap in a rebuilt bundle, so the fix itself is backed
  by the unit tests, the type-check and the lint run rather than by a live
  before/after screenshot. Worth a manual pass: resize the window in bubble
  view, and switch table→bubble on a short window.

Model used: Claude Opus 5 (1M context).

[pieer]

/review

---

AI review started.

[github-actions]

Advanced AI Review

• Type: Agentic (crush)
• Model: glm-5.3

Click to expand review

I'll review these changes by first examining the full files involved.Now let me look at the repo-history.ts changes in full context:Now let me examine the layout/render functions and the template to understand how the measured elements fit together:Let me check the test setup conventions and run the new tests:Let me verify a few remaining details — event listeners across the codebase and the exact diff of repo-history.ts:# Review: Fix #348 — re-measure the bubble graph when its container resizes

I traced the measurement/relayout paths, the view-switch flow in repo-history.ts (including the Vue scheduler ordering the new event relies on), and all consumers of the measured width/height. The overall structure (raw-vs-clamped bookkeeping, single funnel with bail-out, cleanup on unmount) is sound, and the new unit tests genuinely pin the old bug. I found one medium issue, several lower-priority ones, and no spelling/grammar problems in the new strings or comments.

---

1. 🟡 The bail-out compares RAW width, but nothing consumes raw width — horizontal-only resizes above the 1100px cap still wipe the user's pan/zoom

• Addressed
• Dismissed

Addressed in 78a665d — sizeChanged() now runs both sides through layoutWidthFor(), so a width-only change above the cap is not a change, with the suggested test plus one pinning the first measurement to the same clamp.

sizeChanged compares the raw measurement:

// graph-viewport.ts:57-59
export function sizeChanged(prev: ContainerSize, next: ContainerSize, epsilon = SIZE_EPSILON): boolean {
  return Math.abs(next.width - prev.width) > epsilon || Math.abs(next.height - prev.height) > epsilon;
}

but the raw width never reaches the layout. Every consumer reads containerWidth, which is always the clamped value assigned in measureContainer():

// FishboneGraph.vue:873-875
  measured = next;
  containerWidth = layoutWidthFor(next.width);
  containerHeight = next.height;

(applyResponsiveDials at FishboneGraph.vue:767, computeDetailSize at FishboneGraph.vue:1770, and the zoomExtent() fallback at FishboneGraph.vue:1256 all use containerWidth; the <svg>'s on-screen width comes from CSS — tw-w-full inside .mx-auto max-w-[1100px] — not from this measurement at all.)

So when a desktop window resizes horizontally from, say, 1200px to 1300px content width (both above MAX_LAYOUT_WIDTH), containerWidth stays 1100 and containerHeight is unchanged — the layout inputs are identical — yet measureContainer() returns true, and:

// FishboneGraph.vue:895-899
  pendingRaf = requestAnimationFrame(() => {
    pendingRaf = null;
    layoutAndRender();
    resetView();
  });

resetView() reapplies the fitted transform (FishboneGraph.vue:1342), discarding the user's pan/zoom for a resize that cannot change anything on screen. This is the exact symptom the PR description calls out as the bug ("re-ran the layout … throwing away the user's pan/zoom … for a resize that never happened"), surviving in the narrower form of "a resize that happened but that nothing depends on". It also contradicts measureContainer()'s own contract:

// FishboneGraph.vue:862-864
/** Read the container box and adopt it. Returns true when the numbers the
   layout depends on actually moved, false for "nothing to do" — ...

Raw width is not "a number the layout depends on"; the clamped one is.

Suggested fix — compare the width the layout will actually run at (this keeps the existing tests green, including the redelivery test, since equal raw widths still compare equal):

// graph-viewport.ts
/** Bail-out test for the re-measure path: compares the numbers the layout
   actually runs at — the CLAMPED width and the raw height — so neither an
   unchanged container nor a width-only change above the cap (which cannot
   alter the layout) triggers a re-render, and the observer cannot feed
   itself. */
export function sizeChanged(prev: ContainerSize, next: ContainerSize, epsilon = SIZE_EPSILON): boolean {
  return Math.abs(layoutWidthFor(next.width) - layoutWidthFor(prev.width)) > epsilon
      || Math.abs(next.height - prev.height) > epsilon;
}

and pin the new behaviour with a test next to the existing cap test:

// graph-viewport.test.ts
test('a width-only change above the cap changes nothing the layout runs at', () => {
  expect(sizeChanged({width: 1200, height: 486}, {width: 1300, height: 486})).toBe(false);
  expect(sizeChanged({width: 1000, height: 486}, {width: 1080, height: 486})).toBe(true);
});

Note the clamped comparison also fixes a latent edge in the current design: containerWidth is initialised to DEFAULT_CONTAINER_WIDTH (1100), so a first real measurement of an exactly-1100×799 container is only guaranteed to be adopted if the comparison goes through measured (raw 0) rather than through the derived values — comparing layoutWidthFor on both sides of measured preserves that property.

---

2. ⚪️ The repo:bubble-visible handoff silently depends on cross-file microtask FIFO ordering

• Addressed
• Dismissed

Addressed in 78a665d — the four listeners moved into registerRemeasureTriggers() in graph-viewport.ts and are registered at the top of onMounted, before any await; the ordering coupling is gone and the wiring now has unit tests.

// repo-history.ts:400-402
       not the size it now has. The listener is registered inside the
       component's own mount, which awaits nextTick before this call does, so
       it is in place by the time the event goes out. FishboneGraph re-measures

This is currently true, but only because (a) app.mount() runs onMounted synchronously, (b) the component's await nextTick() (FishboneGraph.vue:1444) is queued before ensureBubbleView's second await nextTick() (repo-history.ts:404), and (c) microtask continuations resolve FIFO. Nothing enforces this: if anyone later inserts an await between line 1444 and the listener registration block (FishboneGraph.vue:1473-1476), or the component grows an async setup, the first-mount delivery is lost with no error, and symptom two of #348 returns.

The four window/document listeners don't depend on the container element, so they can be hoisted above the first await, removing the coupling entirely:

onMounted(async () => {
  /* #348 triggers — registered before any await so an event dispatched
     right after app.mount() cannot beat them. */
  const onRemeasure = () => scheduleRemeasure();
  const onVisibility = () => { if (!document.hidden) scheduleRemeasure(); };
  window.addEventListener('resize', onRemeasure, {passive: true});
  window.addEventListener('orientationchange', onRemeasure, {passive: true});
  window.addEventListener('repo:bubble-visible', onRemeasure);
  document.addEventListener('visibilitychange', onVisibility);
  remeasureCleanup = () => { /* ... unchanged ... */ };

  svgSel = select(svgRef.value!);
  ...

---

3. ⚪️ Stale comment (and redundant call) in scheduleRemeasure

• Addressed
• Dismissed

Addressed in 78a665d — the call stays, the comment no longer claims the empty state misses layoutAndRender(); it now says what the call actually buys (one frame earlier).

// FishboneGraph.vue:888-891
  /* Synchronously, before the re-layout: a resize with no data (the empty
     state) never reaches layoutAndRender below, and even with data the box
     should not be a frame stale. */
  syncCanvasHeight();

The claim is no longer accurate: the rAF callback calls layoutAndRender() unconditionally, and layoutAndRender()'s first action is the very same sync, before the empty-graph early return:

// FishboneGraph.vue:1357-1361, 1375
function layoutAndRender() {
  cancelReflow();
  /* Before anything else, and whatever the data: the empty and error states
     return early below and would otherwise keep a stale canvas box. */
  syncCanvasHeight();
  ...
  if (!Object.keys(state.graph).length) {

So the empty state does reach layoutAndRender, one frame later, and gets a correct height there. The synchronous call's only real justification is the one-frame-earlier application, which the second half of the comment already gives. Either drop the duplicate call and keep the comment about frame staleness, or fix the first clause, e.g.:

  /* Synchronously, so the new box lands in this frame's flush rather than
     the next: layoutAndRender() re-syncs it too, but only inside the rAF. */
  syncCanvasHeight();

---

4. ⚪️ ensureBubbleView can run twice concurrently — the guard is on the wrong side of the await

• Addressed
• Dismissed

Addressed in 78a665d — viewLoaded.bubble is claimed before the await so the two first-switch callers cannot both pass the guard; the dispatch still runs on every call, as the backstop it is.

On the first switch to bubble, two callers race: switchView (repo-history.ts:577) and the activeView watcher (repo-history.ts:659). Both pass the check before either sets the flag, because the flag is set only after the first await:

// repo-history.ts:390-395
  async function ensureBubbleView() {
    if (!viewLoaded.bubble) {
      await nextTick();
      initRepoBubbleView();
      viewLoaded.bubble = true;
    }

Today this is doubly benign — initRepoBubbleView() is guarded by data-mounted (repo-bubble-view.ts:7) and the second repo:bubble-visible dispatch bails out in measureContainer() — but the race pre-dates this PR and the PR adds a second side effect (the dispatch) to the racy function. Since viewLoaded.bubble is not read anywhere else, the flag can be set eagerly:

  async function ensureBubbleView() {
    if (!viewLoaded.bubble) {
      viewLoaded.bubble = true;   // eager: a second concurrent caller must not re-mount
      await nextTick();
      initRepoBubbleView();
    }
    await nextTick();
    window.dispatchEvent(new CustomEvent('repo:bubble-visible'));
  }

(Keep dispatching the event on every call — it is the backstop for resizes that happened while the table view was showing, not just for the first mount.)

---

5. ⚪️ After unifying on the clamped width, WIDTH_BREAKPOINT_MAX can never be reached

• Addressed
• Dismissed

Addressed in 78a665d — WIDTH_BREAKPOINT_MAX is now MAX_LAYOUT_WIDTH, with the pairing and the 0.86 saturation spelled out in the comment.

// FishboneGraph.vue:151-152
const WIDTH_BREAKPOINT_MIN = 480;    // Minimum container width for responsive calculations
const WIDTH_BREAKPOINT_MAX = 1200;   // Maximum container width for responsive calculations

Before this PR the mount path adopted the raw width (containerWidth = rect0.width), so on a wide screen the first layout could compute widthFactor = 1. Now every path clamps to MAX_LAYOUT_WIDTH (1100), so:

// FishboneGraph.vue:770
  const widthFactor = Math.min(1, Math.max(0, (w - WIDTH_BREAKPOINT_MIN) / (WIDTH_BREAKPOINT_MAX - WIDTH_BREAKPOINT_MIN)));

saturates at (1100 − 480) / (1200 − 480) ≈ 0.861 — the wide-screen end of the responsive dials is permanently unreachable (max lane padding, max branch spacing, etc. can never fully engage). Either import and use MAX_LAYOUT_WIDTH as the breakpoint, or lower WIDTH_BREAKPOINT_MAX to 1100 with a comment explaining the pairing.

---

6. ⚪️ resetView()'s self-retry rAF escapes the new unmount cleanup

• Addressed
• Dismissed

Addressed in 78a665d — the zero-box retry goes through pendingRaf so onBeforeUnmount cancels it, and resetView() returns early on a null ref instead of svgRef.value!.

The PR (correctly) cancels pendingRaf on unmount:

// FishboneGraph.vue:1508
  if (pendingRaf !== null) { cancelAnimationFrame(pendingRaf); pendingRaf = null; }

but resetView re-schedules itself with a bare, untracked rAF whenever the svg box is zero:

// FishboneGraph.vue:1274-1278
  const svg = svgRef.value!;
  const box = svg.getBoundingClientRect();
  if (!box.width || !box.height) {
    requestAnimationFrame(() => resetView(animated));
    return;
  }

If that retry fires after unmount, svgRef.value! is null (template refs are nulled on unmount) and svg.getBoundingClientRect() throws. It is theoretical today (the component is mounted once and never unmounted at runtime — the section is hidden, not destroyed), but since this PR's stated goal includes "any pending rAF cancelled on unmount", routing the retry through pendingRaf would make that claim complete:

  if (!box.width || !box.height) {
    if (pendingRaf !== null) cancelAnimationFrame(pendingRaf);
    pendingRaf = requestAnimationFrame(() => { pendingRaf = null; resetView(animated); });
    return;
  }

---

7. ⚪️ Minor DRY: the remeasure callback is written twice

• Addressed
• Dismissed

Addressed in 78a665d — one callback (scheduleRemeasure) passed to both observeContainerResize() and registerRemeasureTriggers(); the no-op {passive: true} is dropped, with a comment saying why.

// FishboneGraph.vue:1461
  ro = new ResizeObserver(() => scheduleRemeasure());
...
// FishboneGraph.vue:1471
  const onRemeasure = () => scheduleRemeasure();

Define onRemeasure once, above the observer, and pass it to both new ResizeObserver(onRemeasure) and the addEventListener calls — one identity, one behaviour. (Also, {passive: true} on resize/orientationchange at lines 1473-1474 has no effect — those events are not cancelable — but it is harmless.)

---

Review generated using glm-5.3 via Z.AI. Comment /review to re-run.

---

Follow-up notes (not review items):

• Pan/zoom on a genuine resize. Item 1 removes the no-op case; the real one is handled too — a view the user has moved by hand (a d3 zoom event carrying a sourceEvent) keeps its transform across a re-measure, and only the pan bound and the zoom floor are refreshed. resetView() takes ownership back.
• Not verified in a browser. The pre-fix symptoms were reproduced in Chrome; this patched build has not been. npx vitest run (190 tests), eslint and vue-tsc --noEmit are clean, but the webpack bundle could not be built from this worktree (no TS loader for webpack.config.ts resolves here), so nobody has yet watched the fixed graph resize.
• Overlap with Fix #149: keep the footer consistent on the bubble view #343. Fix #149: keep the footer consistent on the bubble view #343 (fix/149-consistent-footer) owns the bubble-view sizing strategy in CSS and has moved on to styling .history-bubble-root (the #bubble-view-root id stays because repo-bubble-view.ts mounts on it). Nothing here touches that strategy — this branch is JS/measurement only — but bubble-view sizing is worth re-checking once both land, since the flex-basis-0 container and the measured canvas have never been exercised together.

[pieer]

Need more testing before approving

[pieer]

Approved