Files
FabledScribe/src/scribe/mcp/tools/search.py
T
bvandeusenandClaude Opus 5 7a2aff7bc1
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / integration (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 24s
fix(telemetry): a surface that stopped recording is not one that never ran (#3720)
`out["sources"]` was built only from the windowed aggregate, so a source
with rows in `retrieval_logs` but none inside the window got no bucket at
all. Absent is exactly how a source that never existed renders, so a
surface that WAS recording and went silent became unreadable — #2663 one
level up, the failure that looks like the correct answer.

Two queries at different scopes, and only one shaped the output.
`_complete_from` reads all-time and knows every source the table has ever
held; the windowed loop dropped whatever it did not return.

Every such source now gets a zero bucket. Zero is a real measurement here
rather than a manufactured one: the all-time query proves the source was
recording, and it made no calls across a window it fully covers. No
`covers_window` special case is needed either — a source whose first row
fell after `since` would have that row IN the window and already hold a
bucket, so anything reaching this branch began before it.

The counts are 0 and everything else is null. A sampled distribution is
not the same claim as a call count, and rendering p50 as 0.0 for a source
nobody sampled would assert a measurement — #3311's mistake, in the
readout built to prevent it.

Found while fixing #3712's fixture, which failed with KeyError for this
exact reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ
2026-09-08 11:07:21 -04:00

293 lines
14 KiB
Python

"""search — semantic search across the user's notes and tasks.
Mirrors the existing fable-mcp contract so Claude's prior usage pattern keeps
working. Differences from fable-mcp:
- calls services.embeddings.semantic_search_notes directly instead of HTTP
- user_id comes from mcp.current_user_id() rather than a global API key
"""
from __future__ import annotations
import time
from scribe.mcp._context import current_user_id
from scribe.services.access import owner_names_for
from scribe.services.embeddings import (
DEFAULT_SIMILARITY_THRESHOLD, semantic_search_notes, semantic_search_rules,
)
from scribe.services import rulebooks as rulebooks_svc
from scribe.services.retrieval_telemetry import record_retrieval, retrieval_summary
async def _search_rules(uid: int, q: str, limit: int) -> dict:
"""Rules by meaning — a separate result shape because a rule IS different.
A rule hit carries `why` and `how_to_apply`: they are the operational half
of a rule and the session-start payload never includes them, so a caller
who went looking should get the whole thing rather than a summary they then
have to re-fetch. It also carries the rule's check (`verify_with`,
`expires_when`, `last_verified`) when it has one — a search hit is exactly
the moment someone is about to act on a rule, and "this asserts a fact
nobody has confirmed" is part of what the rule says.
Rules are not project-scoped the way notes are (a family rule belongs to no
project), so `project_id` and `system_id` do not apply here.
"""
raw = await semantic_search_rules(uid, q, limit=limit)
return {
"results": [
{
"id": rule.id,
"title": rule.title,
"statement": rule.statement,
"when_to_apply": rule.when_to_apply or "",
"tier": rule.tier,
"why": rule.why or "",
"how_to_apply": rule.how_to_apply or "",
"verify_with": rule.verify_with or "",
"expires_when": rule.expires_when or "",
# Only on a rule that carries a check; its absence means the
# rule is a decision, not that nobody has looked.
**(
{"last_verified": rulebooks_svc.last_verified_label(rule)}
if rule.verify_with else {}
),
"topic_id": rule.topic_id,
"project_id": rule.project_id,
"similarity": float(score),
}
for score, rule in raw
],
"total": len(raw),
}
async def search(
q: str,
content_type: str = "all",
limit: int = 10,
project_id: int = 0,
system_id: int = 0,
) -> dict:
"""Semantic search over the user's existing notes and tasks — Scribe's recall.
Reach for this BEFORE answering a question about the user's work or starting
a task: the user's second-brain almost always already holds related prior
art. Check for an existing ticket before opening a new one (search with
content_type='task'), and for prior notes/decisions before re-deriving them.
Treating Scribe as the first place to look — not a place to only write — is
the difference between it being a trustworthy record and a write-only log.
Args:
q: search query string.
content_type: 'all' (default), 'note' (notes only), 'task' (tasks
only), or 'rule' (RULES only — the operator's standing
instructions, searchable by meaning since milestone 307).
Reach for 'rule' when you want to know whether a standing
instruction covers something: "is there a rule about release
tagging?". A hit carries the rule's `why` and `how_to_apply`,
which the session-start payload does not.
limit: maximum number of results (1-50).
project_id: Scope results to one project. PASS THE ACTIVE PROJECT'S ID
whenever a project is in scope (the one you entered with
enter_project) — otherwise this searches across ALL projects and
bleeds unrelated work into the result set. 0 = search everything
(use only when you genuinely want a cross-project sweep).
system_id: Narrow to records tagged to one System (a named
subsystem/area — enter_project lists them). Use when investigating
a specific subsystem: it cuts the candidates to records someone
deliberately filed under that area. 0 = no system filter.
list_system_records gives the same slice unranked.
Returns:
{"results": [{"id", "title", "body", "is_task", "tags", "similarity"}],
"total": int}
A result marked `shared: true` with an `owner` belongs to another user —
that person's suggestion, not the operator's own record or settled practice.
Weigh it on its merits and say whose it is when you use it.
"""
uid = current_user_id()
limit = max(1, min(limit, 50))
if content_type == "rule":
return await _search_rules(uid, q, limit)
is_task = {"note": False, "task": True}.get(content_type) # None => any
t0 = time.perf_counter()
raw = await semantic_search_notes(
uid, q, limit=limit, is_task=is_task,
project_id=project_id or None,
system_id=system_id or None,
# An explicit search reaches everything the operator may read, including
# records shared with them one-to-one.
scope="read",
)
record_retrieval(
user_id=uid, source="mcp_search", query=q,
threshold=DEFAULT_SIMILARITY_THRESHOLD, limit=limit,
project_id=project_id or None, is_task=is_task, results=raw,
duration_ms=(time.perf_counter() - t0) * 1000.0,
)
owners = await owner_names_for(
{int(note.user_id) for _s, note in raw if note.user_id != uid}
)
return {
"results": [
{
"id": note.id,
"title": note.title,
"body": (note.body or "")[:240],
"is_task": bool(note.is_task),
"tags": list(note.tags or []),
"similarity": float(score),
**(
{"shared": True, "owner": owners.get(int(note.user_id))}
if note.user_id != uid else {}
),
}
for score, note in raw
],
"total": len(raw),
}
async def retrieval_telemetry(days: int = 30) -> dict:
"""What the retrieval telemetry says about YOUR surfaces, over a window.
The read half of the loop the ranker's thresholds are meant to be tuned
from (#2975). Reach for it before changing a similarity threshold, a top-k,
or deciding whether a reranker is worth building — the alternative is
hand-probing the live instance, which is how the last such decision had to
be made.
Three readouts, from the three tables built for them:
`sources` — per retrieval surface (`auto_inject`, `write_path`,
`mcp_search`, …), from `retrieval_logs`: `calls`, `zero_result_calls`,
`cleared_threshold` (how often the best hit beat the threshold in force for
that call), the `top_score` spread (p10/p50/p90/min/max), `avg_result_count`
and `p90_duration_ms`. THE number to read first is `cleared_threshold`
against `calls`, with the spread beside it: a surface that clears its bar
on nearly every call is either well-tuned or too loose, and p10 says which.
READ `cleared_threshold` AND `zero_result_calls` TOGETHER, and check
`suppression` before concluding anything from either. A zero-result call is
two different events wearing one number: the ranker found nothing above the
bar, or it found only what this session had already been shown. Just the
first is evidence the bar is too high. `suppression` splits them where the
surface can tell — `zero_because_already_shown` comes off
`zero_result_calls` to leave the true ranker declines.
`suppression` is `null` when NO row in the window reported it, and that is
"not measured here", NOT "none suppressed". Surfaces that pass their
exclusions into the search never see what was dropped, so they cannot say.
Do not read a null as a zero: reading an artifact as a measurement is how
this surface got mis-scoped once already (#3311, #3497).
`usage` — NOTES ONLY, from `note_usage_events`, at the per-note grain
`retrieval_logs` cannot be indexed at: `surfaced` (ranked surfacings — a
scored surface CHOSE the record), `ambient` (the rest), `pulled` split into
`pulled_by_agent` / `pulled_by_human`, the distinct-note counts, and
`pull_through`. That ratio is the corpus-side precision signal: records
surfaced often and opened never are dead weight competing for the injection
budget every turn.
`pull_through` is AGENT pulls over RANKED surfacings, and both halves of
that matter. "Is this record dead weight?" is answered by any pull; "was
that injected line useful?" — the question a threshold or a reranker is
tuned against — only by a pull the agent made. Aggregating across the
mcp_/rest_ prefix would silently answer the wrong one.
`usage["by_source"]` — THE number to tune a threshold against, because the
top-level `pull_through` is a corpus average and averages the surfaces
together. Per surface: `notes_surfaced`, `notes_pulled`, `pull_through`,
and `ambient: true` on surfaces whose surfacings were not scored choices
(their ratio is null — "surfaced often, opened never" is not a judgment
about a record nothing chose). Read it as: of the distinct notes THIS
surface put in front of the agent, how many did the agent then open?
It is an UPPER BOUND per surface: a pull records the door it came
through, not the surface that led there, so a note surfaced by two surfaces
and opened once counts for both — attribution would need the session
identity #2085 declined to invent. `by_source_failed: true` means that one
query failed while the rest of the readout stood.
`rule_usage` — the same question for RULES, from `rule_usage_events`:
`surfaced` and `ambient`, `pulled` split into `pulled_by_agent` /
`pulled_by_human`, the distinct-rule counts, and `pull_through` on the same
definition (agent pulls over RANKED surfacings).
A SEPARATE BLOCK, not folded into `usage`, and reading it as one number
with that is the mistake to avoid. The corpora differ by orders of
magnitude — a few dozen eligible rules against thousands of notes — so a
blended ratio would be the note ratio with noise on it and would hide the
rule arm entirely.
`surfaced` VS `ambient` IS THE READING THAT MATTERS HERE. `surfaced` counts
rules a ranker chose — today only the write-path arm — and those are claims
a pull can settle. `ambient` counts BULK DELIVERIES: the SessionStart
preload, `list_always_on_rules`, and the `rules_payload` surfaces
(`enter_project`, `get_project`, `get_milestone`, `start_planning`,
`get_task`), which hand over the whole applicable set at once with nobody
choosing anything. A large `ambient` says the resident set is big and
arrives often — never that it is useful, and never that it is read.
`pull_through` therefore divides by `surfaced` alone. Fold the preload in
and growing the always-on set would depress the arm's measured precision
while trimming it would flatter it, for reasons having nothing to do with
the arm. To judge the PRELOAD instead, compare `ambient` against pulls of
those same rules over time: a resident set surfaced thousands of times and
opened never is the dead-weight signal, one tier up.
Read it against `sources["write_path_rule"]`. That arm was once believed
never to decline — the reading that scoped #3311 — but it was the arm's
`retrieval_logs` row being written only on calls that FOUND something, so
the zeros were missing rather than absent (#3497). Measured since, it
declines the large majority of its calls like any other surface.
EVERY COUNTER BLOCK CARRIES ITS OWN COVERAGE — `complete_from` and
`covers_window`. `complete_from` is when the number became trustworthy:
for one source, its first recorded row; for a section that sums several,
the LATEST of theirs, because a total is complete only once every
contributor was being written. `covers_window: false` means the window
reaches back further than the recording does, so the count is a fraction
of the period it appears to describe.
READ IT BEFORE COMPARING TWO NUMBERS, and especially before comparing
across a deploy. A counter added last week, read over a 30-day window,
reports a real count against an imagined denominator — and the result is
a plausible fraction rather than an obvious zero, which is what makes it
dangerous. That reading cost milestone #379 five steps aimed at a defect
that did not exist.
`covers_window` is null, never false, when nothing was ever recorded:
"no measurement" is not "partial measurement", the same distinction
`suppression`'s null carries a few paragraphs up.
A SOURCE SHOWING `calls: 0` WAS RECORDING AND MADE NO CALLS. `sources`
lists every source the table has ever held, not only those active in the
window, so a surface that stopped firing stays visible rather than
disappearing — being absent is reserved for a source that has never
recorded at all. Its score fields are null, not zero: the calls are a
real observation, the distribution is not one.
`rule_usage_failed: true` means that read failed while the rest of the
readout stood. The counts are still present so a caller can render, but
they are zeros meaning "could not find out", not "nothing happened" — do
not report a pull-through from a block carrying that flag.
Scoped to your own telemetry — a retrieval log records what your agent
asked for, query text included, and is not a shared record kind.
`read_failed: true` means the query itself failed — deliberately distinct
from an empty window, because those two looked identical for weeks once
(#2663) and every counter silently read zero.
Args:
days: window size, default 30. Clamped to at least 1.
"""
return await retrieval_summary(current_user_id(), days=days)
def register(mcp) -> None:
mcp.tool(name="search")(search)
mcp.tool(name="retrieval_telemetry")(retrieval_telemetry)