From d49e4ad106b04df48d1858ac97ef880be06fa91f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 18:27:05 -0400 Subject: [PATCH 1/7] refactor(tests): derive the two duplicated test helpers into tests/helpers.py (#4276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Derive-first, ahead of the tests shape pay-down. An AST pass over all 2628 definitions under `tests/` found exactly three helper bodies duplicated across files. Two are real copies and are consolidated here; the third is not, and is left alone. `need_tools(*tools)` — byte-identical in three hook-test modules, each skipping when `jq`/`awk`/`git` is absent from PATH. Now snippet #4277. The `import shutil` each file carried existed only to serve it and goes with it. `rule_row(rule_id)` — byte-identical in two integration modules, reading a Rule back through a SEPARATE session so the assertion is about what Postgres holds rather than what the writing session's identity map remembers. Now snippet #4278. Its imports are lazy, because `tests/helpers.py` is imported by unit tests that have no database, which is the same reason `plugin_config` defers its service imports. NOT consolidated: `_side(uid, k, d="")` in test_services_plugin_context and test_write_path_trigger. The body is identical but it closes over a module-local `stored` dict, so it is not self-contained and "moving" it would mean inventing a parameter neither call site wants. That is convention plumbing — two tests independently writing the same one-line side_effect — and it is dismissed in the ledger rather than lifted. Worth recording for the next pass: a repeated NAME is not a family. `_row` is defined in five modules and only two of those share a body; the other three (test_list_rows_brief, test_calibration_stamp, test_shape_ledger) build entirely different objects. Grouping by name would have consolidated three things that have nothing in common. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- tests/helpers.py | 52 +++++++++++++++++++++ tests/test_contract_around_the_change.py | 15 ++---- tests/test_hook_json_reader.py | 15 ++---- tests/test_integration_rule_move.py | 13 ++---- tests/test_integration_rule_verification.py | 23 ++++----- tests/test_session_slippage_readout.py | 17 +++---- 6 files changed, 81 insertions(+), 54 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 8bc80f1..f3b6090 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -418,3 +418,55 @@ def writepath_cfg(**over): } cfg.update(over) return cfg + + +def need_tools(*tools): + """Skip unless every named executable is on PATH. + + For the hook tests, which drive `plugin/hooks/*.sh` through a real shell + and therefore depend on whatever that shell reaches for — `jq`, `awk`, + `git`, `curl`. Those are present on the CI image and routinely absent from + a developer's box, and the honest answer there is "not exercised", not a + failure: a red test would say the hook is broken when nothing about the + hook was ever run. + + A skip rather than a stub on purpose. Stubbing `jq` would test the stub — + these tests exist precisely because the shell's behaviour is the thing in + question (#2932), so anything short of the real tool proves nothing. + + Consolidated 2026-09-21 from three byte-identical copies in + test_contract_around_the_change, test_hook_json_reader and + test_session_slippage_readout. + """ + import shutil + + import pytest + + for t in tools: + if shutil.which(t) is None: + pytest.skip(f"hook runtime tool {t!r} not installed") + + +async def rule_row(rule_id: int): + """Read a Rule straight from Postgres, outside whatever session the code + under test used. + + THE POINT IS THE SEPARATE SESSION. An integration test that asserts on the + object the service just returned is asserting on that session's identity + map, which can hold a value the database never accepted — a column the + write never reached, a default the ORM supplied rather than the schema. + Opening a new session forces a real read and is the only way `verify_with` + being NULL rather than "" is distinguishable at all (milestone 312). + + Import is lazy because this module is imported by unit tests that have no + database and must not pay for one — the same reason `plugin_config` defers + its service imports. + + Consolidated 2026-09-21 from two byte-identical copies in + test_integration_rule_move and test_integration_rule_verification. + """ + from scribe.models import async_session + from scribe.models.rulebook import Rule + + async with async_session() as s: + return await s.get(Rule, rule_id) diff --git a/tests/test_contract_around_the_change.py b/tests/test_contract_around_the_change.py index 839ae69..919b3b1 100644 --- a/tests/test_contract_around_the_change.py +++ b/tests/test_contract_around_the_change.py @@ -34,24 +34,19 @@ A FIXTURE REPO, NEVER THIS ONE. Asserting against Scribe's own files would make the test a description of today's tree, failing the next time someone renames something (rule 115's reasoning, one floor down). """ -import shutil import subprocess from pathlib import Path import pytest +from tests.helpers import need_tools + DEFS = Path(__file__).resolve().parents[1] / "plugin" / "hooks" / "scribe_defs.sh" HOOK = Path(__file__).resolve().parents[1] / "plugin" / "hooks" / "scribe_prior_art.sh" -def _need(*tools): - for t in tools: - if shutil.which(t) is None: - pytest.skip(f"hook runtime tool {t!r} not installed") - - def run(script: str) -> str: - _need("bash", "awk", "git", "grep", "sed") + need_tools("bash", "awk", "git", "grep", "sed") r = subprocess.run( ["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'], capture_output=True, text=True, @@ -63,7 +58,7 @@ def run(script: str) -> str: @pytest.fixture() def repo(tmp_path): """A small git repo: a definition with a reader, and one without.""" - _need("git") + need_tools("git") # `other` lives in lib.py BESIDE widget, and nothing references it. That # placement is the point of the no-readers case: putting it in its own # file would leave that file as its reader, since only the file being @@ -248,6 +243,6 @@ def test_the_hook_asks_the_contract_question_first(): def test_the_hook_is_still_shell_valid(): - _need("bash") + need_tools("bash") subprocess.run(["bash", "-n", str(HOOK)], check=True) subprocess.run(["bash", "-n", str(DEFS)], check=True) diff --git a/tests/test_hook_json_reader.py b/tests/test_hook_json_reader.py index fcdc371..c94b16c 100644 --- a/tests/test_hook_json_reader.py +++ b/tests/test_hook_json_reader.py @@ -22,25 +22,20 @@ from __future__ import annotations import json import re -import shutil import subprocess from pathlib import Path from urllib.parse import quote import pytest +from tests.helpers import need_tools + HOOKS = Path(__file__).resolve().parents[1] / "plugin" / "hooks" DEFS = HOOKS / "scribe_defs.sh" PARSER = HOOKS / "scribe_json.awk" TURN = HOOKS / "scribe_turn.awk" -def _need(*tools): - for t in tools: - if shutil.which(t) is None: - pytest.skip(f"hook runtime tool {t!r} not installed") - - def sh(script: str, stdin: str = "") -> str: """Run a snippet with scribe_defs.sh sourced, under the hooks' own flags. @@ -50,7 +45,7 @@ def sh(script: str, stdin: str = "") -> str: quietly normalise the characters it exists to check: the first version of this file did, and reported a round-trip failure that was entirely its own. """ - _need("bash", "awk") + need_tools("bash", "awk") r = subprocess.run( ["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'], input=stdin.encode(), capture_output=True, @@ -60,7 +55,7 @@ def sh(script: str, stdin: str = "") -> str: def flat(doc: str, mode: str = "whole") -> list[tuple[str, str, str]]: - _need("awk") + need_tools("awk") r = subprocess.run(["awk", "-v", f"mode={mode}", "-f", str(PARSER)], input=doc.encode(), capture_output=True) assert r.returncode == 0, r.stderr.decode() @@ -260,7 +255,7 @@ def test_urlenc_and_the_envelope_can_fail(): # The transcript turn, which is the largest thing jq was doing here. def _turn(records: list[dict]) -> dict: - _need("awk") + need_tools("awk") doc = "\n".join(json.dumps(r) for r in records) + "\n" p1 = subprocess.run(["awk", "-v", "mode=lines", "-f", str(PARSER)], input=doc, capture_output=True, text=True) diff --git a/tests/test_integration_rule_move.py b/tests/test_integration_rule_move.py index d37c4f7..93ef55b 100644 --- a/tests/test_integration_rule_move.py +++ b/tests/test_integration_rule_move.py @@ -20,7 +20,7 @@ from scribe.models.rulebook import Rule from scribe.services import canonical_systems as canonical_svc from scribe.services import rule_versions as rv_svc from scribe.services import rulebooks as rulebooks_svc -from tests.helpers import ensure_user +from tests.helpers import ensure_user, rule_row pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")] @@ -69,17 +69,12 @@ async def homes(): return ids -async def _row(rule_id: int) -> Rule: - async with async_session() as s: - return await s.get(Rule, rule_id) - - async def test_a_project_rule_becomes_global_and_keeps_everything(homes): owner, rule_id = homes["owner"], homes["rule"] moved = await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"]) assert moved.id == rule_id - row = await _row(rule_id) + row = await rule_row(rule_id) assert (row.topic_id, row.project_id) == (homes["topic"], None) assert len(await rv_svc.list_versions(rule_id)) == 1, "the move must not drop history" areas = await rulebooks_svc.list_rule_systems([rule_id]) @@ -100,7 +95,7 @@ async def test_a_global_rule_can_move_onto_a_project(homes): owner, rule_id = homes["owner"], homes["rule"] await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"]) await rulebooks_svc.move_rule(rule_id, owner, project_id=homes["other"]) - row = await _row(rule_id) + row = await rule_row(rule_id) assert (row.topic_id, row.project_id) == (None, homes["other"]) @@ -126,7 +121,7 @@ async def test_refusals_happen_before_anything_is_written(homes): with pytest.raises(ValueError, match=f"rule {clash.id}"): await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"]) - row = await _row(rule_id) + row = await rule_row(rule_id) assert (row.topic_id, row.project_id) == (None, homes["home"]) diff --git a/tests/test_integration_rule_verification.py b/tests/test_integration_rule_verification.py index 9e94f52..2c3e01e 100644 --- a/tests/test_integration_rule_verification.py +++ b/tests/test_integration_rule_verification.py @@ -23,7 +23,7 @@ import pytest_asyncio from scribe.models import async_session from scribe.models.rulebook import Rule from scribe.services import rulebooks as rulebooks_svc -from tests.helpers import ensure_user +from tests.helpers import ensure_user, rule_row pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")] @@ -57,13 +57,8 @@ async def constraint(): return {"uid": uid, "rule": rule.id} -async def _row(rule_id: int) -> Rule: - async with async_session() as s: - return await s.get(Rule, rule_id) - - async def test_the_check_and_its_expiry_persist(constraint): - row = await _row(constraint["rule"]) + row = await rule_row(constraint["rule"]) assert row.verify_with == "read the workflow's shell setting" assert row.expires_when == "the runner can be given a bash shell" assert row.verified_at is not None @@ -79,7 +74,7 @@ async def test_an_empty_string_becomes_null_not_an_empty_column(constraint): await rulebooks_svc.update_rule( constraint["rule"], constraint["uid"], verify_with="", expires_when="", ) - row = await _row(constraint["rule"]) + row = await rule_row(constraint["rule"]) assert row.verify_with is None assert row.expires_when is None @@ -89,7 +84,7 @@ async def test_naming_a_field_in_clear_empties_it(constraint): await rulebooks_svc.update_rule( constraint["rule"], constraint["uid"], clear=["verify_with"], ) - row = await _row(constraint["rule"]) + row = await rule_row(constraint["rule"]) assert row.verify_with is None # expires_when was NOT named, so it survives — clearing is per-field, and # a caller retiring one field must not lose the others. @@ -107,7 +102,7 @@ async def test_rewording_the_check_drops_the_stamp(constraint): constraint["rule"], constraint["uid"], verify_with="read the runner's container shell, not the image's", ) - row = await _row(constraint["rule"]) + row = await rule_row(constraint["rule"]) assert row.verified_at is None @@ -115,7 +110,7 @@ async def test_clearing_the_check_drops_the_stamp(constraint): await rulebooks_svc.update_rule( constraint["rule"], constraint["uid"], clear=["verify_with"], ) - row = await _row(constraint["rule"]) + row = await rule_row(constraint["rule"]) assert row.verified_at is None @@ -132,7 +127,7 @@ async def test_editing_anything_else_leaves_the_stamp_alone(constraint): "applies to the build, not to `run:`.", expires_when="the runner grows a shell setting", ) - row = await _row(constraint["rule"]) + row = await rule_row(constraint["rule"]) assert row.verified_at is not None assert row.why.startswith("act_runner picks the shell") @@ -221,11 +216,11 @@ async def test_a_failed_check_writes_nothing(rulebook_of_three): not in a special condition — it is WRONG. Recording the failure would let it sit there being false with the sweep satisfied that someone looked. """ - before = await _row(rulebook_of_three["stale"]) + before = await rule_row(rulebook_of_three["stale"]) await rulebooks_svc.mark_rule_verified( rulebook_of_three["stale"], rulebook_of_three["uid"], still_true=False, ) - after = await _row(rulebook_of_three["stale"]) + after = await rule_row(rulebook_of_three["stale"]) assert after.verified_at == before.verified_at diff --git a/tests/test_session_slippage_readout.py b/tests/test_session_slippage_readout.py index 73b656d..29f0a15 100644 --- a/tests/test_session_slippage_readout.py +++ b/tests/test_session_slippage_readout.py @@ -32,13 +32,14 @@ from __future__ import annotations import json import os -import shutil import subprocess import time from pathlib import Path import pytest +from tests.helpers import need_tools + ROOT = Path(__file__).resolve().parents[1] HOOKS = ROOT / "plugin" / "hooks" DEFS = HOOKS / "scribe_defs.sh" @@ -47,14 +48,8 @@ RECORDER = HOOKS / "scribe_record_outcome.sh" HOOKS_JSON = HOOKS / "hooks.json" -def _need(*tools): - for t in tools: - if shutil.which(t) is None: - pytest.skip(f"hook runtime tool {t!r} not installed") - - def sh(script: str) -> str: - _need("bash", "awk") + need_tools("bash", "awk") r = subprocess.run( ["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'], capture_output=True, text=True, timeout=30, @@ -173,7 +168,7 @@ def test_a_session_no_rule_touched_says_nothing_at_all(tmp_path): # ── The hook that carries it ────────────────────────────────────────────── def run_precompact(event: dict, tmpdir: Path) -> subprocess.CompletedProcess: - _need("bash") + need_tools("bash") env = dict(os.environ) env["TMPDIR"] = str(tmpdir) return subprocess.run(["bash", str(PRECOMPACT)], input=json.dumps(event), @@ -210,7 +205,7 @@ def test_the_compaction_hook_never_emits_a_json_envelope(tmp_path): # ── The recorder that makes `acted` mean anything ───────────────────────── def run_recorder(event: dict, tmpdir: Path) -> subprocess.CompletedProcess: - _need("bash") + need_tools("bash") env = {"PATH": os.environ["PATH"], "HOME": str(tmpdir), "TMPDIR": str(tmpdir)} return subprocess.run(["bash", str(RECORDER)], input=json.dumps(event), capture_output=True, text=True, timeout=30, env=env) @@ -257,7 +252,7 @@ def test_the_recorder_is_registered_on_the_rule_outcome_tool(): def test_the_recorder_is_shell_valid(): - _need("bash") + need_tools("bash") subprocess.run(["bash", "-n", str(RECORDER)], check=True) -- 2.54.0 From 91cde6c3e4ca21e7a0add270e614243d3ebf6445 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 20:35:55 -0400 Subject: [PATCH 2/7] fix(shapes): let a measured meaning-miss silence a divergence prompt (#4208) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4204 gave the divergence check a structural gate, which silenced one of the five false prompts it was filed for. The other four are `def` helpers in a directory whose canon is an `async def` service unit. No refinement of `shape_form` reaches them: they differ from #2793's acceptance case — a hand-rolled sync `confirmDanger` where an async confirm helper is canon — only by the JOB they do, and a signature does not carry a job. The proposer's semantic arm already reads bodies per symbol, which is the comparison option 2 asked for and was thought to be missing. What it did not do was record its MISSES: a hit became `proposal_basis = "semantic"`, a miss left the row indistinguishable from one nobody had looked at. So `flag_divergence` could ask the proposer "do you agree this is the canon?" but never "did you check, and is it not?". `_semantic_canon` now reports whether an empty answer is evidence, and `flag_divergence` withholds the prompt when it is. The whole risk is in the negative, so only a conclusive miss is stored. A body too thin to embed, a row the per-refresh cap never reached, an arm that threw, and a result set that came back full — and may therefore have hidden the canon behind the limit — all stay "cannot tell" and still ask the question. That is the discipline `FORM_UNKNOWN` already enforces here: not knowing must make a check quieter, never more confident. `_SEMANTIC_LIMIT` is named for that reason; the number is load-bearing, not a tuning knob. No migration: `proposal_basis` is nullable Text with no CHECK constraint (verified in the model and across alembic/versions), so rule 36 does not bite. Nothing can mistake the miss for a proposal either — every reader keys on `proposed_snippet_id` or `proposal_group`, and `confirm_shape_proposals` requires the id non-NULL before it will confirm anything. `_PROPOSER_VERSION` 3 -> 4, per its own contract: rows remember the ruleset they were examined under, and without the bump no already-examined row would ever acquire a miss. Option 1 (widening `kind`) stays closed, on the merits rather than on cost: bucketing density by exact form takes the async canon out of a sync candidate's denominator and silences #2793's acceptance case by the identical mechanism, one layer down. The reasoning is on #4208. Tests: tests/test_divergence_meaning_gate.py pins the report contract, with the truncation case tested hardest — reading a cut-off as a negative would weaken the guard in proportion to how many snippets the operator has. The end-to-end discrimination is in test_integration_shape_classify.py on deliberately the SAME fixture as #2793's acceptance case, so the two runs differ in exactly one thing: whether the arm claims to have looked. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- src/scribe/services/shape_ledger.py | 90 ++++++++++- tests/test_divergence_meaning_gate.py | 182 +++++++++++++++++++++++ tests/test_integration_shape_classify.py | 85 +++++++++++ 3 files changed, 353 insertions(+), 4 deletions(-) create mode 100644 tests/test_divergence_meaning_gate.py diff --git a/src/scribe/services/shape_ledger.py b/src/scribe/services/shape_ledger.py index 4784541..bff44ae 100644 --- a/src/scribe/services/shape_ledger.py +++ b/src/scribe/services/shape_ledger.py @@ -1521,11 +1521,37 @@ _SEMANTIC_CAP = 150 # "both are about migrations"; first live run paired every alembic # upgrade()/downgrade() with an unrelated canon at exactly that band. _SEMANTIC_FLOOR = 0.8 +# How many above-floor hits the semantic arm asks for. Named because the +# NUMBER is load-bearing twice over: it caps the work, and a result set that +# came back short of it is a complete picture of what cleared the floor — +# which is what lets a miss be read as evidence rather than as a cut-off +# (`BASIS_NO_SEMANTIC_MATCH`). +_SEMANTIC_LIMIT = 3 +# The proposer looked at this body, compared it against every canon in its +# language family, and matched none of them above `_SEMANTIC_FLOOR` (#4208). +# +# This is a NEGATIVE RESULT, and it is stored because it is the only evidence +# in the ledger that speaks to what a shape MEANS rather than what it looks +# like. `proposal_basis` otherwise names how a proposal was arrived at; here +# it records that the arm ran and came back empty, with `proposed_snippet_id` +# left NULL. Every reader keys "is there a proposal" on `proposed_snippet_id` +# or `proposal_group`, never on the basis, so this cannot be mistaken for one: +# `list_shapes(proposal=...)` and `confirm_shape_proposals` both filter on the +# id, and the latter requires it non-NULL before it will confirm anything. +# +# It is deliberately NOT written for the two cases that merely look the same: +# a body too thin to compare (`_substance` below the write-path minimum), and +# a row the per-refresh cap never reached. Those are "I cannot tell", and the +# ledger's standing discipline — the one `FORM_UNKNOWN` enforces everywhere +# else — is that not knowing must make a check quieter, never more confident. +BASIS_NO_SEMANTIC_MATCH = "no-semantic-match" # Bump when a basis's rule changes: rows remember the (body, ruleset) they # were examined under, so a tightened rule re-examines everything once. # v3: language-family gate on the sym bases, reference stoplist, semantic # restricted to the shape's own project (#2871). -_PROPOSER_VERSION = 3 +# v4: the semantic arm records its misses as well as its hits (#4208), so +# every already-examined row must be looked at once more to acquire one. +_PROPOSER_VERSION = 4 # Signature resemblance floor, name blanked (difflib ratio) — and a length # floor, because `def NAME():` resembles `def NAME(x):` at 0.95 while saying # nothing; a family shape has parameters to resemble. @@ -1760,8 +1786,29 @@ def _substance(text: str) -> int: async def _semantic_canon( - user_id: int, body: str, allowed: set[int] + user_id: int, body: str, allowed: set[int], *, report: dict | None = None ) -> tuple[int, float] | None: + """The canon this body MEANS, or None. + + `report` is an out-param in the style `semantic_search_notes` already + uses, and it carries the one thing the return value cannot: whether a + None is EVIDENCE. `report["conclusive"] = True` says the arm really + compared this body against the allowed canons and none cleared the floor. + It is left unset whenever the arm could not form an opinion — a body with + too little substance to embed, no allowed canon to compare against, or a + result set that came back full and may therefore have been truncated. + + The truncation case is why `_SEMANTIC_LIMIT` is named. The search returns + the top N above the floor; if it returns fewer than N, N was not binding + and we have seen everything that cleared the floor, so "no allowed canon + among them" is a fact about the corpus. If it returns exactly N, an + allowed canon could be sitting at N+1 and the same silence means nothing. + Reading the second case as the first is how a cut-off becomes a finding. + + Callers must treat a missing key as "cannot tell", never as "no match" — + which is also what makes the existing test double, an `AsyncMock` that + returns None and touches no report, stay correct by default. + """ from scribe.services.embeddings import semantic_search_notes from scribe.services.plugin_context import ( WRITEPATH_DEFAULT_THRESHOLD, WRITEPATH_MIN_CODE_CHARS, concept_query, @@ -1771,13 +1818,15 @@ async def _semantic_canon( return None query = concept_query(body) or body hits = await semantic_search_notes( - user_id, query, limit=3, + user_id, query, limit=_SEMANTIC_LIMIT, threshold=max(WRITEPATH_DEFAULT_THRESHOLD, _SEMANTIC_FLOOR), note_type="snippet", scope="browse", ) for score, note in hits: if int(note.id) in allowed: return int(note.id), round(float(score), 3) + if report is not None and len(hits) < _SEMANTIC_LIMIT: + report["conclusive"] = True return None @@ -1870,16 +1919,29 @@ async def propose_for_repo( row.proposed_sha = "" continue checked += 1 + verdict: dict = {} try: - found = await _semantic_canon(user_id, d[5], semantic_allowed(row.path)) + found = await _semantic_canon( + user_id, d[5], semantic_allowed(row.path), report=verdict, + ) except Exception: logger.warning("semantic proposal failed", exc_info=True) found = None + # An arm that threw formed no opinion. Clearing this is not + # belt-and-braces: a partially-filled report would record a + # failure as a finding about the code. + verdict = {} if found: row.proposed_snippet_id, row.proposal_score = found row.proposal_basis = "semantic" row.proposal_group = None proposed += 1 + elif verdict.get("conclusive"): + # No canon, and the arm is sure of it. Kept as the row's basis + # with `proposed_snippet_id` still NULL, so it reads as "asked + # and answered" rather than "not asked" — the distinction + # `flag_divergence` needs and could not previously make. + row.proposal_basis = BASIS_NO_SEMANTIC_MATCH await session.commit() return {"examined": examined, "proposed": proposed, "semantic_checked": checked} @@ -2435,6 +2497,26 @@ async def flag_divergence(project_id: int, *, since: datetime | None) -> int: continue if r.proposed_snippet_id == dom[0]: continue # the proposer already says "instance of the canon" + # ...and the converse, which is the only evidence here that + # is about MEANING rather than shape (#4208). The four false + # prompts #4204 left standing are callables in a directory of + # callables: at the signature level they are indistinguishable + # from #2793's acceptance case, a sync `confirmDanger` beside + # an async confirm canon, and no refinement of `shape_form` + # ever separates them — a registry accessor and a service unit + # differ by the JOB they do, which a signature does not carry. + # + # The proposer does read bodies, and when its semantic arm + # compared this one against every canon in its language family + # and matched none of them, that is a positive finding that + # this shape is not the canon's work. Urging the canon anyway + # would be asserting over a measurement we already hold. + # + # Only the conclusive miss is stored, so an unexamined row and + # a body too thin to embed still ask the question rather than + # being quietly excused. + if r.proposal_basis == BASIS_NO_SEMANTIC_MATCH: + continue # The same structural test the write-time check applies # (#4204). The sweep and the hook must agree about what counts # as divergence, or an audit contradicts the line the writer diff --git a/tests/test_divergence_meaning_gate.py b/tests/test_divergence_meaning_gate.py new file mode 100644 index 0000000..0e18e28 --- /dev/null +++ b/tests/test_divergence_meaning_gate.py @@ -0,0 +1,182 @@ +"""A divergence prompt may be silenced by MEANING, never by silence (#4208). + +WHAT THIS IS ABOUT. #4204 gave the divergence check a structural gate: a canon +is only urged on a shape whose form could plausibly BE it. That silenced one of +the five false prompts it was filed for. The other four are `def` helpers in a +directory whose canon is an `async def` service unit — callables beside a +callable — and no refinement of `shape_form` ever separates them, because they +differ from #2793's acceptance case (a hand-rolled sync `confirmDanger` where +an async confirm helper is canon) only by the JOB they do. A signature does not +carry a job. + +So the lever has to be meaning, and the ledger already holds one reading of it: +the proposer's semantic arm embeds each definition's own BODY against canon. +What it did not do was record its misses. A hit became `proposal_basis = +"semantic"`; a miss left the row indistinguishable from a row nobody had looked +at yet. `flag_divergence` could therefore ask the proposer "do you agree this is +the canon?" but never "did you check, and did you find it is not?". + +THE WHOLE RISK IS IN THE NEGATIVE. A miss is only evidence if the arm actually +formed an opinion, and there are three ways for it to come back empty that look +identical from the outside: + + body too thin to embed -> no opinion + no allowed canon to test -> no opinion + result set was truncated -> no opinion (the canon may be at N+1) + compared, nothing above the floor -> EVIDENCE + +Only the last may silence a prompt. Reading any of the others as a negative is +how "I cannot tell" turns into "I checked" — the exact failure #4204 was opened +on, and the one `FORM_UNKNOWN` already guards against everywhere else in this +module: not knowing must make a check QUIETER, never more confident. + +These tests pin the report contract that carries that distinction. The +end-to-end behaviour — a conclusive miss silencing a real prompt while #2793's +acceptance case still raises — is in +tests/test_integration_shape_classify.py, because it needs real rows. +""" +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest + +from scribe.services.shape_ledger import ( + _SEMANTIC_LIMIT, BASIS_NO_SEMANTIC_MATCH, _semantic_canon, +) + +# Comfortably over WRITEPATH_MIN_CODE_CHARS (48 non-whitespace characters), so +# these tests exercise the comparison rather than the substance guard. One of +# #4204's four survivors, quoted rather than invented. +BODY = ( + "def is_registered(source: str) -> bool:\n" + " return source in _REGISTRY and _REGISTRY[source].enabled\n" +) +TOO_THIN = "def f():\n pass\n" + +CANON = 2860 # the allowed canon, as a caller would pass it +OTHER = 9999 # a snippet that is not in the allowed set + + +class _FakeNote: + """Only `.id` is read off a hit.""" + + def __init__(self, note_id: int) -> None: + self.id = note_id + + +def _hits(*hits: tuple[float, int]) -> AsyncMock: + return AsyncMock(return_value=[(score, _FakeNote(nid)) for score, nid in hits]) + + +def _patch(mock: AsyncMock): + return patch("scribe.services.embeddings.semantic_search_notes", mock) + + +# ── the miss that IS evidence ──────────────────────────────────────────── + + +async def test_a_short_result_set_is_a_conclusive_miss() -> None: + """Fewer hits than asked for means the limit was not binding: everything + above the floor came back, and the canon was not among it. That is a fact + about the corpus, not an artefact of where the list was cut.""" + mock = _hits((0.91, OTHER)) + report: dict = {} + with _patch(mock): + found = await _semantic_canon(1, BODY, {CANON}, report=report) + assert found is None + assert report.get("conclusive") is True + + +async def test_an_empty_result_set_is_also_conclusive() -> None: + """Nothing cleared the floor at all — the strongest form of the miss.""" + report: dict = {} + with _patch(_hits()): + assert await _semantic_canon(1, BODY, {CANON}, report=report) is None + assert report.get("conclusive") is True + + +# ── the three misses that are NOT ──────────────────────────────────────── + + +async def test_a_full_result_set_may_have_been_truncated() -> None: + """The case that makes `_SEMANTIC_LIMIT` load-bearing rather than a tuning + knob. The search returns the top N above the floor; when it returns + exactly N, an allowed canon can be sitting at N+1 and this same silence + would mean nothing. Reading it as a negative would silence real + divergences in direct proportion to how many snippets the operator has — + a check that quietly weakens as the corpus grows, which is the worst + possible failure mode for a guard nobody is watching.""" + mock = _hits(*[(0.9, OTHER + i) for i in range(_SEMANTIC_LIMIT)]) + report: dict = {} + with _patch(mock): + assert await _semantic_canon(1, BODY, {CANON}, report=report) is None + assert "conclusive" not in report + + +async def test_a_body_too_thin_to_embed_forms_no_opinion() -> None: + """And does not spend an embedding finding that out.""" + mock = _hits() + report: dict = {} + with _patch(mock): + assert await _semantic_canon(1, TOO_THIN, {CANON}, report=report) is None + assert "conclusive" not in report + mock.assert_not_awaited() + + +async def test_no_allowed_canon_means_nothing_was_compared() -> None: + """An empty allowed set is not "the canons all missed" — there were none + to miss. Distinct because the language-family gate (#2871) empties this + set routinely: a Vue body simply has no Python canon to be compared to.""" + mock = _hits() + report: dict = {} + with _patch(mock): + assert await _semantic_canon(1, BODY, set(), report=report) is None + assert "conclusive" not in report + mock.assert_not_awaited() + + +# ── a hit is a proposal, not a miss ────────────────────────────────────── + + +async def test_a_hit_returns_the_canon_and_claims_no_miss() -> None: + mock = _hits((0.88, CANON)) + report: dict = {} + with _patch(mock): + found = await _semantic_canon(1, BODY, {CANON}, report=report) + assert found == (CANON, 0.88) + assert "conclusive" not in report + + +async def test_an_allowed_canon_below_the_top_hit_still_wins() -> None: + """The scan is over the whole result set, so a disallowed snippet ranking + first does not hide an allowed one behind it. Pinned because if it did, + the short-list case above would start reporting conclusive misses for + bodies that DO have a canon.""" + mock = _hits((0.95, OTHER), (0.83, CANON)) + report: dict = {} + with _patch(mock): + found = await _semantic_canon(1, BODY, {CANON}, report=report) + assert found == (CANON, 0.83) + assert "conclusive" not in report + + +# ── the contract callers depend on ─────────────────────────────────────── + + +async def test_a_caller_that_passes_no_report_still_gets_an_answer() -> None: + """The existing test double is an `AsyncMock(return_value=None)` that + never touches a report. Absence of the key must therefore mean "cannot + tell" at every call site — so a stub, an older caller, or an arm that + threw all default to asking the question rather than excusing it.""" + with _patch(_hits()): + assert await _semantic_canon(1, BODY, {CANON}) is None + + +@pytest.mark.parametrize("value", ["semantic", "symbol", "reference", "derive"]) +def test_the_miss_basis_is_not_one_of_the_proposal_bases(value: str) -> None: + """It shares a column with them and must not collide: every reader keys + "is there a proposal" on `proposed_snippet_id`, but `confirm_shape_proposals` + filters BY basis, and a collision there would mean confirming a miss as + though it were a match.""" + assert BASIS_NO_SEMANTIC_MATCH != value diff --git a/tests/test_integration_shape_classify.py b/tests/test_integration_shape_classify.py index 034cfdc..d7943c6 100644 --- a/tests/test_integration_shape_classify.py +++ b/tests/test_integration_shape_classify.py @@ -889,6 +889,91 @@ async def test_a_second_confirm_dialog_is_detected_and_named(seeded): assert total == 0 +@pytest.mark.integration +async def test_a_conclusive_meaning_miss_silences_what_the_signature_cannot(seeded): + """#4208: the four false prompts #4204's form gate provably cannot reach. + + THE FIXTURE IS THE ACCEPTANCE CASE ABOVE, DELIBERATELY. That is the whole + difficulty of this issue: a hand-rolled `confirmDanger` beside an async + confirm canon is structurally IDENTICAL to a registry helper beside an + async service canon — same family, same form contradiction, same directory + density. The form gate has to keep asking about both, so nothing derived + from a signature can separate them. The only difference is whether the + shape does the canon's JOB, and the only reading of that the ledger holds + is the proposer's per-symbol body comparison. + + So the two runs differ in exactly one thing. In the test above the semantic + arm is quiet — it answers "nothing" without claiming to have looked — and + the prompt is RAISED, which is what milestone #2793 exists to produce. Here + it answers "I compared this body against the canons in its family and it is + none of them", and the prompt is WITHHELD. Holding the fixture identical is + what makes this a test of the meaning gate rather than of the setup. + + Asserted on the stored basis as well as the outcome, so that a future + change which silences the prompt for some other reason fails here instead + of reading as a pass. + """ + from datetime import datetime, timedelta, timezone + from unittest.mock import AsyncMock, patch + + from scribe.services import shape_ledger + from scribe.services import snippets as snippets_svc + from scribe.services.shape_ledger import ( + BASIS_NO_SEMANTIC_MATCH, flag_divergence, live_rows, propose_for_repo, + ) + + owner, pid = seeded["owner"], seeded["pid"] + canon = await snippets_svc.create_snippet( + owner, name="cls_confirm_factory_meaning", + code="export async function factory(): Promise {\n return true;\n}\n", + language="typescript", repo="Widget", + path="frontend/src/composables/useConfirm.ts", symbol="factory", + project_id=pid, + ) + sid = int(canon.id) + comp = "frontend/src/components" + base = _defs( + *[(f"{comp}/{n}.vue", "sym", f"on{n}", f"async function on{n}() {{", + f"async function on{n}() {{\n const ok = await factory();\n if (!ok) return;\n}}") + for n in ("Trash", "Delete", "Remove", "Restore")], + ) + await sync_repo_shapes(pid, REPO, base, seen_marker="aaa111") + await classify_shapes(owner, pid, [ + {"path": f"{comp}/{n}.vue", "symbol": f"on{n}", "status": "instance", "snippet_id": sid} + for n in ("Trash", "Delete", "Remove", "Restore") + ], via="audit") + previous = datetime.now(timezone.utc) + + later = base + _defs( + (f"{comp}/Danger.vue", "sym", "confirmDanger", "function confirmDanger() {", + "function confirmDanger() {\n return window.confirm('Really?');\n}"), + ) + await sync_repo_shapes(pid, REPO, later, seen_marker="bbb222") + + def _conclusive_miss(*_args, report=None, **_kw): + """The arm ran, compared, and found no canon — the one empty answer + that is evidence. `_semantic_canon` itself decides when it may say + this (a result set shorter than the limit); the unit tests for that + judgment are in tests/test_divergence_meaning_gate.py.""" + if report is not None: + report["conclusive"] = True + return None + + with patch.object(shape_ledger, "_semantic_canon", + AsyncMock(side_effect=_conclusive_miss)): + await propose_for_repo(owner, pid, REPO, later) + + rows = await live_rows(pid) + danger = next(r for r in rows if r.symbol == "confirmDanger") + assert danger.proposal_basis == BASIS_NO_SEMANTIC_MATCH + # The miss is not a proposal: nothing may read it as one. + assert danger.proposed_snippet_id is None + + assert await flag_divergence(pid, since=previous - timedelta(seconds=1)) == 0 + _, total = await list_project_shapes(owner, pid, flag="divergence") + assert total == 0, "a shape the proposer measured as unrelated must not be urged" + + @pytest.mark.integration async def test_history_records_what_was_used_when_and_drift_asks_for_a_recheck(seeded): from scribe.services.shape_ledger import shape_history -- 2.54.0 From 62f3a485addac1767063ed814a6554b55b7e8f99 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 20:47:16 -0400 Subject: [PATCH 3/7] fix(usage): one seam attaches the surfaced-vs-opened chip, and the Knowledge browse uses it (#4230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `usage_for_notes` is named for notes and works on every note row, yet the chip reached snippets and rules only. Notes had it nowhere. Lessons had it collected and shown nowhere a person could reach, because #4196 taught `/api/lessons` to attach it and `KnowledgeView` — the only lesson list in the UI — browses through `/api/knowledge`, so `listLessons` still has no consumer. The cause was not a missing line. SEVEN call sites carried their own copy of the same few lines: two REST lists, two REST details, two MCP lists, one MCP detail. Each read perfectly well alone, so "which doors attach usage?" had no answer anywhere in the code — the same asymmetry test_system_tagging_door_parity.py records for System tagging (#4249), where whichever door nobody exercised for a kind is the one that never grew the feature. `attach_usage(rows, key="id")` is now that answer, and all seven go through it. A detail payload is a one-row list, so the single-record doors share the seam rather than keeping a second shape beside it. Deliberately NO try/except: the fail-open already lives in `usage_for_notes`, which reports through `_report_failure("readout")` and returns the zero-filled map. Wrapping it again would swallow the REPORT as well as the error, and a silently-swallowed readout failure is exactly #2663 — every counter reading zero in production for weeks while the writes landed fine. `/api/knowledge` now attaches usage, which closes both holes at once: it is how notes, lessons and processes are all browsed. `KnowledgeView` renders the badge on the card footer, looking the advice up per row because the feed is mixed. The advice moves to utils/deadWeight.ts. Canon #3460 says each caller owns its own const, and that held while each caller showed ONE kind; a mixed feed would need five of its own and the next surface another five. The canon's actual invariant — advice is kind-specific and never baked into the badge — is kept: it is still a prop. The three existing callers now read the same table, so the sentence has one home rather than four. Recorded against #3460 so the next reader is not left re-litigating it. `_row_id` rejects bools explicitly: `int(True)` is 1, so a row carrying a flag under the key would be credited with note #1's counts, and a wrong chip is worse than no chip because it reads as a measurement. A row with no usable id is skipped rather than failing the page. Tests pin the PROPERTY, not one route: no door calls the aggregate directly (AST, so a comment naming it is not a false positive), and every door that shows usage reaches the seam. Plus the N+1 guard — one aggregate per page, asserted on await_count, because the per-row version reads more naturally and is invisible in review. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- .../src/components/rules/RuleListPane.vue | 13 +- frontend/src/utils/deadWeight.ts | 68 ++++++ frontend/src/views/KnowledgeView.vue | 17 ++ frontend/src/views/LessonDetailView.vue | 3 +- frontend/src/views/SnippetListView.vue | 11 +- src/scribe/mcp/tools/lessons.py | 10 +- src/scribe/mcp/tools/snippets.py | 6 +- src/scribe/routes/knowledge.py | 16 +- src/scribe/routes/lessons.py | 10 +- src/scribe/routes/snippets.py | 10 +- src/scribe/services/note_usage.py | 62 ++++++ tests/test_usage_attach_seam.py | 205 ++++++++++++++++++ 12 files changed, 385 insertions(+), 46 deletions(-) create mode 100644 frontend/src/utils/deadWeight.ts create mode 100644 tests/test_usage_attach_seam.py diff --git a/frontend/src/components/rules/RuleListPane.vue b/frontend/src/components/rules/RuleListPane.vue index 87a8311..82f63f6 100644 --- a/frontend/src/components/rules/RuleListPane.vue +++ b/frontend/src/components/rules/RuleListPane.vue @@ -1,16 +1,7 @@