Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Made the default home page configurable with `DEFAULT_HOME_VIEW_PAGE`, defaulting to Code Search and supporting Ask. [#1677](https://github.com/sourcebot-dev/sourcebot/pull/1677)
- Require authentication for the streaming and blocking Ask APIs in Public SaaS deployments. [#1679](https://github.com/sourcebot-dev/sourcebot/pull/1679)
- Fixed duplicate repository metadata lookups within search result chunks. [#1684](https://github.com/sourcebot-dev/sourcebot/pull/1684)

## [5.1.14] - 2026-09-17

Expand Down
94 changes: 93 additions & 1 deletion packages/web/src/features/search/zoektSearcher.test.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
import type { PrismaClient } from '@sourcebot/db';
import type { SearchRequest as ZoektGrpcSearchRequest } from '@/proto/zoekt/webserver/v1/SearchRequest';
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { EventEmitter } from 'node:events';

const mocks = vi.hoisted(() => {
const close = vi.fn();
const search = vi.fn();
const streamSearch = vi.fn();

class WebserverService {
Search = search;
StreamSearch = streamSearch;
close = close;
}

return {
close,
loadSync: vi.fn(() => ({})),
search,
streamSearch,
WebserverService,
};
});
Expand Down Expand Up @@ -60,10 +64,27 @@ vi.mock('@/lib/posthog', () => ({
captureEvent: vi.fn(),
}));

import { zoektSearch } from './zoektSearcher';
import { zoektSearch, zoektStreamSearch } from './zoektSearcher';

const searchRequest = {} as ZoektGrpcSearchRequest;

const createFile = (id: number | undefined, repository = 'github.com/org/repo') => ({
repository_id: id,
repository,
file_name: Buffer.from('src/index.ts'),
chunk_matches: [],
branches: ['main'],
language: 'TypeScript',
});

const createRepo = (id: number, name = 'github.com/org/repo') => ({
id,
name,
displayName: name,
webUrl: null,
external_codeHostType: 'github',
});

describe('zoektSearch', () => {
beforeEach(() => {
vi.clearAllMocks();
Expand Down Expand Up @@ -104,4 +125,75 @@ describe('zoektSearch', () => {
await expect(zoektSearch(searchRequest, prisma)).rejects.toThrow('database unavailable');
expect(mocks.close).toHaveBeenCalledOnce();
});

test.each([1, 2])('looks up each of %i repositories only once for 100 files', async (repoCount) => {
const files = Array.from({ length: 100 }, (_, index) => createFile(index % repoCount + 1));
mocks.search.mockImplementation((_request, _metadata, callback) => {
callback(null, { files });
});
const findUnique = vi.fn(async ({ where: { id } }) => createRepo(id));
const prisma = { repo: { findUnique } } as unknown as PrismaClient;

const response = await zoektSearch(searchRequest, prisma);

expect(findUnique).toHaveBeenCalledTimes(repoCount);
expect(response.files).toHaveLength(100);
expect(response.files.map(file => file.repositoryId)).toEqual(files.map(file => file.repository_id));
expect(response.repositoryInfo).toHaveLength(repoCount);
});

test('deduplicates lookups by name for legacy shards without repository IDs', async () => {
mocks.search.mockImplementation((_request, _metadata, callback) => {
callback(null, { files: Array.from({ length: 100 }, () => createFile(undefined)) });
});
const findFirst = vi.fn().mockResolvedValue(createRepo(1));
const prisma = { repo: { findFirst } } as unknown as PrismaClient;

const response = await zoektSearch(searchRequest, prisma);

expect(findFirst).toHaveBeenCalledExactlyOnceWith({ where: { name: 'github.com/org/repo' } });
expect(response.files).toHaveLength(100);
});

test('looks up a missing repository once and omits its files', async () => {
mocks.search.mockImplementation((_request, _metadata, callback) => {
callback(null, { files: Array.from({ length: 100 }, () => createFile(1)) });
});
const findUnique = vi.fn().mockResolvedValue(null);
const prisma = { repo: { findUnique } } as unknown as PrismaClient;

const response = await zoektSearch(searchRequest, prisma);

expect(findUnique).toHaveBeenCalledOnce();
expect(response.files).toEqual([]);
expect(response.repositoryInfo).toEqual([]);
});

test('deduplicates streaming chunks and reuses repository metadata across chunks', async () => {
const grpcStream = Object.assign(new EventEmitter(), {
pause: vi.fn(),
resume: vi.fn(),
cancel: vi.fn(),
});
mocks.streamSearch.mockReturnValue(grpcStream);
const findUnique = vi.fn(async ({ where: { id } }) => createRepo(id));
const prisma = { repo: { findUnique } } as unknown as PrismaClient;
const stream = await zoektStreamSearch(searchRequest, prisma);
const reader = stream.getReader();

for (const ids of [[1, 1, 2, 2], [1, 2, 3, 3]]) {
grpcStream.emit('data', { response_chunk: { files: ids.map(id => createFile(id)) } });
const chunk = await reader.read();
const response = JSON.parse(new TextDecoder().decode(chunk.value).slice('data: '.length));
expect(response.files).toHaveLength(ids.length);
Comment thread
dipeshbabu marked this conversation as resolved.
Outdated
expect(response.repositoryInfo).toHaveLength(new Set(ids).size);
}

expect(findUnique).toHaveBeenCalledTimes(3);
grpcStream.emit('end');
while (!(await reader.read()).done) {
// Drain the final statistics and completion marker.
}
expect(mocks.close).toHaveBeenCalledOnce();
});
});
5 changes: 2 additions & 3 deletions packages/web/src/features/search/zoektSearcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,8 @@ const encodeSSEREsponseChunk = (response: object | string) => {
// chunk. The mapping allows us to efficiently lookup repository metadata.
const createReposMapForChunk = async (chunk: ZoektGrpcSearchResponse, reposMapCache: Map<string | number, Repo>, prisma: PrismaClient): Promise<Map<string | number, Repo>> => {
const reposMap = new Map<string | number, Repo>();
await Promise.all(chunk.files.map(async (file) => {
const id = getRepoIdForFile(file);

const repoIds = [...new Set(chunk.files.map(getRepoIdForFile))];
Comment thread
dipeshbabu marked this conversation as resolved.
await Promise.all(repoIds.map(async (id) => {
const repo = await (async () => {
// If it's in the cache, return the cached value.
if (reposMapCache.has(id)) {
Expand Down