YPE-1034: wire BibleReader highlights to the highlights API (1/3, dark-launched)#283
Draft
cameronapak wants to merge 4 commits into
Draft
YPE-1034: wire BibleReader highlights to the highlights API (1/3, dark-launched)#283cameronapak wants to merge 4 commits into
cameronapak wants to merge 4 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dark-launch flag Replace the localStorage highlight store (YPE-642 stand-in) with real API wiring per YPE-1034 ADR-001: highlights are server-only account data, deleted locally with no migration. - New internal useBibleReaderHighlights seam: fetches the current chapter's highlights via useHighlights (chapter USFM passage_id, keyed on version), renders through an in-memory optimistic overlay, reverts + console.error on write failure (toasts and 401/403 handling are PR 2) - Contiguous verse runs collapse to range USFMs on the wire (usfm-ranges.ts, mirroring the verse-share run-grouping idiom); colors sent as lowercase hex - Internal HIGHLIGHTS_LIVE dark-launch flag (off, not exported from the package entry) gates fetches, writes, and rendering; setHighlightsLive() is the test/Storybook-only override - Auth-gated: reads YouVersionAuthContext directly (now exported from hooks) so a missing auth provider degrades to signed-out instead of the useYVAuth throw; sign-out un-renders highlights immediately - better-result added to the ui package for error typing at the new seam's write boundary; core's throwing clients are untouched Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… responses Adversarial review of the highlights wiring surfaced one blocker and two races, all rooted in useApiData: - The fetch effect ignored enabled transitions: a hook that mounted disabled (auth still resolving) never fetched once enabled flipped true, so a signed-in reader rendered zero highlights until a write or navigation. enabled now rides alongside the caller-supplied deps. - Disabling kept the last response, so stale account data could render across sign-out or a host-controlled auth user switch. Disabling now clears data and error. - refetch-initiated requests escaped cancellation: a stale refetch for a previous chapter resolving late could clobber the new chapter's data. All requests now go through a monotonic sequence; only the latest-issued request may commit state. In the BibleReader seam hook: - Successfully settled writes now hand their verses to a confirmed set whose overlay entries are dropped when the post-write refetch lands, so server truth wins again instead of the optimistic entry masking later remote changes until navigation. - Documented the two remaining apply/remove concurrency windows (in-flight POST vs DELETE ordering, snapshot revert vs concurrent write) at the write boundary; a real operation queue is deliberately PR 2. New tests: useApiData enabled transitions and stale-response handling, and a seam-hook integration test through the real useHighlights/useApiData path (module-level mocks had hidden the enabled-flip bug). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 5b7d334 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This was referenced Jul 10, 2026
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
1 participant
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
PR 1 of 3 for YPE-1034: BibleReader highlights now read/write the real
/v1/highlightsAPI instead of localStorage, behind an internal dark-launch flag.HIGHLIGHTS_LIVE = falseinternal constant (packages/ui/src/lib/feature-flags.ts, not exported from the package entry). Flag off = feature inert: no fetches, no writes; copy/share unaffected. Flip is tracked in YPE-3874, blocked by the snackbar work (YPE-3873).useBibleReaderHighlightswith bridge-safe signaturesapply(color, verses)/remove(color, verses)(the seam YPE-3705's controlled mode will plug into). ConsumesuseHighlights; chapter-USFM fetch gated on flag + auth; safe without an auth provider.useApiDatafixes surfaced by adversarial review:enabledfalse→true transitions now fetch (previously auth resolving after mount never triggered the initial fetch), disabling clears data (prevents cross-user stale renders in host-controlled auth), and stale refetch responses are dropped via latest-wins sequencing.Testing
Follow-ups (stacked)
🤖 Generated with Claude Code