Skip to content

[codex] Batch file explorer updates#105

Draft
den-sq wants to merge 1 commit into
codex/issue51-watch-guardrailsfrom
codex/issue51-batch-folder-updates
Draft

[codex] Batch file explorer updates#105
den-sq wants to merge 1 commit into
codex/issue51-watch-guardrailsfrom
codex/issue51-batch-folder-updates

Conversation

@den-sq

@den-sq den-sq commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #103
Stacks on #104
Part of #51

Summary

  • queue file explorer watcher events in the main process
  • flush queued events every 100 ms or every 500 events
  • send batches through folder-contents-update-batch
  • apply each batch in one renderer state update while keeping the old single-event listener

Root Cause

Large initial folder scans could send one IPC message and one React state update per path. That amplified the memory and UI churn from recursive watching.

Validation

  • npm run typecheck
    • passed

@tavateva tavateva left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Draft Promotion Criteria PASS ✅

Fixes #103. Stacks on #104. All 7 criteria hold (criterion 1 clarified for stacked PRs).

  1. CI & mergeability — no configured checks fire on this branch because it targets `codex/issue51-watch-guardrails` (i.e. #104's branch), not `main` — the coverage workflow is scoped to main-targeted PRs. Once #104 merges, this PR's base auto-updates to `main` and CI runs at that point. `mergeable: MERGEABLE`, `mergeStateStatus: CLEAN`. Accept as pass with the note that CI verification is deferred to the post-#104 merge. ✓ (soft)

  2. Issue #103 AC — 4 items, all addressed:

    • Batched IPC payloads — `queueFSEvent` accumulates in `pendingFSEvents`, flushes on either 500-event limit or 100ms timer. `folder-contents-update-batch` IPC channel carries `FSEvent[]`. ✓
    • One state update per batch — renderer listener applies `setNodes((prev) => handleFSEvents(prev, fsEvents))` where `handleFSEvents` reduces the array through the existing per-event reducer. Single state update. ✓
    • Rename/delete/add handling remains covered by existing reducer — `handleFSEvents` = `fsEvents.reduce(handleFSEvent, nodes)`, so every batch entry flows through the unchanged `handleFSEvent` logic. ✓
    • `npm run typecheck` passes — reported. ✓
  3. Test Plan checkboxes — N/A.

  4. Tests run — `npm run typecheck` → passed. Same treatment as #104. ✓

  5. Scope — 2 files: `filesystem.ts` (queue + flush + clear) and `DirectoryContext.tsx` (batch listener + `handleFSEvents` helper). Tight. ✓

  6. Regressions & patterns — kept the old single-event `folder-contents-update` listener alongside the new batch listener ("keeps old single-event listener" in PR summary). Good back-compat during the rollout. `clearPendingFSEvents` is called on new `fetch-folder-contents` invocations so a batch from a previous folder doesn't leak into the next. ✓

  7. Judgement calls — 500 batch / 100ms interval — reasonable UI-latency + IPC-overhead tradeoff, not explicitly reasoned inline (small nit). Keeping the old single-event listener alongside the batch listener isn't explicitly justified — I'd read it as either back-compat during rollout or a safety net for edge-case events that don't go through `queueFSEvent`, but a one-liner would help. Both are doc nits, non-blocking. ✓

All seven criteria hold. Approving.

Note on stack: once #104 merges, please rebase this branch so CI fires against `main` before final merge.

Sign up for free to 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