fix(cli): prevent flicker on tall approval dialogs, refactor logic out from plan into other inlines (#777)

Co-authored-by: Letta <noreply@letta.com>
2026-02-01 18:04:01 -08:00
parent ab00aae7c5
commit ac2950b2a2
5 changed files with 260 additions and 6 deletions
--- a/src/cli/App.tsx
+++ b/src/cli/App.tsx
@@ -326,6 +326,49 @@ function isNonStateCommand(msg: string): boolean {
  return false;
 }

+const APPROVAL_OPTIONS_HEIGHT = 8;
+const APPROVAL_PREVIEW_BUFFER = 4;
+const MIN_WRAP_WIDTH = 10;
+const TEXT_WRAP_GUTTER = 6;
+const DIFF_WRAP_GUTTER = 12;
+
+function countWrappedLines(text: string, width: number): number {
+  if (!text) return 0;
+  const wrapWidth = Math.max(1, width);
+  return text.split(/\r?\n/).reduce((sum, line) => {
+    const len = line.length;
+    const wrapped = Math.max(1, Math.ceil(len / wrapWidth));
+    return sum + wrapped;
+  }, 0);
+}
+
+function countWrappedLinesFromList(lines: string[], width: number): number {
+  if (!lines.length) return 0;
+  const wrapWidth = Math.max(1, width);
+  return lines.reduce((sum, line) => {
+    const len = line.length;
+    const wrapped = Math.max(1, Math.ceil(len / wrapWidth));
+    return sum + wrapped;
+  }, 0);
+}
+
+function estimateAdvancedDiffLines(
+  diff: AdvancedDiffSuccess,
+  width: number,
+): number {
+  const wrapWidth = Math.max(1, width);
+  let total = 0;
+  for (const hunk of diff.hunks) {
+    for (const line of hunk.lines) {
+      const raw = line.raw || "";
+      if (raw.startsWith("\\")) continue;
+      const text = raw.slice(1);
+      total += Math.max(1, Math.ceil(text.length / wrapWidth));
+    }
+  }
+  return total;
+}
+
 // tiny helper for unique ids (avoid overwriting prior user lines)
 function uid(prefix: string) {
  return `${prefix}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`;
@@ -1653,6 +1696,163 @@ export default function App({
  // This prevents double-committing when the approval changes
  const eagerCommittedPreviewsRef = useRef<Set<string>>(new Set());

+  const estimateApprovalPreviewLines = useCallback(
+    (approval: ApprovalRequest): number => {
+      const toolName = approval.toolName;
+      if (!toolName) return 0;
+      const args = safeJsonParseOr<Record<string, unknown>>(
+        approval.toolArgs || "{}",
+        {},
+      );
+      const wrapWidth = Math.max(MIN_WRAP_WIDTH, columns - TEXT_WRAP_GUTTER);
+      const diffWrapWidth = Math.max(
+        MIN_WRAP_WIDTH,
+        columns - DIFF_WRAP_GUTTER,
+      );
+
+      if (isShellTool(toolName)) {
+        const t = toolName.toLowerCase();
+        let command = "(no command)";
+        let description = "";
+
+        if (t === "shell") {
+          const cmdVal = args.command;
+          command = Array.isArray(cmdVal)
+            ? cmdVal.join(" ")
+            : typeof cmdVal === "string"
+              ? cmdVal
+              : "(no command)";
+          description =
+            typeof args.justification === "string" ? args.justification : "";
+        } else {
+          command =
+            typeof args.command === "string" ? args.command : "(no command)";
+          description =
+            typeof args.description === "string"
+              ? args.description
+              : typeof args.justification === "string"
+                ? args.justification
+                : "";
+        }
+
+        let lines = 3; // solid line + header + blank line
+        lines += countWrappedLines(command, wrapWidth);
+        if (description) {
+          lines += countWrappedLines(description, wrapWidth);
+        }
+        return lines;
+      }
+
+      if (
+        isFileEditTool(toolName) ||
+        isFileWriteTool(toolName) ||
+        isPatchTool(toolName)
+      ) {
+        const headerLines = 4; // solid line + header + dotted lines
+        let diffLines = 0;
+        const toolCallId = approval.toolCallId;
+
+        if (isPatchTool(toolName) && typeof args.input === "string") {
+          const operations = parsePatchOperations(args.input);
+          operations.forEach((op, idx) => {
+            if (idx > 0) diffLines += 1; // blank line between operations
+            diffLines += 1; // filename line
+
+            const diffKey = toolCallId ? `${toolCallId}:${op.path}` : undefined;
+            const opDiff =
+              diffKey && precomputedDiffsRef.current.has(diffKey)
+                ? precomputedDiffsRef.current.get(diffKey)
+                : undefined;
+
+            if (opDiff) {
+              diffLines += estimateAdvancedDiffLines(opDiff, diffWrapWidth);
+              return;
+            }
+
+            if (op.kind === "add") {
+              diffLines += countWrappedLines(op.content, wrapWidth);
+              return;
+            }
+            if (op.kind === "update") {
+              if (op.patchLines?.length) {
+                diffLines += countWrappedLinesFromList(
+                  op.patchLines,
+                  wrapWidth,
+                );
+              } else {
+                diffLines += countWrappedLines(op.oldString || "", wrapWidth);
+                diffLines += countWrappedLines(op.newString || "", wrapWidth);
+              }
+              return;
+            }
+
+            diffLines += 1; // delete placeholder
+          });
+
+          return headerLines + diffLines;
+        }
+
+        const diff =
+          toolCallId && precomputedDiffsRef.current.has(toolCallId)
+            ? precomputedDiffsRef.current.get(toolCallId)
+            : undefined;
+
+        if (diff) {
+          diffLines += estimateAdvancedDiffLines(diff, diffWrapWidth);
+          return headerLines + diffLines;
+        }
+
+        if (Array.isArray(args.edits)) {
+          for (const edit of args.edits) {
+            if (!edit || typeof edit !== "object") continue;
+            const oldString =
+              typeof edit.old_string === "string" ? edit.old_string : "";
+            const newString =
+              typeof edit.new_string === "string" ? edit.new_string : "";
+            diffLines += countWrappedLines(oldString, wrapWidth);
+            diffLines += countWrappedLines(newString, wrapWidth);
+          }
+          return headerLines + diffLines;
+        }
+
+        if (typeof args.content === "string") {
+          diffLines += countWrappedLines(args.content, wrapWidth);
+          return headerLines + diffLines;
+        }
+
+        const oldString =
+          typeof args.old_string === "string" ? args.old_string : "";
+        const newString =
+          typeof args.new_string === "string" ? args.new_string : "";
+        diffLines += countWrappedLines(oldString, wrapWidth);
+        diffLines += countWrappedLines(newString, wrapWidth);
+        return headerLines + diffLines;
+      }
+
+      return 0;
+    },
+    [columns],
+  );
+
+  const shouldEagerCommitApprovalPreview = useCallback(
+    (approval: ApprovalRequest): boolean => {
+      if (!terminalRows) return false;
+      const previewLines = estimateApprovalPreviewLines(approval);
+      if (previewLines === 0) return false;
+      return (
+        previewLines + APPROVAL_OPTIONS_HEIGHT + APPROVAL_PREVIEW_BUFFER >=
+        terminalRows
+      );
+    },
+    [estimateApprovalPreviewLines, terminalRows],
+  );
+
+  const currentApprovalShouldCommitPreview = useMemo(() => {
+    if (!currentApproval) return false;
+    if (currentApproval.toolName === "ExitPlanMode") return false;
+    return shouldEagerCommitApprovalPreview(currentApproval);
+  }, [currentApproval, shouldEagerCommitApprovalPreview]);
+
  // Recompute UI state from buffers after each streaming chunk
  const refreshDerived = useCallback(() => {
    const b = buffersRef.current;
@@ -1820,6 +2020,36 @@ export default function App({
    }
  }, [currentApproval]);

+  // Eager commit for large approval previews (bash/file edits) to avoid flicker
+  useEffect(() => {
+    if (!currentApproval) return;
+    if (currentApproval.toolName === "ExitPlanMode") return;
+
+    const toolCallId = currentApproval.toolCallId;
+    if (!toolCallId) return;
+    if (eagerCommittedPreviewsRef.current.has(toolCallId)) return;
+    if (!currentApprovalShouldCommitPreview) return;
+
+    const previewItem: StaticItem = {
+      kind: "approval_preview",
+      id: `approval-preview-${toolCallId}`,
+      toolCallId,
+      toolName: currentApproval.toolName,
+      toolArgs: currentApproval.toolArgs || "{}",
+    };
+
+    if (
+      (isFileEditTool(currentApproval.toolName) ||
+        isFileWriteTool(currentApproval.toolName)) &&
+      precomputedDiffsRef.current.has(toolCallId)
+    ) {
+      previewItem.precomputedDiff = precomputedDiffsRef.current.get(toolCallId);
+    }
+
+    setStaticItems((prev) => [...prev, previewItem]);
+    eagerCommittedPreviewsRef.current.add(toolCallId);
+  }, [currentApproval, currentApprovalShouldCommitPreview]);
+
  // Backfill message history when resuming (only once)
  useEffect(() => {
    if (
@@ -9780,6 +10010,11 @@ Plan file path: ${planFilePath}`;
  const inputVisible = !showExitStats;
  const inputEnabled =
    !showExitStats && pendingApprovals.length === 0 && !anySelectorOpen;
+  const currentApprovalPreviewCommitted = currentApproval?.toolCallId
+    ? eagerCommittedPreviewsRef.current.has(currentApproval.toolCallId)
+    : false;
+  const showApprovalPreview =
+    !currentApprovalShouldCommitPreview && !currentApprovalPreviewCommitted;

  useEffect(() => {
    trajectoryTokenDisplayRef.current = trajectoryTokenDisplay;
@@ -9920,6 +10155,7 @@ Plan file path: ${planFilePath}`;
                            allowPersistence={
                              currentApprovalContext?.allowPersistence ?? true
                            }
+                            showPreview={showApprovalPreview}
                          />
                        ) : ln.kind === "user" ? (
                          <UserMessage line={ln} />
@@ -9998,6 +10234,7 @@ Plan file path: ${planFilePath}`;
                    allowPersistence={
                      currentApprovalContext?.allowPersistence ?? true
                    }
+                    showPreview={showApprovalPreview}
                  />
                </Box>
              )}
--- a/src/cli/components/ApprovalSwitch.tsx
+++ b/src/cli/components/ApprovalSwitch.tsx
@@ -67,6 +67,7 @@ type Props = {
  isFocused?: boolean;
  approveAlwaysText?: string;
  allowPersistence?: boolean;
+  showPreview?: boolean;

  // Special handlers for ExitPlanMode
  onPlanApprove?: (acceptEdits: boolean) => void;
@@ -209,6 +210,7 @@ export const ApprovalSwitch = memo(
    onEnterPlanModeReject,
    precomputedDiff,
    allDiffs,
+    showPreview = true,
  }: Props) => {
    const toolName = approval.toolName;

@@ -245,6 +247,7 @@ export const ApprovalSwitch = memo(
            isFocused={isFocused}
            approveAlwaysText={approveAlwaysText}
            allowPersistence={allowPersistence}
+            showPreview={showPreview}
          />
        );
      }
@@ -264,6 +267,7 @@ export const ApprovalSwitch = memo(
            isFocused={isFocused}
            approveAlwaysText={approveAlwaysText}
            allowPersistence={allowPersistence}
+            showPreview={showPreview}
          />
        );
      }
@@ -328,6 +332,7 @@ export const ApprovalSwitch = memo(
        isFocused={isFocused}
        approveAlwaysText={approveAlwaysText}
        allowPersistence={allowPersistence}
+        showPreview={showPreview}
      />
    );
  },
--- a/src/cli/components/InlineBashApproval.tsx
+++ b/src/cli/components/InlineBashApproval.tsx
@@ -21,6 +21,7 @@ type Props = {
  isFocused?: boolean;
  approveAlwaysText?: string;
  allowPersistence?: boolean;
+  showPreview?: boolean;
 };

 // Horizontal line character for Claude Code style
@@ -42,6 +43,7 @@ export const InlineBashApproval = memo(
    isFocused = true,
    approveAlwaysText,
    allowPersistence = true,
+    showPreview = true,
  }: Props) => {
    const [selectedOption, setSelectedOption] = useState(0);
    const {
@@ -153,13 +155,15 @@ export const InlineBashApproval = memo(
        : "Type reason · Esc to cancel"
      : "Enter to select · Esc to cancel";

+    const optionsMarginTop = showPreview ? 1 : 0;
+
    return (
      <Box flexDirection="column">
        {/* Static command content - memoized to prevent re-render on keystroke */}
-        {memoizedCommandContent}
+        {showPreview && memoizedCommandContent}

        {/* Options */}
-        <Box marginTop={1} flexDirection="column">
+        <Box marginTop={optionsMarginTop} flexDirection="column">
          {/* Option 1: Yes */}
          <Box flexDirection="row">
            <Box width={5} flexShrink={0}>
--- a/src/cli/components/InlineFileEditApproval.tsx
+++ b/src/cli/components/InlineFileEditApproval.tsx
@@ -44,6 +44,7 @@ type Props = {
  isFocused?: boolean;
  approveAlwaysText?: string;
  allowPersistence?: boolean;
+  showPreview?: boolean;
 };

 // Horizontal line characters for Claude Code style
@@ -158,6 +159,7 @@ export const InlineFileEditApproval = memo(
    isFocused = true,
    approveAlwaysText,
    allowPersistence = true,
+    showPreview = true,
  }: Props) => {
    const [selectedOption, setSelectedOption] = useState(0);
    const {
@@ -429,13 +431,15 @@ export const InlineFileEditApproval = memo(
        : "Type reason · Esc to cancel"
      : "Enter to select · Esc to cancel";

+    const optionsMarginTop = showPreview ? 1 : 0;
+
    return (
      <Box flexDirection="column">
        {/* Static diff content - memoized to prevent re-render on keystroke */}
-        {memoizedDiffContent}
+        {showPreview && memoizedDiffContent}

        {/* Options */}
-        <Box marginTop={1} flexDirection="column">
+        <Box marginTop={optionsMarginTop} flexDirection="column">
          {/* Option 1: Yes */}
          <Box flexDirection="row">
            <Box width={5} flexShrink={0}>
--- a/src/cli/components/InlineGenericApproval.tsx
+++ b/src/cli/components/InlineGenericApproval.tsx
@@ -16,6 +16,7 @@ type Props = {
  isFocused?: boolean;
  approveAlwaysText?: string;
  allowPersistence?: boolean;
+  showPreview?: boolean;
 };

 // Horizontal line character for Claude Code style
@@ -56,6 +57,7 @@ export const InlineGenericApproval = memo(
    isFocused = true,
    approveAlwaysText,
    allowPersistence = true,
+    showPreview = true,
  }: Props) => {
    const [selectedOption, setSelectedOption] = useState(0);
    const {
@@ -165,13 +167,15 @@ export const InlineGenericApproval = memo(
        : "Type reason · Esc to cancel"
      : "Enter to select · Esc to cancel";

+    const optionsMarginTop = showPreview ? 1 : 0;
+
    return (
      <Box flexDirection="column">
        {/* Static tool content - memoized to prevent re-render on keystroke */}
-        {memoizedToolContent}
+        {showPreview && memoizedToolContent}

        {/* Options */}
-        <Box marginTop={1} flexDirection="column">
+        <Box marginTop={optionsMarginTop} flexDirection="column">
          {/* Option 1: Yes */}
          <Box flexDirection="row">
            <Box width={5} flexShrink={0}>