feat(review): the pin follows the lock

apps/server/api/openapi.yaml

@@ -1369,6 +1369,18 @@ paths:
         same before and after.
       tags:
         - reviews
+      requestBody:
+        required: false
+        content:
+          application/json:
+            schema:
+              type: object
+              properties:
+                fresh:
+                  type: boolean
+                  description: |
+                    Opening the page (true) re-stamps the hold onto the latest
+                    edition; the heartbeat (false, the default) keeps the bytes.
       responses:
         '200':
           description: The caller holds the case.

apps/server/api/src/paths/case-lock.yaml

@@ -19,6 +19,18 @@ post:
     answers 423, naming them. Occupancy is never a state — the case reads the
     same before and after.
   tags: [reviews]
+  requestBody:
+    required: false
+    content:
+      application/json:
+        schema:
+          type: object
+          properties:
+            fresh:
+              type: boolean
+              description: |
+                Opening the page (true) re-stamps the hold onto the latest
+                edition; the heartbeat (false, the default) keeps the bytes.
   responses:
     "200":
       description: The caller holds the case.

apps/server/db/migrations/00022_the_pin_follows_the_lock.sql

@@ -0,0 +1,14 @@
+-- +goose Up
+-- The displayed edition is derived, never stored (ADR 0024): a live lock
+-- pins the bytes stamped at claim, a free case reads at the latest. The
+-- stored pin and its maintenance go.
+ALTER TABLE case_locks
+    ADD COLUMN edition_id text REFERENCES editions(id) ON DELETE SET NULL;
+DROP INDEX cases_current_edition_idx;
+ALTER TABLE cases DROP COLUMN current_edition_id;
+
+-- +goose Down
+ALTER TABLE cases
+    ADD COLUMN current_edition_id text REFERENCES editions(id) ON DELETE SET NULL;
+CREATE INDEX cases_current_edition_idx ON cases (current_edition_id);
+ALTER TABLE case_locks DROP COLUMN edition_id;

apps/server/db/queries/aggregates.sql

