Kobo/KEPUB conversion lacks the drop-cap hardening (unguarded text search) #1

Open
opened 2026-09-09 09:07:34 -04:00 by john-okeefe · 0 comments
Owner

Summary

The usableContextText guard added to the CREngine converter (commit f7c4dfe,
fix(sync): resolve CRE positions structurally with text as verification) was
never ported to the KEPUB converter. Both text-search paths in
internal/sync/kepub_cfi_converter.go accept any non-empty context string, so a
short context (e.g. a single drop-cap character C) matches the first
occurrence in the chapter and is stored with Precision: "exact" — the same
confident-but-wrong doc-start failure that affected KOReader progress sync.

Affected code

  • internal/sync/kepub_cfi_converter.go:49-75 — ConvertKEPUBCFIToStandard
    (forward: device → server): searchText = normalizeWhitespace(contextText)
    with no minimum-length/word-count check.
  • internal/sync/kepub_cfi_converter.go:103-128 — ConvertStandardCFIToKEPUB
    (reverse: server → device): same pattern.

Current risk: latent, not active

The only call site (internal/handlers/kobo.go:997) passes contextText: "",
which routes to the safe path: extractSurroundingText(node, offset, 80) pulls
bridge text from the resolved node itself (always ~80 chars, multi-word). The
bug activates the moment any Kobo path starts passing device-supplied context
or the converter is reused elsewhere.

Related: dead facade branch

ConvertToCanonical/ConvertFromCanonical (internal/sync/locators.go:97,
:147) have a LocatorSourceKobo branch — it has zero callers. Kobo
converts inline at kobo.go:997 with NewKEPUBCFIConverter (which re-parses
both the EPUB and the KEPUB per call; the CRE converter cache in locators.go
does not cover KEPUB converters).

Proposed fix

  1. Guard both text searches with usableContextText (≥8 runes, ≥2 words);
    short contexts fall through to the percentage fallback instead of claiming
    an exact match.
  2. Prefer node-extracted bridge text (extractSurroundingText) over
    device-supplied context; demote device context to verification only.
  3. Wire kobo.go through ConvertToCanonical/ConvertFromCanonical so the
    facade owns routing, and add a bounded cache for KEPUBCFIConverter
    instances (mirroring cachedConverter in locators.go).

Note: the CRE structural walk cannot be ported literally — KEPUB CFIs index a
modified DOM (~1KB file splits, koboSpan wrappers), which is why the
kepub→epub text bridge exists. The discipline ports (guard, honest precision
labels, structural-resolution-inside-kepub first), not the algorithm.

Testing

Blocked on access to a Kobo device or emulator — cannot verify against real
KEPUB output otherwise. Unit tests with a synthetic KEPUB fixture
(writeTestEPUB-style, kepub_cfi_converter_test.go already has fixtures) can
land first:

  • short-context forward conversion must not return Precision: "exact" at doc
    start
  • short-context reverse conversion must fall back to percentage
  • empty-context conversion must use node-extracted bridge text (current
    behavior, keep green)

References

  • f7c4dfe fix(sync): resolve CRE positions structurally with text as
    verification (the guard being ported)
  • 9425edb (bookhoard.koplugin) fix(sync): capture block context across
    inline splits (drop-caps) — plugin-side counterpart
  • KOReader unification work (this repo, same date): all KOReader features now
    route through the facade; Kobo remains the sole inline converter caller.
## Summary The `usableContextText` guard added to the CREngine converter (commit `f7c4dfe`, *fix(sync): resolve CRE positions structurally with text as verification*) was never ported to the KEPUB converter. Both text-search paths in `internal/sync/kepub_cfi_converter.go` accept any non-empty context string, so a short context (e.g. a single drop-cap character `C`) matches the **first** occurrence in the chapter and is stored with `Precision: "exact"` — the same confident-but-wrong doc-start failure that affected KOReader progress sync. ## Affected code - `internal/sync/kepub_cfi_converter.go:49-75` — `ConvertKEPUBCFIToStandard` (forward: device → server): `searchText = normalizeWhitespace(contextText)` with no minimum-length/word-count check. - `internal/sync/kepub_cfi_converter.go:103-128` — `ConvertStandardCFIToKEPUB` (reverse: server → device): same pattern. ## Current risk: latent, not active The only call site (`internal/handlers/kobo.go:997`) passes `contextText: ""`, which routes to the safe path: `extractSurroundingText(node, offset, 80)` pulls bridge text from the resolved node itself (always ~80 chars, multi-word). The bug activates the moment any Kobo path starts passing device-supplied context or the converter is reused elsewhere. ## Related: dead facade branch `ConvertToCanonical`/`ConvertFromCanonical` (`internal/sync/locators.go:97`, `:147`) have a `LocatorSourceKobo` branch — it has **zero callers**. Kobo converts inline at `kobo.go:997` with `NewKEPUBCFIConverter` (which re-parses both the EPUB and the KEPUB per call; the CRE converter cache in `locators.go` does not cover KEPUB converters). ## Proposed fix 1. Guard both text searches with `usableContextText` (≥8 runes, ≥2 words); short contexts fall through to the percentage fallback instead of claiming an exact match. 2. Prefer node-extracted bridge text (`extractSurroundingText`) over device-supplied context; demote device context to verification only. 3. Wire `kobo.go` through `ConvertToCanonical`/`ConvertFromCanonical` so the facade owns routing, and add a bounded cache for `KEPUBCFIConverter` instances (mirroring `cachedConverter` in `locators.go`). Note: the CRE structural walk cannot be ported literally — KEPUB CFIs index a modified DOM (~1KB file splits, `koboSpan` wrappers), which is why the kepub→epub text bridge exists. The *discipline* ports (guard, honest precision labels, structural-resolution-inside-kepub first), not the algorithm. ## Testing Blocked on access to a Kobo device or emulator — cannot verify against real KEPUB output otherwise. Unit tests with a synthetic KEPUB fixture (`writeTestEPUB`-style, kepub_cfi_converter_test.go already has fixtures) can land first: - short-context forward conversion must not return `Precision: "exact"` at doc start - short-context reverse conversion must fall back to percentage - empty-context conversion must use node-extracted bridge text (current behavior, keep green) ## References - `f7c4dfe` fix(sync): resolve CRE positions structurally with text as verification (the guard being ported) - `9425edb` (bookhoard.koplugin) fix(sync): capture block context across inline splits (drop-caps) — plugin-side counterpart - KOReader unification work (this repo, same date): all KOReader features now route through the facade; Kobo remains the sole inline converter caller.
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Bookhoard/bookhoard#1