# Spec: inline code comments on diffs

Status: **draft / proposal** — not yet implemented.
Author: bigbes. Last updated: 2026-07-18.

## 1. Motivation

The reference diff viewers (GitHub, the `diffshub` design we mirror) let a viewer
attach a comment to a specific line of a diff and reply in a thread. compare.sr.ht
renders commit and compare (`base...head`) diffs but has no way to discuss them
in place. SourceHut's native review flow is patch-over-mailing-list
(`lists.sr.ht`); inline comments are a **net-new capability** for the instance,
not something we can federate from an existing service.

This document specifies adding line-anchored comment threads to the commit and
compare pages.

## 2. Key constraint: the service is stateless *by choice*

compare.sr.ht owns no persistence today. `cmd/comparesrht/main.go` spells it out:

> it deliberately does NOT call `WithDefaultMiddleware`: compare.sr.ht owns no
> Postgres or Redis and must serve anonymous viewers

Crucially this is a **choice, not a missing capability**. The shared
`sr-ht-core` library already ships the storage layer the rest of the fleet uses,
and it is already in our build (as an indirect dependency: `github.com/lib/pq`):

- `sr-ht-core/database` — connection-pool context middleware
  (`Middleware`, `DBForContext`, `ForContext`), transaction helpers
  (`WithTx`, `WithReadOnlyTx`), and a squirrel-based query builder with a
  `Model`/`Scan` layer (`sq.go`, `ql.go`).
- `sr-ht-core/server` `WithDefaultMiddleware()` opens the pool from a
  `[compare.sr.ht]::connection-string` key via `sql.Open("postgres", …)`.

So the storage foundation is sitting unused. Adding comments means **turning that
path back on** for the comment routes while keeping anonymous, DB-free browsing
of diffs. The datastore is **Postgres** (the instance already runs it for
`git.sr.ht`/`meta.sr.ht`) — not SQLite.

### What flipping storage on costs us

Adding a DB makes the service no longer stateless. Downstream updates required:

- `README.md` and the `server.go` package doc ("owns no state of its own").
- The `main.go` "no database" comment and middleware group.
- `contrib/compare-srht.service` — add a Postgres ordering dependency.
- Ops: the service now has backup/restore concerns and a schema to migrate.

## 3. Anchoring model (the crux)

A comment must pin to a place in a diff and survive re-rendering. `@pierre/diffs`
gives us the exact primitive: a `DiffLineAnnotation` is anchored by

```ts
type DiffLineAnnotation<T> = { side: 'additions' | 'deletions'; lineNumber: number } & { metadata?: T }
```

So the natural per-file anchor is **`(file_path, side, line_number)`**, where
`side` selects the new-file column (`additions`) or old-file column
(`deletions`) and `line_number` is 1-based within that side. That maps 1:1 to a
DB row and to the widget the library renders.

The harder question is *which diff* the anchor lives in:

- **Commit page** (`/commit/{rev}`): the diff is immutable — a fixed commit SHA
  vs. its first parent. Anchor = `(commit_sha, file_path, side, line_number)`.
  Stable forever. **This is the easy, correct MVP surface.**
- **Compare page** (`/compare/{base}...{head}`): `base`/`head` are usually
  branch refs that *move*. Anchoring to symbolic ref names is fragile (a push
  silently re-targets the comment). Resolve refs to SHAs at comment time and
  anchor to `(base_sha, head_sha, three_dot, file, side, line)`. When the refs
  later move, show the thread as **outdated** (like GitHub) rather than
  silently re-hanging it on unrelated lines.

**Recommendation:** ship commit-page comments first (immutable anchor, no
outdated-state machine), add compare-range comments in a second phase.

## 4. Data model

One table, self-threading via `thread_root`.

