Skip to content

Add hunk session review --json for full live-session exports#160

Merged
benvinegar merged 5 commits intomainfrom
pi/issue-158-session-review
Apr 6, 2026
Merged

Add hunk session review --json for full live-session exports#160
benvinegar merged 5 commits intomainfrom
pi/issue-158-session-review

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • add a hunk session review command and daemon API action for exporting the full loaded live-session review model
  • keep full review data in session registration state, including ordered files, raw patches, and hunk headers/ranges
  • update docs and the repo skills/hunk-review/SKILL.md so agents know to use the new command

Testing

  • bun run typecheck
  • bun test

Part of #158.

This PR description was generated by Pi using OpenAI o3

@benvinegar benvinegar changed the title Add for full live-session exports Add hunk session review --json for full live-session exports Apr 3, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR adds a hunk session review command that exports the full loaded live-session review model — including ordered files, raw patches, and hunk headers/ranges — so agents can inspect a complete changeset in one pass rather than navigating file-by-file. It threads the new review action through the CLI parser, session protocol, daemon state, and MCP server consistently, and updates the SKILL.md agent guide to recommend session review --json as the first step in a review workflow.

Key changes:

  • New SessionReviewFile / SessionReviewHunk / SessionReview types added to src/mcp/types.ts; HunkSessionRegistration gains a reviewFiles field alongside the existing files field
  • buildSessionReviewFiles builds rich per-hunk data (patch, header, line ranges) at registration/reload time in sessionRegistration.ts
  • HunkDaemonState.getSessionReview reads directly from the registration (no round-trip to the TUI) using a new private getSessionEntry helper
  • The review action is wired end-to-end: CLI → HTTP client → daemon server → state → JSON/text output
  • Good test coverage across daemon state, server, session-commands, CLI parsing, and help output

Findings:

  • formatHunkHeader in sessionRegistration.ts is an identical copy of the private hunkHeader function in src/ui/diff/pierre.ts; per the project's architecture rules, this should be extracted to a shared helper module
  • HunkSessionRegistration now carries both files: SessionFileSummary[] and reviewFiles: SessionReviewFile[]; since SessionReviewFile extends SessionFileSummary, the two arrays carry overlapping data and must be kept in sync by both createSessionRegistration and updateSessionRegistration

Confidence Score: 5/5

Safe to merge — all remaining findings are P2 style/architecture suggestions with no runtime impact.

The new command is correctly plumbed end-to-end with no logic errors. Both open findings (duplicated hunk-header helper, parallel files/reviewFiles arrays) are maintainability concerns rather than defects, and they do not affect correctness or reliability of the feature.

src/mcp/sessionRegistration.ts (duplicated formatHunkHeader) and src/mcp/types.ts (redundant files/reviewFiles in HunkSessionRegistration)

Important Files Changed

Filename Overview
src/mcp/sessionRegistration.ts Adds buildSessionReviewFiles to populate rich per-hunk data; formatHunkHeader duplicates logic already present in src/ui/diff/pierre.ts.
src/mcp/types.ts Introduces SessionReviewHunk, SessionReviewFile, and SessionReview types; HunkSessionRegistration now stores both files and reviewFiles, which are redundant since SessionReviewFile extends SessionFileSummary.
src/mcp/daemonState.ts Adds getSessionReview and private getSessionEntry helper; logic is clean and consistent with existing patterns.
src/mcp/server.ts Adds review to the supported actions list and routes it to getSessionReview; straightforward addition.
src/session/commands.ts Wires up review action in CLI client, action-to-daemon map, and runSessionCommand switch; adds formatReviewOutput for text mode.
src/session/protocol.ts Extends protocol with review action type in request/response unions; clean extension.
src/core/types.ts Adds review to the SessionGetCommandInput action union; minimal and correct.
src/core/cli.ts Registers session review in the help text and subcommand parser; correct branching and description text.
test/mcp-daemon.test.ts Adds reviewFiles to test fixtures and a new test for getSessionReview; coverage is adequate.
test/session-commands.test.ts Adds a full end-to-end review command test with JSON output assertion; thorough coverage of the new path.

Sequence Diagram

