|
| 1 | +"""Recurrence guard: every verdict that can prove absence must be able to see a rebuild. |
| 2 | +
|
| 3 | +The absence chokepoint in ``server.py`` is deliberately **generic** — it fires on |
| 4 | +any ``_meta.verdict`` dict and mints a citable ``absent:<sha>`` ref whenever the |
| 5 | +state is ``absent``. That generality is what makes the contract cheap to extend, |
| 6 | +and it is also what makes it easy to break: a tool only has to *emit* a verdict |
| 7 | +to start minting absence proofs, whether or not anyone wired it for the v1.108.168 |
| 8 | +rebuilding rule. |
| 9 | +
|
| 10 | +That has now gone wrong twice for the same structural reason: |
| 11 | +
|
| 12 | +* v1.108.168 wired ``search_symbols`` and ``get_ranked_context`` and missed |
| 13 | + ``search_text`` (fixed in v1.108.169). |
| 14 | +* The index-aware wrappers ``symbol_verdict_for_index`` / ``file_verdict_for_index`` |
| 15 | + accept an index but delegate to builders that have no ``index_changed`` parameter |
| 16 | + at all, so the file/symbol tools have never been able to refuse an absence claim |
| 17 | + over an index being rewritten. |
| 18 | +
|
| 19 | +These tests encode the invariant itself rather than re-checking the instances, so |
| 20 | +the *next* verdict-emitting tool cannot quietly join the list. |
| 21 | +""" |
| 22 | + |
| 23 | +import ast |
| 24 | +import pathlib |
| 25 | + |
| 26 | +import pytest |
| 27 | + |
| 28 | +SRC = pathlib.Path(__file__).resolve().parents[1] / "src" / "jcodemunch_mcp" |
| 29 | +TOOLS = SRC / "tools" |
| 30 | +VERDICT_MODULE = SRC / "retrieval" / "verdict.py" |
| 31 | + |
| 32 | +# Verdict builders that take a live index and can therefore, in principle, |
| 33 | +# re-stat it to detect a rebuild underneath the scan. |
| 34 | +INDEX_AWARE_WRAPPERS = {"symbol_verdict_for_index", "file_verdict_for_index"} |
| 35 | + |
| 36 | +# KNOWN GAP — a ratchet, not an exemption. CLOSED in v1.108.170. |
| 37 | +# |
| 38 | +# Both wrappers now thread `index_changed` into their builders, so the file and |
| 39 | +# symbol tools refuse an absence claim over an index being rewritten just as the |
| 40 | +# search tools do. The set is kept (empty) rather than deleted: it is the |
| 41 | +# mechanism that forces a future gap to be declared here instead of shipping |
| 42 | +# silently, and `test_index_aware_wrapper_gap_does_not_widen` fails if a listed |
| 43 | +# wrapper turns out to be wired — so a stale entry can never rot into an |
| 44 | +# exemption. |
| 45 | +KNOWN_UNWIRED_WRAPPERS: set[str] = set() |
| 46 | + |
| 47 | + |
| 48 | +def _call_name(node: ast.Call) -> str: |
| 49 | + f = node.func |
| 50 | + if isinstance(f, ast.Name): |
| 51 | + return f.id |
| 52 | + if isinstance(f, ast.Attribute): |
| 53 | + return f.attr |
| 54 | + return "" |
| 55 | + |
| 56 | + |
| 57 | +def _calls_in(path: pathlib.Path): |
| 58 | + tree = ast.parse(path.read_text(encoding="utf-8")) |
| 59 | + for node in ast.walk(tree): |
| 60 | + if isinstance(node, ast.Call): |
| 61 | + yield node |
| 62 | + |
| 63 | + |
| 64 | +def _aliases_for(path: pathlib.Path, target: str) -> set[str]: |
| 65 | + """Local names bound to ``target`` in this module. |
| 66 | +
|
| 67 | + Every tool imports it lazily and renames it |
| 68 | + (``from ..retrieval.verdict import build_verdict as _build_verdict``), so a |
| 69 | + plain name match finds nothing and the scan passes vacuously. That is the |
| 70 | + exact failure `test_the_guard_is_not_vacuous` exists to catch. |
| 71 | + """ |
| 72 | + names = {target} |
| 73 | + tree = ast.parse(path.read_text(encoding="utf-8")) |
| 74 | + for node in ast.walk(tree): |
| 75 | + if isinstance(node, ast.ImportFrom): |
| 76 | + for a in node.names: |
| 77 | + if a.name == target: |
| 78 | + names.add(a.asname or a.name) |
| 79 | + return names |
| 80 | + |
| 81 | + |
| 82 | +def _verdict_calls(path: pathlib.Path): |
| 83 | + names = _aliases_for(path, "build_verdict") |
| 84 | + for call in _calls_in(path): |
| 85 | + if _call_name(call) in names: |
| 86 | + yield call |
| 87 | + |
| 88 | + |
| 89 | +def _has_kwarg(node: ast.Call, name: str) -> bool: |
| 90 | + return any(kw.arg == name for kw in node.keywords) |
| 91 | + |
| 92 | + |
| 93 | +def _tool_modules(): |
| 94 | + return sorted(p for p in TOOLS.glob("*.py") if p.name != "__init__.py") |
| 95 | + |
| 96 | + |
| 97 | +def test_every_build_verdict_call_passes_index_changed(): |
| 98 | + """A tool that builds a verdict must say whether the index moved under it. |
| 99 | +
|
| 100 | + Without this, a zero-result scan reaches `absent` and the generic chokepoint |
| 101 | + mints a citable ref, with the 5th refusal rule structurally unable to fire. |
| 102 | + """ |
| 103 | + offenders = [] |
| 104 | + for path in _tool_modules(): |
| 105 | + for call in _verdict_calls(path): |
| 106 | + if not _has_kwarg(call, "index_changed"): |
| 107 | + offenders.append(f"{path.name}:{call.lineno}") |
| 108 | + assert not offenders, ( |
| 109 | + "build_verdict() called without index_changed= at: " |
| 110 | + + ", ".join(offenders) |
| 111 | + + ". These can mint absence evidence over an index being rewritten. " |
| 112 | + "Pass index_changed=index_changed_since_load(index)." |
| 113 | + ) |
| 114 | + |
| 115 | + |
| 116 | +def test_build_verdict_still_accepts_index_changed(): |
| 117 | + """Guard the guard: if the parameter is renamed, the test above goes vacuous.""" |
| 118 | + import inspect |
| 119 | + |
| 120 | + from jcodemunch_mcp.retrieval.verdict import build_verdict |
| 121 | + |
| 122 | + assert "index_changed" in inspect.signature(build_verdict).parameters |
| 123 | + |
| 124 | + |
| 125 | +def test_the_guard_is_not_vacuous(): |
| 126 | + """At least one real build_verdict call must exist, or the scan proves nothing.""" |
| 127 | + found = sum(1 for path in _tool_modules() for _ in _verdict_calls(path)) |
| 128 | + assert found >= 3, f"expected the known verdict-emitting tools, found {found}" |
| 129 | + |
| 130 | + |
| 131 | +@pytest.mark.parametrize("wrapper", sorted(INDEX_AWARE_WRAPPERS)) |
| 132 | +def test_index_aware_wrapper_gap_does_not_widen(wrapper): |
| 133 | + """The wrappers take an index; each must either use it or be a tracked gap.""" |
| 134 | + tree = ast.parse(VERDICT_MODULE.read_text(encoding="utf-8")) |
| 135 | + fn = next( |
| 136 | + ( |
| 137 | + n |
| 138 | + for n in ast.walk(tree) |
| 139 | + if isinstance(n, ast.FunctionDef) and n.name == wrapper |
| 140 | + ), |
| 141 | + None, |
| 142 | + ) |
| 143 | + assert fn is not None, f"{wrapper} not found — update this guard" |
| 144 | + |
| 145 | + wired = any( |
| 146 | + _has_kwarg(call, "index_changed") |
| 147 | + for call in ast.walk(fn) |
| 148 | + if isinstance(call, ast.Call) |
| 149 | + ) |
| 150 | + if wrapper in KNOWN_UNWIRED_WRAPPERS: |
| 151 | + assert not wired, ( |
| 152 | + f"{wrapper} now passes index_changed — remove it from " |
| 153 | + "KNOWN_UNWIRED_WRAPPERS so the ratchet tightens." |
| 154 | + ) |
| 155 | + else: |
| 156 | + assert wired, ( |
| 157 | + f"{wrapper} takes an index but never passes index_changed, so the " |
| 158 | + "tools using it can mint absence evidence over a rebuilding index." |
| 159 | + ) |
| 160 | + |
| 161 | + |
| 162 | +def test_known_gap_set_is_a_subset_of_real_wrappers(): |
| 163 | + """A stale entry would silently exempt a wrapper that no longer exists.""" |
| 164 | + assert KNOWN_UNWIRED_WRAPPERS <= INDEX_AWARE_WRAPPERS |
| 165 | + |
| 166 | + |
| 167 | +def test_wrapper_verdicts_really_can_reach_absent(): |
| 168 | + """Pins why the gap matters: these states feed note_absence's `absent` gate.""" |
| 169 | + from jcodemunch_mcp.retrieval.verdict import ( |
| 170 | + build_file_verdict, |
| 171 | + build_symbol_verdict, |
| 172 | + ) |
| 173 | + |
| 174 | + assert build_symbol_verdict(found_count=0, requested_id="a.py::f")["state"] == "absent" |
| 175 | + assert build_file_verdict(present=False, requested_path="a.py")["state"] == "absent" |
0 commit comments