You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "chihumyum (via GitHub)" <gi...@apache.org> on 2026/09/01 17:03:54 UTC
[PR] refactor(desktop): move Task Entry controller below AppShell [maka]
chihumyum opened a new pull request, #4491:
URL: https://github.com/apache/maka/pull/4491
## 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 #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 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.
## Verification
- `npm run check:renderer-architecture -- --base upstream/main` — 61/61 checker fixtures and real checkout passed
- `npm run typecheck --workspace @maka/desktop -- --pretty false`
- `npm run build:main --workspace @maka/desktop`
- `npm run build:renderer --workspace @maka/desktop`
- Task Entry directed suites — 26/26 passed
- Desktop full suite — 1,879/1,879 passed
- `npm run lint`
- `npm run format:check`
- `git diff --check`
A root `npm test` run reached one unrelated Storage harness failure under local Node 24.4.0: the managed-dependency child-process test treats Node's SQLite `ExperimentalWarning` on stderr as a test error. Its exact 5-test file passed with `NODE_NO_WARNINGS=1`; Desktop remained 1,879/1,879 green.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] 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, audited remote overlap, and prepared this PR. The commit includes a `Generated-by: OpenAI Codex` trailer.
## 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?
- [x] Yes — Task Entry controller-only updates no longer re-render AppShell-wide consumers; there is no intended user-visible UI change
- [ ] 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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5543774266
Rebased onto `main` `2c948e4e3` (after #4315) and registered Task Entry with the `controllerOwners` guard, as the description promised. The guard requires the component AppShell mounts to be the one that calls the controller, so two things changed:
- `TaskEntryRoot` now owns `useTaskEntryController` directly (the inner `TaskEntryProvider` is gone) and memoizes the render prop on the shell projection, so a controller-only update re-renders the one owner fiber and reuses the frame element it built last time; the render-scope test still holds. The public entry exports `TaskEntryShellProjection`, so AppShell takes the type directly instead of deriving it from component props.
- The boundary assertions the guard now proves (raw controller absent from the public entry, a single call site, the controller module imported only by its owner and the testing seam) are dropped, per the #4315 review.
Exact head: `f8c057f0fd36f17f0a753eabbc0803de4d732ef4`. Full local verification on it with Node 24: desktop `test:dist`, `typecheck` (four projects), lint, format, `check:renderer-architecture --base upstream/main` (now validating both registered owners), `check:app-shell-hooks`, Astryx inventory, Knip, ASF headers, `git diff --check` — all green.
@Astro-Han this is the next one in the chain after #4315, whenever you have time.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5528184189
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.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5527857066
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.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5512623747
@Astro-Han This PR is now rebased onto current `main@b714a3921` with the AppShell, Task Entry public-entry, and generated architecture-ledger conflicts resolved.
Exact head: `3f5931edd88d30acce5ddeda8b69e3bd93d331ac`. Local verification is green: Desktop 1,969/1,969, renderer architecture 71/71, AppShell hook inventory 41 hooks / 77 call sites, 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.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5557547891
Rebased onto `main` `c0229b0c6`; the ledger was the only conflict, regenerated with the Task Entry owner registration replayed. Exact head: `541f968c56281c711c226acaf9edd7902069475f`. Local verification is green except `typecheck`, which fails in `stories/app-shell.stories.tsx` on `rightCollapsed` — that is `main`'s own breakage after #4694, with the fix in #4895; I will rebase once more when it lands so the merging head has green CI.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5514269038
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.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5549721367
Rebased onto `main` `cbeb1a93c`; the Astryx inventory was the only conflict and was regenerated. Exact head: `0151abc72a48bedfd6d7fe8e8bc37c54e975f0a3`; the full local verification is green on it.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5550965358
Rebased onto `main` `efbcc6c08`. The ledger conflicted this time (#4640 and #4823 both regenerated it), so it was regenerated rather than merged, and the `useTaskEntryController → TaskEntryRoot` registration was replayed into the regenerated policy; `check:renderer-architecture --base upstream/main` validates it again. Exact head: `4a352a3ca7c8fff8ddda412643763f63ffab3242`; the full local verification is green on it.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4491:
URL: https://github.com/apache/maka/pull/4491
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5557580916
#4895 landed, so this is rebased once more onto `main` `e226ac532`. No conflicts; the ledger and the inventory are byte-identical to the previous head. Exact head: `bab5ffc61`. Local verification with Node 24 is now fully green, `typecheck` included (desktop `test:dist` 2300/2300, lint, format, `check:renderer-architecture --base upstream/main`, `check:app-shell-hooks`, Astryx inventory, Knip, ASF headers, `git diff --check`, preload/overlay builds).
Posted by Claude Code on behalf of the PR author.
--
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] refactor(desktop): move Task Entry controller below AppShell [maka]
Posted by "chihumyum (via GitHub)" <gi...@apache.org>.
chihumyum commented on PR #4491:
URL: https://github.com/apache/maka/pull/4491#issuecomment-5514563974
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.
--
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