Test - #571
Merged
Merged
Conversation
update_page lacked the graph-integrity ref checks that put_page and update_claim already enforce. Vault-edit approvals could land pages citing claims deleted between propose and approve. Co-authored-by: Cursor <cursoragent@cursor.com>
…paths Batch CLI already calls check_approvable(); web/MCP approve() did not, so stale vault-edit refs surfaced as ValueError instead of ProposalError. Defense-in-depth alongside update_page ref validation. Co-authored-by: Cursor <cursoragent@cursor.com>
approve() now runs _payload_block_reason before delete, so the error message uses referenced by instead of still referenced by. Co-authored-by: Cursor <cursoragent@cursor.com>
citation_summary called list_sources(), which skips unreadable meta via _load_or_skip. Claims citing those sources were misreported as broken_citation in kb.stats / vouch stats — lint already treats the source as present (unreadable_source finding instead). Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
walk approved claims grouped by shared entity, flag same-topic pairs that disagree in polarity, and file a pending `contradicts` relation proposal per surviving pair via proposals.propose_relation. read-only and advisory: the scanner never sets ClaimStatus.CONTESTED, never writes a Relation, and never touches Claim.contradicts — those only land once a human runs `vouch approve` (or the pre-existing manual lifecycle.contradict). scoring/grouping lives in the new src/vouch/contradictions.py, not storage.py. `vouch contradict-scan` mirrors `vouch dedup`'s cli-only shape: --threshold, --entity, --limit, and --dry-run/--no-dry-run (default dry-run). a pair already cross-linked via Claim.contradicts, joined by an approved contradicts edge, or with a pending contradicts proposal is skipped so repeat scans don't duplicate proposals. closes #314
the fix landed in #484 without a changelog entry, and the `## [Unreleased]` / `### Fixed` section is what the next release notes are rolled from — a user-visible behaviour change (lint stops emitting `stale_claim` warnings for retired claims) would otherwise ship undocumented.
workflow_dispatch that reviews open prs and posts a verdict comment on each. never merges, closes, arms auto-merge, or labels. reviews the diff as data via the messages api (no checkout or execution of pr code), so no environment gate is needed. owner-dispatched; optional prs input to scope to specific numbers.
trust-gate.yml, auto-merge.yml, and comment-command.yml built the
changed-file list from `gh pr view --json files`, which is backed by
the GraphQL PullRequestFile type and carries no previous-filename
field. When GitHub detects a rename, only the new path appears there,
so `git mv src/vouch/http_server.py src/vouch/web_server.py` (plus any
edit inside it) made the changed core file invisible to pr_bot's exact-
match CORE_GLOBS check: trust-gate reported no core paths touched for
an untrusted author, and the PR was eligible for auto-merge arming as
klass=code instead of being permanently blocked as klass=core.
switch all three workflows to the REST pulls/{n}/files endpoint, which
does populate previous_filename on renamed entries, and add a
`changed-files` pr_bot subcommand that flattens that payload into both
the new and previous filename so a rename can no longer drop a core
path off the radar.
Fixes #505
…lassification-bypass # Conflicts: # .github/workflows/comment-command.yml
matches house style per CodeRabbit review on #506.
parse_codex_transcript only read session_meta, which carries cwd, branch and timestamps but not the model — codex puts the turn model on the turn_context record. so the normalized transcript's session model was always none for codex, while the claude parser fills it from the assistant message. read the model off turn_context (first non-empty wins) so both agents expose it.
ground_prompt gated its items on isinstance(pack, ContextPack), but build_context_pack returns a model_dump'd dict in production, so the guard was always false and grounding always returned "the knowledge base has nothing on this topic yet." — even when the kb had matching claims. dual-solve therefore never fed the code engines any kb context. read items from the dict (tolerating a ContextPack object too), mirroring synthesize's grounding. the existing test monkeypatched build_context_pack to return a ContextPack, so it exercised the dead branch and masked the bug; add a regression that drives the real, unmocked pipeline.
the gate job publishes the required `coderabbit-approved` commit status
via POST /repos/{repo}/statuses/{sha}. for a pr opened from a fork the
GITHUB_TOKEN can't write a status onto the fork's head sha, so the api
returns 403 "resource not accessible by integration". that failed the
whole job and put a red check on every external contribution — even when
the review verdict was `approved`.
the unset status already blocks native auto-merge on its own (the ruleset
keeps waiting for it), so failing this infra job adds only noise for
outside contributors. treat a 403 on a fork head as expected: warn and
succeed. same-repo prs still hard-fail if the publish fails.
phase 3 of global vouch. `vouch hub init-personal` creates + registers a personal kb at ~/.local/share/vouch/personal (XDG_DATA_HOME honoured, VOUCH_PERSONAL_KB overrides). with its own config's personal.fallback_capture flag on (opt-in: one question on a terminal at `install-mcp --global`, or --personal-fallback / `vouch hub fallback on`), sessions in folders WITHOUT a project kb capture into it — origin folder stamped on every captured source — and recall reads it back from the same folders. the session banner announces the routing; a guard refusal (discovery landing on a personal kb from below, the hijack shape) never falls through to fallback. `vouch adopt`, run inside a project that now has its own kb, drains those captures home THROUGH the project's review gate: sources copy byte-identically (content-addressed ids survive), each live personal claim re-proposes against the copied source, receipts re-verify mechanically, and the project's own review config decides durability. idempotent; --retire archives the personal copies; both kbs log a kb.adopt event carrying the other side's id.
the "a folder without a kb never captures anywhere" wording gains its "by default" qualifier now that the opt-in personal fallback exists; both readmes explain the opt-in, the origin stamping, the banner announcement, and that adoption re-verifies receipts through the project kb's own review gate.
adversarial review of the phase-3 diff confirmed 12 findings (6 refuted). the blocker: `adopt --retire` archived personal claims that had only landed PENDING in the project, so rejecting or expiring the proposal left the knowledge live in neither kb with no unarchive path. retire now covers exactly the claims that became durable. the rest: - re-running adopt filed a duplicate pending proposal per pass under a human-only gate (`_already_durable` never saw the queue) — the guard now checks pending payload ids too, and --dry-run predicts against the project's real gate instead of assuming receipts auto-approve. - fallback recall reads the whole personal kb, which is what a catch-all is — but the injected digest called it "knowledge for this repo". the digest header, the prompt-hook block, the banner, the opt-in question and both readmes now say it is one store shared by every kb-less folder, so recall there can surface knowledge captured elsewhere. - session rollups filed by the fallback carried no origin: they now record the folder like captured sources do, and adopt reports the ones still pending in the personal kb instead of leaving them silent. - a stale personal registry row could shadow a live one and switch fallback off; live rows now win and init-personal retires rows pointing elsewhere. - OSError from the personal-kb bootstrap escaped as a traceback and made a successful --global install exit 1; both entry points now report a remedy, and the install warns without failing. - `vouch status` in a kb-less folder said only "run vouch init" while the hook plane was capturing to the personal kb; it now names it. - set_personal_fallback took the kb's cross-process lock like identity minting does, instead of a lock-free read-modify-write of config.yaml.
retire covers only claims that landed durable, adopt is idempotent against the pending queue too, dry-run predicts against the real gate, session rollups are reported rather than moved, and the personal kb is described as the one shared store it is.
ci(coderabbit-gate): tolerate status-publish 403 on fork prs
…no-session-id fix(capture): read session id from stdin payload in finalize-all
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vouch/web/console.py (1)
190-201: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCache headers computed from untrusted
rel, not from the actual servedcandidate.When a path-traversal attempt resets
candidatetoindex(Line 198), the response headers are still chosen via_cache_headers(rel)using the raw, untrustedrel. A request like/assets/../../etc/passwdnormalizes torel = "assets/../../etc/passwd", which still starts with"assets/", so the SPA shell gets served with_ASSET_CACHE(immutable, 1-year) instead of_SHELL_CACHE. This defeats the exact "must always revalidate" guarantee this diff introduces — a browser hitting that specific deep link would cache the stale shell for a year.Base the header choice on what is actually served (
candidate), not on the raw request path.🐛 Proposed fix
if candidate.is_file(): - return FileResponse(candidate, headers=_cache_headers(rel)) + headers = _SHELL_CACHE if candidate == index else _cache_headers(rel) + return FileResponse(candidate, headers=headers) return FileResponse(index, headers=_SHELL_CACHE)🤖 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/web/console.py` around lines 190 - 201, Update the _spa response flow so cache headers are selected from the file actually served, not the untrusted rel value. When traversal validation falls back to index, or when serving the shell, use _SHELL_CACHE; use _cache_headers only for a validated asset candidate returned by FileResponse.
🧹 Nitpick comments (9)
src/vouch/cli.py (1)
1864-1872: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBound the new numeric options.
--max-claims 0/ negative values flow straight intoextract_mod.ingest_source;click.IntRangerejects them at parse time with a clear message.♻️ Proposed tweak
- "--max-claims", type=int, default=None, + "--max-claims", type=click.IntRange(min=1), default=None, @@ - "--budget-chars", type=int, default=None, + "--budget-chars", type=click.IntRange(min=1), default=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 `@src/vouch/cli.py` around lines 1864 - 1872, Update the Click option definitions for max_claims and budget_chars to use click.IntRange validation, rejecting zero and negative values during argument parsing while preserving None as the unset default.contrib/strategies/relevance_guard.py (1)
63-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
CONFLICT_PENALTY/POSSESSIVE_PENALTYare unused; the real penalties are inline literals.
rank()uses hardcoded10.0,6.0, and4.0(Lines 104, 122, 124), so these two module constants are dead and actively misleading to anyone tuning this file. Either wire them in or drop them.♻️ Suggested cleanup
SCORE_W = 0.7 OVERLAP_W = 0.3 -CONFLICT_PENALTY = 3.0 -POSSESSIVE_PENALTY = 4.0 +DANGER_SCALE = 4.0 +INSTRUCTION_PENALTY = 10.0 +SUSPECT_PENALTY = 6.0🤖 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/relevance_guard.py` around lines 63 - 66, Update rank() to use the existing CONFLICT_PENALTY and POSSESSIVE_PENALTY constants wherever the corresponding inline penalty literals are applied, or remove those constants if they are not intended to control the calculations; eliminate the misleading unused definitions while preserving the current penalty behavior.src/vouch/chatgpt_import.py (1)
460-475: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftThe refresh path mutates a proposal payload directly, sidestepping
propose_page's validation.New conversations go through
propose_page(page-kind validation, scope, slug handling); the re-import path writestitle/body/sourcesstraight into the stored payload withstore.update_proposal. Any validationpropose_pagegains later won't apply to refreshes, andproposed_atis left stale. Prefer aproposals.*helper for "refresh a pending page proposal" so both surfaces share one code path.As per path instructions, "MCP/JSONL/CLI share the same storage + proposals + audit code path … avoid introducing divergence."
🤖 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/chatgpt_import.py` around lines 460 - 475, Replace the direct payload mutation and store.update_proposal flow in the existing-proposal branch with the shared proposals helper used to refresh a pending page proposal, routing title, body, and source updates through propose_page or its equivalent. Preserve the existing audit event and actor/object metadata while ensuring the shared path performs page-kind, scope, slug, and proposed_at handling consistently with new imports.Source: Path instructions
src/vouch/context.py (1)
330-356: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winconfig.yaml is now parsed up to four times per query.
_retrieval_configre-reads and re-parses the file on every call, and_maybe_strategycan call it twice (params arm, then dotted arm) on top of the existing_configured_recency/_configured_rerank/_configured_backendparses. Retrieval is the hot path; consider a single per-store read (e.g.functools.lru_cachekeyed on path+mtime, or threading one parsed mapping through_retrieve) and have_configured_recency/_configured_rerankconsume_retrieval_configtoo.🤖 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 330 - 356, The retrieval configuration is reparsed multiple times per query, including separate reads in strategy and existing retrieval option helpers. Refactor the retrieval flow around _retrieval_config, _maybe_strategy, _configured_recency, _configured_rerank, and _configured_backend to perform one per-store config read and reuse the parsed retrieval mapping, either by threading it through _retrieve or using a path/mtime-aware cache. Ensure all retrieval option helpers consume that shared result without changing configuration behavior.src/vouch/capture.py (1)
896-911: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
finalize_all_except's docstring still describes the old receipt-only drain.Line 928 documents
auto_approvedas "count of pending receipt-verified claims drained on the way", but_drain_approval_backlognow drains whatever the configured gate allows (pages, entities, relations included undertrusted-agent). Worth updating so the reported number isn't misread.Also applies to: 941-941
🤖 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 896 - 911, Update the auto_approved documentation in finalize_all_except to describe the count returned by _drain_approval_backlog as all pending proposals approved under the configured gate, including claims, pages, entities, and relations where applicable, rather than only receipt-verified claims.src/vouch/proposals.py (1)
527-538: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
store.config_pathinstead of re-deriving the filename.
KBStore.config_pathalready resolvesCONFIG_FILENAME; hardcoding"config.yaml"here is a second source of truth.♻️ Proposed tweak
- loaded = yaml.safe_load( - (store.kb_dir / "config.yaml").read_text(encoding="utf-8") - ) + loaded = yaml.safe_load(store.config_path.read_text(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 `@src/vouch/proposals.py` around lines 527 - 538, Update the YAML loading logic in the review configuration path to read from store.config_path instead of constructing store.kb_dir / "config.yaml". Preserve the existing parsing, error handling, and review normalization behavior.webapp/src/views/ChatView.tsx (1)
120-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo hand-maintained copies of the drawer-kind allowlist.
This
SetandDRAWER_KINDSinwebapp/src/views/BrowseView.tsx(Line 16) both enumerate the same kinds asOpenKind; a future kind added toOpenKindwill silently stay unclickable in one of them. Exporting one canonical list next toOpenKindinArtifactDrawer.tsxand deriving both call sites from it keeps them in lockstep.♻️ Sketch
// ArtifactDrawer.tsx export const OPEN_KINDS = ['claim', 'page', 'entity', 'relation', 'evidence', 'source'] as const export type OpenKind = (typeof OPEN_KINDS)[number]-const DRAWER_KINDS = new Set(['claim', 'page', 'entity', 'relation', 'evidence', 'source']) +const DRAWER_KINDS: ReadonlySet<string> = new Set(OPEN_KINDS)🤖 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 `@webapp/src/views/ChatView.tsx` at line 120, Replace the local DRAWER_KINDS Set in ChatView.tsx and the corresponding allowlist in BrowseView.tsx with a canonical OPEN_KINDS list exported from ArtifactDrawer.tsx. Derive OpenKind from OPEN_KINDS there, and update both call sites to use the shared list while preserving their existing membership-check behavior.webapp/src/components/ArtifactDrawer.tsx (1)
79-96: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueA 10k-event audit fetch to render one event row.
Expanding a single provenance edge pulls the whole tail; it's cached per endpoint, but the first expansion pays the full transfer and parse on a KB with a long history. If
kb.auditaccepts filtering (e.g. by object id or a since cursor), prefer that; otherwise consider a smaller tail with a "load more" fallback.🤖 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 `@webapp/src/components/ArtifactDrawer.tsx` around lines 79 - 96, Update EventDetail’s audit query to avoid fetching AUDIT_TAIL events when resolving a single provenance edge: use the kb.audit filtering capability by event/object id or an appropriate since cursor if supported. If filtering is unavailable, reduce the initial tail size and add a load-more path that fetches older events only when the requested event is not found, while preserving the existing endpoint-level query caching.tests/test_adopt.py (1)
121-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the repeated closed-gate setup into a helper.
The same three-line block (set
auto_approve_on_receipt = False, popapprover_role, write back) is duplicated across four tests. A small helper would reduce duplication and centralize the "closed gate" definition if it needs to evolve.♻️ Proposed helper extraction
+def _close_review_gate(project: KBStore) -> None: + cfg = yaml.safe_load(project.config_path.read_text(encoding="utf-8")) + cfg["review"]["auto_approve_on_receipt"] = False + cfg["review"].pop("approver_role", None) + project.config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8")Then replace each 4-line inline block with
_close_review_gate(project).Also applies to: 277-289, 357-370, 392-402
🤖 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_adopt.py` around lines 121 - 130, Extract the repeated review-gate configuration block into a `_close_review_gate(project)` helper that disables `auto_approve_on_receipt`, removes `approver_role`, and writes the updated configuration. Replace the duplicated setup in `test_adopt_respects_a_closed_gate` and the other referenced tests with calls to this helper.
🤖 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 @.github/scripts/promote_champion.py:
- Around line 41-47: Update the module-docstring removal logic around stripped
in the promotion flow to recognize and remove valid leading docstrings using
both triple-double-quote and triple-single-quote delimiters. Preserve the
existing behavior of removing only a leading docstring and trimming the
following newline so the champion header remains the sole provenance header.
In @.github/workflows/koth-ledger.yml:
- Around line 81-93: Validate the value assigned to mean in the engine branch
before invoking promote_champion.py: parse .challenger.mean only when it is
numeric, and skip promotion when the field is absent, null, or malformed. Wrap
the existing winner check and promotion call in a non-empty mean guard,
preserving the remaining PR iteration steps.
In `@CHANGELOG.md`:
- Line 150: Merge the duplicate Fixed and Added headings in CHANGELOG.md into
the existing corresponding sections under [Unreleased]. Move the entries from
the newly added headings into those sections and remove the redundant headings
so changelog lint no longer reports repeated headings.
In `@docs/mining-on-vouch.md`:
- Around line 78-81: Update CHANGELOG.md under the [Unreleased] section to
document the user-visible automatic champion promotion and merge-threshold
ratcheting behavior described in the mining-on-vouch documentation.
In `@src/vouch/chatgpt_import.py`:
- Around line 372-383: Replace the per-call scan in _find_existing_page with a
single _page_proposals_by_session index built before the import_export
conversation loop. Use existing_pages.get(row["session_id"]) during processing,
and update the index whenever a new page proposal is filed so duplicate session
IDs within the same export still deduplicate.
- Around line 111-128: Update the ZIP handling in the import flow to enforce
_MAX_EXPORT_BYTES on the decompressed data actually read, rather than trusting
info.file_size from the archive metadata. Ensure zf.read(info) cannot
materialize more than the allowed limit, and retain the existing
ChatGPTImportError behavior for oversized entries.
In `@src/vouch/cli.py`:
- Around line 4131-4152: Update the [Unreleased] section of CHANGELOG.md to
document the new user-facing CLI commands import-chatgpt, read-evidence,
read-source, and wipe-dead-refs, matching the changelog’s existing style and
grouping.
In `@src/vouch/stats.py`:
- Around line 94-96: Lowercase the newly added comment prose in the
stats-related code: change its opening “Use” to “use” and ensure the entire
comment remains lowercase, without altering its meaning or surrounding
implementation.
In `@src/vouch/storage.py`:
- Around line 84-94: Remove the default "approver_role": "trusted-agent" entry
from the starter configuration in the initialization flow so the review gate
remains enabled by default. Keep auto_approve_on_receipt enabled as the safe
default, and update vouch init to surface the trusted-agent shortcut as an
explicit opt-in configuration rather than enabling it implicitly.
In `@tests/test_read_evidence_source.py`:
- Around line 38-41: Update the _rpc helper and the related kb.* method tests to
validate the complete JSONL response envelope: assert the echoed id and
successful result, and add failure-path assertions for ok=false with an error
field. Apply the same envelope checks to the tests referenced around lines 69-74
while preserving their existing method-specific assertions.
In `@tests/test_strategy_configured.py`:
- Around line 27-28: Update the configured-strategy tests to load the current
champion from contrib/strategies/baseline.py instead of the static
RELEVANCE_GUARD path. Adjust the affected test setup and assertions around the
referenced lines so the invariant compares against the baseline file that
promotions update, while preserving the existing test behavior.
In `@tests/test_trusted_agent_auto_approve.py`:
- Around line 38-44: Update the starter configuration defaults exercised by
test_starter_config_defaults_to_trusted_agent so review.approver_role is a
non-trusted approver and auto_approve_on_receipt does not bypass human review by
default. Preserve trusted-agent approval only when review.approver_role is
explicitly set to trusted-agent, and revise the test assertions to verify the
opt-in requirement.
In `@webapp/src/components/ArtifactDrawer.tsx`:
- Around line 235-252: Update the catch block in resolveOpen to rethrow
VouchHttpError for authentication, transport, and non-2xx failures, and continue
probing only when the caught VouchRpcError represents a missing artifact for the
current kind. Preserve the existing success callback and final toast behavior
for ids that do not resolve in any readable kind.
---
Outside diff comments:
In `@src/vouch/web/console.py`:
- Around line 190-201: Update the _spa response flow so cache headers are
selected from the file actually served, not the untrusted rel value. When
traversal validation falls back to index, or when serving the shell, use
_SHELL_CACHE; use _cache_headers only for a validated asset candidate returned
by FileResponse.
---
Nitpick comments:
In `@contrib/strategies/relevance_guard.py`:
- Around line 63-66: Update rank() to use the existing CONFLICT_PENALTY and
POSSESSIVE_PENALTY constants wherever the corresponding inline penalty literals
are applied, or remove those constants if they are not intended to control the
calculations; eliminate the misleading unused definitions while preserving the
current penalty behavior.
In `@src/vouch/capture.py`:
- Around line 896-911: Update the auto_approved documentation in
finalize_all_except to describe the count returned by _drain_approval_backlog as
all pending proposals approved under the configured gate, including claims,
pages, entities, and relations where applicable, rather than only
receipt-verified claims.
In `@src/vouch/chatgpt_import.py`:
- Around line 460-475: Replace the direct payload mutation and
store.update_proposal flow in the existing-proposal branch with the shared
proposals helper used to refresh a pending page proposal, routing title, body,
and source updates through propose_page or its equivalent. Preserve the existing
audit event and actor/object metadata while ensuring the shared path performs
page-kind, scope, slug, and proposed_at handling consistently with new imports.
In `@src/vouch/cli.py`:
- Around line 1864-1872: Update the Click option definitions for max_claims and
budget_chars to use click.IntRange validation, rejecting zero and negative
values during argument parsing while preserving None as the unset default.
In `@src/vouch/context.py`:
- Around line 330-356: The retrieval configuration is reparsed multiple times
per query, including separate reads in strategy and existing retrieval option
helpers. Refactor the retrieval flow around _retrieval_config, _maybe_strategy,
_configured_recency, _configured_rerank, and _configured_backend to perform one
per-store config read and reuse the parsed retrieval mapping, either by
threading it through _retrieve or using a path/mtime-aware cache. Ensure all
retrieval option helpers consume that shared result without changing
configuration behavior.
In `@src/vouch/proposals.py`:
- Around line 527-538: Update the YAML loading logic in the review configuration
path to read from store.config_path instead of constructing store.kb_dir /
"config.yaml". Preserve the existing parsing, error handling, and review
normalization behavior.
In `@tests/test_adopt.py`:
- Around line 121-130: Extract the repeated review-gate configuration block into
a `_close_review_gate(project)` helper that disables `auto_approve_on_receipt`,
removes `approver_role`, and writes the updated configuration. Replace the
duplicated setup in `test_adopt_respects_a_closed_gate` and the other referenced
tests with calls to this helper.
In `@webapp/src/components/ArtifactDrawer.tsx`:
- Around line 79-96: Update EventDetail’s audit query to avoid fetching
AUDIT_TAIL events when resolving a single provenance edge: use the kb.audit
filtering capability by event/object id or an appropriate since cursor if
supported. If filtering is unavailable, reduce the initial tail size and add a
load-more path that fetches older events only when the requested event is not
found, while preserving the existing endpoint-level query caching.
In `@webapp/src/views/ChatView.tsx`:
- Line 120: Replace the local DRAWER_KINDS Set in ChatView.tsx and the
corresponding allowlist in BrowseView.tsx with a canonical OPEN_KINDS list
exported from ArtifactDrawer.tsx. Derive OpenKind from OPEN_KINDS there, and
update both call sites to use the shared list while preserving their existing
membership-check 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: 1e6e4816-5edd-45f0-adb2-3e6e4d09619e
📒 Files selected for processing (101)
.github/scripts/promote_champion.py.github/scripts/score_strategy.py.github/scripts/validate_kit.py.github/workflows/koth-engine-gate.yml.github/workflows/koth-ledger.yml.github/workflows/pr-review-sweep.yml.gitignore.vouch/.gitignore.vouch/audit.log.jsonl.vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl.vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl.vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl.vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl.vouch/claims/vouch-starter-reviewed-knowledge.yaml.vouch/claims/vouch-uses-a-review-gated-proposal-workflow-agents-propose-c.yaml.vouch/config.yaml.vouch/decided/20260521-055206-7d6d92d6.yaml.vouch/decided/20260707-092531-154cb163.yaml.vouch/decided/20260707-093005-05345f47.yaml.vouch/decided/20260707-093125-4d6972f9.yaml.vouch/pages/edit-in-obsidian.md.vouch/pages/reviewed-knowledge-store.md.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md.vouch/schema_version.vouch/sources/06d8519f8dcf4149d23c8a48984541b2e9365ec364e7e58192e28ed149a2c47c/content.vouch/sources/06d8519f8dcf4149d23c8a48984541b2e9365ec364e7e58192e28ed149a2c47c/meta.yaml.vouch/sources/67478e72acfb8fac3a059143e95c95f5cc6f7e8d4dccc05fbcea8dbccb8a4eba/content.vouch/sources/67478e72acfb8fac3a059143e95c95f5cc6f7e8d4dccc05fbcea8dbccb8a4eba/meta.yaml.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yamlCHANGELOG.mdREADME.mdcompetition/LEADERBOARD.mdcontrib/strategies/relevance_guard.pydocs/koth-ladder.mddocs/koth-strategy-lane.mddocs/mining-on-vouch.mdsrc/vouch/admission.pysrc/vouch/bench.pysrc/vouch/capabilities.pysrc/vouch/capture.pysrc/vouch/chatgpt_import.pysrc/vouch/cli.pysrc/vouch/compile.pysrc/vouch/config_coerce.pysrc/vouch/context.pysrc/vouch/extract.pysrc/vouch/hot_memory.pysrc/vouch/http_server.pysrc/vouch/inbox.pysrc/vouch/jsonl_server.pysrc/vouch/lifecycle.pysrc/vouch/proposals.pysrc/vouch/recall.pysrc/vouch/sandbox.pysrc/vouch/server.pysrc/vouch/session_split.pysrc/vouch/stats.pysrc/vouch/storage.pysrc/vouch/strategies/configured.pysrc/vouch/strategy.pysrc/vouch/verify.pysrc/vouch/volunteer_context.pysrc/vouch/web/console.pytests/test_admission.pytests/test_adopt.pytests/test_bench.pytests/test_capture.pytests/test_chatgpt_import.pytests/test_compile.pytests/test_config_coerce.pytests/test_console.pytests/test_context.pytests/test_delete.pytests/test_extract.pytests/test_hot_memory.pytests/test_http_server.pytests/test_inbox.pytests/test_jsonl_server.pytests/test_koth_kit.pytests/test_lifecycle.pytests/test_promote_champion.pytests/test_read_evidence_source.pytests/test_recall.pytests/test_receipt_auto_approve.pytests/test_sandbox.pytests/test_session_split.pytests/test_session_transcript.pytests/test_stats.pytests/test_strategy_configured.pytests/test_trusted_agent_auto_approve.pytests/test_verify.pytests/test_volunteer_context.pytests/test_web.pywebapp/.gitignorewebapp/src/components/ArtifactDrawer.test.tsxwebapp/src/components/ArtifactDrawer.tsxwebapp/src/lib/types.tswebapp/src/views/BrowseView.tsxwebapp/src/views/ChatView.test.tsxwebapp/src/views/ChatView.tsx
💤 Files with no reviewable changes (24)
- .vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content
- .vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl
- .vouch/sources/67478e72acfb8fac3a059143e95c95f5cc6f7e8d4dccc05fbcea8dbccb8a4eba/content
- .vouch/decided/20260521-055206-7d6d92d6.yaml
- .vouch/decided/20260707-093125-4d6972f9.yaml
- .vouch/.gitignore
- .vouch/claims/vouch-starter-reviewed-knowledge.yaml
- .vouch/claims/vouch-uses-a-review-gated-proposal-workflow-agents-propose-c.yaml
- .vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl
- .vouch/sources/06d8519f8dcf4149d23c8a48984541b2e9365ec364e7e58192e28ed149a2c47c/content
- .vouch/sources/67478e72acfb8fac3a059143e95c95f5cc6f7e8d4dccc05fbcea8dbccb8a4eba/meta.yaml
- .vouch/decided/20260707-093005-05345f47.yaml
- .vouch/sources/06d8519f8dcf4149d23c8a48984541b2e9365ec364e7e58192e28ed149a2c47c/meta.yaml
- .vouch/pages/edit-in-obsidian.md
- .vouch/schema_version
- .vouch/audit.log.jsonl
- .vouch/decided/20260707-092531-154cb163.yaml
- .vouch/pages/session-what-are-the-non-development-related-files-and-folde.md
- .vouch/pages/reviewed-knowledge-store.md
- .vouch/config.yaml
- .vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yaml
- .vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl
- .vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl
- README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- competition/LEADERBOARD.md
- docs/koth-strategy-lane.md
- docs/koth-ladder.md
- tests/test_admission.py
- src/vouch/jsonl_server.py
- tests/test_capture.py
- tests/test_compile.py
Comment on lines
+41
to
+47
| # drop the submission's own module docstring so the champion header is | ||
| # the single source of provenance | ||
| stripped = body | ||
| if stripped.startswith('"""'): | ||
| end = stripped.find('"""', 3) | ||
| if end != -1: | ||
| stripped = stripped[end + 3 :].lstrip("\n") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --glob '*.py' "^[[:space:]]*'''" contrib/strategies testsRepository: vouchdev/vouch
Length of output: 190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate promote_champion.py =="
fd -a 'promote_champion.py' . || true
echo
echo "== relevant promote script =="
if [ -f .github/scripts/promote_champion.py ]; then
cat -n .github/scripts/promote_champion.py
fi
echo
echo "== tests =="
if [ -f tests/test_promote_champion.py ]; then
cat -n tests/test_promote_champion.py
fi
echo
echo "== occurrences of promote_champion imports/calls =="
rg -n "promote_champion|promote\\(" .github scripts tests contrib strategies 2>/dev/null || true
echo
echo "== read-only parser probe for leading module docstrings =="
python3 - <<'PY'
from ast import parse
samples = [
'"""double"""',
"'''single'''",
'"""multi\nline"""',
"'''multi\nline'''",
'# note\n"""doc"""',
'import sys\n"""doc"""',
]
for src in samples:
tree = parse(src)
print(repr(src), "=>", ast.get_docstring(tree) if hasattr(ast, "get_docstring") else getattr(tree, "get_docstring", lambda: None)())
PYRepository: vouchdev/vouch
Length of output: 6982
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant parser probe =="
python3 - <<'PY'
import ast
samples = [
'"""double"""',
"'''single'''",
'"""multi\nline"""',
"'''multi\nline'''",
'# note\n"""doc"""',
'import sys\n"""doc"""',
]
for src in samples:
tree = parse(src)
print(repr(src), "=>", ast.get_docstring(tree))
PYRepository: vouchdev/vouch
Length of output: 293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Python docstring probe =="
python3 - <<'PY'
import ast
samples = [
'"""double"""',
"'''single'''",
'"""multi\nline"""',
"'''multi\nline'''",
'# note\n"""doc"""',
'import sys\n"""doc"""',
]
for src in samples:
tree = ast.parse(src)
print(repr(src), "=>", ast.get_docstring(tree))
PY
echo
echo "== behavioral probe for current promote() logic =="
python3 - <<'PY'
import ast
HEADER_PREFIX = '"""The reigning engine-lane champion (auto-promoted, do not edit).'
def promote(body: str) -> str:
stripped = body
if stripped.startswith('"""'):
end = stripped.find('"""', 3)
if end != -1:
stripped = stripped[end + 3 :]
new = HEADER_PREFIX + '..."'
new += stripped
return new
for winner_doc in ['"""double"""', "'''single'''", '"""multi\nline"""', "'''multi\nline'''"]:
body = winner_doc + '\n\ndef rank(query, candidates, *, limit):\n return [c.id for c in candidates]\n'
result = promote(body)
print("winner_doc=", repr(winner_doc))
print(result)
print("single header count=", result.count('"""The reigning engine-lane champion'))
print("contains original winning docstring in output?", winner_doc[3:-3] in result[3:])
PYRepository: vouchdev/vouch
Length of output: 1458
strip every valid module-docstring form.
A valid leading '''...''' module docstring is retained, leaving the promoted code with both the provenance header and the original submission docstring. Parse module docstrings robustly, or at least support both triple-quote delimiters.
🤖 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/promote_champion.py around lines 41 - 47, Update the
module-docstring removal logic around stripped in the promotion flow to
recognize and remove valid leading docstrings using both triple-double-quote and
triple-single-quote delimiters. Preserve the existing behavior of removing only
a leading docstring and trimming the following newline so the champion header
remains the sole provenance header.
Comment on lines
+81
to
+93
| lane=$(jq -r '.lane // "kit"' /tmp/report.json) | ||
| if [ "$lane" = "engine" ]; then | ||
| mean=$(jq -r '.challenger.mean' /tmp/report.json) | ||
| winner=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr}/files" \ | ||
| --paginate \ | ||
| --jq '.[] | select(.status != "removed") | .filename' \ | ||
| | grep -E '^contrib/strategies/[A-Za-z0-9_]+\.py$' \ | ||
| | grep -v 'baseline\.py' | head -1 || true) | ||
| if [ -n "$winner" ] && [ -f "$winner" ]; then | ||
| python3 .github/scripts/promote_champion.py \ | ||
| --strategy "$winner" --pr "$pr" --mean "$mean" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Guard mean before promoting.
jq -r '.challenger.mean' yields the literal string null when the key is absent or not a number, and that value is passed straight into promote_champion.py --mean, so a malformed report ratchets the champion threshold with garbage instead of skipping.
🛡️ Proposed guard
- mean=$(jq -r '.challenger.mean' /tmp/report.json)
+ mean=$(jq -r '(.challenger.mean | numbers) // empty' /tmp/report.json)
+ if [ -z "$mean" ]; then
+ echo "report has no numeric challenger.mean - skipping ratchet" >&2
+ continue
+ fiNote continue skips the rest of the PR loop iteration; use an if [ -n "$mean" ] wrapper instead if the remaining steps in the iteration must still run.
📝 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
| lane=$(jq -r '.lane // "kit"' /tmp/report.json) | |
| if [ "$lane" = "engine" ]; then | |
| mean=$(jq -r '.challenger.mean' /tmp/report.json) | |
| winner=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr}/files" \ | |
| --paginate \ | |
| --jq '.[] | select(.status != "removed") | .filename' \ | |
| | grep -E '^contrib/strategies/[A-Za-z0-9_]+\.py$' \ | |
| | grep -v 'baseline\.py' | head -1 || true) | |
| if [ -n "$winner" ] && [ -f "$winner" ]; then | |
| python3 .github/scripts/promote_champion.py \ | |
| --strategy "$winner" --pr "$pr" --mean "$mean" | |
| fi | |
| fi | |
| lane=$(jq -r '.lane // "kit"' /tmp/report.json) | |
| if [ "$lane" = "engine" ]; then | |
| mean=$(jq -r '(.challenger.mean | numbers) // empty' /tmp/report.json) | |
| if [ -z "$mean" ]; then | |
| echo "report has no numeric challenger.mean - skipping ratchet" >&2 | |
| continue | |
| fi | |
| winner=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${pr}/files" \ | |
| --paginate \ | |
| --jq '.[] | select(.status != "removed") | .filename' \ | |
| | grep -E '^contrib/strategies/[A-Za-z0-9_]+\.py$' \ | |
| | grep -v 'baseline\.py' | head -1 || true) | |
| if [ -n "$winner" ] && [ -f "$winner" ]; then | |
| python3 .github/scripts/promote_champion.py \ | |
| --strategy "$winner" --pr "$pr" --mean "$mean" | |
| fi | |
| fi |
🤖 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 81 - 93, Validate the value
assigned to mean in the engine branch before invoking promote_champion.py: parse
.challenger.mean only when it is numeric, and skip promotion when the field is
absent, null, or malformed. Wrap the existing winner check and promotion call in
a non-empty mean guard, preserving the remaining PR iteration steps.
Comment on lines
+78
to
+81
| credited in the changelog. on merge, the ledger sweep promotes the | ||
| winner to reigning champion automatically — the merge threshold ratchets | ||
| up with every merged win, because the next challenger is scored against | ||
| you. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
add an Unreleased changelog entry.
Automatic champion promotion and threshold ratcheting are user-visible behavior; record them in CHANGELOG.md in this PR.
As per coding guidelines, “When adding a user-visible feature, update CHANGELOG.md under [Unreleased] in the same PR.”
🤖 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 `@docs/mining-on-vouch.md` around lines 78 - 81, Update CHANGELOG.md under the
[Unreleased] section to document the user-visible automatic champion promotion
and merge-threshold ratcheting behavior described in the mining-on-vouch
documentation.
Source: Coding guidelines
Comment on lines
+111
to
+128
| if is_zip: | ||
| with zipfile.ZipFile(path) as zf: | ||
| infos = [ | ||
| i for i in zf.infolist() | ||
| if i.filename.rsplit("/", 1)[-1] == "conversations.json" | ||
| ] | ||
| if not infos: | ||
| raise ChatGPTImportError( | ||
| f"{path.name} has no conversations.json — this doesn't " | ||
| "look like a ChatGPT data export ZIP" | ||
| ) | ||
| info = infos[0] | ||
| if info.file_size > _MAX_EXPORT_BYTES: | ||
| raise ChatGPTImportError( | ||
| f"{info.filename} is too large to import " | ||
| f"({info.file_size} bytes > {_MAX_EXPORT_BYTES} byte limit)" | ||
| ) | ||
| return zf.read(info) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
The ZIP size guard trusts the header-declared file_size.
info.file_size comes from the archive's own central directory, so a crafted entry can understate it and zf.read(info) will still decompress the full payload into memory. Cap the actual read instead.
🛡️ Proposed hardening
info = infos[0]
if info.file_size > _MAX_EXPORT_BYTES:
raise ChatGPTImportError(
f"{info.filename} is too large to import "
f"({info.file_size} bytes > {_MAX_EXPORT_BYTES} byte limit)"
)
- return zf.read(info)
+ with zf.open(info) as fh:
+ data = fh.read(_MAX_EXPORT_BYTES + 1)
+ if len(data) > _MAX_EXPORT_BYTES:
+ raise ChatGPTImportError(
+ f"{info.filename} is too large to import "
+ f"(> {_MAX_EXPORT_BYTES} byte limit)"
+ )
+ return data📝 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
| if is_zip: | |
| with zipfile.ZipFile(path) as zf: | |
| infos = [ | |
| i for i in zf.infolist() | |
| if i.filename.rsplit("/", 1)[-1] == "conversations.json" | |
| ] | |
| if not infos: | |
| raise ChatGPTImportError( | |
| f"{path.name} has no conversations.json — this doesn't " | |
| "look like a ChatGPT data export ZIP" | |
| ) | |
| info = infos[0] | |
| if info.file_size > _MAX_EXPORT_BYTES: | |
| raise ChatGPTImportError( | |
| f"{info.filename} is too large to import " | |
| f"({info.file_size} bytes > {_MAX_EXPORT_BYTES} byte limit)" | |
| ) | |
| return zf.read(info) | |
| if is_zip: | |
| with zipfile.ZipFile(path) as zf: | |
| infos = [ | |
| i for i in zf.infolist() | |
| if i.filename.rsplit("/", 1)[-1] == "conversations.json" | |
| ] | |
| if not infos: | |
| raise ChatGPTImportError( | |
| f"{path.name} has no conversations.json — this doesn't " | |
| "look like a ChatGPT data export ZIP" | |
| ) | |
| info = infos[0] | |
| if info.file_size > _MAX_EXPORT_BYTES: | |
| raise ChatGPTImportError( | |
| f"{info.filename} is too large to import " | |
| f"({info.file_size} bytes > {_MAX_EXPORT_BYTES} byte limit)" | |
| ) | |
| with zf.open(info) as fh: | |
| data = fh.read(_MAX_EXPORT_BYTES + 1) | |
| if len(data) > _MAX_EXPORT_BYTES: | |
| raise ChatGPTImportError( | |
| f"{info.filename} is too large to import " | |
| f"(> {_MAX_EXPORT_BYTES} byte limit)" | |
| ) | |
| return data |
🤖 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/chatgpt_import.py` around lines 111 - 128, Update the ZIP handling
in the import flow to enforce _MAX_EXPORT_BYTES on the decompressed data
actually read, rather than trusting info.file_size from the archive metadata.
Ensure zf.read(info) cannot materialize more than the allowed limit, and retain
the existing ChatGPTImportError behavior for oversized entries.
Comment on lines
+84
to
94
| # auto approval is the default: the capturing agent's proposals | ||
| # self-approve through the same proposals.approve() gate (one | ||
| # audit event per artifact, duplicates rejected, protected page | ||
| # kinds and DELETE proposals still held for a reviewer). Remove | ||
| # approver_role to put every other write behind `vouch review`. | ||
| "approver_role": "trusted-agent", | ||
| # phase d — the receipt is the reviewer. When true, a claim whose | ||
| # byte-offset receipts all verify is auto-approved with no human; | ||
| # a claim that cannot quote its source is left pending, as is | ||
| # every page/entity/relation proposal. Set false to put every | ||
| # write behind `vouch review`. | ||
| # a claim that cannot quote its source is left pending. Only | ||
| # consulted when approver_role does not already clear the write. | ||
| "auto_approve_on_receipt": True, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Shipping approver_role: trusted-agent in the starter config makes the trusted-agent shortcut opt-out.
Every newly vouch init-ed KB now self-approves every non-protected write: proposals._approval_block_reason returns None for self-approval under this role, and the new proposals.auto_approve_pending drains all pending claims/pages/entities/relations on a SessionStart hook. The documented contract is that the shortcut is opt-in, so the default should leave it unset (receipt-based auto-approval alone is the safe default) and vouch init should surface the flip explicitly.
As per coding guidelines, "Do not implement SaaS/hosted mode, remove the review gate for trusted agents, …"; as per path instructions, "every kb.* write tool is review-gated with no trusted-agent shortcut except opt-in review.approver_role: trusted-agent".
🔒 Suggested default
- # auto approval is the default: the capturing agent's proposals
- # self-approve through the same proposals.approve() gate (one
- # audit event per artifact, duplicates rejected, protected page
- # kinds and DELETE proposals still held for a reviewer). Remove
- # approver_role to put every other write behind `vouch review`.
- "approver_role": "trusted-agent",
+ # opt in by setting `approver_role: trusted-agent` to let the
+ # capturing agent's proposals self-approve through the same
+ # proposals.approve() gate. Left unset so writes stay behind
+ # `vouch review` unless the receipt gate below clears them.📝 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
| # auto approval is the default: the capturing agent's proposals | |
| # self-approve through the same proposals.approve() gate (one | |
| # audit event per artifact, duplicates rejected, protected page | |
| # kinds and DELETE proposals still held for a reviewer). Remove | |
| # approver_role to put every other write behind `vouch review`. | |
| "approver_role": "trusted-agent", | |
| # phase d — the receipt is the reviewer. When true, a claim whose | |
| # byte-offset receipts all verify is auto-approved with no human; | |
| # a claim that cannot quote its source is left pending, as is | |
| # every page/entity/relation proposal. Set false to put every | |
| # write behind `vouch review`. | |
| # a claim that cannot quote its source is left pending. Only | |
| # consulted when approver_role does not already clear the write. | |
| "auto_approve_on_receipt": True, | |
| # opt in by setting `approver_role: trusted-agent` to let the | |
| # capturing agent's proposals self-approve through the same | |
| # proposals.approve() gate. Left unset so writes stay behind | |
| # `vouch review` unless the receipt gate below clears them. | |
| # phase d — the receipt is the reviewer. When true, a claim whose | |
| # byte-offset receipts all verify is auto-approved with no human; | |
| # a claim that cannot quote its source is left pending. Only | |
| # consulted when approver_role does not already clear the write. | |
| "auto_approve_on_receipt": True, |
🤖 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 84 - 94, Remove the default
"approver_role": "trusted-agent" entry from the starter configuration in the
initialization flow so the review gate remains enabled by default. Keep
auto_approve_on_receipt enabled as the safe default, and update vouch init to
surface the trusted-agent shortcut as an explicit opt-in configuration rather
than enabling it implicitly.
Sources: Coding guidelines, Path instructions
Comment on lines
+38
to
+41
| def _rpc(method: str, params: dict) -> dict: | ||
| resp = handle_request({"id": "t", "method": method, "params": params}) | ||
| assert resp["ok"], resp | ||
| return resp["result"] # type: ignore[no-any-return] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Envelope-shape assertions incomplete for new kb.* method tests.
Per coding guidelines, new kb.* method tests must assert the full JSONL envelope shape ({id, ok, result} on success, {id, ok: false, error} on failure). Here only ok is checked; id echo and error presence on failure are never asserted.
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 fix
def _rpc(method: str, params: dict) -> dict:
resp = handle_request({"id": "t", "method": method, "params": params})
assert resp["ok"], resp
+ assert resp["id"] == "t"
return resp["result"] # type: ignore[no-any-return] def test_jsonl_read_missing_is_an_error(
store: KBStore, monkeypatch: pytest.MonkeyPatch, method: str, params: dict
) -> None:
monkeypatch.chdir(store.root)
resp = handle_request({"id": "t", "method": method, "params": params})
assert not resp["ok"]
+ assert "error" in respAlso applies to: 69-74
🤖 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_read_evidence_source.py` around lines 38 - 41, Update the _rpc
helper and the related kb.* method tests to validate the complete JSONL response
envelope: assert the echoed id and successful result, and add failure-path
assertions for ok=false with an error field. Apply the same envelope checks to
the tests referenced around lines 69-74 while preserving their existing
method-specific assertions.
Source: Coding guidelines
Comment on lines
+27
to
+28
| REPO = Path(__file__).resolve().parents[1] | ||
| RELEVANCE_GUARD = REPO / "contrib" / "strategies" / "relevance_guard.py" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
test against the actual reigning champion.
relevance_guard.py is static, while promotions update contrib/strategies/baseline.py. Load the baseline file here so this invariant catches drift after a champion promotion.
Also applies to: 55-61
🤖 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_configured.py` around lines 27 - 28, Update the
configured-strategy tests to load the current champion from
contrib/strategies/baseline.py instead of the static RELEVANCE_GUARD path.
Adjust the affected test setup and assertions around the referenced lines so the
invariant compares against the baseline file that promotions update, while
preserving the existing test behavior.
Comment on lines
+38
to
+44
| def test_starter_config_defaults_to_trusted_agent(store: KBStore) -> None: | ||
| # a fresh kb ships with auto approval on: no config edit, no human step. | ||
| import yaml | ||
|
|
||
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | ||
| assert loaded["review"]["approver_role"] == "trusted-agent" | ||
| assert loaded["review"]["auto_approve_on_receipt"] is True |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
keep trusted-agent approval opt-in.
This test codifies a starter default that drains pending writes without a human review step. Default to a non-trusted approver and require an explicit review.approver_role: trusted-agent opt-in instead.
As per coding guidelines, “do not ... remove the review gate for trusted agents”; as per path instructions, the trusted-agent shortcut is allowed only as an opt-in.
🤖 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_trusted_agent_auto_approve.py` around lines 38 - 44, Update the
starter configuration defaults exercised by
test_starter_config_defaults_to_trusted_agent so review.approver_role is a
non-trusted approver and auto_approve_on_receipt does not bypass human review by
default. Preserve trusted-agent approval only when review.approver_role is
explicitly set to trusted-agent, and revise the test assertions to verify the
opt-in requirement.
Sources: Coding guidelines, Path instructions
Comment on lines
+235
to
+252
| // Kindless ids (relation endpoints, audit object ids): probe the readable | ||
| // kinds and open the first that resolves. Proposal ids resolve to nothing. | ||
| const resolveOpen = | ||
| onOpen && | ||
| (async (id: string) => { | ||
| for (const kind of RESOLVE_ORDER) { | ||
| if (!canRead(project, kind)) continue | ||
| try { | ||
| await rpc(project.conn, READ_METHOD[kind].method, { [READ_METHOD[kind].param]: id }) | ||
| onOpen(kind, id) | ||
| return | ||
| } catch { | ||
| // not this kind — try the next | ||
| } | ||
| } | ||
| toast('error', `${id.slice(0, 24)} is not a readable artifact (a proposal, or deleted)`) | ||
| }) | ||
| const resolve = resolveOpen ? (id: string) => void resolveOpen(id) : undefined |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)ArtifactDrawer\.tsx$|(^|/)ChatView\.tsx$|(^|/)rpc\.ts$|(^|/)rpc\.(tsx?)$' || true
echo "== ArtifactDrawer relevant section =="
fs="webapp/src/components/ArtifactDrawer.tsx"
if [ -f "$fs" ]; then
sed -n '200,280p' "$fs" | nl -ba -v200
fi
echo "== ChatView around 333 =="
cfs="webapp/src/views/ChatView.tsx"
if [ -f "$cfs" ]; then
sed -n '310,355p' "$cfs" | nl -ba -v310
fi
echo "== rpc exports/usages =="
for f in $(git ls-files | rg '(^|/)webapp/src[^/]*(rpc|api|lib)[^/]*\.(ts|tsx)$'); do
if rg -q 'export .*VouchHttpError|class .*VouchHttpError|function .*VouchHttpError|interface .*RpcError|throw .*Error|type RpcError' "$f"; then
echo "--- $f ---"
rg -n 'VouchHttpError|RpcError|rpc\(|throw .*Error|interface .*|export .*Error' "$f" | head -80
fi
doneRepository: vouchdev/vouch
Length of output: 343
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ArtifactDrawer relevant section =="
fs="webapp/src/components/ArtifactDrawer.tsx"
sed -n '1,320p' "$fs" | awk '{printf "%5d\t%s\n", NR, $0}' | sed -n '220,270p'
echo "== ChatView around 333 =="
cfs="webapp/src/views/ChatView.tsx"
sed -n '300,365p' "$cfs" | awk '{printf "%5d\t%s\n", NR, $0}'
echo "== rpc.ts contents =="
rfs="webapp/src/lib/rpc.ts"
sed -n '1,240p' "$rfs" | awk '{printf "%5d\t%s\n", NR, $0}'
echo "== usages of throw/catch around rpc/rpc function in ArtifactDrawer and ChatView =="
rg -n "catch\s*\(|try\s*\{|rpc\(|VouchHttpError|VouchError|throw" webapp/src/components/ArtifactDrawer.tsx webapp/src/views/ChatView.tsx webapp/src/lib/rpc.tsRepository: vouchdev/vouch
Length of output: 12209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ArtifactDrawer imports and READ_METHOD/RESOLVE_ORDER =="
sed -n '1,80p' webapp/src/components/ArtifactDrawer.tsx | awk '{printf "%5d\t%s\n", NR, $0}'
rg -n "READ_METHOD|RESOLVE_ORDER|canRead|useErrorToast|import .*rpc|import .*Vouch" webapp/src/components/ArtifactDrawer.tsx
echo "== ArtifactDrawer full toast/error handling section =="
sed -n '212,255p' webapp/src/components/ArtifactDrawer.tsx | awk '{printf "%5d\t%s\n", NR, $0}'
echo "== RPC request/error probe (source-only behavioral extractor) =="
python3 - <<'PY'
from pathlib import Path
p = Path("webapp/src/lib/rpc.ts").read_text()
def has(pattern):
print(f"{pattern!r} present: {pattern in p}")
has("throw new VouchHttpError")
has("throw new VouchRpcError")
print("fetch catch absent:", "\n" in p and "fetch('/proxy/rpc" in p and "catch" not in p.split("fetch('/proxy/rpc", 1)[-1].split("}", 1)[0][:500])
PYRepository: vouchdev/vouch
Length of output: 6635
Only continue on RPC missing-artifact errors
rpc() exposes VouchHttpError for auth/transport and VouchRpcError for server errors, but the bare catch treats 401, non-2xx HTTP status, and dropped connections as “wrong kind” and then shows “not a readable artifact”. Re-throw VouchHttpError and continue only for RPC errors that mean this id isn’t that 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 `@webapp/src/components/ArtifactDrawer.tsx` around lines 235 - 252, Update the
catch block in resolveOpen to rethrow VouchHttpError for authentication,
transport, and non-2xx failures, and continue probing only when the caught
VouchRpcError represents a missing artifact for the current kind. Preserve the
existing success callback and final toast behavior for ids that do not resolve
in any readable kind.
citations went dead for a specific slice of ids and never worked at all inside page bodies. `_slugify` strips dashes before truncating to 60 chars, so cutting mid-word re-introduces a trailing dash — 135 of 1585 claim ids and 7 of 104 page ids here end in `-`. the client matcher required the last character to be alphanumeric, so all of those rendered as dead text. separately the `[claim: <id>]` marker, which is vouch's canonical inline reference in page bodies (compile.py `_CLAIM_MARKER_RE`), never matched at all. the matcher now accepts trailing-dash ids and an optional `kind:` prefix, while still leaving markdown task boxes, footnote refs and `[[wikilinks]]` as prose. a remark plugin rewrites bracketed ids in markdown into `vouch:` links, so page bodies, session summaries, claude answers and proposal payloads all resolve; a custom urlTransform is needed because react-markdown otherwise sanitises that scheme away. code spans stay literal, and evidence quotes are deliberately left verbatim — they are receipts, and styling text inside them as a link misrepresents the span. the drawer's kind probe moves to lib/resolveArtifact so chat, claims and the review queue share it, widened from three kinds to six with a shape hint so `ev-…` and 64-char source hashes cost one read instead of six. views without a drawer of their own route through /browse/<kind>/<id>. a named kind opens directly rather than being caps-gated: synthesize already verified it, and a failed read reports itself better than a guess. also rstrip the slug after truncating so new ids stop growing the defect. existing ids are untouched; the matcher fix is what makes those 142 work. verified against this kb: 43 `[claim: id]` markers across 5 pages now linkify, 0 of 5859 real ids fail to match, wikilinks intact.
the propose-time payload validation added a page->claim existence check to _payload_block_reason. approve() runs that precheck before its own dead-claim-refs handling, so a page citing a deleted claim raised a flat ProposalError and DeadClaimRefsError could never surface -- the drop_missing_claims strip path became unreachable and three tests/test_dead_claim_refs.py cases went red on test. approve() now passes skip_dead_claim_refs=True; check_approvable keeps the check so the batch cli precheck still flags dangling refs and stays all-or-nothing. the storage test that asserted the old wording now pins the specific DeadClaimRefsError and that the page body is untouched.
picks up the approve-gate fix (#601) so this branch's ci is green on its own base rather than inheriting the three red dead-claim-refs cases from test.
…adow fix(proposals): stop payload precheck shadowing dead claim refs
Fix/linkable ids
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vouch/cli.py (1)
5569-5572: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winturn invalid
--seedsinto usage errors.
--seeds "abc"or"1,x"raises an unhandledValueError, and values like" ,,"parse to an empty seed list. Validate each token before adding it toseed_listand reject an empty list with a click parameter 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/cli.py` around lines 5569 - 5572, Update the seed parsing that builds seed_list to validate every --seeds token as an integer, converting invalid values and an empty resulting list into a Click parameter error rather than allowing ValueError or empty input to proceed. Preserve the existing single-seed fallback when --seeds is omitted.
🧹 Nitpick comments (2)
src/vouch/proposals.py (1)
877-886: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDropping every cited claim can flip the page into a
PageKindErrorrefusal.
validate_page(..., has_citations=bool(page.claims or page.sources))runs right after the strip; if all claims were missing and the page has no sources, an explicitdrop_missing_claims=Trueretry fails with a citation-requirement error instead of a message tied to the drop. Worth surfacing that as a dedicated reason so the reviewer knows stripping is not a viable remediation for that proposal.🤖 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/proposals.py` around lines 877 - 886, Update the missing-claim handling around missing_claim_refs and the drop_missing_claims branch to detect when removing missing claims leaves the proposal with neither claims nor sources. Surface a dedicated drop-related refusal reason before or alongside validate_page, while preserving normal stripping behavior when citations remain.src/vouch/contradictions.py (1)
107-141: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueFull-KB load plus per-entity pairwise scoring — fine for a CLI, worth bounding later.
find_candidates()loads every claim and every relation/proposal (via_already_flagged_pairs) and then compares all same-entity pairs; a hub-entity with many claims makes this quadratic on tokenized text. An entity-group size cap or precomputed token sets per claim would keep the advisory scan predictable on large KBs.🤖 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/contradictions.py` around lines 107 - 141, The candidate scan in find_candidates can become unbounded for large entity groups. Add a predictable bound, such as a maximum group size, before the nested pairwise scoring loop, or reuse precomputed token sets to reduce repeated text work; preserve read-only behavior and existing filtering, threshold, deduplication, and sorting semantics.
🤖 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 @.github/workflows/trust-gate.yml:
- Around line 29-30: The GitHub REST file listing can truncate at 3,000 files,
so each workflow must fail closed before classifying changes. In
.github/workflows/trust-gate.yml lines 29-30, fetch changed_files metadata and
abort when it exceeds 3,000 before invoking vouch.pr_bot changed-files; apply
the same pre-classification check in .github/workflows/auto-merge.yml lines
73-74 and before adding auto-merge in .github/workflows/comment-command.yml
lines 55-56.
In `@src/vouch/cli.py`:
- Around line 3731-3732: Update the contradict-scan CLI’s --limit option to use
click.IntRange(min=0), ensuring negative values are rejected before the command
invokes contradictions.scan(). Keep the existing None default and valid
non-negative limit behavior unchanged.
In `@src/vouch/contradictions.py`:
- Around line 81-104: The _already_flagged_pairs function must also exclude
contradiction pairs from decided proposals, including rejected proposals, so
prior reviewer decisions are not re-proposed. Extend its proposal scan beyond
ProposalStatus.PENDING to include decided statuses, while preserving the
existing relation-kind and source/target payload checks.
In `@webapp/src/components/Markdown.tsx`:
- Line 26: Validate custom vouch URLs before preserving them: in
webapp/src/components/Markdown.tsx#L26, call parseVouchHref(url) and retain the
custom scheme only when parsing succeeds. In webapp/src/lib/vouchIds.ts#L16-L24,
make parseVouchHref safely handle malformed decodeURIComponent input and reject
prefixes not included in ArtifactKind; add tests covering malformed encoding and
unknown kinds.
---
Outside diff comments:
In `@src/vouch/cli.py`:
- Around line 5569-5572: Update the seed parsing that builds seed_list to
validate every --seeds token as an integer, converting invalid values and an
empty resulting list into a Click parameter error rather than allowing
ValueError or empty input to proceed. Preserve the existing single-seed fallback
when --seeds is omitted.
---
Nitpick comments:
In `@src/vouch/contradictions.py`:
- Around line 107-141: The candidate scan in find_candidates can become
unbounded for large entity groups. Add a predictable bound, such as a maximum
group size, before the nested pairwise scoring loop, or reuse precomputed token
sets to reduce repeated text work; preserve read-only behavior and existing
filtering, threshold, deduplication, and sorting semantics.
In `@src/vouch/proposals.py`:
- Around line 877-886: Update the missing-claim handling around
missing_claim_refs and the drop_missing_claims branch to detect when removing
missing claims leaves the proposal with neither claims nor sources. Surface a
dedicated drop-related refusal reason before or alongside validate_page, while
preserving normal stripping behavior when citations remain.
🪄 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: 603282ad-2915-4a9f-b0cd-8f1a132a06fa
📒 Files selected for processing (36)
.github/workflows/auto-merge.yml.github/workflows/coderabbit-gate.yml.github/workflows/comment-command.yml.github/workflows/trust-gate.ymlCHANGELOG.mdsrc/vouch/cli.pysrc/vouch/contradictions.pysrc/vouch/dual_solve.pysrc/vouch/pr_bot.pysrc/vouch/proposals.pysrc/vouch/recall.pysrc/vouch/storage.pysrc/vouch/transcript.pytests/test_capture.pytests/test_contradictions.pytests/test_delete.pytests/test_dual_solve.pytests/test_pr_bot.pytests/test_recall.pytests/test_session_transcript.pytests/test_storage.pywebapp/src/components/ArtifactDrawer.tsxwebapp/src/components/CitedText.tsxwebapp/src/components/IdChip.tsxwebapp/src/components/Markdown.test.tsxwebapp/src/components/Markdown.tsxwebapp/src/lib/citations.test.tswebapp/src/lib/citations.tswebapp/src/lib/resolveArtifact.tswebapp/src/lib/useOpenArtifact.tswebapp/src/lib/vouchIds.tswebapp/src/views/ChatView.tsxwebapp/src/views/ClaimsView.test.tsxwebapp/src/views/ClaimsView.tsxwebapp/src/views/PendingView.test.tsxwebapp/src/views/PendingView.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- webapp/src/views/PendingView.test.tsx
- tests/test_recall.py
- CHANGELOG.md
Comment on lines
+29
to
+30
| gh api "repos/$REPO/pulls/$PR/files" --paginate > files.json | ||
| PYTHONPATH=src python -m vouch.pr_bot changed-files --json-file files.json > changed.txt |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
fail closed when the REST file listing is truncated.
GitHub’s REST PR-files endpoint returns at most 3,000 files even with pagination, so a protected path beyond that ceiling is omitted from classification. This can bypass the core-path trust gate and incorrectly arm auto-merge. (docs.github.com)
.github/workflows/trust-gate.yml#L29-L30: fetchchanged_filesmetadata and fail if it exceeds 3,000 before classifying..github/workflows/auto-merge.yml#L73-L74: apply the same fail-closed check before deriving the merge class..github/workflows/comment-command.yml#L55-L56: apply the same fail-closed check before addingauto-merge.
As per path instructions, these are “core, security-sensitive workflows.”
📍 Affects 3 files
.github/workflows/trust-gate.yml#L29-L30(this comment).github/workflows/auto-merge.yml#L73-L74.github/workflows/comment-command.yml#L55-L56
🤖 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/trust-gate.yml around lines 29 - 30, The GitHub REST file
listing can truncate at 3,000 files, so each workflow must fail closed before
classifying changes. In .github/workflows/trust-gate.yml lines 29-30, fetch
changed_files metadata and abort when it exceeds 3,000 before invoking
vouch.pr_bot changed-files; apply the same pre-classification check in
.github/workflows/auto-merge.yml lines 73-74 and before adding auto-merge in
.github/workflows/comment-command.yml lines 55-56.
Source: Path instructions
Comment on lines
+3731
to
+3732
| @click.option("--limit", default=None, type=int, | ||
| help="Cap the number of pairs proposed in one run.") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate cli.py and contradictions.scan definitions"
fd -a 'cli.py|capacities.py|capabilities.py' . | sed 's#^\./##' | head -50
echo
rg -n "def scan|class .*Contradictions|def contradict_scan|`@click.option`.*--limit|scan\\(" src/vouch tests -S | sed -n '1,200p'
echo
echo "Relevant cli.py sections"
file=$(fd 'cli.py' src/vouch | head -1)
if [ -n "$file" ]; then
wc -l "$file"
sed -n '3680,3770p' "$file" | cat -n
fi
echo
echo "Relevant scan implementations"
rg -n -A 40 -B 10 "def scan|candidates\\[:-limit\\]|limit|scan\\(" src/vouch -S | sed -n '1,260p'Repository: vouchdev/vouch
Length of output: 22734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "src/vouch/contradictions.py outline and limit handling"
ast-grep outline src/vouch/contradictions.py --view expanded | sed -n '1,160p'
echo
sed -n '130,230p' src/vouch/contradictions.py | cat -n
echo
echo "MCP and JSONL references to contradict scan/limit"
rg -n -A 10 -B 8 "contradict|kb_.*contrad|scan\\(" src/vouch/server.py src/vouch/jsonl_server.py src/vouch/capabilities.py | sed -n '1,240p'
echo
echo "Behavioral probe for Python slice semantics"
python3 - <<'PY'
for n in range(3):
candidates = list(range(8 + n))
for m in [-1, -2, 3, 0, None]:
result_count = len(candidates[None if m is None else :m])
print({
"n": n,
"m": m,
"candidates": candidates,
"result": candidates[None if m is None else :m],
"result_count": result_count,
})
PYRepository: vouchdev/vouch
Length of output: 12662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Python slice semantics probe"
python3 - <<'PY'
for n in range(3):
candidates = list(range(8 + n))
for m in [-1, -2, 3, 0, None]:
result = candidates[:m]
print(dict(
n=n,
m=m,
candidates=list(candidates),
result=result,
result_count=len(result),
original_count=len(candidates),
count_dropped=len(candidates) - len(result),
))
PY
echo
echo "Check available surfaces for contradict scan"
python3 - <<'PY'
from pathlib import Path
roots = [Path(p) for p in ("src/vouch/server.py","src/vouch/jsonl_server.py","src/vouch/capabilities.py","src/vouch/cli.py")]
for root in roots:
text = root.read_text()
print(root, "has contradict/dedup scan:", "contradict" in text and "scan" in text)
for needle in ["contradict_scan", "kb_contradict_scan", "kb_dedup_scan", "dedup_scan"]:
if needle in text:
print(" ", needle)
PYRepository: vouchdev/vouch
Length of output: 2773
validate --limit in the CLI path.
contradict-scan passes limit=None through to contradictions.scan(), but only the CLI option is being changed here. Use click.IntRange(min=0) for the CLI option and validate/reject negative limits before scan() applies it as a Python slice.
🤖 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 3731 - 3732, Update the contradict-scan CLI’s
--limit option to use click.IntRange(min=0), ensuring negative values are
rejected before the command invokes contradictions.scan(). Keep the existing
None default and valid non-negative limit behavior unchanged.
Comment on lines
+81
to
+104
| def _already_flagged_pairs(store: KBStore) -> set[frozenset[str]]: | ||
| """Pairs that already have a contradiction on record, in any form. | ||
|
|
||
| Covers claims cross-linked via `Claim.contradicts` (set by | ||
| `lifecycle.contradict`), claims already joined by an approved | ||
| `RelationType.CONTRADICTS` edge (e.g. a prior scan's proposal that was | ||
| approved), and pairs with a still-pending `contradicts` relation | ||
| proposal from an earlier scan run. Any one of these means re-proposing | ||
| the pair would be a duplicate. | ||
| """ | ||
| flagged: set[frozenset[str]] = set() | ||
| for c in store.list_claims(): | ||
| for other in c.contradicts: | ||
| flagged.add(frozenset({c.id, other})) | ||
| for rel in store.list_relations(): | ||
| if rel.relation == RelationType.CONTRADICTS: | ||
| flagged.add(frozenset({rel.source, rel.target})) | ||
| for p in store.list_proposals(ProposalStatus.PENDING): | ||
| if p.kind == ProposalKind.RELATION and p.payload.get("relation") == "contradicts": | ||
| src = p.payload.get("source") | ||
| target = p.payload.get("target") | ||
| if src and target: | ||
| flagged.add(frozenset({src, target})) | ||
| return flagged |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Rejected pairs from earlier scans get re-proposed.
_already_flagged_pairs() covers Claim.contradicts, approved CONTRADICTS edges, and pending proposals — but not decided/rejected ones. A reviewer who rejects a false positive will see the same pair re-filed on the next scan(dry_run=False), which erodes trust in the queue. Consider also scanning decided proposals (or recording rejections) for contradicts pairs.
🤖 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/contradictions.py` around lines 81 - 104, The
_already_flagged_pairs function must also exclude contradiction pairs from
decided proposals, including rejected proposals, so prior reviewer decisions are
not re-proposed. Extend its proposal scan beyond ProposalStatus.PENDING to
include decided statuses, while preserving the existing relation-kind and
source/target payload checks.
| // `vouch:` hrefs are minted by remarkVouchIds, never by the author's | ||
| // markdown, and are consumed by the anchor override below rather than | ||
| // reaching the DOM. Everything else keeps the default sanitiser. | ||
| urlTransform={(url) => (url.startsWith(VOUCH_SCHEME) ? url : defaultUrlTransform(url))} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate custom vouch: hrefs before preserving them.
An author can supply [label](vouch:claim/%E0%A4%A): Line 26 bypasses the normal URL sanitizer, then decodeURIComponent throws while rendering. Unknown kinds are also cast as valid artifact kinds. Safely decode and whitelist kinds in parseVouchHref, then preserve vouch: URLs only when parsing succeeds.
webapp/src/components/Markdown.tsx#L26-L26: callparseVouchHref(url)before retaining a custom-scheme URL.webapp/src/lib/vouchIds.ts#L16-L24: catch malformed URI decoding and reject non-ArtifactKindprefixes; add malformed and unknown-kind tests.
📍 Affects 2 files
webapp/src/components/Markdown.tsx#L26-L26(this comment)webapp/src/lib/vouchIds.ts#L16-L24
🤖 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 `@webapp/src/components/Markdown.tsx` at line 26, Validate custom vouch URLs
before preserving them: in webapp/src/components/Markdown.tsx#L26, call
parseVouchHref(url) and retain the custom scheme only when parsing succeeds. In
webapp/src/lib/vouchIds.ts#L16-L24, make parseVouchHref safely handle malformed
decodeURIComponent input and reject prefixes not included in ArtifactKind; add
tests covering malformed encoding and unknown kinds.
…opose-time fix(proposals): validate claim/relation/entity payloads at propose time
…e-status fix(vault-sync): mirror post-approve WORKING/DRAFT artifacts (#583)
fix(secrets): mask JSON/quoted-key credentials
…thout-kb fix(transcript): degrade session_transcript when no kb resolves
fix(context): apply rerank on embedding and fts5 backends too
…d-pages fix(context): drop archived pages from context packs
…d-citation-547 fix(vault-sync): surface deleted-citation as VaultSyncError
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
12 participants
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.
What changed
Why
What might break
VEP
Tests
make checkpasses locally (lint + mypy + pytest)CHANGELOG.mdupdated under## [Unreleased]Summary by CodeRabbit