fix(extract): treat any non-whitespace-flanked period as non-boundary - #703
Conversation
_SEGMENT_RE only skipped a "." as a sentence boundary when it was
flanked by digits on both sides, correctly keeping decimal/version
numbers like 6.8.3 intact. every other non-whitespace-flanked period -
file paths (src/vouch/cli.py), module/line refs (cli.py:2550), domains
(github.com) - still split the sentence, since the lookaround only
special-cased digits.
segment_source has no coherence check afterward, only a min-length and
letter-ratio filter, so the resulting mid-sentence shards ("see the
changelog at github.") survived and got filed via
extract_receipt_claims -> propose_quoted_claim, which auto-approves
anything that verifies as a byte-exact quote of the source -
review.auto_approve_on_receipt defaults to true, so these fragments
landed as durable, approved claims indistinguishable from real facts.
broaden the lookaround from digit-flanked to non-whitespace-flanked:
(?<=\S)\.(?=\S). this subsumes the existing decimal case (digits are
non-whitespace) while covering file paths, module refs, and domains -
a period only ends a segment when followed by whitespace or
end-of-string.
Fixes vouchdev#702
|
the bug is real and the repro is convincing — my concern is that the replacement is very broad in the other direction, and the PR only tests the direction it is trying to fix.
the result is a longer run, and concretely: the three new assertions are all "these stay intact", and none is "these still split". worth adding the negative half — a two-sentence string with a missing space after the period, and one ending the CHANGELOG wording is accurate about what the regex does, so no issue there. |
…-boundary-non-digit-periods # Conflicts: # CHANGELOG.md
a6c6862 (vouchdev#686) fixed capture.load_config's min_observations and dedup_window_seconds to fall back to their defaults on a malformed config value via the coerce_numeric() helper, instead of raising ValueError straight out of load_config. 47eaf56 (vouchdev#645, realtime opt-in) branched off the pre-fix capture.py and reintroduced the bare int()/float() calls when it merged into test - the coerce_numeric import survived (nothing else referenced it), but the two call sites it fed didn't, silently reverting the fix and breaking ruff's unused-import gate for every PR built on top of `test`. restore the coerce_numeric() calls, matching recall.load_config's still-intact equivalent. unrelated to this PR's own change (extract.py segmentation); needed only to get CI green on top of a currently-broken `test`.
…periods changelog union only; no source conflict.
…periods changelog union only; no source conflict.
|
diff coverage: n/a — this PR changes no python under |
What changed
extract.py's_SEGMENT_REnow treats any period flanked by non-whitespaceon both sides as a non-boundary, instead of only one flanked by digits.
Why
The old lookaround,
(?<=\d)\.(?=\d), correctly kept decimal/versionnumbers intact ("6.8.3") but did nothing for the far more common case of
periods inside file paths (
src/vouch/cli.py), module/line references(
cli.py:2550), or domains (github.com) — every one of those stillsplit the sentence.
segment_sourcehas no coherence check afterward,only a minimum-length and letter-ratio filter, so the resulting
mid-sentence shards survived and got filed via
extract_receipt_claims→
propose_quoted_claim, which auto-approves anything that verifies asa byte-exact quote of the source (
review.auto_approve_on_receiptdefaults to
True).Confirmed with a repro: ingesting "the parser bug lives in
src/vouch/cli.py:2550 near the top. see the changelog at
github.com/vouchdev/vouch/blob/main/CHANGELOG.md for details." produced
four durably-approved claims, two of them nonsensical fragments
(
"see the changelog at github.","py:2550 near the top.").Fixes #702
What might break
Nothing for users with an existing
.vouch/directory — no on-diskshape,
kb.*method, or object model change. Behaviorally: sentencescontaining a file path, module reference, or domain now stay intact
instead of fracturing, which is strictly the intended, documented
behavior for decimals extended to a broader (and more common) class of
non-boundary periods. No legitimate sentence-ending period changes
behavior, since a true sentence end is followed by whitespace or
end-of-string, which still terminates the segment.
VEP
Not applicable — no object model,
kb.*method, on-disk layout, bundleformat, or audit-log shape change. A regex-correctness fix inside the
ingest segmenter.
Related prior work
A related PR, #518 (
fix(extract): stop fragment claims and stopword-matched injection), touched adjacent parts of this file butwas closed unmerged with an unresolved CodeRabbit nit (an unrelated
markdown-bullet-prefix gap) and predates
select_spans'sdensity-selection logic added since, so its diff no longer applies. This
PR targets a narrower, independently-verified defect in the current
code — the segmentation regex's digit-only lookaround — unrelated to
what #518 was reviewing.
Tests
make check-equivalent: ruff clean (src+tests); mypyclean on
extract.py; all 14tests/test_extract.pycases pass(13 pre-existing + 1 new);
tests/test_bench.py,tests/test_capture_answer.py,tests/test_capture_scope.py, andtests/test_health.py(all consumeextract.py) pass with onepre-existing, unrelated failure
(
test_finalize_supersedes_updated_claims), confirmed presentidentically on
testHEAD before this change viagit stashcomparison
test_segment_source_keeps_file_paths_and_urls_intactCHANGELOG.mdupdated under## [Unreleased]