Skip to content

feat(FR-2867): add unified memory accelerator UX in Resource Allocation#7360

Open
yomybaby wants to merge 1 commit into
mainfrom
worktree-agent-ae66c51627aacc8e2
Open

feat(FR-2867): add unified memory accelerator UX in Resource Allocation#7360
yomybaby wants to merge 1 commit into
mainfrom
worktree-agent-ae66c51627aacc8e2

Conversation

@yomybaby
Copy link
Copy Markdown
Member

@yomybaby yomybaby commented May 12, 2026

Resolves #7358(FR-2867)

Summary

Adds Resource Allocation UX for unified memory architecture accelerator slots (slot name suffix .unified, e.g. cuda.unified) in the Session Launcher.

When the selected accelerator slot is .unified:

  • The accelerator-memory input/slider is disabled — users cannot edit it directly.
  • Its value mirrors the host memory (mem) slider, converted into the slot's reported display_unit (falls back to GiB when the unit is not available).
  • An info note is rendered under the accelerator field: "Accelerator memory is shared with host memory on this device and cannot be set separately."

For regular discrete accelerator slots (no .unified suffix), behavior is unchanged.

Identification keys off the slot-name suffix only, so the convention generalizes beyond cuda.unified to any future unified-memory accelerator family.

Approach

The first revision used a useEffect watching mem and writing to ['resource', 'accelerator']. Review feedback pointed out two problems with that shape: ordering races against preset/image effects, and a submit-time lag risk. This revision drops the watcher effect in favor of event-driven writes:

  • When the host mem onChange fires and the active accelerator slot is .unified, the accelerator value is written in the same callback.
  • When the accelerator-type onChange fires:
    • Switching INTO a .unified slot writes the current mem-derived value.
    • Switching OUT to a discrete slot resets accelerator to that slot's min (no stale GiB number left behind).
  • The accelerator Form.Item already lists ['resource', 'mem'] in dependencies, so the field re-renders when mem changes; disabled is true while .unified.

There is no parallel state to keep in lockstep — the form value is the single source of truth, written once at the moment either input changes.

The slot's display_unit (e.g. GiB, MiB, TiB) is mapped to the binary-size prefix consumed by convertToBinaryUnit. If display_unit is missing or unrecognized, the value falls back to GiB.

Changes

  • react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx
    • Exported helper isUnifiedAcceleratorSlot(slotName).
    • Exported helpers displayUnitToInputSizeUnit and getUnifiedAcceleratorValueFromMem.
    • Event-driven writes in mem InputNumber onChange and accelerator-type BAISelect onChange.
    • Form.Item extra info note when .unified is selected; slider/input disabled in that mode.
  • react/src/components/SessionFormItems/ResourceAllocationFormItems.test.ts
    • Unit tests for isUnifiedAcceleratorSlot (matches .unified, rejects discrete including cuda.mem, handles nullish).
  • resources/i18n/en.json
    • New key session.launcher.UnifiedAcceleratorMemoryNote (English source; translations follow via the i18n pass).

Verification Results

  • Relay: PASS
  • Lint: PASS
  • Format: PASS
  • TypeScript: PASS for the touched file. Pre-existing failures on main in 9 unrelated files are not introduced by this PR.
  • Tests: PASS (868 tests / 40 files in react/).

Out of scope (separate follow-up)

  • Resource Preset / resource display rendering for .unified slots — tracked in FR-2868. Surfaces such as ResourceNumber.tsx, ResourcePresetSelect.tsx, ResourcePresetList.tsx, MyResource.tsx, and session/service detail displays still render accelerator memory independently for unified slots; folding those into a single combined display is out of scope here.
  • Filtering / hinting based on whether the Resource Group has unified-memory agents available (depends on a backend matching/query API).
  • Rewriting the legacy "2× GPU memory in RAM" tooltip when .unified is selected.

Test plan

  • Select a .unified accelerator type in Session Launcher → accelerator-memory slider is disabled
  • Change the host memory slider → accelerator-memory value mirrors it, expressed in the slot's display_unit
  • Switch back to a discrete accelerator type → slider becomes editable and resets to the discrete slot's min (no stale GiB number)
  • Info note appears below the accelerator field in unified mode and is absent in discrete mode
  • Submit a session with a .unified accelerator and verify accelerator value equals mem (in the slot's unit)

@github-actions github-actions Bot added area:ux UI / UX issue. area:i18n Localization size:M 30~100 LoC labels May 12, 2026
Copy link
Copy Markdown
Member Author


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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Coverage Report for react-coverage (./react)

Status Category Percentage Covered / Total
🔵 Lines 6.49% 1794 / 27604
🔵 Statements 5.34% 1990 / 37218
🔵 Functions 5.25% 300 / 5712
🔵 Branches 3.75% 1305 / 34753
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx 15.65% 10.42% 16.88% 15.16% 63, 160-1647, 1709
Generated in workflow #542 for commit 25d47b1 by the Vitest Coverage Report Action

@yomybaby yomybaby force-pushed the worktree-agent-ae66c51627aacc8e2 branch from 193c1c8 to 4677880 Compare May 12, 2026 03:16
@github-actions github-actions Bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels May 12, 2026
@yomybaby yomybaby marked this pull request as ready for review May 12, 2026 03:17
Copilot AI review requested due to automatic review settings May 12, 2026 03:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds unified-memory accelerator UX to the Session Launcher’s Resource Allocation step by detecting accelerator slot names ending in .unified and enforcing “accelerator memory mirrors host memory” behavior with a localized info note.

Changes:

  • Added unified-slot detection and unit conversion helpers, and updated Resource Allocation handlers to sync accelerator memory from mem and disable direct edits in unified mode.
  • Added unit tests for .unified slot-name detection.
  • Added an i18n string for the unified-memory info note (English).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx Adds unified-slot detection/conversion helpers; disables accelerator slider/input in unified mode; syncs accelerator value from mem via event handlers; shows i18n note.
react/src/components/SessionFormItems/ResourceAllocationFormItems.test.ts Adds tests for isUnifiedAcceleratorSlot behavior.
resources/i18n/en.json Adds session.launcher.UnifiedAcceleratorMemoryNote English string.

Comment thread react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx Outdated
Comment thread resources/i18n/en.json
Comment thread react/src/components/SessionFormItems/ResourceAllocationFormItems.tsx Outdated
@yomybaby yomybaby requested a review from Copilot May 12, 2026 03:22
@yomybaby yomybaby force-pushed the worktree-agent-ae66c51627aacc8e2 branch from 4677880 to 05f03c6 Compare May 12, 2026 03:28
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

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

@yomybaby yomybaby force-pushed the worktree-agent-ae66c51627aacc8e2 branch from 05f03c6 to 25d47b1 Compare May 12, 2026 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n Localization area:ux UI / UX issue. size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Resource Allocation UX for unified memory accelerator slots (.unified)

2 participants