Test

[plind-junior]

What changed

Why

What might break

VEP

Tests

• make check passes locally (lint + mypy + pytest)
• New / changed behaviour has a test
• CHANGELOG.md updated under ## [Unreleased]

Summary by CodeRabbit

• New Features
  • Added a Sessions view for browsing, filtering, and opening compiled session summaries.
  • Added personal fallback knowledge-base setup, adoption, gated imports, and hub synchronization.
  • Added benchmark commands, strategy-based retrieval, wiki rendering, and retrieval event visibility.
  • Added proposal rejection inspection and tools for removing obsolete claim references.
  • Added reviewer identity support in the web interface and RPC requests.
• Improvements
  • Enhanced capture, session summaries, provenance, retrieval guidance, and approval workflows.
• Documentation
  • Added competition, strategy, benchmark, seasons, and personal knowledge-base guidance.
• Tests
  • Expanded coverage across CLI, web UI, retrieval, capture, synchronization, and validation.

[coderabbitai]

Walkthrough

This PR adds VouchBench competition infrastructure, retrieval strategies, admission and worthiness checks, personal-KB fallback and adoption, gated federation, session enrichment, dead-reference cleanup, wiki rendering, documentation, and related webapp routes and controls.

Changes

Competition and benchmark infrastructure

Layer / File(s)	Summary
Benchmark scoring and KOTH tooling .github/scripts/*, src/vouch/bench.py, .github/workflows/*, competition/*	Adds deterministic benchmark generation, paired dethrone verdicts, kit validation, strategy scoring, leaderboard updates, season workflows, and competition documentation.
Strategy execution and retrieval integration src/vouch/strategy.py, src/vouch/strategies/*, src/vouch/context.py, contrib/strategies/*	Adds pluggable ranking strategies, sandboxed execution, provenance-aware ranking, candidate ordering, pages-first retrieval, and retrieval telemetry.
Benchmark and strategy validation tests/test_bench.py, tests/test_strategy.py, tests/test_retrieval_backend.py, tests/test_retrieval_events.py	Tests deterministic scoring, statistical verdicts, sandbox restrictions, retrieval ordering, and event logging.

Capture, admission, and federation

Layer / File(s)	Summary
Admission, capture, and enrichment src/vouch/admission.py, src/vouch/capture.py, src/vouch/enrich.py, src/vouch/session_split.py	Adds deterministic admission filtering, session-mode answer capture, enrichment metadata, citation-aware rollups, and supersession updates.
Personal KB fallback and adoption src/vouch/hub.py, src/vouch/adopt.py, src/vouch/cli.py	Adds opt-in personal-KB routing, origin metadata, adoption into project KBs, retirement and dry-run handling, and related CLI commands.
Gated imports and provenance src/vouch/bundle.py, src/vouch/sync.py, src/vouch/hub_client.py, src/vouch/models.py, schemas/*	Adds bundle exclusions, instance identities, proposal-only imports and sync, hub push/pull/status operations, and federated origin fields.
Proposal cleanup and contract surfaces src/vouch/proposals.py, src/vouch/lifecycle.py, src/vouch/server.py, src/vouch/jsonl_server.py, src/vouch/memory_contract.py	Adds dead-claim reference handling, bulk cleanup, gated memory operations, new wire methods, and approval controls.
Capture, federation, and admission tests tests/test_admission.py, tests/test_adopt.py, tests/test_capture*.py, tests/test_gated_import.py, tests/test_sync.py, tests/test_dead_claim_refs.py, tests/test_hub*.py	Covers rejection behavior, fallback routing, adoption, session capture, provenance, proposal-only imports, sync, and dead-reference remediation.

Compilation and webapp

Layer / File(s)	Summary
Compilation and wiki rendering src/vouch/compile.py, src/vouch/wiki_render.py, tests/test_compile.py, tests/test_wiki_render.py	Adds two-phase topic planning, citation-density checks, draft metadata, alias-aware links, and derived wiki index/MOC rendering.
Webapp sessions and reviewer identity webapp/src/App.tsx, webapp/src/components/*, webapp/src/lib/rpc.ts, webapp/src/views/SessionsView.tsx	Adds session browsing, navigation, reviewer identity persistence, RPC headers, and list-envelope compatibility.
Webapp approval remediation and proxying webapp/src/views/PendingView.tsx, webapp/plugins/vouch-proxy.ts, webapp/*test*	Adds batch approval, dead-reference strip-and-retry, KB-wide wipe controls, reviewer-header forwarding, and corresponding tests.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

• vouchbench season 1 — submit ranking code, dethrone the champion, ship as the default #573 — Covers the VouchBench, KOTH season, benchmark, ranking-strategy, and competition infrastructure added here.

Possibly related PRs

• vouchdev/vouch#561 — Adds the VouchBench foundation used by the new scoring scripts and season workflow.
• vouchdev/vouch#557 — Covers the dead-claim-reference approval, cleanup, API, CLI, and UI flows.
• vouchdev/vouch#575 — Adds the same sessions webapp route and SessionsView feature.

Suggested labels: review-ui, openclaw

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is too vague and generic to convey the PR’s main change.	Replace it with a concise, specific summary of the primary change, such as the main feature or area updated.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 11

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (17)

src/vouch/bundle.py-883-903 (1)

883-903: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Inbound lifecycle links are dropped silently.

claim.supersedes, contradicts, and superseded_by are not forwarded to propose_claim, so an inbound claim that retires another arrives with its lifecycle edges stripped. The docstring only calls out pages/entities/relations as deferred — worth mentioning these too (or reporting them alongside deferred) so a reviewer doesn't read the import as lossless.

🤖 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/bundle.py` around lines 883 - 903, Update the inbound claim import
flow around Claim.model_validate and _proposals.propose_claim to forward
claim.supersedes, claim.contradicts, and claim.superseded_by when creating the
proposal. Also update the surrounding docstring or deferred-reporting behavior
to explicitly identify these lifecycle links as deferred or otherwise reported,
without implying the import is lossless.

src/vouch/worthiness.py-210-217 (1)

210-217: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

min_score: 0 is silently rewritten to 0.4.

_as_float(...) or DEFAULT_MIN_SCORE treats a valid 0.0 as absent, so an operator disabling the threshold gets the default back instead. Distinguish "missing" from "zero".

🐛 Proposed fix

+    min_score = _as_float(raw.get("min_score"))
     return WorthinessConfig(
         # YAML 1.1 parses a bare ``off`` as boolean False — coerce it back so
         # ``scorer: off`` disables scoring rather than becoming the string "False".
         scorer=_normalize_scorer(raw.get("scorer", DEFAULT_SCORER)),
-        min_score=_as_float(raw.get("min_score")) or DEFAULT_MIN_SCORE,
+        min_score=DEFAULT_MIN_SCORE if min_score is None else min_score,
         action=str(raw.get("action", DEFAULT_ACTION)),
         apply_to=apply_to,
     )

admission.load_config (Line 107) carries the same pattern; it is harmless today only because its default is 0.0 — worth tightening there as well so the trap doesn't fire the day that default changes.

🤖 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/worthiness.py` around lines 210 - 217, Update the min_score
assignment in WorthinessConfig construction to distinguish a missing value from
a valid 0.0, preserving zero instead of falling back to DEFAULT_MIN_SCORE. Also
tighten the equivalent defaulting logic in admission.load_config so its parsed
zero value is retained independently of the current default.

src/vouch/memory_contract.py-113-125 (1)

113-125: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Unguarded source read can abort a whole search.

_evidence_of tolerates a missing Evidence record, but self.store.read_source_content(ev.source_id) is unprotected — a pruned/absent source blob raises out of _hit and takes down search_memories, fetch_by_id, and search_in_subject. Also note the source bytes are re-read for every evidence of every hit.

🛡️ Proposed guard

+    def _receipt_backed(self, claim: Claim) -> bool:
+        for ev in self._evidence_of(claim):
+            try:
+                content = self.store.read_source_content(ev.source_id)
+            except (ArtifactNotFoundError, OSError):
+                continue
+            if verify_receipt(ev, content).verified:
+                return True
+        return False

then receipt_backed=self._receipt_backed(claim).

🤖 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/memory_contract.py` around lines 113 - 125, Update the MemoryHit
construction in _hit to use a dedicated _receipt_backed(claim) helper. Implement
that helper to safely handle missing or pruned source content while checking
each evidence receipt, treating unreadable sources as unverified, and avoid
rereading the same source blob when multiple evidence records reference it.

tests/test_hub_client.py-134-142 (1)

134-142: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Close the server socket after shutdown().

shutdown() only stops the serve loop; without server_close() the listening socket stays open for the life of the test session, leaking a file descriptor per test using this fixture.

🛡️ Proposed fix

     yield f"http://127.0.0.1:{srv.server_port}"
     srv.shutdown()
+    srv.server_close()
+    t.join(timeout=5)

🤖 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_hub_client.py` around lines 134 - 142, Update the fake_hub fixture
teardown to call server_close() after shutdown(), ensuring the
ThreadingHTTPServer listening socket is released for every test using the
fixture.

src/vouch/hub.py-465-473 (1)

465-473: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Path(entry.path) here skips the expanduser() that personal_entries applies.

A registry row whose path contains ~ passes the live-row filter on line 354 but then fails (root / KB_DIRNAME).is_dir() here, so fallback silently reports off. Same asymmetry exists in the CLI's hub fallback/adopt resolution of entry.path, which is why fixing it at the source is preferable.

🛡️ Proposed fix

-    root = Path(entry.path)
+    root = Path(entry.path).expanduser()

🤖 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/hub.py` around lines 465 - 473, Update the fallback resolution
around personal_entry and the CLI hub fallback/adopt entry-path resolution to
expand user-home syntax before constructing Path objects. Reuse the same
expanduser behavior applied by personal_entries so registry paths containing ~
resolve consistently and existing directory and fallback checks operate on the
expanded path.

.github/scripts/update_leaderboard.py-55-61 (1)

55-61: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

max() over an empty sequence raises when the ledger has no table row yet.

next_row_number already handles the no-table case (returns 0), but append_row then calls max() on an empty generator and dies with an unhandled ValueError traceback — and main never returns 1, so the documented "exit code 1 = error" contract isn't met either. Appending at EOF is the natural fallback.

🛡️ Proposed fix

     lines = text.splitlines(keepends=True)
-    last_row = max(
-        i for i, line in enumerate(lines) if line.startswith("| ")
-    )
-    lines.insert(last_row + 1, row + "\n")
+    row_indices = [i for i, line in enumerate(lines) if line.startswith("| ")]
+    insert_at = row_indices[-1] + 1 if row_indices else len(lines)
+    lines.insert(insert_at, row + "\n")
     ledger.write_text("".join(lines), encoding="utf-8")

🤖 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 @.github/scripts/update_leaderboard.py around lines 55 - 61, Update
append_row to handle ledgers with no lines starting with "| " by appending the
new row at EOF instead of calling max() on an empty generator; preserve the
existing insertion-after-last-row behavior when a table row exists. Ensure main
converts append_row failures into the documented exit code 1 rather than
allowing an unhandled ValueError traceback.

src/vouch/cli.py-5440-5443 (1)

5440-5443: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

A malformed --seeds value raises a bare ValueError traceback.

int(s) on e.g. --seeds 1,x escapes as an unhandled exception rather than a click usage error.

🛡️ Proposed fix

-    seed_list = (
-        [int(s) for s in seeds.replace(" ", "").split(",") if s]
-        if seeds else [seed]
-    )
+    if seeds:
+        try:
+            seed_list = [int(s) for s in seeds.replace(" ", "").split(",") if s]
+        except ValueError as e:
+            raise click.UsageError(f"--seeds must be a comma-separated integer list: {e}") from e
+    else:
+        seed_list = [seed]

🤖 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/cli.py` around lines 5440 - 5443, Update the seed_list parsing in
the CLI command to catch invalid integer values from --seeds and raise a Click
usage error instead of allowing ValueError to escape. Preserve valid
comma-separated seed parsing and the existing fallback to seed when --seeds is
omitted.

tests/test_dead_claim_refs.py-134-160 (1)

134-160: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert the id echo in the JSONL envelope.

Both tests check ok/result/error but not that the response echoes the request id, which is part of the envelope contract these tests exist to pin.

As per coding guidelines, "For each new kb.* method, add a test that asserts the JSONL envelope shape: {id, ok, result} on success and {id, ok: false, error} on failure."

💚 Proposed addition

     resp = handle_request({"id": "r1", "method": "kb.approve",
                            "params": {"proposal_id": pr.id}})
+    assert resp["id"] == "r1"
     assert not resp["ok"]
     assert resp["error"]["code"] == "dead_claim_refs"

🤖 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_dead_claim_refs.py` around lines 134 - 160, Update
test_jsonl_approve_dead_refs_error_code and test_jsonl_wipe_dead_refs to assert
that every handle_request response echoes the corresponding request id: r1 for
the first response and r2 for the second. Preserve the existing success, error,
and result assertions.

Source: Coding guidelines

README.md-132-135 (1)

132-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Blank line inside blockquote (MD028).

The blank line between the two >-prefixed paragraphs can render inconsistently across Markdown renderers. Prefix the blank separator line with > (or merge into one blockquote) to keep rendering consistent.

🤖 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 `@README.md` around lines 132 - 135, Update the blockquote containing the
“Prefer one install for every project?” and “Optionally, a personal catch-all
KB.” paragraphs by prefixing the blank separator line with `>`; preserve both
paragraphs and their content unchanged.

Source: Linters/SAST tools

CHANGELOG.md-9-47 (1)

9-47: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Missing changelog entry for gated federation import / provenance surfacing.

The PR objectives and src/vouch/models.py (ContextItem.origin) plus tests/test_gated_import.py point to a shipped "federated provenance surfaced during retrieval" feature (a claim imported from another KB reports its origin at recall time via pack["origins"]/item["origin"]). This doesn't appear as its own bullet in the shown [Unreleased] section — the closest entry ("shipped ranking champion") is about a ranking algorithm, not the import/origin-tracking feature. As per coding guidelines, "When adding a user-visible feature, update CHANGELOG.md under [Unreleased] in the same PR; do not omit the changelog entry."

🤖 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 9 - 47, Add a separate [Unreleased] changelog
bullet documenting gated federation import and provenance surfacing during
retrieval. Mention that imported claims expose their originating knowledge base
through retrieval results, including pack["origins"] and item["origin"], and
reference the relevant user-visible behavior without conflating it with the
ranking entry.

Source: Coding guidelines

.github/workflows/koth-ledger.yml-58-61 (1)

58-61: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Avoid swallowing PR comment fetch failures

pull-requests: read is the right job permission for reading pull request comment data via the Issues comments endpoint. The concern is the || continue: a permission issue, rate limit, network error, or malformed API response can skip the entire PR’s comment parsing per loop iteration while the workflow still proceeds to “sweep done”. Treat API non-2xx/JSON failures as errors and only skip due to an empty/no-match result set, e.g. by returning a distinct exit code for that case.

🤖 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 @.github/workflows/koth-ledger.yml around lines 58 - 61, Update the PR
comment fetch loop around the gh api invocation so permission, rate-limit,
network, non-2xx, and malformed-response failures stop the workflow instead of
being swallowed by || continue. Preserve skipping only when the query produces
no matching GitHub Actions bot comments, using a distinct no-match status and
handling it separately from API errors before proceeding to “sweep done”.

src/vouch/lifecycle.py-263-291 (1)

263-291: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

scan inline markers independently of frontmatter claims.

Dead ids are discovered only from page.claims / payload["claims"]. A stale [claim: missing-id] marker absent from those lists remains unchanged, contradicting the cleanup contract and leaving an orphaned reference behind.

🤖 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/lifecycle.py` around lines 263 - 291, Update the dead-claim cleanup
flow around the page and proposal processing to scan inline claim markers in
each body independently of frontmatter claims or payload["claims"]. Merge any
marker IDs whose claim paths do not exist into the dead set, then remove them
with strip_claim_markers and persist/index the changed page or proposal while
preserving dry-run behavior.

src/vouch/retrieval_events.py-164-165 (1)

164-165: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

return no records for limit=0.

out[-0:] is equivalent to out[0:], so this returns every event rather than the newest zero events.

proposed fix

-    if limit is not None and limit >= 0:
+    if limit == 0:
+        return []
+    if limit is not None and limit > 0:
         return out[-limit:]

🤖 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 164 - 165, Update the limit
handling in the retrieval function around the `if limit is not None and limit >=
0` condition so `limit=0` returns an empty result instead of slicing with
`out[-0:]`; preserve the existing newest-record behavior for positive limits and
current behavior when the limit is unset.

src/vouch/enrich.py-99-102 (1)

99-102: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

UnicodeDecodeError escapes this loader, breaking enrich_session's "never raises" contract.

read_text(encoding="utf-8") on a non-utf8 config.yaml raises UnicodeDecodeError, which is not caught here and propagates through enrich_session into session_split.summarize → capture.finalize, losing the session rollup. capture.load_config already guards against this.

🛡️ Proposed fix

-    except (OSError, yaml.YAMLError):
+    except (OSError, UnicodeDecodeError, yaml.YAMLError):
         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 - 102, Update the config-loading
try/except in the enrich configuration loader to catch UnicodeDecodeError
alongside OSError and yaml.YAMLError, returning EnrichConfig() for invalid UTF-8
input. Preserve the existing behavior for valid configurations and ensure
enrich_session retains its never-raises contract.

src/vouch/enrich.py-256-257 (1)

256-257: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

updates is capped by max_subjects.

capture.enrich.max_subjects: 1 would silently drop all but the first detected update, and updates drive supersession of durable claims — an unrelated knob shouldn't govern that. Use a dedicated bound.

🐛 Proposed fix

+MAX_UPDATES = 10
+
...
-            if len(updates) == max_subjects:
+            if len(updates) >= MAX_UPDATES:
                 break

🤖 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 256 - 257, Replace the max_subjects-based
stopping condition in the update collection loop with a dedicated bound for
detected updates, so capture.enrich.max_subjects does not truncate updates or
durable-claim supersession; define or reuse the appropriate update-limit
configuration near the surrounding enrichment logic and preserve the existing
subject limit behavior separately.

src/vouch/strategy.py-276-290 (1)

276-290: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

The open-mode check doesn't cover filesystem mutations that never open a file.

os.remove, os.unlink, os.rename, os.replace, os.rmdir, os.mkdir, os.truncate, os.chmod, and os.symlink/os.link each raise their own audit events, so a submission can delete or rename files on the runner while the docstring and error message both claim "filesystem writes are blocked". Cheap to close by adding them to the blocked set (defence in depth, not a boundary — the runner remains the trust root).

🔒️ Proposed change

 _BLOCKED_EXACT = frozenset({
     "os.system",
     "os.exec",
     "os.fork",
     "os.forkpty",
     "os.posix_spawn",
     "os.spawn",
+    "os.remove",
+    "os.rename",
+    "os.rmdir",
+    "os.mkdir",
+    "os.truncate",
+    "os.chmod",
+    "os.symlink",
+    "os.link",
     "subprocess.Popen",

🤖 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/strategy.py` around lines 276 - 290, Extend the audit-event
blocking configuration used by _hook to include filesystem mutation events for
remove/unlink, rename/replace, rmdir/mkdir, truncate, chmod, symlink, and link.
Keep these events covered by the existing blocked_exact check so they raise the
same sandbox PermissionError and preserve the current open-mode and flag checks.

tests/test_codex_rollout.py-195-211 (1)

195-211: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale test name: no longer matches its own assertions.

test_reingest_grown_session_updates_pending_in_place now asserts the exact opposite — the grown turn returns "already-ingested" and the body is not refreshed. The docstring-style comment above it already explains this reversal; only the function name is out of date.

✏️ Suggested rename

-def test_reingest_grown_session_updates_pending_in_place(
+def test_reingest_grown_turn_is_noop_once_admission_rejects(
     store: KBStore, tmp_path: Path
 ) -> None:

🤖 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_codex_rollout.py` around lines 195 - 211, Rename the test function
test_reingest_grown_session_updates_pending_in_place to reflect that
re-ingesting the grown session is skipped as already ingested and does not
refresh the rejected proposal body. Leave the existing assertions and
explanatory comment unchanged.

🧹 Nitpick comments (26)

src/vouch/worthiness.py (1)

33-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate actor set — import admission.AUTO_CAPTURE_ACTORS.

This is byte-identical to admission.py:50-52; two copies will drift the first time a new capture actor is added on one side only.

♻️ Proposed refactor

-AUTO_CAPTURE_ACTORS: frozenset[str] = frozenset({"vouch-capture", "session-split", "codex"})
+from .admission import AUTO_CAPTURE_ACTORS  # tier 1 owns the firehose list

🤖 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/worthiness.py` around lines 33 - 36, Remove the duplicate
AUTO_CAPTURE_ACTORS definition from worthiness.py and import and reuse
admission.AUTO_CAPTURE_ACTORS instead. Update references in the worthiness logic
to resolve through the imported admission symbol, preserving the existing actor
classification behavior.

.github/scripts/koth_score.py (1)

46-50: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Wall-clock budget is 24 × (sessions−1) × 2s ≈ 4 minutes of pure sleep.

With N_SEEDS = 12 and two arms, score() invokes bench.run 24 times, each sleeping SESSION_GAP_SECONDS between session ingests. Worth confirming the ladder job's timeout has headroom, or consider threading the gap only where recency actually matters.

🤖 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 @.github/scripts/koth_score.py around lines 46 - 50, Review the
session-ingest timing in score() and bench.run: with N_SEEDS and both arms, the
current SESSION_GAP_SECONDS produces roughly four minutes of cumulative
wall-clock sleep. Confirm the ladder job timeout has sufficient headroom; if
not, reduce or eliminate the gap for paths where recency ordering is not
required while preserving it where timestamps must be spread.

tests/test_strategy.py (2)

92-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the tmp_path fixture here like the neighbouring tests.

tempfile.TemporaryDirectory duplicates what pytest already provides and makes this the only test in the file with a different temp strategy.

♻️ Proposed change

-def test_sandbox_only_returns_known_ids() -> None:
+def test_sandbox_only_returns_known_ids(tmp_path: Path) -> None:
@@
-    # the child filters to valid ids before returning.
-    import tempfile
-
-    with tempfile.TemporaryDirectory() as d:
-        path = _write(Path(d), body)
-        assert run_sandboxed(path, "q", CANDS, limit=10) == ["a", "c"]
+    # the child filters to valid ids before returning.
+    assert run_sandboxed(_write(tmp_path, body), "q", CANDS, limit=10) == ["a", "c"]

🤖 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_strategy.py` around lines 92 - 103, Update
test_sandbox_only_returns_known_ids to use pytest’s tmp_path fixture instead of
tempfile.TemporaryDirectory, passing the fixture’s directory to _write and
removing the temporary-directory import and context manager while preserving the
existing assertion.

---

211-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

String-replace config edit is brittle.

text.replace('strategy: vouch.strategies.provenance', 'strategy: null') silently no-ops if the starter config ever quotes or re-orders that key, and the assertion would then pass for the wrong reason. Prefer parsing the YAML, setting retrieval.strategy = None, and dumping it back.

🤖 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_strategy.py` around lines 211 - 224, The test’s configuration
update uses a brittle string replacement that can silently fail. In
test_configured_strategy_defaults_and_opt_out, parse store.config_path as YAML,
set the retrieval.strategy value to None, and dump the modified configuration
back before asserting _configured_strategy(store) is None.

tests/test_retrieval_backend.py (1)

478-484: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Recency and pages_first tests don't opt out of the shipped strategy.

_set_rerank disables retrieval.strategy precisely because the shipped champion is final-say, but _set_pages_first and _set_recency leave it on — so these ordering assertions also depend on the provenance blend (and on the strategy pool over-fetch) not inverting the boost. Mirroring the opt-out here keeps each stage's test isolated to that stage.

♻️ Proposed change

     cfg.setdefault("retrieval", {})["pages_first"] = pf_cfg
+    # isolate this stage: the shipped champion strategy is final-say.
+    cfg["retrieval"]["strategy"] = None
     store.config_path.write_text(yaml.safe_dump(cfg))

🤖 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_retrieval_backend.py` around lines 478 - 484, Update
_set_pages_first and _set_recency to disable retrieval.strategy when configuring
their respective test settings, matching _set_rerank. Keep each helper’s
existing pages_first or recency configuration intact so ordering assertions
isolate the targeted stage.

src/vouch/memory_contract.py (2)

107-111: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

search_in_subject can under-return silently.

Filtering a max(limit * 5, 25) pool by subject means a subject with many low-ranked memories returns fewer than limit even though more exist. Worth documenting the approximation (or looping until the pool is exhausted) so a contract-scored run isn't penalised for a harness artifact.

🤖 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/memory_contract.py` around lines 107 - 111, Update
search_in_subject to avoid silently under-returning: continue fetching and
filtering broader search results until limit subject matches are found or the
search pool is exhausted. Preserve ranking order and the existing limit
behavior, and document the approximation if the underlying search API cannot
support reliable pagination.

---

41-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

"Not a live claim" is now expressed three ways. The root cause is that the retracted/retired status set has no shared public home, so each new consumer either copies the literal or reaches into a retrieval-module private.

• src/vouch/memory_contract.py#L41-L45: drop the local _RETRACTED literal and consume the shared constant instead.
• src/vouch/health.py#L84-L90: replace the local from .context import _RETRACTED_CLAIM_STATUSES with an import of that shared public constant (e.g. exported from models.py), so health no longer depends on a private retrieval symbol.

🤖 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/memory_contract.py` around lines 41 - 45, Define or reuse one
shared public constant for retracted claim statuses, such as in models.py, and
update src/vouch/memory_contract.py lines 41-45 to consume it instead of
defining _RETRACTED. Update src/vouch/health.py lines 84-90 to import that
public constant rather than the private context symbol
_RETRACTED_CLAIM_STATUSES; preserve existing status-check behavior at both
sites.

contrib/strategies/provenance_rank.py (1)

32-40: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Docstring overstates the hearsay guard.

With re.IGNORECASE, [a-z][a-z0-9-]* matches any leading word — pronouns included — so "i told bob his key rotated" is demoted as hearsay despite being first-hand. Either tighten the pattern (e.g. exclude first-person pronouns) or drop the "leading name-like token" claim.

🤖 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 `@contrib/strategies/provenance_rank.py` around lines 32 - 40, Update the
_HEARSAY pattern or its accompanying documentation so the implementation matches
the stated first-hand safeguard: exclude first-person pronouns from the leading
token pattern, preserving detection of named third-party speech while leaving
phrases such as “i told bob his key rotated” untouched.

src/vouch/health.py (1)

61-61: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

status() now scans claims and evidence twice.

receipt_coverage(store) walks every claim and evidence YAML, and lines 62/67 walk them again for the counts. For a "quick, no deep checks" summary consider computing the lists once and passing them in (or deriving the counts from the coverage pass).

♻️ Sketch

-        "receipt_coverage": receipt_coverage(store),
-        "claims": len(store.list_claims()),
+        "receipt_coverage": coverage,
+        "claims": len(claims),

with claims = store.list_claims() / evidence = store.list_evidence() hoisted and receipt_coverage accepting them.

🤖 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` at line 61, Update status() and receipt_coverage() to
avoid rescanning claims and evidence: hoist store.list_claims() and
store.list_evidence() into status(), pass the resulting collections into
receipt_coverage(), and derive the existing counts from those same collections.
Preserve the current coverage and summary behavior while ensuring each YAML
listing occurs only once.

src/vouch/hooks.py (1)

289-370: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Gated path searches on stripped tokens while legacy searches the raw prompt.

_gated_injection queries " ".join(_informative_tokens(prompt)) but _legacy_injection still passes prompt, so the two modes can return different item sets for the same turn. Intentional per the docstring, but worth a note in the module docstring (or applying the token filter to both) so a future comparison of gated vs legacy recall isn't confounded.

🤖 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/hooks.py` around lines 289 - 370, The gated and legacy injection
paths use different retrieval queries, with _gated_injection filtering prompt
tokens while _legacy_injection searches the raw prompt. Apply
_informative_tokens consistently to both paths, or document this intentional
difference in the module docstring; preserve each path’s existing injection
behavior and ensure comparisons are not misleading.

src/vouch/context.py (1)

210-263: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Each optional stage re-reads and re-parses config.yaml.

_configured_pages_first joins _configured_recency, _configured_strategy (called twice per pack), and _configured_backend — one build_context_pack now does five-plus reads/parses of the same file on every query path. Consider loading the config once per call and threading the parsed mapping into the stage resolvers.

🤖 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 210 - 263, Update build_context_pack and
the optional-stage resolvers, including _configured_pages_first,
_configured_recency, _configured_strategy, and _configured_backend, to load and
parse config.yaml once per context-pack call. Thread the parsed configuration
mapping through each resolver, including both _configured_strategy calls, while
preserving existing defaults and behavior for invalid or missing configuration.

.github/scripts/score_strategy.py (1)

39-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider surfacing SandboxProxy failure counts in the report.

SandboxProxy.rank swallows sandbox failures and returns [] (no reordering) while incrementing failures. If the champion's sandbox child fails for environmental reasons, its scores silently degrade to raw backend order and the verdict can read "dethroned" for the wrong reason. Recording proxy.failures per arm in the JSON makes that visible to the human reviewer.

♻️ Proposed change

-def score(path: str, seeds: list[int]) -> list[float]:
+def score(path: str, seeds: list[int]) -> tuple[list[float], int]:
     proxy = SandboxProxy(path)
-    return [
+    scores = [
         bench.run(s, strategy=proxy, session_gap_seconds=SESSION_GAP_SECONDS)[
             "composite"
         ]
         for s in seeds
     ]
+    return scores, proxy.failures

with main() unpacking both and adding "sandbox_failures": {"champion": ..., "challenger": ...} to report.

🤖 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 @.github/scripts/score_strategy.py around lines 39 - 46, Update score to
return both the computed composite scores and the associated SandboxProxy
failure count, then have main() unpack these values for each arm and add a
sandbox_failures object to report with champion and challenger counts. Preserve
the existing scoring behavior while making failures visible in the generated
JSON.

tests/test_hooks.py (1)

287-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Config-mutation helpers are near-duplicates.

Three helpers plus the inline block at lines 443-447 each re-import yaml and repeat the load/mutate/dump dance. A single _patch_retrieval(store, **keys) helper (module-level import yaml) would collapse them.

Also applies to: 340-347, 443-447

🤖 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_hooks.py` around lines 287 - 301, The config mutation helpers and
inline retrieval update duplicate YAML load/write logic. Add a module-level yaml
import and introduce a shared _patch_retrieval(store, **keys) helper that loads
the configuration, updates retrieval with the supplied keys, and writes it back;
refactor _enable_gate, _disable_gate, the helper around lines 340-347, and the
inline block around lines 443-447 to use it while preserving their existing
mutations.

.github/scripts/update_leaderboard.py (1)

32-42: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

format_row will raise KeyError on a report missing challenger/mean_diff.

lane/date are read defensively but the two numeric fields aren't, so a malformed scorecard produces a traceback in CI rather than the documented error exit. Low priority given both scorers emit the same shape.

🤖 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 @.github/scripts/update_leaderboard.py around lines 32 - 42, Update
format_row to read challenger.mean and mean_diff defensively, matching the
existing lane/date handling for malformed reports. Ensure missing numeric fields
follow the script’s documented error-exit behavior instead of raising an
uncaught KeyError, while preserving normal formatting for valid scorecards.

tests/test_update_leaderboard.py (1)

44-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the table-less ledger path.

next_row_number is tested for "no table yet", but append_row isn't — which is exactly the input that currently raises from max() in .github/scripts/update_leaderboard.py (flagged there). A test on that input would pin the fallback behaviour.

🤖 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_update_leaderboard.py` around lines 44 - 62, Add a test for
append_row using a ledger with no table yet, covering the fallback row-number
behavior that avoids the max() failure in next_row_number. Assert the call
succeeds and verify the generated row is written with the expected initial
numbering and placement.

tests/test_capture.py (1)

1019-1022: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This assertion can pass vacuously.

If the sources/*/meta.yaml glob doesn't match the actual on-disk layout, the left operand is always true and the fallback-off guarantee is never checked. Assert the invariant directly instead.

