Merge pull request #1864 from binaricat/codex/fix-long-line-highlight-lock

[codex] Fix long-line keyword highlight lockup

Author: binaricat

components/terminal/keywordHighlight.test.ts

@@ -107,6 +107,64 @@ function createFakeTerminalFromLines(lines: Array<{ text: string; isWrapped: boo
   return { term, decorations };
 }
 
+function createFakeTerminalFromLargeWrappedBlock({
+  lineCount,
+  lineText,
+  viewportY,
+  rows,
+}: {
+  lineCount: number;
+  lineText: string;
+  viewportY: number;
+  rows: number;
+}) {
+  let getLineCount = 0;
+  const decorations: Array<{ x: number; width: number; foregroundColor: string }> = [];
+  const noopDisposable = { dispose() {} };
+  const term = {
+    rows,
+    cols: lineText.length,
+    buffer: {
+      active: {
+        type: "normal",
+        viewportY,
+        baseY: 0,
+        cursorY: viewportY,
+        length: lineCount,
+        getLine: (lineY: number) => {
+          getLineCount += 1;
+          if (lineY < 0 || lineY >= lineCount) return undefined;
+          return createFakeWrappedLine(lineText, lineY > 0);
+        },
+      },
+    },
+    onScroll: () => noopDisposable,
+    onWriteParsed: () => noopDisposable,
+    onResize: () => noopDisposable,
+    onRender: () => noopDisposable,
+    registerMarker(offset: number) {
+      return {
+        line: offset,
+        isDisposed: false,
+        dispose() {
+          this.isDisposed = true;
+        },
+      };
+    },
+    registerDecoration(options: { x: number; width: number; foregroundColor: string }) {
+      decorations.push(options);
+      return {
+        isDisposed: false,
+        dispose() {
+          this.isDisposed = true;
+        },
+      };
+    },
+    refresh() {},
+  };
+  return { term, decorations, getLineCount: () => getLineCount };
+}
+
 function createFakeTerminal(lineText: string, options: { lineCount?: number } = {}) {
   const lineCount = options.lineCount ?? 1;
   let translateCount = 0;
@@ -310,3 +368,39 @@ test("wrapped highlight scanning falls back when the logical line exceeds the sc
     raf.restore();
   }
 });
+
+test("wrapped highlight scanning stops before walking an oversized soft-wrapped line", () => {
+  const raf = installAnimationFrameQueue();
+  try {
+    const lineText = "a".repeat(80);
+    const { term, decorations, getLineCount } = createFakeTerminalFromLargeWrappedBlock({
+      lineCount: 30_000,
+      lineText,
+      viewportY: 29_990,
+      rows: 3,
+    });
+    const highlighter = new KeywordHighlighter(term as never);
+    const rules: KeywordHighlightRule[] = [
+      {
+        id: "wrapped",
+        label: "Wrapped",
+        patterns: [`${lineText}${lineText}`],
+        color: "#F87171",
+        enabled: true,
+      },
+    ];
+
+    highlighter.setRules(rules, true);
+    raf.flush();
+    highlighter.dispose();
+    resetTerminalOutputPressure(term as never);
+
+    assert.deepEqual(decorations, []);
+    assert.ok(
+      getLineCount() < 3_000,
+      `expected capped wrapped scan, got ${getLineCount()} getLine calls`,
+    );
+  } finally {
+    raf.restore();
+  }
+});

components/terminal/keywordHighlight.ts

@@ -53,6 +53,13 @@ interface WrappedBlockContext {
   segmentBounds: Map<number, { lineStart: number; lineEnd: number }>;
 }
 
+type WrappedBlockCacheEntry = WrappedBlockContext | null;
+
+interface WrappedBlockScanCache {
+  contexts: Map<number, WrappedBlockCacheEntry>;
+  cappedMiss: DirtyLineSegment | null;
+}
+
 /** Shared empty array for non-matching lines to avoid per-call allocations. */
 const EMPTY_RANGES: readonly CachedDecorationRange[] = Object.freeze([]);
 
