fix(check-new-line-breaks): split at a lowercase-opening sentence boundary

[d-morrison]

Closes #389

Problem

check-new-line-breaks.py's sentence splitter required the next sentence to
begin with an uppercase letter or markup ((?=[A-Z"' `*[])`), so a line
opening its second sentence with a bare lowercase word read as one sentence and
shipped unflagged:

it went red. renv restored the lockfile.   →  1 sentence (WRONG)

That is exactly the shape our prose writes most, since it routinely opens a
sentence with a lowercase package or repo name (renv, serodynamics,
dplyr). The failure direction is a false negative — nothing turns red — so a
clean report was weaker evidence than it looked.

Fix

Add a second regex branch, _SENT_BREAK_LOWER_RE, for the lowercase-follower
case, applied after the existing (unchanged) uppercase branch:

_SENT_BREAK_LOWER_RE = re.compile(r"(?<=[a-z][a-z])([.!?][`\"')\]]*)\s+(?=[a-z])")

The two-lowercase-letter lookbehind (?<=[a-z][a-z]) is the guard the
uppercase lookahead gave the other branch for free: it splits only when the
previous sentence ends in a genuine word (≥2 trailing lowercase letters), which
rejects every documented false-positive shape:

input	why it does not split
v2.1, 0.9012	ends in a digit (and no space after the .)
U.S. economy	terminal . follows a single uppercase letter
a.m. sharp	terminal . follows .m, not two letters
wait... foo	terminal . follows ..
Option a. really	a is a one-letter token
It is **critical.** yet	closing class omits */_ (preserves #397's guard)

Tests

Adds 7 tests: 2 positive (the reprex cases, confirmed to fail against
pre-fix code) and 5 negative guard-rail cases. Full suite: 75 passed.

Follow-up

The same regex is duplicated in ai-config's scripts/semantic-line-breaks.py
(the reformatter this check is the detector half of); filed
Morrison-Lab/ai-config#1212 to port this fix there.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[d-morrison]

Note: the red review / antigravity-review check is a disabled-reviewer artifact, not a code issue. Antigravity's model API is out of quota (HTTP 503), so its caller workflow was disabled repo-wide (tracked in #426); the check will not re-run on this PR. The gating review here is review / claude-review.

[d-morrison]

Round 1 — ARD summary (all 4 findings Addressed)

Thorough review, thank you — all four were real. Fixed in 9dd6cb0:

#	Finding	Disposition
1	Closing-char class re-opens the over-split (blocking)	Addressed — dropped the closer class; branch now requires terminal punct immediately followed by whitespace. Your 5 real cases (incl. README.md:190) no longer split.
2	Ellipsis misattributed to lookbehind	Addressed — corrected in comment, CLAUDE.md, changelog; ellipsis/quotes/emphasis now attributed to the immediate-whitespace guard, decimals to \s+, only U.S./a.m./single-letter to the lookbehind. Dropped the CLAUDE.md overclaim.
3	Case-sensitive _ABBREV_RE now reachable (3 sec.)	Addressed — added re.IGNORECASE; dropped No so a lowercase no. (the word) still splits.
4	decimal/version tests don't exercise the lookbehind	Addressed — relabeled to state they pin \s+; added a digit-ending-token test (plan9. really) that mutation-verified exercises the lookbehind.

Every fix was empirically verified against the real module, and each new guard-rail test was mutation-checked to confirm it goes red when its guard is removed. Full suite: 79 passed. All 4 threads resolved.

[d-morrison]

Round 2 — ARD summary

Both round-2 findings Addressed in 4f59d9b:

• Blocking (No. regression): round 1's fix dropped No from the abbreviation list, which — since _ABBREV_RE runs before both branches — un-protected No. on the pre-existing uppercase branch (Item No. Three split). Replaced the blanket re.IGNORECASE with a scoped rule: protect each abbreviation in its conventional and all-lowercase form, not its all-caps form, and keep No case-sensitive. No. (number) stays protected; lowercase no. (word) still splits.
• Non-blocking (IGNORECASE over-widening): resolved by the same change — no blanket IGNORECASE means all-caps SEC. is no longer protected, so filed with the SEC. The case ... splits again.

Both got mutation-verified regression tests (test_number_abbreviation_before_uppercase_does_not_split, test_all_caps_abbreviation_lookalike_still_splits). Also cross-referenced ai-config#1212 from CLAUDE.md's duplicate-regex note (round-2 soft observation). Full suite: 81 passed. Both threads resolved.

On the other soft observation (the "shape our prose writes most" phrasing appearing in three files): left as-is deliberately — CLAUDE.md, the changelog, and the code comment serve different audiences, and the phrase is a qualitative framing rather than the measured 2837→3398 claim it sits near.

[d-morrison]

Round 3 — ARD summary

Both round-3 findings Addressed in 44b2ac1:

• Blocking (lowercase-form leak onto the uppercase branch): this was the third instance of the cross-branch abbreviation class, so I fixed the architecture. Conventional-case abbreviations stay protected on both branches; the lowercase forms now go in a second pass (_ABBREV_LOWER_RE) applied after the uppercase branch runs, scoping them to the lowercase branch only. The disambiguator is the follower's case — 3 sec. then (lowercase, mid-sentence) stays intact; 300 ms. The next ... (uppercase, genuine boundary) splits. Also added min/hr/hrs to cut the 5 min. and ... false positive (your secondary observation).
• Stale docstring: rewritten to describe the two-pass mechanism, consistent with the all-caps test below it.

New regression tests (test_lowercase_abbreviation_before_uppercase_does_split, test_added_time_unit_before_lowercase_does_not_split), both mutation-verified. Full suite: 83 passed. Both threads resolved.

The min/Jan/Inc completeness point is now a disclosed limitation in the code comment and CLAUDE.md: the lowercase list is curated, not exhaustive, so an unlisted lowercase abbreviation before a lowercase word can still false-split on this warn-only check.

[d-morrison]

Round 4 — ARD summary

Round 4 found no functional regressions (the two-pass mechanism is confirmed sound). All three findings were comment/documentation-accuracy issues in the self-referential "map future widenings are read against" block. Addressed in 017b42d — this time each attribution is mutation-verified, not reasoned:

• Ellipsis attribution: the wait... foo exclusion is the lookbehind's doing (at the third dot, whose two preceding chars are both dots), not the immediate-whitespace guard. My round-2 fix had overcorrected this. Fixed in code comment, CLAUDE.md, changelog, and the test docstring.
• Decimal/version attribution: an internal dot (0.9012, v2.1 mid-token) is blocked by \s+ (no following space); a clause-ending version (v2.1.) is blocked by the lookbehind. Added test_version_at_clause_end_does_not_split (mutation-verified).
• Disclosed-limitation wording: broadened to "any unlisted abbreviation ending in two lowercase letters regardless of case (Inc., Prof., Mon.)", since the lookbehind checks trailing chars only.

No code or test behavior changed. Full suite: 84 passed. All 3 threads resolved.

To avoid a further round on this exact class: every guard attribution in the comment, CLAUDE.md, and test docstrings is now backed by a mutation check (remove the guard, confirm the case starts splitting), rather than by plausible-sounding reasoning.

[d-morrison]

Round 5 — ARD summary

Round 5 confirmed the regex/abbreviation logic is fully sound (all five prior regressions fixed, no ReDoS, no cross-branch leak, corpus scan clean). One doc-accuracy error remained: I had inverted #397's history at three sites — claiming it "dropped */_ to close an over-split" when #397 actually added */_ to fix a **bold.** under-split (false negative), per the issue itself and this file's own unchanged CLAUDE.md:714.

Corrected in 1149789 at all three sites (code comment, CLAUDE.md, test docstring) to the accurate and more useful framing: the uppercase branch safely carries closers because its uppercase-follower lookahead refuses a mid-construct lowercase continuation; the lowercase branch's follower is lowercase, so a closer there would over-split — which is the real reason it omits the closing class. Grepped all #397 references across the changed files for consistency. No code change; 84 passed. All 3 threads resolved.

[d-morrison]

Round 6 — ARD summary

Round 6 verdict was Ready for merge; this clears the two non-blocking items so the PR is fully clean. Addressed in 3ac7383:

• Inline (ms. example): ms enters the lowercase list incidentally (as Ms.lower()), not as a curated addition. Reworded the changelog to use the deliberate min. as the example, and added test_incidental_lowercase_abbrev_before_lowercase_does_not_split to cover the previously-untested lowercase-follower direction.
• DRY suggestion: extracted _abbrev_pattern() shared by _ABBREV_RE and _ABBREV_LOWER_RE. Both compiled patterns verified byte-identical to before, so behavior (and the confirmed-untouched _ABBREV_RE) is unchanged.

85 passed, thread resolved. No functional change; the regex/abbreviation logic verified sound in round 5 is untouched.

[github-actions]

Claude finished review — View run

---

Code review — #425

This is a confirming review for round 7. The most recent prior review (round 6, comment, run) concluded Ready for merge with one non-blocking finding (the ms. abbreviation's undocumented/untested incidental-inclusion) and a minor DRY suggestion (_ABBREV_LOWER_RE duplicating _ABBREV_RE's pattern-building). After that review posted, the author pushed one further commit (3ac73837, "Address review round 6 (non-blocking): DRY helper, ms. coverage, changelog wording") that hadn't been reviewed yet, so I reviewed that commit's diff and re-verified the cumulative state of the file rather than just re-stating the prior verdict.

What I checked, all empirically against the actual code on disk (which reflects the PR's merged head) rather than by inspection alone:

• All seven previously-flagged regressions/findings from rounds 1–6 (closer-class over-split, ellipsis-attribution factual error, No. uppercase-branch regression, IGNORECASE scoping leak, lowercase-abbreviation cross-branch leak, the two non-guard-exercising negative tests, and the round-6 ms./DRY items) are fixed in the current code. I re-ran the exact repro strings from each finding directly against check-new-line-breaks.py via importlib (pytest execution itself was denied in this sandbox) and all produced the expected, non-regressed output — see the transcript above for the full case table (19 cases, all matching expected).
• Mutation-verified the two "does this test actually exercise the lookbehind" claims in the newly reworded test docstrings: removing (?<=[a-z][a-z]) does flip test_digit_ending_token_before_lowercase_does_not_split and test_version_at_clause_end_does_not_split (genuine guard tests), and does not flip test_decimal_between_digits_does_not_split/test_version_between_digits_does_not_split (they pin the pre-existing \s+ requirement instead) — exactly as the new docstrings state.
• Adversarially probed beyond the PR's own test table (prefix-colliding abbreviations hr/hrs, mr/mrs, plus vol., dept., e.g., ca., ibid. before lowercase followers) — no new false splits or false negatives found.
• Fact-checked the external references this round's diff adds/keeps: Morrison-Lab/ai-config#1212 (open, correctly described as the mirror-fix tracking issue), gha#397 (closed, and its title confirms the "bold-close blind spot" / false-negative framing the corrected CLAUDE.md text now uses — round 5's fix to that attribution is accurate), and gha#389 (open, correctly described as the bug this PR fixes).
• Changelog fragment (changelog.d/nlb-lowercase-sentence-boundary.fixed.md) is correctly named/categorized per changelog.d/README.md's <slug>.<category>.md convention, and its prose no longer presents ms. as a deliberate curated example (round 6's fix), using min. instead.
• Scanned the new/changed prose (CLAUDE.md, the changelog fragment, code comments) for AI-authorship tells, unresolved forward references, and redundant content — none found; the three-location duplication (code comment / CLAUDE.md / changelog) is the repo's own established, required convention for this kind of change, not incidental redundancy.

One process note, not a PR defect: this session's working-tree CLAUDE.md differs from what's actually committed at the PR's head (the working tree holds the pre-PR text) — that's the review harness deliberately sourcing my own operating instructions from the base branch rather than the PR's proposed content, not a bug in the PR. I verified the PR's actual proposed CLAUDE.md text via gh pr diff instead.

No new issues found, and the prior round's one non-blocking item and DRY suggestion are both resolved in this commit.

Verdict

Ready for merge

[github-actions]

💰 Cost: $3.6373 (review) — run