Skip to content

Commit fd557a7

Browse files
committed
fix(hunk): collapse last agent source
1 parent ab1f817 commit fd557a7

1 file changed

Lines changed: 115 additions & 54 deletions

File tree

patches/hunk/0001-add-source-switch-menu.patch

Lines changed: 115 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -546,12 +546,20 @@ index 00cc1f2..03a69d0 100644
546546
kind: "item",
547547
diff --git i/src/ui/lib/sourceSwitch.test.ts w/src/ui/lib/sourceSwitch.test.ts
548548
new file mode 100644
549-
index 0000000..bc015e2
549+
index 0000000..89daa83
550550
--- /dev/null
551551
+++ w/src/ui/lib/sourceSwitch.test.ts
552-
@@ -0,0 +1,217 @@
552+
@@ -0,0 +1,264 @@
553553
+import { execFileSync } from "node:child_process";
554-
+import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
554+
+import {
555+
+ chmodSync,
556+
+ mkdirSync,
557+
+ mkdtempSync,
558+
+ readFileSync,
559+
+ rmSync,
560+
+ utimesSync,
561+
+ writeFileSync,
562+
+} from "node:fs";
555563
+import { tmpdir } from "node:os";
556564
+import { dirname, join } from "node:path";
557565
+import { describe, expect, test } from "bun:test";
@@ -593,8 +601,7 @@ index 0000000..bc015e2
593601
+ "staged",
594602
+ "commit",
595603
+ "branch",
596-
+ "pi-last-turn",
597-
+ "omp-last-turn",
604+
+ "last-agent-turn",
598605
+ ]);
599606
+ expect(items.find((item) => item.id === "staged")?.active).toBe(true);
600607
+ expect(activeReviewSource(input)).toBe("staged");
@@ -645,12 +652,12 @@ index 0000000..bc015e2
645652
+ }
646653
+ });
647654
+
648-
+ test("missing Pi marker returns a notice", () => {
655+
+ test("missing last-agent marker returns a notice", () => {
649656
+ const repo = createRepo();
650657
+ try {
651-
+ expect(resolveReviewSourceInput("pi-last-turn", repo, {})).toEqual({
658+
+ expect(resolveReviewSourceInput("last-agent-turn", repo, {})).toEqual({
652659
+ kind: "notice",
653-
+ message: "No Last Pi turn source for this repo.",
660+
+ message: "No Last agent turn source for this repo.",
654661
+ });
655662
+ } finally {
656663
+ rmSync(repo, { force: true, recursive: true });
@@ -672,7 +679,7 @@ index 0000000..bc015e2
672679
+ writeFileSync(patchPath, "diff --git a/file.txt b/file.txt\n");
673680
+ writeFileSync(markerPath, JSON.stringify({ version: 1, kind: "patch", file: patchPath }));
674681
+
675-
+ expect(resolveReviewSourceInput("pi-last-turn", repo, { mode: "split" })).toMatchObject({
682+
+ expect(resolveReviewSourceInput("last-agent-turn", repo, { mode: "split" })).toMatchObject({
676683
+ kind: "input",
677684
+ input: { kind: "patch", file: patchPath, options: { mode: "split" } },
678685
+ });
@@ -681,7 +688,7 @@ index 0000000..bc015e2
681688
+ );
682689
+ expect(repoPathForInput({ kind: "patch", file: patchPath, options: {} }, "")).toBe(repo);
683690
+ expect(activeReviewSource({ kind: "patch", file: patchPath, options: {} })).toBe(
684-
+ "pi-last-turn",
691+
+ "last-agent-turn",
685692
+ );
686693
+ expect(JSON.parse(readFileSync(markerPath, "utf8"))).toMatchObject({
687694
+ kind: "patch",
@@ -707,7 +714,7 @@ index 0000000..bc015e2
707714
+ writeFileSync(patchPath, "diff --git a/file.txt b/file.txt\n");
708715
+ writeFileSync(markerPath, JSON.stringify({ version: 1, kind: "patch", file: patchPath }));
709716
+
710-
+ const result = resolveReviewSourceInput("omp-last-turn", repo, { mode: "split" });
717+
+ const result = resolveReviewSourceInput("last-agent-turn", repo, { mode: "split" });
711718
+ expect(result).toMatchObject({
712719
+ kind: "input",
713720
+ input: { kind: "patch", file: patchPath, options: { mode: "split" } },
@@ -717,14 +724,54 @@ index 0000000..bc015e2
717724
+ "Last OMP turn",
718725
+ );
719726
+ expect(activeReviewSource({ kind: "patch", file: patchPath, options: {} })).toBe(
720-
+ "omp-last-turn",
727+
+ "last-agent-turn",
721728
+ );
722729
+ expect(repoPathForInput({ kind: "patch", file: patchPath, options: {} }, "")).toBe(repo);
723730
+ } finally {
724731
+ rmSync(repo, { force: true, recursive: true });
725732
+ }
726733
+ });
727734
+
735+
+ test("last-agent source uses the newest agent marker", () => {
736+
+ const repo = createRepo();
737+
+ try {
738+
+ const piMarkerPath = join(
739+
+ repo,
740+
+ git(repo, ["rev-parse", "--git-path", "hunk/last-pi-turn.json"]),
741+
+ );
742+
+ const piPatchPath = join(
743+
+ repo,
744+
+ git(repo, ["rev-parse", "--git-path", "hunk/last-pi-turn.patch"]),
745+
+ );
746+
+ const ompMarkerPath = join(
747+
+ repo,
748+
+ git(repo, ["rev-parse", "--git-path", "hunk/last-omp-turn.json"]),
749+
+ );
750+
+ const ompPatchPath = join(
751+
+ repo,
752+
+ git(repo, ["rev-parse", "--git-path", "hunk/last-omp-turn.patch"]),
753+
+ );
754+
+ mkdirSync(dirname(piMarkerPath), { recursive: true });
755+
+ writeFileSync(piPatchPath, "diff --git a/file.txt b/file.txt\n");
756+
+ writeFileSync(ompPatchPath, "diff --git a/file.txt b/file.txt\n");
757+
+ writeFileSync(piMarkerPath, JSON.stringify({ version: 1, kind: "patch", file: piPatchPath }));
758+
+ writeFileSync(
759+
+ ompMarkerPath,
760+
+ JSON.stringify({ version: 1, kind: "patch", file: ompPatchPath }),
761+
+ );
762+
+ utimesSync(piMarkerPath, new Date("2026-01-01T00:00:00Z"), new Date("2026-01-01T00:00:00Z"));
763+
+ utimesSync(ompMarkerPath, new Date("2026-01-02T00:00:00Z"), new Date("2026-01-02T00:00:00Z"));
764+
+
765+
+ const result = resolveReviewSourceInput("last-agent-turn", repo, {});
766+
+ expect(result).toMatchObject({
767+
+ kind: "input",
768+
+ input: { kind: "patch", file: ompPatchPath },
769+
+ label: "Last OMP turn",
770+
+ });
771+
+ } finally {
772+
+ rmSync(repo, { force: true, recursive: true });
773+
+ }
774+
+ });
728775
+ test("adds open PRs for the current branch and resolves PR diffs", () => {
729776
+ const repo = createRepo();
730777
+ const bin = mkdtempSync(join(tmpdir(), "hunk-source-gh-"));
@@ -769,18 +816,18 @@ index 0000000..bc015e2
769816
+});
770817
diff --git i/src/ui/lib/sourceSwitch.ts w/src/ui/lib/sourceSwitch.ts
771818
new file mode 100644
772-
index 0000000..faf5aff
819+
index 0000000..0681aa0
773820
--- /dev/null
774821
+++ w/src/ui/lib/sourceSwitch.ts
775-
@@ -0,0 +1,432 @@
822+
@@ -0,0 +1,446 @@
776823
+import { execFileSync } from "node:child_process";
777-
+import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
824+
+import { existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
778825
+import { basename, dirname, isAbsolute, join, resolve, sep } from "node:path";
779826
+import type { CliInput, CommonOptions } from "../../core/types";
780827
+
781828
+export type BaseReviewSourceId = "unstaged" | "staged" | "commit" | "branch";
782-
+export type LastTurnReviewSourceId = "pi-last-turn" | "omp-last-turn";
783-
+export type BuiltInReviewSourceId = BaseReviewSourceId | LastTurnReviewSourceId;
829+
+export type LastAgentTurnReviewSourceId = "last-agent-turn";
830+
+export type BuiltInReviewSourceId = BaseReviewSourceId | LastAgentTurnReviewSourceId;
784831
+export type PullRequestReviewSourceId = `pr:${number}`;
785832
+export type ReviewSourceId = BuiltInReviewSourceId | PullRequestReviewSourceId;
786833
+
@@ -801,10 +848,8 @@ index 0000000..faf5aff
801848
+ title: string;
802849
+}
803850
+
804-
+interface LastTurnSource {
805-
+ id: LastTurnReviewSourceId;
851+
+interface AgentLastTurnSource {
806852
+ label: string;
807-
+ hint: string;
808853
+ markerBasename: string;
809854
+}
810855
+
@@ -822,17 +867,23 @@ index 0000000..faf5aff
822867
+ branch: "b",
823868
+};
824869
+
825-
+const LAST_TURN_SOURCES: LastTurnSource[] = [
826-
+ { id: "pi-last-turn", label: "Last Pi turn", hint: "p", markerBasename: "last-pi-turn" },
827-
+ { id: "omp-last-turn", label: "Last OMP turn", hint: "o", markerBasename: "last-omp-turn" },
870+
+const LAST_AGENT_TURN_SOURCE = {
871+
+ id: "last-agent-turn",
872+
+ label: "Last agent turn",
873+
+ hint: "a",
874+
+} as const;
875+
+
876+
+const AGENT_LAST_TURN_SOURCES: AgentLastTurnSource[] = [
877+
+ { label: "Last Pi turn", markerBasename: "last-pi-turn" },
878+
+ { label: "Last OMP turn", markerBasename: "last-omp-turn" },
828879
+];
829880
+
830881
+export const REVIEW_SOURCE_ORDER: BuiltInReviewSourceId[] = [
831882
+ "unstaged",
832883
+ "staged",
833884
+ "commit",
834885
+ "branch",
835-
+ ...LAST_TURN_SOURCES.map((source) => source.id),
886+
+ LAST_AGENT_TURN_SOURCE.id,
836887
+];
837888
+
838889
+function run(repo: string, command: string, args: string[], timeout?: number) {
@@ -853,21 +904,19 @@ index 0000000..faf5aff
853904
+ return run(repo, "gh", args, timeout);
854905
+}
855906
+
856-
+function lastTurnSource(id: string) {
857-
+ return LAST_TURN_SOURCES.find((source) => source.id === id);
858-
+}
859-
+
860-
+function lastTurnSourceByPatch(path: string) {
907+
+function agentLastTurnSourceByPatch(path: string) {
861908
+ const patchBasename = basename(path).replace(/\.patch$/, "");
862-
+ return LAST_TURN_SOURCES.find((source) => source.markerBasename === patchBasename);
909+
+ return AGENT_LAST_TURN_SOURCES.find((source) => source.markerBasename === patchBasename);
863910
+}
864911
+
865912
+function sourceLabel(id: BuiltInReviewSourceId) {
866-
+ return BASE_SOURCE_LABELS[id as BaseReviewSourceId] ?? lastTurnSource(id)?.label ?? id;
913+
+ if (id === LAST_AGENT_TURN_SOURCE.id) return LAST_AGENT_TURN_SOURCE.label;
914+
+ return BASE_SOURCE_LABELS[id];
867915
+}
868916
+
869917
+function sourceHint(id: BuiltInReviewSourceId) {
870-
+ return BASE_SOURCE_HINTS[id as BaseReviewSourceId] ?? lastTurnSource(id)?.hint ?? "";
918+
+ if (id === LAST_AGENT_TURN_SOURCE.id) return LAST_AGENT_TURN_SOURCE.hint;
919+
+ return BASE_SOURCE_HINTS[id];
871920
+}
872921
+
873922
+function parsePullRequestId(id: string): number | null {
@@ -891,7 +940,7 @@ index 0000000..faf5aff
891940
+ id === "staged" ||
892941
+ id === "commit" ||
893942
+ id === "branch" ||
894-
+ Boolean(lastTurnSource(id))
943+
+ id === LAST_AGENT_TURN_SOURCE.id
895944
+ );
896945
+}
897946
+
@@ -994,8 +1043,8 @@ index 0000000..faf5aff
9941043
+ if (typeof metadata?.sourceId === "string" && isReviewSourceId(metadata.sourceId)) {
9951044
+ return metadata.sourceId;
9961045
+ }
997-
+ const lastTurn = lastTurnSourceByPatch(input.file);
998-
+ if (lastTurn) return lastTurn.id;
1046+
+ const lastTurn = agentLastTurnSourceByPatch(input.file);
1047+
+ if (lastTurn) return LAST_AGENT_TURN_SOURCE.id;
9991048
+
10001049
+ const label = pullRequestLabelFromPath(input.file);
10011050
+ return label ? (`pr:${label.slice(4)}` as PullRequestReviewSourceId) : null;
@@ -1007,14 +1056,15 @@ index 0000000..faf5aff
10071056
+}
10081057
+
10091058
+export function sourceIndicatorLabel(input: CliInput, sourceLabel: string): string | null {
1010-
+ const active = activeReviewSource(input);
1011-
+ if (active) return reviewSourceLabel(active);
1012-
+ if (input.kind !== "patch") return null;
1059+
+ if (input.kind === "patch") {
1060+
+ const patchFile = input.file ?? sourceLabel;
1061+
+ const metadata = sourceMetadataForPatch(patchFile);
1062+
+ if (typeof metadata?.label === "string") return metadata.label;
1063+
+ return agentLastTurnSourceByPatch(patchFile)?.label ?? pullRequestLabelFromPath(patchFile);
1064+
+ }
10131065
+
1014-
+ const patchFile = input.file ?? sourceLabel;
1015-
+ const metadata = sourceMetadataForPatch(patchFile);
1016-
+ if (typeof metadata?.label === "string") return metadata.label;
1017-
+ return lastTurnSourceByPatch(patchFile)?.label ?? pullRequestLabelFromPath(patchFile);
1066+
+ const active = activeReviewSource(input);
1067+
+ return active ? reviewSourceLabel(active) : null;
10181068
+}
10191069
+
10201070
+export function buildReviewSourceItems(input: CliInput, repo?: string): ReviewSourceItem[] {
@@ -1070,7 +1120,7 @@ index 0000000..faf5aff
10701120
+ }
10711121
+}
10721122
+
1073-
+function lastTurnMarker(repo: string, source: LastTurnSource) {
1123+
+function lastTurnMarker(repo: string, source: AgentLastTurnSource) {
10741124
+ const markerName = `hunk/${source.markerBasename}.json`;
10751125
+ try {
10761126
+ const marker = runGit(repo, ["rev-parse", "--git-path", markerName]);
@@ -1080,21 +1130,29 @@ index 0000000..faf5aff
10801130
+ }
10811131
+}
10821132
+
1133+
+function newestLastAgentTurnMarker(repo: string) {
1134+
+ return AGENT_LAST_TURN_SOURCES.map((source) => ({
1135+
+ source,
1136+
+ markerPath: lastTurnMarker(repo, source),
1137+
+ }))
1138+
+ .filter(({ markerPath }) => existsSync(markerPath))
1139+
+ .sort((left, right) => statSync(right.markerPath).mtimeMs - statSync(left.markerPath).mtimeMs)
1140+
+ .at(0);
1141+
+}
1142+
+
10831143
+function resolveMarkerPath(repo: string, value: unknown) {
10841144
+ if (typeof value !== "string" || value.trim() === "") return null;
10851145
+ return isAbsolute(value) ? value : resolve(repo, value);
10861146
+}
10871147
+
1088-
+function lastTurnInput(
1089-
+ source: LastTurnSource,
1090-
+ repo: string,
1091-
+ options: CommonOptions,
1092-
+): ReviewSourceSwitchResult {
1093-
+ const markerPath = lastTurnMarker(repo, source);
1094-
+ if (!existsSync(markerPath)) {
1095-
+ return { kind: "notice", message: `No ${source.label} source for this repo.` };
1148+
+function lastAgentTurnInput(repo: string, options: CommonOptions): ReviewSourceSwitchResult {
1149+
+ const markerSource = newestLastAgentTurnMarker(repo);
1150+
+ if (!markerSource) {
1151+
+ return { kind: "notice", message: "No Last agent turn source for this repo." };
10961152
+ }
10971153
+
1154+
+ const { source, markerPath } = markerSource;
1155+
+
10981156
+ let marker: unknown;
10991157
+ try {
11001158
+ marker = JSON.parse(readFileSync(markerPath, "utf8"));
@@ -1112,7 +1170,11 @@ index 0000000..faf5aff
11121170
+ if (!file || !existsSync(file)) {
11131171
+ return { kind: "notice", message: `${source.label} patch is missing.` };
11141172
+ }
1115-
+ persistSourceMetadata(file, { repo, label: source.label, sourceId: source.id });
1173+
+ persistSourceMetadata(file, {
1174+
+ repo,
1175+
+ label: source.label,
1176+
+ sourceId: LAST_AGENT_TURN_SOURCE.id,
1177+
+ });
11161178
+ return {
11171179
+ kind: "input",
11181180
+ input: { kind: "patch", file, options },
@@ -1164,8 +1226,7 @@ index 0000000..faf5aff
11641226
+ const prNumber = parsePullRequestId(sourceId);
11651227
+ if (prNumber !== null) return pullRequestInput(repo, prNumber, options);
11661228
+
1167-
+ const lastTurn = lastTurnSource(sourceId);
1168-
+ if (lastTurn) return lastTurnInput(lastTurn, repo, options);
1229+
+ if (sourceId === LAST_AGENT_TURN_SOURCE.id) return lastAgentTurnInput(repo, options);
11691230
+
11701231
+ switch (sourceId) {
11711232
+ case "unstaged":

0 commit comments

Comments
 (0)