@@ -1023,7 +1030,10 @@ export class KeywordHighlighter implements IDisposable {
   private processLineRange(start: number, end: number, cursorAbsoluteY: number) {
     if (end < start) return;
     const buffer = this.term.buffer.active;
-    const wrappedBlockCache = new Map<number, WrappedBlockContext>();
+    const wrappedBlockCache: WrappedBlockScanCache = {
+      contexts: new Map<number, WrappedBlockCacheEntry>(),
+      cappedMiss: null,
+    };
     const pressure = getTerminalOutputPressure(this.term);
     for (let lineY = start; lineY <= end; lineY++) {
       const line = buffer.getLine(lineY);
@@ -1103,22 +1113,39 @@ export class KeywordHighlighter implements IDisposable {
     return !!nextLine?.isWrapped;
   }
 
-  private findWrappedBlockStart(buffer: IBuffer, lineY: number): number {
+  private findWrappedBlockStart(buffer: IBuffer, lineY: number): { startY: number; cappedRange?: DirtyLineSegment } {
     let startY = lineY;
+    let scannedRows = 0;
+    const maxRows = this.getWrappedContextScanRowLimit();
     while (startY > 0) {
+      scannedRows += 1;
+      if (scannedRows > maxRows) {
+        return { startY: -1, cappedRange: { start: startY, end: lineY } };
+      }
       const current = buffer.getLine(startY);
       if (!current?.isWrapped) break;
       startY -= 1;
     }
-    return startY;
+    return { startY };
+  }
+
+  private getWrappedContextScanRowLimit(): number {
+    const cols = Math.max(1, this.term.cols || 1);
+    return Math.max(1, Math.ceil(TERMINAL_AUX_LONG_LINE_SCAN_LIMIT_CHARS / cols) + 1);
   }
 
   private buildWrappedBlockContext(buffer: IBuffer, startY: number): WrappedBlockContext | null {
     let logicalLineText = "";
     const segmentBounds = new Map<number, { lineStart: number; lineEnd: number }>();
     let cursorY = startY;
+    let scannedRows = 0;
+    const maxRows = this.getWrappedContextScanRowLimit();
 
     while (true) {
+      scannedRows += 1;
+      if (scannedRows > maxRows) {
+        return null;
+      }
       const segment = buffer.getLine(cursorY);
       if (!segment) break;
       const segmentText = segment.translateToString(true);
@@ -1142,16 +1169,22 @@ export class KeywordHighlighter implements IDisposable {
   private getWrappedContext(
     buffer: IBuffer,
     lineY: number,
-    cache: Map<number, WrappedBlockContext>,
+    line: IBufferLine,
+    cache: WrappedBlockScanCache,
   ): { logicalLineText: string; lineStart: number; lineEnd: number } | null {
-    const startY = this.findWrappedBlockStart(buffer, lineY);
-    let block = cache.get(startY);
-    if (!block) {
-      block = this.buildWrappedBlockContext(buffer, startY) ?? undefined;
-      if (block) {
-        cache.set(startY, block);
-      }
+    if (this.isInCappedWrappedMiss(lineY, line, cache)) {
+      return null;
     }
+
+    const { startY, cappedRange } = this.findWrappedBlockStart(buffer, lineY);
+    if (startY < 0) {
+      cache.cappedMiss = cappedRange ?? { start: lineY, end: lineY };
+      return null;
+    }
+    if (!cache.contexts.has(startY)) {
+      cache.contexts.set(startY, this.buildWrappedBlockContext(buffer, startY));
+    }
+    const block = cache.contexts.get(startY);
     if (!block) return null;
     const bounds = block.segmentBounds.get(lineY);
     if (!bounds) return null;
@@ -1162,14 +1195,32 @@ export class KeywordHighlighter implements IDisposable {
     };
   }
 
+  private isInCappedWrappedMiss(
+    lineY: number,
+    line: IBufferLine,
+    cache: WrappedBlockScanCache,
+  ): boolean {
+    const miss = cache.cappedMiss;
+    if (!miss) return false;
+    if (lineY >= miss.start && lineY <= miss.end) return true;
+    if (lineY === miss.end + 1 && line.isWrapped) {
+      miss.end = lineY;
+      return true;
+    }
+    if (lineY > miss.end) {
+      cache.cappedMiss = null;
+    }
+    return false;
+  }
+
   private scanWrappedLine(
     buffer: IBuffer,
     lineY: number,
     line: IBufferLine,
     lineText: string,
-    wrappedBlockCache: Map<number, WrappedBlockContext>,
+    wrappedBlockCache: WrappedBlockScanCache,
   ): CachedDecorationRange[] {
-    const context = this.getWrappedContext(buffer, lineY, wrappedBlockCache);
+    const context = this.getWrappedContext(buffer, lineY, line, wrappedBlockCache);
     if (!context || context.logicalLineText === lineText) {
       return this.scanLine(line, lineText);
     }