Converge round-trips: dedup-key identity, drawer classification, no imposed colors

- Identity: applied entries carry bookhoard_dedup_key (served by the
  server) and echo it back in pushes, so a pull→push cycle updates the
  original server row instead of minting a duplicate (device locators
  never matched web locators under the computed key — every round-trip
  duplicated every annotation). Matching in applyServerAnnotations is
  key-first; a keyed entry that is absent is genuinely new, so pos0
  fallback applies to legacy keyless entries only — a different
  annotation that merely shares a position can no longer be updated
  or, for tombstones, cross-deleted (demonstrated: a deleted
  duplicate's tombstone used to pos0-match a live highlight sharing
  the spot and remove it). Tombstone matching is likewise key-only
  for keyed entries; legacy pos0 fallback is restricted to the same
  annotation kind.
- Classification (v2): the drawer field is the only reliable
  highlight discriminator — KOReader auto-fills text="in Chapter X"
  on page bookmarks, so text-presence turned every echoed bookmark
  into a junk highlight on the web. Bookmarks now take their label
  from note.
- Colors: no color is imposed on applied entries; devices render
  their own default. The web color only changes when the user edits
  the highlight on the device (the edit sets a device color name,
  which the server maps and stores).
This commit is contained in:
2026-08-19 19:42:09 -04:00
parent 87e4ca13c5
commit fa48615e65
+72 -20
View File
@@ -879,8 +879,18 @@ function Bookhoard:collectAnnotations()
for _, bm in ipairs(entries) do for _, bm in ipairs(entries) do
local sel_text, user_note local sel_text, user_note
if model == "v2" then if model == "v2" then
if bm.drawer then
sel_text = bm.text or "" -- highlighted text sel_text = bm.text or "" -- highlighted text
user_note = bm.note or "" -- user note / bookmark label user_note = bm.note or "" -- user note
else
-- Page bookmark. `drawer` is the only reliable highlight
-- discriminator: KOReader auto-fills text = "in Chapter X"
-- on bookmarks (updateItemByXPointer), so text-presence
-- would misclassify every bookmark as a highlight on echo.
-- The user label lives in `note`.
sel_text = ""
user_note = bm.note or ""
end
else else
sel_text = bm.notes or "" -- v1: notes held the highlighted text sel_text = bm.notes or "" -- v1: notes held the highlighted text
user_note = bm.text or "" -- v1: text held the note/label user_note = bm.text or "" -- v1: text held the note/label
@@ -910,6 +920,13 @@ function Bookhoard:collectAnnotations()
if percentage then if percentage then
entry.percentage = Math.roundPercent(percentage) entry.percentage = Math.roundPercent(percentage)
end end
if bm.bookhoard_dedup_key then
-- Echo identity for entries received from the server: lets the
-- server match this push to the original row instead of
-- minting a duplicate (device locators ≠ web locators, so the
-- computed key would never match).
entry.dedup_key = bm.bookhoard_dedup_key
end
local has_text = sel_text ~= "" local has_text = sel_text ~= ""
local has_notes = user_note ~= "" local has_notes = user_note ~= ""
@@ -1202,7 +1219,22 @@ function Bookhoard:applyServerAnnotations(annotations)
local changed = false local changed = false
local has_pages = self.ui.document.info.has_pages local has_pages = self.ui.document.info.has_pages
local function findLocalByPos0(pos0) local function findLocal(server_entry)
-- Identity match by dedup key: survives pos0 drift (improved
-- server conversion) and, crucially, never cross-matches a
-- DIFFERENT annotation that merely shares the position.
local key = server_entry.dedup_key
if key and key ~= "" then
for i, bm in ipairs(entries) do
if bm.bookhoard_dedup_key == key then
return i
end
end
return nil -- keyed but not present: a real new entry
end
-- Legacy serve (no key): fall back to pos0 matching.
local pos0 = server_entry.pos0 or ""
if pos0 == "" then return nil end
for i, bm in ipairs(entries) do for i, bm in ipairs(entries) do
local p = bm.pos0 local p = bm.pos0
if type(p) == "table" then p = tostring(p.page or "") end if type(p) == "table" then p = tostring(p.page or "") end
@@ -1267,7 +1299,7 @@ function Bookhoard:applyServerAnnotations(annotations)
if not isValidPos0(pos0) then return end if not isValidPos0(pos0) then return end
local srv_text = server_entry.text or "" local srv_text = server_entry.text or ""
local srv_notes = server_entry.notes or "" local srv_notes = server_entry.notes or ""
local idx = findLocalByPos0(pos0) local idx = findLocal(server_entry)
if idx then if idx then
local bm = entries[idx] local bm = entries[idx]
local cur_text, cur_note = getFields(bm) local cur_text, cur_note = getFields(bm)
@@ -1286,18 +1318,17 @@ function Bookhoard:applyServerAnnotations(annotations)
pos1 = makeLocalPos(server_entry.pos1 ~= "" and server_entry.pos1 or pos0), pos1 = makeLocalPos(server_entry.pos1 ~= "" and server_entry.pos1 or pos0),
page = makePageFromPos(pos0), page = makePageFromPos(pos0),
} }
if server_entry.dedup_key and server_entry.dedup_key ~= "" then
entry.bookhoard_dedup_key = server_entry.dedup_key
end
if has_text then if has_text then
entry.drawer = "lighten" entry.drawer = "lighten"
entry.text = srv_text entry.text = srv_text
if srv_notes ~= "" then entry.note = srv_notes end if srv_notes ~= "" then entry.note = srv_notes end
-- KOReader renders highlight colors from a fixed name set -- No color imposed: devices render their own default and
-- (Blitbuffer.HIGHLIGHT_COLORS); anything else (e.g. a web -- cannot round-trip web colors; the web color only changes
-- hex value slipping through) draws nothing useful, so only -- when the highlight is edited here (the edit sets a device
-- keep plain names and let the device default apply. -- color, which syncs back by name).
local srv_color = server_entry.color or ""
if srv_color ~= "" and not srv_color:find("^#") then
entry.color = srv_color
end
else else
if srv_text ~= "" then entry.note = srv_text end -- bookmark label if srv_text ~= "" then entry.note = srv_text end -- bookmark label
end end
@@ -1312,7 +1343,9 @@ function Bookhoard:applyServerAnnotations(annotations)
notes = srv_text, notes = srv_text,
text = srv_notes, text = srv_notes,
} }
if server_entry.color and server_entry.color ~= "" then entry.color = server_entry.color end if server_entry.dedup_key and server_entry.dedup_key ~= "" then
entry.bookhoard_dedup_key = server_entry.dedup_key
end
if server_entry.chapter and server_entry.chapter ~= "" then entry.chapter = server_entry.chapter end if server_entry.chapter and server_entry.chapter ~= "" then entry.chapter = server_entry.chapter end
table.insert(entries, entry) table.insert(entries, entry)
changed = true changed = true
@@ -1337,11 +1370,34 @@ function Bookhoard:applyServerAnnotations(annotations)
end end
end end
local function findLocalByTombstone(del, want_highlight)
local key = del.dedup_key
if key and key ~= "" then
for i, bm in ipairs(entries) do
if bm.bookhoard_dedup_key == key then
return i
end
end
return nil -- keyed tombstone: only its own entry may be removed
end
-- Legacy tombstone (no key): pos0 match, restricted to the same
-- annotation kind so a highlight tombstone never eats a bookmark
-- that shares the position.
local pos0 = del.pos0 or ""
if pos0 == "" then return nil end
for i, bm in ipairs(entries) do
local p = bm.pos0
if type(p) == "table" then p = tostring(p.page or "") end
if p == pos0 and (not not bm.drawer) == want_highlight then
return i
end
end
return nil
end
if annotations.deleted_highlights then if annotations.deleted_highlights then
for _, del in ipairs(annotations.deleted_highlights) do for _, del in ipairs(annotations.deleted_highlights) do
local pos0 = del.pos0 or "" local idx = findLocalByTombstone(del, true)
if pos0 ~= "" then
local idx = findLocalByPos0(pos0)
local bm = idx and entries[idx] local bm = idx and entries[idx]
local cur_text = bm and (getFields(bm)) or "" local cur_text = bm and (getFields(bm)) or ""
if idx and cur_text ~= "" then if idx and cur_text ~= "" then
@@ -1350,20 +1406,16 @@ function Bookhoard:applyServerAnnotations(annotations)
end end
end end
end end
end
if annotations.deleted_bookmarks then if annotations.deleted_bookmarks then
for _, del in ipairs(annotations.deleted_bookmarks) do for _, del in ipairs(annotations.deleted_bookmarks) do
local pos0 = del.pos0 or "" local idx = findLocalByTombstone(del, false)
if pos0 ~= "" then
local idx = findLocalByPos0(pos0)
if idx then if idx then
table.remove(entries, idx) table.remove(entries, idx)
changed = true changed = true
end end
end end
end end
end
if changed then if changed then
-- addItem() inserts v2 entries at their sorted position already; -- addItem() inserts v2 entries at their sorted position already;