feat(panel): read-only diff pop-out from Accomplished rows - #20
Conversation
Reading what an agent actually changed meant leaving the app: the
Accomplished panel names the file ("Edited src/bar.rs") but not the change.
Clicking a Wrote/Edited row now opens a read-only pop-out with that file's
unified diff.
The two halves already existed but did not line up: `git_diff_cached(cwd)`
returns a whole directory's staged diff, while a row carries one file path
that may live in a different repo than the tab's cwd. Bridged entirely in
TypeScript, with no new Tauri command: run the existing command in the
file's own directory, then slice that file's section out of the unified
diff in `src/lib/diff.ts`. Diff paths are repo-relative and the row's path
is absolute, so they are matched by separator-anchored suffix — src/b/x.ts
never answers for a row about src/a/x.ts.
Staged-only is a real limit, and it is surfaced rather than worked around:
nothing is staged on the user's behalf (that would mutate the repo behind a
human who is mid-commit), so an unstaged edit, or a file outside any repo
git can see, renders an empty state naming the constraint. Every failure
path — git absent, directory gone, path unparseable — resolves to that same
empty state; the lookup can never throw into the panel tree.
Which rows are diffable is decided in `listToolEvents`, not in the
component: `ToolEvent.filePath` is set only for tools that write the file
they name, so Read/Grep/Bash rows stay plain text and panels stay dumb.
Read-only by design — see docs/IDEAS.md for why the editable code view is
rejected. No schema change, no migration, no new dependency. `diff:check`
covers the slicing and rides the existing `npm run check` gate.
Closes SuperLogicAI#10
|
Review findings (CI green, but flagging before merge): src/lib/diff.ts:30 — src/lib/diff.ts:34 — same class of break under src/lib/repo.ts:116 — Minor, non-blocking:
|
|
Yeah, the Rust wrapper is the right call. Parsing diff headers to find a file was solving the problem one layer too late. A thin git_diff_file(cwd, path) in pty.rs running git diff --cached -- lets git do the scoping, so quotePath and mnemonicPrefix stop being my problem. sectionPaths, extractFileDiff and most of diff-check.ts go in the bin. dirOf stays so it runs in the file's own repo, tab cwd as fallback. One question while there's a Rust command in play anyway: do you want git diff HEAD -- in this PR too, so the pop-out isn't an empty box 90% of the time? Or keep this strictly staged and do that separately. Going staged-only unless you say otherwise. NotebookEdit: reading notebook_path next to file_path in listToolEvents, plus the same one-liner in delta.ts since it's the same hole. WRITES gets hoisted next to VERB. Modal dedupe I'm leaving alone, this PR doesn't need to grow a refactor. Still draft until somebody with a Mac actually clicks the thing. Prompy to Claude to fix the issue dependent on the choice of the head |
package.json: keep both check-script lists, diff:check appended. docs/TESTING.md: interleave both phases' checklist entries. SidePanel.tsx: keep both branches' new state hooks (diffPath + expandedSessions/plannedCard/seededCwdRef), no logic overlap. Verified: tsc --noEmit clean, npm run check all green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138tkBPv1EsZBHzC445NxeK
Closes #10.
Clicking a Wrote/Edited row in the Accomplished panel now opens a read-only pop-out with that file's unified diff.
Read this first: staged-only is a real limit
The issue anticipated it —
— so this PR does exactly that. But it's worth being blunt about how often the empty state wins: an agent that edits a file doesn't stage it, so unless the user has run
git addthemselves, the common path is the empty state rather than a diff. The plumbing is right; the only available source is usually dry.git diff HEAD -- <path>would fix it, at the cost of a new Tauri command — scope this issue didn't ask for. Happy to add it here or as a follow-up, whichever you prefer. Flagging it rather than quietly shipping a feature that mostly shows an empty box.Bridging the signature mismatch
src-tauri/is untouched — no Rust change, no new Tauri command. The whole bridge issrc/lib/diff.ts:git_diff_cached(cwd)returns a directory's entire staged diff, while a row carries one file path that may live in a different repo than the tab's cwd. So:dirOf(file_path)gives the row's own parent directory (both separators), and the existing command runs there.git diff --cachedignores the subdirectory for scoping, so this returns the staged diff of the repo that file actually lives in — which solves the different-repo half for free.extractFileDiffslices out that one file'sdiff --gitsection. Split on/^(?=diff --git )/m, so a+diff --gitline inside hunk content can't fabricate a section; collect paths from--- a//+++ b/, falling back to the header for hunkless sections like a pure rename.src/b/x.tscan never answer for a row aboutsrc/a/x.ts./dev/nullnever matches; renames match on either side.loadFileDiffretries once against the tab'scwdwhen the file's own directory is gone (pruned worktree, deleted dir) but the tab's repo still holds the staged change.The path reaches git as a
Command::arg, never a shell string.The no-diff case
An empty state worded to name the constraint rather than look broken:
Nothing is staged on the user's behalf. That would mutate the index behind a human who may be mid-commit, and the Commit & Push footer's
git_add_u/git_add_allare explicitly user-initiated. Read-only stays read-only.Every failure path — git absent, directory gone, path unparseable — resolves to that same empty state. The lookup is
.catch(() => "")at both the invoke and the effect, so nothing can throw into the panel tree, and neither the surrounding panel nor the terminals are affected either way.Scope
DiffModal.tsxmirrorsLandingNoteModal/IsolateLoopModalexactly — same overlay geometry (thetop-7titlebar-drag comment preserved), same card classes, overlay-click dismiss plus the window Esc handler, same footer button. Emerald to match the Accomplished section.<pre>, monospace. No highlighting, no colorization, no new dependency.listToolEventssetsToolEvent.filePathonly for tools that write the file they name (Edit/Write/NotebookEdit/MultiEdit), so Read/Grep/Bash rows stay inert and the panel stays dumb. Row text is unchanged.docs/IDEAS.md.Verification
npx tsc --noEmit— clean.npm run check— 13/13.npm run golden— not run; no extraction prompt touched.New
scripts/diff-check.ts+npm run diff:check, wired into the aggregatenpm run checkso CI gates it without aci.ymledit. It coversdirOfon posix/Windows/bare paths, andextractFileDiffon single-section extraction, no bleed into neighbours, the same-basename-in-a-sibling-directory non-match, unstaged →"", empty diff →"", empty path →"", a backslashed Windows path against git's forward slashes,new filewith--- /dev/null, a hunkless rename on both sides, and a fakediff --gitline inside hunk content.Draft, because nothing was clicked. This was written on Windows and the app is macOS-only, so the issue's two behavioural criteria — clicking a file row shows its diff and a row with no diff shows an empty state — are not verified. Neither is the modal's visual result: layout, wide-diff overflow, and scroll behaviour at
max-h-[80vh]have never been seen. The dir-argument behaviour is reasoned from git semantics, not observed.Manual steps are in docs/TESTING.md § 26, in that file's style with every box unchecked.