PT-4026: conflict-note card (Accept/Reject with dynamic Result preview)

[rolfheij-sil]

What & why

Part of epic PT-4024 (Donna resolves merge conflicts), stacked on #2482 (the PT-4028 read-contract — this PR's base is pt-4028-conflict-note-fields). It adds the presentational conflict-resolution card so a translator can see what is rejected, what is accepted, and how the verse will read after their choice (NN3), and accept or reject it (NN4).

ConflictNoteCard — a body fragment in the shared comment-list lib — renders:

• a conflictType-keyed description,
• an Accept/Reject selector,
• Rejected & Accepted diff regions (PT9 <u>/<s> markup, colored via theme tokens),
• a read-only Result preview that tracks the selection — resultText on accept, rejectedResultText on reject.

PT9-anchored behavior

Matches PT9's AppendConflictNoteDetails: the structured card renders when conflictType === 'verseText' && !!resultText; the Accepted region is shown only when acceptedText is present. So no-common-ancestor conflicts — two translators independently drafting a previously-absent verse, which have no accepted-side diff — still get the full resolution UI, just without the Accepted panel (the selector + dynamic Result preview still work). Genuinely non-verseText notes fall back to the existing contents render.

The dynamic Result preview relies on rejectedResultText (the loser's plain USFM via PT9's public GetDiffVerseUsfm), which was added to the read-contract in the base PR #2482 (along with a no-ancestor converter test and softened acceptedText docs).

Scope

• Presentation only — no PAPI, no live data. The card emits the choice via onResolutionChange; canAcceptReject gates the selector.
• verseText conflicts only (v1).
• Integration (rendering conflict threads with this card, wiring resolve to PT-4029, contributing localized string values) is PT-4030. ConflictNoteCard + CONFLICT_NOTE_STRING_KEYS are exported from platform-bible-react for that ticket.

Testing

• Vitest: 3-region render, dynamic Result toggle (including a real Radix Select interaction firing onResolutionChange), the no-ancestor case (Accepted omitted, selector + Result still work), non-verseText fallback, disabled selector, and read-only Result.
• Storybook (Advanced/ConflictNoteCard): Default, NoAncestor, RestrictedPermissions. Sample field values are captured verbatim from the PT-4028 C# converter — not hand-written.

🤖 Generated with Claude Code

---

This change is 

---

Review summary (/code-review max + /review-paratext, 2026-07-03)

Reviewed with a max-effort multi-agent /code-review (32 agents → 15 distinct defects) plus the four
/review-paratext analyzers. All Critical and Important findings were fixed in-review (commit
9f0c52a). Branch is green: typecheck, lint, format:base:check, unit 230/230, Storybook 3/3, C#
23/23. Full findings (incl. dismissed/deferred) are in the GitHub review on this PR.

⚠️ Supersedes "PT9-anchored behavior" above: the structured card now renders when
conflictType === 'verseText' alone (the && !!resultText gate was removed). A two-agent spike
against PT9 source confirmed PT9 gates only on isFirstComment && Contents?.FirstChild != null and
never on result-emptiness; that blank-resultText state is not even producible from a real merge
(a deleted verse creates no note; an emptied verse keeps its \v N marker). The card now degrades
gracefully per-region instead of hiding the whole resolution UI.

Key changes made in review:

• Robustness / PT9 parity: gate on conflictType alone; gate the Rejected region on
  !!rejectedText (like Accepted); neutral empty-state in the Result preview when the chosen side has
  no result USFM.
• Ship-blockers: regenerated the tracked platform-bible-react dist/ (new exports were missing);
  fixed the prettier failure that would have red-CI'd the branch.
• Dark-mode contrast: new per-scheme --diff-deleted theme token (deletion text was
  background-grade --destructive, failing AA in Platform dark).
• Localization: frozen as const key tuple + derived type + optional localizedStrings prop
  (guide-conforming); keys renamed to camelCase %conflictNote_*% before PT-4030 locks values in.
• A11y / quality: aria-live on the Result preview; shared prose classes with CommentItem;
  controlled-mode state-leak guard; min-w selector; renamed the both-sides C# fixture/test/sample;
  added edge-case + regression tests (11 unit tests).

Dismissed: whitespace-only-diff invisibility (degenerate conflict); missing pr-twp (family
consistency). Deferred to PT-4030: reset uncontrolled state on comment change (key the card by
comment id) + contributing the 9 %conflictNote_*% string values. Follow-up: shared jsdom test
setup (4th copy).

[rolfheij-sil]

Automated code review — /code-review max + /review-paratext

Reviewed 2026-07-03 with a max-effort multi-agent /code-review (32 agents, 36 verified findings → 15 distinct) and the four /review-paratext analyzers (api / style / compliance / ux). Where I1/I2/I3 hinged on PT9 behavior, a two-agent spike established ground truth against PT9 source (ParatextBase/ParatextData) and the PT10 converter before any behavior change.

Everything found is listed below — including items dismissed or deferred — with how each was handled. All Critical and Important findings were fixed in-review (commit 9f0c52a). Branch is green: typecheck · eslint · format:base:check · Vitest unit 230/230 · Storybook (chromium) 3/3 · C# PlatformCommentConverterTests 23/23. The tracked dist/ and themes.data.json were regenerated/synced.

Critical

#	Finding	Handling
C1	Stale tracked dist/ — new exports were added to src/index.ts but the git-tracked build output consumers resolve was not rebuilt, so PT-4030's import { ConflictNoteCard } would fail TS2305 / crash at runtime; no CI step catches it.	Fixed — regenerated platform-bible-react dist; exports now present in dist/index.d.ts/index.js.

Important

#	Finding	Handling
I1	!!resultText gate hid the entire resolution UI when the merged result verse is blank, diverging from PT9 (AppendConflictNoteDetails gates only on isFirstComment && Contents?.FirstChild != null, never on result-emptiness).	Fixed — gate on conflictType === 'verseText' alone. Spike finding: the trigger state is not producible from real merge data (a deleted verse creates no note — BookFileMerger.cs:441-445; an emptied verse keeps its \v N marker so Comment.Verse is non-blank). So this was a defensive-but-wrong gate, not a live stranding bug — but the fix restores PT9 parity and hardens the card.
I2	Rejected region heading rendered unconditionally → orphan "Rejected" heading over an empty body when rejectedText is absent.	Fixed — gated on !!rejectedText, symmetric with the Accepted region (matches PT9, which prints the reject label only when the rejected side is non-null).
I3	Blank <p> in the Result preview when Reject is selected and rejectedResultText is absent.	Fixed — neutral empty-state string ("The verse will be empty.") instead of an empty paragraph.
I4	Deletion text below AA contrast in Platform dark mode — <s> used background-grade --destructive (~2–2.5:1), while insertions used the text-grade --success-foreground.	Fixed — added a per-scheme text-grade --diff-deleted token (red-600 light / red-400 dark) in index.css; themes.data.json auto-synced; <s> now uses it.
I5	CI prettier/format gate would fail on the component + sample data (root --check misses it via .prettierignorerun; the lib workspace check exits 1).	Fixed — reformatted; whole-lib format:base:check passes.
I6a	Localization contract deviated from the Localization-Guide — mutable LocalizeKey[] + required localizedStrings prop (guide/newer components use a frozen tuple + derived type + optional prop).	Fixed — Object.freeze([...] as const), derived ConflictNoteCardLocalizedStrings, optional prop.
I6b	Snake_case key prefix %conflict_note_*% (guide prefers camelCase; keys are immutable once shipped).	Fixed — renamed to %conflictNote_*% before PT-4030 contributes values.
I6c	?? 'English' fallbacks are dead through the documented hook path (useLocalizedStrings returns missing keys as the key itself).	Addressed by the contract change; the fallbacks remain a defensive default. Contributing string values is PT-4030's job.
I7	Fallback path skipped trimDiffSpanWhitespace while styling with diff classes → potential dangling strikethrough.	Obsoleted by the I1 fix — the fallback now renders only non-verseText conflicts, which carry message-only contents with no <s>/<u> markup.
I8	Result preview not announced to assistive tech — no aria-live; screen-reader users don't hear the Accept↔Reject swap.	Fixed — aria-live="polite" region + aria-labelledby to the Result label, keyed with useId for list uniqueness.
I9	DIFF_HTML_CLASSES copy-pasted CommentItem's prose/blockquote stack — silent drift risk.	Fixed — extracted COMMENT_BODY_PROSE_CLASSES; both consume it.
I10	Confusing C# fixture naming — new CreateVerseTextConflictCommentWithReplacement vs base's …Replacement; TS sample named after the wrong fixture.	Fixed — renamed the fixture, its test, and the sample to …ReplacementBothSides.
I11	Missing edge-case tests for the new gating/absent-field logic.	Fixed — added tests for verseText-without-resultText, absent-rejectedText, absent-rejectedResultText-on-reject, aria-live, and the controlled-mode state-leak (11 unit tests total).

