The consolidation half of H3 — shared LOGIC with thin per-type remains:
- lwwDecision: the three compareIncoming* functions were the same
skeleton (client timestamp -> strictly-newer wins; no timestamp ->
any content difference wins) drifting apart only in per-type
equality and the schema-driven fallback timestamp (bookmarks carry
no updated_at). The decision now lives in one function; each compare
keeps only its equality expression.
- idTargetOwnership + tombstoneWins: the id-targeted resolution blocks
(HighlightID/BookmarkID/NoteID) shared the ownership check and the
deleted-row gate — now one gate each instead of three copies of the
resurrect semantics.
- queue.go: syncNote/syncHighlight/syncBookmark decoded device payloads
through three 10-key if-type-assertion chains; typed map accessors
(mapStr/mapF64/mapI32/mapDeviceSyncData) replace them — same
missing-key-is-zero semantics, declarative field lists. The progress
decoder is untouched (H1 territory, separately tested).
Per-type remains are the things that genuinely cannot merge: request
structs, dedup triples, the per-table sqlc param builders, and the
KOReader/Kobo payload adapters. Pins, H1/H3 tests, both unit suites
green.
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).
H1's doctrine applied to the annotation service, which had been left on
setters: NewMediaHandler/NewKoboHandler/NewKOReaderHandler/
NewSyncQueueProcessor(WithConfig) now take it as a parameter; the four
SetAnnotationService setters are deleted; wiring updated (main.go,
router/sync.go, test harness).
With the service unskippable, the 18 nil-guards and their plain-query
fallbacks die:
- media.go: the create fallbacks for notes/highlights/bookmarks (the
bookmark one was the H3 census's Bug #1 — it dropped
percentage/chapter_reference the sync path carries), the highlight
PUT fallback, and the three delete-path plain deletes (tombstone is
now the only delete semantics).
- kobo.go: both markup-ingest annotation loops (the bookmark fallback
wrote the text into media_notes — a category error the fallback
never should have had).
- koreader.go: all three push-stack guards.
- queue.go: the three "annotation service not available" errors in
syncNote/syncHighlight/syncBookmark.
Behavior note: the deleted highlight create/PUT fallbacks were the only
paths passing note_id to the plain query — the sync service has no
NoteID field, so the web's highlight→note link was already being
dropped on the live path; this commit makes that pre-existing
limitation explicit rather than papering it over (surfacing note_id on
SaveHighlightRequest is left for a behavior commit that needs it).
Pins (now 20 incl. the 3 annotation CRUD suites) + H1 tests + both
unit suites green.
Pin-first batch for H3: freeze web annotation CRUD response shapes
(create 201 / update 200 / delete 204; full sqlc row models) before
the stack consolidation touches anything.
The new pins immediately caught a live server bug the 28 unit tests
could never see (annotations_test.go is pure-unit: keys and compare
logic, no DB): applyNoteLWW built UpdateMediaNoteForSyncParams without
UserID, so the query's WHERE id=$1 AND user_id=$13 compared against
zero — every dedup-hit note re-save (same content+position) returned
500 "update note: no rows in result set" and the update never landed.
The bookmark twin passes UserID and works; the note stack is the clone
that dropped the field.
Fix: pass UserID in the params (one line). Pins go green and become
the regression net; changelog records the new surfaces.
The family-lens hardening: library_visibility was only enforced where
file bytes flowed (download/reader/OPDS) — metadata surfaces leaked
hidden libraries to any authenticated account, and registration was
open by default with no admin-side user creation.
- Series are grouped PER LIBRARY now (same name in two libraries =
two entries: manga adaptations / LN splits no longer merge);
/api/series aggregates only the requester's VISIBLE libraries and
/api/series/books REQUIRES a visible library_id (breaking contract;
web + app clients updated in the same change).
- /api/media-items, item detail, and the reader content endpoints
(chapters/panels/outline/THUMBNAILS — page images leaked unguarded
next to a gated GetPage) all enforce the visible set; resolveLibrary
no longer honors a hand-typed library_id outside the visible set.
- Annotation/note/bookmark mutations (update/delete/tombstone, sync
LWW paths incl. conflict resolution) carry ownership predicates:
another account's bare row id is a 404, not a silent delete.
- Admin user creation: POST /api/auth/users (AdminMiddleware) + a
Create User card in the admin Users UI — accounts stay creatable
with self-service registration OFF, which is now the schema default
(fresh installs; the first-admin bootstrap exception is unchanged).
- Tests: series contract re-pinned (per-library, visibility 403s),
new ownership + admin-create integration suites, OPDS all=1 test
now passes an explicit page size (shared-DB growth made the fresh
books fall off page 1), device-cap helper switched to the admin
endpoint.
The LWW skip path compared only annotation content (text, color, note,
percentages) — locator columns were not part of 'changed'. A device echo
with identical content therefore resolved to skip, and the freshly
re-derived canonical CFIs were discarded in the same request that
computed them: a highlight whose stored start anchor had been corrupted
by the old percentage-exact bug could never heal, because every
subsequent echo carried the same text and was skipped before the
locator columns were written. Observed live: a push converted the
cross-block highlight's anchors correctly (structural, heading to
paragraph) yet the row kept its garbage start CFI and the highlight
stayed unpaintable on the web.
Echo saves now treat a non-empty incoming locator that differs from the
stored one as a change (locatorRefresher): empty locators still coalesce
(no drift), and once healed the echo produces identical CFIs, so the
steady state remains skip — no write churn. Highlights check
epubcfi_start/end; bookmarks check cfi_position/position.
applyBookmarkLWW also gains the empty-locator coalescing the highlight
path already had: web bookmark edits carry no device locators, and a
title/note edit must not wipe the stored device-native position.
PUT /highlights/:id parsed the row id from the URL and then dropped it:
the sync-aware path routed through SaveHighlight's content-derived dedup
key, on the assumption that the same text + CFI always resolves to the
same key. That assumption breaks in practice — the stored epubcfi_start
drifts from foliate's range shape to the converter's point shape after a
device echo rewrites the row (bucketPosition cuts at the last colon, so
'…/6,/1:367,…' and '…/6/1:367' bucket differently), and the user can
edit the selection text. The recomputed key then misses the row being
edited and createHighlight mints a second one: the edited row (with
note, no device pos0) beside the original — served to KOReader as two
highlights, one noted and one not. Editing a selection's text would hit
the same trap.
SaveHighlightRequest gains an optional HighlightID. When set, the save
resolves the row by id (ownership-checked), LWWs against it under its
stored dedup key, and never re-derives identity from content. The PUT
handler passes the already-parsed id. Device pushes, Kobo, and the sync
queue send no id and keep the identity-based flow untouched.
applyLWW also stops wiping stored locators on web edits: the web reader
sends empty start/end positions (it never had a CRE xpointer), so a
note/color edit now keeps the device-native positions and CFIs instead
of blanking them — round-trip serve-back for device-created highlights
survives web-side edits.
- Drop UNIQUE(media_item_id,user_id,title): titles are display labels
shared verbatim across clients; same-title bookmarks on different pages
now coexist instead of 500ing (deleting over a tombstone no longer
blocks future creates with that title)
- Add origin_source column recording the creating client (android/web/
koreader), set once at insert, exposed in API responses
- Web reader auto-title mirrors KOReader's 'in <chapter>' convention,
falling back to 'Bookmark'; adds bookmark rename in the drawer
RestoreAnnotationByID and PurgeAnnotationByID dispatch on annotation kind
(highlight/note/bookmark) to the new queries, broadcasting an annotation
update on restore so connected web sessions refresh. Both report whether
a row actually changed.
TombstoneBookmarkByDedupKey mirrors the existing TombstoneHighlight for
bookmarks: devices report deletions by dedup key (they have no row IDs),
and until now only highlights had a key-based tombstone path — device
bookmark deletions had nowhere to land.
ValidAnnotationKind centralizes the kind check the HTTP handlers share.
Echo duplication: devices push their full annotation list on every
sync, and an echo of a web-created annotation computed a different
dedup key than the original (device locators differ from web locators)
— every pull→push cycle minted a duplicate row, and cleaning those up
on the web tombstoned them back to the device, deleting the
just-applied copies. That was the "web highlights never appear on
KOReader" experience. GetMetadata now serves each annotation's
dedup_key; the device stores it on the applied entry and echoes it in
pushes; SaveHighlight/SaveBookmark/SaveNote accept a DedupKey
override so echoes converge onto the original row (verified: pull →
echo push creates no rows, LWW skips identical content).
Color semantics (per user preference): devices render their own
default and cannot round-trip web colors, so GetMetadata no longer
serves colors at all — every highlight syncs regardless of its web
color and the device draws its default. An echo carries no color;
ingest then PRESERVES the stored web color (existingHighlightColor
lookup by dedup key) so round-trips never change it. A non-empty
device color means the user edited the highlight there: it maps
name→hex (green→#a5d6a7, default yellow) and wins. Verified: echo
kept #ffd54f; a simulated device edit with "green" updated the web
row to #a5d6a7.
Classification: KOReader auto-fills text="in Chapter X" on page
bookmarks (ReaderAnnotation:updateItemByXPointer), so the plugin's
text-presence classification turned every echoed bookmark into a junk
highlight on the web. v2 classification now keys off the drawer field
(present = highlight/note, absent = bookmark with its label in note).
Deleting a bookmark/highlight/note and then re-adding the same content
at the same position (same dedup key — e.g. the reader's auto-titled
'Bookmark at X%') was silently swallowed: the save hit the tombstone
branch, returned 201 with the deleted row, and the list (which filters
deleted) stayed empty. Bookmarks were further blocked by the
UNIQUE(media_item_id, user_id, title) slot the tombstoned row holds,
and notes had no TTL escape at all.
Tombstones now only block saves that predate them (stale replays from
a device that still has the annotation). A save whose modification
time is newer than max(deleted_at, last_modified_at) — a deliberate
re-create from the web or a device — resurrects the row via the LWW
update queries, which now clear deleted/deleted_at.
CleanupExpiredRefreshTokens and CleanupExpiredOpdsTokens were generated
by sqlc but never invoked anywhere in the codebase, so expired/revoked
tokens accumulated in the database indefinitely. The refresh-token query
was parameterized in the settings-registry work specifically so its
retention window could follow the configurable session duration, but the
periodic caller was never wired up.
annotations.go:
- Rename StartTombstonePurger to StartDailyMaintenance, which now runs
all periodic cleanup tasks from a single 24h-tick goroutine.
- Add runDailyMaintenance helper: tombstones, then OPDS tokens, then
refresh tokens, each logging independently so one failure never skips
the others.
- Refresh-token retention is read from the registry (SessionDuration)
on every tick so live admin edits are honored; guarded on the registry
being wired so unwired test paths simply skip cleanup.
- All three queries only delete rows that are already expired or
revoked, so active sessions are never logged out.
main.go:
- Update the call site: tombstonePurgerCancel becomes maintenanceCancel
and calls StartDailyMaintenance.
Net footprint: still one goroutine and one ticker; the cleanup adds one
DELETE per table per day.
The 30-day retention window for soft-deleted annotations was a package
const; move it behind the registry so it can be tuned live.
annotations.go:
- AnnotationService gains an optional *database.SettingsRegistry and a
tombstoneTTL() helper. The skip-resurrect checks and the purge cutoff
now call it instead of reading the TombstoneTTL const directly.
- Add ActiveTombstoneTTL() so callers outside the sync package can
compute cutoffs consistently with the service.
- The package-level TombstoneTTL const is retained as the fallback for
tests / unwired code paths.
kobo.go, koreader.go:
- The per-book tombstone sweep cutoff now uses
h.annotationSvc.ActiveTombstoneTTL() instead of the wsync.TombstoneTTL
const, so both the service and the handlers honor the configured TTL.
AnnotationService is the central service for cross-device annotation sync.
It provides SaveHighlight, SaveNote, and SaveBookmark methods that handle
the full sync lifecycle:
Identity (3-layer):
1. Server UUID (primary key)
2. Per-device native ID stored in device_sync_data JSONB
3. Content dedup_key: sha1(normalize(selection_text) + bucket_position)
- CFI character offsets are stripped for bucketing so the same
highlight at slightly different offsets still deduplicates
- Raw positions are preserved in the DB for precise restoration
Resolution policy (LWW):
- When the incoming annotation has an explicit ModifiedAt timestamp,
last_modified_at wins
- When the device sends zero ModifiedAt (creation time only), field-diff
mode compares content fields (text/color/note/percentage) — if all
match, the save is skipped; if any differ, the save is applied with
server-receive-time as the new last_modified_at
Conflict detection:
- When incoming and existing annotations have different sources (e.g.
koreader vs kobo) and content differs, an auto_resolved sync_conflict
is recorded with both sides' data for audit trail
- Broadcasts a WebSocket conflict notification for real-time UI updates
Tombstone management:
- Delete-wins: tombstoned annotations block recreation from stale pushes
- 30-day TTL before physical purge
- PurgeExpiredTombstones method + StartTombstonePurger goroutine (24h ticker)
Add locators.go with unified bidirectional CFI conversion:
ConvertToCanonical / ConvertFromCanonical
- CRE XPointer <-> standard EPUB CFI (for KOReader)
- KEPUB CFI passthrough (for Kobo)
- Skips non-reflowable formats (PDF, CBZ, fixed-layout EPUBs)
Add 25 unit tests covering:
- Dedup key determinism, text normalization, position sensitivity
- Offset insensitivity (CFI char-offset bucketing)
- Device sync data merge (preserves existing, overwrites same source)
- Cross-source detection
- LWW comparison (newer wins, older skipped, fallback to updated_at)
- Field-diff mode (identical content skipped, changes applied)
- Tombstone TTL constant
- CRE XPointer parsing and classification
- Standard EPUB CFI classification
John O'Keefe
john-okeefe