Skip to content

Commit 271fe97

Browse files
ymansurozerclaude
andcommitted
feat: wrap-around navigation that seeks unreviewed files
Approving a file now advances to the next unreviewed file, wrapping past the end of the review order — a reviewer who jumped ahead is carried back to the files they skipped instead of dead-ending on the last file with the review-complete prompt never firing. Plain ⇧→/⇧← wrap at the ends too: next from the last file goes to the first unreviewed file (else cycles), prev mirrors it, and stepping back from the guide Overview wraps to the end. The seek order is guide order when a guide is attached, extended with any changed files the guide doesn't list so a partial guide can't strand its "Other" files; without a guide it's the file array. A file whose sign-off hash no longer matches (the agent rewrote it) counts as unreviewed and is a legitimate wrap target. The guide-bar Next/Prev buttons now dim only when nothing unreviewed remains, so the visible affordance agrees with the keyboard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jcfPg9g4TTWQksf8vhHps
1 parent 062225a commit 271fe97

5 files changed

Lines changed: 203 additions & 9 deletions

File tree

src/ui/decisions.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { S, D, toast, api, persist } from "./store";
22
import { currentFile, applyDecisionToDiff, fileObjections, fileReviewState } from "./changes";
33
import { render, deferRender } from "./render";
4-
import { nextFileIndex, guideProgress } from "./guide";
4+
import { nextUnreviewedFileIndex, guideProgress } from "./guide";
55
import type { ChangeState, Decision } from "./types";
66

77
// The explicit decision record is the source of truth for accept/reject (decoupled
@@ -67,7 +67,9 @@ export async function approveCurrentFile() {
6767
// moment it moves, not just visible in the bar. % matches the strip (LOC-weighted by default).
6868
const done = S.state.files.filter((f) => fileReviewState(f.path) !== "pending").length;
6969
toast(`${label}${done} of ${S.state.files.length} files · ${guideProgress().pct}%`);
70-
const next = nextFileIndex(S.fileIndex);
70+
// Seek the next unreviewed file, wrapping past the end so a reviewer who jumped ahead is
71+
// carried back to the files they skipped instead of dead-ending here.
72+
const next = nextUnreviewedFileIndex(S.fileIndex);
7173
if (next !== null && S.selectFile) S.selectFile(next);
7274
else render();
7375
}

src/ui/guide.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { S, $, esc } from "./store";
2-
import { fileReviewState } from "./changes";
2+
import { fileReviewState, fileFinished } from "./changes";
3+
import { nextUnreviewed, wrapNextTarget, wrapPrevTarget } from "./seek";
34
import { renderMarkdown, renderMarkdownInline } from "./markdown";
45
import { lineStats, walkthroughGroups, walkRows } from "./walkthrough";
56
import type { WalkGroup, WalkRow, WalkFile } from "./walkthrough";
@@ -46,6 +47,49 @@ export function prevFileIndex(cur: number): number | null {
4647
return cur - 1 >= 0 ? cur - 1 : null;
4748
}
4849

