feat(daemon): persist team-collab comment anchor state - #5225
Conversation
Storage layer for the read-only comment drift ladder (C lane, spec D2), building on the contract from #5222. - preview_comments gains anchor_state / anchored_version / author_member_id / last_good_position_json. The migration runs after the slide-key table rebuild so a legacy rebuild cannot drop the new columns. - upsert persists creation metadata (anchoredVersion, authorMemberId); resolved state stays null for the drift ladder to fill in. - updatePreviewCommentAnchor is the engine write-back: COALESCE keeps the last-good position/version on a lost resolve, and it never bumps updated_at. - contract: anchoredVersion on PreviewCommentTarget, authorMemberId on the upsert request (server-set), and PreviewCommentAnchorUpdateRequest. Tests cover fresh-db columns, creation round-trip, engine write-back with lost-keeps-last-good, and legacy-migration backfill.
lefarcen
added
size/M
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@lefarcen Thanks for the focused storage-layer work here. I reviewed the migration order, the new read/write paths for anchor metadata, and the daemon coverage added around fresh DBs, legacy migration, and lost-anchor preservation. I found one small validation gap that looks follow-up sized rather than merge-blocking.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| id: string, | ||
| input: DbRow, | ||
| ) { | ||
| const anchorState = typeof input?.anchorState === 'string' ? input.anchorState : null; |
There was a problem hiding this comment.
Consider validating anchorState against the four contract states before writing it. The shared contract now exposes PreviewCommentAnchorState as anchored | reanchored | stale | lost, but this storage helper currently persists any string and normalizePreviewComment will then return that string as anchorState. If the follow-up engine or route sends a typo, the DB can store a value the web layer cannot safely switch on, which undermines the drift-ladder states this PR is setting up. A small PREVIEW_COMMENT_ANCHOR_STATES set, mirroring the nearby PREVIEW_COMMENT_STATUSES check in updatePreviewCommentStatus, would keep persisted rows aligned with the contract and make invalid write-backs fail fast.
|
Quick PR-body follow-up: the write-up is already clear overall, but could you switch the body to the repo's full Surface area checklist and add an explicit Validation section? The current notes are useful, but the standard checklist + validation heading make stacked branch review a lot easier to scan. |
2 participants
Why
Storage layer for the team-edition read-only comment drift ladder (C lane, OPEND-444, spec §D2), building on the anchor contract from #5222. The comment layer resolves each render through a ladder (anchored → reanchored → stale → lost) instead of today's silent exact-match; that resolution needs a durable home.
What users will see
Nothing yet — backend storage only (no route/UI wired). The drift-ladder engine (next PR) and UI consume it.
What changed
preview_commentsgainsanchor_state,anchored_version,author_member_id,last_good_position_json. Migration ALTERs run after the slide-key table rebuild so a legacy rebuild can't drop them.upsertPreviewCommentpersists creation metadata (anchoredVersion,authorMemberId); resolved state stays null for the engine.updatePreviewCommentAnchor= the engine write-back.COALESCEkeeps the last-good position/version on alostresolve; it never bumpsupdated_at(anchor resolution is a derived read, not an edit).anchoredVersiononPreviewCommentTarget,authorMemberIdon the upsert request (server-set from identity),PreviewCommentAnchorUpdateRequest.Surface area
packages/contracts)apps/daemon— storage + migration)Tests
apps/daemon/tests/comment-attachments.test.ts: fresh-db columns, creation round-trip, engine write-back + lost-keeps-last-good, legacy-migration backfill. 21/21 green; contracts + daemon typecheck green.Notes
Stacked on #5222 (merged). Series into
feat/workspace-team: PR1 contracts ✓ · PR2 storage (this) · PR3 xpath anchoring engine + drift ladder (web) · PR4 comment UI · PR5 sync trigger.