♻️ Proposed change

-    assert not list((personal.kb_dir / "sources").glob("*/meta.yaml")) or all(
-        "origin_path" not in s.metadata for s in personal.list_sources()
-    )
+    assert all("origin_path" not in s.metadata for s in personal.list_sources())

🤖 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_capture.py` around lines 1019 - 1022, Update the assertions in the
capture test to validate the fallback-off invariant directly, rather than
conditionally skipping it when the sources glob is empty. Remove the vacuous
filesystem-glob condition and assert that every source returned by
personal.list_sources() lacks "origin_path" in its metadata, while preserving
the existing empty output assertion.

src/vouch/cli.py (1)

1635-1666: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Detecting the dead-ref case by substring-matching check_approvable's message is brittle.

proposals.DeadClaimRefsError already exists as the typed signal for exactly this case (see src/vouch/proposals.py lines 41-59). Keying the branch on "references unknown claim" in why means any reword of that validator message silently turns the strip-and-approve path back into a hard abort. Consider having check_approvable return a structured reason (or call missing_claim_refs unconditionally for page proposals) instead of parsing prose.

🤖 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/cli.py` around lines 1635 - 1666, The dead-reference branch in the
batch approval flow should not depend on the prose returned by check_approvable.
Update check_approvable and its caller to propagate or inspect the typed
proposals.DeadClaimRefsError, or otherwise use the structured dead-reference
result, then pass those proposal IDs through missing_claim_refs and the existing
drop_missing_claims prompt while preserving other approval blockers.

