specs/015-gerrit-style-patchsets/tasks.md

Ref: main   Size: 13.6 KiB

# Tasks: Patch Revisions

**Input**: Design documents from `/specs/015-gerrit-style-patchsets/`
**Prerequisites**: plan.md, spec.md, data-model.md, contracts/cli.md

**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story.

## Format: `[ID] [P?] [Story] Description`

- **[P]**: Can run in parallel (different files, no dependencies)
- **[Story]**: Which user story this task belongs to (e.g., US1, US2, US3)
- Include exact file paths in descriptions

---

## Phase 1: Foundational — Event Schema & State Materialization

**Purpose**: Core data model changes that ALL user stories depend on. Must complete before any story work begins.

**⚠️ CRITICAL**: No user story work can begin until this phase is complete.

<!-- parallel-group: 1 (max 3 concurrent) -->
- [x] T001 [P] Add `PatchRevision` variant to `Action` enum in `src/event.rs`: fields `commit: String`, `tree: String`, `body: Option<String>`, serialized as `"patch.revision"`. Add required `commit: String` and `tree: String` fields to `Action::PatchCreate`. Add required `revision: u32` field to `Action::PatchInlineComment` and `Action::PatchReview`. Remove the `PatchRevise` variant entirely.
- [x] T002 [P] Add `Revision` struct to `src/state.rs`: `{ number: u32, commit: String, tree: String, body: Option<String>, timestamp: String }` with Serialize/Deserialize. Add `revisions: Vec<Revision>` field to `PatchState`.
- [x] T003 [P] Add `revision: u32` field to `InlineComment` struct and `Review` struct in `src/state.rs`.

<!-- sequential -->
- [x] T004 Update `PatchState::from_ref_uncached()` materialization in `src/state.rs`: handle `Action::PatchCreate` to extract `commit`/`tree` into revision 1. Handle `Action::PatchRevision` to append revisions with dedup by commit OID (skip if commit already seen). Derive revision numbers from insertion order (1-indexed). Handle `Action::PatchInlineComment` and `Action::PatchReview` to store the `revision` field on `InlineComment`/`Review` structs. Remove the `Action::PatchRevise` match arm.

**Checkpoint**: Event schema and state materialization support revisions. Existing tests updated for new required fields.

---

## Phase 2: User Story 1 — Automatic Revision Recording (Priority: P1) 🎯 MVP

**Goal**: Write operations auto-detect branch changes and insert PatchRevision events before their own event.

**Independent Test**: Create a patch, push a commit, run `patch comment`, verify a PatchRevision event was auto-inserted before the comment.

<!-- sequential -->
- [x] T005 [US1] Implement `auto_detect_revision()` helper in `src/patch.rs`: takes `repo`, `ref_name`, `&PatchState`, and `&SigningKey`. Resolves current branch tip via `resolve_head()`. Compares to last revision's commit OID in `state.revisions`. If different, appends a `PatchRevision` event (commit OID + tree OID from `repo.find_commit(tip)?.tree()?.id()`). Returns the current revision number (u32). If branch is deleted or resolve fails, returns last known revision number without inserting. Note: auto-detect runs for ALL write ops including thread comments (records revision boundary), even though thread comments don't get a revision field.
- [x] T006 [US1] Modify `comment()` in `src/patch.rs` to call `auto_detect_revision()` before appending the comment event. Set the `revision` field on `PatchInlineComment` to the returned revision number. Thread comments (`PatchComment`) do not get a revision field — no change needed there.
- [x] T007 [US1] Modify `review()` in `src/patch.rs` to call `auto_detect_revision()` before appending the review event. Set the `revision` field on `PatchReview` to the returned revision number.
- [x] T008 [US1] Modify `merge()` in `src/patch.rs` to call `auto_detect_revision()` before appending the merge event (no revision field on PatchMerge, but the revision should be recorded if branch changed).
- [x] T009 [US1] Modify `create()` in `src/patch.rs` to populate `commit` and `tree` fields on `PatchCreate` event: resolve the branch tip commit OID and its tree OID.
- [x] T010 [US1] Rewrite `revise()` in `src/patch.rs`: create a `PatchRevision` event with commit OID, tree OID, and optional body. Compare branch tip to last recorded revision — reject with error "no changes since revision N" if unchanged.
- [x] T010a [US1] Wire Phase 2 dispatch in `src/lib.rs`: update `PatchCmd::Revise` dispatch to call the rewritten `revise()`. Ensure `PatchCmd::Comment`, `PatchCmd::Review`, and `PatchCmd::Merge` dispatch passes through to the modified functions that now call `auto_detect_revision()`.

