docs(6205): note_usage comments state the reason, not the instance that taught it
The silent-outage story (#2663), the ranked/ambient split (#2477), the transfer evidence (#3735) and the attach_usage consolidation (#4230) are stated as the property each guards, without the incident. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -15,13 +15,12 @@ Design notes (mirrors retrieval_telemetry, for the same reasons):
|
|||||||
- Writes are fire-and-forget. `record_surfaced` / `record_pulled` extract
|
- Writes are fire-and-forget. `record_surfaced` / `record_pulled` extract
|
||||||
plain ints synchronously and schedule the insert as a background task, so
|
plain ints synchronously and schedule the insert as a background task, so
|
||||||
telemetry never adds latency to — or can break — the surface it observes.
|
telemetry never adds latency to — or can break — the surface it observes.
|
||||||
- Failures degrade, but they must not degrade SILENTLY. The original version
|
- Failures degrade, but they must not degrade SILENTLY. Swallowed into a
|
||||||
swallowed everything into logger.debug, and the deployed instance ran with
|
debug log, a failure leaves every counter at zero while surfacing fires —
|
||||||
every counter at zero for weeks while surfacing demonstrably fired — a
|
a total outage indistinguishable from "nobody uses this". A subsystem
|
||||||
total outage indistinguishable from "nobody uses this" (#2663). A
|
whose every failure mode is invisible cannot report its own death, so
|
||||||
subsystem whose every failure mode is invisible cannot report its own
|
failures log at WARNING and drop one AppLog error row per process per
|
||||||
death, so failures now log at WARNING and drop one AppLog error row per
|
site, where the admin UI shows it.
|
||||||
process per site, where the admin UI shows it.
|
|
||||||
- Reads (`usage_for_notes`) are NOT fire-and-forget — a readout the caller
|
- Reads (`usage_for_notes`) are NOT fire-and-forget — a readout the caller
|
||||||
awaits, aggregated in one round-trip for a whole page of snippets rather
|
awaits, aggregated in one round-trip for a whole page of snippets rather
|
||||||
than per row.
|
than per row.
|
||||||
@@ -151,7 +150,7 @@ def record_pulled(
|
|||||||
`project_id` is where the READER was — the caller's active project, never
|
`project_id` is where the READER was — the caller's active project, never
|
||||||
the record's own. Compared against the record's `project_id`, it answers
|
the record's own. Compared against the record's `project_id`, it answers
|
||||||
whether this was opened somewhere other than where it was written, which
|
whether this was opened somewhere other than where it was written, which
|
||||||
is the evidence that a record TRANSFERRED (milestone 385, #3735).
|
is the evidence that a record TRANSFERRED.
|
||||||
|
|
||||||
Unlike a surfacing arm, a getter only knows what it was handed, so this
|
Unlike a surfacing arm, a getter only knows what it was handed, so this
|
||||||
stays optional and null is an ordinary answer meaning "not reported". A
|
stays optional and null is an ordinary answer meaning "not reported". A
|
||||||
@@ -178,7 +177,7 @@ def record_pulled(
|
|||||||
# top-N-by-recency happen to be; the skill sync installs every Process the
|
# top-N-by-recency happen to be; the skill sync installs every Process the
|
||||||
# operator can reach. Counting those alongside auto-inject would make a note's
|
# operator can reach. Counting those alongside auto-inject would make a note's
|
||||||
# surfaced_count dominated by "it was recently updated in a project you
|
# surfaced_count dominated by "it was recently updated in a project you
|
||||||
# opened", and dead-weight detection would read that as popularity (#2477).
|
# opened", and dead-weight detection would read that as popularity.
|
||||||
# They still matter — a pull that follows one must not float unattributed — so
|
# They still matter — a pull that follows one must not float unattributed — so
|
||||||
# they land in their own bucket rather than not landing at all.
|
# they land in their own bucket rather than not landing at all.
|
||||||
AMBIENT_SOURCES = ("enter_project", "process_skill_sync")
|
AMBIENT_SOURCES = ("enter_project", "process_skill_sync")
|
||||||
@@ -191,14 +190,13 @@ def empty_usage() -> dict:
|
|||||||
reads as "never surfaced, never pulled" rather than as a missing key.
|
reads as "never surfaced, never pulled" rather than as a missing key.
|
||||||
|
|
||||||
`surfaced_count` is RANKED surfacings only — a scored surface chose this
|
`surfaced_count` is RANKED surfacings only — a scored surface chose this
|
||||||
record. `ambient_count` is the rest (see AMBIENT_SOURCES). The split is the
|
record. `ambient_count` is the rest (see AMBIENT_SOURCES). The split exists
|
||||||
readout half of #2477: the "high surfaced, zero pulls → dead weight"
|
because the "high surfaced, zero pulls → dead weight" reading is only valid over surfacings that were choices.
|
||||||
reading is only valid over surfacings that were choices.
|
|
||||||
|
|
||||||
`surfaced_away_count` / `pulled_away_count` are the subsets that happened
|
`surfaced_away_count` / `pulled_away_count` are the subsets that happened
|
||||||
on a project other than the one the record was written on — the evidence
|
on a project other than the one the record was written on — the evidence
|
||||||
that a record TRANSFERRED, which is the claim the lesson kind rests on
|
that a record TRANSFERRED, which is the claim the lesson kind rests on.
|
||||||
(milestone 385, #3735). Counted only where both projects are known: an
|
Counted only where both projects are known: an
|
||||||
event with no reader project, or a record with no project of its own,
|
event with no reader project, or a record with no project of its own,
|
||||||
cannot speak to "away" and is left out rather than guessed.
|
cannot speak to "away" and is left out rather than guessed.
|
||||||
"""
|
"""
|
||||||
@@ -231,9 +229,8 @@ async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]:
|
|||||||
# distinct source. ONE labelled expression, grouped by its label — a second
|
# distinct source. ONE labelled expression, grouped by its label — a second
|
||||||
# case() instance in GROUP BY renders with its own expanding-IN bind names
|
# case() instance in GROUP BY renders with its own expanding-IN bind names
|
||||||
# under asyncpg, so the database sees two DIFFERENT expressions and rejects
|
# under asyncpg, so the database sees two DIFFERENT expressions and rejects
|
||||||
# the query with a GroupingError. That rejection was swallowed, which is
|
# the query with a GroupingError — and a swallowed rejection reads as
|
||||||
# how every counter read zero in production while the writes were landing
|
# every counter at zero while the writes land fine.
|
||||||
# fine (#2663).
|
|
||||||
ambient = case(
|
ambient = case(
|
||||||
(NoteUsageEvent.source.in_(AMBIENT_SOURCES), True),
|
(NoteUsageEvent.source.in_(AMBIENT_SOURCES), True),
|
||||||
else_=False,
|
else_=False,
|
||||||
@@ -262,7 +259,7 @@ async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]:
|
|||||||
# it needs the join to `notes` and the first must keep counting
|
# it needs the join to `notes` and the first must keep counting
|
||||||
# events whose note has since been deleted (the table is FK-free
|
# events whose note has since been deleted (the table is FK-free
|
||||||
# so that evidence outlives the row). Ranked surfacings only, for
|
# so that evidence outlives the row). Ranked surfacings only, for
|
||||||
# the same reason surfaced_count is (#2477).
|
# the same reason surfaced_count is.
|
||||||
away_rows = (
|
away_rows = (
|
||||||
await session.execute(
|
await session.execute(
|
||||||
select(
|
select(
|
||||||
@@ -284,7 +281,7 @@ async def usage_for_notes(note_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
|
||||||
|
|
||||||
@@ -335,15 +332,12 @@ def _row_id(row: dict, key: str) -> int | None:
|
|||||||
|
|
||||||
|
|
||||||
async def attach_usage(rows: Sequence[dict], *, key: str = "id") -> None:
|
async def attach_usage(rows: Sequence[dict], *, key: str = "id") -> None:
|
||||||
"""Add `usage` to every row of a payload a door is about to return (#4230).
|
"""Add `usage` to every row of a payload a door is about to return.
|
||||||
|
|
||||||
The one seam both doors and every record kind share. Before this, four call
|
The one seam both doors and every record kind share — list routes, detail
|
||||||
sites carried their own copy of the same lines — two list routes and two
|
routes and `/api/knowledge`, the list a person actually browses notes and
|
||||||
detail routes — and `/api/knowledge`, which is the list a person ACTUALLY
|
lessons in. A copy per call site lets the chip reach some record kinds and
|
||||||
browses notes and lessons in, was about to become a fifth. That is how the
|
not others while each door reads fine on its own.
|
||||||
chip came to reach two record kinds out of four while a service named
|
|
||||||
`usage_for_notes` worked on all of them: each door read fine on its own,
|
|
||||||
and nobody was comparing them.
|
|
||||||
|
|
||||||
ONE AGGREGATE FOR THE WHOLE PAGE. `usage_for_notes` is a single GROUP BY
|
ONE AGGREGATE FOR THE WHOLE PAGE. `usage_for_notes` is a single GROUP BY
|
||||||
over the id set; calling it per row would be N+1 by construction, which is
|
over the id set; calling it per row would be N+1 by construction, which is
|
||||||
@@ -360,11 +354,10 @@ async def attach_usage(rows: Sequence[dict], *, key: str = "id") -> None:
|
|||||||
reports it through `_report_failure("readout")` and returns the zero-filled
|
reports it through `_report_failure("readout")` and returns the zero-filled
|
||||||
map, so a broken readout degrades without breaking the list it decorates.
|
map, so a broken readout degrades without breaking the list it decorates.
|
||||||
Wrapping it again would swallow the REPORT along with the error, and a
|
Wrapping it again would swallow the REPORT along with the error, and a
|
||||||
silently-swallowed readout failure is exactly #2663 — every counter reading
|
silently-swallowed readout failure reads as every counter at zero while
|
||||||
zero in production for weeks while the writes landed fine.
|
the writes land fine.
|
||||||
|
|
||||||
Mutates in place and returns None, matching how the call sites already used
|
Mutates in place and returns None: these rows are the payload, not a copy of it.
|
||||||
it: these rows are the payload, not a copy of it.
|
|
||||||
|
|
||||||
A detail payload is just a one-row list — `await attach_usage([data])` —
|
A detail payload is just a one-row list — `await attach_usage([data])` —
|
||||||
so the single-record doors share this seam rather than keeping a second
|
so the single-record doors share this seam rather than keeping a second
|
||||||
|
|||||||
Reference in New Issue
Block a user