Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
My Good catch on the corner my fix missed — this is the fleet improving on its own work rather than leaving it half-done. 👍 — |
…unctuation ebd5203 (#11573) fixed the dead e.g/i.e entries by matching [\w.]+ but anchored the match with (?:^|\s), which rejects abbreviations preceded by quotes/parens/asterisks ('"Dr' / '(Mr') that the original \b handled — the tts first-sentence early-emit path chopped mid-name ('He cited "Dr.'). drop the anchor: leftmost matching already captures the maximal trailing [\w.] run and any other char or start-of-string delimits it.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Problem
ebd5203 (#11573) fixed
extractFirstSentence's deade.g/i.eabbreviation entries by matching[\w.]+instead of\w+, but anchored the preceding-word match with(?:^|\s)— requiring start-of-string or whitespace immediately before the token. The original\b(\w+)$treated any non-word char as a boundary, so abbreviations preceded by punctuation now fail the abbreviation check and the sentence boundary fires at the abbreviation's period:Consumer impact: the cloud-TTS first-sentence early-emit path (
services/message.ts,hasFirstSentence/extractFirstSentenceon the raw stream) speaks the truncated fragment as the first sentence — the only guard isfirst.length > 5, which'He cited "Dr.'passes — and streaming split decisions land at the wrong point.Fix
Drop the prefix anchor:
preText.match(/([\w.]+)$/). Leftmost-match semantics already capture the maximal trailing[\w.]run, and any other character (space, quote, paren, asterisk, dash, bracket) or start-of-string delimits it — restoring the original\bboundary behavior for punctuation-preceded abbreviations while keeping #11573's dotted-abbreviation fix (an enumerated char class like[\s("'\[*_—-]would just move the hole).Evidence
Failing test first (on develop head 16b69a6):
Post-fix, real output:
vitest run src/utils/text-splitting.test.ts: 5/5 pass (new punctuation-preceded regression test + the fix(core): parseKeyValueXml prefix-tag corruption + extractFirstSentence dead abbrevs + secrets settingUpdated #11573 dotted-abbreviation tests kept green)vitest run src/utils/: 23 files, 144/144 passbun run --cwd packages/core typecheck: cleanN/A - screenshots/video/trajectories: pure-function text utility; behavior fully evidenced by executed before/after output above.