feat(extract): ingest selection knob + keep dotted numbers intact#541
Open
plind-junior wants to merge 2 commits into
Open
feat(extract): ingest selection knob + keep dotted numbers intact#541plind-junior wants to merge 2 commits into
plind-junior wants to merge 2 commits into
Conversation
…hars) ingest filed every substantive sentence of a source — complete, but a restatement of the whole document rather than the facts worth a claim, and that beats nothing on accuracy-per-token, not grep. add extract.select_spans: rank candidate spans by information density (sum over distinct content words of 1/document-frequency, so rare specific terms outweigh stopword-heavy filler) and keep the best under a claim-count or character budget. deterministic and llm-free, and it only ever returns a subset of the verbatim spans — never a paraphrase — so every kept claim's receipt still verifies by construction. thread max_claims/budget_chars through extract_receipt_claims and ingest_source, and expose vouch ingest --max-claims / --budget-chars. unset, ingest keeps every span exactly as before (the unbudgeted baseline stays untouched); the older positional limit used by session-answer capture is unchanged. this is the selection step the compiler pivot needs: fewer, denser claims are what move the fidelity number against the grep baseline.
WalkthroughChangesIngest selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant vouch_ingest
participant ingest_source
participant extract_receipt_claims
participant select_spans
User->>vouch_ingest: run ingest with selection bounds
vouch_ingest->>ingest_source: pass max_claims and budget_chars
ingest_source->>extract_receipt_claims: extract receipt-backed claims
extract_receipt_claims->>select_spans: rank and bound candidate spans
select_spans-->>extract_receipt_claims: return selected spans
extract_receipt_claims-->>ingest_source: return filed claims
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
Bot
added
docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/extract.py`:
- Around line 47-49: Update the new comments near the filler-word definition and
the corresponding comment around line 102 to begin with lowercase prose,
preserving their existing wording and meaning.
- Around line 96-117: Reject negative non-None bounds in the shared selector in
src/vouch/extract.py#L81-L107 by raising ValueError before selection; update
src/vouch/cli.py#L1757-L1765 to use click.IntRange(min=0) for both options so
the CLI reports validation errors, and add negative-bound coverage for direct
helper calls and the command in src/vouch/extract.py#L81-L107,
src/vouch/cli.py#L1757-L1765, tests/test_extract.py#L103-L133, and
tests/test_extract.py#L166-L185.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a4480e2-fba8-48e5-a945-847b51a8453f
📒 Files selected for processing (4)
CHANGELOG.mdsrc/vouch/cli.pysrc/vouch/extract.pytests/test_extract.py
Comment on lines
+96
to
+117
| if max_claims is None and budget_chars is None: | ||
| return segments | ||
| doc_freq: dict[str, int] = {} | ||
| for seg in segments: | ||
| for word in set(_content_words(seg)): | ||
| doc_freq[word] = doc_freq.get(word, 0) + 1 | ||
| # Rank by score (desc), ties broken by original position for determinism. | ||
| ranked = sorted( | ||
| range(len(segments)), | ||
| key=lambda i: (-_span_score(segments[i], doc_freq), i), | ||
| ) | ||
| kept: list[int] = [] | ||
| used = 0 | ||
| for i in ranked: | ||
| if max_claims is not None and len(kept) >= max_claims: | ||
| break | ||
| length = len(segments[i]) | ||
| if budget_chars is not None and used + length > budget_chars: | ||
| continue | ||
| kept.append(i) | ||
| used += length | ||
| return [segments[i] for i in sorted(kept)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the Click version declared by this repository support click.IntRange(min=0) as a click.option type?
💡 Result:
Yes, the Click library supports click.IntRange(min=0) as an option type [1]. click.IntRange is specifically designed to restrict integer input to a defined range [1][2]. When you use click.IntRange(min=0), you effectively create a parameter that accepts any integer greater than or equal to 0 [1]. If the user provides a value less than 0, Click will raise an error by default, unless the clamp=True option is also enabled, in which case the value would be clamped to the minimum boundary [1][2]. This functionality is a standard part of the Click API and is consistent across documented versions [1][2][3]. Top results: [1][2]
Citations:
- 1: https://click.palletsprojects.com/en/stable/parameter-types/
- 2: https://dokk.org/documentation/pallets-click/7.1.2/options/
- 3: https://pocoo-click.readthedocs.io/en/latest/parameters/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- src/vouch/extract.py (relevant section) ---\n'
sed -n '1,220p' src/vouch/extract.py
printf '\n--- src/vouch/cli.py (relevant option section) ---\n'
sed -n '1720,1785p' src/vouch/cli.py
printf '\n--- tests/test_extract.py (relevant tests) ---\n'
sed -n '1,260p' tests/test_extract.py
printf '\n--- search for select_spans usages ---\n'
rg -n "select_spans\(" -S src testsRepository: vouchdev/vouch
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/vouch/extract.py')
print('extract.py lines', sum(1 for _ in p.open()))
p = Path('src/vouch/cli.py')
print('cli.py lines', sum(1 for _ in p.open()))
p = Path('tests/test_extract.py')
print('test_extract.py lines', sum(1 for _ in p.open()))
PYRepository: vouchdev/vouch
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe runtime semantics only if click is importable in the sandbox.
python3 - <<'PY'
try:
import click
except Exception as e:
print(f'click import failed: {e}')
raise SystemExit(0)
opt = click.Option(['--n'], type=click.IntRange(min=0), default=None)
ctx = click.Context(click.Command('cmd', params=[opt]))
def parse(args):
try:
return opt.consume_value(ctx, opt.process_value(ctx, args))
except Exception as e:
return type(e).__name__, str(e)
print('click', click.__version__)
print('type', type(opt.type).__name__)
for value in [None, 0, 3, -1]:
try:
rv = opt.type.convert(value, opt, ctx) if value is not None else None
print(value, '=>', rv)
except Exception as e:
print(value, '=>', type(e).__name__, e)
PYRepository: vouchdev/vouch
Length of output: 433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/vouch/extract.py (relevant section) ---'
sed -n '70,140p' src/vouch/extract.py
echo
echo '--- src/vouch/cli.py (relevant option section) ---'
sed -n '1748,1778p' src/vouch/cli.py
echo
echo '--- tests/test_extract.py (relevant tests) ---'
sed -n '96,190p' tests/test_extract.py
echo
echo '--- direct usage search ---'
rg -n "select_spans\(" src tests -SRepository: vouchdev/vouch
Length of output: 8672
reject negative selection budgets.
max_claims < 0 or budget_chars < 0 silently produces an empty selection, so an invalid input looks like a valid empty result. reject negative values at the shared selector and surface a cli validation error.
src/vouch/extract.py#L81-L107: raiseValueErrorfor negative non-Nonebounds so direct callers cannot bypass the check.src/vouch/cli.py#L1757-L1765: switch these options toclick.IntRange(min=0)so the cli rejects invalid input up front.tests/test_extract.py#L103-L133andtests/test_extract.py#L166-L185: add negative-bound coverage for the helper and the command.
📍 Affects 3 files
src/vouch/extract.py#L96-L117(this comment)src/vouch/cli.py#L1757-L1765tests/test_extract.py#L103-L133tests/test_extract.py#L166-L185
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/extract.py` around lines 96 - 117, Reject negative non-None bounds
in the shared selector in src/vouch/extract.py#L81-L107 by raising ValueError
before selection; update src/vouch/cli.py#L1757-L1765 to use
click.IntRange(min=0) for both options so the CLI reports validation errors, and
add negative-bound coverage for direct helper calls and the command in
src/vouch/extract.py#L81-L107, src/vouch/cli.py#L1757-L1765,
tests/test_extract.py#L103-L133, and tests/test_extract.py#L166-L185.
segment_source split on every `.`, so a version or decimal — 6.8.3, 3.14 — was fractured across segment boundaries and the number-valued answer atom fell out of every quotable span. on a synthetic lookup corpus that dropped ~11% of the ground-truth facts before any budget was applied: the answer simply was not present in any claim to retrieve. a `.` flanked by digits is a decimal/version dot, never a sentence boundary, so the segment regex now keeps it inside the span (sentence-ending periods are unaffected). measured recall ceiling on that corpus rises from 89% to 100% of facts — a cap that bounded every downstream compiler, not just this one.
plind-junior
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the compiler pivot's go/no-go is beat-grep: a compiled bundle has to win on
accuracy-per-token, and it can't while ingest restates the whole document.
this lands the ingest-side machinery for that — a selection knob and an
extraction fix — and reports what measuring it on a real corpus actually says.
selection knob.
extract.select_spansranks candidate spans by informationdensity (sum over distinct content words of 1/document-frequency) and keeps the
best under a claim-count or character budget, threaded through
extract_receipt_claims+ingest_sourceand surfaced asvouch ingest --max-claims / --budget-chars. it is deterministic and llm-free, and — theload-bearing property — it only ever returns a subset of the verbatim spans,
never a paraphrase, so every kept claim's receipt still verifies by
construction. selection cannot fabricate; it can only choose which real quotes
to keep. unset, ingest keeps every span exactly as before.
extraction fix.
segment_sourcesplit on every., so a version ordecimal (
6.8.3,3.14) was fractured across segment boundaries and itsanswer atom fell out of every span — ~11% of the ground-truth facts on a
synthetic lookup corpus, lost before any budget applied. a
.between digitsis now kept inside the span; the recall ceiling of the whole pipeline rises
89% → 100% on that corpus. this cap bounded any downstream compiler, not just
this one.
what the measurement says — honest result. an in-repo probe over the d6
corpus (ingest each doc, select, check how many of the 46 answer atoms survive)
shows coverage tracking fraction-kept almost linearly: keep 70% of spans → 72%
of facts, 50% → 52%, 35% → 35%. the density ranking is ~uncorrelated with
answer-relevance — it drops answer spans as readily as filler, because grep
wins by being query-conditioned and query-agnostic compression at ingest
cedes exactly that. so this PR is the receipt-safe plumbing and a real
recall-ceiling fix; the ranking is the lever that actually moves the number,
and choosing it (query/answer-aware selection vs read-time retrieval precision
vs an llm picker) is deliberately a separate step — measure early, don't
over-build. full gate green — pytest, mypy, ruff.
Summary by CodeRabbit
New Features
--max-claimsand--budget-charstovouch ingestfor density-based span selection.Bug Fixes
6.8.3,3.14) aren’t split.Documentation