[PR] fix(desktop): keep session workspace action identities fixed [maka]

["Astro-Han (via GitHub)" <gi...@apache.org>]

Astro-Han opened a new pull request, #4110:
URL: https://github.com/apache/maka/pull/4110

   ## Summary
   
   Switching a session held the renderer at 34% CPU with peaks near 77%, and roughly 58% of that was React render and commit work. The cause is one function identity.
   
   `setActiveId` was a function declaration in the `useAppShellSessionWorkspace` body, so it changed identity on every AppShell render. As `activateSession` it invalidated `openSession`'s `useCallback`, then the Session navigation controller's `commands`, then the host's `rowActions` and `onSelectSession`, then `renderSessionRow` — which defeated `SessionNavRow`'s `memo` on every commit. A single switch re-rendered all 32 sidebar rows about twenty times, and because every Astryx button removes and re-adds its inline `anchor-name` per render, that switch also produced roughly 2,500 style writes and the style recalculation they force.
   
   Every dependency these actions close over is a ref box, a React state setter, or a method of the once-created session-UI controller, so they are constant by construction rather than by discipline. `createSessionWorkspaceActions` moves them out of the render body and the hook instantiates it once; `refreshSessions` and `seedSessions` get the same treatment. They are deliberately not routed through `useStableActions`, whose facade exists for factories whose closures do capture changing deps — paying for that indirection here would buy nothing.
   
   Two smaller pieces ride along, each its own commit:
   
   - `@maka/ui`'s `formatAbsoluteTimestamp` was a second copy of the `Intl` options `@maka/core/relative-time` already owned, and built a formatter per call — about 1,300 per switch, since the sidebar reads one per row for the tooltip and one for the accessible name. Core's cache could not have absorbed them either: `getRelativeFormat` and `getAbsoluteFormat` shared one `cachedLocale` and cleared each other on a miss, so alternating readings rebuilt a formatter every call. Core now caches each with its own locale and exports the function; the UI copy is re-exported rather than reimplemented. Profiling attributed ~33 ms per switch to this, and an A/B swapping in a memoising `Intl.DateTimeFormat` moved renderer JS by less than the run-to-run spread — a duplicate-authority removal, not a measurable win.
   - A contract spec that budgets the rail's DOM writes for one switch, described under Review focus.
   
   Refs #4109
   
   ## Verification
   
   All measurements are same-instance A/B: the two identities were made runtime-switchable and alternated inside one running dev app, six switches each. Cross-instance comparison is not usable here — restarting the app moves these numbers by more than the effect.
   
   | per session switch | unstable | stable |
   |---|---|---|
   | Row renders | 459 | 85 |
   | `memo` hits | 0 | 432 |
   | DOM mutations | 3,438 | 1,992 |
   | Renderer JS | 521 ms | 380 ms |
   | Renderer CPU under a repeated-switch loop | 34% (peak 77%) | 24% (peak 55%) |
   
   Renderer JS fell in 6 of 6 paired runs. Idle CPU was 0.1% before and after; this workload is entirely interaction-driven.
   
   Ran locally:
   
   - `session-workspace-action-identity`, `session-navigation-controller`, `relative-time` — pass
   - `session-rail-render-contract` (Playwright) — pass
   - `tsc -p apps/desktop/tsconfig.renderer.json --noEmit`, `biome check` on the touched files — clean
   
   Not run: the full repository suite, and the end-to-end check on a dev app built from this branch — a dev app from another checkout holds the shared profile lock. The Playwright spec covers the same observable on a clean fixture, so the gap is the manual pass, not the assertion.
   
   Falsifiability was checked for both new tests. Reverting `??=` to `=` in the workspace hook fails the identity test with `setActiveId changed identity between renders` and the contract spec with `Expected: <= 36 / Received: 336`. Restoring the mutual cache invalidation in core fails the formatter test.
   
   ## Review focus
   
   The contract spec is the piece worth arguing about. It asserts an outcome — at most three inline style writes per rail row for one switch — rather than an identity, because this rail has had several independent regressions of the same shape and each was invisible to the others. An identity assertion pins one mechanism in one hook; the next plain function declaration upstream passes every existing check, since the dependency arrays stay correct and `useExhaustiveDependencies` has nothing to flag.
   
   The budget is measured, not guessed. On the new twelve-row fixture the rail writes 4 styles when it behaves — the leaving row and the arriving row, two each, stable across runs — against 336 with the defect restored. 36 sits an order of magnitude clear of both.
   
   It also covers ground this PR does not fix. A switch still produces about twenty commits, and why is not yet attributed; it is not animation-driven, since emulating `prefers-reduced-motion` cuts `requestAnimationFrame` callbacks from 188 to 13 while the render count is unchanged. That cascade multiplies whatever the rail costs per render, and anything that raises it shows up in this budget. Tracked in #4109 along with the structural direction — letting the rail subscribe through the store seam `app-shell-session-ui-state.ts` already establishes, instead of receiving props through five layers.
   
   ## AI use
   
   Select exactly one:
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Claude Code. It drove the CDP profiling and DOM probes that isolated the cause and produced every number above, then wrote the change, the tests, and this description. The human contributor reviewed the diff, the commit messages, and the measurement method. `Generated-by` trailers are on all three commits.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [ ] Yes — described under Summary above
   - [x] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): keep session workspace action identities fixed [maka]

["Astro-Han (via GitHub)" <gi...@apache.org>]

Astro-Han merged PR #4110:
URL: https://github.com/apache/maka/pull/4110


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org