feat(checks): highlight the repeated word in its verse on each check card#345
Conversation
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds verse-context highlighting for repeated-word checks: a new ChangesVerse-context highlighting feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/checks/highlight/verse-window.ts`:
- Around line 56-63: The start-boundary selection in pickNearest can incorrectly
prefer a whitespace candidate over a true verse-boundary candidate when
distances and indices tie, causing truncatedStart to be set at the actual verse
start. Update the tie-break logic in pickNearest (and the start-selection path
that uses it) to compare isBoundary before applying the index-based outward
preference, so exact START ties always favor the boundary candidate over
whitespace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eab3f926-fda8-4554-b775-b673bc714104
📒 Files selected for processing (11)
src/features/bible/components/DraftingUI.tsxsrc/features/checks/components/ChecksPanel.test.tsxsrc/features/checks/components/ChecksPanel.tsxsrc/features/checks/components/FindingRow.test.tsxsrc/features/checks/components/FindingRow.tsxsrc/features/checks/highlight/verse-window.constants.tssrc/features/checks/highlight/verse-window.test.tssrc/features/checks/highlight/verse-window.tssrc/features/checks/hooks/useRepeatedWordsCheck.test.tssrc/features/checks/hooks/useRepeatedWordsCheck.tssrc/features/flags/useFeatureFlags.ts
…ghlight
Task 1 of the highlight-repeated-word-in-verse follow-on. Adds a pure,
framework-free windowing util plus its single constants module and unit
tests. No call sites changed.
- verse-window.constants.ts: single frozen VERSE_WINDOW config (context
chars before/after + bidirectional maxSpaceSearchDistance).
- verse-window.ts: buildVerseWindow(verseText, matchStart, matchLength)
returns { before, match, after, truncatedStart, truncatedEnd }. Snaps
each cut to the nearest whitespace/verse-boundary within the +/- radius,
strips whole whitespace runs, and hard-cuts when no candidate is in
range (space-less-script safe). Surf-agnostic: match is always derived
from verseText.slice.
- verse-window.test.ts: 24 cases covering short/long verses, nearest-snap
and tie-breaks, boundary candidates, whitespace-run stripping, match at
verse edges, surf-agnostic slicing, and defensive clamps.
…on each check card Thread an as-checked snt_id → verseText snapshot (hydrated onto the check result in useRepeatedWordsCheck, keyed by the shared buildSntId) through DraftingUI → ChecksPanel → FindingRow, and render the verse text windowed around the match via buildVerseWindow with the matched slice emphasized in place: red + bold on active cards, bold only on dimmed/ignored cards. Falls back to the bare finding.surf when the snapshot has no entry. Wire types untouched (web-only post-hydration layer).
The LeftPanel tabpanel slot clips overflow (its ancestor sidebar wrapper is overflow-hidden), and ChecksPanel had no scroll container of its own — so findings past the fold were unreachable. Give the panel root h-full + overflow-y-auto, mirroring how ResourcePanel owns its own scrolling in the same slot. Adds a regression test asserting the scroll-container classes.
501fb9d to
1e58309
Compare
CodeRabbit flagged that when the verse text begins with whitespace, the START snap could tie between the whitespace candidate at index 0 and the verse-start boundary candidate, pick the whitespace one, and render a spurious leading ellipsis. Break distance ties in favor of the boundary candidate, and strip the leading-whitespace run on boundary reaches so 'before' stays whitespace-free. The match slice is untouched (and the existing step-7 clamp keeps the strip from ever crossing into it), so the highlight cannot shift.
2 participants
Show the repeated word highlighted in its verse, inside the check card
Follow-up to the recently merged Repeated Word Check feature (#277 / #278). Each check card previously showed only the bare matched fragment (
finding.surf, e.g. "the the") with no surrounding context. Joel asked for the repeated word to be highlighted so it's easy to spot. This PR renders, on every check card, a window of the verse text around the match with the repeated word emphasized in place — red + bold on active cards, bold-only on ignored/dimmed cards.Background — why this supersedes #328
Joel's highlight request was initially understood as highlighting in the editor pane, which would have meant turning the plain
<textarea>drafting surface into a rich-text/overlay editor — a significant change, analyzed in the docs-only proposal PR #328. Talking it through in person, the actual ask was much simpler: highlight the word in the check card. Since the in-card version is a small, contained change, this PR skips the separate proposal round and proposes the solution via the implementation itself. #328 is closed as superseded (its editor-surface analysis remains useful historical context if in-editor highlighting is ever wanted).What's in here
1. A pure verse-window util —
src/features/checks/highlight/verse-window.tsbuildVerseWindow(verseText, matchStart, matchLength)returns{ before, match, after, truncatedStart, truncatedEnd }. It slices the highlightedmatchfrom the verse text itself (surf-agnostic: what's shown is exactly what's in the verse, even if casing/whitespace differs from the finding'ssurf), extends the context window to word boundaries where possible, strips boundary whitespace, and reports truncation so the renderer can add…only at real cuts. All tunables (context width, word-boundary search distance) live in a single constants module,verse-window.constants.ts— no magic numbers in the render path.2. An as-checked verse-text snapshot ("hydration") —
useRepeatedWordsCheckFindings carry offsets (
start_position,surf.length) relative to the exact verse text sent in the check request, but the response doesn't echo that text back. The hook is the one place that both builds the request and receives the response, so it captures asnt_id → verseTextmap from the request it just sent and layers it onto the settled result (HydratedRepeatedWordsResponse). Because snapshot and findings travel together, offsets always line up and the cards hold still while the translator keeps typing — the "live text drifted out from under stale offsets" failure mode is designed out rather than guarded. The snake_case wire contract (fluent-ai pass-through) is untouched; the snapshot is a web-only, post-hydration value.3. Card rendering —
DraftingUI→ChecksPanel→FindingRowThe snapshot is threaded as a
verseTextBySntIdprop onto the inline<ChecksPanel>and down to eachFindingRow. A single shared sub-render (VerseContext) is used by both the active and the ignored branches:text-red-600 font-semibold.font-semiboldonly — ignored means "designated not a problem", so the alarm-red is dropped; bold still shows which word was flagged. (Bold rather than a darker color because the dimmed card'sopacity-50wrapper creates a stacking context its children can't escape — color would be washed out, font-weight isn't.)…only where the window actually truncates the verse.snt_id, the card renders the barefinding.surfexactly as before — never a crash, never an empty card.Verse text is looked up by the finding's own
snt_idfield; no occurrence-key parsing was added.Overlapping triples ("the the the") behave as before: two findings → two cards, each highlighting its own span; their windows may overlap, which is accepted for v1. Ignore Here on one card still leaves the other active (unchanged suppression semantics).
4. Checks panel scrolling fix — with taller cards the panel could overflow past the fold with no way to reach the rest; the panel now owns its own vertical scrolling (
h-full overflow-y-auto), mirroring howResourcePanelscrolls in the same tab slot.Testing
buildVerseWindow(boundaries, word-boundary snapping, truncation flags, degenerate inputs).FindingRow/ChecksPaneltests: windowed render + highlight classes per branch, surf-agnostic slice (verse text ≠ surf), ellipsis behavior, missing-snapshot fallback, overlapping-triple cards, snapshot threading, scroll-container regression; all pre-existing grouping/toggle/wiring tests still pass.useRepeatedWordsChecktest locks in the hydration contract (snapshot keyed by the samebuildSntIdas the findings, mirroring the request's non-empty filter).Relates to #277 / #278. Supersedes #328.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes