Skip to content

refactor(ui): React architecture debt cleanup (post #430 converge) #477

Closed
Closed
refactor(ui): React architecture debt cleanup (post #430 converge)#477
Labels
enhancementNew feature or request

Description

@Astro-Han
Astro-Han
opened on Jul 3, 2026

Background

The #430 converge series (motion / typography / spacing) is fully merged and token governance is complete. The React architecture cleanup surfaced by that work is tracked here; the CSS-side cleanup is tracked separately in #476.

Verified against main at 2791ea4c on 2026-07-04, after #489, #497, #502, #508, and #512 merged.

Scope

This issue covers React component structure, AppShell state ownership, async UI polish, effect callback stability, and evidence-based render performance. It does not cover token values, visual redesign, or CSS architecture debt.

The high-priority React cleanup is now effectively complete. The remaining question is whether a very small evidence-based memo pass is worth doing. Do not add broad memoization without prop-stability or render-count evidence.

Completed

PR4 — Async and lazy boundaries are done

Done in #489 fix/ui-react): harden async and lazy boundaries.

What changed:

  • Made chat model switching and session row actions rejection-safe while preserving owner-side toast feedback.
  • Added compact Suspense fallbacks only where the target surface is already known to be visible.
  • Kept ArtifactPane fallback intentionally null unless AppShell owns an explicit artifact/list-error visibility gate.
  • Added contracts for async pending cleanup, lazy fallback scope, and startup fail-soft behavior.

PR5 — Tool activity preview split is done

Done in #497 refactor(ui-react): split tool activity previews.

What changed:

  • Reduced tool-activity.tsx from the original 1494-line catch-all into a smaller shell plus focused preview owners.
  • Preserved result rendering behavior, redaction, truncation, and fallback ordering through behavior contracts.
  • Removed topology-heavy tests during review and kept coverage on user-visible preview contracts.

Current snapshot: packages/ui/src/tool-activity.tsx is 326 lines on main at 2791ea4c.

PR6 — Session list module split is done

Done in #502 refactor(ui-react): split session list modules.

What changed:

  • Reduced session-list-panel.tsx from the original 1103-line mixed owner into a small shell plus session-history-list.tsx and session-sidebar-nav.tsx owners.
  • Removed stale module-panel props and dead sessions nav branches.
  • Kept behavior contracts for row actions, busy state, scroll area, and time bucket copy.

Current snapshot:

  • packages/ui/src/session-list-panel.tsx: 48 lines.
  • packages/ui/src/session-history-list.tsx: 854 lines.

PR7 — AppShell session UI state is done

Done in #508 refactor(ui-react): consolidate app shell session state.

What changed:

  • Consolidated the per-session UI map state behind a local hook/reducer boundary.
  • Preserved same-session cleanup and active-session behavior.
  • Fixed the nested thinking/truncated update risk during review by making map updates merge against latest state.

PR8 — AppShell effect callback stability is done

Done in #512 refactor(ui-react): stabilize app shell effect callbacks.

What changed:

  • Replaced the local latest-ref pattern with React 19 useEffectEvent for long-lived subscription, timer, and listener callbacks.
  • Preserved mount-once bootstrap subscriptions and activeId-keyed active-session subscriptions.
  • Kept ordinary UI callbacks, such as the plan reminder toast action, outside the Effect Event boundary.
  • Added behavior tests proving captured old subscription callbacks dispatch to the latest handler without resubscribing.

Current Size Snapshot

Measured on main at 2791ea4c:

File Lines Status
apps/desktop/src/renderer/app-shell.tsx 1464 Still large, but per-session state and effect callback boundaries are now separated.
packages/ui/src/tool-activity.tsx 326 Split target addressed.
packages/ui/src/chat-view.tsx 1506 Still large; hot path already has memoized materialization and TurnView.
packages/ui/src/session-list-panel.tsx 48 Split target addressed.
packages/ui/src/session-history-list.tsx 854 New focused owner; revisit only if it becomes hard to review.

app-shell.tsx and chat-view.tsx are still large, but the original highest-risk seams have been reduced. Further splitting should require a concrete review or maintenance problem, not line count alone.

Remaining

Optional PR9 — Evidence-based memo pass

Candidate title: perf(ui-react): assess targeted memo boundaries.

Recommended handling:

  • First prove prop stability or repeated render cost for a narrow boundary, likely SessionRow, ToolActivity, or an extracted tool preview component.
  • Add memo only where the test or measurement shows it avoids unrelated rerenders without hiding unstable props.
  • Do not touch TurnView as a first move; it is already memoized.
  • If there is no useful evidence, leave the code alone and close this issue with a note that memo work is not justified right now.

Close criteria

This issue can be closed once we either:

  • decide to skip PR9 because no render evidence justifies it, or
  • land one small PR9 that adds only proven memo boundaries.

Out of scope

Activity

  1. Astro-Han
    added
    enhancementNew feature or request
    on Jul 3, 2026

  2. Astro-Han commented on Jul 3, 2026

    @Astro-Han
    Astro-Han
    on Jul 3, 2026
    ContributorAuthor

    React-side work is tracked in the shared post-#430 cleanup roadmap on #476:

    #476 (comment)

    This issue owns PR 4 and PR 5 from that roadmap:

    • fix(ui-react): harden async and lazy boundaries
    • refactor(ui-react): split large components and state

    Use Part of #477 in PR bodies until the final React cleanup PR is ready to close this issue.

  3. Astro-Han commented on Jul 3, 2026

    @Astro-Han
    Astro-Han
    on Jul 3, 2026
    ContributorAuthor

    PR4 opened: #489 fix(ui-react): harden async and lazy boundaries

    Refs #477.

    Scope:

    • async UI actions: model switcher and session row actions now use a local rejection-safe boundary
    • lazy UI chunks: shell and module Suspense boundaries now render compact non-null fallbacks
    • contracts: async boundary, lazy fallback, and existing source-grep lifecycle contracts were updated to guard the new shape

    Verification:

    • npm run -w @maka/desktop test passed (1725 tests)
    • npm run -w @maka/desktop build:renderer passed, with the existing Vite large chunk warning
    • node --test packages/ui/dist/__tests__/async-action-boundary.test.js passed (2 tests)

    Commit split:

    • fix(ui-react): catch model switcher rejection
    • fix(ui-react): catch session row action rejection
    • fix(ui-react): add shell lazy fallbacks
    • fix(ui-react): add module lazy fallbacks

  4. Astro-Han commented on Jul 4, 2026

    @Astro-Han
    Astro-Han
    on Jul 4, 2026
    ContributorAuthor

    Roadmap update for the remaining React cleanup work.

    The original PR5 bucket (refactor(ui-react): split large components and state) is now split into three top-level PRs, not sub-PRs:

    • PR5: refactor(ui-react): split tool activity previews

      • Split tool-activity.tsx by preview ownership, for example terminal, web search, diff, subagent, and generic tool result previews.
      • Intended impact: structural only, no visual redesign.

    • PR6: refactor(ui-react): split session list modules

      • Split session-list-panel.tsx into sessions, skills, plan reminders, and daily review owned surfaces.
      • Intended impact: structural only, no visual redesign.

    • PR7: refactor(ui-react): consolidate app shell session state

      • Aggregate AppShell per-session UI maps behind a local reducer.
      • This is intentionally after the component splits because it touches higher-risk state ownership.

    Use Part of #477 in these PR bodies until the final React cleanup PR is ready to close this issue.

  5. Astro-Han commented on Jul 4, 2026

    @Astro-Han
    Astro-Han
    on Jul 4, 2026
    ContributorAuthor

    PR5 is open for review: #497 refactor(ui-react): split tool activity previews.

    Scope:

    • Split packages/ui/src/tool-activity.tsx result previews into focused owner files under packages/ui/src/tool-activity/.
    • Preserve public exports and existing preview DOM/CVA parts.
    • Add an ownership contract so preview implementations do not drift back into the shell.
    • Include one semantic-token CSS unblock for .maka-lazy-fallback so the full desktop test suite stays green.

    Local verification:

    • npm run -w @maka/desktop test passed, 1829 tests.
    • npm run -w @maka/desktop build:renderer passed with the existing Vite large chunk warning.

    Next after PR5: PR6, refactor(ui-react): split session list modules.

  6. Astro-Han
    added a commit that references this issue on Jul 4, 2026

    refactor(ui-react): split tool activity previews

    ee16339

  7. Astro-Han commented on Jul 4, 2026

    @Astro-Han
    Astro-Han
    on Jul 4, 2026
    ContributorAuthor

    PR6 is now in review: #502 refactor(ui-react): split session list modules.

    Scope note: current main already renders Skills, Automations, and Daily Review through ChatView, so this PR keeps those module panels there and splits the remaining sidebar owner into shell, nav/footer, and session-history modules. It also removes stale module-panel callback props from SessionListPanel.

  8. Astro-Han commented on Jul 4, 2026

    @Astro-Han
    Astro-Han
    on Jul 4, 2026
    ContributorAuthor

    PR9 is open for review: #514 perf(ui-react): memoize stable session rows.\n\nScope:\n- Memoize the private SessionRow boundary inside SessionHistoryList.\n- Add a render-count contract through the public SessionHistoryList surface. The contract failed before the memo boundary with stable rows rendering twice across an unrelated parent update, then passed after the memo boundary.\n- Leave ToolActivity and other candidate memo boundaries unchanged because this PR only found evidence for the session-row path.\n\nLocal verification:\n- npm run -w @maka/ui build passed.\n- Focused desktop contract passed.\n- npm run -w @maka/desktop build:renderer passed with the existing Vite large chunk warning.\n- npm run -w @maka/desktop test passed, 1864 tests.\n- Known baseline: npm run -w @maka/ui test still fails on pre-existing chat-primitives.test footer/lineage merge assertions, unrelated to this PR.

  9. Astro-Han commented on Jul 4, 2026

    @Astro-Han
    Astro-Han
    on Jul 4, 2026
    ContributorAuthor

    React 侧 roadmap 全部完成，关闭本 issue。

    main 上 React 侧 contract（renderer-async-boundaries / renderer-lazy-fallback / session-row-actions-fail-soft / tool-activity-result-preview / app-shell-session-ui-state / ui-render-memo-boundary）全绿。

  10. Astro-Han
    closed this as completedon Jul 4, 2026
  11. Astro-Han
    added 2 commits that reference this issue on Jul 4, 2026

    refactor(ui): unify tabs on primitives/tabs, drop ui.tsx hand-rolled set

    7bf0f04

    feat(ui): tab component spec — maka-tab + variants, migrate 4 tab sur…

    304b64a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions