Skip to content

Commit 431d7b6

Browse files
rolfheij-silclaude
andcommitted
markers-checklist follow-up: TJ review polish + ScriptureRange contract fix
- ChecklistService: honor platform ScriptureRange contract — End == null now narrows to a single-verse range at Start (was: scan to last verse of project). No production caller triggers the changed branch; new regression test BuildChecklistData_VerseRangeEndOmitted_ScansSingleVerseAtStart covers it. - platform-scripture.d.ts: tighten verseRange.end TSDoc to reference the canonical ScriptureRange semantics; document '' fallback on ResolvedComparativeTexts.texts[].name / fullName when available is false. - LinkedScrRefButton: mirror the @experimental marker on the function itself so IDE / TypeDoc surfaces the warning at every call site (was: only on LinkedScrRefButtonProps). - popover.tsx (PopoverPortalContainerProvider): rephrase JSDoc so the inline useState<HTMLElement | null>(null) reference lands mid-line and isn't reflowed across paragraphs by Prettier. - Rename test ResolveComparativeTexts_MixedResolutionPaths_… → ResolveComparativeTexts_MixedAvailability_… (only one resolution path remains after name-fallback was removed). - Regenerate lib/platform-bible-react/dist for the JSDoc / @experimental changes. Includes benign CSS reordering churn (build-tool non-determinism). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 53580a9 commit 431d7b6

11 files changed

Lines changed: 115 additions & 41 deletions

File tree

c-sharp-tests/Checklists/ChecklistServiceBuildChecklistDataTests.cs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,65 @@ public void BuildChecklistData_VerseRangeStartAtChapter1Verse2_DoesNotAdjust()
542542
);
543543
}
544544

545+
[Test]
546+
[Category("Contract")]
547+
[Property("CapabilityId", "CAP-006")]
548+
[Property("Contract", "BuildChecklistData")]
549+
public void BuildChecklistData_VerseRangeEndOmitted_ScansSingleVerseAtStart()
550+
{
551+
// Platform ScriptureRange contract (lib/papi-dts):
552+
// "If not provided, then only the verse indicated by start is included."
553+
// Verify the checklist honors that contract — passing { Start, End: null }
554+
// must narrow to a single-verse range, NOT fall back to the project bounds.
555+
const string usfm = @"\id GEN \c 1 \v 1 alpha \p \v 2 bravo \p \v 3 charlie \p \v 4 delta";
556+
var scrText = RegisterDummyProject(usfm, bookNum: 1);
557+
558+
var request = BuildRequest(
559+
activeProjectId: scrText.Guid.ToString(),
560+
verseRange: new ScriptureRange(
561+
Start: new VerseRef("GEN", "1", "2", ScrVers.English),
562+
End: null
563+
),
564+
showVerseText: true
565+
);
566+
567+
ChecklistResult result = ChecklistService.BuildChecklistData(
568+
request,
569+
CancellationToken.None
570+
);
571+
572+
var verseTexts = result
573+
.Rows.SelectMany(r => r.Cells)
574+
.SelectMany(c => c.Paragraphs)
575+
.SelectMany(p => p.Items)
576+
.OfType<TextItem>()
577+
.Select(t => t.Text.Trim())
578+
.Where(t => !string.IsNullOrEmpty(t))
579+
.ToList();
580+
581+
// Only verse 2 should appear; verses 1, 3, 4 must be excluded.
582+
Assert.That(
583+
verseTexts,
584+
Has.Some.Contains("bravo"),
585+
"End == null must include the verse at Start (GEN 1:2)"
586+
);
587+
Assert.That(
588+
verseTexts,
589+
Has.None.Contains("alpha"),
590+
"End == null must NOT scan the whole project (verse 1 should be excluded)"
591+
);
592+
Assert.That(
593+
verseTexts,
594+
Has.None.Contains("charlie"),
595+
"End == null must NOT scan past the Start verse (verse 3 should be excluded)"
596+
);
597+
Assert.That(
598+
verseTexts,
599+
Has.None.Contains("delta"),
600+
"End == null must NOT scan past the Start verse (verse 4 should be excluded)"
601+
);
602+
}
603+
545604
// =====================================================================
546605
// Group D — Max rows truncation (TS-049, INV-012)
547606
// =====================================================================

