# Milestone 4b: pull request reviews and line comments (contract between workers)

The global rules from `docs/m1-interfaces.md`, `docs/m3-interfaces.md` and `docs/m4-interfaces.md` apply:
- No JS and no inline styles.
- Full hex OIDs only, validated paths, and user values after `--end-of-options`.
- No unwrap in non-test code.
- Tests use `tempfile::TempDir`.
- Never put cargo target dirs or big files in /tmp.

## Concepts
- A **review** belongs to a PR (`issue_id`) and has an author. Its state is one of:
  - `pending`: a draft, visible only to its author, with at most one per author per PR.
  - `commented`, `approved` or `changes_requested`: submitted.
  - `dismissed`: a Write holder dismissed an approval or a change request, giving a reason.
- A **review comment** is a comment on one line of the PR diff, identified by:
  - `path`: the new path; for a deleted file, the old path;
  - `side`: `new` for added and context lines, `old` for deleted lines;
  - `line`: the line number on that side;
  - `commit_oid`: the head OID when it was written.
  It belongs to a review. A comment whose `reply_to` is set is a reply in the root comment's thread; replies are submitted immediately, through an auto-created `commented` review without a body. A root carries `resolved`.
- **Outdated:** after the head moves, a root comment is outdated when the blob at `path` differs between its `commit_oid` and the new head (this is coarse on purpose; line remapping comes later). Outdated threads still show in the conversation, with an "Outdated" badge, and no longer show on the Files tab.
- `diff_hunk` stores the last up to 4 diff lines ending at the commented line, as plain text with a `+`, `-` or space prefix, captured at creation. The conversation shows it as the thread's context without running git again.
- PR authors can't approve or request changes on their own PR; they can only comment. Anyone who can Read the base repo and is signed in can review. A review by a Read user counts in the summary, but merge rules don't enforce anything yet.

## Schema (migration `0008_reviews.sql`)
```sql
CREATE TABLE pull_reviews (
    id TEXT PRIMARY KEY, issue_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE,
    author_id TEXT NOT NULL REFERENCES users(id),
    state TEXT NOT NULL,                 -- pending | commented | approved | changes_requested | dismissed
    body TEXT NOT NULL DEFAULT '',
    commit_oid TEXT NOT NULL,            -- head at submit (or at creation while pending)
    dismissed_reason TEXT,
    created_at TEXT NOT NULL, submitted_at TEXT);
CREATE INDEX pull_reviews_issue ON pull_reviews(issue_id, submitted_at);
CREATE TABLE review_comments (
    id TEXT PRIMARY KEY, issue_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE,
    review_id TEXT NOT NULL REFERENCES pull_reviews(id) ON DELETE CASCADE,
    author_id TEXT NOT NULL REFERENCES users(id),
    path TEXT NOT NULL, side TEXT NOT NULL, line INTEGER NOT NULL,
    commit_oid TEXT NOT NULL, diff_hunk TEXT NOT NULL DEFAULT '',
    reply_to TEXT REFERENCES review_comments(id) ON DELETE CASCADE,
    body TEXT NOT NULL, outdated INTEGER NOT NULL DEFAULT 0, resolved INTEGER NOT NULL DEFAULT 0,
    resolved_by TEXT REFERENCES users(id),
    created_at TEXT NOT NULL, updated_at TEXT NOT NULL);
CREATE INDEX review_comments_issue ON review_comments(issue_id, path, created_at);
```
Issue events gain the kinds `review_dismissed` (data `{"review","reason"}`), `thread_resolved` and `thread_unresolved`.

