diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d335d2f1..72e04670c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- Improved diff tool UI to truncate full commit SHAs to 7 characters, use RepoBadge component for repository display, and output diffs in git-diff format instead of JSON for better token efficiency. [#1146](https://github.com/sourcebot-dev/sourcebot/pull/1146) + ### Fixed - Fixed a missing error boundary in `getFileSourceForRepo` introduced in v4.16.14: the function was extracted outside `sew()` but still re-threw unrecognised git exceptions, causing fatal Next.js task-runner errors. All error paths now return a `ServiceError`. Also tightened the error message for unresolved git refs (e.g. an unfetched `head_sha`) to distinguish them from syntactically invalid refs. [#1145](https://github.com/sourcebot-dev/sourcebot/pull/1145) diff --git a/packages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsx b/packages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsx index b25fb6656..e52225cf0 100644 --- a/packages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsx +++ b/packages/web/src/features/chat/components/chatThread/tools/getDiffToolComponent.tsx @@ -2,6 +2,16 @@ import { Separator } from '@/components/ui/separator'; import { GetDiffMetadata, ToolResult } from '@/features/tools'; +import { GitCommitHorizontalIcon } from 'lucide-react'; +import { RepoBadge } from './repoBadge'; + +function truncateSha(ref: string): string { + const match = ref.match(/^([0-9a-f]{40})(.*)$/i); + if (match) { + return match[1].substring(0, 7) + match[2]; + } + return ref; +} export const GetDiffToolComponent = ({ metadata }: ToolResult) => { const fileCount = metadata.files.length; @@ -9,17 +19,17 @@ export const GetDiffToolComponent = ({ metadata }: ToolResult) return (
Compared - - {metadata.base} + + + {truncateSha(metadata.base)} to - - {metadata.head} + + + {truncateSha(metadata.head)} in - - {metadata.repo} - + {fileCount} changed {fileCount === 1 ? 'file' : 'files'} diff --git a/packages/web/src/features/tools/getDiff.ts b/packages/web/src/features/tools/getDiff.ts index 803a9429d..a19cb668a 100644 --- a/packages/web/src/features/tools/getDiff.ts +++ b/packages/web/src/features/tools/getDiff.ts @@ -2,11 +2,21 @@ import { getDiff, GetDiffResult } from '@/features/git'; import { getDiffRequestSchema } from '@/features/git/schemas'; import { isServiceError } from '@/lib/utils'; import description from './getDiff.txt'; +import { formatDiffAsGitDiff } from './utils'; import { logger } from './logger'; import { ToolDefinition } from './types'; +import { CodeHostType } from '@sourcebot/db'; +import { getRepoInfoByName } from '@/actions'; + +export type GetDiffRepoInfo = { + name: string; + displayName: string; + codeHostType: CodeHostType; +}; export type GetDiffMetadata = GetDiffResult & { repo: string; + repoInfo: GetDiffRepoInfo; base: string; head: string; }; @@ -27,11 +37,24 @@ export const getDiffDefinition: ToolDefinition<'get_diff', typeof getDiffRequest throw new Error(response.message); } + const repoInfoResult = await getRepoInfoByName(repo); + if (isServiceError(repoInfoResult) || !repoInfoResult) { + throw new Error(`Repository "${repo}" not found.`); + } + const repoInfo: GetDiffRepoInfo = { + name: repoInfoResult.name, + displayName: repoInfoResult.displayName ?? repoInfoResult.name, + codeHostType: repoInfoResult.codeHostType, + }; + + const gitDiffOutput = formatDiffAsGitDiff(response); + return { - output: JSON.stringify(response), + output: gitDiffOutput, metadata: { ...response, repo, + repoInfo, base, head, }, diff --git a/packages/web/src/features/tools/utils.test.ts b/packages/web/src/features/tools/utils.test.ts new file mode 100644 index 000000000..56bc0796d --- /dev/null +++ b/packages/web/src/features/tools/utils.test.ts @@ -0,0 +1,205 @@ +import { describe, it, expect } from 'vitest'; +import type { z } from 'zod'; +import type { getDiffResponseSchema } from '@/features/git/schemas'; +import { formatDiffAsGitDiff } from './utils'; + +type GetDiffResult = z.infer; + +describe('formatDiffAsGitDiff', () => { + it('should format a simple file change correctly', () => { + const input: GetDiffResult = { + files: [ + { + oldPath: 'file.txt', + newPath: 'file.txt', + hunks: [ + { + oldRange: { start: 1, lines: 3 }, + newRange: { start: 1, lines: 4 }, + heading: undefined, + body: ' context line\n-removed line\n+added line\n context line', + }, + ], + }, + ], + }; + + const expected = `--- a/file.txt ++++ b/file.txt +@@ -1,3 +1,4 @@ + context line +-removed line ++added line + context line +`; + + expect(formatDiffAsGitDiff(input)).toBe(expected); + }); + + it('should handle file deletion', () => { + const input: GetDiffResult = { + files: [ + { + oldPath: 'deleted.txt', + newPath: null, + hunks: [ + { + oldRange: { start: 1, lines: 2 }, + newRange: { start: 0, lines: 0 }, + heading: undefined, + body: '-line 1\n-line 2', + }, + ], + }, + ], + }; + + const expected = `--- a/deleted.txt ++++ /dev/null +@@ -1,2 +0,0 @@ +-line 1 +-line 2 +`; + + expect(formatDiffAsGitDiff(input)).toBe(expected); + }); + + it('should handle file addition', () => { + const input: GetDiffResult = { + files: [ + { + oldPath: null, + newPath: 'new.txt', + hunks: [ + { + oldRange: { start: 0, lines: 0 }, + newRange: { start: 1, lines: 2 }, + heading: undefined, + body: '+line 1\n+line 2', + }, + ], + }, + ], + }; + + const expected = `--- /dev/null ++++ b/new.txt +@@ -0,0 +1,2 @@ ++line 1 ++line 2 +`; + + expect(formatDiffAsGitDiff(input)).toBe(expected); + }); + + it('should include hunk heading when present', () => { + const input: GetDiffResult = { + files: [ + { + oldPath: 'code.ts', + newPath: 'code.ts', + hunks: [ + { + oldRange: { start: 10, lines: 5 }, + newRange: { start: 10, lines: 6 }, + heading: 'function myFunction()', + body: ' function myFunction() {\n+ console.log("new line");\n return true;\n }', + }, + ], + }, + ], + }; + + const expected = `--- a/code.ts ++++ b/code.ts +@@ -10,5 +10,6 @@ function myFunction() + function myFunction() { ++ console.log("new line"); + return true; + } +`; + + expect(formatDiffAsGitDiff(input)).toBe(expected); + }); + + it('should handle multiple files', () => { + const input: GetDiffResult = { + files: [ + { + oldPath: 'file1.txt', + newPath: 'file1.txt', + hunks: [ + { + oldRange: { start: 1, lines: 1 }, + newRange: { start: 1, lines: 2 }, + heading: undefined, + body: ' old\n+new', + }, + ], + }, + { + oldPath: 'file2.txt', + newPath: 'file2.txt', + hunks: [ + { + oldRange: { start: 1, lines: 1 }, + newRange: { start: 1, lines: 1 }, + heading: undefined, + body: ' unchanged', + }, + ], + }, + ], + }; + + const expected = `--- a/file1.txt ++++ b/file1.txt +@@ -1,1 +1,2 @@ + old ++new +--- a/file2.txt ++++ b/file2.txt +@@ -1,1 +1,1 @@ + unchanged +`; + + expect(formatDiffAsGitDiff(input)).toBe(expected); + }); + + it('should handle multiple hunks in a single file', () => { + const input: GetDiffResult = { + files: [ + { + oldPath: 'file.txt', + newPath: 'file.txt', + hunks: [ + { + oldRange: { start: 1, lines: 2 }, + newRange: { start: 1, lines: 2 }, + heading: undefined, + body: ' line 1\n+line 2', + }, + { + oldRange: { start: 10, lines: 2 }, + newRange: { start: 11, lines: 2 }, + heading: undefined, + body: '-line 10\n line 11', + }, + ], + }, + ], + }; + + const expected = `--- a/file.txt ++++ b/file.txt +@@ -1,2 +1,2 @@ + line 1 ++line 2 +@@ -10,2 +11,2 @@ +-line 10 + line 11 +`; + + expect(formatDiffAsGitDiff(input)).toBe(expected); + }); +}); diff --git a/packages/web/src/features/tools/utils.ts b/packages/web/src/features/tools/utils.ts new file mode 100644 index 000000000..0a704dc93 --- /dev/null +++ b/packages/web/src/features/tools/utils.ts @@ -0,0 +1,33 @@ +import type { z } from 'zod'; +import type { getDiffResponseSchema } from '@/features/git/schemas'; + +type DiffResult = z.infer; + +export function formatDiffAsGitDiff(result: DiffResult): string { + let output = ''; + + for (const file of result.files) { + output += file.oldPath ? `--- a/${file.oldPath}\n` : `--- /dev/null\n`; + output += file.newPath ? `+++ b/${file.newPath}\n` : `+++ /dev/null\n`; + + for (const hunk of file.hunks) { + const oldStart = hunk.oldRange.start; + const oldLines = hunk.oldRange.lines; + const newStart = hunk.newRange.start; + const newLines = hunk.newRange.lines; + + output += `@@ -${oldStart},${oldLines} +${newStart},${newLines} @@`; + if (hunk.heading) { + output += ` ${hunk.heading}`; + } + output += '\n'; + + output += hunk.body; + if (!hunk.body.endsWith('\n')) { + output += '\n'; + } + } + } + + return output; +}