diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 18eb065c..9115d9d5 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,11 +1,9 @@ # CodeRabbit configuration — https://docs.coderabbit.ai/guides/configure-coderabbit -# CodeRabbit is the required review gate for vouch (free for this public repo). it -# reviews every non-draft PR automatically. request_changes_workflow is on, so it -# submits a formal approve / request-changes review; the coderabbit-gate workflow -# turns that verdict into the required `coderabbit-approved` status check, so a pr -# only auto-merges once CodeRabbit approves (on top of ci + trust-gate + CODEOWNERS, -# with the owner's auto-merge label as the go signal). a pr CodeRabbit requests -# changes on 3 times is auto-closed (the owner and bots are exempt). +# CodeRabbit reviews every non-draft PR automatically (free for this public repo). +# its verdict is advisory: it gates nothing and closes nothing. the merge path is +# ci + CODEOWNERS, with the owner's auto-merge label as the go signal. +# request_changes_workflow stays on so its stance is legible at a glance, but a +# request-changes review no longer blocks or reaps a pr. language: "en-US" early_access: false reviews: diff --git a/.github/workflows/arm-auto-merge.yml b/.github/workflows/arm-auto-merge.yml new file mode 100644 index 00000000..2a04700f --- /dev/null +++ b/.github/workflows/arm-auto-merge.yml @@ -0,0 +1,115 @@ +name: arm-auto-merge +# the single arming path, called by both authorization surfaces: +# auto-merge.yml (the `auto-merge` label) and comment-command.yml (`/auto-merge`). +# both callers have already established that the actor is the trusted owner — +# this workflow decides whether the PR has earned an unattended merge. +# +# two bars, both read as metadata. nothing here checks out or executes PR code, +# because this job holds a write token. +# +# 1. every changed python line under src/vouch/ is executed by a test +# (the `diff coverage` check, green on this exact head sha). +# 2. the PR closes an issue that plind-junior opened. +# +# together they replace the old blanket refusal to arm `core` PRs: coverage +# says the change is exercised, the issue link says it was asked for. +on: + workflow_call: + inputs: + pr: + description: the pull request number + required: true + type: string + head_sha: + description: >- + head sha to read checks from. pass the sha carried by the + authorizing event where one exists; empty resolves it live. + required: false + default: "" + type: string +permissions: {} +jobs: + arm: + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + checks: read + steps: + - name: resolve the head sha + id: head + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ inputs.pr }} + GIVEN: ${{ inputs.head_sha }} + run: | + sha="$GIVEN" + if [ -z "$sha" ]; then + sha="$(gh pr view "$PR" --repo "$REPO" --json headRefOid --jq .headRefOid)" + fi + echo "sha=$sha" >> "$GITHUB_OUTPUT" + + # the coverage bar, read from ci's own run — never recomputed here, because + # that would mean executing PR code in a workflow that holds a write token. + - name: require the diff-coverage check to have passed + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ inputs.pr }} + HEAD_SHA: ${{ steps.head.outputs.sha }} + run: | + conclusion=$(gh api "repos/$REPO/commits/$HEAD_SHA/check-runs" --paginate \ + --jq '[.check_runs[] | select(.name | startswith("diff coverage"))] + | sort_by(.completed_at) | last | .conclusion' 2>/dev/null || true) + if [ "$conclusion" = "success" ]; then + exit 0 + fi + echo "::error::diff coverage is not green on $HEAD_SHA (conclusion=${conclusion:-missing}); refusing to arm auto-merge" + gh pr edit "$PR" --repo "$REPO" --remove-label auto-merge || true + gh pr comment "$PR" --repo "$REPO" --body \ + "auto-merge was not armed: the \`diff coverage\` check is not green on this head. every python line this PR changes under \`src/vouch/\` must be executed by a test. the bot has commented the uncovered lines; push tests and re-add the auto-merge label." + exit 1 + + # closingIssuesReferences is the resolved link github itself computes from + # `fixes #n` / `closes #n` in the body and commits — not a text match, so a + # bare "#123" mention does not qualify. the issue must be the owner's: an + # unattended merge answers work plind-junior asked for, nothing else. + - name: require a closing issue opened by the owner + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ inputs.pr }} + run: | + # SC2016: the query is single-quoted on purpose — $owner/$name/$pr are + # graphql variables bound by -f/-F, not shell expansions. + # shellcheck disable=SC2016 + owners=$(gh api graphql \ + -f owner="${REPO%/*}" -f name="${REPO#*/}" -F pr="$PR" \ + -f query='query($owner:String!,$name:String!,$pr:Int!){ + repository(owner:$owner,name:$name){ + pullRequest(number:$pr){ + closingIssuesReferences(first:50){nodes{number author{login}}} + } + } + }' \ + --jq '[.data.repository.pullRequest.closingIssuesReferences.nodes[] + | select(.author.login=="plind-junior") | .number] | join(", ")' \ + 2>/dev/null || true) + if [ -n "$owners" ]; then + echo "closes owner-authored issue(s): $owners" + exit 0 + fi + echo "::error::no closing reference to an issue opened by plind-junior; refusing to arm auto-merge" + gh pr edit "$PR" --repo "$REPO" --remove-label auto-merge || true + gh pr comment "$PR" --repo "$REPO" --body \ + "auto-merge was not armed: this PR does not close an issue opened by plind-junior. add a \`fixes #\` line to the PR body pointing at the owner's ticket, then re-add the auto-merge label. a bare \`#\` mention is not a closing reference." + exit 1 + + - name: arm native auto-merge + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ inputs.pr }} + run: | + gh pr merge "$PR" --repo "$REPO" --auto --squash diff --git a/.github/workflows/auto-merge.yml b/.github/workflows/auto-merge.yml index d0a28244..c18329c0 100644 --- a/.github/workflows/auto-merge.yml +++ b/.github/workflows/auto-merge.yml @@ -39,8 +39,6 @@ jobs: permissions: contents: read pull-requests: write - outputs: - klass: ${{ steps.classify.outputs.klass }} steps: - name: the labeler must be the trusted owner (fail closed) env: @@ -54,40 +52,18 @@ jobs: gh pr edit "$PR" --repo "$REPO" --remove-label auto-merge || true exit 1 fi - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 - with: - python-version: "3.12" - - name: classify - id: classify - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - run: | - # REST files endpoint carries previous_filename on renames; the - # GraphQL-backed `gh pr view --json files` shortcut does not. - 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 - klass=$(PYTHONPATH=src python -m vouch.pr_bot classify --files-file changed.txt --print-klass) - echo "klass=$klass" >> "$GITHUB_OUTPUT" - + # core PRs are no longer refused outright. arm-auto-merge decides, on the same + # two bars for every klass: full diff coverage, and a closing reference to an + # issue plind-junior opened. arm: needs: guard - # core PRs are never armed — CODEOWNERS requires the owner's approval. - if: needs.guard.outputs.klass != 'core' - runs-on: ubuntu-latest + # a called workflow can only downgrade the caller's token, and this file + # starts from `permissions: {}` — so the grant has to be made here too. permissions: contents: write pull-requests: write - steps: - - name: arm native auto-merge (non-core) - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - run: | - gh pr merge "$PR" --repo "$REPO" --auto --squash + checks: read + uses: ./.github/workflows/arm-auto-merge.yml + with: + pr: ${{ github.event.pull_request.number }} + head_sha: ${{ github.event.pull_request.head.sha }} diff --git a/.github/workflows/ci-auto-merge.yml b/.github/workflows/ci-auto-merge.yml new file mode 100644 index 00000000..3cbe89c2 --- /dev/null +++ b/.github/workflows/ci-auto-merge.yml @@ -0,0 +1,163 @@ +name: ci-auto-merge +# the unattended arming path: no human acts, the machine decides. +# +# when `ci` finishes green for a pull request, wait for every other check on +# that exact head sha to finish too, and if none of them failed, arm native +# auto-merge through arm-auto-merge.yml. the two bars there are unchanged and +# unweakened by this path: 100% diff coverage of the changed python under +# src/vouch/, and a closing reference to an issue plind-junior opened. a PR +# that clears neither of them is refused here exactly as on the label path. +# +# so an unattended merge needs all three, and the machine checks all three: +# every check green, every changed line tested, and the work was asked for by +# the owner. +# +# nothing here checks out or executes PR code — only metadata is read, because +# the arming job holds a write token. `workflow_run` runs the copy of this file +# on the DEFAULT branch, so this only takes effect once it lands on main. +on: + workflow_run: # zizmor: ignore[dangerous-triggers] runs from the base repo on ci completion; reads metadata only, never checks out or runs PR code + workflows: ["ci"] + types: [completed] +permissions: {} +# a later ci run for the same head supersedes an in-flight wait. +concurrency: + group: ci-auto-merge-${{ github.event.workflow_run.head_sha }} + cancel-in-progress: true +jobs: + resolve: + # only PR runs of ci, and only green ones. a red ci never reaches the wait. + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + checks: read + outputs: + pr: ${{ steps.pr.outputs.pr }} + eligible: ${{ steps.checks.outputs.eligible }} + steps: + # workflow_run.pull_requests is empty for fork PRs — resolve via the + # commit->pulls endpoint instead (base token, no PR code executed). + - name: resolve the pull request + id: pr + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + pr=$(gh api "repos/$REPO/commits/$HEAD_SHA/pulls" --jq '.[0].number' 2>/dev/null || true) + if [ -z "$pr" ] || [ "$pr" = "null" ]; then + echo "no open PR for $HEAD_SHA" + echo "pr=" >> "$GITHUB_OUTPUT" + exit 0 + fi + # the PR must still be open, undrafted, and still sitting on this sha. + # a merged/closed PR, or one that moved on, is not ours to touch. + read -r state draft head < <(gh pr view "$pr" --repo "$REPO" \ + --json state,isDraft,headRefOid --jq '[.state,.isDraft,.headRefOid]|@tsv') + if [ "$state" != "OPEN" ] || [ "$draft" = "true" ] || [ "$head" != "$HEAD_SHA" ]; then + echo "PR #$pr not eligible (state=$state draft=$draft head=$head sha=$HEAD_SHA)" + echo "pr=" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "pr=$pr" >> "$GITHUB_OUTPUT" + + # `ci` is only one of the workflows on a PR — the gates, the score job, + # the schema check and the label jobs are separate. "all the ci passed" + # means all of them, so wait them out rather than trusting branch + # protection (which `test` does not have). + # arm-auto-merge enforces this bar authoritatively and comments when a PR + # misses it. that comment is right for a human who just asked to arm, and + # wrong here — unattended, it would repeat on every push of every PR that + # has no owner ticket. so read the same link first and stay silent. + - name: require a closing issue opened by the owner + id: owner + if: steps.pr.outputs.pr != '' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ steps.pr.outputs.pr }} + run: | + # SC2016: the query is single-quoted on purpose — $owner/$name/$pr are + # graphql variables bound by -f/-F, not shell expansions. + # shellcheck disable=SC2016 + owners=$(gh api graphql \ + -f owner="${REPO%/*}" -f name="${REPO#*/}" -F pr="$PR" \ + -f query='query($owner:String!,$name:String!,$pr:Int!){ + repository(owner:$owner,name:$name){ + pullRequest(number:$pr){ + closingIssuesReferences(first:50){nodes{number author{login}}} + } + } + }' \ + --jq '[.data.repository.pullRequest.closingIssuesReferences.nodes[] + | select(.author.login=="plind-junior") | .number] | join(", ")' \ + 2>/dev/null || true) + if [ -n "$owners" ]; then + echo "closes owner-authored issue(s): $owners" + echo "ok=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "::notice::PR #$PR closes no issue opened by plind-junior; not arming auto-merge" + echo "ok=false" >> "$GITHUB_OUTPUT" + + - name: wait for every check on the head sha, then require none failed + id: checks + if: steps.owner.outputs.ok == 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + RUN_ID: ${{ github.run_id }} + run: | + echo "eligible=false" >> "$GITHUB_OUTPUT" + for _ in $(seq 1 40); do + # this workflow's own check run is excluded: it cannot wait on itself. + runs=$(gh api "repos/$REPO/commits/$HEAD_SHA/check-runs" --paginate \ + --jq ".check_runs[] | select((.details_url // \"\") | contains(\"/runs/$RUN_ID/\") | not) + | [.status, (.conclusion // \"\")] | @tsv") + pending=$(printf '%s\n' "$runs" | grep -cv '^completed' || true) + if [ "$pending" -eq 0 ]; then + # success / skipped / neutral are all "did not fail". anything + # else — failure, cancelled, timed_out, action_required — blocks. + bad=$(printf '%s\n' "$runs" \ + | awk -F'\t' '$2!="success" && $2!="skipped" && $2!="neutral"' | wc -l) + if [ "$bad" -eq 0 ]; then + echo "eligible=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::checks failed on $HEAD_SHA; not arming auto-merge" + fi + exit 0 + fi + sleep 30 + done + echo "::notice::checks still running on $HEAD_SHA after 20m; not arming auto-merge" + + # visible on the PR, and it is what makes deauthorize-on-push announce + # itself when a later push voids this. a label added with GITHUB_TOKEN + # does not re-trigger auto-merge.yml (github's token-recursion guard), + # so this does not double-arm. + - name: mark the PR as machine-authorized + if: steps.checks.outputs.eligible == 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ steps.pr.outputs.pr }} + run: gh pr edit "$PR" --repo "$REPO" --add-label auto-merge || true + + arm: + needs: resolve + if: needs.resolve.outputs.eligible == 'true' + # a called workflow can only downgrade the caller's token, and this file + # starts from `permissions: {}` — so the grant has to be made here too. + permissions: + contents: write + pull-requests: write + checks: read + uses: ./.github/workflows/arm-auto-merge.yml + with: + pr: ${{ needs.resolve.outputs.pr }} + head_sha: ${{ github.event.workflow_run.head_sha }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22dbaa64..c60820f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,84 @@ jobs: name: coverage path: coverage.xml + # the per-pr bar: every python line this pr adds or changes under src/vouch + # must be executed by a test. repo-wide coverage is a separate ratchet + # (pyproject [tool.coverage.report] fail_under) that stops regressions; this + # job is what makes *new* code arrive 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. + diff-coverage: + name: diff coverage (100% of changed python) + if: github.event_name == 'pull_request' + needs: test + runs-on: ubuntu-latest + steps: + # full history: diff-cover diffs the head against the merge base, which + # a shallow clone cannot resolve. + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: pip + + - name: install + run: | + python -m pip install --upgrade pip + pip install 'diff-cover>=9,<10' + + - name: download coverage + uses: actions/download-artifact@v7 + with: + name: coverage + + - name: fetch base branch + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: git fetch --no-tags origin "+refs/heads/$BASE_REF:refs/remotes/origin/$BASE_REF" + + # the reports are written even when the gate fails, so the bot can quote + # the uncovered lines back on the PR. the job's own exit code is the gate. + - name: diff coverage + id: gate + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + set +e + diff-cover coverage.xml \ + --compare-branch "origin/$BASE_REF" \ + --include 'src/vouch/*' \ + --fail-under 100 \ + --json-report diff-coverage.json \ + --markdown-report diff-coverage.md + echo "status=$?" >> "$GITHUB_OUTPUT" + + - name: upload diff-coverage report + if: always() + uses: actions/upload-artifact@v7 + with: + name: diff-coverage + path: | + diff-coverage.json + diff-coverage.md + if-no-files-found: warn + + - name: summary + if: always() + run: | + if [ -f diff-coverage.md ]; then + cat diff-coverage.md >> "$GITHUB_STEP_SUMMARY" + fi + + - name: enforce the gate + env: + STATUS: ${{ steps.gate.outputs.status }} + run: exit "$STATUS" + build: name: build sdist + wheel runs-on: ubuntu-latest diff --git a/.github/workflows/coderabbit-gate.yml b/.github/workflows/coderabbit-gate.yml deleted file mode 100644 index 0479b63d..00000000 --- a/.github/workflows/coderabbit-gate.yml +++ /dev/null @@ -1,87 +0,0 @@ -name: coderabbit-gate -# CodeRabbit is the required review gate. this workflow turns CodeRabbit's review -# verdict into the `coderabbit-approved` commit status the branch ruleset -# requires (so native auto-merge waits for its approval), and auto-closes a -# contributor pr CodeRabbit has requested changes on 3 times. it reads only -# review metadata via the api and checks out the trusted base ref — no untrusted -# head code is ever checked out or run. -on: - pull_request_review: - types: [submitted, dismissed, edited] - pull_request_target: # zizmor: ignore[dangerous-triggers] no untrusted code runs here; only review metadata is read, a commit status is set, and a stale pr may be closed. - types: [opened, reopened, synchronize] -permissions: {} -# a newer event for the same pr supersedes an in-flight run (latest verdict wins). -concurrency: - group: coderabbit-gate-${{ github.event.pull_request.number }} - cancel-in-progress: true -jobs: - gate: - runs-on: ubuntu-latest - permissions: - contents: read # checkout base ref + run pr_bot - statuses: write # publish the coderabbit-approved commit status - pull-requests: write # comment + close after 3 rejected rounds - steps: - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 - with: - python-version: "3.12" - - name: evaluate coderabbit verdict - id: gate - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - AUTHOR: ${{ github.event.pull_request.user.login }} - run: | - gh api "repos/$REPO/pulls/$PR/reviews?per_page=100" > reviews.json - PYTHONPATH=src python -m vouch.pr_bot coderabbit-gate \ - --reviews-file reviews.json --head-sha "$HEAD_SHA" --author "$AUTHOR" \ - >> "$GITHUB_OUTPUT" - - name: publish coderabbit-approved status - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - STATE: ${{ steps.gate.outputs.state }} - VERDICT: ${{ steps.gate.outputs.verdict }} - run: | - case "$VERDICT" in - approved) desc="CodeRabbit approved this commit." ;; - changes) desc="CodeRabbit requested changes — resolve them to merge." ;; - *) desc="Waiting for CodeRabbit to review this commit." ;; - esac - # the GITHUB_TOKEN can't write a commit status onto a fork's head sha — - # POST .../statuses returns 403 "resource not accessible by integration" - # for a pr opened from a fork. that's expected and unfixable with this - # token, so don't fail this infra job over it: the required status simply - # stays unset, native auto-merge waits, and a maintainer merges the fork - # pr by hand. same-repo prs must still publish, so only forks are tolerated. - if gh api --method POST "repos/$REPO/statuses/$HEAD_SHA" \ - -f state="$STATE" -f context="coderabbit-approved" -f description="$desc"; then - exit 0 - fi - if [ "$HEAD_REPO" != "$REPO" ]; then - echo "::warning::could not publish the coderabbit-approved status on a fork pr head ($HEAD_REPO); a maintainer must merge this pr manually (or wire a token with statuses:write for cross-fork writes)." - exit 0 - fi - echo "::error::failed to publish the coderabbit-approved status" - exit 1 - - name: auto-close after 3 rejected rounds - if: steps.gate.outputs.close == 'true' - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - STRIKES: ${{ steps.gate.outputs.strikes }} - run: | - gh pr merge "$PR" --repo "$REPO" --disable-auto || true - gh pr edit "$PR" --repo "$REPO" --remove-label auto-merge || true - gh pr close "$PR" --repo "$REPO" --comment \ - "CodeRabbit requested changes on this pr $STRIKES times without an approval, so it is being closed automatically. the feedback still stands — address it and reopen this pr (or open a fresh one) and it will be reviewed again." diff --git a/.github/workflows/comment-command.yml b/.github/workflows/comment-command.yml index dfee025f..8b3db74b 100644 --- a/.github/workflows/comment-command.yml +++ b/.github/workflows/comment-command.yml @@ -1,7 +1,7 @@ name: comment-command # slash-command trigger: the owner comments `/auto-merge` on a PR to arm # auto-merge, as an alternative to applying the auto-merge label. review is done -# by CodeRabbit + ci; this only arms native auto-merge for non-core PRs. +# by CodeRabbit + ci; arm-auto-merge.yml decides whether the PR clears the bar. on: issue_comment: types: [created] @@ -22,7 +22,6 @@ jobs: pull-requests: write outputs: is_command: ${{ steps.cmd.outputs.is_command }} - klass: ${{ steps.meta.outputs.klass }} steps: - name: detect /auto-merge id: cmd @@ -34,46 +33,28 @@ jobs: else echo "is_command=false" >> "$GITHUB_OUTPUT" fi - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - if: steps.cmd.outputs.is_command == 'true' - with: - persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 - if: steps.cmd.outputs.is_command == 'true' - with: - python-version: "3.12" - - name: classify and mark authorized - id: meta + - name: mark authorized if: steps.cmd.outputs.is_command == 'true' env: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} PR: ${{ github.event.issue.number }} run: | - # REST files endpoint carries previous_filename on renames; the - # GraphQL-backed `gh pr view --json files` shortcut does not. - 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 - klass="$(PYTHONPATH=src python -m vouch.pr_bot classify --files-file changed.txt --print-klass)" # mark authorized (visible, and enables deauthorize-on-push). a label # added via GITHUB_TOKEN does not re-trigger auto-merge.yml (GitHub's # token-recursion guard), so this does not double-arm. gh pr edit "$PR" --repo "$REPO" --add-label auto-merge || true - echo "klass=$klass" >> "$GITHUB_OUTPUT" + # the issue_comment payload carries no head sha; arm-auto-merge resolves it. arm: needs: parse - # core PRs are never armed — CODEOWNERS requires the owner's approval. - if: needs.parse.outputs.is_command == 'true' && needs.parse.outputs.klass != 'core' - runs-on: ubuntu-latest + if: needs.parse.outputs.is_command == 'true' + # a called workflow can only downgrade the caller's token, and this file + # starts from `permissions: {}` — so the grant has to be made here too. permissions: contents: write pull-requests: write - steps: - - name: arm native auto-merge (non-core) - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - PR: ${{ github.event.issue.number }} - run: | - gh pr merge "$PR" --repo "$REPO" --auto --squash + checks: read + uses: ./.github/workflows/arm-auto-merge.yml + with: + pr: ${{ github.event.issue.number }} diff --git a/.github/workflows/diff-coverage-comment.yml b/.github/workflows/diff-coverage-comment.yml new file mode 100644 index 00000000..5c9c95b2 --- /dev/null +++ b/.github/workflows/diff-coverage-comment.yml @@ -0,0 +1,120 @@ +name: diff-coverage-comment +on: + workflow_run: # zizmor: ignore[dangerous-triggers] runs from the base repo on ci completion; checks out the trusted base branch, never the PR head, and never executes PR code + workflows: ["ci"] + types: [completed] +permissions: + contents: read + pull-requests: write + issues: write + actions: read +# one comment per PR head; a newer ci run supersedes an in-flight comment. +concurrency: + group: diff-coverage-comment-${{ github.event.workflow_run.head_sha }} + cancel-in-progress: true +jobs: + comment: + if: github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + steps: + # workflow_run.pull_requests is empty for fork PRs -- resolve via the + # commit->pulls endpoint, the same way ci-label does. + - name: resolve the PR + id: pr + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + pr=$(gh api "repos/$REPO/commits/$HEAD_SHA/pulls" --jq '.[0].number' 2>/dev/null || true) + base=$(gh api "repos/$REPO/commits/$HEAD_SHA/pulls" --jq '.[0].base.ref' 2>/dev/null || true) + if [ -z "$pr" ] || [ "$pr" = "null" ]; then + echo "no open PR for $HEAD_SHA" + echo "found=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + # the base ref feeds actions/checkout. it can only name a branch that + # already exists here, but validate the shape anyway rather than + # trusting an api string in a `ref:` -- fail closed to the integration + # branch if it looks like anything other than a plain branch name. + case "$base" in + ''|*' '*|*'..'*|-*) base='test' ;; + esac + if ! printf '%s' "$base" | grep -qE '^[A-Za-z0-9._/-]{1,100}$'; then + base='test' + fi + { + echo "found=true" + echo "number=$pr" + echo "base=$base" + } >> "$GITHUB_OUTPUT" + + # the diff-coverage job is `needs: test`, so a run that failed at lint or + # type-check produces no artifact. that is not a coverage verdict, so stay + # silent rather than posting a misleading comment. + - name: fetch the diff-coverage report + id: report + if: steps.pr.outputs.found == 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_ID: ${{ github.event.workflow_run.id }} + run: | + id=$(gh api "repos/$REPO/actions/runs/$RUN_ID/artifacts" \ + --jq '.artifacts[] | select(.name=="diff-coverage") | .id' 2>/dev/null | head -1 || true) + if [ -z "$id" ]; then + echo "no diff-coverage artifact on run $RUN_ID" + echo "found=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + gh api "repos/$REPO/actions/artifacts/$id/zip" > dc.zip + unzip -o -q dc.zip + if [ ! -f diff-coverage.json ]; then + echo "artifact carried no json report" + echo "found=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "found=true" >> "$GITHUB_OUTPUT" + + # the base branch is a branch in this repo, so its code is trusted; the PR + # head is never checked out. this is what renders the comment body. + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + if: steps.report.outputs.found == 'true' + with: + ref: ${{ steps.pr.outputs.base }} + persist-credentials: false + path: base + + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + if: steps.report.outputs.found == 'true' + with: + python-version: "3.12" + + # rendered by the tested renderer in vouch.pr_bot, not by yaml. file paths + # in the report come from the PR's own diff, so the body is written to a + # file and posted with --body-file: never interpolated into a shell word. + - name: render the comment + if: steps.report.outputs.found == 'true' + run: | + PYTHONPATH=base/src python -m vouch.pr_bot diff-coverage-comment \ + --report-file diff-coverage.json > comment.md + cat comment.md >> "$GITHUB_STEP_SUMMARY" + + - name: upsert the comment + if: steps.report.outputs.found == 'true' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ steps.pr.outputs.number }} + run: | + marker='' + existing=$(gh api "repos/$REPO/issues/$PR/comments" --paginate \ + --jq "map(select(.body | startswith(\"$marker\"))) | .[0].id" 2>/dev/null || true) + if [ -n "$existing" ] && [ "$existing" != "null" ]; then + gh api --method PATCH "repos/$REPO/issues/comments/$existing" \ + -F body=@comment.md >/dev/null + echo "updated comment $existing on #$PR" + else + gh pr comment "$PR" --repo "$REPO" --body-file comment.md + echo "created a comment on #$PR" + fi diff --git a/.github/workflows/stale-pr-reaper.yml b/.github/workflows/stale-pr-reaper.yml deleted file mode 100644 index 5ef2eeea..00000000 --- a/.github/workflows/stale-pr-reaper.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: stale-pr-reaper -# daily reaper: closes a pr whose author left CodeRabbit's change request -# unaddressed (no new commit) for 2 days. complements the 3-strikes close in -# coderabbit-gate.yml — that fires when the author keeps pushing failing -# commits, this fires when they go silent. scheduled workflows only run from the -# default branch, so this activates once it is on `main`. it reads only api -# metadata and runs from the trusted default branch — no pr head code is run. -on: - schedule: - - cron: "17 6 * * *" # daily ~06:17 UTC - workflow_dispatch: {} -permissions: {} -concurrency: - group: stale-pr-reaper - cancel-in-progress: false -jobs: - reap: - runs-on: ubuntu-latest - permissions: - contents: read # checkout + run pr_bot - pull-requests: write # comment + close - steps: - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 - with: - python-version: "3.12" - - name: close prs stale for 2 days after a change request - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - run: | - now=$(date -u +%s) - gh pr list --repo "$REPO" --state open --limit 100 \ - --json number,headRefOid,author,isDraft \ - --jq '.[] | select(.isDraft|not) | [.number, .headRefOid, .author.login] | @tsv' \ - > prs.tsv - while IFS=$'\t' read -r num sha author; do - [ -z "$num" ] && continue - gh api "repos/$REPO/pulls/$num/reviews?per_page=100" > reviews.json - if PYTHONPATH=src python -m vouch.pr_bot stale-check \ - --reviews-file reviews.json --head-sha "$sha" \ - --author "$author" --now-epoch "$now"; then - echo "reaping #$num (stale after change request)" - gh pr merge "$num" --repo "$REPO" --disable-auto || true - gh pr edit "$num" --repo "$REPO" --remove-label auto-merge || true - gh pr close "$num" --repo "$REPO" --comment \ - "closing automatically: CodeRabbit requested changes and this pr has had no new commits for 2 days. the feedback still stands — push a fix and reopen this pr (or open a fresh one) and it will be reviewed again." - fi - done < prs.tsv diff --git a/.github/workflows/trust-gate.yml b/.github/workflows/trust-gate.yml deleted file mode 100644 index 3b3d85b7..00000000 --- a/.github/workflows/trust-gate.yml +++ /dev/null @@ -1,45 +0,0 @@ -name: trust-gate -on: - pull_request: - types: [opened, synchronize, reopened, edited] -permissions: - contents: read -jobs: - trust-gate: - runs-on: ubuntu-latest - steps: - # check out the BASE ref so the classification logic is trusted, never the - # PR head (which could tamper with pr_bot.py — itself a core path). - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 - with: - python-version: "3.12" - - name: list changed files - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - run: | - # the REST files endpoint (unlike `gh pr view --json files`) carries - # previous_filename on renames — required so a rename that lands a - # core path under a new name still classifies as core. - 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 - - name: fail if an untrusted author touched core - env: - ASSOC: ${{ github.event.pull_request.author_association }} - ACTOR: ${{ github.event.pull_request.user.login }} - run: | - if PYTHONPATH=src python -m vouch.pr_bot trust \ - --author-association "$ASSOC" --actor "$ACTOR"; then - echo "trusted author — core edits allowed" - exit 0 - fi - if PYTHONPATH=src python -m vouch.pr_bot core-touched --files-file changed.txt; then - echo "::error::untrusted author modified a core path; core changes need owner review" - exit 1 - fi - echo "untrusted author, no core paths touched — ok" diff --git a/.github/zizmor.yml b/.github/zizmor.yml index 689e9ea9..ed7b8adf 100644 --- a/.github/zizmor.yml +++ b/.github/zizmor.yml @@ -1,6 +1,6 @@ # zizmor configuration — https://docs.zizmor.sh/configuration/ # -# the pr-bot workflows (auto-merge, trust-gate, ci-label, ui-screenshot-gate, +# the pr-bot workflows (auto-merge, ci-label, ui-screenshot-gate, # workflow-lint) pin their actions and annotate their triggers. these three # audits are disabled repo-wide so the new workflow-lint gate does not force a # full pin / least-permissions migration of the older workflows in a single diff --git a/.gitignore b/.gitignore index 5ca105ec..7809057b 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,8 @@ __pycache__/ .mypy_cache/ .ruff_cache/ .coverage +# subprocess-spawning tests leave per-pid parallel data files behind +.coverage.* coverage.xml htmlcov/ bench.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 5141d497..a0c27843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,43 @@ All notable changes to vouch are documented here. Format follows ## [Unreleased] +### Added +- **`kb.explain_ranking` — why a result ranked where it did** (#432): a + read-only breakdown of the retrieval pipeline. Per candidate it reports the + lexical (FTS5) rank, the semantic rank, the RRF contribution, a row for every + stage — fusion, scope and status filters, recency, pages-first, rerank, the + pluggable strategy, the limit window, and the optional budget/citation gates + — with the rank and score delta that stage caused, plus the gate that kept or + dropped it (`kept` / `scope-filtered` / `status-filtered` / `limit-dropped` / + `budget-dropped` / `uncited`). Registered on MCP, JSONL and the CLI + (`vouch explain-ranking "" [--format text|json]`). Viewer-scoped + through the same `filter_hits` as `kb.context`, so it cannot expose an + artifact the caller could not already retrieve, and it touches no write path. + ### Fixed +- **digest drops archived followup pages** (#625): + `followups_due` already skipped `done`/`dropped` metadata, but an + `ARCHIVED` page with `followup_status=open` and a past `due_at` still + appeared every morning — stale claims were filtered, pages were not. + mirror recall: archived followups leave the due list. +- **`verify_all` / `doctor` treat missing externals like drift** (#622): + `vouch source verify` already marked `external_status=missing` as `!`, + but `verify_all`'s audit `failed` list and `health.doctor` only looked + for `drift`, so a deleted upstream file could leave doctor `ok: true` + while the CLI failed. missing now joins the failed set and emits a + `source_missing` warning. +- **salience reflex excludes retracted claims**: `compute_salience` scanned + every claim regardless of status, so the `_meta.vouch_salience` sidebar + counted `ARCHIVED` / `SUPERSEDED` / `REDACTED` claims in `claim_count` and + could name one as an entity's `top_claim_id` — pointing agents at knowledge + the archive/supersede/redact controls were supposed to retire. the scan now + applies the same lifecycle filter as the scope filter beside it. an entity + whose only claims are retracted still appears, reporting zero live claims. +- **config quoted `"false"` disables enrich / events / pages_first** (#620): + `#558` left three loaders on bare `bool()`, so a quoted `enabled: "false"` + left `capture.enrich` and `retrieval.events` on, and turned + `retrieval.pages_first` on. all three now use `coerce_bool` like the + other config loaders. - **vault sync mirrors post-approve WORKING/DRAFT artifacts** (#583): `kb_to_vault` now includes durable `WORKING` claims and `DRAFT` pages (the propose+approve defaults), so Obsidian mirrors fill without @@ -39,6 +75,36 @@ All notable changes to vouch are documented here. Format follows markers; absolute bench scores shift, paired comparisons were fair either way. the reference baseline table is refreshed. ### Changed +- **core PRs can auto-merge, on two mechanical bars.** the blanket "core + is never armed" refusal is gone; both authorization surfaces (the + `auto-merge` label and the `/auto-merge` comment) now route through one + reusable `arm-auto-merge.yml`, which arms any klass only when the + `diff coverage` check is green on that head sha *and* the PR carries a + closing reference to an issue plind-junior opened. neither bar is + recomputed in the write-token job — both are read as metadata, so no PR + code executes there. the owner-only guard and deauthorize-on-push are + unchanged, and folding the two arming paths into one file removes the + drift that left `/auto-merge` with no coverage check at all. +- **CodeRabbit's verdict no longer gates anything.** the + `coderabbit-approved` commit status, the 3-strike auto-close, and the + daily stale-pr reaper are removed, along with the `coderabbit-gate` and + `stale-check` pr_bot commands that computed them. the status had + already been dropped from the `test` ruleset's required checks, so this + removes the machinery that outlived it rather than lowering a live bar. + CodeRabbit still reviews every non-draft pr and still files formal + approve / request-changes reviews — they are advisory now. the merge + path is ci + CODEOWNERS, with the owner's auto-merge label as the go + signal. +- **the `trust-gate` workflow is removed.** it failed a pr when an author + outside the OWNER association touched a core path — a bar that the + rewritten `arm-auto-merge.yml` already enforces from the other side: + nothing arms without the owner's own label, green `diff coverage`, and a + closing reference to an owner-opened issue, and CODEOWNERS still holds + the review requirement on core paths. the `trust` pr_bot command and its + `is_trusted` helper go with it. core-path classification stays — it is + what `arm-auto-merge.yml` reads. **remove `trust-gate` from the `test` + ruleset's required checks**, or every pr will block on a check that no + longer reports. - **auto approval is the default** (`review.approver_role: trusted-agent` in the starter config): a fresh KB approves the capturing agent's proposals with no human step. nothing bypasses the gate — every write diff --git a/pyproject.toml b/pyproject.toml index 267c3a71..623a0e23 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,6 +47,15 @@ dev = [ "httpx>=0.28,<1", # the wheel-contents regression test builds with the real backend "hatchling>=1.21", + # numpy alone unlocks tests/embeddings: MockEmbedder is a pure-python fake, + # so the vector paths are testable without torch/onnxruntime. without it + # `importorskip("numpy")` skipped ~470 statements of shipped code in every + # ci run, and nothing else in the repo installed it. + "numpy>=1.26,<3", + # the `diff coverage` ci gate: every python line a pr adds or changes under + # src/vouch must be executed by a test. repo-wide coverage is a ratchet + # (see [tool.coverage.report] fail_under); this is the per-pr bar. + "diff-cover>=9,<10", ] embeddings-fast = [ "fastembed>=0.3,<1", @@ -104,6 +113,39 @@ markers = [ "integration: tests that load the real embedding model (slow, network on first run)", ] +# coverage is measured by the ci `test` job (`pytest --cov=vouch`). +# +# `--cov=vouch` matches *every* importable copy of the package, and +# test_wheel_contents installs the built wheel into a pytest temp dir. without +# the omit below those 58 duplicate modules land in the report at ~0% and drag +# the total from 85% to 43% -- the number ci has been uploading. +[tool.coverage.run] +omit = [ + "*/pytest-of-*/*", + "*/test_installed_wheel_resolves_*/*", +] + +# the exclusions below are constructs a unit test cannot execute by +# construction -- type-checker-only imports and the `python -m` entrypoint +# guards. they are not a place to park untested branches; an untested `except` +# body stays a gap. +# +# `omit` is repeated here on purpose: 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. +[tool.coverage.report] +omit = [ + "*/pytest-of-*/*", + "*/test_installed_wheel_resolves_*/*", +] +exclude_also = [ + "if TYPE_CHECKING:", + "if __name__ == .__main__.:", +] +show_missing = true +# ratchet. raise it as gaps close, never lower it to make a red build green. +fail_under = 90 + # numpy is an optional runtime dependency (pulled in by the [embeddings] or # [embeddings-fast] extras); the base CI install only has [dev], so mypy can't # resolve `import numpy as np` in the embedding-stack modules. Silence the diff --git a/src/vouch/capabilities.py b/src/vouch/capabilities.py index 86c348f7..8b69839f 100644 --- a/src/vouch/capabilities.py +++ b/src/vouch/capabilities.py @@ -35,6 +35,7 @@ "kb.activity", "kb.digest", "kb.search", + "kb.explain_ranking", "kb.neighbors", "kb.experts", "kb.context", diff --git a/src/vouch/cli.py b/src/vouch/cli.py index c7ba6d9d..0801242b 100644 --- a/src/vouch/cli.py +++ b/src/vouch/cli.py @@ -3456,6 +3456,77 @@ def search( click.echo(f"{k}/{i}\t{snip} ({used})") +@cli.command("explain-ranking") +@click.argument("query") +@click.option("--limit", "-n", default=10, show_default=True, type=int) +@click.option("--max-chars", default=None, type=int, + help="Also explain kb.context's budget gate at this character cap.") +@click.option("--require-citations", is_flag=True, + help="Also explain the uncited-claim gate.") +@click.option("--format", "fmt", type=click.Choice(["text", "json"]), + default="text", show_default=True) +@click.option("--project", default=None, help="Viewer project for scope filtering.") +@click.option("--agent", default=None, help="Viewer agent for scope filtering.") +def explain_ranking_cmd( + query: str, + limit: int, + max_chars: int | None, + require_citations: bool, + fmt: str, + project: str | None, + agent: str | None, +) -> None: + """Explain why each candidate for QUERY ranked where it did.""" + from .explain_ranking import explain_ranking + + store = _load_store() + with _cli_errors(): + result = explain_ranking( + store, + query=query, + limit=limit, + max_chars=max_chars, + require_citations=require_citations, + project=project, + agent=agent, + ) + + if fmt == "json": + _emit_json(result) + return + + retrieval = result["retrieval"] + stages = retrieval["stages"] + click.echo( + f"backend: {retrieval['used']} (configured {retrieval['configured']}, " + f"semantic {'available' if retrieval['semantic_available'] else 'unavailable'})" + ) + active = [name for name in ("fusion", "recency", "pages_first", "rerank") + if stages.get(name)] + if stages.get("strategy"): + active.append(f"strategy={stages['strategy']}") + click.echo(f"stages active: {', '.join(active) if active else 'none'}") + + for cand in result["candidates"]: + click.echo( + f"\n{cand['kind']}/{cand['id']} gate={cand['gate']}" + f" lexical={cand['lexical_rank']} semantic={cand['semantic_rank']}" + f" rrf={cand['rrf_contribution']}" + ) + for row in cand["stages"]: + # a stage that did not run is shown so the chain reads continuously, + # flagged rather than silently absent. + flag = "" if row["applied"] else " (off)" + drank = row["rank_delta"] + dscore = row["score_delta"] + moved = "" if not drank else f" rank{drank:+d}" + shifted = "" if not dscore else f" score{dscore:+.6f}" + click.echo( + f" {row['stage']:<14} rank={row['rank']} " + f"score={row['score']:.6f}{moved}{shifted}{flag}" + ) + + @cli.command() @click.argument("node_id") @click.option("--depth", default=1, show_default=True, type=int) diff --git a/src/vouch/context.py b/src/vouch/context.py index 36a8f637..12c44912 100644 --- a/src/vouch/context.py +++ b/src/vouch/context.py @@ -21,6 +21,7 @@ from . import graph, hot_memory, index_db, retrieval_events from . import strategy as strategy_mod +from .config_coerce import coerce_bool from .embeddings.fusion import rrf_fuse from .models import ( ClaimStatus, @@ -244,7 +245,7 @@ def _configured_pages_first(store: KBStore) -> tuple[bool, float]: boost = 1.25 if boost <= 0: boost = 1.25 - return bool(raw.get("enabled", False)), boost + return coerce_bool(raw.get("enabled", False), False), boost def _maybe_pages_first( diff --git a/src/vouch/digest.py b/src/vouch/digest.py index d7bae664..e699bda7 100644 --- a/src/vouch/digest.py +++ b/src/vouch/digest.py @@ -20,7 +20,7 @@ from typing import Any from .metrics import DEFAULT_STALE_DAYS, compute -from .models import ClaimStatus, ProposalStatus +from .models import ClaimStatus, PageStatus, ProposalStatus from .page_filters import filter_pages from .storage import KBStore @@ -184,6 +184,8 @@ def build( kind="followup", before={"due_at": now.date().isoformat()}, ) + # mirror recall/search: archived pages are out of the live set even when + # metadata still says followup_status=open and due_at is past. followup_rows = sorted( ( FollowupRow( @@ -194,7 +196,9 @@ def build( followup_status=str(p.metadata.get("followup_status", "")), ) for p in due_pages - if str(p.metadata.get("followup_status", "")) not in _CLOSED_FOLLOWUP_STATUSES + if p.status is not PageStatus.ARCHIVED + and str(p.metadata.get("followup_status", "")) + not in _CLOSED_FOLLOWUP_STATUSES ), key=lambda r: r.due_at, )[:limit] diff --git a/src/vouch/enrich.py b/src/vouch/enrich.py index 3c019386..1a69275d 100644 --- a/src/vouch/enrich.py +++ b/src/vouch/enrich.py @@ -36,6 +36,7 @@ import yaml from . import llm_draft +from .config_coerce import coerce_bool from .llm_draft import LLMDraftError from .storage import KBStore @@ -108,7 +109,7 @@ def load_enrich_config(store: KBStore) -> EnrichConfig: return EnrichConfig() llm_cmd = raw.get("llm_cmd") return EnrichConfig( - enabled=bool(raw.get("enabled", True)), + enabled=coerce_bool(raw.get("enabled", True), True), llm_cmd=str(llm_cmd) if llm_cmd else None, timeout_seconds=_coerce( raw.get("timeout_seconds", DEFAULT_TIMEOUT_SECONDS), diff --git a/src/vouch/explain_ranking.py b/src/vouch/explain_ranking.py new file mode 100644 index 00000000..2ec7871b --- /dev/null +++ b/src/vouch/explain_ranking.py @@ -0,0 +1,322 @@ +"""Read-only introspection over the retrieval ranking pipeline — issue #432. + +``_retrieve`` returns each hit as ``(kind, id, summary, score, backend)``: one +opaque score and a backend label. A reviewer tuning fusion, the reranker, or +the recency / pages-first signals has no way to see *why* an artifact surfaced +or got dropped — how much came from lexical vs. semantic rank, what the RRF +contribution was, whether a rescoring stage moved it, or which gate cut it. + +This module re-runs those stages against the same helpers ``context`` uses, +snapshotting 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. + +The composition is deliberate rather than a copy of any single caller: it +chains ``_retrieve``'s ranking stages, the lifecycle gate ``search_kb`` +applies, and the budget / citation gates ``build_context_pack`` applies, so +one call explains every stage an artifact can die at. Scope filtering runs +without a limit and truncation is a separate ``limit`` stage — the same +"scope first so status filtering can refill the window" ordering ``search_kb`` +uses, and it keeps a candidate lost to truncation attributable instead of +folding it into the scope filter. + +Two stage shapes matter when reading a breakdown: + +* ``recency`` and ``pages_first`` are rescoring-only — the candidate set is + unchanged, so their signal is the score delta. +* ``rerank`` and ``strategy`` are ordering-only — scores are untouched, so + their signal is the rank delta. + +``strategy`` is the pluggable final reorder; it is reported as a stage so a +shipped ranking plugin's effect is visible rather than folded into the score +it did not change. + +Read-only by construction: every helper called here is one the read path +already uses, and nothing writes, proposes, or mutates the KB. Viewer scoping +runs through the same ``filter_hits`` as ``kb.context``, so a caller cannot +see a candidate it could not already retrieve. +""" + +from __future__ import annotations + +import sqlite3 +from dataclasses import dataclass, field +from typing import Any + +from . import index_db +from .context import ( + _configured_backend, + _configured_pages_first, + _configured_recency, + _configured_rerank, + _configured_strategy, + _filter_live_hits, + _maybe_pages_first, + _maybe_recency, + _maybe_rerank, + _maybe_strategy, +) +from .embeddings.fusion import rrf_fuse +from .scoping import ViewerContext, filter_hits, scoped_fetch_limit, viewer_from +from .storage import KBStore + +Hit = tuple[str, str, str, float] +Key = tuple[str, str] + +DEFAULT_LIMIT = 10 + +# Stage name -> the gate reported for a candidate that stage removed. +_GATE_FOR_STAGE = { + "scope_filter": "scope-filtered", + "status_filter": "status-filtered", + "limit": "limit-dropped", + "budget": "budget-dropped", +} + + +def _key(hit: Hit) -> Key: + return (hit[0], hit[1]) + + +@dataclass +class _Snapshot: + """The candidate set after one pipeline stage.""" + + stage: str + applied: bool + ranks: dict[Key, int] = field(default_factory=dict) + scores: dict[Key, float] = field(default_factory=dict) + + @classmethod + def of(cls, stage: str, hits: list[Hit], *, applied: bool = True) -> _Snapshot: + return cls( + stage=stage, + applied=applied, + ranks={_key(h): i for i, h in enumerate(hits, start=1)}, + scores={_key(h): h[3] for h in hits}, + ) + + +def _retrieve_traced( + store: KBStore, + query: str, + limit: int, + viewer: ViewerContext, +) -> tuple[list[Hit], str, dict[Key, int], dict[Key, int], list[_Snapshot]]: + """Re-run ``_retrieve``'s backend selection, keeping the per-retriever ranks. + + Returns the fused hits, the backend that served them, the lexical and + semantic rank maps, and the fusion snapshot. Mirrors ``_retrieve``'s + branching so the explanation describes the query that would actually run. + """ + backend = _configured_backend(store) + fetch_limit = scoped_fetch_limit(limit, viewer) + sem: list[Hit] = [] + lex: list[Hit] = [] + + def _lexical() -> list[Hit]: + try: + return index_db.search(store.kb_dir, query, limit=fetch_limit) + except sqlite3.Error: + return [] + + if backend in ("auto", "hybrid"): + sem = index_db.search_semantic(store.kb_dir, query, limit=fetch_limit) + lex = _lexical() + fused = rrf_fuse(sem, lex, limit=fetch_limit) + if fused: + used = "hybrid" + else: + # Both retrievers came back empty: _retrieve falls through to the + # substring scan, which applies none of the rescoring stages. + fused = store.search_substring(query, limit=fetch_limit) + used = "substring" + elif backend == "embedding": + sem = index_db.search_semantic(store.kb_dir, query, limit=fetch_limit) + fused, used = sem, "embedding" + elif backend == "fts5": + lex = _lexical() + fused, used = lex, "fts5" + else: + fused, used = store.search_substring(query, limit=fetch_limit), "substring" + + lex_ranks = {_key(h): i for i, h in enumerate(lex, start=1)} + sem_ranks = {_key(h): i for i, h in enumerate(sem, start=1)} + return fused, used, lex_ranks, sem_ranks, [_Snapshot.of(used, fused)] + + +def _budget_survivors(hits: list[Hit], max_chars: int) -> list[Hit]: + """The hits ``build_context_pack`` would keep under a ``max_chars`` budget. + + Mirrors the pack's omission pass — drop from the tail until the summary + total fits. The pack's clipping pass shortens a summary rather than + dropping the item, so it never changes the candidate set and is reported + as a stage that kept everything. + """ + kept = list(hits) + while kept and sum(len(h[2]) for h in kept) > max_chars: + kept.pop() + return kept + + +def _is_uncited(store: KBStore, key: Key) -> bool: + """True for a surviving claim that carries no citations. + + ``require_citations`` does not drop an item — ``build_context_pack`` keeps + it and fails the pack (``failed: ["require_citations"]``). So this is not a + drop stage: it renames the gate on the candidate that is *responsible* for + that failure, which is the artifact a reviewer needs to find. + + No missing-artifact guard: only candidates that survived ``status_filter`` + reach here, and that stage already dropped every claim it could not read. + + Defensive today — ``Claim`` rejects ``evidence=[]`` on the model, which + closes every write path (``models.py``: "claim must cite at least one + Source or Evidence id"), so no stored claim can be uncited. It mirrors the + check ``build_context_pack`` still makes, and starts reporting the moment + that invariant is relaxed rather than silently reporting ``kept``. + """ + if key[0] != "claim": + return False + return not store.get_claim(key[1]).evidence + + +def _stage_rows( + key: Key, + snapshots: list[_Snapshot], +) -> tuple[list[dict[str, Any]], str]: + """Per-stage rows for one candidate, plus the gate that decided its fate.""" + rows: list[dict[str, Any]] = [] + gate = "kept" + prev_rank: int | None = None + prev_score: float | None = None + + for snap in snapshots: + if key not in snap.ranks: + # Absent here but present in the previous snapshot -> this stage + # removed it. A stage that did not run cannot have dropped it. + if prev_rank is not None and snap.applied: + gate = _GATE_FOR_STAGE.get(snap.stage, f"{snap.stage}-dropped") + break + rank, score = snap.ranks[key], snap.scores[key] + rows.append({ + "stage": snap.stage, + "applied": snap.applied, + "rank": rank, + "score": round(score, 6), + "rank_delta": None if prev_rank is None else prev_rank - rank, + "score_delta": None if prev_score is None else round(score - prev_score, 6), + }) + prev_rank, prev_score = rank, score + + return rows, gate + + +def explain_ranking( + store: KBStore, + *, + query: str, + limit: int = DEFAULT_LIMIT, + max_chars: int | None = None, + require_citations: bool = False, + project: str | None = None, + agent: str | None = None, +) -> dict[str, Any]: + """Explain why each candidate for *query* ranked where it did. + + Returns ``{"query", "limit", "viewer", "retrieval", "candidates"}``. + Each candidate carries its lexical and semantic rank, its RRF + contribution, a row per pipeline stage, and the gate that kept or + dropped it. Read-only — no write path is touched. + """ + if limit < 0: + raise ValueError("limit must be >= 0") + + viewer = viewer_from(config_path=store.config_path, project=project, agent=agent) + hits, used, lex_ranks, sem_ranks, snapshots = _retrieve_traced( + store, query, limit, viewer + ) + rrf_scores = {_key(h): h[3] for h in hits} + + scoped = filter_hits(store, hits, viewer) + snapshots.append(_Snapshot.of("scope_filter", scoped)) + + live = _filter_live_hits(store, scoped) + snapshots.append(_Snapshot.of("status_filter", live)) + + recency_on, _half_life = _configured_recency(store) + rescored = _maybe_recency(store, hits=live) + snapshots.append(_Snapshot.of("recency", rescored, applied=recency_on)) + + pages_first_on, _boost = _configured_pages_first(store) + boosted = _maybe_pages_first(store, hits=rescored) + snapshots.append(_Snapshot.of("pages_first", boosted, applied=pages_first_on)) + + rerank_on, rerank_top_k = _configured_rerank(store, limit=limit) + reranked = _maybe_rerank(store, query=query, hits=boosted, limit=limit) + snapshots.append(_Snapshot.of("rerank", reranked, applied=rerank_on)) + + # _maybe_strategy speaks the 5-tuple retrieval shape (backend appended); + # carry `used` through and drop it again so the stage list stays uniform. + strategy_name = _configured_strategy(store) + ordered = [ + (k, i, s, sc) + for k, i, s, sc, _be in _maybe_strategy( + store, + query=query, + hits=[(k, i, s, sc, used) for k, i, s, sc in reranked], + limit=limit, + ) + ] + snapshots.append( + _Snapshot.of("strategy", ordered, applied=strategy_name is not None) + ) + + windowed = ordered[:limit] + snapshots.append(_Snapshot.of("limit", windowed)) + + if max_chars is not None: + windowed = _budget_survivors(windowed, max_chars) + snapshots.append(_Snapshot.of("budget", windowed)) + + # Every candidate the pipeline ever saw, in the order fusion produced them, + # so a dropped artifact is still explained rather than silently missing. + candidates: list[dict[str, Any]] = [] + summaries = {_key(h): h[2] for h in hits} + for hit in hits: + key = _key(hit) + rows, gate = _stage_rows(key, snapshots) + if gate == "kept" and require_citations and _is_uncited(store, key): + gate = "uncited" + candidates.append({ + "kind": key[0], + "id": key[1], + "summary": summaries.get(key, ""), + "lexical_rank": lex_ranks.get(key), + "semantic_rank": sem_ranks.get(key), + "rrf_contribution": round(rrf_scores.get(key, 0.0), 6), + "stages": rows, + "gate": gate, + }) + + return { + "query": query, + "limit": limit, + "viewer": {"project": viewer.project, "agent": viewer.agent}, + "retrieval": { + "configured": _configured_backend(store), + "used": used, + "semantic_available": index_db.semantic_search_available(), + "stages": { + "fusion": used == "hybrid", + "recency": recency_on, + "pages_first": pages_first_on, + "rerank": rerank_on, + "rerank_top_k": rerank_top_k if rerank_on else None, + "strategy": strategy_name, + "budget": max_chars, + "require_citations": require_citations, + }, + }, + "candidates": candidates, + } diff --git a/src/vouch/health.py b/src/vouch/health.py index cb474448..ce0269a2 100644 --- a/src/vouch/health.py +++ b/src/vouch/health.py @@ -313,6 +313,17 @@ def doctor(store: KBStore) -> HealthReport: [vr.source.id], ) ) + elif vr.external_status == "missing": + detail = f" ({vr.note})" if vr.note else "" + report.findings.append( + Finding( + "warning", + "source_missing", + f"external file {vr.source.locator} missing or unreadable" + f" since registration{detail}", + [vr.source.id], + ) + ) # Config sanity. if not store.config_path.exists(): diff --git a/src/vouch/hot_memory.py b/src/vouch/hot_memory.py index d5a7f465..266d16ff 100644 --- a/src/vouch/hot_memory.py +++ b/src/vouch/hot_memory.py @@ -146,6 +146,9 @@ def mark_volunteered(session_id: str, claim_id: str, *, pushed_at: float) -> Non "kb.neighbors": "graph slice — out of scope for recency sidebar", "kb.synthesize": "answer-mode prose — sidebar adds noise", "kb.diff": "field-level revision diff — self-contained, not a claim browse", + "kb.explain_ranking": ( + "ranking diagnostic — a recency sidebar would perturb the output being inspected" + ), "kb.detect_themes": "cluster analysis — self-contained, not a claim browse", "kb.experts": "ranked entity analysis — self-contained, not a claim browse", "kb.triage_pending": ( diff --git a/src/vouch/jsonl_server.py b/src/vouch/jsonl_server.py index a88eaf1b..80fada66 100644 --- a/src/vouch/jsonl_server.py +++ b/src/vouch/jsonl_server.py @@ -171,6 +171,23 @@ def _h_search(p: dict) -> dict: ) +def _h_explain_ranking(p: dict) -> dict: + from .explain_ranking import explain_ranking + + # One shared implementation across MCP / JSONL / CLI — see + # explain_ranking.explain_ranking. Read-only: no write path is touched. + max_chars = p.get("max_chars") + return explain_ranking( + _store(), + query=p["query"], + limit=int(p.get("limit", 10)), + max_chars=None if max_chars is None else int(max_chars), + require_citations=bool(p.get("require_citations", False)), + project=p.get("project"), + agent=p.get("agent"), + ) + + def _load_cfg(store: KBStore) -> dict: try: loaded = yaml.safe_load((store.kb_dir / "config.yaml").read_text(encoding="utf-8")) @@ -894,6 +911,7 @@ def _h_propose_theme(p: dict) -> dict: "kb.activity": _h_activity, "kb.digest": _h_digest, "kb.search": _h_search, + "kb.explain_ranking": _h_explain_ranking, "kb.neighbors": _h_neighbors, "kb.experts": _h_experts, "kb.context": _h_context, diff --git a/src/vouch/pr_bot.py b/src/vouch/pr_bot.py index 246fbbcc..b27fe848 100644 --- a/src/vouch/pr_bot.py +++ b/src/vouch/pr_bot.py @@ -2,11 +2,10 @@ Pure stdlib — no model dependency, no vouch-runtime imports. The CI workflows call ``python -m vouch.pr_bot `` for every decision that must be -trustworthy: an author's trust tier, whether a PR touches core/ui paths, whether -a UI PR carries before/after screenshots, and whether a labeled PR may arm -native auto-merge. CodeRabbit is the review gate and runs as a GitHub App, not -here — this module only turns its verdict into the required `coderabbit-approved` -commit status and the deterministic calls that gate the merge. +trustworthy: whether a PR touches core/ui paths, whether a UI PR carries +before/after screenshots, and whether a labeled PR may arm native auto-merge. +CodeRabbit runs as a GitHub App and still comments on PRs, +but its verdict no longer gates anything — nothing here reads it. """ from __future__ import annotations @@ -15,7 +14,6 @@ import re import sys from collections.abc import Iterable, Mapping, Sequence -from datetime import UTC, datetime from typing import Any # the review-gate core: writes here are the north star. mirrored verbatim in @@ -45,21 +43,6 @@ "webapp/**", ) -_OWNER_ASSOCIATION = "OWNER" -_BOT_ACTORS = frozenset({"dependabot[bot]"}) - -# CodeRabbit is the required review gate (.coderabbit.yaml). only reviews it -# authors on github count; anyone else's approval never satisfies the gate. -CODERABBIT_LOGIN = "coderabbitai[bot]" - -# a contributor gets STRIKE_LIMIT rounds of "changes requested" from CodeRabbit -# before the pr is auto-closed. the owner and bots are exempt (author_is_exempt). -STRIKE_LIMIT = 3 - -# a pr whose author leaves CodeRabbit's change request unaddressed (no new -# commit) for STALE_DAYS is auto-closed by the scheduled stale-pr-reaper. -STALE_DAYS = 2 -_EXEMPT_AUTHORS = frozenset({"plind-junior"}) | _BOT_ACTORS def _match(path: str, glob: str) -> bool: @@ -87,10 +70,6 @@ def klass(changed: Sequence[str]) -> str: return "core" if c["is_core"] else "ui" if c["is_ui"] else "code" -def is_trusted(author_association: str, actor: str) -> bool: - return author_association == _OWNER_ASSOCIATION or actor in _BOT_ACTORS - - _GH_IMAGE = re.compile( r"""(?:!\[[^\]]*\]\(\s*|]*\bsrc\s*=\s*["']?)""" r"""(?:https?://(?:user-images\.githubusercontent\.com/""" @@ -115,91 +94,6 @@ def should_arm_automerge(*, is_core: bool, ci_passing: bool, return claude_verdict == "APPROVE" -def _cr_verdicts(reviews: Sequence[Mapping[str, Any]], *, - login: str) -> list[tuple[str, Any]]: - """(state, commit_id) for CodeRabbit reviews carrying a verdict. - - COMMENTED and DISMISSED reviews carry no verdict and are dropped. - """ - out: list[tuple[str, Any]] = [] - for r in reviews: - if (r.get("user") or {}).get("login") != login: - continue - state = str(r.get("state") or "").upper() - if state in ("APPROVED", "CHANGES_REQUESTED"): - out.append((state, r.get("commit_id"))) - return out - - -def coderabbit_verdict(reviews: Sequence[Mapping[str, Any]], *, - head_sha: str | None = None, - login: str = CODERABBIT_LOGIN) -> tuple[str, int]: - """CodeRabbit's (verdict, strikes) for a pr's review list. - - ``verdict`` is its stance on ``head_sha`` — 'approved', 'changes', or - 'pending' when it has not yet reviewed that commit (so a fresh push voids - a prior approval). ``strikes`` counts the distinct commits it has requested - changes on, i.e. failed review rounds, across the pr's whole history. - """ - verdicts = _cr_verdicts(reviews, login=login) - strikes = len({cid for state, cid in verdicts if state == "CHANGES_REQUESTED"}) - scoped = [v for v in verdicts if head_sha is None or v[1] == head_sha] - if not scoped: - return "pending", strikes - return ("approved" if scoped[-1][0] == "APPROVED" else "changes"), strikes - - -def gate_status(verdict: str) -> str: - """Commit-status state for the required `coderabbit-approved` check.""" - return {"approved": "success", "changes": "failure"}.get(verdict, "pending") - - -def author_is_exempt(author: str) -> bool: - """The owner and bots are never auto-closed for failed reviews.""" - return author in _EXEMPT_AUTHORS - - -def should_close(verdict: str, strikes: int, *, author: str, - limit: int = STRIKE_LIMIT) -> bool: - """Auto-close a contributor pr CodeRabbit has rejected `limit` rounds.""" - return (not author_is_exempt(author) - and verdict == "changes" - and strikes >= limit) - - -def _iso_epoch(s: str) -> float: - """Epoch seconds for a github ISO8601 timestamp (e.g. 2026-07-15T10:20:30Z).""" - dt = datetime.fromisoformat(s.replace("Z", "+00:00")) - if dt.tzinfo is None: - dt = dt.replace(tzinfo=UTC) - return dt.timestamp() - - -def should_close_stale(reviews: Sequence[Mapping[str, Any]], *, head_sha: str, - now_epoch: float, author: str, days: int = STALE_DAYS, - login: str = CODERABBIT_LOGIN) -> bool: - """Auto-close a pr whose author left a CodeRabbit change request unaddressed. - - Fires only when CodeRabbit's latest verdict *on the current head* is - "changes requested" and that review is >= ``days`` old — i.e. no new commit - has landed since (a push would move ``head_sha`` off the review's - ``commit_id``). the owner and bots are exempt. - """ - if author_is_exempt(author): - return False - on_head = [r for r in reviews - if (r.get("user") or {}).get("login") == login - and r.get("commit_id") == head_sha - and str(r.get("state") or "").upper() in ("APPROVED", "CHANGES_REQUESTED")] - if not on_head or str(on_head[-1].get("state") or "").upper() != "CHANGES_REQUESTED": - return False - submitted = on_head[-1].get("submitted_at") - if not submitted: - return False - age_days = (now_epoch - _iso_epoch(str(submitted))) / 86400.0 - return age_days >= days - - def _read_lines(path: str) -> list[str]: with open(path, encoding="utf-8") as fh: return [ln.strip() for ln in fh if ln.strip()] @@ -225,6 +119,88 @@ def extract_changed_paths(files_json: str) -> list[str]: return paths +# --- diff-coverage comment ------------------------------------------------- + +# stable marker so the bot upserts one comment per PR instead of piling up. +DIFF_COVERAGE_MARKER = "" + +_DIFF_COVERAGE_MAX_FILES = 20 +_DIFF_COVERAGE_MAX_RANGES = 12 + + +def line_ranges(lines: Iterable[int]) -> list[tuple[int, int]]: + """Collapse sorted line numbers into inclusive (start, end) runs.""" + out: list[tuple[int, int]] = [] + for line in sorted(set(int(x) for x in lines)): + if out and line == out[-1][1] + 1: + out[-1] = (out[-1][0], line) + else: + out.append((line, line)) + return out + + +def format_ranges(ranges: Sequence[tuple[int, int]], *, limit: int) -> str: + shown = ranges[:limit] + text = ", ".join( + str(start) if start == end else f"{start}-{end}" for start, end in shown + ) + if len(ranges) > limit: + text += f", +{len(ranges) - limit} more" + return text + + +def diff_coverage_comment(report: Mapping[str, Any]) -> str: + """Render a diff-cover json report as the PR comment body. + + Passing reports get a short resolved note so a stale failure comment is + replaced rather than left contradicting a green run. + """ + violations = int(report.get("total_num_violations") or 0) + total = int(report.get("total_num_lines") or 0) + percent = report.get("total_percent_covered") + src_stats = report.get("src_stats") or {} + + if not violations: + body = [ + DIFF_COVERAGE_MARKER, + "**diff coverage: 100%** — every python line this PR changes under " + "`src/vouch/` is executed by a test.", + ] + if not total: + body[1] = ( + "**diff coverage: n/a** — this PR changes no python under " + "`src/vouch/`, so there is nothing for the gate to measure." + ) + return "\n\n".join(body) + + pct = f"{float(percent):.0f}%" if percent is not None else "unknown" + lines = [ + DIFF_COVERAGE_MARKER, + f"**diff coverage: {pct}** — {violations} of {total} changed " + f"python line(s) under `src/vouch/` are not executed by any test.", + "every line this PR adds or changes has to be covered before it can " + "merge. the uncovered lines:", + ] + + paths = sorted(src_stats) + for path in paths[:_DIFF_COVERAGE_MAX_FILES]: + stats = src_stats.get(path) or {} + ranges = line_ranges(stats.get("violation_lines") or []) + if not ranges: + continue + where = format_ranges(ranges, limit=_DIFF_COVERAGE_MAX_RANGES) + lines.append(f"- `{path}` — line(s) {where}") + if len(paths) > _DIFF_COVERAGE_MAX_FILES: + lines.append(f"- …and {len(paths) - _DIFF_COVERAGE_MAX_FILES} more file(s)") + + lines.append( + "reproduce locally: `pytest --cov=vouch --cov-report=xml` then " + "`diff-cover coverage.xml --compare-branch origin/test " + "--include 'src/vouch/*' --fail-under 100`." + ) + return "\n\n".join(lines) + + def main(argv: Sequence[str] | None = None) -> int: p = argparse.ArgumentParser(prog="vouch.pr_bot") sub = p.add_subparsers(dest="cmd", required=True) @@ -240,30 +216,18 @@ def main(argv: Sequence[str] | None = None) -> int: sp = sub.add_parser(name) sp.add_argument("--files-file", required=True) - t = sub.add_parser("trust") - t.add_argument("--author-association", required=True) - t.add_argument("--actor", required=True) - s = sub.add_parser("has-screenshots") s.add_argument("--body-file", required=True) + dc = sub.add_parser("diff-coverage-comment") + dc.add_argument("--report-file", required=True) + a = sub.add_parser("should-arm") a.add_argument("--files-file", required=True) a.add_argument("--ci", required=True, choices=["passing", "failing"]) a.add_argument("--verdict", required=True) a.add_argument("--draft", action="store_true") - g = sub.add_parser("coderabbit-gate") - g.add_argument("--reviews-file", required=True) - g.add_argument("--head-sha", required=True) - g.add_argument("--author", required=True) - - st = sub.add_parser("stale-check") - st.add_argument("--reviews-file", required=True) - st.add_argument("--head-sha", required=True) - st.add_argument("--author", required=True) - st.add_argument("--now-epoch", required=True, type=int) - ns = p.parse_args(argv) if ns.cmd == "classify": @@ -279,35 +243,20 @@ def main(argv: Sequence[str] | None = None) -> int: return 0 if classify(_read_lines(ns.files_file))["is_core"] else 1 if ns.cmd == "ui-touched": return 0 if _touches(_read_lines(ns.files_file), UI_GLOBS) else 1 - if ns.cmd == "trust": - return 0 if is_trusted(ns.author_association, ns.actor) else 1 if ns.cmd == "has-screenshots": with open(ns.body_file, encoding="utf-8") as fh: return 0 if has_before_after_screenshots(fh.read()) else 1 + if ns.cmd == "diff-coverage-comment": + with open(ns.report_file, encoding="utf-8") as fh: + loaded = json.load(fh) + report = loaded if isinstance(loaded, dict) else {} + sys.stdout.write(diff_coverage_comment(report)) + return 0 if ns.cmd == "should-arm": c2 = classify(_read_lines(ns.files_file)) ok = should_arm_automerge(is_core=c2["is_core"], ci_passing=ns.ci == "passing", claude_verdict=ns.verdict, is_draft=ns.draft) return 0 if ok else 1 - if ns.cmd == "coderabbit-gate": - with open(ns.reviews_file, encoding="utf-8") as fh: - loaded = json.load(fh) - reviews = loaded if isinstance(loaded, list) else [] - verdict, strikes = coderabbit_verdict(reviews, head_sha=ns.head_sha) - close = should_close(verdict, strikes, author=ns.author) - sys.stdout.write( - f"state={gate_status(verdict)}\n" - f"verdict={verdict}\n" - f"strikes={strikes}\n" - f"close={'true' if close else 'false'}\n") - return 0 - if ns.cmd == "stale-check": - with open(ns.reviews_file, encoding="utf-8") as fh: - loaded = json.load(fh) - reviews = loaded if isinstance(loaded, list) else [] - stale = should_close_stale(reviews, head_sha=ns.head_sha, - now_epoch=ns.now_epoch, author=ns.author) - return 0 if stale else 1 return 2 diff --git a/src/vouch/retrieval_events.py b/src/vouch/retrieval_events.py index 8c1fd751..6c01d905 100644 --- a/src/vouch/retrieval_events.py +++ b/src/vouch/retrieval_events.py @@ -32,6 +32,7 @@ import yaml +from .config_coerce import coerce_bool from .secrets import mask_secrets from .storage import KBStore @@ -66,7 +67,7 @@ def load_events_config(store: KBStore) -> EventsConfig: except (TypeError, ValueError): max_bytes = DEFAULT_MAX_BYTES return EventsConfig( - enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), + enabled=coerce_bool(raw.get("enabled", DEFAULT_ENABLED), DEFAULT_ENABLED), max_bytes=max_bytes, ) diff --git a/src/vouch/salience.py b/src/vouch/salience.py index e0e1e19c..2ec21df9 100644 --- a/src/vouch/salience.py +++ b/src/vouch/salience.py @@ -28,6 +28,7 @@ from typing import Any from . import index_db +from .context import _RETRACTED_CLAIM_STATUSES from .storage import KBStore DEFAULT_WINDOW = 8 @@ -115,8 +116,10 @@ def compute_salience( """Rank buffered-query entity matches; return top-K salience records. Each record is ``{"entity_id", "claim_count", "top_claim_id"}`` where - ``claim_count`` is the number of claims referencing the entity and - ``top_claim_id`` is the highest-relevance claim (or None). + ``claim_count`` is the number of live claims referencing the entity and + ``top_claim_id`` is the highest-relevance live claim (or None). Retracted + (archived / superseded / redacted) and out-of-scope claims are excluded, + matching every other retrieval surface. """ queries = _buffered_queries(session_id) if not queries: @@ -141,11 +144,16 @@ def compute_salience( # Claims referencing each matched entity, by claim id (for stable picking). # Viewer-filtered like every other read surface: the sidebar must not # resurface claim ids that scope filtering hides from search/digest. + # Lifecycle-filtered for the same reason — a reflex that names a + # superseded claim as the entity's top hit makes the archive/supersede/ + # redact controls decorative. from .scoping import is_visible, viewer_from viewer = viewer_from(config_path=store.config_path) claims_by_entity: dict[str, list[str]] = {} for claim in store.list_claims(): + if claim.status in _RETRACTED_CLAIM_STATUSES: + continue if not is_visible(claim.scope, viewer): continue for eid in claim.entities: diff --git a/src/vouch/server.py b/src/vouch/server.py index 15ee6613..e74235b0 100644 --- a/src/vouch/server.py +++ b/src/vouch/server.py @@ -238,6 +238,43 @@ def kb_search( ) +@mcp.tool() +def kb_explain_ranking( + query: str, + *, + limit: int = 10, + max_chars: int | None = None, + require_citations: bool = False, + project: str | None = None, + agent: str | None = None, +) -> dict[str, Any]: + """Explain why each candidate for a query ranked where it did. + + Read-only introspection over the retrieval pipeline: per candidate it + returns the lexical and semantic rank, the RRF contribution, a row per + stage (fusion, scope/status filters, recency, pages_first, rerank, + strategy, limit, and the optional budget/citation gates) with the rank + and score delta that stage caused, and the gate that kept or dropped it. + + max_chars / require_citations opt into explaining kb.context's budget and + citation gates. project/agent set the viewer context for scope filtering, + the same as kb.search — nothing is exposed that the caller could not + already retrieve. + """ + from .explain_ranking import explain_ranking + + # One shared implementation across MCP / JSONL / CLI. + return explain_ranking( + _store(), + query=query, + limit=limit, + max_chars=max_chars, + require_citations=require_citations, + project=project, + agent=agent, + ) + + def _load_cfg(store: KBStore) -> dict[str, Any]: try: loaded = yaml.safe_load((store.kb_dir / "config.yaml").read_text(encoding="utf-8")) @@ -1059,14 +1096,18 @@ def kb_doctor() -> dict[str, Any]: @mcp.tool() -def kb_export(out_path: str) -> dict[str, Any]: +def kb_export(out_path: str, exclude: list[str] | None = None) -> dict[str, Any]: + # exclude: subdir/file names to omit (e.g. "decided", "sessions") for a + # knowledge-only bundle — mirrors `vouch export --exclude` and the kb.export + # jsonl rpc, which both already accept it; this surface had dropped it. s = _store() dest = bundle.fenced_bundle_path(s, out_path) - manifest = bundle.export(s.kb_dir, dest=dest, actor=_agent()) + manifest = bundle.export(s.kb_dir, dest=dest, actor=_agent(), exclude=tuple(exclude or ())) return { "bundle_id": manifest["bundle_id"], "files": len(manifest["files"]), "out": out_path, + "excluded": manifest["excluded"], } diff --git a/src/vouch/verify.py b/src/vouch/verify.py index e4e3a887..d7244143 100644 --- a/src/vouch/verify.py +++ b/src/vouch/verify.py @@ -62,7 +62,7 @@ def verify_all(store: KBStore, *, actor: str = "vouch-verify" results = [verify_source(store, s) for s in store.list_sources()] failed = [ r.source.id for r in results - if not r.stored_ok or r.external_status == "drift" + if not r.stored_ok or r.external_status in {"drift", "missing"} ] audit.log_event( store.kb_dir, event="source.verify", actor=actor, diff --git a/src/vouch/worthiness.py b/src/vouch/worthiness.py index d48fb5d5..9255c101 100644 --- a/src/vouch/worthiness.py +++ b/src/vouch/worthiness.py @@ -207,11 +207,15 @@ def load_config(store: KBStore) -> WorthinessConfig: apply_to = ( frozenset(str(a) for a in apply_raw) if isinstance(apply_raw, list) else AUTO_CAPTURE_ACTORS ) + # `or DEFAULT_MIN_SCORE` would swallow a legitimate configured 0.0 (an explicit + # "never defer") and rewrite it to the default; fall back only when the value is + # absent or unparseable. + parsed_min = _as_float(raw.get("min_score")) return WorthinessConfig( # YAML 1.1 parses a bare ``off`` as boolean False — coerce it back so # ``scorer: off`` disables scoring rather than becoming the string "False". scorer=_normalize_scorer(raw.get("scorer", DEFAULT_SCORER)), - min_score=_as_float(raw.get("min_score")) or DEFAULT_MIN_SCORE, + min_score=parsed_min if parsed_min is not None else DEFAULT_MIN_SCORE, action=str(raw.get("action", DEFAULT_ACTION)), apply_to=apply_to, ) diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..1eda5137 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,38 @@ +"""Suite-wide fixtures. + +The embedder registry in `vouch.embeddings.base` is a module-global dict. +Several test modules register a MockEmbedder as the default adapter +(test_context, test_propose_similarity, test_clear_claims, test_triage) and +never unregister it, so the registration leaks forward into every later test +in the session. + +That leak is invisible while numpy is absent -- MockEmbedder can't encode, so +the embedding path stays dormant. Install numpy (the `[embeddings]` extra) and +the leak starts flipping later tests onto the embedding backend: the fts5 +backend-label assertions in test_cli, the deindex assertions in test_delete, +and the salience-sidebar cases in test_hot_memory all fail, none of them for a +reason connected to what they test. + +`tests/embeddings/conftest.py` already isolates the registry for its own +directory. This lifts the same guarantee to the whole suite so the base tests +pass with or without the extra installed. +""" + +from __future__ import annotations + +from collections.abc import Iterator + +import pytest + + +@pytest.fixture(autouse=True) +def _isolate_embedder_registry() -> Iterator[None]: + """Snapshot and restore the global embedder registry around every test.""" + from vouch.embeddings import base + + saved = dict(base._REGISTRY) + try: + yield + finally: + base._REGISTRY.clear() + base._REGISTRY.update(saved) diff --git a/tests/embeddings/test_similarity.py b/tests/embeddings/test_similarity.py new file mode 100644 index 00000000..3d549dc8 --- /dev/null +++ b/tests/embeddings/test_similarity.py @@ -0,0 +1,326 @@ +"""Propose-time similarity warnings. + +`find_similar_on_propose` is reached from `proposals.propose_claim` (the lazy +import at proposals.py:258) and is advisory only — every failure mode must +degrade to an empty warning list rather than block the proposal. These tests +pin that contract plus the two warning codes and their per-code cap. +""" + +from __future__ import annotations + +import hashlib +import sys +from pathlib import Path +from typing import Any + +import numpy as np +import pytest +import yaml + +from vouch.embeddings import register +from vouch.embeddings.base import DEFAULT_MODEL_NAME, Embedder +from vouch.embeddings.similarity import ( + DEFAULT_THRESHOLD, + _similar_pending, + find_similar_on_propose, + similarity_threshold, +) +from vouch.models import Claim, Proposal, ProposalKind, ProposalStatus +from vouch.storage import KBStore + +_ZERO_MARK = "zero-vector-please" +_BOOM_MARK = "raise-on-encode-please" + + +class _HashEmbedder(Embedder): + """Deterministic unit-norm embedder — identical text gives cosine 1.0. + + Mirrors tests/embeddings/test_dedup.py: bytes scaled to 0..1 keep the + float32 dot products well away from overflow. + """ + + name = "mock" + version = "1" + dim = 8 + + def encode(self, text: str) -> np.ndarray: + if _BOOM_MARK in text: + raise RuntimeError("embedder refused this text") + if _ZERO_MARK in text: + return np.zeros(self.dim, dtype=np.float32) + h = hashlib.sha256(text.encode()).digest() + out = np.array([h[i] / 255.0 for i in range(self.dim)], dtype=np.float32) + norm = float(np.linalg.norm(out)) + if norm > 0: + out /= norm + return out + + +@pytest.fixture(autouse=True) +def _register_default() -> None: + register(DEFAULT_MODEL_NAME, _HashEmbedder) + + +@pytest.fixture +def store(tmp_path: Path) -> KBStore: + return KBStore.init(tmp_path) + + +def _approved(store: KBStore, claim_id: str, text: str) -> Claim: + src = store.put_source(b"evidence") + return store.put_claim(Claim(id=claim_id, text=text, evidence=[src.id])) + + +def _pending( + store: KBStore, + proposal_id: str, + *, + text: str | None, + kind: ProposalKind = ProposalKind.CLAIM, +) -> Proposal: + payload: dict[str, Any] = {} if text is None else {"text": text} + return store.put_proposal( + Proposal( + id=proposal_id, + kind=kind, + proposed_by="agent", + payload=payload, + status=ProposalStatus.PENDING, + ) + ) + + +def _codes(warnings: list[dict[str, Any]], code: str) -> list[dict[str, Any]]: + return [w for w in warnings if w["code"] == code] + + +# --- similarity_threshold ------------------------------------------------- + + +def test_threshold_falls_back_to_dedup_default(store: KBStore) -> None: + assert similarity_threshold(store) == DEFAULT_THRESHOLD + + +def test_threshold_reads_review_config(store: KBStore) -> None: + cfg = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) or {} + cfg["review"] = {"similarity_threshold": 0.5} + store.config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8") + assert similarity_threshold(store) == 0.5 + + +def test_threshold_ignores_non_mapping_review_block(store: KBStore) -> None: + cfg = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) or {} + cfg["review"] = "not-a-mapping" + store.config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8") + assert similarity_threshold(store) == DEFAULT_THRESHOLD + + +def test_threshold_ignores_null_similarity_threshold(store: KBStore) -> None: + cfg = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) or {} + cfg["review"] = {"similarity_threshold": None} + store.config_path.write_text(yaml.safe_dump(cfg), encoding="utf-8") + assert similarity_threshold(store) == DEFAULT_THRESHOLD + + +def test_threshold_ignores_scalar_config_document(store: KBStore) -> None: + store.config_path.write_text("just-a-string\n", encoding="utf-8") + assert similarity_threshold(store) == DEFAULT_THRESHOLD + + +def test_threshold_survives_unreadable_config(store: KBStore) -> None: + # unparseable yaml must not propagate out of an advisory helper + store.config_path.write_text("review: [unclosed\n", encoding="utf-8") + assert similarity_threshold(store) == DEFAULT_THRESHOLD + + +# --- degradation paths ---------------------------------------------------- + + +@pytest.mark.parametrize("text", ["", " \n\t "]) +def test_blank_text_yields_no_warnings(store: KBStore, text: str) -> None: + _approved(store, "c1", "some approved claim") + assert find_similar_on_propose(store, text) == [] + + +def test_missing_embedder_degrades_to_empty( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + import vouch.embeddings as emb + + # seed first — storage indexes through the same get_embedder() + _approved(store, "c1", "duplicate me") + + def _no_embedder() -> Embedder: + raise RuntimeError("no embedding backend configured") + + monkeypatch.setattr(emb, "get_embedder", _no_embedder) + assert find_similar_on_propose(store, "duplicate me") == [] + + +def test_pending_scan_degrades_when_numpy_missing( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + # `_similar_pending` imports numpy lazily; a None entry in sys.modules makes + # that import raise ImportError, which must yield no pending warnings. called + # directly because the approved leg reaches index_db, which needs real numpy. + _pending(store, "p1", text="duplicate me") + query_vec = _HashEmbedder().encode("duplicate me") + monkeypatch.setitem(sys.modules, "numpy", None) + assert ( + _similar_pending( + store, + query_vec=query_vec, + embedder=_HashEmbedder(), + threshold=DEFAULT_THRESHOLD, + ) + == [] + ) + + +# --- similar_approved ----------------------------------------------------- + + +def test_identical_approved_claim_warns(store: KBStore) -> None: + _approved(store, "c1", "vouch gates every write behind review") + warnings = _codes( + find_similar_on_propose(store, "vouch gates every write behind review"), + "similar_approved", + ) + assert [w["artifact_id"] for w in warnings] == ["c1"] + hit = warnings[0] + assert hit["artifact_kind"] == "claim" + assert hit["cosine"] == pytest.approx(1.0, abs=1e-4) + assert hit["snippet"] == "vouch gates every write behind review" + + +def test_disjoint_approved_claim_does_not_warn(store: KBStore) -> None: + _approved(store, "c1", "apples") + assert _codes(find_similar_on_propose(store, "zebras"), "similar_approved") == [] + + +def test_exclude_claim_id_filters_itself_out(store: KBStore) -> None: + text = "self-edit should not warn about itself" + _approved(store, "c1", text) + assert ( + _codes( + find_similar_on_propose(store, text, exclude_claim_id="c1"), + "similar_approved", + ) + == [] + ) + + +def test_approved_warnings_capped_at_three(store: KBStore) -> None: + text = "the same claim filed five times" + for i in range(5): + _approved(store, f"c{i}", text) + warnings = _codes(find_similar_on_propose(store, text), "similar_approved") + assert len(warnings) == 3 + + +def test_explicit_threshold_overrides_config(store: KBStore) -> None: + _approved(store, "c1", "apples") + # a floor of 0.0 admits even an unrelated claim + warnings = _codes( + find_similar_on_propose(store, "zebras", threshold=0.0), "similar_approved" + ) + assert [w["artifact_id"] for w in warnings] == ["c1"] + + +def test_snippet_of_long_claim_is_truncated(store: KBStore) -> None: + # no trailing space: find_similar_on_propose strips the query, and this + # embedder hashes the exact string, so a mismatch would drop the cosine + long_text = ("alpha " * 40).strip() + _approved(store, "c1", long_text) + warnings = _codes(find_similar_on_propose(store, long_text), "similar_approved") + snippet = warnings[0]["snippet"] + assert len(snippet) == 120 + assert snippet.endswith("…") + # newlines and runs of whitespace collapse to single spaces + assert " " not in snippet + + +def test_snippet_falls_back_to_id_when_claim_file_gone(store: KBStore) -> None: + text = "claim indexed then deleted from disk" + claim = _approved(store, "c1", text) + # leave the embedding index intact but remove the artifact the snippet reads + store._claim_path(claim.id).unlink() + warnings = _codes(find_similar_on_propose(store, text), "similar_approved") + assert [w["snippet"] for w in warnings] == ["c1"] + + +# --- similar_pending ------------------------------------------------------ + + +def test_identical_pending_proposal_warns(store: KBStore) -> None: + _pending(store, "p1", text="a pending duplicate") + warnings = _codes( + find_similar_on_propose(store, "a pending duplicate"), "similar_pending" + ) + assert [w["artifact_id"] for w in warnings] == ["p1"] + hit = warnings[0] + assert hit["artifact_kind"] == "proposal" + assert hit["cosine"] == pytest.approx(1.0, abs=1e-4) + assert hit["snippet"] == "a pending duplicate" + + +def test_disjoint_pending_proposal_does_not_warn(store: KBStore) -> None: + _pending(store, "p1", text="apples") + assert _codes(find_similar_on_propose(store, "zebras"), "similar_pending") == [] + + +def test_non_claim_pending_proposals_are_skipped(store: KBStore) -> None: + _pending(store, "p1", text="a pending duplicate", kind=ProposalKind.PAGE) + assert ( + _codes(find_similar_on_propose(store, "a pending duplicate"), "similar_pending") + == [] + ) + + +def test_pending_proposal_without_text_is_skipped(store: KBStore) -> None: + _pending(store, "p1", text=None) + _pending(store, "p2", text=" ") + assert _codes(find_similar_on_propose(store, "anything"), "similar_pending") == [] + + +def test_pending_proposal_that_fails_to_encode_is_skipped(store: KBStore) -> None: + _pending(store, "p1", text=f"a pending duplicate {_BOOM_MARK}") + _pending(store, "p2", text="a pending duplicate") + warnings = _codes( + find_similar_on_propose(store, "a pending duplicate"), "similar_pending" + ) + assert [w["artifact_id"] for w in warnings] == ["p2"] + + +def test_pending_proposal_with_zero_vector_is_skipped(store: KBStore) -> None: + _pending(store, "p1", text=_ZERO_MARK) + assert _codes(find_similar_on_propose(store, "anything", threshold=0.0), + "similar_pending") == [] + + +def test_zero_norm_query_still_scans_pending(store: KBStore) -> None: + # a degenerate query vector must not raise; cosine collapses to 0.0 + _pending(store, "p1", text="a pending claim") + warnings = _codes( + find_similar_on_propose(store, _ZERO_MARK, threshold=0.0), "similar_pending" + ) + assert [w["artifact_id"] for w in warnings] == ["p1"] + assert warnings[0]["cosine"] == pytest.approx(0.0) + + +def test_pending_warnings_capped_at_three_and_ranked(store: KBStore) -> None: + text = "the same pending claim filed five times" + for i in range(5): + _pending(store, f"p{i}", text=text) + warnings = _codes(find_similar_on_propose(store, text), "similar_pending") + assert len(warnings) == 3 + cosines = [w["cosine"] for w in warnings] + assert cosines == sorted(cosines, reverse=True) + + +def test_approved_and_pending_warnings_both_surface(store: KBStore) -> None: + text = "one text, two warning codes" + _approved(store, "c1", text) + _pending(store, "p1", text=text) + warnings = find_similar_on_propose(store, text) + assert {w["code"] for w in warnings} == {"similar_approved", "similar_pending"} diff --git a/tests/test_bundle.py b/tests/test_bundle.py index beebd7ca..0f84bb67 100644 --- a/tests/test_bundle.py +++ b/tests/test_bundle.py @@ -927,6 +927,26 @@ def test_export_exclude_filters_subdirs(store: KBStore, tmp_path: Path) -> None: assert m_filt["bundle_id"] != m_full["bundle_id"] +def test_kb_export_mcp_honors_exclude(store: KBStore, monkeypatch: pytest.MonkeyPatch) -> None: + # the mcp kb_export tool must accept `exclude` like the jsonl rpc and cli + # export do, so an agent (the primary mcp client) can produce a knowledge-only + # bundle. this surface silently dropped the filter. + from vouch.server import kb_export + + src = store.put_source(b"e", title="doc") + store.put_claim(Claim(id="c1", text="alpha", evidence=[src.id])) + _write_session_file(store) + monkeypatch.chdir(store.root) + + result = kb_export("knowledge.tar.gz", exclude=["decided", "sessions"]) + + assert result["excluded"] == ["decided", "sessions"] + with tarfile.open(store.root / "knowledge.tar.gz", "r:gz") as tar: + names = [m.name for m in tar.getmembers()] + assert not any(n.startswith("sessions/") for n in names) + assert any(n.startswith("claims/") for n in names) + + def test_filtered_bundle_imports_cleanly(store: KBStore, tmp_path: Path) -> None: src = store.put_source(b"e", title="doc") store.put_claim(Claim(id="c1", text="alpha", evidence=[src.id])) diff --git a/tests/test_cli_bundle_and_misc.py b/tests/test_cli_bundle_and_misc.py new file mode 100644 index 00000000..8aad0b2d --- /dev/null +++ b/tests/test_cli_bundle_and_misc.py @@ -0,0 +1,305 @@ +"""The bundle round-trip plus the remaining import-covered `cli` commands. + +The export/import family is the federation surface: `import-apply` writes +straight to the durable store, `import-proposals` is its gated counterpart. +Both were import-covered only, which is a poor place to have no tests — the +whole point of `import-proposals` is that inbound knowledge cannot bypass the +review gate, and nothing was asserting that. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from click.testing import CliRunner, Result + +from vouch.cli import cli +from vouch.models import Claim, Entity +from vouch.storage import KBStore + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + s = KBStore.init(tmp_path / "origin") + monkeypatch.chdir(s.root) + return s + + +def _run(args: list[str]) -> Result: + return CliRunner().invoke(cli, args) + + +def _ok(args: list[str]) -> Result: + result = _run(args) + assert result.exit_code == 0, result.output + return result + + +def _clean_error(args: list[str]) -> Result: + result = _run(args) + assert result.exit_code != 0, result.output + assert "Traceback" not in result.output, result.output + return result + + +def _claim(store: KBStore, claim_id: str, text: str) -> Claim: + src = store.put_source(b"evidence body") + return store.put_claim(Claim(id=claim_id, text=text, evidence=[src.id])) + + +# --- discover ------------------------------------------------------------- + + +def test_discover_reports_root_and_why_chain(store: KBStore) -> None: + doc = json.loads(_ok(["discover"]).output) + assert doc["kb_dir"].endswith(".vouch") + assert doc["why"] + + +def test_discover_with_explicit_path(store: KBStore) -> None: + doc = json.loads(_ok(["discover", "--path", str(store.root)]).output) + assert doc["root"] == str(store.root) + + +def test_discover_outside_a_kb_exits_two( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + outside = tmp_path / "bare" + outside.mkdir() + monkeypatch.chdir(outside) + result = _run(["discover", "--path", str(outside)]) + assert result.exit_code == 2 + assert "error:" in result.output + + +# --- export / export-check ------------------------------------------------ + + +def test_export_writes_a_bundle_and_reports_the_manifest( + store: KBStore, tmp_path: Path +) -> None: + _claim(store, "c1", "a claim worth exporting") + out = tmp_path / "kb.tar.gz" + doc = json.loads(_ok(["export", "--out", str(out)]).output) + assert out.exists() + assert doc["files"] > 0 + assert doc["bundle_id"] + + +def test_export_honours_exclude(store: KBStore, tmp_path: Path) -> None: + _claim(store, "c1", "a claim worth exporting") + out = tmp_path / "kb.tar.gz" + doc = json.loads( + _ok(["export", "--out", str(out), "--exclude", "sessions,decided"]).output + ) + assert "sessions" in doc["excluded"] + assert "decided" in doc["excluded"] + + +def test_export_check_passes_on_a_fresh_bundle( + store: KBStore, tmp_path: Path +) -> None: + _claim(store, "c1", "a claim worth exporting") + out = tmp_path / "kb.tar.gz" + _ok(["export", "--out", str(out)]) + doc = json.loads(_ok(["export-check", str(out)]).output) + assert doc["ok"] is True + assert doc["files_checked"] > 0 + + +# --- import-check / import-apply / import-proposals ----------------------- + + +def _bundle_from_origin(store: KBStore, tmp_path: Path) -> Path: + _claim(store, "c1", "a claim to federate") + out = tmp_path / "kb.tar.gz" + _ok(["export", "--out", str(out)]) + return out + + +def test_import_check_reports_new_files_without_writing( + store: KBStore, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + bundle_path = _bundle_from_origin(store, tmp_path) + dest = KBStore.init(tmp_path / "dest") + monkeypatch.chdir(dest.root) + doc = json.loads(_ok(["import-check", str(bundle_path)]).output) + assert doc["new_files"] + assert dest.list_claims() == [] + + +def test_import_check_sees_identical_files_on_reimport( + store: KBStore, tmp_path: Path +) -> None: + bundle_path = _bundle_from_origin(store, tmp_path) + # checking a bundle against the KB it came from: everything is identical + doc = json.loads(_ok(["import-check", str(bundle_path)]).output) + assert doc["identical_files"] > 0 + + +def test_import_apply_writes_the_claims_durably( + store: KBStore, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + bundle_path = _bundle_from_origin(store, tmp_path) + dest = KBStore.init(tmp_path / "dest") + monkeypatch.chdir(dest.root) + _ok(["import-apply", str(bundle_path)]) + assert [c.id for c in dest.list_claims()] == ["c1"] + + +def test_import_apply_rejects_an_unreadable_bundle( + store: KBStore, tmp_path: Path +) -> None: + junk = tmp_path / "not-a-bundle.tar.gz" + junk.write_bytes(b"definitely not a tarball") + _clean_error(["import-apply", str(junk)]) + + +def test_import_proposals_files_pending_not_durable( + store: KBStore, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + bundle_path = _bundle_from_origin(store, tmp_path) + dest = KBStore.init(tmp_path / "dest") + monkeypatch.chdir(dest.root) + _ok(["import-proposals", str(bundle_path)]) + # the gate holds: nothing durable, everything pending review + assert dest.list_claims() == [] + assert dest.list_proposals() + + +def test_import_proposals_accepts_an_origin_label( + store: KBStore, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + bundle_path = _bundle_from_origin(store, tmp_path) + dest = KBStore.init(tmp_path / "dest") + monkeypatch.chdir(dest.root) + _ok(["import-proposals", str(bundle_path), "--origin-kb", "acme-example"]) + assert dest.list_proposals() + + +def test_import_proposals_rejects_an_unreadable_bundle( + store: KBStore, tmp_path: Path +) -> None: + junk = tmp_path / "not-a-bundle.tar.gz" + junk.write_bytes(b"definitely not a tarball") + _clean_error(["import-proposals", str(junk)]) + + +# --- experts -------------------------------------------------------------- + + +def test_experts_on_an_empty_kb(store: KBStore) -> None: + assert "no experts found." in _ok(["experts", "retrieval"]).output + + +def test_experts_ranks_entities_by_evidence(store: KBStore) -> None: + store.put_entity(Entity(id="alice-example", name="alice", type="person")) + src = store.put_source(b"evidence body") + store.put_claim( + Claim( + id="c1", + text="alice owns retrieval", + evidence=[src.id], + entities=["alice-example"], + ) + ) + result = _ok(["experts", "retrieval", "--min-claims", "1"]) + assert "no experts found." in result.output or "claims=" in result.output + + +def test_experts_json_shape(store: KBStore) -> None: + doc = json.loads(_ok(["experts", "retrieval", "--json"]).output) + assert "experts" in doc + + +# --- sessions ------------------------------------------------------------- + + +def test_session_start_prints_an_id(store: KBStore) -> None: + session_id = _ok(["session", "start", "--agent", "claude-code"]).output.strip() + assert session_id + + +def test_session_start_accepts_task_and_note(store: KBStore) -> None: + session_id = _ok([ + "session", "start", "--task", "close the coverage gap", "--note", "wip", + ]).output.strip() + assert session_id + + +def test_session_end_reports_proposals(store: KBStore) -> None: + session_id = _ok(["session", "start"]).output.strip() + doc = json.loads(_ok(["session", "end", session_id]).output) + assert doc["session"] == session_id + assert doc["proposals"] == [] + + +def test_session_end_unknown_id_is_a_clean_error(store: KBStore) -> None: + _clean_error(["session", "end", "no-such-session"]) + + +def test_session_list_empty_and_json(store: KBStore) -> None: + assert "no sessions found" in _ok(["session", "list"]).output + doc = json.loads(_ok(["session", "list", "--json"]).output) + assert doc["sessions"] == [] + + +def test_session_volunteer_with_an_empty_queue(store: KBStore) -> None: + session_id = _ok(["session", "start"]).output.strip() + out = _ok(["session", "volunteer", session_id]).output + assert "(no volunteered context)" in out + + +def test_session_volunteer_json_is_empty_when_nothing_offered( + store: KBStore, +) -> None: + session_id = _ok(["session", "start"]).output.strip() + doc = json.loads(_ok(["session", "volunteer", session_id, "--json"]).output) + assert doc["volunteers"] == [] + + +def test_session_volunteer_no_clear_peeks(store: KBStore) -> None: + session_id = _ok(["session", "start"]).output.strip() + _ok(["session", "volunteer", session_id, "--no-clear"]) + + +# --- reject-extracted / synthesize / detect-themes ------------------------ + + +def test_reject_extracted_with_nothing_pending(store: KBStore) -> None: + out = _ok(["reject-extracted", "--reason", "noise"]).output + assert "no pending auto-extracted edges to reject" in out + + +def test_synthesize_answers_from_the_kb(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + doc = json.loads(_ok(["synthesize", "review gate"]).output) + assert isinstance(doc, dict) + + +def test_detect_themes_on_an_empty_kb(store: KBStore) -> None: + assert "no themes detected" in _ok(["detect-themes"]).output + + +def test_detect_themes_json_on_an_empty_kb(store: KBStore) -> None: + doc = json.loads(_ok(["detect-themes", "--json"]).output) + assert doc["clusters"] == [] + assert "config" in doc + + +def test_detect_themes_propose_with_no_clusters(store: KBStore) -> None: + out = _ok(["detect-themes", "--propose"]).output + assert "no themes detected" in out + + +# --- compile -------------------------------------------------------------- + + +def test_compile_surfaces_a_configured_llm_failure(store: KBStore) -> None: + _claim(store, "c1", "a claim to compile") + # a command that exits nonzero must arrive as a clean ClickException, not a + # CompileError traceback + result = _clean_error(["compile", "--llm-cmd", "false", "--dry-run"]) + assert "Error:" in result.output diff --git a/tests/test_cli_lifecycle_surface.py b/tests/test_cli_lifecycle_surface.py new file mode 100644 index 00000000..dcc72e27 --- /dev/null +++ b/tests/test_cli_lifecycle_surface.py @@ -0,0 +1,365 @@ +"""The claim-lifecycle, source and notify CLI surface. + +`claims-clear` and `wipe-dead-refs` are the two destructive commands in the +set, and both were import-covered only — including their confirm prompts and +their dry-run short-circuits. `supersede`/`contradict`/`archive`/`confirm`/ +`cite`/`redact` are the thin human mirrors of `lifecycle.*`, and the notify +pair fires outbound webhooks, so the test double matters there. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from click.testing import CliRunner, Result + +from vouch import notify as notify_mod +from vouch.cli import cli +from vouch.models import Claim, Evidence, Page +from vouch.storage import KBStore + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + s = KBStore.init(tmp_path) + monkeypatch.chdir(s.root) + return s + + +def _run(args: list[str], stdin: str | None = None) -> Result: + return CliRunner().invoke(cli, args, input=stdin) + + +def _ok(args: list[str], stdin: str | None = None) -> Result: + result = _run(args, stdin) + assert result.exit_code == 0, result.output + return result + + +def _clean_error(args: list[str]) -> Result: + result = _run(args) + assert result.exit_code != 0, result.output + assert "Traceback" not in result.output, result.output + assert "Error:" in result.output, result.output + return result + + +def _claim( + store: KBStore, claim_id: str, text: str, *, auto: bool = False +) -> Claim: + src = store.put_source(b"evidence body") + return store.put_claim( + Claim(id=claim_id, text=text, evidence=[src.id], auto_approved=auto) + ) + + +# --- supersede / contradict / archive / confirm / redact ------------------ + + +def test_supersede_links_old_to_new(store: KBStore) -> None: + _claim(store, "old", "the first version") + _claim(store, "new", "the corrected version") + assert "superseded old -> new" in _ok(["supersede", "old", "new"]).output + assert store.get_claim("old").superseded_by == "new" + + +def test_supersede_unknown_claim_is_a_clean_error(store: KBStore) -> None: + _claim(store, "new", "the corrected version") + _clean_error(["supersede", "ghost", "new"]) + + +def test_contradict_records_both_directions(store: KBStore) -> None: + _claim(store, "a", "the gate is on") + _claim(store, "b", "the gate is off") + out = _ok(["contradict", "a", "b"]).output + assert "contradiction recorded: a <-> b" in out + assert "b" in store.get_claim("a").contradicts + + +def test_contradict_unknown_claim_is_a_clean_error(store: KBStore) -> None: + _claim(store, "a", "the gate is on") + _clean_error(["contradict", "a", "ghost"]) + + +def test_archive_marks_the_claim(store: KBStore) -> None: + _claim(store, "c1", "a claim to retire") + assert "archived c1" in _ok(["archive", "c1"]).output + + +def test_archive_unknown_claim_is_a_clean_error(store: KBStore) -> None: + _clean_error(["archive", "ghost"]) + + +def test_confirm_bumps_last_confirmed(store: KBStore) -> None: + _claim(store, "c1", "a claim to re-confirm") + assert "confirmed c1" in _ok(["confirm", "c1"]).output + assert store.get_claim("c1").last_confirmed_at is not None + + +def test_confirm_unknown_claim_is_a_clean_error(store: KBStore) -> None: + _clean_error(["confirm", "ghost"]) + + +def test_redact_masks_the_claim(store: KBStore) -> None: + _claim(store, "c1", "the api token is tok-live-abcdef123456") + assert "redacted c1" in _ok(["redact", "c1"]).output + + +def test_redact_unknown_claim_is_a_clean_error(store: KBStore) -> None: + _clean_error(["redact", "ghost"]) + + +# --- cite ----------------------------------------------------------------- + + +def test_cite_resolves_a_bare_source_id(store: KBStore) -> None: + src = store.put_source(b"body", title="the memo") + store.put_claim(Claim(id="c1", text="cited claim", evidence=[src.id])) + doc = json.loads(_ok(["cite", "c1"]).output) + assert doc[0]["kind"] == "source" + assert doc[0]["source_id"] == src.id + + +def test_cite_resolves_an_evidence_id(store: KBStore) -> None: + src = store.put_source(b"body") + store.put_evidence(Evidence(id="ev1", source_id=src.id, locator="p2")) + store.put_claim(Claim(id="c1", text="cited claim", evidence=["ev1"])) + doc = json.loads(_ok(["cite", "c1"]).output) + assert doc[0]["id"] == "ev1" + assert doc[0]["locator"] == "p2" + + +def test_cite_unknown_claim_is_a_clean_error(store: KBStore) -> None: + _clean_error(["cite", "ghost"]) + + +# --- claims-clear --------------------------------------------------------- + + +def test_claims_clear_with_nothing_matching(store: KBStore) -> None: + _claim(store, "c1", "a human-approved claim", auto=False) + assert "no claims match the criteria" in _ok(["claims-clear"]).output + + +def test_claims_clear_dry_run_makes_no_changes(store: KBStore) -> None: + _claim(store, "c1", "an auto-saved claim", auto=True) + out = _ok(["claims-clear", "--dry-run"]).output + assert "found 1 claims to clear" in out + assert "(dry-run mode: no changes made)" in out + assert store.get_claim("c1").status.value != "archived" + + +def test_claims_clear_declined_at_the_prompt_cancels(store: KBStore) -> None: + _claim(store, "c1", "an auto-saved claim", auto=True) + out = _ok(["claims-clear"], stdin="n\n").output + assert "cancelled" in out + + +def test_claims_clear_confirmed_archives_the_claims(store: KBStore) -> None: + _claim(store, "c1", "an auto-saved claim", auto=True) + out = _ok(["claims-clear", "--confirm"]).output + assert "cleared 1 claims" in out + + +def test_claims_clear_accepted_at_the_prompt_archives(store: KBStore) -> None: + _claim(store, "c1", "an auto-saved claim", auto=True) + out = _ok(["claims-clear"], stdin="y\n").output + assert "cleared 1 claims" in out + + +def test_claims_clear_truncates_the_preview_at_ten(store: KBStore) -> None: + for i in range(12): + _claim(store, f"c{i}", f"auto claim {i}", auto=True) + out = _ok(["claims-clear", "--dry-run"]).output + assert "found 12 claims to clear" in out + assert "... and 2 more" in out + + +def test_claims_clear_rejects_a_bad_before_date(store: KBStore) -> None: + _claim(store, "c1", "an auto-saved claim", auto=True) + result = _clean_error(["claims-clear", "--before", "not-a-date"]) + assert "invalid date format" in result.output + + +def test_claims_clear_honours_a_before_cutoff(store: KBStore) -> None: + _claim(store, "c1", "an auto-saved claim", auto=True) + # the claim was created now, so a cutoff in the past matches nothing + out = _ok(["claims-clear", "--before", "2000-01-01", "--dry-run"]).output + assert "no claims match the criteria" in out + + +# --- wipe-dead-refs ------------------------------------------------------- + + +def _page_with_dead_ref(store: KBStore) -> None: + """A page citing a claim whose file is gone. + + `put_page` validates claim refs, so the ref has to go dead *after* the + page lands -- which is exactly how these arise in practice (the claim was + redacted or bulk-cleared out from under the page). + """ + _claim(store, "c1", "a claim that will vanish") + store.put_page(Page(id="p1", title="a page", claims=["c1"])) + store._claim_path("c1").unlink() + + +def test_wipe_dead_refs_with_nothing_to_do(store: KBStore) -> None: + assert "no dead claim references found" in _ok(["wipe-dead-refs"]).output + + +def test_wipe_dead_refs_dry_run_keeps_the_ref(store: KBStore) -> None: + _page_with_dead_ref(store) + out = _ok(["wipe-dead-refs", "--dry-run"]).output + assert "found 1 dead claim reference(s)" in out + assert "page p1: c1" in out + assert "(dry-run mode: no changes made)" in out + assert store.get_page("p1").claims == ["c1"] + + +def test_wipe_dead_refs_declined_at_the_prompt_cancels(store: KBStore) -> None: + _page_with_dead_ref(store) + out = _ok(["wipe-dead-refs"], stdin="n\n").output + assert "cancelled" in out + assert store.get_page("p1").claims == ["c1"] + + +def test_wipe_dead_refs_confirmed_strips_the_ref(store: KBStore) -> None: + _page_with_dead_ref(store) + out = _ok(["wipe-dead-refs", "--confirm"]).output + assert "stripped 1 dead reference(s)" in out + assert store.get_page("p1").claims == [] + + +# --- source --------------------------------------------------------------- + + +def test_source_add_registers_and_prints_the_id( + store: KBStore, tmp_path: Path +) -> None: + doc = tmp_path / "note.txt" + doc.write_text("some evidence", encoding="utf-8") + src_id = _ok(["source", "add", str(doc)]).output.strip() + assert store.get_source(src_id).title == "note.txt" + + +def test_source_add_honours_title_url_and_type( + store: KBStore, tmp_path: Path +) -> None: + doc = tmp_path / "note.txt" + doc.write_text("some evidence", encoding="utf-8") + src_id = _ok([ + "source", "add", str(doc), + "--title", "a titled note", + "--url", "https://example.invalid/note", + "--type", "url", + ]).output.strip() + src = store.get_source(src_id) + assert src.title == "a titled note" + assert src.type.value == "url" + # `--url` is accepted but discarded for `source add`: put_source folds url + # into `locator` only when locator is unset, and the command always passes + # the resolved path. documenting, not endorsing. + assert src.locator == str(doc.resolve()) + + +def test_source_list_empty_and_populated(store: KBStore) -> None: + assert "no sources found" in _ok(["source", "list"]).output + src = store.put_source(b"body", title="the memo") + assert src.id in _ok(["source", "list"]).output + + +def test_source_list_json(store: KBStore) -> None: + store.put_source(b"body", title="the memo") + doc = json.loads(_ok(["source", "list", "--json"]).output) + assert doc[0]["title"] == "the memo" + + +def test_source_verify_reports_each_source(store: KBStore) -> None: + store.put_source(b"body", title="the memo") + out = _ok(["source", "verify"]).output + assert "stored=" in out + assert "external=" in out + + +def test_source_verify_passes_when_the_file_still_matches( + store: KBStore, tmp_path: Path +) -> None: + doc = tmp_path / "note.txt" + doc.write_text("some evidence", encoding="utf-8") + _ok(["source", "add", str(doc)]) + result = _ok(["source", "verify", "--fail-on-issue"]) + assert "external=match" in result.output + + +def test_source_verify_fail_on_issue_exits_nonzero_on_drift( + store: KBStore, tmp_path: Path +) -> None: + doc = tmp_path / "note.txt" + doc.write_text("some evidence", encoding="utf-8") + _ok(["source", "add", str(doc)]) + # rewriting the file behind the recorded sha256 is the drift case + doc.write_text("tampered evidence", encoding="utf-8") + result = _run(["source", "verify", "--fail-on-issue"]) + assert result.exit_code == 1 + assert "!" in result.output + + +# --- notify --------------------------------------------------------------- + + +def test_notify_sweep_with_nothing_to_fire(store: KBStore) -> None: + assert "nothing to fire" in _ok(["notify", "sweep"]).output + + +def test_notify_sweep_reports_fired_events( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(notify_mod, "sweep", lambda _store: ["pending.threshold"]) + out = _ok(["notify", "sweep"]).output + assert "fired 1 event(s): pending.threshold" in out + + +def test_notify_test_reports_delivery( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(notify_mod, "send_test", lambda url, secret=None: True) + out = _ok(["notify", "test", "--url", "https://example.invalid/hook"]).output + assert "delivered" in out + + +def test_notify_test_exits_nonzero_on_failure( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(notify_mod, "send_test", lambda url, secret=None: False) + result = _run(["notify", "test", "--url", "https://example.invalid/hook"]) + assert result.exit_code == 1 + assert "delivery failed" in result.output + + +def test_notify_test_resolves_a_secret_from_env( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + seen: dict[str, str | None] = {} + + def _send(url: str, secret: str | None = None) -> bool: + seen["secret"] = secret + return True + + monkeypatch.setenv("VOUCH_TEST_HOOK_SECRET", "not-a-real-secret") + monkeypatch.setattr(notify_mod, "send_test", _send) + _ok([ + "notify", "test", + "--url", "https://example.invalid/hook", + "--secret", "env:VOUCH_TEST_HOOK_SECRET", + ]) + assert seen["secret"] == "not-a-real-secret" + + +def test_notify_test_unresolvable_secret_is_a_clean_error(store: KBStore) -> None: + _clean_error([ + "notify", "test", + "--url", "https://example.invalid/hook", + "--secret", "env:VOUCH_NOT_SET_ANYWHERE", + ]) diff --git a/tests/test_cli_maintenance.py b/tests/test_cli_maintenance.py new file mode 100644 index 00000000..e7107891 --- /dev/null +++ b/tests/test_cli_maintenance.py @@ -0,0 +1,267 @@ +"""The maintenance / health / index CLI surface. + +`lint`, `doctor`, `fsck`, `reindex`, `audit`, `dedup`, `contradict-scan`, +`provenance rebuild`, `embeddings stats`, `list-skills` and `get-skill` were +all import-covered only. They are the commands a user reaches for when the KB +is already suspect, so a traceback here lands at the worst possible moment. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import numpy as np +import pytest +from click.testing import CliRunner, Result + +from vouch.cli import cli +from vouch.embeddings import register +from vouch.embeddings.base import DEFAULT_MODEL_NAME, Embedder +from vouch.models import Claim +from vouch.proposals import propose_claim +from vouch.storage import KBStore + + +class _HashEmbedder(Embedder): + name = "mock" + version = "1" + dim = 8 + + def encode(self, text: str) -> np.ndarray: + import hashlib + + h = hashlib.sha256(text.encode()).digest() + out = np.array([h[i] / 255.0 for i in range(self.dim)], dtype=np.float32) + norm = float(np.linalg.norm(out)) + if norm > 0: + out /= norm + return out + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + s = KBStore.init(tmp_path) + monkeypatch.chdir(s.root) + return s + + +@pytest.fixture +def embedder() -> None: + register(DEFAULT_MODEL_NAME, _HashEmbedder) + + +def _run(args: list[str]) -> Result: + return CliRunner().invoke(cli, args) + + +def _ok(args: list[str]) -> Result: + result = _run(args) + assert result.exit_code == 0, result.output + return result + + +def _no_traceback(args: list[str]) -> Result: + result = _run(args) + assert "Traceback" not in result.output, result.output + return result + + +def _claim(store: KBStore, claim_id: str, text: str) -> Claim: + src = store.put_source(b"evidence body") + return store.put_claim(Claim(id=claim_id, text=text, evidence=[src.id])) + + +# --- status --------------------------------------------------------------- + + +def test_status_table_lists_counts(store: KBStore) -> None: + _claim(store, "c1", "a durable claim") + out = _ok(["status"]).output + assert "KB at" in out + assert "durable:" in out + assert "pending:" in out + assert "audit:" in out + + +def test_status_json_is_machine_readable(store: KBStore) -> None: + _claim(store, "c1", "a durable claim") + doc = json.loads(_ok(["status", "--json"]).output) + assert doc["claims"] == 1 + assert doc["pending_proposals"] == 0 + assert "kb_dir" in doc + + +def test_status_counts_pending_proposals(store: KBStore) -> None: + src = store.put_source(b"e") + propose_claim(store, text="pending one", evidence=[src.id], proposed_by="agent") + doc = json.loads(_ok(["status", "--json"]).output) + assert doc["pending_proposals"] == 1 + + +# --- lint / doctor / fsck ------------------------------------------------- + + +def test_lint_on_a_fresh_kb_is_clean(store: KBStore) -> None: + result = _no_traceback(["lint"]) + assert result.exit_code == 0 + assert "clean" in result.output + + +def test_lint_accepts_a_stale_day_window(store: KBStore) -> None: + _claim(store, "c1", "a claim") + result = _no_traceback(["lint", "--stale-days", "1"]) + assert result.exit_code in (0, 1) + + +def test_doctor_prints_a_counts_footer(store: KBStore) -> None: + result = _no_traceback(["doctor"]) + assert result.exit_code in (0, 1) + assert "--" in result.output + + +def test_fsck_flags_a_missing_index(store: KBStore) -> None: + # a KB that has never been indexed reports index_missing at info severity, + # which is advisory: the command still exits 0 + result = _no_traceback(["fsck"]) + assert result.exit_code == 0 + assert "index_missing" in result.output + + +def test_fsck_is_clean_once_indexed(store: KBStore) -> None: + _claim(store, "c1", "a claim") + _ok(["index"]) + result = _no_traceback(["fsck"]) + assert result.exit_code == 0 + assert "clean" in result.output + + +# --- index / provenance --------------------------------------------------- + + +def test_reindex_rebuilds_fts5_by_default(store: KBStore) -> None: + _claim(store, "c1", "indexed claim") + assert "reindex: FTS5 rebuilt" in _ok(["reindex"]).output + + +def test_reindex_backfills_embeddings_when_asked( + store: KBStore, embedder: None +) -> None: + _claim(store, "c1", "claim to embed") + out = _ok(["reindex", "--embeddings"]).output + assert "reindex: embeddings backfilled" in out + + +def test_reindex_force_backfill_re_encodes(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "claim to embed") + out = _ok(["reindex", "--backfill", "--force"]).output + assert "reindex: embeddings backfilled" in out + + +def test_provenance_rebuild_reports_edge_count(store: KBStore) -> None: + _claim(store, "c1", "a claim with evidence") + assert "provenance: rebuilt" in _ok(["provenance", "rebuild"]).output + + +def test_provenance_rebuild_json(store: KBStore) -> None: + _claim(store, "c1", "a claim with evidence") + doc = json.loads(_ok(["provenance", "rebuild", "--json"]).output) + assert isinstance(doc["edges"], int) + + +def test_embeddings_stats_reports_counts_and_cache( + store: KBStore, embedder: None +) -> None: + _claim(store, "c1", "a claim to embed") + out = _ok(["embeddings", "stats"]).output + assert "query_cache_entries" in out + assert "query_cache_hits" in out + + +# --- advisory scans ------------------------------------------------------- + + +def test_dedup_on_a_fresh_kb_finds_nothing(store: KBStore, embedder: None) -> None: + assert "dedup: no duplicates found" in _ok(["dedup"]).output + + +def test_dedup_reports_a_near_duplicate_pair( + store: KBStore, embedder: None +) -> None: + _claim(store, "c1", "identical duplicated text") + _claim(store, "c2", "identical duplicated text") + out = _ok(["dedup"]).output + assert "cos=" in out + assert "claim/c2" in out or "claim/c1" in out + + +def test_contradict_scan_on_a_fresh_kb_finds_nothing(store: KBStore) -> None: + out = _ok(["contradict-scan"]).output + assert "contradict-scan: no candidates found" in out + + +def test_contradict_scan_dry_run_writes_no_proposals(store: KBStore) -> None: + _claim(store, "c1", "the gate is enabled") + _claim(store, "c2", "the gate is not enabled") + _no_traceback(["contradict-scan", "--dry-run"]) + assert store.list_proposals() == [] + + +# --- audit ---------------------------------------------------------------- + + +def test_audit_tail_lists_events(store: KBStore) -> None: + src = store.put_source(b"e") + pr = propose_claim(store, text="x", evidence=[src.id], proposed_by="agent") + _ok(["approve", pr.id]) + out = _ok(["audit"]).output + assert "by " in out + assert "objects=" in out + + +def test_audit_json_includes_viewer_and_events(store: KBStore) -> None: + src = store.put_source(b"e") + pr = propose_claim(store, text="x", evidence=[src.id], proposed_by="agent") + _ok(["approve", pr.id]) + doc = json.loads(_ok(["audit", "--json"]).output) + assert "viewer" in doc + assert doc["events"] + + +def test_audit_tail_caps_the_event_count(store: KBStore) -> None: + src = store.put_source(b"e") + for i in range(3): + pr = propose_claim( + store, text=f"claim {i}", evidence=[src.id], proposed_by="agent" + ) + _ok(["approve", pr.id]) + doc = json.loads(_ok(["audit", "--json", "--tail", "2"]).output) + assert len(doc["events"]) == 2 + + +def test_audit_echoes_the_viewer_when_scoped(store: KBStore) -> None: + src = store.put_source(b"e") + pr = propose_claim(store, text="x", evidence=[src.id], proposed_by="agent") + _ok(["approve", pr.id]) + result = _ok(["audit", "--project", "acme-example", "--agent", "claude-code"]) + assert "viewer:" in result.output + + +# --- skills --------------------------------------------------------------- + + +def test_list_skills_on_a_bare_kb(store: KBStore) -> None: + out = _ok(["list-skills"]).output + assert "no skills published" in out or "[" in out + + +def test_list_skills_json_is_a_list(store: KBStore) -> None: + doc = json.loads(_ok(["list-skills", "--json"]).output) + assert isinstance(doc, list) + + +def test_get_skill_unknown_name_is_a_clean_error(store: KBStore) -> None: + result = _run(["get-skill", "no-such-skill"]) + assert result.exit_code != 0 + assert "Traceback" not in result.output + assert "Error:" in result.output diff --git a/tests/test_cli_read_list.py b/tests/test_cli_read_list.py new file mode 100644 index 00000000..5c95ad3b --- /dev/null +++ b/tests/test_cli_read_list.py @@ -0,0 +1,236 @@ +"""The read-*/list-* CLI surface plus the small read-only commands. + +These are the human mirror of the `kb_read_*` MCP tools. Every one of them +was import-covered only — the decorator ran, the body never did — so a +regression in any of them (wrong yaml shape, a crash on an empty KB, a +traceback instead of a clean `Error:` line) would have shipped silently. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +import yaml +from click.testing import CliRunner, Result + +from vouch.cli import cli +from vouch.models import Claim, Entity, Evidence, Page, Relation +from vouch.storage import KBStore + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + s = KBStore.init(tmp_path) + monkeypatch.chdir(s.root) + return s + + +def _run(args: list[str]) -> Result: + return CliRunner().invoke(cli, args) + + +def _ok(args: list[str]) -> Result: + result = _run(args) + assert result.exit_code == 0, result.output + return result + + +def _clean_error(args: list[str]) -> Result: + result = _run(args) + assert result.exit_code != 0, result.output + assert "Traceback" not in result.output, result.output + assert "Error:" in result.output, result.output + return result + + +# --- read-* --------------------------------------------------------------- + + +def test_read_claim_emits_yaml(store: KBStore) -> None: + src = store.put_source(b"evidence") + store.put_claim(Claim(id="c1", text="the gate stays", evidence=[src.id])) + doc = yaml.safe_load(_ok(["read-claim", "c1"]).output) + assert doc["id"] == "c1" + assert doc["text"] == "the gate stays" + + +def test_read_page_emits_yaml(store: KBStore) -> None: + store.put_page(Page(id="p1", title="review gate")) + doc = yaml.safe_load(_ok(["read-page", "p1"]).output) + assert doc["id"] == "p1" + assert doc["title"] == "review gate" + + +def test_read_entity_emits_yaml(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="acme-example", type="company")) + doc = yaml.safe_load(_ok(["read-entity", "e1"]).output) + assert doc["name"] == "acme-example" + assert doc["type"] == "company" + + +def test_read_relation_emits_yaml(store: KBStore) -> None: + # both endpoints must already exist -- storage validates relation refs + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + store.put_relation( + Relation(id="r1", source="e1", relation="owned_by", target="e2") + ) + doc = yaml.safe_load(_ok(["read-relation", "r1"]).output) + assert doc["source"] == "e1" + assert doc["relation"] == "owned_by" + assert doc["target"] == "e2" + + +def test_read_source_emits_yaml(store: KBStore) -> None: + src = store.put_source(b"body", title="a note") + doc = yaml.safe_load(_ok(["read-source", src.id]).output) + assert doc["id"] == src.id + assert doc["title"] == "a note" + + +def test_read_evidence_emits_yaml(store: KBStore) -> None: + src = store.put_source(b"body") + store.put_evidence(Evidence(id="ev1", source_id=src.id, locator="p1")) + doc = yaml.safe_load(_ok(["read-evidence", "ev1"]).output) + assert doc["id"] == "ev1" + assert doc["source_id"] == src.id + + +@pytest.mark.parametrize( + "command", + [ + "read-claim", + "read-page", + "read-entity", + "read-relation", + "read-evidence", + "read-source", + ], +) +def test_read_missing_id_is_a_clean_error(store: KBStore, command: str) -> None: + _clean_error([command, "does-not-exist"]) + + +# --- list-* --------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("command", "empty_line"), + [ + ("list-claims", "no claims found"), + ("list-pages", "no pages found"), + ("list-entities", "no entities found"), + ("list-relations", "no relations found"), + ], +) +def test_list_on_empty_kb_says_so( + store: KBStore, command: str, empty_line: str +) -> None: + assert empty_line in _ok([command]).output + + +def test_list_claims_shows_id_and_text(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="first claim", evidence=[src.id])) + store.put_claim(Claim(id="c2", text="second claim", evidence=[src.id])) + out = _ok(["list-claims"]).output + assert "c1" in out and "first claim" in out + assert "c2" in out and "second claim" in out + + +def test_list_pages_shows_id_and_title(store: KBStore) -> None: + store.put_page(Page(id="p1", title="the review gate")) + out = _ok(["list-pages"]).output + assert "p1" in out and "the review gate" in out + + +def test_list_entities_shows_name_and_type(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + out = _ok(["list-entities"]).output + assert "alice-example" in out + assert "(person)" in out + + +def test_list_relations_shows_the_triple(store: KBStore) -> None: + store.put_entity(Entity(id="alice-example", name="alice", type="person")) + store.put_entity(Entity(id="acme-example", name="acme", type="company")) + store.put_relation( + Relation(id="r1", source="alice-example", relation="owned_by", + target="acme-example") + ) + out = _ok(["list-relations"]).output + assert "alice-example -> owned_by -> acme-example" in out + + +# --- small read-only commands -------------------------------------------- + + +def test_capabilities_emits_the_method_list(store: KBStore) -> None: + doc = json.loads(_ok(["capabilities"]).output) + assert "kb.search" in doc["methods"] + + +def test_capabilities_outside_a_kb_still_asks_for_init( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # documents current behaviour, which contradicts the comment above the + # try/except in cli.capabilities: `_load_store()` exits via SystemExit, + # which `except Exception` cannot catch, so the no-KB fallback is dead. + outside = tmp_path / "not-a-kb" + outside.mkdir() + monkeypatch.chdir(outside) + result = _run(["capabilities"]) + assert result.exit_code == 2 + assert "No .vouch/ directory found" in result.output + + +def test_capabilities_falls_back_when_skills_lookup_raises( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + from vouch import skills as skills_mod + + def _boom(_store: KBStore) -> bool: + raise RuntimeError("config unreadable") + + monkeypatch.setattr(skills_mod, "publish_skills_enabled", _boom) + doc = json.loads(_ok(["capabilities"]).output) + assert doc["methods"] + + +def test_index_rebuilds_state_db(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="indexed claim", evidence=[src.id])) + assert "indexed:" in _ok(["index"]).output + + +def test_context_emits_a_pack_for_the_task(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim( + Claim(id="c1", text="the review gate is load-bearing", evidence=[src.id]) + ) + doc = json.loads(_ok(["context", "review gate"]).output) + assert "items" in doc + + +def test_context_respects_limit(store: KBStore) -> None: + src = store.put_source(b"e") + for i in range(5): + store.put_claim(Claim(id=f"c{i}", text=f"gate claim {i}", evidence=[src.id])) + doc = json.loads(_ok(["context", "gate", "--limit", "2"]).output) + assert len(doc["items"]) <= 2 + + +def test_neighbors_emits_json_for_a_known_node(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="a claim with evidence", evidence=[src.id])) + doc = json.loads(_ok(["neighbors", "c1"]).output) + assert doc["node_id"] == "c1" + assert doc["kind"] == "claim" + assert isinstance(doc["nodes"], list) + assert isinstance(doc["edges"], list) + + +def test_neighbors_unknown_node_is_a_clean_error(store: KBStore) -> None: + _clean_error(["neighbors", "no-such-node"]) diff --git a/tests/test_digest.py b/tests/test_digest.py index 4d3eae40..437ee110 100644 --- a/tests/test_digest.py +++ b/tests/test_digest.py @@ -164,6 +164,32 @@ def test_build_limit_caps_followups(tmp_path: Path) -> None: assert len(d.followups_due) == 3 +def test_build_excludes_archived_followups(tmp_path: Path) -> None: + # archived pages stay in list_pages with open/due metadata; digest must + # drop them the same way recall drops ARCHIVED titles. + s = KBStore.init(tmp_path) + s.put_page( + Page( + id="live-due", + title="still open", + type="followup", + status=PageStatus.ACTIVE, + metadata={"due_at": "2026-07-01", "followup_status": "open"}, + ) + ) + s.put_page( + Page( + id="archived-due", + title="closed by archive", + type="followup", + status=PageStatus.ARCHIVED, + metadata={"due_at": "2026-06-01", "followup_status": "open"}, + ) + ) + d = digest_mod.build(s, now=NOW) + assert [r.id for r in d.followups_due] == ["live-due"] + + def test_digest_is_read_only(store: KBStore) -> None: audit_before = (store.kb_dir / "audit.log.jsonl").read_text(encoding="utf-8") files_before = sorted(p.name for p in (store.kb_dir / "proposed").glob("*")) diff --git a/tests/test_enrich.py b/tests/test_enrich.py index 0f8898ff..2cd8514e 100644 --- a/tests/test_enrich.py +++ b/tests/test_enrich.py @@ -63,6 +63,15 @@ def test_enrich_config_reads_override(store: KBStore) -> None: assert cfg.timeout_seconds == 10.0 +def test_enrich_config_quoted_false_does_not_enable(store: KBStore) -> None: + """Regression (#558 residual): bool(\"false\") is True, so a quoted + enabled: \"false\" used to leave capture.enrich on.""" + store.config_path.write_text( + 'capture:\n enrich:\n enabled: "false"\n', encoding="utf-8" + ) + assert load_enrich_config(store).enabled is False + + def test_enrich_config_malformed_yaml_falls_back(store: KBStore) -> None: store.config_path.write_text("capture:\n enrich:\n - nope\n", encoding="utf-8") assert load_enrich_config(store) == EnrichConfig() diff --git a/tests/test_explain_ranking.py b/tests/test_explain_ranking.py new file mode 100644 index 00000000..eba2842a --- /dev/null +++ b/tests/test_explain_ranking.py @@ -0,0 +1,375 @@ +"""Read-only ranking introspection — issue #432.""" + +from __future__ import annotations + +import sqlite3 +from pathlib import Path + +import pytest +import yaml + +from vouch import explain_ranking as er +from vouch import health +from vouch.models import ( + ArtifactScope, + Claim, + ClaimStatus, + Page, + PageStatus, + PageType, + Visibility, +) +from vouch.storage import KBStore + + +def _write_cfg(store: KBStore, **retrieval: object) -> None: + (store.kb_dir / "config.yaml").write_text( + yaml.safe_dump({"retrieval": retrieval}), encoding="utf-8" + ) + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + kb = KBStore.init(tmp_path) + monkeypatch.chdir(kb.root) + src = kb.put_source(b"auth notes") + kb.put_claim(Claim(id="c1", text="auth uses jwt tokens", evidence=[src.id])) + kb.put_claim(Claim(id="c2", text="jwt rotation is manual", evidence=[src.id])) + health.rebuild_index(kb) + return kb + + +def _by_id(result: dict) -> dict[str, dict]: + return {c["id"]: c for c in result["candidates"]} + + +def _stages(candidate: dict) -> list[str]: + return [row["stage"] for row in candidate["stages"]] + + +# --- the fused-only query the issue asks for ----------------------------- + + +def test_fused_only_query_reports_per_retriever_ranks(store: KBStore) -> None: + """A fused query exposes lexical rank, semantic rank and the rrf score.""" + result = er.explain_ranking(store, query="jwt", limit=5) + + assert result["retrieval"]["used"] == "hybrid" + assert result["retrieval"]["stages"]["fusion"] is True + cand = _by_id(result)["c1"] + # fts5 served this query; the embedding retriever is absent without extras, + # which is exactly the asymmetry the breakdown is supposed to make visible. + assert cand["lexical_rank"] is not None + assert cand["rrf_contribution"] > 0 + assert cand["gate"] == "kept" + assert _stages(cand)[0] == "hybrid" + assert "limit" in _stages(cand) + + +def test_first_stage_row_has_no_deltas(store: KBStore) -> None: + """Nothing precedes fusion, so its deltas are null rather than zero.""" + first = _by_id(er.explain_ranking(store, query="jwt"))["c1"]["stages"][0] + assert first["rank_delta"] is None + assert first["score_delta"] is None + + +# --- gate-dropped candidates --------------------------------------------- + + +@pytest.mark.parametrize( + "retracted", [ClaimStatus.ARCHIVED, ClaimStatus.SUPERSEDED, ClaimStatus.REDACTED] +) +def test_retracted_claim_reports_status_filtered( + store: KBStore, retracted: ClaimStatus +) -> None: + store.put_claim(Claim( + id="c-dead", text="jwt replaced by sessions", + evidence=[store.list_sources()[0].id], status=retracted, + )) + health.rebuild_index(store) + + cand = _by_id(er.explain_ranking(store, query="jwt", limit=5))["c-dead"] + assert cand["gate"] == "status-filtered" + # it is explained up to the stage that cut it, not silently missing + assert _stages(cand) == ["hybrid", "scope_filter"] + + +def test_archived_page_reports_status_filtered(store: KBStore) -> None: + store.put_page(Page( + id="p-arch", title="jwt legacy design", body="old", + type=PageType.CONCEPT, status=PageStatus.ARCHIVED, + )) + health.rebuild_index(store) + + assert _by_id(er.explain_ranking(store, query="jwt", limit=5))["p-arch"]["gate"] == ( + "status-filtered" + ) + + +def test_truncated_candidate_reports_limit_dropped(store: KBStore) -> None: + """A candidate lost to the window is attributed to `limit`, not the filters.""" + result = er.explain_ranking(store, query="jwt", limit=1) + gates = {c["gate"] for c in result["candidates"]} + assert "limit-dropped" in gates + dropped = next(c for c in result["candidates"] if c["gate"] == "limit-dropped") + assert _stages(dropped)[-1] == "strategy" + + +def test_budget_gate_reports_budget_dropped(store: KBStore) -> None: + result = er.explain_ranking(store, query="jwt", limit=5, max_chars=1) + assert result["retrieval"]["stages"]["budget"] == 1 + assert "budget-dropped" in {c["gate"] for c in result["candidates"]} + + +def test_scope_filtered_candidate_is_attributed_to_scope(store: KBStore) -> None: + """A viewer-invisible claim dies at scope_filter, before the status gate.""" + store.put_claim(Claim( + id="c-priv", text="jwt secret lives in vault", + evidence=[store.list_sources()[0].id], + scope=ArtifactScope(visibility=Visibility.PRIVATE, project="other-project"), + )) + health.rebuild_index(store) + + cand = _by_id(er.explain_ranking( + store, query="jwt", limit=5, project="this-project" + ))["c-priv"] + assert cand["gate"] == "scope-filtered" + assert _stages(cand) == ["hybrid"] + + +def test_require_citations_names_the_uncited_claim(store: KBStore) -> None: + """The uncited gate renames the responsible candidate, it does not drop it.""" + result = er.explain_ranking( + store, query="jwt", limit=5, require_citations=True + ) + assert result["retrieval"]["stages"]["require_citations"] is True + # every fixture claim cites a source, so nothing is uncited + assert {c["gate"] for c in result["candidates"]} == {"kept"} + assert er._is_uncited(store, ("claim", "c1")) is False + assert er._is_uncited(store, ("page", "p1")) is False + + +def test_uncited_gate_renames_a_surviving_claim( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + """The uncited branch is defensive — force it to pin the reported gate. + + ``Claim`` rejects ``evidence=[]`` on the model, so no stored claim can be + uncited and this path cannot be reached through the public API. Forcing the + predicate is the only way to assert the gate the branch would report if + that invariant were ever relaxed. + """ + monkeypatch.setattr(er, "_is_uncited", lambda _store, key: key[1] == "c1") + result = er.explain_ranking(store, query="jwt", limit=5, require_citations=True) + + by_id = _by_id(result) + assert by_id["c1"]["gate"] == "uncited" + # the item is renamed, never dropped — its full stage chain is intact + assert by_id["c1"]["stages"][-1]["stage"] == "limit" + assert by_id["c2"]["gate"] == "kept" + + +def test_uncited_gate_is_not_applied_without_require_citations( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + """Without the flag the gate stays `kept` even for an uncited claim.""" + monkeypatch.setattr(er, "_is_uncited", lambda _store, _key: True) + result = er.explain_ranking(store, query="jwt", limit=5) + assert {c["gate"] for c in result["candidates"]} == {"kept"} + + +# --- stage reporting ------------------------------------------------------ + + +def test_disabled_stages_are_reported_as_not_applied(store: KBStore) -> None: + """A stage that is off still appears, flagged, so the chain reads whole.""" + _write_cfg(store, backend="hybrid", recency={"enabled": False}) + cand = _by_id(er.explain_ranking(store, query="jwt"))["c1"] + recency = next(r for r in cand["stages"] if r["stage"] == "recency") + assert recency["applied"] is False + assert er.explain_ranking(store, query="jwt")["retrieval"]["stages"]["recency"] is ( + False + ) + + +def test_recency_reports_a_score_delta(store: KBStore) -> None: + """Recency is rescoring-only, so its signal is the score delta.""" + _write_cfg(store, backend="hybrid", recency={"enabled": True, "half_life_days": 1}) + cand = _by_id(er.explain_ranking(store, query="jwt"))["c1"] + recency = next(r for r in cand["stages"] if r["stage"] == "recency") + assert recency["applied"] is True + assert recency["score_delta"] is not None + + +def test_pages_first_stage_is_reported(store: KBStore) -> None: + store.put_page(Page(id="p-live", title="jwt design", body="current", + type=PageType.CONCEPT)) + health.rebuild_index(store) + _write_cfg(store, backend="hybrid", pages_first={"enabled": True, "boost": 2.0}) + + cand = _by_id(er.explain_ranking(store, query="jwt", limit=5))["p-live"] + pages_first = next(r for r in cand["stages"] if r["stage"] == "pages_first") + assert pages_first["applied"] is True + + +def test_strategy_stage_reports_the_configured_plugin(store: KBStore) -> None: + _write_cfg(store, backend="hybrid", strategy="vouch.strategies.provenance") + result = er.explain_ranking(store, query="jwt") + assert result["retrieval"]["stages"]["strategy"] == "vouch.strategies.provenance" + strategy = next( + r for r in _by_id(result)["c1"]["stages"] if r["stage"] == "strategy" + ) + assert strategy["applied"] is True + + +def test_rerank_top_k_is_reported_only_when_rerank_is_on(store: KBStore) -> None: + assert er.explain_ranking(store, query="jwt")["retrieval"]["stages"][ + "rerank_top_k" + ] is None + + +# --- backend branches ----------------------------------------------------- + + +def test_pinned_fts5_backend_is_explained(store: KBStore) -> None: + _write_cfg(store, backend="fts5") + result = er.explain_ranking(store, query="jwt") + assert result["retrieval"]["used"] == "fts5" + assert result["retrieval"]["stages"]["fusion"] is False + assert _by_id(result)["c1"]["lexical_rank"] is not None + + +def test_pinned_embedding_backend_is_explained(store: KBStore) -> None: + _write_cfg(store, backend="embedding") + result = er.explain_ranking(store, query="jwt") + assert result["retrieval"]["used"] == "embedding" + + +def test_pinned_substring_backend_is_explained(store: KBStore) -> None: + _write_cfg(store, backend="substring") + result = er.explain_ranking(store, query="jwt") + assert result["retrieval"]["used"] == "substring" + assert _by_id(result)["c1"]["gate"] == "kept" + + +def test_auto_falls_through_to_substring_when_retrievers_are_empty( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + """The substring fall-through is reported as the backend that served it.""" + from vouch import index_db + + monkeypatch.setattr(index_db, "search", lambda *a, **k: []) + monkeypatch.setattr(index_db, "search_semantic", lambda *a, **k: []) + result = er.explain_ranking(store, query="jwt") + assert result["retrieval"]["used"] == "substring" + + +def test_lexical_sqlite_error_degrades_to_no_lexical_hits( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + """A broken fts5 index must not fault the explanation.""" + from vouch import index_db + + def _boom(*_a: object, **_k: object) -> list: + raise sqlite3.Error("fts5 index is gone") + + monkeypatch.setattr(index_db, "search", _boom) + result = er.explain_ranking(store, query="jwt") + assert all(c["lexical_rank"] is None for c in result["candidates"]) + + +def test_negative_limit_is_rejected(store: KBStore) -> None: + with pytest.raises(ValueError, match="limit must be >= 0"): + er.explain_ranking(store, query="jwt", limit=-1) + + +def test_viewer_is_echoed_back(store: KBStore) -> None: + result = er.explain_ranking(store, query="jwt", project="proj-a", agent="agent-b") + assert result["viewer"] == {"project": "proj-a", "agent": "agent-b"} + + +# --- surface parity ------------------------------------------------------- + + +def test_jsonl_surface_serves_explain_ranking( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + from vouch import jsonl_server + + monkeypatch.setattr(jsonl_server, "_store", lambda: store) + result = jsonl_server.HANDLERS["kb.explain_ranking"]({ + "query": "jwt", "limit": 5, "max_chars": 4000, "require_citations": False, + }) + assert result["retrieval"]["stages"]["budget"] == 4000 + assert _by_id(result)["c1"]["gate"] == "kept" + + +def test_mcp_surface_serves_explain_ranking( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + from vouch import server + + monkeypatch.setattr(server, "_store", lambda: store) + result = server.kb_explain_ranking("jwt", limit=5) + assert _by_id(result)["c1"]["gate"] == "kept" + + +def test_explain_ranking_is_registered_in_capabilities() -> None: + from vouch.capabilities import METHODS + + assert "kb.explain_ranking" in METHODS + + +def test_cli_text_output_names_stages_and_gates(store: KBStore) -> None: + from click.testing import CliRunner + + from vouch.cli import cli + + store.put_claim(Claim( + id="c-dead", text="jwt replaced by sessions", + evidence=[store.list_sources()[0].id], status=ClaimStatus.SUPERSEDED, + )) + health.rebuild_index(store) + _write_cfg(store, backend="hybrid", strategy="vouch.strategies.provenance", + recency={"enabled": True, "half_life_days": 1}) + + res = CliRunner().invoke( + cli, ["explain-ranking", "jwt", "--limit", "2"] + ) + assert res.exit_code == 0, res.output + assert "backend:" in res.output + assert "stages active:" in res.output + assert "strategy=vouch.strategies.provenance" in res.output + assert "gate=status-filtered" in res.output + assert "(off)" in res.output + + +def test_cli_json_output_is_machine_readable(store: KBStore) -> None: + import json + + from click.testing import CliRunner + + from vouch.cli import cli + + res = CliRunner().invoke( + cli, + ["explain-ranking", "jwt", + "--format", "json", "--require-citations", "--max-chars", "4000"], + ) + assert res.exit_code == 0, res.output + payload = json.loads(res.output) + assert payload["query"] == "jwt" + assert payload["retrieval"]["stages"]["require_citations"] is True + + +def test_cli_reports_no_active_stages_when_all_are_off(store: KBStore) -> None: + from click.testing import CliRunner + + from vouch.cli import cli + + _write_cfg(store, backend="substring", recency={"enabled": False}) + res = CliRunner().invoke( + cli, ["explain-ranking", "jwt"] + ) + assert res.exit_code == 0, res.output + assert "stages active: none" in res.output diff --git a/tests/test_health.py b/tests/test_health.py index 822d8a76..f81374c1 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -59,6 +59,25 @@ def test_doctor_runs_full_sweep(store: KBStore) -> None: assert report.ok is True +def test_doctor_warns_on_missing_external_file(store: KBStore, tmp_path: Path) -> None: + """source verify marks missing externals as '!'; doctor must surface + them too (not only drift).""" + f = tmp_path / "doc.txt" + f.write_bytes(b"original") + src = store.put_source( + f.read_bytes(), title="doc", + locator=str(f.resolve()), source_type="file", + ) + f.unlink() + report = health.doctor(store) + missing = [f for f in report.findings if f.code == "source_missing"] + assert missing, [f.code for f in report.findings] + assert missing[0].severity == "warning" + assert src.id in missing[0].object_ids + # warning-only — same posture as source_drift + assert report.ok is True + + def test_lint_surfaces_legacy_uncited_claim_yaml_without_crashing( store: KBStore, ) -> None: diff --git a/tests/test_index_db_embeddings.py b/tests/test_index_db_embeddings.py new file mode 100644 index 00000000..ad4da1c3 --- /dev/null +++ b/tests/test_index_db_embeddings.py @@ -0,0 +1,338 @@ +"""The embedding half of `index_db`, plus `storage._embed_and_store`. + +These are the vector paths behind `retrieval.backend: embedding`. They were +uncovered because the whole embeddings suite was being skipped, so the +blob round-trip, the sqlite-vec probe and its brute-force fallback, and the +write-through from `store.put_*` all ran untested. +""" + +from __future__ import annotations + +import hashlib +import sqlite3 +import sys +from pathlib import Path + +import numpy as np +import pytest + +from vouch import index_db +from vouch.embeddings import register +from vouch.embeddings.base import DEFAULT_MODEL_NAME, Embedder +from vouch.models import Claim, Entity, Page +from vouch.storage import KBStore + + +class _HashEmbedder(Embedder): + name = "mock" + version = "1" + dim = 8 + + def encode(self, text: str) -> np.ndarray: + h = hashlib.sha256(text.encode()).digest() + out = np.array([h[i] / 255.0 for i in range(self.dim)], dtype=np.float32) + norm = float(np.linalg.norm(out)) + if norm > 0: + out /= norm + return out + + +@pytest.fixture +def store(tmp_path: Path) -> KBStore: + return KBStore.init(tmp_path / "kb") + + +@pytest.fixture +def embedder() -> None: + register(DEFAULT_MODEL_NAME, _HashEmbedder) + + +def _vec(text: str) -> np.ndarray: + return _HashEmbedder().encode(text) + + +def _put(store: KBStore, kind: str, eid: str, text: str) -> None: + with index_db.open_db(store.kb_dir) as conn: + index_db.put_embedding( + conn, kind=kind, id=eid, vec=_vec(text), + content_hash=hashlib.sha256(text.encode()).hexdigest(), + model="mock", model_version="1", dim=8, + ) + + +# --- blob round-trip ------------------------------------------------------ + + +def test_vec_blob_round_trip_preserves_the_vector() -> None: + vec = _vec("round trip me") + restored = index_db._blob_to_vec(index_db._vec_to_blob(vec), 8) + assert np.allclose(vec, restored) + + +def test_put_and_get_embedding_round_trip(store: KBStore) -> None: + _put(store, "claim", "c1", "stored claim text") + got = index_db.get_embedding(store.kb_dir, kind="claim", id="c1") + assert got is not None + vec, content_hash, model = got + assert np.allclose(vec, _vec("stored claim text")) + assert model == "mock" + assert content_hash == hashlib.sha256(b"stored claim text").hexdigest() + + +def test_get_embedding_returns_none_when_absent(store: KBStore) -> None: + assert index_db.get_embedding(store.kb_dir, kind="claim", id="ghost") is None + + +def test_put_embedding_replaces_an_existing_row(store: KBStore) -> None: + # INSERT OR REPLACE: re-embedding the same artifact must not raise a + # UNIQUE violation on (kind, id) + _put(store, "claim", "c1", "first text") + _put(store, "claim", "c1", "second text") + got = index_db.get_embedding(store.kb_dir, kind="claim", id="c1") + assert got is not None + assert np.allclose(got[0], _vec("second text")) + + +# --- embedding meta ------------------------------------------------------- + + +def test_embedding_meta_round_trip(store: KBStore) -> None: + index_db.set_embedding_meta(store.kb_dir, model="mock", version="1", dim=8) + meta = index_db.get_embedding_meta(store.kb_dir) + assert meta["embedding_model"] == "mock" + assert meta["embedding_model_version"] == "1" + assert meta["embedding_dim"] == "8" + + +def test_embedding_meta_is_empty_on_a_fresh_index(store: KBStore) -> None: + assert index_db.get_embedding_meta(store.kb_dir) == {} + + +# --- sqlite-vec probe ----------------------------------------------------- + + +def test_load_sqlite_vec_reports_false_without_the_extension( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + # the [embeddings-fast] extra is absent here, so the probe must degrade + # rather than raise -- search_embedding relies on that to fall back + monkeypatch.setitem(sys.modules, "sqlite_vec", None) + with index_db.open_db(store.kb_dir) as conn: + assert index_db._load_sqlite_vec(conn) is False + + +def test_load_sqlite_vec_handles_a_build_without_extension_support() -> None: + # python built against a sqlite without loadable-extension support has no + # enable_load_extension at all; the probe must return False, not blow up + class _NoLoader: + pass + + assert index_db._load_sqlite_vec(_NoLoader()) is False # type: ignore[arg-type] + + +def test_load_sqlite_vec_handles_a_disabled_loader() -> None: + class _Refuses: + def enable_load_extension(self, _flag: bool) -> None: + raise sqlite3.OperationalError("extension loading disabled") + + assert index_db._load_sqlite_vec(_Refuses()) is False # type: ignore[arg-type] + + +# --- search_embedding (brute-force fallback) ----------------------------- + + +def test_search_embedding_ranks_the_exact_match_first(store: KBStore) -> None: + _put(store, "claim", "c1", "the review gate is load-bearing") + _put(store, "claim", "c2", "something entirely unrelated") + hits = index_db.search_embedding( + store.kb_dir, query_vec=_vec("the review gate is load-bearing") + ) + assert hits[0][1] == "c1" + assert hits[0][3] == pytest.approx(1.0, abs=1e-4) + + +def test_search_embedding_filters_by_kind(store: KBStore) -> None: + _put(store, "claim", "c1", "shared text") + _put(store, "page", "p1", "shared text") + hits = index_db.search_embedding( + store.kb_dir, query_vec=_vec("shared text"), kinds=("page",) + ) + assert [h[0] for h in hits] == ["page"] + + +def test_search_embedding_honours_min_score(store: KBStore) -> None: + _put(store, "claim", "c1", "the matching text") + _put(store, "claim", "c2", "a different string") + hits = index_db.search_embedding( + store.kb_dir, query_vec=_vec("the matching text"), min_score=0.999 + ) + assert [h[1] for h in hits] == ["c1"] + + +def test_search_embedding_honours_limit(store: KBStore) -> None: + for i in range(5): + _put(store, "claim", f"c{i}", f"text number {i}") + hits = index_db.search_embedding( + store.kb_dir, query_vec=_vec("text number 1"), limit=2 + ) + assert len(hits) == 2 + + +def test_search_embedding_on_an_empty_index(store: KBStore) -> None: + assert index_db.search_embedding(store.kb_dir, query_vec=_vec("nothing")) == [] + + +def test_search_embedding_tolerates_a_zero_query_vector(store: KBStore) -> None: + _put(store, "claim", "c1", "some text") + hits = index_db.search_embedding( + store.kb_dir, query_vec=np.zeros(8, dtype=np.float32) + ) + assert all(h[3] == pytest.approx(0.0) for h in hits) + + +# --- search_embeddings (legacy json table) ------------------------------- + + +def test_legacy_search_embeddings_ranks_by_cosine(store: KBStore) -> None: + # NOTE: `search_embeddings` (plural) reads the legacy `embeddings` table and + # has no callers left in src/ or tests/. Covered here so the number is + # honest, but deleting it would be the better fix. + with index_db.open_db(store.kb_dir) as conn: + index_db.index_embedding( + conn, kind="claim", id="c1", vec=_vec("target text").tolist() + ) + index_db.index_embedding( + conn, kind="claim", id="c2", vec=_vec("other text").tolist() + ) + hits = index_db.search_embeddings( + store.kb_dir, _vec("target text").tolist() + ) + assert hits[0][1] == "c1" + + +def test_legacy_search_embeddings_rejects_an_empty_query(store: KBStore) -> None: + assert index_db.search_embeddings(store.kb_dir, []) == [] + + +def test_legacy_search_embeddings_rejects_a_zero_query(store: KBStore) -> None: + assert index_db.search_embeddings(store.kb_dir, [0.0] * 8) == [] + + +def test_legacy_search_embeddings_skips_mismatched_dims(store: KBStore) -> None: + with index_db.open_db(store.kb_dir) as conn: + index_db.index_embedding(conn, kind="claim", id="c1", vec=[1.0, 0.0]) + assert index_db.search_embeddings(store.kb_dir, _vec("q").tolist()) == [] + + +def test_legacy_search_embeddings_honours_limit(store: KBStore) -> None: + with index_db.open_db(store.kb_dir) as conn: + for i in range(4): + index_db.index_embedding( + conn, kind="claim", id=f"c{i}", vec=_vec(f"t{i}").tolist() + ) + hits = index_db.search_embeddings( + store.kb_dir, _vec("t1").tolist(), limit=2 + ) + assert len(hits) == 2 + + +# --- _snippet_for -------------------------------------------------------- + + +def test_snippet_falls_back_to_the_id_when_no_file_exists(store: KBStore) -> None: + assert index_db._snippet_for(store.kb_dir, "claim", "ghost") == "ghost" + + +def test_snippet_reads_the_yaml_artifact(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="snippet source text", evidence=[src.id])) + snippet = index_db._snippet_for(store.kb_dir, "claims", "c1") + assert snippet == "c1" or "\n" not in snippet + + +# --- semantic search availability ---------------------------------------- + + +def test_semantic_search_available_with_an_embedder( + store: KBStore, embedder: None +) -> None: + assert index_db.semantic_search_available() is True + + +def test_semantic_search_unavailable_without_an_embedder(store: KBStore) -> None: + # the suite-wide registry isolation means no adapter is registered here + assert index_db.semantic_search_available() is False + + +def test_search_semantic_degrades_without_an_embedder(store: KBStore) -> None: + assert index_db.search_semantic(store.kb_dir, "anything") == [] + + +def test_search_semantic_finds_the_match(store: KBStore, embedder: None) -> None: + _put(store, "claim", "c1", "the review gate is load-bearing") + hits = index_db.search_semantic( + store.kb_dir, "the review gate is load-bearing" + ) + assert [h[1] for h in hits] == ["c1"] + + +def test_search_semantic_caches_the_query_vector( + store: KBStore, embedder: None +) -> None: + _put(store, "claim", "c1", "cache me") + first = index_db.search_semantic(store.kb_dir, "cache me") + second = index_db.search_semantic(store.kb_dir, "cache me") + assert first == second + + +# --- storage write-through ------------------------------------------------ + + +def test_put_claim_writes_an_embedding(store: KBStore, embedder: None) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="embedded on write", evidence=[src.id])) + assert index_db.get_embedding(store.kb_dir, kind="claim", id="c1") is not None + + +def test_put_page_writes_an_embedding(store: KBStore, embedder: None) -> None: + store.put_page(Page(id="p1", title="a page", body="prose")) + assert index_db.get_embedding(store.kb_dir, kind="page", id="p1") is not None + + +def test_put_entity_writes_an_embedding(store: KBStore, embedder: None) -> None: + store.put_entity(Entity(id="e1", name="acme-example", type="company")) + assert index_db.get_embedding(store.kb_dir, kind="entity", id="e1") is not None + + +def test_updating_a_claim_replaces_its_embedding( + store: KBStore, embedder: None +) -> None: + # write-through runs on update_claim too; the second write must replace the + # embedding row rather than raise a UNIQUE violation on (kind, id) + src = store.put_source(b"e") + claim = store.put_claim(Claim(id="c1", text="first text", evidence=[src.id])) + before = index_db.get_embedding(store.kb_dir, kind="claim", id="c1") + store.update_claim(claim.model_copy(update={"text": "second text"})) + after = index_db.get_embedding(store.kb_dir, kind="claim", id="c1") + assert before is not None and after is not None + assert not np.allclose(before[0], after[0]) + + +def test_put_claim_refuses_to_overwrite(store: KBStore, embedder: None) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="first text", evidence=[src.id])) + with pytest.raises(ValueError, match="already exists"): + store.put_claim(Claim(id="c1", text="second text", evidence=[src.id])) + + +def test_put_claim_without_an_embedder_skips_embedding(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_claim(Claim(id="c1", text="no embedder here", evidence=[src.id])) + assert index_db.get_embedding(store.kb_dir, kind="claim", id="c1") is None + + +def test_embed_and_store_is_a_noop_for_empty_text( + store: KBStore, embedder: None +) -> None: + store._embed_and_store(kind="claim", id="c-empty", text="") + assert index_db.get_embedding(store.kb_dir, kind="claim", id="c-empty") is None diff --git a/tests/test_jsonl_server_surface.py b/tests/test_jsonl_server_surface.py new file mode 100644 index 00000000..814f78a7 --- /dev/null +++ b/tests/test_jsonl_server_surface.py @@ -0,0 +1,322 @@ +"""The JSONL handlers that `tests/test_jsonl_server.py` doesn't reach. + +`test_capabilities` enforces method-list parity between the MCP, JSONL and CLI +surfaces, but parity of *names* is not parity of *behaviour* — 23 of the 71 +handlers here had never executed. This walks the rest of the map through +`handle_request`, the same entry point the real stdio loop uses, and asserts +each returns a result envelope rather than an error. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import numpy as np +import pytest + +from vouch import bundle +from vouch.embeddings import register +from vouch.embeddings.base import DEFAULT_MODEL_NAME, Embedder +from vouch.jsonl_server import HANDLERS, handle_request +from vouch.models import Claim, Entity, Page +from vouch.storage import KBStore + + +class _HashEmbedder(Embedder): + name = "mock" + version = "1" + dim = 8 + + def encode(self, text: str) -> np.ndarray: + import hashlib + + h = hashlib.sha256(text.encode()).digest() + out = np.array([h[i] / 255.0 for i in range(self.dim)], dtype=np.float32) + norm = float(np.linalg.norm(out)) + if norm > 0: + out /= norm + return out + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + s = KBStore.init(tmp_path / "kb") + monkeypatch.chdir(s.root) + return s + + +@pytest.fixture +def embedder() -> None: + register(DEFAULT_MODEL_NAME, _HashEmbedder) + + +_counter = iter(range(1, 10_000)) + + +def _call(method: str, **params: Any) -> dict: + return handle_request( + {"id": str(next(_counter)), "method": method, "params": params} + ) + + +def _result(method: str, **params: Any) -> Any: + resp = _call(method, **params) + assert "error" not in resp, resp + return resp["result"] + + +def _error(method: str, **params: Any) -> dict: + resp = _call(method, **params) + assert "error" in resp, resp + return resp["error"] + + +def _claim(store: KBStore, claim_id: str, text: str, **kw: Any) -> Claim: + src = store.put_source(b"evidence body") + return store.put_claim(Claim(id=claim_id, text=text, evidence=[src.id], **kw)) + + +# --- sources -------------------------------------------------------------- + + +def test_register_source_returns_an_id(store: KBStore) -> None: + out = _result("kb.register_source", content="some evidence", title="a note") + assert store.get_source(out["id"]).title == "a note" + + +def test_source_verify_lists_every_source(store: KBStore) -> None: + store.put_source(b"body", title="the memo") + out = _result("kb.source_verify") + assert isinstance(out, list) + assert len(out) == 1 + + +# --- proposals ------------------------------------------------------------ + + +def test_propose_page_files_a_proposal(store: KBStore) -> None: + out = _result("kb.propose_page", title="a page", body="prose") + assert out["proposal_id"] + + +def test_propose_entity_files_a_proposal(store: KBStore) -> None: + out = _result("kb.propose_entity", name="acme-example", entity_type="company") + assert out["proposal_id"] + + +def test_propose_relation_files_a_proposal(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + out = _result("kb.propose_relation", src="e1", relation="owned_by", target="e2") + assert out["proposal_id"] + + +def test_reject_records_the_reason(store: KBStore) -> None: + src = store.put_source(b"e") + pr = _result("kb.propose_claim", text="reject me", evidence=[src.id]) + _result("kb.reject", proposal_id=pr["proposal_id"], reason="not useful") + assert store.list_claims() == [] + + +def test_reject_unknown_proposal_is_an_error_envelope(store: KBStore) -> None: + assert _error("kb.reject", proposal_id="ghost", reason="nope") + + +def test_reject_extracted_with_nothing_pending(store: KBStore) -> None: + assert _result("kb.reject_extracted") is not None + + +def test_propose_theme_files_a_theme_page(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + _claim(store, "c1", "a themed claim", entities=["e1"]) + resp = _call( + "kb.propose_theme", entities=["e1"], claim_ids=["c1"], session_ids=["s1"] + ) + # a theme page needs a real cluster; either it files or it reports why + assert "result" in resp or "error" in resp + + +# --- lifecycle mirrors ---------------------------------------------------- + + +def test_supersede_links_old_to_new(store: KBStore) -> None: + _claim(store, "old", "the first version") + _claim(store, "new", "the corrected version") + _result("kb.supersede", old_claim_id="old", new_claim_id="new") + assert store.get_claim("old").superseded_by == "new" + + +def test_contradict_records_the_pair(store: KBStore) -> None: + _claim(store, "a", "the gate is on") + _claim(store, "b", "the gate is off") + _result("kb.contradict", claim_a="a", claim_b="b") + assert "b" in store.get_claim("a").contradicts + + +def test_archive_marks_the_claim(store: KBStore) -> None: + _claim(store, "c1", "a claim to retire") + assert _result("kb.archive", claim_id="c1") is not None + + +def test_confirm_bumps_last_confirmed(store: KBStore) -> None: + _claim(store, "c1", "a claim to re-confirm") + _result("kb.confirm", claim_id="c1") + assert store.get_claim("c1").last_confirmed_at is not None + + +@pytest.mark.parametrize( + ("method", "params"), + [ + ("kb.supersede", {"old_claim_id": "ghost", "new_claim_id": "ghost2"}), + ("kb.contradict", {"claim_a": "ghost", "claim_b": "ghost2"}), + ("kb.archive", {"claim_id": "ghost"}), + ("kb.confirm", {"claim_id": "ghost"}), + ], +) +def test_lifecycle_mirrors_error_on_unknown_claims( + store: KBStore, method: str, params: dict[str, Any] +) -> None: + assert _error(method, **params) + + +def test_clear_claims_dry_run_keeps_the_claim(store: KBStore) -> None: + _claim(store, "c1", "an auto claim", auto_approved=True) + out = _result("kb.clear_claims", dry_run=True) + assert out["count"] == 1 + assert store.get_claim("c1") + + +def test_clear_claims_applied_reports_the_ids(store: KBStore) -> None: + _claim(store, "c1", "an auto claim", auto_approved=True) + out = _result("kb.clear_claims") + assert out["claim_ids"] == ["c1"] + + +def test_clear_claims_rejects_a_bad_before_date(store: KBStore) -> None: + assert _error("kb.clear_claims", before="not-a-date") + + +def test_cite_resolves_citations(store: KBStore) -> None: + src = store.put_source(b"body", title="the memo") + store.put_claim(Claim(id="c1", text="cited", evidence=[src.id])) + out = _result("kb.cite", claim_id="c1") + assert isinstance(out, list) + assert out + + +def test_cite_unknown_claim_is_an_error_envelope(store: KBStore) -> None: + assert _error("kb.cite", claim_id="ghost") + + +# --- index / provenance --------------------------------------------------- + + +def test_index_rebuild_reports_stats(store: KBStore) -> None: + _claim(store, "c1", "indexed claim") + assert _result("kb.index_rebuild") is not None + + +def test_provenance_rebuild_reports_edges(store: KBStore) -> None: + _claim(store, "c1", "a claim with evidence") + out = _result("kb.provenance_rebuild") + assert isinstance(out["edges"], int) + + +# --- bundles -------------------------------------------------------------- + + +def test_export_check_passes_on_a_fresh_bundle( + store: KBStore, tmp_path: Path +) -> None: + _claim(store, "c1", "a claim to export") + dest = tmp_path / "kb.tar.gz" + bundle.export(store.kb_dir, dest=dest, actor="test") + assert _result("kb.export_check", bundle_path=str(dest))["ok"] is True + + +def test_export_check_flags_a_corrupt_bundle( + store: KBStore, tmp_path: Path +) -> None: + junk = tmp_path / "not-a-bundle.tar.gz" + junk.write_bytes(b"definitely not a tarball") + resp = _call("kb.export_check", bundle_path=str(junk)) + assert "error" in resp or resp["result"]["ok"] is False + + +# --- themes --------------------------------------------------------------- + + +def test_detect_themes_on_an_empty_kb(store: KBStore) -> None: + assert _result("kb.detect_themes")["clusters"] == [] + + +# --- embeddings ----------------------------------------------------------- + + +def test_embeddings_stats_reports_cache_counters( + store: KBStore, embedder: None +) -> None: + _claim(store, "c1", "a claim to embed") + out = _result("kb.embeddings_stats") + assert isinstance(out, dict) + assert out + + +def test_reindex_embeddings_backfills(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "a claim to embed") + assert _result("kb.reindex_embeddings", backfill=True) is not None + + +def test_dedup_scan_finds_the_pair(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "identical duplicated text") + _claim(store, "c2", "identical duplicated text") + assert _result("kb.dedup_scan") is not None + + +def test_eval_embeddings_on_an_empty_query_set( + store: KBStore, embedder: None, tmp_path: Path +) -> None: + queries = tmp_path / "q.jsonl" + queries.write_text("", encoding="utf-8") + resp = _call("kb.eval_embeddings", queries_path=str(queries)) + assert "result" in resp or "error" in resp + + +# --- dispatch contract ---------------------------------------------------- + + +def test_unknown_method_is_an_error_envelope(store: KBStore) -> None: + assert _error("kb.definitely_not_a_method") + + +def test_every_registered_handler_is_reachable(store: KBStore) -> None: + # guards the parity invariant from the other direction: a name in HANDLERS + # that dispatch cannot route would be a silently dead surface + for method in HANDLERS: + resp = _call(method) + assert "result" in resp or "error" in resp, (method, resp) + if "error" in resp: + assert "not found" not in str(resp["error"]).lower(), method + + +def test_list_pages_supports_type_and_meta_filters(store: KBStore) -> None: + store.put_page(Page(id="p1", title="a page")) + assert "items" in _result("kb.list_pages", type="concept") + + +def test_list_claims_supports_a_status_filter(store: KBStore) -> None: + _claim(store, "c1", "a claim") + assert "items" in _result("kb.list_claims", status="working") + + +def test_list_entities_supports_a_type_filter(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + assert "items" in _result("kb.list_entities", entity_type="person") + + +def test_list_relations_supports_a_node_filter(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + _result("kb.propose_relation", src="e1", relation="owned_by", target="e2") + assert "items" in _result("kb.list_relations", node_id="e1") diff --git a/tests/test_migrations_rewriter.py b/tests/test_migrations_rewriter.py new file mode 100644 index 00000000..e2881393 --- /dev/null +++ b/tests/test_migrations_rewriter.py @@ -0,0 +1,251 @@ +"""Atomic writes and the manifest transform verbs. + +`atomic_write_text` is the single mutation path every schema migration goes +through, so its temp-file cleanup on failure is the difference between a +crashed migration and a `.vouch/` littered with `.mig-*.tmp` files. The +`split`/`merge` verbs and the markdown-frontmatter branch of `transform_text` +had never executed. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from vouch.migrations.rewriter import ( + ARTIFACT_KINDS, + apply_transforms, + artifact_files, + atomic_write_text, + transform_text, +) + +# --- atomic_write_text --------------------------------------------------- + + +def test_atomic_write_creates_parent_directories(tmp_path: Path) -> None: + target = tmp_path / "deep" / "nested" / "claim.yaml" + atomic_write_text(target, "id: c1\n") + assert target.read_text(encoding="utf-8") == "id: c1\n" + + +def test_atomic_write_replaces_existing_content(tmp_path: Path) -> None: + target = tmp_path / "claim.yaml" + target.write_text("id: old\n", encoding="utf-8") + atomic_write_text(target, "id: new\n") + assert target.read_text(encoding="utf-8") == "id: new\n" + + +def test_atomic_write_leaves_no_temp_files_behind(tmp_path: Path) -> None: + target = tmp_path / "claim.yaml" + atomic_write_text(target, "id: c1\n") + assert [p.name for p in tmp_path.iterdir()] == ["claim.yaml"] + + +def test_atomic_write_cleans_up_the_temp_file_on_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "claim.yaml" + + def _boom(_src: str, _dst: str) -> None: + raise OSError("rename failed") + + monkeypatch.setattr(os, "replace", _boom) + with pytest.raises(OSError, match="rename failed"): + atomic_write_text(target, "id: c1\n") + # the whole point: a failed migration must not strand .mig-*.tmp files + assert list(tmp_path.iterdir()) == [] + + +def test_atomic_write_cleans_up_on_keyboard_interrupt( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "claim.yaml" + + def _interrupt(_src: str, _dst: str) -> None: + raise KeyboardInterrupt + + monkeypatch.setattr(os, "replace", _interrupt) + with pytest.raises(KeyboardInterrupt): + atomic_write_text(target, "id: c1\n") + assert list(tmp_path.iterdir()) == [] + + +# --- transform verbs ----------------------------------------------------- + + +def test_rename_moves_the_value() -> None: + out = apply_transforms({"old": 1}, [{"rename": {"from": "old", "to": "new"}}]) + assert out == {"new": 1} + + +def test_rename_is_a_noop_when_the_field_is_absent() -> None: + assert apply_transforms({"a": 1}, [{"rename": {"from": "x", "to": "y"}}]) == {"a": 1} + + +def test_default_fills_only_a_missing_field() -> None: + assert apply_transforms({}, [{"default": {"field": "f", "value": 7}}]) == {"f": 7} + assert apply_transforms( + {"f": 1}, [{"default": {"field": "f", "value": 7}}] + ) == {"f": 1} + + +def test_drop_removes_the_field_and_tolerates_absence() -> None: + assert apply_transforms({"a": 1, "b": 2}, [{"drop": {"field": "b"}}]) == {"a": 1} + assert apply_transforms({"a": 1}, [{"drop": {"field": "zz"}}]) == {"a": 1} + + +def test_split_fans_a_field_into_parts() -> None: + out = apply_transforms( + {"name": "alice example"}, + [{"split": {"field": "name", "into": ["first", "last"]}}], + ) + assert out == {"first": "alice", "last": "example"} + + +def test_split_honours_a_custom_separator() -> None: + out = apply_transforms( + {"path": "a/b"}, [{"split": {"field": "path", "into": ["x", "y"], "on": "/"}}] + ) + assert out == {"x": "a", "y": "b"} + + +def test_split_pads_missing_parts_with_empty_strings() -> None: + out = apply_transforms( + {"name": "alice"}, [{"split": {"field": "name", "into": ["first", "last"]}}] + ) + assert out == {"first": "alice", "last": ""} + + +def test_split_keeps_the_source_field_when_it_is_a_target() -> None: + out = apply_transforms( + {"name": "alice example"}, + [{"split": {"field": "name", "into": ["name", "last"]}}], + ) + assert out == {"name": "alice", "last": "example"} + + +def test_split_is_a_noop_when_the_field_is_absent() -> None: + out = apply_transforms( + {"a": 1}, [{"split": {"field": "missing", "into": ["x", "y"]}}] + ) + assert out == {"a": 1} + + +def test_merge_joins_fields_and_drops_the_sources() -> None: + out = apply_transforms( + {"first": "alice", "last": "example"}, + [{"merge": {"fields": ["first", "last"], "into": "name"}}], + ) + assert out == {"name": "alice example"} + + +def test_merge_honours_a_custom_joiner() -> None: + out = apply_transforms( + {"a": "x", "b": "y"}, + [{"merge": {"fields": ["a", "b"], "into": "c", "with": "-"}}], + ) + assert out == {"c": "x-y"} + + +def test_merge_treats_missing_sources_as_empty() -> None: + out = apply_transforms( + {"first": "alice"}, + [{"merge": {"fields": ["first", "last"], "into": "name"}}], + ) + assert out == {"name": "alice "} + + +def test_merge_keeps_a_source_that_is_also_the_target() -> None: + out = apply_transforms( + {"name": "alice", "last": "example"}, + [{"merge": {"fields": ["name", "last"], "into": "name"}}], + ) + assert out == {"name": "alice example"} + + +def test_transforms_apply_in_order() -> None: + out = apply_transforms( + {"old": "alice example"}, + [ + {"rename": {"from": "old", "to": "name"}}, + {"split": {"field": "name", "into": ["first", "last"]}}, + ], + ) + assert out == {"first": "alice", "last": "example"} + + +def test_apply_transforms_does_not_mutate_the_input() -> None: + original = {"old": 1} + apply_transforms(original, [{"rename": {"from": "old", "to": "new"}}]) + assert original == {"old": 1} + + +# --- artifact_files ------------------------------------------------------ + + +def test_artifact_files_lists_yaml_kinds_sorted(tmp_path: Path) -> None: + (tmp_path / "claims").mkdir() + for name in ("b.yaml", "a.yaml"): + (tmp_path / "claims" / name).write_text("id: x\n", encoding="utf-8") + (tmp_path / "claims" / "ignore.md").write_text("nope", encoding="utf-8") + assert [p.name for p in artifact_files(tmp_path, "claims")] == ["a.yaml", "b.yaml"] + + +def test_artifact_files_lists_markdown_for_pages(tmp_path: Path) -> None: + (tmp_path / "pages").mkdir() + (tmp_path / "pages" / "p1.md").write_text("---\nid: p1\n---\nbody", encoding="utf-8") + (tmp_path / "pages" / "ignore.yaml").write_text("id: x\n", encoding="utf-8") + assert [p.name for p in artifact_files(tmp_path, "pages")] == ["p1.md"] + + +def test_artifact_files_on_a_missing_subdir(tmp_path: Path) -> None: + assert artifact_files(tmp_path, "claims") == [] + + +def test_artifact_kinds_covers_every_durable_dir() -> None: + assert set(ARTIFACT_KINDS) == { + "claims", "entities", "relations", "evidence", "sessions", "pages", + } + + +# --- transform_text ------------------------------------------------------ + + +def test_transform_text_rewrites_yaml_artifacts() -> None: + out = transform_text( + "old: 1\n", "claims", [{"rename": {"from": "old", "to": "new"}}] + ) + assert "new: 1" in out + assert "old" not in out + + +def test_transform_text_leaves_non_mapping_yaml_untouched() -> None: + text = "- just\n- a\n- list\n" + assert transform_text(text, "claims", [{"drop": {"field": "x"}}]) == text + + +def test_transform_text_rewrites_page_frontmatter_only() -> None: + text = "---\nold: 1\n---\nthe body stays [claim: c1]\n" + out = transform_text(text, "pages", [{"rename": {"from": "old", "to": "new"}}]) + assert "new: 1" in out + assert "the body stays [claim: c1]" in out + + +def test_transform_text_leaves_a_page_without_frontmatter_untouched() -> None: + text = "no frontmatter here\n" + assert transform_text(text, "pages", [{"drop": {"field": "x"}}]) == text + + +def test_transform_text_handles_empty_page_frontmatter() -> None: + text = "---\n\n---\nbody\n" + out = transform_text(text, "pages", [{"default": {"field": "f", "value": 1}}]) + assert "f: 1" in out + assert "body" in out + + +def test_transform_text_leaves_non_mapping_frontmatter_untouched() -> None: + text = "---\n- a\n- b\n---\nbody\n" + assert transform_text(text, "pages", [{"drop": {"field": "x"}}]) == text diff --git a/tests/test_pr_bot.py b/tests/test_pr_bot.py index 7941c310..3cfbe958 100644 --- a/tests/test_pr_bot.py +++ b/tests/test_pr_bot.py @@ -1,6 +1,5 @@ import subprocess import sys -from datetime import UTC, datetime from pathlib import Path from vouch import pr_bot @@ -28,12 +27,6 @@ def test_core_paths_all_flagged(): assert pr_bot.classify([f])["is_core"] is True, f -def test_trust(): - assert pr_bot.is_trusted("OWNER", "plind-junior") is True - assert pr_bot.is_trusted("CONTRIBUTOR", "rando") is False - assert pr_bot.is_trusted("NONE", "dependabot[bot]") is True - - def test_screenshots_two_gh_images(): body = ( "before\n![a](https://user-images.githubusercontent.com/1/a.png)\n" @@ -90,14 +83,6 @@ def test_cli_classify_print_klass(tmp_path): assert out.stdout == "ui" -def test_cli_trust_exit_codes(): - ok = subprocess.run([sys.executable, "-m", "vouch.pr_bot", "trust", - "--author-association", "OWNER", "--actor", "plind-junior"]) - bad = subprocess.run([sys.executable, "-m", "vouch.pr_bot", "trust", - "--author-association", "NONE", "--actor", "rando"]) - assert ok.returncode == 0 and bad.returncode == 1 - - def test_extract_changed_paths_plain_file(): files_json = '[{"filename": "src/vouch/context.py"}]' assert pr_bot.extract_changed_paths(files_json) == ["src/vouch/context.py"] @@ -114,8 +99,8 @@ def test_extract_changed_paths_includes_previous_filename_on_rename(): def test_rename_of_core_path_still_classifies_core(): - # a rename that lands a core path under a new name must not slip past - # trust-gate — the pre-rename path has to stay in the classified list. + # a rename that lands a core path under a new name must not slip past the + # core gate — the pre-rename path has to stay in the classified list. files_json = ( '[{"filename": "src/vouch/web_server.py", "status": "renamed", ' '"previous_filename": "src/vouch/http_server.py"}]' @@ -145,139 +130,3 @@ def test_codeowners_covers_every_core_glob(): needle = "/" + glob.replace("/**", "/") assert needle in text, f"{glob} missing from .github/CODEOWNERS" - -def _review(state, sha, login="coderabbitai[bot]"): - return {"user": {"login": login}, "state": state, "commit_id": sha} - - -def test_coderabbit_pending_when_no_review_on_head(): - # approved an earlier commit, but head has no coderabbit review yet. - reviews = [_review("APPROVED", "old")] - assert pr_bot.coderabbit_verdict(reviews, head_sha="new") == ("pending", 0) - - -def test_coderabbit_approved_on_head(): - reviews = [_review("CHANGES_REQUESTED", "c1"), _review("APPROVED", "c2")] - assert pr_bot.coderabbit_verdict(reviews, head_sha="c2") == ("approved", 1) - - -def test_coderabbit_changes_on_head(): - reviews = [_review("CHANGES_REQUESTED", "c1")] - assert pr_bot.coderabbit_verdict(reviews, head_sha="c1") == ("changes", 1) - - -def test_coderabbit_strikes_count_distinct_commits(): - reviews = [ - _review("CHANGES_REQUESTED", "c1"), - _review("CHANGES_REQUESTED", "c1"), # same commit, still one strike - _review("CHANGES_REQUESTED", "c2"), - _review("CHANGES_REQUESTED", "c3"), - ] - verdict, strikes = pr_bot.coderabbit_verdict(reviews, head_sha="c3") - assert (verdict, strikes) == ("changes", 3) - - -def test_coderabbit_ignores_other_reviewers_and_comments(): - reviews = [ - _review("APPROVED", "c1", login="rando"), # not coderabbit - _review("COMMENTED", "c1"), # no verdict - _review("CHANGES_REQUESTED", "c1"), - ] - assert pr_bot.coderabbit_verdict(reviews, head_sha="c1") == ("changes", 1) - - -def test_gate_status_maps_verdicts(): - assert pr_bot.gate_status("approved") == "success" - assert pr_bot.gate_status("changes") == "failure" - assert pr_bot.gate_status("pending") == "pending" - - -def test_should_close_after_three_strikes(): - assert pr_bot.should_close("changes", 3, author="rando") is True - assert pr_bot.should_close("changes", 2, author="rando") is False - - -def test_should_close_never_when_approved(): - assert pr_bot.should_close("approved", 5, author="rando") is False - - -def test_should_close_exempts_owner_and_bots(): - assert pr_bot.should_close("changes", 9, author="plind-junior") is False - assert pr_bot.should_close("changes", 9, author="dependabot[bot]") is False - - -def _iso(epoch): - return datetime.fromtimestamp(epoch, UTC).strftime("%Y-%m-%dT%H:%M:%SZ") - - -def _review_ts(state, sha, epoch, login="coderabbitai[bot]"): - return {"user": {"login": login}, "state": state, "commit_id": sha, - "submitted_at": _iso(epoch)} - - -_NOW = 1_700_000_000 - - -def test_stale_closes_after_two_days(): - reviews = [_review_ts("CHANGES_REQUESTED", "head", _NOW - 3 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="rando") is True - - -def test_stale_not_within_two_days(): - reviews = [_review_ts("CHANGES_REQUESTED", "head", _NOW - 1 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="rando") is False - - -def test_stale_ignored_when_author_pushed_a_new_commit(): - # the change request is on an old commit; head moved on and has no review. - reviews = [_review_ts("CHANGES_REQUESTED", "old", _NOW - 5 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="new", now_epoch=_NOW, author="rando") is False - - -def test_stale_never_when_approved_on_head(): - reviews = [_review_ts("APPROVED", "head", _NOW - 5 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="rando") is False - - -def test_stale_exempts_owner_and_bots(): - reviews = [_review_ts("CHANGES_REQUESTED", "head", _NOW - 9 * 86400)] - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="plind-junior") is False - assert pr_bot.should_close_stale( - reviews, head_sha="head", now_epoch=_NOW, author="dependabot[bot]") is False - - -def test_cli_stale_check_exit_codes(tmp_path): - import json as _json - f = tmp_path / "reviews.json" - f.write_text(_json.dumps( - [_review_ts("CHANGES_REQUESTED", "head", _NOW - 3 * 86400)]), encoding="utf-8") - stale = subprocess.run( - [sys.executable, "-m", "vouch.pr_bot", "stale-check", "--reviews-file", str(f), - "--head-sha", "head", "--author", "rando", "--now-epoch", str(_NOW)]) - fresh = subprocess.run( - [sys.executable, "-m", "vouch.pr_bot", "stale-check", "--reviews-file", str(f), - "--head-sha", "head", "--author", "rando", "--now-epoch", str(_NOW - 3 * 86400)]) - assert stale.returncode == 0 and fresh.returncode == 1 - - -def test_cli_coderabbit_gate_outputs(tmp_path): - import json as _json - f = tmp_path / "reviews.json" - f.write_text(_json.dumps([ - _review("CHANGES_REQUESTED", "c1"), - _review("CHANGES_REQUESTED", "c2"), - _review("CHANGES_REQUESTED", "head"), - ]), encoding="utf-8") - out = subprocess.run( - [sys.executable, "-m", "vouch.pr_bot", "coderabbit-gate", - "--reviews-file", str(f), "--head-sha", "head", "--author", "rando"], - capture_output=True, text=True, check=True) - assert "state=failure" in out.stdout - assert "verdict=changes" in out.stdout - assert "strikes=3" in out.stdout - assert "close=true" in out.stdout diff --git a/tests/test_pr_bot_diff_coverage.py b/tests/test_pr_bot_diff_coverage.py new file mode 100644 index 00000000..35793724 --- /dev/null +++ b/tests/test_pr_bot_diff_coverage.py @@ -0,0 +1,297 @@ +"""The diff-coverage PR comment the bot posts when the gate fails. + +The comment is built from diff-cover's json report. File paths in that report +come from the PR's own diff, so an attacker controls them — the renderer must +therefore stay pure string work with no shell or markup escape hatch, and the +workflow posts the result with `--body-file` rather than interpolating it. + +The marker is load-bearing: the workflow upserts on it, so one PR gets one +comment instead of a pile. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +from vouch.pr_bot import ( + DIFF_COVERAGE_MARKER, + diff_coverage_comment, + format_ranges, + line_ranges, + main, +) + +# --- line_ranges --------------------------------------------------------- + + +def test_line_ranges_collapses_a_run() -> None: + assert line_ranges([1, 2, 3]) == [(1, 3)] + + +def test_line_ranges_splits_on_a_gap() -> None: + assert line_ranges([1, 2, 5, 6, 9]) == [(1, 2), (5, 6), (9, 9)] + + +def test_line_ranges_sorts_and_dedupes() -> None: + assert line_ranges([5, 1, 2, 2, 1]) == [(1, 2), (5, 5)] + + +def test_line_ranges_on_empty_input() -> None: + assert line_ranges([]) == [] + + +def test_line_ranges_accepts_a_single_line() -> None: + assert line_ranges([42]) == [(42, 42)] + + +def test_line_ranges_coerces_stringy_numbers() -> None: + assert line_ranges(["3", "4"]) == [(3, 4)] # type: ignore[list-item] + + +# --- format_ranges ------------------------------------------------------- + + +def test_format_ranges_renders_singletons_and_spans() -> None: + assert format_ranges([(1, 1), (4, 7)], limit=10) == "1, 4-7" + + +def test_format_ranges_truncates_past_the_limit() -> None: + ranges = [(n, n) for n in range(1, 6)] + assert format_ranges(ranges, limit=2) == "1, 2, +3 more" + + +def test_format_ranges_on_empty_input() -> None: + assert format_ranges([], limit=5) == "" + + +# --- diff_coverage_comment: passing ------------------------------------ + + +def test_comment_reports_full_coverage() -> None: + body = diff_coverage_comment( + {"total_num_violations": 0, "total_num_lines": 12, "total_percent_covered": 100} + ) + assert body.startswith(DIFF_COVERAGE_MARKER) + assert "diff coverage: 100%" in body + + +def test_comment_reports_nothing_to_measure() -> None: + # docs-only PRs pass the gate; the comment must not claim a coverage win + body = diff_coverage_comment({"total_num_violations": 0, "total_num_lines": 0}) + assert "diff coverage: n/a" in body + assert "no python" in body + + +def test_comment_on_an_empty_report_does_not_crash() -> None: + body = diff_coverage_comment({}) + assert body.startswith(DIFF_COVERAGE_MARKER) + + +# --- diff_coverage_comment: failing ----------------------------------- + + +def _failing() -> dict[str, Any]: + return { + "total_num_violations": 6, + "total_num_lines": 7, + "total_percent_covered": 14, + "src_stats": { + "src/vouch/mod.py": { + "percent_covered": 33.3, + "violation_lines": [6, 7], + }, + "src/vouch/other.py": { + "percent_covered": 0.0, + "violation_lines": [1, 2, 3, 4], + }, + }, + } + + +def test_comment_names_the_uncovered_lines() -> None: + body = diff_coverage_comment(_failing()) + assert body.startswith(DIFF_COVERAGE_MARKER) + assert "diff coverage: 14%" in body + assert "6 of 7 changed" in body + assert "`src/vouch/mod.py` — line(s) 6-7" in body + assert "`src/vouch/other.py` — line(s) 1-4" in body + + +def test_comment_includes_a_local_reproduction() -> None: + body = diff_coverage_comment(_failing()) + assert "diff-cover coverage.xml" in body + assert "--fail-under 100" in body + + +def test_comment_lists_files_in_a_stable_order() -> None: + body = diff_coverage_comment(_failing()) + assert body.index("src/vouch/mod.py") < body.index("src/vouch/other.py") + + +def test_comment_skips_a_file_with_no_violation_lines() -> None: + report = { + "total_num_violations": 2, + "total_num_lines": 4, + "total_percent_covered": 50, + "src_stats": { + "src/vouch/a.py": {"violation_lines": [3, 4]}, + "src/vouch/b.py": {"violation_lines": []}, + }, + } + body = diff_coverage_comment(report) + assert "src/vouch/a.py" in body + assert "src/vouch/b.py" not in body + + +def test_comment_truncates_a_very_wide_pr() -> None: + stats = { + f"src/vouch/f{n:02d}.py": {"violation_lines": [1]} for n in range(30) + } + report = { + "total_num_violations": 30, + "total_num_lines": 60, + "total_percent_covered": 50, + "src_stats": stats, + } + body = diff_coverage_comment(report) + assert "and 10 more file(s)" in body + + +def test_comment_truncates_a_file_with_many_scattered_lines() -> None: + scattered = list(range(1, 60, 2)) # 30 non-adjacent lines -> 30 ranges + report = { + "total_num_violations": len(scattered), + "total_num_lines": 100, + "total_percent_covered": 70, + "src_stats": {"src/vouch/wide.py": {"violation_lines": scattered}}, + } + body = diff_coverage_comment(report) + assert "more" in body + + +def test_comment_handles_a_missing_percentage() -> None: + report = { + "total_num_violations": 1, + "total_num_lines": 2, + "src_stats": {"src/vouch/a.py": {"violation_lines": [2]}}, + } + assert "diff coverage: unknown" in diff_coverage_comment(report) + + +def test_comment_does_not_execute_or_expand_attacker_paths() -> None: + # a contributor names the file; the renderer must emit it verbatim inside a + # code span and never build a shell word from it + nasty = "src/vouch/$(id).py" + report = { + "total_num_violations": 1, + "total_num_lines": 1, + "total_percent_covered": 0, + "src_stats": {nasty: {"violation_lines": [1]}}, + } + body = diff_coverage_comment(report) + assert f"`{nasty}`" in body + assert "uid=" not in body + + +# --- the CLI surface the workflow calls -------------------------------- + + +def test_cli_emits_the_comment( + tmp_path: Path, capsys: Any +) -> None: + report = tmp_path / "dc.json" + report.write_text(json.dumps(_failing()), encoding="utf-8") + assert main(["diff-coverage-comment", "--report-file", str(report)]) == 0 + out = capsys.readouterr().out + assert out.startswith(DIFF_COVERAGE_MARKER) + assert "src/vouch/mod.py" in out + + +def test_cli_tolerates_a_non_object_report( + tmp_path: Path, capsys: Any +) -> None: + report = tmp_path / "dc.json" + report.write_text("[]", encoding="utf-8") + assert main(["diff-coverage-comment", "--report-file", str(report)]) == 0 + assert DIFF_COVERAGE_MARKER in capsys.readouterr().out + + +def test_cli_emits_the_passing_comment( + tmp_path: Path, capsys: Any +) -> None: + report = tmp_path / "dc.json" + report.write_text( + json.dumps({"total_num_violations": 0, "total_num_lines": 5}), + encoding="utf-8", + ) + assert main(["diff-coverage-comment", "--report-file", str(report)]) == 0 + assert "diff coverage: 100%" in capsys.readouterr().out + + +# --- the rest of the pr_bot CLI the workflows shell out to --------------- +# +# exit-code contract, not text: every one of these is consumed as `if +# python -m vouch.pr_bot ... ; then` in a workflow, so an inverted code would +# silently flip a gate open. + + +_seq = iter(range(1, 10_000)) + + +def _files(tmp_path: Path, *paths: str) -> str: + # a fresh name per call: reusing one path silently clobbers an earlier + # list, which made a core-touching case look non-core + f = tmp_path / f"files-{next(_seq)}.txt" + f.write_text("\n".join(paths) + "\n", encoding="utf-8") + return str(f) + + +def test_cli_core_touched_exit_codes(tmp_path: Path) -> None: + assert main(["core-touched", "--files-file", + _files(tmp_path, "src/vouch/proposals.py")]) == 0 + assert main(["core-touched", "--files-file", + _files(tmp_path, "README.md")]) == 1 + + +def test_cli_ui_touched_exit_codes(tmp_path: Path) -> None: + assert main(["ui-touched", "--files-file", + _files(tmp_path, "webapp/src/App.tsx")]) == 0 + assert main(["ui-touched", "--files-file", + _files(tmp_path, "README.md")]) == 1 + + +def test_cli_has_screenshots_exit_codes(tmp_path: Path) -> None: + body = tmp_path / "body.md" + body.write_text( + "before ![a](https://github.com/x/y/assets/1/aaa)\n" + "after ![b](https://github.com/x/y/assets/1/bbb)\n", + encoding="utf-8", + ) + assert main(["has-screenshots", "--body-file", str(body)]) == 0 + body.write_text("no images here\n", encoding="utf-8") + assert main(["has-screenshots", "--body-file", str(body)]) == 1 + + +def test_cli_should_arm_approves_a_clean_non_core_pr(tmp_path: Path) -> None: + assert main([ + "should-arm", "--files-file", _files(tmp_path, "docs/guide.md"), + "--ci", "passing", "--verdict", "APPROVE", + ]) == 0 + + +def test_cli_should_arm_refuses_core_failing_ci_and_drafts( + tmp_path: Path +) -> None: + core = _files(tmp_path, "src/vouch/proposals.py") + plain = _files(tmp_path, "docs/guide.md") + # core is never armed, whatever the verdict + assert main(["should-arm", "--files-file", core, + "--ci", "passing", "--verdict", "APPROVE"]) == 1 + assert main(["should-arm", "--files-file", plain, + "--ci", "failing", "--verdict", "APPROVE"]) == 1 + assert main(["should-arm", "--files-file", plain, + "--ci", "passing", "--verdict", "REQUEST_CHANGES"]) == 1 + assert main(["should-arm", "--files-file", plain, "--ci", "passing", + "--verdict", "APPROVE", "--draft"]) == 1 diff --git a/tests/test_pr_cache_helpers.py b/tests/test_pr_cache_helpers.py new file mode 100644 index 00000000..4fd03353 --- /dev/null +++ b/tests/test_pr_cache_helpers.py @@ -0,0 +1,444 @@ +"""The `gh` shell-out, LLM analysis, and parsing helpers in `pr_cache`. + +Every one of these is an integration edge — a subprocess, a network call, or a +model's free-text output — and every one is documented as best-effort: a +failure must degrade (`None`, `[]`, `""`) rather than abort a `pr-cache build`. +That contract was entirely untested, so a raised exception anywhere in here +would have taken down the whole cache build. + +No test touches the network or a real `gh`; the process and urlopen boundaries +are stubbed. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +import urllib.error +import urllib.request +from pathlib import Path +from typing import Any + +import pytest + +from vouch import pr_cache +from vouch.pr_cache import GHError, RepoRef + +REPO = RepoRef(owner="acme-example", name="widget") + + +class _Res: + def __init__(self, returncode: int = 0, stdout: str = "", stderr: str = "") -> None: + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + + +@pytest.fixture +def gh_on_path(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}") + + +# --- default_cache_dir --------------------------------------------------- + + +def test_cache_dir_honours_the_explicit_override( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.setenv("VOUCH_PR_CACHE_DIR", str(tmp_path / "mine")) + assert pr_cache.default_cache_dir() == tmp_path / "mine" + + +def test_cache_dir_uses_xdg_cache_home( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.delenv("VOUCH_PR_CACHE_DIR", raising=False) + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "xdg")) + assert pr_cache.default_cache_dir() == tmp_path / "xdg" / "vouch" / "pr-cache" + + +def test_cache_dir_falls_back_to_dot_cache( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.delenv("VOUCH_PR_CACHE_DIR", raising=False) + monkeypatch.delenv("XDG_CACHE_HOME", raising=False) + monkeypatch.setattr(Path, "home", classmethod(lambda _cls: tmp_path / "home")) + expected = tmp_path / "home" / ".cache" / "vouch" / "pr-cache" + assert pr_cache.default_cache_dir() == expected + + +# --- _run_gh ------------------------------------------------------------- + + +def test_run_gh_requires_the_cli(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(shutil, "which", lambda _name: None) + with pytest.raises(GHError, match="GitHub CLI"): + pr_cache._run_gh(["pr", "list"]) + + +def test_run_gh_returns_stdout( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _Res(0, stdout='{"ok":true}') + ) + assert pr_cache._run_gh(["pr", "list"]) == '{"ok":true}' + + +def test_run_gh_raises_on_timeout( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + def _timeout(*_a: Any, **_k: Any) -> Any: + raise subprocess.TimeoutExpired(cmd="gh", timeout=60) + + monkeypatch.setattr(subprocess, "run", _timeout) + with pytest.raises(GHError, match="timed out"): + pr_cache._run_gh(["pr", "list"]) + + +def test_run_gh_surfaces_stderr_on_failure( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _Res(1, stderr="gh auth required") + ) + with pytest.raises(GHError, match="gh auth required"): + pr_cache._run_gh(["pr", "list"]) + + +def test_run_gh_falls_back_to_stdout_when_stderr_is_empty( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _Res(1, stdout="rate limited") + ) + with pytest.raises(GHError, match="rate limited"): + pr_cache._run_gh(["pr", "list"]) + + +# --- _gh_pr_files -------------------------------------------------------- + + +def test_pr_files_returns_paths(monkeypatch: pytest.MonkeyPatch) -> None: + payload = json.dumps({"files": [{"path": "a.py"}, {"path": "b.py"}]}) + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: payload) + assert pr_cache._gh_pr_files(REPO, 1) == ["a.py", "b.py"] + + +def test_pr_files_skips_malformed_entries(monkeypatch: pytest.MonkeyPatch) -> None: + payload = json.dumps({"files": [{"path": "a.py"}, {"no": "path"}, "junk"]}) + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: payload) + assert pr_cache._gh_pr_files(REPO, 1) == ["a.py"] + + +def test_pr_files_degrades_on_gh_error(monkeypatch: pytest.MonkeyPatch) -> None: + def _boom(*_a: Any, **_k: Any) -> str: + raise GHError("no auth") + + monkeypatch.setattr(pr_cache, "_run_gh", _boom) + assert pr_cache._gh_pr_files(REPO, 1) == [] + + +def test_pr_files_handles_empty_output(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: "") + assert pr_cache._gh_pr_files(REPO, 1) == [] + + +# --- _gh_pr_review_comments --------------------------------------------- + + +def test_review_comments_concatenates_comments_and_reviews( + monkeypatch: pytest.MonkeyPatch, +) -> None: + payload = json.dumps({ + "comments": [{"body": "please rebase", "author": {"login": "maintainer"}}], + "reviews": [ + {"body": "wrong approach", "author": {"login": "reviewer"}, + "state": "CHANGES_REQUESTED"}, + ], + }) + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: payload) + out = pr_cache._gh_pr_review_comments(REPO, 1) + assert "[comment by maintainer]" in out + assert "please rebase" in out + assert "[review by reviewer (CHANGES_REQUESTED)]" in out + assert "wrong approach" in out + + +def test_review_comments_skips_empty_bodies(monkeypatch: pytest.MonkeyPatch) -> None: + payload = json.dumps({ + "comments": [{"body": " ", "author": {"login": "x"}}, {"body": None}], + "reviews": [{"body": "", "author": {"login": "y"}, "state": "APPROVED"}], + }) + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: payload) + assert pr_cache._gh_pr_review_comments(REPO, 1) == "" + + +def test_review_comments_defaults_a_missing_author( + monkeypatch: pytest.MonkeyPatch, +) -> None: + payload = json.dumps({"comments": [{"body": "anon note"}], "reviews": []}) + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: payload) + assert "[comment by ?]" in pr_cache._gh_pr_review_comments(REPO, 1) + + +def test_review_comments_tolerates_null_entries( + monkeypatch: pytest.MonkeyPatch, +) -> None: + payload = json.dumps({"comments": [None], "reviews": [None]}) + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: payload) + assert pr_cache._gh_pr_review_comments(REPO, 1) == "" + + +def test_review_comments_degrades_on_gh_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + def _boom(*_a: Any, **_k: Any) -> str: + raise GHError("rate limited") + + monkeypatch.setattr(pr_cache, "_run_gh", _boom) + assert pr_cache._gh_pr_review_comments(REPO, 1) == "" + + +def test_review_comments_handles_empty_output( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(pr_cache, "_run_gh", lambda *_a, **_k: "") + assert pr_cache._gh_pr_review_comments(REPO, 1) == "" + + +# --- _parse_analysis_json ----------------------------------------------- + + +def test_parse_analysis_json_extracts_a_wrapped_blob() -> None: + raw = 'Sure! Here you go:\n{"reason": "stale"}\nHope that helps.' + assert pr_cache._parse_analysis_json(raw) == {"reason": "stale"} + + +def test_parse_analysis_json_on_plain_json() -> None: + assert pr_cache._parse_analysis_json('{"a": 1}') == {"a": 1} + + +@pytest.mark.parametrize("raw", ["", " ", None]) +def test_parse_analysis_json_rejects_blank(raw: str | None) -> None: + assert pr_cache._parse_analysis_json(raw) is None # type: ignore[arg-type] + + +def test_parse_analysis_json_rejects_text_without_braces() -> None: + assert pr_cache._parse_analysis_json("no json here") is None + + +def test_parse_analysis_json_rejects_reversed_braces() -> None: + assert pr_cache._parse_analysis_json("} not really {") is None + + +def test_parse_analysis_json_rejects_invalid_json() -> None: + assert pr_cache._parse_analysis_json("{not: valid, json}") is None + + +# --- _analyze_via_claude_cli -------------------------------------------- + + +def test_claude_cli_absent_returns_none(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(shutil, "which", lambda _name: None) + assert pr_cache._analyze_via_claude_cli("prompt", 5.0) is None + + +def test_claude_cli_returns_stdout( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _Res(0, stdout='{"reason":"stale"}') + ) + assert pr_cache._analyze_via_claude_cli("prompt", 5.0) == '{"reason":"stale"}' + + +def test_claude_cli_timeout_returns_none( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + def _timeout(*_a: Any, **_k: Any) -> Any: + raise subprocess.TimeoutExpired(cmd="claude", timeout=5) + + monkeypatch.setattr(subprocess, "run", _timeout) + assert pr_cache._analyze_via_claude_cli("prompt", 5.0) is None + + +def test_claude_cli_nonzero_exit_returns_none( + gh_on_path: None, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _Res(1, stderr="not logged in") + ) + assert pr_cache._analyze_via_claude_cli("prompt", 5.0) is None + + +# --- _analyze_via_anthropic_api ----------------------------------------- + + +class _Resp: + def __init__(self, body: bytes) -> None: + self._body = body + + def read(self) -> bytes: + return self._body + + def __enter__(self) -> _Resp: + return self + + def __exit__(self, *_a: Any) -> None: + return None + + +def test_anthropic_api_without_a_key_returns_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) is None + + +def test_anthropic_api_joins_text_blocks(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "not-a-real-key") + body = json.dumps({ + "content": [ + {"type": "text", "text": '{"reason":'}, + {"type": "thinking", "text": "ignored"}, + {"type": "text", "text": '"stale"}'}, + ] + }).encode() + monkeypatch.setattr(urllib.request, "urlopen", lambda *_a, **_k: _Resp(body)) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) == '{"reason":"stale"}' + + +def test_anthropic_api_honours_base_url_and_model( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "not-a-real-key") + monkeypatch.setenv("ANTHROPIC_BASE_URL", "https://proxy.invalid/") + monkeypatch.setenv("ANTHROPIC_MODEL", "claude-sonnet-5") + seen: dict[str, Any] = {} + + def _urlopen(req: Any, timeout: float | None = None) -> _Resp: + seen["url"] = req.full_url + seen["payload"] = json.loads(req.data) + return _Resp(json.dumps({"content": [{"type": "text", "text": "ok"}]}).encode()) + + monkeypatch.setattr(urllib.request, "urlopen", _urlopen) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) == "ok" + assert seen["url"] == "https://proxy.invalid/v1/messages" + assert seen["payload"]["model"] == "claude-sonnet-5" + + +def test_anthropic_api_network_error_returns_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "not-a-real-key") + + def _boom(*_a: Any, **_k: Any) -> Any: + raise urllib.error.URLError("dns failure") + + monkeypatch.setattr(urllib.request, "urlopen", _boom) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) is None + + +def test_anthropic_api_timeout_returns_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "not-a-real-key") + + def _boom(*_a: Any, **_k: Any) -> Any: + raise TimeoutError("too slow") + + monkeypatch.setattr(urllib.request, "urlopen", _boom) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) is None + + +def test_anthropic_api_non_json_returns_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "not-a-real-key") + monkeypatch.setattr( + urllib.request, "urlopen", lambda *_a, **_k: _Resp(b"502") + ) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) is None + + +def test_anthropic_api_empty_text_returns_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("ANTHROPIC_API_KEY", "not-a-real-key") + monkeypatch.setattr( + urllib.request, + "urlopen", + lambda *_a, **_k: _Resp(json.dumps({"content": []}).encode()), + ) + assert pr_cache._analyze_via_anthropic_api("prompt", 5.0) is None + + +# --- _labels ------------------------------------------------------------- + + +def test_labels_reads_gh_label_objects() -> None: + assert pr_cache._labels([{"name": "bug"}, {"name": "wontfix"}]) == [ + "bug", "wontfix", + ] + + +def test_labels_accepts_bare_strings() -> None: + assert pr_cache._labels(["bug", "wontfix"]) == ["bug", "wontfix"] + + +def test_labels_skips_nameless_objects() -> None: + assert pr_cache._labels([{"name": ""}, {"colour": "red"}, {"name": "keep"}]) == [ + "keep", + ] + + +@pytest.mark.parametrize("raw", [None, "bug", 7, {"name": "bug"}]) +def test_labels_rejects_a_non_list(raw: Any) -> None: + assert pr_cache._labels(raw) == [] + + +# --- similarity helpers -------------------------------------------------- + + +def test_jaccard_identical_sets_is_one() -> None: + assert pr_cache._jaccard({"a", "b"}, {"a", "b"}) == pytest.approx(1.0) + + +def test_jaccard_disjoint_sets_is_zero() -> None: + assert pr_cache._jaccard({"a"}, {"b"}) == pytest.approx(0.0) + + +def test_jaccard_partial_overlap() -> None: + assert pr_cache._jaccard({"a", "b"}, {"b", "c"}) == pytest.approx(1 / 3) + + +def test_jaccard_with_an_empty_set() -> None: + assert pr_cache._jaccard(set(), {"a"}) == pytest.approx(0.0) + assert pr_cache._jaccard(set(), set()) == pytest.approx(0.0) + + +def test_containment_is_one_when_a_is_a_subset() -> None: + assert pr_cache._containment({"a"}, {"a", "b"}) == pytest.approx(1.0) + + +def test_containment_partial() -> None: + # overlap coefficient: |A ∩ B| / min(|A|, |B|), so the smaller side is the + # denominator -- a subset always scores 1.0 regardless of the other's size + assert pr_cache._containment({"a", "b", "c"}, {"a", "d"}) == pytest.approx(0.5) + assert pr_cache._containment({"a", "b"}, {"a"}) == pytest.approx(1.0) + + +def test_containment_with_an_empty_set() -> None: + assert pr_cache._containment(set(), {"a"}) == pytest.approx(0.0) + assert pr_cache._containment({"a"}, set()) == pytest.approx(0.0) + + +# --- _now_iso ------------------------------------------------------------ + + +def test_now_iso_is_a_utc_timestamp() -> None: + stamp = pr_cache._now_iso() + assert stamp.endswith("Z") + assert len(stamp) == 20 diff --git a/tests/test_retrieval_backend.py b/tests/test_retrieval_backend.py index 4e1d552b..077ce298 100644 --- a/tests/test_retrieval_backend.py +++ b/tests/test_retrieval_backend.py @@ -568,6 +568,23 @@ def test_pages_first_disabled_keeps_order( assert [item["id"] for item in pack["items"]] == ["c1", "p1"] +def test_pages_first_quoted_false_does_not_enable( + page_claim_store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + """Regression (#558 residual): bool(\"false\") is True, so a quoted + pages_first.enabled: \"false\" used to turn the boost on.""" + _page_and_claim_fts(monkeypatch) + _set_backend(page_claim_store, "hybrid") + page_claim_store.config_path.write_text( + 'retrieval:\n backend: hybrid\n pages_first:\n enabled: "false"\n' + " boost: 5.0\n", + encoding="utf-8", + ) + assert context._configured_pages_first(page_claim_store) == (False, 5.0) + pack = context.build_context_pack(page_claim_store, query="JWT", limit=2) + assert [item["id"] for item in pack["items"]] == ["c1", "p1"] + + def test_pages_first_never_boosts_session_pages( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_retrieval_events.py b/tests/test_retrieval_events.py index d235e474..2c0756f5 100644 --- a/tests/test_retrieval_events.py +++ b/tests/test_retrieval_events.py @@ -44,6 +44,19 @@ def test_config_disable(store: KBStore) -> None: assert not (store.kb_dir / FILENAME).exists() +def test_config_quoted_false_does_not_enable(store: KBStore) -> None: + """Regression (#558 residual): bool(\"false\") is True, so a quoted + enabled: \"false\" used to leave retrieval.events on.""" + store.config_path.write_text( + 'retrieval:\n events:\n enabled: "false"\n', encoding="utf-8" + ) + assert load_events_config(store).enabled is False + assert log_event( + store, query="q", backend="fts5", limit=5, budget_chars=None, items=[], + ) is False + assert not (store.kb_dir / FILENAME).exists() + + def test_log_event_writes_masked_record(store: KBStore) -> None: tok = "ghp_" + "a" * 36 # same synthetic github-token shape test_secrets uses ok = log_event( diff --git a/tests/test_salience.py b/tests/test_salience.py index bfd47c68..f2ba8114 100644 --- a/tests/test_salience.py +++ b/tests/test_salience.py @@ -7,7 +7,7 @@ import pytest from vouch import health, salience -from vouch.models import Claim, Entity, EntityType +from vouch.models import Claim, ClaimStatus, Entity, EntityType from vouch.storage import KBStore @@ -105,3 +105,77 @@ def test_window_bounds_buffer(store: KBStore) -> None: for i in range(20): salience.record_query("sess-1", f"q{i}", window=8) assert len(salience._BUFFERS["sess-1"].queries) == 8 + + +@pytest.mark.parametrize( + "retracted", [ClaimStatus.ARCHIVED, ClaimStatus.SUPERSEDED, ClaimStatus.REDACTED] +) +def test_compute_salience_excludes_retracted_claims( + store: KBStore, retracted: ClaimStatus, +) -> None: + """Each retracted status must leave the sidebar, like every read surface. + + ``c0`` sorts ahead of the live ``c1``, so an unfiltered scan both + over-counts and names the retracted claim as the entity's top hit. + """ + src = store.list_sources()[0] + store.put_claim(Claim( + id="c0", text="auth uses sessions", evidence=[src.id], + entities=["jwt"], status=retracted, + )) + health.rebuild_index(store) + + for _ in range(3): + salience.record_query("sess-1", "jwt") + rec = salience.compute_salience(store, "sess-1")[0] + + assert rec["claim_count"] == 1 + assert rec["top_claim_id"] == "c1" + + +def test_entity_with_only_retracted_claims_reports_none(store: KBStore) -> None: + """A matched entity stays in the sidebar, but with nothing to prefetch. + + Entities carry no status of their own, so the record is still emitted — + it just honestly reports zero live claims instead of pointing the agent + at a retracted one. + """ + store.put_entity(Entity(id="saml", name="SAML", type=EntityType.CONCEPT)) + src = store.list_sources()[0] + store.put_claim(Claim( + id="c9", text="saml is the login path", evidence=[src.id], + entities=["saml"], status=ClaimStatus.SUPERSEDED, + )) + health.rebuild_index(store) + + for _ in range(3): + salience.record_query("sess-1", "saml") + rec = next( + r for r in salience.compute_salience(store, "sess-1") + if r["entity_id"] == "saml" + ) + + assert rec["claim_count"] == 0 + assert rec["top_claim_id"] is None + + +def test_attached_sidebar_never_names_a_retracted_claim( + store: KBStore, monkeypatch, +) -> None: + """End-to-end through the jsonl read path that ships the sidebar.""" + from vouch import jsonl_server + + src = store.list_sources()[0] + store.put_claim(Claim( + id="c0", text="auth uses sessions", evidence=[src.id], + entities=["jwt"], status=ClaimStatus.REDACTED, + )) + health.rebuild_index(store) + monkeypatch.setattr(jsonl_server, "_store", lambda: store) + + for _ in range(3): + jsonl_server._h_context({"task": "jwt", "session_id": "sess-1"}) + result = jsonl_server._h_context({"task": "jwt", "session_id": "sess-1"}) + + salient = result["_meta"]["vouch_salience"] + assert "c0" not in {rec["top_claim_id"] for rec in salient} diff --git a/tests/test_server_tool_surface.py b/tests/test_server_tool_surface.py new file mode 100644 index 00000000..66abd77d --- /dev/null +++ b/tests/test_server_tool_surface.py @@ -0,0 +1,525 @@ +"""The `kb_*` MCP tool surface in `vouch.server`. + +`@mcp.tool()` returns the undecorated function, so each tool is callable +directly — which is how the existing tests reach `kb_propose_delete`. Most of +the surface was import-covered only: the decorator ran at import, the body +never did. These are the functions every MCP host (Claude Code, Cursor, Codex) +actually calls, and the contract they rely on is that a missing artifact comes +back as a `ValueError` the host can render, not an internal traceback. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import numpy as np +import pytest + +from vouch import server +from vouch.embeddings import register +from vouch.embeddings.base import DEFAULT_MODEL_NAME, Embedder +from vouch.models import Claim, Entity, Evidence, Page, Relation +from vouch.proposals import propose_claim +from vouch.storage import KBStore + + +class _HashEmbedder(Embedder): + name = "mock" + version = "1" + dim = 8 + + def encode(self, text: str) -> np.ndarray: + import hashlib + + h = hashlib.sha256(text.encode()).digest() + out = np.array([h[i] / 255.0 for i in range(self.dim)], dtype=np.float32) + norm = float(np.linalg.norm(out)) + if norm > 0: + out /= norm + return out + + +@pytest.fixture +def store(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> KBStore: + s = KBStore.init(tmp_path / "kb") + monkeypatch.chdir(s.root) + return s + + +@pytest.fixture +def embedder() -> None: + register(DEFAULT_MODEL_NAME, _HashEmbedder) + + +def _claim(store: KBStore, claim_id: str, text: str, **kw: Any) -> Claim: + src = store.put_source(b"evidence body") + return store.put_claim(Claim(id=claim_id, text=text, evidence=[src.id], **kw)) + + +# --- capabilities / status ------------------------------------------------ + + +def test_kb_capabilities_lists_methods(store: KBStore) -> None: + assert "kb.search" in server.kb_capabilities()["methods"] + + +def test_kb_status_counts_artifacts(store: KBStore) -> None: + _claim(store, "c1", "a durable claim") + assert server.kb_status()["claims"] == 1 + + +def test_load_cfg_returns_a_mapping(store: KBStore) -> None: + assert isinstance(server._load_cfg(store), dict) + + +def test_load_cfg_survives_unparseable_config(store: KBStore) -> None: + (store.kb_dir / "config.yaml").write_text("review: [unclosed\n", encoding="utf-8") + assert server._load_cfg(store) == {} + + +def test_load_cfg_ignores_a_scalar_document(store: KBStore) -> None: + (store.kb_dir / "config.yaml").write_text("just-a-string\n", encoding="utf-8") + assert server._load_cfg(store) == {} + + +def test_current_model_name_is_a_string(store: KBStore) -> None: + assert isinstance(server._current_model_name(), str) + + +# --- kb_read_* ----------------------------------------------------------- + + +def test_kb_read_claim_returns_the_claim(store: KBStore) -> None: + _claim(store, "c1", "the review gate holds") + assert server.kb_read_claim("c1")["text"] == "the review gate holds" + + +def test_kb_read_page_returns_the_page(store: KBStore) -> None: + store.put_page(Page(id="p1", title="review gate", body="prose")) + assert server.kb_read_page("p1")["title"] == "review gate" + + +def test_kb_read_entity_returns_the_entity(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="acme-example", type="company")) + assert server.kb_read_entity("e1")["name"] == "acme-example" + + +def test_kb_read_relation_returns_the_triple(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + store.put_relation(Relation(id="r1", source="e1", relation="owned_by", target="e2")) + assert server.kb_read_relation("r1")["relation"] == "owned_by" + + +def test_kb_read_evidence_returns_the_span(store: KBStore) -> None: + src = store.put_source(b"body") + store.put_evidence(Evidence(id="ev1", source_id=src.id, locator="p2")) + assert server.kb_read_evidence("ev1")["locator"] == "p2" + + +def test_kb_read_source_returns_metadata(store: KBStore) -> None: + src = store.put_source(b"body", title="the memo") + assert server.kb_read_source(src.id)["title"] == "the memo" + + +@pytest.mark.parametrize( + "tool", + [ + "kb_read_claim", + "kb_read_page", + "kb_read_entity", + "kb_read_relation", + "kb_read_evidence", + "kb_read_source", + ], +) +def test_kb_read_missing_artifact_raises_value_error( + store: KBStore, tool: str +) -> None: + # the MCP contract: a host must see ValueError, never ArtifactNotFoundError + with pytest.raises(ValueError): + getattr(server, tool)("does-not-exist") + + +# --- kb_list_* ----------------------------------------------------------- + + +def test_kb_list_claims_and_status_filter(store: KBStore) -> None: + _claim(store, "c1", "a claim") + assert len(server.kb_list_claims()["items"]) == 1 + assert "items" in server.kb_list_claims(status="working") + + +def test_kb_list_pages(store: KBStore) -> None: + store.put_page(Page(id="p1", title="a page")) + assert len(server.kb_list_pages()["items"]) == 1 + + +def test_kb_list_pages_type_filter(store: KBStore) -> None: + store.put_page(Page(id="p1", title="a page")) + assert "items" in server.kb_list_pages(type="concept") + + +def test_kb_list_entities_and_type_filter(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + assert len(server.kb_list_entities()["items"]) == 1 + assert "items" in server.kb_list_entities(entity_type="person") + + +def test_kb_list_relations_and_node_filter(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + store.put_relation(Relation(id="r1", source="e1", relation="owned_by", target="e2")) + assert len(server.kb_list_relations()["items"]) == 1 + assert "items" in server.kb_list_relations(node_id="e1") + + +def test_kb_list_sources(store: KBStore) -> None: + store.put_source(b"body", title="the memo") + assert len(server.kb_list_sources()["items"]) == 1 + + +def test_kb_list_pending_is_empty_on_a_fresh_kb(store: KBStore) -> None: + assert server.kb_list_pending()["items"] == [] + + +def test_kb_list_pending_shows_a_proposal(store: KBStore) -> None: + src = store.put_source(b"e") + propose_claim(store, text="pending", evidence=[src.id], proposed_by="agent") + assert server.kb_list_pending()["items"] + + +def test_kb_triage_pending_is_opt_in(store: KBStore) -> None: + src = store.put_source(b"e") + propose_claim(store, text="pending", evidence=[src.id], proposed_by="agent") + # off by default: the tool refuses rather than silently ranking nothing + with pytest.raises(ValueError, match="triage is disabled"): + server.kb_triage_pending() + + +def test_kb_triage_pending_returns_rows_when_enabled(store: KBStore) -> None: + src = store.put_source(b"e") + propose_claim(store, text="pending", evidence=[src.id], proposed_by="agent") + (store.kb_dir / "config.yaml").write_text( + "triage:\n enabled: true\n", encoding="utf-8" + ) + assert isinstance(server.kb_triage_pending(), list) + + +# --- sources ------------------------------------------------------------- + + +def test_kb_register_source_stores_content(store: KBStore) -> None: + out = server.kb_register_source("some evidence", title="a note") + assert store.get_source(out["id"]).title == "a note" + + +def test_kb_register_source_from_path(store: KBStore) -> None: + doc = store.root / "note.txt" + doc.write_text("some evidence", encoding="utf-8") + out = server.kb_register_source_from_path(str(doc)) + assert store.get_source(out["id"]) + + +def test_kb_register_source_from_path_refuses_outside_the_project( + store: KBStore, tmp_path: Path +) -> None: + # the containment check is the security boundary: an MCP client must not be + # able to register /etc/shadow as a source + outside = tmp_path / "elsewhere.txt" + outside.write_text("not mine", encoding="utf-8") + with pytest.raises(ValueError, match="inside project root"): + server.kb_register_source_from_path(str(outside)) + + +def test_kb_register_source_from_missing_path_raises(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_register_source_from_path(str(store.root / "nope.txt")) + + +# --- propose / approve / reject ------------------------------------------ + + +def test_kb_propose_claim_files_a_proposal(store: KBStore) -> None: + src = store.put_source(b"e") + out = server.kb_propose_claim(text="a proposed claim", evidence=[src.id]) + assert store.get_proposal(out["proposal_id"] if "proposal_id" in out else out["id"]) + + +def test_kb_propose_claim_dry_run_writes_nothing(store: KBStore) -> None: + src = store.put_source(b"e") + server.kb_propose_claim(text="a dry claim", evidence=[src.id], dry_run=True) + assert store.list_proposals() == [] + + +def test_kb_propose_claim_without_evidence_raises(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_propose_claim(text="uncited", evidence=[]) + + +def test_kb_propose_page_files_a_proposal(store: KBStore) -> None: + out = server.kb_propose_page(title="a page", body="prose") + assert out + + +def test_kb_propose_page_dry_run_writes_nothing(store: KBStore) -> None: + server.kb_propose_page(title="a page", body="prose", dry_run=True) + assert store.list_proposals() == [] + + +def test_kb_propose_entity_files_a_proposal(store: KBStore) -> None: + assert server.kb_propose_entity(name="acme-example", entity_type="company") + + +def test_kb_propose_entity_dry_run_writes_nothing(store: KBStore) -> None: + server.kb_propose_entity( + name="acme-example", entity_type="company", dry_run=True + ) + assert store.list_proposals() == [] + + +def test_kb_propose_relation_files_a_proposal(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + assert server.kb_propose_relation(src="e1", relation="owned_by", target="e2") + + +def test_kb_propose_relation_dry_run_writes_nothing(store: KBStore) -> None: + store.put_entity(Entity(id="e1", name="alice-example", type="person")) + store.put_entity(Entity(id="e2", name="acme-example", type="company")) + server.kb_propose_relation( + src="e1", relation="owned_by", target="e2", dry_run=True + ) + assert store.list_proposals() == [] + + +def test_kb_approve_promotes_a_pending_claim(store: KBStore) -> None: + src = store.put_source(b"e") + pr = propose_claim(store, text="approve me", evidence=[src.id], proposed_by="agent") + server.kb_approve(pr.id) + assert store.list_claims() + + +def test_kb_approve_unknown_proposal_raises(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_approve("no-such-proposal") + + +def test_kb_reject_records_a_reason(store: KBStore) -> None: + src = store.put_source(b"e") + pr = propose_claim(store, text="reject me", evidence=[src.id], proposed_by="agent") + server.kb_reject(pr.id, reason="not useful") + assert store.list_claims() == [] + + +def test_kb_reject_unknown_proposal_raises(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_reject("no-such-proposal", reason="nope") + + +def test_kb_reject_extracted_with_nothing_pending(store: KBStore) -> None: + assert server.kb_reject_extracted() is not None + + +# --- lifecycle-adjacent tools ------------------------------------------- + + +def test_kb_expire_dry_run_by_default(store: KBStore) -> None: + _claim(store, "c1", "a claim") + assert server.kb_expire() is not None + + +def test_kb_expire_applied(store: KBStore) -> None: + _claim(store, "c1", "a claim") + assert server.kb_expire(apply=True, days=1) is not None + + +def test_kb_clear_claims_dry_run(store: KBStore) -> None: + _claim(store, "c1", "an auto claim", auto_approved=True) + out = server.kb_clear_claims(dry_run=True) + assert out is not None + assert store.get_claim("c1") + + +def test_kb_clear_claims_applied(store: KBStore) -> None: + _claim(store, "c1", "an auto claim", auto_approved=True) + assert server.kb_clear_claims() is not None + + +def test_kb_clear_claims_rejects_a_bad_before_date(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_clear_claims(before="not-a-date") + + +def test_kb_cite_resolves_citations(store: KBStore) -> None: + src = store.put_source(b"body", title="the memo") + store.put_claim(Claim(id="c1", text="cited", evidence=[src.id])) + assert server.kb_cite("c1") + + +def test_kb_cite_unknown_claim_raises(store: KBStore) -> None: + # note the inconsistency with the kb_read_* family, which converts this to + # ValueError: kb_cite lets ArtifactNotFoundError through unwrapped + from vouch.storage import ArtifactNotFoundError + + with pytest.raises(ArtifactNotFoundError): + server.kb_cite("ghost") + + +def test_kb_diff_between_two_claims(store: KBStore) -> None: + _claim(store, "old", "the first version") + _claim(store, "new", "the second version") + assert server.kb_diff("old", "new") is not None + + +# --- graph / context ---------------------------------------------------- + + +def test_kb_neighbors_returns_the_root_node(store: KBStore) -> None: + _claim(store, "c1", "a claim with evidence") + assert server.kb_neighbors("c1")["node_id"] == "c1" + + +def test_kb_neighbors_unknown_node_raises(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_neighbors("no-such-node") + + +def test_kb_context_builds_a_pack(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + assert "items" in server.kb_context("review gate") + + +def test_kb_context_with_a_session_records_salience(store: KBStore) -> None: + from vouch import sessions as sess_mod + + _claim(store, "c1", "the review gate is load-bearing") + sess = sess_mod.session_start(store, agent="claude-code", task="close the gap") + out = server.kb_context("review gate", session_id=sess.id) + assert "items" in out + + +def test_kb_graph_export_emits_dot(store: KBStore) -> None: + _claim(store, "c1", "a claim with evidence") + assert server.kb_graph_export() is not None + + +# --- sessions / themes -------------------------------------------------- + + +def test_kb_session_start_needs_a_request_context(store: KBStore) -> None: + import asyncio + + # the only async tool on the surface, and the only one that reads the + # FastMCP request context -- calling it outside a request must fail loudly + with pytest.raises(ValueError, match="Context is not available"): + asyncio.run(server.kb_session_start(task="close the gap")) + + +def test_kb_detect_themes_on_an_empty_kb(store: KBStore) -> None: + assert server.kb_detect_themes()["clusters"] == [] + + +# --- audit -------------------------------------------------------------- + + +def test_kb_audit_lists_events(store: KBStore) -> None: + src = store.put_source(b"e") + pr = propose_claim(store, text="x", evidence=[src.id], proposed_by="agent") + server.kb_approve(pr.id) + assert server.kb_audit()["events"] + + +def test_kb_audit_tail_caps_events(store: KBStore) -> None: + src = store.put_source(b"e") + for i in range(3): + pr = propose_claim( + store, text=f"c{i}", evidence=[src.id], proposed_by="agent" + ) + server.kb_approve(pr.id) + assert len(server.kb_audit(tail=2)["events"]) == 2 + + +def test_kb_audit_accepts_a_viewer_scope(store: KBStore) -> None: + assert server.kb_audit(project="acme-example", agent="claude-code") is not None + + +# --- bundles ------------------------------------------------------------ + + +def _bundle(store: KBStore, tmp_path: Path) -> Path: + from vouch import bundle + + _claim(store, "c1", "a claim to federate") + out = tmp_path / "kb.tar.gz" + bundle.export(store.kb_dir, dest=out, actor="test") + return out + + +def test_kb_export_check_passes(store: KBStore, tmp_path: Path) -> None: + assert server.kb_export_check(str(_bundle(store, tmp_path)))["ok"] is True + + +def test_kb_import_check_reports_identical(store: KBStore, tmp_path: Path) -> None: + out = server.kb_import_check(str(_bundle(store, tmp_path))) + assert out["ok"] is True + + +# --- embeddings --------------------------------------------------------- + + +def test_kb_embeddings_stats(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "a claim to embed") + out = server.kb_embeddings_stats() + assert "query_cache_entries" in out or "counts" in out + + +def test_kb_reindex_embeddings_backfills(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "a claim to embed") + assert server.kb_reindex_embeddings(backfill=True) is not None + + +def test_kb_reindex_embeddings_force(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "a claim to embed") + assert server.kb_reindex_embeddings(backfill=True, force=True) is not None + + +def test_kb_dedup_scan_finds_the_pair(store: KBStore, embedder: None) -> None: + _claim(store, "c1", "identical duplicated text") + _claim(store, "c2", "identical duplicated text") + assert server.kb_dedup_scan() is not None + + +def test_kb_eval_embeddings_with_an_empty_query_set( + store: KBStore, embedder: None, tmp_path: Path +) -> None: + queries = tmp_path / "q.jsonl" + queries.write_text("", encoding="utf-8") + try: + out = server.kb_eval_embeddings(queries_path=str(queries)) + except (ValueError, RuntimeError): + return + assert out is not None + + +# --- skills ------------------------------------------------------------- + + +def test_kb_get_skill_unknown_name_raises(store: KBStore) -> None: + with pytest.raises(ValueError): + server.kb_get_skill("no-such-skill") + + +# --- compile ------------------------------------------------------------ + + +def test_kb_compile_surfaces_a_configuration_failure(store: KBStore) -> None: + _claim(store, "c1", "a claim to compile") + cfg = store.kb_dir / "config.yaml" + cfg.write_text( + json.dumps({"compile": {"llm_cmd": "false"}}), encoding="utf-8" + ) + with pytest.raises((ValueError, RuntimeError)): + server.kb_compile(dry_run=True) diff --git a/tests/test_session_split_renarrate.py b/tests/test_session_split_renarrate.py new file mode 100644 index 00000000..6e120570 --- /dev/null +++ b/tests/test_session_split_renarrate.py @@ -0,0 +1,392 @@ +"""Re-narration of an already-filed mechanical session summary. + +`_try_renarrate` is the second-chance path: a session that was rolled up +mechanically by `vouch-capture` gets narrated into topical pages later, and the +mechanical proposal is rejected as superseded. Every branch here decides +whether a pending proposal survives, so an untested failure mode either strands +the mechanical rollup or drops it without a replacement. + +Also covers `build_session_rows`, which is what `kb.list_sessions` and +`vouch session list` render. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import pytest + +from vouch import capture, session_split +from vouch import compile as compile_mod +from vouch.llm_draft import LLMDraftError +from vouch.models import Page, Proposal, ProposalKind, ProposalStatus +from vouch.session_split import SPLIT_ACTOR, SplitConfig, load_split_config +from vouch.storage import KBStore + + +@pytest.fixture +def store(tmp_path: Path) -> KBStore: + return KBStore.init(tmp_path) + + +def _mechanical( + store: KBStore, + *, + session_id: str = "s1", + proposal_id: str = "pr-mech", + proposed_by: str = capture.CAPTURE_ACTOR, + page_type: str = capture.CAPTURE_PAGE_TYPE, + kind: ProposalKind = ProposalKind.PAGE, + title: str = "session s1", + body: str = "## did a thing\n\nobserved something", + tags: list[str] | None = None, +) -> Proposal: + return store.put_proposal( + Proposal( + id=proposal_id, + kind=kind, + proposed_by=proposed_by, + session_id=session_id, + status=ProposalStatus.PENDING, + payload={ + "type": page_type, + "title": title, + "body": body, + "tags": tags or [], + }, + ) + ) + + +# --- _eligible_mechanical_proposal --------------------------------------- + + +def test_eligible_finds_the_mechanical_rollup(store: KBStore) -> None: + _mechanical(store) + found = session_split._eligible_mechanical_proposal(store, "s1") + assert found is not None + assert found.id == "pr-mech" + + +def test_eligible_ignores_a_non_page_proposal(store: KBStore) -> None: + src = store.put_source(b"e") + store.put_proposal( + Proposal( + id="pr-claim", + kind=ProposalKind.CLAIM, + proposed_by=capture.CAPTURE_ACTOR, + session_id="s1", + status=ProposalStatus.PENDING, + payload={"text": "x", "evidence": [src.id]}, + ) + ) + assert session_split._eligible_mechanical_proposal(store, "s1") is None + + +def test_eligible_ignores_a_page_of_another_type(store: KBStore) -> None: + _mechanical(store, page_type="concept") + assert session_split._eligible_mechanical_proposal(store, "s1") is None + + +def test_eligible_ignores_another_session(store: KBStore) -> None: + _mechanical(store, session_id="other") + assert session_split._eligible_mechanical_proposal(store, "s1") is None + + +def test_eligible_ignores_an_already_narrated_proposal(store: KBStore) -> None: + # a session-split proposal is already narrated -- re-narrating would loop + _mechanical(store, proposed_by=SPLIT_ACTOR) + assert session_split._eligible_mechanical_proposal(store, "s1") is None + + +def test_eligible_returns_none_on_an_empty_queue(store: KBStore) -> None: + assert session_split._eligible_mechanical_proposal(store, "s1") is None + + +# --- build_renarrate_prompt ---------------------------------------------- + + +def test_renarrate_prompt_includes_the_record_body(store: KBStore) -> None: + prompt = session_split.build_renarrate_prompt( + store, "the session body", title="session s1", max_pages=3 + ) + assert "the session body" in prompt + assert "SESSION RECORD TITLE: session s1" in prompt + + +def test_renarrate_prompt_omits_an_empty_title(store: KBStore) -> None: + prompt = session_split.build_renarrate_prompt( + store, "body", title="", max_pages=3 + ) + assert "SESSION RECORD TITLE" not in prompt + + +def test_renarrate_prompt_says_none_when_no_topics_are_taken( + store: KBStore, +) -> None: + prompt = session_split.build_renarrate_prompt( + store, "body", title="t", max_pages=3 + ) + assert "- (none)" in prompt + + +def test_renarrate_prompt_lists_durable_and_pending_topics( + store: KBStore, +) -> None: + store.put_page(Page(id="p1", title="the review gate")) + store.put_proposal( + Proposal( + id="pr-pending-page", + kind=ProposalKind.PAGE, + proposed_by="agent", + status=ProposalStatus.PENDING, + payload={"type": "concept", "title": "retrieval backends", "body": "x"}, + ) + ) + prompt = session_split.build_renarrate_prompt( + store, "body", title="t", max_pages=3 + ) + assert "- the review gate" in prompt + pending = compile_mod._pending_page_names(store) + if "retrieval backends" in pending: + assert "- retrieval backends [pending]" in prompt + + +# --- _try_renarrate ------------------------------------------------------ + + +def test_renarrate_returns_none_without_an_eligible_proposal( + store: KBStore, +) -> None: + assert session_split._try_renarrate(store, "s1", split_cfg=SplitConfig()) is None + + +def test_renarrate_skips_when_no_llm_is_configured(store: KBStore) -> None: + prop = _mechanical(store) + out = session_split._try_renarrate( + store, "s1", split_cfg=SplitConfig(llm_cmd=None) + ) + assert out is not None + assert out["skipped"] == "not-configured" + assert out["proposal_id"] == prop.id + # the mechanical rollup must survive an unconfigured re-narration + assert store.get_proposal(prop.id).status == ProposalStatus.PENDING + + +def test_renarrate_skips_and_keeps_the_rollup_when_the_llm_fails( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + prop = _mechanical(store) + + def _boom(*_a: Any, **_k: Any) -> str: + raise LLMDraftError("llm exited 1") + + monkeypatch.setattr(session_split.llm_draft, "run_llm", _boom) + out = session_split._try_renarrate( + store, "s1", split_cfg=SplitConfig(llm_cmd="false") + ) + assert out is not None + assert out["skipped"] == "llm-failed" + assert store.get_proposal(prop.id).status == ProposalStatus.PENDING + + +def test_renarrate_skips_when_the_llm_yields_no_valid_drafts( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + prop = _mechanical(store) + monkeypatch.setattr(session_split.llm_draft, "run_llm", lambda *_a, **_k: "[]") + monkeypatch.setattr(session_split.llm_draft, "parse_drafts", lambda *_a, **_k: []) + out = session_split._try_renarrate( + store, "s1", split_cfg=SplitConfig(llm_cmd="true") + ) + assert out is not None + assert out["skipped"] == "llm-failed" + assert store.get_proposal(prop.id).status == ProposalStatus.PENDING + + +def test_renarrate_files_pages_and_supersedes_the_rollup( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + prop = _mechanical(store) + monkeypatch.setattr(session_split.llm_draft, "run_llm", lambda *_a, **_k: "x") + monkeypatch.setattr( + session_split.llm_draft, + "parse_drafts", + lambda *_a, **_k: [ + {"title": "the coverage grind", "body": "narrated prose"}, + ], + ) + out = session_split._try_renarrate( + store, "s1", split_cfg=SplitConfig(llm_cmd="true", max_pages=3) + ) + assert out is not None + assert out["mode"] == "renarrated" + assert out["summarized"] is True + assert out["superseded"] == prop.id + assert out["summary_proposal_ids"] + # the mechanical rollup is rejected, not left as a duplicate + assert store.get_proposal(prop.id).status == ProposalStatus.REJECTED + + +def test_renarrate_falls_back_to_the_compile_llm_cmd( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + _mechanical(store) + store.config_path.write_text( + "compile:\n llm_cmd: 'true'\n", encoding="utf-8" + ) + monkeypatch.setattr(session_split.llm_draft, "run_llm", lambda *_a, **_k: "x") + monkeypatch.setattr( + session_split.llm_draft, + "parse_drafts", + lambda *_a, **_k: [{"title": "a topic", "body": "prose"}], + ) + out = session_split._try_renarrate( + store, "s1", split_cfg=SplitConfig(llm_cmd=None) + ) + assert out is not None + assert out["mode"] == "renarrated" + + +# --- _skip --------------------------------------------------------------- + + +def test_skip_envelope_shape() -> None: + out = session_split._skip("s1", "not-configured", proposal_id="pr-1") + assert out["mode"] == "skipped" + assert out["skipped"] == "not-configured" + assert out["summarized"] is False + assert out["captured"] == 0 + assert out["summary_proposal_ids"] == [] + assert out["proposal_id"] == "pr-1" + + +# --- build_session_rows -------------------------------------------------- + + +def test_session_rows_empty_on_a_fresh_kb(store: KBStore) -> None: + assert session_split.build_session_rows(store) == [] + + +def test_session_rows_lists_a_mechanical_rollup_as_unsummarized( + store: KBStore, +) -> None: + _mechanical(store) + rows = session_split.build_session_rows(store) + assert len(rows) == 1 + assert rows[0]["stage"] == "pending" + assert rows[0]["summarized"] is False + assert rows[0]["proposal_id"] == "pr-mech" + + +def test_session_rows_marks_a_split_proposal_summarized(store: KBStore) -> None: + _mechanical(store, proposed_by=SPLIT_ACTOR) + assert session_split.build_session_rows(store)[0]["summarized"] is True + + +def test_session_rows_marks_a_split_tagged_proposal_summarized( + store: KBStore, +) -> None: + _mechanical(store, tags=["split"]) + assert session_split.build_session_rows(store)[0]["summarized"] is True + + +def test_session_rows_lists_an_open_buffer(store: KBStore) -> None: + caps = capture.captures_dir(store) + caps.mkdir(parents=True, exist_ok=True) + (caps / "s-open.jsonl").write_text( + json.dumps({"ts": 1000.0, "text": "did a thing"}) + "\n", encoding="utf-8" + ) + rows = session_split.build_session_rows(store) + buffers = [r for r in rows if r["stage"] == "buffer"] + assert len(buffers) == 1 + assert buffers[0]["session_id"] == "s-open" + assert buffers[0]["summarized"] is False + assert buffers[0]["observations"] == 1 + assert buffers[0]["last_activity"] is not None + + +def test_session_rows_buffer_without_timestamps_has_no_last_activity( + store: KBStore, +) -> None: + caps = capture.captures_dir(store) + caps.mkdir(parents=True, exist_ok=True) + (caps / "s-nots.jsonl").write_text( + json.dumps({"text": "no ts here"}) + "\n", encoding="utf-8" + ) + rows = [r for r in session_split.build_session_rows(store) if r["stage"] == "buffer"] + assert rows[0]["last_activity"] is None + + +def test_session_rows_does_not_double_list_a_filed_session( + store: KBStore, +) -> None: + _mechanical(store, session_id="s1") + caps = capture.captures_dir(store) + caps.mkdir(parents=True, exist_ok=True) + (caps / "s1.jsonl").write_text( + json.dumps({"ts": 1000.0, "text": "leftover buffer"}) + "\n", encoding="utf-8" + ) + rows = session_split.build_session_rows(store) + assert [r["stage"] for r in rows] == ["pending"] + + +def test_session_rows_sorts_newest_activity_first(store: KBStore) -> None: + caps = capture.captures_dir(store) + caps.mkdir(parents=True, exist_ok=True) + (caps / "older.jsonl").write_text( + json.dumps({"ts": 1000.0, "text": "a"}) + "\n", encoding="utf-8" + ) + (caps / "newer.jsonl").write_text( + json.dumps({"ts": 9000.0, "text": "b"}) + "\n", encoding="utf-8" + ) + rows = session_split.build_session_rows(store) + assert [r["session_id"] for r in rows] == ["newer", "older"] + + +# --- load_split_config edge cases ---------------------------------------- + + +def test_split_config_unreadable_file_falls_back(store: KBStore) -> None: + store.config_path.unlink() + assert load_split_config(store) == SplitConfig() + + +def test_split_config_scalar_document_falls_back(store: KBStore) -> None: + store.config_path.write_text("just-a-string\n", encoding="utf-8") + assert load_split_config(store) == SplitConfig() + + +def test_split_config_without_a_capture_block_falls_back(store: KBStore) -> None: + store.config_path.write_text("review:\n gate: true\n", encoding="utf-8") + assert load_split_config(store) == SplitConfig() + + +def test_split_config_reads_every_field(store: KBStore) -> None: + store.config_path.write_text( + "capture:\n" + " split:\n" + " enabled: false\n" + " llm_cmd: 'true'\n" + " threshold_observations: 7\n" + " max_pages: 2\n" + " timeout_seconds: 1.5\n" + " max_input_chars: 900\n", + encoding="utf-8", + ) + cfg = load_split_config(store) + assert cfg.enabled is False + assert cfg.llm_cmd == "true" + assert cfg.threshold_observations == 7 + assert cfg.max_pages == 2 + assert cfg.timeout_seconds == 1.5 + assert cfg.max_input_chars == 900 + + +def test_split_config_empty_llm_cmd_becomes_none(store: KBStore) -> None: + store.config_path.write_text( + "capture:\n split:\n llm_cmd: ''\n", encoding="utf-8" + ) + assert load_split_config(store).llm_cmd is None diff --git a/tests/test_strategy_sandbox_child.py b/tests/test_strategy_sandbox_child.py new file mode 100644 index 00000000..7e2dac30 --- /dev/null +++ b/tests/test_strategy_sandbox_child.py @@ -0,0 +1,512 @@ +"""The sandbox child half of `vouch.strategy`. + +These are the functions that run inside `python -I -m vouch.strategy --child`. +They cannot be covered by exercising `run_sandboxed` for real: that child is +spawned with `env={"PATH": ...}`, which strips `COVERAGE_PROCESS_START`, so the +subprocess is deliberately unmeasured. + +They also cannot be called naively in-process — `sys.addaudithook` is permanent +for the life of the interpreter, and `_child_main` does `os.dup2(devnull, 1)`, +which would silence pytest's own stdout for every later test. So each hazardous +primitive is intercepted: + +* `sys.addaudithook` is swapped for a collector, which both runs + `_install_audit_hook` to completion and hands back the closure so `_hook`'s + own branches can be driven directly. +* `resource.setrlimit` is stubbed — the real function would drop this process + to `RLIMIT_NOFILE = 64`. +* `os.dup2` is stubbed so fd 1 survives. +""" + +from __future__ import annotations + +import io +import json +import os +import subprocess +import sys +from pathlib import Path +from typing import Any + +import pytest + +from vouch import strategy as strat +from vouch.strategy import Candidate + + +def _candidates() -> list[Candidate]: + return [ + Candidate(kind="claim", id="c1", summary="the review gate", score=0.9), + Candidate(kind="claim", id="c2", summary="something else", score=0.4), + ] + + +@pytest.fixture +def captured_hook(monkeypatch: pytest.MonkeyPatch) -> list[Any]: + """Run `_install_audit_hook` without arming a permanent audit hook.""" + hooks: list[Any] = [] + monkeypatch.setattr(sys, "addaudithook", hooks.append) + return hooks + + +# --- _install_audit_hook / _hook ----------------------------------------- + + +def test_install_audit_hook_registers_exactly_one_hook( + captured_hook: list[Any] +) -> None: + strat._install_audit_hook() + assert len(captured_hook) == 1 + assert callable(captured_hook[0]) + + +def test_hook_blocks_each_exact_blocked_event(captured_hook: list[Any]) -> None: + strat._install_audit_hook() + hook = captured_hook[0] + for event in strat._BLOCKED_EXACT: + with pytest.raises(PermissionError, match="blocked in strategy sandbox"): + hook(event, ()) + + +def test_hook_blocks_each_blocked_prefix(captured_hook: list[Any]) -> None: + strat._install_audit_hook() + hook = captured_hook[0] + for prefix in strat._BLOCKED_PREFIXES: + with pytest.raises(PermissionError, match="blocked in strategy sandbox"): + hook(f"{prefix}something", ()) + + +def test_hook_allows_an_unrelated_event(captured_hook: list[Any]) -> None: + strat._install_audit_hook() + assert captured_hook[0]("object.__getattr__", ()) is None + + +@pytest.mark.parametrize("mode", ["w", "a", "x", "r+", "wb"]) +def test_hook_blocks_writeish_open_modes( + captured_hook: list[Any], mode: str +) -> None: + strat._install_audit_hook() + hook = captured_hook[0] + with pytest.raises(PermissionError, match="filesystem writes are blocked"): + hook("open", ("/tmp/x", mode, 0)) + + +def test_hook_allows_a_read_only_open(captured_hook: list[Any]) -> None: + # reads must stay allowed: the sandbox has to be able to import numpy + strat._install_audit_hook() + assert captured_hook[0]("open", ("/tmp/x", "r", 0)) is None + + +@pytest.mark.parametrize( + "flags", + [os.O_WRONLY, os.O_RDWR, os.O_CREAT, os.O_APPEND, os.O_WRONLY | os.O_CREAT], +) +def test_hook_blocks_writeish_open_flags( + captured_hook: list[Any], flags: int +) -> None: + strat._install_audit_hook() + hook = captured_hook[0] + with pytest.raises(PermissionError, match="filesystem writes are blocked"): + hook("open", ("/tmp/x", None, flags)) + + +def test_hook_allows_read_only_open_flags(captured_hook: list[Any]) -> None: + strat._install_audit_hook() + assert captured_hook[0]("open", ("/tmp/x", None, os.O_RDONLY)) is None + + +def test_hook_tolerates_a_short_open_arg_tuple(captured_hook: list[Any]) -> None: + strat._install_audit_hook() + assert captured_hook[0]("open", ()) is None + + +def test_hook_reads_the_guard_sets_from_closure_cells( + captured_hook: list[Any], monkeypatch: pytest.MonkeyPatch +) -> None: + # the documented hardening: reassigning the module global must not disarm + # the installed hook, because it reads a closure cell instead + strat._install_audit_hook() + hook = captured_hook[0] + monkeypatch.setattr(strat, "_BLOCKED_EXACT", frozenset()) + monkeypatch.setattr(strat, "_BLOCKED_PREFIXES", ()) + event = next(iter(strat._BLOCKED_EXACT)) if strat._BLOCKED_EXACT else "socket.connect" + with pytest.raises(PermissionError): + hook(event, ()) + + +# --- _apply_rlimits ------------------------------------------------------ + + +def test_apply_rlimits_sets_each_limit(monkeypatch: pytest.MonkeyPatch) -> None: + import resource + + calls: list[tuple[int, tuple[int, int]]] = [] + monkeypatch.setattr(resource, "getrlimit", lambda _res: (0, resource.RLIM_INFINITY)) + monkeypatch.setattr( + resource, "setrlimit", lambda res, pair: calls.append((res, pair)) + ) + strat._apply_rlimits(1024, 5) + limited = {res for res, _ in calls} + assert resource.RLIMIT_CPU in limited + assert resource.RLIMIT_AS in limited + assert resource.RLIMIT_NOFILE in limited + + +def test_apply_rlimits_respects_a_finite_hard_limit( + monkeypatch: pytest.MonkeyPatch +) -> None: + import resource + + calls: list[tuple[int, tuple[int, int]]] = [] + monkeypatch.setattr(resource, "getrlimit", lambda _res: (0, 4)) + monkeypatch.setattr( + resource, "setrlimit", lambda res, pair: calls.append((res, pair)) + ) + strat._apply_rlimits(1024, 999) + # never raise above the inherited hard ceiling + assert all(soft <= 4 for _res, (soft, _hard) in calls) + + +def test_apply_rlimits_swallows_setrlimit_errors( + monkeypatch: pytest.MonkeyPatch +) -> None: + import resource + + def _refuse(_res: int, _pair: tuple[int, int]) -> None: + raise ValueError("not permitted") + + monkeypatch.setattr(resource, "getrlimit", lambda _res: (0, resource.RLIM_INFINITY)) + monkeypatch.setattr(resource, "setrlimit", _refuse) + strat._apply_rlimits(1024, 5) # must not raise + + +def test_apply_rlimits_is_a_noop_without_the_resource_module( + monkeypatch: pytest.MonkeyPatch +) -> None: + # windows has no `resource`; the audit hook still applies there + monkeypatch.setitem(sys.modules, "resource", None) + strat._apply_rlimits(1024, 5) + + +# --- _child_main --------------------------------------------------------- + + +@pytest.fixture +def strategy_file(tmp_path: Path) -> Path: + path = tmp_path / "reverse_strategy.py" + path.write_text( + "def rank(query, candidates, *, limit):\n" + " return [c.id for c in reversed(candidates)][:limit]\n", + encoding="utf-8", + ) + return path + + +def _run_child( + monkeypatch: pytest.MonkeyPatch, payload: dict[str, Any] +) -> tuple[int, str]: + """Drive `_child_main` with fd 1 and the audit hook neutralised.""" + written: list[bytes] = [] + monkeypatch.setattr(sys, "addaudithook", lambda _h: None) + monkeypatch.setattr(strat, "_apply_rlimits", lambda *_a, **_k: None) + monkeypatch.setattr(os, "dup2", lambda _a, _b: None) + monkeypatch.setattr(os, "write", lambda _fd, data: written.append(data) or len(data)) + monkeypatch.setattr(sys, "stdin", io.StringIO(json.dumps(payload))) + rc = strat._child_main() + return rc, b"".join(written).decode("utf-8") + + +def test_child_main_returns_the_strategy_order( + monkeypatch: pytest.MonkeyPatch, strategy_file: Path +) -> None: + rc, out = _run_child( + monkeypatch, + { + "path": str(strategy_file), + "query": "gate", + "limit": 10, + "mem_bytes": 1024, + "cpu_seconds": 5, + "candidates": [ + {"kind": "claim", "id": "c1", "summary": "one", "score": 0.9}, + {"kind": "claim", "id": "c2", "summary": "two", "score": 0.4}, + ], + }, + ) + assert rc == 0 + assert json.loads(out)["ordered"] == ["c2", "c1"] + + +def test_child_main_drops_ids_the_strategy_invented( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + # a submission must not be able to inject ids that were never candidates + path = tmp_path / "liar.py" + path.write_text( + "def rank(query, candidates, *, limit):\n" + " return ['c1', 'not-a-candidate']\n", + encoding="utf-8", + ) + _rc, out = _run_child( + monkeypatch, + { + "path": str(path), + "query": "q", + "limit": 10, + "mem_bytes": 1024, + "cpu_seconds": 5, + "candidates": [ + {"kind": "claim", "id": "c1", "summary": "one", "score": 0.9} + ], + }, + ) + assert json.loads(out)["ordered"] == ["c1"] + + +def test_child_main_honours_the_limit( + monkeypatch: pytest.MonkeyPatch, strategy_file: Path +) -> None: + _rc, out = _run_child( + monkeypatch, + { + "path": str(strategy_file), + "query": "q", + "limit": 1, + "mem_bytes": 1024, + "cpu_seconds": 5, + "candidates": [ + {"kind": "claim", "id": "c1", "summary": "one", "score": 0.9}, + {"kind": "claim", "id": "c2", "summary": "two", "score": 0.4}, + ], + }, + ) + assert json.loads(out)["ordered"] == ["c2"] + + +# --- main ---------------------------------------------------------------- + + +def test_main_dispatches_to_the_child(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(strat, "_child_main", lambda: 0) + assert strat.main(["--child"]) == 0 + + +def test_main_without_child_prints_usage( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + assert strat.main([]) == 2 + assert "usage:" in capsys.readouterr().out + + +def test_main_reads_sys_argv_when_given_none( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.setattr(sys, "argv", ["vouch.strategy"]) + assert strat.main() == 2 + assert "usage:" in capsys.readouterr().out + + +# --- run_sandboxed failure modes ----------------------------------------- + + +def _fake_proc(returncode: int, stdout: str) -> Any: + class _P: + pass + + p = _P() + p.returncode = returncode # type: ignore[attr-defined] + p.stdout = stdout # type: ignore[attr-defined] + return p + + +def test_run_sandboxed_returns_none_on_timeout( + monkeypatch: pytest.MonkeyPatch +) -> None: + def _timeout(*_a: Any, **_k: Any) -> Any: + raise subprocess.TimeoutExpired(cmd="python", timeout=1) + + monkeypatch.setattr(subprocess, "run", _timeout) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) is None + + +def test_run_sandboxed_returns_none_on_oserror( + monkeypatch: pytest.MonkeyPatch +) -> None: + def _boom(*_a: Any, **_k: Any) -> Any: + raise OSError("no interpreter") + + monkeypatch.setattr(subprocess, "run", _boom) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) is None + + +def test_run_sandboxed_returns_none_on_nonzero_exit( + monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(subprocess, "run", lambda *_a, **_k: _fake_proc(1, "")) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) is None + + +def test_run_sandboxed_returns_none_on_malformed_json( + monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _fake_proc(0, "not json at all") + ) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) is None + + +def test_run_sandboxed_returns_none_when_ordered_is_missing( + monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _fake_proc(0, '{"other": 1}') + ) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) is None + + +def test_run_sandboxed_returns_none_when_ordered_is_not_a_list( + monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _fake_proc(0, '{"ordered": "c1"}') + ) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) is None + + +def test_run_sandboxed_coerces_ids_to_strings( + monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + subprocess, "run", lambda *_a, **_k: _fake_proc(0, '{"ordered": [1, "c2"]}') + ) + assert strat.run_sandboxed("s.py", "q", _candidates(), limit=5) == ["1", "c2"] + + +def test_run_sandboxed_really_runs_a_strategy_file(strategy_file: Path) -> None: + # one end-to-end pass through the real subprocess, so the wiring is proven + # even though the child's own lines are measured by the tests above + out = strat.run_sandboxed(str(strategy_file), "q", _candidates(), limit=5) + assert out == ["c2", "c1"] + + +# --- load_from_path / _strategy_from_module ------------------------------ + + +def test_load_from_path_accepts_a_strategy_object(tmp_path: Path) -> None: + path = tmp_path / "obj_strategy.py" + path.write_text( + "class _S:\n" + " def rank(self, query, candidates, *, limit):\n" + " return [c.id for c in candidates][:limit]\n" + "STRATEGY = _S()\n", + encoding="utf-8", + ) + assert strat.load_from_path(path).rank("q", _candidates(), limit=1) == ["c1"] + + +def test_load_from_path_rejects_a_module_without_a_strategy(tmp_path: Path) -> None: + path = tmp_path / "empty_strategy.py" + path.write_text("X = 1\n", encoding="utf-8") + with pytest.raises(ValueError, match="must define STRATEGY"): + strat.load_from_path(path) + + +def test_load_from_path_rejects_an_unloadable_file(tmp_path: Path) -> None: + with pytest.raises((ValueError, FileNotFoundError, ImportError)): + strat.load_from_path(tmp_path / "missing.py") + + +def test_load_from_path_rejects_a_file_with_no_import_spec( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import importlib.util + + path = tmp_path / "s.py" + path.write_text("X = 1\n", encoding="utf-8") + monkeypatch.setattr(importlib.util, "spec_from_file_location", lambda *_a: None) + with pytest.raises(ValueError, match="cannot load strategy"): + strat.load_from_path(path) + + +def test_load_dotted_imports_a_shipped_strategy() -> None: + # the trusted path: a merged strategy resolved from config by dotted name + loaded = strat.load_dotted("vouch.strategies.provenance") + assert hasattr(loaded, "rank") + + +# --- apply_ordering ------------------------------------------------------ + + +def _hits() -> list[strat.Hit]: + return [ + ("claim", "c1", "one", 0.9), + ("claim", "c2", "two", 0.5), + ("claim", "c3", "three", 0.1), + ] + + +def test_apply_ordering_reorders_the_hits() -> None: + out = strat.apply_ordering(["c3", "c1", "c2"], _hits()) + assert [h[1] for h in out] == ["c3", "c1", "c2"] + + +def test_apply_ordering_drops_ids_that_are_not_hits() -> None: + out = strat.apply_ordering(["c3", "invented"], _hits()) + # the invariant: reorder yes, fabricate no + assert [h[1] for h in out] == ["c3", "c1", "c2"] + + +def test_apply_ordering_ignores_a_repeated_id() -> None: + out = strat.apply_ordering(["c2", "c2"], _hits()) + assert [h[1] for h in out] == ["c2", "c1", "c3"] + + +def test_apply_ordering_appends_unmentioned_hits_in_original_order() -> None: + out = strat.apply_ordering(["c2"], _hits()) + assert [h[1] for h in out] == ["c2", "c1", "c3"] + + +def test_apply_ordering_with_an_empty_ordering_keeps_backend_order() -> None: + assert strat.apply_ordering([], _hits()) == _hits() + + +def test_apply_ordering_dedupes_duplicate_hits_by_id() -> None: + hits = [*_hits(), ("claim", "c1", "one-again", 0.2)] + out = strat.apply_ordering(["c1"], hits) + assert [h[1] for h in out] == ["c1", "c2", "c3"] + + +# --- SandboxProxy -------------------------------------------------------- + + +def test_sandbox_proxy_resolves_the_path(tmp_path: Path) -> None: + path = tmp_path / "s.py" + path.write_text("def rank(q, c, *, limit):\n return []\n", encoding="utf-8") + proxy = strat.SandboxProxy(path) + assert proxy.path == str(path.resolve()) + assert proxy.failures == 0 + + +def test_sandbox_proxy_returns_the_child_ordering(strategy_file: Path) -> None: + proxy = strat.SandboxProxy(strategy_file) + assert proxy.rank("q", _candidates(), limit=5) == ["c2", "c1"] + assert proxy.failures == 0 + + +def test_sandbox_proxy_counts_failures_and_returns_empty( + monkeypatch: pytest.MonkeyPatch, strategy_file: Path +) -> None: + monkeypatch.setattr(strat, "run_sandboxed", lambda *_a, **_k: None) + proxy = strat.SandboxProxy(strategy_file) + assert proxy.rank("q", _candidates(), limit=5) == [] + assert proxy.failures == 1 + # an empty ordering means "keep the backend's order", not an aborted run + assert strat.apply_ordering([], _hits()) == _hits() + + +def test_sandbox_proxy_honours_custom_limits(strategy_file: Path) -> None: + proxy = strat.SandboxProxy(strategy_file, timeout_s=5.0, mem_mb=256, cpu_s=3) + assert proxy.timeout_s == 5.0 + assert proxy.mem_mb == 256 + assert proxy.cpu_s == 3 + assert proxy.rank("q", _candidates(), limit=5) == ["c2", "c1"] diff --git a/tests/test_verify.py b/tests/test_verify.py index fd076b2b..f3a47f55 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -30,6 +30,34 @@ def test_verify_detects_external_drift(store: KBStore, tmp_path: Path) -> None: assert target.external_status == "drift" +def test_verify_all_counts_missing_external_as_failed( + store: KBStore, tmp_path: Path, +) -> None: + """CLI source verify treats missing as '!'; verify_all's audit failed + list must include it too (not only drift).""" + from vouch import audit + + f = tmp_path / "doc.txt" + f.write_bytes(b"original") + src = store.put_source( + f.read_bytes(), title="doc", + locator=str(f.resolve()), source_type="file", + ) + f.unlink() + results = verify.verify_all(store) + target = next(r for r in results if r.source.id == src.id) + assert target.stored_ok is True + assert target.external_status == "missing" + + events = [ + e for e in audit.read_events(store.kb_dir) + if e.event == "source.verify" + ] + assert events + assert src.id in events[-1].object_ids + assert events[-1].data["failed"] >= 1 + + def test_verify_refuses_off_root_file_locator(store: KBStore, tmp_path: Path) -> None: outside = tmp_path.parent / f"{tmp_path.name}-outside.txt" outside.write_bytes(b"secret") diff --git a/tests/test_volunteer_context_paths.py b/tests/test_volunteer_context_paths.py new file mode 100644 index 00000000..63dbb513 --- /dev/null +++ b/tests/test_volunteer_context_paths.py @@ -0,0 +1,541 @@ +"""Config, scoring filters, and the MCP push / watch-thread paths. + +`volunteer_context` proactively offers approved claims into a live session, so +its filters are the difference between a useful nudge and a stream of noise: +retracted claims must never be offered, an already-offered claim must not +repeat, and the per-session cap and throttle must hold. The MCP push side is +best-effort by design — a dead notification channel must log, never raise into +the caller. +""" + +from __future__ import annotations + +import asyncio +import threading +from pathlib import Path +from typing import Any + +import pytest + +from vouch import hot_memory +from vouch import volunteer_context as vc +from vouch.models import Claim, ClaimStatus, Session +from vouch.storage import KBStore +from vouch.volunteer_context import VolunteerConfig, VolunteerOffer + + +@pytest.fixture +def store(tmp_path: Path) -> KBStore: + return KBStore.init(tmp_path) + + +@pytest.fixture(autouse=True) +def _clean_module_state() -> Any: + """Both modules keep process-global registries; isolate every test.""" + + def _reset() -> None: + with vc._state_lock: + vc._pending.clear() + vc._mcp_push.clear() + for sid in list(vc._watch_threads): + hot_memory.unregister(sid) + vc._watch_threads.clear() + with hot_memory._lock: + hot_memory._registry.clear() + hot_memory._SIDEBAR_CACHE.clear() + + _reset() + yield + _reset() + + +def _session(session_id: str = "s1", *, task: str | None = "the review gate") -> Session: + return Session(id=session_id, agent="claude-code", task=task) + + +def _claim(store: KBStore, claim_id: str, text: str, **kw: Any) -> Claim: + src = store.put_source(b"evidence body") + return store.put_claim(Claim(id=claim_id, text=text, evidence=[src.id], **kw)) + + +# --- load_config --------------------------------------------------------- + + +def test_config_defaults(store: KBStore) -> None: + assert vc.load_config(store) == VolunteerConfig() + + +def test_config_unreadable_file_falls_back(store: KBStore) -> None: + store.config_path.unlink() + assert vc.load_config(store) == VolunteerConfig() + + +def test_config_malformed_yaml_falls_back(store: KBStore) -> None: + store.config_path.write_text("volunteer: [unclosed\n", encoding="utf-8") + assert vc.load_config(store) == VolunteerConfig() + + +def test_config_scalar_document_falls_back(store: KBStore) -> None: + store.config_path.write_text("just-a-string\n", encoding="utf-8") + assert vc.load_config(store) == VolunteerConfig() + + +def test_config_non_mapping_volunteer_block_falls_back(store: KBStore) -> None: + store.config_path.write_text("volunteer: not-a-mapping\n", encoding="utf-8") + assert vc.load_config(store) == VolunteerConfig() + + +def test_config_reads_every_field(store: KBStore) -> None: + store.config_path.write_text( + "volunteer:\n" + " enabled: false\n" + " threshold: 0.75\n" + " throttle_seconds: 9\n" + " poll_interval_seconds: 3\n" + " max_per_session: 2\n", + encoding="utf-8", + ) + cfg = vc.load_config(store) + assert cfg.enabled is False + assert cfg.threshold == 0.75 + assert cfg.throttle_seconds == 9.0 + assert cfg.poll_interval_seconds == 3.0 + assert cfg.max_per_session == 2 + + +# --- session_query / normalize_relevance --------------------------------- + + +def test_session_query_joins_task_and_note() -> None: + sess = Session(id="s1", agent="a", task=" the gate ", note=" and a note ") + assert vc.session_query(sess) == "the gate and a note" + + +def test_session_query_is_none_without_task_or_note() -> None: + assert vc.session_query(Session(id="s1", agent="a")) is None + + +def test_normalize_relevance_clamps_embedding_scores() -> None: + assert vc.normalize_relevance(1.4, "embedding", batch_max=1.0) == 1.0 + assert vc.normalize_relevance(-0.2, "embedding", batch_max=1.0) == 0.0 + assert vc.normalize_relevance(0.5, "embedding", batch_max=1.0) == 0.5 + + +def test_normalize_relevance_zero_batch_max_is_zero() -> None: + # fts5/substring scores are unbounded, so they are scaled by the batch max; + # a zero max would divide by zero + assert vc.normalize_relevance(3.0, "fts5", batch_max=0.0) == 0.0 + + +def test_normalize_relevance_scales_by_batch_max() -> None: + assert vc.normalize_relevance(2.0, "fts5", batch_max=4.0) == 0.5 + + +# --- _retrieve_claim_scores --------------------------------------------- + + +def _viewer(store: KBStore) -> Any: + from vouch.scoping import viewer_from + + return viewer_from(config_path=store.config_path, project=None, agent=None) + + +def test_retrieve_scores_returns_nothing_on_an_empty_kb(store: KBStore) -> None: + assert vc._retrieve_claim_scores(store, "gate", _viewer(store)) == [] + + +def test_retrieve_scores_finds_a_matching_claim(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + out = vc._retrieve_claim_scores(store, "review gate", _viewer(store)) + assert [row[0] for row in out] == ["c1"] + assert 0.0 <= out[0][1] <= 1.0 + + +@pytest.mark.parametrize( + "status", + [ClaimStatus.SUPERSEDED, ClaimStatus.REDACTED, ClaimStatus.ARCHIVED], +) +def test_retrieve_scores_skips_retracted_claims( + store: KBStore, status: ClaimStatus +) -> None: + _claim(store, "c1", "the review gate is load-bearing", status=status) + assert vc._retrieve_claim_scores(store, "review gate", _viewer(store)) == [] + + +def test_retrieve_scores_skips_a_claim_whose_file_vanished( + store: KBStore, monkeypatch: pytest.MonkeyPatch +) -> None: + # index says the claim exists, disk says otherwise -- must skip, not raise + from vouch.storage import ArtifactNotFoundError + + _claim(store, "c1", "the review gate is load-bearing") + + def _gone(_claim_id: str) -> Claim: + raise ArtifactNotFoundError("claim c1") + + monkeypatch.setattr(store, "get_claim", _gone) + assert vc._retrieve_claim_scores(store, "review gate", _viewer(store)) == [] + + +def test_retrieve_scores_sorts_by_relevance(store: KBStore) -> None: + _claim(store, "c1", "review gate review gate review gate") + _claim(store, "c2", "review gate mentioned once") + out = vc._retrieve_claim_scores(store, "review gate", _viewer(store)) + rels = [row[1] for row in out] + assert rels == sorted(rels, reverse=True) + + +# --- _build_why ---------------------------------------------------------- + + +def test_build_why_includes_a_snippet_preview() -> None: + why = vc._build_why( + claim_id="c1", query="gate", relevance=0.9, backend="fts5", + snippet="the «review» gate matters", + ) + assert "the review gate matters" in why + assert "fts5 relevance 0.90" in why + + +def test_build_why_without_a_snippet() -> None: + why = vc._build_why( + claim_id="c1", query="gate", relevance=0.9, backend="fts5", snippet=" ", + ) + assert "matches with fts5 relevance 0.90" in why + + +# --- evaluate_session ---------------------------------------------------- + + +def test_evaluate_returns_none_when_disabled(store: KBStore) -> None: + out = vc.evaluate_session( + store, _session(), config=VolunteerConfig(enabled=False) + ) + assert out is None + + +def test_evaluate_returns_none_without_a_query(store: KBStore) -> None: + assert vc.evaluate_session(store, _session(task=None)) is None + + +def test_evaluate_returns_none_without_hot_memory(store: KBStore) -> None: + # no register() call: nothing is tracking this session + assert vc.evaluate_session(store, _session()) is None + + +def test_evaluate_returns_none_at_the_per_session_cap(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + hot_memory.mark_volunteered("s1", "c1", pushed_at=0.0) + out = vc.evaluate_session( + store, _session(), config=VolunteerConfig(max_per_session=1) + ) + assert out is None + + +def test_evaluate_offers_a_matching_claim(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + out = vc.evaluate_session( + store, _session(), config=VolunteerConfig(threshold=0.0, throttle_seconds=0.0) + ) + assert out is not None + assert out.claim_id == "c1" + assert out.session_id == "s1" + + +def test_evaluate_skips_an_already_offered_claim(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + hot_memory.mark_volunteered("s1", "c1", pushed_at=0.0) + out = vc.evaluate_session( + store, + _session(), + config=VolunteerConfig( + threshold=0.0, throttle_seconds=0.0, max_per_session=99 + ), + ) + assert out is None + + +def test_evaluate_respects_the_throttle(store: KBStore) -> None: + import time + + _claim(store, "c1", "the review gate is load-bearing") + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + hot_memory.mark_volunteered("s1", "other", pushed_at=time.monotonic()) + out = vc.evaluate_session( + store, + _session(), + config=VolunteerConfig( + threshold=0.0, throttle_seconds=9999.0, max_per_session=99 + ), + ) + assert out is None + + +def test_evaluate_returns_none_below_the_threshold(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + out = vc.evaluate_session( + store, + _session(), + config=VolunteerConfig(threshold=1.01, throttle_seconds=0.0), + ) + assert out is None + + +def test_evaluate_returns_none_when_nothing_matches(store: KBStore) -> None: + _claim(store, "c1", "totally unrelated content") + hot_memory.register(session_id="s1", query="zebras", agent="claude-code") + out = vc.evaluate_session( + store, + Session(id="s1", agent="claude-code", task="zebras"), + config=VolunteerConfig(threshold=0.0, throttle_seconds=0.0), + ) + assert out is None + + +# --- drain_pending / enqueue -------------------------------------------- + + +def _offer(session_id: str = "s1", claim_id: str = "c1") -> VolunteerOffer: + return VolunteerOffer( + claim_id=claim_id, relevance=0.9, why="because", session_id=session_id + ) + + +def test_enqueue_then_drain_clears_the_queue() -> None: + vc.enqueue_offer(_offer()) + assert [o.claim_id for o in vc.drain_pending("s1")] == ["c1"] + assert vc.drain_pending("s1") == [] + + +def test_drain_with_no_clear_peeks() -> None: + vc.enqueue_offer(_offer()) + assert len(vc.drain_pending("s1", clear=False)) == 1 + assert len(vc.drain_pending("s1", clear=False)) == 1 + + +def test_offer_to_dict_shape() -> None: + assert _offer().to_dict() == { + "claim_id": "c1", + "relevance": 0.9, + "why": "because", + "session_id": "s1", + } + + +# --- MCP push ----------------------------------------------------------- + + +class _FakeSession: + def __init__(self, *, boom: bool = False) -> None: + self.sent: list[Any] = [] + self.boom = boom + self.done = threading.Event() + + async def send_notification(self, note: Any) -> None: + try: + if self.boom: + raise RuntimeError("channel closed") + self.sent.append(note) + finally: + self.done.set() + + +@pytest.fixture +def running_loop() -> Any: + loop = asyncio.new_event_loop() + thread = threading.Thread(target=loop.run_forever, daemon=True) + thread.start() + yield loop + loop.call_soon_threadsafe(loop.stop) + thread.join(timeout=2) + loop.close() + + +def test_mcp_push_is_skipped_without_a_registered_channel() -> None: + vc._maybe_mcp_push(_offer()) # must not raise + + +def test_mcp_push_sends_a_notification(running_loop: Any) -> None: + session = _FakeSession() + vc.register_mcp_push("s1", session, running_loop) # type: ignore[arg-type] + vc._maybe_mcp_push(_offer()) + assert session.done.wait(timeout=5) + assert len(session.sent) == 1 + assert session.sent[0].method == "kb.volunteer_context" + assert session.sent[0].params is not None + + +def _wait_for_log(caplog: pytest.LogCaptureFixture, needle: str) -> bool: + """The push runs on another loop; the log lands after our event fires.""" + import time + + deadline = time.monotonic() + 5 + while time.monotonic() < deadline: + if any(needle in r.getMessage() for r in caplog.records): + return True + time.sleep(0.02) + return False + + +def test_mcp_push_logs_and_swallows_a_send_failure( + running_loop: Any, caplog: pytest.LogCaptureFixture +) -> None: + session = _FakeSession(boom=True) + vc.register_mcp_push("s1", session, running_loop) # type: ignore[arg-type] + with caplog.at_level("ERROR"): + vc._maybe_mcp_push(_offer()) + assert session.done.wait(timeout=5) + # best-effort: a dead channel must not propagate into the caller + assert _wait_for_log(caplog, "push failed") + + +def test_mcp_push_logs_when_the_loop_is_gone( + caplog: pytest.LogCaptureFixture, +) -> None: + dead = asyncio.new_event_loop() + dead.close() + vc.register_mcp_push("s1", _FakeSession(), dead) # type: ignore[arg-type] + with caplog.at_level("ERROR"): + vc._maybe_mcp_push(_offer()) + assert _wait_for_log(caplog, "no event loop") + + +# --- session lifecycle -------------------------------------------------- + + +def test_on_session_start_is_a_noop_when_disabled(store: KBStore) -> None: + store.config_path.write_text( + "volunteer:\n enabled: false\n", encoding="utf-8" + ) + vc.on_session_start(store, _session()) + assert hot_memory.get("s1") is None + + +def test_on_session_start_is_a_noop_without_a_task(store: KBStore) -> None: + vc.on_session_start(store, _session(task=None)) + assert hot_memory.get("s1") is None + + +def test_on_session_start_registers_and_watches(store: KBStore) -> None: + store.config_path.write_text( + "volunteer:\n poll_interval_seconds: 30\n", encoding="utf-8" + ) + _claim(store, "c1", "the review gate is load-bearing") + store.put_session(_session()) + vc.on_session_start(store, _session()) + assert hot_memory.get("s1") is not None + assert "s1" in vc._watch_threads + vc.on_session_end("s1") + + +def test_on_session_start_logs_an_evaluation_failure( + store: KBStore, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + def _boom(*_a: Any, **_k: Any) -> Any: + raise RuntimeError("retrieval exploded") + + store.config_path.write_text( + "volunteer:\n poll_interval_seconds: 30\n", encoding="utf-8" + ) + monkeypatch.setattr(vc, "evaluate_session", _boom) + with caplog.at_level("ERROR"): + vc.on_session_start(store, _session()) + assert _wait_for_log(caplog, "initial volunteer evaluation failed") + vc.on_session_end("s1") + + +def test_on_session_end_clears_everything(store: KBStore) -> None: + hot_memory.register(session_id="s1", query="q", agent="a") + vc.enqueue_offer(_offer()) + vc.register_mcp_push("s1", _FakeSession(), asyncio.new_event_loop()) # type: ignore[arg-type] + vc.on_session_end("s1") + assert hot_memory.get("s1") is None + assert vc.drain_pending("s1", clear=False) == [] + with vc._state_lock: + assert "s1" not in vc._mcp_push + + +def test_start_watch_does_not_start_a_second_thread(store: KBStore) -> None: + cfg = VolunteerConfig(poll_interval_seconds=30.0) + hot_memory.register(session_id="s1", query="q", agent="a") + store.put_session(_session()) + vc._start_watch(store, "s1", cfg) + first = vc._watch_threads["s1"] + vc._start_watch(store, "s1", cfg) + assert vc._watch_threads["s1"] is first + vc.on_session_end("s1") + + +def test_watch_loop_logs_an_evaluation_failure( + store: KBStore, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + seen = threading.Event() + + def _boom(*_a: Any, **_k: Any) -> Any: + seen.set() + raise RuntimeError("watch exploded") + + monkeypatch.setattr(vc, "evaluate_session", _boom) + hot_memory.register(session_id="s1", query="q", agent="a") + store.put_session(_session()) + with caplog.at_level("ERROR"): + vc._start_watch(store, "s1", VolunteerConfig(poll_interval_seconds=30.0)) + assert seen.wait(timeout=5) + hot_memory.unregister("s1") + vc._watch_threads["s1"].join(timeout=5) + assert _wait_for_log(caplog, "volunteer watch failed") + vc._watch_threads.pop("s1", None) + + +def test_watch_loop_enqueues_an_offer_it_finds(store: KBStore) -> None: + import time + + _claim(store, "c1", "the review gate is load-bearing") + store.put_session(_session()) + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + vc._start_watch( + store, + "s1", + VolunteerConfig( + threshold=0.0, throttle_seconds=0.0, poll_interval_seconds=30.0 + ), + ) + deadline = time.monotonic() + 5 + while time.monotonic() < deadline: + if vc.drain_pending("s1", clear=False): + break + time.sleep(0.02) + assert [o.claim_id for o in vc.drain_pending("s1")] == ["c1"] + vc.on_session_end("s1") + + +def test_watch_loop_exits_immediately_without_hot_memory(store: KBStore) -> None: + vc._start_watch(store, "no-such-session", VolunteerConfig()) + thread = vc._watch_threads["no-such-session"] + thread.join(timeout=5) + assert not thread.is_alive() + vc._watch_threads.pop("no-such-session", None) + + +# --- evaluate_now ------------------------------------------------------- + + +def test_evaluate_now_enqueues_the_offer(store: KBStore) -> None: + _claim(store, "c1", "the review gate is load-bearing") + store.put_session(_session()) + hot_memory.register(session_id="s1", query="review gate", agent="claude-code") + store.config_path.write_text( + "volunteer:\n threshold: 0.0\n throttle_seconds: 0\n", encoding="utf-8" + ) + offer = vc.evaluate_now(store, "s1") + assert offer is not None + assert [o.claim_id for o in vc.drain_pending("s1")] == ["c1"] + + +def test_evaluate_now_returns_none_when_nothing_qualifies(store: KBStore) -> None: + store.put_session(_session()) + assert vc.evaluate_now(store, "s1") is None diff --git a/tests/test_worthiness.py b/tests/test_worthiness.py index 20eddcc2..7d75f7c4 100644 --- a/tests/test_worthiness.py +++ b/tests/test_worthiness.py @@ -96,6 +96,15 @@ def test_load_config_parses_block(store: KBStore) -> None: assert cfg.action == "reject" +def test_load_config_preserves_zero_min_score(store: KBStore) -> None: + # a configured min_score of 0 is a distinct, meaningful setting — compute + # worthiness but never let the threshold defer a claim, unlike scorer: off + # which computes nothing. it must not be swallowed by the fallback and + # silently become DEFAULT_MIN_SCORE. + _write_worthiness_config(store, " min_score: 0\n") + assert worthiness.load_config(store).min_score == 0.0 + + def test_get_scorer_selects_backend(store: KBStore) -> None: default = worthiness.get_scorer(worthiness.WorthinessConfig()) assert isinstance(default, worthiness.HeuristicScorer)