Merge pull request 'Auto-inject looks up records named by number (#4796)' (#199) from dev into main
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 59s
CI & Build / Python tests (push) Successful in 1m47s
CI & Build / Build & push image (push) Successful in 18s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 59s
CI & Build / Python tests (push) Successful in 1m47s
CI & Build / Build & push image (push) Successful in 18s
This commit was merged in pull request #199.
This commit is contained in:
@@ -24,6 +24,7 @@ from scribe.services import design_systems as design_systems_svc
|
||||
from scribe.services import knowledge as knowledge_svc
|
||||
from scribe.services import notes as notes_svc
|
||||
from scribe.services import projects as projects_svc
|
||||
from scribe.services import record_refs as record_refs_svc
|
||||
from scribe.services import shape_ledger as shape_ledger_svc
|
||||
from scribe.services import snippets as snippets_svc
|
||||
from scribe.services import task_claims as task_claims_svc
|
||||
@@ -123,6 +124,34 @@ 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}"
|
||||
|
||||
|
||||
# How much of a named record's body sits under its line. The opening, not an
|
||||
# elided middle: 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.
|
||||
_NAMED_OPENING_CHARS = 240
|
||||
|
||||
|
||||
def _named_opening(body: str | None) -> str:
|
||||
text = " ".join((body or "").split())
|
||||
if len(text) <= _NAMED_OPENING_CHARS:
|
||||
return text
|
||||
return text[:_NAMED_OPENING_CHARS].rsplit(" ", 1)[0] + " …"
|
||||
|
||||
|
||||
async def _named_records(user_id: int, ids: list[int]) -> list:
|
||||
"""The records behind ids the operator named that this user may open.
|
||||
|
||||
An id that is trashed, someone else's private record or not a record at all
|
||||
is dropped without a word: the parser cannot tell "4448" the task from
|
||||
"4448" a quantity it misread, and the lookup is what settles it.
|
||||
"""
|
||||
found = []
|
||||
for nid in ids:
|
||||
got = await notes_svc.get_note_for_user(user_id, nid)
|
||||
if got is not None and got[0].deleted_at is None:
|
||||
found.append(got[0])
|
||||
return found
|
||||
|
||||
# Max chars of a Process body to fold into the auto-surface description.
|
||||
_PROC_PREVIEW_CHARS = 200
|
||||
|
||||
@@ -1071,6 +1100,13 @@ async def build_autoinject_hint(
|
||||
q = (query or "").strip()
|
||||
if not cfg["enabled"] or not q:
|
||||
return empty
|
||||
# RECORDS NAMED BY NUMBER (#4796), read from the operator's own words before
|
||||
# the query is enriched: a number in the assistant's reply is not one the
|
||||
# operator named. "go ahead with 4448" says which record is meant, and a
|
||||
# number means nothing to an embedding, so these are looked up rather than
|
||||
# left to a ranking that can only match the words around them.
|
||||
named = await _named_records(user_id, record_refs_svc.named_record_ids(q))
|
||||
named_ids = [int(n.id) for n in named]
|
||||
# Everything below searches, logs and fills its slots on the ENRICHED
|
||||
# query, so the telemetry row records what was actually asked (#4364).
|
||||
q = _autoinject_query(q, context)
|
||||
@@ -1122,7 +1158,13 @@ async def build_autoinject_hint(
|
||||
# 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.
|
||||
fresh = [(s, n) for s, n in hits if int(n.id) not in already]
|
||||
# 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"],
|
||||
@@ -1133,36 +1175,89 @@ async def build_autoinject_hint(
|
||||
suppressed=len(hits) - len(fresh),
|
||||
duration_ms=(time.perf_counter() - t0) * 1000.0,
|
||||
)
|
||||
if not hits:
|
||||
if not hits and not named:
|
||||
return empty
|
||||
|
||||
# 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]
|
||||
kept = await _reserve_slot_for_reuse(
|
||||
user_id, q, kept, cfg, project_id=(project_id or None),
|
||||
already=already,
|
||||
)
|
||||
# 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=already,
|
||||
)
|
||||
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.
|
||||
owners = await owner_names_for({
|
||||
int(n.user_id) for _s, n in kept if n.user_id != user_id
|
||||
int(n.user_id) for n in [*named, *(n for _s, n in kept)]
|
||||
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.
|
||||
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}
|
||||
)
|
||||
|
||||
lines: list[str] = []
|
||||
note_ids: list[int] = []
|
||||
# FIRST, because the operator said which record they meant and everything
|
||||
# after this block is a guess at what else might bear on it. The header
|
||||
# says these were found by NUMBER: the parser reads "with 4448" as a
|
||||
# reference, and the reader is the one placed to notice it was a quantity.
|
||||
if named:
|
||||
lines.append(
|
||||
"> Named in your message — the records with those numbers, looked "
|
||||
"up by id rather than matched. Open any in full with `get_note(id)`; "
|
||||
"a line marked `seen` is already in your context:"
|
||||
)
|
||||
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}")
|
||||
|
||||
# "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
|
||||
# already claim they're all one thing.
|
||||
@@ -1170,13 +1265,14 @@ async def build_autoinject_hint(
|
||||
# is shown again with a marker rather than withheld, so the header must stop
|
||||
# promising the old contract. It now says what the marker means instead,
|
||||
# once, rather than each repeated line having to explain itself.
|
||||
lines = [
|
||||
"> Possibly relevant from your Scribe records — open any in full with "
|
||||
"`get_note(id)`, or `get_snippet` / `get_process` / `get_lesson` for "
|
||||
"those kinds. Each line is a record's name, its kind and System, and "
|
||||
"the passage that matched; a line marked `seen` is a pointer to one "
|
||||
"already shown this session, so it is in your context:",
|
||||
]
|
||||
if kept:
|
||||
lines.append(
|
||||
"> Possibly relevant from your Scribe records — open any in full with "
|
||||
"`get_note(id)`, or `get_snippet` / `get_process` / `get_lesson` for "
|
||||
"those kinds. Each line is a record's name, its kind and System, and "
|
||||
"the passage that matched; a line marked `seen` is a pointer to one "
|
||||
"already shown this session, so it is in your context:"
|
||||
)
|
||||
# THE REGISTER, SAID ONCE AND ONLY WHEN IT APPLIES (milestone 385 step 5).
|
||||
#
|
||||
# The operator's requirement for this kind was "they don't always have to
|
||||
@@ -1201,21 +1297,15 @@ async def build_autoinject_hint(
|
||||
"what you are doing and use your judgement — a lesson is not a "
|
||||
"rule and binds nothing."
|
||||
)
|
||||
# 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.
|
||||
stale = await superseded_ids([int(n.id) for _s, n in kept])
|
||||
|
||||
# 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 {}
|
||||
|
||||
systems = await system_names_for({int(n.id) for _s, n in kept if int(n.id) not in already})
|
||||
note_ids: list[int] = []
|
||||
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.
|
||||
@@ -1270,18 +1360,31 @@ async def build_autoinject_hint(
|
||||
record_surfaced(
|
||||
user_id=user_id,
|
||||
note_ids=[
|
||||
i for i in 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.
|
||||
named_fresh = [i for i in named_ids if i not in already]
|
||||
if named_fresh:
|
||||
record_surfaced(
|
||||
user_id=user_id, note_ids=named_fresh, source="named_ref",
|
||||
project_id=project_id,
|
||||
)
|
||||
|
||||
# The lessons this menu put in front of the reader, repeats included — for
|
||||
# the soft-link recorder (#4637), which pairs them with the rule arm's
|
||||
# lines in the same response. Relevance, not the session ledger, is what
|
||||
# makes a co-arrival, so a `seen` lesson counts.
|
||||
lesson_ids = [int(n.id) for _s, n in kept if _record_kind(n) == LESSON_NOTE_TYPE]
|
||||
# makes a co-arrival, so a `seen` lesson counts — and so does one the
|
||||
# operator named, which is relevance by their own say-so.
|
||||
lesson_ids = [
|
||||
int(n.id) for n in [*named, *(n for _s, n in kept)]
|
||||
if _record_kind(n) == LESSON_NOTE_TYPE
|
||||
]
|
||||
return {
|
||||
"context": "\n".join(lines), "note_ids": note_ids, "config": cfg,
|
||||
"lesson_ids": lesson_ids,
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Record references written into prose: refusing guessed ids, resolving placeholders.
|
||||
"""Record references written into prose: refusing guessed ids, resolving placeholders, reading the ids a message names.
|
||||
|
||||
THE DEFECT THIS EXISTS FOR (#4016)
|
||||
|
||||
@@ -129,3 +129,127 @@ def resolve_placeholders(text: str | None, refs: dict[str, str]) -> str | None:
|
||||
if not text:
|
||||
return text
|
||||
return PLACEHOLDER_RE.sub(lambda m: refs[m.group(1)], text)
|
||||
|
||||
|
||||
# ── Records an operator names in a message (#4796) ─────────────────────────
|
||||
#
|
||||
# "yes go ahead with 4448" says exactly which record is meant, and the prompt
|
||||
# menu could not use it: a number carries no meaning to an embedding, so the
|
||||
# menu filled with whatever resembled the words AROUND it. These ids are looked
|
||||
# up directly instead (plugin_context.build_autoinject_hint).
|
||||
#
|
||||
# A guard against a false READ, not a false lookup. A number that is not a
|
||||
# record id still has to resolve to a record this user can open before anything
|
||||
# is shown, and the line says it was found by number — but every wrong id is a
|
||||
# record dropped in front of the reader for no reason, so the parser takes a
|
||||
# number only where the sentence makes it a reference.
|
||||
|
||||
# How many named records one message can bring in. A real message names one or
|
||||
# two; past a handful the numbers are a pasted list or a log, not a reference.
|
||||
NAMED_LIMIT = 5
|
||||
|
||||
# A number, with its `#` when it has one. The lookbehind keeps out a number
|
||||
# inside a word, a path or URL (`pulls/198`), a version or date (`1.2`,
|
||||
# `2026-10-03`), money and entities; the lookahead keeps out the same on the
|
||||
# other side, a unit glued on (`300ms`, `40%`) and a thousands separator.
|
||||
_NAMED_RE = re.compile(
|
||||
r"(?<![\w#&$€£.:/\\=@~-])(#?)(\d{1,7})(?![\w%]|[.:/-]\d|,\d{3}(?!\d))"
|
||||
)
|
||||
|
||||
# Words after which `#N` belongs to a different numbering: a forge's (a PR, a
|
||||
# CI run, a job) or one of Scribe's own id spaces that is not a note's. Rules,
|
||||
# milestones, Systems, projects and work-logs each draw from their own
|
||||
# sequence, so "rule #153" is not note 153 — and note 153 is somebody's record.
|
||||
_OTHER_SPACE = frozenset({
|
||||
"pr", "prs", "pull", "pulls", "request", "requests", "mr", "mrs",
|
||||
"run", "runs", "ci", "build", "builds", "job", "jobs", "pipeline",
|
||||
"action", "actions", "commit", "github", "gitlab", "gitea", "gh",
|
||||
"rule", "rules", "preference", "preferences", "milestone", "milestones",
|
||||
"system", "systems", "project", "projects", "log", "logs", "topic",
|
||||
"topics", "rulebook", "rulebooks", "step", "steps", "rank", "line",
|
||||
"lines", "page", "item", "option", "version", "port",
|
||||
})
|
||||
|
||||
# Words after which a BARE number is a reference. Bare numbers are most of what
|
||||
# a message holds — sizes, counts, durations — so a bare one is taken only
|
||||
# after a word that points at something, at the very start of the message, or
|
||||
# continuing a list that started as a reference.
|
||||
_REF_WORDS = frozenset({
|
||||
"with", "for", "on", "at", "about", "re", "regarding", "into", "up",
|
||||
"task", "tasks", "issue", "issues", "note", "notes", "record", "records",
|
||||
"snippet", "snippets", "lesson", "lessons", "spike", "spikes", "process",
|
||||
"continue", "resume", "start", "reopen", "open", "close", "finish",
|
||||
"see", "check", "read", "do", "id", "ids",
|
||||
})
|
||||
|
||||
# A word after a bare number that makes it a quantity: "for 300 seconds",
|
||||
# "about 500 records". Not applied after `#`, which is never a quantity.
|
||||
_UNITS = frozenset({
|
||||
"s", "sec", "secs", "second", "seconds", "ms", "min", "mins", "minute",
|
||||
"minutes", "h", "hr", "hrs", "hour", "hours", "day", "days", "week",
|
||||
"weeks", "month", "months", "year", "years", "char", "chars", "character",
|
||||
"characters", "token", "tokens", "line", "lines", "word", "words", "byte",
|
||||
"bytes", "kb", "mb", "gb", "px", "row", "rows", "record", "records",
|
||||
"note", "notes", "task", "tasks", "item", "items", "file", "files",
|
||||
"test", "tests", "call", "calls", "time", "times", "x", "percent",
|
||||
"result", "results", "hit", "hits", "query", "queries", "message",
|
||||
"messages", "prompt", "prompts", "step", "steps", "point", "points",
|
||||
})
|
||||
|
||||
_FENCE_RE = re.compile(r"```.*?(?:```|\Z)", re.S)
|
||||
_PREV_WORD_RE = re.compile(r"([A-Za-z]+)[\s\"'(\[*_:]*\Z")
|
||||
_NEXT_WORD_RE = re.compile(r"\s*([A-Za-z]+)")
|
||||
_OPENS_RE = re.compile(r"[\s\"'(\[*_>-]*")
|
||||
_LIST_JOIN_RE = re.compile(r"\s*(?:,|,?\s*(?:and|or|&|\+|/))\s*", re.I)
|
||||
|
||||
|
||||
def _prev_word(before: str) -> str:
|
||||
"""The word right before a number, past a space, a quote or a bracket."""
|
||||
m = _PREV_WORD_RE.search(before)
|
||||
return m.group(1).lower() if m else ""
|
||||
|
||||
|
||||
def _next_word(after: str) -> str:
|
||||
m = _NEXT_WORD_RE.match(after)
|
||||
return m.group(1).lower() if m else ""
|
||||
|
||||
|
||||
def named_record_ids(prompt: str | None, limit: int = NAMED_LIMIT) -> list[int]:
|
||||
"""The record ids a message names, in the order it names them.
|
||||
|
||||
`#4448` unless the word before it is another numbering ("PR #198"); a bare
|
||||
number of three or more digits when it opens the message ("4417 sounds
|
||||
right"), follows a word that points at something ("go ahead with 4448") or
|
||||
continues a list one of those started ("4448, 4449 and 4450"), and is not
|
||||
a quantity ("for 300 seconds"). Fenced code is skipped: a pasted log is
|
||||
full of numbers and names nothing.
|
||||
|
||||
Read the operator's OWN words, never a query enriched with the assistant's
|
||||
reply — a number the agent wrote is not one the operator named.
|
||||
"""
|
||||
text = _FENCE_RE.sub("\n", prompt or "")
|
||||
found: list[int] = []
|
||||
last_end, last_ok = -1, False
|
||||
for m in _NAMED_RE.finditer(text):
|
||||
hashed, digits = bool(m.group(1)), m.group(2)
|
||||
before, after = text[:m.start()], text[m.end():]
|
||||
# A list inherits its head's reading, either way: "PR #198 and #199"
|
||||
# is two PRs, "with 4448, 4449" is two records.
|
||||
if last_end >= 0 and _LIST_JOIN_RE.fullmatch(text[last_end:m.start()]):
|
||||
ok = last_ok
|
||||
elif hashed:
|
||||
ok = _prev_word(before) not in _OTHER_SPACE
|
||||
else:
|
||||
ok = (bool(_OPENS_RE.fullmatch(before))
|
||||
or _prev_word(before) in _REF_WORDS)
|
||||
last_end, last_ok = m.end(), ok
|
||||
if not ok or digits[0] == "0" or len(digits) < (2 if hashed else 3):
|
||||
continue
|
||||
if not hashed and _next_word(after) in _UNITS:
|
||||
continue
|
||||
nid = int(digits)
|
||||
if nid not in found:
|
||||
found.append(nid)
|
||||
if len(found) >= limit:
|
||||
break
|
||||
return found
|
||||
|
||||
@@ -137,6 +137,16 @@ POINTS: dict[str, Point] = dict([
|
||||
"the one line reserved for a preference at the prompt boundary"),
|
||||
_p("reuse_slot", UNBIDDEN, "the one line reserved for a reusable snippet"),
|
||||
_p("lesson_slot", UNBIDDEN, "the one line reserved for a lesson"),
|
||||
# A LOOKUP, not a ranker (#4796): a record the operator named by number
|
||||
# in the message, fetched by id. No score and no bar, so it writes no
|
||||
# retrieval_logs row and no score-shaped warning can apply; its rows are
|
||||
# in `note_usage_events`, where pull-through says whether naming a record
|
||||
# is followed by opening it.
|
||||
_p("named_ref", UNBIDDEN,
|
||||
"a record the operator named by number, looked up directly",
|
||||
expects_traffic=False,
|
||||
quiet_because="speaks only when a message names a record by its id; "
|
||||
"a window in which none did is correctly silent here"),
|
||||
_p("rule_via_lesson", UNBIDDEN,
|
||||
"a rule reached through a lesson confirmed as an instance of it",
|
||||
expects_traffic=False,
|
||||
|
||||
@@ -0,0 +1,191 @@
|
||||
"""A record the operator names by number is looked up, not left to ranking (#4796).
|
||||
|
||||
"yes go ahead with 4448" says exactly which record is meant, and the prompt
|
||||
menu could not use it: a number means nothing to an embedding, so the menu
|
||||
filled with whatever resembled the words around it. The parser cases below are
|
||||
drawn from real operator prompts, and the half that must NOT be read as a
|
||||
reference matters as much as the half that must: every misread number is a
|
||||
stranger's record dropped in front of the reader.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services.record_refs import NAMED_LIMIT, named_record_ids
|
||||
from tests.helpers import fake_note
|
||||
|
||||
|
||||
# ── The parser ─────────────────────────────────────────────────────────────
|
||||
|
||||
NAMES = [
|
||||
("yes go ahead with 4448", [4448]),
|
||||
("4364 - we had talked about the follow-up case", [4364]),
|
||||
("4417 sounds like the right call", [4417]),
|
||||
("awesome go for 3898", [3898]),
|
||||
("let's continue #4772 then", [4772]),
|
||||
("does #12 still hold?", [12]),
|
||||
("look at 4448, 4449 and 4450", [4448, 4449, 4450]),
|
||||
("with #4448 & #4449", [4448, 4449]),
|
||||
("merged (#4794) this morning", [4794]),
|
||||
("task 4448 first, then #4449", [4448, 4449]),
|
||||
("#4448 again, and #4448", [4448]), # once, in mention order
|
||||
]
|
||||
|
||||
NOT_NAMES = [
|
||||
"merge PR #198 to main",
|
||||
"PR #198 and #199 are both green", # a list inherits its head
|
||||
"CI #7855 went red",
|
||||
"rule 153 says plain merges",
|
||||
"rule #153 says plain merges",
|
||||
"milestone 444 is next",
|
||||
"wait for 300 seconds first",
|
||||
"about 500 records came back",
|
||||
"move the budget 3 to 6",
|
||||
"it ran 7855 times", # no reference word before it
|
||||
"bump it to 1.2.300",
|
||||
"on 2026-10-03 it broke",
|
||||
"see https://forge/x/pulls/198",
|
||||
"the cut is at 1,000",
|
||||
"costs $250 a month",
|
||||
"use 300ms",
|
||||
"rank #1 was unrelated", # one digit after `#`
|
||||
"with 42 of them", # two digits, bare
|
||||
"```\nfor 4448 in range\n```", # fenced code names nothing
|
||||
"",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("prompt,want", NAMES, ids=[p for p, _ in NAMES])
|
||||
def test_a_named_record_is_read(prompt, want):
|
||||
assert named_record_ids(prompt) == want
|
||||
|
||||
|
||||
@pytest.mark.parametrize("prompt", NOT_NAMES, ids=[p[:30] or "empty" for p in NOT_NAMES])
|
||||
def test_a_number_that_is_not_a_reference_is_left(prompt):
|
||||
assert named_record_ids(prompt) == []
|
||||
|
||||
|
||||
def test_a_long_list_is_capped():
|
||||
prompt = "with " + ", ".join(str(1000 + i) for i in range(NAMED_LIMIT + 3))
|
||||
assert len(named_record_ids(prompt)) == NAMED_LIMIT
|
||||
|
||||
|
||||
# ── The menu ───────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
async def _hint(prompt, *, hits=(), records=None, seen=(), context=""):
|
||||
"""Run the prompt arm with the search, lookup and recorders stubbed.
|
||||
|
||||
`records` is what the ACL lookup can see, by id; anything else is
|
||||
inaccessible.
|
||||
"""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
records = records or {}
|
||||
calls: list[int] = []
|
||||
|
||||
async def _search(*_a, **_kw):
|
||||
calls.append(1)
|
||||
return list(hits) if len(calls) == 1 else []
|
||||
|
||||
async def _get(_uid, nid):
|
||||
note = records.get(nid)
|
||||
return (note, "owner") if note is not None else None
|
||||
|
||||
surfaced = MagicMock()
|
||||
retrieval = MagicMock()
|
||||
with patch.object(pc, "get_autoinject_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.55, "top_k": 3})), \
|
||||
patch.object(pc, "semantic_search_notes", _search), \
|
||||
patch.object(pc.notes_svc, "get_note_for_user", AsyncMock(side_effect=_get)), \
|
||||
patch.object(pc, "superseded_ids", AsyncMock(return_value=set())), \
|
||||
patch.object(pc, "system_names_for", AsyncMock(return_value={})), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \
|
||||
patch.object(pc, "record_retrieval", retrieval), \
|
||||
patch.object(pc, "record_surfaced", surfaced):
|
||||
out = await pc.build_autoinject_hint(
|
||||
1, prompt, project_id=2, exclude_ids=list(seen), context=context,
|
||||
)
|
||||
by_source = {c.kwargs["source"]: c.kwargs["note_ids"] for c in surfaced.call_args_list}
|
||||
return out, by_source, retrieval
|
||||
|
||||
|
||||
def _task(nid, title, body=""):
|
||||
return fake_note(id=nid, title=title, body=body, user_id=1,
|
||||
is_task=True, status="todo", task_kind="issue")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_named_record_arrives_even_when_nothing_matched():
|
||||
"""The case the whole change is for: a short prompt whose words match
|
||||
nothing still names its record, and the record arrives."""
|
||||
rec = _task(4448, "Filing runs itself after each scan",
|
||||
"## Symptom\nBooks sat unfiled until someone pressed the button.")
|
||||
out, surfaced, _ = await _hint("yes go ahead with 4448", records={4448: rec})
|
||||
|
||||
ctx = out["context"]
|
||||
assert "Named in your message" in ctx
|
||||
assert '#4448 [issue (todo)] "Filing runs itself after each scan"' in ctx
|
||||
assert "↳ ## Symptom Books sat unfiled" in ctx
|
||||
assert "Possibly relevant" not in ctx, "a menu header over no menu lines"
|
||||
assert out["note_ids"] == [4448], "the named record must reach the session ledger"
|
||||
assert surfaced["named_ref"] == [4448]
|
||||
assert not surfaced.get("auto_inject"), "the ranking arm surfaced nothing here"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_named_record_comes_first_and_is_not_repeated_as_a_match():
|
||||
named = _task(4448, "the named one")
|
||||
other = fake_note(id=11, title="something that matched the words", user_id=1)
|
||||
hits = [(0.80, named), (0.78, other)]
|
||||
out, surfaced, retrieval = await _hint(
|
||||
"go ahead with 4448", hits=hits, records={4448: named},
|
||||
)
|
||||
|
||||
ctx = out["context"]
|
||||
assert ctx.index("Named in your message") < ctx.index("Possibly relevant")
|
||||
assert ctx.count("#4448") == 1, "the named record was shown twice"
|
||||
assert out["note_ids"] == [4448, 11]
|
||||
# Booked once, under the lookup — the ranking arm did not surface it, and
|
||||
# its log row counts it as suppressed so the row and the usage table agree.
|
||||
assert surfaced["named_ref"] == [4448]
|
||||
assert surfaced["auto_inject"] == [11]
|
||||
row = retrieval.call_args_list[0].kwargs
|
||||
assert row["source"] == "auto_inject"
|
||||
assert [int(n.id) for _s, n in row["results"]] == [11]
|
||||
assert row["suppressed"] == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_named_record_already_shown_is_a_pointer():
|
||||
rec = _task(4448, "the named one", "a long body that is already in context")
|
||||
out, surfaced, _ = await _hint("with 4448", records={4448: rec}, seen=[4448])
|
||||
|
||||
assert "> - #4448 [issue (todo) · seen] the named one" in out["context"]
|
||||
assert "already in context" not in out["context"]
|
||||
assert "named_ref" not in surfaced, "a repeat is not a new surfacing"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_id_the_reader_cannot_open_is_dropped():
|
||||
"""Not found, not theirs, or in the trash — all three say nothing, because
|
||||
the parser cannot tell a record id from a number it misread."""
|
||||
trashed = _task(4449, "gone")
|
||||
trashed.deleted_at = "2026-10-01T00:00:00Z"
|
||||
out, surfaced, _ = await _hint("with 4448 and 4449", records={4449: trashed})
|
||||
|
||||
assert out["context"] == ""
|
||||
assert out["note_ids"] == []
|
||||
assert "named_ref" not in surfaced
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_number_in_the_assistants_reply_is_not_named():
|
||||
"""The query is enriched with the reply before a short prompt is searched
|
||||
(#4364); only the operator's own words can name a record."""
|
||||
rec = _task(4448, "the named one")
|
||||
out, _, _ = await _hint(
|
||||
"sounds good", records={4448: rec},
|
||||
context="I could start on #4448 next if you want.",
|
||||
)
|
||||
assert "Named in your message" not in out["context"]
|
||||
Reference in New Issue
Block a user