sequenceDiagram
    participant Agent
    participant CLI as hunk session review
    participant Client as HttpHunkDaemonCliClient
    participant Server as MCP Daemon Server
    participant State as HunkDaemonState
    participant Reg as HunkSessionRegistration

    Agent->>CLI: hunk session review --repo . --json
    CLI->>Client: getSessionReview(selector)
    Client->>Server: POST /session-api { action: "review", selector }
    Server->>State: getSessionReview(selector)
    State->>State: getSessionEntry(selector)
    State->>Reg: registration.reviewFiles
    State->>State: findSelectedReviewFile(reviewFiles, snapshot)
    State-->>Server: SessionReview { files, selectedFile, selectedHunk, patch, hunks... }
    Server-->>Client: { review: SessionReview }
    Client-->>CLI: SessionReview
    CLI-->>Agent: JSON output
Loading

Reviews (1): Last reviewed commit: "Add session review export command" | Re-trigger Greptile

Comment thread src/mcp/sessionRegistration.ts Outdated
Comment on lines +47 to +53
function formatHunkHeader(file: AppBootstrap["changeset"]["files"][number], hunkIndex: number) {
const hunk = file.metadata.hunks[hunkIndex]!;
const specs =
hunk.hunkSpecs ??
`@@ -${hunk.deletionStart},${hunk.deletionLines} +${hunk.additionStart},${hunk.additionLines} @@`;
return hunk.hunkContext ? `${specs} ${hunk.hunkContext}` : specs;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Duplicated hunk-header logic from pierre.ts

formatHunkHeader is an exact copy of the private hunkHeader function in src/ui/diff/pierre.ts (lines 317–322). Both functions use the same hunkSpecs ?? "@@ ... @@" fallback and hunkContext suffix pattern. Per the project's architecture rule — "make that structure explicit in helper modules and reuse it across rendering and interaction code instead of re-deriving it ad hoc in multiple places" — this should be extracted to a shared helper (e.g. src/core/hunkHeader.ts) and imported from both call sites instead of being duplicated.

// src/core/hunkHeader.ts (example extraction)
import type { Hunk } from "@pierre/diffs";

/** Format a unified-diff hunk header string (specs + optional context). */
export function formatHunkHeader(hunk: Hunk): string {
  const specs =
    hunk.hunkSpecs ??
    `@@ -${hunk.deletionStart},${hunk.deletionLines} +${hunk.additionStart},${hunk.additionLines} @@`;
  return hunk.hunkContext ? `${specs} ${hunk.hunkContext}` : specs;
}

Then sessionRegistration.ts and pierre.ts both import from there.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I extracted the shared hunk-header formatter into src/core/hunkHeader.ts and switched both src/ui/diff/pierre.ts and src/mcp/sessionRegistration.ts to reuse it.

This comment was generated by Pi using OpenAI o3

Comment thread src/mcp/types.ts
Comment on lines 62 to 65
terminal?: SessionTerminalMetadata;
files: SessionFileSummary[];
reviewFiles: SessionReviewFile[];
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant parallel arrays in HunkSessionRegistration

HunkSessionRegistration now carries both files: SessionFileSummary[] and reviewFiles: SessionReviewFile[], where SessionReviewFile extends SessionFileSummary. Every entry in files is also present (with additional fields) in reviewFiles, so the two arrays are always kept in sync by both createSessionRegistration and updateSessionRegistration. The files array is used downstream only by listSessions()ListedSession.files.

Consider deriving files from reviewFiles at the listSessions() call site (e.g. registration.reviewFiles.map(({ patch: _, hunks: __, ...summary }) => summary)) and dropping the redundant files field, or at minimum documenting why the two arrays must stay in sync so the invariant is visible to future maintainers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I removed the redundant files field from HunkSessionRegistration and now derive the lightweight file summaries from reviewFiles inside listSessions(), so there is a single source of truth for the review export.

This comment was generated by Pi using OpenAI o3

@benvinegar benvinegar force-pushed the pi/issue-158-session-review branch from e9f8084 to f92a75f Compare April 4, 2026 00:28
@benvinegar benvinegar force-pushed the pi/issue-158-session-review branch from f92a75f to 85b8d4b Compare April 6, 2026 15:57
@benvinegar benvinegar merged commit 7be4d17 into main Apr 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant