From b72c9a92e203b6a2b93934f08402c90b86cbdf83 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 5 Oct 2026 08:07:26 -0400 Subject: [PATCH] test(retrieval): two source guards read the pipeline where the rule arms now live (milestone 456 step 2, #4904) CI run 8137 had two failures, both source-scanning guards whose property moved to the pipeline. The property itself still held: - test_every_surface_name_is_a_real_telemetry_source looked for source="" literals. The rule arms now record under their spec field, so the spec is the join key, read from RULE_ARMS. - test_every_hook_rule_search_says_which_project_it_is_for counted 4 direct searches in plugin_context. It now expects 0 there, so a copied arm fails the count. It also asserts that the pipeline has exactly one search and that its keyword literal carries project_id and never everywhere. Co-Authored-By: Claude Opus 5.5 --- tests/test_retrieval_surfaces.py | 9 ++++++++- tests/test_rule_usage_wiring.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/test_retrieval_surfaces.py b/tests/test_retrieval_surfaces.py index 4a0e9966..be539964 100644 --- a/tests/test_retrieval_surfaces.py +++ b/tests/test_retrieval_surfaces.py @@ -67,9 +67,16 @@ def test_every_surface_name_is_a_real_telemetry_source(): # a literal, so that one name is satisfied by the constant holding it. from scribe.services.reply_preferences import SOURCE + # The rule arms record through the one pipeline (milestone 456), where + # `source` is the spec's own field — so for them the spec IS the string + # `record_retrieval` receives, and the join key is checked against it. + from scribe.services.retrieval_pipeline import RULE_ARMS + + via_pipeline = {arm.source for arm in RULE_ARMS} missing = [ s.name for s in rs.SURFACES.values() - if f'source="{s.name}"' not in blob and s.name != SOURCE + if f'source="{s.name}"' not in blob + and s.name != SOURCE and s.name not in via_pipeline ] assert not missing, ( f"these surfaces can be tuned but never measured: {missing}. The " diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index 32b61645..ff836c4f 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -1698,7 +1698,10 @@ def test_every_hook_rule_search_says_which_project_it_is_for(): `everywhere` is not an acceptable answer in a hook, which speaks unasked. """ sources = { - "src/scribe/services/plugin_context.py": 4, + # Since milestone 456 the hook arms search through the pipeline's one + # ranked-search stage, checked below — so a direct search appearing + # here again is a copy of the arm coming back, and fails the count. + "src/scribe/services/plugin_context.py": 0, "src/scribe/services/reply_preferences.py": 1, } for path, expected in sources.items(): @@ -1724,6 +1727,31 @@ def test_every_hook_rule_search_says_which_project_it_is_for(): f"{path}:{call.lineno} searches every project's rules from a hook" ) + # The pipeline: exactly one search, in `_ranked`, whose keyword set is + # built as a dict literal — so the keys are read from that literal. + pipeline = ast.parse(Path("src/scribe/services/retrieval_pipeline.py").read_text()) + searches = [ + n for n in ast.walk(pipeline) + if isinstance(n, ast.Call) and getattr(n.func, "attr", None) == "search" + ] + assert len(searches) == 1, ( + f"the pipeline has {len(searches)} rule searches, expected 1 — every " + f"arm is meant to reach the ranker through `_ranked`" + ) + ranked = next(n for n in ast.walk(pipeline) + if isinstance(n, ast.AsyncFunctionDef) and n.name == "_ranked") + keys = { + k.value for d in ast.walk(ranked) if isinstance(d, ast.Dict) + for k in d.keys if isinstance(k, ast.Constant) + } + assert "project_id" in keys, ( + "the pipeline's rule search no longer passes project_id, so every hook " + "arm gets global rules only and never its own project's" + ) + assert "everywhere" not in keys and not any( + isinstance(n, ast.keyword) and n.arg == "everywhere" for n in ast.walk(ranked) + ), "the pipeline searches every project's rules from a hook" + @pytest.mark.asyncio @pytest.mark.parametrize("bound, scope", [(7, 7), (0, None)])