src/vouch/adopt.py (2)

277-284: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Bare except Exception in retire loop swallows all failures silently.

Archiving is intentionally best-effort so a failure shouldn't abort adoption, but catching bare Exception with no logging means a genuine bug in lifecycle.archive (not just an expected "can't archive this" case) disappears with zero diagnostic trail — report.retired just quietly ends up shorter than expected.

🩹 Suggested minimal logging

     if retire:
         for claim_id in landed_durable:
             try:
                 lifecycle.archive(personal, claim_id=claim_id, actor=actor)
-            except Exception:
+            except Exception as exc:
                 # Retiring is best-effort tidying of the personal KB; a claim
                 # that cannot be archived must not fail the adoption.
+                logger.warning("adopt: failed to retire %s: %s", claim_id, exc)
                 continue
             report.retired.append(claim_id)

🤖 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/adopt.py` around lines 277 - 284, Update the retire loop in the
adoption flow around lifecycle.archive to log the caught exception with claim_id
and sufficient context before continuing. Preserve best-effort behavior so
archive failures do not abort adoption, while ensuring unexpected failures leave
a diagnostic trail.

---

166-182: 🎯 Functional Correctness | 🔵 Trivial | ⚖️ Poor tradeoff

Dry-run preview doesn't model duplicate-content rejection.

dry_run assumes receipt is not None and gate_open always lands durable, but the real path (lines 240-258) can still reject as "a duplicate of an already-durable claim" (same content, different id) and fall through to claims_skipped instead. Given the explicit design goal in the CLI comment that "a dry run that promises durable claims a closed gate will leave pending is worse than no preview at all," this specific edge case (already-durable-by-content-but-different-id) can still make --dry-run over-promise a durable landing that the real run skips.

🤖 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/adopt.py` around lines 166 - 182, Update the dry-run branch in the
adoption flow to model duplicate-content rejection consistently with the real
path: before appending a claim to claims_durable, detect whether its content
already matches an existing durable claim even when the IDs differ, and append
it to claims_skipped instead. Reuse the existing duplicate-detection helper or
logic from the real processing path near the durable-claim handling, while
preserving the current ID and queued-claim checks.

