3 Commits
Author SHA1 Message Date
john-okeefe e40530824e feat(koreader): one conversion route for every feature; bookmarks get web CFIs
Release / build-and-push (push) Successful in 2m14s
Progress, highlights, notes, and bookmarks entered position conversion
through three different doors: progress converted inline with an
uncached converter, annotations through the facade, bookmarks not at
all (the raw xpointer was stored verbatim, cfi_position stayed empty,
and the web drawer's goToBookmark silently no-ops on cfi-less entries).

Unify on the facade (ConvertToCanonical/ConvertFromCanonical):

- annotationEpub context resolved once per push: media item + EPUB path
  shared by every annotation instead of re-fetched per entry
- progress forward: the inline block becomes one facade call;
  non-reflowable formats pass through unchanged, and the cached
  converter stops re-parsing the book on every sync
- progress reverse: convertCFIToXPointer delegates to reverseConvertCFI,
  keeping the stored percentage in play for the fallback ladder
- bookmarks (bulk progress and /sync-bookmarks): pos0 resolves
  structural-only — bookmark text is a display label, never book text,
  so no context is supplied; webUsableCFI stores the result only for
  structural/exact epubcfi landings, discarding href/percentage results
  rather than storing dead drawer links. Also records percentage_location
  and origin_source on the legacy endpoint.
- percentages thread through: highlights/notes/bookmarks pass the device
  percentage or the derived section percentage instead of a hardcoded 0,
  so the last-resort fallback lands near the true position instead of
  the document start
- extendCFIByLength end-derivation now also fires on structural starts
  (it had silently stopped matching when the structural rung began
  landing starts with precision 'structural' rather than 'exact')

Tests: the drop-cap xpointer through the facade with empty context (the
bookmark scenario) must land structurally, not doc-start; webUsableCFI
table covers the store/discard gate.
2026-09-09 09:04:23 -04:00
john-okeefe 5a6c361c11 perf(sync): share the bounded converter cache for section percentages
Per-annotation percentage derivation (deriveAnnotationPercentage) built a
fresh CFIConverter for every highlight/note/bookmark, re-reading and
re-parsing the whole EPUB each time. Export SectionPercentageCached so
handlers reach the same bounded cache ConvertToCanonical already uses
(8 books, insertion-order eviction): one parse per book per push instead
of one per annotation.
2026-09-09 09:04:10 -04:00
john-okeefe 7b1c809ae3 feat(bookmarks): location identity, origin provenance, KOReader-style labels
- 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
2026-09-09 08:17:43 -04:00
12 changed files with 338 additions and 125 deletions
+12 -2
View File
@@ -1344,8 +1344,10 @@ CREATE TABLE IF NOT EXISTS media_bookmarks (
title VARCHAR(255) NOT NULL,
position VARCHAR(100), -- 'pdf:page:45', 'comic:page:12', 'chapter:3' for consistency
notes TEXT,
created_at TIMESTAMPTZ DEFAULT NOW(),
UNIQUE(media_item_id, user_id, title)
created_at TIMESTAMPTZ DEFAULT NOW()
-- No UNIQUE(media_item_id, user_id, title): bookmarks are identified by
-- their location (dedup_key), titles are display labels shared verbatim
-- across clients (KOReader auto-labels repeat within a chapter).
);
CREATE INDEX IF NOT EXISTS idx_media_bookmarks_media ON media_bookmarks(media_item_id);
@@ -1386,6 +1388,14 @@ ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS epubcfi_location TEXT;
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS chapter_reference INTEGER;
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS deleted BOOLEAN DEFAULT FALSE;
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS deleted_at TIMESTAMPTZ;
-- Provenance: the client that CREATED the bookmark (unlike
-- last_modified_source, which tracks the last writer). Set once at insert.
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS origin_source VARCHAR(30);
-- Identity is the dedup_key (location), not the title; drop the legacy
-- unique-title constraint so same-title bookmarks on different pages can
-- coexist (re-creating over a tombstone with a changed position also
-- relied on this). Catalog-only change, safe to re-run.
ALTER TABLE media_bookmarks DROP CONSTRAINT IF EXISTS media_bookmarks_media_item_id_user_id_title_key;
CREATE UNIQUE INDEX IF NOT EXISTS idx_media_highlights_dedup
ON media_highlights (user_id, media_item_id, dedup_key)
+1
View File
@@ -185,6 +185,7 @@ type MediaBookmarks struct {
ChapterReference pgtype.Int4 `db:"chapter_reference" json:"chapter_reference"`
Deleted pgtype.Bool `db:"deleted" json:"deleted"`
DeletedAt pgtype.Timestamptz `db:"deleted_at" json:"deleted_at"`
OriginSource pgtype.Text `db:"origin_source" json:"origin_source"`
}
type MediaHighlights struct {
+18 -9
View File
@@ -624,7 +624,7 @@ func (q *Queries) CreateLibrary(ctx context.Context, arg CreateLibraryParams) (L
const CreateMediaBookmark = `-- name: CreateMediaBookmark :one
INSERT INTO media_bookmarks (media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type CreateMediaBookmarkParams struct {
@@ -670,6 +670,7 @@ func (q *Queries) CreateMediaBookmark(ctx context.Context, arg CreateMediaBookma
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
@@ -680,10 +681,10 @@ INSERT INTO media_bookmarks (
cfi_position, title, position, notes,
percentage_location, epubcfi_location, chapter_reference,
dedup_key, last_modified_at, last_modified_source,
device_sync_data
device_sync_data, origin_source
) VALUES (
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
) RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16
) RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type CreateMediaBookmarkFullParams struct {
@@ -702,6 +703,7 @@ type CreateMediaBookmarkFullParams struct {
LastModifiedAt pgtype.Timestamptz `db:"last_modified_at" json:"last_modified_at"`
LastModifiedSource pgtype.Text `db:"last_modified_source" json:"last_modified_source"`
DeviceSyncData []byte `db:"device_sync_data" json:"device_sync_data"`
OriginSource pgtype.Text `db:"origin_source" json:"origin_source"`
}
func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBookmarkFullParams) (MediaBookmarks, error) {
@@ -721,6 +723,7 @@ func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBo
arg.LastModifiedAt,
arg.LastModifiedSource,
arg.DeviceSyncData,
arg.OriginSource,
)
var i MediaBookmarks
err := row.Scan(
@@ -743,6 +746,7 @@ func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBo
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
@@ -4524,7 +4528,7 @@ func (q *Queries) GetLibraryWithType(ctx context.Context, id pgtype.UUID) (GetLi
}
const GetMediaBookmark = `-- name: GetMediaBookmark :one
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at FROM media_bookmarks WHERE id = $1
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source FROM media_bookmarks WHERE id = $1
`
func (q *Queries) GetMediaBookmark(ctx context.Context, id pgtype.UUID) (MediaBookmarks, error) {
@@ -4550,13 +4554,14 @@ func (q *Queries) GetMediaBookmark(ctx context.Context, id pgtype.UUID) (MediaBo
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
const GetMediaBookmarkByDedupKey = `-- name: GetMediaBookmarkByDedupKey :one
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at FROM media_bookmarks
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source FROM media_bookmarks
WHERE user_id = $1 AND media_item_id = $2 AND dedup_key = $3
ORDER BY deleted ASC, deleted_at DESC NULLS LAST
LIMIT 1
@@ -4594,12 +4599,13 @@ func (q *Queries) GetMediaBookmarkByDedupKey(ctx context.Context, arg GetMediaBo
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
const GetMediaBookmarks = `-- name: GetMediaBookmarks :many
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at FROM media_bookmarks
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source FROM media_bookmarks
WHERE media_item_id = $1 AND user_id = $2 AND COALESCE(deleted, FALSE) = FALSE
ORDER BY created_at DESC
`
@@ -4638,6 +4644,7 @@ func (q *Queries) GetMediaBookmarks(ctx context.Context, arg GetMediaBookmarksPa
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
); err != nil {
return nil, err
}
@@ -11513,7 +11520,7 @@ SET
position = $4,
last_modified_at = NOW()
WHERE id = $1 AND user_id = $5
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type UpdateMediaBookmarkParams struct {
@@ -11553,6 +11560,7 @@ func (q *Queries) UpdateMediaBookmark(ctx context.Context, arg UpdateMediaBookma
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
@@ -11575,7 +11583,7 @@ UPDATE media_bookmarks SET
deleted = FALSE,
deleted_at = NULL
WHERE id = $1
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type UpdateMediaBookmarkForSyncParams struct {
@@ -11631,6 +11639,7 @@ func (q *Queries) UpdateMediaBookmarkForSync(ctx context.Context, arg UpdateMedi
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
+2 -2
View File
@@ -898,9 +898,9 @@ INSERT INTO media_bookmarks (
cfi_position, title, position, notes,
percentage_location, epubcfi_location, chapter_reference,
dedup_key, last_modified_at, last_modified_source,
device_sync_data
device_sync_data, origin_source
) VALUES (
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16
) RETURNING *;
-- name: UpdateMediaBookmarkForSync :one
+144 -108
View File
@@ -50,35 +50,88 @@ func (h *KOReaderHandler) SetAnnotationService(svc *wsync.AnnotationService) {
h.annotationSvc = svc
}
func (h *KOReaderHandler) convertHighlightPositions(ctx context.Context, mediaItemID pgtype.UUID, pos0, pos1, contextText string) (string, string) {
if pos0 == "" || h.libraryService == nil {
return "", ""
}
// annotationEpub carries the per-book context every locator conversion
// needs: the media item (format gating) and the resolved EPUB path. It is
// resolved once per request so all annotations in a push share one
// converter-cache entry instead of re-resolving (and re-parsing the book)
// per annotation.
type annotationEpub struct {
mediaItem *database.MediaItems
epubPath string
}
func (ec annotationEpub) convertible() bool {
return ec.mediaItem != nil && ec.epubPath != ""
}
func (h *KOReaderHandler) loadAnnotationEpub(ctx context.Context, mediaItemID pgtype.UUID) annotationEpub {
var ec annotationEpub
mediaItem, err := h.db.GetMediaItem(ctx, mediaItemID)
if err != nil {
return ec
}
ec.mediaItem = &mediaItem
if h.libraryService != nil {
if epubPath, err := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath); err == nil && epubPath != "" {
ec.epubPath = epubPath
}
}
return ec
}
// convertHighlightPositions resolves a device annotation's pos0/pos1
// locators to canonical CFIs through the shared facade. contextText is the
// selection's own text — the ideal anchor for the converter's verification
// and text-search rungs. percentage anchors the last-resort fallback so a
// failed conversion degrades to the neighborhood of the true position
// rather than the document start.
func (h *KOReaderHandler) convertHighlightPositions(ec annotationEpub, pos0, pos1, contextText string, percentage float64) (string, string) {
if pos0 == "" || !ec.convertible() {
return "", ""
}
epubPath, err := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath)
if err != nil || epubPath == "" {
return "", ""
}
// The annotation's own text is the ideal anchor for the converter's
// text-search path: clients (thin, underpowered) send only raw
// locators, the server resolves them against the actual book.
startLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos0, 0, contextText, mediaItem.FormatGroup, epubPath, "")
endLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos1, 0, "", mediaItem.FormatGroup, epubPath, "")
startLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos0, percentage, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
endLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos1, percentage, "", ec.mediaItem.FormatGroup, ec.epubPath, "")
endCFI := endLoc.CFI
// The end conversion carries no context text, so unless it resolved
// exactly it degenerates to a percentage fallback anchored at the
// document start — useless as a range end. When the START resolved
// exactly, derive the end from it: same node, character offset
// advanced by the selection's UTF-16 length (the CFI offset unit).
if endLoc.Precision != "exact" && startLoc.Precision == "exact" && contextText != "" {
// structurally/exactly, derive the end from it: same node, character
// offset advanced by the selection's UTF-16 length (the CFI offset
// unit).
if endLoc.Precision != "exact" && endLoc.Precision != "structural" &&
(startLoc.Precision == "exact" || startLoc.Precision == "structural") && contextText != "" {
endCFI = extendCFIByLength(startLoc.CFI, contextText)
}
return startLoc.CFI, endCFI
}
// convertBookmarkPosition resolves a device bookmark's locator to the
// canonical CFI for the web reader. Structural-only by design: bookmark
// text is a display label ("in <chapter>" auto-fill or a user note), never
// book text, so no context is supplied and only a structural/exact landing
// is trusted — lower rungs would store a confident-looking guess the web
// drawer would present as a real destination.
func (h *KOReaderHandler) convertBookmarkPosition(ec annotationEpub, position string, percentage float64) string {
if position == "" || !ec.convertible() || !wsync.IsCREXPointer(position) {
return ""
}
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, position, percentage, "", ec.mediaItem.FormatGroup, ec.epubPath, "")
return webUsableCFI(loc)
}
// webUsableCFI keeps only high-confidence conversions: the web reader
// navigates bookmarks by CFI, so href/percentage/fallback results are
// discarded instead of stored as dead links.
func webUsableCFI(loc wsync.CanonicalLocator) string {
if loc.Precision != "structural" && loc.Precision != "exact" {
return ""
}
if !strings.HasPrefix(loc.CFI, "epubcfi(") || !strings.HasSuffix(loc.CFI, ")") {
return ""
}
return loc.CFI
}
// extendCFIByLength advances a point CFI's trailing character offset by the
// UTF-16 length of text (EPUB CFI character offsets are UTF-16 code units).
// Selections spanning multiple text nodes produce an out-of-range offset —
@@ -127,22 +180,19 @@ func (h *KOReaderHandler) existingHighlightColor(ctx context.Context, mediaItemI
// deriveAnnotationPercentage computes a percentage for device-pushed
// annotations when the client didn't send one (thin clients skip their own
// per-annotation page lookups; arithmetic is only free on paging documents).
func (h *KOReaderHandler) deriveAnnotationPercentage(ctx context.Context, mediaItemID pgtype.UUID, pos0 string, page int) float64 {
mediaItem, err := h.db.GetMediaItem(ctx, mediaItemID)
if err != nil {
func (h *KOReaderHandler) deriveAnnotationPercentage(ec annotationEpub, pos0 string, page int) float64 {
if ec.mediaItem == nil {
return 0
}
formatGroup := wsync.FormatGroup(mediaItem.FormatGroup)
formatGroup := wsync.FormatGroup(ec.mediaItem.FormatGroup)
if formatGroup == wsync.FormatGroupFixedLayout || formatGroup == wsync.FormatGroupComicArchive {
if page > 0 && mediaItem.PageCount.Valid && mediaItem.PageCount.Int32 > 0 {
return float64(page) / float64(mediaItem.PageCount.Int32)
if page > 0 && ec.mediaItem.PageCount.Valid && ec.mediaItem.PageCount.Int32 > 0 {
return float64(page) / float64(ec.mediaItem.PageCount.Int32)
}
return 0
}
if wsync.IsCREXPointer(pos0) && h.libraryService != nil {
if epubPath, err := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath); err == nil && epubPath != "" {
return wsync.NewCFIConverter(epubPath).SectionPercentage(pos0)
}
if wsync.IsCREXPointer(pos0) && ec.epubPath != "" {
return wsync.SectionPercentageCached(ec.epubPath, pos0)
}
return 0
}
@@ -614,21 +664,23 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
if h.annotationSvc == nil {
return
}
ec := h.loadAnnotationEpub(ctx, mediaItemID)
for _, hl := range book.Highlights {
startPos := hl.Pos0
endPos := hl.Pos1
// The highlight's own text anchors the conversion exactly.
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ctx, mediaItemID, startPos, endPos, hl.Text)
pctStart := 0.0
if hl.Percentage != nil {
pctStart = *hl.Percentage
}
if pctStart == 0 {
pctStart = h.deriveAnnotationPercentage(ctx, mediaItemID, startPos, int(hl.Page))
pctStart = h.deriveAnnotationPercentage(ec, startPos, int(hl.Page))
}
// The highlight's own text anchors the conversion exactly.
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ec, startPos, endPos, hl.Text, pctStart)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": hl.Datetime,
"pos0": hl.Pos0,
@@ -676,16 +728,17 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
for _, note := range book.Notes {
startPos := note.Pos0
endPos := note.Pos1
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ctx, mediaItemID, startPos, endPos, note.Text)
pctStart := 0.0
if note.Percentage != nil {
pctStart = *note.Percentage
}
if pctStart == 0 {
pctStart = h.deriveAnnotationPercentage(ctx, mediaItemID, startPos, int(note.Page))
pctStart = h.deriveAnnotationPercentage(ec, startPos, int(note.Page))
}
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ec, startPos, endPos, note.Text, pctStart)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": note.Datetime,
"pos0": note.Pos0,
@@ -723,6 +776,19 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
position = fmt.Sprintf("page:%d", bookmark.Page)
}
pctLoc := 0.0
if bookmark.Percentage != nil {
pctLoc = *bookmark.Percentage
}
if pctLoc == 0 {
pctLoc = h.deriveAnnotationPercentage(ec, position, int(bookmark.Page))
}
// Device-native xpointer → canonical CFI so the web drawer can
// navigate KOReader-created bookmarks (page-only positions have no
// convertible locator; the drawer's page fallback covers those).
cfiPosition := h.convertBookmarkPosition(ec, position, pctLoc)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": bookmark.Datetime,
"pos0": bookmark.Pos0,
@@ -739,8 +805,11 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
UserID: userID,
Title: bookmark.Text,
Position: position,
CFIPosition: cfiPosition,
PercentageLoc: pctLoc,
ChapterNumber: int32(bookmark.Chapter),
Source: "koreader",
OriginSource: "koreader",
DeviceSyncData: deviceData,
DedupKey: dedupKey,
})
@@ -783,53 +852,24 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
if h.progressSvc != nil {
epubcfi := book.Epubcfi
if epubcfi != nil && wsync.IsCREXPointer(*epubcfi) {
log.Printf("Bookhoard: CRE→CFI attempting conversion for %s", *epubcfi)
mediaItem, err := h.db.GetMediaItem(ctx, mediaItemID)
if err != nil {
log.Printf("Bookhoard: CRE→CFI failed to get media item: %v", err)
} else if mediaItem.FormatGroup == string(wsync.FormatGroupFixedLayout) ||
mediaItem.FormatGroup == string(wsync.FormatGroupComicArchive) {
// Image-based fixed content (fixed-layout comic EPUBs, PDF,
// comic archives) has no extractable text, so CRE→CFI conversion
// cannot succeed. The page index (page/total_pages) is the
// canonical locator. Keep the incoming xpointer for device-native
// restore; the web reader restores by page.
log.Printf("Bookhoard: CRE→CFI skipped for %s format", mediaItem.FormatGroup)
} else if h.libraryService == nil {
log.Printf("Bookhoard: CRE→CFI libraryService is nil, skipping conversion")
} else {
epubPath, resolveErr := h.libraryService.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath)
if resolveErr != nil {
log.Printf("Bookhoard: CRE→CFI failed to resolve media path: %v", resolveErr)
} else if epubPath == "" {
log.Printf("Bookhoard: CRE→CFI resolved empty epub path for %s", mediaItem.FilePath)
} else {
log.Printf("Bookhoard: CRE→CFI resolved epub path: %s", epubPath)
converter := wsync.NewCFIConverter(epubPath)
pct := 0.0
if book.Percentage >= 0 {
pct = book.Percentage
}
contextText := ""
if book.ContextText != nil {
contextText = *book.ContextText
}
result, convErr := converter.ConvertCREToStandard(*epubcfi, pct, contextText)
if convErr != nil {
log.Printf("Bookhoard: CRE→CFI conversion error: %v", convErr)
} else if result != nil {
if result.EPUBCFI != "" {
convertedCFI := result.EPUBCFI
epubcfi = &convertedCFI
log.Printf("Bookhoard: CRE→CFI converted to epubcfi: %s", convertedCFI)
} else if result.Href != "" {
convertedHref := result.Href
epubcfi = &convertedHref
log.Printf("Bookhoard: CRE→CFI converted to href: %s", convertedHref)
} else {
log.Printf("Bookhoard: CRE→CFI conversion: %s precision for %s", result.Precision, *epubcfi)
}
}
// Same facade every annotation uses: cached converter,
// structural-first resolution, guarded fallbacks. The facade
// passes non-reflowable formats through untouched.
ec := h.loadAnnotationEpub(ctx, mediaItemID)
if ec.convertible() {
pct := 0.0
if book.Percentage >= 0 {
pct = book.Percentage
}
contextText := ""
if book.ContextText != nil {
contextText = *book.ContextText
}
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, *epubcfi, pct, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
if loc.CFI != "" && loc.CFI != *epubcfi {
converted := loc.CFI
epubcfi = &converted
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to %s", loc.Precision, converted)
}
}
}
@@ -1168,40 +1208,20 @@ func (h *KOReaderHandler) GetMetadata(c *echo.Context) error {
}
func (h *KOReaderHandler) convertCFIToXPointer(c *echo.Context, mediaItem database.MediaItems, progress database.GetUniversalProgressRow, progressData *KOReaderProgressData) {
if h.libraryService == nil {
log.Printf("Bookhoard: CFI→CRE libraryService is nil, skipping reverse conversion")
return
}
epubPath, err := h.libraryService.ResolveMediaPath(c.Request().Context(), mediaItem.LibraryID, mediaItem.FilePath)
if err != nil {
log.Printf("Bookhoard: CFI→CRE failed to resolve media path: %v", err)
return
}
if epubPath == "" {
log.Printf("Bookhoard: CFI→CRE resolved empty epub path for %s", mediaItem.FilePath)
return
}
converter := wsync.NewCFIConverter(epubPath)
contextText := ""
if progress.ContextText.Valid {
contextText = progress.ContextText.String
}
pct := progress.Percentage.Float64
result, err := converter.ConvertStandardToCRE(progress.Epubcfi.String, pct, contextText)
if err != nil {
log.Printf("Bookhoard: CFI→CRE conversion error: %v", err)
return
}
if result != nil && result.XPointer != "" {
progressData.KoreaderXPointer = &result.XPointer
log.Printf("Bookhoard: CFI→CRE converted to XPointer: %s", result.XPointer)
// Same facade path annotations use on serve: structural resolution
// first, guarded text search only as fallback, cached converter. The
// stored percentage anchors the reverse fallback ladder.
if xp := h.reverseConvertCFI(c, mediaItem, progress.Epubcfi.String, contextText, progress.Percentage.Float64); xp != "" {
progressData.KoreaderXPointer = &xp
log.Printf("Bookhoard: CFI→CRE converted to XPointer: %s", xp)
}
}
func (h *KOReaderHandler) reverseConvertCFI(c *echo.Context, mediaItem database.MediaItems, epubcfi string, contextText string) string {
func (h *KOReaderHandler) reverseConvertCFI(c *echo.Context, mediaItem database.MediaItems, epubcfi string, contextText string, percentage float64) string {
if h.libraryService == nil || epubcfi == "" {
return ""
}
@@ -1209,7 +1229,7 @@ func (h *KOReaderHandler) reverseConvertCFI(c *echo.Context, mediaItem database.
if err != nil || epubPath == "" {
return ""
}
loc := wsync.ConvertFromCanonical(wsync.LocatorSourceKOReader, epubcfi, 0, contextText, mediaItem.FormatGroup, epubPath, "")
loc := wsync.ConvertFromCanonical(wsync.LocatorSourceKOReader, epubcfi, percentage, contextText, mediaItem.FormatGroup, epubPath, "")
if loc.Position != "" && loc.Position != epubcfi {
return loc.Position
}
@@ -1316,7 +1336,7 @@ func (h *KOReaderHandler) koreaderPos0(c *echo.Context, mediaItem database.Media
cfi = strings.TrimPrefix(startPosition, "cfi:")
}
if cfi != "" && wsync.IsStandardEPUBCFI(cfi) {
if converted := h.reverseConvertCFI(c, mediaItem, cfi, contextText); converted != "" {
if converted := h.reverseConvertCFI(c, mediaItem, cfi, contextText, 0); converted != "" {
return converted
}
// Conversion failed; fall through so numeric positions still work.
@@ -1482,6 +1502,17 @@ func (h *KOReaderHandler) SyncBookmarks(c *echo.Context) error {
}
if h.annotationSvc != nil {
ec := h.loadAnnotationEpub(ctx, mediaItemID)
pctLoc := 0.0
if bookmark.Percentage != nil {
pctLoc = *bookmark.Percentage
}
if pctLoc == 0 {
pctLoc = h.deriveAnnotationPercentage(ec, position, int(bookmark.Page))
}
cfiPosition := h.convertBookmarkPosition(ec, position, pctLoc)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": bookmark.Datetime,
"pos0": bookmark.Pos0,
@@ -1493,8 +1524,11 @@ func (h *KOReaderHandler) SyncBookmarks(c *echo.Context) error {
UserID: pgUserID,
Title: bookmark.Text,
Position: position,
CFIPosition: cfiPosition,
PercentageLoc: pctLoc,
ChapterNumber: int32(bookmark.Chapter),
Source: "koreader",
OriginSource: "koreader",
DeviceSyncData: deviceData,
})
if err == nil && result.Outcome != wsync.SaveOutcomeDeleted {
@@ -1588,13 +1622,15 @@ func (h *KOReaderHandler) SyncBookmarks(c *echo.Context) error {
}
if h.annotationSvc != nil {
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ctx, mediaItemID, highlight.Pos0, highlight.Pos1, highlight.Text)
ec := h.loadAnnotationEpub(ctx, mediaItemID)
pctStart := 0.0
if highlight.Percentage != nil {
pctStart = *highlight.Percentage
}
epubcfiStart, epubcfiEnd := h.convertHighlightPositions(ec, highlight.Pos0, highlight.Pos1, highlight.Text, pctStart)
deviceData, _ := json.Marshal(map[string]interface{}{
"datetime": highlight.Datetime,
"pos0": highlight.Pos0,
+52
View File
@@ -4,6 +4,8 @@ import (
"encoding/json"
"testing"
wsync "bookhoard/internal/sync"
"github.com/stretchr/testify/assert"
)
@@ -50,3 +52,53 @@ func TestKOReaderProgressRequest_DeletedAnnotationsOmitted(t *testing.T) {
assert.Empty(t, req.Books[0].DeletedHighlights)
assert.Empty(t, req.Books[0].DeletedBookmarks)
}
// The web drawer navigates bookmarks by CFI, so only high-confidence
// conversions may be stored: href/percentage/fallback results would
// become dead links. This is the sole gate for KOReader→web bookmark
// positions (bookmark text is a label, never book text, so the
// conversion runs structural-only with empty context).
func TestWebUsableCFI(t *testing.T) {
cases := []struct {
name string
loc wsync.CanonicalLocator
expected string
}{
{
name: "structural landing stored",
loc: wsync.CanonicalLocator{CFI: "epubcfi(/6/52!/4/28/2/1:0)", Precision: "structural", Percentage: 0.52},
expected: "epubcfi(/6/52!/4/28/2/1:0)",
},
{
name: "exact text-search landing stored",
loc: wsync.CanonicalLocator{CFI: "epubcfi(/6/52!/4/28/2/1:0)", Precision: "exact", Percentage: 0.52},
expected: "epubcfi(/6/52!/4/28/2/1:0)",
},
{
name: "percentage guess discarded",
loc: wsync.CanonicalLocator{CFI: "epubcfi(/6/52!/4/2/1:0)", Precision: "percentage", Percentage: 0.52},
expected: "",
},
{
name: "fallback passthrough (raw xpointer) discarded",
loc: wsync.CanonicalLocator{CFI: "/body/DocFragment[2]/body/p[3]", Precision: "fallback", Percentage: 0.52},
expected: "",
},
{
name: "section href discarded (not a CFI)",
loc: wsync.CanonicalLocator{CFI: "ch10.xhtml", Precision: "section", Percentage: 0.52},
expected: "",
},
{
name: "structural but non-CFI value discarded",
loc: wsync.CanonicalLocator{CFI: "ch10.xhtml#h1", Precision: "structural", Percentage: 0.52},
expected: "",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, webUsableCFI(tc.loc))
})
}
}
+8
View File
@@ -152,6 +152,9 @@ type CreateMediaBookmarkRequest struct {
ChapterNumber int32 `json:"chapter_number"`
Percentage float64 `json:"percentage"`
ChapterReference int32 `json:"chapter_reference"`
// Origin labels the creating client for display ("android", "web").
// Optional: defaults to "web" for browser callers.
Origin string `json:"origin" validate:"omitempty,max=30"`
}
// UpdateMediaBookmarkRequest represents the request for updating a media bookmark
@@ -1750,6 +1753,10 @@ func (mh *MediaHandler) CreateMediaBookmark(c *echo.Context) error {
// The sync-aware path (dedup + LWW + tombstones) is preferred; fall back
// to the plain query when the service isn't wired (e.g. some tests).
if mh.annotationSvc != nil {
origin := req.Origin
if origin == "" {
origin = "web"
}
result, err := mh.annotationSvc.SaveBookmark(c.Request().Context(), wsync.SaveBookmarkRequest{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgtype.UUID{Bytes: userUUID, Valid: true},
@@ -1762,6 +1769,7 @@ func (mh *MediaHandler) CreateMediaBookmark(c *echo.Context) error {
PercentageLoc: req.Percentage,
ChapterReference: req.ChapterReference,
Source: "web",
OriginSource: origin,
ModifiedAt: time.Now(),
})
if err != nil {
+7 -3
View File
@@ -590,6 +590,9 @@ type SaveBookmarkRequest struct {
Source string
ModifiedAt time.Time
DeviceSyncData json.RawMessage
// OriginSource records the client that created the bookmark (set once
// at insert; unlike Source it is not updated by later writers).
OriginSource string
// DedupKey overrides the computed key for device echoes (see
// SaveHighlightRequest).
DedupKey string
@@ -624,9 +627,9 @@ func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRe
if !incomingNewerThanTombstone(req.ModifiedAt, existing.DeletedAt, existing.LastModifiedAt) {
return &SaveBookmarkResult{Bookmark: existing, Outcome: SaveOutcomeDeleted}, nil
}
// Newer than the tombstone: a deliberate re-create. Resurrect via the
// LWW update instead of INSERT (the tombstoned row still holds the
// UNIQUE(media_item_id, user_id, title) slot).
// Newer than the tombstone: a deliberate re-create at the same
// location. Resurrect via the LWW update so the row keeps its id
// and origin.
return s.applyBookmarkLWW(ctx, req, existing, dedupKey)
}
@@ -656,6 +659,7 @@ func (s *AnnotationService) createBookmark(ctx context.Context, req SaveBookmark
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""},
DeviceSyncData: deviceData,
OriginSource: pgText(req.OriginSource),
})
if err != nil {
return nil, fmt.Errorf("create bookmark: %w", err)
+21
View File
@@ -666,3 +666,24 @@ func TestReverseIgnoresSingleCharContext(t *testing.T) {
t.Errorf("single-char reverse context must fall back to percentage, got %s", reverse.Precision)
}
}
// The bookmark route supplies no context (bookmark text is a display
// label, never book text), so the facade must still resolve the drop-cap
// xpointer structurally instead of collapsing to the document start.
func TestConvertToCanonical_DropCapEmptyContext(t *testing.T) {
epubPath := writeDropCapEPUB(t)
xp := "/body/DocFragment[2]/body/p[3]/span[1]/text().0"
loc := ConvertToCanonical(LocatorSourceKOReader, xp, 0.52, "", string(FormatGroupReflowable), epubPath, "")
t.Logf("facade drop-cap (no context) → %s (%s)", loc.CFI, loc.Precision)
if loc.Precision != "structural" && loc.Precision != "exact" {
t.Fatalf("expected structural/exact precision, got %s (%s)", loc.Precision, loc.CFI)
}
if !strings.HasPrefix(loc.CFI, "epubcfi(") {
t.Fatalf("expected standard epubcfi, got %s", loc.CFI)
}
if strings.HasSuffix(loc.CFI, "/4/2/1:0)") {
t.Errorf("empty-context conversion collapsed to doc start: %s", loc.CFI)
}
}
+7
View File
@@ -58,6 +58,13 @@ func cachedConverter(epubPath string) *CFIConverter {
return c
}
// SectionPercentageCached resolves the spine-section percentage of a CRE
// xpointer through the shared bounded converter cache, so per-annotation
// lookups parse the EPUB once per book instead of once per annotation.
func SectionPercentageCached(epubPath, xpointer string) float64 {
return cachedConverter(epubPath).SectionPercentage(xpointer)
}
func ConvertToCanonical(
source LocatorSource,
devicePos string,
+23
View File
@@ -730,10 +730,33 @@ templ ReaderAnnotationsDrawer() {
href="#"
@click.prevent="goToBookmark(bookmark)"
class="flex-1 min-w-0 block py-2 hover:bg-gray-700 rounded px-2"
x-show="!bookmark.renameOpen"
>
<span class="font-medium block truncate" x-text="bookmark.title"></span>
<span class="text-xs block truncate" style="color: var(--text-secondary)" x-text="bookmark.positionLabel"></span>
</a>
<div class="flex-1 min-w-0 py-2" x-show="bookmark.renameOpen" @click.stop>
<input
type="text"
class="reader-note-input"
x-model="bookmark.renameText"
@keydown.enter="renameBookmark(bookmark)"
@keydown.escape="bookmark.renameOpen = false"
placeholder="Bookmark name…"
></input>
<div class="flex gap-2 mt-1">
<button @click="renameBookmark(bookmark)" class="flex-1 py-1 text-xs bg-blue-600 text-white rounded hover:bg-blue-700">Save</button>
<button @click="bookmark.renameOpen = false" class="flex-1 py-1 text-xs border rounded hover:opacity-80" style="border-color: var(--border);">Cancel</button>
</div>
</div>
<button
@click="startBookmarkRename(bookmark)"
class="p-2 rounded hover:bg-gray-600 opacity-0 group-hover:opacity-100 transition-opacity"
title="Rename bookmark"
aria-label="Rename bookmark"
>
</button>
<button
@click="deleteBookmark(bookmark.id)"
class="p-2 rounded hover:bg-red-900/60 opacity-0 group-hover:opacity-100 transition-opacity"
+43 -1
View File
@@ -472,6 +472,8 @@ document.addEventListener("alpine:init", () => {
positionLabel: string;
cfi: string;
page: number | null;
renameOpen?: boolean;
renameText?: string;
}[],
tocItems: [] as any[],
mediaItemId: "" as string,
@@ -2079,6 +2081,14 @@ document.addEventListener("alpine:init", () => {
const page = this.isFixedLayout
? (this.renderer?.index ?? 0) + 1
: 0;
// Auto-label mirrors KOReader's convention ("in <chapter title>")
// so every client names unnamed bookmarks identically; falls back
// to a plain "Bookmark" for fixed-layout or TOC-less books.
const chapter = this.lastRelocateDetail?.tocItem?.label as
| string
| undefined;
const title =
!this.isFixedLayout && chapter ? `in ${chapter}` : "Bookmark";
try {
const resp = await fetch(
@@ -2090,7 +2100,7 @@ document.addEventListener("alpine:init", () => {
"Content-Type": "application/json",
},
body: JSON.stringify({
title: `Bookmark at ${this.progressText || "current position"}`,
title,
position: this.isFixedLayout
? `page:${page}`
: cfi
@@ -2130,6 +2140,38 @@ document.addEventListener("alpine:init", () => {
/* ignore bookmark errors for now */
}
},
startBookmarkRename(bookmark: (typeof this.bookmarkItems)[number]) {
bookmark.renameOpen = true;
bookmark.renameText = bookmark.title;
},
async renameBookmark(bookmark: (typeof this.bookmarkItems)[number]) {
const token = getToken();
const title = (bookmark.renameText ?? "").trim();
if (!token || !this.mediaItemId || !title) return;
try {
const resp = await fetch(
`/api/media-items/${this.mediaItemId}/bookmarks/${bookmark.id}`,
{
method: "PUT",
headers: {
Authorization: `Bearer ${token}`,
"Content-Type": "application/json",
},
body: JSON.stringify({
title,
notes: "",
position: bookmark.positionLabel || "",
}),
},
);
if (resp.ok) {
bookmark.title = title;
}
bookmark.renameOpen = false;
} catch (_e) {
bookmark.renameOpen = false;
}
},
chapterNumberForProgress(): number {
const tocItem = this.lastRelocateDetail?.tocItem;
if (!tocItem?.label) return 0;