Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Fixed

- `diff` pane notes no longer persist. `i` / `x` / `a` keep working, but drafts
live in the overlay's memory instead of `.phi/review.json`, and switching
diffs drops them. The store was per project and outlived the session, so a
later `a` re-sent comments the agent had already answered.

- Compaction preserves the previous summary when the history bucket is empty,
including mid-turn cuts that only summarize the current turn prefix. Inherited
file-operation lists are refreshed once instead of accumulating duplicate blocks.
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ line notes, then hand them to the agent without leaving the terminal.

Slash-picker Enter inserts `/diff` plus a trailing space into the composer; submit to open. Inside the overlay:
`s` side-by-side, `i` add/edit a note, `x` delete, `a` send notes to the agent,
`?` help, `q` / `Esc` close. Notes persist under `.phi/review.json`.
`?` help, `q` / `Esc` close. Notes live in the overlay's memory only — switching
diffs drops them and nothing is written to disk.

## Code viewer

Expand Down
2 changes: 1 addition & 1 deletion README.zh-CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ permissions:

斜杠选择器里回车会把 `/diff` 连同一个空格填进输入框;再提交才打开。
审阅层内:`j`/`k` 移动,`s` 左右对照,`i` 添加/编辑批注,`x` 删除,`a` 发给代理,
`?` 帮助,`q` / `Esc` 关闭。批注保存在 `.phi/review.json`。
`?` 帮助,`q` / `Esc` 关闭。批注只存在内存里,切 diff 就丢,不会落盘。

## 代码查看器