## forge-core (`src/reviews.rs` model plus `store/reviews.rs`, an `impl Store` block)
```rust
pub enum ReviewState { Pending, Commented, Approved, ChangesRequested, Dismissed }   // as_str / FromStr
pub enum Side { Old, New }                                                          // "old" | "new"
pub struct Review { pub id: Id, pub issue_id: Id, pub author_id: Id, pub author_name: String, pub state: ReviewState,
    pub body: String, pub commit_oid: String, pub dismissed_reason: Option<String>, pub created_at: String, pub submitted_at: Option<String> }
pub struct ReviewComment { pub id: Id, pub review_id: Id, pub author_id: Id, pub author_name: String, pub path: String, pub side: Side,
    pub line: u32, pub commit_oid: String, pub diff_hunk: String, pub reply_to: Option<Id>, pub body: String,
    pub outdated: bool, pub resolved: bool, pub created_at: String, pub updated_at: String, pub pending: bool /* review is pending */ }
pub struct Thread { pub root: ReviewComment, pub replies: Vec<ReviewComment> }

// store/reviews.rs
pub async fn pending_review(&self, issue_id: &str, author_id: &str) -> Result<Option<Review>>;
pub async fn add_review_comment(&self, issue_id: &str, author_id: &str, pending: bool, path: &str, side: Side, line: u32,
    commit_oid: &str, diff_hunk: &str, body: &str) -> Result<ReviewComment>;
    // pending=true: put it in the author's pending review, creating that review if needed;
    // pending=false: create a submitted `commented` review holding just this comment
pub async fn reply_review_comment(&self, issue_id: &str, author_id: &str, root_id: &str, body: &str) -> Result<ReviewComment>;
    // root must be a submitted root comment of this PR; copies path/side/line/commit_oid
pub async fn submit_review(&self, issue_id: &str, author_id: &str, state: ReviewState /* not Pending/Dismissed */,
    body: &str, head_oid: &str) -> Result<Review>;
    // turns the pending review into `state` (or creates one with no comments); Commented with an empty body
    // and no comments is Invalid; the PR author's Approved or ChangesRequested is Forbidden
pub async fn discard_pending_review(&self, issue_id: &str, author_id: &str) -> Result<()>;
pub async fn dismiss_review(&self, review_id: &str, actor_id: &str, reason: &str) -> Result<()>;  // approved or changes_requested only; adds the event
pub async fn list_reviews(&self, issue_id: &str) -> Result<Vec<Review>>;                         // submitted ones, oldest first
pub async fn review_threads(&self, issue_id: &str, viewer_id: Option<&str>) -> Result<Vec<Thread>>;
    // submitted comments plus the viewer's own pending ones; replies oldest first
pub async fn review_comment(&self, id: &str) -> Result<Option<ReviewComment>>;
pub async fn edit_review_comment(&self, id: &str, body: &str) -> Result<()>;   // records edit_history target_kind 'review_comment'
pub async fn delete_review_comment(&self, id: &str) -> Result<()>;             // deleting a root deletes its replies
pub async fn set_thread_resolved(&self, root_id: &str, actor_id: &str, resolved: bool) -> Result<()>; // adds an event
pub async fn mark_outdated(&self, issue_id: &str, ids: &[Id]) -> Result<()>;
pub async fn review_summary(&self, issue_id: &str) -> Result<ReviewSummary>;
    // per reviewer, their latest non-commented, non-dismissed submitted state
pub struct ReviewSummary { pub approved_by: Vec<String>, pub changes_requested_by: Vec<String> }
```
Bodies use the same limits as issue comments. `author_name` comes from a join on `users`.

## forge-git (added to `src/pulls.rs`)
```rust
pub async fn blob_at(env: &GitEnv, repo: &Path, commit: &str, path: &str) -> Result<Option<String>>; // `rev-parse <commit>:<path>` gives the blob oid; None if absent. Validate path: no NUL, not absolute, no `..` segments
```

## Outdated marking (forge-web `pulls_sync.rs`, extending the head_updated path)
After `update_pull_head`, for each submitted root comment that isn't outdated, compare `blob_at(old commit_oid, path)` with `blob_at(new_head, path)`. Mark it outdated if they differ (cache by `(commit_oid, path)`).

## Web
- **Files tab** `/pulls/{n}/files`:
  - **Line anchors.** Each diff line gets an id `L-<file_index>-<side>-<line>` and a small "+" link to `?comment=<file_index>:<side>:<line>#L-…`. Use `file_index` (the file's position in the diff), not the path, to keep URLs short. The server maps it back to the path and re-validates it against the current diff.
  - **Inline comment form.** When `comment=` matches a line, render a form right under that line. It POSTs `/pulls/{n}/review_comments` with csrf, `file_index`, `side`, `line`, `head_oid` (rejected if stale: "the pull request changed, reload"), body, and two buttons: `action=single` ("Add single comment") or `action=review` ("Start a review" / "Add review comment" if one is pending).
  - **Threads.** Existing, non-outdated threads on the current diff render under their line: comments (markdown-rendered like issue comments), a reply form, a Resolve/Unresolve button (PR author or Write), and edit/delete for the author.
  - **Review bar** at the top for signed-in viewers: "Finish your review" with a pending comment count, a body textarea, radio comment/approve/request_changes (approve and request changes hidden for the PR author), Submit and Discard. Posts to `/pulls/{n}/reviews` or `/pulls/{n}/reviews/discard`.
  - The `diff_section` component gains an optional slot. Implement it as an optional `&HashMap<(usize, Side, u32), Vec<...>>` of rendered thread data plus an optional open-form target; keep `commit.rs` and `compare.rs` calls working, passing none.
- **Conversation timeline:** submitted reviews appear in created order:
  - Avatar/name with the verb ("approved these changes" / "requested changes" / "reviewed"), the body, then its threads, each showing the `diff_hunk` as a small code block, the path:line, an "Outdated" badge, comments and replies, and a "Resolved" collapsed marker using `<details>`.
  - Dismissed reviews show struck-through with the reason.
- **Merge box:** a summary line: "Approved by a, b", "Changes requested by c". Write holders see a "Dismiss" form for each approve or request-changes review, with a reason.
- **Routes** (all POST with CSRF, all redirecting 303 back to the referring tab with an anchor):
  - `/pulls/{n}/review_comments`
  - `/pulls/{n}/review_comments/{id}/reply`
  - `/pulls/{n}/review_comments/{id}/edit` (GET form + POST)
  - `/pulls/{n}/review_comments/{id}/delete`
  - `/pulls/{n}/review_comments/{id}/resolve` (with `resolved=1|0`)
  - `/pulls/{n}/reviews`, `/pulls/{n}/reviews/discard`, `/pulls/{n}/reviews/{id}/dismiss`
- **Permissions:**
  - Read and signed in to comment or review.
  - Author of the comment to edit or delete; Triage+ may also delete.
  - PR author or Write to resolve; Write to dismiss.
  - Archived repo: no writes. Merged or closed PR: comments and replies still allowed, new reviews not.