feat(4636): rule pull-through per surface, and per-rule usage with confirmed lessons on list_rules
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m3s
CI & Build / Python tests (push) Successful in 1m56s
CI & Build / Build & push image (push) Successful in 22s

retrieval_telemetry gains rule_usage.by_source, the twin of usage.by_source:
distinct rules each surface showed, how many an agent opened, and a ratio
that is null on a bulk delivery. list_rules(with_usage=True, days=N) adds
each rule's usage, windowed, and how many lessons are confirmed instances
of it, so linked and unlinked rules can be compared through MCP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-22 19:20:32 -04:00
co-authored by Claude Opus 5.5
parent 71260a28e2
commit d05d4f3632
9 changed files with 287 additions and 9 deletions
+30 -6
View File
@@ -10,13 +10,16 @@ preview-style warning.
""" """
from __future__ import annotations from __future__ import annotations
from datetime import datetime, timedelta, timezone
from grimoire.mcp._context import current_user_id from grimoire.mcp._context import current_user_id
from grimoire.services import dedup as dedup_svc from grimoire.services import dedup as dedup_svc
from grimoire.services import moments as moments_svc from grimoire.services import moments as moments_svc
from grimoire.services import rulebooks as rulebooks_svc from grimoire.services import rulebooks as rulebooks_svc
from grimoire.services import trash as trash_svc from grimoire.services import trash as trash_svc
from grimoire.services.lesson_rules import confirmed_lesson_counts
from grimoire.services.rule_usage import ( from grimoire.services.rule_usage import (
record_rule_outcome, record_rule_pulled, empty_rule_usage, record_rule_outcome, record_rule_pulled, usage_for_rules,
) )
from grimoire.services import moment_delivery from grimoire.services import moment_delivery
@@ -204,6 +207,7 @@ def _rule_summary(r, moments: list[str] | None = None) -> dict:
async def list_rules( async def list_rules(
rulebook_id: int = 0, topic_id: int = 0, project_id: int = 0, rulebook_id: int = 0, topic_id: int = 0, project_id: int = 0,
with_usage: bool = False, days: int = 0,
) -> dict: ) -> dict:
"""List rules — filter by rulebook, topic, and/or project. """List rules — filter by rulebook, topic, and/or project.
@@ -213,9 +217,21 @@ async def list_rules(
project_id: 0 = no filter; positive = that project's OWN rules. project_id: 0 = no filter; positive = that project's OWN rules.
Global rules apply to every project, so they are listed by Global rules apply to every project, so they are listed by
rulebook or topic (or unfiltered), not under each project. rulebook or topic (or unfiltered), not under each project.
with_usage: add each rule's `usage` and `confirmed_lessons`. Off by
default: a listing is for applying rules, and this is for judging
them.
days: with `with_usage`, count only the last N days; 0 = everything
recorded.
rulebook_id and topic_id AND-combine; project_id lists a project's rules rulebook_id and topic_id AND-combine; project_id lists a project's rules
on its own. Ownership-scoped. on its own. Ownership-scoped.
`usage` is the rule's own telemetry: `surfaced_count` (a ranked surface
chose it), `ambient_count` (a bulk delivery carried it), `pull_count`,
`applied_count`, `departed_count` and the latest of each.
`confirmed_lessons` counts the lessons judged instances of the rule. Read
together they answer whether rules that situations keep proving are the
ones that surface and get opened.
""" """
uid = current_user_id() uid = current_user_id()
rows = await rulebooks_svc.list_rules( rows = await rulebooks_svc.list_rules(
@@ -224,12 +240,20 @@ async def list_rules(
topic_id=topic_id or None, topic_id=topic_id or None,
project_id=project_id or None, project_id=project_id or None,
) )
ids = [r.id for r in rows]
# One batched read for the page, not one per row. # One batched read for the page, not one per row.
mounted = await rulebooks_svc.list_rule_moments([r.id for r in rows]) mounted = await rulebooks_svc.list_rule_moments(ids)
return { rules = [_rule_summary(r, mounted.get(r.id)) for r in rows]
"rules": [_rule_summary(r, mounted.get(r.id)) for r in rows], if with_usage:
"total": len(rows), since = (
} datetime.now(timezone.utc) - timedelta(days=days) if days > 0 else None
)
usage = await usage_for_rules(ids, since=since)
lessons = await confirmed_lesson_counts(uid, ids)
for row in rules:
row["usage"] = usage.get(row["id"], empty_rule_usage())
row["confirmed_lessons"] = lessons.get(row["id"], 0)
return {"rules": rules, "total": len(rows)}
async def get_rule(rule_id: int) -> dict: async def get_rule(rule_id: int) -> dict:
+9
View File
@@ -509,6 +509,15 @@ async def retrieval_telemetry(
Read it against `sources["write_path_rule"]`, which declines the large Read it against `sources["write_path_rule"]`, which declines the large
majority of its calls like any other surface. majority of its calls like any other surface.
`rule_usage.by_source` is the per-surface split, the twin of
`usage.by_source`: for each surface that showed rules, `rules_surfaced`
(distinct rules), `rules_pulled` (how many of those an agent opened in the
window) and `pull_through`, null on an ambient surface. It is the readout
that says which arm — the prompt arm, the tool arm, a lesson pointing at
its rule — earns its lines. The same upper bound as the note split: a rule
two surfaces showed and an agent opened once counts for both.
`by_source_failed: true` means that one query failed.
`system_usage` — the RULINGS arm, from `system_usage_events`: how `system_usage` — the RULINGS arm, from `system_usage_events`: how
often an area's rulings were shown because a command (`rulings_pre_tool`) often an area's rulings were shown because a command (`rulings_pre_tool`)
or an edit (`rulings_write_path`) touched its files, how often a System or an edit (`rulings_write_path`) touched its files, how often a System
+28
View File
@@ -324,6 +324,34 @@ async def lessons_for_rule(user_id: int, rule_id: int) -> list[dict]:
] ]
async def confirmed_lesson_counts(user_id: int, rule_ids) -> dict[int, int]:
"""{rule_id: confirmed lessons the caller can read}, for a list of rules.
`lessons_for_rule` in bulk, counted rather than listed: one GROUP BY, so a
rule list can say which rules are proven by situations without a query
per row. Rules with none read 0.
"""
from grimoire.services.access import readable_notes_clause
ids = [int(i) for i in rule_ids or []]
out = {i: 0 for i in ids}
if not ids:
return out
async with async_session() as session:
rows = (await session.execute(
select(LessonRuleLink.rule_id, func.count())
.join(Note, Note.id == LessonRuleLink.lesson_id)
.where(LessonRuleLink.rule_id.in_(ids))
.where(LessonRuleLink.state == CONFIRMED)
.where(Note.deleted_at.is_(None))
.where(readable_notes_clause(user_id))
.group_by(LessonRuleLink.rule_id)
)).all()
for rule_id, n in rows:
out[int(rule_id)] = int(n)
return out
async def judgments_for_lessons(lesson_ids) -> dict[int, dict]: async def judgments_for_lessons(lesson_ids) -> dict[int, dict]:
"""{lesson_id: {"rule_judgment": linked|no_rule|unjudged, "no_rule"?}}. """{lesson_id: {"rule_judgment": linked|no_rule|unjudged, "no_rule"?}}.
+70 -1
View File
@@ -1109,7 +1109,7 @@ async def retrieval_summary(
# Assigned inside the try below; named here so the readout can tell # Assigned inside the try below; named here so the readout can tell
# "this query failed" from "this window has no rows". # "this query failed" from "this window has no rows".
by_source_rows = None by_source_rows = rule_by_source_rows = None
rule_rows = None rule_rows = None
distinct_rules_surfaced = distinct_rules_pulled = 0 distinct_rules_surfaced = distinct_rules_pulled = 0
# None means the coverage read did not happen — distinct from a table with # None means the coverage read did not happen — distinct from a table with
@@ -1426,6 +1426,53 @@ async def retrieval_summary(
rule_rows = None rule_rows = None
distinct_rules_surfaced = distinct_rules_pulled = 0 distinct_rules_surfaced = distinct_rules_pulled = 0
distinct_rules_acted = 0 distinct_rules_acted = 0
# The note block's per-source join, at the rule grain. The
# top-level rule ratio blends every arm — the prompt arm, the tool
# arm, a lesson pointing at its rule — and "which of them earns its
# lines" is the question each arm's bar is tuned against. Its own
# guard, for the reason the note join has one.
try:
rule_pulled_ids = (
select(RuleUsageEvent.rule_id)
.where(
RuleUsageEvent.created_at >= since,
RuleUsageEvent.user_id == user_id,
RuleUsageEvent.event == RULE_PULLED,
RuleUsageEvent.source.startswith("mcp_", autoescape=True),
)
.distinct()
.subquery()
)
rule_surfaced_pairs = (
select(RuleUsageEvent.source, RuleUsageEvent.rule_id)
.where(
RuleUsageEvent.created_at >= since,
RuleUsageEvent.user_id == user_id,
RuleUsageEvent.event == RULE_SURFACED,
)
.distinct()
.subquery()
)
rule_by_source_rows = (
await session.execute(
select(
rule_surfaced_pairs.c.source,
func.count().label("rules_surfaced"),
func.count(rule_pulled_ids.c.rule_id).label("rules_pulled"),
)
.select_from(
rule_surfaced_pairs.outerjoin(
rule_pulled_ids,
rule_pulled_ids.c.rule_id == rule_surfaced_pairs.c.rule_id,
)
)
.group_by(rule_surfaced_pairs.c.source)
)
).all()
except Exception:
logger.warning("per-source rule pull-through read failed", exc_info=True)
rule_by_source_rows = None
except Exception: except Exception:
logger.warning("retrieval summary read failed", exc_info=True) logger.warning("retrieval summary read failed", exc_info=True)
out["read_failed"] = True out["read_failed"] = True
@@ -1596,6 +1643,28 @@ async def retrieval_summary(
round(rule_usage["pulled_by_agent"] / rule_usage["surfaced"], 4) round(rule_usage["pulled_by_agent"] / rule_usage["surfaced"], 4)
if rule_usage["surfaced"] else None if rule_usage["surfaced"] else None
) )
# Per surface, the note block's shape and its caveat: a pull records the
# door, not the surface that led to it, so a rule two arms surfaced and an
# agent opened once counts as pulled for both. An upper bound — a surface
# reading near zero here is not being flattered.
if rule_by_source_rows is None:
rule_usage["by_source"] = {}
rule_usage["by_source_failed"] = True
else:
rule_usage["by_source"] = {
source: {
"rules_surfaced": int(n_surfaced or 0),
"rules_pulled": int(n_pulled or 0),
# None on an ambient surface, as for notes: nothing chose
# those rules, so the ratio would be misread.
"pull_through": (
None if is_ambient(source) or not n_surfaced
else round(int(n_pulled or 0) / int(n_surfaced), 4)
),
"ambient": is_ambient(source),
}
for source, n_surfaced, n_pulled in rule_by_source_rows
}
rule_usage.update(_coverage((rule_complete or {}).get("*"), since)) rule_usage.update(_coverage((rule_complete or {}).get("*"), since))
out["rule_usage"] = rule_usage out["rule_usage"] = rule_usage
out["system_usage"] = await _system_usage(user_id, since) out["system_usage"] = await _system_usage(user_id, since)
+12 -2
View File
@@ -59,8 +59,9 @@ as short and as slow-moving as possible.
from __future__ import annotations from __future__ import annotations
import logging import logging
from datetime import datetime
from sqlalchemy import case, func, select from sqlalchemy import case, func, select, true
from grimoire.models import async_session from grimoire.models import async_session
from grimoire.models.base import iso from grimoire.models.base import iso
@@ -333,13 +334,18 @@ def empty_rule_usage() -> dict:
} }
async def usage_for_rules(rule_ids: list[int]) -> dict[int, dict]: async def usage_for_rules(
rule_ids: list[int], since: datetime | None = None,
) -> dict[int, dict]:
"""Aggregate usage for a set of rules: {rule_id: {counts + timestamps}}. """Aggregate usage for a set of rules: {rule_id: {counts + timestamps}}.
One GROUP BY for the whole page rather than a query per row — this feeds a One GROUP BY for the whole page rather than a query per row — this feeds a
list view, so the per-row shape would be N+1 by construction. Rules with no list view, so the per-row shape would be N+1 by construction. Rules with no
events come back with `empty_rule_usage()`, so the caller never has to tell events come back with `empty_rule_usage()`, so the caller never has to tell
"no events" from "not in the result". "no events" from "not in the result".
`since` limits the counts to events at or after it, so two windows can be
compared; without it they cover everything recorded.
""" """
ids = [int(r) for r in rule_ids] ids = [int(r) for r in rule_ids]
out: dict[int, dict] = {rid: empty_rule_usage() for rid in ids} out: dict[int, dict] = {rid: empty_rule_usage() for rid in ids}
@@ -370,6 +376,10 @@ async def usage_for_rules(rule_ids: list[int]) -> dict[int, dict]:
ambient, ambient,
) )
.where(RuleUsageEvent.rule_id.in_(ids)) .where(RuleUsageEvent.rule_id.in_(ids))
.where(
RuleUsageEvent.created_at >= since
if since is not None else true()
)
.group_by( .group_by(
RuleUsageEvent.rule_id, RuleUsageEvent.rule_id,
RuleUsageEvent.event, RuleUsageEvent.event,
@@ -93,6 +93,22 @@ async def test_naming_rules_confirms_them_and_both_sides_read_the_link(world):
assert [(l["id"], l["state"]) for l in lessons] == [(world["lesson"], "confirmed")] assert [(l["id"], l["state"]) for l in lessons] == [(world["lesson"], "confirmed")]
async def test_a_rule_list_counts_confirmed_lessons_only(world):
"""The bulk count a rule list reads: a rejected link is not a lesson
proving the rule, a rule with no links reads 0, and a stranger counts
nothing through a lesson they cannot read."""
owner = world["owner"]
await links_svc.set_lesson_rules(owner, world["lesson"], [world["r1"], world["r2"]])
await links_svc.judge_link(owner, world["lesson"], world["r2"], "reject", "not it")
rules = [world["r1"], world["r2"], world["foreign"]]
assert await links_svc.confirmed_lesson_counts(owner, rules) == {
world["r1"]: 1, world["r2"]: 0, world["foreign"]: 0,
}
assert (await links_svc.confirmed_lesson_counts(world["stranger"], rules))[world["r1"]] == 0
assert await links_svc.confirmed_lesson_counts(owner, []) == {}
async def test_a_rule_left_out_of_the_set_is_rejected_not_forgotten(world): async def test_a_rule_left_out_of_the_set_is_rejected_not_forgotten(world):
owner = world["owner"] owner = world["owner"]
await links_svc.set_lesson_rules(owner, world["lesson"], [world["r1"], world["r2"]]) await links_svc.set_lesson_rules(owner, world["lesson"], [world["r1"], world["r2"]])
+33
View File
@@ -573,3 +573,36 @@ def test_rule_outcome_is_registered_and_is_not_read_only():
assert "rule_outcome" in mcp.names assert "rule_outcome" in mcp.names
assert "rule_outcome" in _WRITE_TOOLS assert "rule_outcome" in _WRITE_TOOLS
assert "rule_outcome" not in _READ_ONLY_TOOLS assert "rule_outcome" not in _READ_ONLY_TOOLS
@pytest.mark.asyncio
async def test_list_rules_carries_usage_only_when_asked():
"""A listing is for applying rules, so it stays lean; `with_usage` adds
each rule's telemetry and confirmed-lesson count for judging them, with
`days` passed down as a window."""
from grimoire.mcp.tools import rulebooks as tools
from grimoire.services.rule_usage import empty_rule_usage
rows = [fake_rule(id=1, title="dev is home"), fake_rule(id=2, title="ask first")]
seen = {**empty_rule_usage(), "surfaced_count": 4, "pull_count": 1}
usage = AsyncMock(return_value={1: seen})
lessons = AsyncMock(return_value={1: 3})
with patch.object(tools.rulebooks_svc, "list_rules", AsyncMock(return_value=rows)), \
patch.object(tools.rulebooks_svc, "list_rule_moments", AsyncMock(return_value={})), \
patch.object(tools, "usage_for_rules", usage), \
patch.object(tools, "confirmed_lesson_counts", lessons):
lean = await tools.list_rules()
assert all("usage" not in r and "confirmed_lessons" not in r for r in lean["rules"])
usage.assert_not_awaited()
out = await tools.list_rules(with_usage=True, days=14)
by_id = {r["id"]: r for r in out["rules"]}
assert by_id[1]["usage"]["surfaced_count"] == 4
assert by_id[1]["confirmed_lessons"] == 3
# A rule missing from either read is zero-filled, never absent.
assert by_id[2]["usage"] == empty_rule_usage()
assert by_id[2]["confirmed_lessons"] == 0
assert usage.await_args.args[0] == [1, 2]
assert usage.await_args.kwargs["since"] is not None
lessons.assert_awaited_once_with(7, [1, 2])
@@ -618,6 +618,54 @@ async def test_the_preload_lands_in_ambient_and_never_in_the_ratio(_dispose_engi
await cleanup() await cleanup()
@pytest.mark.integration
@pytest.mark.asyncio
async def test_rule_by_source_says_which_arm_earns_its_lines(_dispose_engine):
"""The rule ratio blends every arm. Per surface it can say which one is
working: distinct rules per arm, the agent opens among them, and no ratio
on a bulk delivery."""
from grimoire.services.retrieval_telemetry import retrieval_summary
cleanup = await _rule_events(990016, [
# The write arm chose rule 5301 twice — one rule — and 5302 once.
(5301, "surfaced", "write_path_rule"),
(5301, "surfaced", "write_path_rule"),
(5302, "surfaced", "write_path_rule"),
# The tool arm chose 5303 and nothing came of it.
(5303, "surfaced", "pre_tool_rule"),
# The preload carried 5301 as well.
(5301, "surfaced", "session_start"),
# An agent opened 5301; a person opened 5303, which is not a hint
# landing.
(5301, "pulled", "mcp_get_rule"),
(5303, "pulled", "rest_rule"),
])
try:
ru = (await retrieval_summary(990016, days=30))["rule_usage"]
assert "by_source_failed" not in ru, "the join did not execute"
by_source = ru["by_source"]
wp = by_source["write_path_rule"]
assert (wp["rules_surfaced"], wp["rules_pulled"]) == (2, 1)
assert wp["pull_through"] == 0.5
assert wp["ambient"] is False
tool = by_source["pre_tool_rule"]
assert (tool["rules_surfaced"], tool["rules_pulled"]) == (1, 0)
assert tool["pull_through"] == 0.0
preload = by_source["session_start"]
# The pull is counted against every surface that showed the rule,
# the upper bound the readout states.
assert (preload["rules_surfaced"], preload["rules_pulled"]) == (1, 1)
assert preload["pull_through"] is None
assert preload["ambient"] is True
assert "mcp_get_rule" not in by_source, "a pull is not a surface"
finally:
await cleanup()
@pytest.mark.integration @pytest.mark.integration
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_ambient_alone_reports_no_ratio(_dispose_engine): async def test_ambient_alone_reports_no_ratio(_dispose_engine):
+41
View File
@@ -198,6 +198,47 @@ async def test_usage_for_rules_aggregates_per_rule(_dispose_engine):
await s.commit() await s.commit()
@pytest.mark.integration
@pytest.mark.asyncio
async def test_usage_for_rules_counts_only_the_window_it_is_given(_dispose_engine):
"""Two windows have to be comparable, so an event before `since` is left
out of every count — and without `since` it is counted."""
from datetime import datetime, timedelta, timezone
from sqlalchemy import delete
from grimoire.models import async_session
from grimoire.models.rule_usage import RuleUsageEvent
old = datetime.now(timezone.utc) - timedelta(days=30)
async with async_session() as s:
s.add_all([
RuleUsageEvent(user_id=990022, rule_id=6011, event=SURFACED,
source="write_path_rule", created_at=old),
RuleUsageEvent(user_id=990022, rule_id=6011, event=PULLED,
source="mcp_get_rule", created_at=old),
RuleUsageEvent(user_id=990022, rule_id=6011, event=SURFACED,
source="write_path_rule"),
])
await s.commit()
try:
since = datetime.now(timezone.utc) - timedelta(days=14)
recent = (await rule_usage.usage_for_rules([6011], since=since))[6011]
assert recent["surfaced_count"] == 1
assert recent["pull_count"] == 0
assert recent["last_pulled_at"] is None
everything = (await rule_usage.usage_for_rules([6011]))[6011]
assert everything["surfaced_count"] == 2
assert everything["pull_count"] == 1
finally:
async with async_session() as s:
await s.execute(
delete(RuleUsageEvent).where(RuleUsageEvent.user_id == 990022)
)
await s.commit()
@pytest.mark.integration @pytest.mark.integration
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_usage_for_rules_on_an_empty_id_list_asks_the_database_nothing( async def test_usage_for_rules_on_an_empty_id_list_asks_the_database_nothing(