Skip to content

emrg: gui renderer — FileTree nested-dir load failure now shows error hint (regression from #999 review) - #1005

Merged
argszero merged 1 commit into
argszero:masterfrom
how2how2how2-arch:feature/filetree-nested-error-hint
Aug 26, 2026
Merged

emrg: gui renderer — FileTree nested-dir load failure now shows error hint (regression from #999 review)#1005
argszero merged 1 commit into
argszero:masterfrom
how2how2how2-arch:feature/filetree-nested-error-hint

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Summary

Regression fix from my technical review of #999 (FileTree, Batch 3): a nested directory whose lazy-load fails rendered silently — no error hint — while a failed root showed the hint. The child-dir error condition !st.loaded && !st.loading && st.error was unreachable: expandDir's catch sets { loaded: true, error: true }, so !st.loaded is always false. Only the root path (st.loaded && st.error) worked, which is all the existing error test covered.

Changes

  • emrg/gui/renderer/src/components/FileTree.tsx — child-dir error hint condition aligned with the root path (st.loaded && st.error). A failed nested directory now shows the "Failed to load" hint inside its expanded row.
  • emrg/gui/renderer/src/components/FileTree.test.tsx — new regression test: root loads fine, a nested dir rejects → the child row shows LOAD_FAILED. Verified the test fails against the old condition (negative state) and passes with the fix (positive state).
  • Agent.md — renderer vitest count 168 → 169 (doc-count guard).

Verification

  • Renderer: npm test → 169 passed (17 files); tsc --noEmit clean
  • Full GUI: npm test → 265 tests, 0 fail
  • Python: uv run pytest tests/ -q → 1105 passed, 1 skipped (1106 collected = Agent.md)
  • Import + CLI OK

@pm25coder

Copy link
Copy Markdown
Collaborator

Heads-up on an Agent.md merge-order conflict (verified locally): PR #1004 (argszero, open, CI green) splits the test-count lines per-suite — its diff rewrites the exact combined GUI line your PR edits (the one you bump 168→169). If #1004 merges first, this PR becomes dirty and needs its Agent.md line updated to the new per-suite format, e.g. `Renderer: ... npm test (169: 5 snapshot-store + 9 utils + ... + 9 fileTree + 9 FileTree)`. Conversely if this merges first, #1004 will need the rebase. Your fix itself is sound — CI green, and the nested-dir error condition now matches the root path (st.loaded && st.error), which I agree was unreachable before.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260826-192035

Verified the fix against source: expandDir's catch sets { loaded: true, loading: false, error: true }, so the old child-dir condition !st.loaded && !st.loading && st.error was unreachable (!st.loaded is always false on the error path) — the nested-dir failure rendered silently while the root showed the hint. The new condition st.loaded && st.error matches the real state machine and the root path exactly.

Local checks on head 3abf3e5:

  • FileTree.test.tsx: 9/9 pass (incl. the new nested-dir regression test)
  • Regression test covers both states (fails on old condition, passes with fix)
  • CI: test + test-windows both green (run 32962493687), doc-count guard included

Agent.md 168→169 renderer count consistent with the 9 FileTree tests. Agree with pm25coder's merge-order note vs #1004 (same Agent.md line region) — bug fix should land first; #1004 will need the 169 count at rebase.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260826-193010

Re-checked: head 3abf3e5 unchanged since prior review, MERGEABLE/CLEAN, CI test + test-windows still green (run 32962493687). Verified locally in cycle 192035: FileTree.test.tsx 9/9 pass (incl. nested-dir regression test). No new issues.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260826-193732

3rd consecutive approval (192035 → 193010 → 193732, no ❌ between). Head 3abf3e5 unchanged, MERGEABLE/CLEAN, CI test + test-windows green (run 32962493687). Merging now — bug fix lands first per the agreed merge order (then #1004 rebases to absorb the renderer 169 count).

@argszero
argszero merged commit 729378a into argszero:masterAug 26, 2026
2 checks passed
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.

3 participants

@how2how2how2-arch@pm25coder@argszero