50+
// The order file navigation walks and wraps around. Without a guide it's the file array; with
51+
// one it's the guide order followed by every changed file the guide DIDN'T list (the
52+
// walkthrough's "Other" group), in file-array order — so the seek reaches unlisted files and
53+
// never dead-ends on a partial guide. Plain mid-list stepping (nextFileIndex) is unaffected;
54+
// only the seek/wrap helpers read this extended order.
55+
export function navOrder(): number[] {
56+
const n = S.state?.files?.length ?? 0;
57+
const all = Array.from({ length: n }, (_, i) => i);
58+
if (!hasGuide()) return all;
59+
const order = guideOrder();
60+
const listed = new Set(order);
61+
return order.concat(all.filter((i) => !listed.has(i)));
62+
}
63+
64+
// "Unreviewed" for the seek — a file not signed off in the current state, matching the tree
65+
// badges and floating approve button (an agent edit after sign-off invalidates the hash, so
66+
// the file counts as unreviewed again). Store-reading wrapper over fileFinished.
67+
function seekFinished(i: number): boolean {
68+
const path = S.state?.files?.[i]?.path;
69+
return !!path && fileFinished(path);
70+
}
71+
72+
// Is there any unreviewed file left anywhere in the nav order?
73+
export function anyUnreviewed(): boolean {
74+
return navOrder().some((i) => !seekFinished(i));
75+
}
76+
77+
// The next unreviewed file after `cur`, wrapping past the end — approve-advance's seek.
78+
// null when no unreviewed file remains (the caller falls back to the review-complete prompt).
79+
export function nextUnreviewedFileIndex(cur: number): number | null {
80+
return nextUnreviewed(navOrder(), cur, seekFinished);
81+
}
82+
83+
// Where plain "next" lands when it steps off the last file: first unreviewed, else first file.
84+
export function nextWrapIndex(): number | null {
85+
return wrapNextTarget(navOrder(), seekFinished);
86+
}
87+
88+
// Where plain "prev" lands when it steps off the first position: last unreviewed, else last file.
89+
export function prevWrapIndex(): number | null {
90+
return wrapPrevTarget(navOrder(), seekFinished);
91+
}
92+
4993
// Changed lines (additions + deletions) per file path — the weight used for progress, so
5094
// finishing a big file advances the bar more than a tiny one. Min 1 so every file counts.
5195
function locByPath(): Map<string, number> {

src/ui/main.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ import {
2626
guideStale,
2727
nextFileIndex,
2828
prevFileIndex,
29+
nextWrapIndex,
30+
prevWrapIndex,
31+
anyUnreviewed,
2932
walkthroughRows,
3033
} from "./guide";
3134
import { setBaseTitle, reviewStats } from "./progress";
@@ -215,33 +218,50 @@ S.guideNext = () => {
215218
S.startGuided?.();
216219
return;
217220
}
221+
// Off the last file, wrap to the first unreviewed file (else cycle to the first) instead
222+
// of dead-ending, so skipped files are surfaced.
218223
const n = nextFileIndex(S.fileIndex);
219-
if (n !== null) S.selectFile?.(n);
224+
const target = n !== null ? n : nextWrapIndex();
225+
if (target !== null) S.selectFile?.(target);
220226
};
221227
S.guidePrev = () => {
222-
if (S.overviewOpen) return;
228+
// Stepping back from the Overview (the position before the first file) wraps to the end —
229+
// the last unreviewed file, else the last file.
230+
if (S.overviewOpen) {
231+
const w = prevWrapIndex();
232+
if (w !== null) S.selectFile?.(w);
233+
return;
234+
}
235+
// Off the first file, drop to the Overview (unchanged); the wrap happens from there.
223236
const p = prevFileIndex(S.fileIndex);
224237
if (p === null) S.openOverview?.();
225238
else S.selectFile?.(p);
226239
};
227-
S.guideAtStart = () => !!S.overviewOpen;
228-
S.guideAtLast = () => !S.overviewOpen && nextFileIndex(S.fileIndex) === null;
240+
// The nav buttons dim only when the review is fully signed off (nothing left to wrap to) —
241+
// while unreviewed work remains, next/prev stay live because they now seek it.
242+
S.guideAtStart = () => !!S.overviewOpen && !anyUnreviewed();
243+
S.guideAtLast = () => !S.overviewOpen && nextFileIndex(S.fileIndex) === null && !anyUnreviewed();
229244
// Review-order file stepping (⇧←/⇧→) — guide order when guided (or on the Overview), else sequential.
230245
S.nextFile = () => {
231246
if (hasGuide() || S.overviewOpen) {
232247
S.guideNext?.();
233248
return;
234249
}
235250
const n = S.fileIndex + 1;
236-
if (n < S.state.files.length) S.selectFile?.(n);
251+
// Off the last file, wrap to the first unreviewed file (else the first file).
252+
const target = n < S.state.files.length ? n : nextWrapIndex();
253+
if (target !== null) S.selectFile?.(target);
237254
};
238255
S.prevFile = () => {
239256
if (hasGuide() || S.overviewOpen) {
240257
S.guidePrev?.();
241258
return;
242259
}
243260
const p = S.fileIndex - 1;
244-
if (p >= 0) S.selectFile?.(p);
261+
// Off the first file, wrap to the last unreviewed file (else the last file). Without a guide
262+
// there's no Overview to step back into, so the wrap applies straight from the first file.
263+
const target = p >= 0 ? p : prevWrapIndex();
264+
if (target !== null) S.selectFile?.(target);
245265
};
246266
// Tree-order file stepping (⇧↑/⇧↓) — walk the file rows as shown in the tree (skip folders),
247267
// selecting the prev/next one (preview for unchanged files).

