Skip to content

perf(commands): consolidate command search validation - #620

Open
JustYannicc wants to merge 8 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-wave5-command-search-validation
Open

perf(commands): consolidate command search validation#620
JustYannicc wants to merge 8 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/consolidate-wave5-command-search-validation

Conversation

@JustYannicc

Copy link
Copy Markdown
Collaborator

What changed

  • Consolidates targeted quicklink command refresh, app/settings command discovery metadata caching, root-search perf budget enforcement, and file-index tombstone query reduction into one upstream branch.
  • Adds focused harnesses for quicklink refresh, command discovery metadata caching, root-search perf budgets, root-search scoring perf, and file delete tombstone batches.
  • Updates the shared TS test import helper so validation harnesses can bundle production TS/TSX modules with Electron/React stubs.

Why

  • Reduces the number of open performance micro PRs while keeping this group scoped to commands, search, file-index, and validation.
  • Keeps quicklink edits from triggering full structural command rediscovery when a warm command cache is available.
  • Avoids repeated app/settings metadata subprocess work during command discovery refreshes.
  • Makes root-search performance budgets enforceable in CI-style harnesses.
  • Reduces deleted-file tombstone overhead for large file-index batches and post-delete queries.

Compatibility impact

  • Preserves existing Raycast API compatibility and command/file-search behavior.
  • Quicklink create/update/delete/duplicate handlers still fall back to the existing full command refresh path if targeted refresh cannot use a warm cache.
  • Command discovery caches are path-signature based and keep existing command IDs, titles, keywords, icons, and settings discovery behavior stable.
  • File-index tombstones continue to hide deleted files/directories while compacting deleted-heavy prefix buckets.

How tested

  • npm ci --ignore-scripts --force --no-audit --no-fund
  • node scripts/test-quicklink-command-refresh-cache.mjs
  • node scripts/test-command-discovery-metadata-cache.mjs
  • node --test scripts/test-root-search-perf-budgets.mjs
  • node scripts/test-root-search-perf.mjs with explicit permissive budget env for this local validation run
  • node scripts/test-file-search-delete-batch.mjs
  • node scripts/benchmark-file-search-delete-batch.mjs
  • ./node_modules/.bin/tsc -p tsconfig.main.json --noEmit --pretty false
  • ./node_modules/.bin/tsc -p tsconfig.renderer.json --noEmit --pretty false fails on the same pre-existing upstream-main renderer errors outside the touched renderer utility
  • git diff --check origin/main...HEAD
  • codex-lsp diagnostics for src/main/commands.ts, src/main/file-search-index.ts, src/main/main.ts, and src/renderer/src/utils/command-helpers.tsx: no diagnostics found

Performance evidence

  • Quicklink refresh benchmark: 40 iterations, 2,008 base commands, 80 quicklinks; legacy median 9.272ms with 40 structural discovery starts, targeted median 2.577ms with 0 structural discovery starts and 40 avoided structural rediscoveries.
  • Command discovery metadata benchmark: legacy cold refresh 25 subprocess calls and 14 plist reads; optimized cold refresh 20 subprocess calls and 9 plist reads; stale-cache background refresh 1 subprocess call and 0 plist reads; command parity preserved.
  • Root-search perf harness: 50 commands, 8 queries, 1 iteration; legacy median 25.40ms, optimized median 12.23ms, indexed median 12.01ms, index compile median 0.03ms, legacy-vs-indexed speedup 2.11x.
  • File delete batch benchmark: direct-file batch deleted 500 of 80,161 entries in 5.956ms with post-delete query 8.941ms; directory-tree batch deleted 8,041 of 68,162 entries in 13.65ms with post-delete query 11.98ms and compacted the touched bucket from 8,041 to 0.

Stack validation

  • Branch is based directly on SuperCmdLabs/SuperCmd:main at 9bdd3d2.
  • Final diff scope is limited to command discovery/quicklink refresh, root-search validation/scoring helpers, file-index tombstones, and test validation harness support.
  • Final touched files: scripts/benchmark-file-search-delete-batch.mjs, scripts/lib/ts-import.mjs, scripts/test-command-discovery-metadata-cache.mjs, scripts/test-file-search-delete-batch.mjs, scripts/test-quicklink-command-refresh-cache.mjs, scripts/test-root-search-perf-budgets.mjs, scripts/test-root-search-perf.mjs, src/main/commands.ts, src/main/file-search-index.ts, src/main/main.ts, src/renderer/src/utils/command-helpers.tsx.
  • The listed source PRs referenced earlier same-domain command/search/validation stacks. This branch ports only the minimal prerequisite helpers needed for this consolidation and does not include unrelated renderer/runtime changes or supersede those larger stacks wholesale.

Replaces

@JustYannicc JustYannicc changed the title perf(commands): consolidate wave5 command search validation perf(commands): consolidate command search validation Jul 5, 2026
@shobhit99

Copy link
Copy Markdown
Contributor

Reviewed the full diff (4 production files: commands.ts, file-search-index.ts, main.ts, command-helpers.tsx) plus the 6 test/harness files. This is a perf/scaffolding consolidation; ranking membership and ordering are preserved. It declares Replaces #602/#608/#615/#617 and states it does not wholesale-supersede the larger command/search stacks (only ports minimal helpers), so I reviewed it on its own merits.

