fix(core): align HighlightsClient with the live highlights API contract

[Dustin-Kelley]

Summary

HighlightsClient was written against an outdated version of the highlights API and has never worked against the live endpoints (its integration tests were describe.skip'd from day one). Every request was rejected, which surfaced as highlights silently failing to fetch/create/delete in the BibleReader wiring work (YPE-1034).

Verified against the current API reference; the live endpoint responds 401 "Missing or invalid Bearer token" to the old request shape.

Changes:

• Auth: send the OAuth token as an Authorization: Bearer <token> header instead of a lat query parameter
• Naming: use the API's bible_id field on the wire for GET/POST/DELETE; the SDK keeps version_id in its public types (consistent with the rest of the SDK) and maps at the boundary
• Create envelope: POST now sends the required { request_id: <uuid>, highlight: { bible_id, passage_id, color } } body for idempotent retries
• Required params: getHighlights now requires version_id + passage_id (verse or chapter USFM) and deleteHighlight requires version_id, matching the API's required parameters; useHighlights signatures updated accordingly
• Validation: responses are parsed with Zod (wire schema) and mapped to the SDK shape
• Tests: un-skipped and rewrote highlights.test.ts (10 tests); the shared MSW handlers now enforce the documented contract (401 without Bearer, 400 on wrong params/body) so future drift fails tests

Notes for reviewers

• Consumers who signed in before requesting the highlights permission need to re-authenticate to get a token with read_highlights/write_highlights scopes.
• Pre-existing failures not touched by this PR: 4 hooks tests (YouVersionProvider.test.tsx, missing localStorage in jsdom env) and 5 UI tests (bible-reader.test.tsx theme settings) fail identically on main.

Test plan

• pnpm --filter @youversion/platform-core test — 308 passed, including 10 rewritten highlights contract tests
• pnpm --filter @youversion/platform-react-hooks test — only the 4 pre-existing YouVersionProvider failures remain (identical on main)
• pnpm typecheck / pnpm lint — clean
• Manual verification against the live API with a signed-in session that has the highlights scope (blocked on a real token; follow-up in YPE-1034 wiring)

Made with Cursor

Greptile Summary

This PR fixes HighlightsClient to match the live YouVersion highlights API contract. Every request was previously rejected with 401 because auth was sent as a lat query parameter instead of an Authorization: Bearer header, and field names / request shapes differed from the documented API. The rewrite also un-skips and rewrites the highlights test suite so the MSW handlers now enforce the contract (auth, field names, enveloped POST body), preventing future drift.

• Auth & field mapping: Bearer header replaces lat query param; bible_id is used on the wire while the SDK keeps version_id in public types, with a clean toHighlight() mapper at the boundary.
• Request / response hardening: POST sends the required { request_id, highlight: { … } } envelope; getHighlights and deleteHighlight now enforce their required parameters; Zod schemas validate all responses; 204 (no highlights) is treated as an empty collection; uppercase hex colors are normalized to lowercase before sending.
• New surface: getRecentColors() added to HighlightsClient and exposed from useHighlights for color-picker UIs.

Confidence Score: 5/5

Safe to merge — all four endpoint contract fixes are correctly implemented and the MSW handlers now enforce them so any future drift will fail CI.

The implementation changes are precise and internally consistent: Bearer auth, wire-field mapping, enveloped POST body, 204 handling, and Zod validation all align with the documented API. The HEX_COLOR_REGEX was updated to case-insensitive, addressing the mismatch flagged in the previous review cycle. Tests cover the happy path, 204 edge case, malformed-response error, and uppercase color normalization. The changeset bump level is worth a second look but does not affect runtime correctness.

.changeset/fix-highlights-api-contract.md — worth a second look on the minor-vs-major bump decision for 2.x packages.

Important Files Changed

Filename	Overview
packages/core/src/highlights.ts	Rewrites HighlightsClient to use Bearer auth, bible_id field mapping, enveloped POST body, Zod response validation, and 204 handling; adds getRecentColors(). All contract fixes are correct and well-commented.
packages/core/src/schemas/highlight.ts	Adds wire schemas (HighlightWireSchema, HighlightCollectionWireSchema, RecentHighlightColorsWireSchema) and toHighlight() mapper. HEX_COLOR_REGEX is now case-insensitive, fixing the previously flagged mismatch.
packages/core/src/client.ts	Adds optional headers parameter to post() and delete(), enabling callers to inject Authorization: Bearer headers without touching the shared defaultHeaders.
packages/hooks/src/useHighlights.ts	Makes options and deleteOptions required to match the now-required API params; adds getRecentColors() callback; dep-array comment explains the primitive-keying strategy.
packages/core/src/tests/handlers.ts	MSW handlers now enforce Bearer auth (401 on missing token), bible_id naming, and enveloped POST body (400 on wrong shape), so client drift breaks tests immediately.
packages/core/src/tests/highlights.test.ts	Un-skipped and rewrote 10 tests covering the new contract; tests the Bearer header, bible_id param, enveloped POST, 204 handling, malformed-response error, and uppercase color normalization.
packages/hooks/src/useHighlights.test.tsx	Test suite updated to match the new required-options signatures and adds coverage for getRecentColors delegation.
.changeset/fix-highlights-api-contract.md	Changeset marked as minor for both 2.x packages; the required-param promotions are technically semver-major, though the feature was never functional before.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant App as App / useHighlights
    participant HC as HighlightsClient
    participant AC as ApiClient
    participant API as Highlights API

    Note over HC: getAuthToken(lat?) → Bearer header

    App->>HC: "getHighlights({ version_id, passage_id }, lat?)"
    HC->>HC: validateVersionId / validatePassageId
    HC->>AC: "get("/v1/highlights", { bible_id, passage_id }, { Authorization: Bearer })"
    AC->>API: "GET /v1/highlights?bible_id=&passage_id= + Bearer"
    API-->>AC: "200 { data: [{ bible_id, passage_id, color }] } OR 204"
    AC-->>HC: raw response
    HC->>HC: HighlightCollectionWireSchema.safeParse()
    HC->>HC: data.map(toHighlight) — bible_id → version_id
    HC-->>App: "Collection<Highlight>"

    App->>HC: "createHighlight({ version_id, passage_id, color }, lat?)"
    HC->>HC: validate + color.toLowerCase()
    HC->>AC: "post("/v1/highlights", { request_id, highlight: { bible_id, passage_id, color } }, headers)"
    AC->>API: POST /v1/highlights + Bearer
    API-->>AC: "201 { bible_id, passage_id, color }"
    AC-->>HC: raw response
    HC->>HC: HighlightWireSchema.safeParse() → toHighlight()
    HC-->>App: Highlight

    App->>HC: "deleteHighlight(passageId, { version_id }, lat?)"
    HC->>HC: validatePassageId / validateVersionId
    HC->>AC: "delete("/v1/highlights/{passageId}", { bible_id }, { Authorization: Bearer })"
    AC->>API: "DELETE /v1/highlights/{passageId}?bible_id= + Bearer"
    API-->>AC: 204
    HC-->>App: void

    App->>HC: getRecentColors(lat?)
    HC->>AC: "get("/v1/highlights/recent-colors", undefined, { Authorization: Bearer })"
    AC->>API: GET /v1/highlights/recent-colors + Bearer
    API-->>AC: "200 { data: [{ color }] } OR 204"
    HC->>HC: RecentHighlightColorsWireSchema.safeParse()
    HC-->>App: string[]

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"}}}%%
sequenceDiagram
    participant App as App / useHighlights
    participant HC as HighlightsClient
    participant AC as ApiClient
    participant API as Highlights API

    Note over HC: getAuthToken(lat?) → Bearer header

    App->>HC: "getHighlights({ version_id, passage_id }, lat?)"
    HC->>HC: validateVersionId / validatePassageId
    HC->>AC: "get("/v1/highlights", { bible_id, passage_id }, { Authorization: Bearer })"
    AC->>API: "GET /v1/highlights?bible_id=&passage_id= + Bearer"
    API-->>AC: "200 { data: [{ bible_id, passage_id, color }] } OR 204"
    AC-->>HC: raw response
    HC->>HC: HighlightCollectionWireSchema.safeParse()
    HC->>HC: data.map(toHighlight) — bible_id → version_id
    HC-->>App: "Collection<Highlight>"

    App->>HC: "createHighlight({ version_id, passage_id, color }, lat?)"
    HC->>HC: validate + color.toLowerCase()
    HC->>AC: "post("/v1/highlights", { request_id, highlight: { bible_id, passage_id, color } }, headers)"
    AC->>API: POST /v1/highlights + Bearer
    API-->>AC: "201 { bible_id, passage_id, color }"
    AC-->>HC: raw response
    HC->>HC: HighlightWireSchema.safeParse() → toHighlight()
    HC-->>App: Highlight

    App->>HC: "deleteHighlight(passageId, { version_id }, lat?)"
    HC->>HC: validatePassageId / validateVersionId
    HC->>AC: "delete("/v1/highlights/{passageId}", { bible_id }, { Authorization: Bearer })"
    AC->>API: "DELETE /v1/highlights/{passageId}?bible_id= + Bearer"
    API-->>AC: 204
    HC-->>App: void

    App->>HC: getRecentColors(lat?)
    HC->>AC: "get("/v1/highlights/recent-colors", undefined, { Authorization: Bearer })"
    AC->>API: GET /v1/highlights/recent-colors + Bearer
    API-->>AC: "200 { data: [{ color }] } OR 204"
    HC->>HC: RecentHighlightColorsWireSchema.safeParse()
    HC-->>App: string[]

Loading

Comments Outside Diff (2)

1. packages/core/src/highlights.ts, line 59-71 (link)

   localStorage guard makes the auth fallback silently different across environments. When typeof localStorage === 'undefined' (Node.js test environment, SSR), getAuthToken(undefined) returns null and immediately throws. That's the intended path for unauthenticated callers. However, the guard also means that any call inside a server-side context that should find a token in YouVersionPlatformConfiguration will always throw, even if the platform configuration has been populated another way. Worth a comment that makes this intent explicit, and a test confirming SSR callers must pass lat explicitly.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/core/src/highlights.ts
   Line: 59-71
   
   Comment:
   **`localStorage` guard makes the auth fallback silently different across environments.** When `typeof localStorage === 'undefined'` (Node.js test environment, SSR), `getAuthToken(undefined)` returns `null` and immediately throws. That's the intended path for unauthenticated callers. However, the guard also means that any call inside a server-side context that *should* find a token in `YouVersionPlatformConfiguration` will always throw, even if the platform configuration has been populated another way. Worth a comment that makes this intent explicit, and a test confirming SSR callers must pass `lat` explicitly.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

   

2. packages/hooks/src/useHighlights.ts, line 45-51 (link)

   options object identity is unstable if passed as an inline literal. The factory () => highlightsClient.getHighlights(options) captures the full options reference, but the dependency array correctly uses only the primitives options.version_id and options.passage_id. This means the closure may hold a stale object reference while the primitives stay stable — in practice this is fine here, but callers who add fields to GetHighlightsOptions later might be surprised. A quick clarifying comment on the dep-array strategy would help future maintainers.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/hooks/src/useHighlights.ts
   Line: 45-51
   
   Comment:
   **`options` object identity is unstable if passed as an inline literal.** The factory `() => highlightsClient.getHighlights(options)` captures the full `options` reference, but the dependency array correctly uses only the primitives `options.version_id` and `options.passage_id`. This means the closure may hold a stale object reference while the primitives stay stable — in practice this is fine here, but callers who add fields to `GetHighlightsOptions` later might be surprised. A quick clarifying comment on the dep-array strategy would help future maintainers.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

   

_{Reviews (5): Last reviewed commit: "docs(highlights): clarify auth fallback ..." | Re-trigger Greptile}

[changeset-bot]

🦋 Changeset detected

Latest commit: cef7b5b

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-core	Minor
@youversion/platform-react-hooks	Minor
@youversion/platform-react-ui	Minor
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

[Dustin-Kelley]

@greptile review