Test - #643
Merged
Merged
Conversation
the kb.export mcp tool dropped the `exclude` filter that the jsonl rpc and `vouch export --exclude` cli both accept (added in fdf1520, which missed server.py), so an mcp agent — the primary vouch client — could never produce a knowledge-only bundle and got no `excluded` in the response. thread it through to bundle.export, matching the sibling surfaces.
`_as_float(...) or DEFAULT_MIN_SCORE` swallowed a legitimate min_score: 0 (an explicit "never defer") and rewrote it to the 0.4 default, conflating unset/unparseable/zero. fall back only when the value is absent or unparseable, matching every other numeric config loader in the codebase.
repo-wide line coverage of src/vouch goes 82.96% -> 90.14% (3172 -> 1841 uncovered statements) and 18 modules reach 100%. 571 new tests. three setup problems were doing most of the damage, and none of them were visible from the coverage number alone: the embedder registry in embeddings/base is a module-global dict, and test_context, test_propose_similarity, test_clear_claims and test_triage each register a MockEmbedder into it and never restore it. the leak is inert while numpy is absent, so nobody noticed; install numpy and it flips later tests onto the embedding backend -- the fts5 backend-label assertions in test_cli, the deindex assertions in test_delete, and the salience cases in test_hot_memory all fail for reasons unrelated to what they test. tests/embeddings/conftest already isolated the registry for its own directory; tests/conftest now lifts that to the whole suite. nothing installed numpy, so tests/embeddings was skipped everywhere via `importorskip` and ~470 statements of shipped code never ran in any ci job. numpy alone unlocks it -- MockEmbedder is a pure-python fake -- so it joins the dev extra rather than requiring the embeddings extra and torch. `--cov=vouch` matches every importable copy of the package, and test_wheel_contents installs the built wheel into a pytest temp dir. those 58 duplicate modules landed in the report at ~0% and dragged the total from 85% to 43%, which is the number ci has been uploading. omitted in both [run] and [report] -- the [run] copy governs collection, but the temp-wheel modules still reach the data file and only [report] omit keeps them out of the totals. fail_under is a ratchet at 90: raise it as gaps close, never lower it to make a red build green. the two hard modules are done. strategy.py sandbox child needed sys.addaudithook swapped for a collector, so _install_audit_hook runs to completion and hands back the closure without permanently arming a write-blocking hook on the test interpreter; its child_main needed the fd primitives stubbed because os.dup2(devnull, 1) would silence pytest for every later test, and the real child is spawned with a stripped env that drops COVERAGE_PROCESS_START, so it is unmeasurable by design. volunteer_context needed a real event loop on a background thread for the mcp notification push, a closed loop for the RuntimeError path, and poll-until-logged assertions because the push resolves after the test's event fires. embeddings/similarity.py went 0% -> 100%. it is live code reached from proposals.py for propose-time duplicate warnings and had no tests at all. three findings pinned as tests rather than fixed, since changing them is behaviour and belongs in its own pr: `vouch capabilities` outside a kb cannot reach its except-Exception fallback because _load_store raises SystemExit; `vouch source add --url` silently discards the url because put_source only folds url into locator when locator is unset; kb_cite leaks ArtifactNotFoundError where every kb_read_* converts to ValueError. index_db.search_embeddings (plural) has no callers left in src or tests. covered so the number is honest, but deleting it is the better fix.
every python line a pr adds or changes under src/vouch must be executed by a test. the repo-wide fail_under ratchet stops regressions; this is the per-pr bar, so new code arrives covered instead of adding to the debt. a pr that touches no python under src/vouch passes trivially -- diff-cover reports "no lines with coverage information in this diff" and exits 0 -- so docs-only and workflow-only prs are unaffected. verified the gate actually gates rather than decorates: added uncovered lines exit 1, added covered lines exit 0, a docs-only diff exits 0, --fail-under 0 on the same failing diff exits 0 (so the flag is genuinely read), and an unknown flag errors out. rehearsed end to end in a worktree against a deliberately uncovered helper in admission.py: the gate failed, the bot named `src/vouch/admission.py -- line(s) 235-237` at 25%, and adding the covering test flipped it to 100% and exit 0. the first rehearsal was invalid and worth recording: the editable install made coverage measure the main checkout's src instead of the worktree's, so the gate passed a deliberately uncovered line. if coverage.xml describes different files than the diff, diff-cover silently skips them -- the job therefore reads the xml produced by ci's own test run in the same commit. when the gate fails, diff-coverage-comment posts the uncovered lines on the pr. it follows ci-label's pattern: workflow_run on ci completion, pr resolved via commits/<sha>/pulls so fork prs work, base-branch checkout only, never the pr head. the body is rendered by vouch.pr_bot, not by yaml, because file paths in the report come from the pr's own diff and belong in tested code; it is written to a file and posted with --body-file so it is never interpolated into a shell word. one marker-keyed comment per pr, updated in place and flipped to a resolved note when coverage goes green. auto-merge now reads the diff-coverage check-run conclusion for the head sha before arming, and on failure strips the label and comments why. it reads a conclusion rather than recomputing coverage: that workflow holds a write token and must never execute pr code. full coverage is a necessary condition for arming, not a sufficient one -- arming still needs the owner's label, because making a passing test suite sufficient would let any contributor merge non-core code unreviewed. the gate binds only once the check is required. `test` currently has no branch protection and `main` has no required contexts, so native auto-merge has nothing to wait for and merges as soon as a pr is conflict-free. adding "diff coverage (100% of changed python)" as a required status check is a repo-settings change and is not part of this pr. actionlint could not be run locally (no go toolchain); zizmor --persona regular reports no findings across all 23 workflows.
actionlint runs shellcheck over every `run:` block, and the new diff-coverage-comment workflow tripped three findings in its resolve-the-PR step. `base=test` reads as an attempt to capture the output of the `test` command (SC2209), so quote the branch name; the three consecutive `$GITHUB_OUTPUT` appends become one grouped redirect (SC2129). both edits are semantics-preserving -- the fail-closed base ref and the step outputs are unchanged.
Ci/diff coverage gate
fix(worthiness): preserve a configured min_score of 0
compute_salience scanned every claim regardless of status, so the _meta.vouch_salience sidebar counted archived, superseded and redacted claims in claim_count and could name one as an entity's top_claim_id. the reflex exists to prefetch what an agent should read next, so it was handing back exactly the knowledge the archive/supersede/redact controls were supposed to retire. the claim scan already filters by viewer scope for this reason — the comment beside it says the sidebar must not resurface ids that scope filtering hides from search and digest. lifecycle status is the other half of that argument and was missed. the fix reuses the shared _RETRACTED_CLAIM_STATUSES denylist, matching recall, compile, volunteer_context and context itself. retracted claims also fed the entity ranking tiebreak, so an entity with many dead claims outranked a live one; filtering the scan fixes the ordering with it. an entity whose only claims are retracted still appears in the sidebar, now reporting zero live claims rather than pointing at a dead one — entities carry no status of their own, so dropping them would be a separate retrieval decision.
core prs were refused outright — codeowners wanted the owner's approval, so the arm job bailed on klass == 'core'. replace that blanket refusal with two mechanical bars that any klass must clear: - the `diff coverage` check is green on this exact head sha, so every changed python line under src/vouch/ is executed by a test. - the pr carries a closing reference to an issue plind-junior opened. this is github's resolved closingIssuesReferences link, not a text match, so a bare `#123` mention does not qualify. both are read as metadata. nothing here checks out or executes pr code, because the job holds a write token. the decision moves into one reusable arm-auto-merge.yml called by both authorization surfaces. they had already drifted: the `/auto-merge` comment path armed on klass alone and never looked at coverage. the label path and the comment path now share a single implementation. the owner-only guard and deauthorize-on-push are untouched. the classify steps are dropped from both callers — klass no longer gates anything, and each was doing a checkout plus python setup to compute it. calling jobs declare their own permissions, since a called workflow can only downgrade the caller's token and both files start at `permissions: {}`.
actionlint runs shellcheck over every run block. the graphql query is single-quoted so $owner/$name/$pr reach the server as variables bound by -f/-F rather than being expanded by the shell, which reads as SC2016. the local run missed it because shellcheck was not installed alongside actionlint; re-verified with both present.
_retrieve hands back (kind, id, summary, score, backend): one opaque score and a backend label. a reviewer tuning fusion, the reranker, or the recency and pages-first signals cannot see how much of a score came from lexical vs semantic rank, what the rrf contribution was, which stage moved a candidate, or which gate cut it. the ranker is a black box exactly where it most needs to be inspectable. kb.explain_ranking re-runs the pipeline stage by stage and snapshots every candidate's rank and score after each one. a candidate present in one snapshot and absent from the next was removed by that stage, which is what the reported gate names — kept, scope-filtered, status-filtered, limit-dropped, budget-dropped, or uncited. the stages compose _retrieve's ranking chain with the lifecycle gate search_kb applies and the budget gate build_context_pack applies, so one call explains every stage an artifact can die at. scope filtering runs without a limit and truncation is its own stage, mirroring search_kb's "scope first so status filtering can refill the window" ordering and keeping a candidate lost to the window attributable instead of folded into the scope filter. no context.py edits: every stage helper is already importable, so the instrumentation re-runs them rather than threading a trace flag through the hot path. rescoring stages (recency, pages_first) report a score delta; ordering stages (rerank, strategy) report a rank delta. a stage that is off still appears, flagged, so the chain reads continuously. read-only throughout — no writes, no proposals, no lifecycle change, and viewer scoping goes through the same filter_hits as kb.context, so nothing is exposed that the caller could not already retrieve. registered on mcp, jsonl, capabilities and the cli, plus the hot-memory coverage map, where it is excluded: a recency sidebar would perturb the output being inspected. the uncited gate is defensive. Claim rejects evidence=[] on the model, so no stored claim can be uncited today; the branch mirrors the check build_context_pack still makes and starts reporting if that invariant is ever relaxed. Closes #432
coderabbit's verdict no longer gates or reaps anything. removed: - coderabbit-gate.yml, which published the `coderabbit-approved` commit status and auto-closed a pr after 3 rounds of requested changes. - stale-pr-reaper.yml, the daily sweep that closed prs sitting on an unaddressed change request. - the pr_bot commands behind both (`coderabbit-gate`, `stale-check`) and the verdict/strike/staleness helpers they called, plus their tests. the status had already been dropped from the test ruleset's required checks, which require five: the three test legs, the build, and trust-gate. so this removes machinery that outlived the requirement rather than lowering a bar that was still load-bearing. coderabbit still reviews every non-draft pr and still files formal approve / request-changes reviews — they are advisory now. the merge path is ci + trust-gate + codeowners, with the owner's auto-merge label as the go signal.
ci(coderabbit): drop the coderabbit approval gate
* fix(config): coerce quoted false for enrich, events, pages_first #558 taught config loaders to use coerce_bool so enabled: "false" disables a feature, but enrich, retrieval.events, and pages_first still used bool(). quoted false left the first two on and turned pages_first on; route them through coerce_bool and add the same regression tests as recall/capture. Fixes #620 * docs(changelog): note coerce_bool fix for enrich/events/pages_first record the #620 config quoted-false residual in Unreleased Fixed. --------- Co-authored-by: plind <59729252+plind-junior@users.noreply.github.com>
ci(auto-merge): gate core prs on coverage plus an owner issue
fix(salience): exclude retracted claims from the reflex sidebar
trust-gate.yml failed a pr when an author outside the OWNER association touched a core path. the rewritten arm-auto-merge.yml already enforces that bar from the other side: nothing arms without the owner's own label, a green `diff coverage` on that head sha, and a closing reference to an owner-opened issue. CODEOWNERS still holds the review requirement on core paths. removed with it: the `trust` pr_bot command and the `is_trusted` helper it called, plus their tests. core-path classification stays — it is what arm-auto-merge.yml reads. trust-gate is still listed in the `test` ruleset's required checks, so that entry has to come out or every pr will block on a check that no longer reports.
ci(trust-gate): drop the trust gate
verify_all and doctor only treated external_status=drift as a problem, while vouch source verify already marked missing as !. deleted upstream files could leave doctor ok:true while the CLI failed. include missing in the failed set and emit a source_missing warning. Fixes #622
feat(server): kb.explain_ranking — why a result ranked where it did
fix(server): honor exclude on the mcp kb_export tool
…-external fix(verify/doctor): count missing externals as failures
digest already skipped done/dropped followup metadata and retired claims, but an ARCHIVED page with followup_status=open and a past due_at still appeared in the morning briefing. mirror recall: archived leave the list. Fixes #625
auto-merge had two arming surfaces, both requiring the owner to act: the auto-merge label and the /auto-merge comment. green ci triggered nothing, and deauthorize-on-push voids arming on every contributor push — so a PR like #626 was armed twice and disarmed twice by force-pushes, then sat clean and green with no one left to re-arm it. add a third arming path with no human in it. on `ci` completing green for a pull request, resolve the PR from the head sha, confirm it still sits on that sha and is open and undrafted, wait for every other check run on the sha to finish, and refuse if any of them failed — `ci` is only one of the workflows a PR runs, and `test` has no branch protection to wait on the rest. the bars are unchanged. arming still goes through arm-auto-merge.yml, so full diff coverage of changed python under src/vouch/ and a closing reference to an issue plind-junior opened are both still required. the owner-issue link is read once up front here too, silently: arm-auto-merge comments when a PR misses that bar, which is right for a human who just asked to arm and would be per-push noise unattended. no checkout, no PR code — only metadata, since the arming job holds a write token. workflow_run resolves this file from the default branch, so it takes effect once it reaches main, not on merge to test.
ci(auto-merge): arm auto-merge unattended when ci goes green
…d-followups fix(digest): drop archived pages from followups-due
plind-junior
enabled auto-merge
July 30, 2026 15:12
plind-junior
disabled auto-merge
July 30, 2026 15:12
There was a problem hiding this comment.
Pull request overview
Adds a read-only ranking introspection surface (kb.explain_ranking / vouch explain-ranking) and tightens operational/testing gates around retrieval, verification, and CI automation. The PR also removes the legacy CodeRabbit/trust-gate machinery in favor of a diff-coverage-based bar plus centralized auto-merge arming workflows.
Changes:
- Add
kb.explain_rankingacross MCP + JSONL + CLI, with a newsrc/vouch/explain_ranking.pyimplementation and end-to-end tests. - Fix/align several read-path behaviors (worthiness
min_score: 0, verify/doctor “missing” externals, digest excluding archived followups, salience excluding retracted claims, quoted"false"config booleans). - Introduce a 100% diff-coverage PR gate and bot comment workflow; refactor auto-merge arming and remove obsolete trust/CodeRabbit gate workflows.
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_worthiness.py | Adds regression test ensuring min_score: 0 is preserved. |
| tests/test_verify.py | Ensures verify_all counts missing externals as failures in audit metadata. |
| tests/test_session_split_renarrate.py | Adds coverage for session re-narration and session-row building behavior. |
| tests/test_salience.py | Adds tests ensuring salience excludes retracted claims (and end-to-end JSONL sidebar behavior). |
| tests/test_retrieval_events.py | Adds regression test for quoted "false" not enabling retrieval events. |
| tests/test_retrieval_backend.py | Adds regression test for quoted "false" not enabling pages_first. |
| tests/test_pr_cache_helpers.py | Adds resilience/contract tests for pr_cache helpers (gh/LLM/network parsing fallbacks). |
| tests/test_pr_bot.py | Removes CodeRabbit/trust-related tests; keeps core path/screenshot parsing tests. |
| tests/test_pr_bot_diff_coverage.py | Adds unit tests for diff-coverage comment rendering + CLI entry behavior. |
| tests/test_migrations_rewriter.py | Adds tests for atomic writes, transform verbs, and artifact enumeration. |
| tests/test_jsonl_server_surface.py | Adds coverage for otherwise-unreached JSONL handlers via handle_request. |
| tests/test_index_db_embeddings.py | Adds coverage for embedding indexing/search paths and write-through behavior. |
| tests/test_health.py | Adds doctor test ensuring missing externals surface as warnings. |
| tests/test_explain_ranking.py | Adds end-to-end tests for the new ranking explanation surface and gates/stages. |
| tests/test_enrich.py | Adds regression test for quoted "false" not enabling capture enrich. |
| tests/test_digest.py | Adds regression test ensuring archived followups are excluded from the digest due list. |
| tests/test_cli_read_list.py | Adds coverage for read/list CLI commands and clean error behavior. |
| tests/test_cli_maintenance.py | Adds coverage for maintenance/health/index CLI commands and non-traceback behavior. |
| tests/test_cli_lifecycle_surface.py | Adds coverage for lifecycle/source/notify CLI commands and prompts/dry-run behavior. |
| tests/test_cli_bundle_and_misc.py | Adds coverage for bundle export/import CLI paths and misc read-only commands. |
| tests/test_bundle.py | Adds test ensuring MCP kb_export honors exclude. |
| tests/embeddings/test_similarity.py | Adds tests for propose-time similarity warnings and degradation behavior. |
| tests/conftest.py | Adds autouse fixture to isolate the global embedder registry per test. |
| src/vouch/worthiness.py | Fixes min_score: 0 parsing so 0.0 isn’t swallowed by fallback logic. |
| src/vouch/verify.py | Treats external_status == "missing" as a failed verify in verify_all audit data. |
| src/vouch/server.py | Adds MCP kb_explain_ranking; updates kb_export to accept exclude. |
| src/vouch/salience.py | Excludes retracted claim statuses from salience computation. |
| src/vouch/retrieval_events.py | Uses coerce_bool so quoted "false" doesn’t enable events. |
| src/vouch/pr_bot.py | Removes CodeRabbit gating logic; adds diff-coverage comment rendering + CLI subcommand. |
| src/vouch/jsonl_server.py | Adds JSONL handler for kb.explain_ranking and registers it. |
| src/vouch/hot_memory.py | Excludes kb.explain_ranking from hot-memory sidebar attachment. |
| src/vouch/health.py | Adds source_missing warning finding when an external source is missing/unreadable. |
| src/vouch/explain_ranking.py | New module implementing retrieval/ranking introspection with per-stage snapshots and gates. |
| src/vouch/enrich.py | Uses coerce_bool so quoted "false" doesn’t enable enrich. |
| src/vouch/digest.py | Excludes archived followup pages from followups_due. |
| src/vouch/context.py | Uses coerce_bool for pages_first.enabled parsing. |
| src/vouch/cli.py | Adds explain-ranking CLI command with text/json output modes. |
| src/vouch/capabilities.py | Registers kb.explain_ranking in capabilities method list. |
| pyproject.toml | Adds numpy + diff-cover to dev extras; adds coverage omit/exclusions and sets fail_under = 90. |
| CHANGELOG.md | Documents new kb.explain_ranking and multiple fixes; notes CI/auto-merge gate changes and removal of CodeRabbit gating. |
| .gitignore | Ignores .coverage.* parallel coverage data files. |
| .github/zizmor.yml | Updates comment referencing removed workflows. |
| .github/workflows/trust-gate.yml | Removes trust-gate workflow. |
| .github/workflows/stale-pr-reaper.yml | Removes stale PR reaper workflow. |
| .github/workflows/diff-coverage-comment.yml | Adds workflow_run-based bot that upserts a diff-coverage PR comment from CI artifacts. |
| .github/workflows/comment-command.yml | Routes /auto-merge through centralized arm-auto-merge.yml. |
| .github/workflows/coderabbit-gate.yml | Removes CodeRabbit verdict→status gate workflow. |
| .github/workflows/ci.yml | Adds diff-coverage job that enforces 100% coverage of changed src/vouch/* lines and uploads reports. |
| .github/workflows/ci-auto-merge.yml | Adds unattended auto-merge arming path after all checks are green on a PR head SHA. |
| .github/workflows/auto-merge.yml | Refactors label-based auto-merge to call arm-auto-merge.yml (no longer blocks core PRs outright). |
| .github/workflows/arm-auto-merge.yml | New reusable workflow that enforces diff-coverage + owner-opened closing issue before arming auto-merge. |
| .coderabbit.yaml | Updates CodeRabbit config commentary to reflect advisory-only posture. |
| hits, used, lex_ranks, sem_ranks, snapshots = _retrieve_traced( | ||
| store, query, limit, viewer | ||
| ) | ||
| rrf_scores = {_key(h): h[3] for h in hits} |
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
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]