diff --git a/src/scribe/services/rule_usage.py b/src/scribe/services/rule_usage.py index e9437798..7ba2c5c7 100644 --- a/src/scribe/services/rule_usage.py +++ b/src/scribe/services/rule_usage.py @@ -1,81 +1,60 @@ """Rule usage telemetry — did a surfaced rule ever get read? -The sibling of `note_usage`, for the one retrieval surface in Scribe that -could not be measured at all. +The sibling of `note_usage`, for rules. -Two event streams, deliberately independent: +Event streams, deliberately independent: - - SURFACED: the write-path standing-rule arm put this rule in front of the - agent, unbidden, during a write. + - SURFACED: a surface put this rule in front of the agent — a ranked arm, + unbidden, or a bulk delivery (see AMBIENT VS RANKED below). - PULLED: someone then opened it in full (`get_rule`, or the REST detail route). -WHY THIS ARM AND NOT ANOTHER. Every other surface declines most of the time — -`write_path` returns nothing on 78% of calls, `reuse_slot` on 79%, auto-inject -on 39%. The rule arm APPEARED never to have returned nothing (#3311), and this -docstring used to put that forward as the puzzle worth measuring: "either a -perfectly tuned surface or a bar it cannot fail to clear". - -It was neither, and the correction belongs here rather than being quietly -deleted. The arm wrote its `retrieval_logs` row only on calls that FOUND -something (#3497), so `zero_result_calls` sat at 0 and `cleared_threshold` at -`calls` because of the shape of the code — at any threshold whatsoever. A -statistic that could not vary was read as a finding about the corpus. It is the -#2663 failure mode one level up: there the broken readout was a zero, here it -was a hundred percent, which is far better camouflage. - -The reason to measure this arm survives the correction, and is stronger for it. -`retrieval_logs` records what the ranker scored, never whether the hint was any -use, so even an honest clear-rate would not settle the question. The ratio these -two streams produce is the missing half, and without it any threshold change is -a number picked off a histogram. +WHY RULES NEED THEIR OWN STREAM. `retrieval_logs` records what the ranker +scored, never whether the hint was any use, so even an honest clear-rate cannot +settle whether a rule arm earns its lines. The ratio these two streams produce +is the missing half, and without it any threshold change is a number picked +off a histogram. (A clear-rate can also mislead outright: an arm that logs +only the calls that FOUND something reports a decline count of zero at any +threshold — a statistic that cannot vary, read as a finding about the corpus.) Design notes, mirroring `note_usage`: - Writes are fire-and-forget through `background.spawn`, so telemetry never adds latency to — or can break — the surface it observes. This module does NOT carry its own copy of the strong-reference dance; `background` is the - one place that gets it right, and a fourth copy is how one of them drifts. + one place that gets it right, and another copy is how one of them drifts. - Failures degrade, but never SILENTLY. `report_telemetry_failure` logs at - WARNING and drops one AppLog row per process per site. #2663 is the record - of this exact subsystem class running at zero for weeks — indistinguishable - from "nobody uses this" — because every failure went to `logger.debug`. + WARNING and drops one AppLog row per process per site. A telemetry + subsystem whose failures go to `logger.debug` can run at zero for weeks, + indistinguishable from "nobody uses this". - Reads (`usage_for_rules`) are awaited and aggregated in one round-trip for a whole page, never per row. AMBIENT VS RANKED. The note twin splits ranked surfacings from ambient ones because `enter_project` and the skill sync put records in front of the agent without choosing them, and counting those as surfacings makes recency read as -popularity (#2477). Rules have exactly that shape: the SessionStart preload, -and every `rules_payload` surface hand over the whole -applicable set at once, chosen by nobody. +popularity. Rules have exactly that shape: the SessionStart preload, and every +`rules_payload` surface hand over the whole applicable set at once, chosen by +nobody. -Until 2026-09-03 those bulk surfaces emitted nothing, and this module said so — -"an empty `AMBIENT_SOURCES` would be machinery pretending to a distinction the -data does not yet contain". True as far as it went, but it had a consequence -worth naming, because it is the reason the bucket exists now: the always-on -set's token cost was certain and its usefulness was UNFALSIFIABLE, permanently -and by construction. The one surface whose value was actually in question was -the one surface exempt from the scoreboard that judges every other. - -They emit now. The split is the readout-level change the old note promised — a -`case()`, no migration, because `event` and `source` are plain Text with no -CHECK constraint. `source` stays granular so a reader can still tell the -preload from `enter_project` from the ranked arm. +Those bulk surfaces emit too, as ambient. Without them the always-on set's +token cost would be certain and its usefulness UNFALSIFIABLE — the one surface +whose value is actually in question would be the one exempt from the +scoreboard that judges every other. The split is a readout-level `case()`, no +migration, because `event` and `source` are plain Text with no CHECK +constraint. `source` stays granular so a reader can still tell the preload +from `enter_project` from a ranked arm. WHY THIS NAMES THE RANKED SOURCES AND THE TWIN NAMES THE AMBIENT ONES. A deliberate divergence, on the failure mode rather than on symmetry. Both shapes fail silently when someone adds a surface and forgets the list, so the question -is which list changes more often — and here it is emphatically the ambient one: -there are TWO ranked rule sources (the write-path arm and the pre-tool arm) -against the seven bulk ones the preload alone contributes. Ranked sources are -added when somebody builds a ranker, which is rare and deliberate; bulk ones -appear whenever a surface hands rules over, which is most of them. Naming the -rare, slow-moving half means a newly-added bulk surface defaults to +is which list changes more often — and here it is the ambient one. Ranked +sources are added when somebody builds a ranker, which is rare and deliberate; +bulk ones appear whenever a surface hands rules over, which is most of them. +Naming the rare, slow-moving half means a newly-added bulk surface defaults to `ambient`, which merely under-counts it, instead of defaulting to `ranked`, which would quietly pad the pull-through denominator with surfacings nobody -chose and make the arm look imprecise. Same argument #3191 and #3430 make -against hand-kept lists: keep the list that must be remembered as short and as -slow-moving as possible. +chose and make the arm look imprecise. Keep the list that must be remembered +as short and as slow-moving as possible. """ from __future__ import annotations @@ -99,15 +78,15 @@ logger = logging.getLogger(__name__) # Membership is the whole definition of the pull-through denominator: a ranked # surfacing is a claim ("this rule may apply to what you are doing") that a pull # can confirm or refute, while an ambient one is a delivery nobody decided on. -# The ranked half is read off the retrieval pipeline's specs (milestone 456 -# step 5): every rule arm — the reply backstop included, which records only +# The ranked half is read off the retrieval pipeline's specs: every rule arm +# — the reply backstop included, which records only # the rule that HELD the reply — the preference slot, which is a ranker's # choice twice over, and the rule reached through a confirmed lesson link, -# whose own source lets its pull-through read apart from a direct match -# (#4636). An arm added there is counted here by construction. +# whose own source lets its pull-through read apart from a direct match. +# An arm added there is counted here by construction. RANKED_SOURCES = ( *RULE_RANKED_SOURCES, - # A rule mounted on a moment (milestone 458). Not a ranker's pick, but + # A rule mounted on a moment. Not a ranker's pick, but # not bulk either: somebody decided this rule applies at this moment, and # that is exactly the claim a pull can confirm or refute. Its # pull-through is the evidence for whether a mount earns its line. @@ -119,11 +98,10 @@ def is_ambient(source: str) -> bool: """Was this surfacing a bulk delivery rather than a ranked choice? One definition, read by both the per-rule badge readout and the aggregate - in `retrieval_telemetry` — the two used to be able to disagree about what - "surfaced" counted, which is the class of drift #3246 found across the - rules system. + in `retrieval_telemetry`, so the two cannot disagree about what + "surfaced" counts. - Sync and pure, per the service canon (#2860), but deliberately PUBLIC where + Sync and pure, per the service canon, but deliberately PUBLIC where that canon says such helpers stay `_private`. The departure is the point: a module-private copy in each caller is exactly the second definition this exists to prevent. @@ -174,7 +152,7 @@ def record_rule_surfaced( care which kind it is recording. `detail` maps a rule id to what to record beside its row — the moment a - mounted rule arrived at (milestone 458), so a mount's pull-through can be + mounted rule arrived at, so a mount's pull-through can be read per moment rather than only per arm. """ try: @@ -225,14 +203,13 @@ def record_rule_outcome( source: str, detail: str = "", ) -> None: - """Fire-and-forget: record what a rule ACTUALLY CHANGED (#4212). + """Fire-and-forget: record what a rule ACTUALLY CHANGED. - The third stream, and the one milestone 419 exists for. `surfaced` says - the system offered a rule; `pulled` says somebody opened it. Neither says - whether it made any difference, so a rule that fires constantly and is - always obeyed and a rule that fires constantly and is never obeyed have, - until now, produced identical telemetry. The second is far the more - urgent and is precisely the one the readout could not name. + The third stream. `surfaced` says the system offered a rule; `pulled` + says somebody opened it. Neither says whether it made any difference, so + without this a rule that fires constantly and is always obeyed and a rule + that fires constantly and is never obeyed produce identical telemetry. + The second is far the more urgent. Two outcomes, because there are only two a judge can honestly report: @@ -245,14 +222,14 @@ def record_rule_outcome( states this exists to separate. THERE IS NO THIRD CALL, and the absence is the design. Read-and-silently- - unchanged is real — it is the failure this milestone was opened on — but + unchanged is real — it is the failure this stream exists to expose — but it cannot be reported, because an agent that knew it was ignoring a rule would not be ignoring it. It is derived: a rule pulled, with no outcome behind it. See `outcome_state`. Guarded rather than trusting: a bad outcome or a reasonless departure is dropped and REPORTED, never written. Telemetry that lies is worse than - telemetry that is missing (#2663), and a `departed` row with an empty + telemetry that is missing, and a `departed` row with an empty reason is a lie the readout cannot detect. """ if outcome not in OUTCOMES: @@ -280,9 +257,8 @@ def record_rule_outcome( # What a rule's usage says happened to it, in one word. The four states are -# ordered by how much the system actually knows, and only the last two are -# new — the point of the milestone is that UNACTED used to be invisible -# inside APPLIED. +# ordered by how much the system actually knows; recorded outcomes are what +# keep UNACTED from hiding inside FOLLOWED. UNREAD = "unread" # surfaced, never opened UNACTED = "unacted" # opened, and nothing recorded after — the blind spot FOLLOWED = "followed" # opened and applied @@ -290,13 +266,12 @@ DEPARTED_FROM = "departed" # opened and deliberately not followed, with a why def outcome_state(usage: dict) -> str: - """The three states milestone 419 asked to be able to tell apart, plus - the one that already existed. + """Which of the four states a rule's usage is in. Pure, and reading only the aggregate `usage_for_rules` already returns — so the readout, the badge and any later session summary all answer this question the same way. Two callers computing "was this followed" from raw - counts is the drift #3246 found across the rules system, arriving again. + counts would drift. PRECEDENCE, and it is deliberate: a departure outranks an application. A rule both applied and departed from in the same window is a rule @@ -306,8 +281,8 @@ def outcome_state(usage: dict) -> str: UNACTED is the derived state and the reason this function exists. It is not "no data"; it is a rule that was surfaced, deliberately OPENED, and then left no trace of having mattered. That is a much stronger signal - than never having been opened at all, and it is the signal that was - previously indistinguishable from compliance. + than never having been opened at all, and without recorded outcomes it + is indistinguishable from compliance. """ if int(usage.get("departed_count") or 0): return DEPARTED_FROM @@ -338,9 +313,9 @@ def empty_rule_usage() -> dict: "surfaced_count": 0, "ambient_count": 0, "pull_count": 0, - # The outcome half (#4212). Zero here means "nothing recorded", which - # for a rule that was also never pulled is simply silence — and for - # one that WAS pulled is the blind spot this milestone is named for. + # The outcome half. Zero here means "nothing recorded", which for a + # rule that was also never pulled is simply silence — and for one + # that WAS pulled is the blind spot (UNACTED). # `outcome_state` is what tells those apart; no caller should be # reading these counts raw to decide it. # @@ -349,7 +324,7 @@ def empty_rule_usage() -> dict: # DISTINCT ON alongside it, and a key that the aggregate could never # fill would read as "no reason given" on every rule that has one — # a permanently-null field that lies. The readout that needs the - # prose reads the rows (#4213). + # prose reads the rows. "applied_count": 0, "departed_count": 0, "last_outcome_at": None, @@ -377,9 +352,8 @@ async def usage_for_rules(rule_ids: list[int]) -> dict[int, dict]: # GROUP BY — a second `case()` instance there renders its own expanding-IN # bind names under asyncpg, so the database sees two DIFFERENT expressions # and rejects the query with a GroupingError. The note twin carries the - # same warning for the same reason, and #2663 is what it cost: the - # rejection was swallowed and every counter read zero in production while - # the writes were landing fine. + # same warning for the same reason: the rejection is swallowed and every + # counter reads zero while the writes are landing fine. ambient = case( (RuleUsageEvent.source.notin_(RANKED_SOURCES), True), else_=False, @@ -406,7 +380,7 @@ async def usage_for_rules(rule_ids: list[int]) -> dict[int, dict]: except Exception: # A telemetry readout must not be able to break the list it decorates — # but it must say it failed, or a broken readout is indistinguishable - # from a corpus nobody uses (#2663). + # from a corpus nobody uses. await _report_failure("readout") return out