Repository navigation
fix(web): deduplicate repository lookups in search results - #1684
dipeshbabu wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/web/src/features/search/zoektSearcher.test.ts">
<violation number="1" location="packages/web/src/features/search/zoektSearcher.test.ts:188">
P3: The streaming test verifies only file and repositoryInfo counts, never that each `response.files[i].repositoryId` corresponds to the emitted `repository_id`. The unary tests assert this mapping; the streaming chunk test should too, so a cache mis-association or reordering that keeps counts unchanged cannot slip through — it also directly covers the PR's "result order preserved" claim for the cross-chunk cache path.</violation>
</file>
<file name="packages/web/src/features/search/zoektSearcher.ts">
<violation number="1" location="packages/web/src/features/search/zoektSearcher.ts:344">
P3: Within a chunk the dedup is correct, but repositories that are not in the database never get cached (`if (repo) { reposMapCache.set(id, repo); }` only stores hits), so a missing repo referenced by a shard is still looked up once per streaming chunk instead of once per stream. Since this PR exists to collapse repeated lookups, track ids already queried in this stream (e.g., a `Set` of looked-up ids next to `_reposMapCache`) and skip re-querying them.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Great catch @dipeshbabu! Deduplicating repository IDs before resolving metadata eliminates redundant lookups when multiple file matches belong to the same repository. One additional optimization to consider: Currently, the PR deduplicates IDs but still uses Promise.all(repoIds.map(...)) with individual findUnique calls: While this collapses 100 files in 1 repo to 1 query, in broad or multi-repository searches where a chunk spans matches across 15–20 repositories, repoIds.map(findUnique) still fires 15–20 individual concurrent database queries. We could batch all uncached lookups into a single query using Prisma's findMany: This guarantees that regardless of how many repositories a chunk touches (1 or 20), exactly one single batch query is dispatched to PostgreSQL, completely preventing connection pool contention. |
|
Batched uncached IDs with findMany, keeping the scoped Prisma client. Missing rows are cached per stream too. Tests, lint and Docker builds passed. |
|
thanks for the PR - unfortunately considering that this is on a critical path and do not have the bandwidth atm to review this thoroughly, I'm going to close this for the time being. |
Fixes #1681
Batch uncached repository IDs into one scoped Prisma query per chunk and cache missing IDs/names for the request. Preserve legacy lookups and file order; tests cover mixed identifiers and streamed metadata mappings.
Validation: application tests, lint, Docker builds, Prisma migration gate, and regression checks passed.
Note
Low Risk
Search-path performance fix with behavior preserved (file order, legacy name lookups, omitting unknown repos); risk is limited to repository metadata resolution logic, now covered by expanded tests.
Overview
Fixes redundant Prisma calls when enriching Zoekt file matches with repository metadata.
createReposMapForChunknow collects unique repository identifiers per chunk, batches numeric IDs into a singlefindMany, and records missing IDs/names in the per-request cache (Repo | null) so later files and streamed chunks do not re-query. Legacy shards withoutrepository_idstill resolve via onefindFirstper distinct name; numeric IDs and name"1"stay separate.Adds changelog entry and regression tests for unary/streaming search (batching, missing repos, cache scope per stream, mixed ID/name shards).
Reviewed by Cursor Bugbot for commit 0f2600d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #1681 so search results with many files from the same repository no longer trigger a database lookup per file.
Deduplicates repository identifiers before resolving repository metadata in
zoektSearcher.ts, so 100 files from one repository make one lookup and 100 files across two repositories make two. Legacy name-based lookups, missing-repository handling, result order, and the existing metadata cache across streamed chunks are preserved. Adds a changelog entry.Tests
Written for commit cde93da. Summary will update on new commits.
Summary by CodeRabbit