fix: segment embedded and parenthesized lists - #156
mldangelo-oai merged 36 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (8)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 396c7af4da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06a85a2224
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1394f70b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: f1394f70b6
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52a982e8ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 52a982e8ce
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08d10748f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f247d65499
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: def68349b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: def68349b2
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfb4914e52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🔵 Needs a closer look
Five moderate segmentation issues remain unresolved in src/utils.ts.
Review details
Suppressed comments (5)
src/utils.ts:1108
findListCandidatecallsnextListMarkerwithout apreviousmarker while it discovers candidates, so theauthorInitialguard innextListMarkeris never applied during this phase. A dotted initial in a body is therefore selected as a list boundary:A. J. Smith will attend B. K. Brown will attendcan be split atJ.instead of retainingA. J. Smith...as the first item. Carry the relevant prior-marker/name context into candidate discovery (and the subsequent scan) before accepting same-family initial markers.
let current = nextListMarker(input, expression, state, familyFilter);
src/utils.ts:745
- This angle pre-scan matches an opener with every later
>without consulting the other delimiter types. For a crossing span such as<[literal >] Options: a) Alpha b) Beta>, the first>is incorrectly recorded as the angle closer; the later list is then scanned outside the literal even though the final>is the matching closer. Use the same current-top-delimiter rule astrackListBracketswhen confirming angle openers.
} else if (input[index] === '>' && depth > 0) {
matched ??= new Uint8Array(input.length);
matched[pending[--depth]] = 1;
}
src/utils.ts:987
- The reference-gap boundary scan treats punctuation inside protected literals as a real boundary because it examines the raw substring without quote, code-span, or bracket state. For example,
See sections 1) "Intro.", 2) Scope, and 3) Details.clearsreferenceThroughat the quoted period, allowing2)/3)to be misclassified as an embedded list. Scan this gap with the same literal/delimiter state used by marker detection so punctuation inside those spans is ignored.
for (const terminal of input.slice(start, end).matchAll(/[.!?:;\r\n]/g)) {
const index = start + terminal.index;
if (terminal[0] !== '.') {
return true;
}
src/utils.ts:959
- When a year-shaped list has already been accepted,
state.yearListdisablesyearInProsefor every later year, so this word-list is the remaining guard. It omits the commonforcue:2020. Alpha 2021. Beta. Results for 2022. More analysis 2023. Gamma.therefore treats2022.as another list marker and movesResults forout of the prose sentence. Includeforin the prose-count guard (and cover this continuation) so unrelated year prose remains protected.
const countInProse =
/^\d+\.$/.test(marker[0].trim()) &&
/\b(?:am|is|are|was|were|be|been|being|has|have|had|reached|numbered|total(?:ed)?|hit|equals?|equaled|became|remained|scored|costs?|in|of|at|by|to|from|about|around|roughly|approximately)$/i.test(
input.slice(Math.max(0, marker.index - 24), marker.index).trimEnd(),
);
src/utils.ts:917
- The new Markdown bullet syntax is not treated as a boundary for an ambiguous dotted numeric marker. Because
marker.indexpoints to the whitespace consumed before the bullet,Heading -\t1. Alpha -\t2. Beta.sees the precedingghere and refuses to seed the1.family; the same parenthesized form works only because1)is non-ambiguous. Treat an accepted bullet prefix inmarker[0]as a valid boundary so dotted bullet lists follow the documented marker rules too.
return {
empty: index < 0,
boundary: lineBreak || /[.!?:\p{Pd}]/u.test(input[index] ?? ''),
joinedNameInitial,
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Latest review findings reconciled at
All 998 tests, source/test types, strict Biome, build and packed consumers pass. Independent final source audit found no blocker. Exact-source1k/2k/4k/8k protected-reference runs retain one sentence and measure approximately5/8/14/28ms. Fresh standalone Codex and Copilot reviews are required on this new head. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate src/utils.ts findings remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
src/utils.ts:850
- The active quote scan closes at every matching quote token without checking backslash parity. An odd backslash run in an assignment value therefore closes the quote early and exposes the embedded
a)/b)markers, contrary to the added escaped-quote cases. Require!isEscapedListQuote(input, index)before consuming this closer.
if (input.startsWith(state.quote, index) && !apostrophe) {
src/utils.ts:1136
- In neutral mode, lower-equivalent alphabetical labels share the same
identity, soIntro: A. Alpha a. Beta.(and its lowercasedß/ßform) hasdistinctMarker === falseand is never selected as a list when it has a nonempty prefix. Allow repeated cased labels in neutral mode while retaining the existing repeated-numeric guard, otherwise the added case-folding invariance cases fail.
if (first?.hasBody && (first.emptyPrefix || distinctMarker)) {
src/utils.ts:770
- The parenthesis pre-scan also closes its quote state at an escaped matching quote. This makes bracket ownership depend on the backslash parity and can expose parenthesized label candidates inside the still-open literal. Apply the same escaped-closer check used by the main list scan.
if (input.startsWith(quote, index) && !isLiteralListQuote(input, index, quoteFlags)) {
src/utils.ts:1123
markProseInitialidentifies the firstA./B.in prose such asI met John A. B. Smith. Intro: C. Alpha D. Beta., but this result is only saved inproseInitialEnd; the current marker is still allowed to become the candidate returned bynextListMarker, and a laterB.can seed the uppercase family. That makes the embedded-list path split the author sentence instead of leaving it intact (the new regression attest/tests.ts:722exercises this case). Skip the marker whenmarkProseInitialreturns an end offset before continuing the scan.
proseInitialEnd = markProseInitial(
input,
current,
first === undefined,
context,
proseInitialEnd,
);
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
|
Copilot review PRR_kwDOMlO9es8AAAABNj9u9Q critically reconciled on unchanged
All998 tests, types, strict Biome and packed consumers remain valid on this unchanged head. Frozen main/earlier/current source comparisons and 30 exact inputs are preserved in the workspace. Standalone Codex code and security reviews completed clean on this head. No duplicate unchanged-head review is requested; the cancelled aggregate can now be retried because its log shows it was waiting only for the now-completed Copilot review. |
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Embedded numeric, alphabetical, and parenthesized lists now retain their selected outer marker family, including lists after quoted introductions. Ordinary sentences in item bodies still split; periods belonging to confirmed nested marker families remain inside those bodies.
The list scan preserves straight, curly, Treebank and guillemet quoted context, assignment-style quoted values with backslash-parity escapes, bracketed context, punctuation-led bodies, and joined name initials. Punctuation-led possessive modifiers, paired 'n'/’n’ elisions and inner numeric elisions retain their enclosing quote. Paired-n classification uses only bounded three-character context shared by quote consumers. Author-name matching is anchored at the current initial and preserves contiguous name groups with apostrophes, dashes and combining marks in surnames. Fixed marker-family state invalidates stale deferred candidates while retaining other families. Full-source flags confirm numeric quotations and ambiguous possessives only with closing evidence, so quoted apparent list markers remain protected while later actual lists are recognized. An otherwise-unmatched final structural parenthesis can confirm that earlier label-shaped closers belong to enclosing prose; a new opening parenthesis cancels that provisional interpretation.
Matched unquoted angle spans retain both spaced and unspaced literal forms. Unmatched comparisons do not suppress later lists, quotes immediately after
<retain their literal contents, and a compact typed stack bounds angle-position storage. Bullet-prefixed numeric markers use the same value and outlier rules as plain numeric markers. A year-shaped numeric family accepted at an empty or valid boundary prefix retains its later labels while unrelated prose-year suppression remains in place. Neutral marker identity uses exactly JavaScripttoLowerCase(): dotless-i and final-sigma labels retain their distinct lowercase values, while lower-equivalent labels still match. U+0085 normalization remains consistent.ROUGE-L's existing 100,000 sentence-pair work limit is documented in the README and API comments, including applicability with a custom tokenizer and the empty-token early return. Behavioral tests verify the exact 100,000 boundary, 100,001 rejection and explicit callback bypasses.
Plural reference forms remain prose, numeric Markdown prefixes stay attached to their items, and a shared compact bracket stack keeps parenthesis-label candidates inside their literal container. Neutral author windows preserve the documented lowercase-input invariant after expanding mappings.
Paired single-backtick code spans are opaque across every list scan. A confirmed closing backtick also exposes an existing terminal inside the code phrase to the list-prefix boundary check; nonterminal/unmatched code cues retain their previous meaning. Numeric bullet prefixes stay on the same CR/LF-delimited line as their marker across matching, family and value parsing. Reference cues carry across a prose run, and selected alphabetical-family continuation uses the existing bounded author guard. Numeric outlier decisions compare normalized decimal text exactly, including labels beyond JavaScript’s safe-integer and finite-number ranges.
The shared quote-opening context accepts a local > cue, including quoted literals after tag closers or comparison symbols, without changing angle delimiter matching. Reference-boundary punctuation is recorded by the existing source cursor only outside shared quote/code/bracket state; one latest-boundary offset replaces raw gap scanning.
Validation and complexity
Integration at
1cf6249753fb2d6f4b76c9c739a2da61d01a0ad5preserves the original list fixes together with the landed Unicode identifier, finite-mean and NEL-whitespace changes. 1,072 tests pass locally with global coverage, source/test type checks, strict Biome, Markdown formatting, build, and packed CommonJS/ESM/type/production consumers. Local checks used the existing Biome 2.5.10 and Node types 24.13.3; GitHub CI must validate the locked 2.5.13/24.13.4 versions on this head before merge. Regressions cover selected outer families, nested dotted markers, structural-parenthesis competitors, guillemet quotations and introductions, possessives versus quoted words, punctuation-led modifiers, nested numeric elisions, paired-n/quoted-letter controls, later numeric-quote recovery, anchored punctuated author groups, year-shaped starting families, exact work-limit/callback boundaries, escaped quote parity, numeric quotes before angle closers, bullet-prefixed numeric outliers, 200,000-character backslash runs, Unicode, a seven-million-angle input under a 64 MB heap, 20,000 enclosed labels, and 20,000 paired-n elisions.Quote and marker iterators advance monotonically. Backslash runs preceding distinct quote tokens are disjoint and are inspected only at quote positions. Confirmed ranges do not overlap within a family. Nested-marker protection reuses the existing candidate rules across at most six configured families. Deferred-prefix text is cached once per first marker, fixing the measured repeated-prefix quadratic behavior. An independent agent reviewed the final source diff; tracked-file hashes match validation and post-hook publication. Prior validated-source1k/2k/4k/8k scaling is5/8/14/27ms for code spans,5/9/17/36ms for reference runs, and1.5/1.5/3/6ms for numeric-label lengths. The decimal recurrence keeps its intermediate difference bounded and matched24,048 exact-integer oracle decisions. Prior validated-source abbreviation-reference1k/2k/4k/8k runs retain one sentence and measure19/30/56/110ms. Previously recorded protected-reference 1k/2k/4k/8k runs measured 5/8/14/28ms; these timings were not rerun for the integration. The existing cursor visits each code unit once and every eligible terminal uses a bounded suffix.
Heuristic limits
2020. Alpha 2021. Beta. Results for 2022. More analysis 2023. Gamma.therefore keeps the same ambiguous year-marker interpretation as main. An item body ending in for does not encode whether the next year is prose or another label; no general semantic count/year classifier is promised.1) One 2) Two. Options: a) Alpha b) Beta.) can be a separate list or a nested continuation of item2 (1) Pick 2) Configure. Options: a) Fast b) Safe.). It does not by itself resume independent list discovery. There is no indentation/topic grammar resolving that ambiguity.Authors: A. Smith. Options: B. First C. Last.follows the same A/B/C marker precedence asChoices: A. Alpha. Options: B. First C. Last.. Existing joined-initial evidence does not identify the isolated surname; no lexical author/role dictionary or colon reset is inferred. This is an introduced tradeoff relative to ordinary main sentence splitting..!?:;or a line break), excluding existing abbreviation-exception/acronym periods and decimal interiors. Ambiguous title-versus-sentence punctuation retains that bounded structural precedence; there is no full cross-reference grammar.'twas'can therefore confirm an earlier unpaired'99and suppress intervening apparent list markers. Resetting that candidate instead breaks supported numeric quotations with inner elisions, even when those quotations contain sentence periods. The existing inner-elision precedence is retained.toLowerCase(), as documented in the README; it does not promise full Unicode folding (ßversusSS) or invariance under lossy uppercase expansion. MixedA./b.markers therefore need case-neutral mode to share a family.Results: 1. The answer measured 5. More analysis followed 2. Final.has the same marker syntax asResults: 1. Alpha 5. Beta 2. Final.No measured-verb dictionary distinguishes them. Accepted year-shaped list families have the same ambiguity for later bare numeric prose; explicit existing count cues such asin 2022remain protected.He said '99 options a) Alpha b) Betas' aloud.falls back to list parsing, while punctuation/EOF closers remain eligible.(section a)closures retain their normal meaning without that extra evidence.