**Checkpoint**: `patch create` records revision 1. `patch comment`, `patch review`, `patch merge` auto-detect branch changes. `patch revise` creates explicit revisions.

---

## Phase 3: User Story 2 — Comments Anchored to Revisions (Priority: P1)

**Goal**: Inline comments are anchored to specific revisions. `patch show --revision N` filters by revision.

**Independent Test**: Create a patch, comment on revision 1, push a commit, comment on revision 2, verify `patch show --revision 1` shows only revision 1's comments.

<!-- sequential -->
- [x] T011 [US2] Add `--revision` optional arg (type `Option<u32>`) to `PatchCmd::Comment` in `src/cli.rs`. Validate: if `--revision` is provided with `--file`/`--line` (inline comment), accept it. If `--revision` is provided without `--file`/`--line` (thread comment), reject with error "thread comments are not revision-scoped".
- [x] T012 [US2] Modify `comment()` in `src/patch.rs` to accept an optional `target_revision: Option<u32>` parameter. If provided, validate it exists in `state.revisions` (error if not). Use it instead of the auto-detected current revision for the `revision` field on the inline comment event.
- [x] T013 [US2] Add `--revision` optional arg (type `Option<u32>`) to `PatchCmd::Show` in `src/cli.rs`.
- [x] T014 [US2] Modify patch show output in `src/lib.rs` (or wherever show is dispatched): when `--revision N` is provided, filter `inline_comments` and `reviews` to only those with `revision == Some(N)`. Thread comments are always shown. When no `--revision`, group inline comments/reviews by revision number in output. Display "(revision: unknown)" for comments with `revision: None`.
- [x] T015 [US2] Update `show_json()` in `src/patch.rs` to include `revisions` array in JSON output. Ensure `InlineComment` and `Review` structs serialize their `revision` field.
- [x] T015a [US2] Wire Phase 3 dispatch in `src/lib.rs`: pass `--revision` arg through to `comment()` and show output. List revisions in `patch show` output.

**Checkpoint**: Inline comments anchored to revisions. `patch show --revision N` filters correctly.

---

## Phase 4: User Story 3 — Interdiff Between Revisions (Priority: P1)

**Goal**: `patch diff --between N M` shows interdiff. `patch diff --revision N` shows historical diff.

**Independent Test**: Create a patch with 3 revisions modifying different files, run `patch diff --between 1 3`, verify diff shows only changes between those snapshots.

<!-- sequential -->
- [x] T016 [US3] Implement `interdiff()` function in `src/patch.rs`: takes `repo`, `&PatchState`, `from_rev: u32`, `to_rev: u32`. Looks up tree OIDs from `state.revisions` for both revision numbers (error if not found). Computes `repo.diff_tree_to_tree(from_tree, to_tree, None)` and formats as unified diff (reuse the existing diff formatting logic from `generate_diff`).
- [x] T017 [US3] Modify `generate_diff()` in `src/patch.rs` to accept an optional `revision: Option<u32>` parameter. If provided, use that revision's tree OID (from `state.revisions`) instead of resolving the current branch tip. Merge-base still computed against the base branch.
- [x] T018 [US3] Add `--revision` optional arg (type `Option<u32>`) and `--between` arg (type `Option<Vec<u32>>`, 1-2 values) to `PatchCmd::Diff` in `src/cli.rs`. Validate: `--revision` and `--between` are mutually exclusive (error if both). `--between` with single value N means "N to latest".
- [x] T019 [US3] Wire Phase 4 dispatch in `src/lib.rs`: if `--between` provided, call `interdiff()`. If `--revision` provided, call `generate_diff()` with the revision parameter. If neither, call `generate_diff()` without revision (existing behavior).

**Checkpoint**: Interdiff works between any two revisions. Historical diff shows a specific revision against the base.

---

## Phase 5: User Story 4 — Revision Log (Priority: P2)

**Goal**: `patch log <id>` lists all revisions with timestamps and file-change summaries.

**Independent Test**: Create a patch with 3 revisions, run `patch log`, verify all 3 listed with correct data.

<!-- sequential -->
- [x] T020 [US4] Implement `patch_log()` function in `src/patch.rs`: takes `repo` and `id_prefix`. Materializes PatchState, iterates `state.revisions`. For each consecutive pair, computes file-change summary by diffing their tree OIDs (`repo.diff_tree_to_tree`). Returns structured data (Vec of revision info with file stats).
- [x] T021 [US4] Implement `patch_log_to_writer()` in `src/patch.rs` for human-readable output: format each revision as `Revision N  <short_oid>  <timestamp>  <files_summary>  "<body>"`. First revision gets "(initial)" label. Implement `patch_log_json()` for JSON output.
- [x] T022 [US4] Add `PatchCmd::Log` variant to `src/cli.rs` with `id: String` and `--json` flag. Wire Phase 5 dispatch in `src/lib.rs` to call `patch_log_to_writer()` or `patch_log_json()`.

