fix: bounded Locator enrichment - #203
Merged
Merged
Conversation
The elvis fallback in emitOnPageChanged built a lambda and discarded it, so the
diagnostic never printed when getPageInformation returned nothing. Use run {}.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ze the stream emitOnPageChanged awaited JS page-info and a ToC lookup before emitting on onTextLocatorChanged, both unbounded. Upstream's EPUBSpreadView.evaluateScript starts with an uncapped await spreadLoaded(), so a WebContent/GPU renderer restart could leave the spread never signalling loaded — and since ready is emitted synchronously from locationDidChange, consumers saw ReadiumReaderStatus .ready followed by permanent silence, with no error and no recovery. Enrichment now times out after 5s on both platforms and the raw locator (href + progression) is emitted instead, with a warning logged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A cold CI simulator can spend minutes thrashing WebKit helper processes, which says nothing about the code under test — yet the warm-up's 120s locator assert could fail an otherwise-green 41-test suite on its own. Its coverage is already duplicated by the reader-lifecycle, preferences and navigation suites, so it now waits 45s and logs on timeout instead of failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Return nil for overlay lists that come back empty, and bail out of initNavigator when the derived audio readingOrder has no links, rather than handing an empty manifest to AudioNavigator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
emitOnPageChangedawaited JS page-info and a ToC lookup before emitting ononTextLocatorChanged— both unbounded. Upstream'sEPUBSpreadView.evaluateScriptbegins with an uncappedawait spreadLoaded(), so a WebContent/GPU renderer restart can leave the spread never signalling loaded. Because ready is emitted synchronously fromlocationDidChange(which upstream gates only on navigator idle), consumers could seeReadiumReaderStatus.readyfollowed by permanent silence on the locator stream — no error, no recovery.Enrichment now times out after 5s on iOS and Android, emitting the raw locator (href + progression) with a warning.
Why now
This surfaced as the
Warm-up the platform reader viewfailure on #194's iOS job, where 40 of 41 tests passed. The simulator log archive shows WebKit thrashing throughout the warm-up window — GPU process taking 15.6s to launch, then killed twice as unresponsive — and the warm-up page never reachingdidCommitLoadForFrame. So it was a real product bug that a cold CI runner happened to trigger, not test flakiness.The warm-up itself is now best-effort (45s, logs instead of failing). Its assertion duplicated coverage the reader-lifecycle, preferences and navigation suites already provide, so a cold-start stall no longer fails an otherwise-green suite.
Also included
Tangential fixes that landed on this branch: guided-nav now accepts imgref as a text anchor (DiViNa panel audio, not just textref), empty media-overlay guards on iOS, an Android log-fallback that built a lambda and never called it, and two new CLAUDE.md conventions on changelog/comment brevity.
Verification
bin/format, bin/analyze, ktlint, compileDebugKotlin, flutter build ios --no-codesignall clean. Full integration suite passes: iOS 41/41, Android 39/39 (+2 expected platform skips), with zero timeout warnings — the 5s budget doesn't clip the happy path.The timeout path was proven by stubbing
getPageInformation()to sleep 60s: the iOS suite still passed, which is impossible without the fallback since those tests only wait 30s. Not verified: the same stubbed-hang check was not run on Android — that side is compile-verified and mirrors the iOS logic.