feat: beat-ditto foundation — bench, enrichment, supersession, retrieval fixes - #561
Merged
Conversation
per-turn stop-hook extraction saw one answer at a time, which produced
episodic fragments like "test coverage and green gates are noted at the
end" — grammatical enough to pass admission, useless out of context,
and filed with a fresh claim budget every turn.
capture.answer_mode (default "session") moves answer memory to
SessionEnd: finalize hands the whole transcript to
capture_session_answers, which ingests every turn's final answer as one
content-addressed document (questions stay in metadata so claims can
only quote assistant prose), extracts receipt-backed claims with the
full session in view, and spends one claim budget per session.
identical spans collapse across turns and re-finalizing an unchanged
transcript is a no-op.
the stop hook stays wired: capture_answer now defers in session mode
("deferred-to-session-end"), and capture.answer_mode: turn restores the
legacy per-turn behaviour unchanged.
port the strongest idea in ditto's dream pipeline — small-model subject extraction as strict json — into the capture path, adapted to vouch's shape: one llm pass over the whole session record (intent + tool activity + changed files + git stat) instead of per exchange, output decorating the pending mechanical rollup page (title, summary and subjects sections, slugified subject tags, metadata) rather than writing anything durable. subjects become fts-indexed page tags, so a session is findable by subject even when the raw observation text never used those words together. the model is deployment config (capture.enrich.llm_cmd, falling back to compile.llm_cmd); the base install behaves exactly as before. failure never blocks: any llm or parse error files the plain page. the split-failure fallback path skips enrichment so a broken llm_cmd is not called twice in one run. finalize now runs session answers before the summary so the rollup page cites the session-answers source; a cited session page clears the admission gate's uncited-diary rule via the existing citation rule instead of a new exemption. already-captured answers return their source id so a re-finalize still cites it.
the measurement layer of the beat-ditto plan: "better" becomes a table, not an adjective. `vouch bench run --seed N` generates a deterministic dataset (coined values a grep cannot luck into, a decoy person holding same-attribute facts, knowledge updates where only the latest value is correct, a stored-instruction injection note, cross-person abstention probes), ingests every session through the real receipt-gated capture loop into a throwaway kb, retrieves through build_context_pack under a character budget, and grades by substring checks against a typed answer key. no llm, no network, no judge: a score is a pure function of (seed, code), reproducible on any machine — the property the planned github competition needs. grading has the dump-guard property on purpose: surfacing a forbidden value (a decoy, a superseded value) zeroes the case, so stuffing the whole kb into the pack loses on decoy and abstention categories and the only way to score is to rank well under the budget. category names follow the dittobench taxonomy where semantics match so cross-system comparisons read 1:1. baseline at head (seeds 1-6): composite 0.57 ± 0.04. perfect single-session recall and point-in-time; the zeros — knowledge-update (no recency signal), decoy-discrimination, abstention leak — are exactly the levers the planned retrieval composite targets. `vouch bench gen --seed N` prints the dataset and answer key for inspection. multi-seed runs report mean composite with a standard error, the input a margin-band comparison is built from.
two defects found by running the new benchmark against the recency stage. first, whole-day age quantization silently turned any sub-day half-life into a no-op: every same-day artifact truncated to age zero, decay 1.0, order unchanged — the opt-in knob did nothing at session scale. ages stay fractional when half_life_days < 1; the whole-day quantization (and its byte-stable repeat-query property) is kept for half-lives of a day or more, where sub-day age really is noise. second, backend parity: the recency stage ran only on the hybrid path. a kb configured with backend fts5 or embedding never applied it even with retrieval.recency.enabled true. both paths now rescore through _maybe_recency like hybrid does. the bench grows the two knobs the a/b needed: extra_config appends verbatim yaml to the throwaway kb's config (one-moving-part arms) and session_gap_seconds spaces session ingests so claim timestamps carry the sessions' temporal order. measured result, recorded honestly: with recency on, knowledge-update stays 0.00 at the default budget — the superseded value still sits in the pack and the dump-guard zeroes the case. reordering is not exclusion; the category needs conflict collapse (supersede-aware retrieval), which is the next lever.
the flywheel input for learned retrieval — the role ditto's retrieval_events table plays for its weight-predictor mlp, in vouch's shape: a plaintext jsonl beside the kb, local-only, never a cloud row. each build_context_pack call appends what was asked (secret-masked, same rule as the capture buffer) and what came back (type/id/score only — summaries would re-copy kb content into telemetry). positive labels arrive later from the lifecycle: a kb_confirm re-citation of a claim an event surfaced is a human-verified relevance judgment, a label source ditto does not have. properties: gitignored (init template lists it; first write backfills the ignore line on pre-existing kbs), size-capped with one rotated generation, retrieval.events.enabled: false turns it off, and logging is never load-bearing — a broken log file changes nothing about retrieval results.
seeds 1-6 at head: composite 0.57 ± 0.04. the zeros are the levers: knowledge-update needs lifecycle-driven supersession (reranking cannot clear the dump-guard), decoy-discrimination and abstention need person scoping. ditto's own stock-harness numbers included for calibration only — different benchmarks, not directly comparable.
closes the knowledge-update gap the benchmark exposed, the gated way. the enrich pass now also emits updates — value changes the session record itself states, both values verbatim — and finalize resolves them onto durable claims with lifecycle.supersede, after which the superseded claim leaves every context pack (the existing retracted- status exclusion). precision over recall at every step: supersession runs only under the same self-approval conditions capture's claims already use (trusted- agent or the receipt gate; gate closed means nothing is touched), a value must identify exactly one live claim (a hallucinated value matches nothing and the update is dropped), and the new claim must be strictly newer than the old. every attempted update is reported in the finalize result with what happened and why. this is the fix reranking could not deliver: the bench showed recency reordering leaves the superseded value in the pack where the dump- guard zeroes the case. exclusion via the lifecycle is vouch's native machinery — auditable, reversible in history, and something ditto's rewrite-in-place consolidation cannot express.
…number two beat-ditto roadmap items. pages-first: an opt-in stage (retrieval.pages_first.enabled, boost default 1.25) that lifts compiled topic pages above raw claims in context packs — consolidation done at write time beats a pile of fragments when a reviewed synthesis answers the query. session/log pages are raw material (the same line admission and compile draw) and are never boosted. applied uniformly on the hybrid, fts5, and embedding paths. receipt-coverage: kb_status / vouch status now report the fidelity headline — the share of live claims citing at least one byte-offset evidence receipt. structural coverage only; byte-level verification stays in doctor/fsck, keeping status cheap. the receipt gate proves a quote is real; this number says how much of the kb carries that proof.
the p5 scaffolding from the beat-ditto plan: practice scoring on every pull request (public seeds, no secrets, pure function of seed + code) and a maintainer-dispatched scored run whose seeds derive from the drand round at the season cutoff — commit-reveal, so entries are frozen before the seeds exist. dispatch inputs pass through env, never interpolated into the script body. docs/vouchbench-seasons.md holds the season mechanics: freeze rules, margin band max(0.007, 1.64 x paired se), first-seen tie protection, payout shares, the two ladders, and the disqualification line (benchmark-keyed logic, gate bypasses).
WalkthroughThis PR adds session-scoped answer capture and enrichment, a deterministic VouchBench benchmark with CLI and CI workflows, retrieval scoring and telemetry enhancements, and receipt coverage reporting with corresponding tests and documentation. ChangesSession answer memory and enrichment
VouchBench scoring
Retrieval scoring and telemetry
Receipt coverage reporting
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SessionEnd
participant capture.finalize
participant capture_session_answers
participant session_split
participant enrich_session
participant propose_page
SessionEnd->>capture.finalize: finalize transcript
capture.finalize->>capture_session_answers: extract session answers
capture.finalize->>session_split: summarize with sources
session_split->>enrich_session: enrich session when enabled
enrich_session-->>session_split: enrichment subjects and updates
session_split->>propose_page: create mechanical proposal
sequenceDiagram
participant PullRequest
participant GitHubActions
participant BenchCLI
participant VouchBench
participant Retrieval
PullRequest->>GitHubActions: matching source changes
GitHubActions->>BenchCLI: run practice seeds
BenchCLI->>VouchBench: execute benchmark
VouchBench->>Retrieval: retrieve context packs
Retrieval-->>VouchBench: scored results
VouchBench-->>GitHubActions: JSON report and summary
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 15
🧹 Nitpick comments (7)
src/vouch/health.py (1)
61-67: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winavoid rescanning claims and evidence in
status()
receipt_coverage(store)already loads all claims and evidence, thenstatus()loads both collections again for the"claims"and"evidence"fields. derive both values from one snapshot, or pass preloaded collections into a private helper, to avoid duplicate filesystem work on every status request.🤖 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/health.py` around lines 61 - 67, Update status() to avoid rescanning claims and evidence after receipt_coverage(store) has loaded them. Reuse a single claims/evidence snapshot for receipt coverage and the "claims"/"evidence" counts, or pass those preloaded collections through a private helper while preserving the existing status fields.src/vouch/session_split.py (1)
174-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe fallback predicate is computed twice with a double negative.
not (mode != "mechanical" and want_split)on line 178 and the same expression on line 190 are the fallback test. Hoist it once.♻️ Proposed refactor
- enrichment = None - if not (mode != "mechanical" and want_split): + fell_back = mode != "mechanical" and want_split + enrichment = None + if not fell_back: enrichment = enrich.enrich_session(and reuse
fell_backforfinal_mode.🤖 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/session_split.py` around lines 174 - 190, Hoist the repeated fallback predicate into a clearly named boolean such as fell_back before the enrichment block, then reuse it for both the enrichment guard and final_mode assignment. Preserve the existing behavior: skip enrichment and use "fallback" only when mode is non-mechanical and want_split is true.src/vouch/capture.py (2)
570-579: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSwallowed extraction failure leaves no trace.
except Exceptioncollapses every capture_session_answers failure intoskipped: "error"with no message, so a persistently broken extraction path is invisible in logs. Log the exception (logger.warning("capture: session answer extraction failed for %s (%s)", session_id, e)) while keeping the never-lose-the-summary behavior.🤖 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/capture.py` around lines 570 - 579, Update the capture_session_answers exception handler in the session answer flow to bind the caught exception and emit the specified logger.warning with session_id and the exception details, while preserving the existing _answer_skip fallback and summary-retention behavior.
421-436: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBudget accounting ignores the
"\n\n"joiners.
totalsums only answer lengths, so the emitted document can exceedmax_session_charsby 2×(exchanges−1). Minor, but easy to make exact.♻️ Proposed tweak
- if kept and total + len(a) > max_session_chars: + if kept and total + len(a) + 2 > max_session_chars: break kept.append((_excerpt(q, max_chars=max_question_chars), a)) - total += len(a) + total += len(a) + 2🤖 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/capture.py` around lines 421 - 436, Update the session budget accounting in the exchange loop that builds `kept` and `document` so `total` includes the `"\n\n"` separators between answers, matching the length of the final joined document. Preserve the existing behavior for the first answer and stop adding exchanges once the exact `max_session_chars` limit would be exceeded.tests/test_enrich.py (1)
79-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo unit coverage for
updatesparsing.
parse_enrichmentgained anupdatesbranch with its own validation (all three fields must be strings, non-empty after strip,old != newcase-insensitively) that drives claim supersession, but every test here only exercisessummary/subjects. Add cases for a valid update, a non-string entry, and anold == newentry being dropped.Happy to draft them if useful.
🤖 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 `@tests/test_enrich.py` around lines 79 - 107, Add focused tests for the updates branch in parse_enrichment: verify a valid update with three non-empty string fields is retained, a non-string update entry is discarded, and an update whose old and new values match case-insensitively is discarded. Assert the parsed updates collection contains only valid entries while preserving existing summary/subjects behavior.Source: Coding guidelines
src/vouch/storage.py (1)
432-440: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExisting KBs won't pick up the new ignore.
The block only writes when
.gitignoreis absent, so KBs initialized before this change will start emittingretrieval_events.jsonlinto a tracked directory. Consider appending missing patterns to an existing file.🤖 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/storage.py` around lines 432 - 440, Update the .gitignore handling in the KB initialization flow so existing files are also checked for and receive any missing patterns, especially retrieval_events.jsonl and its related generated files. Preserve existing ignore entries, avoid duplicate lines, and continue creating the complete ignore content for KBs without a .gitignore.src/vouch/context.py (1)
203-257: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider consolidating the recency/pages-first scaffolding.
Two things worth tidying as more retrieval stages land on this pattern:
_configured_pages_firstre-implements the same yaml-load/dict-guard scaffolding as_configured_recency; a small shared helper would remove the duplication._maybe_pages_firstre-fetches every "page" hit viastore.get_page, duplicating the read_maybe_recencyalready performed via_artifact_timestampfor the same hits. Threading the already-fetched artifact through (or a per-call cache) avoids the double I/O.🤖 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/context.py` around lines 203 - 257, Consolidate the YAML loading and dictionary validation shared by _configured_pages_first and _configured_recency into a helper, then reuse it while preserving each setting’s defaults. Eliminate duplicate page reads between _maybe_recency and _maybe_pages_first by threading the artifact already obtained through the retrieval flow or using a per-call cache, and use it for pages-first eligibility checks.
🤖 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 `@CHANGELOG.md`:
- Around line 10-19: Expand the changelog’s Added section to document the other
user-visible features introduced by this PR: session enrichment via
capture.enrich, enrichment-driven supersession, the vouch bench command, and
receipt coverage in vouch status. Keep the existing answer_mode entry unchanged.
In `@src/vouch/bench.py`:
- Around line 379-385: The benchmark setup around KBStore.init must not reuse or
overwrite a caller-supplied KB. When workdir is provided, create an isolated
child directory for this run or fail if workdir / "kb" already exists, ensuring
prior claims are not retained and the existing config write cannot mutate a
non-throwaway store.
- Around line 253-254: Ensure generated contrast values are distinct: in
src/vouch/bench.py lines 253-254, update the generation around v1 and v2 to
regenerate v2 until it differs from v1; at lines 267-268, regenerate decoy_value
until it differs from value. Preserve the existing _coin_value generation
behavior otherwise.
- Around line 259-262: Update the “point-in-time” MemoryCase construction in the
benchmark cases to include v2 in its forbidden values, ensuring retrieval of the
newer value fails; add a corresponding assertion in tests/test_bench.py covering
this rejection behavior.
- Around line 207-224: Validate that sessions is at least two at the start of
generate, before initializing the random generator or _Sessions, and reject
smaller values with an appropriate exception. Preserve the existing dataset
generation flow for valid session counts.
In `@src/vouch/capture.py`:
- Around line 825-833: Replace the use of review_cfg["auto_approve_on_receipt"]
in the gate_open condition with a dedicated opt-in for applying receipt-free
updates, such as capture.enrich.apply_updates, defaulting to disabled. Keep
trusted-agent authorization unchanged, and ensure the updates path remains
closed unless the new setting is explicitly enabled.
- Around line 840-843: The match function’s unanchored substring lookup can
identify unrelated claims; require word-boundary matching or a minimum value
length before accepting a unique hit, while preserving the single-hit behavior
otherwise. Also replace updates[:DEFAULT_MAX_ANSWER_CLAIMS] with a dedicated
constant representing the maximum number of updates.
- Around line 748-782: Mask secrets in the session document and question
metadata before hashing or persisting them in the session-answer path around
session_history, sha256_hex, and store.put_source. Derive the source ID from the
masked document so stored bytes and quoted spans remain consistent, and use the
masked questions when populating metadata and the source title; preserve the
existing skip and deduplication behavior.
In `@src/vouch/cli.py`:
- Around line 5420-5422: Update the --seeds parsing around seed_list and
bench_mod.run_seeds to validate each value and reject malformed or empty input
with a user-facing Click error instead of allowing ValueError or StatisticsError
tracebacks. Preserve valid comma-separated integer seeds and only call run_seeds
after confirming the parsed list is non-empty.
In `@src/vouch/context.py`:
- Around line 211-228: Update _configured_pages_first so the enabled value is
returned as true only when raw.get("enabled", False) is an actual bool, matching
_configured_recency; otherwise default it to False. Leave the existing boost
parsing and fallback behavior unchanged.
In `@src/vouch/enrich.py`:
- Around line 237-238: Update the subject and update accumulation limits in the
surrounding enrichment logic: use a dedicated max_updates bound for updates
instead of max_subjects, and change both post-append equality checks to >= so
max_subjects and max_updates set to 0 immediately stop accumulation. Preserve
the existing break behavior while applying each knob only to its corresponding
list.
- Around line 99-108: Update load_enrich_config to catch UnicodeDecodeError
alongside OSError and yaml.YAMLError, and ensure the
compile_mod.load_config(store) call within enrich_session is also guarded so
these configuration-loading failures return the existing safe fallback instead
of escaping the never-raises contract.
In `@src/vouch/retrieval_events.py`:
- Around line 145-166: Update read_events so limit=0 returns an empty list
instead of slicing with out[-0:]. Preserve the existing newest-N behavior for
positive limits and the unlimited behavior when limit is None or negative.
In `@src/vouch/session_split.py`:
- Around line 179-182: Prevent automatic enrichment LLM calls for existing
configurations by either defaulting enrichment disabled in _starter_config() or
removing enrich_session’s fallback to compile.llm_cmd when
capture.enrich.llm_cmd is unset. Preserve explicitly configured enrichment
commands and ensure the starter configuration advertises the intended opt-in
behavior.
In `@tests/test_health.py`:
- Around line 409-430: Extend test_receipt_coverage_fidelity_number with a claim
whose status is one of the health module’s _RETRACTED_CLAIM_STATUSES, using the
existing claim setup path. Recalculate and assert receipt_coverage excludes that
retracted claim from live_claims, receipted, and ratio, and keep the status
receipt_coverage assertion aligned with the updated expected result.
---
Nitpick comments:
In `@src/vouch/capture.py`:
- Around line 570-579: Update the capture_session_answers exception handler in
the session answer flow to bind the caught exception and emit the specified
logger.warning with session_id and the exception details, while preserving the
existing _answer_skip fallback and summary-retention behavior.
- Around line 421-436: Update the session budget accounting in the exchange loop
that builds `kept` and `document` so `total` includes the `"\n\n"` separators
between answers, matching the length of the final joined document. Preserve the
existing behavior for the first answer and stop adding exchanges once the exact
`max_session_chars` limit would be exceeded.
In `@src/vouch/context.py`:
- Around line 203-257: Consolidate the YAML loading and dictionary validation
shared by _configured_pages_first and _configured_recency into a helper, then
reuse it while preserving each setting’s defaults. Eliminate duplicate page
reads between _maybe_recency and _maybe_pages_first by threading the artifact
already obtained through the retrieval flow or using a per-call cache, and use
it for pages-first eligibility checks.
In `@src/vouch/health.py`:
- Around line 61-67: Update status() to avoid rescanning claims and evidence
after receipt_coverage(store) has loaded them. Reuse a single claims/evidence
snapshot for receipt coverage and the "claims"/"evidence" counts, or pass those
preloaded collections through a private helper while preserving the existing
status fields.
In `@src/vouch/session_split.py`:
- Around line 174-190: Hoist the repeated fallback predicate into a clearly
named boolean such as fell_back before the enrichment block, then reuse it for
both the enrichment guard and final_mode assignment. Preserve the existing
behavior: skip enrichment and use "fallback" only when mode is non-mechanical
and want_split is true.
In `@src/vouch/storage.py`:
- Around line 432-440: Update the .gitignore handling in the KB initialization
flow so existing files are also checked for and receive any missing patterns,
especially retrieval_events.jsonl and its related generated files. Preserve
existing ignore entries, avoid duplicate lines, and continue creating the
complete ignore content for KBs without a .gitignore.
In `@tests/test_enrich.py`:
- Around line 79-107: Add focused tests for the updates branch in
parse_enrichment: verify a valid update with three non-empty string fields is
retained, a non-string update entry is discarded, and an update whose old and
new values match case-insensitively is discarded. Assert the parsed updates
collection contains only valid entries while preserving existing
summary/subjects behavior.
🪄 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: f9d91f3b-0183-4181-8617-cca34543fdca
📒 Files selected for processing (22)
.github/workflows/vouchbench-season.ymlCHANGELOG.mddocs/vouchbench-seasons.mdsrc/vouch/bench.pysrc/vouch/capture.pysrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/enrich.pysrc/vouch/health.pysrc/vouch/retrieval_events.pysrc/vouch/session_split.pysrc/vouch/storage.pytests/test_adopt.pytests/test_bench.pytests/test_capture.pytests/test_capture_answer.pytests/test_capture_scope.pytests/test_enrich.pytests/test_health.pytests/test_retrieval_backend.pytests/test_retrieval_events.pytests/test_session_split.py
Comment on lines
+10
to
+19
| - **session-mode answer memory** (`capture.answer_mode`, default `session`): | ||
| claims are extracted once at SessionEnd from the full transcript history | ||
| (`capture_session_answers`, wired into `capture finalize`) instead of on | ||
| every Stop hook. per-turn extraction saw one answer at a time, which is | ||
| where single-turn fragments like "… are noted at the end" came from; the | ||
| session document gives the extractor every turn at once, collapses | ||
| duplicate spans across turns, and spends one `max_claims` budget per | ||
| session rather than per turn. `capture.answer_mode: turn` restores the | ||
| legacy per-turn behaviour; the Stop hook stays wired either way (it defers | ||
| with `deferred-to-session-end` in session mode). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Changelog covers answer_mode only.
The Added section has no entry for the other user-visible features in this PR — session enrichment (capture.enrich), enrichment-driven supersession, vouch bench, or receipt coverage in vouch status. Please add them here.
🤖 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 `@CHANGELOG.md` around lines 10 - 19, Expand the changelog’s Added section to
document the other user-visible features introduced by this PR: session
enrichment via capture.enrich, enrichment-driven supersession, the vouch bench
command, and receipt coverage in vouch status. Keep the existing answer_mode
entry unchanged.
Source: Coding guidelines
Comment on lines
+207
to
+224
| def generate(seed: int, *, sessions: int = DEFAULT_SESSIONS) -> Dataset: | ||
| """Build the deterministic dataset for ``seed``. | ||
|
|
||
| Each of the seven categories gets one attribute from the pool (seeded | ||
| shuffle), its statements planted across ``sessions`` session documents, | ||
| and filler prose everywhere so retrieval has a real haystack. | ||
| """ | ||
| rng = random.Random(seed) | ||
| attrs = list(_ATTRIBUTES) | ||
| rng.shuffle(attrs) | ||
| docs = _Sessions(sessions, rng) | ||
| cases: list[MemoryCase] = [] | ||
|
|
||
| def spot(exclude: int | None = None) -> int: | ||
| idx = rng.randrange(sessions) | ||
| while idx == exclude: | ||
| idx = rng.randrange(sessions) | ||
| return idx |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
require at least two sessions.
sessions=1 loops forever in spot(exclude=home); sessions=0 also reaches invalid random ranges. reject values below two before building the dataset.
proposed fix
def generate(seed: int, *, sessions: int = DEFAULT_SESSIONS) -> Dataset:
+ if sessions < 2:
+ raise ValueError("sessions must be at least 2")
"""Build the deterministic dataset for ``seed``.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def generate(seed: int, *, sessions: int = DEFAULT_SESSIONS) -> Dataset: | |
| """Build the deterministic dataset for ``seed``. | |
| Each of the seven categories gets one attribute from the pool (seeded | |
| shuffle), its statements planted across ``sessions`` session documents, | |
| and filler prose everywhere so retrieval has a real haystack. | |
| """ | |
| rng = random.Random(seed) | |
| attrs = list(_ATTRIBUTES) | |
| rng.shuffle(attrs) | |
| docs = _Sessions(sessions, rng) | |
| cases: list[MemoryCase] = [] | |
| def spot(exclude: int | None = None) -> int: | |
| idx = rng.randrange(sessions) | |
| while idx == exclude: | |
| idx = rng.randrange(sessions) | |
| return idx | |
| def generate(seed: int, *, sessions: int = DEFAULT_SESSIONS) -> Dataset: | |
| """Build the deterministic dataset for ``seed``. | |
| Each of the seven categories gets one attribute from the pool (seeded | |
| shuffle), its statements planted across ``sessions`` session documents, | |
| and filler prose everywhere so retrieval has a real haystack. | |
| """ | |
| if sessions < 2: | |
| raise ValueError("sessions must be at least 2") | |
| rng = random.Random(seed) | |
| attrs = list(_ATTRIBUTES) | |
| rng.shuffle(attrs) | |
| docs = _Sessions(sessions, rng) | |
| cases: list[MemoryCase] = [] | |
| def spot(exclude: int | None = None) -> int: | |
| idx = rng.randrange(sessions) | |
| while idx == exclude: | |
| idx = rng.randrange(sessions) | |
| return idx |
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 213-213: use secrets package over random package
Context: random.Random(seed)
Note: [CWE-330] Use of Insufficiently Random Values.
(avoid-random-python)
🤖 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/bench.py` around lines 207 - 224, Validate that sessions is at
least two at the start of generate, before initializing the random generator or
_Sessions, and reject smaller values with an appropriate exception. Preserve the
existing dataset generation flow for valid session counts.
Comment on lines
+253
to
+254
| v1 = _coin_value(rng, attr) | ||
| v2 = _coin_value(rng, attr) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ensure contrast values are distinct.
independent generation can produce identical values—especially for deploy day—making an expected value forbidden and producing an invalid benchmark case.
src/vouch/bench.py#L253-L254: regeneratev2until it differs fromv1.src/vouch/bench.py#L267-L268: regeneratedecoy_valueuntil it differs fromvalue.
📍 Affects 1 file
src/vouch/bench.py#L253-L254(this comment)src/vouch/bench.py#L267-L268
🤖 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/bench.py` around lines 253 - 254, Ensure generated contrast values
are distinct: in src/vouch/bench.py lines 253-254, update the generation around
v1 and v2 to regenerate v2 until it differs from v1; at lines 267-268,
regenerate decoy_value until it differs from value. Preserve the existing
_coin_value generation behavior otherwise.
Comment on lines
+259
to
+262
| cases.append(MemoryCase("knowledge-update", question(asks), v2, (v1,))) | ||
| cases.append(MemoryCase( | ||
| "point-in-time", f"what was the {attr} before it changed?", v1, | ||
| )) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
make point-in-time reject the newer value.
the case passes when both v1 and v2 are packed, so it does not enforce temporal retrieval or the stated dump guard. add v2 to forbidden and cover that failure mode in tests/test_bench.py.
proposed fix
cases.append(MemoryCase(
- "point-in-time", f"what was the {attr} before it changed?", v1,
+ "point-in-time", f"what was the {attr} before it changed?", v1, (v2,),
))🤖 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/bench.py` around lines 259 - 262, Update the “point-in-time”
MemoryCase construction in the benchmark cases to include v2 in its forbidden
values, ensuring retrieval of the newer value fails; add a corresponding
assertion in tests/test_bench.py covering this rejection behavior.
Comment on lines
+379
to
+385
| with tempfile.TemporaryDirectory(prefix="vouch-bench-") as tmp: | ||
| root = workdir or Path(tmp) | ||
| store = KBStore.init(root / "kb") | ||
| config_text = "review:\n auto_approve_on_receipt: true\n" | ||
| if extra_config: | ||
| config_text += extra_config.rstrip() + "\n" | ||
| store.config_path.write_text(config_text, encoding="utf-8") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
do not reuse or overwrite a caller-supplied kb.
a reused workdir silently retains prior benchmark claims, and lines 383-385 overwrite its config. this makes scores history-dependent and can mutate a non-throwaway KB. create an isolated child directory or fail when workdir / "kb" already exists.
🤖 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/bench.py` around lines 379 - 385, The benchmark setup around
KBStore.init must not reuse or overwrite a caller-supplied KB. When workdir is
provided, create an isolated child directory for this run or fail if workdir /
"kb" already exists, ensuring prior claims are not retained and the existing
config write cannot mutate a non-throwaway store.
Comment on lines
+99
to
+108
| try: | ||
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | ||
| except (OSError, yaml.YAMLError): | ||
| return EnrichConfig() | ||
| if not isinstance(loaded, dict): | ||
| return EnrichConfig() | ||
| cap = loaded.get("capture") | ||
| raw = cap.get("enrich") if isinstance(cap, dict) else None | ||
| if not isinstance(raw, dict): | ||
| return EnrichConfig() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
enrich_session's "never raises" contract has holes.
load_enrich_config catches OSError/YAMLError but not UnicodeDecodeError — capture.load_config catches all three for the same read. compile_mod.load_config(store) on line 283 is likewise uncaught. A config file with non-UTF-8 bytes would propagate out of enrich_session and, via session_split.summarize, lose the session summary.
🛡️ Proposed fix
- except (OSError, yaml.YAMLError):
+ except (OSError, UnicodeDecodeError, yaml.YAMLError):
return EnrichConfig()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | |
| except (OSError, yaml.YAMLError): | |
| return EnrichConfig() | |
| if not isinstance(loaded, dict): | |
| return EnrichConfig() | |
| cap = loaded.get("capture") | |
| raw = cap.get("enrich") if isinstance(cap, dict) else None | |
| if not isinstance(raw, dict): | |
| return EnrichConfig() | |
| try: | |
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | |
| except (OSError, UnicodeDecodeError, yaml.YAMLError): | |
| return EnrichConfig() | |
| if not isinstance(loaded, dict): | |
| return EnrichConfig() | |
| cap = loaded.get("capture") | |
| raw = cap.get("enrich") if isinstance(cap, dict) else None | |
| if not isinstance(raw, dict): | |
| return EnrichConfig() |
🤖 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/enrich.py` around lines 99 - 108, Update load_enrich_config to
catch UnicodeDecodeError alongside OSError and yaml.YAMLError, and ensure the
compile_mod.load_config(store) call within enrich_session is also guarded so
these configuration-loading failures return the existing safe fallback instead
of escaping the never-raises contract.
Comment on lines
+237
to
+238
| if len(subjects) == max_subjects: | ||
| break |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
max_subjects doubles as the updates cap, and a cap of 0 is a no-op.
len(updates) == max_subjects bounds a different list with the subjects knob — an operator lowering max_subjects silently shrinks the updates budget too. Separately, both == max_subjects checks run after the append, so max_subjects: 0 never trips and every entry is kept. Use >= and a dedicated max_updates.
♻️ Proposed fix
- if len(subjects) == max_subjects:
+ if len(subjects) >= max_subjects:
break(and the same for updates, against its own bound)
Also applies to: 256-257
🤖 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/enrich.py` around lines 237 - 238, Update the subject and update
accumulation limits in the surrounding enrichment logic: use a dedicated
max_updates bound for updates instead of max_subjects, and change both
post-append equality checks to >= so max_subjects and max_updates set to 0
immediately stop accumulation. Preserve the existing break behavior while
applying each knob only to its corresponding list.
Comment on lines
+145
to
+166
| def read_events(store: KBStore, *, limit: int | None = None) -> list[dict[str, Any]]: | ||
| """Parsed events, oldest first; ``limit`` keeps the newest N.""" | ||
| path = _events_path(store) | ||
| if not path.exists(): | ||
| return [] | ||
| out: list[dict[str, Any]] = [] | ||
| try: | ||
| for line in path.read_text(encoding="utf-8").splitlines(): | ||
| line = line.strip() | ||
| if not line: | ||
| continue | ||
| try: | ||
| obj = json.loads(line) | ||
| except json.JSONDecodeError: | ||
| continue | ||
| if isinstance(obj, dict): | ||
| out.append(obj) | ||
| except OSError: | ||
| return [] | ||
| if limit is not None and limit >= 0: | ||
| return out[-limit:] | ||
| return out |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
read_events(limit=0) returns all events, not zero.
out[-limit:] with limit=0 is out[-0:] == out[0:], the full list, since Python has no negative zero for ints. Contradicts the docstring's "keeps the newest N" contract.
🐛 Proposed fix
if limit is not None and limit >= 0:
- return out[-limit:]
+ return out[-limit:] if limit > 0 else []
return out📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def read_events(store: KBStore, *, limit: int | None = None) -> list[dict[str, Any]]: | |
| """Parsed events, oldest first; ``limit`` keeps the newest N.""" | |
| path = _events_path(store) | |
| if not path.exists(): | |
| return [] | |
| out: list[dict[str, Any]] = [] | |
| try: | |
| for line in path.read_text(encoding="utf-8").splitlines(): | |
| line = line.strip() | |
| if not line: | |
| continue | |
| try: | |
| obj = json.loads(line) | |
| except json.JSONDecodeError: | |
| continue | |
| if isinstance(obj, dict): | |
| out.append(obj) | |
| except OSError: | |
| return [] | |
| if limit is not None and limit >= 0: | |
| return out[-limit:] | |
| return out | |
| def read_events(store: KBStore, *, limit: int | None = None) -> list[dict[str, Any]]: | |
| """Parsed events, oldest first; ``limit`` keeps the newest N.""" | |
| path = _events_path(store) | |
| if not path.exists(): | |
| return [] | |
| out: list[dict[str, Any]] = [] | |
| try: | |
| for line in path.read_text(encoding="utf-8").splitlines(): | |
| line = line.strip() | |
| if not line: | |
| continue | |
| try: | |
| obj = json.loads(line) | |
| except json.JSONDecodeError: | |
| continue | |
| if isinstance(obj, dict): | |
| out.append(obj) | |
| except OSError: | |
| return [] | |
| if limit is not None and limit >= 0: | |
| return out[-limit:] if limit > 0 else [] | |
| return out |
🤖 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/retrieval_events.py` around lines 145 - 166, Update read_events so
limit=0 returns an empty list instead of slicing with out[-0:]. Preserve the
existing newest-N behavior for positive limits and the unlimited behavior when
limit is None or negative.
Comment on lines
+179
to
+182
| enrichment = enrich.enrich_session( | ||
| store, session_id, observations, changed_files, git_stat, | ||
| intent=intent, | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Every mechanical rollup now makes an LLM call on the SessionEnd hook.
enrich_session falls back to compile.llm_cmd when capture.enrich.llm_cmd is unset, and capture.enrich.enabled defaults to True — so any KB that already configured a compile command starts paying an enrichment subprocess (default timeout 60s) at the end of every session, with no config change and no starter-config key advertising it. Consider defaulting the enrich block off in _starter_config(), or not inheriting compile.llm_cmd here.
🤖 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/session_split.py` around lines 179 - 182, Prevent automatic
enrichment LLM calls for existing configurations by either defaulting enrichment
disabled in _starter_config() or removing enrich_session’s fallback to
compile.llm_cmd when capture.enrich.llm_cmd is unset. Preserve explicitly
configured enrichment commands and ensure the starter configuration advertises
the intended opt-in behavior.
Comment on lines
+409
to
+430
| def test_receipt_coverage_fidelity_number(store: KBStore) -> None: | ||
| from vouch.extract import ingest_source | ||
| from vouch.models import Claim | ||
|
|
||
| assert health.receipt_coverage(store) == { | ||
| "live_claims": 0, "receipted": 0, "ratio": None, | ||
| } | ||
| # a receipt-backed claim (cites an Evidence record) | ||
| ingest_source( | ||
| store, | ||
| b"The deploy window opens every wednesday at nine in the morning.", | ||
| proposed_by="test", | ||
| ) | ||
| cov = health.receipt_coverage(store) | ||
| assert cov["live_claims"] == cov["receipted"] == 1 | ||
| assert cov["ratio"] == 1.0 | ||
| # a plain claim citing only a source — no receipt | ||
| src = store.put_source(b"unquoted background material") | ||
| store.put_claim(Claim(id="plain", text="a plain claim", evidence=[src.id])) | ||
| cov = health.receipt_coverage(store) | ||
| assert cov == {"live_claims": 2, "receipted": 1, "ratio": 0.5} | ||
| assert health.status(store)["receipt_coverage"] == cov |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
cover the retracted-claim boundary
This test covers empty, receipt-backed, and source-only claims, but never adds a claim with a status in _RETRACTED_CLAIM_STATUSES. A regression could therefore include archived, superseded, or redacted claims in live_claims without failing the test.
🧪 add a retracted-claim case
-from vouch.models import Claim
+from vouch.models import Claim, ClaimStatus
...
assert cov == {"live_claims": 2, "receipted": 1, "ratio": 0.5}
+ store.put_claim(
+ Claim(
+ id="retracted",
+ text="retracted claim",
+ evidence=[src.id],
+ status=ClaimStatus.REDACTED,
+ )
+ )
+ assert health.receipt_coverage(store) == cov
assert health.status(store)["receipt_coverage"] == cov📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_receipt_coverage_fidelity_number(store: KBStore) -> None: | |
| from vouch.extract import ingest_source | |
| from vouch.models import Claim | |
| assert health.receipt_coverage(store) == { | |
| "live_claims": 0, "receipted": 0, "ratio": None, | |
| } | |
| # a receipt-backed claim (cites an Evidence record) | |
| ingest_source( | |
| store, | |
| b"The deploy window opens every wednesday at nine in the morning.", | |
| proposed_by="test", | |
| ) | |
| cov = health.receipt_coverage(store) | |
| assert cov["live_claims"] == cov["receipted"] == 1 | |
| assert cov["ratio"] == 1.0 | |
| # a plain claim citing only a source — no receipt | |
| src = store.put_source(b"unquoted background material") | |
| store.put_claim(Claim(id="plain", text="a plain claim", evidence=[src.id])) | |
| cov = health.receipt_coverage(store) | |
| assert cov == {"live_claims": 2, "receipted": 1, "ratio": 0.5} | |
| assert health.status(store)["receipt_coverage"] == cov | |
| def test_receipt_coverage_fidelity_number(store: KBStore) -> None: | |
| from vouch.extract import ingest_source | |
| from vouch.models import Claim, ClaimStatus | |
| assert health.receipt_coverage(store) == { | |
| "live_claims": 0, "receipted": 0, "ratio": None, | |
| } | |
| # a receipt-backed claim (cites an Evidence record) | |
| ingest_source( | |
| store, | |
| b"The deploy window opens every wednesday at nine in the morning.", | |
| proposed_by="test", | |
| ) | |
| cov = health.receipt_coverage(store) | |
| assert cov["live_claims"] == cov["receipted"] == 1 | |
| assert cov["ratio"] == 1.0 | |
| # a plain claim citing only a source — no receipt | |
| src = store.put_source(b"unquoted background material") | |
| store.put_claim(Claim(id="plain", text="a plain claim", evidence=[src.id])) | |
| cov = health.receipt_coverage(store) | |
| assert cov == {"live_claims": 2, "receipted": 1, "ratio": 0.5} | |
| store.put_claim( | |
| Claim( | |
| id="retracted", | |
| text="retracted claim", | |
| evidence=[src.id], | |
| status=ClaimStatus.REDACTED, | |
| ) | |
| ) | |
| assert health.receipt_coverage(store) == cov | |
| assert health.status(store)["receipt_coverage"] == cov |
🤖 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 `@tests/test_health.py` around lines 409 - 430, Extend
test_receipt_coverage_fidelity_number with a claim whose status is one of the
health module’s _RETRACTED_CLAIM_STATUSES, using the existing claim setup path.
Recalculate and assert receipt_coverage excludes that retracted claim from
live_claims, receipted, and ratio, and keep the status receipt_coverage
assertion aligned with the updated expected result.
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.
this branch is the foundation layer of the beat-ditto plan: measure first, then close the gaps the measurement exposes, without ever trading away the review gate.
the capture side lands three pieces. answer claims are now extracted once per session from the full transcript (61b1358). session pages gain a semantic layer ported from ditto's dream pipeline — one llm pass over the whole session record emits a summary plus typed subjects that become fts-indexed tags — and the page now cites the session-answers source, so it clears the admission gate on the existing citation rule instead of a new exemption (2b8d48e). enrichment-detected value updates resolve onto durable claims and apply lifecycle supersession under the same self-approval conditions capture already honours; a superseded claim leaves every context pack (99bde14).
vouchbench is the measurement: a seeded, judge-free benchmark that ingests generated sessions through the real receipt-gated pipeline and grades retrieval by typed answer keys with forbidden-value zeroing, so over-stuffing a pack loses on decoys and abstention (65f4d9e). the reference baseline at head is composite 0.57 ± 0.04 over seeds 1-6, recorded in the module docstring with the levers annotated (3543b5b).
running the bench against the retrieval stack found two real defects, both fixed with regression tests: whole-day age quantization silently made any sub-day recency half-life a no-op, and the pure fts5/embedding backends never applied recency at all (5192045). retrieval also gains the flywheel: every context-pack build appends a masked, size-rotated, gitignored event record — the dataset a learned ranking stage trains on later, with kb_confirm re-citations as human-verified labels (864d5c0).
two smaller roadmap items ride along: an opt-in pages-first stage that lifts compiled topic pages above raw claims (never session/log pages), and a receipt-coverage fidelity number in status — the share of live claims carrying a byte-offset receipt (1ec183a). the season scaffolding (practice scoring on prs, maintainer-dispatched scored runs with cutoff-derived seeds) is in da4d61c and needs no secrets.
all gates green throughout: full pytest, mypy src (108 files), ruff. the honest caveat is recorded in the bench baseline: knowledge-update scores zero in the llm-free arm by design — supersession needs the enrichment llm, and deterministic lexical collapse was rejected as benchmark-keyed heuristics.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes