From 950c93c5d42bbfca40b05605ffef93caa09dedeb Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 8 Sep 2026 10:34:57 -0400 Subject: [PATCH] fix(telemetry): the coverage helpers were defined inside the function they serve (#3712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_complete_from` and `_coverage` landed between `retrieval_summary`'s docstring and its body. Python does not object to that the way it looks like it should: blank lines do not close a block, so the whole remaining body — indented four spaces, sitting after `_coverage`'s `return` — became unreachable code INSIDE `_coverage`, and `retrieval_summary` became a function that is nothing but a docstring. The error surfaced three ways at once, none of which named the cause: fourteen F821s for `days` and `user_id` (real: those are `retrieval_summary`'s parameters, and the body no longer lived there), a SyntaxError on `async with` (real: `_coverage` is sync), and every test module that imports this file failing to collect. Moved both helpers above `retrieval_summary`, beside `_bucket` and `_round`, where the file's other helpers already are. No behaviour change — this is the code that was meant to be there. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/services/retrieval_telemetry.py | 58 +++++++++++----------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/scribe/services/retrieval_telemetry.py b/src/scribe/services/retrieval_telemetry.py index 2d88211..2a75c26 100644 --- a/src/scribe/services/retrieval_telemetry.py +++ b/src/scribe/services/retrieval_telemetry.py @@ -217,35 +217,6 @@ def _round(v, places: int = 4): return None if v is None else round(float(v), places) -async def retrieval_summary(user_id: int | None, *, days: int = 30) -> dict: - """What the retrieval telemetry says, per surface, over a window. - - Three aggregates side by side, each read from the table built for it — NOT - a join. `usage` is notes, `rule_usage` is rules, and they stay apart - because a few dozen eligible rules blended into thousands of notes is the - note ratio with noise on it (milestone 333). `NoteUsageEvent`'s own docstring is explicit that the two are - complements ("RetrievalLog tunes the threshold, this tunes the corpus") and - that RetrievalLog's JSONB `result_ids` "can't be indexed at" the per-note - grain. So the score distribution comes from `retrieval_logs` on its indexed - columns, and surfaced-vs-pulled comes from `note_usage_events` at the grain - it was built for. Reading each from its own table is both cheaper and more - honest than correlating them through JSONB. - - `usage["by_source"]` is the one join, and it stays INSIDE - `note_usage_events` — surfaced rows against pulled rows on note_id. That - answers "of the notes this surface chose, how many were opened", which the - top-level ratio averages away. It does not cross into `retrieval_logs`, so - the sentence above still holds. - - Scoped to one user's own telemetry. There is no sharing model for a - retrieval log — it records what THIS user's agent asked for, including the - query text — so an owner filter is the whole access rule here rather than a - shortcut around `services/access.py` (P#78 governs shared record kinds). - - Never raises: a telemetry readout that can break its caller is worse than - no readout. It does distinguish "no rows" from "the read failed", because - #2663 is exactly the bug where those two looked identical for weeks. - """ async def _complete_from(session, model, user_id) -> dict[str, Any]: """When each source in `model` started being recorded, and the instant the WHOLE table is complete from. Returns {source: earliest_row, "*": latest}. @@ -297,6 +268,35 @@ def _coverage(complete_from, since) -> dict: } +async def retrieval_summary(user_id: int | None, *, days: int = 30) -> dict: + """What the retrieval telemetry says, per surface, over a window. + + Three aggregates side by side, each read from the table built for it — NOT + a join. `usage` is notes, `rule_usage` is rules, and they stay apart + because a few dozen eligible rules blended into thousands of notes is the + note ratio with noise on it (milestone 333). `NoteUsageEvent`'s own docstring is explicit that the two are + complements ("RetrievalLog tunes the threshold, this tunes the corpus") and + that RetrievalLog's JSONB `result_ids` "can't be indexed at" the per-note + grain. So the score distribution comes from `retrieval_logs` on its indexed + columns, and surfaced-vs-pulled comes from `note_usage_events` at the grain + it was built for. Reading each from its own table is both cheaper and more + honest than correlating them through JSONB. + + `usage["by_source"]` is the one join, and it stays INSIDE + `note_usage_events` — surfaced rows against pulled rows on note_id. That + answers "of the notes this surface chose, how many were opened", which the + top-level ratio averages away. It does not cross into `retrieval_logs`, so + the sentence above still holds. + + Scoped to one user's own telemetry. There is no sharing model for a + retrieval log — it records what THIS user's agent asked for, including the + query text — so an owner filter is the whole access rule here rather than a + shortcut around `services/access.py` (P#78 governs shared record kinds). + + Never raises: a telemetry readout that can break its caller is worse than + no readout. It does distinguish "no rows" from "the read failed", because + #2663 is exactly the bug where those two looked identical for weeks. + """ since = datetime.now(timezone.utc) - timedelta(days=max(1, int(days))) out: dict = { "window_days": int(days),