diff --git a/frontend/src/components/rules/RuleListPane.vue b/frontend/src/components/rules/RuleListPane.vue
index 87a8311..82f63f6 100644
--- a/frontend/src/components/rules/RuleListPane.vue
+++ b/frontend/src/components/rules/RuleListPane.vue
@@ -1,16 +1,7 @@
@@ -431,7 +424,7 @@ const SNIPPET_DEAD_WEIGHT =
{{ driftBadge(s) }}
-
+
by {{ s.owner ?? "another user" }}
diff --git a/src/scribe/mcp/tools/lessons.py b/src/scribe/mcp/tools/lessons.py
index 0e507ac..24d46c9 100644
--- a/src/scribe/mcp/tools/lessons.py
+++ b/src/scribe/mcp/tools/lessons.py
@@ -17,7 +17,7 @@ from scribe.services import lessons as lessons_svc
from scribe.services import systems as systems_svc
from scribe.services import trash as trash_svc
from scribe.mcp.tools import systems as systems_tools
-from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
+from scribe.services.note_usage import attach_usage, record_pulled
# The payload shape lives in the service (`lesson_to_dict`), shared with the
@@ -65,7 +65,7 @@ async def list_lessons(
# per lesson (#4196). An agent listing lessons can see which of its own
# triggers are firing and which are not, which is the reading that leads to
# `update_lesson` rather than to a second lesson about the same failure.
- usage = await usage_for_notes([int(it["id"]) for it in labelled])
+ await attach_usage(labelled)
rows = [
{
"id": it["id"], "title": it["title"], "tags": it.get("tags", []),
@@ -73,7 +73,7 @@ async def list_lessons(
# Projected by `_note_to_item` straight off the `data` mirror —
# absent when the row carries none, rather than an empty string.
"when_to_apply": it.get("when_to_apply", ""),
- "usage": usage.get(int(it["id"]), empty_usage()),
+ "usage": it["usage"],
**({"shared": True, "owner": it.get("owner")} if it.get("shared") else {}),
}
for it in labelled
@@ -212,9 +212,7 @@ async def get_lesson(lesson_id: int, project_id: int = 0) -> dict:
# Read BEFORE the pull is recorded, so the number an agent is shown is the
# one that was true when it asked — otherwise every first read of a lesson
# reports a pull that is its own.
- out["usage"] = (await usage_for_notes([int(note.id)])).get(
- int(note.id), empty_usage()
- )
+ await attach_usage([out])
record_pulled(
user_id=uid, note_id=int(note.id),
source="mcp_get_lesson", project_id=project_id,
diff --git a/src/scribe/mcp/tools/snippets.py b/src/scribe/mcp/tools/snippets.py
index 5824d6b..f9a0b47 100644
--- a/src/scribe/mcp/tools/snippets.py
+++ b/src/scribe/mcp/tools/snippets.py
@@ -15,7 +15,7 @@ from scribe.mcp.tools import systems as systems_tools
from scribe.services import access as access_svc
from scribe.services import dedup as dedup_svc
from scribe.services import snippets as snippets_svc
-from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
+from scribe.services.note_usage import attach_usage, record_pulled
from scribe.services import systems as systems_svc
@@ -90,9 +90,7 @@ async def list_snippets(
repo=repo, path=path, symbol=symbol, verification=verification,
)
labeled = await access_svc.label_shared_items(uid, items)
- usage = await usage_for_notes([int(it["id"]) for it in labeled])
- for it in labeled:
- it["usage"] = usage.get(int(it["id"]), empty_usage())
+ await attach_usage(labeled)
return {"snippets": labeled, "total": total}
diff --git a/src/scribe/routes/knowledge.py b/src/scribe/routes/knowledge.py
index b30ce93..c7c79d3 100644
--- a/src/scribe/routes/knowledge.py
+++ b/src/scribe/routes/knowledge.py
@@ -7,6 +7,7 @@ from scribe.auth import get_current_user_id, login_required
from scribe.routes.utils import parse_pagination
from scribe.services.access import label_shared_items
from scribe.services.knowledge import FACET_TYPES
+from scribe.services.note_usage import attach_usage
logger = logging.getLogger(__name__)
@@ -62,10 +63,23 @@ async def list_knowledge():
offset=offset,
)
+ items = await label_shared_items(uid, items)
+ # The surfaced-vs-opened counts, on the list a person actually browses
+ # (#4230). `usage_for_notes` always worked on every note row, but only the
+ # snippet and rule lists ever attached it — and this is the ONLY lesson and
+ # note list in the UI, so those two kinds had the counter collected and
+ # shown nowhere. Attaching here rather than teaching `/api/lessons` a
+ # second time is what closes both holes at once: `/knowledge` is how notes,
+ # lessons and processes are all browsed.
+ #
+ # Mixed kinds is not a problem for this: usage keys on the note row, which
+ # every facet of this feed is.
+ await attach_usage(items)
+
return jsonify({
# Mark rows another user owns: this feed can be mixed-ownership, and an
# unmarked card reads as one the viewer wrote.
- "items": await label_shared_items(uid, items),
+ "items": items,
"total": total,
"page": page,
"per_page": limit,
diff --git a/src/scribe/routes/lessons.py b/src/scribe/routes/lessons.py
index e7e19d6..9137796 100644
--- a/src/scribe/routes/lessons.py
+++ b/src/scribe/routes/lessons.py
@@ -36,7 +36,7 @@ from scribe.services.access import (
describe_provenance,
label_shared_items,
)
-from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
+from scribe.services.note_usage import attach_usage, record_pulled
logger = logging.getLogger(__name__)
@@ -83,9 +83,7 @@ async def list_lessons_route():
# surfaced AND repeatedly opened, and the far commoner reading of the same
# row is that the trigger fires on the wrong situation, which `update_lesson`
# exists to fix.
- usage = await usage_for_notes([int(it["id"]) for it in items])
- for it in items:
- it["usage"] = usage.get(int(it["id"]), empty_usage())
+ await attach_usage(items)
return jsonify({"lessons": items, "total": total})
@@ -194,9 +192,7 @@ async def get_lesson_route(lesson_id: int):
uid, out["learned_from"]
)
out.update(await describe_provenance(uid, note))
- out["usage"] = (await usage_for_notes([lesson_id])).get(
- lesson_id, empty_usage()
- )
+ await attach_usage([out])
# Opening the detail view IS a pull — the operator chose to look. Tagged
# apart from the MCP sources so "an agent was handed it" and "a human read
# it" stay distinguishable; they mean different things for pruning (#2085).
diff --git a/src/scribe/routes/snippets.py b/src/scribe/routes/snippets.py
index ff28699..585d803 100644
--- a/src/scribe/routes/snippets.py
+++ b/src/scribe/routes/snippets.py
@@ -19,7 +19,7 @@ from scribe.routes.utils import not_found, parse_pagination
from scribe.services import dedup as dedup_svc
from scribe.services import snippets as snippets_svc
from scribe.services import systems as systems_svc
-from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
+from scribe.services.note_usage import attach_usage, record_pulled
from scribe.services.access import (
can_write_note,
describe_provenance,
@@ -75,9 +75,7 @@ async def list_snippets_route():
# One aggregate for the whole page — a per-row lookup here would be N+1 by
# construction. Every row gets the key, zero-filled, so the UI renders
# "never pulled" rather than having to treat a missing field as a state.
- usage = await usage_for_notes([int(it["id"]) for it in items])
- for it in items:
- it["usage"] = usage.get(int(it["id"]), empty_usage())
+ await attach_usage(items)
return jsonify({"snippets": items, "total": total})
@@ -168,9 +166,7 @@ async def get_snippet_route(snippet_id: int):
for s in await systems_svc.list_record_systems(note.user_id, snippet_id)
]
data.update(await describe_provenance(uid, note))
- data["usage"] = (await usage_for_notes([snippet_id])).get(
- snippet_id, empty_usage()
- )
+ await attach_usage([data])
# Opening the detail view IS a pull — the operator chose to look. Tagged
# apart from the MCP sources so "the agent reused it" and "a human read it"
# stay distinguishable; they mean different things for pruning (#2085).
diff --git a/src/scribe/services/note_usage.py b/src/scribe/services/note_usage.py
index 5b0400e..cdada2b 100644
--- a/src/scribe/services/note_usage.py
+++ b/src/scribe/services/note_usage.py
@@ -30,6 +30,7 @@ from __future__ import annotations
import asyncio
import logging
+from collections.abc import Sequence
from sqlalchemy import case, func, select
@@ -271,3 +272,64 @@ async def usage_for_notes(note_ids: list[int]) -> dict[int, dict]:
if latest and (slot["last_pulled_at"] or "") < latest:
slot["last_pulled_at"] = latest
return out
+
+
+def _row_id(row: dict, key: str) -> int | None:
+ """The note id on a payload row, or None when there is not one to read.
+
+ Skipping is deliberate: an id this cannot parse is not a reason to fail a
+ whole list, and GUESSING one would credit another record's counts to this
+ row — a wrong chip is worse than no chip, because it reads as a
+ measurement. `bool` is excluded explicitly because `int(True)` is 1, which
+ would quietly attach note #1's usage to a row carrying a flag.
+ """
+ raw = row.get(key)
+ if raw is None or isinstance(raw, bool):
+ return None
+ try:
+ return int(raw)
+ except (TypeError, ValueError):
+ return 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).
+
+ The one seam both doors and every record kind share. Before this, four call
+ sites carried their own copy of the same lines — two list routes and two
+ detail routes — and `/api/knowledge`, which is the list a person ACTUALLY
+ browses notes and lessons in, was about to become a fifth. That is how the
+ 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
+ over the id set; calling it per row would be N+1 by construction, which is
+ the one shape a list route must not have.
+
+ EVERY ROW GETS THE KEY, zero-filled, so a record predating the table reads
+ as "never surfaced, never pulled" rather than making the UI treat a missing
+ field as a state. `UsageBadge` then renders nothing at all below one
+ surfacing, because "0/0" would look like a verdict where there is only an
+ absence of evidence.
+
+ NO try/except HERE, deliberately — it is not an oversight. The fail-open
+ already lives one layer down: `usage_for_notes` catches its own failure,
+ reports it through `_report_failure("readout")` and returns the zero-filled
+ 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
+ silently-swallowed readout failure is exactly #2663 — every counter reading
+ zero in production for weeks while the writes landed fine.
+
+ Mutates in place and returns None, matching how the call sites already used
+ it: these rows are the payload, not a copy of it.
+
+ 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
+ shape that could drift from it.
+ """
+ pairs = [(row, _row_id(row, key)) for row in rows]
+ usage = await usage_for_notes([nid for _, nid in pairs if nid is not None])
+ for row, nid in pairs:
+ if nid is not None:
+ row["usage"] = usage.get(nid, empty_usage())
diff --git a/tests/test_usage_attach_seam.py b/tests/test_usage_attach_seam.py
new file mode 100644
index 0000000..c5d936d
--- /dev/null
+++ b/tests/test_usage_attach_seam.py
@@ -0,0 +1,205 @@
+"""One seam attaches `usage`, and every door that shows it uses that seam (#4230).
+
+WHAT WENT WRONG. `usage_for_notes` is named for notes and works on every note
+row. Yet the surfaced-vs-opened chip reached snippets and rules only: notes had
+it nowhere, and lessons had it collected but shown nowhere a person could
+reach, because the only lesson LIST in the UI is the Knowledge browse and that
+route never attached it.
+
+The cause was not any one missing line. Seven call sites carried their own copy
+of the same few lines — two REST lists, two REST details, two MCP lists, one
+MCP detail — and each read perfectly well on its own. Nobody was comparing
+them, so "which doors attach usage?" had no answer anywhere in the code. That
+is the same failure `test_system_tagging_door_parity.py` records for System
+tagging (#4249): whichever door nobody exercised for a kind is the one that
+never grew the feature, and a human reviewer does not reliably catch it because
+each door is only ever read alone.
+
+So this file asserts the PROPERTY, not the behaviour of one route: the attach
+logic exists once, and no door re-implements it. A kind added next month either
+goes through the seam or fails here.
+"""
+from __future__ import annotations
+
+import ast
+import pathlib
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+from scribe.services.note_usage import attach_usage, empty_usage
+
+ROOT = pathlib.Path(__file__).resolve().parents[1] / "src" / "scribe"
+
+# The aggregate the seam is built around. Calling it from a door is the shape
+# this file exists to prevent — not because the call is wrong, but because
+# seven of them drift.
+AGGREGATE = "usage_for_notes"
+
+
+def _counts(surfaced: int = 5, pulled: int = 0) -> dict:
+ u = empty_usage()
+ u["surfaced_count"] = surfaced
+ u["pull_count"] = pulled
+ return u
+
+
+def _aggregate_returns(mapping: dict[int, dict]) -> AsyncMock:
+ return patch(
+ "scribe.services.note_usage.usage_for_notes",
+ AsyncMock(return_value=mapping),
+ )
+
+
+# ── the seam itself ───────────────────────────────────────────────────────
+
+
+async def test_every_row_gets_the_key_even_with_no_events() -> None:
+ """Zero-filled, never absent. The UI must not have to tell "no events"
+ from "no field" — and `UsageBadge` renders nothing below one surfacing, so
+ an un-surfaced record is quiet without the caller doing anything."""
+ rows = [{"id": 1}, {"id": 2}]
+ with _aggregate_returns({1: _counts(surfaced=3)}):
+ await attach_usage(rows)
+ assert rows[0]["usage"]["surfaced_count"] == 3
+ assert rows[1]["usage"] == empty_usage()
+
+
+async def test_one_aggregate_for_the_whole_page() -> None:
+ """The N+1 guard. A per-row lookup here would be N+1 by construction, which
+ is the one shape a list route must not have — and it is invisible in
+ review, because the per-row version reads more naturally."""
+ rows = [{"id": n} for n in range(25)]
+ mock = AsyncMock(return_value={})
+ with patch("scribe.services.note_usage.usage_for_notes", mock):
+ await attach_usage(rows)
+ assert mock.await_count == 1, "usage must be read once per page, not per row"
+ assert sorted(mock.await_args.args[0]) == list(range(25))
+
+
+async def test_a_detail_payload_is_just_a_one_row_list() -> None:
+ """The single-record doors share the seam rather than keeping a second
+ shape beside it. Two shapes for one job is how the seven copies started."""
+ data = {"id": 7, "title": "x"}
+ with _aggregate_returns({7: _counts(surfaced=9, pulled=2)}):
+ await attach_usage([data])
+ assert data["usage"]["pull_count"] == 2
+
+
+async def test_a_row_with_no_id_is_skipped_rather_than_failing_the_list() -> None:
+ """An unusable id is not a reason to 500 a page of otherwise fine rows."""
+ rows = [{"id": 1}, {"title": "no id here"}]
+ with _aggregate_returns({1: _counts()}):
+ await attach_usage(rows)
+ assert "usage" in rows[0]
+ assert "usage" not in rows[1]
+
+
+async def test_a_boolean_is_not_an_id() -> None:
+ """`int(True)` is 1, so a row carrying a flag under the key would silently
+ be credited with note #1's counts. A wrong chip is worse than no chip: it
+ reads as a measurement."""
+ rows = [{"id": True}]
+ with _aggregate_returns({1: _counts(surfaced=40)}):
+ await attach_usage(rows)
+ assert "usage" not in rows[0]
+
+
+async def test_a_string_id_still_resolves() -> None:
+ """Payload rows come from several serialisers; one of them handing back a
+ stringified id should not silently drop the chip."""
+ rows = [{"id": "12"}]
+ with _aggregate_returns({12: _counts(surfaced=4)}):
+ await attach_usage(rows)
+ assert rows[0]["usage"]["surfaced_count"] == 4
+
+
+@pytest.mark.parametrize("key", ["note_id", "record_id"])
+async def test_the_key_can_be_named(key: str) -> None:
+ rows = [{key: 3}]
+ with _aggregate_returns({3: _counts()}):
+ await attach_usage(rows, key=key)
+ assert "usage" in rows[0]
+
+
+async def test_an_empty_page_asks_nothing_and_breaks_nothing() -> None:
+ mock = AsyncMock(return_value={})
+ with patch("scribe.services.note_usage.usage_for_notes", mock):
+ await attach_usage([])
+ assert mock.await_args.args[0] == []
+
+
+# ── the property: one seam, and every door uses it ────────────────────────
+
+
+def _calls(tree: ast.Module) -> set[str]:
+ out = set()
+ for node in ast.walk(tree):
+ if isinstance(node, ast.Call):
+ fn = node.func
+ name = fn.attr if isinstance(fn, ast.Attribute) else getattr(fn, "id", None)
+ if name:
+ out.add(name)
+ return out
+
+
+def _door_modules() -> list[pathlib.Path]:
+ return sorted(
+ [*(ROOT / "routes").glob("*.py"), *(ROOT / "mcp" / "tools").glob("*.py")]
+ )
+
+
+def test_no_door_calls_the_aggregate_directly() -> None:
+ """THE GUARD. Seven doors each called `usage_for_notes` and zero-filled by
+ hand; the eighth would have been `/api/knowledge`, and the chip would have
+ kept reaching some kinds and not others.
+
+ Keyed on the CALL, not on the text, so a module that merely names the
+ function in a comment explaining the seam is not a false positive — and a
+ hand-kept skip list, which would itself go stale, is not needed (rule 167).
+ """
+ offenders = []
+ for path in _door_modules():
+ if AGGREGATE in _calls(ast.parse(path.read_text())):
+ offenders.append(str(path.relative_to(ROOT.parent.parent)))
+ assert not offenders, (
+ f"these doors call {AGGREGATE}() themselves instead of attach_usage(); "
+ f"that is how the chip came to reach two record kinds out of four: "
+ f"{offenders}"
+ )
+
+
+# (module, the functions that return note-bearing payloads)
+#
+# Not a list of everything that COULD attach usage — a list of the doors that
+# demonstrably show it today. A door dropping its call silently is the exact
+# regression this pins.
+DOORS = [
+ ("routes/lessons.py", "list_lessons_route or get_lesson_route"),
+ ("routes/snippets.py", "list/get snippet routes"),
+ ("routes/knowledge.py", "list_knowledge — the only note & lesson list in the UI"),
+ ("mcp/tools/lessons.py", "list_lessons / get_lesson"),
+ ("mcp/tools/snippets.py", "list_snippets"),
+]
+
+
+@pytest.mark.parametrize(("module", "why"), DOORS)
+def test_every_door_that_shows_usage_goes_through_the_seam(module: str, why: str) -> None:
+ assert "attach_usage" in _calls(ast.parse((ROOT / module).read_text())), (
+ f"{module} no longer attaches usage ({why}). If that is deliberate, "
+ f"remove it from DOORS and say why; a door that silently stops "
+ f"attaching looks exactly like a corpus nobody uses."
+ )
+
+
+def test_the_knowledge_browse_is_covered_because_it_is_the_only_note_list() -> None:
+ """Pinned on its own, with the reason, because it is the non-obvious one.
+
+ `/api/lessons` already attached usage and it did not help: no view calls
+ it. `KnowledgeView` is the only list in the UI that renders notes and
+ lessons, so `/api/knowledge` is the only route through which those two
+ kinds can show the counter at all. Deleting this line would restore the
+ original bug while every other test here still passed.
+ """
+ assert any(m == "routes/knowledge.py" for m, _ in DOORS)
+ assert "attach_usage" in _calls(ast.parse((ROOT / "routes" / "knowledge.py").read_text()))