fix(annotations): bookmark and note PUTs land through the sync service (LWW by id)

H3 census bugs #2 and #3: both web PUTs used plain queries that
bypassed the sync-aware layer entirely — no LWW, no source bookkeeping,
and locator fields the client couldn't even send. The highlight PUT was
the only one already routed (via HighlightID); the other two now mirror
it:

- SaveBookmarkRequest gains BookmarkID, SaveNoteRequest gains NoteID:
  id-targeted resolution reads the exact row, verifies ownership,
  LWWs under its stored dedup key, and resurrects tombstones when the
  edit is newer (same doctrine and comment structure as the highlight
  path).
- Fields the PUT omits are coalesced from the stored row before the
  full-column sync update — the legacy web PUT sends only
  title/notes/position, and zeroing cfi/percentage/page on every edit
  would have been data loss. Verified both directions by
  TestH3_BookmarkPUTPreservesLocators (omitted survives, explicit
  lands).
- The bookmark PUT's wire DTO gains the create endpoint's locator keys
  as optional fields (documented additive request keys in
  contract_pins.md); the note PUT keeps its content+position wire.

UpdateMediaBookmark/UpdateMediaNote plain queries are now uncalled
(the generated queries stay until the H4 query sweep).

cmd/server/tests/contract_pins_test.go

@@ -1218,3 +1218,91 @@ func TestContractPin_AnnotationCRUDBookmark(t *testing.T) {
 		},
 		_WEBBookmarkRowKeys)
 }
+
+// TestH3_BookmarkPUTPreservesLocators locks the H3 PUT routing: the web
+// bookmark PUT lands through the sync service as an id-targeted LWW edit.
+// Fields the PUT omits must be COALESCED with the stored row (the old
+// plain query never touched them; a naive full-column sync write would
+// have zeroed them), fields it sends must land, and the row's
+// source/modified_at bookkeeping must update — the plain query bypassed
+// it entirely.
+func TestH3_BookmarkPUTPreservesLocators(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	setup := setupTestServer(t)
+	fx := createContractFixture(t, setup)
+	url := annotationURL(setup.Server.URL, fx.itemID, "bookmarks", "")
+
+	create := doReq(t, "POST", url, map[string]interface{}{
+		"title":             "H3 bookmark",
+		"position":          "p1",
+		"cfi_position":      "epubcfi(/6/4/2:30)",
+		"page_number":       12,
+		"chapter_number":    3,
+		"percentage":        0.25,
+		"chapter_reference": 4,
+	}, setup.RegularToken)
+	defer create.Body.Close()
+	require.Equal(t, http.StatusCreated, create.StatusCode)
+	created := decodeJSON(t, create)
+	id, _ := created["id"].(string)
+	require.NotEmpty(t, id)
+
+	// PUT carrying ONLY the legacy fields: locators must survive.
+	put := doReq(t, "PUT", annotationURL(setup.Server.URL, fx.itemID, "bookmarks", id), map[string]interface{}{
+		"title": "H3 bookmark renamed",
+		"notes": "edited",
+	}, setup.RegularToken)
+	defer put.Body.Close()
+	raw, _ := io.ReadAll(put.Body)
+	require.Equal(t, http.StatusOK, put.StatusCode, "body: %s", raw)
+	var row map[string]interface{}
+	require.NoError(t, json.Unmarshal(raw, &row))
+	require.Equal(t, "H3 bookmark renamed", getStringField(t, row, "title"))
+	require.Equal(t, "epubcfi(/6/4/2:30)", getStringField(t, row, "cfi_position"),
+		"an omitted locator must be coalesced, not zeroed")
+	require.Equal(t, 0.25, getFloatField(t, row, "percentage_location"),
+		"an omitted locator must be coalesced, not zeroed")
+	require.Equal(t, float64(12), getFloatField(t, row, "page_number"))
+	require.Equal(t, "web", getStringField(t, row, "last_modified_source"),
+		"the sync-routed PUT must maintain source bookkeeping")
+
+	// PUT carrying a locator explicitly: it lands.
+	put2 := doReq(t, "PUT", annotationURL(setup.Server.URL, fx.itemID, "bookmarks", id), map[string]interface{}{
+		"title":        "H3 bookmark renamed",
+		"notes":        "edited",
+		"percentage":   0.5,
+		"cfi_position": "epubcfi(/6/4/2:44)",
+	}, setup.RegularToken)
+	defer put2.Body.Close()
+	raw2, _ := io.ReadAll(put2.Body)
+	require.Equal(t, http.StatusOK, put2.StatusCode, "body: %s", raw2)
+	var row2 map[string]interface{}
+	require.NoError(t, json.Unmarshal(raw2, &row2))
+	require.Equal(t, 0.5, getFloatField(t, row2, "percentage_location"))
+	require.Equal(t, "epubcfi(/6/4/2:44)", getStringField(t, row2, "cfi_position"))
+	// ...and the fields omitted THIS time coalesce again (page survived).
+	require.Equal(t, float64(12), getFloatField(t, row2, "page_number"))
+
+	// The note PUT carries its content edit through the service too.
+	noteURL := annotationURL(setup.Server.URL, fx.itemID, "notes", "")
+	noteCreate := doReq(t, "POST", noteURL, map[string]interface{}{
+		"content": "H3 note", "position": "p9",
+	}, setup.RegularToken)
+	defer noteCreate.Body.Close()
+	require.Equal(t, http.StatusCreated, noteCreate.StatusCode)
+	note := decodeJSON(t, noteCreate)
+	noteID, _ := note["id"].(string)
+	notePut := doReq(t, "PUT", annotationURL(setup.Server.URL, fx.itemID, "notes", noteID), map[string]interface{}{
+		"content": "H3 note edited", "position": "p9b",
+	}, setup.RegularToken)
+	defer notePut.Body.Close()
+	raw3, _ := io.ReadAll(notePut.Body)
+	require.Equal(t, http.StatusOK, notePut.StatusCode, "body: %s", raw3)
+	var noteRow map[string]interface{}
+	require.NoError(t, json.Unmarshal(raw3, &noteRow))
+	require.Equal(t, "H3 note edited", getStringField(t, noteRow, "content"))
+	require.Equal(t, "web", getStringField(t, noteRow, "last_modified_source"),
+		"the note PUT must maintain source bookkeeping via the sync service")
+}

