fix VerseRef.chapterNum setter infinite recursion

[irahopkinson]

Fixes the VerseRef.chapterNum setter, which recursed infinitely on every assignment. Built test-first: the ported C# test was confirmed failing with RangeError before the fix went in.

The bug

src/verse-ref.ts assigned the property to itself instead of the backing field, behind a ToDo placeholder:

set chapterNum(value: number) {
  // ToDo: replace or remove this placeholder
  this.chapterNum = value;
}

There's no guard in front of it, so every input threw RangeError: Maximum call stack size exceeded — = 5, = 0, = -1 alike. The property was completely unusable.

Nothing inside the library was affected: the numeric constructor writes _chapterNum directly (verse-ref.ts:244), and setEmpty() and the chapter string setter do the same. That line was the only write through the setter, so this was reachable only by external consumers.

The fix

Ported from the C# VerseRef.ChapterNum, which guards negatives then assigns:

public int ChapterNum
{
    get { return chapterNum; }
    set
    {
        if (value < 0)
            throw new VerseRefException("ChapterNum can not be negative");
        chapterNum = (short)value;
    }
}

giving:

set chapterNum(value: number) {
  if (value < 0) {
    throw new VerseRefException('ChapterNum can not be negative');
  }
  this._chapterNum = value;
}

This mirrors the existing bookNum setter, which already ports its C# counterpart's throw verbatim. The (short) cast is deliberately not replicated — TS has no short, and emulating 16-bit wraparound would be fidelity at the cost of sense.

The -1 sentinel path is untouched. setEmpty() and set chapter('') write _chapterNum directly, so the new guard never intercepts them — exactly as in the C#, where Chapter sets the field to -1 without going through ChapterNum. Chapter and Verse as Empty Strings still passes.

Tests

Ports BuildVerseRefByProps from SIL.Scripture.Tests, which builds a VerseRef entirely through property setters and so covers this path directly.

One assertion is commented out. After bookNum = 13, the C# expects OutOfRange because chapter 0 is invalid, but internalValid() has its chapter/verse range check commented out pending the versification port (see the TODO at verse-ref.ts:623), so it returns Valid. Commented rather than weakened, following the existing convention in this file, and it should be restored when that port lands. The other five validStatus/valid assertions in the ported test pass legitimately.

Also adds two TS-only tests under Extra (TS-only tests) for the negative guard and the zero boundary, since the C# has no direct test for the setter's validation.

Is this a behaviour change?

Technically yes, but not a breaking one. Since every prior assignment crashed, there is no code path that worked before and behaves differently now — this is a patch-level repair of a property that was 100% unusable. The public API surface is unchanged; the .d.ts declarations for chapterNum are identical.

The only way to have depended on the old behaviour was catching the stack overflow as control flow, and even that largely survives: negatives still throw, now as VerseRefException rather than RangeError.

Verification

• npm run test:ci — 43 passed (was 40)
• npm run lint — clean
• npm run prettier:ci — clean
• npm run build — clean

Red was confirmed twice before the fix: first RangeError from the recursion for the ported test, then expected function to throw an error, but it didn't for the negative guard.

Follow-up

set verseNum is the same class of mis-port and still carries its ToDo. The C# is:

if (value < 0) throw new VerseRefException("VerseNum can not be negative");
verseNum = (short)value;
verse = null;

The TS does a bare this._verseNum = value — missing both the guard and the verse = null. So new VerseRef('LUK','3','4b-5a') then verseNum = 9 leaves a stale '4b-5a' in the verse getter. Deliberately left out of this PR: unlike chapterNum, that setter works today, so fixing it changes the observable output of working consumer code and deserves its own review.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.93%. Comparing base (7c885cf) to head (bf456ee).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
+ Coverage   81.40%   83.93%   +2.53%     
==========================================
  Files           4        4              
  Lines         328      330       +2     
  Branches       76       77       +1     
==========================================
+ Hits          267      277      +10     
+ Misses         39       33       -6     
+ Partials       22       20       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.