src/ui/seek.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { test } from "node:test";
2+
import assert from "node:assert/strict";
3+
import { nextUnreviewed, wrapNextTarget, wrapPrevTarget } from "./seek.js";
4+
5+
// A finished predicate backed by a set of signed-off file indices. A file absent from the set
6+
// is unreviewed — this is how a hash-invalidated file (signed off, then edited by the agent)
7+
// re-enters the seek: the store's fileFinished returns false for it, so it's just "not in set".
8+
const finishedIn = (done: number[]) => {
9+
const s = new Set(done);
10+
return (i: number) => s.has(i);
11+
};
12+
13+
test("nextUnreviewed wraps past the end to the first unreviewed file — guide order", () => {
14+
// Guide order is not the file-array order; the seek must follow it.
15+
const order = [3, 1, 4, 0, 2];
16+
// Everything signed off except file index 1; cur is the last file in the order (2).
17+
const next = nextUnreviewed(order, 2, finishedIn([3, 4, 0, 2]));
18+
assert.equal(next, 1, "scans forward from the last slot, wraps, lands on 1");
19+
});
20+
21+
test("nextUnreviewed wraps past the end to the first unreviewed file — file order", () => {
22+
const order = [0, 1, 2, 3, 4];
23+
// Only file 2 remains; cur is the last file (4).
24+
assert.equal(nextUnreviewed(order, 4, finishedIn([0, 1, 3, 4])), 2);
25+
});
26+
27+
test("nextUnreviewed returns null when the current file is the last remaining one", () => {
28+
// Approve-advance on the final unreviewed file: after approving, cur is finished and every
29+
// other file already was — no wrap target, so the caller shows the review-complete prompt.
30+
const order = [0, 1, 2];
31+
assert.equal(nextUnreviewed(order, 1, finishedIn([0, 1, 2])), null);
32+
});
33+
34+
test("nextUnreviewed never returns the current file, even when it is unreviewed", () => {
35+
// The just-approved file is finished in practice, but guard the wrap regardless.
36+
const order = [0, 1, 2];
37+
assert.equal(nextUnreviewed(order, 1, finishedIn([0, 2])), null);
38+
});
39+
40+
test("nextUnreviewed treats a hash-invalidated (dropped-out) file as a wrap target", () => {
41+
// File 0 was signed off then invalidated by an agent edit → back to unreviewed.
42+
const order = [0, 1, 2];
43+
assert.equal(nextUnreviewed(order, 2, finishedIn([1, 2])), 0);
44+
});
45+
46+
test("nextUnreviewed starts from the top when cur is not in the order", () => {
47+
// A diff file absent from the guide order isn't in `order`; scan from the first slot.
48+
const order = [3, 1, 4];
49+
assert.equal(nextUnreviewed(order, 99, finishedIn([3])), 1);
50+
});
51+
52+
test("partial guide: approve-advance and wrap reach the unlisted 'Other' files", () => {
53+
// navOrder() builds guide order followed by changed files the guide didn't list. Here the
54+
// guide covers files 2 and 0; files 1 and 3 are unlisted, appended in file-array order.
55+
const order = [2, 0, 1, 3];
56+
// Approve the last guide-listed file (0) while an unlisted file (1) is still pending →
57+
// advance must land on it, not dead-end at the end of the guided sequence.
58+
assert.equal(nextUnreviewed(order, 0, finishedIn([2, 0])), 1);
59+
// The plain-next wrap likewise targets an unlisted pending file.
60+
assert.equal(wrapNextTarget(order, finishedIn([2, 0])), 1);
61+
assert.equal(wrapPrevTarget(order, finishedIn([2, 0])), 3);
62+
});
63+
64+
test("wrapNextTarget lands on the first unreviewed file when work remains", () => {
65+
assert.equal(wrapNextTarget([3, 1, 4, 0, 2], finishedIn([3, 1])), 4, "guide order");
66+
assert.equal(wrapNextTarget([0, 1, 2, 3], finishedIn([0])), 1, "file order");
67+
});
68+
69+
test("wrapNextTarget cycles to the first file when everything is reviewed", () => {
70+
assert.equal(wrapNextTarget([3, 1, 4], finishedIn([3, 1, 4])), 3);
71+
});
72+
73+
test("wrapPrevTarget lands on the last unreviewed file when work remains", () => {
74+
assert.equal(wrapPrevTarget([3, 1, 4, 0, 2], finishedIn([2, 0])), 4, "guide order");
75+
assert.equal(wrapPrevTarget([0, 1, 2, 3], finishedIn([3])), 2, "file order");
76+
});
77+
78+
test("wrapPrevTarget cycles to the last file when everything is reviewed", () => {
79+
assert.equal(wrapPrevTarget([3, 1, 4], finishedIn([3, 1, 4])), 4);
80+
});
81+
82+
test("empty order yields no target", () => {
83+
const none = finishedIn([]);
84+
assert.equal(nextUnreviewed([], 0, none), null);
85+
assert.equal(wrapNextTarget([], none), null);
86+
assert.equal(wrapPrevTarget([], none), null);
87+
});

