chat: fix undo/redo skipping multiple no-edit requests (#299330)

- Fixes _willRedoToEpoch to advance one request at a time when there are
  no edit operations ahead, instead of jumping past all remaining requests
- When redoing with no operations in the queue, now finds the next
  request-start checkpoint boundary and advances there, following the same
  single-step pattern as undo
- Adds test case verifying undo and redo step through consecutive no-edit
  requests one at a time

Fixes #275234

(Commit message generated by Copilot)
This commit is contained in:
Connor Peet
2026-03-04 16:51:57 -08:00
committed by GitHub
parent 5222fc6f55
commit 9a207cb696
2 changed files with 61 additions and 1 deletions

View File

@@ -123,7 +123,19 @@ export class ChatEditingCheckpointTimelineImpl implements IChatEditingCheckpoint
// Find the next edit operation that would be applied...
const nextOperation = operations.find(op => op.epoch >= currentEpoch);
const nextCheckpoint = nextOperation && checkpoints.find(op => op.epoch > nextOperation.epoch);
// When there are no more operations, advance one request at a time
// by finding the next request-start checkpoint boundary.
if (!nextOperation) {
const nextRequestStart = checkpoints.find(cp => cp.epoch >= currentEpoch && cp.undoStopId === undefined);
if (!nextRequestStart) {
return maxEncounteredEpoch + 1;
}
const requestAfter = checkpoints.find(cp => cp.epoch > nextRequestStart.epoch && cp.undoStopId === undefined);
return requestAfter ? requestAfter.epoch : (maxEncounteredEpoch + 1);
}
const nextCheckpoint = checkpoints.find(op => op.epoch > nextOperation.epoch);
// And figure out where we're going if we're navigating across request
// 1. If there is no next request or if the next target checkpoint is in

View File

@@ -1248,6 +1248,54 @@ suite('ChatEditingCheckpointTimeline', function () {
await timeline.navigateToCheckpoint(stop2NewCheckpointId);
assert.strictEqual(fileContents.get(uri), 'replacement edit', 'Content should still be correct after full timeline traversal');
});
test('undo/redo with multiple no-edit requests advances one request at a time', async function () {
// req1: no edits
timeline.createCheckpoint('req1', undefined, 'Start req1');
// req2: no edits
timeline.createCheckpoint('req2', undefined, 'Start req2');
// req3: no edits
timeline.createCheckpoint('req3', undefined, 'Start req3');
// req4: no edits
timeline.createCheckpoint('req4', undefined, 'Start req4');
// Undo should step one request at a time
assert.strictEqual(timeline.canUndo.get(), true);
await timeline.undoToLastCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4']);
await timeline.undoToLastCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4', 'req3']);
await timeline.undoToLastCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4', 'req3', 'req2']);
await timeline.undoToLastCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4', 'req3', 'req2', 'req1']);
assert.strictEqual(timeline.canUndo.get(), false);
// Redo should also step one request at a time (not skip all at once)
assert.strictEqual(timeline.canRedo.get(), true);
await timeline.redoToNextCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4', 'req3', 'req2']);
await timeline.redoToNextCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4', 'req3']);
await timeline.redoToNextCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), ['req4']);
await timeline.redoToNextCheckpoint();
assert.deepStrictEqual(timeline.requestDisablement.get().map(d => d.requestId), []);
assert.strictEqual(timeline.canRedo.get(), false);
});
});
// Mock notebook service for tests that don't need notebook functionality