review/14 annotations store

Cargo.toml

@@ -1,7 +1,13 @@
 [workspace]
 resolver = "2"
 default-members = ["git-workon"]
-members = ["git-workon", "git-workon-lib", "git-workon-fixture", "git-workon-review"]
+members = [
+  "git-workon",
+  "git-workon-lib",
+  "git-workon-fixture",
+  "git-workon-review",
+  "git-workon-annotations",
+]
 
 [workspace.package]
 authors = ["Eric Eldredge <lettertwo@gmail.com>"]

docs/adr/039-review-annotations-substrate.md

@@ -0,0 +1,103 @@
+# 039: Annotations, One Substrate for Comments and Walkthroughs
+
+Status: accepted (2026-09-02 plan-mode interview)
+
+## Context
+
+The RFC (`docs/rfc/workon-review.md`) deferred two capabilities behind "the eventual payoff":
+review comments fed back to a coding agent via MCP, and a `git workon mcp` bridge to
+`git-workon-lib`'s worktree tools. Separately, the user wants an integrated version of the
+`/explain-diff` skill: an agent-authored walkthrough that steps a reviewer through a stack,
+changeset by changeset. Both need the same three things: a place to anchor content to a
+specific line (or a whole changeset) that survives the file changing underneath it, a way for
+an agent to write that content over MCP, and a way for the TUI to watch for and render it.
+
+Treating them as two features would mean two anchoring schemes, two stores, and two watchers
+for what is structurally the same problem: attach text to a resolved location in a changeset,
+author it from either the human or an agent, and keep the TUI's view live as the underlying
+diff moves.
+
+## Decision
+
+**One substrate, two uses.** `AnnotationKind::{Comment, TourStop, Chapter}` share one table,
+one anchoring scheme, and one store API. A walkthrough is annotations carrying a `tour` name
+and `seq` order; a chapter is per-changeset prose with no line anchor.
+
+**Content-hash context anchoring.** An anchor stores the target line's text plus up to 3
+context lines each way. It's re-resolved every load, not on write: exact match first (stored
+line number, text, and context all agree), then a windowed outward scan for the target text
+scored by how much surrounding context still matches, then a whitespace-tolerant repeat of
+that scan, else `Orphaned`. A failed resolution renders as "unanchored" (never silently
+wrong, never crashes the view). `Orphaned` is derived per load, not persisted: only `Open` and
+`Resolved` are real lifecycle states, so a discarded edit that restores the original content
+un-orphans the annotation for free, with nothing to reconcile.
+
+**sqlite store at `<commondir>/workon-review/annotations.db`.** `commondir`, not
+`repo.path()` (every worktree of a repo shares one store, the same discipline
+`git-workon-lib`'s graphite-metadata reader already uses for `.graphite_metadata.db`). WAL +
+`busy_timeout(3000)` on the writer; `SQLITE_OPEN_READ_ONLY | SQLITE_OPEN_NO_MUTEX` for a
+read-only handle. `rusqlite` (bundled) is already a workspace dependency, so this adds no new
+runtime.
+
+**New crate `git-workon-annotations`, `publish = false` + `[package.metadata.dist] dist =
+false`.** Same posture ADR-033 set for `git-workon-review`: a scaffold-stage crate stays out
+of release-plz's auto-publish and cargo-dist's auto-bin-inclusion until it's actually ready to
+ship, and `dist = false` is the explicit tripwire for that later flip so removing `publish =
+false` alone can't silently start shipping an undesigned binary. Dependencies: `rusqlite`,
+`thiserror`, `miette`. Serde-free (plain structs with rusqlite row mapping): the store is a
+lib, `git-workon-mcp` (ADR-040) is its second consumer, and the serde-free/git2-free posture
+exists so that consumer owns the JSON boundary. No `git2` dependency: the store takes a
+`commondir: &Path` the caller resolves, so this crate never needs to open a repository itself.
+
+**Why a new crate at all, when the RFC says "no separate core crate until a second consumer
+exists."** That condition is now met. `git workon mcp` is a second consumer of the same
+comment/annotation data the TUI reads and writes (a lib both `git-workon-review` and, via
+its MCP binary, `git-workon` need). This is exactly the fork the RFC's Agent-loop bullet left
+open. Putting the store in `git-workon-review`'s own lib would mean either the CLI depends on
+the review crate's whole diff/render surface just to reach a sqlite table, or the annotation
+code gets duplicated. A dedicated crate is the smaller dependency edge either way.
+
+**MCP lives in its own crate, `git-workon-mcp` (ADR-040), this store's second consumer.**
+This crate stays a lib with no bin target and no `rmcp`/tokio dependency; `git-workon-mcp`
+depends on it, owns the JSON boundary, and is reached from `git-workon` via the existing
+external-subcommand PATH dispatch (`git-workon/src/dispatch.rs`), not a compile-time
+dependency or a built-in `Cmd::Mcp`. ADR-040 covers the crate split, the publish-blocker
+reasoning for keeping it off the `git-workon` dependency graph, and the transport choice
+(`rmcp` 3.2, minimal features, current-thread tokio).
+
+**Gutter marker, not an edge glyph, for the TUI's annotation indicator (deferred to the read
+slice, recorded here for continuity).** Both content edges of a diff row are already claimed
+by horizontal-scroll affordances; the gutter's trailing space survives panning, so that's
+where the marker goes.
+
+## Consequences
+
+- Comments and walkthrough stops are the same row shape, so the TUI's marker index, overlay,
+  and store watcher are built once and serve both, instead of twice.
+- The anchoring scheme is a genuine trade: it can misplace an annotation on a heavily
+  rewritten line (context match is a heuristic, not a guarantee), but it never crashes or
+  silently attaches to the wrong line without saying so: an unresolved anchor always renders
+  as `Orphaned`, visibly.
+- The store is `rusqlite::Connection`, which is `!Sync`: every consumer (the TUI's
+  event-loop thread, each MCP tool call) must own or briefly borrow its own connection, and no
+  connection crosses a thread boundary as shared state.
+- `git-workon-annotations` has no bin target and never will: `git-workon-mcp` (ADR-040)
+  depends on it as a lib, the same relationship `git-workon-review`'s TUI has to it.
+- The initial-publish flip for this crate follows the same three steps ADR-033 lists for
+  `git-workon-review`: drop `publish = false`, add the crate to `release-plz.toml` with no
+  shared `version_group`, and decide `dist = false`'s fate deliberately rather than by
+  omission. `git-workon-mcp`'s own distribution (a second binary through cargo-dist and the
+  homebrew formula patch step) is a separate decision ADR-033 already flagged as unsolved for
+  a second binary generally; this ADR doesn't resolve it.
+
+## References
+
+- `docs/rfc/workon-review.md`: Agent-loop bullet and Comments decision row, updated
+  alongside this ADR
+- [ADR-008](008-error-handling-strategy.md): the two-layer error pattern this crate's
+  `error.rs` follows
+- [ADR-033](033-review-crate-workspace-placement.md): the `publish = false` / `dist = false`
+  scaffold posture this crate adopts, and the second-binary distribution gap it already
+  flagged
+- `git-workon-lib/src/stack/graphite.rs`: the `commondir`-based, `READ_ONLY | NO_MUTEX`
+  sqlite reader this store's open/open_read_only follow

