@@ 0,0 1,256 @@
# 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 — `authz.ForContext(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`),
`authz/identity.go` (`ForContext`), `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`.