c-sharp-tests/Checklists/ChecklistServiceResolveComparativeTextsTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace TestParanextDataProvider.Checklists;
1616
/// <para>
1717
/// Per strategic-plan-backend.md §CAP-009, this capability uses
1818
/// <b>Outside-In TDD</b>: the outer acceptance test
19-
/// (<see cref="ResolveComparativeTexts_MixedResolutionPaths_PreservesOrderAndCorrectlyFlagsAvailability"/>)
19+
/// (<see cref="ResolveComparativeTexts_MixedAvailability_PreservesOrderAndCorrectlyFlagsAvailability"/>)
2020
/// drives the full INV-014 contract; focused tests pin individual cases
2121
/// (TS-048 / PTX-23529 duplicate short names resolved by GUID,
2222
/// active-project exclusion).
@@ -97,7 +97,7 @@ private DummyScrText RegisterProject(string shortName)
9797
[Property("Contract", "ResolveComparativeTexts")]
9898
[Property("BehaviorId", "BHV-605")]
9999
[Property("Invariant", "INV-014")]
100-
public void ResolveComparativeTexts_MixedResolutionPaths_PreservesOrderAndCorrectlyFlagsAvailability()
100+
public void ResolveComparativeTexts_MixedAvailability_PreservesOrderAndCorrectlyFlagsAvailability()
101101
{
102102
// Arrange: register 3 projects in the shared ScrTextCollection.
103103
// - active : the "active" project (self-reference target)

c-sharp/Checklists/ChecklistService.cs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,10 +327,13 @@ ScrText active
327327
// EXPLANATION:
328328
// Mirrors PT9's pre-flight that converts the optional ScriptureRange
329329
// carried on the request into a concrete [startRef, endRef] pair,
330-
// falling back to mainScrText.FirstVerseRef() / LastVerseRef() when
331-
// the request's bounds are null or default. `FirstVerseRef` returns
332-
// "GEN 1:0" (intro verse) and `LastVerseRef` returns the final verse
333-
// of the final book of the versification.
330+
// honoring the platform ScriptureRange contract:
331+
// - range == null ⇒ whole project (FirstVerseRef..LastVerseRef)
332+
// - range.End == null ⇒ single-verse range at Start
333+
// - range.{Start,End}.IsDefault ⇒ defensive fallback to the project bounds
334+
// for malformed wire payloads (defaulted VerseRef)
335+
// `FirstVerseRef` returns "GEN 1:0" (intro verse) and `LastVerseRef`
336+
// returns the final verse of the final book of the versification.
334337
private static (VerseRef start, VerseRef end) ResolveVerseRange(
335338
ScrText mainScrText,
336339
ScriptureRange? range
@@ -353,8 +356,15 @@ private static (VerseRef start, VerseRef end) ResolveVerseRange(
353356
return (firstDefault, lastDefault);
354357

355358
VerseRef start = range.Start.IsDefault ? firstDefault : range.Start;
356-
VerseRef end =
357-
range.End == null || range.End.Value.IsDefault ? lastDefault : range.End.Value;
359+
// Platform ScriptureRange contract: End omitted ⇒ single-verse range at Start.
360+
// A defaulted End (malformed wire payload) falls back to the project bounds.
361+
VerseRef end;
362+
if (range.End == null)
363+
end = start;
364+
else if (range.End.Value.IsDefault)
365+
end = lastDefault;
366+
else
367+
end = range.End.Value;
358368
return (start, end);
359369
}
360370

extensions/src/platform-scripture/src/types/platform-scripture.d.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,7 +1661,8 @@ declare module 'platform-scripture' {
16611661
markerSettings: ChecklistMarkerSettings;
16621662
/**
16631663
* Inclusive verse range to limit the checklist to. When `undefined`, the entire project is
1664-
* scanned. When the range omits `end`, only the verse at `start` is included.
1664+
* scanned. Otherwise the standard {@link ScriptureRange} semantics apply: an omitted `end`
1665+
* narrows the request to the single verse at `start`.
16651666
*/
16661667
verseRange: ScriptureRange | undefined;
16671668
/** When true, rows whose cells all match (per `equivalentMarkers`) are omitted from output. */
@@ -1732,9 +1733,9 @@ declare module 'platform-scripture' {
17321733
texts: {
17331734
/** GUID of the resolved project (or the original requested id if resolution failed). */
17341735
id: string;
1735-
/** Short name of the resolved project. */
1736+
/** Short name of the resolved project, or `''` when `available` is `false`. */
17361737
name: string;
1737-
/** Full descriptive name of the resolved project. */
1738+
/** Full descriptive name of the resolved project, or `''` when `available` is `false`. */
17381739
fullName: string;
17391740
/** True when the project was successfully resolved and is currently registered. */
17401741
available: boolean;

lib/platform-bible-react/dist/index.cjs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4448,13 +4448,6 @@ video:where(.pr-twp,.pr-twp *) {
44484448
[data-side=secondary] .\\[\\[data-side\\=secondary\\]_\\&\\]\\:tw-cursor-w-resize {
44494449
cursor: w-resize;
44504450
}
4451-
.banded-row:hover {
4452-
cursor: pointer;
4453-
}
4454-
4455-
.banded-row[data-state='selected']:hover {
4456-
cursor: default;
4457-
}
44584451
/* By default the editor is too tall for the footnote editor, even while empty, so this makes it
44594452
shorter. */
44604453
.footnote-editor .editor-input {
@@ -4474,6 +4467,13 @@ video:where(.pr-twp,.pr-twp *) {
44744467
.footnote-editor .text-spacing .usfm_p {
44754468
text-indent: 0;
44764469
}
4470+
.banded-row:hover {
4471+
cursor: pointer;
4472+
}
4473+
4474+
.banded-row[data-state='selected']:hover {
4475+
cursor: default;
4476+
}
44774477
/**
44784478
* This file was automatically generated on installation of the Shadcn/Lexical editor. The default
44794479
* location of this file has been changed to integrate better with our project structure.

lib/platform-bible-react/dist/index.cjs.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/platform-bible-react/dist/index.d.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2080,6 +2080,10 @@ export type LinkedScrRefButtonProps = {
20802080
*
20812081
* If no `onClick` is provided, the button is disabled and the tooltip still surfaces (useful for
20822082
* read-only contexts where the reference should not be navigable but should still be readable).
2083+
*
2084+
* @experimental This component is expected to be removed once
2085+
* [`LinkedScrRefDisplay`](https://github.com/paranext/paranext-core/pull/1949) lands. See
2086+
* {@link LinkedScrRefButtonProps} for the migration plan.
20832087
*/
20842088
export declare function LinkedScrRefButton({ scrRef, onClick, tooltipContent, ariaLabel, className, testId, }: LinkedScrRefButtonProps): import("react/jsx-runtime").JSX.Element | undefined;
20852089
/**
@@ -2678,13 +2682,11 @@ export declare const PopoverAnchor: React$1.ForwardRefExoticComponent<PopoverPri
26782682
* rendered as React children. It does not retroactively re-portal already-mounted popovers, and it
26792683
* does not affect popovers in sibling subtrees.
26802684
*
2681-
* Initial-mount behavior: pass `null` for `container` (the initial value of a `useState<HTMLElement
2682-
*
2683-
* | null>(null)` paired with a ref callback on the ancestor) to keep Radix's default
2684-
*
2685-
* `document.body` behavior until the ancestor mounts. Once the element exists, future popover opens
2686-
* portal into it. The triggering ancestor (the trap owner) must wrap, not be wrapped by, this
2687-
* provider.
2685+
* Initial-mount behavior: pass `null` for `container` to keep Radix's default `document.body`
2686+
* behavior until the ancestor mounts. The typical pattern is to hold the ancestor element in
2687+
* `useState<HTMLElement | null>(null)` paired with a ref callback on the ancestor — once the
2688+
* element exists, future popover opens portal into it. The triggering ancestor (the trap owner)
2689+
* must wrap, not be wrapped by, this provider.
26882690
* @example
26892691
*
26902692
* ```tsx

lib/platform-bible-react/dist/index.js

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/platform-bible-react/dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/platform-bible-react/src/components/basics/linked-scr-ref-button.component.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ export type LinkedScrRefButtonProps = {
6262
*
6363
* If no `onClick` is provided, the button is disabled and the tooltip still surfaces (useful for
6464
* read-only contexts where the reference should not be navigable but should still be readable).
65+
*
66+
* @experimental This component is expected to be removed once
67+
* [`LinkedScrRefDisplay`](https://github.com/paranext/paranext-core/pull/1949) lands. See
68+
* {@link LinkedScrRefButtonProps} for the migration plan.
6569
*/
6670
export function LinkedScrRefButton({
6771
scrRef,

0 commit comments

Comments
 (0)