docs/rfc/workon-review.md

@@ -15,7 +15,7 @@ It is the productization of a working Neovim prototype (`~/.config/nvim/lua/app/
 |---|---|
 | Positioning | Changeset review tool; not a lazygit competitor. Comments-to-agent is a first-class capability, not a stretch. **Reprioritized 2026-07-08 (direction B):** near-term goal is the author's own daily diff-review + git driver; the agent-loop/comments become the eventual payoff, not the next work. See "Roadmap reprioritized" under Milestones. |
 | Home | This workspace, as sibling crate `git-workon-review`. |
-| Crate layout | ONE crate, lib+bin targets. lib = review domain (diff parse, word-diff, staging, changeset views); bin = TUI + `mcp` subcommand. No separate core crate until a second consumer exists. |
+| Crate layout | `git-workon-review`: lib+bin targets, lib = review domain (diff parse, word-diff, staging, changeset views), bin = TUI. **Superseded 2026-09-02 (ADR-039):** the "no separate core crate until a second consumer exists" condition is now met — `git workon mcp` is a second consumer of the annotation store (see the Agent-loop bullet under Milestones). The store lives in its own crate, `git-workon-annotations`, `publish = false` like `git-workon-review` itself; `git workon mcp` is served from its own `git-workon-mcp` crate (ADR-040), reached from the published `git-workon` binary via the existing external-subcommand PATH dispatch rather than a compile-time dependency (the publish blocker: a published crate can't depend on a `publish = false` one). |
 | Name | Package == binary == `git-workon-review`. `git workon-review` works via git's native `git-*` dispatch. (`git-review` is squatted on crates.io + Gerrit-loaded; `docket` too docker-adjacent; bare `review` superseded by suite framing; `signoff` was the free runner-up.) |
 | `git-workon review` dispatch | `git-workon` adds cargo-style external-subcommand dispatch: unknown subcommand → exec `git-workon-<cmd>` on PATH, args passed through. |
 | NO `workon` binary | Deliberate: Python virtualenvwrapper keeps the `workon` name. Do not re-propose. |
@@ -26,7 +26,7 @@ It is the productization of a working Neovim prototype (`~/.config/nvim/lua/app/
 | Highlighting | tree-sitter (tree-sitter-highlight), syntect as long-tail fallback. Measured: ts ~0.01ms/line vs syntect ~0.19ms/line, and better output. Grammar set + gotchas are in the spike. |
 | View model | Full parity with the prototype's four zoom states (split/combined/unstaged/staged + attributed rendering). If v1 must shrink, cut zoom states — never the comments loop. |
 | v1 sources | uncommitted, stack, ref/range, **PR** — all folded into **the source-selector work ("review any source")** (PR was deferred; now first, via git-workon-lib's `pr.rs`). |
-| Comments | MCP: on-disk comment store (`.review/` JSON or sqlite) + `git-workon-review mcp` stdio subcommand serving get/resolve tools; TUI watches the store. Degrades to a plain file convention for non-MCP harnesses. |
+| Comments | **Superseded 2026-09-02 (ADR-039):** comments are one `AnnotationKind` in the shared annotations substrate (see the Agent-loop bullet under Milestones), not a standalone `.review/` store. sqlite at `<commondir>/workon-review/annotations.db`, content-hash context anchoring (re-resolved per load; a stale anchor renders `Orphaned`, never wrong), served over MCP by `git workon mcp` (`git-workon-mcp`, its own crate — ADR-040 — dispatched from `git-workon` over PATH, not a `git-workon-review mcp` subcommand). TUI watches the store via `PRAGMA data_version`. |
 | Edit flow | Embedded: `nvim --server $NVIM --remote +<line> <file>`. Standalone: `$EDITOR`. File watcher refreshes on save. |
 | Completions | Full clap_complete (unstable-dynamic, already a workspace dep) on the direct binary. Work item: git-workon's dynamic completer enumerates `git-workon-*` on PATH and delegates post-subcommand completion via `COMPLETE=<shell> git-workon-review -- <partial>`. Git-level shims: on demand only. |
 | Study first | `jjr` crate (agent jj-stack review surface), `triage-tui`, `wb300` — adjacent tools found during naming research. |
@@ -160,7 +160,7 @@ The remaining roadmap is resequenced around the tool being **the author's own ev
 
 - **Conflict resolution** *(stretch)*. Resolve merge/rebase conflicts in the SBS view. Large surface; may not make v1.
 
-- **Agent loop** *(the eventual north star)*. On-disk comment store keyed to `(changeset_id, path, side, lnum)` with a rebase-survival anchoring strategy + TUI comment UX (create/view/resolve, store-watch refresh), and a **unified `git workon mcp`** stdio server bridging git-workon-lib worktree tools (`agent-integration.md` Model C) *and* the comment store. **Open forks (unchanged, resolve at design time):** comment-store home — a lib both the review crate and `git-workon` depend on, since `git workon mcp` is a second consumer (reopens the "no separate core crate" decision); the anchoring strategy; MCP crate/transport (`rmcp` vs hand-rolled JSON-RPC-over-stdio). Deferred behind the daily-driver work.
+- **Agent loop** *(the eventual north star)*. **Design locked 2026-09-02 (ADR-039); the three open forks below are resolved, not just re-flagged.** One substrate serves both review comments and an integrated `/explain-diff`-style walkthrough: `AnnotationKind::{Comment, TourStop, Chapter}` in one sqlite table at `<commondir>/workon-review/annotations.db`, anchored by content-hash context (target line + 3 lines each way, re-resolved per load: exact match, then a scored windowed scan, then whitespace-tolerant, else `Orphaned` — never silently wrong) rather than the originally-proposed `(changeset_id, path, side, lnum)` key. **Comment-store home:** its own crate, `git-workon-annotations` (`publish = false`, serde-free, no git2) — `git workon mcp` is the second consumer the "no separate core crate" rule was waiting on, so that condition is now met and the rule no longer applies to it. **MCP crate/transport:** `rmcp` 3.2 (minimal features, current-thread tokio), not hand-rolled JSON-RPC, served from its own `git-workon-mcp` crate (ADR-040) and reached from `git-workon` via the existing PATH-dispatch mechanism (not a built-in `Cmd::Mcp`, and not a `git-workon-review mcp` subcommand) — this is what lets the published `git-workon` binary depend on the feature without depending on the unpublished annotations crate. Landing as five stacked slices (crate scaffold → TUI read → TUI authoring → prose/walkthrough polish → MCP crate); each lands alone. Still deferred behind the daily-driver work, but no longer blocked on open design questions.
 
 ## Orchestration notes
 

git-workon-annotations/Cargo.toml

@@ -0,0 +1,37 @@
+[package]
+authors.workspace = true
+categories = ["command-line-utilities", "development-tools"]
+description = "Anchored-annotation store shared by review comments and walkthroughs"
+edition.workspace = true
+homepage.workspace = true
+keywords = ["git", "review", "annotations", "workon"]
+license.workspace = true
+name = "git-workon-annotations"
+repository.workspace = true
+rust-version.workspace = true
+version = "0.1.0"
+include = [
+  "src/**/*",
+  "Cargo.toml",
+  "LICENSE*",
+]
+# Not yet published: this crate has no bin target (the MCP server is its own crate,
+# git-workon-mcp — ADR-040) and the store schema is still settling. Follow the ADR-033
+# posture: flipping to publish is a deferred sub-decision, not an oversight to fix later.
+publish = false
+
+[lib]
+name = "workon_annotations"
+
+[dependencies]
+miette.workspace = true
+rusqlite.workspace = true
+thiserror.workspace = true
+
+[package.metadata.dist]
+# Redundant with publish = false today; load-bearing once a bin target and the publish flip
+# land (see ADR-033's posture, adopted here) so cargo-dist doesn't silently start shipping it.
+dist = false
+
+[dev-dependencies]
+tempfile = "3"