From 13dc977f39f6d7bf6af2a2c72f1e4a44143d9e13 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Mon, 19 Oct 2026 16:57:38 -0500 Subject: [PATCH] fix(server): disable text conversion when measuring review-guide hunks The review guide reads hunk line numbers from git diff to keep excerpts on changed source lines. A configured textconv driver could rewrite that diff, so excerpts of real changes were dropped. Pass --no-textconv as the review diff already does. Co-Authored-By: Claude Opus 5.5 --- .../PullRequestReviewGuideService.test.ts | 45 +++++++++++++++++++ .../PullRequestReviewGuideService.ts | 5 ++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts b/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts index 66efa337ecd19..a8384605e60ae 100644 --- a/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts +++ b/apps/server/src/pullRequest/PullRequestReviewGuideService.test.ts @@ -706,6 +706,51 @@ it.layer(layerBase, { excludeTestServices: true })("PullRequestReviewGuideServic ), ); + it.effect("keeps excerpts of source lines a configured diff driver would convert away", () => + Effect.scoped( + Effect.gen(function* () { + const repositories = yield* makeRepositories; + const fs = yield* FileSystem.FileSystem; + yield* fs.writeFileString( + `${repositories.workspace}/.git/info/attributes`, + "feature.txt diff=blank\n", + ); + yield* git(repositories.workspace, ["config", "diff.blank.textconv", "true"]); + const stored = yield* Deferred.make(); + const context = yield* Layer.build( + makeLayer({ + ...repositories, + stored, + generate: () => + Effect.succeed({ + stops: [ + { + title: "The feature", + summary: "The change.", + attention: null, + excerpts: [{ path: "feature.txt", side: "new", start: 1, end: 3, caption: "" }], + files: [], + notes: [], + questions: [], + }, + ], + }), + }), + ); + const service = Context.get( + context, + PullRequestReviewGuideService.PullRequestReviewGuideService, + ); + yield* service.generate(reference); + yield* Deferred.await(stored); + const result = yield* service.get(reference); + assert.deepStrictEqual(result.guide?.stops[0]?.excerpts, [ + { path: "feature.txt", side: "new", start: 1, end: 3, caption: "" }, + ]); + }), + ), + ); + it.effect("refuses generation when the verified remote disappeared despite a usable origin", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/pullRequest/PullRequestReviewGuideService.ts b/apps/server/src/pullRequest/PullRequestReviewGuideService.ts index edd5dd2c72865..53799ae6cb0c6 100644 --- a/apps/server/src/pullRequest/PullRequestReviewGuideService.ts +++ b/apps/server/src/pullRequest/PullRequestReviewGuideService.ts @@ -351,7 +351,10 @@ const make = Effect.gen(function* () { }) .pipe(Effect.map((result) => result.stdout)); const [nameOnly, patch] = yield* Effect.all( - [diff(["--name-only", "-z"]), diff(["--unified=0", "--no-color", "--no-ext-diff"])], + [ + diff(["--name-only", "-z"]), + diff(["--unified=0", "--no-color", "--no-ext-diff", "--no-textconv"]), + ], { concurrency: "unbounded" }, ); return parseReviewGuideChange(nameOnly, patch);