feat: submit inline diff reviews to coding agents #574

New issue

Open

opened 2026-08-28 12:00:18 +02:00 by dries · 0 comments

dries commented

2026-08-28 12:00:18 +02:00

Owner

Goal

Let a user review code produced in an ocman session from either fullscreen diff viewer—Working Tree or Session Changes—attach feedback to changed lines, add an optional overall note, and submit the complete review to the displayed coding-agent session as one structured message.

This is user-to-agent feedback, not forge PR review. The normal session transcript remains the conversation and history after submission.

User experience

Open the fullscreen viewer from Working Tree or Session Changes.
Hover or keyboard-focus a changed line or its line number to reveal an accessible Add comment action.
Activate it to open an editor directly below that line.
Save, edit, or delete the draft comment. There is at most one comment per exact line anchor.
Optionally enter an overall review note in the fullscreen review controls.
Navigate between files without losing draft comments. The file rail indicates which files have comments and how many.
Submit the entire review:
- while the session is idle, label the primary action Submit review and send immediately;
- while it is busy, label it Queue review and enqueue it for the next idle edge.
On success, clear that source's draft and let the resulting user message in the normal transcript serve as review history. On failure, retain the complete draft and surface the existing send error.

Require at least one inline comment or a non-empty overall note before enabling submission.

Review scope and anchors

Support only the two existing fullscreen diff sources:

working-tree: the current net git diff HEAD, including staged, unstaged, and displayable untracked files.
session-changes: the historical sequence of edits captured from session tool activity, including repeated patch sections for the same file.

An inline comment attaches to one changed line on either the old/deletion side or new/addition side. Line ranges, unchanged-line comments, threads, replies, and multiple comments on one anchor are out of scope.

Each draft anchor must retain enough immutable context to remain meaningful even after a refresh:

source kind;
current path and old path when applicable;
old/new side;
displayed line number;
captured line text;
patch-section identity;
content fingerprint for the rendered patch section;
for Session Changes, the exact historical patch occurrence rather than only path + line.

Use a stable patch-section fingerprint plus the occurrence metadata already available while splitting/rendering patches. Do not identify a Session Changes comment by path and line alone: one session can edit the same numbered line several times.

Draft persistence

Persist drafts in browser localStorage, following the existing composer-draft/bookmark patterns. Key drafts by compound session identity and source:

platform + sessionId + (working-tree | session-changes)

Keep Working Tree and Session Changes reviews separate. Drafts must survive closing the modal, switching files, and reloading the page. No state.db migration or server-side draft API is required.

Version the serialized local schema so malformed or future-incompatible values can be discarded safely. Never let one local or remote session read another session's draft.

Refresh and stale comments

Both diff sources refresh while the modal can remain open. Reconcile each draft anchor against the newly rendered patch section using its fingerprint and anchor context.

A matching anchor remains active.
A missing or changed anchor is visibly marked stale while retaining its captured path, side, line number, and line text.
A stale comment may not be submitted accidentally.
The user must either delete it or explicitly choose to include that stale comment before submission.
Included stale comments are clearly marked as stale in the generated review message.

Do not silently retarget a comment to a nearby line.

If an API response reports truncation, preserve the existing truncation UI and do not imply that the review covers unseen content.

Agent message

Do not introduce a new review endpoint or platform operation. Thread a review-submit callback from SessionDetail through RightPanel into the fullscreen viewer and reuse the existing session send path. This preserves:

explicit compound platform routing for local and remote sessions;
the currently selected composer agent/model settings;
stale-route, port, pending-permission, and pending-question guards;
immediate-versus-queued behavior;
optimistic/failed-send handling.

Do not call api.sendMessage directly from the modal.

Serialize the review deterministically as Markdown, for example:

## Code review

Source: Working Tree

Overall: Please address these before continuing.

### `frontend/src/App.tsx`
- New line 42 — `const value = ...`
  This should handle the empty state.

### `internal/server/foo.go`
- Old line 18 — `return nil`
  Preserve this error.

The exact serializer should:

