Skip to content

perf(commands): reduce command and file refresh overhead#573

Open
JustYannicc wants to merge 7 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-command-file-wave2
Open

perf(commands): reduce command and file refresh overhead#573
JustYannicc wants to merge 7 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-command-file-wave2

Conversation

@JustYannicc

Copy link
Copy Markdown
Collaborator

What changed

  • Batch file-search delete tombstone handling by collapsing nested delete roots and marking descendants in one pass.
  • Cache script command file icon data by path signature so repeated discoveries reuse unchanged data URLs and refresh changed icons.
  • Patch cached runtime command metadata in place for subtitle updates instead of invalidating command discovery.
  • Add focused harnesses for delete tombstones, script command icon caching, and command metadata cache behavior on current upstream main.

Why

These hot paths can run during filesystem watcher bursts, script command rediscovery, and @raycast/api metadata updates. The consolidated changes reduce repeated scans and rediscovery while preserving command ordering and existing search/script semantics.

Compatibility impact

  • Filesystem search semantics are preserved; deletes still tombstone exact indexed files, directory descendants, unknown deleted roots, and avoid prefix-like neighbors.
  • Script command icon behavior is preserved; unchanged file icons reuse cached data, changed icon files refresh by size/mtime signature, and emoji icons are unchanged.
  • Metadata updates still persist settings and notify renderers, while cached command subtitles are patched without structural rediscovery.

How tested

  • node --test scripts/test-file-search-delete-batch.mjs
  • node --test scripts/test-script-command-runner.mjs
  • node --test scripts/test-command-metadata-cache.mjs
  • SUPERCMD_BENCH_SCRIPT_ICON_BYTES=4096 node scripts/bench-script-command-discovery.mjs
  • node scripts/benchmark-file-search-delete-batch.mjs
  • tsc -p tsconfig.main.json --noEmit --pretty false
  • Codex LSP diagnostics for src/main/file-search-index.ts, src/main/script-command-runner.ts, src/main/commands.ts, and src/main/main.ts
  • git diff --check

Performance evidence

  • File delete benchmark: direct-file batch tombstoned 500 of 80,161 entries in 5.681 ms; directory-tree batch tombstoned 8,041 of 68,162 entries across 4,041 delete paths in 8.145 ms.
  • Script command benchmark with 40 one MiB scripts and 4 KiB file icons: initial discovery read 80 files / 40.2 MiB; invalidated discovery read 40 files / 40.0 MiB, skipping the 40 unchanged icon reads.
  • Command metadata benchmark: 5,000 commands across 60 iterations avoided 60 structural discovery starts; optimized median get-commands path was 0.035 ms versus legacy 0.167 ms.

Stack validation

Replaces

@shobhit99

Copy link
Copy Markdown
Contributor

Review — perf(commands): reduce command and file refresh overhead

Focused, well-tested perf refactor of three independent hot paths (command-metadata cache, file-search delete tombstones, script-command icon caching). I traced each against upstream main; semantics are preserved and the new harnesses are meaningful.

Strengths

  • Command metadata: applyCommandMetadataUpdate correctly preserves the subtitle-eligibility gate (!(script && mode !== 'inline')), keys extension commands by both id and path, and patches the live CommandInfo objects shared by cachedCommands and staleCommandsFallback. Because both sites saveSettings(...) before patching, the change survives a concurrent rediscovery (which re-applies from settings via applyStoredRuntimeCommandMetadata). saveCommandsDiskCache now writes the base subtitle and re-applies overrides on load — correct round-trip.
  • main.ts:15240 is a net improvement: the old code only called invalidateCache() (never broadcast despite the "Notify all windows" comment); the new path actively broadcastCommandsUpdated() on change and avoids a full rediscovery.
  • File-search tombstones: collapse is sound — sorting by length guarantees an ancestor always precedes its descendant, so hasDeletedPathAncestor de-dupes nested roots correctly. File-vs-directory descendant handling and the unknown-root case match the old prefix behavior. The path.resolve re-normalization in normalizeDeletedPath is safe: index entries (file-search-index.ts:700/781/785) and watch delete paths are already path.resolve'd, so keys still match pathToEntryId.
  • Icon cache: size:mtimeMs signature, drops the entry when the file disappears, and re-reads on change — behavior parity with the old existsSync+read (statSync/existsSync both follow symlinks). Good targeted test for cache hit vs. refresh.

Concerns / potential bugs

  • Minor — src/main/main.ts:15240 (update-command-metadata): unlike the old unconditional invalidateCache(), the new path does nothing when changedCommands === 0. If a fresh (non-stale) cache genuinely doesn't contain the target command, the persisted subtitle won't surface in the in-memory list until CACHE_TTL (30 min) or the next invalidation. This self-heals in the common cases (a running extension's command is always in cache; a null cachedCommands re-discovers via the stale path; the value is always persisted to settings), so it's narrow — but a one-line fallback (if (patchResult.matchedCommands === 0) invalidateCache();) would fully restore the old guarantee.
  • Very minor — src/main/script-command-runner.ts (getCachedIconDataUrl): a negative result (dataUrl: undefined from a transient read failure) is cached until the file's size/mtime changes, whereas the old code retried every discovery. Also the module-level iconDataUrlCache is unbounded, though bounded in practice by the number of script-command icons. Both are acceptable.

Suggestions (optional)

  • Consider the matchedCommands === 0 → invalidateCache() fallback above for full parity.
  • useBackgroundRefresh.ts is untouched (interval behavior unchanged) — worth confirming that's intentional given the PR title mentions "refresh overhead."

Overlaps conceptually with #620 (command search validation) but touches different code (discovery/refresh vs. search), so no duplication. Types are sound (subtitle?/mode?/path? all optional, so the in-place mutate + delete are valid) and the change is plausibly compiling per the reported tsc -p tsconfig.main.json.

🟡 Approve with minor comments

🤖 Automated review by Claude Code (Opus 4.8).

@JustYannicc

JustYannicc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the maintainer follow-up from the top-level review.

Changes:

  • Restored the old metadata-update guarantee for the narrow no-match case: update-command-metadata now patches the live cache when possible, but falls back to invalidating the command cache when matchedCommands === 0, so persisted metadata is re-applied on the next command fetch instead of waiting for the TTL.
  • Added regression coverage for that fresh-cache/missing-command fallback path.
  • Hardened the script-command icon cache by only caching successful data URLs, retrying after transient read failures, and capping the cache size.
  • Added regression coverage for the transient icon read-failure case.

Notes:

  • Confirmed via GraphQL that there are no inline review threads or formal review bodies on this PR; this addresses the actionable top-level feedback.
  • src/renderer/src/hooks/useBackgroundRefresh.ts remains intentionally untouched: the PR changes are scoped to command metadata patching, file-search delete tombstones, and script-command icon reads; renderer interval behavior is unchanged.

Verification:

  • node --test scripts/test-command-metadata-cache.mjs scripts/test-file-search-delete-batch.mjs scripts/test-script-command-runner.mjs scripts/test-root-search-ranking.mjs ✅ 14 passed
  • npm exec -- tsc -p tsconfig.main.json --noEmit
  • git diff --check

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