check-new-line-breaks: sentence splitter misses a sentence starting with a lowercase identifier

[d-morrison]

Summary

check-new-line-breaks.py's sentence splitter does not split at a sentence boundary when the following sentence opens with a bare lowercase word. Since our prose routinely starts a sentence with a package or repo name (serodynamics, renv, dplyr, pkgdown), the check is silent on exactly the multi-sentence lines we are most likely to write.

The failure direction is a false negative, so nothing turns red and the line ships.

Reprex

import importlib.util
spec = importlib.util.spec_from_file_location("nlb", "check-new-line-breaks/check-new-line-breaks.py")
m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m)

m.split_sentences("agent disabled in both. serodynamics needed the /review path built.")
#=> 1 sentence   <- WRONG, should be 2
m.split_sentences("agent disabled in both. Serodynamics needed the /review path built.")
#=> 2 sentences  <- correct

m.split_sentences("it went red. renv restored the lockfile.")
#=> 1 sentence   <- WRONG
m.split_sentences("it went red. The lockfile was restored.")
#=> 2 sentences  <- correct

Only the capitalization of the second sentence's first word differs, and the count flips.

Cause

_SENT_BREAK_RE (line 61):

_SENT_BREAK_RE = re.compile(r"([.!?][`\"')\]]*)\s+(?=[A-Z\"'`*\[])")

The lookahead (?=[A-Z\"'*[])` requires the next sentence to begin with an uppercase letter, a quote, a backtick, an asterisk, or a bracket. A bare lowercase identifier matches none of these, so no boundary is found.

Note that a backticked identifier already works, since ` is in the class. It is specifically the unbackticked lowercase word that escapes.

Why the lookahead exists

Presumably to avoid splitting on a period that is not a sentence end. That is a real concern, and the reason to be careful here rather than just adding a-z to the class: a decimal (0.9012), a version (v2.1), or a file extension (.github) would start producing false positives, and false positives on an advisory check train people to ignore it.

Worth noting the script already has a dedicated guard for one part of that problem: the ABBREVIATIONS set at line 51, whose whole job is to suppress splits after e.g., i.e. and friends. So the design already accepts that the lookahead is not the only line of defence.

Suggested direction (not a patch)

I have not written a fix, since the tradeoff belongs to whoever owns this check. Two options that seem worth weighing:

1. Widen the lookahead to lowercase, and lean on the preceding context instead. Require the boundary's preceding token not to be a digit, a single letter, or a known abbreviation. Catches the corpus's real habit; risks new false positives on version numbers and decimals mid-line.
2. Widen it only for a plausible-identifier shape — e.g. a lowercase word containing a hyphen, a dot, or a following backtick, or one appearing in a repo/package position. Narrower, so fewer false positives, but it will still miss a plain lowercase word like "it".

Either way the regression cases are the four in the reprex above, plus a decimal and a version string that must not split.

Impact

Found while addressing a review on Morrison-Lab/ai-config#964. The reviewer caught the line by reading; the check reported clean. That round also found 12 genuine multi-sentence lines the check did catch, so this is a gap in an otherwise working instrument rather than a reason to distrust it.

Since the check is advisory (warns, exits 0), this does not break any build. The cost is that a clean report is weaker evidence than it looks.