fix(approval): show file content/diff and open full-screen preview on ctrl+e - #139
Merged
Conversation
After #26 the WriteTool/EditTool input display was reduced to `{kind: 'file_io', operation, path}`, dropping the args carried by the previous generic fallback. The approval panel then only had a path to show — no file content for Write, no diff hunk for Edit — and ctrl+e expanded to the same one-liner. Extend the file_io display with optional `content` / `before` / `after` fields so Write can attach its full content and Edit can attach its old_string/new_string hunk. The adapter promotes file_io+content to a file_content block and file_io+before/after to a diff block, matching what the panel renders for the legacy generic-fallback path.
Inline ctrl+e expand-in-place inflated the approval panel past one viewport for any non-trivial Edit / Write, which collided with pi-tui's inline differential renderer and the terminal's "snap to bottom on stdout" reflex: scrolling back glitched and the screen flickered. On top of that, the diff renderer's O(m·n) LCS DP ran every frame the panel was visible, so each spinner tick re-paid the cost. Make ctrl+e hand off to a dedicated full-screen viewer instead. The viewer renders all body lines once at construction and slices them on scroll, so per-frame cost is O(viewport) regardless of payload size. It uses the same nested-takeover pattern as TaskOutputViewer; the approval panel instance is preserved and refocused on close so the selection / feedback state survives. The panel itself drops its local `expanded` toggle and always renders the compact cluster view; ctrl+e now exclusively forwards to the host when there is something to preview, and falls through to the existing plan-expand toggle otherwise.
🦋 Changeset detectedLatest commit: 4530d43 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
fix #132
Problem
After the recent permission decision policies rework, the Write and Edit input display was reduced to
{kind: 'file_io', operation, path}, dropping the args the previous generic-fallback path carried. The approval prompt could then only show the path — no file content for Write, no diff hunk for Edit — and ctrl+e expanded to the same one-liner.Even with the data restored, pressing ctrl+e on a non-trivial Edit / Write inflated the panel past one viewport. That collided with the TUI's inline differential renderer and the terminal's default "snap to bottom on stdout" behavior: scrolling back glitched, the screen flickered, and the only stable position was the very bottom. Worse, the diff was recomputed with an O(m·n) LCS DP every frame the panel was visible, so each spinner tick paid the cost again.
What changed
Restore content on the data side: extend the
file_ioinput display with optionalcontent/before/after. Write attaches its full content and Edit attaches itsold_string/new_string; the adapter promotes these into thefile_content/diffdisplay blocks the panel already knows how to render, matching what the old generic-fallback path produced.Replace inline expand on the UI side: ctrl+e now hands off to a dedicated full-screen viewer (modeled after the task output viewer). The viewer builds its body lines once at construction and slices them on scroll, so per-frame cost stays O(viewport) regardless of payload size. The approval panel keeps its compact cluster view and forwards ctrl+e to the host, falling through to the existing plan-expand toggle when there is nothing to preview. The panel instance is preserved across open/close so selection and feedback state survive.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.