Expand Down
2 changes: 1 addition & 1 deletion doc/project-layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
| `internal/tui/controller/` | Engine lifecycle, Bus/Msg, activity |
| `internal/version/` | Build-time `Version` (splash / `phi update`) |
| `internal/util/` | Shared helpers (diff, retry, SSE, file search, …) |
| `internal/util/diffreview/` | Unified-diff parse/render, review comments, git load |
| `internal/util/diffreview/` | Unified-diff parse/render, in-memory review notes, git load |
| `internal/permission/` | Permission policy and ask gate |
| `internal/extension/` | PXB extension discover/spawn/runner |
| `internal/mcp/` | MCP config + stdio client + pool (meta-tool route) |
Expand Down
2 changes: 1 addition & 1 deletion doc/tui.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal/tui/
| `composer` | Keyboard routing for chat, `/` slash, `?` shortcuts, `@` mention, `!` shell completion, Ctrl+K palette |
| `footer` | Composer status slot (activity ↔ tokens), bottom footer row (ext status, jobs, update hint) |
| `overlays` | Modal permission / continue-ask panels; replaces composer when active |
| `diffpane` | Full-screen git diff review; comments persist under `.phi/review.json` |
| `diffpane` | Full-screen git diff review; notes stay in memory |
| `codepane` | Full-screen source viewer: syntax highlight, caret, line selection |
| `submit` | User submit path: agent prompt, slash commands, `!bash`, cancel |
| `commands` | Slash/palette registry; session load/clear; extension command bridge |
Expand Down
76 changes: 21 additions & 55 deletions internal/tui/diffpane/pane.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ type Pane struct {

rows []diffreview.Row
drafts []diffreview.CommentDraft
commentPath string
highlighted map[int][]components.Span

cursor int
Expand All @@ -53,7 +52,6 @@ type Pane struct {

help bool
status string
dirty bool

picker listpicker.Picker

Expand Down Expand Up @@ -103,27 +101,31 @@ func (p *Pane) OpenGit(cwd string, spec []string) {
if cwd != "" {
p.cwd = cwd
}
keepNotes := slices.Equal(p.spec, spec)
p.spec = append([]string(nil), spec...)
p.label = diffreview.LabelForSpec(spec)
text, err := diffreview.LoadGit(context.Background(), p.cwd, spec)
if err != nil {
p.openParsed("", err.Error())
p.openParsed("", err.Error(), keepNotes)
return
}
p.openParsed(text, "")
p.openParsed(text, "", keepNotes)
}

// OpenText shows a preloaded unified diff (tests / paste).
func (p *Pane) OpenText(input string, spec []string) {
if p == nil {
return
}
keepNotes := slices.Equal(p.spec, spec)
p.spec = append([]string(nil), spec...)
p.label = diffreview.LabelForSpec(spec)
p.openParsed(input, "")
p.openParsed(input, "", keepNotes)
}

func (p *Pane) openParsed(input, loadErr string) {
// openParsed rebuilds rows for input. Notes are dropped unless keepNotes is set,
// so a note never outlives the diff it was written against.
func (p *Pane) openParsed(input, loadErr string, keepNotes bool) {
p.active = true
p.err = loadErr
p.help = false
Expand All @@ -132,11 +134,12 @@ func (p *Pane) openParsed(input, loadErr string) {
p.picker.Hide()
p.pendingG = false
p.pendingBracket = 0
p.commentPath = diffreview.CommentPath(p.cwd)
if !keepNotes {
p.drafts = nil
}

if loadErr != "" {
p.rows = nil
p.drafts = nil
p.highlighted = nil
p.cursor = 0
p.scroll = 0
Expand All @@ -151,13 +154,6 @@ func (p *Pane) openParsed(input, loadErr string) {
return
}
p.rows = doc.Rows()
file, err := diffreview.LoadFile(p.commentPath)
if err != nil {
p.status = "comments: " + err.Error()
p.drafts = nil
} else {
p.drafts = file.Comments
}
p.highlighted = diffview.HighlightRows(p.rows, p.theme)
p.cursor = 0
p.scroll = 0
Expand All @@ -170,14 +166,11 @@ func (p *Pane) openParsed(input, loadErr string) {
}
}

// Close hides the overlay, saving comments first.
// Close hides the overlay.
func (p *Pane) Close() {
if p == nil {
return
}
if p.dirty {
_ = p.saveComments()
}
p.active = false
p.help = false
p.searchMode = false
Expand Down Expand Up @@ -457,34 +450,23 @@ func (p *Pane) submitComment() {
p.commentEdit = false
if body == "" {
p.removeDraft(p.commentTarget)
p.dirty = true
if err := p.saveComments(); err != nil {
p.toast(err.Error())
}
p.status = "note removed"
return
}
found := false
for i, d := range p.drafts {
if diffreview.SameTarget(d, p.commentTarget) || (d.ID != "" && d.ID == p.commentTarget.ID) {
p.drafts[i].Body = body
found = true
break
p.status = "note updated"
return
}
}
if !found {
d := p.commentTarget
d.Body = body
if d.ID == "" {
d.ID = strconv.FormatInt(time.Now().UnixNano(), 36)
}
p.drafts = append(p.drafts, d)
d := p.commentTarget
d.Body = body
if d.ID == "" {
d.ID = strconv.FormatInt(time.Now().UnixNano(), 36)
}
p.dirty = true
if err := p.saveComments(); err != nil {
p.toast(err.Error())
return
}
p.status = "note saved"
p.drafts = append(p.drafts, d)
p.status = "note added"
}

func (p *Pane) deleteNote() {
Expand All @@ -495,11 +477,6 @@ func (p *Pane) deleteNote() {
return
}
p.removeDraft(drafts[0])
p.dirty = true
if err := p.saveComments(); err != nil {
p.toast(err.Error())
return
}
p.status = "note deleted"
}

Expand All @@ -514,17 +491,6 @@ func (p *Pane) removeDraft(target diffreview.CommentDraft) {
p.drafts = out
}

func (p *Pane) saveComments() error {
if p.commentPath == "" {
return nil
}
err := diffreview.SaveFile(p.commentPath, diffreview.CommentFile{Version: 1, Comments: p.drafts})
if err == nil {
p.dirty = false
}
return err
}

func (p *Pane) sendToAgent() {
prompt := strings.TrimSpace(diffreview.FormatPrompt(p.drafts))
if prompt == "" {
Expand Down
35 changes: 31 additions & 4 deletions internal/tui/diffpane/pane_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package diffpane

import (
"path/filepath"
"testing"

"github.com/pulseaiclub/xui"
Expand All @@ -21,9 +20,8 @@ const sampleDiff = `diff --git a/main.go b/main.go
`

func TestPaneOpenCommentSearchAndSend(t *testing.T) {
dir := t.TempDir()
var submitted string
p := New(components.DefaultTheme(), dir, func(s string) { submitted = s }, nil, nil)
p := New(components.DefaultTheme(), t.TempDir(), func(s string) { submitted = s }, nil, nil)
p.OpenText(sampleDiff, nil)
require.True(t, p.Active())
require.NotEmpty(t, p.rows)
Expand All @@ -47,7 +45,6 @@ func TestPaneOpenCommentSearchAndSend(t *testing.T) {
p.Handle(ctx, xui.KeyEvent{Press: true, Code: xui.KeyEnter})
require.False(t, p.commentEdit)
require.Len(t, p.drafts, 1)
assert.FileExists(t, filepath.Join(dir, ".phi", "review.json"))

key('/')
for _, r := range "new" {
Expand All @@ -66,6 +63,36 @@ func TestPaneOpenCommentSearchAndSend(t *testing.T) {
assert.Contains(t, submitted, "please rename")
}

func TestPaneDropsNotesWhenDiffChanges(t *testing.T) {
p := New(components.DefaultTheme(), t.TempDir(), nil, nil, nil)
p.OpenText(sampleDiff, nil)

ctx := &components.EventContext{}
key := func(r rune) {
p.Handle(ctx, xui.KeyEvent{Press: true, Code: xui.KeyRune, Rune: r})
}
for i := 0; i < 20 && (p.cursor >= len(p.rows) || p.rows[p.cursor].Code != "new"); i++ {
key('j')
}
require.Equal(t, "new", p.rows[p.cursor].Code)

key('i')
for _, r := range "stale note" {
key(r)
}
p.Handle(ctx, xui.KeyEvent{Press: true, Code: xui.KeyEnter})
require.Len(t, p.drafts, 1)

// Reopening the same spec keeps notes...
p.OpenText(sampleDiff, nil)
require.Len(t, p.drafts, 1)

// ...but switching specs must not carry them over, or `a` would resend
// comments the agent already answered.
p.OpenText(sampleDiff, []string{"staged"})
assert.Empty(t, p.drafts)
}

func TestPaneEscCloses(t *testing.T) {
p := New(components.DefaultTheme(), t.TempDir(), nil, nil, nil)
p.OpenText(sampleDiff, nil)
Expand Down
80 changes: 12 additions & 68 deletions internal/util/diffreview/comment.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
package diffreview

import (
"encoding/json"
"errors"
"os"
"path/filepath"
"sort"
"strconv"
"strings"
)

// DefaultFilePath is the project-local comment store.
const DefaultFilePath = ".phi/review.json"

// Side identifies which side of a unified diff a comment anchors to.
type Side string

Expand All @@ -23,70 +16,21 @@ const (

// Anchor locates one visible diff line for a comment.
type Anchor struct {
Path string `json:"path"`
Line int `json:"line"`
Side Side `json:"side"`
CommitID string `json:"commit_id,omitempty"`
Path string
Line int
Side Side
CommitID string
}

// CommentDraft is one review note on a single line.
// CommentDraft is one review note on a single line. Drafts live only in the
// pane's memory; nothing is written to disk.
type CommentDraft struct {
ID string `json:"id,omitempty"`
Path string `json:"path"`
Body string `json:"body"`
CommitID string `json:"commit_id,omitempty"`
Line int `json:"line"`
Side Side `json:"side"`
}

// CommentFile is the on-disk review store.
type CommentFile struct {
Version int `json:"version"`
Comments []CommentDraft `json:"comments"`
}

// CommentPath returns cwd/.phi/review.json.
func CommentPath(cwd string) string {
if cwd == "" {
return DefaultFilePath
}
return filepath.Join(cwd, DefaultFilePath)
}

// LoadFile reads comments; a missing file is an empty v1 store.
func LoadFile(path string) (CommentFile, error) {
data, err := os.ReadFile(path)
if errors.Is(err, os.ErrNotExist) {
return CommentFile{Version: 1}, nil
}
if err != nil {
return CommentFile{}, err
}

var file CommentFile
if err := json.Unmarshal(data, &file); err != nil {
return CommentFile{}, err
}
if file.Version == 0 {
file.Version = 1
}
return file, nil
}

// SaveFile writes comments as indented JSON.
func SaveFile(path string, file CommentFile) error {
if file.Version == 0 {
file.Version = 1
}
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
return err
}
data, err := json.MarshalIndent(file, "", " ")
if err != nil {
return err
}
data = append(data, '\n')
return os.WriteFile(path, data, 0o644) //nolint:gosec // G306: review notes are meant to be user-readable
ID string
Path string
Body string
CommitID string
Line int
Side Side
}

// CommentIndex maps visible drafts to the rows that render them.
Expand Down
Loading
Loading