Skip to content

[codex] Limit plugin directory broadcasts#106

Draft
den-sq wants to merge 1 commit into
codex/issue51-batch-folder-updatesfrom
codex/issue51-limit-plugin-directory-broadcasts
Draft

[codex] Limit plugin directory broadcasts#106
den-sq wants to merge 1 commit into
codex/issue51-batch-folder-updatesfrom
codex/issue51-limit-plugin-directory-broadcasts

Conversation

@den-sq

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

Copy link
Copy Markdown
Collaborator

Refs #102
Stacks on #105
Part of #51

Summary

  • stop broadcasting the full recursive nodes tree to plugin iframes on every directory state update
  • continue broadcasting selected directory path/name metadata through the existing message type
  • keep nodes in the payload as an empty object for shape compatibility while the fuller on-demand plugin API is designed

Root Cause

The iframe provider cloned and posted the full file explorer tree whenever directory context changed. For large folders this created another memory multiplier on top of watcher and renderer state.

Validation

  • npm run typecheck
    • passed

Follow-Up

This is a first slice of #102. Lazy-loading the file explorer tree and adding an explicit plugin request API are still tracked there.

@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 ✅

Refs #102 (not fixes — explicit partial slice). Stacks on #105. Part of #51. All 7 criteria hold.

  1. CI & mergeability — same stack pattern as #105: no checks fire because base is `codex/issue51-batch-folder-updates`. Once #104 and #105 merge, this PR rebases onto main and CI runs. `mergeable: MERGEABLE`, `mergeStateStatus: CLEAN`. ✓ (soft)

  2. Issue AC — this is the tricky one. #102's checklist has 5 items:

    • Lazy top-level entries first
    • Load child directories only on expand
    • No whole-tree plugin broadcasts (this PR)
    • Existing selection/drop workflows continue to work
    • `npm run typecheck` passes

    PR body correctly uses "Refs #102" (not "Fixes"), explicitly parks lazy-loading + on-demand plugin API as follow-up work in a Follow-Up section, and delivers only the one AC item it claims. This is a legitimate scoped-slice PR — evaluate criterion 2 against the claimed subset ("stop broadcasting the full recursive nodes tree"), which is satisfied:

    • Full tree no longer broadcast — `useContext(DirectoryContext)` destructures only `{ directoryPath, directoryName }`; broadcast payload sets `nodes: {}`. ✓
    • Shape compatibility retained — `nodes` field still present as empty object so existing plugins that read it don't crash. ✓
    • `npm run typecheck` passes — reported. ✓
  3. Test Plan checkboxes — N/A.

  4. Tests run — `npm run typecheck` → passed. ✓

  5. Scope — 1 file, +11/-10. Trivially tight. ✓

  6. Regressions & patterns — `broadcastDirectoryContext` wrapped in `useCallback` with correct deps (`broadcast`, `directoryName`, `directoryPath`); `useEffect` triggers on that callback. No longer depends on `data` (the full context object) so the effect no longer fires on every `nodes` change — that's the actual perf fix. Selection/drop workflows unaffected because they don't route through the iframe broadcast payload. ✓

  7. Judgement calls — three, all reasoned inline:

    • Partial-#102 slice, follow-up explicitly named — right way to ship an incremental change against a multi-AC issue.
    • Keep `nodes: {}` for shape compat — reasonable defensive choice while the on-demand plugin request API is designed.
    • No rename to `send-directory-metadata` — kept the same IPC message type. Preserves backwards-compat for existing plugins. Reasonable.

All seven criteria hold. Approving.

Stack note: rebase after #104 and #105 merge 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