```sql
CREATE TABLE comment (
  id           BIGSERIAL   PRIMARY KEY,
  created      TIMESTAMPTZ NOT NULL DEFAULT now(),
  updated      TIMESTAMPTZ NOT NULL DEFAULT now(),
  author       TEXT        NOT NULL,          -- meta.sr.ht canonical username

  -- repository (from authz.RepoInfo + route params)
  repo_id      INTEGER     NOT NULL,          -- git.sr.ht repository id
  owner        TEXT        NOT NULL,          -- ~owner  (for routing / listing)
  repo_name    TEXT        NOT NULL,

  -- anchor
  target_kind  TEXT        NOT NULL,          -- 'commit' | 'compare'
  commit_sha   TEXT,                          -- target_kind='commit'
  base_sha     TEXT,                          -- target_kind='compare'
  head_sha     TEXT,
  three_dot    BOOLEAN,                       -- compare: '...' vs '..'
  file_path    TEXT        NOT NULL,          -- repo path, no a//b/ prefix
  side         TEXT        NOT NULL,          -- 'additions' | 'deletions'
  line_number  INTEGER     NOT NULL,          -- 1-based, within side

  -- threading + content
  thread_root  BIGINT      REFERENCES comment(id) ON DELETE CASCADE, -- NULL => root
  body         TEXT        NOT NULL,          -- markdown source
  resolved     BOOLEAN     NOT NULL DEFAULT false
);

CREATE INDEX comment_commit_idx  ON comment (repo_id, commit_sha);
CREATE INDEX comment_compare_idx ON comment (repo_id, base_sha, head_sha);
```

A **thread** is a root comment (`thread_root IS NULL`) plus its replies. A root
carries the anchor; replies inherit it (denormalize the anchor onto replies too,
or read it from the root — denormalizing keeps list queries single-table).
`resolved` is a thread-level flag stored on the root.

## 5. Authorization

Reuse the existing seams — do **not** invent a second authz path.

- **Read**: identical to page authz. Every handler already calls `s.resolve()`
  (`authz.Authorizer.Repo`), which 404s a repo the viewer cannot see (private
  existence never leaks). Comments for a repo are only ever returned to a viewer
  who passed that check.
- **Write**: require an authenticated viewer — `login.FromContext(ctx) != ""`.
  For the MVP, **any authenticated viewer who can read the repo may comment**
  (open code review). Edit/delete restricted to the comment's `author`; resolve
  allowed to the thread author or comment author.
- **Repo-member-only gating (optional, later)**: `authz.RepoInfo` currently
  carries no ACL/write-access (`ID, Name, Description, Visibility` only). To gate
  writes to repo members, extend the GraphQL query in `authz/authz.go` to fetch
  the viewer's access level and add it to `RepoInfo`.

**CSRF**: writes are cookie-authenticated (the unified-login cookie), so mutation
endpoints need CSRF protection — issue a per-session token into the page and
require it on POST/PATCH/DELETE (or enforce a custom header + `SameSite` cookie).

## 6. HTTP API

REST/JSON, mounted alongside the current routes in `web/router.go`. All routes
authorize via `s.resolve()` first; mutations additionally require a logged-in
viewer and a valid CSRF token.

```
GET    /~{owner}/{repo}/commit/{rev}/comments        list threads for a commit
POST   /~{owner}/{repo}/commit/{rev}/comments        create root comment or reply
GET    /~{owner}/{repo}/compare/{spec}/comments      list threads for a compare   (phase 2)
POST   /~{owner}/{repo}/compare/{spec}/comments      create                        (phase 2)
PATCH  /~{owner}/{repo}/comments/{id}                 edit body / toggle resolved
DELETE /~{owner}/{repo}/comments/{id}                 delete (author only)
```

`POST` body: `{ file_path, side, line_number, body, thread_root? }`.

### Initial load: embed, don't round-trip

The page is already SSR + one embedded JSON blob (`#compare-data`) and the design
ethos (see `web/server.go`) is "single request, no second authorization
round-trip." Follow it: the commit/compare handlers query the DB and include the
threads in the existing `compareData` payload, so the first paint already shows
comments. The `GET …/comments` endpoint exists for re-fetch after a mutation; the
`POST/PATCH/DELETE` endpoints drive mutations (optimistic update, then reconcile).

## 7. Frontend integration (`frontend/src/app.ts`)

`@pierre/diffs` `FileDiff` already supports comment threads as first-class UI —
no new rendering library:

