fix: recognize terminal three-dot ellipses - #159
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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: 20645fc6c6
ℹ️ 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: 1982350188
ℹ️ 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: 30a994e469
ℹ️ 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: 6c57c8f514
ℹ️ 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/question-annotation-boundaries-20260915
|
@codex review Please review code and security on exact head b782e55. The integrated implementation passes all 2,642 tests with 98.17% branch coverage, source/test types, strict installed lint, formatting, build and packed consumers. The final option-documentation change has complete AST equality and exact comment-reversal proof; the main-ancestry merge preserves all 36 tracked hashes. Exact declared-toolchain CI is running separately. This composes terminal ellipses with the landed #157 quotation/list behavior, #158 citation rules, #166 private email caches, #169 Unicode handling and #180 annotation/question correction. Ordinary, citation and ellipsis grammars remain distinct. Question lookahead and final merging share the real terminal-ellipsis decision, with private incremental state and retained outer quotation ownership. Accepted boundaries release that ownership. Constant-true private adapter modes and their unreachable branches were removed with independent call-graph and constant-fold AST proof. All existing regression inputs and mode combinations are retained. Six original groups have independently adjudicated ordinary-boundary expectation changes; one inherited neutral auxiliary/ellipsis row now follows the real ellipsis boundary and existing quotation-attribution rule. Original nested-elision expectations were preserved by fixing source ownership. The supported contiguous/spaced retained-ellipsis forms, accepted four-dot controls, leading decimals, citation cap and documented language ambiguities remain explicit in the PR description. Please critically inspect the final integration and all prior findings, including suppressed claims. |
🛡️ 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: b782e55406
ℹ️ 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
Two moderate issues in src/utils.ts must be addressed.
Review details
Suppressed comments (2)
src/utils.ts:3343
- This suffix check classifies the last four characters of any longer contiguous period run as a terminal ellipsis. For example, at the final dot of
Alpha.....5 was enough.this setsterminalEllipsisand sends the run through the new ellipsis path, splitting an input that the unchanged period-run policy (and the regression attest/tests.ts:7111-7117) requires to remain one sentence. Detect a complete three/four-dot run with non-dot neighbors, and share that predicate with the merger below.
const spacedEllipsis = protectedPeriods[ellipsisCursor.index]?.boundary === index;
const terminalEllipsis =
spacedEllipsis || /\.{3,4}$/.test(input.slice(Math.max(0, index - 3), index + 1));
src/utils.ts:997
- Because this search is limited to the current
textfragment, an explicit''Treebank opener whose closer follows a provisional ellipsis is tracked as a single quote rather than a Treebank double quote. A nested single quote can then clear that state, so a later ellipsis in the still-open outer span (for example''Outer... 'Inner...' tail... Next'') is treated as outside the quote and may split. Check the full normalized source with an absolute offset, or carry the Treebank state across fragments.
if ((flags & 2) === 0 && ellipsisLegacyQuotationState(text, index, false)) {
state.double = true;
state.doubleInsideSingle = state.single;
return true;
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
@codex review Please review code and security on exact head 52adf66, including the complete integrated implementation and the last repair commit. All2,679 tests,98.23% branch coverage, source/test types, strict installed Biome, Markdown, build and packed consumers pass on its exact tree. All964 prior test registrations remain byte-for-byte and37 expanded regressions were added. Exact declared-toolchain CI is running separately; no blocked local installation was retried. The repair consumes pending single closers before three-dot boundaries and preserves a following ASCII opener, adds existing single-guillemet family support to ellipsis state, shares the existing matched-angle metadata so unmatched compact comparisons cannot create false brackets, and classifies Treebank aliases using their complete normalized-source offsets. One inherited compact-angle feature gap is not claimed as a parent regression. Quotation-index first-pass extraction preserves its exact body and allocation; no new dependency or threshold change. Prior actual Codex findings are addressed individually. Copilot's full-source alias issue is fixed; the suppressed five-dot/digit claim conflicts with the unchanged exact Alpha.....5 regression and the final digit-rejecting boundary gate. Continuation competitors, malformed alias recovery, real paired angles, numeric aside exclusions and quoted-angle competitors are covered. Three-dot repairs preserve the existing four-dot limitations, including misplaced closers in some spaced-single cases. Please critically inspect all findings, including suppressed claims, and report any reproducible introduced defect. |
|
Critical reconciliation of Copilot review 5210970358 on b782e55, including both suppressed findings:
The same additive commit addresses the four actual Codex findings, with separate thread replies. The preserved three-dot/four-dot scope, same-family quote ambiguity, numeric citation grammar and older four-dot misplaced-closer limits remain explicit. All 964 prior registrations are byte-for-byte unchanged; 37 expanded regression cases were added. Exact final-head CI and new Codex/security/Copilot reviews remain separate gates. |
🛡️ 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. |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate numeric boundary issues remain for four-dot and longer period runs.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/utils.ts:1595
- The numeric branch here also fires after four- and longer contiguous dot runs. For the added period-run regression
Alpha....5 was enough.(andAlpha.....5...),isUnspacedEllipsisStarttherefore accepts a boundary before the number, producing a split even though the test requires the existing ambiguous contiguous-run behavior to remain one sentence. Numeric unspaced starts should be enabled only for an actual three-dot run, not the four-dot/longer path.
src/utils.ts:4933
- When a four-dot spaced run is immediately followed by a digit, this still sets
boundaryto the final dot becauseperiods >= 4andsentenceStartis numeric. For example,Alpha . . . .5 was enough.is split at the dot, contradicting the regression attest/tests.ts:7111-7117and the preserved shorter-run policy. Exclude the direct-digit four-dot ambiguity here while retaining whitespace-separated numeric starts and the >=5-dot decimal handling.
const sentenceStart =
numericSentenceStartReg.test(input.slice(next, next + 6)) ||
(caseNeutral
? isCasedCharacter(following)
: following.length > 0 && charIsUpperCase(following));
let boundary = -1;
if (periods >= 4 && sentenceStart) {
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Critical reconciliation of Copilot review 5211450915 on Both claimed splits are contradicted by the complete control flow and the exact existing regressions, each of which asserts one sentence in both modes:
No source change was made for these claims. The unchanged exact fixtures passed in the 2,679-test local run and all four Node 18/20/22/24 lanes of the completed final-head CI run. An independent reviewer confirmed the same gate order and exact test/hash binding without repeating runtime tests. Existing whitespace-separated numeric starts, three-dot opening-delimiter behavior, and five-or-more-spaced-dot decimal handling remain unchanged. Codex code/security reports are clean on this head, and all ten actual checks pass. Bot COMMENTED reviews are not formal approvals. |
Recognize terminal three-dot ellipses before independent sentences, including signed/currency-prefixed numbers, leading decimals and completed quotations or standalone brackets. Preserve hesitation, omission markers, existing complete-quantity rules and continuation phrases.
Behavior
Contiguous and spaced ellipses share supported opening-token and numeric-start recognition. Adjacent opening quotations keep their delimiter with the following sentence; pending outer closers retain priority for the three-dot suppression rule. Two-dot and ordinary-period opening rules remain unchanged.
The scanner and sentence buffer share ASCII single/Treebank quotation state, confirmed apostrophe flags and bracket protection inside quotations. Three-dot lookahead checks the actual remaining ASCII state after inner closers. Relative nesting, triple closers, possessives, bounded leading elisions and English/German closer priority are preserved.
Inline asides use their complete matching span across provisional abbreviation chunks and line wraps. The scanner and merger share one lazy endpoint index. Closed quoted three-dot ellipses use the existing aside continuation rule; independent quoted replies and unmatched brackets remain separate. Complete connector runs are traversed as Unicode code points before a bounded continuation check, preserving astral dashes.
The endpoint index first matches confirmed quotations, then brackets with a LIFO stack while skipping paired quoted spans. Only a matching top-of-stack closer records a bracket endpoint. Mismatched closers retain the existing literal/recovery policy; an inner bracket can finish while an earlier outer remains unmatched. Angle asides require a Unicode letter immediately after
<; mathematical spans retain ordinary terminal behavior. Compact source offsets keep matches aligned with normalized input.Accepted ellipsis boundaries trim leading horizontal separators in the source scanner and chunk merger. Closed-delimiter and spaced-four-dot boundaries preserve the following quotation without carrying its separator prefix; internal and continuation whitespace remain intact.
Neutral three-dot continuations reuse confirmed apostrophe flags for the existing elided because/until forms in both scanning phases. Adjacent curly forms use the same bounded rule; pending right closers and confirmed paired quotations retain priority. Existing ASCII adjacent-elision behavior and four-dot priorities remain.
The closing-delimiter scan counts three-dot ellipsis bracket closers only outside pending quotation spans. Quoted literal brackets cannot satisfy the outer bracket depth. This reuses the existing quote state and removes a second lexical scan; ordinary punctuation, four-dot lexical counting and scalar bracket recovery keep their existing rules.
A pending straight-single closer is consumed before a three-dot boundary, including whitespace before the closer and a following independent ASCII-double quotation. Adjacent bare cased starts pass the existing continuation gates; doubled-apostrophe and spaced-closer/bare-letter ambiguity remain. For Treebank inline asides, an actual triple closer closes a later inner single quotation before its outer pair; malformed two-mark recovery retains its previous behavior.
Scope and limits
These are sentence heuristics, not a full quotation or natural-language parser. Terminal ellipsis starts retain Unicode Cased or numeric evidence; uncased-script examples retain baseline behavior. Neutral mode preserves lowercase invariance and does not promise identical boundaries to ordinary mode. Terminal ellipses take precedence for ambiguous suspense complements such as
include... Alpha and Beta.outside the existing copular, first-person and complete-quantity exceptions. Same-family English/German marks retain documented pending-closer priority. The aside index protects confirmed quote pairs; an unmatched quote alone does not invalidate a paired bracket aside. Neutral bracket-aside handling cannot use a following name's capitalization. Ordinary smart-quote period boundaries use the separately landed #157 rules; citation boundaries use #158’s supported grammar and inference limit.Dot-run priorities are explicit. New closed-quotation aside suppression applies only to contiguous three dots; four-dot terminal precedence and the existing quoted spaced-four early boundary remain. Bare four-dot inline-aside continuations retain baseline behavior; this is distinct from the new closed-quotation three-dot aside rule. A final spaced dot directly before a digit is treated as a decimal point only when at least four preceding spaced dots remain. Shorter ambiguous runs and contiguous period-run behavior remain unchanged. Four-dot rules can split within open quoted spans, with delimiter-family ambiguity. Leading decimals follow the same quantity rules as their leading-zero equivalents, including existing spaced-four-dot precedence.
The endpoint index and quote metadata use linear auxiliary storage. The index is created only after the other terminal, enclosure and continuation gates pass. Scaling and constrained-heap probes cover large inputs; a V8 heap limit is not a process RSS limit. No runtime dependency, public API, configuration or approval-control changes are included.
ASCII and Treebank double markers share one quotation family and do not track arbitrary nesting within that family. The mixed single/double ordering fixes do not change that limit. Completed quoted ellipses use structural sentence-start rules; speech-attribution verbs and names are not inferred. Named reporting clauses can therefore be separated, and neutral mode also separates cased pronoun attributions regardless of capitalization, following the existing ASCII-quote policy.
Integration and simplification
This head composes the ellipsis behavior with the landed quotation/list, Unicode, citation, email-cache and annotation fixes. Ordinary, citation and ellipsis scanners keep their own supported delimiter rules while sharing immutable source metadata. All existing test inputs and mode combinations remain. Six original authored test groups have independently adjudicated ordinary-boundary expectation updates; one inherited neutral auxiliary/ellipsis row follows both the real ellipsis boundary and the current quotation-attribution rule.
Question lookahead and the final merger use the same three-dot boundary decision, so an actual earlier ellipsis stays authoritative. Private incremental question state avoids copying nested stacks between lookaheads. An actually retained ellipsis also preserves its outer typographic quotation when an ordinary inner closer is reached. The ownership rule covers the existing retained contiguous and spaced forms; genuine accepted boundaries, citations and completed quotations release that ownership. Accepted four-dot behavior, leading decimals and the narrower citation inference cap remain distinct.
Private adapter modes that every caller fixed to true are removed. This drops unreachable legacy branches while preserving the dynamic ordinary/citation paths and supported ellipsis decisions. The case-neutral option documents that structural quotation rules can produce boundaries different from default mode, retaining the useful #168 caveat without promising its older local-close policy. No coverage threshold or dependency configuration is changed.
The final review repairs single-closer ownership, complete-source Treebank alias classification and single-guillemet parity. Ordinary and ellipsis scanners share the existing immutable matched-angle mask; unmatched compact comparisons stay prose, while matched angle spans and immediate-letter aside syntax retain their prior rules. Sparse fragment cursors preserve source positions. The quotation-index first pass is extracted without changing its body, allocation or pass count. Several older four-dot cases still place a spaced single closer in the next span; these are documented limits of the preserved four-dot policy.
Validation
Exact head
52adf6657bbab9adbdd323d1ebc91b3fbb069f0f, tree1915362f8b4d40a5dcb3a6e0c1065bcba17f5c5d: 2,679 tests pass, with 98.23% branch coverage. Source/test types, strict installed Biome, Markdown formatting, build and packed CJS/ESM/TypeScript consumers pass. Independent source/test retention review and normal hooks preserve the validated tracked-file hashes.The full local run validates this exact final tree. The additive repair commit retains all 964 previous test registrations byte-for-byte and adds 37 expanded regressions. All 36 tracked hashes are identical before validation, after validation and after normal commit hooks.
Meaningful probes cover nested retained versus accepted ellipses, literal brackets, leading decimals, Unicode numbers and CJK question controls, source-position handoffs, original regression retention and bounded large-input behavior. Prior intermediate failures remain recorded separately; only final successful combined validation is attributed to this head.
Current-head Codex code/security and Copilot reviews plus exact declared-toolchain CI are requested separately. Existing local Biome 2.5.10 and Node types 24.13.3 remain older than declared 2.5.13 and 24.13.4 after the preserved installation denial; no alternate download, install or configuration workaround was used.