Skip to content

fix(FR-2815): break BAITable resizable column infinite update loop#7267

Merged
graphite-app[bot] merged 1 commit into
mainfrom
05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining
May 6, 2026
Merged

fix(FR-2815): break BAITable resizable column infinite update loop#7267
graphite-app[bot] merged 1 commit into
mainfrom
05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining

Conversation

@ironAiken2
Copy link
Copy Markdown
Contributor

@ironAiken2 ironAiken2 commented May 6, 2026

Resolves #7248(FR-2815)

Summary

/session/start crashed with "Maximum update depth exceeded" on the 26.4.x release backend. The trigger surface was VFolderTable inside the Storage step Card while that Card was hidden (display: none).

Root cause

BAITable's ResizableTitle workaround for un-specified column widths self-loops when its measurement target has width 0:

  1. The useEffect that measures the header cell has no dependency array, so it fires on every render.
  2. When width prop is undefined, the effect calls onResize(offsetWidth), which setResizedColumnWidths records.
  3. The parent reads that value back via resizedColumnWidths[key] || column.width. Inside a hidden ancestor the measured value is 0, and 0 || undefined falls through to undefined. So the next render again has width: undefined.
  4. ResizableTitle re-runs the effect, again measures 0, again writes it. React hits its 50-update render limit and throws.

Stack traces pointed at antd EllipsisMeasure because BAIText (in a sibling cell) was the last commit-target when the limit was hit; the actual setState cascade originates in BAITable's ResizableTitle path.

Why it didn't manifest before

  • resizable default was false until recently; ResizableTitle was not used at all so the loop didn't exist.
  • After the default flipped to true, the loop is latent in any BAITable rendered inside a hidden ancestor with un-widthed columns. /session/start is the first surface to hit it because its step Cards stay mounted and toggle visibility with display: none (so the Storage VFolderTable mounts at width 0 on initial render).
  • Other endpoints don't crash because their VFolder responses have fewer rows / different column widths so the loop never accumulates beyond React's 50-update limit.

Fix

  • Use ?? instead of || for the resizedColumnWidths fallback so a measured 0 is preserved and not coalesced back to undefined.
  • Skip the onResize self-call when offsetWidth === 0. A 0-width measurement is meaningless inside a hidden ancestor, and skipping it breaks the loop without affecting the visible-column code path.

The default resizable: true is preserved; only the loop-prone branch is guarded. Other consumers of BAITable inherit the fix automatically.

Verification

  • Relay: PASS
  • Lint: PASS
  • Format: PASS
  • TypeScript: pre-existing failures only on main (unrelated; tracked separately under FR-2816).

Manual smoke test

  • Open /session/start against the 26.4.x release backend → page loads without the React update-depth crash.
  • Navigate through all 5 steps (SessionTypeEnvironmentStorageNetworkReview) → form values persist; transitions work; Storage step's VFolderTable shows rows correctly.
  • Confirm column resize on visible BAITable instances (e.g., /data page) still works as before.

Copilot AI review requested due to automatic review settings May 6, 2026 07:30
@github-actions github-actions Bot added the size:XS ~10 LoC label May 6, 2026
Copy link
Copy Markdown
Contributor Author

ironAiken2 commented May 6, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR opts two hot paths into the React Compiler’s memoization by adding the 'use memo'; directive to a session form component and a resource-limit hook.

Changes:

  • Added 'use memo'; directive to useResourceLimitAndRemaining hook.
  • Added 'use memo'; directive to ResourceAllocationFormItems component.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
react/src/hooks/useResourceLimitAndRemaining.tsx Adds 'use memo'; at the start of the hook body to enable React Compiler memoization for the hook’s returned values/closures.
react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx Adds 'use memo'; at the start of the component body to enable React Compiler memoization for this form section.

Comment thread react/src/hooks/useResourceLimitAndRemaining.tsx Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Coverage Report for react-coverage (./react)

Status Category Percentage Covered / Total
🔵 Lines 6.55% 1783 / 27192
🔵 Statements 5.43% 1978 / 36413
🔵 Functions 5.21% 296 / 5676
🔵 Branches 3.82% 1293 / 33801
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
react/src/pages/SessionLauncherPage.tsx 0% 0% 0% 0% 207-1454
Generated in workflow #312 for commit 38ce9ed by the Vitest Coverage Report Action

@ironAiken2 ironAiken2 force-pushed the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch from 512a7e8 to 6cb8b59 Compare May 6, 2026 07:45
@github-actions github-actions Bot added size:M 30~100 LoC 26.5.0 release-blocker and removed size:XS ~10 LoC labels May 6, 2026
@ironAiken2 ironAiken2 force-pushed the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch from 6cb8b59 to 768441c Compare May 6, 2026 08:16
@github-actions github-actions Bot added size:XL 500~ LoC and removed size:M 30~100 LoC labels May 6, 2026
@ironAiken2 ironAiken2 force-pushed the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch from 768441c to 4cd2388 Compare May 6, 2026 08:40
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Coverage Report for backend-ai-ui-coverage (./packages/backend.ai-ui)

