Conversation
🦋 Changeset detectedLatest commit: 77aa9e6 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Search hits and the structural validator both need the same chapter/verse shape. parseUsfmReference owns that grammar so inverted ranges drop at the boundary instead of leaking into the reader.
The PRD's suggestion-suppression, stale-page, and dedupe rules are consequences of one SearchSession union plus a pure reducer, so the eight phases can be proven without timers or network.
Headless hosts get one phase and five commands. Debounce, language-range gating, and page commits stay inside the hook so UI never sees tokens or TanStack Query.
Content can scroll and dim only after the destination chapter is on screen. A seq mailbox plus a hold timer keeps focus separate from verse selection.
Hosts get a zero-required-prop search dialog that talks to useBibleSearch and navigates through one Root-owned verse-focus mailbox. English keys land here so the UI can ship; platform-localization still owns the canonical source.
The public helpers and Reader search UI are new surface on core, hooks, and ui, so they ship as minors together.
The first keystroke after submit left the submitted lane while debounce still held that query. Suggestions then re-fetched it and showed a spinner. Debounce null while submitted, and skip queriesRequest until the new input settles.
The hold-clear scroll listener sat on the renderer root, which does not scroll. Listen on the nearest overflow ancestor instead. Drop the unused focusedVerses return. Content never read it.
camrun91
force-pushed
the
cl/ype-5766_search_ui
branch
from
September 14, 2026 21:40
1773708 to
b29aaf2
Compare
Widen useDebounce test props so null flush typechecks. Wait for the Reader canvas before clicking Search. Revert human en.json edits and keep English via t() defaultValue until platform-localization syncs bibleSearch* keys.
The previous CI fix copied en.json from the deleted search API tip, which still differed from main. That kept Locale Ownership red and broke version-picker and verse tests that assert main's English copy.
Unsafe integer endpoints stall `verse += 1` at 2^53 and oversized spans allocate huge arrays. Lock the intended rejection before the parser change.
Number.isInteger accepted 2^53 endpoints, so verse += 1 never advanced and search hits could hang. Reject non-safe integers and spans over 250 verses instead of expanding them.
cameronapak
requested changes
Sep 16, 2026
cameronapak
left a comment
cameronapak
left a comment
Collaborator
There was a problem hiding this comment.
Review
Summary
Standards: 0 must-fix. Spec: 10 must-fix. Primary concern: host navigation is unavailable and automatic scrolling clears selected-verse focus before arrival.
For Agents
- CI: failing. Integration Tests reports 582 passing assertions and 11 unhandled rejections; the cause is not established. Other build, typecheck, lint, and unit-test checks pass.
- Bot review: clear; earlier bot findings are resolved.
- Event: REQUEST_CHANGES
- HEAD: 9720ea4
- Spec: the user-supplied artifact of the Search PRD, linked from parent YPE-5622.
- Verification: 92 existing targeted Vitest tests passed. Three focused regression reproductions failed for result deduplication, stale suggestions, and wildcard fallback. Chromium confirmed automatic scrolling clears focus before arrival. Temporary reproduction fixtures were removed.
Written by Code Reviewer bot on behalf of Cam.
cameronapak
reviewed
Sep 17, 2026
Collaborator
There was a problem hiding this comment.
This change allows for us to be able to easily add Zod to the UI package
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
3 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.
Important
This pull request was started by Cameron Llewellyn and passed onto Cameron Pak
Bible Reader search
Adds toolbar search with suggestions, browser-local recents, testament filters, verse previews, and navigation that centers the selected verse while dimming surrounding verses. Hosts can use
useBibleSearchheadlessly or override the toolbar action withonSearchPress.Closes YPE-5766. Parent: YPE-5622.
UI views
Current Chromium captures using live API data. Filters expand as an accordion; reduced motion uses a fade.
Dark mode
Verification and remaining limitations
The PR appears safe to merge.
Summary
The PR adds Bible Reader search across the core API, React hooks, and UI, including suggestions, recents, testament filtering, paginated results, previews, navigation, and transient verse focus. It also completes the previously requested safety and localization fixes.
Diagram
Reviews (13) · Last reviewed commit: "fix(ci): align release tooling manifests..."