Minor

#	Finding	Handling
M1	Whitespace-only diff invisible — trimDiffSpanWhitespace collapses <s> </s> to an empty element.	Dismissed — a whitespace-only conflict is degenerate; the trim is deliberate for the common case.
M2	Controlled-mode internal-state leak — handleChange wrote internalResolution even when controlled, so a declined choice could resurface if the parent later stopped controlling.	Fixed — write internal state only when uncontrolled; test added.
M3	Uncontrolled state not reset on comment change — a reused instance leaks the prior conflict's choice.	Deferred to PT-4030 — the list integration should key the card by comment id (not a presentational-card concern).
M4	w-32 clips long localized selector values (no tooltip fallback).	Fixed — min-w-32 keeps the stable width without clipping.
M5	Missing pr-twp on the root element.	Dismissed — the whole comment-list family omits it and the card only renders inside CommentList; matching the family is correct.
M6	4th verbatim copy of the jsdom NoopResizeObserver/pointer stubs across test files.	Deferred to a follow-up — a shared setupFiles touches 4 test files + vitest config, out of scope here.
M7	"captured verbatim / do not hand-edit" JSDoc contradicted by hand-renamed id/thread.	Fixed — JSDoc now scopes the verbatim claim to the HTML/USFM fields and notes the readability rename.

The workflow also surfaced ~6 additional lowest-severity cleanup candidates that fell under its 15-decision cap; the ones that mattered are captured above (I9, M6).

Positive observations

PT9 anchoring verified against real PT9 source (region order, accepted-region gating, default 'accept', permission gating); purely additive public API with all referenced types exported; DOMPurify on every HTML injection; RTL-safe logical properties; redundant color encoding (semibold/strikethrough) for color-blind users; strong verified-green tests including a real Radix Select interaction; sample data cross-anchored to a matching C# fixture + converter test.

Suggested reviewer focus

• The I1 gate change (drop && !!resultText) — confirm you agree it's the right PT9-parity call; the spike concluded the blank-resultText state is unproducible from real merges.
• The new --diff-deleted theme token — verify the red-600/red-400 values and that nothing else expected <s> to use --destructive.
• The localization key rename (%conflictNote_*%, camelCase) — effectively immutable once PT-4030 ships values.

Automated review by Claude Fable 5 (analysis) and Claude Opus 4.8 (spike + fixes).

[lyonsil]

Follow-up review pass (automated multi-agent review, human-curated). A few non-blocking notes inline — one correctness edge case around the Result empty-state, the rest test-coverage / cleanup / consistency. None block merge for this PR's presentation-only scope.

[lyonsil]

Claude found a few things, and I didn't see anything else in my scan of the changes. I ignored some of the Claude findings and only posted the ones that seemed potentially valid. None of them are really serious, but the ones I posted seem considering at least.

@lyonsil reviewed 22 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on rolfheij-sil).

[lyonsil]

@lyonsil reviewed 13 files and all commit messages, made 1 comment, and resolved 6 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved.