paste transform followup (#17624)

Co-authored-by: Jack Wotherspoon <jackwoth@google.com>

packages/cli/src/ui/components/shared/text-buffer.test.ts

@@ -57,7 +57,7 @@ const initialState: TextBufferState = {
   transformationsByLine: [[]],
   visualLayout: defaultVisualLayout,
   pastedContent: {},
-  expandedPasteInfo: new Map(),
+  expandedPaste: null,
 };
 
 /**
@@ -547,7 +547,7 @@ describe('textBufferReducer', () => {
 
       const action: TextBufferAction = {
         type: 'toggle_paste_expansion',
-        payload: { id: placeholder },
+        payload: { id: placeholder, row: 0, col: 7 },
       };
 
       const state = textBufferReducer(stateWithPlaceholder, action);
@@ -560,9 +560,10 @@ describe('textBufferReducer', () => {
         'line5',
         'line6 suffix',
       ]);
-      expect(state.expandedPasteInfo.has(placeholder)).toBe(true);
-      const info = state.expandedPasteInfo.get(placeholder);
+      expect(state.expandedPaste?.id).toBe(placeholder);
+      const info = state.expandedPaste;
       expect(info).toEqual({
+        id: placeholder,
         startLine: 0,
         lineCount: 6,
         prefix: 'prefix ',
@@ -586,28 +587,24 @@ describe('textBufferReducer', () => {
         cursorRow: 5,
         cursorCol: 5,
         pastedContent: { [placeholder]: content },
-        expandedPasteInfo: new Map([
-          [
-            placeholder,
-            {
-              startLine: 0,
-              lineCount: 6,
-              prefix: 'prefix ',
-              suffix: ' suffix',
-            },
-          ],
-        ]),
+        expandedPaste: {
+          id: placeholder,
+          startLine: 0,
+          lineCount: 6,
+          prefix: 'prefix ',
+          suffix: ' suffix',
+        },
       });
 
       const action: TextBufferAction = {
         type: 'toggle_paste_expansion',
-        payload: { id: placeholder },
+        payload: { id: placeholder, row: 0, col: 7 },
       };
 
       const state = textBufferReducer(expandedState, action);
 
       expect(state.lines).toEqual(['prefix ' + placeholder + ' suffix']);
-      expect(state.expandedPasteInfo.has(placeholder)).toBe(false);
+      expect(state.expandedPaste).toBeNull();
       // Cursor should be at the end of the collapsed placeholder
       expect(state.cursorRow).toBe(0);
       expect(state.cursorCol).toBe(('prefix ' + placeholder).length);
@@ -625,7 +622,7 @@ describe('textBufferReducer', () => {
 
       const state = textBufferReducer(stateWithPlaceholder, {
         type: 'toggle_paste_expansion',
-        payload: { id: singleLinePlaceholder },
+        payload: { id: singleLinePlaceholder, row: 0, col: 0 },
       });
 
       expect(state.lines).toEqual(['some text']);
@@ -636,13 +633,13 @@ describe('textBufferReducer', () => {
     it('should return current state if placeholder ID not found in pastedContent', () => {
       const action: TextBufferAction = {
         type: 'toggle_paste_expansion',
-        payload: { id: 'unknown' },
+        payload: { id: 'unknown', row: 0, col: 0 },
       };
       const state = textBufferReducer(initialState, action);
       expect(state).toBe(initialState);
     });
 
-    it('should preserve expandedPasteInfo when lines change from edits outside the region', () => {
+    it('should preserve expandedPaste when lines change from edits outside the region', () => {
       // Start with an expanded paste at line 0 (3 lines long)
       const placeholder = '[Pasted Text: 3 lines]';
       const expandedState = createStateWithTransformations({
@@ -650,20 +647,16 @@ describe('textBufferReducer', () => {
         cursorRow: 3,
         cursorCol: 0,
         pastedContent: { [placeholder]: 'line1\nline2\nline3' },
-        expandedPasteInfo: new Map([
-          [
-            placeholder,
-            {
-              startLine: 0,
-              lineCount: 3,
-              prefix: '',
-              suffix: '',
-            },
-          ],
-        ]),
+        expandedPaste: {
+          id: placeholder,
+          startLine: 0,
+          lineCount: 3,
+          prefix: '',
+          suffix: '',
+        },
       });
 
-      expect(expandedState.expandedPasteInfo.size).toBe(1);
+      expect(expandedState.expandedPaste).not.toBeNull();
 
       // Insert a newline at the end - this changes lines but is OUTSIDE the expanded region
       const stateAfterInsert = textBufferReducer(expandedState, {
@@ -671,9 +664,9 @@ describe('textBufferReducer', () => {
         payload: '\n',
       });
 
-      // Lines changed, but expandedPasteInfo should be PRESERVED and optionally shifted (no shift here since edit is after)
-      expect(stateAfterInsert.expandedPasteInfo.size).toBe(1);
-      expect(stateAfterInsert.expandedPasteInfo.has(placeholder)).toBe(true);
+      // Lines changed, but expandedPaste should be PRESERVED and optionally shifted (no shift here since edit is after)
+      expect(stateAfterInsert.expandedPaste).not.toBeNull();
+      expect(stateAfterInsert.expandedPaste?.id).toBe(placeholder);
     });
   });
 });
@@ -2914,9 +2907,9 @@ describe('Transformation Utilities', () => {
       expect(result).toEqual(transformations[0]);
     });
 
-    it('should find transformation when cursor is at end', () => {
+    it('should NOT find transformation when cursor is at end', () => {
       const result = getTransformUnderCursor(0, 14, [transformations]);
-      expect(result).toEqual(transformations[0]);
+      expect(result).toBeNull();
     });
 
     it('should return null when cursor is not on a transformation', () => {
@@ -2928,6 +2921,22 @@ describe('Transformation Utilities', () => {
       const result = getTransformUnderCursor(0, 5, []);
       expect(result).toBeNull();
     });
+
+    it('regression: should not find paste transformation when clicking one character after it', () => {
+      const pasteId = '[Pasted Text: 5 lines]';
+      const line = pasteId + ' suffix';
+      const transformations = calculateTransformationsForLine(line);
+      const pasteTransform = transformations.find((t) => t.type === 'paste');
+      expect(pasteTransform).toBeDefined();
+
+      const endPos = pasteTransform!.logEnd;
+      // Position strictly at end should be null
+      expect(getTransformUnderCursor(0, endPos, [transformations])).toBeNull();
+      // Position inside should be found
+      expect(getTransformUnderCursor(0, endPos - 1, [transformations])).toEqual(
+        pasteTransform,
+      );
+    });
   });
 
   describe('calculateTransformedLine', () => {
@@ -3100,4 +3109,46 @@ describe('Transformation Utilities', () => {
       expect(result.current.allVisualLines[2]).toBe('line 3');
     });
   });
+
+  describe('Scroll Regressions', () => {
+    const scrollViewport: Viewport = { width: 80, height: 5 };
+
+    it('should not show empty viewport when collapsing a large paste that was scrolled', () => {
+      const largeContent =
+        'line1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\nline10';
+      const placeholder = '[Pasted Text: 10 lines]';
+
+      const { result } = renderHook(() =>
+        useTextBuffer({
+          initialText: placeholder,
+          viewport: scrollViewport,
+          isValidPath: () => false,
+        }),
+      );
+
+      // Setup: paste large content
+      act(() => {
+        result.current.setText('');
+        result.current.insert(largeContent, { paste: true });
+      });
+
+      // Expand it
+      act(() => {
+        result.current.togglePasteExpansion(placeholder, 0, 0);
+      });
+
+      // Verify scrolled state
+      expect(result.current.visualScrollRow).toBe(5);
+
+      // Collapse it
+      act(() => {
+        result.current.togglePasteExpansion(placeholder, 9, 0);
+      });
+
+      // Verify viewport is NOT empty immediately (clamping in useMemo)
+      expect(result.current.allVisualLines.length).toBe(1);
+      expect(result.current.viewportVisualLines.length).toBe(1);
+      expect(result.current.viewportVisualLines[0]).toBe(placeholder);
+    });
+  });
 });