Skip to content

fix(web): deduplicate repository lookups in search results - #1684

Open
dipeshbabu wants to merge 3 commits into
sourcebot-dev:mainfrom
dipeshbabu:dipeshbabu/fix-search-repository-lookups-1681
Open

dipeshbabu wants to merge 3 commits into
sourcebot-dev:mainfrom
dipeshbabu:dipeshbabu/fix-search-repository-lookups-1681

Conversation

@dipeshbabu

@dipeshbabu dipeshbabu commented Sep 24, 2026 •

Copy link
Copy Markdown

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-result enrichment only; behavior is preserved with broad regression tests and no API or auth changes.

Overview
Fixes redundant Postgres work when Zoekt returns many file matches that share the same repository.

createReposMapForChunk in zoektSearcher.ts no longer runs a findUnique / findFirst per file. It collects unique repository keys from the chunk, skips keys already in the stream/unary cache, batches numeric IDs into a single findMany, and resolves each legacy name once. Missing numeric IDs are cached as null so later streamed chunks do not repeat failed lookups; files for unknown repos are still dropped as before.

Regression tests in zoektSearcher.test.ts cover batching, legacy name shards, missing repos, streaming cache reuse across chunks, and mixed numeric ID vs name identifiers. The unreleased changelog notes the fix.

Reviewed by Cursor Bugbot for commit 5fd8298. 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

  • Adds regression coverage for repository ID deduplication, name-based lookups, missing repositories, and streaming chunks reusing cached metadata.

Written for commit cde93da. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Reduced duplicate repository metadata lookups while processing search result chunks, including across streamed search results.
    • Batched lookups for numeric repository IDs and avoided repeating lookups for legacy repository names.
    • Improved handling of search results with missing repository information, preventing repeated checks and excluding files whose repositories could not be found.

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 43c689ce-5389-440a-a151-b76aef8d5e67

📥 Commits

Reviewing files that changed from the base of the PR and between cde93da and 5fd8298.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • packages/web/src/features/search/zoektSearcher.test.ts
  • packages/web/src/features/search/zoektSearcher.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.


Walkthrough

The searcher now deduplicates repository lookups within each result chunk. It batches uncached numeric IDs, resolves legacy names, and caches missing repositories. Tests cover unary and streaming searches.

Changes

Repository lookup deduplication

Layer / File(s) Summary
Deduplicate repository lookups per chunk
packages/web/src/features/search/zoektSearcher.ts, packages/web/src/features/search/zoektSearcher.test.ts, CHANGELOG.md
createReposMapForChunk batches lookups for unique numeric IDs and resolves legacy names. It caches missing results. Tests cover lookup batching, missing repositories, legacy names, and streaming cache behavior. The changelog records the fix.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: brendan-kellam

Merge Risk: ⚪ Minimal · up to 5fd82

This change reduces redundant repository lookups during search without altering the search request or response contracts. No merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #1681. createReposMapForChunk deduplicates repository identifiers per chunk. It batches numeric repository IDs with one findMany call and deduplicates legacy name lookups with …
Out of Scope Changes check ✅ Passed The changes stay within #1681. The production change removes duplicate repository lookups. The tests verify lookup batching, missing-repository behavior, legacy compatibility, and streamed-cache behav…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: deduplicating repository lookups in web search results.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 3 files

Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot 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.

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

Comment thread packages/web/src/features/search/zoektSearcher.test.ts Outdated
Comment thread packages/web/src/features/search/zoektSearcher.ts
@riteshvish02

Copy link
Copy Markdown

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:

const repoIds = [...new Set(chunk.files.map(getRepoIdForFile))];
await Promise.all(repoIds.map(async (id) => {
    ...
    await prisma.repo.findUnique({ where: { id } })
}));

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:

const repos = await prisma.repo.findMany({
    where: {
        id: { in: uncachedNumericIds }
    }
});

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.

@dipeshbabu

Copy link
Copy Markdown
Author

Batched uncached IDs with findMany, keeping the scoped Prisma client. Missing rows are cached per stream too. Tests, lint and Docker builds passed.

This branch has not been deployed

No deployments
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.

[bug] Search result hydration performs duplicate repository lookups within the same chunk

2 participants