From 6627cfc2f02d90cdfed0a66a9d45f7207922d81a Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Wed, 2 Sep 2026 18:37:55 -0400
Subject: [PATCH] feat(rules): a usage badge on the rule list, and the badge
 becomes canon instead of a second copy (#3319)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Milestone 333 step 5, and rule 27 — the counter had a tuning point from step
4 and no operator-facing one until now.

The task said to reuse the snippet badge's classes rather than mint a parallel
set, citing the eight duplicated CSS families the ledger already carries
(#3207). `.usage-tag` lived in SnippetListView's SCOPED block, so "reuse" was
not available: copying it into the rule pane would have been the ninth family,
and importing it is not a thing a scoped block permits. So it was promoted
rather than copied.

Three pieces, each of which existed once and now exists once:

- `components.css` gains `.usage-tag` / `.usage-dead`, geometry and colour
  only, with the scoped original deleted rather than left behind.
- `UsageBadge.vue` holds the logic the two lists would otherwise duplicate —
  the >=3 dead-weight threshold, the empty-string-renders-nothing rule, the
  tooltip.
- `types/usage.ts` holds `RecordUsage`, one client type over two tables.
  `SnippetUsage` becomes an alias, so no existing consumer changes.

THE ADVICE IS A PROP, and that is the substance rather than the plumbing. The
counts read identically for every kind; the remedy does not. A snippet offered
and never opened should probably be rewritten or deleted — one action. A rule
in the same position has TWO possible causes and the operator has to pick:
its trigger may fire on the wrong work, in which case `when_to_apply` wants
rewording, or it may genuinely not be wanted. Baking "delete it" into the
component would give the wrong nudge half the time on the surface where being
wrong is most expensive, since a deleted rule stops binding behaviour.

The route zero-fills every row through `usage_for_rules`, one aggregate per
page — per-row would be N+1 by construction. That matters more here than for
snippets: every rule on every existing install predates `rule_usage_events`,
so the zero-filled shape IS the common case for a while, and a route that
attached the key only where it found events would leave the badge reading
undefined on almost every row.

`usage_for_rules` had no test at all — step 1 covered the write path and the
zero shape and left the aggregate uncovered, which only became load-bearing
when a list started rendering it. It now has an integration test over real
Postgres, including that a rule with no events comes back zero-filled rather
than absent.

Recorded as snippet #3460, per the design system's own instruction that the
component layer lives as snippets rather than as prose.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN
---
 frontend/src/api/rulebooks.ts                 |  9 +++
 frontend/src/api/snippets.ts                  | 16 ++---
 frontend/src/assets/components.css            | 26 +++++++
 frontend/src/components/UsageBadge.vue        | 62 +++++++++++++++++
 .../src/components/rules/RuleListPane.vue     | 12 ++++
 frontend/src/types/usage.ts                   | 23 +++++++
 frontend/src/views/SnippetListView.vue        | 67 +++----------------
 src/scribe/routes/rulebooks.py                | 19 +++++-
 tests/test_routes_rulebooks.py                | 24 +++++++
 tests/test_services_rule_usage.py             | 62 +++++++++++++++++
 10 files changed, 250 insertions(+), 70 deletions(-)
 create mode 100644 frontend/src/components/UsageBadge.vue
 create mode 100644 frontend/src/types/usage.ts

diff --git a/frontend/src/api/rulebooks.ts b/frontend/src/api/rulebooks.ts
index 9665f42..1a8c175 100644
--- a/frontend/src/api/rulebooks.ts
+++ b/frontend/src/api/rulebooks.ts
@@ -1,3 +1,5 @@
+import type { RecordUsage } from "@/types/usage";
+
 import { apiGet, apiPost, apiPatch, apiDelete } from "@/api/client";
 
 /** How a rule reaches a session (milestone 307). */
@@ -96,6 +98,13 @@ export interface RuleHeader {
    * A date (YYYY-MM-DD), or the literal "never".
    */
   last_verified?: string;
+  /**
+   * Surfaced-vs-opened counts from `rule_usage_events` (milestone 333).
+   * Zero-filled by the list route, so a rule predating the table reads as
+   * "never surfaced" rather than as a missing field — which for a while is
+   * every rule on every install.
+   */
+  usage?: RecordUsage;
 }
 
 export interface ApplicableRules {
diff --git a/frontend/src/api/snippets.ts b/frontend/src/api/snippets.ts
index 25bd2d8..0931e14 100644
--- a/frontend/src/api/snippets.ts
+++ b/frontend/src/api/snippets.ts
@@ -1,3 +1,5 @@
+import type { RecordUsage } from "@/types/usage";
+
 import { apiGet, apiPost, apiPatch, apiDelete } from "@/api/client";
 
 /** One canonical location of a reusable thing. A snippet that unifies several
@@ -50,15 +52,11 @@ export interface Snippet {
   owner?: string | null;
 }
 
-/** How often a record was put in front of an agent versus actually opened.
- *  A high `surfaced_count` with `pull_count: 0` is dead weight — it occupies a
- *  slot in every future auto-inject menu while never being used. */
-export interface SnippetUsage {
-  surfaced_count: number;
-  pull_count: number;
-  last_surfaced_at: string | null;
-  last_pulled_at: string | null;
-}
+/** Kept as a name because every consumer here says "snippet usage" — but it IS
+ *  the shared shape, since rules answer the same question off their own table
+ *  (milestone 333). The reasoning lives on `RecordUsage`; duplicating the four
+ *  fields here is how the two drift. */
+export type SnippetUsage = RecordUsage;
 
 /** Result of the last drift check — does the recorded location and code still
  *  match source? The check runs agent-side (Scribe has no checkout); this is the
diff --git a/frontend/src/assets/components.css b/frontend/src/assets/components.css
index 0d76513..c886ef0 100644
--- a/frontend/src/assets/components.css
+++ b/frontend/src/assets/components.css
@@ -351,3 +351,29 @@
 
 .required { color: var(--fs-error); }
 .field-hint { margin: 0.3rem 0 0; font-size: 0.8rem; color: var(--fs-text-tertiary); }
+
+/* --- usage badge ----------------------------------------------------------
+   "surfaced N×, opened M×" on a list row, for any record kind the retrieval
+   surfaces can choose: snippets and notes from note_usage_events, rules from
+   rule_usage_events. Promoted here from SnippetListView's scoped block when
+   the rule list needed the same chip (milestone 333 step 5) — a second scoped
+   copy is how the ninth duplicated CSS family starts (#3207).
+
+   Geometry and colour only. A view keeps its own spacing as a scoped
+   remainder, the way it does for every other recipe in this file. */
+.usage-tag {
+  font-size: 0.7rem;
+  padding: 0.1rem 0.4rem;
+  border-radius: 4px;
+  white-space: nowrap;
+  font-variant-numeric: tabular-nums;
+  background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent);
+  color: var(--fs-text-tertiary-fg);
+}
+
+/* Dead weight is a nudge, not an error — it warns in the warning colour rather
+   than the danger one, because the record isn't broken, just unearned. */
+.usage-tag.usage-dead {
+  background: color-mix(in srgb, var(--fs-warning) 18%, transparent);
+  color: var(--fs-warning-fg);
+}
diff --git a/frontend/src/components/UsageBadge.vue b/frontend/src/components/UsageBadge.vue
new file mode 100644
index 0000000..47fabbb
--- /dev/null
+++ b/frontend/src/components/UsageBadge.vue
@@ -0,0 +1,62 @@
+<script setup lang="ts">
+/**
+ * "N/M used" on a list row — surfaced vs opened, for any record kind.
+ *
+ * Extracted from SnippetListView when the rule list needed the same chip
+ * (milestone 333 step 5). The counts read identically for both; what differs
+ * is the ADVICE, which is why that is a prop. A snippet surfaced repeatedly
+ * and never opened should probably go; a rule in the same position may simply
+ * have a `when_to_apply` that fires on the wrong thing, and telling an
+ * operator to delete it would be the wrong nudge half the time.
+ */
+import type { RecordUsage } from "@/types/usage";
+
+const props = defineProps<{
+  usage?: RecordUsage | null;
+  /** What to suggest when this record looks like dead weight. Appended to the
+   *  tooltip; kind-specific, because the remedies are. */
+  deadWeightAdvice: string;
+  /** What the record is called in the tooltip's own sentence. */
+  noun?: string;
+}>();
+
+/** Offered repeatedly and never opened. Three rather than one because one or
+ *  two surfacings is noise — the record may simply not have come up in a
+ *  relevant context yet. */
+const isDeadWeight = () =>
+  !!props.usage && props.usage.pull_count === 0 && props.usage.surfaced_count >= 3;
+
+/** "" renders nothing. A record nobody has surfaced yet gets no badge at all:
+ *  "0/0" would read as a verdict when it is an absence of evidence — and on a
+ *  freshly-migrated install that is every row. */
+const label = () => {
+  const u = props.usage;
+  if (!u || u.surfaced_count === 0) return "";
+  return `${u.pull_count}/${u.surfaced_count} used`;
+};
+
+const title = () => {
+  const u = props.usage;
+  if (!u) return "";
+  const last = u.last_pulled_at
+    ? `Last opened ${new Date(u.last_pulled_at).toLocaleDateString()}.`
+    : "Never opened.";
+  const verdict = isDeadWeight() ? ` ${props.deadWeightAdvice}` : "";
+  return (
+    `Surfaced to an agent ${u.surfaced_count}×, opened in full ` +
+    `${u.pull_count}×. ${last}${verdict}`
+  );
+};
+</script>
+
+<template>
+  <span
+    v-if="label()"
+    class="usage-tag"
+    :class="{ 'usage-dead': isDeadWeight() }"
+    :title="title()"
+  >{{ label() }}</span>
+</template>
+
+<!-- The look lives in components.css (canon). Nothing scoped here on purpose:
+     a view that needs different spacing keeps that as its own remainder. -->
diff --git a/frontend/src/components/rules/RuleListPane.vue b/frontend/src/components/rules/RuleListPane.vue
index 6c2a662..32d08f7 100644
--- a/frontend/src/components/rules/RuleListPane.vue
+++ b/frontend/src/components/rules/RuleListPane.vue
@@ -1,5 +1,16 @@
 <script setup lang="ts">
 import type { RuleHeader } from "@/api/rulebooks";
+import UsageBadge from "@/components/UsageBadge.vue";
+
+/** The dead-weight nudge for a RULE — two remedies, not one, which is the
+ *  whole reason this advice is per-kind. A snippet nobody opens should
+ *  probably go. A rule nobody opens may be perfectly good and simply firing on
+ *  the wrong thing, so "delete it" would be the wrong nudge half the time and
+ *  the operator has to be the one who picks. */
+const RULE_DEAD_WEIGHT =
+  "Kept arriving without being read. Either its trigger fires on the wrong " +
+  "work — reword “when to apply” so it says when — or it is not wanted here. " +
+  "Until one or the other, it takes a slot in every write it matches.";
 
 defineProps<{ topicId: number; rules: RuleHeader[] }>();
 const emit = defineEmits<{
@@ -28,6 +39,7 @@ const emit = defineEmits<{
               ? 'Asserts a fact nobody has confirmed yet'
               : `Check last passed ${r.last_verified}`"
           >{{ r.last_verified === "never" ? "unverified" : `checked ${r.last_verified}` }}</span>
+          <UsageBadge :usage="r.usage" :dead-weight-advice="RULE_DEAD_WEIGHT" />
         </div>
         <div class="statement">{{ r.statement }}</div>
         <div v-if="r.when_to_apply || r.updated_at" class="meta">
diff --git a/frontend/src/types/usage.ts b/frontend/src/types/usage.ts
new file mode 100644
index 0000000..42d4c8b
--- /dev/null
+++ b/frontend/src/types/usage.ts
@@ -0,0 +1,23 @@
+/**
+ * How often a record was put in front of an agent, and how often one then
+ * opened it in full.
+ *
+ * One shape for every record kind the retrieval surfaces can choose. Snippets
+ * and notes are counted in `note_usage_events`; rules in `rule_usage_events`,
+ * which is a separate table because a note id and a rule id are different
+ * namespaces resolved through different maps at restore (milestone 333). The
+ * TABLES are separate for that reason; the READOUT is the same question, so
+ * the client type is one.
+ *
+ * A high `surfaced_count` with `pull_count: 0` is dead weight — it occupies a
+ * slot in every future menu while never being used. What to DO about that
+ * differs by kind, which is why the advice is a prop on the badge rather than
+ * a property of this type: a snippet nobody opens should probably be deleted,
+ * while a rule nobody opens may just be mis-triggered.
+ */
+export interface RecordUsage {
+  surfaced_count: number;
+  pull_count: number;
+  last_surfaced_at: string | null;
+  last_pulled_at: string | null;
+}
diff --git a/frontend/src/views/SnippetListView.vue b/frontend/src/views/SnippetListView.vue
index 99541e9..6d86289 100644
--- a/frontend/src/views/SnippetListView.vue
+++ b/frontend/src/views/SnippetListView.vue
@@ -9,6 +9,7 @@ import {
   type SnippetListItem,
 } from "@/api/snippets";
 import { useToastStore } from "@/stores/toast";
+import UsageBadge from "@/components/UsageBadge.vue";
 
 const router = useRouter();
 const toast = useToastStore();
@@ -198,23 +199,6 @@ function languageOf(tags: string[]): string {
   return tags.find((t) => t && t !== "snippet") ?? "";
 }
 
-/** A snippet that has been offered repeatedly and never opened. The threshold
- *  is 3 rather than 1 because one or two surfacings is noise — the record may
- *  simply not have come up in a relevant context yet. */
-function isDeadWeight(s: SnippetListItem): boolean {
-  const u = s.usage;
-  return !!u && u.pull_count === 0 && u.surfaced_count >= 3;
-}
-
-/** Short badge text, or "" to render nothing. A record nobody has surfaced yet
- *  gets no badge at all: "0 / 0" would read as a verdict when it's an absence
- *  of evidence. */
-function usageBadge(s: SnippetListItem): string {
-  const u = s.usage;
-  if (!u || u.surfaced_count === 0) return "";
-  return `${u.pull_count}/${u.surfaced_count} used`;
-}
-
 /** Short label for the drift verdict, or "" when there's nothing to say.
  *  An expired verdict is reported as "unchecked" whatever it used to say —
  *  it was about code that is no longer in the record. */
@@ -254,22 +238,13 @@ function driftTitle(s: SnippetListItem): string {
   return v.detail ? `${when}: ${what}. ${v.detail}` : `${when}: ${what}.`;
 }
 
-function usageTitle(s: SnippetListItem): string {
-  const u = s.usage;
-  if (!u) return "";
-  const last = u.last_pulled_at
-    ? `Last opened ${new Date(u.last_pulled_at).toLocaleDateString()}.`
-    : "Never opened.";
-  const verdict = isDeadWeight(s)
-    ? " Offered repeatedly without ever being opened — consider rewriting its" +
-      " “when to reach for it” so it says when, or deleting it. It takes a slot" +
-      " in every future auto-inject menu."
-    : "";
-  return (
-    `Surfaced to an agent ${u.surfaced_count}×, opened in full ` +
-    `${u.pull_count}×. ${last}${verdict}`
-  );
-}
+/** The dead-weight nudge for a SNIPPET, passed to the shared badge. Kept here
+ *  rather than inside the component because the remedy is kind-specific — a
+ *  rule in the same position gets different advice (milestone 333 step 5). */
+const SNIPPET_DEAD_WEIGHT =
+  "Offered repeatedly without ever being opened — consider rewriting its " +
+  "“when to reach for it” so it says when, or deleting it. It takes a slot " +
+  "in every future auto-inject menu.";
 </script>
 
 <template>
@@ -456,14 +431,7 @@ function usageTitle(s: SnippetListItem): string {
           <span v-if="driftBadge(s)" class="drift-tag" :title="driftTitle(s)">
             {{ driftBadge(s) }}
           </span>
-          <span
-            v-if="usageBadge(s)"
-            class="usage-tag"
-            :class="{ 'usage-dead': isDeadWeight(s) }"
-            :title="usageTitle(s)"
-          >
-            {{ usageBadge(s) }}
-          </span>
+          <UsageBadge :usage="s.usage" :dead-weight-advice="SNIPPET_DEAD_WEIGHT" />
           <span v-if="s.shared" class="shared-tag" :title="`Shared by ${s.owner ?? 'another user'} — a suggestion, not your own record`">
             by {{ s.owner ?? "another user" }}
           </span>
@@ -757,23 +725,6 @@ function usageTitle(s: SnippetListItem): string {
   color: var(--fs-error-fg);
 }
 
-.usage-tag {
-  font-size: 0.7rem;
-  padding: 0.1rem 0.4rem;
-  border-radius: 4px;
-  white-space: nowrap;
-  font-variant-numeric: tabular-nums;
-  background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent);
-  color: var(--fs-text-tertiary-fg);
-}
-
-/* Dead weight is a nudge, not an error — it warns in the warning colour rather
-   than the danger one, because the record isn't broken, just unearned. */
-.usage-tag.usage-dead {
-  background: color-mix(in srgb, var(--fs-warning) 18%, transparent);
-  color: var(--fs-warning-fg);
-}
-
 /* Header + select-mode */
 .header-actions {
   display: flex;
diff --git a/src/scribe/routes/rulebooks.py b/src/scribe/routes/rulebooks.py
index 17ab59c..5f6b77e 100644
--- a/src/scribe/routes/rulebooks.py
+++ b/src/scribe/routes/rulebooks.py
@@ -10,7 +10,9 @@ from quart import Blueprint, jsonify, request
 from scribe.auth import get_current_user_id, login_required
 import scribe.services.rulebooks as rulebooks_svc
 from scribe.services.trash import delete as trash_delete
-from scribe.services.rule_usage import record_rule_pulled
+from scribe.services.rule_usage import (
+    empty_rule_usage, record_rule_pulled, usage_for_rules,
+)
 
 rulebooks_bp = Blueprint("rulebooks", __name__, url_prefix="/api")
 
@@ -137,13 +139,24 @@ async def list_rules():
     except ValueError:
         return jsonify({"error": "rulebook_id, topic_id, project_id must be integers"}), 400
 
+    uid = get_current_user_id()
     rows = await rulebooks_svc.list_rules(
-        user_id=get_current_user_id(),
+        user_id=uid,
         rulebook_id=rulebook_id,
         topic_id=topic_id,
         project_id=project_id,
     )
-    return jsonify({"rules": [r.to_dict() for r in rows]})
+    items = [r.to_dict() for r in rows]
+    # One aggregate for the whole page — a per-row lookup here would be N+1 by
+    # construction, the same reason the snippet list does it this way. Every
+    # row gets the key, zero-filled, so the UI renders "never surfaced" rather
+    # than having to treat a missing field as a state. That matters more here
+    # than for snippets: every rule on every install predates this table, so
+    # for a while the zero-filled shape IS the common case.
+    usage = await usage_for_rules([int(it["id"]) for it in items])
+    for it in items:
+        it["usage"] = usage.get(int(it["id"]), empty_rule_usage())
+    return jsonify({"rules": items})
 
 
 @rulebooks_bp.post("/rulebook-topics/<int:topic_id>/rules")
diff --git a/tests/test_routes_rulebooks.py b/tests/test_routes_rulebooks.py
index 01641f6..afe50d6 100644
--- a/tests/test_routes_rulebooks.py
+++ b/tests/test_routes_rulebooks.py
@@ -122,3 +122,27 @@ def test_rule_and_subscription_handlers_callable():
         "relate_rules", "unrelate_rules",
     ):
         assert callable(getattr(rb_routes, name))
+
+
+def test_the_rule_list_zero_fills_usage_on_every_row():
+    """Milestone 333 step 5, asserted the only way this harness allows.
+
+    There is no live-HTTP fixture here (see this module's docstring), so this
+    reads the handler's source. What it can still prove is the property that
+    gets forgotten: the route must attach the key to EVERY row, zero-filled,
+    rather than only to rows that happen to have events. Every rule on every
+    existing install predates `rule_usage_events`, so a route that only
+    attached the key when it found something would leave the badge component
+    reading `undefined` on almost every row — and the difference between "no
+    events" and "no field" is exactly the distinction #2663 is about.
+    """
+    import inspect
+
+    from scribe.routes import rulebooks as rb_routes
+
+    src = inspect.getsource(rb_routes.list_rules)
+    assert "usage_for_rules" in src, "the rule list does not read usage at all"
+    assert "empty_rule_usage()" in src, (
+        "the rule list does not zero-fill — a rule with no events would come "
+        "back without the key rather than with an empty one"
+    )
diff --git a/tests/test_services_rule_usage.py b/tests/test_services_rule_usage.py
index 3356390..6efb23c 100644
--- a/tests/test_services_rule_usage.py
+++ b/tests/test_services_rule_usage.py
@@ -116,3 +116,65 @@ def test_the_model_serialises_the_fields_the_ratio_needs():
     # created_at is server-defaulted, so it is None until the row is flushed —
     # `iso()` must tolerate that rather than raising on a fresh instance.
     assert row["created_at"] is None
+
+
+# ─── the readout (milestone 333 step 5) ──────────────────────────────────────
+# Integration: a real GROUP BY over a real table. Step 1 unit-tested the WRITE
+# path and the zero shape and left the aggregate uncovered, which only became
+# load-bearing when the rule list started rendering it.
+
+
+@pytest.mark.integration
+@pytest.mark.asyncio
+async def test_usage_for_rules_aggregates_per_rule(_dispose_engine):
+    from sqlalchemy import delete
+
+    from scribe.models import async_session
+    from scribe.models.rule_usage import RuleUsageEvent
+
+    async with async_session() as s:
+        s.add_all([
+            RuleUsageEvent(user_id=990020, rule_id=6001,
+                           event=SURFACED, source="write_path_rule"),
+            RuleUsageEvent(user_id=990020, rule_id=6001,
+                           event=SURFACED, source="write_path_rule"),
+            RuleUsageEvent(user_id=990020, rule_id=6001,
+                           event=PULLED, source="mcp_get_rule"),
+            RuleUsageEvent(user_id=990020, rule_id=6002,
+                           event=SURFACED, source="write_path_rule"),
+        ])
+        await s.commit()
+    try:
+        out = await rule_usage.usage_for_rules([6001, 6002, 6003])
+
+        assert out[6001]["surfaced_count"] == 2
+        assert out[6001]["pull_count"] == 1
+        assert out[6001]["last_surfaced_at"] is not None
+        assert out[6001]["last_pulled_at"] is not None
+
+        # Surfaced twice as often as it was opened — never, in this case.
+        assert out[6002]["surfaced_count"] == 1
+        assert out[6002]["pull_count"] == 0
+        assert out[6002]["last_pulled_at"] is None
+
+        # A rule with NO events still comes back, zero-filled. The caller must
+        # never have to tell "no events" from "not in the result" — and on any
+        # existing install that is nearly every rule.
+        assert out[6003] == rule_usage.empty_rule_usage()
+    finally:
+        async with async_session() as s:
+            await s.execute(
+                delete(RuleUsageEvent).where(RuleUsageEvent.user_id == 990020)
+            )
+            await s.commit()
+
+
+@pytest.mark.integration
+@pytest.mark.asyncio
+async def test_usage_for_rules_on_an_empty_id_list_asks_the_database_nothing(
+    _dispose_engine,
+):
+    """The list route calls this with whatever the page holds, which on an
+    empty topic is nothing. An unguarded `IN ()` is both a pointless round trip
+    and, on some drivers, a syntax error."""
+    assert await rule_usage.usage_for_rules([]) == {}