The identification queue for device files the sync cannot match is now
live end-to-end. Previously the whole feature was dark: unmatched device
files were silently dropped (losing their reading progress), the queue
table sat empty by construction, and the page template had no route.
- Ingest: the KOReader sync loop, on an unresolvable file, upserts
unlinked_books (one row per device + content id; re-syncs bump
last_seen_at and refresh device-reported metadata) instead of
dropping the file. Schema gains the (device_id, content_id) unique
index the upsert needs.
- Page: GET /unlinked-books routes the existing page template, listing
the caller's own unresolved rows (ListUnresolvedUnlinkedBooksForUser
— devices.user_id scoping; no cross-user leakage).
- Dashboard banner: a warning card above the shelves, rendered only
when the caller's queue is non-empty (hidden-unless-needed), linking
to the page.
- Resolve flow unchanged and already gated: MatchingHandler.LinkBook
verifies device ownership and target-item visibility (403s) and
creates the device file alias, so the next sync matches by path.
SyncHandler.LinkBook and Handler.LinkBook are unrouted dead handlers
— triage candidates, not part of the live flow.
The audiobook library type shipped backend-complete (LibraryTypeAudiobooks
constant, allowed extensions, MIME map, schema seed row, GET
/api/libraries/types) but neither create-library form offered it: the
admin Create Library modal and the setup wizard's add-library step both
hardcoded their <option> lists to the original three types
(ebooks/comics/manga), so an audiobook library could not be created from
the UI.
Both dropdowns now render from the same DB-backed library_types table
the create API resolves types against — a future type appears without
template edits (the exact drift the rendering-ownership rule warns
about):
- templates/utils.go: OrderedLibraryTypes pins the historical dropdown
order (ebooks, comics, manga, audiobooks) and appends unknown future
types by name; LibraryTypeDisplayName renders labels with a
capitalize-the-name fallback for types added without a label
- templates/admin_library.templ: new LibraryTypeOptions fragment used
by the modal's select; AdminLibrary takes the types list
- templates/setup.templ: Setup takes the types list, same fragment
- internal/router/helpers.go: libraryTypeOptionsFor fetches via
LibraryService.GetLibraryTypes, falling back to the built-in type
names on DB failure so the forms stay usable (CreateLibrary still
resolves the name against the DB, so the fallback is never
authoritative)
- internal/router/frontend.go, internal/router/setup.go: pass the list
into the two pages
Tests:
- templates/utils_test.go: pure unit tests for ordering + labels
- cmd/server/tests/admin_library_page_test.go: SSR regression —
/admin/library must offer all four library types, audiobooks exactly
once (options come from the DB loop, not a hardcode)
- BookMatch now carries title/author/cover (the fields the old client
renderer always assumed but the API never sent — cards would have
shown 'undefined'); filled during the family-lens pass in QueryBooks
- new partial route GET /unlinked-books/matches renders the scoped
match cascade server-side; searchMatches swaps it via htmx.ajax
instead of rebuilding cards from JSON
- NOTE: the unlinked-books surface is currently dark end-to-end — the
templ page has no route, the live linking.ts renderer targets a
container no template has, and its Link button calls a
window.showMatchModal that does not exist. This commit ships the
scoped backend + partial; wiring the UI is a product decision.
Where the browser supports it (Chrome 111+, Safari 18+, Firefox 139+),
switchWithTransition now uses the native View Transitions API: the old
shelves are snapshotted, the fetch + swap happen behind the dim, and the
two snapshots crossfade — no blank phase, no manual opacity
choreography. The dim + spinner is hidden inside the update so it blends
away as part of the transition. Browsers without the API keep the
existing dim + fade sequence unchanged. Zero dependencies — the
crossfade is the browser's own.
The switch overlay intended a 50% dim (bg-opacity-50) but set its
background with an inline style — Tailwind's bg-opacity variable does
not apply to inline styles, so the overlay rendered fully opaque. Every
library switch wiped the material area to a solid sheet (chrome stays
visible: the sidebar sits at z-60 above the overlay's z-50) and
hard-cut back when the overlay dropped — reading exactly as a new page
instead of a fade. The dashboard showed it worst because its switch
holds the overlay longest (sections fetch + awaited week-widget refresh
behind it).
- overlay background is now theme-aware translucent
(color-mix 55% bg-primary over transparent); the dead bg-opacity-50
class is gone
- the dim lifts before the fade-in starts, so the new content's fade
plays fully visible rather than being hard-cut mid-transition
/api/books/match returned media metadata from ALL libraries regardless
of visibility: the resolver lookups (GetMediaItemBySHA256 and friends)
are global, so match results leaked hidden-library titles, covers and
ids to any authenticated user, and LinkBook accepted a link target from
a hidden library. Both now enforce the family-lens rule:
- QueryBooks post-filters the cascade by the requester's visible-library
set (absent-from-results — a hidden library's book simply never
matches), before determineAction so the suggested action reflects the
filtered set. BookQueryRequest gains a server-side-only
VisibleLibraryIDs field; nil keeps legacy callers unfiltered.
- LinkBook verifies the device belongs to the caller
(DeviceExistsForUser) and the target item's library is visible —
403 otherwise. Linking was previously unscoped on both axes.
- GetUnlinkedBooks likewise verifies device ownership (its service is
still a stub, but the route no longer answers for other users'
devices).
Also: the collections books-grid partial uses a light loader —
requested library validated against the visible set directly instead of
the page-only library resolution (folder reachability, counts,
dropdown payload), removing the extra queries that made library
switching hold the spinner longer.
And the dashboard library switch now folds the This Week widget's
htmx refresh behind the spinner (refreshWeekWidget awaits the widget's
swap, with a safety timeout) instead of letting it pop in after the
fade-in — the two-stage update introduced with the widget's
library-scoped re-fetch.
Guardrail: the rendering lint now counts all innerHTML assignments (not
just template-literal ones) — the old pattern undercounted 20 files at
80 sites, including .join("") builders like renderDashboardCollections.
Baseline rebalanced accordingly.
They never did: filterConflicts/toggleAllConflicts/updateBulkActions
were referenced in the templ but defined nowhere, and the bulk buttons
called bulkResolve()/bulkDismiss() with no arguments against functions
expecting (strategy, conflictIds).
- conflict cards carry data-status; the status dropdown filters them
client-side (pure client state — allowed to be JS), applied on page
init and re-applied after every partial refresh, with a
'no conflicts match this filter' note when everything is filtered out
- select-all toggles only the visible cards; the bulk bar shows the
selected count and resolve/dismiss now gather the checked ids and the
chosen strategy (the bar's own buttons use resolveSelected()/
dismissSelected() wrappers)
- dead onchange= globals became Alpine @change handlers on the server-
rendered controls; partial swaps re-bind via Alpine.initTree (already
in refreshConflicts) and re-apply filter + bulk state
Visible change: the dropdown's stated default (Unresolved Only) now
actually applies on load, so resolved conflicts are hidden until the
filter is changed.
The library admin page carried an inline <script> block of global
functions (toggleLibraryPanel, openEditModal, openDeleteModal,
openFolderBrowser plus the delete-modal close listener). They move into
the admin Alpine component as typed methods; the page body gains
x-data="admin" and the call sites switch onclick= -> @click.
Behavior unchanged.
Guardrail: make lint-rendering (wired into make test) fails when a
web/src file builds more HTML template literals than its recorded
baseline (web/src/rendering-allowlist.txt, seeded at 18 sites across 9
files post-twin-deletion) or when a .templ file gains an inline script
block. theme_flash_guard.templ is the only sanctioned inline script;
web/src/reader is a sanctioned island.
theme.ts: Array.from(classList) — DOMTokenList spread needs
lib.dom.iterable, which the tsconfig does not include.
Both pages had a templ render (initial load) and a TypeScript twin
(innerHTML rebuild on refresh) of the same fragment. The twins are
deleted; the server-rendered markup is now the only markup.
- collections: the library switcher swaps in the new books-grid partial
(GET /collections/:id/books-grid) instead of re-rendering cards from
the /api/collections JSON. Book loading is extracted into
Config.loadCollectionBooks, shared verbatim by the page and the
partial (smart / dashboard-query / manual branches, family-lens
scoping via resolveLibrary + UserVisibleLibraryIDs) so the two cannot
drift.
- conflicts: resolve/dismiss refreshes swap the new list partial
(GET /conflicts/list) with the stats bar as an out-of-band swap,
instead of the TS twin — which rendered a degraded card (no
checkboxes, no timestamps), pointed at a window.showResolveModal that
does not exist, and updated stats via element ids the SSR never had.
- shims.d.ts: window.htmx typing (ajax/process) for TS callers.
Visibility note: the books-grid partial inherits its family-lens
scoping from the shared loader (identical to the detail page); a
dedicated hidden-library pin test for the partial route is still worth
adding in the next test pass.
Codifies the standing rule discussed for the templ/JS split: content
derived from server data is rendered by templ and refreshed via templ
partials (htmx.ajax), pure client-local state may render in TS/Alpine,
web/src is TypeScript-only, and inline <script> in templ files requires
a pre-paint justification (theme_flash_guard.templ only). Lists the
sanctioned JS islands (reader, Chart.js, toasts, password validation).
Also refreshes gotcha #1: the 'harmlessly masked' aside is stale — the
quoted-attribute sweep fixed every occurrence; the note now points at
templates.JSONArg for free text.
templ does not interpolate { expr } inside quoted attribute values — the
braces ship as literal text (AGENTS.md gotcha #1). Swept every templ
source for the pattern and fixed all of it:
Body theme classes: 22 page bodies rendered class="theme-{ user.Theme }"
literally, so every page first-painted the :root default (Tokyo Night)
and flashed when main.js applied the real theme. All now render
class={ "theme-" + user.Theme } server-side.
Attribute interpolations (26 across 8 files):
- api_explorer: initAPIExplorerDoc path/body/response and the Copy
Request/Response buttons (invalid JS, silently dead)
- collections: book-selection :checked/@change/@click ids and four
hx-get search URLs (collection_id={ collection.ID })
- conflicts: Resolve button id
- devices: Kobo sync-url/auth-token element ids (copy lookups only
worked because the id and the lookup were the same literal)
- unlinked_books: search-matches/manual-link clicks and container ids
- queue: status/priority badge classes never matched their CSS
- dashboard: Restore-section tooltip text
- collection_rules/collections: collection icon color
Free text goes through the new templates.JSONArg (json.Marshal) —
templ.JSONString in this templ version returns (string, error) and
cannot compose in attribute expressions; uuids via device.ID.String().
Includes regen-only drift from running templ generate (error-path
normalization) on files with no source change.
Expand the catalog from 11 themes (10 dark, 1 light) to 37 (24/13):
all 18 remaining Omarchy defaults (palettes from each theme's
colors.toml) plus 8 canonical light themes (Tokyo Night Day, Solarized
Light, One Light, Ayu Light, GitHub Light, Gruvbox Light, Everforest
Light, Kanagawa Lotus). Lupine leads the light list as the default
light theme; the app default stays tokyo-night.
Appearance panel: Dark/Light segmented filter (SSR-initialized from the
user's current theme), scrollable theme list, Bookshelf section pinned;
active row revealed on open. Light themes carry color-scheme: light and
the soft-shadow token set. Public-page theme selects get Dark/Light
optgroups. New moon/sun icons.
tailwind.config: safelist the new theme ids — Tailwind v3 purges custom
@layer base class selectors that appear in no scanned content file, so
an unsafelisted id silently ships without its CSS (found the hard way).
theme.ts: applyTheme now swaps only theme-* classes (changing theme
from the login select no longer wipes min-h-screen) and keeps the class
in sync on <html> for the new ThemeFlashGuard — a pre-paint inline
script on the four public pages that applies the saved theme before
first paint.
Kanagawa's UI accent uses crystalBlue (#7e9cd8) instead of Omarchy's
foreground-as-accent so links stay distinguishable from text.
Host-side test runs use CWD=cmd/server/tests, three levels below the repo
root; the location search only went up two, so it never found
database/schema/schema.sql from there and init() fatal-exited. That gap
was being papered over by an untracked local symlink
(cmd/server/tests/database -> ../../../database) hidden behind a bare
`database` line in .git/info/exclude — which also broke `git add` under
database/. Fix the search list instead and drop the filesystem trick:
no symlink to commit, no exclude hack, works on any platform.
Verified: contract pins green from cmd/server/tests with no symlink
present (init resolves via the new entry); parse test green.
audio_metadata (media_items) and audio_offset_seconds (reading_progress)
were declared only inside their CREATE TABLE blocks — fresh installs got
them, upgraded installs never did, because columns reach existing
databases only via the idempotent ALTER block that schema.sql re-runs on
every boot. On prod (v1.15.0) this 42703'd every media-existence check
and the archive pass (1817/1817 scan errors), and every progress save,
since the shared progress upsert names audio_offset_seconds.
Both ALTERs match the declared shapes exactly (nullable JSONB; INTEGER
CHECK (audio_offset_seconds >= 0)); idempotent, applied on every boot.
Verified against a scratch database built to the v1.14.0 shape: overlay
the patched schema -> both columns created; re-run (every boot) skips
cleanly; probes resolve reading_progress.audio_offset_seconds integer
and media_items.audio_metadata jsonb. Parse test and a real container
boot (38 tables, 6 functions verified) green.
Every web-API write endpoint hardcoded Source:"web", so the Android
app's own pushes landed with last_sync_source/last_modified_source
"web" — the app was indistinguishable from the web SPA in progress
conflicts, device_sync_data, and annotation bookkeeping. The app CAN
say who it is (bookmark create already carried origin → origin_source);
the server just ignored it for labeling.
All six write sites (progress PUT, bookmark/note/highlight create+PUT)
now parse an ADDITIVE optional "origin" key, normalized through one
gate: "android"/"ios" pass through, anything absent/unknown stays
"web" (the label keys conflict claims and device_sync_data — free-form
client strings would pollute them). Precedence on progress: a paired
X-Device-Token still outranks origin. Bookmark origin_source now rides
the same gate. The web SPA sends no origin — its rows are unchanged;
KOReader/Kobo handlers keep their own labels.
Wire discipline: additive request keys only (rule 2), response shapes
untouched, pins changelog widened in-commit, locked by
TestOriginWriterAttribution (android lands, default stays web, unknown
normalizes). Full battery: only the two known pre-existing failures
(TestOPDSLibraryFolders rot, TestWebSocketPingPong — both reproduce
with the change stashed).
The stats page's Library-tab switcher (and every other native
<select>) rendered its dropdown LIST in the OS light scheme in dark
themes: the closed control is themed by its own classes, but the
popup list is UA-rendered and ignores page CSS unless the document
declares color-scheme (Chrome path) or the options carry explicit
colors (Firefox path). Both are now in place:
- :root { color-scheme: dark } (the default palette — Tokyo Night —
and all eleven themes except latte are dark),
- .theme-catppuccin-latte { color-scheme: light },
- select option { background-color: var(--bg-secondary); color:
var(--text-primary) } as the belt for engines where the popup
escapes the document cascade.
style.css regenerated via build:css:prod; verified served by the
rebuilt container.
VisibleLibrarySet becomes a package-level function taking the queries
(db-only services like statistics reach it without new wiring); the
LibraryService method stays as the ergonomic entry point. Statistics'
ids-shaper and the reader pages' three hand-rolled membership loops
(empty-rows fallback then linear scans) now delegate; the gates keep
their 403-on-invisible semantics (>null set membership, never silent
pass).
Census addendum from this pass: two OPDS sites call
GetUserVisibleLibraries directly but consume FULL ROWS (per-library
folder enumeration) — a third legitimate pattern alongside set/ids and
resolveLibrary's active-selection; documented, untouched. The library
LIST endpoint and its SSR data helper are row consumers too, by design.
collections.userVisibleLibraryIDs keeps its identity adapter and
delegates; the router's visibleLibraryIDsFor likewise (its identity
source is the session user_id string, the site-specific part it keeps).
The SSR site made the unification slog-proof: resolveLibrary — the SSR
one-of-a-kind (active selection + switcher rows + offline detection) —
stays untouched per the semantic diff.
Series' visibleLibrarySet was the third hand-rolled set builder (with
its own FromBytes-error idiom, the fourth idiom in four copies).
SeriesService.VisibleLibraries — a pure pass-through to
GetUserVisibleLibraries — now has zero callers and dies with the
migration.
One core for "which libraries can this user see": family-lens default-
allow stays where it always was (the GetUserVisibleLibraries SQL);
the core adds the shared set+ids shaping that four endpoints each
hand-rolled. Media's visibleLibrarySet is now a thin identity adapter
(JWT user object -> resolved id) and the download-permission gate uses
the set instead of a manual membership loop.
Semantic diff (H4.5 step 1, in the plan): zero family-lens divergence
found across all six sites — the rule lives in one query; the variation
was shape-level (identity adapter, set-vs-ids, UUID idiom).
The search feed carried a ~55-line inline copy of the acquisition-entry
skeleton, over a DIFFERENT sqlc row type (SearchMediaItemsRow vs
ListMediaItemsByLibraryRow) — the row-type split is why it stayed a
twin. The shared builder now takes opdsEntryInput, the narrow
ten-field subset the feed actually renders (deliberately not a row
converter), and acquisitionEntryOpts makes the two legitimate
differences explicit instead of incidental:
- labelLibraries: unscoped search labels entries with their library.
- includePdfAlternate: catalogs offer the PDF alternate for reflowable
books; search historically does not. Kept as-is — making search match
catalogs would be a behavior change, flagged in the opts doc.
The four OPDS behaviors tests are green in the test-runner container.
Flagged, not from this change: TestOPDSLibraryFolders fails identically
with the change stashed — pre-existing rot worth its own look (it never
ran in the hardening batteries because the -run patterns didn't match
it).
shouldEnablePanelDetection and ShouldTreatAsComic had identical truth
tables (manga/comics library x fixed_layout/comic_archive) under
different names — a drift trap: adding a library type to one silently
missed the other. One predicate now gates both features, with the
re-split instruction in its doc comment should the features ever
legitimately diverge.
The pending-hash-conflict walk existed twice (JSON admin API emitting
numeric counts, SSR page emitting a formatted summary) — deliberately
different shapes, but both duplicated the counts-or-zero lookup and
the five-field total. MediaItemUsageCountsTotal is the shared read;
the per-surface shaping stays local on purpose.
The H4 row-completion commit made abridged/audio_metadata emit on the
search wire (the endpoint serializes the row directly) — exactly what
the pin exists to surface. Whitelisted per the additive rule with the
changelog entry.
The unified search row lagged the table (survey overstated it —
narrators was already there). Both columns come from the
already-scanned media_items side of the joins: EXPLAIN before/after is
plan-identical (same Nested Loop shape, no new scans). Row-completeness
only — no client surface changes; the hand maps select their own keys.
SELECT mi.*, ci.excluded forced every caller to filter tombstone
exclusions in Go over a 69-field row, then a 45-field hand converter
(getCollectionItemsRowToMediaItems) copied it field-by-field into
MediaItems for BuildSections. The query now filters
(ci.excluded IS NOT TRUE) itself and returns sqlc.embed(mi) — callers
consume .MediaItems directly, the Go filter loops die in both
collection-detail ladders (JSON + SSR), and the converter is deleted.
Pins, H1/H3 tests, unit suites green.
checkRulesAgainstBook/evaluateRule and the service's EvaluateRules
existed in two parallel universes: one over database.MediaItems (what
every query actually returns) and one over ListMediaItemsRow (what two
hand-written 45-field mediaItemsToListMediaItemsRow converters
produced — verbatim twins in the handler and the dashboard service,
with a URL-resolution side effect baked into a "converter"). The rule
fields (genre/series/author/language/publisher/copyright_year/tags)
all exist on MediaItems, so the evaluators now take the real row and
the converters, their three call-site loops, and the dead
ApplyAutoAssignRules entry point (zero callers) are gone.
The rules surface is wire-covered by the collections rules scenarios;
handlers+services suites and the full pin/H1/H3 battery green.
ListConflicts, GetConflict, and the SSR media-detail active-conflict
lookup each hand-mapped their sqlc row (list-by-user, get-by-id,
list-by-media-item) into ConflictDetailResponse, triplicating the
optional resolution tail — including the silently-swallowed
ResolutionData unmarshal error, three times. NewConflictDetail is the
single mapper; a malformed resolution blob now logs instead of
vanishing.
Also extracts koreaderSyncEndpoints (the KOReader URL map duplicated
verbatim between token rotation and device approval; kobo's two URL
sets stay local because they deliberately differ).
Verified in the test-runner container (host conflict tests fail
environmentally on the container-only uploads path — baseline behavior).
The same pgtype unwrap-and-fill block (sync_enabled/auto_sync validity,
frequency fallback, pointer timestamps) existed at five DeviceInfo
construction sites, two of which also carried AuthToken.
deviceToInfo(device, includeAuthToken) replaces all five; token-bearing
lists pass the flag explicitly so the surface that exposes the token
stays a deliberate choice.
setOfflineMode/setOnlineMode only mean to flip sync_enabled, but they
called the full-row UpdateDevice with Go zero values — which wrote
DeviceName: "" and NULLed auto_sync/sync_frequency_minutes/
device_metadata on the device row at every offline->online flip. New
targeted SetDeviceSyncEnabled query touches exactly that column; the
real settings update keeps using UpdateDevice.
Caught as part of the H4 helper-census pass (plan §5.4 erasure list).
handlers textToString, router textToString, router getText, and utils
normText were four copies of the same NULL-to-empty unwrap (99 call
sites). One exported utils.PgTextString replaces them all; the private
normText (comparison normalizer) stays but is now the odd one out.
Harness fix found on the way: setupTestServer built a full wiring
literal for route registration and then RETURNED a second, sparser
literal that never copied AnnotationService — setup.AnnotationService
was always nil on the struct every test receives. Nobody noticed until
cross_library_move_test became the first caller (it previously used
the now-retired plain CreateMediaHighlight query); it now exercises
the real SaveHighlight path through the wired service.
H3 leftover: the annotation handlers route everything through the sync
service (dedup/LWW/tombstones), leaving the plain Create/Update/Delete
queries for media_notes/media_highlights/media_bookmarks with zero
callers repo-wide (handlers, services, tests). Queries.sql drops them
so H4's row work doesn't carry dead weight; sqlc regen compiles clean,
proving nothing referenced the generated stubs.
Also removes the literal duplicate `AND mi.missing_scan_count = 0`
line in the library-stats query (harmless but confusing).
Caught by the live round-trip sweep: a dedup-key re-save (a device echo
or a client re-POSTing a bookmark it already had) carried no locator
fields, and the full-column sync update wrote zeroes — which
pgFloat8/pgInt4 turn into NULLs — erasing the stored
percentage/page/cfi on every echo. The highlight stack had string
coalescing for exactly this reason (its locatorRefresher doctrine
comment); the numeric columns and the two cloned stacks never got it.
One primitive now carries the whole doctrine: coalesce[T comparable]
(zero value -> stored value), applied at the single write point of
each LWW apply — highlight (percentages, color, note_text,
chapter_reference added to its existing string coalescing), bookmark
(page/chapter/percentage/epubcfi/chapter_reference added to its
cfi/position pair), note (all locators). The redundant pre-coalescing
in the id-targeted resolution blocks is removed — the apply is the one
coalesce point, so dedup-path and id-path edits behave identically.
Verified live on the rebuilt container: dedup re-save and legacy
3-field PUTs preserve every stored locator on all three stacks; an
explicitly-sent field still lands. Pins, H1/H3 tests, unit suites
green.
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 last behavior gap in the H2 batch: has_conflict was part of the
frozen card shape but stamped false-forever on these three JSON
surfaces (only dashboard sections, search, and the SSR pages had it),
so every conflict-driven card action — the resolve-conflict CTA in
BookCard — never fired there. MarkActiveConflicts now runs after the
builder on GET /api/collections/:id, series books, and smart-collection
preview (all user-scoped; the endpoint already authenticates).
Key set is unchanged — has_conflict was always emitted, this only
makes its value truthful. All 15 contract pins + H1 tests green; the
pin's bookInfoKeys freeze unaffected.
All five branch shapes of GET /collections/:id now build cards via the
cards.go adapters: smart (BookCardsFromMediaItems), system-query (same
— gains format_group/badge it previously dropped), scoped
(BookCardsFromCollectionDashRows with the Excluded filter kept), and
the all-libraries branch whose rows distinguish ID vs MediaItemID
(BookCardsFromCollectionItemRows). LibraryTypeMap hoisted to once per
page; both []BookInfo{} error seeds die with the adapters' non-nil
empties.
The ExecuteSearch-driven bookshelf page fed SearchMediaItemsUnifiedRow
— which HAS FormatGroup and LibraryTypeName — into a bare 4-field
literal that dropped both, so bookshelf cards lost the media-type
badge the shelf once had. BookCardsFromUnified emits badge/format on
this surface (card-marking rides on; the SQL column strategy means
zero extra queries). audiobook shelves regain the corner glyph.
Both BrowseDetail surfaces hand-rolled bare 4-field literals (badges
dropped) and patched the nil-slice with []BookInfo{} seeds after.
BookCardsFromMediaItems feeds GetSeriesBooks/GetBooksByTag rows —
cards gain format_group + badge on the series and tag pages, and the
two seed blocks die with the adapter's non-nil empties. Conflict
stamping at these sites was already adjacent and is preserved.
router/search.go (the templ BooksGrid fragment surface) now uses
BookCardsFromUnified — the unified row's LibraryTypeName column gives
the badge with no LibraryTypeMap query; also drops that site's
needless pgtype.UUID byte-roundtrip on the cover URL.
The dashboard section builder — feeding both the dashboard JSON pin
surface and the SSR dashboard — now calls BookCardsFromMediaItems on
ds.Items. libTypes already arrives as a BuildSections parameter
(once per request); the literal only duplicated the badge lookup.
GET /api/series/detail?name=… books array via BookCardsFromMediaItems.
Series had the third LibraryTypeMap call INSIDE its per-item loop —
ListLibraries ran once per book on the page; now once per request.
POST preview-feed had the ladder's 14th hand-rolled literal. Now
BookCardsFromMediaItems + one LibraryTypeMap per request; the response
carries format_group/badge values for the first time (all seven keys
were already emitted as zero-value JSON, so this is a value change on
a pin-frozen shape, not a key change).
GET /api/collections/:id books — all four branches now build cards via
the cards.go adapters. LibraryTypeMap hoisted to once per handler (it
was 1 query per item in the smart and no-library branches). The system
branch — which before stamped cards BARE (no format_group, no badge —
the ladder's only JSON gap) — now emits the same card shape as its
siblings by feeding section.Items through BookCardsFromMediaItems,
matching what BuildSections produces for the identical data. The
collection-error seed dies via the adapters' non-nil empty slices;
the no-matching-section nil result is preserved as-is (that branch
never seeded — today's 'items': null stays today's 'items': null).
Pins: CollectionsJSON, DashboardSections, MediaListBothBranches green.
BookInfo, LibraryTypeMap, and MediaBadgeKind move verbatim out of
collections.go into handlers/cards.go (the card package surface, same
package: no import-graph or templ changes), joined by the H2 builder:
- buildBookCard (unexported): the ONLY place BookInfo is constructed,
fed by cardSource; cover URL via ResolveMediaURL, badge via
MediaBadgeKind — both inside the builder, not per call site.
- Four exported adapters normalize the four real feed shapes:
database.MediaItems (sections/smart/series/tags/preview),
GetCollectionItemsForDashboardRow (collection dashboard branch),
GetCollectionItemsRow (collection-items joins — MediaItemID field),
SearchMediaItemsUnifiedRow (badge rides its LibraryTypeName column,
so no LibraryTypeMap query at all).
- Adapters return non-nil empty slices, which retires the per-site
[]BookInfo{} empty-seed assignments when call sites migrate.
No call site changes in this commit — the 20 literals and 5 seeds are
migrated one surface per commit next, each with the H0 pin suite.
Verified: build, vet (only the two pre-existing flagged saveCoverImage
warnings), and 14/14 contract pins — the key freeze is byte-identical.
2026-10-17 lessons from the hardening session: most test/bruno failures
were harness/schema/environment drift (drifted body keys, truncated
payloads, container-only paths), found by triaging the test before the
server. Codified as: triage order (test schema -> environment -> cheap
probe), with the guard against "writing tests to work" — test/contract
edits are asymmetric with code fixes: only harness plumbing fixes,
additive pin widenings with a changelog entry, or deletions of tests of
removed behavior. Pins were green at a known commit; git history
arbitrates stale-vs-changed. A right-looking test that fails is prima
facie a server bug: stop and flag.
contract_pins.md rule 4: narrowing/removing a pinned key requires the
user's explicit sign-off — a pin freezing buggy behavior is a ledger,
not a license.
John O'Keefe