From 1fa9245bbf7da176facd90ae8de6f9128090d425 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 22 Oct 2026 19:28:29 -0400 Subject: [PATCH] fix(6247): a getter is seen by its pulls, so the silent list stops naming every pull door usage and rule_usage gain pulled_by_source, read from the per-source rows the readout already had, and _silent_surfaces counts those doors as having emitted. The rules-listing points were seen as of the previous commit, through rule_usage.by_source. Co-Authored-By: Claude Opus 5.5 --- src/grimoire/services/retrieval_telemetry.py | 12 +++++++ tests/test_retrieval_warnings.py | 13 ++++++++ tests/test_services_retrieval_telemetry.py | 34 ++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/grimoire/services/retrieval_telemetry.py b/src/grimoire/services/retrieval_telemetry.py index eaee032f..32daf22d 100644 --- a/src/grimoire/services/retrieval_telemetry.py +++ b/src/grimoire/services/retrieval_telemetry.py @@ -803,6 +803,9 @@ def _silent_surfaces(sources: dict, usage: dict, rule_usage: dict, return [] seen = set(sources) | set((usage.get("by_source") or {})) seen |= set((rule_usage.get("by_source") or {})) + # A getter shows nothing, so it is seen only by its pulls. + seen |= set((usage.get("pulled_by_source") or {})) + seen |= set((rule_usage.get("pulled_by_source") or {})) seen |= set(((system_usage or {}).get("by_source") or {})) return [ {"source": s, "kind": POINTS[s].kind, "what": POINTS[s].what} @@ -1486,6 +1489,9 @@ async def retrieval_summary( "distinct_notes_surfaced": int(distinct_surfaced or 0), "distinct_notes_pulled": int(distinct_pulled or 0), } + # Pulls per door. `by_source` is keyed by the surface that showed a record + # and leaves pulls out, so this is what says each getter is recording. + pulled_by_source: dict[str, int] = {} for event, source, n in urows: n = int(n) if event == SURFACED: @@ -1495,6 +1501,7 @@ async def retrieval_summary( usage["surfaced"] += n elif event == PULLED: usage["pulled"] += n + pulled_by_source[source] = pulled_by_source.get(source, 0) + n # The mcp_/rest_ split is load-bearing (see NoteUsageEvent's own # comment). "Is # this record dead weight?" is answered by ANY pull; "was that @@ -1513,6 +1520,7 @@ async def retrieval_summary( round(usage["pulled_by_agent"] / usage["surfaced"], 4) if usage["surfaced"] else None ) + usage["pulled_by_source"] = pulled_by_source # The same question, per surface — which is the one the top-level ratio # cannot answer. A corpus average of 0.05 is compatible with one surface @@ -1585,6 +1593,8 @@ async def retrieval_summary( "applied": 0, "departed": 0, "distinct_rules_acted": int(distinct_rules_acted or 0), + # Pulls per door, as on the note block. + "pulled_by_source": {}, } if rule_rows is None: # The FLAG is added, the shape is kept — matching `by_source_failed` @@ -1607,6 +1617,8 @@ async def retrieval_summary( rule_usage["surfaced"] += n elif event == RULE_PULLED: rule_usage["pulled"] += n + pulled = rule_usage["pulled_by_source"] + pulled[source] = pulled.get(source, 0) + n # Same split, and it carries MORE weight here than for notes. # The arm's whole claim is "this rule may apply to what you are # writing", and only an agent opening it says the claim landed. diff --git a/tests/test_retrieval_warnings.py b/tests/test_retrieval_warnings.py index f2335130..04889bb2 100644 --- a/tests/test_retrieval_warnings.py +++ b/tests/test_retrieval_warnings.py @@ -279,6 +279,19 @@ def test_a_point_seen_only_in_usage_counts_as_having_emitted() -> None: assert "enter_project" not in quiet +def test_a_getter_seen_only_by_its_pulls_counts_as_having_emitted() -> None: + """A getter surfaces nothing, so no `by_source` ever names it; its pulls + are the only sign it records. Without them every pull door reads silent + on every active window.""" + quiet = {p["source"] for p in _silent_surfaces( + {}, {"pulled_by_source": {"mcp_get_note": 3}}, + {"pulled_by_source": {"mcp_get_rule": 2}}, active=True)} + assert "mcp_get_note" not in quiet + assert "mcp_get_rule" not in quiet + # A door with no pulls is still reported. + assert "mcp_get_snippet" in quiet + + # ── settings parsing ────────────────────────────────────────────────────── @pytest.mark.parametrize("raw,fallback,want", [ diff --git a/tests/test_services_retrieval_telemetry.py b/tests/test_services_retrieval_telemetry.py index 2f80a94f..dfc87ba1 100644 --- a/tests/test_services_retrieval_telemetry.py +++ b/tests/test_services_retrieval_telemetry.py @@ -666,6 +666,40 @@ async def test_rule_by_source_says_which_arm_earns_its_lines(_dispose_engine): await cleanup() +@pytest.mark.integration +@pytest.mark.asyncio +async def test_a_pull_door_and_a_rules_listing_are_not_reported_silent( + _dispose_engine, +): + """One note pulled through its getter and one rules payload on get_task: + both points emitted, so neither may be named as silent.""" + from sqlalchemy import delete + + from grimoire.models import async_session + from grimoire.models.note_usage import NoteUsageEvent + from grimoire.services.retrieval_telemetry import retrieval_summary + + UID = 990017 + cleanup = await _rule_events(UID, [(5401, "surfaced", "get_task")]) + async with async_session() as s: + s.add_all([ + NoteUsageEvent(user_id=UID, note_id=11, event="surfaced", source="auto_inject"), + NoteUsageEvent(user_id=UID, note_id=11, event="pulled", source="mcp_get_note"), + ]) + await s.commit() + try: + out = await retrieval_summary(UID, days=30) + assert out["usage"]["pulled_by_source"] == {"mcp_get_note": 1} + quiet = {p["source"] for p in out["silent_surfaces"]} + assert "mcp_get_note" not in quiet + assert "get_task" not in quiet + finally: + await cleanup() + async with async_session() as s: + await s.execute(delete(NoteUsageEvent).where(NoteUsageEvent.user_id == UID)) + await s.commit() + + @pytest.mark.integration @pytest.mark.asyncio async def test_ambient_alone_reports_no_ratio(_dispose_engine):