fix(FUN-5): replace NSAssert in getNextCharacter with unconditional bounds guard

[kostub]

Summary

• Fixes FUN-5 (issues.md#L223): getNextCharacter protected its array read only with NSAssert, which is compiled out in release builds (NS_BLOCK_ASSERTIONS), leaving an unchecked heap read if any call site ever calls past the end of input.
• Replaces the NSAssert with an unconditional if (![self hasCharacters]) guard that sets MTParseErrorInternalError and returns 0 — consistent with the parser's existing setError: convention.
• All current call sites pre-check hasCharacters before calling getNextCharacter, so this is defense-in-depth with no behavior change on any currently-reachable path.
• Adds a regression test (testGetNextCharacterBoundsGuardDoesNotAffectValidParsing) confirming that valid \frac parsing is unaffected.

Files changed

• iosMath/lib/MTMathListBuilder.m — replace NSAssert with unconditional guard (~+8/-1 LOC)
• iosMathTests/MTMathListBuilderTest.m — add regression test

Test plan

• swift build passes (no warnings)
• swift test — all 293 tests pass, including new regression test
• New guard is identical in Debug and Release builds (unconditional if, not NSAssert)

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request replaces an NSAssert with an unconditional bounds check in getNextCharacter to prevent potential out-of-bounds heap reads in release builds. It also adds a regression test to ensure that this defense-in-depth guard does not affect valid parsing. There are no review comments, so no feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kostub]

EM-REVIEW v1

Verdict: Approve-worthy (non-blocking). No issues found.

Reviewed the diff (gh pr diff 236), surrounding builder code, the enum, setError, and the getNextCharacter call sites. Ran swift test: 293 tests pass, 0 failures.

Correctness of the guard — confirmed

• Safe sentinel: returns 0 (NUL) instead of reading _chars[_currentChar++] out of bounds. 0 is not a meaningful LaTeX token, and the read/advance is skipped entirely.
• Enum case is appropriate: MTParseErrorInternalError is an existing case (MTMathListBuilder.h:87). It correctly signals an internal invariant violation rather than a user-input error — fitting, since the past-end branch is only reachable via a future missed hasCharacters check, not via any input.
• Error propagation is sound: setError records only the first error (if (!_error) at MTMathListBuilder.m:960), so the guard never clobbers a real parse error. The main loop bails at the top of each iteration (if (_error) return nil; at :173-177), and the other getNextCharacter call sites are each preceded by a hasCharacters gate, so the guard is dead code on every reachable path and cannot alter valid-input behavior. Sentinel-0 comparisons (e.g. against a quote or caret) simply fall through and the next _error check catches the condition.

No valid-input behavior changed — confirmed

The replacement is semantically identical to the old NSAssert on all reachable paths (assertions are compiled out in release via NS_BLOCK_ASSERTIONS; in debug the assert would have fired before any return). Hot-path parsing is unaffected.

Test — reasonable

testGetNextCharacterBoundsGuardDoesNotAffectValidParsing is an honest regression check. Its comment correctly states the past-end branch is unreachable via the public API, so it exercises the hot path (a nested fraction) to confirm the guard does not perturb normal parsing. Testing the unreachable branch directly would require white-box reach-arounds with no real-world value.

Minor (non-blocking) notes

• The sentinel 0 relies on every current caller pre-checking hasCharacters; a hypothetical future caller that reads the return without then checking _error could misread 0 as a real char. Purely forward-looking — no current caller does this — and arguably the intended defense-in-depth tradeoff. No change requested.
• Comment accuracy verified: NS_BLOCK_ASSERTIONS does compile out NSAssert, so the stated release-build exposure is real.

Defense-in-depth fix, clean and well-documented. Recommend merge.