diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index c6b4bd9b..4460b456 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -1,9 +1,7 @@ -"""Both ends of the rule-usage loop are actually wired (milestone 333 step 2). +"""Both ends of the rule-usage loop are actually wired. -Step 1 built the table and the service. A counter nobody calls reads zero and -looks exactly like a surface nobody uses — which is #2663's shape and the whole -reason this milestone exists. So this file is about the CALL SITES, not the -storage. +A counter nobody calls reads zero and looks exactly like a surface nobody uses. +So this file is about the CALL SITES, not the storage. Cross-cutting on purpose: the surfaced end lives in `plugin_context`, the pull end in two different doors, and the property under test is that they meet. Split @@ -39,8 +37,8 @@ pytestmark = pytest.mark.usefixtures("_bind_user") # staleness, no synced record, no prior-art menu, no shape signal. The rule arm # sits deliberately on the FAR side of that guard, because it runs a semantic # search and moving it above would mean an embedding query on every write in -# the session (#3311's closing note, and the reason its gating is a separate -# question from precision). +# the session (which is why its gating is a separate question from +# precision). # # So a fixture that stubs every other arm to empty never reaches the rule arm # at all — which is what the first run of this file did. The note hit below is @@ -60,7 +58,7 @@ def _arm_patches(pc, hits, recorder, prior_art=None, cfg=None, rule_search=None, """ return ( patch.object(pc, "get_writepath_config", - # Every key, derived from the surface registry (#4102). + # Every key, derived from the surface registry. # A stub missing one does not fail where a reader would # see it: the arm fails open, so the KeyError becomes an # empty hint and every case in _ARMS reports the arm went @@ -145,7 +143,7 @@ async def test_the_arm_searches_on_its_OWN_bar_not_the_code_one(): """The consuming half of step 4. `get_writepath_config` assembling a separate `rule_threshold` means nothing if the arm still passes `cfg["threshold"]` to its search — the split would exist in the config and - not in the behaviour, and #3311 would be exactly where it was. + not in the behaviour. The two values are deliberately different here so the assertion can tell them apart. @@ -166,12 +164,8 @@ async def test_the_arm_searches_on_its_OWN_bar_not_the_code_one(): kw = search.await_args.kwargs assert kw["threshold"] == 0.81, "the arm is still using the code threshold" assert kw["limit"] == pc.RULEHINT_LIMIT - # NO CATEGORY FILTER (#3702, repointed by 394). This asserted that the arm - # passed no `tier`. That parameter no longer exists, so the assertion had - # become one that could not fail — which rule 167 rates below having none. - # - # The claim it was making is still live, on the axis that DOES still exist: - # the act arms narrow by nothing, so a preference reaches a write exactly + # NO CATEGORY FILTER, on the axis that exists: the act arms narrow by + # nothing, so a preference reaches a write exactly # as a rule does. "Already in the session" was never the same as "in front # of the reader at the moment it applies", and relevance is the # threshold's job rather than a category's. The one place a kind filter @@ -191,9 +185,9 @@ async def test_the_arm_does_not_fire_on_a_write_that_matched_nothing(): A write matching no prior art returns before the rule arm runs. That is deliberate: the arm is a semantic search, and ungating it means an - embedding query on every write in the session. #3311 is explicit that the - gate stays until the arm's precision is fixed, so this failing is a signal - to go read that issue rather than to update the assertion. + embedding query on every write in the session. The gate stays until the + arm's precision is fixed, so this failing is a reason to look at precision + rather than to update the assertion. """ rec = MagicMock() hits = [(0.71, fake_rule(id=156, title="A wait with no deadline is a bug"))] @@ -300,11 +294,10 @@ def test_the_bulk_loaders_are_not_counted_as_pulls(): ) -# ── The AMBIENT end: bulk deliveries (#3473) ─────────────────────────── +# ── The AMBIENT end: bulk deliveries ────────────────────────────────── # -# The preload was the largest rule surface in the product and emitted nothing, -# so its cost was certain and its usefulness unfalsifiable. These assert the -# three delivery shapes now emit — and, just as importantly, that the two +# A bulk delivery that emitted nothing would have a certain cost and an +# unfalsifiable usefulness. These assert the three delivery shapes emit — and, just as importantly, that the two # lookalike call sites which show nobody anything do NOT. @@ -315,10 +308,9 @@ def test_the_bulk_loaders_are_not_counted_as_pulls(): def test_rules_payload_records_both_the_family_and_project_halves(): """One emit site for all five `rules_payload` surfaces. - Per-caller emission would be five sites to remember, and #3430 gap 2 is - what that costs: the process→skill sync went un-emitted through an entire - dedicated telemetry survey because nothing forced its surface to be - accounted for. + Per-caller emission would be five sites to remember, and a site nobody + remembers goes un-emitted with nothing forcing its surface to be accounted + for. """ from scribe.services import rulebooks as svc @@ -341,9 +333,8 @@ def test_rules_payload_records_both_the_family_and_project_halves(): def test_brief_rules_payload_lists_titles_and_records_only_what_it_shows(): - """The handshake's form (#4045): project rules by id and title, nothing - else (subscriptions went in milestone 414). A global rule it doesn't show - must not count as surfaced.""" + """The handshake's form: project rules by id and title, nothing else. A + global rule it doesn't show must not count as surfaced.""" from scribe.services import rulebooks as svc rec = MagicMock() @@ -381,17 +372,16 @@ def test_every_rules_payload_caller_names_itself(): -# ── The PRE-TOOL arm: rules keyed on the action (#3476) ──────────────── +# ── The PRE-TOOL arm: rules keyed on the action ─────────────────────── # -# The write-path arm can only be reached by a code write, so every rule about -# which tool to reach for was unretrievable at the moment it mattered — which -# is why they all had to be resident. These cover the surface that changes it. +# The write-path arm can only be reached by a code write, so a rule about +# which tool to reach for needs an arm keyed on the command. These cover it. def _tool_patches(pc, hits, recorder, cfg=None, retrieval_log=None): return ( patch.object(pc, "get_writepath_config", - # Every key, derived from the surface registry (#4102). + # Every key, derived from the surface registry. # A stub missing one does not fail where a reader would # see it: the arm fails open, so the KeyError becomes an # empty hint and every case in _ARMS reports the arm went @@ -415,13 +405,12 @@ async def _run_tool_arm(hits, recorder, command="curl -s https://git.example/api return await pc.build_tool_rule_hint(1, tool, command, **kwargs) -# ── the prompt-boundary arm (#3852) ───────────────────────────────────── +# ── the prompt-boundary arm ───────────────────────────────────────────── # # The third arm, and it joins _ARMS rather than getting a test file of its -# own. That is the point of the shared parametrisation: #3497's history is -# that the pre-tool arm inherited a defect from its sibling by being MODELLED -# on it instead of sharing with it, and a third arm modelled on two is two -# chances to repeat that. Everything in the family — repeat rendering, +# own. That is the point of the shared parametrisation: an arm MODELLED on +# its sibling instead of sharing with it inherits the sibling's defects, and a +# third arm modelled on two is two chances to repeat one. Everything in the family — repeat rendering, # fresh-only counting, the log-before-bailout order, the kind register, the # two recorders reading one list — is a property of every arm or of none. # @@ -432,7 +421,7 @@ async def _run_tool_arm(hits, recorder, command="curl -s https://git.example/api def _prompt_patches(pc, hits, recorder, retrieval_log=None): return ( # The arm reads its own floor and budget from the surface registry - # rather than a shared config object (#4102) — a third corpus, with a + # rather than a shared config object — a third corpus, with a # pair nothing has yet tuned for it. patch.object(rs, "get_setting", AsyncMock(return_value="0.6")), patch.object(pc, "semantic_search_rules", AsyncMock(return_value=hits)), @@ -509,7 +498,7 @@ async def test_the_prompt_arm_says_nothing_when_asked_nothing(): out = await pc.build_prompt_rule_hint(1, " ") # NO `checkpoint` KEY, and that is the distinction rather than an - # oversight (#4214). The two ACT arms can hold a call because there is + # oversight. The two ACT arms can hold a call because there is # a composed act to hold; this arm fires on the operator's message, # before anything has been decided, so there is nothing to put a rule # in front of. A checkpoint here would have to guess at an act. @@ -520,7 +509,7 @@ async def test_the_prompt_arm_says_nothing_when_asked_nothing(): @pytest.mark.asyncio async def test_the_prompt_arm_logs_the_calls_that_found_nothing(): - """#3497's defect, pinned on the arm that most needs it. + """The log-before-bailout order, pinned on the arm that most needs it. This bar is INHERITED from the act arms and unverified against prose. The zero rows are therefore the whole evidence base for whether 0.72 belongs @@ -531,10 +520,10 @@ async def test_the_prompt_arm_logs_the_calls_that_found_nothing(): out = await _run_prompt_arm([], MagicMock(), retrieval_log=log) assert out["context"] == "" - # BY SOURCE, not by count. The reserved slot (#3894) logs its own query on - # the same call, so a bare call_count would pin the number of arms rather - # than the property — and would go red the next time one is added, which - # is rule 167's false alarm about the very thing being protected. + # BY SOURCE, not by count. The reserved slot logs its own query on the + # same call, so a bare call_count would pin the number of arms rather than + # the property — and would go red the next time one is added, a false + # alarm about the very thing being protected. general = [c for c in log.call_args_list if c.kwargs.get("source") == "prompt_rule"] assert len(general) == 1, ( @@ -582,8 +571,8 @@ async def test_turning_off_the_notes_menu_does_not_turn_off_rules(): @pytest.mark.asyncio async def test_the_tool_arm_names_a_rule_for_the_command_about_to_run(): - """The 2026-09-03 incident in one test: reaching for curl against the forge - API is a Bash call, and nothing watched Bash.""" + """Reaching for curl against the forge API is a Bash call, and this arm is + what watches Bash.""" rec = MagicMock() hits = [(0.71, fake_rule(id=161, title="Reach the forge through its MCP tools, never curl", @@ -601,8 +590,7 @@ async def test_the_tool_arm_names_a_rule_for_the_command_about_to_run(): async def test_the_tool_arm_is_a_ranked_source(): """It CHOSE what it showed, so a pull can settle whether the choice was any good — unlike a preload, which chose nothing. If this drifts into the - ambient class the arm becomes unjudgeable, which is the state #3311 - described and M333 existed to end.""" + ambient class the arm becomes unjudgeable.""" from scribe.services.rule_usage import is_ambient assert not is_ambient("pre_tool_rule") @@ -612,9 +600,8 @@ async def test_the_tool_arm_is_a_ranked_source(): async def test_a_rule_the_session_already_holds_is_referenced_not_re_offered(): """WAS `..._is_not_re_offered`, asserting `"161" not in out["context"]`. - That assertion was the old contract and #3750 deliberately reverses half of - it: a rule already on the ledger now gets a line with a different tail - instead of being dropped in silence. Withholding was only ever right while + Half of that holds and half is reversed: a rule already on the ledger gets + a line with a different tail instead of being dropped in silence. Withholding was only ever right while the session still HELD what it was told, and a compaction breaks exactly that while leaving the id excluded. @@ -633,7 +620,7 @@ async def test_a_rule_the_session_already_holds_is_referenced_not_re_offered(): "actually chosen by this arm" ) assert "get_rule(161)" in out["context"], ( - "the held rule vanished instead of being referenced (#3750)" + "the held rule vanished instead of being referenced" ) assert "get_rule(12)" in out["context"], ( "the fresh rule was lost while adding the reference — both belong in " @@ -729,7 +716,7 @@ def test_the_tool_arm_is_registered_on_bash(): def test_the_hook_and_the_route_agree_on_every_parameter_name(): - """Rule 33, on a brand-new integration between layers. + """Two layers that only this test connects agree on their contract. The hook is shell and the route is Python; nothing but this test connects them. A renamed query arg fails SILENTLY — the route reads an absent value, @@ -744,16 +731,16 @@ def test_the_hook_and_the_route_agree_on_every_parameter_name(): handler = route.split("async def pre_tool_rules")[1].split("\n@plugin_bp")[0] sent = set(re.findall(r"[?&]([a-z_]+)=", hook)) - # `root` and `cwd` (milestone 444): where the command runs, so the paths it + # `root` and `cwd`: where the command runs, so the paths it # names can be made repo-relative for the rulings arm. assert sent == {"tool", "command", "exclude_rule_ids", "root", "cwd"}, sent - # The project-scope key is the shared helper's to choose (#4085): the hook + # The project-scope key is the shared helper's to choose: the hook # splices in `scribe_scope_query`'s output, which is `repo=` inside a git # repo and `project_id=` where a `.scribe` marker names the project. Both # halves are one contract with the route, so both are pinned. assert "scribe_scope_query" in hook, "hook no longer asks for a project scope" - # `held_rule_ids` joins them for the same reason (#4100): the OPENED ledger + # `held_rule_ids` joins them for the same reason: the OPENED ledger # is read by a shared helper so its ageing and format cannot drift from the # naming ledger's, which means the key is spelled in defs rather than here. assert "scribe_held_query" in hook, "hook no longer sends the opened ledger" @@ -765,7 +752,7 @@ def test_the_hook_and_the_route_agree_on_every_parameter_name(): assert "printf '&held_rule_ids=" in defs, ( "the opened ledger's query key is no longer spelled in the helper" ) - # The rulings ledger, shared with the write-path hook (milestone 444) — + # The rulings ledger, shared with the write-path hook — # spelled once in its helper for the same reason. assert "scribe_rulings_query" in hook, "hook no longer sends the rulings ledger" assert "printf '&seen_ruling_systems=" in defs, ( @@ -790,12 +777,12 @@ def test_the_hook_and_the_route_agree_on_every_parameter_name(): ) -# ── The CALL log is unconditional; the SURFACING log is not (#3497) ──── +# ── The CALL log is unconditional; the SURFACING log is not ─────────── # -# Both arms used to write their retrieval_logs row inside a guard on having -# results, so `zero_result_calls` was pinned at 0 and `cleared_threshold` at -# `calls` by the shape of the code — at any threshold whatsoever. #3311 read -# that as a measurement of the corpus and a milestone was scoped on it. +# A retrieval_logs row written inside a guard on having results pins +# `zero_result_calls` at 0 and `cleared_threshold` at `calls` by the shape of +# the code — at any threshold whatsoever — and reads like a measurement of the +# corpus. # # The distinction these tests hold: a CALL happened whether or not it found # anything, and the calls that found nothing are the only evidence a threshold @@ -842,7 +829,7 @@ async def test_the_tool_arm_logs_the_call_that_found_nothing(): """It matters more here than on the sibling. This arm fires on every Bash call, so an empty `sources` row is the normal outcome — and with no row at all, "the ranker declined" is indistinguishable from "the hook never fired", - which is exactly the silent failure the arm was built to stop (#3476).""" + which is exactly the silent failure the arm was built to stop.""" log, rec = MagicMock(), MagicMock() out = await _run_tool_arm([], rec, retrieval_log=log) @@ -890,10 +877,9 @@ async def test_a_command_the_arm_never_searched_writes_no_row_at_all(): def test_no_rule_arm_logs_its_call_behind_a_results_guard(): """Structural, on top of the behavioural tests above, because the defect was one level of indentation and it appeared INDEPENDENTLY in two arms — - the pre-tool arm inherited it by being modelled on its sibling (#3497). + the pre-tool arm inherited it by being modelled on its sibling. - Since milestone 456 there is one implementation to guard rather than a - copy per arm, so the property is checked where it is written: the + There is one implementation to guard rather than a copy per arm, so the property is checked where it is written: the pipeline's ranked-search stage writes the call row unconditionally, the arm runner calls that stage before it can return, and the surfacing row alone stays behind `if rule_ids:`. And no arm writes a rule-source row of @@ -921,7 +907,7 @@ def test_no_rule_arm_logs_its_call_behind_a_results_guard(): assert not early, ( f"the ranked-search stage can return at line {early[0]} before logging " f"its call — a call that found nothing is the only evidence a bar is " - f"too high (#3497)" + f"too high" ) # 2. The runner reaches the stage before its first early return. @@ -958,7 +944,7 @@ def test_no_rule_arm_logs_its_call_behind_a_results_guard(): ) -# ── Suppression: which zeros were the ranker, which were repeats (#3497) ── +# ── Suppression: which zeros were the ranker, which were repeats ────── # # Making the call log unconditional exposed a second ambiguity in the same row. # A zero-result rule call is two unrelated events: the ranker found nothing @@ -1020,19 +1006,15 @@ async def test_a_shown_hit_is_not_counted_as_suppressed(): assert len(log.call_args.kwargs["results"]) == 1 -# ── The identity that falsified this milestone (#3668) ───────────────── +# ── The identity between the two tables ───────────────────────────────── # # `rule_usage.surfaced` == `pre_tool_rule.cleared` + `write_path_rule.cleared`. -# Milestone #379 was scoped on a reconstruction that put ~64% of ranked rule -# surfacings as never reaching `rule_usage_events`. Five steps were planned -# against it. One read of this identity — 17 = 17, then 39 = 39 on a second -# window — falsified the whole thing: the gap was two counters that started -# recording on different days, not a write path dropping rows. +# Two counters that started recording on different days produce a gap that +# reads like a write path dropping rows; this identity tells the two apart. # # So the identity is not a nice-to-have. It is the cheapest true statement -# available about this pair of tables, and its absence is what let a magnitude -# that merely LOOKED wrong survive a code review and a five-step plan. An -# identity that must hold exactly beats a magnitude that looks wrong. +# available about this pair of tables. An identity that must hold exactly +# beats a magnitude that looks wrong. # # WHY THE ARM IS THE RIGHT PLACE TO PIN IT, and the readout is not. Inside an # arm, one `fresh` list feeds both recorders in one function, so the counts @@ -1040,8 +1022,8 @@ async def test_a_shown_hit_is_not_counted_as_suppressed(): # than it looks: `cleared_threshold` counts CALLS that beat the bar while # `surfaced` counts RULES, and those coincide only while `RULEHINT_LIMIT` is 1. # Raise the limit and the readout identity breaks while nothing is wrong. -# `RULEHINT_LIMIT` has already moved once (2 → 1, `2385100`), and that move is -# half of why the original reconstruction misread its own numbers. +# `RULEHINT_LIMIT` can move, and a reader comparing windows across a move +# misreads its own numbers. # # Hence three hits below, where production currently returns at most one. The # test is deliberately in a state the limit does not permit today, because what @@ -1055,7 +1037,7 @@ _THREE_HITS = [ ] def test_the_three_hit_fixture_sits_inside_the_rule_band(): - """The fixture's own precondition, asserted rather than commented (#3851). + """The fixture's own precondition, asserted rather than commented. The act arms band before they dedup, so a fixture whose spread straddles `_RULEHINT_BAND` loses its lowest hit to the BAND and then reports a count @@ -1085,8 +1067,7 @@ _ARMS = [ ("write_path_rule", _run_arm), ("pre_tool_rule", _run_tool_arm), # The prompt arm joins the family rather than being modelled on it — see - # the block above _prompt_patches for why that distinction is the whole - # lesson of #3497. + # the block above _prompt_patches for why that distinction matters. ("prompt_rule", _run_prompt_arm), ] @@ -1140,7 +1121,7 @@ async def test_both_recorders_report_the_same_rules_for_one_call( f"call log recorded {logged} and the surfacing log recorded {surfaced}. " f"Both come from `fresh`, in one function, so any difference is a bug " f"in the wiring — and it is the shape that reads as a lost write when " - f"the two tables are later compared in aggregate (#3668)." + f"the two tables are later compared in aggregate." ) assert len(logged) == expected, ( "the fixture stopped exercising what it claims to; check the exclusion " @@ -1148,18 +1129,15 @@ async def test_both_recorders_report_the_same_rules_for_one_call( ) -# ── A repeat is REFERENCED, not withheld (#3750) ────────────────────────── +# ── A repeat is REFERENCED, not withheld ───────────────────────────────── # -# Both arms used to drop a hit already on the session's exclusion ledger and -# emit nothing at all. That is right only while the session still HOLDS what it -# was told — and it stops being right the moment a compaction summarizes the +# Dropping a hit already on the session's exclusion ledger and emitting +# nothing is right only while the session still HOLDS what it was told — and it stops being right the moment a compaction summarizes the # earlier injection away while the id stays on the ledger, which leaves the # rule absent from context AND unreachable for the rest of the session. # # The tests below pin the emitted LINE, not the prose describing it, and both -# arms are parametrized through one body so the two tails cannot drift apart — -# #3497's history is that the pre-tool arm inherited a defect from its sibling -# by being modelled on it rather than sharing with it. +# arms are parametrized through one body so the two tails cannot drift apart. _HELD = fake_rule( id=156, @@ -1168,11 +1146,10 @@ _HELD = fake_rule( when_to_apply="writing any call that crosses a process boundary", ) _FRESH_TAIL = "not in this session's loaded set" -# `seen` WITHOUT `held` is the middle state since #4100: an arm named this rule -# earlier, and the session never opened it. Every test below drives the arms -# with `exclude_rule_ids` alone — the naming ledger — so this is the tail they -# produce, and it is the one that used to claim "You saw it earlier this -# session" about a teaser nobody had read. +# `seen` WITHOUT `held` is the middle state: an arm named this rule earlier, +# and the session never opened it. Every test below drives the arms with +# `exclude_rule_ids` alone — the naming ledger — so this is the tail they +# produce, and it must not claim the session read a teaser nobody opened. _SEEN_TAIL = "Mentioned earlier this session but not opened" # The third state, reached only when the opened ledger names the rule too. _HELD_TAIL = "You opened it earlier this session" @@ -1181,19 +1158,18 @@ _HELD_TAIL = "You opened it earlier this session" @pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool", "prompt"]) @pytest.mark.asyncio async def test_a_rule_already_on_the_ledger_still_produces_a_line(source, run): - """THE REGRESSION, stated as the thing that used to be absent. + """A repeat still renders a line. - Falsified against the old behaviour: before #3750 both arms filtered to - `fresh` before rendering, so an all-excluded call returned an empty - context and this assertion fails on `context == ""`. + An arm that filters to `fresh` before rendering returns an empty context + for an all-excluded call, and this assertion fails on `context == ""`. """ out = await run([(0.81, _HELD)], MagicMock(), exclude_rule_ids=[156]) # ON `get_rule(156)` RATHER THAN A TRUTHY CONTEXT. The write-path arm's # context also carries the prior-art menu, the staleness line and the shape - # signals, so `assert out["context"]` is TRUE under the old behaviour and - # would pin nothing on that arm while looking identical to a real check on - # the other. The rule line is the only part of the string this changes. + # signals, so `assert out["context"]` is TRUE even when the rule line is + # missing, and would pin nothing on that arm while looking identical to a + # real check on the other. The rule line is the part under test. assert "get_rule(156)" in out["context"], ( f"{source} emitted no rule line for a rule the session had already " f"been shown. Silence is only correct while the session still holds " @@ -1229,14 +1205,14 @@ async def test_the_two_tails_are_distinguishable_and_say_the_true_one(source, ru ) assert fresh_ctx != seen_ctx, "the two tails collapsed into one" # Neither call named an OPENED ledger, so neither may claim the session - # read anything (#4100). Catches `held` defaulting true, which would make + # read anything. Catches `held` defaulting true, which would make # every repeat assert the strongest of the three claims on no evidence. assert _HELD_TAIL not in fresh_ctx and _HELD_TAIL not in seen_ctx, ( f"{source} said the session had opened a rule it was only shown" ) -# ── the third axis: KIND (milestone 399) ──────────────────────────────── +# ── the third axis: KIND ───────────────────────────────────────────────── # # A preference is a rule row that does not bind, so it rides the same arms and # the same line. What must differ is the REGISTER: a rule's line tells the @@ -1245,8 +1221,8 @@ async def test_the_two_tails_are_distinguishable_and_say_the_true_one(source, ru # to find how this has been done, and following it buys consistency rather # than correctness. # -# Rendered in the rule's voice, a preference becomes the thing milestone 399 -# exists to avoid: a rule with a different column. +# Rendered in the rule's voice, a preference becomes a rule with a different +# column. _PREF = fake_rule( id=177, @@ -1290,7 +1266,7 @@ async def test_kind_and_seen_do_not_read_each_other(source, run): Whether a record is already on the exclusion ledger has nothing to do with how much force it carries. Keeping the two unrelated in the code is what - let a second kind arrive without reopening #3750's repeat question — and + let a second kind arrive without reopening the repeat question — and the way that silently breaks is a `seen` branch that grows a kind test, or a kind branch that grows a `seen` test, leaving one of the four combinations rendered by nobody's intention. @@ -1346,9 +1322,9 @@ async def test_neither_tail_injects_the_rule_statement(source, run): ) -# ── What a reference IS in the telemetry: nothing new (#3752) ───────────── +# ── What a reference IS in the telemetry: nothing new ────────────────────── # -# THE RELATION, STATED BEFORE IT SHIPS. #3750 changes what is RENDERED and +# THE RELATION. Rendering a repeat as a reference changes what is RENDERED and # nothing about what is COUNTED: # # result_count counts fresh surfacings — unchanged @@ -1357,13 +1333,12 @@ async def test_neither_tail_injects_the_rule_statement(source, run): # # A reference is a rendering decision, not a retrieval outcome. That answer is # not free: the naive implementation renders repeats by dropping the `fresh` -# filter, which takes `suppressed_count` to zero everywhere — and #3739's -# near-miss fix identifies repeat-caused zeros by `suppressed_count > 0`, so -# the contamination corrected on 2026-09-08 would return by a different route, -# in the same field, with the fix still sitting in the code not working. +# filter, which takes `suppressed_count` to zero everywhere — and the +# near-miss readout identifies repeat-caused zeros by `suppressed_count > 0`, +# so every repeat would be re-counted as a genuine ranker rejection. # -# #3712 gave a reader `complete_from` for a counter that started late. Nothing -# tells a reader a counter's DEFINITION moved. This test is the cheap version +# `complete_from` tells a reader a counter started late. Nothing tells a +# reader a counter's DEFINITION moved. This test is the cheap version # of that guarantee: the claim "nothing moved" is only worth anything if it is # checkable, so it is asserted rather than described. @@ -1371,7 +1346,7 @@ async def test_neither_tail_injects_the_rule_statement(source, run): @pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool", "prompt"]) @pytest.mark.asyncio async def test_a_reference_is_rendered_but_not_counted(source, run): - """The counters must read exactly as they did before #3750.""" + """Rendering a repeat leaves every counter as it was.""" log, rec = MagicMock(), MagicMock() out = await run([(0.81, _HELD)], rec, retrieval_log=log, exclude_rule_ids=[156]) @@ -1384,7 +1359,7 @@ async def test_a_reference_is_rendered_but_not_counted(source, run): f"is not evidence the bar is set correctly." ) assert row["suppressed"] == 1, ( - f"{source} stopped reporting the repeat as suppressed. #3739's " + f"{source} stopped reporting the repeat as suppressed. The " f"near_misses predicate excludes declines with suppressed_count > 0 — " f"if this reads 0, every repeat-caused zero is re-counted as a genuine " f"ranker rejection and the near-miss contamination returns." @@ -1406,7 +1381,7 @@ async def test_a_reference_is_rendered_but_not_counted(source, run): ) -# ── the reserved preference slot (#3894) ──────────────────────────────── +# ── the reserved preference slot ──────────────────────────────────────── # # A rule and a preference are not equally served by one ranking, because their # losses are not equal. A rule crowded out at the prompt boundary still fires @@ -1416,9 +1391,9 @@ async def test_a_reference_is_rendered_but_not_counted(source, run): # # The failure is invisible without this slot. The rule that won is a # legitimate hit, the telemetry reads healthy, and the only symptom is a -# preference that quietly never arrives — which is `reuse_slot`'s shape one -# corpus over (#2463), where snippets kept losing to project records that -# merely resembled the query. +# preference that quietly never arrives — `reuse_slot`'s shape one corpus +# over, where snippets lose to project records that merely resemble the +# query. def _search_by_kind(general, preference): @@ -1537,9 +1512,8 @@ async def test_the_slot_extends_rather_than_displacing(): `reuse_slot` evicts its menu's weakest hit; this one does not, and the reason is the ledger. A displaced hit sits in `prompt_rule`'s retrieval_logs row while never being surfaced, so that source's two - tables stop agreeing — and #3668's identity is the cheapest true - statement available about this pair. Milestone #379 is what losing it - costs: five steps planned against two counters disagreeing. + tables stop agreeing — and the identity is the cheapest true statement + available about this pair. """ log, rec = MagicMock(), MagicMock() out, _ = await _run_slot( @@ -1562,7 +1536,7 @@ async def test_the_slot_extends_rather_than_displacing(): ] assert logged == surfaced, ( f"prompt_rule logged {logged} and surfaced {surfaced} — the identity " - f"#3668 pins, broken by the slot rather than by a write path" + f"pinned above, broken by the slot rather than by a write path" ) @@ -1570,9 +1544,9 @@ async def test_the_slot_extends_rather_than_displacing(): async def test_the_slot_accounts_for_itself_under_its_own_source(): """Both sides of the trade, logged. - #2463's own finding is the warning rather than the precedent: the hit - that slot pushed OUT was in retrieval_logs while the query that pushed it - out was not, so the slot could never be judged against what it displaced. + A slot whose query is not logged cannot be judged against what it + displaced: the hit it pushed OUT is in retrieval_logs and the query that + pushed it out is not. This one logs its query AND records its surfacing, under a source of its own, so it can be evaluated separately from the ranking it bypassed. """ @@ -1597,9 +1571,9 @@ async def test_a_preference_on_the_ledger_keeps_the_slot_and_is_not_recounted(): A preference is the kind of record where being reminded is the point, so one already on the ledger occupies the slot rather than being skipped for - a fresh one — rendered with the repeat tail (#3750). What it must not do - is register a second surfacing, which would count one delivery twice in - the denominator pull-through is read from (#3752). + a fresh one — rendered with the repeat tail. What it must not do is + register a second surfacing, which would count one delivery twice in the + denominator pull-through is read from. """ log, rec = MagicMock(), MagicMock() out, _ = await _run_slot( @@ -1616,19 +1590,17 @@ async def test_a_preference_on_the_ledger_keeps_the_slot_and_is_not_recounted(): ) assert 140 not in out["rule_ids"], ( "a repeat was written back to the hook's ledger, which would keep " - "pushing its stamp forward so it never aged out (#3751)" + "pushing its stamp forward so it never aged out" ) -# ── each act arm uses its OWN bar, end to end (#3853) ─────────────────── +# ── each act arm uses its OWN bar, end to end ─────────────────────────── # -# The two act arms shared one threshold until the telemetry showed them -# behaving like different subsystems at the same number: write_path_rule -# speaking on 37% of 2,325 calls, pre_tool_rule on 2% of 11,768, because a +# The two act arms behave like different subsystems at the same number: a # code payload is long and rich where a shell command is short and carries # less signal for the same relevance. # -# Splitting the bar creates a failure the old single-bar code could not have: +# Separate bars create a failure a single bar could not have: # an arm can now search at one threshold and REPORT another. That row is what # near-miss analysis is read against, so a mismatch does not look like a bug — # it looks like a corpus whose scores sit somewhere they do not, and it would @@ -1652,7 +1624,7 @@ async def test_each_act_arm_searches_at_its_own_bar(): assert search.await_args.kwargs["threshold"] == 0.61, ( "the command arm searched at the write-path arm's bar; the two were " - "split at #3853 precisely because one number cannot serve both" + "split precisely because one number cannot serve both" ) @@ -1685,11 +1657,11 @@ async def test_an_act_arm_reports_the_bar_it_actually_searched_at(): assert rows[0].kwargs["threshold"] == search.await_args.kwargs["threshold"] -# ── scope: a session gets global rules plus its own project's (milestone 414) ── +# ── scope: a session gets global rules plus its own project's ── def test_every_hook_rule_search_says_which_project_it_is_for(): - """Every call site passes `project_id`, walked rather than grepped (rule 167). + """Every call site passes `project_id`, walked rather than grepped. semantic_search_rules defaults to GLOBAL rules only, so an arm that forgets the keyword does not leak another project's rules — it quietly stops @@ -1698,7 +1670,7 @@ 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 = { - # Since milestone 456 the hook arms search through the pipeline's one + # 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, diff --git a/tests/test_services_retrieval_telemetry.py b/tests/test_services_retrieval_telemetry.py index 538879f8..dffd70c3 100644 --- a/tests/test_services_retrieval_telemetry.py +++ b/tests/test_services_retrieval_telemetry.py @@ -57,7 +57,7 @@ def test_build_payload_rounds_scores_to_5dp(): assert p["result_ids"][0]["score"] == 0.12346 -# ─── suppression: "not measured" is not "none" (#3497) ─────────────────────── +# ─── suppression: "not measured" is not "none" ────────────────────────────── def test_a_caller_that_cannot_measure_suppression_stores_null(): @@ -65,7 +65,7 @@ def test_a_caller_that_cannot_measure_suppression_stores_null(): A surface that passes its exclusions into the search never sees what was dropped. Storing 0 would assert a clean run nobody observed — reading an - artifact as a measurement, which is exactly #3311's mistake. + artifact as a measurement. """ p = _build_payload( user_id=1, source="auto_inject", query="q", threshold=0.6, @@ -167,12 +167,11 @@ async def test_insert_retrieval_log_roundtrip(_dispose_engine): # ─── the read half: retrieval_summary (integration) ────────────────────────── -# Integration, not mocked, and deliberately so. #2663 is the bug where a -# GROUP BY the database rejected was swallowed by a broad except, so every -# counter read zero in production while the writes landed fine and the mocked -# tests passed. `retrieval_summary` runs a grouped aggregate with -# percentile_cont ... WITHIN GROUP and a two-label CASE — precisely the shape -# that failed then. Only a real Postgres can say it parses. +# Integration, not mocked, and deliberately so. A GROUP BY the database +# rejects, swallowed by a broad except, reads zero on every counter while the +# writes land fine and mocked tests pass. `retrieval_summary` runs a grouped +# aggregate with percentile_cont ... WITHIN GROUP and a two-label CASE — +# precisely that shape. Only a real Postgres can say it parses. @pytest.mark.integration @@ -226,7 +225,7 @@ async def test_retrieval_summary_reads_what_the_writer_wrote(_dispose_engine): assert ai["zero_result_calls"] == 1 assert "cleared_threshold" not in ai, ( "the tautology is back: it was true exactly when result_count > 0, " - "so it reported nothing zero_result_calls did not (#3670)" + "so it reported nothing zero_result_calls did not" ) # p50 over the three scored calls; the empty one contributes no score. assert ai["top_score"]["p50"] == pytest.approx(0.72, abs=1e-4) @@ -251,8 +250,8 @@ async def test_retrieval_summary_reads_what_the_writer_wrote(_dispose_engine): @pytest.mark.integration @pytest.mark.asyncio async def test_retrieval_summary_is_empty_not_broken_for_a_fresh_install(_dispose_engine): - """Rule #115: an install with no telemetry gets a coherent zero readout, - and `read_failed` stays False — the distinction #2663 says must exist.""" + """An install with no telemetry gets a coherent zero readout, and + `read_failed` stays False — "no rows" is not "the read broke".""" from scribe.services.retrieval_telemetry import retrieval_summary out = await retrieval_summary(990003, days=30) @@ -293,11 +292,10 @@ async def test_retrieval_summary_sees_only_its_own_users_telemetry(_dispose_engi await s.commit() -# ─── per-source pull-through (#3311) ───────────────────────────────────────── +# ─── per-source pull-through ───────────────────────────────────────────────── # Integration for the same reason the block above is: this is a self-join with -# two DISTINCT subqueries and a LIKE escape, which is a new SQL shape in a -# module whose one production outage (#2663) was a new SQL shape the database -# rejected inside a broad except. A mock would pass on a query Postgres refuses. +# two DISTINCT subqueries and a LIKE escape, in a module whose read sits +# inside a broad except: a new SQL shape the database rejects reads as zeros. A mock would pass on a query Postgres refuses. @pytest.mark.integration @@ -426,11 +424,9 @@ async def test_the_agent_pull_filter_does_not_treat_its_underscore_as_a_wildcard await s.commit() -# ─── rule usage (milestone 333 step 3) ─────────────────────────────────────── +# ─── rule usage ────────────────────────────────────────────────────────────── # Integration, for the same reason the block above is: these are real GROUP BYs -# and count(distinct) against a table that did not exist a commit ago, in a -# module whose one production outage (#2663) was a SQL shape the database -# rejected inside a broad except. A mock would agree with whatever the code +# and count(distinct), in a module whose read sits inside a broad except. A mock would agree with whatever the code # does, including nothing. @@ -464,7 +460,7 @@ async def test_rule_usage_is_a_coherent_zero_on_a_fresh_install(_dispose_engine) """Every rule in an existing install predates this table, so "no events" is the normal state for a while. It must read as zero, not as a missing key and not as a failure — the same "no rows" / "read broke" distinction the - rest of this readout keeps (#2663). + rest of this readout keeps. `pull_through` is None rather than 0.0, matching the note block: a ratio of zero asserts "rules were shown and none opened", which with an empty @@ -581,7 +577,7 @@ async def test_rule_usage_sees_only_its_own_users_events(_dispose_engine): @pytest.mark.integration @pytest.mark.asyncio async def test_the_preload_lands_in_ambient_and_never_in_the_ratio(_dispose_engine): - """The split that makes the always-on set judgeable (#3473). + """The split that makes the always-on set judgeable. Pull-through asks "was that hint any use", and only a surface that CHOSE what it showed can be judged by it. If the preload counted toward the @@ -647,10 +643,10 @@ async def test_ambient_alone_reports_no_ratio(_dispose_engine): @pytest.mark.asyncio async def test_the_outcome_counts_come_back_split(_dispose_engine): """A pull says the rule was read; an outcome says it changed something - (#4213). Integration rather than a mock because `distinct_rules_acted` is - a count(distinct) with an IN over a column that carries no CHECK — the - kind of SQL shape #2663 was, where a mock agrees with whatever the code - does including nothing. + Integration rather than a mock because `distinct_rules_acted` is a + count(distinct) with an IN over a column that carries no CHECK — a SQL + shape where a mock agrees with whatever the code does, including + nothing. `applied` and `departed` stay APART. Summed they would say "an outcome was recorded", which is true of both and useful about neither: a departure is @@ -681,7 +677,7 @@ async def test_the_outcome_counts_come_back_split(_dispose_engine): # An outcome is NOT a pull. If `applied` leaked into the pull counters # the silently-unchanged rule would vanish into a compliant-looking - # total, which is the exact confusion #4212 was opened to end. + # total — the confusion keeping them apart exists to prevent. assert ru["pulled"] == 3 finally: await cleanup() @@ -709,19 +705,18 @@ async def test_an_outcome_is_never_split_by_ambient(_dispose_engine): await cleanup() -# ── Window coverage (#3712) ──────────────────────────────────────────── +# ── Window coverage ──────────────────────────────────────────────────── # # A counter added last week, read over a 30-day window, reports a real count # against an imagined denominator. The result is a plausible FRACTION rather -# than an obvious zero, which is what makes it dangerous — #379 spent five -# planned steps on a defect that turned out to be a window opening before the -# recording it was measuring existed. +# than an obvious zero, which is what makes it dangerous: a window opening +# before the recording it measures existed looks like a defect. def test_coverage_says_nothing_rather_than_false_when_nothing_was_recorded(): """Null, never False. "No measurement" is not "partial measurement". - The same distinction `suppression`'s null carries (#3497): absent must not + The same distinction `suppression`'s null carries: absent must not read as a verdict. A False here would assert the window is under-covered, which is a claim nobody is in a position to make. """ @@ -814,11 +809,11 @@ async def test_coverage_is_per_source_because_the_table_is_older_than_its_arms( async def test_a_surface_that_went_silent_is_not_the_same_as_one_that_never_ran( _dispose_engine, ): - """#3720 — absent is how "never existed" renders, so it cannot also be how + """Absent is how "never existed" renders, so it cannot also be how "stopped recording" renders. - A surface losing its recorder is one of the failures this milestone exists - to make visible, and dropping it from the readout is the most complete way + A surface losing its recorder is one of the failures coverage exists to + make visible, and dropping it from the readout is the most complete way to hide it. Zero here is a real measurement: the table proves the source was recording, and it made no calls across a window it fully covers. """ @@ -853,7 +848,7 @@ async def test_a_surface_that_went_silent_is_not_the_same_as_one_that_never_ran( # The window IS covered; what was observed across it is nothing. assert quiet["covers_window"] is True # ...but nothing was sampled, so no distribution may be claimed. A - # zeroed score would assert a measurement, which is #3311's mistake. + # zeroed score would assert a measurement nobody took. assert quiet["top_score"] == { "p10": None, "p50": None, "p90": None, "min": None, "max": None, } @@ -918,16 +913,14 @@ async def test_a_section_is_complete_only_from_its_latest_contributor( await s.commit() -# ─── the bar can only be judged from what it rejected (#3670) ──────────────── +# ─── the bar can only be judged from what it rejected ──────────────────────── # -# `cleared_threshold` was the number the docstring told a reader to look at -# first. It was `calls - zero_result_calls` under another name: the search -# applies the bar before returning, so every returned result cleared it by -# construction and a call with nothing has no score to compare. -# `zero_result_calls + cleared_threshold == calls` held on all nineteen -# source/window readings ever taken — no near-misses, no exceptions. +# A count of calls that cleared the threshold is `calls - zero_result_calls` +# under another name: the search applies the bar before returning, so every +# returned result cleared it by construction and a call with nothing has no +# score to compare. # -# What replaced it cannot go the same way, and the reason is structural rather +# `near_misses` cannot go the same way, and the reason is structural rather # than careful naming: `near_misses` is measured on the calls the bar TURNED # AWAY, using a score the bar never saw. No arrangement of `calls`, # `zero_result_calls` and `result_count` derives it. @@ -957,7 +950,7 @@ def test_a_call_that_returned_nothing_still_records_what_it_nearly_showed(): def test_a_caller_that_did_not_measure_the_near_miss_stores_null(): """Null, never 0.0. A zero here reads as "the corpus held nothing remotely relevant" — a claim about the corpus invented out of a caller's silence, - which is #3311's substitution in a new field.""" + which substitutes an artifact for a measurement.""" p = _build_payload( user_id=1, source="auto_inject", query="q", threshold=0.6, limit=3, project_id=None, is_task=None, results=[], duration_ms=None, @@ -988,13 +981,12 @@ def test_the_readout_reports_unmeasured_near_misses_as_none(): def test_the_readout_carries_no_field_derivable_from_its_neighbours(): - """The guard that would have caught #3670 on the day it shipped. + """No readout field is a sum of the two beside it. - `cleared_threshold` survived because it had its own name and its own - docstring paragraph, and nobody added the two numbers beside it. This - asserts the identity that held on every reading ever taken — and if a - future field reintroduces it under a new name, the sum below is where it - shows up. + A derived field survives review when it has its own name and its own + docstring paragraph, and nobody adds the two numbers beside it. If a + field reintroduces `calls - zero_result_calls` under a new name, the sum + below is where it shows up. """ from scribe.services.retrieval_telemetry import _bucket @@ -1008,7 +1000,7 @@ def test_the_readout_carries_no_field_derivable_from_its_neighbours(): } assert not derivable, ( f"{sorted(derivable)} equals calls - zero_result_calls on this row. " - f"That is how `cleared_threshold` read for its whole life (#3670): a " + f"That is a " f"figure presented as an independent measurement that a reader can " f"compute from the two numbers next to it. Either it is a tautology, " f"or this fixture happens to make it look like one — check which " @@ -1023,10 +1015,9 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e `near_misses` is a `percentile_cont(...) WITHIN GROUP` over a CASE expression, inside the same grouped aggregate that already carries four - other CASEs. That is within one step of the shape that produced #2663 — a - query the database rejected, swallowed by this module's broad `except`, so - every counter read zero in production while the writes landed fine and the - mocked tests passed. Only a real Postgres can say this parses, and if it + other CASEs. A query the database rejects is swallowed by this module's + broad `except`, so every counter reads zero while the writes land fine and + mocked tests pass. Only a real Postgres can say this parses, and if it does not, the symptom is silence rather than an error. The numbers are chosen so a bar at 0.72 is visibly the wrong bar: three @@ -1055,12 +1046,12 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e limit=1, project_id=None, is_task=None, results=[(0.88, _note(7))], duration_ms=4.0, best_available=0.88, )) - # An unmeasured decline, standing in for every row written before #3670. + # An unmeasured decline, standing in for a row written before the column existed. await _insert_retrieval_log(_build_payload( user_id=UID, source="pre_tool_rule", query="ls", threshold=0.72, limit=1, project_id=None, is_task=None, results=[], duration_ms=4.0, )) - # THE CASE WHOSE ABSENCE LET THIS GUARD PASS OVER BROKEN CODE (#3739). + # THE CASE WITHOUT WHICH THIS GUARD PASSES OVER BROKEN CODE. # A zero-result call whose zero was a REPEAT, not a rejection: the ranker # cleared the bar at 0.9 and the session had already been shown that rule, # so the arm dropped it in Python after the search. Without the suppression @@ -1076,7 +1067,7 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e out = await retrieval_summary(UID, days=30) assert out["read_failed"] is False, ( "the aggregate did not execute — a rejected query here reads as " - "zeros everywhere, which is #2663 exactly" + "zeros everywhere" ) src = out["sources"]["pre_tool_rule"] assert src["calls"] == 6 @@ -1089,7 +1080,7 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e "Three qualify. Excluded: the unmeasured row (predates the column, " "not a scoreless decline), the call that showed something (its " "best-available is just its top score), and the REPEAT — a zero " - "the reader caused, not the bar (#3739)" + "the reader caused, not the bar" ) assert nm["max"] == pytest.approx(0.7189, abs=1e-4), ( "the closest thing the bar turned away — 0.7189 against a 0.72 " @@ -1099,7 +1090,7 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e "a rejection that outscores the bar is not a rejection. This is " "structural once the suppression arm is in the predicate: an " "above-bar candidate that was not excluded would have been " - "RETURNED, so its call cannot be in this population (#3739)" + "RETURNED, so its call cannot be in this population" ) assert 0.70 <= nm["p50"] <= 0.7189 finally: @@ -1108,17 +1099,16 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e await s.commit() -# ─── a search that never ran is not a decline (#3765) ──────────────────────── +# ─── a search that never ran is not a decline ──────────────────────────────── # -# `best_available_score` was added by #3670 so a bar could be judged from what -# it rejected, and it arrived null on four unrelated causes: the corpus offered -# nothing, the query was empty, the embedder was down, or the DATABASE QUERY -# FAILED. Only the first is a measurement. The fourth is the #2663 shape — a -# swallowed failure rendering as a clean zero — inside the field added to fix -# an instance of the #2663 shape. +# `best_available_score` lets a bar be judged from what it rejected, and it +# could be null on four unrelated causes: the corpus offered nothing, the +# query was empty, the embedder was down, or the DATABASE QUERY FAILED. Only +# the first is a measurement. The fourth is a swallowed failure rendering as a +# clean zero. # -# The fix is not a new column. A call that never searched writes no row, so -# every remaining null means one thing. That is the convention the pre-tool arm +# The answer is not a new column. A call that never searched writes no row, so +# every null means one thing. That is the convention the pre-tool arm # already follows for a blank command, extended from the case a caller can see # in advance to the ones only the search knows about. @@ -1143,7 +1133,7 @@ def test_a_search_that_never_ran_writes_no_row(): assert not build.called, ( "a search that never ran was recorded as a retrieval. It would " "read as a ranker decline — evidence about a threshold, from a " - "call where no threshold was ever applied (#3765)" + "call where no threshold was ever applied" ) @@ -1151,7 +1141,7 @@ def test_a_search_that_ran_and_found_nothing_still_writes_its_row(): """The half that stops the fix from being 'log less'. A call that searched and came back empty is the ONLY evidence a threshold - is too high (#3497). Dropping it too would trade one silent distortion for + is too high. Dropping it too would trade one silent distortion for another, and this assertion is what makes the pair discriminate: a blanket `return` passes the test above and fails this one. """ @@ -1188,19 +1178,17 @@ def test_a_caller_that_never_asked_is_assumed_to_have_searched(): assert build.called -# ─── the bar's refusals have names now (#3807) ─────────────────────────────── +# ─── the bar's refusals have names ─────────────────────────────────────────── # -# #3670 recorded how CLOSE the bar came to firing. That is the half a decision -# does not need: live, pre_tool_rule sits at a ~0.72 bar with a near-miss p90 of -# 0.7071, so dropping to 0.707 would take it from 22 hits a day to roughly 139. -# The percentile says the mass is there and says nothing about whether it is -# worth showing, and pull-through cannot referee it because the injected rule +# The near-miss score records how CLOSE the bar came to firing. That is the +# half a decision does not need: the percentile says the mass is there and +# says nothing about whether it is worth showing, and pull-through cannot referee it because the injected rule # line already carries title and trigger — a session can comply without ever # calling get_rule. # -# Reading the rejected records is the method that answers it, and until now -# `result_ids` held only what was RETURNED, so on a zero-result call the -# near-missed record had no name. +# Reading the rejected records is the method that answers it, and +# `result_ids` holds only what was RETURNED, so on a zero-result call the +# near-missed record needs a name of its own. def test_the_rejected_record_is_named_beside_the_score_it_scored(): @@ -1271,7 +1259,7 @@ async def test_the_listing_names_the_highest_declines_per_source(_dispose_engine out = await retrieval_summary(UID, days=30, near_miss_samples=2) assert out["read_failed"] is False, ( "the listing query did not execute — a rejected query here reads " - "as an absent listing, which is #2663 again" + "as an absent listing" ) top = out["sources"]["pre_tool_rule"]["near_miss_records"] diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 731f0b54..1319fadd 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -1,7 +1,7 @@ -"""Tests for the write-path trigger (#2082) — prior art at the moment code is written. +"""Tests for the write-path trigger — prior art at the moment code is written. Covers the service (`build_write_path_hint`): the two arms and their precedence, -the anti-bloat gates carried over from milestone 93, telemetry under its own +the anti-bloat gates, telemetry under its own source, and the two ways this must stay silent. Plus the plugin hook contract — a PreToolUse hook that returns a permission decision would be able to block the operator's edit, which this feature must never do. @@ -19,7 +19,7 @@ from tests.helpers import fake_note, http_sink, writepath_cfg @contextlib.contextmanager def _stored_settings(stored): - """Patch BOTH readers, because one call now uses two (#4102). + """Patch BOTH readers, because one call uses two. `get_writepath_config` reads its `enabled` switch through `plugin_context.get_setting` and all six tunable numbers through @@ -49,8 +49,7 @@ def _snippet_item(nid, title, user_id=1): def _cfg(**over): - # `rule_threshold` is the standing-rule arm's own bar (milestone 333 step - # 4). It belongs in the stand-in even though most tests here never reach + # `rule_threshold` is the standing-rule arm's own bar. It belongs in the stand-in even though most tests here never reach # that arm: the arm reads it while BUILDING its search arguments, so a # missing key raises inside its fail-open except and turns the arm into a # silent no-op — which is indistinguishable from it working and finding @@ -59,10 +58,9 @@ def _cfg(**over): # The semantic arm ignores payloads carrying less than WRITEPATH_MIN_CODE_CHARS -# of non-whitespace substance (#2223), so any test that expects it to RUN has to -# pass something a real Edit would plausibly contain. `code="x"` used to be the -# fixture here, which meant the gate tests were passing payloads that in -# production are exactly the noise the floor now drops. +# of non-whitespace substance, so any test that expects it to RUN has to pass +# something a real Edit would plausibly contain. A payload like `code="x"` is +# exactly the noise the floor drops. REAL_CODE = '''def debounce(fn, wait=0.25): """Rate-limit a callback so it fires once after the last call.""" timer = None @@ -121,7 +119,7 @@ async def test_nothing_recorded_is_silent(): @pytest.mark.asyncio async def test_snippet_at_this_path_becomes_the_sync_nudge(): """A snippet recorded AT the exact file is not a reuse suggestion — it IS - the record of the file being edited (#2708). It surfaces without a score, + the record of the file being edited. It surfaces without a score, framed as "updating the record is part of this edit", and is reported in sync_note_ids so the hook feeds the sync dedup channel.""" from scribe.services import plugin_context as pc @@ -143,8 +141,8 @@ async def test_snippet_at_this_path_becomes_the_sync_nudge(): @pytest.mark.asyncio async def test_reuse_dedup_never_suppresses_the_sync_nudge(): - """THE bug #2708 names: a title surfaced as a reuse hint twenty turns ago - landed in exclude_ids — and then silenced "you are editing the recorded + """A title surfaced as a reuse hint twenty turns ago lands in exclude_ids, + and must not silence "you are editing the recorded file right now", the one claim that must fire at the moment of change. The sync class dedups only against its own channel.""" from scribe.services import plugin_context as pc @@ -252,9 +250,9 @@ async def test_semantic_arm_is_snippet_only_and_browse_scoped(): patch.object(pc, "record_retrieval", MagicMock()): await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE) kwargs = search.await_args.kwargs - # Prior art is snippets AND recorded experience (#2246) — an issue saying - # "we tried this and it broke" belongs here — AND lessons (milestone 385 - # step 5), whose founding example is a lesson about a code shape. What + # Prior art is snippets AND recorded experience — an issue saying "we tried + # this and it broke" belongs here — AND lessons, which are often about a + # code shape. What # stays out is the open to-do list, which resembles the code and answers # nothing. assert kwargs["note_type"] == ("snippet", "note", "lesson") @@ -338,11 +336,10 @@ async def test_session_dedup_marks_the_reuse_arms_rather_than_silencing_them(): """exclude_ids governs the REUSE classes — nearby and semantic. (The sync class has its own channel; see the tests above.) - WHAT CHANGED AND WHY (#4101). This used to assert the opposite: the nearby - hit was dropped and its id pushed into the search's `exclude_ids`. That was - #3750's defect on the note arms — the second time a record was the best - answer, the session got silence, which reads exactly like "nothing is - recorded here". Now the repeat is rendered with a `seen` marker, and the + WHY A MARKER AND NOT SILENCE. Dropping the hit and pushing its id into the + search's `exclude_ids` means that the second time a record is the best + answer, the session gets silence, which reads exactly like "nothing is + recorded here". So the repeat is rendered with a `seen` marker, and the ledger never reaches the query, so the score the search reports describes the bar rather than a candidate set the caller had already edited. @@ -381,7 +378,7 @@ async def test_session_dedup_marks_the_reuse_arms_rather_than_silencing_them(): @pytest.mark.asyncio async def test_another_users_snippet_is_attributed(): - """Rule #47 / the #2084 lesson: an unattributed line reads as your own record.""" + """An unattributed line reads as your own record.""" from scribe.services import plugin_context as pc with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \ patch.object(pc.snippets_svc, "list_snippets", @@ -413,15 +410,15 @@ async def test_telemetry_uses_its_own_source(): assert note_arm.kwargs["project_id"] == 4 # The rule arm rides along on the same hint and logs its own call even when - # it finds nothing (#3497). This assertion used to be `assert_called_once`, - # which passed only because that row was never written — the test encoded - # the defect. The second row is the point of having two sources. + # it finds nothing. An `assert_called_once` here would pass only if that + # row were never written. The second row is the point of having two + # sources. assert "write_path_rule" in sources, sources @pytest.mark.asyncio async def test_sync_surfacing_is_measured_under_its_own_usage_source(): - """The sync class's pull-through rate is #2708's scoreboard — whether + """The sync class's pull-through rate is its scoreboard — whether sessions actually update the record when told they're editing it. Folding it into write_path_place would make that unmeasurable.""" from scribe.services import plugin_context as pc @@ -455,12 +452,12 @@ async def test_config_has_its_own_switch_threshold_and_inherited_budget(): # Its own switch is off while auto-inject stays on... assert cfg["enabled"] is False # ...its threshold is its OWN and stricter than auto-inject's, because code - # embeddings have a higher similarity floor than prose (#2223). Sharing 0.55 - # made unrelated code — including `x = 1` at 0.58 — clear the bar. + # embeddings have a higher similarity floor than prose. At 0.55, unrelated + # code — including `x = 1` at 0.58 — clears the bar. assert cfg["threshold"] == pc.WRITEPATH_DEFAULT_THRESHOLD assert cfg["threshold"] > pc.AUTOINJECT_DEFAULT_THRESHOLD - # ...and the budget is INHERITED rather than shared (#4102). This arm has - # its own key now, because it fires before every Write and Edit while + # ...and the budget is INHERITED rather than shared. This arm has its own + # key, because it fires before every Write and Edit while # auto-inject fires once a turn, so the same number buys very different # amounts of attention. Unset, it still reads auto-inject's — which is what # stops the split from silently resetting an install that had tuned the @@ -470,7 +467,7 @@ async def test_config_has_its_own_switch_threshold_and_inherited_budget(): @pytest.mark.asyncio async def test_writepath_threshold_is_operator_tunable_and_clamped(): - """Rule #25 — it's a product knob, so it has to be settable, and a typo in + """It's a product knob, so it has to be settable, and a typo in the settings box must not be able to open the gate to everything.""" from scribe.services import plugin_context as pc @@ -489,11 +486,11 @@ async def test_writepath_threshold_is_operator_tunable_and_clamped(): @pytest.mark.asyncio async def test_the_rule_arm_has_its_own_tunable_bar(): - """Rule #25 again, for the THIRD corpus (milestone 333 step 4). + """Settable and clamped again, for the THIRD corpus. Separate from the code threshold above and separately settable, because the two are measured against different things: 0.68 was derived from code - against note PROSE (#2223), and rules are short imperative technical + against note PROSE, and rules are short imperative technical English — a more homogeneous corpus whose noise floor sits higher. """ from scribe.services import plugin_context as pc @@ -528,12 +525,10 @@ async def test_the_two_write_path_bars_are_independent(): @pytest.mark.asyncio async def test_the_two_act_arms_read_independent_rule_bars(): - """#3853's split: the command arm's bar moves without the write path's. + """The command arm's bar moves without the write path's. - The two act arms shared one key until the telemetry showed them behaving - like different subsystems at the same number — the write-path arm speaking - on 37% of calls against the command arm's 2%, because a code payload is - long and rich where a shell command is short. A config assembler that + The two act arms behave like different subsystems at the same number, + because a code payload is long and rich where a shell command is short. A config assembler that reads one key into both fields would silently undo that, and the symptom would be invisible: both arms would simply agree again. """ @@ -553,7 +548,7 @@ async def test_a_garbage_command_bar_falls_back_to_its_own_default(): Falling back to 0.0 would attach a rule to every Bash call in the session; falling back to the sibling's default would quietly re-merge the two bars - that #3853 separated, which is the harder failure to see because the arm + that are kept separate, which is the harder failure to see because the arm keeps working. """ from scribe.services import plugin_context as pc @@ -566,15 +561,13 @@ async def test_a_garbage_command_bar_falls_back_to_its_own_default(): def test_the_command_bar_defaults_below_the_write_path_bar(): - """A DIRECTION check, like its sibling above, and for the same rule-115 - reason: the value is measured against one corpus, the relationship is not. + """A DIRECTION check, like its sibling above, and for the same reason: the + value is measured against one corpus, the relationship is not. A shell command carries less text than a code payload and therefore scores - lower for the same relevance — measured at #3853, where three of four - consequential commands retrieved nothing at the shared bar while the - write-path arm was healthy at it. Tuning either value stays free; inverting - the relationship would reinstate the mute arm that spoke on 2% of 11,768 - calls. + lower for the same relevance; at the write path's bar, consequential + commands retrieve nothing. Tuning either value stays free; inverting the + relationship would mute the command arm. """ from scribe.services import plugin_context as pc @@ -583,7 +576,7 @@ def test_the_command_bar_defaults_below_the_write_path_bar(): def test_the_rule_bar_defaults_above_the_code_bar(): """Not a number check — a DIRECTION check, and the only part of the default - that is defensible without one instance's histogram (rule 115). + that is defensible without one instance's histogram. The eligible rule corpus is orders of magnitude smaller than the note corpus, so a top-k over it always returns something and a bar calibrated @@ -592,7 +585,7 @@ def test_the_rule_bar_defaults_above_the_code_bar(): facts point the same way: this bar must sit ABOVE the one it inherited. Pinned as an inequality so tuning the value stays free while inverting the - relationship — which would silently reinstate #3311 — does not. + relationship — which would silently drown the arm in noise — does not. """ from scribe.services import plugin_context as pc @@ -600,17 +593,12 @@ def test_the_rule_bar_defaults_above_the_code_bar(): def test_the_rule_arm_asks_for_a_set_and_lets_the_band_narrow_it(): - """WAS `..._asks_for_one_rule_not_two`, pinning `RULEHINT_LIMIT == 1`. + """A set, because this arm is the whole delivery: a `git push` governed by + four rules cannot be served by one slot. - That guarded a real decision: with retrieval SUPPLEMENTING a 33-rule - resident set, k=2 over a few dozen candidates made the second line the - second-best noise wearing the first line's confident framing. Milestone - 394 removes residency, so this arm becomes the whole delivery and a - `git push` governed by four rules cannot be served by one slot (#3851). - - What replaces it is not simply a bigger k — that is the thing the old - test was right to fear, because a fixed k fills its slots whether or not - anything deserves them. The cap is a ceiling and the BAND is the control, + Not simply a bigger k — a fixed k fills its slots whether or not anything + deserves them, and the second line becomes the second-best noise wearing + the first line's confident framing. The cap is a ceiling and the BAND is the control, so both must exist for the arm to be shaped as intended. Pinned as relationships rather than values, like the threshold test above @@ -627,12 +615,12 @@ def test_the_rule_arm_asks_for_a_set_and_lets_the_band_narrow_it(): assert 0 < pc._RULEHINT_BAND < rp._NOTE_BAND -# --- the minimum-substance floor on the semantic arm (#2223) ------------------ +# --- the minimum-substance floor on the semantic arm -------------------------- @pytest.mark.asyncio async def test_trivial_payload_skips_the_semantic_arm_entirely(): - """`x = 1` scored 0.58 against an unrelated helper on the live instance — the - embedder's floor for "some code", not a resemblance. A payload too small to + """`x = 1` scores around 0.58 against an unrelated helper — the embedder's + floor for "some code", not a resemblance. A payload too small to carry meaning must not reach the embedder at all, and must not cost a retrieval_logs row either.""" from scribe.services import plugin_context as pc @@ -707,12 +695,13 @@ def test_the_floor_stays_below_the_smallest_plausible_helper(): assert pc.WRITEPATH_MIN_CODE_CHARS > len("".join("x = 1".split())) -# --- concept extraction: query by what the code IS FOR, not by the code (#2242) --- +# --- concept extraction: query by what the code IS FOR, not by the code --- def test_concept_query_prefers_the_name_and_docstring(): """Snippet documents are prose-forward — `when_to_use` appears in both the title and the body — so a concept query out-scores the code itself (0.823 vs - 0.743 measured against #2222). The shape mirrors a snippet's own title.""" + 0.743 against a recorded snippet). The shape mirrors a snippet's own + title.""" from scribe.services.plugin_context import concept_query q = concept_query( 'def collapse_into_clusters(edges):\n' @@ -753,7 +742,7 @@ def test_concept_query_does_not_let_a_shebang_become_the_description(): def test_concept_query_falls_back_when_there_is_no_doc(): """MEASURED RULE, not a taste call. An identifier alone scored 0.671 against - #2222 where the full code body scored 0.743 — same separation from the noise + a recorded snippet where the full code body scored 0.743 — same separation from the noise floor (0.113), but below the 0.68 bar, so preferring the bare name would turn a comfortable hit into a miss. Undocumented code keeps the raw payload.""" from scribe.services.plugin_context import concept_query @@ -771,7 +760,7 @@ def test_concept_query_falls_back_on_a_doc_that_says_nothing(): def test_concept_query_falls_back_when_nothing_parses(): """A Vue SFC has neither a recognised declaration nor a doc block. Every - unhandled language must degrade to exactly the pre-#2242 behaviour.""" + unhandled language must degrade to querying by the raw payload.""" from scribe.services.plugin_context import concept_query assert concept_query( '