.github/workflows/vouchbench-season.yml (1)

37-41: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add an explicit least-privilege permissions: block.

This job never comments, pushes, or writes anything back to the repo — only benefits from an explicit permissions: contents: read (workflow- or job-level) instead of inheriting the repo/org default, which could be broader.

🔒 Suggested least-privilege permissions

 name: vouchbench-season
 
+permissions:
+  contents: read
+
 on:

🤖 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 @.github/workflows/vouchbench-season.yml around lines 37 - 41, Add an
explicit least-privilege permissions block for the score job, granting only
contents read access. Place it at workflow or job scope near the jobs/score
configuration, without changing the existing execution steps.

tests/test_koth_kit.py (1)

8-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Filename doesn't mirror the module under test.

This file exercises .github/scripts/validate_kit.py; the strict convention would name it tests/test_validate_kit.py.

As per coding guidelines, "Tests must mirror module names using the strict tests/test_<module>.py convention."

🤖 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_koth_kit.py` around lines 8 - 15, Rename the test file from
test_koth_kit.py to test_validate_kit.py so it follows the strict
tests/test_<module>.py convention and mirrors the validate_kit module loaded by
the test.

Source: Coding guidelines

tests/test_enrich.py (1)

79-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a parsing test for updates.

Update entries feed apply_enrich_updates, which supersedes durable claims, but no test here exercises the defensive rules in parse_enrichment: non-string attribute/old/new skipped, blank values skipped, and old.lower() == new.lower() dropped. Only the happy path is covered indirectly via tests/test_capture_answer.py.

As per coding guidelines, "Run and preserve the project test suite, including capability parity tests and feature tests for new methods."

🤖 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 a focused test for the
updates parsing behavior in parse_enrichment, covering valid updates plus
skipping entries with non-string attribute/old/new values, blank values, and
case-insensitively equal old and new values. Assert only valid, meaningful
updates remain and preserve the existing test suite.

Source: Coding guidelines

src/vouch/capture.py (2)

570-579: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log session answer-capture failures.

capture.py swallows all capture_session_answers() exceptions with only _answer_skip(..., "error"), so the summary path always succeeds without making the failure diagnosable. Add a module logger warning around this block with exc_info=True.

♻️ Proposed change

-        except Exception:
-            answers = _answer_skip(session_id, "error")
+        except Exception:
+            logger.warning(
+                "capture: session answer extraction failed for %s", session_id,
+                exc_info=True,
+            )
+            answers = _answer_skip(session_id, "error")

🤖 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 exception handler
around capture_session_answers in the session-answer path to emit a module
logger warning with exc_info=True before returning _answer_skip(session_id,
"error"). Preserve the existing fallback behavior and source handling.

---

825-833: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use proposals._receipts_auto_approve(store) instead of duplicating the self-approval check.

apply_enrich_updates re-implements the trusted-agent/auto-receipt gate via private _review_config, while propagates._receipts_auto_approve() already exposes this predicate in adopt.py. Reusing that helper avoids a parallel write path diverging from the gate decision used by the rest of capture.py.

🤖 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 825 - 833, The apply_enrich_updates gate
currently duplicates the approval predicate through review_cfg and
_review_config. Replace that logic with proposals._receipts_auto_approve(store),
preserving the existing gate-closed return behavior and leaving the outcomes
handling unchanged.

Source: Path instructions

src/vouch/strategy.py (1)

235-256: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Surface why the strategy sandbox failed — failure modes currently all become None.

Timeouts, startup errors, nonzero exits, and malformed output are indistinguishable, and proc.stderr is thrown away. Since -I+env={"PATH": ...} depends on vouch being importable from site-packages, an editable checkout without .pth import paths makes python -m vouch.strategy --child fail silently and treat every strategy as “no reordering”. Logging a truncated stderr plus returncode keeps the non-abort behavior while making the real failure diagnosable.

🤖 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/strategy.py` around lines 235 - 256, Update the subprocess handling
in the strategy sandbox flow to log diagnosable failure details while still
returning None: distinguish timeout, startup OSError, nonzero returncode, and
malformed output; include a safely truncated proc.stderr and returncode where
available. Preserve the non-abort behavior, and ensure the child-module launch
remains usable for editable checkouts by providing the necessary import path in
the environment or invocation.

src/vouch/compile.py (1)

205-237: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Phase-A prompt doesn't exclude already-taken topics.

build_topic_prompt (unlike build_prompt) never inlines store.list_pages() / _pending_page_names(store), so phase A can plan a topic that already exists as a page or pending proposal. Phase B's prompt then simultaneously tells the model to "draft exactly one focused page" for that planned topic (line 303-306) and to not redraft anything in "TAKEN TOPICS" (line 316) — contradictory instructions that, at best, waste one of the two LLM calls two-phase mode is supposed to spend wisely, since _draft_problem's collision guard (line 390-391) will drop the draft anyway.

♻️ Suggested fix: give phase A the same taken-topics context as phase B

 def build_topic_prompt(store: KBStore, *, max_pages: int) -> str:
     claims = [
         c for c in store.list_claims()
         if c.status not in _RETRACTED_CLAIM_STATUSES
     ]
     if not claims:
         raise CompileError("nothing to compile: the KB has no live approved claims")
+    pages = store.list_pages()
+    pending = _pending_page_names(store)
     lines = [
         "You are planning a project's knowledge wiki. Group the approved",
         "claims below into a small set of durable, single-subject topics —",
         "the pages a future reader browses first.",
         "",
         "APPROVED CLAIMS (id: text):",
     ]
     for c in claims:
         lines.append(f"- {c.id}: {c.text}")