docs/contract_pins.md

@@ -74,3 +74,16 @@ cover_image_path, has_conflict, format_group, badge`.
   ("update note: no rows in result set") — the LWW update could never
   land. Fixed in the same commit (server-side fix, freely allowed);
   the pins are its regression net.
+- **H3 (PUT fixes routed through the sync service)** — `PUT
+  /api/media-items/:id/bookmarks/:bookmarkId` now parses ADDITIONAL
+  optional request keys mirroring its create sibling:
+  `cfi_position, page_number, chapter_number, percentage,
+  chapter_reference` (all zero-value-preserving: omitted keys coalesce
+  with the stored row inside the service, so legacy clients that send
+  only title/notes/position keep every stored locator — verified by
+  TestH3_BookmarkPUTPreservesLocators). The PUT lands as an id-targeted
+  LWW edit under the row's stored dedup key (BookmarkID/NoteID mirror
+  SaveHighlightRequest.HighlightID), so `last_modified_source` /
+  `last_modified_at` are now maintained. Note PUT unchanged on the wire
+  (content+position) but routed through the service the same way.
+  Response shapes unchanged (same row models — pins green).

internal/handlers/media.go

@@ -169,6 +169,14 @@ type UpdateMediaBookmarkRequest struct {
 	Title    string `json:"title" validate:"required,min=1,max=255"`
 	Notes    string `json:"notes" validate:"max=100000"`
 	Position string `json:"position" validate:"max=100"`
+	// Optional locator fields (H3): the create endpoint has always
+	// accepted these; the PUT ignored them until it was routed through
+	// the sync service. Zero-valued fields preserve the stored values.
+	CfiPosition      string  `json:"cfi_position" validate:"max=255"`
+	PageNumber       int32   `json:"page_number"`
+	ChapterNumber    int32   `json:"chapter_number"`
+	Percentage       float64 `json:"percentage"`
+	ChapterReference int32   `json:"chapter_reference"`
 }
 
 type MediaHandler struct {
@@ -1875,6 +1883,12 @@ func (mh *MediaHandler) UpdateMediaNote(c *echo.Context) error {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid user"})
 	}
 
+	mediaID := c.Param("id")
+	mediaUUID, err := uuid.Parse(mediaID)
+	if err != nil {
+		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid media item id"})
+	}
+
 	var req UpdateMediaNoteRequest
 	if err := c.Bind(&req); err != nil {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid request"})
@@ -1883,17 +1897,24 @@ func (mh *MediaHandler) UpdateMediaNote(c *echo.Context) error {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": err.Error()})
 	}
 
-	note, err := mh.db.UpdateMediaNote(c.Request().Context(), database.UpdateMediaNoteParams{
+	// Through the sync service (H3): the PUT lands as a proper LWW edit
-		ID:       pgtype.UUID{Bytes: noteUUID, Valid: true},
+	// targeted by id (locator columns coalesced with the stored row), so
-		Content:  req.Content,
+	// source/modified_at bookkeeping is maintained — the plain query
-		Position: pgtype.Text{String: req.Position, Valid: req.Position != ""},
+	// bypassed it entirely.
+	result, err := mh.annotationSvc.SaveNote(c.Request().Context(), wsync.SaveNoteRequest{
+		MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
 		UserID:      pgtype.UUID{Bytes: userUUID, Valid: true},
+		Content:     req.Content,
+		Position:    req.Position,
+		Source:      "web",
+		ModifiedAt:  time.Now(),
+		NoteID:      pgtype.UUID{Bytes: noteUUID, Valid: true},
 	})
 	if err != nil {
 		return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
 	}
 
-	return c.JSON(http.StatusOK, note)
+	return c.JSON(http.StatusOK, result.Note)
 }
 
 // DeleteMediaNote handles DELETE /api/media-items/:id/notes/:noteId
@@ -2178,12 +2199,16 @@ func (mh *MediaHandler) UpdateMediaBookmark(c *echo.Context) error {
 	if err != nil {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid user"})
 	}
-
 	bookmarkID := c.Param("bookmarkId")
 	bookmarkUUID, err := uuid.Parse(bookmarkID)
 	if err != nil {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid bookmark id"})
 	}
+	mediaID := c.Param("id")
+	mediaUUID, err := uuid.Parse(mediaID)
+	if err != nil {
+		return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid media item id"})
+	}
 
 	var req UpdateMediaBookmarkRequest
 	if err := c.Bind(&req); err != nil {
@@ -2193,18 +2218,30 @@ func (mh *MediaHandler) UpdateMediaBookmark(c *echo.Context) error {
 		return c.JSON(http.StatusBadRequest, map[string]string{"error": err.Error()})
 	}
 
-	bookmark, err := mh.db.UpdateMediaBookmark(c.Request().Context(), database.UpdateMediaBookmarkParams{
+	// Through the sync service (H3): the PUT now lands as a proper LWW
-		ID:       pgtype.UUID{Bytes: bookmarkUUID, Valid: true},
+	// edit — source/modified_at bookkeeping maintained, locator fields
-		Title:    req.Title,
+	// carried (coalesced with the stored row for omitted ones), and the
-		Notes:    pgtype.Text{String: req.Notes, Valid: req.Notes != ""},
+	// row targeted by id so an edited title can't miss its dedup key.
-		Position: pgtype.Text{String: req.Position, Valid: req.Position != ""},
+	result, err := mh.annotationSvc.SaveBookmark(c.Request().Context(), wsync.SaveBookmarkRequest{
+		MediaItemID:      pgtype.UUID{Bytes: mediaUUID, Valid: true},
 		UserID:           pgtype.UUID{Bytes: userUUID, Valid: true},
+		Title:            req.Title,
+		Position:         req.Position,
+		Notes:            req.Notes,
+		PageNumber:       req.PageNumber,
+		ChapterNumber:    req.ChapterNumber,
+		CFIPosition:      req.CfiPosition,
+		PercentageLoc:    req.Percentage,
+		ChapterReference: req.ChapterReference,
+		Source:           "web",
+		ModifiedAt:       time.Now(),
+		BookmarkID:       pgtype.UUID{Bytes: bookmarkUUID, Valid: true},
 	})
 	if err != nil {
 		return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
 	}
 
-	return c.JSON(http.StatusOK, bookmark)
+	return c.JSON(http.StatusOK, result.Bookmark)
 }
 
 // DeleteMediaBookmark handles DELETE /api/media-items/:id/bookmarks/:bookmarkId

internal/sync/annotations.go

@@ -521,6 +521,10 @@ type SaveNoteRequest struct {
 	ModifiedAt         time.Time
 	DeviceSyncData     []byte
 	DedupKey           string // overrides the computed key for device echoes
+	// NoteID, when valid, targets that exact row (web PUTs edit by id):
+	// the save LWWs against it under its stored dedup key, mirroring
+	// SaveHighlightRequest.HighlightID.
+	NoteID pgtype.UUID
 }
 
 type SaveNoteResult struct {
@@ -535,6 +539,54 @@ func (s *AnnotationService) SaveNote(ctx context.Context, req SaveNoteRequest) (
 	}
 
 	dedupKey := req.DedupKey
+
+	// Web edits arrive with the row id from the URL: resolve by id and LWW
+	// against that row under its stored key (same doctrine as
+	// SaveHighlight's HighlightID path). Locator columns the note PUT
+	// does not carry are coalesced from the stored row so the full-column
+	// sync update cannot zero them.
+	if req.NoteID.Valid {
+		byID, idErr := s.db.GetMediaNote(ctx, req.NoteID)
+		if idErr != nil && !errors.Is(idErr, pgx.ErrNoRows) {
+			return nil, fmt.Errorf("query note by id: %w", idErr)
+		}
+		if idErr == nil {
+			if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID {
+				return nil, fmt.Errorf("note %s belongs to another user or media item", req.NoteID)
+			}
+			if dedupKey == "" {
+				dedupKey = byID.DedupKey.String
+			}
+			if req.EpubcfiLocation == "" {
+				req.EpubcfiLocation = byID.EpubcfiLocation.String
+			}
+			if req.PercentageLocation == 0 {
+				req.PercentageLocation = byID.PercentageLocation.Float64
+			}
+			if req.CharacterStart == 0 {
+				req.CharacterStart = byID.CharacterStart.Int32
+			}
+			if req.CharacterEnd == 0 {
+				req.CharacterEnd = byID.CharacterEnd.Int32
+			}
+			if req.ChapterReference == 0 {
+				req.ChapterReference = byID.ChapterReference.Int32
+			}
+			if req.ParagraphReference == 0 {
+				req.ParagraphReference = byID.ParagraphReference.Int32
+			}
+			if byID.Deleted.Bool {
+				if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) {
+					return &SaveNoteResult{Note: byID, Outcome: SaveOutcomeDeleted}, nil
+				}
+				// Newer than the tombstone: a deliberate re-create. Resurrect
+				// via the LWW update (which clears deleted/deleted_at).
+			}
+			return s.applyNoteLWW(ctx, req, byID, dedupKey)
+		}
+		// No row with that id: fall through to identity-based resolution.
+	}
+
 	if dedupKey == "" {
 		dedupKey = ComputeDedupKey(req.Content, req.EpubcfiLocation, req.Position)
 	}
@@ -684,6 +736,11 @@ type SaveBookmarkRequest struct {
 	// DedupKey overrides the computed key for device echoes (see
 	// SaveHighlightRequest).
 	DedupKey string
+	// BookmarkID, when valid, targets that exact row (web PUTs edit by
+	// id): the save LWWs against it under its stored dedup key, mirroring
+	// SaveHighlightRequest.HighlightID — an edit must not re-derive
+	// identity from (possibly edited) content.
+	BookmarkID pgtype.UUID
 }
 
 type SaveBookmarkResult struct {
@@ -694,6 +751,55 @@ type SaveBookmarkResult struct {
 
 func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRequest) (*SaveBookmarkResult, error) {
 	dedupKey := req.DedupKey
+
+	// Web edits arrive with the row id from the URL: resolve by id and LWW
+	// against that row under its stored key (same doctrine as
+	// SaveHighlight's HighlightID path). Fields the edit does not carry
+	// are coalesced from the stored row — the web PUT predates the
+	// locator columns and omits them; zeroing them on update would be
+	// data loss.
+	if req.BookmarkID.Valid {
+		byID, idErr := s.db.GetMediaBookmark(ctx, req.BookmarkID)
+		if idErr != nil && !errors.Is(idErr, pgx.ErrNoRows) {
+			return nil, fmt.Errorf("query bookmark by id: %w", idErr)
+		}
+		if idErr == nil {
+			if byID.UserID != req.UserID || byID.MediaItemID != req.MediaItemID {
+				return nil, fmt.Errorf("bookmark %s belongs to another user or media item", req.BookmarkID)
+			}
+			if dedupKey == "" {
+				dedupKey = byID.DedupKey.String
+			}
+			if req.CFIPosition == "" {
+				req.CFIPosition = byID.CfiPosition.String
+			}
+			if req.EpubcfiLocation == "" {
+				req.EpubcfiLocation = byID.EpubcfiLocation.String
+			}
+			if req.PercentageLoc == 0 {
+				req.PercentageLoc = byID.PercentageLocation.Float64
+			}
+			if req.PageNumber == 0 {
+				req.PageNumber = byID.PageNumber.Int32
+			}
+			if req.ChapterNumber == 0 {
+				req.ChapterNumber = byID.ChapterNumber.Int32
+			}
+			if req.ChapterReference == 0 {
+				req.ChapterReference = byID.ChapterReference.Int32
+			}
+			if byID.Deleted.Bool {
+				if !incomingNewerThanTombstone(req.ModifiedAt, byID.DeletedAt, byID.LastModifiedAt) {
+					return &SaveBookmarkResult{Bookmark: byID, Outcome: SaveOutcomeDeleted}, nil
+				}
+				// Newer than the tombstone: a deliberate re-create. Resurrect
+				// via the LWW update (which clears deleted/deleted_at).
+			}
+			return s.applyBookmarkLWW(ctx, req, byID, dedupKey)
+		}
+		// No row with that id: fall through to identity-based resolution.
+	}
+
 	if dedupKey == "" {
 		dedupKey = ComputeDedupKey(req.Title, req.EpubcfiLocation, req.Position)
 	}