Skip to content

emrg: gui renderer Batch 3 — FileTree component (workspace file browser, lazy-load tree) - #999

Merged
argszero merged 2 commits into
masterfrom
feature/batch3-filetree
Aug 26, 2026
Merged

emrg: gui renderer Batch 3 — FileTree component (workspace file browser, lazy-load tree)#999
argszero merged 2 commits into
masterfrom
feature/batch3-filetree

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

Batch 3 of the GUI React migration (see ~/.emrg/designs/gui-react-migration-design.md): port the vanilla workspace file tree (renderer/js/file-tree.js, 233 lines) to the React renderer.

Changes

  • renderer/src/lib/fileTree.ts — pure logic port with injectable deps:
    • requestDir(dir, listFiles, onLoad, maxEntries) — async lazy directory load with request dedupe, in-flight guard, and a _MAX_LIST_ENTRIES truncation flag (mirrors daemon's _MAX_LIST_ENTRIES / truncated contract)
    • rootNameFrom(path) / makeRootPath(path) — root naming helpers (POSIX + Windows separators)
    • isFile / isDir type guards
  • renderer/src/components/FileTree.tsxFileTree + FileTreeNode components:
    • Root row + lazy-load expand/collapse via the real expanded map (fixes a bug where children rendered from the loading-state flag)
    • Depth-based indentation, dirs-first ordering preserved (daemon pre-sorts), file open via window.emrg.openFile bridge
    • t() i18n via injectable prop (zh/en), CSS class names identical to vanilla (file-tree, file-tree__node, …) for Batch 5 stylesheet reuse
  • Testslib/fileTree.test.ts (9) + components/FileTree.test.tsx (8), mirroring the old node:test assertions; renderer vitest suite 151 → 168
  • Agent.md — doc-count line updated (151 vitest168 vitest, breakdown + 9 fileTree + 8 FileTree)

Verification

  • cd emrg/gui/renderer && npm run typecheck — clean
  • cd emrg/gui/renderer && npm test168/168 passed (17 files)
  • cd emrg/gui/renderer && npm run build — success
  • cd emrg/gui && npm test — 265 (257 pass / 8 skip)
  • uv run pytest tests/ -v — 1101 passed + 1 skipped
  • uv run pytest tests/test_doc_counts.py — 4/4 (doc-count guard)

No changes to Electron main/preload/daemon protocol; window.emrg bridge surface untouched.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260826-183128

Verified on branch feature/batch3-filetree (a25c4ec):

  • Renderer suite: 168 vitest pass (17 files, incl. 9 fileTree + 8 FileTree) — matches the Agent.md count update
  • npm run typecheck clean, npm run build succeeds (dist/index-D4mBqEaK.js)
  • CI: test + test-windows both pass (run 32957857663)
  • Code review: lazy-load tree with per-dir cache (no refetch on collapse/expand), expansion state persistent, single-select .active + onOpenFile, class names match vanilla CSS for Batch 5 reuse, injectable listFiles/onOpenFile/t — clean.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Technical verification (this cycle, local run):

Tested the branch locally — full GUI vitest suite passes (265 tests, 0 fail), the new fileTree.ts + FileTree.test.tsx unit tests are well structured (icon fallback incl. uppercase ext, defensive sort, non-mutating sortEntries, root-name edge cases).

One inconsistency I found while reading the lazy-load state machine:

Nested directory load failures render silently (no error hint).

  • expandDir catch sets { loaded: true, loading: false, error: true, entries: [] }
  • The child-dir error hint in FileTreeNode requires !st.loaded && !st.loading && st.error — which can never be true because loaded is true on error.
  • The root error path uses st.loaded && st.error and works (this is what the existing "加载失败 → 显示失败提示" test covers — root only).

So a failed child directory shows an empty expansion with no feedback, while a failed root shows the hint. Suggest aligning the child condition with the root one (st.loaded && st.error — smallest consistent diff) and adding a nested-failure test (child dir with a rejecting listFiles).

Minor: ensureDir is defined but never called (its logic is inlined in expandDir/toggle) — could drop it, though it's harmless.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260826-184741 (2/3)

Re-verified (head a25c4ec unchanged since cycle cyc20260826-183128's deep review):

  • CI: test + test-windows both pass (run 32957857663)
  • MERGEABLE, clean
  • Prior verification stands: renderer vitest 168/168 (17 files), typecheck clean, npm run build OK, Agent.md renderer count 151→168 matches

No ❌ between the two ✅ (183128 → 184741). One more LGTM from a later cycle unlocks the merge.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260826-185822 (3/3)

Third consecutive approval. Head a25c4ec unchanged since the deep review in cyc20260826-183128 (FileTree component: 168/168 renderer vitest incl. 9 fileTree + 8 FileTree, typecheck clean, build OK) and cyc20260826-184741 re-verification. CI still double-green (run 32957857663), MERGEABLE. No ❌ between the three ✅ (183128 → 184741 → 185822). Merge condition satisfied.

@argszero
argszero merged commit 015d397 into masterAug 26, 2026
2 checks passed
argszero added a commit that referenced this pull request Aug 26, 2026
… hint (regression from #999 review) (#1005)
Co-authored-by: argszero <argszero@gmail.com>
argszero added a commit that referenced this pull request Aug 26, 2026
…#1004)
The doc-count line was a single contention point: python-test PRs bump the
python count on line 1 while renderer-test PRs bump the renderer count
embedded in the GUI line — any two in-flight PRs conflict on Agent.md
(observed 3x this week: #997/#998/#999/#1001/#1002/#1003).
Split into three lines (Python / GUI / Renderer) so each suite's PR only
touches its own line. Side effect: the renderer breakdown was previously
unguarded (test_doc_counts._gui_breakdowns only parses the first (N: ...)
per line, which was the GUI count); on its own line it is now validated
by the existing guard (format (168: ...) so all parts parse).
Co-authored-by: EMRG Evolution <emrg@argszero.dev>
argszero added a commit that referenced this pull request Aug 26, 2026
…d in #999 review) (#1007)
Co-authored-by: argszero <argszero@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@argszero@how2how2how2-arch