test(telemetry): the rule-arm fixture never reached the arm — it returned at the guard (#3316)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / integration (push) Successful in 28s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 24s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 35s
CI & Build / integration (push) Successful in 28s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 24s
The product code was right; the test was wrong, and wrong in a way that made two assertions fail and two others pass vacuously. `build_write_path_hint` returns early when a write matched nothing at all — no 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 hoisting it would mean an embedding query on every write in the session. My fixture stubbed every other arm to empty, so it hit the early return and the rule arm never ran: `record_rule_surfaced` was called zero times, and "the recorder was not called" is also what two of the four tests were asserting for their own reasons. The fixture now supplies one prior-art hit — 0.72 against a 0.6 threshold, so it clears the band and the top_k slice — with a comment saying the hit is the arm's precondition rather than scenery. And the gate got its own test, because the fixture now depends on it: a write matching nothing must NOT reach the arm. Without that, a future change to the guard would make every assertion in this file pass without exercising anything. #3311 is explicit that the gate stays until the arm's precision is fixed, so the test says to go read that issue rather than update the assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN
This commit is contained in:
@@ -16,7 +16,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tests.helpers import fake_rule
|
||||
from tests.helpers import fake_note, fake_rule
|
||||
|
||||
# The MCP tool layer reads its caller from a ContextVar the HTTP transport sets
|
||||
# per request; a unit test has no request, so it binds the caller itself. The
|
||||
@@ -33,14 +33,30 @@ pytestmark = pytest.mark.usefixtures("_bind_user")
|
||||
# each of these does.
|
||||
|
||||
|
||||
def _arm_patches(pc, hits, recorder):
|
||||
# The write-path hint returns early when a write matched nothing at all — no
|
||||
# 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).
|
||||
#
|
||||
# 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
|
||||
# not decoration: it is the condition the arm requires in order to fire.
|
||||
_PRIOR_ART = [(0.72, fake_note(id=9, title="debounce helper", user_id=1,
|
||||
note_type="snippet"))]
|
||||
|
||||
|
||||
def _arm_patches(pc, hits, recorder, prior_art=None):
|
||||
"""The minimum stubbing that lets the rule arm run and nothing else."""
|
||||
return (
|
||||
patch.object(pc, "get_writepath_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.6,
|
||||
"top_k": 3})),
|
||||
patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))),
|
||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])),
|
||||
patch.object(pc, "semantic_search_notes",
|
||||
AsyncMock(return_value=_PRIOR_ART if prior_art is None
|
||||
else prior_art)),
|
||||
patch.object(pc, "semantic_search_rules", AsyncMock(return_value=hits)),
|
||||
patch.object(pc, "record_retrieval", MagicMock()),
|
||||
patch.object(pc, "record_surfaced", MagicMock()),
|
||||
@@ -50,10 +66,10 @@ def _arm_patches(pc, hits, recorder):
|
||||
)
|
||||
|
||||
|
||||
async def _run_arm(hits, recorder, **kwargs):
|
||||
async def _run_arm(hits, recorder, prior_art=None, **kwargs):
|
||||
from scribe.services import plugin_context as pc
|
||||
with ExitStack() as stack:
|
||||
for ctx in _arm_patches(pc, hits, recorder):
|
||||
for ctx in _arm_patches(pc, hits, recorder, prior_art):
|
||||
stack.enter_context(ctx)
|
||||
return await pc.build_write_path_hint(
|
||||
1, "frontend/src/api/client.ts", code="x" * 400, **kwargs
|
||||
@@ -106,6 +122,24 @@ async def test_nothing_is_recorded_when_every_hit_was_already_held():
|
||||
assert rec.call_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_arm_does_not_fire_on_a_write_that_matched_nothing():
|
||||
"""The gate, pinned — because the fixture above now depends on it and a
|
||||
silent change would make every other test here pass vacuously.
|
||||
|
||||
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.
|
||||
"""
|
||||
rec = MagicMock()
|
||||
hits = [(0.71, fake_rule(id=156, title="A wait with no deadline is a bug"))]
|
||||
await _run_arm(hits, rec, prior_art=[])
|
||||
|
||||
assert rec.call_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_failing_recorder_does_not_break_the_write():
|
||||
"""Telemetry must never take down the surface it observes. The arm is
|
||||
|
||||
Reference in New Issue
Block a user