group comments by file in viewer order;
preserve comment order within each file;
state the source (Working Tree or Session Changes);
identify old/new side and line number;
quote the captured line text safely;
distinguish repeated historical Session Changes occurrences when ambiguous;
mark explicitly included stale comments;
omit empty sections;
produce identical text for identical draft data.

Plain Markdown is the structured transport for this version. A dedicated backend DTO is deferred until reviews need server-side querying, sharing, or lifecycle state.

Implementation constraints

Refactor the opaque FullscreenDiffFile.body: ReactNode model only as far as needed to expose raw patch metadata and annotation callbacks to the shared fullscreen viewer.
Reuse @pierre/diffs annotation/gutter capabilities available in the installed version where practical; add no diff or review dependency.
Keep both source-specific sidebars using one shared fullscreen review interaction and serializer.
Session Changes may contain concatenated patch sections; integrate with splitPatchSections rather than flattening historical occurrences.
Carry session.platform through Session Changes requests/review identity. Bare session IDs can collide across machines.
Carry explicit remote ownership for Working Tree reads where the session already provides it; do not rely on permissive directory-owner fallback for review correctness.
Preserve existing diff rendering, file selection, rename display, binary-file fallback, lazy sidebar behavior, and modal accessibility/focus handling.
All comment actions and editors must be keyboard accessible and have stable ARIA labels. Do not rely on hover alone.

Likely touch points:

frontend/src/components/DiffFullscreenModal.tsx
frontend/src/components/RawDiffView.tsx
frontend/src/components/DiffView.tsx
frontend/src/components/diffOptions.ts
frontend/src/components/SessionChangesSidebar.tsx
frontend/src/components/WorkingTreeChangesSidebar.tsx
frontend/src/components/RightPanel.tsx
frontend/src/pages/session-detail/SessionDetail.tsx
frontend/src/pages/session-detail/useSessionActions.ts
frontend/src/lib/patchSections.ts
a small local draft model/store and deterministic review serializer under frontend/src/lib/

Non-goals

GitHub/Forgejo PR comments or review synchronization.
Approve/request-changes verdicts.
Submitted, resolved, or reopened review state.
Per-comment agent replies or threads.
Reviewing line ranges or unchanged lines.
Cross-browser/client draft sharing.
Backend persistence, schema migrations, or a review API.
A separate review-history UI.
Automatic code mutation or applying suggestions from the viewer.

Acceptance criteria

Both fullscreen viewers allow an accessible inline comment on an old or new changed line.
One exact anchor has at most one editable draft comment.
An optional overall note can be submitted alone or with inline comments.
File navigation, modal close/reopen, and browser reload preserve the correct source/session draft.
File rows display their draft comment counts.
Session Changes comments remain attached to the exact historical patch occurrence when the same path/line appears more than once.
Refresh reconciliation marks unmatched comments stale and never silently moves them.
Submission is blocked until every stale comment is deleted or explicitly included.
The generated Markdown is deterministic, grouped by file, and contains source, side, line, captured code, comment, and stale/occurrence details where applicable.
Idle submission sends immediately; busy submission is explicitly labeled and queued.
Submission uses the displayed session's compound platform identity and current composer agent/model choices.
A successful send clears only the submitted source's draft; a failed send keeps it intact.
Existing Working Tree and Session Changes fullscreen behavior remains intact when no review is being drafted.
No backend storage/API or new frontend dependency is added.

Verification

Add focused frontend tests covering at least:

anchor identity for old/new sides and repeated Session Changes patch occurrences;
add/edit/delete behavior and one-comment-per-anchor;
localStorage isolation by platform, session, and source;
persistence across modal remount;
refresh reconciliation and explicit stale inclusion;
deterministic Markdown serialization and escaping;
immediate versus queued callback invocation;
successful clear versus failed-send retention;
keyboard-accessible controls and labels;
regressions in existing fullscreen selection, rename, empty, and close behavior.

Run:

cd frontend && pnpm test
cd frontend && pnpm exec tsc -b
cd frontend && pnpm lint

Confirm frontend coverage does not fall under the repository coverage ratchet.