Strengths

  • Root-search parity is genuinely preserved. rankCommandsWithIndex (command-helpers.tsx) computes the matched/score used for filtering and sorting from rankingFields, whose weights [title 1, alias 1.06, subtitle 0.74, keyword 0.7] are identical to the old inline call. scoreRootSearchFields.matched is weight-independent and the sort key + all tie-breaks (hasExactAliasMatchalwaysOnTopscoretitle.localeCompare) are byte-identical to the prior implementation. The sole caller (useLauncherCommandModel.ts:530) destructures only command and re-scores independently, so internal-vs-browser precedence downstream is untouched.
  • Quicklink targeted-refresh command shape matches full discovery exactly. discoverQuickLinkCommandInfos produces the same object (id/title/subtitle/keywords/iconDataUrl/iconName/category) as the discoverAndBuildCommands block, and rebuildCommandsWithFreshQuickLinks re-inserts at the original quicklink slot (before system commands), so ordering and the deeplink assignment via publishCommandCache are consistent. The IPC handlers keep a correct full-refresh fallback on error.
  • Tombstone rework is sound. Prefix-neighbor false positives are guarded by the + path.sep descendant prefix; direct-file deletes correctly skip the descendant scan; deletedNormalizedNameCounts is kept consistent (only markEntryDeleted increments, only indexEntry resurrection decrements — the sole two deleted mutations), and bucket compaction only ever drops deleted/missing ids, so live entries are never lost.

Concerns / potential bugs

  • (Medium-low) Runtime-subtitle base drift across targeted refreshescommands.ts applyRuntimeMetadataAndAliases/publishCommandCache. On a quicklink change, refreshCommandsForQuickLinkChange reuses cachedCommands as the base, but those commands already carry the overlaid subtitle from the previous applyStoredRuntimeCommandMetadata. publishCommandCache then calls resetRuntimeMetadataTracking() + rememberRuntimeMetadataBaseSubtitles(commands), recording the overlaid value as the new "base", and saveCommandsDiskCache persists that overlaid value. Failure scenario: an extension sets a dynamic subtitle via updateCommandMetadata, the user then creates/edits a quicklink, and later the override is cleared without a full rediscovery — the subtitle now reverts to the stale overlaid value instead of the true discovered subtitle (and that stale text still matches as a description search field). Before this PR, quicklink changes did a full discoverAndBuildCommands, so the base was always re-derived from source. A full rediscovery (TTL/invalidate) self-heals it, so impact is bounded, but worth confirming/fixing.
  • (Low) Deleted-name search short-circuit can suppress live resultsfile-search-index.ts searchIndexedFiles early-return when indexedResults.length === 0 && (hasExactDeletedIndexMatch || deletedNormalizedNameCounts.has(normalizedQuery)). This skips the live/Spotlight fallback whenever the query equals the full normalized name of any tombstoned entry, even one in an unrelated directory. If the on-disk index is incomplete/still-building for that term while a real live file of the same name exists, the fallback that would surface it is suppressed. Narrow (needs an un-indexed live file sharing the exact normalized name), and it is a deliberate anti-resurface tradeoff — flagging so it is intentional.
  • (Low) path.resolve normalization asymmetrynormalizeDeletedPath resolves delete paths, but they are matched against raw entry.path and used as pathToEntryId.get(resolved) keys. If any indexed entry.path were ever stored non-resolved (trailing sep, symlink, ~), the lookup/prefix match would miss. Watch-event paths are absolute in practice so this is theoretical, but the two sides are no longer normalized identically.

Suggestions (non-blocking)

  • rankCommandsWithIndex runs a second scoreRootSearchFields pass per matched command to populate matchKind/matchScore, which no production consumer reads yet (useLauncherCommandModel recomputes them). For a perf PR, the only current caller rankCommands also rebuilds the index every call, so it now does strictly more work than before with no realized reuse. Consider deferring the scoringFields double-scoring until a consumer actually reads those fields, or wiring the consumer to reuse them in the same change.

Verdict: 🟡 Approve with minor comments — ranking/validation behavior is preserved; the subtitle-base-drift interaction is the one item I would like confirmed or fixed before/soon after merge.

🤖 Automated review by Claude Code (Opus 4.8).

@JustYannicc

JustYannicc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the review feedback from #620 (comment).

Changes pushed in 8c13de0:

  • Fixed targeted quicklink refresh subtitle-base drift by restoring tracked base subtitles before republishing cached non-quicklink commands. Clearing runtime metadata now falls back to the discovered subtitle, and the disk cache stores that base value.
  • Removed the deleted-name global fallback veto in file search. Spotlight fallback now runs for same-name live files and filters only exact tombstoned indexed paths or descendants of tombstoned directories.
  • Normalized indexed path keys with the same path.resolve path form used for delete events, closing the delete/index lookup asymmetry.
  • Restored prefix-index membership when a tombstoned entry is re-indexed after compaction.
  • Removed the second command scoring pass from rankCommandsWithIndex; callers/harnesses recompute match metadata where they need it.
  • A post-change sub-agent review found the prefix-index resurrection gap above; it is included in this push and covered by the file-search test.

Verification:

  • node --test scripts/test-quicklink-command-refresh-cache.mjs scripts/test-file-search-delete-batch.mjs scripts/test-command-discovery-metadata-cache.mjs scripts/test-root-search-ranking.mjs ✅ 20 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