**Checkpoint**: `patch log` shows revision history with file-change summaries.

---

## Phase 6: User Story 5 — Reviews Anchored to Revisions (Priority: P2)

**Goal**: Reviews are anchored to revisions via `--revision N`. Configurable merge policy checks approval on latest revision.

**Independent Test**: Approve on revision 1, push new code (revision 2), verify `patch show` displays "approved (revision 1)".

<!-- sequential -->
- [x] T023 [US5] Add `--revision` optional arg (type `Option<u32>`) to `PatchCmd::Review` in `src/cli.rs`.
- [x] T024 [US5] Modify `review()` in `src/patch.rs` to accept an optional `target_revision: Option<u32>` parameter. If provided, validate it exists in `state.revisions` (error if not). Use it instead of the auto-detected revision for the `revision` field on the review event.
- [x] T025 [US5] Update review display in patch show output: show review verdict with revision context, e.g., "approved (revision 3)". Highlight latest review per reviewer.
- [x] T026 [US5] Implement merge policy config reader: read `merge.require_approval_on_latest` from `refs/collab/config` (JSON blob). Default to `false` if config ref doesn't exist or key is absent.
- [x] T027 [US5] Modify `merge()` in `src/patch.rs`: after existing checks, if `merge.require_approval_on_latest` is true, verify at least one `Approve` review has `revision == latest_revision_number`. Error if not: "merge requires approval on the latest revision (revision N)".

**Checkpoint**: Reviews show revision context. Merge policy enforces latest-revision approval when configured.

---

## Phase 7: Polish & Cross-Cutting Concerns

**Purpose**: TUI updates, backward compat verification, dispatch wiring

<!-- parallel-group: 1 (max 2 concurrent) -->
- [x] T028 [P] Update `src/tui/widgets.rs`: show revision count (e.g., "r3") alongside patch status in the patch list view. In patch detail view, show revision list and annotate inline comments with their revision number.
- [x] T029 [P] Create `tests/revision_test.rs`: integration tests for revision recording (auto-detect on comment/review/merge), dedup by commit OID, revision numbering from DAG order, interdiff between revisions, anchored comments/reviews, `patch revise` rejection when unchanged, `--between N` single-arg shorthand, sync round-trip for PatchRevision events.

---

## Dependencies & Execution Order

### Phase Dependencies

- **Phase 1 (Foundational)**: No dependencies — start immediately. BLOCKS all user stories.
- **Phase 2 (US1)**: Depends on Phase 1. BLOCKS US2, US5 (they need auto-detect and revision numbers).
- **Phase 3 (US2)**: Depends on Phase 2 (needs revision numbers on comments).
- **Phase 4 (US3)**: Depends on Phase 1 only (needs revision data in state, not auto-detect).
- **Phase 5 (US4)**: Depends on Phase 1 only (needs revision data in state).
- **Phase 6 (US5)**: Depends on Phase 2 (needs revision numbers on reviews).
- **Phase 7 (Polish)**: Depends on all user story phases.

### User Story Dependencies

- **US1 (P1)**: Can start after Phase 1 — no dependencies on other stories
- **US2 (P1)**: Depends on US1 (needs auto-detect for revision numbers)
- **US3 (P1)**: Can start after Phase 1 — independent (only needs revision data in state)
- **US4 (P2)**: Can start after Phase 1 — independent (only needs revision data in state)
- **US5 (P2)**: Depends on US1 (needs auto-detect for revision numbers on reviews)

### Parallel Opportunities

After Phase 1 completes:
- US1 can start immediately
- US3 and US4 can start in parallel with US1 (they don't need auto-detect)

After US1 completes:
- US2 and US5 can start in parallel

---

## Implementation Strategy

### MVP First (US1 Only)

1. Complete Phase 1: Foundational (event schema + state)
2. Complete Phase 2: US1 (auto-detect + revise)
3. **STOP and VALIDATE**: Create a patch, push commits, verify revisions are recorded

### Incremental Delivery

1. Phase 1 → Foundation ready
2. US1 → Revisions are recorded (MVP!)
3. US3 + US4 (parallel) → Interdiff + revision log
4. US2 → Comments anchored to revisions
5. US5 → Reviews anchored + merge policy
6. Phase 7 → TUI + tests + polish

---

## Notes

- [P] tasks = different files, no dependencies
- [Story] label maps task to specific user story for traceability
- Most tasks touch `src/patch.rs` and `src/cli.rs` so cross-story parallelism is limited
- US3 and US4 are the best candidates for parallel execution (independent of US1)