## Goal Let a user review code produced in an ocman session from either fullscreen diff viewer—**Working Tree** or **Session Changes**—attach feedback to changed lines, add an optional overall note, and submit the complete review to the displayed coding-agent session as one structured message. This is user-to-agent feedback, not forge PR review. The normal session transcript remains the conversation and history after submission. ## User experience 1. Open the fullscreen viewer from Working Tree or Session Changes. 2. Hover or keyboard-focus a changed line or its line number to reveal an accessible **Add comment** action. 3. Activate it to open an editor directly below that line. 4. Save, edit, or delete the draft comment. There is at most one comment per exact line anchor. 5. Optionally enter an overall review note in the fullscreen review controls. 6. Navigate between files without losing draft comments. The file rail indicates which files have comments and how many. 7. Submit the entire review: - while the session is idle, label the primary action **Submit review** and send immediately; - while it is busy, label it **Queue review** and enqueue it for the next idle edge. 8. On success, clear that source's draft and let the resulting user message in the normal transcript serve as review history. On failure, retain the complete draft and surface the existing send error. Require at least one inline comment or a non-empty overall note before enabling submission. ## Review scope and anchors Support only the two existing fullscreen diff sources: - `working-tree`: the current net `git diff HEAD`, including staged, unstaged, and displayable untracked files. - `session-changes`: the historical sequence of edits captured from session tool activity, including repeated patch sections for the same file. An inline comment attaches to one changed line on either the old/deletion side or new/addition side. Line ranges, unchanged-line comments, threads, replies, and multiple comments on one anchor are out of scope. Each draft anchor must retain enough immutable context to remain meaningful even after a refresh: - source kind; - current path and old path when applicable; - old/new side; - displayed line number; - captured line text; - patch-section identity; - content fingerprint for the rendered patch section; - for Session Changes, the exact historical patch occurrence rather than only path + line. Use a stable patch-section fingerprint plus the occurrence metadata already available while splitting/rendering patches. Do not identify a Session Changes comment by path and line alone: one session can edit the same numbered line several times. ## Draft persistence Persist drafts in browser `localStorage`, following the existing composer-draft/bookmark patterns. Key drafts by compound session identity and source: `platform + sessionId + (working-tree | session-changes)` Keep Working Tree and Session Changes reviews separate. Drafts must survive closing the modal, switching files, and reloading the page. No `state.db` migration or server-side draft API is required. Version the serialized local schema so malformed or future-incompatible values can be discarded safely. Never let one local or remote session read another session's draft. ## Refresh and stale comments Both diff sources refresh while the modal can remain open. Reconcile each draft anchor against the newly rendered patch section using its fingerprint and anchor context. - A matching anchor remains active. - A missing or changed anchor is visibly marked **stale** while retaining its captured path, side, line number, and line text. - A stale comment may not be submitted accidentally. - The user must either delete it or explicitly choose to include that stale comment before submission. - Included stale comments are clearly marked as stale in the generated review message. Do not silently retarget a comment to a nearby line. If an API response reports truncation, preserve the existing truncation UI and do not imply that the review covers unseen content. ## Agent message Do not introduce a new review endpoint or platform operation. Thread a review-submit callback from `SessionDetail` through `RightPanel` into the fullscreen viewer and reuse the existing session send path. This preserves: - explicit compound platform routing for local and remote sessions; - the currently selected composer agent/model settings; - stale-route, port, pending-permission, and pending-question guards; - immediate-versus-queued behavior; - optimistic/failed-send handling. Do not call `api.sendMessage` directly from the modal. Serialize the review deterministically as Markdown, for example: ```md ## Code review Source: Working Tree Overall: Please address these before continuing. ### `frontend/src/App.tsx` - New line 42 — `const value = ...` This should handle the empty state. ### `internal/server/foo.go` - Old line 18 — `return nil` Preserve this error. ``` The exact serializer should: - group comments by file in viewer order; - preserve comment order within each file; - state the source (`Working Tree` or `Session Changes`); - identify old/new side and line number; - quote the captured line text safely; - distinguish repeated historical Session Changes occurrences when ambiguous; - mark explicitly included stale comments; - omit empty sections; - produce identical text for identical draft data. Plain Markdown is the structured transport for this version. A dedicated backend DTO is deferred until reviews need server-side querying, sharing, or lifecycle state. ## Implementation constraints - Refactor the opaque `FullscreenDiffFile.body: ReactNode` model only as far as needed to expose raw patch metadata and annotation callbacks to the shared fullscreen viewer. - Reuse `@pierre/diffs` annotation/gutter capabilities available in the installed version where practical; add no diff or review dependency. - Keep both source-specific sidebars using one shared fullscreen review interaction and serializer. - Session Changes may contain concatenated patch sections; integrate with `splitPatchSections` rather than flattening historical occurrences. - Carry `session.platform` through Session Changes requests/review identity. Bare session IDs can collide across machines. - Carry explicit remote ownership for Working Tree reads where the session already provides it; do not rely on permissive directory-owner fallback for review correctness. - Preserve existing diff rendering, file selection, rename display, binary-file fallback, lazy sidebar behavior, and modal accessibility/focus handling. - All comment actions and editors must be keyboard accessible and have stable ARIA labels. Do not rely on hover alone. Likely touch points: - `frontend/src/components/DiffFullscreenModal.tsx` - `frontend/src/components/RawDiffView.tsx` - `frontend/src/components/DiffView.tsx` - `frontend/src/components/diffOptions.ts` - `frontend/src/components/SessionChangesSidebar.tsx` - `frontend/src/components/WorkingTreeChangesSidebar.tsx` - `frontend/src/components/RightPanel.tsx` - `frontend/src/pages/session-detail/SessionDetail.tsx` - `frontend/src/pages/session-detail/useSessionActions.ts` - `frontend/src/lib/patchSections.ts` - a small local draft model/store and deterministic review serializer under `frontend/src/lib/` ## Non-goals - GitHub/Forgejo PR comments or review synchronization. - Approve/request-changes verdicts. - Submitted, resolved, or reopened review state. - Per-comment agent replies or threads. - Reviewing line ranges or unchanged lines. - Cross-browser/client draft sharing. - Backend persistence, schema migrations, or a review API. - A separate review-history UI. - Automatic code mutation or applying suggestions from the viewer. ## Acceptance criteria - [ ] Both fullscreen viewers allow an accessible inline comment on an old or new changed line. - [ ] One exact anchor has at most one editable draft comment. - [ ] An optional overall note can be submitted alone or with inline comments. - [ ] File navigation, modal close/reopen, and browser reload preserve the correct source/session draft. - [ ] File rows display their draft comment counts. - [ ] Session Changes comments remain attached to the exact historical patch occurrence when the same path/line appears more than once. - [ ] Refresh reconciliation marks unmatched comments stale and never silently moves them. - [ ] Submission is blocked until every stale comment is deleted or explicitly included. - [ ] The generated Markdown is deterministic, grouped by file, and contains source, side, line, captured code, comment, and stale/occurrence details where applicable. - [ ] Idle submission sends immediately; busy submission is explicitly labeled and queued. - [ ] Submission uses the displayed session's compound platform identity and current composer agent/model choices. - [ ] A successful send clears only the submitted source's draft; a failed send keeps it intact. - [ ] Existing Working Tree and Session Changes fullscreen behavior remains intact when no review is being drafted. - [ ] No backend storage/API or new frontend dependency is added. ## Verification Add focused frontend tests covering at least: - anchor identity for old/new sides and repeated Session Changes patch occurrences; - add/edit/delete behavior and one-comment-per-anchor; - localStorage isolation by platform, session, and source; - persistence across modal remount; - refresh reconciliation and explicit stale inclusion; - deterministic Markdown serialization and escaping; - immediate versus queued callback invocation; - successful clear versus failed-send retention; - keyboard-accessible controls and labels; - regressions in existing fullscreen selection, rename, empty, and close behavior. Run: ```sh cd frontend && pnpm test cd frontend && pnpm exec tsc -b cd frontend && pnpm lint ``` Confirm frontend coverage does not fall under the repository coverage ratchet.