1. **Render existing threads.** Build `DiffLineAnnotation[]` from the embedded
   threads (`{ side, lineNumber, metadata: thread }`) and pass them as
   `lineAnnotations` to `FileDiff.render()`.
2. **Render the widget.** Provide `renderAnnotation(annotation)` in
   `FileDiffOptions` returning the thread DOM (comment list + reply box +
   resolve/edit/delete controls).
3. **Start a new thread.** Set `onDiffLineClick` (from
   `InteractionManagerBaseOptions<'diff'>`) — its props give `{ side, lineNumber }`
   — to inject a transient "new comment" annotation with a composer.
4. **Persist.** On submit, `POST` to the API; on success, replace the transient
   annotation with the saved thread and `rerender()` the instance. The layout
   toggle path (`wireLayoutToggle`) must preserve annotations across split↔unified
   re-renders.

Comment bodies are Markdown → **sanitize** on render (or render server-side to
sanitized HTML). Never inject raw comment HTML.

## 8. Config & wiring

New key:

```ini
[compare.sr.ht]
connection-string=postgres://compare:secret@localhost/compare?sslmode=disable
```

In `cmd/comparesrht/main.go`, open the pool once at startup and add
`database.Middleware(db)` to the existing anonymous `Group` (keeping anonymous
browsing intact — the middleware only injects the pool; only comment handlers
touch it):

```go
db, err := sql.Open("postgres", connString)   // from [compare.sr.ht] connection-string
// …
r.Use(database.Middleware(db))                 // added alongside config + authz middleware
```

Handlers use `database.WithReadOnlyTx` / `WithTx` + the `sq` query builder. Add
`connection-string` to `validateConfig`'s required-key set so a misconfiguration
fails loudly at startup, consistent with the existing validation.

## 9. Schema management

`sr-ht-core/database` provides the connection and query layers but **no migration
runner**. Options, cheapest first:

- Ship `schema.sql` and a `migrations/` dir; apply with a `comparesrht -migrate`
  subcommand run on deploy (self-contained, no new dependency).
- Adopt a Go migration library (e.g. `golang-migrate`) if versioned migrations
  become worth the dependency.

Confirm how the sibling Go services on this instance manage schema and match them.

## 10. Out of scope (for now)

- Email / webhook notifications on new comments (natural follow-up; `sr-ht-core`
  has `email` and `webhooks` packages if we want them later).
- Reactions, attachments, suggested-edit ("commit suggestion") blocks.
- Cross-diff thread migration on rebase beyond the "outdated" marker.

## 11. Phasing

1. **Phase 1 — commit comments.** Schema + DB wiring + immutable-anchor threads
   on `/commit/{rev}`; threads embedded in the page JSON; `POST/PATCH/DELETE`
   REST; `renderAnnotation` + `onDiffLineClick` in the bundle. Auth: any
   authenticated viewer who can read the repo. Edit/delete: author only.
2. **Phase 2 — compare comments.** SHA-pinned anchors on
   `/compare/{base}...{head}`, with an **outdated** state when refs move.
3. **Phase 3 — polish.** Resolve/unresolve UX, Markdown rendering, notifications,
   optional repo-member-only gating (extend `RepoInfo` with access level).

## Appendix: grounding references

- Storage layer: `sr-ht-core/database/{middleware,sq,ql}.go`;
  pool wiring `sr-ht-core/server/server.go` (`WithDefaultMiddleware`,
  `connection-string`).
- Opt-out today: `cmd/comparesrht/main.go` (middleware `Group`, `validateConfig`).
- Authz seams: `authz/authz.go` (`Authorizer`, `RepoInfo`),
  `sr-ht-ecore/login` (`Optional`, `FromContext`), `web/handlers.go`
  (`s.resolve`).
- Routes / SSR contract: `web/router.go` (`Register`), `web/server.go`,
  `web/handlers.go` (`buildCompareJSON`, `#compare-data`).
- Diff library API: `@pierre/diffs` `FileDiffOptions.renderAnnotation`,
  `FileDiffRenderProps.lineAnnotations`, `DiffLineAnnotation` (`side` +
  `lineNumber`), `InteractionManager` `onDiffLineClick` /
  `OnDiffLineClickProps`.
