docs(6205): rule_usage comments state the reason, not the instance that taught it
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 1m6s
CI & Build / integration (push) Successful in 1m24s
CI & Build / Python tests (push) Successful in 2m11s
CI & Build / Build & push image (push) Successful in 26s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 1m6s
CI & Build / integration (push) Successful in 1m24s
CI & Build / Python tests (push) Successful in 2m11s
CI & Build / Build & push image (push) Successful in 26s
The module docstring keeps why rules need their own usage stream and why the ranked list is the one that is named, and drops the dated story of the bulk surfaces starting to emit, measured decline rates and the corrected earlier wording. Outcome docstrings describe the four states without milestone references. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,81 +1,60 @@
|
|||||||
"""Rule usage telemetry — did a surfaced rule ever get read?
|
"""Rule usage telemetry — did a surfaced rule ever get read?
|
||||||
|
|
||||||
The sibling of `note_usage`, for the one retrieval surface in Scribe that
|
The sibling of `note_usage`, for rules.
|
||||||
could not be measured at all.
|
|
||||||
|
|
||||||
Two event streams, deliberately independent:
|
Event streams, deliberately independent:
|
||||||
|
|
||||||
- SURFACED: the write-path standing-rule arm put this rule in front of the
|
- SURFACED: a surface put this rule in front of the agent — a ranked arm,
|
||||||
agent, unbidden, during a write.
|
unbidden, or a bulk delivery (see AMBIENT VS RANKED below).
|
||||||
- PULLED: someone then opened it in full (`get_rule`, or the REST detail
|
- PULLED: someone then opened it in full (`get_rule`, or the REST detail
|
||||||
route).
|
route).
|
||||||
|
|
||||||
WHY THIS ARM AND NOT ANOTHER. Every other surface declines most of the time —
|
WHY RULES NEED THEIR OWN STREAM. `retrieval_logs` records what the ranker
|
||||||
`write_path` returns nothing on 78% of calls, `reuse_slot` on 79%, auto-inject
|
scored, never whether the hint was any use, so even an honest clear-rate cannot
|
||||||
on 39%. The rule arm APPEARED never to have returned nothing (#3311), and this
|
settle whether a rule arm earns its lines. The ratio these two streams produce
|
||||||
docstring used to put that forward as the puzzle worth measuring: "either a
|
is the missing half, and without it any threshold change is a number picked
|
||||||
perfectly tuned surface or a bar it cannot fail to clear".
|
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
|
||||||
It was neither, and the correction belongs here rather than being quietly
|
threshold — a statistic that cannot vary, read as a finding about the corpus.)
|
||||||
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.
|
|
||||||
|
|
||||||
Design notes, mirroring `note_usage`:
|
Design notes, mirroring `note_usage`:
|
||||||
- Writes are fire-and-forget through `background.spawn`, so telemetry never
|
- Writes are fire-and-forget through `background.spawn`, so telemetry never
|
||||||
adds latency to — or can break — the surface it observes. This module does
|
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
|
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
|
- Failures degrade, but never SILENTLY. `report_telemetry_failure` logs at
|
||||||
WARNING and drops one AppLog row per process per site. #2663 is the record
|
WARNING and drops one AppLog row per process per site. A telemetry
|
||||||
of this exact subsystem class running at zero for weeks — indistinguishable
|
subsystem whose failures go to `logger.debug` can run at zero for weeks,
|
||||||
from "nobody uses this" — because every failure went to `logger.debug`.
|
indistinguishable from "nobody uses this".
|
||||||
- Reads (`usage_for_rules`) are awaited and aggregated in one round-trip for
|
- Reads (`usage_for_rules`) are awaited and aggregated in one round-trip for
|
||||||
a whole page, never per row.
|
a whole page, never per row.
|
||||||
|
|
||||||
AMBIENT VS RANKED. The note twin splits ranked surfacings from ambient ones
|
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
|
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
|
without choosing them, and counting those as surfacings makes recency read as
|
||||||
popularity (#2477). Rules have exactly that shape: the SessionStart preload,
|
popularity. Rules have exactly that shape: the SessionStart preload, and every
|
||||||
and every `rules_payload` surface hand over the whole
|
`rules_payload` surface hand over the whole applicable set at once, chosen by
|
||||||
applicable set at once, chosen by nobody.
|
nobody.
|
||||||
|
|
||||||
Until 2026-09-03 those bulk surfaces emitted nothing, and this module said so —
|
Those bulk surfaces emit too, as ambient. Without them the always-on set's
|
||||||
"an empty `AMBIENT_SOURCES` would be machinery pretending to a distinction the
|
token cost would be certain and its usefulness UNFALSIFIABLE — the one surface
|
||||||
data does not yet contain". True as far as it went, but it had a consequence
|
whose value is actually in question would be the one exempt from the
|
||||||
worth naming, because it is the reason the bucket exists now: the always-on
|
scoreboard that judges every other. The split is a readout-level `case()`, no
|
||||||
set's token cost was certain and its usefulness was UNFALSIFIABLE, permanently
|
migration, because `event` and `source` are plain Text with no CHECK
|
||||||
and by construction. The one surface whose value was actually in question was
|
constraint. `source` stays granular so a reader can still tell the preload
|
||||||
the one surface exempt from the scoreboard that judges every other.
|
from `enter_project` from a ranked arm.
|
||||||
|
|
||||||
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.
|
|
||||||
|
|
||||||
WHY THIS NAMES THE RANKED SOURCES AND THE TWIN NAMES THE AMBIENT ONES. A
|
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
|
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
|
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:
|
is which list changes more often — and here it is the ambient one. Ranked
|
||||||
there are TWO ranked rule sources (the write-path arm and the pre-tool arm)
|
sources are added when somebody builds a ranker, which is rare and deliberate;
|
||||||
against the seven bulk ones the preload alone contributes. Ranked sources are
|
bulk ones appear whenever a surface hands rules over, which is most of them.
|
||||||
added when somebody builds a ranker, which is rare and deliberate; bulk ones
|
Naming the rare, slow-moving half means a newly-added bulk surface defaults to
|
||||||
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`,
|
`ambient`, which merely under-counts it, instead of defaulting to `ranked`,
|
||||||
which would quietly pad the pull-through denominator with surfacings nobody
|
which would quietly pad the pull-through denominator with surfacings nobody
|
||||||
chose and make the arm look imprecise. Same argument #3191 and #3430 make
|
chose and make the arm look imprecise. Keep the list that must be remembered
|
||||||
against hand-kept lists: keep the list that must be remembered as short and as
|
as short and as slow-moving as possible.
|
||||||
slow-moving as possible.
|
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
@@ -99,15 +78,15 @@ logger = logging.getLogger(__name__)
|
|||||||
# Membership is the whole definition of the pull-through denominator: a ranked
|
# 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
|
# 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.
|
# 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
|
# The ranked half is read off the retrieval pipeline's specs: every rule arm
|
||||||
# step 5): every rule arm — the reply backstop included, which records only
|
# — the reply backstop included, which records only
|
||||||
# the rule that HELD the reply — the preference slot, which is a ranker's
|
# 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,
|
# 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
|
# whose own source lets its pull-through read apart from a direct match.
|
||||||
# (#4636). An arm added there is counted here by construction.
|
# An arm added there is counted here by construction.
|
||||||
RANKED_SOURCES = (
|
RANKED_SOURCES = (
|
||||||
*RULE_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
|
# not bulk either: somebody decided this rule applies at this moment, and
|
||||||
# that is exactly the claim a pull can confirm or refute. Its
|
# that is exactly the claim a pull can confirm or refute. Its
|
||||||
# pull-through is the evidence for whether a mount earns its line.
|
# 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?
|
"""Was this surfacing a bulk delivery rather than a ranked choice?
|
||||||
|
|
||||||
One definition, read by both the per-rule badge readout and the aggregate
|
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
|
in `retrieval_telemetry`, so the two cannot disagree about what
|
||||||
"surfaced" counted, which is the class of drift #3246 found across the
|
"surfaced" counts.
|
||||||
rules system.
|
|
||||||
|
|
||||||
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:
|
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
|
a module-private copy in each caller is exactly the second definition this
|
||||||
exists to prevent.
|
exists to prevent.
|
||||||
@@ -174,7 +152,7 @@ def record_rule_surfaced(
|
|||||||
care which kind it is recording.
|
care which kind it is recording.
|
||||||
|
|
||||||
`detail` maps a rule id to what to record beside its row — the moment a
|
`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.
|
read per moment rather than only per arm.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
@@ -225,14 +203,13 @@ def record_rule_outcome(
|
|||||||
source: str,
|
source: str,
|
||||||
detail: str = "",
|
detail: str = "",
|
||||||
) -> None:
|
) -> 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 third stream. `surfaced` says the system offered a rule; `pulled`
|
||||||
the system offered a rule; `pulled` says somebody opened it. Neither says
|
says somebody opened it. Neither says whether it made any difference, so
|
||||||
whether it made any difference, so a rule that fires constantly and is
|
without this a rule that fires constantly and is always obeyed and a rule
|
||||||
always obeyed and a rule that fires constantly and is never obeyed have,
|
that fires constantly and is never obeyed produce identical telemetry.
|
||||||
until now, produced identical telemetry. The second is far the more
|
The second is far the more urgent.
|
||||||
urgent and is precisely the one the readout could not name.
|
|
||||||
|
|
||||||
Two outcomes, because there are only two a judge can honestly report:
|
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.
|
states this exists to separate.
|
||||||
|
|
||||||
THERE IS NO THIRD CALL, and the absence is the design. Read-and-silently-
|
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
|
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
|
would not be ignoring it. It is derived: a rule pulled, with no outcome
|
||||||
behind it. See `outcome_state`.
|
behind it. See `outcome_state`.
|
||||||
|
|
||||||
Guarded rather than trusting: a bad outcome or a reasonless departure is
|
Guarded rather than trusting: a bad outcome or a reasonless departure is
|
||||||
dropped and REPORTED, never written. Telemetry that lies is worse than
|
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.
|
reason is a lie the readout cannot detect.
|
||||||
"""
|
"""
|
||||||
if outcome not in OUTCOMES:
|
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
|
# 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
|
# ordered by how much the system actually knows; recorded outcomes are what
|
||||||
# new — the point of the milestone is that UNACTED used to be invisible
|
# keep UNACTED from hiding inside FOLLOWED.
|
||||||
# inside APPLIED.
|
|
||||||
UNREAD = "unread" # surfaced, never opened
|
UNREAD = "unread" # surfaced, never opened
|
||||||
UNACTED = "unacted" # opened, and nothing recorded after — the blind spot
|
UNACTED = "unacted" # opened, and nothing recorded after — the blind spot
|
||||||
FOLLOWED = "followed" # opened and applied
|
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:
|
def outcome_state(usage: dict) -> str:
|
||||||
"""The three states milestone 419 asked to be able to tell apart, plus
|
"""Which of the four states a rule's usage is in.
|
||||||
the one that already existed.
|
|
||||||
|
|
||||||
Pure, and reading only the aggregate `usage_for_rules` already returns —
|
Pure, and reading only the aggregate `usage_for_rules` already returns —
|
||||||
so the readout, the badge and any later session summary all answer this
|
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
|
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.
|
PRECEDENCE, and it is deliberate: a departure outranks an application.
|
||||||
A rule both applied and departed from in the same window is a rule
|
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
|
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
|
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
|
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
|
than never having been opened at all, and without recorded outcomes it
|
||||||
previously indistinguishable from compliance.
|
is indistinguishable from compliance.
|
||||||
"""
|
"""
|
||||||
if int(usage.get("departed_count") or 0):
|
if int(usage.get("departed_count") or 0):
|
||||||
return DEPARTED_FROM
|
return DEPARTED_FROM
|
||||||
@@ -338,9 +313,9 @@ def empty_rule_usage() -> dict:
|
|||||||
"surfaced_count": 0,
|
"surfaced_count": 0,
|
||||||
"ambient_count": 0,
|
"ambient_count": 0,
|
||||||
"pull_count": 0,
|
"pull_count": 0,
|
||||||
# The outcome half (#4212). Zero here means "nothing recorded", which
|
# The outcome half. Zero here means "nothing recorded", which for a
|
||||||
# for a rule that was also never pulled is simply silence — and for
|
# rule that was also never pulled is simply silence — and for one
|
||||||
# one that WAS pulled is the blind spot this milestone is named for.
|
# that WAS pulled is the blind spot (UNACTED).
|
||||||
# `outcome_state` is what tells those apart; no caller should be
|
# `outcome_state` is what tells those apart; no caller should be
|
||||||
# reading these counts raw to decide it.
|
# 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
|
# 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 —
|
# fill would read as "no reason given" on every rule that has one —
|
||||||
# a permanently-null field that lies. The readout that needs the
|
# a permanently-null field that lies. The readout that needs the
|
||||||
# prose reads the rows (#4213).
|
# prose reads the rows.
|
||||||
"applied_count": 0,
|
"applied_count": 0,
|
||||||
"departed_count": 0,
|
"departed_count": 0,
|
||||||
"last_outcome_at": None,
|
"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
|
# GROUP BY — a second `case()` instance there renders its own expanding-IN
|
||||||
# bind names under asyncpg, so the database sees two DIFFERENT expressions
|
# bind names under asyncpg, so the database sees two DIFFERENT expressions
|
||||||
# and rejects the query with a GroupingError. The note twin carries the
|
# 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
|
# same warning for the same reason: the rejection is swallowed and every
|
||||||
# rejection was swallowed and every counter read zero in production while
|
# counter reads zero while the writes are landing fine.
|
||||||
# the writes were landing fine.
|
|
||||||
ambient = case(
|
ambient = case(
|
||||||
(RuleUsageEvent.source.notin_(RANKED_SOURCES), True),
|
(RuleUsageEvent.source.notin_(RANKED_SOURCES), True),
|
||||||
else_=False,
|
else_=False,
|
||||||
@@ -406,7 +380,7 @@ async def usage_for_rules(rule_ids: list[int]) -> dict[int, dict]:
|
|||||||
except Exception:
|
except Exception:
|
||||||
# A telemetry readout must not be able to break the list it decorates —
|
# 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
|
# 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")
|
await _report_failure("readout")
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user