feat(web): commentable prose diff with honest anchor state (spec-by6.3.3)
Reverses the diff view's founding rule. renderDocDiff skipped ChangeEqual
outright — "the review shows only what changed" — but any block of a proposed
document must be commentable, so unchanged blocks now render as collapsed,
dimmed context. Changed blocks keep their border, tint and full body, so the
page still reads as a diff at a glance rather than as a document dump. A
context block carrying a comment renders open. ChangeMoveIn now shows its text
too, since a comment control on invisible text is a control on nothing;
ChangeMoveOut stays a bare marker and is deliberately not commentable, because
the same paragraph is anchorable at its move-in position and two anchors for
one paragraph is the bug that avoids.
Every rendered block carries id="b-<16 hex>", hashed from the whole anchor
tuple. Not the page ordinal: an ordinal renumbers on any insertion above it, so
a saved link would silently scroll to a neighbouring paragraph, whereas
including the block hash makes a stale link resolve to nothing instead.
Threads are placed by anchor and by nothing else. Anything no rendered block
claims — an outdated anchor, an old-side anchor whose block the diff no longer
draws, a document the proposal no longer changes — is collected into a
page-level "comments that lost their anchor" area. Never dropped, never moved
onto a neighbour: a comment reads as authoritative about the block it sits
beside, so attaching it to the wrong one is worse than admitting it lost its
place. An edited anchor is drawn on its block and badged.
The comment form's anchor is built at submit time from the branch as it now
reads, through service.AnchorOf — hand-rolling the ordinal conversion here
would put the browser's comments on different blocks than the MCP tool's, which
is the one way two surfaces of one conversation disagree without either looking
broken. The form's block hash guards it: a block that moved while the page sat
open is a 409, not a comment attached to whatever took its place.
That hash is required rather than checked-when-present. Skipping the guard for
a form that omits it would let a later template refactor drop the hidden field
and disable the staleness check silently, with every test still green.
Authority is surfaced, not re-implemented: compose and resolve are the owner's
because service says so and ErrForbidden becomes a 403.
spec-by6.3.3
feat(service): review threads and the policy auto-merge gate (spec-by6.3.2)
The comment API above db/, plus the rule that gives comments teeth: an
unresolved review thread suppresses policy auto-merge.
The gate matters because tryAutoMerge runs on every Propose, including an
agent's revision of an open proposal — so a proposal the owner stopped to
comment on could otherwise land unattended on the agent's next push. It gates
policy merges only. MergeHuman does not consult it: clicking approve is the
judgement the thread was asking for, and a comment nobody got round to
resolving must not be able to wedge a proposal shut.
Authority follows from that. The owner opens and resolves threads; an agent may
reply but may do neither, because both would hand the gate's control to the
thing it exists to hold back. An agent reply is not a resolution — answering a
critique is not the owner accepting the answer.
AnchorThreads lives here rather than in each surface, for the reason
Service.Archive does: the review page and the MCP tool must agree about whether
a comment still fits, and two surfaces each segmenting and matching would agree
only until one was changed. That is why service/ now imports prosediff. Each
document is segmented at most once per side however many threads hang off it.
AnchorOf is the other half: a surface offering "comment on this block" has a
document-global block ordinal, and the anchor needs the index within the block's
own heading path. Converting in one place is what stops the web form and the MCP
tool numbering blocks differently and putting their comments on different
blocks of the same document. A test asserts the two agree for every block of a
document.
A thread whose document is no longer among the proposal's changes — the agent
reverted it — is outdated, not dropped: a comment that silently vanished would
look like one that was never made.
Tested against a real Postgres 16, with a control test proving policy
auto-merge still fires without a thread, so the gate test cannot pass for the
wrong reason.
spec-by6.3.2
feat(core,db): comment anchors and the comment table (spec-by6.3.1)
DESIGN.md held the comment schema back until the review UI existed, because a
schema is the expensive thing to get wrong here. The UI exists, so this lands
the anchoring model it specifies: (doc_id, heading_path, block_index,
block_hash), resolved content-first — the block's hash wherever it moved to,
then position under the same headings, and outdated when neither matches. A
comment that lost its place says so rather than being relocated to a best
guess, because a comment on the wrong paragraph reads as authoritative.
Two refinements the specified model left open:
block_index is the block's position WITHIN its heading path, not within the
document. prosediff numbers blocks document-globally, but the positional
fallback exists precisely for blocks whose content changed, and a global index
is destroyed by any insertion above it — so a global index would fail exactly
when it is needed.
A hash match is not unique. Documents repeat themselves ("TBD" under half the
headings), so the comment's own section wins outright and the nearest index
breaks what that does not. Without it, which duplicate a comment landed on
would depend on document order.
Anchor state is derived, never stored. A comment is not outdated in general, it
is outdated at a revision, and the proposal branch moves under it as the agent
revises; a column would cache a function of a moving input.
The table encodes the rest as constraints: a root carries the whole anchor and
a reply carries none of it (one predicate over all six columns, so a
half-written anchor is unwritable), agent provenance on proposal's rule, and
resolution only on a thread root.
Verified against a real Postgres 16, which caught what Go-level validation did
not: pq.Array sends a nil slice as SQL NULL, so a comment on a block before the
first heading tripped ck_comment_anchor and every document preamble was
uncommentable.
schema.sql keeps the comment table last, matching migration order — the
agreement test compares the two statement for statement. Its absent-list, which
required a design change rather than a quiet migration to add this table, is
the gate this commit passes through.
spec-by6.3.1
refactor(doc): one route from a revision to an Archive (spec-wcr #2, #4)
doc.Scan/DocumentSource were production-dead after service.Archive landed —
their only consumer was doc's own test fixture — and they were the seam that
made web's layering violation writable: a surface could reach past service/
into gitx and build its own archive. Deleted, so doc/ now owns no way to read
a revision and service.Archive is the single route from rev to Archive. The
fixture reads through ListDocuments + FromDocuments instead.
TestScanReportsGitErrors covered an unknown revision failing rather than
walking empty, which gitx did not test itself. Relocated there as
TestWalkOfAnUnknownRevisionFails rather than dropped.
linkHierarchy passed path.Dir(p.Path) where every other call site passes DirOf.
Unobservable today: the two differ only at the space root, and there the
section-proximity step is subsumed by the same-directory step above it, so "."
only ever skipped a lookup that had already answered. Verified by reverting and
re-running. Changed anyway — it stays unobservable only by coincidence of two
ranking rules — with a test pinning the invariant that `parent:` resolves to
whatever the same wikilink in the body resolves to.
spec-wcr
chore(beads): enable Dolt auto-push to dolt.srht.bigb.es
Beads changes were only reaching the remote on a manual bd dolt push, so
issue state drifted from the tracker between sessions. bd has native
auto-push (debounced, pushes to the configured origin remote); it was
simply never enabled — sync.remote was set but dolt.auto-push was unset.
Enable it with an explicit 5m debounce rather than relying on the implicit
default. Verified end-to-end: a write fires "dolt auto-push: pushed
successfully", and a fresh dolt clone of the remote shows this session's
closed beads (spec-jjo, spec-mfm, spec-ejq.1) as closed.
Also commits the JSONL exports, which were stale from this session's work.
chore(beads): track the spec.sr.ht issue backlog
Seven issues under the epic: Phases 3-5 (write plane -> review UI ->
deferred pile, chained by blocks), the loose-ends bucket of latent bugs,
and two ops tasks (nav restart, stale apk pins). Exported to a
git-tracked JSONL so the backlog is durable and portable, not only in the
local embedded Dolt DB.
bd init: initialize beads issue tracking