src/ui/seek.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Pure file-seek logic for wrap-around navigation. These operate on an explicit `order`
2+
// (file indices in nav order — guide order when a guide is attached, else the file array)
3+
// and a `finished` predicate ("this file is signed off in the current state"), so they're
4+
// unit-testable without the Alpine store. guide.ts holds the thin store-reading wrappers.
5+
6+
// The next unreviewed file after `cur`, scanning `order` forward and wrapping past the end
7+
// back to the beginning. Used by approve-advance to seek remaining work instead of dead-ending.
8+
// `cur`'s own slot is never returned (a just-approved file counts as finished anyway, but the
9+
// wrap must not land back on the starting file). null when no unreviewed file remains anywhere.
10+
export function nextUnreviewed(
11+
order: number[],
12+
cur: number,
13+
finished: (i: number) => boolean,
14+
): number | null {
15+
const n = order.length;
16+
if (!n) return null;
17+
// `cur` may be absent from the order (a diff file not listed in the guide) — start scanning
18+
// from the top of the order in that case (pos === -1 makes the first probe order[0]).
19+
const pos = order.indexOf(cur);
20+
for (let step = 1; step <= n; step++) {
21+
const i = order[(pos + step) % n]!;
22+
if (i === cur) continue;
23+
if (!finished(i)) return i;
24+
}
25+
return null;
26+
}
27+
28+
// The target when plain "next" steps off the LAST file in the order: the first unreviewed
29+
// file if any remains, else the first file (a plain cycle). null only when there are no files.
30+
export function wrapNextTarget(order: number[], finished: (i: number) => boolean): number | null {
31+
if (!order.length) return null;
32+
return order.find((i) => !finished(i)) ?? order[0]!;
33+
}
34+
35+
// Mirror of wrapNextTarget for plain "prev" stepping off the FIRST position: the last
36+
// unreviewed file if any remains, else the last file.
37+
export function wrapPrevTarget(order: number[], finished: (i: number) => boolean): number | null {
38+
if (!order.length) return null;
39+
for (let i = order.length - 1; i >= 0; i--) if (!finished(order[i]!)) return order[i]!;
40+
return order[order.length - 1]!;
41+
}

0 commit comments

Comments
 (0)