Skip to content

perf(diff-viewer): optimize buildFileEntries comment grouping - #619

Merged
matt2e merged 1 commit into
mainfrom
perf/optimize-build-file-entries
Apr 14, 2026
Merged

perf(diff-viewer): optimize buildFileEntries comment grouping#619
matt2e merged 1 commit into
mainfrom
perf/optimize-build-file-entries

Conversation

@matt2e

@matt2e matt2e commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Eliminates O(files × comments) nested loop in buildFileEntries by pre-grouping comments into a Map keyed by basename (filename)
  • Each file now only checks pathsMatch against the small set of comment paths sharing its basename, rather than scanning all comments
  • With 200 files and 50 comments, this reduces pathsMatch calls from ~10,000 to a handful per derived state update

Details

The buildFileEntries function is called from a $derived in DiffModal.svelte, so it re-runs on every state change (file review toggling, new comments, etc.). The previous implementation iterated all comments for every file. The new implementation:

  1. Groups comments by basename into a Map<basename, Map<commentPath, {count, types}>> in a single O(comments) pass
  2. For each file, looks up candidates by basename (O(1)) and only runs pathsMatch against that small group
  3. Preserves the existing pathsMatch suffix-matching semantics exactly

Test plan

  • All existing pathsMatch tests pass (10/10)
  • All staged app tests pass (28/28)
  • svelte-check passes with 0 errors, 0 warnings
  • Open a diff modal with a PR that has many files and many comments; verify comment counts still show correctly on the file tree sidebar
  • Toggle file review status and add comments; verify the diff modal remains responsive

🤖 Generated with Claude Code

…o O(files*comments_per_basename)

Pre-group comments by basename into a Map before iterating files, so each
file only checks the small set of comment paths sharing its basename instead
of scanning all comments. With 200 files and 50 comments this reduces
pathsMatch calls from ~10,000 to a handful per update.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt2e
matt2e requested review from baxen and wesbillman as code owners April 14, 2026 04:11
@matt2e
matt2e merged commit f46e83b into main Apr 14, 2026
6 checks passed
@matt2e
matt2e deleted the perf/optimize-build-file-entries branch April 14, 2026 05:02
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.

1 participant