+    taken = [p.title for p in pages] + sorted(pending)
+    if taken:
+        lines += ["", "ALREADY COVERED — do not propose these as new topics:"]
+        lines += [f"- {t}" for t in taken]
     lines += [
         "",
         "RULES",

🤖 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/compile.py` around lines 205 - 237, Update build_topic_prompt to
include the existing and pending page topics, using store.list_pages() and
_pending_page_names(store) consistently with build_prompt. Add this taken-topics
context and instruct phase A to exclude those topics from its JSON plan, while
preserving the existing max_pages and single-subject topic rules.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4af4a156-dfd9-4a0b-bd38-169ed0f83867

📥 Commits

Reviewing files that changed from the base of the PR and between cabdf5b and 1fe8fb5.

📒 Files selected for processing (95)

• .github/scripts/koth_score.py
• .github/scripts/score_strategy.py
• .github/scripts/update_leaderboard.py
• .github/scripts/validate_kit.py
• .github/workflows/koth-ledger.yml
• .github/workflows/vouchbench-season.yml
• CHANGELOG.md
• CONTRIBUTING.md
• README.md
• adapters/claude-code/README.md
• competition/LEADERBOARD.md
• competition/kits/current/kit.yaml
• contrib/strategies/README.md
• contrib/strategies/baseline.py
• contrib/strategies/example_lexical.py
• contrib/strategies/provenance_rank.py
• docs/koth-ladder.md
• docs/koth-strategy-lane.md
• docs/mining-on-vouch.md
• docs/vouchbench-seasons.md
• schemas/context-item.schema.json
• schemas/context-pack.schema.json
• src/vouch/admission.py
• src/vouch/adopt.py
• src/vouch/bench.py
• src/vouch/bundle.py
• src/vouch/capabilities.py
• src/vouch/capture.py
• src/vouch/cli.py
• src/vouch/compile.py
• src/vouch/context.py
• src/vouch/enrich.py
• src/vouch/health.py
• src/vouch/hooks.py
• src/vouch/hot_memory.py
• src/vouch/hub.py
• src/vouch/hub_client.py
• src/vouch/jsonl_server.py
• src/vouch/lifecycle.py
• src/vouch/memory_contract.py
• src/vouch/models.py
• src/vouch/proposals.py
• src/vouch/recall.py
• src/vouch/retrieval_events.py
• src/vouch/server.py
• src/vouch/session_split.py
• src/vouch/storage.py
• src/vouch/strategies/__init__.py
• src/vouch/strategies/provenance.py
• src/vouch/strategy.py
• src/vouch/sync.py
• src/vouch/wiki_render.py
• src/vouch/worthiness.py
• tests/test_admission.py
• tests/test_adopt.py
• tests/test_bench.py
• tests/test_bundle.py
• tests/test_capture.py
• tests/test_capture_answer.py
• tests/test_capture_scope.py
• tests/test_cli.py
• tests/test_codex_rollout.py
• tests/test_compile.py
• tests/test_dead_claim_refs.py
• tests/test_enrich.py
• tests/test_gated_import.py
• tests/test_health.py
• tests/test_hooks.py
• tests/test_hub.py
• tests/test_hub_client.py
• tests/test_install_adapter.py
• tests/test_koth_kit.py
• tests/test_memory_contract.py
• tests/test_prompt_gate_live.py
• tests/test_retrieval_backend.py
• tests/test_retrieval_events.py
• tests/test_session_split.py
• tests/test_strategy.py
• tests/test_sync.py
• tests/test_update_leaderboard.py
• tests/test_wiki_render.py
• tests/test_worthiness.py
• webapp/plugins/vouch-proxy.test.ts
• webapp/plugins/vouch-proxy.ts
• webapp/src/App.tsx
• webapp/src/components/Shell.test.tsx
• webapp/src/components/Shell.tsx
• webapp/src/lib/rpc.test.ts
• webapp/src/lib/rpc.ts
• webapp/src/views/PendingView.deadrefs.test.tsx
• webapp/src/views/PendingView.multi.test.tsx
• webapp/src/views/PendingView.test.tsx
• webapp/src/views/PendingView.tsx
• webapp/src/views/SessionsView.test.tsx
• webapp/src/views/SessionsView.tsx

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repository files =="
git ls-files '.github/workflows/*' | sed -n '1,120p'

echo
echo "== target workflow =="
cat -n .github/workflows/koth-ledger.yml | sed -n '1,130p'

echo
echo "== search for koth-ledger references and workflow actions =="
rg -n "koth-|LEADERBOARD|dethroned|comments|update_leaderboard|GITHUB_TOKEN|pull-requests: write|issues: write|secrets\.KOTH|permissions:" .github/workflows -S || true

Repository: vouchdev/vouch

Length of output: 11676

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant gate workflow snippets =="
sed -n '1,220p' .github/workflows/koth-gate.yml
echo
sed -n '1,150p' .github/workflows/koth-engine-gate.yml

echo
echo "== scripts that parse leaderboard/report =="
fd -a 'update_leaderboard.py' . | while read -r f; do
  echo "--- ${f#"$PWD/"}"
  cat -n "$f" | sed -n '1,240p'
done

echo
echo "== comments related to dethroned/source of truth =="
rg -n "dethroned|\"dethroned|LEADERBOARD|scorecard|verdict=dethroned|LEADERBOARD.md|report.json|koth-ledger|github-actions\\[bot\\]" .github -S || true

echo
echo "== changed files/stat =="
git diff --stat || true
git diff -- .github/workflows/koth-ledger.yml | sed -n '1,220p' || true

Repository: vouchdev/vouch

Length of output: 21396

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== update_leaderboard.py implementation =="
fd -a 'update_leaderboard.py' .github/scripts | sort | while read -r f; do
  echo "--- ${f#"$PWD/"}"
  cat -n "$f" | sed -n '1,260p'
done

echo
echo "== other workflows with issue/pull-request write and comment creation =="
python3 - <<'PY'
from pathlib import Path
import re

patterns = {
    "pull_request_target_or_pr_event": re.compile(r"^\s+pull_request(_target)?:\s*$|^\s+pull_request:", re.M),
    "issue_or_pr_write": re.compile(r"^\s+(issues|pull-requests|issues\s*,\s*pull-requests):\s*(write|read,?\s*write)", re.M),
    "gh_(cli_)?comment": re.compile(r"\b(g\s*gh\s+|gh\s+)(?:[Aa].*\bcomment\b|pr\s+comment\b|issue\s+comment\b)", re.S),
}
for p in sorted(Path(".github/workflows").glob("*.yml")):
    text = p.read_text()
    print(f"\n--- {p} ---")
    for name,pattern in patterns.items():
        m = pattern.search(text)
        if m:
            start = min(text.count("\n", 0, m.start())+1, len(text.splitlines()))
            print(f"{name}: line {start}; text:\n{m.group(0)!r}")
PY

Repository: vouchdev/vouch

Length of output: 17327

---

Anchor the ledger scorecard to the known scoring run, not just the bot login.

user.login == "github-actions[bot]" also covers comments from any same-repo GITHUB_TOKEN workflow with pull-requests: write; another koth-* gate scorecard containing the same ```json + "dethroned": true shape can reach the ledger unchanged. Anchor trust to workflow-specific metadata, e.g. comment authorship tied to a successful koth-gate.yml run/SHA plus a separate secret/HMAC signing the report.

🤖 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 @.github/workflows/koth-ledger.yml around lines 56 - 72, Update the
comment-filtering and report-selection logic around the GitHub API query and
Python extraction so the ledger accepts scorecards only from the specific
successful koth-gate.yml run and commit SHA, not merely from
github-actions[bot]. Validate the report with workflow-specific metadata and the
configured secret/HMAC signature before writing /tmp/report.json, rejecting
same-shaped reports from other workflows.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

sessions < 2 hangs or crashes; validate the floor.

spot(exclude=home) spins forever when sessions == 1 (every draw equals exclude), and rng.randrange(sessions - 1) at Lines 298 and 363 raises ValueError on randrange(0). sessions is a public kwarg on generate/run/run_seeds, so a caller passing 1 gets an infinite loop rather than a clear error.

🐛 Proposed guard

     rng = random.Random(seed)
+    if sessions < 2:
+        raise ValueError(f"sessions must be >= 2 (the update cases need two), got {sessions}")
     attrs = list(_ATTRIBUTES)

📝 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.
	"""
	rng = random.Random(seed)
	if sessions < 2:
	raise ValueError(f"sessions must be >= 2 (the update cases need two), got {sessions}")
	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] 256-256: 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 250 - 267, Validate that the public sessions
parameter is at least 2 at the start of generate, rejecting smaller values with
a clear ValueError before constructing _Sessions or invoking spot. Ensure the
validation applies through the existing generate/run/run_seeds call path and
prevents both the spot infinite loop and randrange(0) failures.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Re-importing an overlapping bundle aborts the whole import.

store.put_evidence raises ValueError when the evidence id already exists (src/vouch/storage.py:990-1001). hub_client.pull calls import_as_proposals on every pull, so the second pull of a bundle that re-ships already-registered evidence dies here — after put_source has already run and before the audit event is written, leaving a partial, unlogged import. put_source is idempotent; put_evidence is not.

🛡️ Proposed fix

             if m.name.startswith("evidence/") and m.name.endswith(".yaml"):
                 ev = Evidence.model_validate(
                     yaml.safe_load(tar.extractfile(m).read())  # type: ignore[union-attr]
                 )
-                store.put_evidence(ev)
-                evidence_registered += 1
+                try:
+                    store.get_evidence(ev.id)
+                except ArtifactNotFoundError:
+                    store.put_evidence(ev)
+                    evidence_registered += 1

ArtifactNotFoundError comes from .storage; add it to the local import at Line 938.

📝 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

	for m in members:
	if m.name.startswith("evidence/") and m.name.endswith(".yaml"):
	ev = Evidence.model_validate(
	yaml.safe_load(tar.extractfile(m).read()) # type: ignore[union-attr]
	)
	store.put_evidence(ev)
	evidence_registered += 1
	for m in members:
	if m.name.startswith("evidence/") and m.name.endswith(".yaml"):
	ev = Evidence.model_validate(
	yaml.safe_load(tar.extractfile(m).read()) # type: ignore[union-attr]
	)
	try:
	store.get_evidence(ev.id)
	except ArtifactNotFoundError:
	store.put_evidence(ev)
	evidence_registered += 1

🤖 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/bundle.py` around lines 966 - 972, Update the evidence import loop
in import_as_proposals to catch the existing-evidence condition from
store.put_evidence and continue processing overlapping bundle members instead of
aborting the import; import and use ArtifactNotFoundError from .storage as the
requested local exception handling dependency, while preserving
evidence_registered updates only for newly stored evidence.

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Token is written world-readable before the chmod lands.

write_text creates the file under the process umask (typically 0644), so the bearer token exists on disk readable by other local users until chmod(0o600) runs. Create it restricted instead.

🔒 Proposed fix

     data.setdefault("tokens", {})[url.rstrip("/")] = token
-    path.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8")
-    path.chmod(0o600)
+    fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+    with os.fdopen(fd, "w", encoding="utf-8") as f:
+        f.write(yaml.safe_dump(data, sort_keys=False))
+    # existing files keep their old mode through O_CREAT; tighten explicitly.
+    path.chmod(0o600)

📝 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 save_token(url: str, token: str) -> None:
	path = _creds_path()
	path.parent.mkdir(parents=True, exist_ok=True)
	data: dict[str, Any] = {}
	if path.exists():
	data = yaml.safe_load(path.read_text(encoding="utf-8")) or {}
	data.setdefault("tokens", {})[url.rstrip("/")] = token
	path.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8")
	path.chmod(0o600)
	def save_token(url: str, token: str) -> None:
	path = _creds_path()
	path.parent.mkdir(parents=True, exist_ok=True)
	data: dict[str, Any] = {}
	if path.exists():
	data = yaml.safe_load(path.read_text(encoding="utf-8")) or {}
	data.setdefault("tokens", {})[url.rstrip("/")] = token
	fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
	with os.fdopen(fd, "w", encoding="utf-8") as f:
	f.write(yaml.safe_dump(data, sort_keys=False))
	# existing files keep their old mode through o_creat; tighten explicitly.
	path.chmod(0o600)

🤖 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/hub_client.py` around lines 107 - 115, Update save_token to create
or open the credentials file with restrictive 0o600 permissions before writing
its contents, rather than relying on write_text followed by chmod. Preserve the
existing YAML data handling and token storage behavior, and retain chmod as a
final permission safeguard if appropriate.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Validate the regex-edited text before writing — a mismatched-indent config can be corrupted.

The elif isinstance(personal, dict) branch inserts fallback_capture at a hardcoded 2-space indent under personal:. If the existing block's children are indented differently (e.g. 4 spaces), the rewritten file is no longer parseable YAML, and the write happens unconditionally. Similarly, the ^(\s*fallback_capture:\s*) pattern is matched against the whole file, so a same-named key in another block would be rewritten instead. Cheapest robust guard: re-parse the candidate text and confirm it yields the expected value, else fall through to the structural rewrite that already exists below.

🛡️ Proposed fix

+    def _verified(candidate: str) -> bool:
+        try:
+            parsed = yaml.safe_load(candidate)
+        except yaml.YAMLError:
+            return False
+        block = parsed.get("personal") if isinstance(parsed, dict) else None
+        return isinstance(block, dict) and block.get("fallback_capture") is enabled
+
     if isinstance(personal, dict) and "fallback_capture" in personal:
         new_text, n = re.subn(
             r"(?m)^(\s*fallback_capture:\s*).*$",
             rf"\g<1>{value}",
             text,
             count=1,
         )
-        if n == 1:
+        if n == 1 and _verified(new_text):
             cfg_path.write_text(new_text, encoding="utf-8")
             return
     elif isinstance(personal, dict):
         new_text, n = re.subn(
             r"(?m)^personal:[ \t]*$",
             f"personal:\n  fallback_capture: {value}",
             text,
             count=1,
         )
-        if n == 1:
+        if n == 1 and _verified(new_text):
             cfg_path.write_text(new_text, encoding="utf-8")
             return

🤖 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/hub.py` around lines 424 - 443, In the fallback_capture update
logic, validate the regex-produced candidate YAML before cfg_path.write_text:
re-parse it and confirm the personal.fallback_capture value matches the
requested value, otherwise do not write it. Scope the replacement to the
fallback_capture key within the personal block rather than any same-named key
elsewhere, and derive insertion indentation from the existing personal block
instead of hardcoding two spaces; preserve the existing structural rewrite
fallback for unsuccessful validation.

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

route this cleanup through the review gate.

This new MCP/JSONL/CLI write path rewrites durable pages and pending proposals directly, without proposals.approve(). An audit event does not replace reviewer approval; generate reviewed cleanup proposals or otherwise route the changes through the established approval path.

As per coding guidelines, “All writes must pass through the reviewed proposal approval path, specifically proposals.approve(); do not add parallel data paths that bypass the review gate.”

🤖 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/lifecycle.py` around lines 269 - 292, The cleanup logic in the page
and pending-proposal handling directly persists changes through
store.update_page and store.update_proposal, bypassing the reviewed approval
gate. Refactor this path to generate reviewed cleanup proposals and apply
changes exclusively through proposals.approve(), preserving the existing dry-run
and dead-claim cleanup behavior without introducing a parallel write path.

Sources: Coding guidelines, Path instructions

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

avoid unaudited partial cleanup.

Pages and proposals are rewritten incrementally, but the authoritative audit event is emitted only after every write succeeds. A later write or reindex failure leaves earlier durable changes applied with no audit record. Stage the full change set and use a recoverable/transactional commit strategy that records every applied mutation.

As per path instructions, “Audit log (audit.log.jsonl) is authoritative; never edit decided/ or audit JSONL by hand.”

🤖 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/lifecycle.py` around lines 269 - 304, Update the cleanup flow
containing the page rewrites, proposal updates, and final page.dead_refs_wipe
audit event to avoid applying mutations incrementally before auditing. Stage the
complete page and proposal change set, then commit it through a recoverable or
transactional strategy that records every mutation in the authoritative audit
log, including partial-failure recovery; do not modify decided/ or audit JSONL
directly.

Source: Path instructions

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Unlocked read-mint-write: concurrent callers mint different instance ids.

ensure_identity (Lines 487-503) deliberately serializes exactly this sequence under _audit_lock because two racing hooks otherwise "mint different ids, and every artifact stamped with the loser's id becomes unreachable under the winner's identity." This function has the same read-then-write shape with no lock, and it is reached from bundle.export (Line 195) and sync._source_origin, both of which hooks can run concurrently — so bundles/proposals can be stamped with an id that is not the one left on disk.

Separately: the sidecar is not in the .gitignore written at Lines 441-445, so a clone inherits the source KB's instance id, which conflicts with the docstring's "two KBs holding identical artifacts still have different instance ids." Worth confirming which behavior is intended.

🔒 Proposed fix for the race

     path = kb_dir / INSTANCE_ID_FILENAME
     try:
         existing = path.read_text(encoding="utf-8").strip()
     except OSError:
         existing = ""
     if existing:
         return existing
-    new_id = uuid.uuid4().hex
-    path.write_text(new_id + "\n", encoding="utf-8")
-    return new_id
+    from .audit import _audit_lock
+
+    with _audit_lock(kb_dir):
+        try:
+            existing = path.read_text(encoding="utf-8").strip()
+        except OSError:
+            existing = ""
+        if existing:
+            return existing  # lost the race — adopt the winner's id
+        new_id = uuid.uuid4().hex
+        path.write_text(new_id + "\n", encoding="utf-8")
+        return new_id

📝 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

	path = kb_dir / INSTANCE_ID_FILENAME
	try:
	existing = path.read_text(encoding="utf-8").strip()
	except OSError:
	existing = ""
	if existing:
	return existing
	new_id = uuid.uuid4().hex
	path.write_text(new_id + "\n", encoding="utf-8")
	return new_id
	path = kb_dir / INSTANCE_ID_FILENAME
	try:
	existing = path.read_text(encoding="utf-8").strip()
	except OSError:
	existing = ""
	if existing:
	return existing
	from .audit import _audit_lock
	with _audit_lock(kb_dir):
	try:
	existing = path.read_text(encoding="utf-8").strip()
	except OSError:
	existing = ""
	if existing:
	return existing # lost the race — adopt the winner's id
	new_id = uuid.uuid4().hex
	path.write_text(new_id + "\n", encoding="utf-8")
	return new_id

🤖 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 300 - 309, Serialize the read-mint-write
sequence in the identity helper used by bundle.export and sync._source_origin,
reusing the existing _audit_lock pattern from ensure_identity so concurrent
callers return the same on-disk instance ID. Also verify the intended clone
behavior for the instance-ID sidecar and update the .gitignore handling only if
the project contract requires cloned KBs to receive distinct identities.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -A5 'def rank' contrib/strategies/*.py src/vouch/strategy.py src/vouch/strategies/*.py
ast-grep outline src/vouch/strategy.py --items all

Repository: vouchdev/vouch

Length of output: 3712

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== strategies provenance files =="
cat -n src/vouch/strategies/provenance.py | sed -n '1,140p'
printf '\n== contrib provenance rank ==\n'
cat -n contrib/strategies/provenance_rank.py | sed -n '1,120p'
printf '\n== example lexical for comparison ==\n'
cat -n contrib/strategies/example_lexical.py | sed -n '1,90p'

printf '\n== all rank callsites =="
rg -n '\brank\(' src tests scripts contrib -g '*.py' || true

printf '\n== retrieval strategy usages around limit =="
rg -n -A7 -B7 'rank\(|ordered_ids|apply_ordering|RetrievalStrategy|load_dotted|load_from_path' src/vouch/strategy.py src tests -g '*.py' || true

Repository: vouchdev/vouch

Length of output: 8981

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== rank callsites =="
rg -n '\brank\(' . -g '*.py' || true

echo
echo "== retrieval strategy usages =="
rg -n -A7 -B7 'rank\(|ordered_ids|apply_ordering|RetrievalStrategy|load_dotted|load_from_path' . -g '*.py' || true

echo
echo "== deterministic signature and limit behavior probe from source text =="
python3 - <<'PY'
from pathlib import Path
paths = ["src/vouch/strategies/provenance.py", "contrib/strategies/provenance_rank.py", "contrib/strategies/example_lexical.py"]
for path in paths:
    text = Path(path).read_text()
    start = text.index("def rank(query: str, candidates: list[Candidate], *, limit: int) -> list[str]:")
    end = text.index("\n    return ", start) + len("return ")
    body = text[start:end]
    print(f"\n{path}:")
    print("uses_limit_parameter=" + ("limit" in body))
    print("uses_limit_value=" + ("[:limit]" in body or "(" in text[start:text.find("return ", start)] and "limit" in text[start:text.find("return ", start)]) )
    print("returns_unbound_sorted=" + (":[None" not in body and "sorted(" in body and "[:limit]" not in body))
PY

Repository: vouchdev/vouch

Length of output: 48015

---

limit is accepted but never applied in rank().

src/vouch/context.py:_maybe_strategy() is built around the top budget: it over-fetches candidates into a window, calls strat.rank(..., limit=limit), then drops de-prioritized ids below that window. This strategy returns every candidate id instead, so low-scoring hits can remain in the output when the context budget is small.

🐛 Proposed fix

-    return [c.id for c in sorted(candidates, key=key, reverse=True)]
+    return [c.id for c in sorted(candidates, key=key, reverse=True)][:limit]

📝 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 rank(query: str, candidates: list[Candidate], *, limit: int) -> list[str]:
	q = _tokens(query)
	first_person = bool(_FIRST_PERSON_QUERY.search(query))
	def key(c: Candidate) -> float:
	text = c.summary
	overlap = len(q & _tokens(text)) / len(q) if q else 0.0
	score = 0.7 * c.score + 0.3 * overlap
	if _INSTRUCTION.search(text):
	score -= 10.0
	if _HEARSAY.search(text) and first_person:
	score -= 5.0
	elif _HEARSAY.search(text):
	score -= 2.0
	if _UPDATE.search(text):
	score += 1.0
	return score
	return [c.id for c in sorted(candidates, key=key, reverse=True)]
	def rank(query: str, candidates: list[Candidate], *, limit: int) -> list[str]:
	q = _tokens(query)
	first_person = bool(_FIRST_PERSON_QUERY.search(query))
	def key(c: Candidate) -> float:
	text = c.summary
	overlap = len(q & _tokens(text)) / len(q) if q else 0.0
	score = 0.7 * c.score + 0.3 * overlap
	if _INSTRUCTION.search(text):
	score -= 10.0
	if _HEARSAY.search(text) and first_person:
	score -= 5.0
	elif _HEARSAY.search(text):
	score -= 2.0
	if _UPDATE.search(text):
	score += 1.0
	return score
	return [c.id for c in sorted(candidates, key=key, reverse=True)][:limit]

🤖 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/strategies/provenance.py` around lines 65 - 83, Apply the limit
parameter in rank() after sorting candidates by the existing key, returning only
the top limit candidate IDs while preserving the current ranking order and
scoring behavior.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# what does _SyncSource.root hold — the project root or the .vouch dir?
ast-grep run --pattern 'class _SyncSource:
  $$$' --lang python src/vouch/sync.py
rg -nP -C4 '\broot\s*=' src/vouch/sync.py | rg -n -C4 '_load_source|_SyncSource' 
ast-grep run --pattern 'def _load_source($$$):
  $$$' --lang python src/vouch/sync.py
# and how every other caller passes kb_dir into it
rg -nP -C2 '\bread_or_create_instance_id\s*\(' src

Repository: vouchdev/vouch

Length of output: 1450

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== sync.py outline =="
ast-grep outline src/vouch/sync.py --match _SyncSource --view expanded
ast-grep outline src/vouch/sync.py --match _load_source --view expanded
ast-grep outline src/vouch/sync.py --match _source_origin --view expanded

echo "== relevant sync.py lines 45-160 and 280-305 =="
sed -n '45,160p' src/vouch/sync.py | nl -ba -v45
printf '\n---\n'
sed -n '280,305p' src/vouch/sync.py | nl -ba -v280

echo "== storage.py relevant lines 260-315 =="
sed -n '260,315p' src/vouch/storage.py | nl -ba -v260

echo "== bundle.py relevant export lines 180-205 =="
sed -n '180,205p' src/vouch/bundle.py | nl -ba -v180

echo "== all kb_dir usages around source loading =="
rg -n -C5 'kb_dir|source_path|load_source|_SyncSource' src/vouch/sync.py

Repository: vouchdev/vouch

Length of output: 1009

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== sync.py relevant loading and origin paths =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/vouch/sync.py')
lines = p.read_text().splitlines()
for i in range(45, 165):
    print(f"{i:4d}: {lines[i-1]}")
print("\n---")
for i in range(280, 306):
    print(f"{i:4d}: {lines[i-1]}")
print("\n---")
for i in range(130, 190):
    print(f"{i:4d}: {lines[i-1]}")
PY

echo "== storage.py relevant path implementation =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/vouch/storage.py')
lines = p.read_text().splitlines()
for i in range(250, 320):
    print(f"{i:4d}: {lines[i-1]}")
PY

echo "== bundle.py export reference =="
python3 - <<'PY'
from pathlib import Path
p = Path('src/vouch/bundle.py')
lines = p.read_text().splitlines()
for i in range(170, 210):
    print(f"{i:4d}: {lines[i-1]}")
PY

Repository: vouchdev/vouch

Length of output: 13231

---

Use a read-only source-provenance lookup.

src.root is already the .vouch dir, but read_or_create_instance_id() creates and writes instance_id when absent. Sync reads should not mutate the source KB; add/read a sidecar without writing, then fall back to src.source_id.

🤖 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/sync.py` around lines 291 - 300, Update _source_origin for sources
with src.root to use a read-only instance-ID lookup that does not create or
write the instance_id sidecar; return the existing ID when present, otherwise
fall back to src.source_id. Do not mutate the source KB during sync, and remove
the read_or_create_instance_id usage from this path.