From 2b8f41229d70b0e433786095e89a46c586774f55 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 5 Oct 2026 20:02:37 -0400 Subject: [PATCH] refactor(retrieval): the notes arms run on the one pipeline - auto_inject, its reuse and lesson slots, the write path by meaning, and rule_via_lesson are specs (milestone 456 step 4, #4906) retrieval_pipeline gains the notes half: NoteArm / NoteSlot / NoteMoment / NoteIO / NoteResult and run_note_arm, which writes once the stages both notes arms copied: search, withhold this response's own menu (#3739), fresh/repeat split, the call row before any return (#3497, #3752), the band, the reserved slots in their order (reuse evicts, lesson extends), and the surfacing rows. The note renderer (_record_kind, _menu_name, _menu_passage, the seen pointer, menu_entry) moves with it, and run_via_lesson_arm takes rule_via_lesson. Behaviour-preserving, with flags for today's differences: notes still log BEFORE the band and rules after it (step 7's question). One deliberate change: a notes arm now fails open like the rule arms, so a failing search costs its lines and no longer the whole hook response. The I/O is resolved from plugin_context at call time (_note_io), so the existing patches keep working. The review re-run reads the AUTO_INJECT spec instead of restating it, and its guard now compares the two live searches. The registry declares the pipeline's notes fan-out sites. Co-Authored-By: Claude Opus 5.5 --- src/scribe/services/plugin_context.py | 860 ++++------------------ src/scribe/services/retrieval_pipeline.py | 659 +++++++++++++++++ src/scribe/services/retrieval_registry.py | 22 +- src/scribe/services/retrieval_review.py | 15 +- tests/test_note_pipeline.py | 277 +++++++ tests/test_retrieval_review.py | 62 +- tests/test_retrieval_surfaces.py | 4 +- tests/test_rule_usage_wiring.py | 30 +- tests/test_services_plugin_context.py | 3 +- tests/test_write_path_trigger.py | 3 +- 10 files changed, 1172 insertions(+), 763 deletions(-) create mode 100644 tests/test_note_pipeline.py diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index db107a4e..9167cf4b 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -17,7 +17,6 @@ from __future__ import annotations import logging import re import textwrap -import time from scribe.services import design_systems as design_systems_svc @@ -35,7 +34,7 @@ from scribe.services.embeddings import ( semantic_search_rules, ) from scribe.services import lesson_rules as lesson_rules_svc -from scribe.services.lessons import LESSON_NOTE_TYPE, claim_line +from scribe.services.lessons import LESSON_NOTE_TYPE from scribe.services.note_usage import record_surfaced from scribe.services.rule_usage import record_rule_surfaced from scribe.services.supersession import superseded_ids @@ -48,6 +47,8 @@ from scribe.services.retrieval_surfaces import ( from scribe.services import retrieval_pipeline as rp from scribe.services.retrieval_pipeline import ( # noqa: F401 - re-exported _RULEHINT_BAND, _rule_band, _rule_hint_line, checkpoint_for, checkpoint_reason, + VIA_LESSON_LIMIT, _menu_label, _menu_name, _menu_passage, _menu_seen_line, + _record_kind, menu_entry, ) from scribe.services.retrieval_telemetry import record_retrieval from scribe.services.settings import get_setting @@ -60,74 +61,9 @@ logger = logging.getLogger(__name__) # Defensive cap below Claude Code's 10k additionalContext limit. _MAX_CHARS = 9000 -# WHAT A MENU LINE CARRIES (#4364): the record's NAME, its kind and System, -# and the WHOLE passage that matched. Metadata plus the evidence, rather than a -# title asked to be both. -# -# The name, not the title. A snippet's or lesson's title is `name — when it -# applies` by construction (`embeddings.trigger_title`), because that join is -# what makes it rank on its situation. That is an EMBEDDING shape, and rendered -# as a menu line it ran to 1,500+ characters — the trigger paragraph spent -# again on every line, and again on every repeat. The trigger still arrives -# when it is what matched: it is in the chunk, and `_menu_passage` hands it -# over when the title was the whole match. -# -# The whole passage, not 200 characters of it. The search already chose the -# chunk that matched; the old cut kept its head and tail, and the head is the -# title every chunk is prefixed with — so the reader got the title twice and -# lost the middle, which is where the match was (lesson #4248). A chunk is at -# most ~1.4 KB (`embeddings._CHUNK_CHAR_BUDGET`), and it is shown once: a -# repeat is a one-line pointer (`_menu_seen_line`), not a second copy. - - -def _menu_name(title: str | None, note_type: str | None, data=None, body: str | None = "") -> str: - """The record's name — its title without the trigger composed into it.""" - title = (title or "(untitled)").replace("\n", " ").strip() - data = data if isinstance(data, dict) else {} - if note_type == "snippet": - from scribe.services.embeddings import TRIGGER_SEP - return (data.get("name") or title.partition(TRIGGER_SEP)[0]).strip() or title - if note_type == LESSON_NOTE_TYPE: - from types import SimpleNamespace - - from scribe.services.embeddings import untrigger_title - from scribe.services.lessons import lesson_trigger - trigger = lesson_trigger(SimpleNamespace(data=data, body=body or "")) - # One line even when the stored name is a story (#4797). - return claim_line(untrigger_title(title, trigger).strip() or title) - return title - - -def _menu_passage(title: str | None, chunk_text: str | None, name: str = "") -> str: - """The matched chunk on one line, without the title it was embedded under. - - Every chunk is `title\nsection` (`embeddings.embedding_text`), and `title` - here must be the EMBEDDED one (`embeddings.document_title`) — for a snippet - or lesson that is `name — trigger`, not the stored name — so the prefix is - stripped exactly. A chunk that WAS only the title — a short - record, or the head chunk of one — matched on the title, and for a - trigger-keyed kind the part of it the name line no longer shows is the - trigger: that is returned, because it is precisely what matched. - One line, so the menu's blockquote survives it. - """ - title = (title or "").strip() - text = (chunk_text or "").strip() - if title and text.startswith(title): - text = text[len(title):] - text = " ".join(text.split()) - if not text and name and title.startswith(name) and title != name: - text = " ".join(title[len(name):].lstrip(" —-").split()) - return text - - -def _menu_label(kind: str, systems: list[str] | None) -> str: - """`issue (done) · Plugin & hooks` — the kind, then where it belongs.""" - return " · ".join([kind, *systems]) if systems else kind - - -def _menu_seen_line(note_id: int, kind: str, name: str) -> str: - """A pointer to a record this session was already shown, not a copy of it.""" - return f"> - #{note_id} [{kind} · seen] {name}" +# The notes renderer — a record's name, kind, System and matched passage — +# lives in `retrieval_pipeline` with the rest of the notes stages (milestone +# 456 step 4) and is re-exported above. # How much of a named record's body sits under its line. The opening, not an @@ -201,7 +137,7 @@ AUTOINJECT_DEFAULT_TOP_K = SURFACES["auto_inject"].budget_default # Any two Python-shaped payloads share keywords, indentation and structure, so # the floor for "some code" is ~0.55-0.63 — auto-inject's 0.55 lands INSIDE that # noise band, and 6 of 8 probe payloads produced a nudge (4 of them noise). The -# margin gate can't rescue it either: _AUTOINJECT_BAND is relative to the top +# margin gate can't rescue it either: the notes band (retrieval_pipeline._NOTE_BAND) is relative to the top # hit, so with a single hit it never engages. # # 0.68 clears every measured false positive with margin and still sits 0.05 @@ -591,9 +527,6 @@ _CONCEPT_MAX_DECLS = 4 # the code itself (e.g. all we found was `f()`), so we keep the raw payload. _CONCEPT_MIN_CHARS = 16 -# Margin gate: drop any hit more than this far below the top hit's score, so a -# single strong match doesn't drag in a wall of barely-passing neighbours. -_AUTOINJECT_BAND = 0.10 # Hard ceiling on top-k regardless of the user's setting — this is an # awareness menu (titles only), never a content dump. # The budget ceiling, now shared by every surface rather than owned by this @@ -814,243 +747,6 @@ async def get_autoinject_config(user_id: int) -> dict: } -def _record_kind(note) -> str: - """The kind marker for an injected menu line — and, for a task, its status. - - The menu is drawn from every record that carries an embedding, so a snippet, - a stored process, an issue and a stray dev-log all arrive looking identical. - Recorded prior art only stands out if the line says what it is — and the kind - is also what tells the reader which tool opens it. - - Task-ness wins over `note_type` because it's the more useful distinction at a - glance: "there's an open issue about this" beats "there's a note about this". - - A TASK ALSO CARRIES ITS STATUS, because for that kind alone the line is - read as a claim about live work. A finished step and an open one rendered - identically is not a cosmetic gap: a done step was cited as a milestone's - open one on the strength of a line exactly like this, which carries an id, - a kind and a title and said nothing about where the work stood (#4154). - Only for tasks — a note or a snippet has no status to be wrong about. - """ - if note.is_task: - kind = "issue" if note.task_kind == "issue" else "task" - # No fallback for a missing status: `is_task` IS `status is not None` - # (models/note.py), so a branch for a task without one could never be - # taken, and a dead branch is a claim about the data that isn't true. - # - # Parenthesised rather than dot-joined: the write-path prior-art line - # joins its own fields with " · ", so a dotted status would read as - # another flag beside `seen` instead of as part of the kind. - return f"{kind} ({note.status})" - return note.note_type or "note" - - -_REUSE_KINDS = ("snippet", "process") - - -async def _reserve_slot_for_reuse( - user_id: int, - query: str, - kept: list, - cfg: dict, - *, - project_id: int | None, - already: set[int], -) -> list: - """Guarantee the reuse-shaped kinds one slot, if one clears threshold (#2246). - - Ranking by raw cosine is blind to what KIND of record answers what kind of - ask, and the corpus makes that fatal rather than merely imperfect: Scribe's - project records are *about software work*, so a task titled "surface snippets - before the agent writes code" is a near-perfect lexical match for "write a - function…" while being useless as an answer to it. Measured live, a prompt - asking for a helper returned three records about BUILDING the retrieval - system and zero snippets. - - The bias is structural and gets WORSE as the project record grows — which is - the direction Scribe is supposed to grow. Snippets are ~0.5% of the corpus - here; no threshold tuning fixes a 200:1 ratio. - - So the reserved hit is deliberately NOT held to the margin band. The band - measures distance from the top overall score, and that top score is the very - thing snippets lose to. It still has to clear the configured threshold, so a - weak snippet cannot buy the slot — silence stays the default. - """ - if any(_record_kind(n) in _REUSE_KINDS for _s, n in kept): - return kept # reuse already represented; nothing to do - - top_k = cfg["top_k"] - _t0 = time.perf_counter() - _rep: dict = {} - # THE LEDGER IS NOT AN EXCLUSION HERE EITHER (#4101) — but what is already - # in `kept` still is, and the two are different claims. A record sitting in - # this call's own menu must not be shown twice in it; a record shown in an - # EARLIER call is exactly what the slot should be allowed to spend itself - # on, because a snippet that was relevant then and is relevant now is the - # reuse case rather than a duplicate of it. - reuse = await semantic_search_notes( - user_id, query, - limit=1, - threshold=cfg["threshold"], - project_id=project_id, - exclude_ids={int(n.id) for _s, n in kept}, - note_type=_REUSE_KINDS, - scope="browse", - report=_rep, - ) - fresh_reuse = [(s, n) for s, n in reuse if int(n.id) not in already] - # A real semantic query competing for a menu slot — logged like the scored - # arm it displaces. Before this, the hit it PUSHED OUT was in - # retrieval_logs and the query that pushed it out was not, so the slot - # could never be evaluated against what it replaced (#2463; #1038 and - # #2085 are gated on this ledger being complete). - record_retrieval( - user_id=user_id, source="reuse_slot", query=query, - threshold=cfg["threshold"], limit=1, project_id=project_id, - is_task=None, results=fresh_reuse, - best_available=_rep.get("best_available_score"), - best_available_id=_rep.get("best_available_id"), - searched=bool(_rep.get("searched", True)), - suppressed=len(reuse) - len(fresh_reuse), - duration_ms=(time.perf_counter() - _t0) * 1000.0, - ) - # Verify the kind rather than trusting the query that asked for it, and - # dedup against this call's own menu. This slot exists FOR reuse kinds — a - # slot silently spent on something else is worse than no slot, because the - # line is indistinguishable from one that earned its place on score. - kept_ids = {int(n.id) for _s, n in kept} - fresh = [ - (s, n) for s, n in reuse - if _record_kind(n) in _REUSE_KINDS and int(n.id) not in kept_ids - ][:1] - if not fresh: - return kept - - # Take the LAST slot, never the first: the strongest overall hit is still the - # best answer to the prompt, and displacing it would trade one blindness for - # another. - if len(kept) >= top_k: - return kept[:top_k - 1] + fresh - return (kept + fresh)[:top_k] - - -async def _reserve_slot_for_lesson( - user_id: int, - query: str, - kept: list, - cfg: dict, - *, - project_id: int | None, - already: set[int], -) -> tuple[list, int | None]: - """Guarantee a lesson one slot, if one clears the bar (milestone 385 step 5). - - WHY A SLOT, AND WHAT IT ACTUALLY DISPLACES - - The step's own framing was that "a slot spent on a lesson is a slot not - spent on a rule that binds". That is not what happens here, and the - correction matters for judging the cost: the notes menu and the rule hints - are separate functions with separate budgets, composed by the caller - (`build_prompt_rule_hint` says why). A line reserved in THIS menu displaces - a note, a snippet or an issue — never a rule. - - THE ASYMMETRY, which is `preference_slot`'s argument on a different corpus: - - - a NOTE crowded out of this menu is a lost convenience. It stays - searchable, and the operator can ask for it. - - a RULE crowded out still fires at an act arm. The prompt hit is a - preview of a second chance. - - a LESSON crowded out is the feature failing. A lesson exists only to be - met at the moment it applies — nobody browses lessons looking for one — - so the arm that surfaces it IS its delivery, and the loss is total and - silent. Silent delivery failure is the exact shape #3727 recorded: an - insight with no home arrived as a rule proposal instead. - - And the ratio only moves one way. Lessons are by design rare and hard-won - while project records grow with the work, which is the 200:1 problem - `reuse_slot` was built for (#2246), before it has had a chance to be - measured here. - - THE SLOT BUYS POSITION, NOT A LOWER BAR. It reserves at the menu's own - threshold, so a weak lesson cannot buy the line and silence stays the - default — the discipline both existing slots keep. - - IT EXTENDS, IT NEVER DISPLACES, siding with `preference_slot` over - `reuse_slot`. Two reasons, and the second is the one that would be hard to - recover later: a displaced hit was returned by the general search and sits - in that call's `retrieval_logs` row, so evicting it makes the two tables - disagree about the same call for a reason nothing in the data explains - (#3668, and milestone #379 is what that costs). The first is voice — a - lesson does not bind, and a record that does not bind should not be able - to throw a better-scoring one off the menu. - - Returns the possibly-extended list and the id the slot spent. The caller - needs that id to keep each source's surfaced set matching its own log row: - this slot records its own surfacing under its own name, so counting it - again under `auto_inject` would double it. - """ - if any(_record_kind(n) == LESSON_NOTE_TYPE for _s, n in kept): - return kept, None # a lesson already placed on score - - _t0 = time.perf_counter() - _rep: dict = {} - # KIND-FILTERED, so the slot can only ever be spent on what it is for — - # `preference_slot`'s reasoning: verifying the kind after an open search - # would let a stray note buy the line, and that line would be - # indistinguishable from one that earned its place. - # - # `include_global_kinds` is the half that makes a lesson reachable at all - # from a project it was not written on, which is this kind's whole claim - # (#3730). Without it the slot would be a guarantee that silently only - # applies to lessons learned here. - found = await semantic_search_notes( - user_id, query, - limit=1, - threshold=cfg["threshold"], - project_id=project_id, - exclude_ids={int(n.id) for _s, n in kept}, - note_type=(LESSON_NOTE_TYPE,), - include_global_kinds=True, - scope="browse", - report=_rep, - ) - fresh = [(s, n) for s, n in found if int(n.id) not in already] - # ITS OWN SOURCE, from the first deploy. This slot is a claim that a kind - # deserves a guaranteed line, and a claim like that has to be falsifiable: - # `best_available_id` (#3807) names the lesson a bar refused, and the - # result count says how often the guarantee was actually spent. Without - # this row the question "does the lesson slot earn its line?" would have no - # data behind it in either direction — which is #2463's finding, recorded - # about the slot that shipped without one. - record_retrieval( - user_id=user_id, source="lesson_slot", query=query, - threshold=cfg["threshold"], limit=1, project_id=project_id, - is_task=None, results=fresh, - best_available=_rep.get("best_available_score"), - best_available_id=_rep.get("best_available_id"), - searched=bool(_rep.get("searched", True)), - suppressed=len(found) - len(fresh), - duration_ms=(time.perf_counter() - _t0) * 1000.0, - ) - kept_ids = {int(n.id) for _s, n in kept} - slot = [ - (s, n) for s, n in found - if _record_kind(n) == LESSON_NOTE_TYPE and int(n.id) not in kept_ids - ][:1] - if not slot: - return kept, None - - slot_id = int(slot[0][1].id) - # FRESH ONLY, matching the row above: a ledger repeat is rendered (#4101) - # but is not a new surfacing, so this source's two tables stay identical. - if slot_id not in already: - record_surfaced( - user_id=user_id, note_ids=[slot_id], source="lesson_slot", - project_id=project_id, - ) - return kept + slot, slot_id - - async def build_autoinject_hint( user_id: int, query: str, @@ -1062,7 +758,7 @@ async def build_autoinject_hint( The four anti-bloat gates (see the module + milestone-93 design): 1. high-confidence threshold (stricter than pull) — set per-user; - 2. margin gate — keep only hits within _AUTOINJECT_BAND of the top score; + 2. margin gate — keep only hits within retrieval_pipeline._NOTE_BAND of the top score; 3. session marking — caller passes already-injected ids as `exclude_ids` and they are rendered again with `[seen]`, never withheld (#4101); 4. title-first payload — id + kind + title + score only, never bodies. @@ -1107,83 +803,23 @@ async def build_autoinject_hint( # before. A note line is a title and a score — the repeat costs about as # much as the comma in this sentence — so there is nothing here to save. already = {int(i) for i in (exclude_ids or [])} - t0 = time.perf_counter() - _rep_ai: dict = {} - hits = await semantic_search_notes( - user_id, q, - limit=cfg["top_k"], - threshold=cfg["threshold"], - project_id=(project_id or None), - # LESSONS ARE PROJECT-INDEPENDENT (#3730), so they join this menu's - # candidate set from wherever they were learned. Widening it is what - # makes the reserved slot below falsifiable rather than decorative: if - # the slot were the only path a lesson had, the general contest would - # be permanently closed to the kind and "the slot earns its line" would - # be true by construction. The switch adds nothing else — it ORs in - # `GLOBAL_NOTE_TYPES` and no other kind is in it. - include_global_kinds=True, - # Injection is the one retrieval nobody asked for, so it takes the BROWSE - # scope: never a record shared one-to-one with the operator. What can - # still appear is a collaborator's note inside a shared project — legible - # only because the line below names its owner. - scope="browse", - report=_rep_ai, + # The ranked half is `retrieval_pipeline.run_note_arm` (milestone 456): + # search, the fresh/repeat split, the call row, the band, the reuse and + # lesson slots in that order, and the surfacing rows are written there. + # What is decided HERE is what makes this moment this moment — the query + # the operator's words became, and the records they named by number. + result = await rp.run_note_arm( + rp.AUTO_INJECT, + rp.NoteMoment( + user_id=user_id, query=q, project_id=project_id, + seen=frozenset(already), named=frozenset(named_ids), + ), + floor=cfg["threshold"], budget=cfg["top_k"], io=_note_io(), ) - # `results=fresh` and `suppressed` together, on the rule arms' contract - # (#3752): a rendered repeat is not a new surfacing, so it stays out of the - # row's result set and is COUNTED instead. That keeps this source's surfaced - # set identical to its own log row (#3668) while making a zero-result call - # readable — `result_count == 0` with `suppressed_count > 0` is "everything - # that matched, this session has already seen", which is a different fact - # about the bar from "nothing cleared it" and used to be unreportable here. - # A record the operator NAMED is counted the same way: it is shown, but by - # the lookup above and booked under `named_ref`, so this arm did not - # surface it and must not claim it. - fresh = [ - (s, n) for s, n in hits - if int(n.id) not in already and int(n.id) not in named_ids - ] - record_retrieval( - user_id=user_id, source="auto_inject", query=q, - threshold=cfg["threshold"], limit=cfg["top_k"], - project_id=(project_id or None), is_task=None, results=fresh, - best_available=_rep_ai.get("best_available_score"), - best_available_id=_rep_ai.get("best_available_id"), - searched=bool(_rep_ai.get("searched", True)), - suppressed=len(hits) - len(fresh), - duration_ms=(time.perf_counter() - t0) * 1000.0, - ) - if not hits and not named: + kept = result.menu + if not kept and not named: return empty - kept: list = [] - lesson_slot_id = None - if hits: - # Margin gate: keep only hits close to the strongest one. Computed over - # ALL hits, repeats included — the band measures distance from the top - # SCORE, and letting the ledger move that cutoff would make "you were - # shown this" change what counts as relevant, which is the axis - # independence the rule band keeps for the same reason. - top_score = hits[0][0] - kept = [(s, n) for s, n in hits if s >= top_score - _AUTOINJECT_BAND] - # The slots are told about the named records as if already shown, so a - # slot that picks one does not book a surfacing the lookup booked too. - shown = already | set(named_ids) - kept = await _reserve_slot_for_reuse( - user_id, q, kept, cfg, project_id=(project_id or None), - already=shown, - ) - # AFTER the reuse slot, because that one evicts the menu's weakest hit - # while this one extends: running them the other way round would let a - # reserved lesson be the line reuse throws off, and a slot that another - # slot can silently undo is not a guarantee. - kept, lesson_slot_id = await _reserve_slot_for_lesson( - user_id, q, kept, cfg, project_id=(project_id or None), - already=shown, - ) - # A named record is shown once, in its own block, never again as a match. - kept = [(s, n) for s, n in kept if int(n.id) not in named_ids] - # A collaborator's note can reach this menu via a shared project, and the # operator never asked for it — so say whose it is. Unattributed, it reads as # something they wrote and settled. @@ -1192,16 +828,19 @@ async def build_autoinject_hint( if n.user_id != user_id }) - # A superseded record is DEMOTED, not removed (#278) — so one can still reach - # this menu, and when it does the reader has to be told. An agent handed - # stale material with nothing marking it acts on it with full confidence, - # which is worse than never having surfaced it. One query for the whole menu. + # A superseded record is DEMOTED, not removed (#278) — `menu_entry` says + # so on the line. One query for the whole menu. stale = await superseded_ids([*named_ids, *(int(n.id) for _s, n in kept)]) systems = await system_names_for( {i for i in named_ids if i not in already} | {int(n.id) for _s, n in kept if int(n.id) not in already} ) + def _shared_by(note) -> str: + if note.user_id == user_id: + return "" + return owners.get(int(note.user_id)) or "another user" + lines: list[str] = [] note_ids: list[int] = [] # FIRST, because the operator said which record they meant and everything @@ -1217,24 +856,15 @@ async def build_autoinject_hint( for note in named: nid = int(note.id) note_ids.append(nid) - kind = _record_kind(note) - name = _menu_name(note.title, note.note_type, note.data, note.body) - if nid in already: - line = _menu_seen_line(nid, kind, name) - if nid in stale: - line += " — SUPERSEDED" - lines.append(line) - continue - line = f"> - #{nid} [{_menu_label(kind, systems.get(nid))}] \"{name}\"" - if nid in stale: - line += " — SUPERSEDED, a later record covers this; check that first" - if note.user_id != user_id: - who = owners.get(int(note.user_id)) or "another user" - line += f" — shared by {who}" - lines.append(line) - opening = _named_opening(note.body) - if opening: - lines.append(f"> ↳ {opening}") + # The OPENING under the line, not a passage: nothing matched here, so + # there is no passage to keep, and what the reader needs is what the + # record IS — which is where a record says it. + lines.extend(menu_entry( + nid, kind=_record_kind(note), + name=_menu_name(note.title, note.note_type, note.data, note.body), + systems=systems.get(nid), seen=nid in already, stale=nid in stale, + shared_by=_shared_by(note), under=_named_opening(note.body), + )) # "records", not "notes" — the menu can hold snippets, processes and tasks # too, and the kind marker on each line is only legible if the header doesn't @@ -1275,78 +905,35 @@ async def build_autoinject_hint( "what you are doing and use your judgement — a lesson is not a " "rule and binds nothing." ) - # From THIS arm's own search (`_rep_ai`), so a chunk is only ever paired - # with the query that actually matched it. - menu_chunks = _rep_ai.get("best_chunk") or {} - menu_ids: list[int] = [] for score, note in kept: nid = int(note.id) note_ids.append(nid) - menu_ids.append(nid) - kind = _record_kind(note) # The NAME, not the title (#4364): a snippet's or lesson's title is its # embedding shape, trigger and all, and ran past 1,500 characters here. name = _menu_name(note.title, note.note_type, note.data, note.body) - if nid in already: - # A POINTER, not a copy (#4364). The record is in this session's - # context already — the ledger is cleared at compaction, so "seen" - # stays true — and re-rendering it spent its whole line again for - # nothing. What the reader needs is the reminder that it matched - # again, and the id to open it if it has scrolled out of mind. - line = _menu_seen_line(nid, kind, name) - if nid in stale: - line += " — SUPERSEDED" - lines.append(line) - continue - line = f"> - #{nid} [{_menu_label(kind, systems.get(nid))}] \"{name}\" ({score:.2f})" - if nid in stale: - line += " — SUPERSEDED, a later record covers this; check that first" - if note.user_id != user_id: - who = owners.get(int(note.user_id)) or "another user" - line += f" — shared by {who}, treat as a suggestion" - lines.append(line) # The passage that earned the line, WHOLE, indented under it (#4364). # Absent when the record has no stored chunk — an un-embedded row, or # the reserved lesson and reuse slots, which are fetched by their own # queries and so are not in this search's report. No fallback to the # body's opening: on a menu that would be a line of preamble dressed as # a reason, and a reader cannot tell the two apart once indented alike. - passage = _menu_passage( + passage = "" if nid in already else _menu_passage( document_title(note.title, note.note_type, note.data, note.body), - (menu_chunks.get(nid) or {}).get("text"), name, + (result.chunks.get(nid) or {}).get("text"), name, ) - if passage: - lines.append(f"> ↳ {passage}") + who = _shared_by(note) + lines.extend(menu_entry( + nid, kind=_record_kind(note), name=name, systems=systems.get(nid), + seen=nid in already, stale=nid in stale, score=score, + shared_by=f"{who}, treat as a suggestion" if who else "", + under=passage, + )) - # Records what SURVIVED the margin gate, not what the ranker returned — the - # menu the agent actually saw. retrieval_logs already holds the full - # candidate set for threshold tuning; conflating the two would make - # "surfaced" mean two different things depending on the surface (#2085). - # - # FRESH ONLY, which is the same cut the log row above takes (#4101). A - # repeat is rendered but is not a new surfacing, and counting it again would - # make this table disagree with `retrieval_logs` about the same call — - # #3668's identity, which is the cheapest true statement available about - # this pair of tables and is not worth a marker's convenience. - # THE RESERVED LESSON IS NOT THIS ARM'S SURFACING. It was fetched by its own - # query and already recorded under `lesson_slot`, so counting it here would - # book one delivery twice and leave `lesson_slot`'s two tables describing - # different numbers of the same event — #3668's identity, which is the - # cheapest true statement available about this pair of tables. It stays in - # `note_ids`, which is the session LEDGER and must list every line rendered. - record_surfaced( - user_id=user_id, - note_ids=[ - i for i in menu_ids - if i not in already and i != lesson_slot_id - ], - source="auto_inject", - project_id=project_id, - ) # A lookup, not a ranking, so it writes no retrieval_logs row — there is no # score or bar to tune — and its surfacings are its own source, which is - # what lets pull-through say whether a named record gets opened. + # what lets pull-through say whether a named record gets opened. The + # ranked menu's own rows were written by the pipeline. named_fresh = [i for i in named_ids if i not in already] if named_fresh: record_surfaced( @@ -1369,97 +956,46 @@ async def build_autoinject_hint( } -# ── A rule reached through its lessons (milestone 440, #4633) ────────────── -# -# A rule's own document is written in the rule's words, which are general by -# design; the situations that keep proving it are often closer to what a -# session is actually doing. A lesson JUDGED to be an instance of a rule (a -# CONFIRMED link) carries that situation, so a lesson matching the moment -# brings its rule along — in rule voice, naming the lesson that reached it. -# -# ITS OWN SLOT, NOT A RULE SLOT, as a stated default. The plan asked for this -# to be settled by measurement, and there is nothing to measure until links -# are confirmed (#4632). Until then the asymmetry decides it: a via-lesson -# line that took a rule slot could push out a rule the ranker matched -# DIRECTLY, a stronger claim displaced by a weaker one, while an extra line -# costs one line. `rule_via_lesson` is logged as its own source so the -# question can be answered from data later (#4636). -VIA_LESSON_LIMIT = 1 -# Lessons fetched before keeping only the linked ones. The search cannot be -# told "linked lessons only", so it overfetches and filters; on a corpus where -# most lessons are unlinked, a fetch of one would almost always be spent on a -# lesson that carries nothing. -_VIA_LESSON_OVERFETCH = 10 +def _note_io() -> rp.NoteIO: + """The ranker and recorders the notes arms report to, read at call time — + `_rule_io`'s reason: whatever this module's names are when the arm runs.""" + return rp.NoteIO( + search=semantic_search_notes, + record_retrieval=record_retrieval, + record_surfaced=record_surfaced, + ) async def _rules_via_lessons( user_id: int, query: str, *, project_id: int | None, skip: set[int], held: set[int], where: str, ) -> tuple[list[str], list[int]]: - """Rule lines reached through a matching lesson's CONFIRMED links. + """Rule lines reached through a matching lesson's CONFIRMED links — the + pipeline's via-lesson arm (`retrieval_pipeline.run_via_lesson_arm`, where + the design is written), with its I/O read from this module at call time. - The lesson bar is the notes menu's own threshold — the bar a lesson has to - clear to be shown at all — so a lesson too weak to surface cannot carry a - rule in. `skip` is every rule this response already names plus the - session ledger: suppression applies to the RULE, whichever lesson reached - it. Returns (lines, rule ids shown). Fails open, like every arm. + The lesson bar is the notes menu's own threshold. Returns (lines, rule ids + shown). """ - try: - confirmed = await lesson_rules_svc.confirmed_lessons(user_id) - if not confirmed: - return [], [] - bar = (await get_autoinject_config(user_id))["threshold"] - t0 = time.perf_counter() - _rep: dict = {} - found = await semantic_search_notes( - user_id, query, limit=_VIA_LESSON_OVERFETCH, threshold=bar, - project_id=project_id, note_type=(LESSON_NOTE_TYPE,), - include_global_kinds=True, scope="browse", report=_rep, - ) - matched = [(s, n) for s, n in found if int(n.id) in confirmed] - by_lesson = await lesson_rules_svc.confirmed_rules_in_scope( - user_id, [int(n.id) for _s, n in matched], project_id, - ) - candidates = [ - (s, rule, n) for s, n in matched for rule in by_lesson.get(int(n.id), []) - ] - chosen: list = [] - taken: set[int] = set(skip) - for s, rule, n in candidates: - if rule.id in taken: - continue - taken.add(rule.id) - chosen.append((s, rule, n)) - if len(chosen) >= VIA_LESSON_LIMIT: - break - # Logged whenever a search ran, results or not — the #3497 guard. The - # score is the LESSON's, because the lesson is what was matched; the - # best-available id is left off for the same reason, since the row's - # results are rules and an id beside them would read as a rule id. - record_retrieval( - user_id=user_id, source="rule_via_lesson", query=query, - threshold=bar, limit=VIA_LESSON_LIMIT, project_id=project_id, - is_task=None, results=[(s, rule) for s, rule, _n in chosen], - duration_ms=(time.perf_counter() - t0) * 1000.0, - best_available=_rep.get("best_available_score"), - searched=bool(_rep.get("searched", True)), - suppressed=len({r.id for _s, r, _n in candidates}) - len(chosen), - ) - if not chosen: - return [], [] - rule_ids = [rule.id for _s, rule, _n in chosen] - record_rule_surfaced(user_id=user_id, rule_ids=rule_ids, source="rule_via_lesson") - lines = [ - _rule_hint_line(rule, where=where, seen=False, held=rule.id in held) - + f" Reached through lesson #{n.id} " - + f"\u201c{_menu_name(n.title, n.note_type, n.data, n.body)}\u201d, " - + "a recorded instance of it." - for _s, rule, n in chosen - ] - return lines, rule_ids - except Exception: - logger.debug("rule-via-lesson arm failed", exc_info=True) - return [], [] + async def bar() -> float: + return (await get_autoinject_config(user_id))["threshold"] + + result = await rp.run_via_lesson_arm( + rp.RuleMoment( + user_id=user_id, query=query, project_id=project_id, where=where, + held=frozenset(held), + ), + skip=frozenset(skip), + io=rp.ViaLessonIO( + search=semantic_search_notes, + linked=lesson_rules_svc.confirmed_lessons, + rules_for=lesson_rules_svc.confirmed_rules_in_scope, + floor=bar, + record_retrieval=record_retrieval, + record_rule_surfaced=record_rule_surfaced, + ), + ) + return result.lines, result.rule_ids async def _add_rules_via_lessons( @@ -2026,171 +1562,73 @@ async def build_write_path_hint( # empty mapping means every line falls back to its title alone. wp_chunks: dict[int, dict] = {} if remaining > 0 and query: - t0 = time.perf_counter() - # Pulled-and-already-listed ids stay in the query (as evidence for - # `resembles`) but never in the menu, so the limit has to cover them. - # `seen` is this call's own menu now, which is the only thing left that - # is a reason to withhold (#4101). - in_menu = seen - pulled_in_menu = in_menu & set(pulled) - _rep_wp: dict = {} - hits = await semantic_search_notes( - user_id, query, - limit=remaining + len(pulled_in_menu), - threshold=cfg["threshold"], - project_id=scope_project, - exclude_ids=in_menu - set(pulled), - # Snippets AND recorded experience (#2246). This arm was - # snippets-only, which is auto-inject's mistake inverted: an issue - # saying "we tried this and it deadlocked", or a dev-log recording - # how a problem was solved, is prior art for the code about to be - # written — arguably better prior art than a resembling helper, - # because it says what NOT to do. - # - # `task_kind="issue"` keeps the open to-do list out. A task titled - # "add debouncing to the search box" resembles the code being - # written and answers nothing; an ISSUE is corrective work with a - # root cause in it, and a non-task note is durable knowledge. Both - # earned their place; a todo did not. - # - # AND LESSONS (milestone 385 step 5). This arm is kind-FILTERED, so - # a kind absent from this tuple is not merely outranked here — it - # is unreachable, and nothing reports an arm that never had the - # candidate (#3702). The founding example of the kind is a lesson - # about a code shape ("two absolutely-positioned siblings"), which - # is the moment this arm fires and no other. - # - # NO RESERVED SLOT HERE, unlike the prompt menu. This arm fires - # before EVERY Write and Edit, where a guaranteed extra line is a - # guaranteed extra interruption per keystroke-batch — the same - # argument that keeps the act arms' budgets tight. And the contest - # is already fair: the field is snippets, issues and lessons rather - # than the whole corpus, so the 200:1 dilution a slot answers is - # not what happens here. Lessons surfaced by this arm are - # identifiable in the telemetry by their kind. - note_type=("snippet", "note", LESSON_NOTE_TYPE), - task_kind="issue", - # Project-independent, for the reason the prompt menu passes it: - # a lesson's claim is that it transfers, and an arm scoped to the - # project it was written on cannot test that claim (#3730). - include_global_kinds=True, - # Same reasoning as auto-inject: nobody asked for this, so it takes - # the browse scope and never surfaces a one-to-one direct share. - scope="browse", - report=_rep_wp, + # The ranked half is `retrieval_pipeline.run_note_arm` with the + # WRITE_PATH spec (milestone 456): which kinds it asks for and why, + # the withheld-menu accounting (#3739), the fresh/repeat split, the + # call row and the surfacing rows are written there. What is decided + # HERE is the menu above it — what place and sync already listed is + # withheld, and a PULLED snippet among those is still scored, because + # its resemblance to this payload is the stamping feed's evidence. + result = await rp.run_note_arm( + rp.WRITE_PATH, + rp.NoteMoment( + user_id=user_id, query=query, project_id=project_id, + seen=frozenset(excluded), in_menu=frozenset(seen), + still_scored=frozenset(pulled), + ), + floor=cfg["threshold"], budget=remaining, io=_note_io(), ) - wp_chunks = _rep_wp.get("best_chunk") or {} + wp_chunks = result.chunks resembles = { - int(note.id): float(score) for score, note in hits + int(note.id): float(score) for score, note in result.answered if int(note.id) in pulled } - shown = [(s, n) for s, n in hits if int(n.id) not in in_menu] - # WHAT THIS ARM WITHHELD AFTER THE SEARCH ANSWERED, and the reason - # `best_available_score` cannot always be reported here (#3739 again, - # from the side its fix did not reach). - # - # This arm is the one note arm that filters TWICE. `exclude_ids` takes - # the already-listed ids into the search, but the PULLED ones among them - # stay in the query deliberately — `resembles` above needs them — and - # are dropped in the line above instead. So the score the search - # reported is PRE that drop while the row's `result_count` is POST it, - # and a record already listed in this same menu could be logged as - # something the BAR turned away. Live proof on the first read after - # #3739 shipped: write_path's near-miss max was 0.822 while the lowest - # score it ever RETURNED was 0.6857 — a "rejection" that beat every - # acceptance. - # - # So the honest answer is null — "not measured on this call" — whenever - # this filter removed anything, because then the bar is not the only - # thing that turned something away and the reported score may belong to - # a record we withheld ourselves. Calls where nothing was dropped keep - # reporting it, which is most of them. - # - # THE LEDGER IS NO LONGER PART OF THIS (#4101), and that is why the - # suppression column can now be filled in where it could not before. - # The old objection was that a count here would be PARTIAL — covering - # the drops made in this function but not the ones `exclude_ids` made - # inside the search — and a partial number under a name that reads as - # complete is the substitution this milestone exists to stop. That was - # right while the ledger was one of the things `exclude_ids` carried. - # It no longer is: every ledger repeat comes back from the search and is - # rendered, so `suppressed` counts all of them and none are hidden - # inside the query. What `exclude_ids` still removes is this call's own - # menu, which is not suppression at all — those records ARE being shown, - # one block further up. - withheld_here = len(hits) - len(shown) - hits = shown[:remaining] - fresh = [(s, n) for s, n in hits if int(n.id) not in excluded] - record_retrieval( - user_id=user_id, source="write_path", query=query, - threshold=cfg["threshold"], limit=remaining, - # is_task is None, not False: this arm now returns issues too, and - # recording it as a notes-only retrieval would misdescribe the - # candidate set the threshold is being tuned against. - project_id=scope_project, is_task=None, results=fresh, - best_available=( - None if withheld_here else _rep_wp.get("best_available_score") - ), - # Withheld on the SAME condition as the score. A surviving id - # beside a null score would name a record without saying what it - # scored, which is the pair disagreeing in the other direction. - best_available_id=( - None if withheld_here else _rep_wp.get("best_available_id") - ), - searched=bool(_rep_wp.get("searched", True)), - suppressed=len(hits) - len(fresh), - duration_ms=(time.perf_counter() - t0) * 1000.0, - ) - if hits: - top_score = hits[0][0] - for score, note in hits: - if score < top_score - _AUTOINJECT_BAND: - continue - # Name the kind unless it's a snippet — the menu's default and - # the header's default reading. An issue or a dev-log offered - # here is a different KIND of claim ("this was already tried") - # and an unlabelled line would be read as "here is code to - # reuse", which is the opposite of what it says. - kind = _record_kind(note) - marker = ( - f"similar {score:.2f}" if kind == "snippet" - else f"similar {score:.2f} · {kind}" - ) - # The repeat marker rides the same dotted list as the kind, so a - # reference costs four characters and needs no second line - # (#4101). Same word as the auto-inject menu deliberately: a - # reader meeting `seen` on two different surfaces should not - # have to work out whether they mean the same thing. - if int(note.id) in excluded: - marker += " · seen" - scored.append(( - marker, - { - "id": int(note.id), "title": note.title, "user_id": note.user_id, - # The name the line shows, and whether this session has - # it already — carried as data for the reason `kind` is - # (#4364): the line is built from facts, not from - # re-reading its own marker. - "name": _menu_name(note.title, note.note_type, note.data, note.body), - # What its chunks are prefixed with, for stripping. - "doc_title": document_title( - note.title, note.note_type, note.data, note.body, - ), - "seen": int(note.id) in excluded, - # Carried, not re-read off the rendered marker. The - # marker is prose assembled for a human and it already - # varies by kind, language and the `seen` flag — a - # header that decided what to say by matching substrings - # in it would break the next time a marker is reworded, - # silently and in the direction of saying nothing. - "kind": kind, - # Carried so the line can disclose a cross-language hit - # (#2244). The semantic arm is where these actually arise — - # a snippet recorded at the path you're editing is almost - # never in another language, but a concept match easily is. - "language": (note.data or {}).get("language") if note.data else None, - }, - )) + for score, note in result.menu: + # Name the kind unless it's a snippet — the menu's default and + # the header's default reading. An issue or a dev-log offered + # here is a different KIND of claim ("this was already tried") + # and an unlabelled line would be read as "here is code to + # reuse", which is the opposite of what it says. + kind = _record_kind(note) + marker = ( + f"similar {score:.2f}" if kind == "snippet" + else f"similar {score:.2f} · {kind}" + ) + # The repeat marker rides the same dotted list as the kind, so a + # reference costs four characters and needs no second line + # (#4101). Same word as the auto-inject menu deliberately: a + # reader meeting `seen` on two different surfaces should not + # have to work out whether they mean the same thing. + if int(note.id) in excluded: + marker += " · seen" + scored.append(( + marker, + { + "id": int(note.id), "title": note.title, "user_id": note.user_id, + # The name the line shows, and whether this session has + # it already — carried as data for the reason `kind` is + # (#4364): the line is built from facts, not from + # re-reading its own marker. + "name": _menu_name(note.title, note.note_type, note.data, note.body), + # What its chunks are prefixed with, for stripping. + "doc_title": document_title( + note.title, note.note_type, note.data, note.body, + ), + "seen": int(note.id) in excluded, + # Carried, not re-read off the rendered marker. The + # marker is prose assembled for a human and it already + # varies by kind, language and the `seen` flag — a + # header that decided what to say by matching substrings + # in it would break the next time a marker is reworded, + # silently and in the direction of saying nothing. + "kind": kind, + # Carried so the line can disclose a cross-language hit + # (#2244). The semantic arm is where these actually arise — + # a snippet recorded at the path you're editing is almost + # never in another language, but a concept match easily is. + "language": (note.data or {}).get("language") if note.data else None, + }, + )) menu = (placed + scored)[:max(0, top_k - len(synced))] @@ -2403,12 +1841,12 @@ async def build_write_path_hint( for marker, item in menu: # A rendered repeat is not a new surfacing (#4101) — same cut the log # row takes, so this table and `retrieval_logs` keep agreeing about the - # same call (#3668). - if int(item["id"]) in excluded: + # same call (#3668). The semantic arm's rows (`write_path_semantic`) + # were written by the pipeline beside its call row, so only the + # lookups are booked here. + if int(item["id"]) in excluded or not marker.startswith("nearby"): continue - arm = ("write_path_place" if marker.startswith("nearby") - else "write_path_semantic") - by_arm.setdefault(arm, []).append(int(item["id"])) + by_arm.setdefault("write_path_place", []).append(int(item["id"])) for arm, ids in by_arm.items(): record_surfaced( user_id=user_id, note_ids=ids, source=arm, project_id=project_id, diff --git a/src/scribe/services/retrieval_pipeline.py b/src/scribe/services/retrieval_pipeline.py index 8a323166..ec111c06 100644 --- a/src/scribe/services/retrieval_pipeline.py +++ b/src/scribe/services/retrieval_pipeline.py @@ -39,6 +39,8 @@ import time from dataclasses import dataclass, field from typing import Any, Callable +from scribe.services.lessons import LESSON_NOTE_TYPE, claim_line + logger = logging.getLogger(__name__) # ── Shared rendering and ranking helpers (moved from plugin_context) ────── @@ -681,6 +683,663 @@ async def run_rule_arm( return RuleResult() +# ── The notes corpus (milestone 456 step 4) ────────────────────────────── +# +# The same stages over the other corpus: search, withhold what this response +# already lists, split fresh from repeats, log the call before any early +# return, band, reserved slots, surfacing rows. Two arms take them — the +# prompt's menu (`auto_inject`) and the write path's match by meaning +# (`write_path`) — and they differed only in which kinds they ask for, whether +# a menu sits above them in the same response, and which slots they reserve. +# +# ONE DIFFERENCE FROM THE RULE ARMS IS KEPT, ON PURPOSE, FOR STEP 7. A notes +# arm logs the cut BEFORE its band; a rule arm logs after it. #2085 chose the +# first for notes (the log row is the candidate set a floor is tuned against, +# the surfacing rows are what the reader saw) and #3851 the second for rules. +# Both are recorded decisions, so this refactor reproduces both. +# +# What stays OUTSIDE: every lookup. The records named by number, the snippets +# recorded at a path, the rulings, the design system and the shape ledger +# have no score and no floor; the route builders compose them around what +# this returns. + +# Margin gate: drop any hit more than this far below the top hit's score, so a +# single strong match doesn't drag in a wall of barely-passing neighbours. +# Twice the rule band, which is narrow because the rule corpus is flat (#3851). +_NOTE_BAND = 0.10 + + +def _record_kind(note) -> str: + """The kind marker for an injected menu line — and, for a task, its status. + + The menu is drawn from every record that carries an embedding, so a snippet, + a stored process, an issue and a stray dev-log all arrive looking identical. + Recorded prior art only stands out if the line says what it is — and the kind + is also what tells the reader which tool opens it. + + Task-ness wins over `note_type` because it's the more useful distinction at a + glance: "there's an open issue about this" beats "there's a note about this". + + A TASK ALSO CARRIES ITS STATUS, because for that kind alone the line is + read as a claim about live work. A finished step and an open one rendered + identically is not a cosmetic gap: a done step was cited as a milestone's + open one on the strength of a line exactly like this, which carries an id, + a kind and a title and said nothing about where the work stood (#4154). + Only for tasks — a note or a snippet has no status to be wrong about. + """ + if note.is_task: + kind = "issue" if note.task_kind == "issue" else "task" + # No fallback for a missing status: `is_task` IS `status is not None` + # (models/note.py), so a branch for a task without one could never be + # taken, and a dead branch is a claim about the data that isn't true. + # + # Parenthesised rather than dot-joined: the write-path prior-art line + # joins its own fields with " · ", so a dotted status would read as + # another flag beside `seen` instead of as part of the kind. + return f"{kind} ({note.status})" + return note.note_type or "note" + + +# WHAT A MENU LINE CARRIES (#4364): the record's NAME, its kind and System, +# and the WHOLE passage that matched. Metadata plus the evidence, rather than a +# title asked to be both. +# +# The name, not the title. A snippet's or lesson's title is `name — when it +# applies` by construction (`embeddings.trigger_title`), because that join is +# what makes it rank on its situation. That is an EMBEDDING shape, and rendered +# as a menu line it ran to 1,500+ characters — the trigger paragraph spent +# again on every line, and again on every repeat. The trigger still arrives +# when it is what matched: it is in the chunk, and `_menu_passage` hands it +# over when the title was the whole match. +# +# The whole passage, not 200 characters of it. The search already chose the +# chunk that matched; the old cut kept its head and tail, and the head is the +# title every chunk is prefixed with — so the reader got the title twice and +# lost the middle, which is where the match was (lesson #4248). A chunk is at +# most ~1.4 KB (`embeddings._CHUNK_CHAR_BUDGET`), and it is shown once: a +# repeat is a one-line pointer (`_menu_seen_line`), not a second copy. + + +def _menu_name(title: str | None, note_type: str | None, data=None, body: str | None = "") -> str: + """The record's name — its title without the trigger composed into it.""" + title = (title or "(untitled)").replace("\n", " ").strip() + data = data if isinstance(data, dict) else {} + if note_type == "snippet": + from scribe.services.embeddings import TRIGGER_SEP + return (data.get("name") or title.partition(TRIGGER_SEP)[0]).strip() or title + if note_type == LESSON_NOTE_TYPE: + from types import SimpleNamespace + + from scribe.services.embeddings import untrigger_title + from scribe.services.lessons import lesson_trigger + trigger = lesson_trigger(SimpleNamespace(data=data, body=body or "")) + # One line even when the stored name is a story (#4797). + return claim_line(untrigger_title(title, trigger).strip() or title) + return title + + +def _menu_passage(title: str | None, chunk_text: str | None, name: str = "") -> str: + """The matched chunk on one line, without the title it was embedded under. + + Every chunk is `title\nsection` (`embeddings.embedding_text`), and `title` + here must be the EMBEDDED one (`embeddings.document_title`) — for a snippet + or lesson that is `name — trigger`, not the stored name — so the prefix is + stripped exactly. A chunk that WAS only the title — a short + record, or the head chunk of one — matched on the title, and for a + trigger-keyed kind the part of it the name line no longer shows is the + trigger: that is returned, because it is precisely what matched. + One line, so the menu's blockquote survives it. + """ + title = (title or "").strip() + text = (chunk_text or "").strip() + if title and text.startswith(title): + text = text[len(title):] + text = " ".join(text.split()) + if not text and name and title.startswith(name) and title != name: + text = " ".join(title[len(name):].lstrip(" —-").split()) + return text + + +def _menu_label(kind: str, systems: list[str] | None) -> str: + """`issue (done) · Plugin & hooks` — the kind, then where it belongs.""" + return " · ".join([kind, *systems]) if systems else kind + + +def _menu_seen_line(note_id: int, kind: str, name: str) -> str: + """A pointer to a record this session was already shown, not a copy of it.""" + return f"> - #{note_id} [{kind} · seen] {name}" + + +def menu_entry( + note_id: int, *, kind: str, name: str, systems: list[str] | None = None, + seen: bool = False, stale: bool = False, score: float | None = None, + shared_by: str = "", under: str = "", +) -> list[str]: + """One record on a notes menu: its line, and what sits under it. + + The prompt menu's two blocks — records named by number and records ranked + by meaning — used to write this out twice, differing only in what they + pass: a ranked line carries its score, a named one does not, and each puts + its own text under the line (the matched passage, or the record's opening). + + A repeat is a POINTER, not a copy (#4364). The record is in this session's + context already — the ledger is cleared at compaction, so "seen" stays true + — and re-rendering it spent its whole line again for nothing. + + A superseded record is DEMOTED, not removed (#278), so one can still reach + a menu, and when it does the reader has to be told: an agent handed stale + material with nothing marking it acts on it with full confidence. + """ + if seen: + line = _menu_seen_line(note_id, kind, name) + return [line + (" — SUPERSEDED" if stale else "")] + line = f"> - #{note_id} [{_menu_label(kind, systems)}] \"{name}\"" + if score is not None: + line += f" ({score:.2f})" + if stale: + line += " — SUPERSEDED, a later record covers this; check that first" + if shared_by: + line += f" — shared by {shared_by}" + return [line, f"> ↳ {under}"] if under else [line] + + +@dataclass(frozen=True) +class NoteSlot: + """One line reserved for a kind the open ranking keeps losing.""" + + source: str + kinds: tuple[str, ...] + """What the slot is FOR — asked for by the search and checked on the way + out, so a slot is never spent on a line indistinguishable from one that + earned its place on score.""" + + evicts: bool + """Take the menu's last line when the menu is full, rather than adding one.""" + + include_global_kinds: bool = False + books_own: bool = False + """The slot's line is recorded as surfaced under the slot's own source, + and so never again under the arm's.""" + + +# Order is load-bearing: reuse evicts the menu's weakest hit while the lesson +# slot extends, so running them the other way round would let a reserved +# lesson be the line reuse throws off — a slot another slot can silently undo +# is not a guarantee. +REUSE_SLOT = NoteSlot("reuse_slot", ("snippet", "process"), evicts=True) +LESSON_SLOT = NoteSlot( + "lesson_slot", (LESSON_NOTE_TYPE,), evicts=False, + include_global_kinds=True, books_own=True, +) + + +@dataclass(frozen=True) +class NoteArm: + """One ranked notes surface: what it searches, and which stages it takes.""" + + source: str + """The telemetry `source` of its call row. MUST equal the SURFACES key.""" + + surfaced_as: str + """The `source` its surfacing rows carry.""" + + note_type: tuple[str, ...] | None = None + task_kind: str | None = None + include_global_kinds: bool = True + """Lessons are project-independent (#3730), so they join the candidate set + from wherever they were learned.""" + + scope: str = "browse" + """Nobody asked for an injected line, so it takes the BROWSE scope: never + a record shared one-to-one with the operator.""" + + slots: tuple[NoteSlot, ...] = () + withholds: bool = False + """A menu sits above this arm in the same response, and what it lists is + kept out of the search (`exclude_ids`) — same-call duplication, which is a + different claim from the session ledger (#4101).""" + + +AUTO_INJECT = NoteArm( + "auto_inject", surfaced_as="auto_inject", slots=(REUSE_SLOT, LESSON_SLOT), +) +# Snippets AND recorded experience (#2246): an issue saying "we tried this and +# it deadlocked" is prior art for the code about to be written. `task_kind` +# keeps the open to-do list out — a task resembles the code and answers +# nothing. Lessons too (milestone 385 step 5): the arm is kind-FILTERED, so a +# kind absent here is unreachable, not merely outranked (#3702). No reserved +# slot: this arm fires before every Write and Edit, and the field is already +# narrow enough that the 200:1 dilution a slot answers does not happen. +WRITE_PATH = NoteArm( + "write_path", surfaced_as="write_path_semantic", + note_type=("snippet", "note", LESSON_NOTE_TYPE), task_kind="issue", + withholds=True, +) +NOTE_ARMS: tuple[NoteArm, ...] = (AUTO_INJECT, WRITE_PATH) +NOTE_SLOTS: tuple[NoteSlot, ...] = (REUSE_SLOT, LESSON_SLOT) + +# What the notes stages record under, for the registry's fan-out sites. +NOTE_SOURCES: tuple[str, ...] = ( + *(arm.source for arm in NOTE_ARMS), *(slot.source for slot in NOTE_SLOTS), +) +NOTE_SURFACED_SOURCES: tuple[str, ...] = ( + *(arm.surfaced_as for arm in NOTE_ARMS), + *(slot.source for slot in NOTE_SLOTS if slot.books_own), +) + + +def note_search_filters(arm: NoteArm) -> dict: + """The arm's constant search keywords — which kinds, whose records. + + Read by `retrieval_review` too: a re-run that searched with different + visibility or kinds from the arm's would judge a menu nobody was shown. + """ + out: dict = {} + if arm.note_type: + out["note_type"] = arm.note_type + if arm.task_kind: + out["task_kind"] = arm.task_kind + if arm.include_global_kinds: + out["include_global_kinds"] = True + out["scope"] = arm.scope + return out + + +@dataclass(frozen=True) +class NoteIO: + """The ranker and the two recorders a notes arm reports to.""" + + search: Callable[..., Any] + """`semantic_search_notes`, or a stand-in with its signature.""" + + record_retrieval: Callable[..., Any] + record_surfaced: Callable[..., Any] + + +@dataclass(frozen=True) +class NoteMoment: + """What a notes arm searches with, and what this response already holds.""" + + user_id: int + query: str + project_id: int | None + """Searched and logged as `project_id or None`.""" + + seen: frozenset[int] = frozenset() + """The session ledger: rendered as a pointer, never withheld (#4101).""" + + named: frozenset[int] = frozenset() + """Records this response shows by LOOKUP (named by number). Shown in their + own block and booked under `named_ref`, so this arm did not surface them.""" + + in_menu: frozenset[int] = frozenset() + """Records listed earlier in this same response — withheld.""" + + still_scored: frozenset[int] = frozenset() + """Of `in_menu`, those the search must still score (a pulled snippet's + resemblance to the payload is evidence for the shape ledger).""" + + +@dataclass +class NoteResult: + """What a notes arm put on the menu, and what its search answered.""" + + menu: list = field(default_factory=list) + """(score, note) best first: band, slots, and the named records removed.""" + + answered: list = field(default_factory=list) + """Everything the search returned, before this response's own menu was + withheld — what `resembles` is read from.""" + + chunks: dict = field(default_factory=dict) + """The passage each hit matched on, from the arm's OWN search — so a chunk + is only ever paired with the query that matched it. The slots' lines are + fetched by their own queries and so have none here.""" + + slot_ids: dict = field(default_factory=dict) + """{slot source: the id it spent}.""" + + +def _note_band(hits: list) -> list: + """The top hit, plus every hit within `_NOTE_BAND` of it. + + Computed over ALL hits, repeats included: the band measures distance from + the top SCORE, and letting the ledger move that cutoff would make "you + were shown this" change what counts as relevant (#3851's axis + independence). + """ + if not hits: + return [] + top = hits[0][0] + return [(s, n) for s, n in hits if s >= top - _NOTE_BAND] + + +async def _reserve_note_slot( + io: NoteIO, arm: NoteArm, slot: NoteSlot, moment: NoteMoment, menu: list, + *, floor: float, budget: int, shown: set[int], +) -> tuple[list, int | None]: + """Guarantee `slot.kinds` one line, if one clears the arm's own bar. + + WHY A SLOT AT ALL (#2246, milestone 385 step 5). Ranking by raw cosine is + blind to what KIND of record answers what kind of ask, and the corpus + makes that fatal: Scribe's project records are about software work, so a + task about building a helper outranks the snippet that IS one. Snippets + were ~0.5% of the corpus when this was measured; no floor fixes 200:1. A + lesson crowded out is worse still — it exists only to be met at the moment + it applies, so the arm that surfaces it IS its delivery. + + THE SLOT BUYS POSITION, NOT A LOWER BAR. It reserves at the menu's own + floor and is NOT held to the band (the top score is the very thing these + kinds lose to), so a weak record cannot buy the line. + + ITS OWN SOURCE, from the first deploy. A guarantee has to be falsifiable, + and the hit a slot pushed out sits in the arm's row while the query that + pushed it out would otherwise be nowhere (#2463). + + THE LEDGER IS NOT AN EXCLUSION HERE EITHER (#4101), but this call's own + menu is: a record already on it must not be shown twice, while one shown + in an EARLIER call is exactly what a slot may spend itself on — a snippet + relevant then and now is the reuse case, not a duplicate of it. + + Returns the possibly-changed menu and the id the slot spent. + """ + if any(_record_kind(n) in slot.kinds for _s, n in menu): + return menu, None + + t0 = time.perf_counter() + report: dict = {} + on_menu = {int(n.id) for _s, n in menu} + kwargs: dict = { + "limit": 1, "threshold": floor, "project_id": moment.project_id or None, + "exclude_ids": set(on_menu), + # KIND-FILTERED, so the slot can only be spent on what it is for. + "note_type": slot.kinds, "scope": arm.scope, "report": report, + } + if slot.include_global_kinds: + kwargs["include_global_kinds"] = True + found = await io.search(moment.user_id, moment.query, **kwargs) + fresh = [(s, n) for s, n in found if int(n.id) not in shown] + source = slot.source + try: + io.record_retrieval( + user_id=moment.user_id, source=source, query=moment.query, + threshold=floor, limit=1, project_id=moment.project_id or None, + is_task=None, results=fresh, + best_available=report.get("best_available_score"), + best_available_id=report.get("best_available_id"), + searched=bool(report.get("searched", True)), + suppressed=len(found) - len(fresh), + duration_ms=(time.perf_counter() - t0) * 1000.0, + ) + except Exception: # noqa: BLE001 - observation never breaks the observed + _telemetry_failed(source) + # Verified, not trusted: the kind the query asked for, and not a line + # this menu already carries. + placed = [ + (s, n) for s, n in found + if _record_kind(n) in slot.kinds and int(n.id) not in on_menu + ][:1] + if not placed: + return menu, None + + slot_id = int(placed[0][1].id) + # FRESH ONLY, matching the row above: a ledger repeat is rendered (#4101) + # but is not a new surfacing. + if slot.books_own and slot_id not in shown: + try: + io.record_surfaced( + user_id=moment.user_id, note_ids=[slot_id], source=source, + project_id=moment.project_id or None, + ) + except Exception: # noqa: BLE001 - observation never breaks the observed + _telemetry_failed(source) + if not slot.evicts: + # IT EXTENDS, IT NEVER DISPLACES, siding with `preference_slot`: a + # displaced hit sits in the arm's row, and evicting it would make the + # two tables disagree about the same call (#3668). And a record that + # does not bind should not throw a better-scoring one off the menu. + return menu + placed, slot_id + # Take the LAST line, never the first: the strongest overall hit is still + # the best answer, and displacing it would trade one blindness for another. + if len(menu) >= budget: + return menu[:budget - 1] + placed, slot_id + return (menu + placed)[:budget], slot_id + + +async def run_note_arm( + arm: NoteArm, moment: NoteMoment, *, floor: float, budget: int, io: NoteIO, +) -> NoteResult: + """Run one notes arm through every stage it takes, in the one order. + + search → withhold this response's menu → fresh/repeat split → call row → + band → reserved slots → surfacing rows. Fails open to an empty result. + """ + try: + t0 = time.perf_counter() + report: dict = {} + scored = moment.in_menu & moment.still_scored + kwargs: dict = { + # The still-scored ids come back and are dropped below, so the + # limit has to cover them. + "limit": budget + len(scored), "threshold": floor, + "project_id": moment.project_id or None, + **note_search_filters(arm), "report": report, + } + if arm.withholds: + kwargs["exclude_ids"] = set(moment.in_menu - scored) + answered = await io.search(moment.user_id, moment.query, **kwargs) + hits = [(s, n) for s, n in answered if int(n.id) not in moment.in_menu] + # WHAT THIS ARM WITHHELD AFTER THE SEARCH ANSWERED (#3739). The score + # the search reports is measured BEFORE that drop, so a record already + # listed in this response could be logged as one the BAR turned away + # — live proof once read a "rejection" at 0.822 against a lowest + # acceptance of 0.6857. Whenever this removed anything, the honest + # best-available is null: "not measured on this call". + withheld = len(answered) - len(hits) + hits = hits[:budget] + # `results=fresh` and `suppressed` together (#3752): a rendered repeat + # is not a new surfacing, so it is COUNTED rather than reported, and an + # all-repeat zero reads apart from a bar nothing cleared. A record + # NAMED in this response is counted the same way — shown, but by the + # lookup and booked under `named_ref`. + fresh = [ + (s, n) for s, n in hits + if int(n.id) not in moment.seen and int(n.id) not in moment.named + ] + source = arm.source + try: + io.record_retrieval( + user_id=moment.user_id, source=source, query=moment.query, + threshold=floor, limit=budget, + project_id=moment.project_id or None, is_task=None, + results=fresh, + best_available=( + None if withheld else report.get("best_available_score") + ), + # Withheld on the SAME condition: a surviving id beside a null + # score would name a record without saying what it scored. + best_available_id=( + None if withheld else report.get("best_available_id") + ), + searched=bool(report.get("searched", True)), + suppressed=len(hits) - len(fresh), + duration_ms=(time.perf_counter() - t0) * 1000.0, + ) + except Exception: # noqa: BLE001 - observation never breaks the observed + _telemetry_failed(source) + result = NoteResult(answered=answered, chunks=report.get("best_chunk") or {}) + if not hits: + return result + + menu = _note_band(hits) + # The slots are told about the named records as if already shown, so + # a slot that picks one does not book a surfacing the lookup booked. + shown = set(moment.seen | moment.named) + for slot in arm.slots: + menu, slot_id = await _reserve_note_slot( + io, arm, slot, moment, menu, floor=floor, budget=budget, + shown=shown, + ) + if slot_id is not None: + result.slot_ids[slot.source] = slot_id + # A named record is shown once, in its own block, never again as a match. + menu = [(s, n) for s, n in menu if int(n.id) not in moment.named] + result.menu = menu + + # What SURVIVED the band — the menu the reader saw — where the call + # row holds the candidate set a floor is tuned against (#2085). FRESH + # ONLY, the row's own cut (#4101, #3668). A slot that books its own + # line is not this arm's surfacing: counting it twice would leave the + # slot's two tables describing different numbers of one event. + own = { + sid for src, sid in result.slot_ids.items() + if any(slot.books_own and slot.source == src for slot in arm.slots) + } + ids = [ + int(n.id) for _s, n in menu + if int(n.id) not in moment.seen and int(n.id) not in own + ] + if ids: + source = arm.surfaced_as + try: + io.record_surfaced( + user_id=moment.user_id, note_ids=ids, source=source, + project_id=moment.project_id, + ) + except Exception: # noqa: BLE001 - observation never breaks the observed + _telemetry_failed(source) + return result + except Exception: # noqa: BLE001 - a recall aid never breaks its act + logger.debug("%s arm failed", arm.source, exc_info=True) + return NoteResult() + + +# ── A rule reached through its lessons (milestone 440, #4633) ──────────── +# +# A rule's own document is written in the rule's words, which are general by +# design; the situations that keep proving it are often closer to what a +# session is actually doing. A lesson JUDGED to be an instance of a rule (a +# CONFIRMED link) carries that situation, so a lesson matching the moment +# brings its rule along — in rule voice, naming the lesson that reached it. +# It searches the NOTES corpus and answers with rules, which is why it sits +# between the two halves of this module. +# +# ITS OWN SLOT, NOT A RULE SLOT, as a stated default. There was nothing to +# measure until links are confirmed (#4632), so the asymmetry decides it: a +# via-lesson line that took a rule slot could push out a rule the ranker +# matched DIRECTLY, a stronger claim displaced by a weaker one, while an extra +# line costs one line. Logged as its own source so it can be judged (#4636). + +VIA_LESSON_SOURCE = "rule_via_lesson" +VIA_LESSON_LIMIT = 1 +# Lessons fetched before keeping only the linked ones. The search cannot be +# told "linked lessons only", so it overfetches and filters; on a corpus where +# most lessons are unlinked, a fetch of one would almost always be spent on a +# lesson that carries nothing. +_VIA_LESSON_OVERFETCH = 10 + + +@dataclass(frozen=True) +class ViaLessonIO: + """The lesson search, the link lookups, and the recorders.""" + + search: Callable[..., Any] + """`semantic_search_notes`, or a stand-in with its signature.""" + + linked: Callable[..., Any] + """`lesson_rules.confirmed_lessons`: the lessons with a confirmed link.""" + + rules_for: Callable[..., Any] + """`lesson_rules.confirmed_rules_in_scope`: their rules, in scope.""" + + floor: Callable[..., Any] + """The notes menu's own bar — a lesson too weak to be shown cannot carry a + rule in. Awaited only once a linked lesson exists.""" + + record_retrieval: Callable[..., Any] + record_rule_surfaced: Callable[..., Any] + + +async def run_via_lesson_arm( + moment: RuleMoment, *, skip: frozenset[int], io: ViaLessonIO, +) -> RuleResult: + """Rule lines reached through a matching lesson's CONFIRMED links. + + `skip` is every rule this response already names plus the session ledger: + suppression applies to the RULE, whichever lesson reached it. Fails open. + """ + try: + confirmed = await io.linked(moment.user_id) + if not confirmed: + return RuleResult() + bar = await io.floor() + t0 = time.perf_counter() + report: dict = {} + found = await io.search( + moment.user_id, moment.query, limit=_VIA_LESSON_OVERFETCH, + threshold=bar, project_id=moment.project_id, + note_type=(LESSON_NOTE_TYPE,), include_global_kinds=True, + scope="browse", report=report, + ) + matched = [(s, n) for s, n in found if int(n.id) in confirmed] + by_lesson = await io.rules_for( + moment.user_id, [int(n.id) for _s, n in matched], moment.project_id, + ) + candidates = [ + (s, rule, n) for s, n in matched for rule in by_lesson.get(int(n.id), []) + ] + chosen: list = [] + taken: set[int] = set(skip) + for s, rule, n in candidates: + if rule.id in taken: + continue + taken.add(rule.id) + chosen.append((s, rule, n)) + if len(chosen) >= VIA_LESSON_LIMIT: + break + # Logged whenever a search ran, results or not — the #3497 guard. The + # score is the LESSON's, because the lesson is what was matched; the + # best-available id is left off for the same reason, since the row's + # results are rules and an id beside them would read as a rule id. + try: + io.record_retrieval( + user_id=moment.user_id, source=VIA_LESSON_SOURCE, + query=moment.query, threshold=bar, limit=VIA_LESSON_LIMIT, + project_id=moment.project_id, is_task=None, + results=[(s, rule) for s, rule, _n in chosen], + duration_ms=(time.perf_counter() - t0) * 1000.0, + best_available=report.get("best_available_score"), + searched=bool(report.get("searched", True)), + suppressed=len({r.id for _s, r, _n in candidates}) - len(chosen), + ) + except Exception: # noqa: BLE001 - observation never breaks the observed + _telemetry_failed(VIA_LESSON_SOURCE) + if not chosen: + return RuleResult() + rule_ids = [rule.id for _s, rule, _n in chosen] + try: + io.record_rule_surfaced( + user_id=moment.user_id, rule_ids=rule_ids, source=VIA_LESSON_SOURCE, + ) + except Exception: # noqa: BLE001 - observation never breaks the observed + _telemetry_failed(VIA_LESSON_SOURCE) + lines = [ + _rule_hint_line(rule, where=moment.where, seen=False, + held=rule.id in moment.held) + + f" Reached through lesson #{n.id} " + + f"“{_menu_name(n.title, n.note_type, n.data, n.body)}”, " + + "a recorded instance of it." + for _s, rule, n in chosen + ] + return RuleResult( + lines=lines, rule_ids=rule_ids, shown_rule_ids=list(rule_ids), + shown=[(s, rule) for s, rule, _n in chosen], + ) + except Exception: # noqa: BLE001 - a recall aid never breaks its act + logger.debug("%s arm failed", VIA_LESSON_SOURCE, exc_info=True) + return RuleResult() + + # ── The moment arm (milestone 458) ─────────────────────────────────────── # # A LOOKUP beside the ranked arms, not one of them. A rule mounted on a moment diff --git a/src/scribe/services/retrieval_registry.py b/src/scribe/services/retrieval_registry.py index bb2a6422..6d99f9d7 100644 --- a/src/scribe/services/retrieval_registry.py +++ b/src/scribe/services/retrieval_registry.py @@ -42,7 +42,9 @@ from __future__ import annotations from dataclasses import dataclass -from scribe.services.retrieval_pipeline import MOMENT_RULE_SOURCE, RULE_ARMS, RULE_SOURCES +from scribe.services.retrieval_pipeline import ( + MOMENT_RULE_SOURCE, NOTE_SOURCES, NOTE_SURFACED_SOURCES, RULE_ARMS, RULE_SOURCES, +) # ── How a point is reached ──────────────────────────────────────────────── # @@ -262,8 +264,10 @@ POINTS: dict[str, Point] = dict([ # nothing, because the paths are relative to `src/` and so begin `scribe/` — # every site sailed through a check that looked like it was running. FAN_OUT_SITES: dict[str, tuple[str, ...]] = { + # The write path's two LOOKUPS. Its ranked arm (`write_path_semantic`) + # records through the pipeline's site below (milestone 456 step 4). "scribe/services/plugin_context.py::record_surfaced(source=arm)": ( - "write_path_place", "write_path_semantic", "write_path_sync", + "write_path_place", "write_path_sync", ), "scribe/services/system_rulings.py::record_system_surfaced(source=source)": ( "rulings_pre_tool", "rulings_write_path", @@ -272,12 +276,16 @@ FAN_OUT_SITES: dict[str, tuple[str, ...]] = { "enter_project", "start_planning", "get_task", "get_project", "get_milestone", ), - # The one retrieval pipeline (milestone 456): every rule arm's call row and - # surfacing rows are written by the same two sites, so `source` arrives as - # a value. The values are the pipeline's own specs, read rather than - # restated, so an arm added there is declared here by construction. + # The one retrieval pipeline (milestone 456): every ranked arm's call row + # and surfacing rows — both corpora — are written by these sites, so + # `source` arrives as a value. The values are the pipeline's own specs, + # read rather than restated, so an arm added there is declared here by + # construction. "scribe/services/retrieval_pipeline.py::record_retrieval(source=source)": ( - RULE_SOURCES + RULE_SOURCES + NOTE_SOURCES + ), + "scribe/services/retrieval_pipeline.py::record_surfaced(source=source)": ( + NOTE_SURFACED_SOURCES ), "scribe/services/retrieval_pipeline.py::record_rule_surfaced(source=source)": ( tuple(arm.source for arm in RULE_ARMS) diff --git a/src/scribe/services/retrieval_review.py b/src/scribe/services/retrieval_review.py index 29e498ac..cb1a95f3 100644 --- a/src/scribe/services/retrieval_review.py +++ b/src/scribe/services/retrieval_review.py @@ -96,18 +96,20 @@ async def _rerun( ) -> list[dict]: """The logged call's query, searched again the way its arm searches. - `auto_inject`'s parameters, from `build_autoinject_hint`: browse scope, - lessons reachable across projects, the arm's floor as logged. The reserved - slots (reuse, lesson) are left out — each logs its own source, and is - judged as that source when it becomes reviewable. + `auto_inject`'s parameters, read from its spec (`retrieval_pipeline. + AUTO_INJECT`) rather than restated: which kinds, whose records, and the + arm's floor as logged. The reserved slots (reuse, lesson) are left out — + each logs its own source, and is judged as that source when it becomes + reviewable. Records created after the call are dropped before ranking — the call could not have been offered them — and named in `report["postdated"]`. """ # Imported here: plugin_context imports retrieval_telemetry, which reads # this module's judged block. + from scribe.services import retrieval_pipeline as rp from scribe.services.embeddings import document_title, semantic_search_notes - from scribe.services.plugin_context import _menu_name, _menu_passage, _record_kind + from scribe.services.retrieval_pipeline import _menu_name, _menu_passage, _record_kind rep: dict = {} hits = await semantic_search_notes( @@ -115,8 +117,7 @@ async def _rerun( limit=depth + POSTDATED_SLACK, threshold=row.threshold if row.threshold is not None else 0.0, project_id=row.project_id or None, - include_global_kinds=True, - scope="browse", + **rp.note_search_filters(rp.AUTO_INJECT), report=rep, ) postdated = [int(n.id) for _s, n in hits if _after(n.created_at, row.created_at)] diff --git a/tests/test_note_pipeline.py b/tests/test_note_pipeline.py new file mode 100644 index 00000000..3b826cb2 --- /dev/null +++ b/tests/test_note_pipeline.py @@ -0,0 +1,277 @@ +"""The notes corpus on the one retrieval pipeline (milestone 456 step 4). + +`run_note_arm` is driven directly here, with the search and both recorders +stubbed through `NoteIO`: the stage order, the invariants the rule arms +already pin (#3497's call row before any return, #3752's fresh-only cut, +#4101's ledger-renders-not-removes), the write path's withheld menu (#3739), +and the two reserved slots in their load-bearing order. The builders that +compose these arms keep their own tests; these pin the stages they share. +""" +from __future__ import annotations + +import ast +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from scribe.services import retrieval_pipeline as rp +from scribe.services.lessons import LESSON_NOTE_TYPE +from scribe.services.retrieval_registry import FAN_OUT_SITES, POINTS +from scribe.services.retrieval_surfaces import SURFACES +from tests.helpers import fake_note + + +def _io(route): + """A NoteIO whose search answers by the kinds asked for, and records calls.""" + calls: list[dict] = [] + + async def search(_uid, _q, **kw): + calls.append(kw) + return list(route(kw)) + + return rp.NoteIO(search=search, record_retrieval=MagicMock(), + record_surfaced=MagicMock()), calls + + +def _rows(mock, source): + return [c.kwargs for c in mock.call_args_list if c.kwargs["source"] == source] + + +def _note(nid, score=None, **kw): + note = fake_note(id=nid, title=f"record {nid}", user_id=1, **kw) + return (score, note) if score is not None else note + + +async def _run(arm, route, *, budget=3, **moment): + io, calls = _io(route) + result = await rp.run_note_arm( + arm, rp.NoteMoment(user_id=1, query="q", project_id=moment.pop("project_id", 2), + **moment), + floor=0.5, budget=budget, io=io, + ) + return result, io, calls + + +# ── the call row (#3497, #3752) ─────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_a_call_that_found_nothing_is_still_logged(): + result, io, _ = await _run(rp.AUTO_INJECT, lambda kw: []) + assert result.menu == [] + (row,) = _rows(io.record_retrieval, "auto_inject") + assert row["results"] == [] and row["suppressed"] == 0 + io.record_surfaced.assert_not_called() + + +@pytest.mark.asyncio +async def test_repeats_and_named_records_are_counted_not_reported(): + hits = [_note(11, 0.80), _note(22, 0.79), _note(33, 0.78)] + result, io, _ = await _run( + rp.AUTO_INJECT, lambda kw: [] if kw.get("note_type") else hits, + seen=frozenset({11}), named=frozenset({33}), + ) + (row,) = _rows(io.record_retrieval, "auto_inject") + assert [int(n.id) for _s, n in row["results"]] == [22] + assert row["suppressed"] == 2 + # The repeat stays on the menu (#4101); the named record does not — it is + # shown by its own block. + assert [int(n.id) for _s, n in result.menu] == [11, 22] + (surfaced,) = _rows(io.record_surfaced, "auto_inject") + assert surfaced["note_ids"] == [22] + + +def test_every_notes_stage_logs_before_it_can_return(): + """Structural, like the rule arms' guard: no `return` in a notes stage + precedes its call row. The via-lesson arm's one exception is the return + before any search runs — nothing was asked, so there is nothing to log.""" + tree = ast.parse(Path("src/scribe/services/retrieval_pipeline.py").read_text()) + fns = {n.name: n for n in ast.walk(tree) if isinstance(n, ast.AsyncFunctionDef)} + for name in ("run_note_arm", "_reserve_note_slot", "run_via_lesson_arm"): + fn = fns[name] + logged = [n.lineno for n in ast.walk(fn) if isinstance(n, ast.Call) + and getattr(n.func, "attr", None) == "record_retrieval"] + searched = [n.lineno for n in ast.walk(fn) if isinstance(n, ast.Call) + and getattr(n.func, "attr", None) == "search"] + assert logged and searched, f"{name} no longer searches and logs" + early = [n.lineno for n in ast.walk(fn) if isinstance(n, ast.Return) + and min(searched) < n.lineno < min(logged)] + assert not early, f"{name} can return at line {early[0]} before logging" + + +# ── the band, and the arm failing open ──────────────────────────────────── + + +@pytest.mark.asyncio +async def test_the_band_narrows_the_menu_after_the_row_is_written(): + """Notes log BEFORE the band (#2085) — the row is the candidate set a + floor is tuned against, the surfacing rows are what the reader saw.""" + hits = [_note(1, 0.90), _note(2, 0.70)] + result, io, _ = await _run(rp.WRITE_PATH, lambda kw: hits) + (row,) = _rows(io.record_retrieval, "write_path") + assert [int(n.id) for _s, n in row["results"]] == [1, 2] + assert [int(n.id) for _s, n in result.menu] == [1] + (surfaced,) = _rows(io.record_surfaced, "write_path_semantic") + assert surfaced["note_ids"] == [1] + + +@pytest.mark.asyncio +async def test_a_search_that_raises_costs_the_arm_not_the_response(): + async def boom(*_a, **_kw): + raise RuntimeError("index unavailable") + + io = rp.NoteIO(search=boom, record_retrieval=MagicMock(), record_surfaced=MagicMock()) + result = await rp.run_note_arm( + rp.AUTO_INJECT, rp.NoteMoment(user_id=1, query="q", project_id=None), + floor=0.5, budget=3, io=io, + ) + assert result.menu == [] and result.answered == [] + + +@pytest.mark.asyncio +async def test_a_recorder_that_raises_costs_only_its_row(): + hits = [_note(1, 0.90)] + io, _ = _io(lambda kw: [] if kw.get("note_type") else hits) + io.record_retrieval.side_effect = RuntimeError("telemetry down") + result = await rp.run_note_arm( + rp.AUTO_INJECT, rp.NoteMoment(user_id=1, query="q", project_id=None), + floor=0.5, budget=3, io=io, + ) + assert [int(n.id) for _s, n in result.menu] == [1] + + +# ── the write path's withheld menu (#3739) ──────────────────────────────── + + +@pytest.mark.asyncio +async def test_this_responses_menu_is_withheld_but_a_pulled_one_is_still_scored(): + pulled, listed = 5, 6 + answered = [_note(pulled, 0.91), _note(7, 0.88)] + result, io, calls = await _run( + rp.WRITE_PATH, lambda kw: answered, budget=2, + in_menu=frozenset({pulled, listed}), still_scored=frozenset({pulled}), + ) + (call,) = calls + # The listed one never reaches the search; the pulled one does, and the + # limit covers it so the menu still gets its full budget. + assert call["exclude_ids"] == {listed} + assert call["limit"] == 3 + assert [int(n.id) for _s, n in result.answered] == [pulled, 7] + assert [int(n.id) for _s, n in result.menu] == [7] + # Something was withheld after the search answered, so the score it + # reported may be one of ours: not measured on this call. + (row,) = _rows(io.record_retrieval, "write_path") + assert row["best_available"] is None and row["best_available_id"] is None + + +@pytest.mark.asyncio +async def test_the_prompt_menu_never_sends_the_ledger_into_its_search(): + _result, _io_, calls = await _run( + rp.AUTO_INJECT, lambda kw: [], seen=frozenset({11}), + ) + assert "exclude_ids" not in calls[0] + + +# ── the reserved slots, in their order ──────────────────────────────────── + + +def _routed(main, reuse=(), lesson=()): + def route(kw): + kinds = kw.get("note_type") or () + if LESSON_NOTE_TYPE in kinds: + return lesson + if kinds: + return reuse + return main + return route + + +@pytest.mark.asyncio +async def test_reuse_evicts_the_last_line_and_the_lesson_extends(): + main = [_note(1, 0.70), _note(2, 0.69), _note(3, 0.68)] + reuse = [_note(9, 0.60, note_type="snippet")] + lesson = [_note(42, 0.58, note_type=LESSON_NOTE_TYPE)] + result, io, calls = await _run( + rp.AUTO_INJECT, _routed(main, reuse, lesson), budget=3, + ) + # Reuse took the last of three; the lesson was added as a fourth. + assert [int(n.id) for _s, n in result.menu] == [1, 2, 9, 42] + assert result.slot_ids == {"reuse_slot": 9, "lesson_slot": 42} + # Reuse ran first: the lesson query was told about the snippet. + assert [c.get("note_type") for c in calls] == [ + None, ("snippet", "process"), (LESSON_NOTE_TYPE,), + ] + assert 9 in calls[2]["exclude_ids"] + # Each slot logs under its own name. + assert _rows(io.record_retrieval, "reuse_slot") and _rows(io.record_retrieval, "lesson_slot") + # The lesson books its own surfacing and never the arm's; the reuse line + # is counted under the arm, as it always was. + assert _rows(io.record_surfaced, "lesson_slot")[0]["note_ids"] == [42] + assert _rows(io.record_surfaced, "auto_inject")[0]["note_ids"] == [1, 2, 9] + + +@pytest.mark.asyncio +async def test_a_slot_stands_down_when_its_kind_won_on_score(): + main = [_note(9, 0.80, note_type="snippet"), _note(42, 0.79, note_type=LESSON_NOTE_TYPE)] + _result, io, calls = await _run(rp.AUTO_INJECT, _routed(main)) + assert len(calls) == 1 + assert not _rows(io.record_retrieval, "reuse_slot") + assert not _rows(io.record_retrieval, "lesson_slot") + + +@pytest.mark.asyncio +async def test_a_slot_spends_itself_on_a_repeat_without_booking_it_again(): + """A record shown in an EARLIER call is exactly what a slot may spend + itself on (#4101) — it is rendered, but it is not a new surfacing.""" + main = [_note(1, 0.70)] + lesson = [_note(42, 0.58, note_type=LESSON_NOTE_TYPE)] + result, io, _ = await _run( + rp.AUTO_INJECT, _routed(main, lesson=lesson), seen=frozenset({42}), + ) + assert 42 in [int(n.id) for _s, n in result.menu] + assert not _rows(io.record_surfaced, "lesson_slot") + (row,) = _rows(io.record_retrieval, "lesson_slot") + assert row["results"] == [] and row["suppressed"] == 1 + + +@pytest.mark.asyncio +async def test_the_write_path_reserves_no_slot(): + main = [_note(1, 0.80)] + _result, _io_, calls = await _run(rp.WRITE_PATH, _routed(main, [_note(9, 0.7)])) + assert len(calls) == 1 + + +# ── the specs are the declarations ──────────────────────────────────────── + + +def test_every_notes_source_is_registered_and_every_arm_is_tunable(): + for source in (*rp.NOTE_SOURCES, *rp.NOTE_SURFACED_SOURCES): + assert source in POINTS, f"{source} records telemetry but is not registered" + for arm in rp.NOTE_ARMS: + assert arm.source in SURFACES, f"{arm.source} has no floor or budget to tune" + # And the slots are measurable, never tunable: a budget of 1 is their feature. + for slot in rp.NOTE_SLOTS: + assert slot.source not in SURFACES + + +def test_the_pipelines_fan_out_sites_declare_every_notes_source(): + logged = FAN_OUT_SITES[ + "scribe/services/retrieval_pipeline.py::record_retrieval(source=source)"] + surfaced = FAN_OUT_SITES[ + "scribe/services/retrieval_pipeline.py::record_surfaced(source=source)"] + assert set(rp.NOTE_SOURCES) <= set(logged) + assert set(rp.NOTE_SURFACED_SOURCES) == set(surfaced) + # Not the arm's: the slot whose line it is, and the lookups' own site. + assert "write_path_semantic" not in FAN_OUT_SITES[ + "scribe/services/plugin_context.py::record_surfaced(source=arm)"] + + +def test_the_filters_say_which_kinds_and_whose_records(): + assert rp.note_search_filters(rp.AUTO_INJECT) == { + "include_global_kinds": True, "scope": "browse", + } + assert rp.note_search_filters(rp.WRITE_PATH) == { + "note_type": ("snippet", "note", LESSON_NOTE_TYPE), "task_kind": "issue", + "include_global_kinds": True, "scope": "browse", + } diff --git a/tests/test_retrieval_review.py b/tests/test_retrieval_review.py index 8c6388f9..dae1e698 100644 --- a/tests/test_retrieval_review.py +++ b/tests/test_retrieval_review.py @@ -5,12 +5,9 @@ Integration tests pin the parts a mock would make true by construction: which logged calls are offered, that a verdict lands and replaces, that "opened after" reads the usage table, and that the readout counts by rank. """ -import ast -import inspect -import textwrap from datetime import datetime, timedelta, timezone from types import SimpleNamespace -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest import pytest_asyncio @@ -54,31 +51,46 @@ async def test_an_empty_verdict_list_is_refused(): # ── the re-run replays the arm ─────────────────────────────────────────── -def _search_kwargs(fn) -> dict: - """The constant keywords `fn` passes to semantic_search_notes.""" - tree = ast.parse(textwrap.dedent(inspect.getsource(fn))) - for node in ast.walk(tree): - if (isinstance(node, ast.Call) and getattr(node.func, "id", "") - == "semantic_search_notes"): - return { - kw.arg: kw.value.value for kw in node.keywords - if isinstance(kw.value, ast.Constant) - } - return {} +_FILTERS = ("note_type", "task_kind", "include_global_kinds", "scope") -def test_the_re_run_searches_the_way_the_arm_does(): +@pytest.mark.asyncio +async def test_the_re_run_searches_the_way_the_arm_does(): """A re-run with different visibility or kinds from the arm's would judge - a menu nobody was shown. Read from both call sites, so a change to the - arm's search fails here until the review follows it.""" - from scribe.services.plugin_context import build_autoinject_hint + a menu nobody was shown. Both searches are RUN and their keywords + compared, so a change to the arm's spec fails here until the review + follows it — and the arm has to have filters for the check to mean + anything (rule 167).""" + from scribe.services import retrieval_pipeline as rp - arm = _search_kwargs(build_autoinject_hint) - rerun = _search_kwargs(review._rerun) - assert arm, "found no semantic_search_notes call in the arm" - for key in ("include_global_kinds", "scope"): - assert key in arm, f"the arm no longer passes {key} as a constant" - assert rerun.get(key) == arm[key], key + arm_kw: dict = {} + + async def arm_search(_uid, _q, **kw): + arm_kw.update(kw) + return [] + + await rp.run_note_arm( + rp.AUTO_INJECT, rp.NoteMoment(user_id=1, query="q", project_id=None), + floor=0.5, budget=3, + io=rp.NoteIO(search=arm_search, record_retrieval=MagicMock(), + record_surfaced=MagicMock()), + ) + rerun_kw: dict = {} + + async def rerun_search(_uid, _q, **kw): + rerun_kw.update(kw) + return [] + + row = SimpleNamespace(query="q", threshold=0.5, limit_n=3, project_id=None, + created_at=CALL) + with patch("scribe.services.embeddings.semantic_search_notes", side_effect=rerun_search): + await review._rerun(1, row, 3) + + arm = {k: arm_kw[k] for k in _FILTERS if k in arm_kw} + assert "scope" in arm and "include_global_kinds" in arm, ( + f"the arm no longer passes its visibility filters: {arm_kw}" + ) + assert {k: rerun_kw[k] for k in _FILTERS if k in rerun_kw} == arm CALL = datetime(2026, 9, 20, 12, 0, tzinfo=timezone.utc) diff --git a/tests/test_retrieval_surfaces.py b/tests/test_retrieval_surfaces.py index be539964..237761fc 100644 --- a/tests/test_retrieval_surfaces.py +++ b/tests/test_retrieval_surfaces.py @@ -70,9 +70,9 @@ def test_every_surface_name_is_a_real_telemetry_source(): # The rule arms record through the one pipeline (milestone 456), where # `source` is the spec's own field — so for them the spec IS the string # `record_retrieval` receives, and the join key is checked against it. - from scribe.services.retrieval_pipeline import RULE_ARMS + from scribe.services.retrieval_pipeline import NOTE_ARMS, RULE_ARMS - via_pipeline = {arm.source for arm in RULE_ARMS} + via_pipeline = {arm.source for arm in (*RULE_ARMS, *NOTE_ARMS)} missing = [ s.name for s in rs.SURFACES.values() if f'source="{s.name}"' not in blob diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index 32cc71d6..9903da29 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -1727,16 +1727,28 @@ def test_every_hook_rule_search_says_which_project_it_is_for(): f"{path}:{call.lineno} searches every project's rules from a hook" ) - # The pipeline: exactly one search, in `_ranked`, whose keyword set is - # built as a dict literal — so the keys are read from that literal. + # The pipeline: exactly one RULE search, in `_ranked`, whose keyword set + # is built as a dict literal — so the keys are read from that literal. + # Since step 4 the module also searches the NOTES corpus (the two notes + # arms, their slots, and the via-lesson arm's lesson search), so the + # searches are counted per function: a rule search copied anywhere else + # is a new function in this map, and fails it. pipeline = ast.parse(Path("src/scribe/services/retrieval_pipeline.py").read_text()) - searches = [ - n for n in ast.walk(pipeline) - if isinstance(n, ast.Call) and getattr(n.func, "attr", None) == "search" - ] - assert len(searches) == 1, ( - f"the pipeline has {len(searches)} rule searches, expected 1 — every " - f"arm is meant to reach the ranker through `_ranked`" + by_function = {} + for fn in ast.walk(pipeline): + if isinstance(fn, ast.AsyncFunctionDef): + n = sum(1 for c in ast.walk(fn) if isinstance(c, ast.Call) + and getattr(c.func, "attr", None) == "search") + if n: + by_function[fn.name] = n + assert by_function == { + "_ranked": 1, # the rule ranker, every rule arm + "_reserve_note_slot": 1, # notes: a reserved slot + "run_note_arm": 1, # notes: the arm itself + "run_via_lesson_arm": 1, # notes: lessons, then their linked rules + }, ( + f"the pipeline's searches moved: {by_function} — every rule arm is " + f"meant to reach the ranker through `_ranked`" ) ranked = next(n for n in ast.walk(pipeline) if isinstance(n, ast.AsyncFunctionDef) and n.name == "_ranked") diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index 3fc6a63f..0d440870 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -3,6 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from scribe.services import plugin_context as pc_module from scribe.services import retrieval_surfaces as rs +from scribe.services.retrieval_pipeline import REUSE_SLOT from scribe.services.lessons import LESSON_NOTE_TYPE from tests.helpers import fake_note, writepath_cfg @@ -340,7 +341,7 @@ _CFG = {"enabled": True, "threshold": 0.55, "top_k": 3} def _asked_for_reuse(calls: list[dict]) -> bool: """Did the reuse slot issue its reserved query on this run?""" return any( - tuple(c.get("note_type") or ()) == pc_module._REUSE_KINDS for c in calls + tuple(c.get("note_type") or ()) == REUSE_SLOT.kinds for c in calls ) diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 4d1440d2..65d51a54 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -621,9 +621,10 @@ def test_the_rule_arm_asks_for_a_set_and_lets_the_band_narrow_it(): sharper — are narrowed harder than the notes menu is. """ from scribe.services import plugin_context as pc + from scribe.services import retrieval_pipeline as rp assert pc.RULEHINT_LIMIT > 1 - assert 0 < pc._RULEHINT_BAND < pc._AUTOINJECT_BAND + assert 0 < pc._RULEHINT_BAND < rp._NOTE_BAND # --- the minimum-substance floor on the semantic arm (#2223) ------------------