@@ -90,18 +90,16 @@ RETURNING *;
 -- the comment shows for as long as it lives — a step's name is a label, and
 -- positions shift (#132). Null when the step and variant had no capture, which is what
 -- there was to see.
+-- The edition rides in from the caller's resolver (ADR 0024): the pin is
+-- derived, never stored, so no query reads it off the case.
 -- name: AttachCommentVariant :exec
 INSERT INTO comment_variants (comment_id, variant_id, capture_id)
 SELECT @comment_id, @variant_id, (
     SELECT cap.id FROM captures cap
     JOIN comments c ON c.id = @comment_id
-    JOIN cases k ON k.id = c.case_id
     WHERE cap.step_id = c.step_id
       AND cap.variant_id = @variant_id
-      AND cap.edition_id = coalesce(
-            k.current_edition_id,
-            (SELECT e.id FROM editions e WHERE e.project_id = k.project_id
-             ORDER BY e.created_at DESC, e.id DESC LIMIT 1))
+      AND cap.edition_id = @edition_id::text
 )
 ON CONFLICT DO NOTHING;
 
@@ -232,19 +230,6 @@ SET blob_hash   = EXCLUDED.blob_hash,
     approved_by = EXCLUDED.approved_by,
     approved_at = now();
 
--- A review that ends releases the case onto the project's most recent edition:
--- it was only held back so the reviewer judged one fixed set of bytes.
--- name: ReleaseToLatestEdition :exec
-UPDATE cases k
-SET current_edition_id = (
-        SELECT e.id FROM editions e
-        WHERE e.project_id = k.project_id
-        ORDER BY e.created_at DESC, e.id DESC
-        LIMIT 1
-    ),
-    updated_at = now()
-WHERE k.id = @case_id;
-
 -- What a case is judged against right now, and who wrote the reference.
 -- name: CaseReferences :many
 SELECT step_id, variant_id, environment_id, blob_hash, approved_by, approved_at
@@ -368,8 +353,8 @@ WHERE cv.comment_id = @comment_id AND cv.variant_id = @variant_id;
 -- holds it instead. The window rides in as seconds so expiry is read, never
 -- written.
 -- name: ClaimCaseLock :one
-INSERT INTO case_locks (case_id, account_id)
-VALUES (@case_id, @account_id)
+INSERT INTO case_locks (case_id, account_id, edition_id)
+VALUES (@case_id, @account_id, @edition_id)
 ON CONFLICT (case_id) DO UPDATE
 SET account_id = EXCLUDED.account_id,
     claimed_at = CASE
@@ -378,18 +363,37 @@ SET account_id = EXCLUDED.account_id,
         THEN case_locks.claimed_at
         ELSE now()
     END,
+    -- The pin follows the claim (ADR 0024): a heartbeat keeps the bytes, a
+    -- fresh claim — opening the page, expiry included — re-stamps onto what
+    -- is current now. Reloading is leaving and coming back.
+    edition_id = CASE
+        WHEN NOT @fresh::boolean
+         AND case_locks.account_id = EXCLUDED.account_id
+         AND case_locks.beaten_at > now() - make_interval(secs => @window_seconds::int)
+        THEN case_locks.edition_id
+        ELSE EXCLUDED.edition_id
+    END,
     beaten_at = now()
 WHERE case_locks.account_id = EXCLUDED.account_id
    OR case_locks.beaten_at < now() - make_interval(secs => @window_seconds::int)
 RETURNING case_id, account_id, claimed_at;
 
 -- name: ReadCaseLock :one
-SELECT l.account_id, l.claimed_at, u.name AS holder_name
+SELECT l.account_id, l.claimed_at, l.edition_id, u.name AS holder_name
 FROM case_locks l
 JOIN users u ON u.id = l.account_id
 WHERE l.case_id = @case_id
   AND l.beaten_at > now() - make_interval(secs => @window_seconds::int);
 
+-- A delivery advances the case at once (#142, ADR 0024): the live lock is
+-- re-stamped onto the latest edition; with no live lock there is nothing to
+-- do — the case already reads at the latest.
+-- name: RestampLiveLock :exec
+UPDATE case_locks
+SET edition_id = @edition_id
+WHERE case_id = @case_id
+  AND beaten_at > now() - make_interval(secs => @window_seconds::int);
+
 -- Releasing somebody else's lock, or one nobody holds, changes nothing.
 -- name: ReleaseCaseLock :exec
 DELETE FROM case_locks WHERE case_id = $1 AND account_id = $2;

apps/server/db/queries/intake.sql

@@ -103,17 +103,6 @@ SELECT * FROM variants WHERE project_id = $1;
 -- name: RelabelVariant :exec
 UPDATE variants SET label = $2 WHERE id = $1;
 
--- A new edition does not yank the ground from under a reviewer: a case sitting
--- at `to-review` keeps pointing at what its reviewer is judging, and advances
--- once that review ends (product.md §7). A case that points nowhere always
--- advances -- there was nothing to protect.
--- name: AdvanceCurrentEdition :many
-UPDATE cases
-SET current_edition_id = @edition_id, updated_at = now()
-WHERE id = ANY(@case_ids::text[])
-  AND (current_edition_id IS NULL OR state <> 'to-review')
-RETURNING id;
-
 -- name: ProjectThreshold :one
 SELECT pixel_threshold FROM projects WHERE slug = $1;
 

apps/server/internal/adapters/postgres/access_test.go

@@ -238,11 +238,11 @@ func TestACommentAnchorsToTheCaptureItWasWrittenAbout(t *testing.T) {
 		t.Fatalf("edition: %v", err)
 	}
 
-	var stepID, variantID, captureID string
+	var stepID, variantID, captureID, editionID string
 	if err := repo.Pool().QueryRow(ctx,
-		`SELECT c.step_id, c.variant_id, c.id FROM captures c
+		`SELECT c.step_id, c.variant_id, c.id, c.edition_id FROM captures c
 		 JOIN steps s ON s.id = c.step_id WHERE s.case_id = $1`, kase.ID,
-	).Scan(&stepID, &variantID, &captureID); err != nil {
+	).Scan(&stepID, &variantID, &captureID, &editionID); err != nil {
 		t.Fatalf("finding the capture: %v", err)
 	}
 
@@ -255,7 +255,7 @@ func TestACommentAnchorsToTheCaptureItWasWrittenAbout(t *testing.T) {
 		t.Fatalf("creating the comment: %v", err)
 	}
 	if err := q.AttachCommentVariant(ctx, sqlcgen.AttachCommentVariantParams{
-		CommentID: created.ID, VariantID: variantID,
+		CommentID: created.ID, VariantID: variantID, EditionID: editionID,
 	}); err != nil {
 		t.Fatalf("attaching: %v", err)
 	}

apps/server/internal/adapters/postgres/catalogue.go

@@ -349,8 +349,8 @@ func (r *Repository) SummariseCases(ctx context.Context, projectID string, categ
 // than failing the whole listing.
 func countsOf(ctx context.Context, r *Repository, row sqlcgen.CasesWithCaptureCountsRow) catalogue.CaptureCounts {
 	counts := catalogue.CaptureCounts{Total: row.Captures}
-	facts, err := factsOf(ctx, r.q, sqlcgen.Case{
-		ID: row.ID, ProjectID: row.ProjectID, CurrentEditionID: row.CurrentEditionID,
+	facts, err := r.factsOf(ctx, r.q, sqlcgen.Case{
+		ID: row.ID, ProjectID: row.ProjectID,
 	})
 	if err != nil {
 		return counts

apps/server/internal/adapters/postgres/comment.go

@@ -133,20 +133,24 @@ func (r *Repository) Judge(
 			}); err != nil {
 				return "", translate("recording the acceptance", err)
 			}
-			if kase.CurrentEditionID != nil {
+			if shown, err := r.displayedEdition(ctx, q, kase); err != nil {
+				return "", err
+			} else if shown != nil {
 				if err := q.StampCaptureReference(ctx, sqlcgen.StampCaptureReferenceParams{
 					CaseID: c.CaseID, StepID: c.StepID, VariantID: variantID,
-					EditionID: *kase.CurrentEditionID, ApprovedBy: by.ID,
+					EditionID: *shown, ApprovedBy: by.ID,
 				}); err != nil {
 					return "", translate("stamping the reference", err)
 				}
 			}
 		} else {
 			// The refusal's anchor follows the refused variant: the judge
 			// refused these bytes.
-			if kase.CurrentEditionID != nil {
+			if shown, err := r.displayedEdition(ctx, q, kase); err != nil {
+				return "", err
+			} else if shown != nil {
 				if err := q.ReanchorCommentVariant(ctx, sqlcgen.ReanchorCommentVariantParams{
-					CommentID: commentID, VariantID: variantID, EditionID: *kase.CurrentEditionID,
+					CommentID: commentID, VariantID: variantID, EditionID: *shown,
 				}); err != nil {
 					return "", translate("re-anchoring the refusal", err)
 				}
@@ -237,9 +241,11 @@ func (r *Repository) Unjudge(
 			if err != nil {
 				return "", translate("reading the case", err)
 			}
-			if kase.CurrentEditionID != nil {
+			if shown, err := r.displayedEdition(ctx, q, kase); err != nil {
+				return "", err
+			} else if shown != nil {
 				if err := q.RestoreCommentVariant(ctx, sqlcgen.RestoreCommentVariantParams{
-					CommentID: commentID, VariantID: variantID, EditionID: *kase.CurrentEditionID,
+					CommentID: commentID, VariantID: variantID, EditionID: *shown,
 				}); err != nil {
 					return "", translate("restoring the coverage", err)
 				}
@@ -347,20 +353,28 @@ func (r *Repository) Edit(
 	if err := q.DetachCommentVariants(ctx, commentID); err != nil {
 		return appcomment.Outcome{}, translate("clearing the variants", err)
 	}
+	kase, err := q.CaseInProject(ctx, sqlcgen.CaseInProjectParams{ID: comment.CaseID, Slug: slug})
+	if err != nil {
+		return appcomment.Outcome{}, translate("reading the case", err)
+	}
+	shown, err := r.displayedEdition(ctx, q, kase)
+	if err != nil {
+		return appcomment.Outcome{}, err
+	}
+	anchor := ""
+	if shown != nil {
+		anchor = *shown
+	}
 	for _, variantID := range variantIDs {
 		if err := q.AttachCommentVariant(ctx, sqlcgen.AttachCommentVariantParams{
-			CommentID: commentID, VariantID: variantID,
+			CommentID: commentID, VariantID: variantID, EditionID: anchor,
 		}); err != nil {
 			return appcomment.Outcome{}, translate("attaching a variant", err)
 		}
 	}
 
-	kase, err := q.CaseInProject(ctx, sqlcgen.CaseInProjectParams{ID: comment.CaseID, Slug: slug})
-	if err != nil {
-		return appcomment.Outcome{}, translate("reading the case", err)
-	}
 	before := review.CaseState(kase.State)
-	facts, err := factsOf(ctx, q, kase)
+	facts, err := r.factsOf(ctx, q, kase)
 	if err != nil {
 		return appcomment.Outcome{}, err
 	}
@@ -510,15 +524,19 @@ func (r *Repository) move(
 	// the reference is stamped for every covered capture — otherwise the
 	// derivation would read the fix's own pixels as "moved" against the
 	// pre-fix reference (#206, seen on production as "moved · 19203 px").
-	if m == review.MoveAccept && to == review.CommentAccepted && kase.CurrentEditionID != nil {
+	shownForSettle, err := r.displayedEdition(ctx, q, kase)
+	if err != nil {
+		return appcomment.Outcome{}, err
+	}
+	if m == review.MoveAccept && to == review.CommentAccepted && shownForSettle != nil {
 		covered, err := q.CommentCoveredVariants(ctx, comment.ID)
 		if err != nil {
 			return appcomment.Outcome{}, translate("reading the covered variants", err)
 		}
 		for _, variantID := range covered {
 			if err := q.StampCaptureReference(ctx, sqlcgen.StampCaptureReferenceParams{
 				CaseID: kase.ID, StepID: comment.StepID, VariantID: variantID,
-				EditionID: *kase.CurrentEditionID, ApprovedBy: by.ID,
+				EditionID: *shownForSettle, ApprovedBy: by.ID,
 			}); err != nil {
 				return appcomment.Outcome{}, translate("stamping the reference", err)
 			}
@@ -529,12 +547,20 @@ func (r *Repository) move(
 	// bytes that claim to fix it, and the pin was showing the reviewer the
 	// screen from before the fix (product.md §7, #142).
 	if m == review.MoveDeliver {
-		if err := q.ReleaseToLatestEdition(ctx, kase.ID); err != nil {
-			return appcomment.Outcome{}, translate("advancing onto the delivery", err)
+		latest, err := r.latestEditionID(ctx, q, kase.ProjectID)
+		if err != nil {
+			return appcomment.Outcome{}, err
+		}
+		if latest != nil {
+			if err := q.RestampLiveLock(ctx, sqlcgen.RestampLiveLockParams{
+				CaseID: kase.ID, EditionID: latest, WindowSeconds: r.window(),
+			}); err != nil {
+				return appcomment.Outcome{}, translate("advancing onto the delivery", err)
+			}
 		}
 	}
 
-	facts, err := factsOf(ctx, q, kase)
+	facts, err := r.factsOf(ctx, q, kase)
 	if err != nil {
 		return appcomment.Outcome{}, err
 	}

apps/server/internal/adapters/postgres/deriver.go

@@ -13,7 +13,17 @@ import (
 //
 // Captures are read at the edition the case is judged against — its pin —
 // falling back to the project's latest when the case was never pinned.
-func factsOf(ctx context.Context, q *sqlcgen.Queries, kase sqlcgen.Case) (review.Facts, error) {
+func (r *Repository) factsOf(ctx context.Context, q *sqlcgen.Queries, kase sqlcgen.Case) (review.Facts, error) {
+	editionID, err := r.displayedEdition(ctx, q, kase)
+	if err != nil {
+		return review.Facts{}, err
+	}
+	return factsOfAt(ctx, q, kase, editionID)
+}
+
+// factsOfAt reads the facts against one already-resolved edition — the
+// settle-time re-derivation resolves to the latest itself (ADR 0024).
+func factsOfAt(ctx context.Context, q *sqlcgen.Queries, kase sqlcgen.Case, editionID *string) (review.Facts, error) {
 	var facts review.Facts
 
 	threshold, err := q.PixelThresholdByProject(ctx, kase.ProjectID)
@@ -22,21 +32,6 @@ func factsOf(ctx context.Context, q *sqlcgen.Queries, kase sqlcgen.Case) (review
 	}
 	facts.PixelThreshold = int(threshold)
 
-	editionID := kase.CurrentEditionID
-	if editionID == nil {
-		edition, err := q.LatestEdition(ctx, kase.ProjectID)
-		if err != nil {
-			if isNoRows(err) {
-				// A book can start empty (ADR 0008): no edition, no captures.
-				editionID = nil
-			} else {
-				return facts, translate("reading the edition", err)
-			}
-		} else {
-			editionID = &edition.ID
-		}
-	}
-
 	if editionID != nil {
 		rows, err := q.CaseCaptureFacts(ctx, sqlcgen.CaseCaptureFactsParams{
 			CaseID: kase.ID, EditionID: *editionID,