diff --git a/src/features/bible/components/DraftingUI.tsx b/src/features/bible/components/DraftingUI.tsx index d34dc73..053dfed 100644 --- a/src/features/bible/components/DraftingUI.tsx +++ b/src/features/bible/components/DraftingUI.tsx @@ -37,6 +37,14 @@ import { DraftingGridVerse, DraftingTargetColumn } from './DraftingGridVerse'; import { DraftingHeader } from './DraftingHeader'; import { DraftingResourceSidebar } from './DraftingResourceSidebar'; +/** + * Stable empty snapshot for renders before the first check result settles — + * a module-level constant so the `ChecksPanel` prop reference doesn't change + * (and re-render) on every DraftingUI render. FindingRow falls back to + * `finding.surf` on a miss, so "empty" is always safe. + */ +const EMPTY_VERSE_TEXT_SNAPSHOT: ReadonlyMap = new Map(); + const RESOURCE_NAMES: ResourceName[] = [ { id: 'UWTranslationNotes', name: 'TN' }, { id: 'Images', name: 'Images & Maps' }, @@ -253,6 +261,14 @@ export const DraftingUI: React.FC = ({ globalRules, }); + // As-checked verse-text snapshot, hydrated onto the settled check result by + // useRepeatedWordsCheck (NOT derived from live `verses` — the findings' + // offsets are relative to the text as it was checked, so the two must travel + // together and update only when a new result arrives). `checkQuery.data` is + // referentially stable per settled result, so this reference only changes + // when new findings do. + const verseTextBySntId = checkQuery.data?.verseTextBySntId ?? EMPTY_VERSE_TEXT_SNAPSHOT; + // Active-finding count drives both notification dots; computed once here and // threaded to the tab and the closed-panel toggle button (S5, §6.4). When the // Checks feature is disabled it is forced to 0 so neither dot can flash while @@ -634,6 +650,7 @@ export const DraftingUI: React.FC = ({ globalIgnoresAvailable={globalIgnoresAvailable} isError={checkQuery.isError} resolved={resolved} + verseTextBySntId={verseTextBySntId} onIgnoreEverywhere={ignoreEverywhere} onIgnoreHere={ignoreHere} onStopIgnoringEverywhere={stopIgnoringEverywhere} diff --git a/src/features/checks/components/ChecksPanel.test.tsx b/src/features/checks/components/ChecksPanel.test.tsx index 8e5a404..e350ec4 100644 --- a/src/features/checks/components/ChecksPanel.test.tsx +++ b/src/features/checks/components/ChecksPanel.test.tsx @@ -38,12 +38,23 @@ const handlers = () => ({ onStopIgnoringEverywhere: vi.fn(), }); -const renderPanel = (resolved: ResolvedFindings, over: Partial<{ isError: boolean }> = {}) => +/** + * Empty as-checked snapshot: the pre-existing grouping/toggle/wiring tests + * exercise the fallback path (rows render `finding.surf` bare), which keeps + * their text assertions unchanged. + */ +const emptySnapshot: ReadonlyMap = new Map(); + +const renderPanel = ( + resolved: ResolvedFindings, + over: Partial<{ isError: boolean; verseTextBySntId: ReadonlyMap }> = {} +) => renderWithProviders( ); @@ -105,7 +116,13 @@ describe('ChecksPanel — Show Ignored toggle', () => { inactive: [inactive('JDG 4:1', 'and and', 'occurrence')], }; const { user } = renderWithProviders( - + ); // Default OFF: the inactive snippet is not shown. @@ -127,7 +144,13 @@ describe('ChecksPanel — Show Ignored toggle', () => { inactive: [inactive('JDG 4:2', 'and and', 'occurrence')], }; const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('switch', { name: 'Show Ignored' })); @@ -150,7 +173,13 @@ describe('ChecksPanel — Show Ignored toggle', () => { inactive: [inactive('JDG 4:1', 'and and', 'global')], }; const { user, unmount } = renderWithProviders( - + ); await user.click(screen.getByRole('switch', { name: 'Show Ignored' })); expect(screen.getByText('and and (in JDG 4:1)')).toBeInTheDocument(); @@ -158,7 +187,13 @@ describe('ChecksPanel — Show Ignored toggle', () => { // Fresh mount = fresh session: toggle is OFF again. renderWithProviders( - + ); expect(screen.queryByText('and and (in JDG 4:1)')).not.toBeInTheDocument(); }); @@ -188,7 +223,13 @@ describe('ChecksPanel — "Show Ignored" toggle visibility', () => { inactive: [inactive('JDG 4:1', 'the the', 'legitimate')], }; renderWithProviders( - + ); expect(screen.queryByTestId('ignored-section')).not.toBeInTheDocument(); }); @@ -201,7 +242,13 @@ describe('ChecksPanel — "Show Ignored" toggle visibility', () => { inactive: [inactive('JDG 4:1', 'the the', 'occurrence')], }; renderWithProviders( - + ); expect(screen.getByText('No issues found')).toBeInTheDocument(); expect(screen.queryByTestId('ignored-section')).not.toBeInTheDocument(); @@ -213,7 +260,13 @@ describe('ChecksPanel — "Show Ignored" toggle visibility', () => { inactive: [inactive('JDG 4:1', 'the the', 'occurrence')], }; const { user } = renderWithProviders( - + ); // Toggle is OFF: zero state is visible. expect(screen.getByText('No issues found')).toBeInTheDocument(); @@ -235,7 +288,13 @@ describe('ChecksPanel — "Show Ignored" toggle visibility', () => { ], }; const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('switch', { name: 'Show Ignored' })); @@ -254,7 +313,13 @@ describe('ChecksPanel — callback wiring', () => { inactive: [], }; const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('button', { name: 'Ignore Here' })); expect(h.onIgnoreHere).toHaveBeenCalledWith('JDG 4:1|the the|0'); @@ -268,10 +333,98 @@ describe('ChecksPanel — callback wiring', () => { inactive: [inactiveFinding], }; const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('switch', { name: 'Show Ignored' })); await user.click(screen.getByRole('button', { name: 'Undo Ignore' })); expect(h.onUndo).toHaveBeenCalledWith(inactiveFinding); }); }); + +describe('ChecksPanel — scroll container', () => { + it('owns its own vertical scrolling (the LeftPanel tabpanel slot clips overflow)', () => { + // Regression: without h-full + overflow-y-auto on the panel root, findings + // past the fold were unreachable ("lost to the land of the living"). + renderPanel({ active: [active('JDG 4:1', 'the the')], inactive: [] }); + const root = screen.getByTestId('checks-panel-scroll'); + expect(root).toHaveClass('h-full', 'overflow-y-auto'); + }); +}); + +describe('ChecksPanel — verse-context snapshot threading', () => { + it('threads verseTextBySntId down so rows render the windowed highlight', () => { + // 'the the' at [4, 11) in the as-checked verse text for JDG 4:1. + const verse = 'And the the light was made'; + const finding: ResolvedFinding = { + ...active('JDG 4:1', 'the the'), + finding: { ...active('JDG 4:1', 'the the').finding, surf: 'the the', start_position: 4 }, + }; + renderPanel( + { active: [finding], inactive: [] }, + { verseTextBySntId: new Map([['JDG 4:1', verse]]) } + ); + const highlight = screen.getByTestId('verse-highlight'); + expect(highlight.textContent).toBe('the the'); + expect(highlight.parentElement?.textContent).toBe('And the the light was made'); + }); + + it('overlapping triple: two findings in one verse each render their own window', () => { + // "the the the" produces TWO findings (ordinals 0 and 1) with their own + // start_positions; each card highlights its own span independently. + const verse = 'And the the the light'; + const base = active('JDG 4:1', 'the the'); + const first: ResolvedFinding = { + ...base, + finding: { ...base.finding, surf: 'the the', start_position: 4 }, + ordinal: 0, + occurrenceKey: 'JDG 4:1|the the|0', + }; + const second: ResolvedFinding = { + ...base, + finding: { ...base.finding, surf: 'the the', start_position: 8 }, + ordinal: 1, + occurrenceKey: 'JDG 4:1|the the|1', + }; + renderPanel( + { active: [first, second], inactive: [] }, + { verseTextBySntId: new Map([['JDG 4:1', verse]]) } + ); + const highlights = screen.getAllByTestId('verse-highlight'); + expect(highlights).toHaveLength(2); + // Both highlight a 'the the' span; the windows around them differ by offset. + const rows = highlights.map(h => h.parentElement?.textContent); + expect(rows).toEqual(['And the the the light', 'And the the the light']); + expect(highlights[0].textContent).toBe('the the'); + expect(highlights[1].textContent).toBe('the the'); + }); + + it('ignoring one occurrence of an overlapping pair leaves the other card active', async () => { + // Reuses the cascade contract: "Ignore Here" is keyed by the FULL + // occurrence key including the ordinal, so ignoring card 0 must not ignore + // card 1 (the panel simply forwards the row's own occurrenceKey). + const base = active('JDG 4:1', 'the the'); + const first: ResolvedFinding = { ...base, ordinal: 0, occurrenceKey: 'JDG 4:1|the the|0' }; + const second: ResolvedFinding = { ...base, ordinal: 1, occurrenceKey: 'JDG 4:1|the the|1' }; + const h = handlers(); + const { user } = renderWithProviders( + + ); + const buttons = screen.getAllByRole('button', { name: 'Ignore Here' }); + expect(buttons).toHaveLength(2); + await user.click(buttons[0]); + expect(h.onIgnoreHere).toHaveBeenCalledTimes(1); + expect(h.onIgnoreHere).toHaveBeenCalledWith('JDG 4:1|the the|0'); + }); +}); diff --git a/src/features/checks/components/ChecksPanel.tsx b/src/features/checks/components/ChecksPanel.tsx index 66affa3..6bed1b9 100644 --- a/src/features/checks/components/ChecksPanel.tsx +++ b/src/features/checks/components/ChecksPanel.tsx @@ -17,6 +17,13 @@ import { FindingRow } from './FindingRow'; export interface ChecksPanelProps { /** Cascade output: `{ active[], inactive[] }` from `useResolvedFindings`. */ resolved: ResolvedFindings; + /** + * As-checked verse text keyed by `snt_id` — the hydrated snapshot that + * travels with the check result (captured in `useRepeatedWordsCheck`), NOT + * the live drafting text. Each row windows its finding against this so the + * offsets always line up and the cards hold still between checks. + */ + verseTextBySntId: ReadonlyMap; /** The check query errored — show the inline red line (W9). */ isError: boolean; /** Hide the global "Ignore Everywhere" affordance when unavailable (W8). */ @@ -97,6 +104,7 @@ const groupByVerse = (findings: ResolvedFinding[]): VerseGroup[] => { const renderGroups = ( groups: VerseGroup[], globalIgnoresAvailable: boolean, + verseTextBySntId: ReadonlyMap, handlers: Pick< ChecksPanelProps, 'onIgnoreHere' | 'onIgnoreEverywhere' | 'onUndo' | 'onStopIgnoringEverywhere' @@ -111,6 +119,7 @@ const renderGroups = ( key={row.occurrenceKey} globalIgnoresAvailable={globalIgnoresAvailable} resolved={row} + verseTextBySntId={verseTextBySntId} onIgnoreEverywhere={handlers.onIgnoreEverywhere} onIgnoreHere={handlers.onIgnoreHere} onStopIgnoringEverywhere={handlers.onStopIgnoringEverywhere} @@ -135,6 +144,7 @@ const renderGroups = ( */ export const ChecksPanel: React.FC = ({ resolved, + verseTextBySntId, isError, globalIgnoresAvailable, onIgnoreHere, @@ -156,7 +166,10 @@ export const ChecksPanel: React.FC = ({ const showZeroState = !isError && activeGroups.length === 0 && !showingIgnored; return ( -
+ // Like ResourcePanel, each tab body owns its own scrolling: the LeftPanel + // tabpanel is a clipped `min-h-0 flex-1` slot, so without `overflow-y-auto` + // here findings past the fold would be unreachable. +
{isError && (

Checks failed to refresh @@ -171,7 +184,9 @@ export const ChecksPanel: React.FC = ({ {showZeroState ? (

No issues found

) : ( -
{renderGroups(activeGroups, globalIgnoresAvailable, handlers)}
+
+ {renderGroups(activeGroups, globalIgnoresAvailable, verseTextBySntId, handlers)} +
)} {hasIgnored && ( @@ -189,7 +204,7 @@ export const ChecksPanel: React.FC = ({ {showingIgnored && (
- {renderGroups(ignoredGroups, globalIgnoresAvailable, handlers)} + {renderGroups(ignoredGroups, globalIgnoresAvailable, verseTextBySntId, handlers)}
)} diff --git a/src/features/checks/components/FindingRow.test.tsx b/src/features/checks/components/FindingRow.test.tsx index bb7a222..9e15cf8 100644 --- a/src/features/checks/components/FindingRow.test.tsx +++ b/src/features/checks/components/FindingRow.test.tsx @@ -28,10 +28,22 @@ const handlers = () => ({ onStopIgnoringEverywhere: vi.fn(), }); +/** + * Empty as-checked snapshot: exercises the fallback path (render `finding.surf` + * bare) in the pre-existing action/label tests, which don't care about the + * verse window. + */ +const emptySnapshot: ReadonlyMap = new Map(); + describe('FindingRow — active', () => { it('renders the surface snippet and both ignore buttons', () => { renderWithProviders( - + ); expect(screen.getByText('and The the made')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Ignore Here' })).toBeInTheDocument(); @@ -41,7 +53,12 @@ describe('FindingRow — active', () => { it('fires onIgnoreHere with the occurrence key', async () => { const h = handlers(); const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('button', { name: 'Ignore Here' })); expect(h.onIgnoreHere).toHaveBeenCalledWith('JDG 4:3|the the|0'); @@ -49,7 +66,12 @@ describe('FindingRow — active', () => { it('hides Ignore Everywhere when global ignores are unavailable', () => { renderWithProviders( - + ); expect(screen.queryByRole('button', { name: 'Ignore Everywhere' })).not.toBeInTheDocument(); }); @@ -57,7 +79,12 @@ describe('FindingRow — active', () => { it('Ignore Everywhere opens a confirm dialog and only fires on confirm', async () => { const h = handlers(); const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('button', { name: 'Ignore Everywhere' })); @@ -74,7 +101,12 @@ describe('FindingRow — active', () => { it('Cancel in the confirm dialog does not fire onIgnoreEverywhere', async () => { const h = handlers(); const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('button', { name: 'Ignore Everywhere' })); await user.click(screen.getByRole('button', { name: 'Cancel' })); @@ -94,6 +126,7 @@ describe('FindingRow — inactive', () => { ); @@ -108,7 +141,12 @@ describe('FindingRow — inactive', () => { const h = handlers(); const resolved = makeResolved({ isActive: false, inactiveReason: 'occurrence' }); const { user } = renderWithProviders( - + ); await user.click(screen.getByRole('button', { name: 'Undo Ignore' })); expect(h.onUndo).toHaveBeenCalledWith(resolved); @@ -120,6 +158,7 @@ describe('FindingRow — inactive', () => { ); @@ -133,6 +172,7 @@ describe('FindingRow — inactive', () => { ); @@ -141,3 +181,87 @@ describe('FindingRow — inactive', () => { expect(screen.getByRole('button', { name: 'Undo Ignore' })).toBeInTheDocument(); }); }); + +describe('FindingRow — verse-context window', () => { + // The as-checked verse text for the default finding: the highlighted slice at + // [4, 4 + surf.length) is 'The the' — deliberately DIFFERENT casing from + // `finding.surf` ('and The the made' would not match anywhere) to lock in the + // surf-agnostic contract: the card must highlight the verse slice, never surf. + const verse = 'And The the made light'; + const snapshot: ReadonlyMap = new Map([['JDG 4:3', verse]]); + // surf's length (7) is all that matters for the window; its text is unused. + const windowed = makeResolved({ + finding: { ...makeResolved().finding, surf: 'the the', start_position: 4 }, + }); + + it('active card renders the verse window with the VERSE slice highlighted (red + bold)', () => { + renderWithProviders( + + ); + const highlight = screen.getByTestId('verse-highlight'); + // The verse slice ('The the'), NOT finding.surf ('the the'). + expect(highlight).toHaveTextContent('The the'); + expect(highlight.textContent).toBe('The the'); + expect(highlight).toHaveClass('text-red-600', 'font-semibold'); + // before + match + after reassemble the (short, untruncated) verse — no ellipses. + expect(highlight.parentElement?.textContent).toBe('And The the made light'); + }); + + it('renders leading and trailing ellipses when the window cuts a long verse on both ends', () => { + // Match at [34, 41): 26 chars of raw context on each side cut inside the + // verse, snapping to whitespace (constants: 26/26, radius 10). + const longVerse = + 'one two three four five six seven the the eight nine ten eleven twelve thirteen'; + renderWithProviders( + + ); + const highlight = screen.getByTestId('verse-highlight'); + expect(highlight.textContent).toBe('the the'); + expect(highlight.parentElement?.textContent).toBe( + '… three four five six seven the the eight nine ten eleven twelve …' + ); + }); + + it('dimmed/ignored card renders the same window but with bold-only highlight (no red)', () => { + renderWithProviders( + + ); + const highlight = screen.getByTestId('verse-highlight'); + expect(highlight.textContent).toBe('The the'); + expect(highlight).toHaveClass('font-semibold'); + expect(highlight).not.toHaveClass('text-red-600'); + // The window renders alongside the ignore-type label, not instead of it. + expect(screen.getByTestId('inactive-label')).toHaveTextContent('Ignore Here'); + }); + + it('falls back to finding.surf when the snapshot has no text for the snt_id', () => { + renderWithProviders( + + ); + // Today's behavior: the bare surface text, no highlight span, no crash. + expect(screen.getByText('and The the made')).toBeInTheDocument(); + expect(screen.queryByTestId('verse-highlight')).not.toBeInTheDocument(); + }); +}); diff --git a/src/features/checks/components/FindingRow.tsx b/src/features/checks/components/FindingRow.tsx index 883f6da..5919dd0 100644 --- a/src/features/checks/components/FindingRow.tsx +++ b/src/features/checks/components/FindingRow.tsx @@ -19,7 +19,12 @@ import { } from '@/components/ui/dropdown-menu'; import { cn } from '@/lib/utils'; -import { type InactiveReason, type ResolvedFinding } from '../checks.types'; +import { + type InactiveReason, + type RepeatedWordsFinding, + type ResolvedFinding, +} from '../checks.types'; +import { buildVerseWindow } from '../highlight/verse-window'; /** * Human-readable ignore-type label for an inactive finding (§6.4, revised #278). @@ -38,6 +43,13 @@ const INACTIVE_LABEL: Record = { export interface FindingRowProps { /** The cascade-resolved finding to render. */ resolved: ResolvedFinding; + /** + * As-checked verse text keyed by `snt_id` (the hydrated snapshot captured by + * `useRepeatedWordsCheck` — the exact text the finding's offsets are relative + * to, NOT the live drafting text). Used to render the verse window around + * the repeated word; a missing entry falls back to `finding.surf`. + */ + verseTextBySntId: ReadonlyMap; /** Hide the global "Ignore Everywhere" affordance when the backend half is * absent (feature detection, W8). */ globalIgnoresAvailable: boolean; @@ -53,6 +65,65 @@ export interface FindingRowProps { onStopIgnoringEverywhere: (repeatedWord: string) => void; } +interface VerseContextProps { + /** The raw wire finding (offsets are relative to the snapshot text). */ + finding: RepeatedWordsFinding; + /** As-checked `snt_id → verseText` snapshot (see FindingRowProps). */ + verseTextBySntId: ReadonlyMap; + /** Active rows flag the word in red + bold; dimmed rows use bold alone. */ + isActive?: boolean; +} + +/** + * The verse-context snippet shared by BOTH the active and the inactive row + * branches: the verse text windowed around the match (Task 1's + * `buildVerseWindow`), with the repeated word emphasized in place. + * + * - Highlights `w.match` — the actual slice of the verse text — NOT + * `finding.surf` (the util is surf-agnostic; `surf` contributes only its + * length, so casing/whitespace always render as they appear in the verse). + * - Active rows: `text-red-600 font-semibold` (red flags "this is the flagged + * repeated word"). Dimmed/ignored rows: `font-semibold` only — ignored means + * "designated not a problem", so the alarm-red is deliberately dropped. + * NOTE: the dimmed card's `opacity-50` wrapper creates a stacking context + * descendants cannot escape (a child `opacity-100`/near-black is still + * washed to 50%), so bold — not color — is the reliable distinguisher there. + * - A leading/trailing `…` renders only for a real (non-boundary) cut; the + * spaces around the `…` are purely presentational (Task 1 already strips + * boundary whitespace from `before`/`after`). + */ +const VerseContext: React.FC = ({ + finding, + verseTextBySntId, + isActive = false, +}) => { + const verseText = verseTextBySntId.get(finding.snt_id); + + // Fallback: no snapshot text for this snt_id (should be rare — findings and + // their verse text are hydrated together, so a miss signals a malformed or + // absent snapshot rather than a routine race). Render the bare surface text + // exactly as before this feature: never crash, never render an empty card. + if (verseText === undefined) { + return

{finding.surf}

; + } + + const w = buildVerseWindow(verseText, finding.start_position, finding.surf.length); + const highlightClass = isActive ? 'text-red-600 font-semibold' : 'font-semibold'; + + return ( +

+ {w.truncatedStart && '… '} + {w.before} + {/* highlight w.match — the actual verse slice — NOT finding.surf */} + + {w.match} + + {w.after} + {w.truncatedEnd && ' …'} +

+ ); +}; + /** * One repeated-word finding row (§5.1, §6.4–§6.5). * @@ -70,6 +141,7 @@ export interface FindingRowProps { */ export const FindingRow: React.FC = ({ resolved, + verseTextBySntId, globalIgnoresAvailable, onIgnoreHere, onIgnoreEverywhere, @@ -78,14 +150,11 @@ export const FindingRow: React.FC = ({ }) => { const [confirmOpen, setConfirmOpen] = useState(false); const { finding, isActive, inactiveReason, occurrenceKey } = resolved; - // Display the original surface text (preserves casing); never compare on it — - // all matching already happened in the cascade (conventions §C). - const snippet = finding.surf; if (isActive) { return (
-

{snippet}

+