Status Category Percentage Covered / Total
🔵 Lines 8.63% 361 / 4179
🔵 Statements 7.78% 410 / 5266
🔵 Functions 9.2% 94 / 1021
🔵 Branches 7% 362 / 5171
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/backend.ai-ui/src/components/Table/BAITable.tsx 0% 0% 0% 0% 123-693
Generated in workflow #329 for commit 979d465 by the Vitest Coverage Report Action

@ironAiken2 ironAiken2 force-pushed the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch 2 times, most recently from 38ce9ed to 601d2e1 Compare May 6, 2026 10:05
@ironAiken2 ironAiken2 changed the title fix(FR-2815): add 'use memo' to ResourceAllocationFormItems and useResourceLimitAndRemaining fix(FR-2815): break BAITable resizable column infinite update loop May 6, 2026
@ironAiken2 ironAiken2 force-pushed the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch from 601d2e1 to e089638 Compare May 6, 2026 10:14
@github-actions github-actions Bot added size:S 10~30 LoC and removed size:XL 500~ LoC labels May 6, 2026
Copy link
Copy Markdown
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

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

LGTM

@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented May 6, 2026

Merge activity

…7267)

Resolves #7248(FR-2815)

## Summary

`/session/start` crashed with **"Maximum update depth exceeded"** on the 26.4.x release backend. The trigger surface was `VFolderTable` inside the Storage step Card while that Card was hidden (`display: none`).

## Root cause

BAITable's `ResizableTitle` workaround for un-specified column widths self-loops when its measurement target has width 0:

1. The `useEffect` that measures the header cell has **no dependency array**, so it fires on every render.
2. When `width` prop is `undefined`, the effect calls `onResize(offsetWidth)`, which `setResizedColumnWidths` records.
3. The parent reads that value back via `resizedColumnWidths[key] || column.width`. Inside a hidden ancestor the measured value is `0`, and **`0 || undefined`** falls through to `undefined`. So the next render again has `width: undefined`.
4. `ResizableTitle` re-runs the effect, again measures `0`, again writes it. React hits its 50-update render limit and throws.

Stack traces pointed at antd `EllipsisMeasure` because `BAIText` (in a sibling cell) was the last commit-target when the limit was hit; the actual setState cascade originates in BAITable's `ResizableTitle` path.

## Why it didn't manifest before

- `resizable` default was `false` until recently; `ResizableTitle` was not used at all so the loop didn't exist.
- After the default flipped to `true`, the loop is latent in any BAITable rendered inside a hidden ancestor with un-widthed columns. `/session/start` is the first surface to hit it because its step Cards stay mounted and toggle visibility with `display: none` (so the Storage VFolderTable mounts at width 0 on initial render).
- Other endpoints don't crash because their VFolder responses have fewer rows / different column widths so the loop never accumulates beyond React's 50-update limit.

## Fix

- Use `??` instead of `||` for the `resizedColumnWidths` fallback so a measured `0` is preserved and not coalesced back to `undefined`.
- Skip the `onResize` self-call when `offsetWidth === 0`. A 0-width measurement is meaningless inside a hidden ancestor, and skipping it breaks the loop without affecting the visible-column code path.

The default `resizable: true` is preserved; only the loop-prone branch is guarded. Other consumers of BAITable inherit the fix automatically.

## Verification

- Relay: PASS
- Lint: PASS
- Format: PASS
- TypeScript: pre-existing failures only on `main` (unrelated; tracked separately under FR-2816).

## Manual smoke test

- Open `/session/start` against the 26.4.x release backend → page loads without the React update-depth crash.
- Navigate through all 5 steps (`SessionType` → `Environment` → `Storage` → `Network` → `Review`) → form values persist; transitions work; Storage step's VFolderTable shows rows correctly.
- Confirm column resize on visible BAITable instances (e.g., `/data` page) still works as before.
@graphite-app graphite-app Bot force-pushed the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch from e089638 to 979d465 Compare May 6, 2026 10:29
@graphite-app graphite-app Bot merged commit 979d465 into main May 6, 2026
12 checks passed
@graphite-app graphite-app Bot deleted the 05-06-fix_fr-2815_add_use_memo_to_resourceallocationformitems_and_useresourcelimitandremaining branch May 6, 2026 10:30
@github-pages github-pages Bot temporarily deployed to github-pages May 6, 2026 10:31 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session start page (/session/start) throws "Maximum update depth exceeded" on load

3 participants