feat(highlights): wire BibleReader to the highlights API (YPE-1034)

[cameronapak]

Summary

BibleReader highlights now read/write the real /v1/highlights API — with a sign-in/permission flow, recent colors, and staging-hardening fixes — all dark-launched behind the internal HIGHLIGHTS_LIVE flag (off; flip tracked in YPE-3874). Consolidates the YPE-1034 stack (#283, #285, #286, #287) into one reviewable PR, rebased onto main.

Changes

1. Highlights persist to the user's YouVersion account instead of localStorage, with optimistic per-verse overlay and contiguous runs collapsed to range USFMs on the wire (flag off = feature inert: no fetches, no writes).
2. Tapping a color without a session or the highlights permission routes through sign-in or a just-in-time data-exchange grant, preserving the user's intent across the redirect (pending highlight in sessionStorage, ~10-min expiry).
3. Failures route by cause: 401/403 invalidates the permission cache and re-prompts, network/5xx reverts the overlay (snackbar is YPE-3873), decline/cancel discards the pending highlight but keeps the selection.
4. The color row shows the user's recent colors (GET /v1/highlights/recent-colors, fallback to the 5-color palette), and active swatches show a checkmark instead of an X.
5. Staging-session fixes: the permission dialog's Continue button is visible again (Button default variant was white-on-white), overlay entries survive read-after-write replica lag, writes refetch once per settled batch, removes send per-verse DELETEs (range DELETE fails server-side), and fills fade in (250ms, reduced-motion honored).
6. useApiData now fetches when enabled flips false→true, clears data on disable, and drops stale responses via latest-wins sequencing.

Start here: change 5 — overlay entries retire only when a fetch reflects the write; the accepted trade-off (concurrent same-verse edits from another device render stale until navigation/next write) needs product sign-off before the flag flips.

Flow

flowchart LR
    A[Tap color] --> B{Signed in +<br>highlights permission?}
    B -- yes --> C[Optimistic overlay + POST]
    B -- no session --> D[Sign-in requesting highlights]
    B -- no permission --> E[Confirm dialog] --> F[Data-exchange grant]
    D --> G[Pending highlight resumes]
    F --> G
    G --> C

Loading

Breaking / Migration

Local-only highlights are gone: the localStorage store is deleted without migration per the server-only ADR — highlights are account data and un-render on sign-out.

Test plan

• pnpm test: 865 passing (core 334, hooks 285, ui 246) after rebase onto main
• pnpm typecheck and pnpm lint clean
• Rebase conflict (main's localization sync vs new highlight strings in en.json) resolved by keeping both key sets
• Staging fixes verified against a real session recording (Jam + console/network capture)
• Needs manual check: one live data-exchange round-trip on staging before HIGHLIGHTS_LIVE flips — callback params (data_exchange_status, granted_permissions) are assumed from spec; handling is isolated in buildDataExchangeUrl/parseDataExchangeCallback

Replaces the stacked drafts #283, #285, #286, #287.

🤖 Generated with Claude Code

Greptile Summary

This PR wires BibleReader highlights to the server-backed highlights API. The main changes are:

• Server-only highlight fetch, create, and delete paths behind the live flag.
• Sign-in and data-exchange permission flow for highlight writes.
• Optimistic overlay reconciliation and batched write refetching.
• Recent highlight colors and active-swatch checkmark UI.
• Core data-exchange utilities, hook exports, tests, and ADR updates.

Confidence Score: 4/5

The highlight auth and permission-cache paths need fixes before merging.

• A resumed pending highlight can be deleted before the write succeeds.
• A stale cached permission can be read for a later user/session.
• The remaining issues are contained to callback cleanup and range-response hardening.

packages/ui/src/components/use-bible-reader-highlights.ts, packages/core/src/YouVersionPlatformConfiguration.ts

Security Review

The permission cache can carry a stale highlights grant across user/session changes because it is user-unscoped and merge-only. That can skip the intended consent branch until the server rejects the request.

Important Files Changed

Filename	Overview
packages/ui/src/components/use-bible-reader-highlights.ts	Adds the BibleReader highlights seam with auth resume, optimistic overlay, write queueing, recent colors, and reconciliation.
packages/core/src/YouVersionPlatformConfiguration.ts	Adds persistent permission-cache helpers used by sign-in, data-exchange, and write-failure handling.
packages/core/src/data-exchange.ts	Adds data-exchange token, consent URL, callback parsing, permission-cache seeding, and callback URL cleanup.
packages/hooks/src/useHighlightAuthActions.ts	Adds highlight auth callbacks for sign-in, permission invalidation, callback consumption, and data-exchange redirect.
packages/hooks/src/useApiData.ts	Updates enabled transitions, stale-response handling, and disable-time data clearing.
packages/hooks/src/useHighlights.ts	Removes automatic mutation refetching so callers can coalesce batched highlight writes.
packages/ui/src/components/bible-reader.tsx	Replaces the local highlight store with the new server-backed highlight hook and permission dialog.
packages/ui/src/components/verse-action-popover.tsx	Adds recent color palette support and swaps the active swatch icon to a checkmark.
packages/ui/src/lib/pending-highlight.ts	Adds sessionStorage-backed pending highlight persistence with validation and expiry.
packages/ui/src/lib/usfm-ranges.ts	Adds contiguous verse-run collapsing and range passage-id construction.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Tap[Tap highlight color] --> Auth{Signed in?}
  Auth -- no --> Pending[Stash pending highlight]
  Pending --> SignIn[Sign-in requesting highlights]
  Auth -- yes --> Perm{Cached highlights permission?}
  Perm -- no --> Dialog[Permission dialog]
  Dialog --> DataExchange[Data-exchange grant]
  SignIn --> Resume[Resume pending highlight]
  DataExchange --> Resume
  Perm -- yes --> Write[Optimistic overlay and API write]
  Resume --> Write
  Write --> Refetch[Refetch highlights]
  Refetch --> Reconcile[Retire overlay when server reflects write]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
  Tap[Tap highlight color] --> Auth{Signed in?}
  Auth -- no --> Pending[Stash pending highlight]
  Pending --> SignIn[Sign-in requesting highlights]
  Auth -- yes --> Perm{Cached highlights permission?}
  Perm -- no --> Dialog[Permission dialog]
  Dialog --> DataExchange[Data-exchange grant]
  SignIn --> Resume[Resume pending highlight]
  DataExchange --> Resume
  Perm -- yes --> Write[Optimistic overlay and API write]
  Resume --> Write
  Write --> Refetch[Refetch highlights]
  Refetch --> Reconcile[Retire overlay when server reflects write]

Loading

Comments Outside Diff (2)

1. packages/ui/src/components/use-bible-reader-highlights.ts, line 3837 (link)

   Pending Intent Clears Too Early

   When the resumed highlight write fails after a sign-in or data-exchange return, this path has already deleted the pending highlight. A 401/403 or transient write failure then only logs and reverts, so the tap that started the auth flow is lost instead of being kept and re-prompted like the normal apply path.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/ui/src/components/use-bible-reader-highlights.ts
   Line: 3837
   
   Comment:
   **Pending Intent Clears Too Early**
   
   When the resumed highlight write fails after a sign-in or data-exchange return, this path has already deleted the pending highlight. A 401/403 or transient write failure then only logs and reverts, so the tap that started the auth flow is lost instead of being kept and re-prompted like the normal apply path.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. packages/ui/src/components/use-bible-reader-highlights.ts, line 3436-3437 (link)

   Range Response Loses Verses

   If GET /v1/highlights ever returns a range passage such as JHN.3.16-18, parseInt maps it only to verse 16. The trailing verses are omitted from highlightedVerses, and the reconcile path using the same parsing can leave overlay entries for verses 17-18 pinned until navigation.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/ui/src/components/use-bible-reader-highlights.ts
   Line: 3436-3437
   
   Comment:
   **Range Response Loses Verses**
   
   If `GET /v1/highlights` ever returns a range passage such as `JHN.3.16-18`, `parseInt` maps it only to verse 16. The trailing verses are omitted from `highlightedVerses`, and the reconcile path using the same parsing can leave overlay entries for verses 17-18 pinned until navigation.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
packages/ui/src/components/use-bible-reader-highlights.ts:3837
**Pending Intent Clears Too Early**

When the resumed highlight write fails after a sign-in or data-exchange return, this path has already deleted the pending highlight. A 401/403 or transient write failure then only logs and reverts, so the tap that started the auth flow is lost instead of being kept and re-prompted like the normal apply path.

### Issue 2 of 4
packages/core/src/YouVersionPlatformConfiguration.ts:102-106
**Permission Cache Outlives User**

This merge keeps old `localStorage` permissions when a later authenticated user is established without first calling `clearAuthTokens()` or when the new callback does not echo `granted_permissions`. In that state `hasPermission('highlights')` can return true for the wrong user, so the highlight flow skips consent and attempts account writes under a stale grant until the server rejects them.

### Issue 3 of 4
packages/core/src/data-exchange.ts:135-136
**Callback Cleanup Drops App Params**

A successful data-exchange return clears the entire query string, not just `data_exchange_status` and `granted_permissions`. If the host route uses query params for app state, referral data, or navigation context, completing the permission flow silently removes that unrelated state from the URL.

### Issue 4 of 4
packages/ui/src/components/use-bible-reader-highlights.ts:3436-3437
**Range Response Loses Verses**

If `GET /v1/highlights` ever returns a range passage such as `JHN.3.16-18`, `parseInt` maps it only to verse 16. The trailing verses are omitted from `highlightedVerses`, and the reconcile path using the same parsing can leave overlay entries for verses 17-18 pinned until navigation.

_{Reviews (1): Last reviewed commit: "fix(highlights): settle write batches pe..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

Context used (8)

• Rule used - Unified versioning: All packages must maintain exa... (source)
• Rule used - Zero React: Pure TypeScript, no React dependencies (source)
• Rule used - Context and Provider in separate files, exported v... (source)
• Rule used - Auto-injected styles: index.ts calls injectStyles(... (source)
• Context used - CLAUDE.md (source)
• Context used - AGENTS.md (source)
• File used - docs/review-guidelines.md (source)
• Context used - Enforce project-specific review guidelines - see d... (source)

[changeset-bot]

🦋 Changeset detected

Latest commit: 385486a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@youversion/platform-react-hooks	Patch
@youversion/platform-react-ui	Patch
@youversion/platform-core	Patch
vite-react	Patch

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

[greptile-apps]

Permission Cache Outlives User

This merge keeps old localStorage permissions when a later authenticated user is established without first calling clearAuthTokens() or when the new callback does not echo granted_permissions. In that state hasPermission('highlights') can return true for the wrong user, so the highlight flow skips consent and attempts account writes under a stale grant until the server rejects them.

Context Used: Enforce project-specific review guidelines - see d... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/core/src/YouVersionPlatformConfiguration.ts
Line: 102-106

Comment:
**Permission Cache Outlives User**

This merge keeps old `localStorage` permissions when a later authenticated user is established without first calling `clearAuthTokens()` or when the new callback does not echo `granted_permissions`. In that state `hasPermission('highlights')` can return true for the wrong user, so the highlight flow skips consent and attempts account writes under a stale grant until the server rejects them.

**Context Used:** Enforce project-specific review guidelines - see d... ([source](greptile.json))

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Callback Cleanup Drops App Params

A successful data-exchange return clears the entire query string, not just data_exchange_status and granted_permissions. If the host route uses query params for app state, referral data, or navigation context, completing the permission flow silently removes that unrelated state from the URL.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/core/src/data-exchange.ts
Line: 135-136

Comment:
**Callback Cleanup Drops App Params**

A successful data-exchange return clears the entire query string, not just `data_exchange_status` and `granted_permissions`. If the host route uses query params for app state, referral data, or navigation context, completing the permission flow silently removes that unrelated state from the URL.

How can I resolve this? If you propose a fix, please make it concise.