diff --git a/tests/helpers.py b/tests/helpers.py index c68f5da..d1fe6f9 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -374,7 +374,18 @@ def writepath_cfg(**over): So the keys come from `retrieval_surfaces.SURFACES`. A seventh surface, or a rename, changes this helper for free and cannot quietly disable an arm in ten hand-written dicts that each looked complete on the day they were typed. + + NOT EVERY KEY IS A SURFACE, and that gap already bit once (#4214). The + checkpoint bar has no entry in SURFACES on purpose — everything in that + table is a floor/budget pair belonging to one QUERY, and the checkpoint + runs none, it re-reads hits the rule arms already produced. Adding it there + would give it a phantom budget. So it is taken from the module constant + instead, which keeps the "one literal, in the product" property even though + the derivation differs. `test_the_config_stand_in_carries_every_key_the_ + real_one_does` is what makes the next addition fail loudly here rather than + silently no-op an arm, which is the whole claim this docstring makes. """ + from scribe.services.plugin_context import _CHECKPOINT_DEFAULT from scribe.services.retrieval_surfaces import SURFACES cfg = { @@ -385,6 +396,7 @@ def writepath_cfg(**over): "rule_top_k": SURFACES["write_path_rule"].budget_default, "tool_rule_threshold": SURFACES["pre_tool_rule"].floor_default, "tool_rule_top_k": SURFACES["pre_tool_rule"].budget_default, + "checkpoint_threshold": _CHECKPOINT_DEFAULT, } cfg.update(over) return cfg diff --git a/tests/test_retrieval_surfaces.py b/tests/test_retrieval_surfaces.py index 1f6a720..4a0e996 100644 --- a/tests/test_retrieval_surfaces.py +++ b/tests/test_retrieval_surfaces.py @@ -183,5 +183,9 @@ async def test_the_write_path_config_carries_every_arm_it_drives(): cfg = await pc.get_writepath_config(1) for key in ("threshold", "top_k", "rule_threshold", "rule_top_k", - "tool_rule_threshold", "tool_rule_top_k"): + "tool_rule_threshold", "tool_rule_top_k", + # Not a surface, and that is why it is spelled out here: it + # has no floor/budget pair to derive from, so nothing else in + # this file would notice it going missing (#4214). + "checkpoint_threshold"): assert key in cfg, f"{key} missing — its arm will silently no-op" diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index 554f2af..eff499e 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -508,7 +508,7 @@ async def test_the_prompt_arm_says_nothing_when_asked_nothing(): stack.enter_context(patch.object(pc, "record_rule_surfaced", MagicMock())) out = await pc.build_prompt_rule_hint(1, " ") - assert out == {"context": "", "rule_ids": []} + assert out == {"context": "", "rule_ids": [], "checkpoint": {}} search.assert_not_called() log.assert_not_called() @@ -653,7 +653,7 @@ async def test_an_empty_command_asks_the_ranker_nothing(): stack.enter_context(patch.object(pc, "record_rule_surfaced", rec)) out = await pc.build_tool_rule_hint(1, "Bash", " ") - assert out == {"context": "", "rule_ids": []} + assert out == {"context": "", "rule_ids": [], "checkpoint": {}} search.assert_not_called() rec.assert_not_called() @@ -669,7 +669,7 @@ async def test_the_tool_arm_fails_open(): AsyncMock(side_effect=RuntimeError("boom")))) out = await pc.build_tool_rule_hint(1, "Bash", "docker compose up -d") - assert out == {"context": "", "rule_ids": []} + assert out == {"context": "", "rule_ids": [], "checkpoint": {}} @pytest.mark.asyncio @@ -833,7 +833,7 @@ async def test_the_tool_arm_logs_the_call_that_found_nothing(): log, rec = MagicMock(), MagicMock() out = await _run_tool_arm([], rec, retrieval_log=log) - assert out == {"context": "", "rule_ids": []} + assert out == {"context": "", "rule_ids": [], "checkpoint": {}} assert log.call_count == 1 assert log.call_args.kwargs["source"] == "pre_tool_rule" assert log.call_args.kwargs["results"] == [] diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index e3e327e..0d893be 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -518,3 +518,37 @@ async def test_write_path_labels_a_non_snippet_hit_with_its_kind(): assert '[similar 0.72] "debounce helper"' in ctx # the snippet does not # The header now names the right opener for each kind. assert "get_task(id)" in ctx and "get_snippet(id)" in ctx + + +# ── The config stand-in cannot fall behind the real one (#4214) ─────────── + +@pytest.mark.asyncio +async def test_the_config_stand_in_carries_every_key_the_real_one_does(): + """THE GUARD `writepath_cfg`'s DOCSTRING ALREADY CLAIMED AND DID NOT HAVE. + + Three arms read their numbers out of that dict inside a fail-open + `except`, so a missing key does not raise where anyone can see it — the + arm silently becomes a no-op, which is indistinguishable from the arm + working and finding nothing. The helper derives its SURFACE keys from the + registry to prevent exactly that, and then #4214 added a key that is + deliberately not a surface: the whole derivation missed it, ten tests went + red at once, and the diagnosis cost a CI round. + + Asserting the KEY SETS match, not the values: the stand-in exists to let a + test set different numbers. + """ + from unittest.mock import AsyncMock, patch + + from scribe.services import plugin_context as pc + from tests.helpers import writepath_cfg + + with patch.object(pc, "get_setting", AsyncMock(return_value="0.6")), \ + patch.object(pc, "floor_for", AsyncMock(return_value=0.6)), \ + patch.object(pc, "budget_for", AsyncMock(return_value=3)): + real = await pc.get_writepath_config(1) + + assert set(writepath_cfg()) == set(real), ( + "tests/helpers.writepath_cfg has fallen behind get_writepath_config; " + "a key the real config has and the stand-in does not turns an arm " + "into a silent no-op under test" + )