Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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 @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- [EE] Improved Ask Sourcebot prompt caching by splitting static and dynamic prompt sections and advancing cache breakpoints after every agent step instead of only after each message. [#1366](https://github.com/sourcebot-dev/sourcebot/pull/1366)
- Refactored Ask Sourcebot user message text extraction into a shared helper that robustly handles non-text message parts. [#1371](https://github.com/sourcebot-dev/sourcebot/pull/1371)
- [EE] Pinned Ask Sourcebot file citations to the commit they were sourced at so their content and line ranges stay aligned with the code as it was when the answer was generated. [#1397](https://github.com/sourcebot-dev/sourcebot/pull/1397)

### Added
- Added per-step token cost tracking and estimated tool call token usage to Ask Sourcebot chat history. [#1353](https://github.com/sourcebot-dev/sourcebot/pull/1353)
Expand Down
9 changes: 9 additions & 0 deletions docs/api-reference/sourcebot-public.openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,9 @@
},
"webUrl": {
"type": "string"
},
"indexedCommitHash": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -576,6 +579,9 @@
},
"externalWebUrl": {
"type": "string"
},
"commitSha": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -931,6 +937,9 @@
},
"webUrl": {
"type": "string"
},
"indexedCommitHash": {
"type": "string"
}
},
"required": [
Expand Down
1 change: 1 addition & 0 deletions packages/web/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ export const getRepoInfoByName = async (repoName: string) => sew(() =>
externalWebUrl: repo.webUrl ?? undefined,
imageUrl: repo.imageUrl ?? undefined,
indexedAt: repo.indexedAt ?? undefined,
indexedCommitHash: repo.indexedCommitHash ?? undefined,
}
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { getFileSource } from "@/app/api/(client)/client";
import { VscodeFileIcon } from "@/app/components/vscodeFileIcon";
import { Skeleton } from "@/components/ui/skeleton";
import { isServiceError, unwrapServiceError } from "@/lib/utils";
import { ErrorCode } from "@/lib/errorCodes";
import { useQuery } from "@tanstack/react-query";
import { ReactCodeMirrorRef } from '@uiw/react-codemirror';
import { memo, useCallback } from "react";
Expand Down Expand Up @@ -38,13 +39,37 @@ const ReferencedFileSourceListItemContainerComponent = ({
}: ReferencedFileSourceListItemContainerProps) => {
const fileName = fileSource.path.split('/').pop() ?? fileSource.path;

// Prefer the pinned commit SHA so the file renders as it was when answered,
// with line ranges still aligned. Falls back to the symbolic ref.
const fetchRef = fileSource.commitSha ?? fileSource.revision;

const { data, isLoading, isError, error } = useQuery({
queryKey: ['fileSource', fileSource.path, fileSource.repo, fileSource.revision],
queryFn: () => unwrapServiceError(getFileSource({
path: fileSource.path,
repo: fileSource.repo,
ref: fileSource.revision,
})),
queryKey: ['fileSource', fileSource.path, fileSource.repo, fetchRef, fileSource.revision],
queryFn: async () => {
const pinned = await getFileSource({
path: fileSource.path,
repo: fileSource.repo,
ref: fetchRef,
});

// The pinned commit can disappear (e.g. a force-push + GC prunes it),
// which surfaces as an unresolvable git ref. Only that case falls
// back to the symbolic ref; other errors (repo/path/access) are
// surfaced as-is so we don't silently render the wrong revision.
if (
isServiceError(pinned) &&
pinned.errorCode === ErrorCode.INVALID_GIT_REF &&
fetchRef !== fileSource.revision
) {
return unwrapServiceError(getFileSource({
path: fileSource.path,
repo: fileSource.repo,
ref: fileSource.revision,
}));
}

return unwrapServiceError(Promise.resolve(pinned));
},
staleTime: Infinity,
});

Expand Down
5 changes: 3 additions & 2 deletions packages/web/src/features/chat/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,11 @@ export const convertLLMOutputToPortableMarkdown = (text: string, baseUrl: string
end: { lineNumber: parseInt(endLine || startLine) },
} : undefined;

// Construct full browse URL
// Prefer the pinned commit SHA so copied links resolve to the code
// as it was when answered; fall back to the symbolic ref.
const browsePath = getBrowsePath({
repoName: repo,
revisionName: source.revision,
revisionName: source.commitSha ?? source.revision,
path: fileName,
pathType: 'blob',
highlightRange,
Expand Down
55 changes: 42 additions & 13 deletions packages/web/src/features/git/getFileSourceApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,13 @@ describe('getFileSourceForRepo', () => {
mockSimpleGit.mockReturnValue({ cwd: mockCwd });
mockFindFirst.mockResolvedValue(MOCK_REPO);

// Default: file show succeeds; .gitattributes not present
// Default: ref resolves to a concrete sha, file show succeeds, and
// .gitattributes is absent. The SUT resolves the ref first (rev-parse),
// then reads content + .gitattributes at the resolved sha.
mockGitRaw.mockImplementation(async (args: string[]) => {
if (args[0] === 'rev-parse') {
return 'resolvedsha\n';
}
if (args[1]?.endsWith('.gitattributes')) {
throw new Error('does not exist in HEAD');
}
Expand Down Expand Up @@ -170,7 +175,7 @@ describe('getFileSourceForRepo', () => {

describe('git error handling', () => {
it('returns FILE_NOT_FOUND when git reports the file does not exist', async () => {
mockGitRaw.mockRejectedValueOnce(
mockGitRaw.mockRejectedValue(
new Error("fatal: path 'src/missing.ts' does not exist in 'main'"),
);

Expand All @@ -183,7 +188,7 @@ describe('getFileSourceForRepo', () => {
});

it('returns FILE_NOT_FOUND for "fatal: path" errors', async () => {
mockGitRaw.mockRejectedValueOnce(new Error('fatal: path not found'));
mockGitRaw.mockRejectedValue(new Error('fatal: path not found'));

const result = await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo' },
Expand All @@ -196,9 +201,9 @@ describe('getFileSourceForRepo', () => {
it('returns INVALID_GIT_REF with an unresolved-ref message when head_sha has not been fetched ("unknown revision")', async () => {
// This is the scenario from the v4.16.14 regression: the review agent passes
// pr_payload.head_sha as ref, but the bare clone hasn't fetched it yet.
mockGitRaw.mockRejectedValueOnce(
mockGitRaw.mockRejectedValue(
new Error("fatal: ambiguous argument 'deadbeef': unknown revision or path not in the working tree"),
);
); // rejects rev-parse (swallowed) and the show, which drives the result

const result = await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'deadbeef' },
Expand All @@ -212,7 +217,7 @@ describe('getFileSourceForRepo', () => {
});

it('returns INVALID_GIT_REF with an unresolved-ref message for "bad revision" errors', async () => {
mockGitRaw.mockRejectedValueOnce(new Error('fatal: bad revision'));
mockGitRaw.mockRejectedValue(new Error('fatal: bad revision'));

const result = await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'nonexistent' },
Expand All @@ -226,7 +231,7 @@ describe('getFileSourceForRepo', () => {
});

it('returns INVALID_GIT_REF with an unresolved-ref message for "invalid object name" errors', async () => {
mockGitRaw.mockRejectedValueOnce(new Error('fatal: invalid object name HEAD'));
mockGitRaw.mockRejectedValue(new Error('fatal: invalid object name HEAD'));

const result = await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo' },
Expand All @@ -243,7 +248,7 @@ describe('getFileSourceForRepo', () => {
// Before the fix, getFileSourceForRepo re-threw unknown errors.
// Outside sew(), this caused a fatal Next.js task-runner exception.
// After the fix, all errors are returned as ServiceError.
mockGitRaw.mockRejectedValueOnce(new Error('I/O error: device busy'));
mockGitRaw.mockRejectedValue(new Error('I/O error: device busy'));

const result = await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo' },
Expand All @@ -254,7 +259,7 @@ describe('getFileSourceForRepo', () => {
});

it('never rejects its returned promise for unrecognised git errors', async () => {
mockGitRaw.mockRejectedValueOnce(new Error('transient I/O error'));
mockGitRaw.mockRejectedValue(new Error('transient I/O error'));

await expect(
getFileSourceForRepo(
Expand All @@ -280,13 +285,16 @@ describe('getFileSourceForRepo', () => {
});
});

it('uses the provided ref for the git show command', async () => {
it('resolves the provided ref to a commit, then reads content at it', async () => {
await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'abc123sha' },
{ org: MOCK_ORG, prisma: mockPrisma },
);

expect(mockGitRaw).toHaveBeenCalledWith(['show', 'abc123sha:src/index.ts']);
// The provided ref is resolved up front...
expect(mockGitRaw).toHaveBeenCalledWith(['rev-parse', 'abc123sha^{commit}']);
// ...and content is read at the resolved sha, not the symbolic ref.
expect(mockGitRaw).toHaveBeenCalledWith(['show', 'resolvedsha:src/index.ts']);
});

it('falls back to defaultBranch when ref is omitted', async () => {
Expand All @@ -295,7 +303,7 @@ describe('getFileSourceForRepo', () => {
{ org: MOCK_ORG, prisma: mockPrisma },
);

expect(mockGitRaw).toHaveBeenCalledWith(['show', 'main:src/index.ts']);
expect(mockGitRaw).toHaveBeenCalledWith(['rev-parse', 'main^{commit}']);
});

it('falls back to HEAD when both ref and defaultBranch are absent', async () => {
Expand All @@ -306,7 +314,28 @@ describe('getFileSourceForRepo', () => {
{ org: MOCK_ORG, prisma: mockPrisma },
);

expect(mockGitRaw).toHaveBeenCalledWith(['show', 'HEAD:src/index.ts']);
expect(mockGitRaw).toHaveBeenCalledWith(['rev-parse', 'HEAD^{commit}']);
});

it('reads content at the symbolic ref when rev-parse fails', async () => {
mockGitRaw.mockImplementation(async (args: string[]) => {
if (args[0] === 'rev-parse') {
throw new Error('unknown revision');
}
if (args[1]?.endsWith('.gitattributes')) {
throw new Error('does not exist');
}
return 'console.log("hello");';
});

const result = await getFileSourceForRepo(
{ path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'main' },
{ org: MOCK_ORG, prisma: mockPrisma },
);

expect(mockGitRaw).toHaveBeenCalledWith(['show', 'main:src/index.ts']);
expect(result).toMatchObject({ source: 'console.log("hello");' });
expect((result as { commitSha?: string }).commitSha).toBeUndefined();
});

it('uses the repo path from getRepoPath for the git working directory', async () => {
Expand Down
17 changes: 15 additions & 2 deletions packages/web/src/features/git/getFileSourceApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,21 @@ export const getFileSourceForRepo = async (

const gitRef = ref ?? repo.defaultBranch ?? 'HEAD';

// Resolve the symbolic ref to a concrete commit up front so the content,
// language, and commitSha all come from the same revision even if the ref
// moves mid-request. `^{commit}` peels annotated tags. Reads below fall back
// to the symbolic ref when resolution fails.
let commitSha: string | undefined;
try {
commitSha = (await git.raw(['rev-parse', `${gitRef}^{commit}`])).trim();
} catch {
// Leave unpinned; the reads below use the symbolic ref.
}
const readRef = commitSha ?? gitRef;

let fileContent: string;
try {
fileContent = await git.raw(['show', `${gitRef}:${filePath}`]);
fileContent = await git.raw(['show', `${readRef}:${filePath}`]);
} catch (error: unknown) {
const errorMessage = error instanceof Error ? error.message : String(error);
if (errorMessage.includes('does not exist') || errorMessage.includes('fatal: path')) {
Expand All @@ -63,7 +75,7 @@ export const getFileSourceForRepo = async (

let gitattributesContent: string | undefined;
try {
gitattributesContent = await git.raw(['show', `${gitRef}:.gitattributes`]);
gitattributesContent = await git.raw(['show', `${readRef}:.gitattributes`]);
} catch {
// No .gitattributes in this repo/ref, that's fine
}
Expand Down Expand Up @@ -97,6 +109,7 @@ export const getFileSourceForRepo = async (
repoExternalWebUrl: repo.webUrl ?? undefined,
webUrl,
externalWebUrl,
commitSha,
} satisfies FileSourceResponse;
});

Expand Down
2 changes: 2 additions & 0 deletions packages/web/src/features/git/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export const fileSourceResponseSchema = z.object({
repoExternalWebUrl: z.string().optional(),
webUrl: z.string(),
externalWebUrl: z.string().optional(),
// The concrete commit SHA that `ref` resolved to. Undefined if unresolvable.
commitSha: z.string().optional(),
Comment thread
whoisthey marked this conversation as resolved.
});

export const getDiffRequestSchema = z.object({
Expand Down
2 changes: 2 additions & 0 deletions packages/web/src/features/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export const repositoryInfoSchema = z.object({
name: z.string(),
displayName: z.string().optional(),
webUrl: z.string().optional(),
// The commit Zoekt last indexed; lets callers pin a result to that commit.
indexedCommitHash: z.string().optional(),
});
export type RepositoryInfo = z.infer<typeof repositoryInfoSchema>;

Expand Down
1 change: 1 addition & 0 deletions packages/web/src/features/search/zoektSearcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ const transformZoektSearchResponse = async (response: ZoektGrpcSearchResponse, r
name: repo.name,
displayName: repo.displayName ?? undefined,
webUrl: repo.webUrl ?? undefined,
indexedCommitHash: repo.indexedCommitHash ?? undefined,
})),
stats,
}
Expand Down
9 changes: 9 additions & 0 deletions packages/web/src/features/tools/findSymbolDefinitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ export const findSymbolDefinitionsDefinition: ToolDefinition<
codeHostType: repoInfoResult.codeHostType,
};

// Pin to the indexed commit carried by the same search snapshot that
// produced these matches, rather than a follow-up repo-info lookup that
// could drift if the index advances in between.
const indexedCommitShaByRepo = new Map(
response.repositoryInfo.map((info) => [info.name, info.indexedCommitHash]),
);

const metadata: FindSymbolDefinitionsMetadata = {
symbol,
matchCount,
Expand All @@ -72,6 +79,7 @@ export const findSymbolDefinitionsDefinition: ToolDefinition<
fileName: file.fileName,
repo: file.repository,
revision,
commitSha: indexedCommitShaByRepo.get(file.repository),
})),
};

Expand Down Expand Up @@ -105,6 +113,7 @@ export const findSymbolDefinitionsDefinition: ToolDefinition<
path: file.fileName,
name: file.fileName.split('/').pop() ?? file.fileName,
revision: file.revision,
commitSha: file.commitSha,
}));

return {
Expand Down
10 changes: 10 additions & 0 deletions packages/web/src/features/tools/findSymbolReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type FindSymbolFile = {
fileName: string;
repo: string;
revision: string;
commitSha?: string;
};

export type FindSymbolReferencesMetadata = {
Expand Down Expand Up @@ -73,6 +74,13 @@ export const findSymbolReferencesDefinition: ToolDefinition<
codeHostType: repoInfoResult.codeHostType,
};

// Pin to the indexed commit carried by the same search snapshot that
// produced these matches, rather than a follow-up repo-info lookup that
// could drift if the index advances in between.
const indexedCommitShaByRepo = new Map(
response.repositoryInfo.map((info) => [info.name, info.indexedCommitHash]),
);

const metadata: FindSymbolReferencesMetadata = {
symbol,
matchCount,
Expand All @@ -82,6 +90,7 @@ export const findSymbolReferencesDefinition: ToolDefinition<
fileName: file.fileName,
repo: file.repository,
revision,
commitSha: indexedCommitShaByRepo.get(file.repository),
})),
};

Expand Down Expand Up @@ -115,6 +124,7 @@ export const findSymbolReferencesDefinition: ToolDefinition<
path: file.fileName,
name: file.fileName.split('/').pop() ?? file.fileName,
revision: file.revision,
commitSha: file.commitSha,
}));

return {
Expand Down
Loading
Loading