Skip to content

refactor(desktop): move Task Entry controller below AppShell - #4491

Open
chihumyum wants to merge 1 commit into
apache:mainfrom
chihumyum:refactor/task-entry-controller-scope
Open

refactor(desktop): move Task Entry controller below AppShell#4491
chihumyum wants to merge 1 commit into
apache:mainfrom
chihumyum:refactor/task-entry-controller-scope

Conversation

@chihumyum

@chihumyum chihumyum commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make TaskEntryRoot / TaskEntryProvider the sole production owner of useTaskEntryController
  • expose a stable semantic projection to AppShell while Host and Workspace Picker subscribe at their actual render boundaries
  • remove the raw controller from the production feature entry and keep it behind the testing seam
  • add render-scope and exact ownership/import provenance tests so controller ownership cannot silently flow back into AppShell
  • preserve routing, UI, IPC/storage contracts, catalog fencing, focus restoration, and draft identity behavior

This is the controller-scope follow-up to the Task Entry feature slice from #3723.

Refs #4582 (supersedes #3439)

Performance evidence

I measured the pure controller-only remote-directory handoff that previously re-rendered the AppShell-owned frame. This is a same-process alternating A/B benchmark with a synthetic 2,000-leaf shell frame, 20 warmups, and 120 paired trials per run. The table reports each run's median:

Run Rendered fibers, AppShell owner Rendered fibers, scoped owner React actualDuration Scoped Synchronous JS wall Scoped
1 2,003 1 3.315 ms 0.248 ms 4.022 ms 0.313 ms
2 2,003 1 3.258 ms 0.171 ms 3.989 ms 0.304 ms
3 2,003 1 3.473 ms 0.206 ms 4.271 ms 0.289 ms
Median of run medians 2,003 1 3.315 ms 0.206 ms 4.022 ms 0.304 ms

That is 99.95% fewer rendered fibers, 93.8% lower React render duration, and 92.4% lower synchronous JS wall time for this isolated update. All 360 paired trials favored the scoped owner.

This does not measure the full create-task/pause/IPC/Runtime Host chain and is not a claim that the entire application is 92% faster. The percentages come from a synthetic 2,000-leaf frame in a same-process harness, not from a running Maka instance, so they demonstrate the scope mechanism rather than an in-app gain; the deterministic render-scope test is the contract. The benchmark scaffold was temporary and is not included in the production diff. The permanent deterministic test proves the scope boundary without timing: the same real controller handoff leaves the shell and unrelated frame at zero renders and wakes only the Host reader.

Review focus

The generic controllerOwners registry is still under review in #4315, so this branch does not duplicate that checker. Against current main, exact Task Entry tests pin the sole controller call site, the only deep importers, the public/testing split, and the absence of controller/Host/Workspace ownership in AppShell. If #4315 lands first, this PR should be rebased and Task Entry registered in that generic policy.

The latest rebase resolves AppShell, Task Entry public-entry, and generated architecture-ledger conflicts against main@b714a3921. Task Entry now uses one namespace public-entry import, so the updated root ratchet improves rather than grows: AppShell import specifiers 186 → 185, and the obsolete useTaskEntryController: 1 hook inventory entry is retired.

Verification

Exact head: 8a59de8fd5ac7d230513ce1a993ccd141e95765c

  • npm run rebuild — full production build, including renderer entry and third-party notice checks
  • npm --workspace @maka/desktop run test:dist — 1,969/1,969 passed
  • npm run check:renderer-architecture -- --base upstream/main — 71/71 checker fixtures and real checkout passed
  • npm run check:app-shell-hooks — 41 hooks / 77 call sites passed
  • npm run astryx:surface-inventory — 244 files, 1 exclusion
  • npm run astryx:surface-inventory:test — 15/15 passed
  • npx knip --workspace apps/desktop
  • npm run typecheck --workspace @maka/desktop -- --pretty false
  • npm run lint
  • npm run format:check
  • git diff --check upstream/main...HEAD
  • git merge-tree --write-tree upstream/main HEAD

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex designed and implemented the controller ownership boundary, added the architecture/render-scope tests, built and ran the temporary benchmark, resolved the current-main conflicts, audited remote overlap, and prepared this PR. The commit includes a Generated-by: OpenAI Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — Task Entry controller-only updates no longer re-render AppShell-wide consumers; there is no intended user-visible UI change
  • No

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 1, 2026
@chihumyum
chihumyum force-pushed the refactor/task-entry-controller-scope branch from de29c32 to 3f5931e Compare September 2, 2026 16:09
@chihumyum

chihumyum commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@Astro-Han This PR is now rebased onto current main@b714a3921 with the AppShell, Task Entry public-entry, generated architecture-ledger, hook-inventory, and Astryx-inventory conflicts resolved.

Exact head: 8a59de8fd5ac7d230513ce1a993ccd141e95765c. Local verification is green: Desktop 1,969/1,969, renderer architecture 71/71, AppShell hook inventory 41 hooks / 77 call sites, Astryx inventory 244 files / 1 exclusion, Desktop Knip, full rebuild, typecheck, lint, format, diff check, and a clean merge tree. Could you review it when convenient? Thanks.

Automated update from OpenAI Codex on behalf of the PR author.

@chihumyum

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (c86da40d7, after #4571) and regenerated the Astryx surface inventory; the renderer architecture ledger needed no change. Exact head: f22555b425ab6cddcd804ea21f649bcfdf3b7050.

Local verification on this head with Node 24: Desktop test:dist 1975/1975 (8 previously listed cancellations reproduce only under Node 22, on main too), typecheck (four projects), lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks, Astryx inventory, Knip (desktop), ASF headers, git diff --check — all green.

The description now states that the performance table comes from a synthetic harness rather than a running instance.

Automated update from Claude Code on behalf of the PR author.

@chihumyum
chihumyum force-pushed the refactor/task-entry-controller-scope branch from f22555b to 2e104af Compare September 2, 2026 18:41
@chihumyum

Copy link
Copy Markdown
Contributor Author

Rebased again onto current main (ad18da42c, #4455); the Astryx inventory was the only conflict and was regenerated. Exact head: 2e104afd9161a4461cb3d659f6fdbcca0402fd9c.

On the previous head's CI run, the test job failed only in Desktop e2e, on sidebar-geometry.spec.ts:102 (resize handle drag) and transcript-scroll.spec.ts:251 (Turn anchor after a Session switch), with 100 passed. Neither spec touches Task Entry, both were changed on main today (#4546, #4414), and both passed locally on that head under --repeat-each 2 (26/26 across the two files). I cannot rerun the job from this account, so this push is the retry.

Same local verification on this head with Node 24: desktop test:dist, typecheck (four projects), lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks, Astryx inventory, Knip, ASF headers, git diff --check — all green.

Automated update from Claude Code on behalf of the PR author.

@chihumyum

Copy link
Copy Markdown
Contributor Author

Rebased onto main 68cda0b54 to clear the conflict. Exact head: 3b37afd04b6a665efa6caccf1d6890d8016d7ab0. Generated files were regenerated; the full local verification is green on it.

Posted by Claude Code on behalf of the PR author.

@chihumyum
chihumyum force-pushed the refactor/task-entry-controller-scope branch from 3b37afd to 3dc2ee8 Compare September 3, 2026 15:39
@chihumyum

Copy link
Copy Markdown
Contributor Author

Rebased once more onto main b9748a773; the ledger was the only conflict and was regenerated. Exact head: 3dc2ee85a592e83db898d2546d0728ae9c2d220a; the full local verification is green on it.

Posted by Claude Code on behalf of the PR author.

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

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant