Merge pull request 'Retrieval: the passage that matched, every kind searchable, work logs and charters findable' (#175) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 1m3s
CI & Build / Python tests (push) Successful in 1m36s
CI & Build / Build & push image (push) Successful in 13s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / integration (push) Successful in 1m3s
CI & Build / Python tests (push) Successful in 1m36s
CI & Build / Build & push image (push) Successful in 13s
This commit was merged in pull request #175.
This commit is contained in:
@@ -0,0 +1,67 @@
|
||||
"""system_embeddings — a charter becomes an ANSWER, not just a filter (#4251)
|
||||
|
||||
Revision ID: 0107
|
||||
Revises: 0106
|
||||
Create Date: 2026-09-21
|
||||
|
||||
A System's `description` is a charter: several hundred words saying what
|
||||
belongs in that area and what does not. It is the answer to "which part of
|
||||
this codebase does X live in", and there was no semantic path to it.
|
||||
`list_systems` enumerates and `search(system_id=…)` uses a System as a FILTER
|
||||
over notes — so a System could narrow a search and could never be the answer
|
||||
to one, and an agent asking where a record belonged had to read every charter
|
||||
or guess.
|
||||
|
||||
The fourth sibling of note_embeddings (0067), rule_embeddings (0089) and
|
||||
milestone_embeddings (0102), and for the same reason note 3163 gives about the
|
||||
third: the row could be shared, the search cannot. "Where does this belong?"
|
||||
is a different question from "what prior art is there?", and no note or rule
|
||||
search can answer it, because a charter is not a note.
|
||||
|
||||
The vectors are DERIVED: nothing is backfilled here, the startup backfill
|
||||
writes them.
|
||||
"""
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
revision = "0107"
|
||||
down_revision = "0106"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
# Matches its three siblings — bge-small-en-v1.5, 384-dim.
|
||||
_EMBEDDING_DIM = 384
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.create_table(
|
||||
"system_embeddings",
|
||||
sa.Column(
|
||||
"system_id", sa.Integer(),
|
||||
sa.ForeignKey("systems.id", ondelete="CASCADE"), primary_key=True,
|
||||
),
|
||||
sa.Column("chunk_index", sa.Integer(), primary_key=True),
|
||||
sa.Column("chunk_text", sa.Text(), nullable=False),
|
||||
sa.Column("chunker_version", sa.Integer(), nullable=False),
|
||||
sa.Column(
|
||||
"updated_at", sa.DateTime(timezone=True), nullable=False,
|
||||
server_default=sa.text("now()"),
|
||||
),
|
||||
)
|
||||
# Raw DDL for the vector column, as 0067, 0089 and 0102 do: the type comes
|
||||
# from the pgvector extension, not SQLAlchemy's type system.
|
||||
op.execute(
|
||||
f"ALTER TABLE system_embeddings ADD COLUMN embedding vector({_EMBEDDING_DIM}) NOT NULL"
|
||||
)
|
||||
op.execute(
|
||||
"""
|
||||
CREATE INDEX ix_system_embeddings_embedding_hnsw
|
||||
ON system_embeddings
|
||||
USING hnsw (embedding vector_cosine_ops)
|
||||
"""
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.execute("DROP INDEX IF EXISTS ix_system_embeddings_embedding_hnsw")
|
||||
op.drop_table("system_embeddings")
|
||||
@@ -29,7 +29,17 @@ interface KnowledgeItem {
|
||||
id: number;
|
||||
note_type: "note" | "task" | "process" | "snippet" | "lesson";
|
||||
title: string;
|
||||
/** ONE SPAN of the record, never the whole thing. On a search this is the
|
||||
* passage that actually matched; on a plain browse it is the opening,
|
||||
* because nothing was matched and no span is better than another. */
|
||||
snippet: string;
|
||||
/** Which span `snippet` holds. A reader who cannot tell the matched passage
|
||||
* from the document's first paragraph cannot tell whether a card that looks
|
||||
* unrelated really is. */
|
||||
snippet_is?: "matched_passage" | "body_opening";
|
||||
/** Characters in the whole record, so a long record shown by a short span is
|
||||
* visible as one. */
|
||||
body_length?: number;
|
||||
tags: string[];
|
||||
project_id: number | null;
|
||||
created_at: string;
|
||||
@@ -181,6 +191,16 @@ const CONTENT_PAGE = 24; // items loaded per sentinel trigger
|
||||
const REFILL_THRESHOLD = 48; // fetch more IDs when queue drops below this
|
||||
|
||||
const items = ref<KnowledgeItem[]>([]);
|
||||
|
||||
// True only when a search actually returned matched passages — not merely when
|
||||
// the box has text in it. A keyword-only result set, or an embedder that is
|
||||
// down, carries `body_opening` rows, and claiming otherwise would be the same
|
||||
// species of lie this whole change is about (#4243).
|
||||
const showsMatchedPassages = computed(
|
||||
() =>
|
||||
searchQuery.value.trim().length > 0 &&
|
||||
items.value.some((i) => i.snippet_is === "matched_passage"),
|
||||
);
|
||||
const allTags = ref<string[]>([]);
|
||||
const idQueue = ref<number[]>([]); // unloaded IDs ready to be content-fetched
|
||||
const idOffset = ref(0); // next offset for ID batch requests
|
||||
@@ -573,8 +593,16 @@ onUnmounted(() => {
|
||||
<p v-else class="empty-narrator">Your story is unwritten. Create your first note to begin.</p>
|
||||
</div>
|
||||
|
||||
<!-- Said once above the grid rather than per card: it is true of every
|
||||
row at once, and a label repeated on each would cost more than it
|
||||
tells. Only while a query is active — on a plain browse nothing
|
||||
matched, so there is no matched passage to explain. -->
|
||||
<p v-else-if="showsMatchedPassages" class="k-excerpt-note">
|
||||
Excerpts below are the passage that matched your search, not the start of each record.
|
||||
</p>
|
||||
|
||||
<!-- Card grid -->
|
||||
<div v-else class="card-grid">
|
||||
<div v-if="items.length" class="card-grid">
|
||||
<div
|
||||
v-for="item in items"
|
||||
:key="item.id"
|
||||
@@ -1004,6 +1032,11 @@ onUnmounted(() => {
|
||||
line-height: 1.45;
|
||||
margin: 0;
|
||||
}
|
||||
.k-excerpt-note {
|
||||
font-size: 0.8rem;
|
||||
color: var(--fs-text-tertiary);
|
||||
margin: 0 0 var(--fs-space-3);
|
||||
}
|
||||
.k-card-footer {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
|
||||
+9
-1
@@ -173,7 +173,8 @@ def create_app() -> Quart:
|
||||
|
||||
from scribe.services.auth import start_auth_token_retention_loop
|
||||
from scribe.services.embeddings import (
|
||||
backfill_milestone_embeddings, backfill_note_embeddings, backfill_rule_embeddings,
|
||||
backfill_milestone_embeddings, backfill_note_embeddings,
|
||||
backfill_rule_embeddings, backfill_system_embeddings,
|
||||
)
|
||||
from scribe.services.logging import start_log_retention_loop
|
||||
from scribe.services.notifications import start_notification_loop
|
||||
@@ -243,6 +244,13 @@ def create_app() -> Quart:
|
||||
await backfill_milestone_embeddings()
|
||||
except Exception:
|
||||
logger.warning("Milestone embedding backfill failed", exc_info=True)
|
||||
# Systems got vectors in #4251, so before this pass every charter
|
||||
# ever written is unfindable — a System could narrow a search and
|
||||
# never be the answer to one.
|
||||
try:
|
||||
await backfill_system_embeddings()
|
||||
except Exception:
|
||||
logger.warning("System embedding backfill failed", exc_info=True)
|
||||
# Snippets written before migration 0070 have no `notes.data` mirror,
|
||||
# and the location reverse lookup queries that column — an unfilled
|
||||
# row would read as "no snippet here" rather than as a gap. Separate
|
||||
|
||||
@@ -15,6 +15,7 @@ from scribe.mcp._context import current_user_id
|
||||
from scribe.services import dedup as dedup_svc
|
||||
from scribe.services import milestones as milestones_svc
|
||||
from scribe.services import notes as notes_svc
|
||||
from scribe.services import task_logs as task_logs_svc
|
||||
from scribe.services import rulebooks as rulebooks_svc
|
||||
from scribe.services import trash as trash_svc
|
||||
from scribe.services.record_refs import refuse_guessed_ids
|
||||
@@ -53,6 +54,11 @@ async def get_milestone(milestone_id: int) -> dict:
|
||||
what it is and where it stands, not its whole body — read a step in full
|
||||
with get_task(id). A plan with forty long steps is otherwise too big to
|
||||
arrive inline, and the design is in the milestone body.
|
||||
|
||||
Each step also carries `log_count`. A step's STATUS is set by hand and a
|
||||
plan is exactly where that goes stale; the count says which steps have a
|
||||
work log that would say otherwise, so `get_task(id)` goes to the step
|
||||
with a record rather than to each one in turn (#4241).
|
||||
"""
|
||||
uid = current_user_id()
|
||||
milestone = await milestones_svc.get_milestone(uid, milestone_id)
|
||||
@@ -65,11 +71,19 @@ async def get_milestone(milestone_id: int) -> dict:
|
||||
applicable = await rulebooks_svc.get_applicable_rules(
|
||||
project_id=milestone.project_id, user_id=uid,
|
||||
)
|
||||
log_counts = await task_logs_svc.log_counts_for_tasks(
|
||||
uid, [int(t.id) for t in steps]
|
||||
)
|
||||
out = milestone.to_dict()
|
||||
out.update(progress)
|
||||
return {
|
||||
"milestone": out,
|
||||
"steps": [notes_svc.brief_row(t, {milestone.id: milestone.title}) for t in steps],
|
||||
"steps": [
|
||||
notes_svc.brief_row(
|
||||
t, {milestone.id: milestone.title}, log_counts=log_counts
|
||||
)
|
||||
for t in steps
|
||||
],
|
||||
**rulebooks_svc.rules_payload(applicable, user_id=uid, source="get_milestone"),
|
||||
}
|
||||
|
||||
|
||||
+175
-11
@@ -11,14 +11,36 @@ import time
|
||||
|
||||
from scribe.mcp._context import current_user_id
|
||||
from scribe.services.access import owner_names_for
|
||||
from scribe.services.knowledge import content_type_filters
|
||||
from scribe.services.text import MATCHED_PASSAGE, excerpt_fields
|
||||
from scribe.services.embeddings import (
|
||||
DEFAULT_SIMILARITY_THRESHOLD, semantic_search_milestones, semantic_search_notes,
|
||||
semantic_search_rules,
|
||||
semantic_search_rules, semantic_search_systems,
|
||||
)
|
||||
from scribe.services import rulebooks as rulebooks_svc
|
||||
from scribe.services.retrieval_telemetry import record_retrieval, retrieval_summary
|
||||
|
||||
|
||||
# A matched chunk is at most _CHUNK_CHAR_BUDGET (1400) characters, and it is
|
||||
# the evidence the ranking was built on — so it is worth more room than the 240
|
||||
# characters of document opening this used to send. Elision inside a chunk is
|
||||
# far less lossy than a head cut of a whole record: the region is already the
|
||||
# right one.
|
||||
_EXCERPT_CHARS = 1000
|
||||
|
||||
|
||||
# The kinds `content_type` accepts are DERIVED from the facet table, not listed
|
||||
# again here — that table is where a kind is declared (#3161), and a second
|
||||
# hand-kept copy in this module is precisely how the agent's door came to offer
|
||||
# two kinds while browse offered nine (#4250). `content_type_filters` carries
|
||||
# the mapping, the 'all'/'note' special cases and the refusal.
|
||||
#
|
||||
# These two do not reach `semantic_search_notes` at all: they have their own
|
||||
# search and their own result shape, so they are dispatched before the mapping
|
||||
# and passed in only so the refusal message lists everything THIS door takes.
|
||||
_OWN_SEARCH = ("rule", "milestone", "system")
|
||||
|
||||
|
||||
async def _search_rules(uid: int, q: str, limit: int, project_id: int) -> dict:
|
||||
"""Rules by meaning — a separate result shape because a rule IS different.
|
||||
|
||||
@@ -73,10 +95,19 @@ async def _search_milestones(uid: int, q: str, limit: int, project_id: int) -> d
|
||||
|
||||
Its own result shape, like rules: a milestone is a plan with progress, not
|
||||
a note with a body. The plan itself is left out — get_milestone reads it —
|
||||
because a search hit is for recognising a plan, and bodies run long.
|
||||
because a search hit is for recognising a plan, and bodies run long. What
|
||||
does come along is `matched`: the one passage of the body the query
|
||||
actually hit, with `matched_is` saying whether it is that passage or
|
||||
merely the body's opening. Recognising a plan means recognising the part
|
||||
of it you were asking about, and a description written at the start need
|
||||
not mention that part (#4243).
|
||||
Not part of content_type="all", whose results are note-shaped.
|
||||
"""
|
||||
raw = await semantic_search_milestones(uid, q, project_id=project_id or None, limit=limit)
|
||||
report: dict = {}
|
||||
raw = await semantic_search_milestones(
|
||||
uid, q, project_id=project_id or None, limit=limit, report=report,
|
||||
)
|
||||
chunks = report.get("best_chunk") or {}
|
||||
progress: dict[int, dict] = {}
|
||||
if raw:
|
||||
from scribe.services import milestones as milestones_svc
|
||||
@@ -90,6 +121,14 @@ async def _search_milestones(uid: int, q: str, limit: int, project_id: int) -> d
|
||||
"id": m.id,
|
||||
"title": m.title,
|
||||
"description": m.description or "",
|
||||
# The plan body stays out — get_milestone reads it — but the
|
||||
# passage that MATCHED comes along, because a plan is
|
||||
# recognised by the part of it the query was about and a
|
||||
# description need not mention that part at all (#4243).
|
||||
**excerpt_fields(
|
||||
m.body or "", chunks.get(int(m.id)), _EXCERPT_CHARS,
|
||||
key="matched",
|
||||
),
|
||||
"status": m.status,
|
||||
"project_id": m.project_id,
|
||||
"total": progress.get(m.id, {}).get("total", 0),
|
||||
@@ -102,6 +141,77 @@ async def _search_milestones(uid: int, q: str, limit: int, project_id: int) -> d
|
||||
}
|
||||
|
||||
|
||||
async def _search_systems(uid: int, q: str, limit: int, project_id: int) -> dict:
|
||||
"""Systems by meaning — "where does this belong?" (#4251).
|
||||
|
||||
A System's `description` is a charter: several hundred words saying what
|
||||
belongs in that area and what does not. `list_systems` enumerates them and
|
||||
`system_id` filters by one, so before this a System could NARROW a search
|
||||
and could never be the answer to one — an agent asking where a record
|
||||
belonged had to read every charter or guess.
|
||||
|
||||
Its own result shape and its own search, not a `content_type` over notes,
|
||||
because the question is different: "where does this belong?" is not "what
|
||||
prior art is there?". A charter competing with the whole note corpus for
|
||||
one top-k would also be outranked by the records filed under it, and the
|
||||
right answer would be crowded out by its own contents.
|
||||
|
||||
The charter's `matched` passage comes along rather than the whole thing.
|
||||
A charter is long and the paragraph that decides where a record belongs is
|
||||
the one worth reading; the rest is get_system (#4243).
|
||||
|
||||
The id that comes back is the one `search(system_id=…)`,
|
||||
`list_system_records` and every `system_ids` argument take — so the answer
|
||||
to "where does this belong?" is directly usable as "show me what is there"
|
||||
and as "file it here".
|
||||
"""
|
||||
report: dict = {}
|
||||
raw = await semantic_search_systems(
|
||||
uid, q, project_id=project_id or None, limit=limit, report=report,
|
||||
)
|
||||
chunks = report.get("best_chunk") or {}
|
||||
return {
|
||||
"results": [
|
||||
{
|
||||
"id": sys_.id,
|
||||
"name": sys_.name,
|
||||
**excerpt_fields(
|
||||
sys_.description or "", chunks.get(int(sys_.id)),
|
||||
_EXCERPT_CHARS, key="matched",
|
||||
),
|
||||
"project_id": sys_.project_id,
|
||||
"similarity": float(score),
|
||||
}
|
||||
for score, sys_ in raw
|
||||
],
|
||||
"total": len(raw),
|
||||
}
|
||||
|
||||
|
||||
def result_excerpt(note, chunk: dict | None) -> dict:
|
||||
"""The part of a record a caller judges "should I open this?" on.
|
||||
|
||||
This used to be `(note.body or "")[:240]` — the document's opening, with
|
||||
no marker that anything had been cut, so a 240-character preview of a
|
||||
4000-character record was indistinguishable from a complete short one.
|
||||
|
||||
The opening is the wrong span. The match was semantic and per-chunk, and
|
||||
`semantic_search_notes` collapses to best-chunk-per-note — so the system
|
||||
already knows which passage earned the hit and used to discard it. A
|
||||
record could rank first on its sixth paragraph and be previewed by its
|
||||
first, which the search had already judged less relevant, and the caller
|
||||
would decide from that and never know (#4243).
|
||||
|
||||
The choice of span lives in services/text.py, shared with the web's
|
||||
knowledge search, so the two doors cannot drift on which text a reader is
|
||||
shown or on whether they are told what it is.
|
||||
"""
|
||||
out = excerpt_fields(note.body or "", chunk, _EXCERPT_CHARS)
|
||||
if out.get("excerpt_is") == MATCHED_PASSAGE and (chunk or {}).get("index") is not None:
|
||||
out["chunk_index"] = int(chunk["index"])
|
||||
return out
|
||||
|
||||
|
||||
async def search(
|
||||
q: str,
|
||||
content_type: str = "all",
|
||||
@@ -120,9 +230,35 @@ async def search(
|
||||
|
||||
Args:
|
||||
q: search query string.
|
||||
content_type: 'all' (default), 'note' (notes only), 'task' (tasks
|
||||
only), or 'rule' (RULES only — the operator's standing
|
||||
instructions, searchable by meaning since milestone 307).
|
||||
content_type: which kind of record to search. 'all' (default) spans
|
||||
every note and task.
|
||||
|
||||
THE BROAD TWO: 'note' is any non-task record — it still includes
|
||||
snippets, lessons and processes, so it means "knowledge, not work
|
||||
items". 'task' is any task whatever its kind.
|
||||
|
||||
THE SPECIFIC KINDS, each narrowing to one: 'snippet' (recorded
|
||||
prior art — reach for this BEFORE writing a helper, rather than
|
||||
searching 'all' and reading past the issues), 'lesson' (a
|
||||
transferable insight, the kind that exists to be recalled by
|
||||
situation), 'process' (a stored procedure the operator saved),
|
||||
'issue' (corrective work — "has this already been reported?"),
|
||||
'spike' (a time-boxed investigation, whose output is an answer),
|
||||
'work', and 'plan' (retired; the ~90 legacy plan-tasks).
|
||||
|
||||
An unrecognised value is REFUSED with the list of valid ones
|
||||
rather than quietly returning nothing: an empty result set is a
|
||||
claim that the corpus holds nothing, and a typo must not be able
|
||||
to make that claim.
|
||||
|
||||
THREE KINDS WITH THEIR OWN SEARCH AND THEIR OWN RESULT SHAPE,
|
||||
because each answers a question no note search can:
|
||||
'rule' (the operator's standing instructions — searchable by
|
||||
meaning since milestone 307), 'milestone' ("is there already a
|
||||
plan for this?"), and 'system' ("where does this belong?" — a
|
||||
System's charter says what belongs in an area and what does not,
|
||||
and the id it returns is the one `system_id` and `system_ids`
|
||||
take).
|
||||
Reach for 'rule' when you want to know whether a standing
|
||||
instruction covers something: "is there a rule about release
|
||||
tagging?". A hit carries the rule's `why` and `how_to_apply`,
|
||||
@@ -150,9 +286,21 @@ async def search(
|
||||
list_system_records gives the same slice unranked.
|
||||
|
||||
Returns:
|
||||
{"results": [{"id", "title", "body", "is_task", "tags", "similarity"}],
|
||||
{"results": [{"id", "title", "excerpt", "excerpt_is", "body_length",
|
||||
"is_task", "tags", "similarity"}],
|
||||
"total": int}
|
||||
|
||||
`excerpt` is a SPAN of the record, not the record. `excerpt_is` says
|
||||
which span: "matched_passage" is the chunk that actually earned the
|
||||
hit — the evidence the ranking was built on, and the right thing to
|
||||
judge relevance from. "body_opening" is a fallback for a record with
|
||||
no stored chunk, and is only the beginning of the text, which may say
|
||||
nothing about why it matched. `body_length` is the whole record's
|
||||
size, so a long record previewed by a short span is visible as one;
|
||||
`read_full` appears when there is more, and opening the id by
|
||||
get_note / get_task is how you get it. Judge from the passage, not
|
||||
from the fact that a preview looked thin.
|
||||
|
||||
A result marked `shared: true` with an `owner` belongs to another user —
|
||||
that person's suggestion, not the operator's own record or settled practice.
|
||||
Weigh it on its merits and say whose it is when you use it.
|
||||
@@ -163,11 +311,14 @@ async def search(
|
||||
return await _search_rules(uid, q, limit, project_id)
|
||||
if content_type == "milestone":
|
||||
return await _search_milestones(uid, q, limit, project_id)
|
||||
is_task = {"note": False, "task": True}.get(content_type) # None => any
|
||||
if content_type == "system":
|
||||
return await _search_systems(uid, q, limit, project_id)
|
||||
filters = content_type_filters(content_type, extra=_OWN_SEARCH)
|
||||
is_task = filters.get("is_task")
|
||||
t0 = time.perf_counter()
|
||||
report: dict = {}
|
||||
raw = await semantic_search_notes(
|
||||
uid, q, limit=limit, is_task=is_task,
|
||||
uid, q, limit=limit, **filters,
|
||||
project_id=project_id or None,
|
||||
system_id=system_id or None,
|
||||
# A LESSON is reachable from any project (milestone 385). The kind
|
||||
@@ -195,12 +346,13 @@ async def search(
|
||||
owners = await owner_names_for(
|
||||
{int(note.user_id) for _s, note in raw if note.user_id != uid}
|
||||
)
|
||||
chunks = report.get("best_chunk") or {}
|
||||
return {
|
||||
"results": [
|
||||
{
|
||||
"id": note.id,
|
||||
"title": note.title,
|
||||
"body": (note.body or "")[:240],
|
||||
**result_excerpt(note, chunks.get(int(note.id))),
|
||||
"is_task": bool(note.is_task),
|
||||
"tags": list(note.tags or []),
|
||||
"similarity": float(score),
|
||||
@@ -421,7 +573,19 @@ It is an UPPER BOUND per surface: a pull records the door it came
|
||||
Only ever raised for unbidden arms known to log unconditionally: a
|
||||
search returning a list every time is doing its job, and an arm whose
|
||||
zeros were never written would flag a LOGGING bug while pointing you at
|
||||
a threshold, which is #3497 exactly.
|
||||
a threshold, which is #3497 exactly. Nor for an arm whose query never
|
||||
changes — see the next entry.
|
||||
- `fixed_query_never_clears` — an arm that always searches the SAME query
|
||||
returned nothing on every call. Its score is one constant, so this is
|
||||
not a quiet window: the bar sits above that constant and no amount of
|
||||
further traffic will produce a different result. The arm is off rather
|
||||
than silent, and nothing else here would say so. The same property is
|
||||
why `cannot_decline` is not raised for these arms: with a constant
|
||||
score the decline rate is 0% or 100% by construction, so "never
|
||||
declined" is arithmetic and not evidence about the floor. Read the
|
||||
refused record (`near_miss_samples`) BEFORE moving the dial — the last
|
||||
time an arm sat here, every percentile said lower it and the refused
|
||||
record showed the refusal was right.
|
||||
- `band_hugs_floor` — the weakest tenth of what an arm returns sits on
|
||||
its floor. The bar is doing the selecting and the score is not, so
|
||||
moving that floor changes how MUCH you get, not how good it is.
|
||||
|
||||
@@ -3,7 +3,13 @@
|
||||
Tasks are notes with a non-null `status` — same model, different filter.
|
||||
Wrappers call services/notes.py for CRUD with is_task=True and add the
|
||||
task-specific fields (status, priority, due_date, parent_id), plus
|
||||
services/task_logs.py for add_task_log.
|
||||
services/task_logs.py for add_task_log AND for reading those logs back —
|
||||
`get_task` returns them and `list_tasks` counts them. For most of this
|
||||
module's life `add_task_log` wrote to a surface no agent could read: the
|
||||
entries reached the web UI and nothing else, so a session opening a task
|
||||
saw only the body — a claim written before the work — with the record
|
||||
written during it invisible beside it. A stale body then had nothing to
|
||||
contradict it, and shipped work got rebuilt (#4241).
|
||||
|
||||
There is no delete_task — matches the existing fable-mcp surface.
|
||||
Cancel by updating status to "cancelled".
|
||||
@@ -39,6 +45,68 @@ from scribe.services import task_logs as task_logs_svc
|
||||
from scribe.services import trash as trash_svc
|
||||
from scribe.services.note_usage import record_pulled
|
||||
from scribe.services.record_refs import refuse_guessed_ids
|
||||
from scribe.services.text import elide
|
||||
|
||||
|
||||
# A work log entry is prose, often long — the discipline asks for what was
|
||||
# decided and why, not a line of status. Two caps, not one, because the entries
|
||||
# are not equally useful: the NEWEST answers "where does this actually stand",
|
||||
# which is the question the block exists for, so it arrives whole up to a
|
||||
# generous ceiling. Older entries are there to say what happened and when, and
|
||||
# a headline does that.
|
||||
_WORK_LOG_ENTRIES = 3
|
||||
_WORK_LOG_CHARS = 800
|
||||
_WORK_LOG_LATEST_CHARS = 4000
|
||||
|
||||
_WORK_LOG_ADVICE = (
|
||||
"The body is a CLAIM, written once before the work. These entries are the "
|
||||
"RECORD, written during and after it. Where the two disagree the log is "
|
||||
"later — read it before acting on what the body says the status is."
|
||||
)
|
||||
|
||||
|
||||
def work_log_payload(
|
||||
logs: list, total: int, chars: int, latest_chars: int = _WORK_LOG_LATEST_CHARS
|
||||
) -> dict:
|
||||
"""The `work_log` block: recent entries newest-first, plus what was elided.
|
||||
|
||||
`total` is the count of ALL entries, not of `logs` — a reader has to be
|
||||
able to tell "this task has no record" from "you were shown the last three
|
||||
of nine", and those are the same response if the count comes from the
|
||||
entries handed over.
|
||||
"""
|
||||
entries = []
|
||||
for i, log in enumerate(logs):
|
||||
row = log.to_dict() if hasattr(log, "to_dict") else dict(log)
|
||||
row.pop("updated_at", None)
|
||||
content = row.get("content") or ""
|
||||
# The first row IS the newest — logs_for_task orders descending.
|
||||
budget = latest_chars if i == 0 else chars
|
||||
if chars <= 0:
|
||||
budget = 0
|
||||
text, cut = elide(content, budget)
|
||||
row["content"] = text
|
||||
if cut:
|
||||
row["full_length"] = len(content)
|
||||
row["truncated"] = True
|
||||
entries.append(row)
|
||||
|
||||
out: dict = {"total": total, "entries": entries}
|
||||
if total == 0:
|
||||
return out
|
||||
|
||||
out["advice"] = _WORK_LOG_ADVICE
|
||||
not_shown = total - len(entries)
|
||||
if not_shown > 0:
|
||||
out["not_shown"] = not_shown
|
||||
if not_shown > 0 or any(e.get("truncated") for e in entries):
|
||||
out["read_all"] = (
|
||||
"Entries were shortened or omitted. "
|
||||
"get_task(task_id, log_limit=0, log_chars=0) returns every entry "
|
||||
"in full — reach for it rather than judging from what is here, "
|
||||
"which was selected by recency and length, not by relevance."
|
||||
)
|
||||
return out
|
||||
|
||||
|
||||
async def list_tasks(
|
||||
@@ -75,10 +143,27 @@ async def list_tasks(
|
||||
offset=max(0, offset),
|
||||
)
|
||||
titles = await milestones_svc.titles_for({n.milestone_id for n in rows})
|
||||
return {"tasks": [notes_svc.brief_row(n, titles) for n in rows], "total": total}
|
||||
# One aggregate for the page, zero-filled: a reader scanning a list needs
|
||||
# to know WHICH rows carry a record before choosing what to open, and a
|
||||
# missing key would read as "no logs" on every row rather than on the
|
||||
# rows that have none.
|
||||
log_counts = await task_logs_svc.log_counts_for_tasks(
|
||||
uid, [int(n.id) for n in rows]
|
||||
)
|
||||
return {
|
||||
"tasks": [
|
||||
notes_svc.brief_row(n, titles, log_counts=log_counts) for n in rows
|
||||
],
|
||||
"total": total,
|
||||
}
|
||||
|
||||
|
||||
async def get_task(task_id: int, project_id: int = 0) -> dict:
|
||||
async def get_task(
|
||||
task_id: int,
|
||||
project_id: int = 0,
|
||||
log_limit: int = _WORK_LOG_ENTRIES,
|
||||
log_chars: int = _WORK_LOG_CHARS,
|
||||
) -> dict:
|
||||
"""Fetch a single Scribe task by ID.
|
||||
|
||||
Returns id, title, body, status, priority, tags, project_id, milestone_id,
|
||||
@@ -89,6 +174,27 @@ async def get_task(task_id: int, project_id: int = 0) -> dict:
|
||||
kind=plan tasks, the response also includes the project's applicable_rules
|
||||
and project_rules (new plans are milestones — use get_milestone for those).
|
||||
|
||||
AND `work_log` — the entries add_task_log wrote, newest first, with
|
||||
`total` for how many exist. READ IT BEFORE TRUSTING THE BODY. A body is
|
||||
written once, at the start, when the least is known; the log is written
|
||||
during the work and after it. A task whose body says "not started" and
|
||||
whose log records a partial ship is not a contradiction to resolve — the
|
||||
log is simply later. This block exists because its absence cost a session
|
||||
a day of rebuilding work that had already shipped (#4241).
|
||||
|
||||
The NEWEST entry arrives whole (to 4000 characters); older ones are
|
||||
shortened to a headline. Anything shortened is cut from the MIDDLE, so
|
||||
the opening and the closing both survive — a log entry's conclusion is at
|
||||
its end — and the gap states how many characters went. An entry that was
|
||||
cut says so and carries its `full_length`, and the block as a whole says
|
||||
when it is showing you less than the record holds.
|
||||
|
||||
Args:
|
||||
log_limit: How many of the most recent entries to include (default 3).
|
||||
0 returns every entry.
|
||||
log_chars: Budget for the OLDER entries (default 800). 0 returns
|
||||
every entry in full, the newest included.
|
||||
|
||||
A task another user shared with you also carries `shared`, `owner` and
|
||||
`permission` — it's their work item, not one you took on.
|
||||
|
||||
@@ -127,6 +233,17 @@ async def get_task(task_id: int, project_id: int = 0) -> dict:
|
||||
await systems_tools.attach_systems(
|
||||
uid, getattr(note, "user_id", uid) or uid, data, note.id, note.project_id
|
||||
)
|
||||
# Counted separately rather than inferred from the rows handed back: with a
|
||||
# limit applied, len(entries) is the size of the window, not of the record,
|
||||
# and "3 entries" and "the last 3 of 9" have to read differently.
|
||||
log_rows = await task_logs_svc.logs_for_task(
|
||||
uid, int(note.id), limit=max(0, log_limit)
|
||||
)
|
||||
log_total = (
|
||||
len(log_rows) if log_limit <= 0
|
||||
else await task_logs_svc.count_logs_for_task(uid, int(note.id))
|
||||
)
|
||||
data["work_log"] = work_log_payload(log_rows, log_total, max(0, log_chars))
|
||||
record_pulled(
|
||||
user_id=uid, note_id=int(note.id),
|
||||
source="mcp_get_task", project_id=project_id,
|
||||
@@ -353,6 +470,13 @@ async def add_task_log(task_id: int, content: str) -> dict:
|
||||
without overwriting the task's main body. Each entry is stored separately
|
||||
and shown chronologically in the task view.
|
||||
|
||||
What you write here comes back from `get_task` as `work_log`, newest
|
||||
first, and is counted on every row of `list_tasks` and every step of
|
||||
`get_milestone` — so write for the session that opens this task next, not
|
||||
for a reader who already knows what you were doing. What that reader
|
||||
cannot get from the body or the diff is what you tried, what you ruled
|
||||
out, and where it actually stands.
|
||||
|
||||
The response shows the task's `systems` — or, if the task is an untagged
|
||||
project record, the `systems_hint` question: logging work IS working in
|
||||
some area, so answer it (update_task with system_ids, or create_system
|
||||
|
||||
@@ -53,7 +53,9 @@ from scribe.models.user import User # noqa: E402, F401
|
||||
from scribe.models.app_log import AppLog # noqa: E402, F401
|
||||
from scribe.models.password_reset import PasswordResetToken # noqa: E402, F401
|
||||
from scribe.models.invitation import InvitationToken # noqa: E402, F401
|
||||
from scribe.models.embedding import MilestoneEmbedding, NoteEmbedding, RuleEmbedding # noqa: E402, F401
|
||||
from scribe.models.embedding import ( # noqa: E402, F401
|
||||
MilestoneEmbedding, NoteEmbedding, RuleEmbedding, SystemEmbedding,
|
||||
)
|
||||
from scribe.models.retrieval_log import RetrievalLog # noqa: E402, F401
|
||||
from scribe.models.retrieval_tuning import RetrievalTuningEvent # noqa: E402, F401
|
||||
from scribe.models.note_usage import NoteUsageEvent # noqa: E402, F401
|
||||
|
||||
@@ -128,3 +128,42 @@ class MilestoneEmbedding(Base):
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
|
||||
class SystemEmbedding(Base):
|
||||
"""One embedding vector per CHUNK of a System's charter (#4251).
|
||||
|
||||
The fourth sibling, for the reason note 3163 gives about the third: the row
|
||||
could be shared, the search cannot. A System's `description` is a charter —
|
||||
several hundred words saying what belongs in that area and what does not —
|
||||
and it is the answer to "which part of this codebase does X live in".
|
||||
Before this there was no semantic path to one: `list_systems` enumerates,
|
||||
and `search(system_id=…)` uses a System as a FILTER over notes. So a System
|
||||
could narrow a search and could never be the answer to one.
|
||||
|
||||
"Where does this belong?" is a different question from "what prior art is
|
||||
there?", which is why this is its own search rather than a note_type: a
|
||||
charter competing with two thousand notes for the same top-k would be
|
||||
outranked by the records filed under it, and the right answer would be
|
||||
crowded out by its own contents.
|
||||
|
||||
The document is the name and the charter. Derived data: the startup
|
||||
backfill regenerates it, which is also how a chunker-version bump is
|
||||
handled.
|
||||
"""
|
||||
|
||||
__tablename__ = "system_embeddings"
|
||||
|
||||
system_id: Mapped[int] = mapped_column(
|
||||
Integer,
|
||||
ForeignKey("systems.id", ondelete="CASCADE"),
|
||||
primary_key=True,
|
||||
)
|
||||
chunk_index: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||
embedding: Mapped[list] = mapped_column(Vector(EMBEDDING_DIM), nullable=False)
|
||||
chunk_text: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
chunker_version: Mapped[int] = mapped_column(Integer, nullable=False)
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
+13
-12
@@ -8,6 +8,7 @@ from scribe.services.embeddings import (
|
||||
INTERACTIVE_SEARCH_THRESHOLD as _REST_SEARCH_THRESHOLD,
|
||||
)
|
||||
from scribe.services.embeddings import semantic_search_notes
|
||||
from scribe.services.knowledge import content_type_filters
|
||||
from scribe.services.retrieval_telemetry import record_retrieval
|
||||
|
||||
# The interactive floor lives in embeddings.py now, shared with Browse search
|
||||
@@ -16,15 +17,6 @@ from scribe.services.retrieval_telemetry import record_retrieval
|
||||
search_bp = Blueprint("search", __name__, url_prefix="/api/search")
|
||||
|
||||
|
||||
def _content_type_to_is_task(content_type: str) -> bool | None:
|
||||
"""Map content_type query param to semantic_search_notes is_task arg."""
|
||||
if content_type == "note":
|
||||
return False
|
||||
if content_type == "task":
|
||||
return True
|
||||
return None # "all" or unknown → no filter
|
||||
|
||||
|
||||
@search_bp.route("", methods=["GET"])
|
||||
@login_required
|
||||
async def search_route():
|
||||
@@ -33,9 +25,18 @@ async def search_route():
|
||||
if not q:
|
||||
return jsonify({"error": "q is required"}), 400
|
||||
|
||||
content_type = request.args.get("content_type", "all")
|
||||
limit = min(request.args.get("limit", 10, type=int), 50)
|
||||
is_task = _content_type_to_is_task(content_type)
|
||||
# Every kind the facet table declares, derived rather than mapped here —
|
||||
# this route used to know exactly two and read anything else as "no
|
||||
# filter", so `?content_type=snippets` silently returned the whole corpus
|
||||
# (#4250). An unknown kind is now a 400 naming the ones that exist: a
|
||||
# result set is an answer, and it should not be able to answer a question
|
||||
# nobody asked.
|
||||
try:
|
||||
filters = content_type_filters(request.args.get("content_type", "all"))
|
||||
except ValueError as exc:
|
||||
return jsonify({"error": str(exc)}), 400
|
||||
is_task = filters.get("is_task")
|
||||
# Same association filters the MCP tool takes (#33). Optional, default
|
||||
# global: this route has NO frontend consumer today (measured 2026-08-08 —
|
||||
# the web UI searches through /api/knowledge), so it serves API callers,
|
||||
@@ -46,7 +47,7 @@ async def search_route():
|
||||
t0 = time.perf_counter()
|
||||
report: dict = {}
|
||||
results = await semantic_search_notes(
|
||||
uid, q, limit=limit, is_task=is_task, threshold=_REST_SEARCH_THRESHOLD,
|
||||
uid, q, limit=limit, **filters, threshold=_REST_SEARCH_THRESHOLD,
|
||||
project_id=project_id, system_id=system_id,
|
||||
# The user typed this, so it reaches everything they may read.
|
||||
scope="read",
|
||||
|
||||
@@ -123,7 +123,7 @@ _BACKED_UP = [
|
||||
|
||||
# Tables intentionally NOT in the backup, surfaced in the payload so the gap is
|
||||
# explicit rather than silent. ACL (groups/shares) is a coherent follow-up;
|
||||
# note_embeddings and rule_embeddings are derived (regenerated at startup
|
||||
# the four *_embeddings tables are derived (regenerated at startup
|
||||
# from the records themselves, which is also how a chunker bump is handled); api_keys are
|
||||
# sensitive credentials; retrieval_logs is observational telemetry that nothing
|
||||
# reads for correctness and that grows per query; the rest are
|
||||
@@ -135,6 +135,7 @@ _BACKED_UP = [
|
||||
_NOT_INCLUDED = [
|
||||
"groups", "group_memberships", "project_shares", "note_shares",
|
||||
"api_keys", "note_embeddings", "rule_embeddings", "milestone_embeddings",
|
||||
"system_embeddings",
|
||||
"app_logs", "notifications",
|
||||
"invitation_tokens", "password_reset_tokens", "user_profiles",
|
||||
"retrieval_logs",
|
||||
|
||||
@@ -23,11 +23,13 @@ from sqlalchemy import delete, or_, select
|
||||
from scribe.models import async_session
|
||||
from scribe.models.embedding import NoteEmbedding, RuleEmbedding
|
||||
from scribe.models.note import Note
|
||||
from scribe.models.task_log import TaskLog
|
||||
from scribe.services.access import can_read_project, notes_visibility_clause
|
||||
|
||||
if TYPE_CHECKING: # resolves forward refs without importing at runtime
|
||||
from scribe.models.milestone import Milestone
|
||||
from scribe.models.rulebook import Rule
|
||||
from scribe.models.system import System
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -271,11 +273,20 @@ def untrigger_title(title: str | None, trigger: str | None) -> str:
|
||||
# search. The fix is the document shape: one vector per meaningful chunk, and a
|
||||
# record is as findable as its best-matching section.
|
||||
|
||||
# Bumped whenever chunk_document's output can change for the same input. Stored
|
||||
# on every note_embeddings row so the startup backfill can re-embed exactly the
|
||||
# notes whose stored shape is stale — a version comparison instead of the table
|
||||
# wipe migrations 0067/0077 had to do.
|
||||
CHUNKER_VERSION = 1
|
||||
# Bumped whenever THE DOCUMENT A RECORD IS EMBEDDED AS can change for a record
|
||||
# that itself has not changed. Stored on every note_embeddings row so the
|
||||
# startup backfill re-embeds exactly the stale notes — a version comparison
|
||||
# instead of the table wipe migrations 0067/0077 had to do.
|
||||
#
|
||||
# Stated that way rather than as "chunk_document's output for the same input",
|
||||
# which is what it used to say: `chunk_document` is only the last step, and
|
||||
# version 2 moves without touching it. A task's document now carries its work
|
||||
# logs (#4251), so every task that has one embeds differently than it did while
|
||||
# its title, body and the chunker are all untouched — exactly the case the
|
||||
# narrower wording would have read as "nothing to re-embed".
|
||||
#
|
||||
# 1 → 2: work logs joined the task document.
|
||||
CHUNKER_VERSION = 2
|
||||
|
||||
|
||||
# The public name of the space every score lives in, and the two facts that
|
||||
@@ -483,6 +494,47 @@ async def _claim_parent_row(session, id_column, row_id: int, label: str) -> bool
|
||||
return True
|
||||
|
||||
|
||||
async def _work_log_sections(note_id: int) -> list[tuple[object, str | None]]:
|
||||
"""A task's work logs, oldest first, for its embedded document (#4251).
|
||||
|
||||
Reads the table directly rather than through `task_logs.logs_for_task`,
|
||||
because that function asks a PERMISSION question — may this user read this
|
||||
task — and there is no user here. An index build acts for the record, and
|
||||
the record's vectors carry the owner's `user_id`, so the access decision is
|
||||
made once at search time by the clause that already scopes every hit.
|
||||
|
||||
That also settles what happens on a shared task: a collaborator's log is
|
||||
part of the task's document, so it becomes findable by everyone who can
|
||||
read the task and by nobody else. The same answer `logs_for_task` gives a
|
||||
reader (#4241), which is the point — a log that can be read and not found
|
||||
is the half-surface that issue was about.
|
||||
|
||||
Asked for every note, not only tasks. `upsert_note_embedding` is handed a
|
||||
note_id and no kind — and the three writers that call it would each have to
|
||||
learn to pass one — so a non-task simply has no rows and gets []. One
|
||||
indexed lookup beside an ONNX forward pass over every chunk is not the
|
||||
expense worth adding a parameter to three call sites for.
|
||||
|
||||
Returns [] on any failure. A task whose logs could not be read should embed
|
||||
as its own prose rather than not embed at all: less findable is recoverable
|
||||
at the next write, unindexed is not.
|
||||
"""
|
||||
try:
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(TaskLog.created_at, TaskLog.content)
|
||||
.where(TaskLog.task_id == note_id)
|
||||
.order_by(TaskLog.created_at.asc(), TaskLog.id.asc())
|
||||
)
|
||||
return list(result.all())
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Could not read work logs for note %d; embedding its own prose only",
|
||||
note_id, exc_info=True,
|
||||
)
|
||||
return []
|
||||
|
||||
|
||||
async def upsert_note_embedding(
|
||||
note_id: int, user_id: int, title: str | None, body: str | None
|
||||
) -> None:
|
||||
@@ -496,6 +548,7 @@ async def upsert_note_embedding(
|
||||
inserted in one transaction, so a concurrent read sees the old shape or the
|
||||
new one, never a mixture.
|
||||
"""
|
||||
title, body = task_document(title, body, await _work_log_sections(note_id))
|
||||
chunks = chunk_document(title, body)
|
||||
try:
|
||||
if not chunks:
|
||||
@@ -577,6 +630,30 @@ GLOBAL_NOTE_TYPES: tuple[str, ...] = ("lesson",)
|
||||
# write telemetry pass a dict and read `best_available_score` back out of it.
|
||||
|
||||
|
||||
def record_best_chunk(report: dict | None, chunks: dict[int, dict]) -> None:
|
||||
"""Publish the winning chunk per record into `report["best_chunk"]`.
|
||||
|
||||
Every semantic search here collapses several chunk rows to the best one per
|
||||
record, which means each of them KNOWS which passage earned the hit — and
|
||||
each of them used to drop it, leaving every caller to preview the head of
|
||||
the document instead. The head is a different span, one the search has
|
||||
already scored lower, and nothing in the result said so (#4243).
|
||||
|
||||
It rides in `report` rather than in the return value because all three
|
||||
searches return `list[tuple[float, Record]]` and roughly thirty sites
|
||||
unpack that pair; widening it would be an interface change to every one of
|
||||
them with nothing to catch a miss (lesson #4207). `report` is already the
|
||||
side-channel these functions use for `searched` and `best_available_score`,
|
||||
so this adds a key to a channel callers already open.
|
||||
|
||||
Shape: {record_id: {"index": int, "text": str}}. A caller that passed no
|
||||
report simply doesn't get it, and every consumer falls back to the body.
|
||||
"""
|
||||
if report is None:
|
||||
return
|
||||
report["best_chunk"] = chunks
|
||||
|
||||
|
||||
async def semantic_search_notes(
|
||||
user_id: int,
|
||||
query: str,
|
||||
@@ -686,7 +763,16 @@ async def semantic_search_notes(
|
||||
# to the note's owner, so filtering it would pin every scope to "own"
|
||||
# and leave shared records unreachable by meaning.
|
||||
stmt = (
|
||||
select(Note, distance.label("distance"))
|
||||
# chunk_index/chunk_text ride along so the collapse below can
|
||||
# say WHICH passage matched. Without them the caller is left
|
||||
# previewing the head of the body — a span this query has
|
||||
# already determined is not why the record ranked (#4243).
|
||||
select(
|
||||
Note,
|
||||
distance.label("distance"),
|
||||
NoteEmbedding.chunk_index,
|
||||
NoteEmbedding.chunk_text,
|
||||
)
|
||||
.select_from(NoteEmbedding)
|
||||
.join(Note, NoteEmbedding.note_id == Note.id)
|
||||
.where(
|
||||
@@ -774,11 +860,23 @@ async def semantic_search_notes(
|
||||
# Recover similarity (1 - distance); order stays highest-first.
|
||||
scored: list[tuple[float, Note]] = []
|
||||
seen: set[int] = set()
|
||||
for note, dist in rows:
|
||||
# The winning row IS the best chunk, by the ordering above — so this is the
|
||||
# one place that knows which passage earned the hit. Kept beside the score
|
||||
# rather than returned with it: the return type is list[tuple[float, Note]]
|
||||
# and ten callers unpack it at ~18 sites, so widening the tuple would be an
|
||||
# interface change to every one of them with nothing to catch the misses
|
||||
# (lesson #4207). `report` is the side-channel this function already uses
|
||||
# for best_available_score.
|
||||
best_chunk: dict[int, dict] = {}
|
||||
for note, dist, chunk_index, chunk_text in rows:
|
||||
if int(note.id) in seen:
|
||||
continue
|
||||
seen.add(int(note.id))
|
||||
scored.append((1.0 - float(dist), note))
|
||||
best_chunk[int(note.id)] = {
|
||||
"index": int(chunk_index),
|
||||
"text": chunk_text or "",
|
||||
}
|
||||
# The best score anything reached, bar or no bar. Recorded BEFORE the
|
||||
# filter because a call that returns nothing is exactly when it matters.
|
||||
if report is not None:
|
||||
@@ -797,8 +895,17 @@ async def semantic_search_notes(
|
||||
report["best_available_id"] = int(best[1].id) if best else None
|
||||
scored = [pair for pair in scored if pair[0] >= threshold]
|
||||
if not demote_superseded:
|
||||
return scored[:limit]
|
||||
return await _apply_supersession_penalty(scored, limit)
|
||||
final = scored[:limit]
|
||||
else:
|
||||
final = await _apply_supersession_penalty(scored, limit)
|
||||
# Only for what actually came back, so a caller can key straight off the
|
||||
# results without carrying chunks for records it never saw.
|
||||
record_best_chunk(report, {
|
||||
int(n.id): best_chunk[int(n.id)]
|
||||
for _s, n in final
|
||||
if int(n.id) in best_chunk
|
||||
})
|
||||
return final
|
||||
|
||||
|
||||
async def backfill_note_embeddings() -> None:
|
||||
@@ -854,6 +961,71 @@ async def backfill_note_embeddings() -> None:
|
||||
|
||||
# ── Rules (milestone 307, note 3026) ────────────────────────────────────
|
||||
|
||||
# The heading a work log gets inside its task's embedded document. A CONSTANT
|
||||
# because it is load-bearing twice over: `_split_sections` splits on it, so it
|
||||
# is what keeps a log from being merged into the task's own prose, and it is
|
||||
# what a reader sees at the top of a matched passage — "this is a log entry,
|
||||
# not the task's description". Changing it changes the chunk boundaries of
|
||||
# every task that has one, which is a CHUNKER_VERSION move.
|
||||
WORK_LOG_HEADING = "## Work log"
|
||||
|
||||
|
||||
def task_document(
|
||||
title: str | None,
|
||||
body: str | None,
|
||||
logs: "Sequence[tuple[object, str | None]]" = (),
|
||||
) -> tuple[str | None, str | None]:
|
||||
"""The (title, body) a TASK is EMBEDDED as — its prose plus its work logs.
|
||||
|
||||
A synthesised embed-time shape, like `rule_document` and unlike a lesson:
|
||||
the stored record is the task row, and the logs live in their own table, so
|
||||
the document that should be searchable exists nowhere until it is built
|
||||
here (#4251).
|
||||
|
||||
WHY THE LOGS BELONG IN THE TASK'S DOCUMENT rather than in rows of their
|
||||
own. "Has anyone tried this before?" is answered by a log and asked of a
|
||||
task — a hit on a bare log would have to be resolved back to its task to be
|
||||
worth anything, so the useful result is the task either way. The objection
|
||||
to folding them in is that a long log drowns a short title, and that was
|
||||
true before #280: one vector per record meant a 2,000-word log averaged the
|
||||
task's own subject away, and everything past ~400 words was truncated
|
||||
unread. Chunking removed both. Each log becomes its own section, each
|
||||
section its own title-anchored vector, each scored separately — so a task
|
||||
is as findable as its best-matching log, and the task's own prose keeps the
|
||||
chunk it always had.
|
||||
|
||||
That the result is LEGIBLE is the other half, and it is this session's
|
||||
other build: a search now hands back the chunk that won (#4243), so a hit
|
||||
earned by a log shows that log's passage under the task's title. Without it
|
||||
the reader would get `body[:240]` of the task — the opening of a record
|
||||
whose relevance lives three hundred lines further down.
|
||||
|
||||
Ordering is oldest-first, matching how the web renders the narrative. Only
|
||||
the heading date distinguishes the sections, so it is part of the shape:
|
||||
"when was this tried" is half of what a log answers.
|
||||
|
||||
An entry with no content is skipped rather than emitted as a bare heading —
|
||||
an empty section is a vector with nothing in it but the task's title, which
|
||||
competes with the task's real chunk and says nothing.
|
||||
"""
|
||||
sections = []
|
||||
for created_at, content in logs:
|
||||
text = (content or "").strip()
|
||||
if not text:
|
||||
continue
|
||||
stamp = getattr(created_at, "date", None)
|
||||
heading = (
|
||||
f"{WORK_LOG_HEADING} — {stamp()}" if callable(stamp)
|
||||
else WORK_LOG_HEADING
|
||||
)
|
||||
sections.append(f"{heading}\n\n{text}")
|
||||
if not sections:
|
||||
return title, body
|
||||
prose = (body or "").strip()
|
||||
joined = "\n\n".join(sections)
|
||||
return title, f"{prose}\n\n{joined}" if prose else joined
|
||||
|
||||
|
||||
def rule_document(
|
||||
title: str | None, statement: str | None, when_to_apply: str | None,
|
||||
) -> tuple[str | None, str | None]:
|
||||
@@ -1044,7 +1216,12 @@ async def semantic_search_rules(
|
||||
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(Rule, distance.label("distance"))
|
||||
select(
|
||||
Rule,
|
||||
distance.label("distance"),
|
||||
RuleEmbedding.chunk_index,
|
||||
RuleEmbedding.chunk_text,
|
||||
)
|
||||
.select_from(RuleEmbedding)
|
||||
.join(Rule, RuleEmbedding.rule_id == Rule.id)
|
||||
.outerjoin(RulebookTopic, Rule.topic_id == RulebookTopic.id)
|
||||
@@ -1067,11 +1244,22 @@ async def semantic_search_rules(
|
||||
return []
|
||||
|
||||
best: dict[int, tuple[float, object]] = {}
|
||||
for rule, dist in rows:
|
||||
# Which chunk won, kept beside the score it won with — a rule's `why` and
|
||||
# `how_to_apply` are long, and a caller shown only the head cannot see the
|
||||
# clause that actually matched (#4243).
|
||||
won: dict[int, dict] = {}
|
||||
for rule, dist, chunk_index, chunk_text in rows:
|
||||
score = 1.0 - float(dist)
|
||||
if rule.id not in best or score > best[rule.id][0]:
|
||||
best[rule.id] = (score, rule)
|
||||
won[int(rule.id)] = {
|
||||
"index": int(chunk_index), "text": chunk_text or "",
|
||||
}
|
||||
ranked = sorted(best.values(), key=lambda pair: pair[0], reverse=True)
|
||||
kept = [pair for pair in ranked if pair[0] >= threshold][:limit]
|
||||
record_best_chunk(report, {
|
||||
int(r.id): won[int(r.id)] for _s, r in kept if int(r.id) in won
|
||||
})
|
||||
if report is not None:
|
||||
# See the sibling search: absent means the search never ran (#3765).
|
||||
report["searched"] = True
|
||||
@@ -1079,7 +1267,7 @@ async def semantic_search_rules(
|
||||
best = ranked[0] if ranked else None
|
||||
report["best_available_score"] = best[0] if best else None
|
||||
report["best_available_id"] = int(best[1].id) if best else None
|
||||
return [pair for pair in ranked if pair[0] >= threshold][:limit]
|
||||
return kept
|
||||
|
||||
|
||||
async def backfill_rule_embeddings() -> None:
|
||||
@@ -1187,6 +1375,7 @@ async def semantic_search_milestones(
|
||||
status: str | None = None,
|
||||
limit: int = 5,
|
||||
threshold: float = _SIMILARITY_THRESHOLD,
|
||||
report: dict | None = None,
|
||||
) -> list[tuple[float, "Milestone"]]:
|
||||
"""Return up to *limit* (score, milestone) pairs most like *query*.
|
||||
|
||||
@@ -1223,7 +1412,12 @@ async def semantic_search_milestones(
|
||||
scope = Milestone.user_id == user_id
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(Milestone, distance.label("distance"))
|
||||
select(
|
||||
Milestone,
|
||||
distance.label("distance"),
|
||||
MilestoneEmbedding.chunk_index,
|
||||
MilestoneEmbedding.chunk_text,
|
||||
)
|
||||
.select_from(MilestoneEmbedding)
|
||||
.join(Milestone, MilestoneEmbedding.milestone_id == Milestone.id)
|
||||
.where(
|
||||
@@ -1239,12 +1433,228 @@ async def semantic_search_milestones(
|
||||
return []
|
||||
|
||||
best: dict[int, tuple[float, object]] = {}
|
||||
for milestone, dist in rows:
|
||||
# A milestone's `body` IS the plan, and search results show its short
|
||||
# `description` — so a match on the design was previewed by a sentence that
|
||||
# need not mention it. The winning chunk is what the caller should see.
|
||||
won: dict[int, dict] = {}
|
||||
for milestone, dist, chunk_index, chunk_text in rows:
|
||||
score = 1.0 - float(dist)
|
||||
if milestone.id not in best or score > best[milestone.id][0]:
|
||||
best[milestone.id] = (score, milestone)
|
||||
won[int(milestone.id)] = {
|
||||
"index": int(chunk_index), "text": chunk_text or "",
|
||||
}
|
||||
ranked = sorted(best.values(), key=lambda pair: pair[0], reverse=True)
|
||||
return [pair for pair in ranked if pair[0] >= threshold][:limit]
|
||||
kept = [pair for pair in ranked if pair[0] >= threshold][:limit]
|
||||
record_best_chunk(report, {
|
||||
int(m.id): won[int(m.id)] for _s, m in kept if int(m.id) in won
|
||||
})
|
||||
return kept
|
||||
|
||||
|
||||
# --- Systems: the charter as an ANSWER, not a filter (#4251) -----------------
|
||||
|
||||
|
||||
def system_document(
|
||||
name: str | None, description: str | None
|
||||
) -> tuple[str | None, str | None]:
|
||||
"""The (title, body) a System is EMBEDDED as — its name and its charter.
|
||||
|
||||
The plainest of the four document shapes, and deliberately so. A System's
|
||||
`description` is already written as the thing this search has to match: it
|
||||
says what belongs in the area and what does not, which is the answer to
|
||||
"where does this go?" in the words someone asking would use. There is no
|
||||
trigger to synthesise, as `rule_document` must, and no separate record to
|
||||
gather in, as `task_document` must — the stored charter IS the sharp
|
||||
document, the way a snippet's is.
|
||||
|
||||
`color`, `status` and `order_index` are left out. They are presentation and
|
||||
bookkeeping; a vector that carried them would be answering a question
|
||||
nobody asks of a charter.
|
||||
|
||||
A System with no charter yet degrades to its name. It still embeds, just
|
||||
weakly — a bare name is exactly what the model docstring says is never
|
||||
enough, and this is an argument for writing the charter, not for padding
|
||||
the document with whatever is to hand.
|
||||
"""
|
||||
return (name or "").strip() or None, (description or "").strip() or None
|
||||
|
||||
|
||||
async def upsert_system_embedding(
|
||||
system_id: int, name: str | None, description: str | None
|
||||
) -> None:
|
||||
"""Chunk, embed and persist a System's vectors. Safe to fire-and-forget.
|
||||
|
||||
The third sibling's contract exactly: one document definition shared by the
|
||||
write path and the backfill, and an atomic per-System replacement guarded
|
||||
by the parent-row claim (#3262), so a System deleted mid-refresh wins.
|
||||
"""
|
||||
from scribe.models.embedding import SystemEmbedding
|
||||
from scribe.models.system import System
|
||||
|
||||
doc_title, doc_body = system_document(name, description)
|
||||
chunks = chunk_document(doc_title, doc_body)
|
||||
try:
|
||||
if not chunks:
|
||||
async with async_session() as session:
|
||||
await session.execute(
|
||||
delete(SystemEmbedding).where(SystemEmbedding.system_id == system_id)
|
||||
)
|
||||
await session.commit()
|
||||
return
|
||||
except Exception:
|
||||
logger.warning("Failed to clear embedding for system %d", system_id, exc_info=True)
|
||||
return
|
||||
|
||||
try:
|
||||
vectors = await get_embeddings(chunks)
|
||||
except Exception:
|
||||
logger.debug("Skipping embedding for system %d — embedder unavailable", system_id)
|
||||
return
|
||||
|
||||
try:
|
||||
async with async_session() as session:
|
||||
if not await _claim_parent_row(session, System.id, system_id, "system"):
|
||||
return
|
||||
await session.execute(
|
||||
delete(SystemEmbedding).where(SystemEmbedding.system_id == system_id)
|
||||
)
|
||||
for index, (chunk, vector) in enumerate(zip(chunks, vectors)):
|
||||
session.add(SystemEmbedding(
|
||||
system_id=system_id, chunk_index=index, embedding=vector,
|
||||
chunk_text=chunk, chunker_version=CHUNKER_VERSION,
|
||||
))
|
||||
await session.commit()
|
||||
except Exception:
|
||||
logger.warning("Failed to persist embedding for system %d", system_id, exc_info=True)
|
||||
|
||||
|
||||
async def semantic_search_systems(
|
||||
user_id: int,
|
||||
query: str,
|
||||
*,
|
||||
project_id: int | None = None,
|
||||
limit: int = 5,
|
||||
threshold: float = _SIMILARITY_THRESHOLD,
|
||||
report: dict | None = None,
|
||||
) -> list[tuple[float, "System"]]:
|
||||
"""Return up to *limit* (score, system) pairs most like *query*.
|
||||
|
||||
Answers "where does this belong?" — the question asked before filing a
|
||||
record or opening a file, and the one that had no tool: `list_systems`
|
||||
enumerates and `system_id` filters, so a charter could narrow a search and
|
||||
could never be the answer to one.
|
||||
|
||||
ITS OWN SEARCH rather than a note kind, for what note 3163 says about
|
||||
milestones: a charter competing with the whole note corpus for one top-k
|
||||
would be outranked by the records filed under it, and the right answer
|
||||
would be crowded out by its own contents. The questions are different too —
|
||||
"where does this belong?" is not "what prior art is there?" — and a caller
|
||||
asking one should not have to read past answers to the other.
|
||||
|
||||
SCOPE. With `project_id`, that project's Systems, provided the caller can
|
||||
read the project (access.can_read_project, rule 78) — a collaborator on a
|
||||
shared project sees its areas, which is the point of a charter. Without
|
||||
one, the Systems the caller owns across their projects. Archived Systems
|
||||
are excluded: an archived area is one the operator has said is no longer
|
||||
where things go, and that is exactly the question being asked.
|
||||
|
||||
Collapses to best-chunk-per-System and publishes the winning chunk, like
|
||||
the sibling searches — from the start rather than retrofitted (#4243).
|
||||
Returns an empty list if the embedder is unavailable, the project is not
|
||||
readable, or on any error: a recall aid must never break the call it
|
||||
serves.
|
||||
"""
|
||||
from scribe.models.embedding import SystemEmbedding
|
||||
from scribe.models.system import System
|
||||
|
||||
if not query or not query.strip():
|
||||
return []
|
||||
try:
|
||||
query_vec = await get_embedding(query)
|
||||
except Exception:
|
||||
logger.debug("System search skipped — embedder unavailable")
|
||||
return []
|
||||
|
||||
distance = SystemEmbedding.embedding.cosine_distance(query_vec)
|
||||
try:
|
||||
if project_id:
|
||||
if not await can_read_project(user_id, project_id):
|
||||
return []
|
||||
scope = System.project_id == project_id
|
||||
else:
|
||||
scope = System.user_id == user_id
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(
|
||||
System,
|
||||
distance.label("distance"),
|
||||
SystemEmbedding.chunk_index,
|
||||
SystemEmbedding.chunk_text,
|
||||
)
|
||||
.select_from(SystemEmbedding)
|
||||
.join(System, SystemEmbedding.system_id == System.id)
|
||||
.where(
|
||||
scope,
|
||||
System.deleted_at.is_(None),
|
||||
System.status != "archived",
|
||||
)
|
||||
.order_by(distance)
|
||||
.limit(limit * _CHUNK_OVERFETCH)
|
||||
)).all()
|
||||
except Exception:
|
||||
logger.warning("System semantic search failed", exc_info=True)
|
||||
return []
|
||||
|
||||
best: dict[int, tuple[float, object]] = {}
|
||||
# A charter runs to several hundred words and a result shows its NAME — so
|
||||
# a match on the paragraph that actually decides where a record belongs
|
||||
# would be previewed by two words that cannot. The winning chunk is what
|
||||
# the caller should see (#4243).
|
||||
won: dict[int, dict] = {}
|
||||
for system, dist, chunk_index, chunk_text in rows:
|
||||
score = 1.0 - float(dist)
|
||||
if system.id not in best or score > best[system.id][0]:
|
||||
best[system.id] = (score, system)
|
||||
won[int(system.id)] = {
|
||||
"index": int(chunk_index), "text": chunk_text or "",
|
||||
}
|
||||
ranked = sorted(best.values(), key=lambda pair: pair[0], reverse=True)
|
||||
kept = [pair for pair in ranked if pair[0] >= threshold][:limit]
|
||||
record_best_chunk(report, {
|
||||
int(sys_.id): won[int(sys_.id)] for _s, sys_ in kept if int(sys_.id) in won
|
||||
})
|
||||
return kept
|
||||
|
||||
|
||||
async def backfill_system_embeddings() -> None:
|
||||
"""Embed Systems that have no current vectors. Runs at startup beside the
|
||||
note, rule and milestone backfills; a CHUNKER_VERSION bump re-embeds.
|
||||
|
||||
This is also the pass that makes every existing charter findable at all —
|
||||
Systems got vectors in #4251, so before it runs there are none."""
|
||||
from scribe.models.embedding import SystemEmbedding
|
||||
from scribe.models.system import System
|
||||
|
||||
try:
|
||||
async with async_session() as session:
|
||||
current = select(SystemEmbedding.system_id).where(
|
||||
SystemEmbedding.chunker_version == CHUNKER_VERSION
|
||||
)
|
||||
stale = (await session.execute(
|
||||
select(System.id, System.name, System.description)
|
||||
.where(System.deleted_at.is_(None), System.id.notin_(current))
|
||||
)).all()
|
||||
except Exception:
|
||||
logger.warning("System embedding backfill: failed to query systems", exc_info=True)
|
||||
return
|
||||
|
||||
if not stale:
|
||||
logger.info("System embedding backfill: all systems current at chunker v%d", CHUNKER_VERSION)
|
||||
return
|
||||
logger.info("System embedding backfill: embedding %d system(s)", len(stale))
|
||||
for system_id, name, description in stale:
|
||||
await upsert_system_embedding(system_id, name, description)
|
||||
|
||||
|
||||
async def backfill_milestone_embeddings() -> None:
|
||||
|
||||
@@ -25,6 +25,7 @@ from scribe.models import async_session
|
||||
from scribe.models.note import Note
|
||||
from scribe.models.base import iso
|
||||
from scribe.services.access import browsable_notes_clause, readable_notes_clause
|
||||
from scribe.services.text import excerpt_fields
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -211,12 +212,30 @@ def _verification_clause(value: str):
|
||||
)
|
||||
|
||||
|
||||
def _note_to_item(note: Note) -> dict:
|
||||
def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict:
|
||||
"""One row for a browse or search listing.
|
||||
|
||||
`chunks` is `report["best_chunk"]` from a semantic search, when this row
|
||||
came from one. With it the card shows the passage that MATCHED; without it
|
||||
— a plain listing, where nothing was matched and so no span is better than
|
||||
any other — it shows the record's opening. `snippet_is` says which, on
|
||||
every row, so the two never have to be told apart by guessing.
|
||||
|
||||
This was `(note.body or "")[:_SNIPPET_LEN]` on every row alike: the head of
|
||||
the document, on the human's main search surface, with nothing marking the
|
||||
cut. A record matched on its sixth paragraph was shown its first, which the
|
||||
search had already scored lower (#4243).
|
||||
"""
|
||||
item: dict = {
|
||||
"id": note.id,
|
||||
"note_type": note.note_type or "note",
|
||||
"title": note.title,
|
||||
"snippet": (note.body or "")[:_SNIPPET_LEN],
|
||||
**excerpt_fields(
|
||||
note.body or "",
|
||||
(chunks or {}).get(int(note.id)),
|
||||
_SNIPPET_LEN,
|
||||
key="snippet",
|
||||
),
|
||||
"tags": note.tags or [],
|
||||
"project_id": note.project_id,
|
||||
# These lists now include records shared with the caller, so the client
|
||||
@@ -357,6 +376,69 @@ def matches_facet(note, note_type: str | None) -> bool:
|
||||
return not note.is_task and note.note_type == value
|
||||
|
||||
|
||||
def search_filters_for(facet: str) -> dict:
|
||||
"""The semantic-search kwargs one facet implies: is_task + note_type/task_kind.
|
||||
|
||||
A third dialect of the same table, for the arm that has no SQL statement to
|
||||
narrow and no fetched row to test — it is passing filters INTO
|
||||
`semantic_search_notes`. `_apply_type_filter` is the SQL dialect and
|
||||
`matches_facet` the Python one; all three read `_FACETS`, which is what
|
||||
keeps "adding a kind" a single edit (#3161).
|
||||
|
||||
Returns kwargs rather than a tuple so a caller splats it and cannot pair
|
||||
`task_kind` with `is_task=False` by writing the positions out of order.
|
||||
"""
|
||||
is_task, value = _facet(facet)
|
||||
if is_task:
|
||||
return {"is_task": True, "task_kind": value}
|
||||
return {"is_task": False, "note_type": value}
|
||||
|
||||
|
||||
# The two names a `content_type` parameter carries that are not facets. Both
|
||||
# search doors that take one — the MCP tool and /api/search — have always
|
||||
# spelled them this way, so they are the contract rather than a convenience:
|
||||
#
|
||||
# 'all' (and empty) — no kind filter at all.
|
||||
# 'note' — ANY non-task record, so snippets, lessons and processes
|
||||
# are all still in scope. The BROWSE facet of the same
|
||||
# name is narrower (`note_type == 'note'` exactly). The
|
||||
# two are deliberately different: narrowing this one
|
||||
# would stop returning snippets to every caller that
|
||||
# already asks this way, and the doors document the
|
||||
# containment instead.
|
||||
_CONTENT_TYPE_BROAD = {
|
||||
"": {"is_task": None},
|
||||
"all": {"is_task": None},
|
||||
"note": {"is_task": False},
|
||||
}
|
||||
|
||||
|
||||
def content_type_filters(content_type: str, extra: tuple = ()) -> dict:
|
||||
"""The search kwargs a door's `content_type` parameter implies.
|
||||
|
||||
Lives here, beside `_FACETS`, because a door that keeps its own map is how
|
||||
this went wrong: the agent's search offered two kinds and browse offered
|
||||
nine, and a third copy in `/api/search` offered two more quietly still
|
||||
(#4250). Derived, so adding a kind stays one edit (#3161).
|
||||
|
||||
Raises on anything unrecognised. `_facet` alone would fall back to reading
|
||||
an unknown string as a note_type, which matches no row — so a typo comes
|
||||
back as a confident empty result, and an empty result is a CLAIM that the
|
||||
corpus holds nothing of the sort. A door has to be able to say "that is not
|
||||
a kind" instead. `extra` names kinds the caller dispatches elsewhere (the
|
||||
MCP tool's 'rule' and 'milestone'), so the refusal lists what that door
|
||||
really accepts and not a vocabulary from some other door.
|
||||
"""
|
||||
if content_type in _CONTENT_TYPE_BROAD:
|
||||
return dict(_CONTENT_TYPE_BROAD[content_type])
|
||||
if content_type in FACET_TYPES:
|
||||
return search_filters_for(content_type)
|
||||
raise ValueError(
|
||||
f"unknown content_type {content_type!r}. Valid: "
|
||||
+ ", ".join(sorted({"all", *FACET_TYPES, *extra}))
|
||||
)
|
||||
|
||||
|
||||
def _apply_type_filter(stmt, note_type: str | None):
|
||||
"""Apply the type facet to a Note select. Trashed rows are always excluded."""
|
||||
stmt = stmt.where(Note.deleted_at.is_(None))
|
||||
@@ -507,6 +589,9 @@ async def _semantic_knowledge_search(
|
||||
# record would be findable by wording and invisible by meaning — which is the
|
||||
# case a semantic search exists to serve.
|
||||
semantic_notes: list[Note] = []
|
||||
# Filled by the search below; stays empty when the embedder is down or the
|
||||
# call raises, in which case every row falls back to its opening.
|
||||
_semantic_report: dict = {}
|
||||
try:
|
||||
from scribe.services.embeddings import (
|
||||
INTERACTIVE_SEARCH_THRESHOLD,
|
||||
@@ -519,6 +604,7 @@ async def _semantic_knowledge_search(
|
||||
user_id=user_id,
|
||||
scope="read",
|
||||
query=q,
|
||||
report=_semantic_report,
|
||||
limit=min(200, limit * 4),
|
||||
# The shared interactive floor — this was a bare `0.3` while
|
||||
# routes/search.py had the same number as a commented constant, the
|
||||
@@ -571,7 +657,10 @@ async def _semantic_knowledge_search(
|
||||
|
||||
total = len(merged)
|
||||
page_items = merged[offset: offset + limit]
|
||||
return [_note_to_item(n) for n in page_items], total
|
||||
return [
|
||||
_note_to_item(n, _semantic_report.get("best_chunk"))
|
||||
for n in page_items
|
||||
], total
|
||||
|
||||
|
||||
async def get_knowledge_tags(user_id: int, note_type: str | None = None) -> list[str]:
|
||||
|
||||
@@ -1096,7 +1096,20 @@ async def get_note_for_user(
|
||||
# A field most rows leave empty (a one-line description, a parent, a due date)
|
||||
# is attached only when set: a hundred rows of `null` are a hundred chances to
|
||||
# learn to skip the key (#2483), and the bytes are the thing being cut.
|
||||
def brief_row(note: Note, milestone_titles: dict[int, str] | None = None) -> dict:
|
||||
def brief_row(
|
||||
note: Note,
|
||||
milestone_titles: dict[int, str] | None = None,
|
||||
log_counts: dict[int, int] | None = None,
|
||||
) -> dict:
|
||||
"""One task as a list row — what it is and where it stands, not its body.
|
||||
|
||||
`log_counts` is a whole page's work-log counts fetched in one aggregate
|
||||
(task_logs.log_counts_for_tasks). Passed in rather than looked up here so
|
||||
the N+1 stays impossible, and applied to tasks only: a note has no work
|
||||
log. Zero-filled when the mapping is given, because a row that omits the
|
||||
key says "no record" on every row rather than on the ones with none, and
|
||||
knowing WHICH rows carry a record is how a reader decides what to open.
|
||||
"""
|
||||
row = {
|
||||
"id": note.id,
|
||||
"title": note.title,
|
||||
@@ -1120,4 +1133,6 @@ def brief_row(note: Note, milestone_titles: dict[int, str] | None = None) -> dic
|
||||
row["parent_id"] = note.parent_id
|
||||
if note.due_date:
|
||||
row["due_date"] = iso(note.due_date)
|
||||
if log_counts is not None:
|
||||
row["log_count"] = log_counts.get(int(note.id), 0)
|
||||
return row
|
||||
|
||||
@@ -40,12 +40,31 @@ from scribe.services.retrieval_surfaces import (
|
||||
)
|
||||
from scribe.services.retrieval_telemetry import record_retrieval
|
||||
from scribe.services.settings import get_setting
|
||||
from scribe.services.text import elide
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Defensive cap below Claude Code's 10k additionalContext limit.
|
||||
_MAX_CHARS = 9000
|
||||
|
||||
# Max chars of the matched passage shown under an injected menu line.
|
||||
#
|
||||
# The menu used to be titles alone, on the reasoning that its job is AWARENESS —
|
||||
# make the agent know the record exists and reach for it, not dump it. That
|
||||
# holds for a lesson or a snippet, whose title carries its trigger by
|
||||
# construction ("what — when it applies"). It does not hold for an issue, a
|
||||
# dev-log or a plain note, where the title is a headline and the reason this
|
||||
# record matched is a sentence somewhere inside it. The reader was being asked
|
||||
# "is this worth opening?" and handed the one part of the record guaranteed not
|
||||
# to answer it.
|
||||
#
|
||||
# 200 rather than more because this is a menu: eight lines at 200 is ~1.6KB,
|
||||
# which buys the decision without turning an awareness push into a dump. It is
|
||||
# the PASSAGE THAT MATCHED, not the record's opening — the search already knows
|
||||
# which one that is and used to throw it away (#4243) — so 200 characters here
|
||||
# are worth far more than 200 characters of preamble.
|
||||
_MENU_PASSAGE_CHARS = 200
|
||||
|
||||
# Max chars of a Process body to fold into the auto-surface description.
|
||||
_PROC_PREVIEW_CHARS = 200
|
||||
|
||||
@@ -1095,6 +1114,10 @@ async def build_autoinject_hint(
|
||||
# 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 {}
|
||||
|
||||
note_ids: list[int] = []
|
||||
for score, note in kept:
|
||||
note_ids.append(int(note.id))
|
||||
@@ -1113,6 +1136,16 @@ async def build_autoinject_hint(
|
||||
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, indented under it. 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 they are indented identically.
|
||||
passage = (menu_chunks.get(int(note.id)) or {}).get("text") or ""
|
||||
if passage.strip():
|
||||
short, _cut = elide(" ".join(passage.split()), _MENU_PASSAGE_CHARS)
|
||||
lines.append(f"> ↳ {short}")
|
||||
|
||||
# Records what SURVIVED the margin gate, not what the ranker returned — the
|
||||
# menu the agent actually saw. retrieval_logs already holds the full
|
||||
@@ -2035,6 +2068,11 @@ async def build_write_path_hint(
|
||||
# floor. Applying the floor after this would throw away the best queries.
|
||||
if query:
|
||||
query = concept_query(query) or query
|
||||
# Declared out here because the search below is conditional — this arm runs
|
||||
# only when the menu has room AND a query survived the floor. The render
|
||||
# loop is not conditional, so it needs something to read either way, and an
|
||||
# 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
|
||||
@@ -2089,6 +2127,7 @@ async def build_write_path_hint(
|
||||
scope="browse",
|
||||
report=_rep_wp,
|
||||
)
|
||||
wp_chunks = _rep_wp.get("best_chunk") or {}
|
||||
resembles = {
|
||||
int(note.id): float(score) for score, note in hits
|
||||
if int(note.id) in pulled
|
||||
@@ -2337,6 +2376,15 @@ async def build_write_path_hint(
|
||||
for item, marker, owner, foreign_lang in rendered:
|
||||
note_ids.append(int(item["id"]))
|
||||
lines.append(_prior_art_line(item, marker, owner, foreign_lang))
|
||||
# Only the semantically-matched lines carry a passage. The records-this-
|
||||
# file lines came from a LOCATION lookup — nothing was matched, so there
|
||||
# is no matching passage and the body's opening would be a fabricated
|
||||
# reason. Absence here is meaningful: a line with no passage under it is
|
||||
# one that earned its place by where it lives, not by what it says.
|
||||
passage = (wp_chunks.get(int(item["id"])) or {}).get("text") or ""
|
||||
if passage.strip():
|
||||
short, _cut = elide(" ".join(passage.split()), _MENU_PASSAGE_CHARS)
|
||||
lines.append(f"> ↳ {short}")
|
||||
|
||||
if stamped:
|
||||
lines.append(_stamp_line(path, stamped))
|
||||
|
||||
@@ -86,6 +86,29 @@ class Point:
|
||||
|
||||
quiet_because: str = ""
|
||||
|
||||
fixed_query: bool = False
|
||||
"""Whether this arm always searches the SAME query string.
|
||||
|
||||
THE #3497 GUARD, ONE STEP OVER. `logs_unconditionally` below exists
|
||||
because a warning computed over a LOGGING property read as a ranking
|
||||
problem. This field exists because a warning computed over a QUERY-SHAPE
|
||||
property does the same thing.
|
||||
|
||||
An arm with a fixed query scores against one constant. Its decline rate is
|
||||
therefore 0% or 100% and nothing in between — which of the two depends
|
||||
only on whether the bar sits below or above that single number. So "never
|
||||
returned nothing" says nothing at all about whether a floor is applied,
|
||||
and `cannot_decline` — whose whole remedy is "check that it applies its
|
||||
floor" — is uninformative here and skips these arms.
|
||||
|
||||
What IS informative for them is the mirror image, and `reply_preferences`
|
||||
names it in its own docstring: every call returning nothing means the bar
|
||||
sits above the constant, no traffic will ever move it, and the arm is
|
||||
dead. That has happened — 69 consecutive declines at 0.0006 under the bar
|
||||
(see `retrieval_surfaces`) — so it gets its own warning rather than
|
||||
inheriting one written for arms whose score can vary.
|
||||
"""
|
||||
|
||||
logs_unconditionally: bool = True
|
||||
"""Whether this arm writes a row even when it returns NOTHING.
|
||||
|
||||
@@ -114,8 +137,13 @@ 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"),
|
||||
# `fixed_query`: COMPLETION_QUERY is a module constant in
|
||||
# services/reply_preferences.py, so this arm's top score is the same number
|
||||
# on every call — measured at 0.791 across 45 consecutive calls, with p10,
|
||||
# p50, p90, min and max all identical. Five equal percentiles is the tell.
|
||||
_p("report_preference", UNBIDDEN,
|
||||
"the fixed question asked when a task finishes: how should this report read"),
|
||||
"the fixed question asked when a task finishes: how should this report read",
|
||||
fixed_query=True),
|
||||
|
||||
# ── Asked: a caller wanted a ranked list ─────────────────────────────
|
||||
_p("mcp_search", ASKED, "an agent called search"),
|
||||
|
||||
@@ -556,12 +556,22 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict,
|
||||
# arms once recorded only their hits, so their decline count was
|
||||
# structurally zero and this warning would have fired on a LOGGING
|
||||
# defect while pointing the reader at the threshold.
|
||||
#
|
||||
# FOUR NOW. A FIXED-QUERY arm is exempt for the same reason one step
|
||||
# over (#4232): it scores against one constant, so its decline rate is
|
||||
# 0% or 100% and never in between, and which one it is depends only on
|
||||
# where the bar sits relative to that single number. "Never returned
|
||||
# nothing" is then not evidence about the floor — it is arithmetic —
|
||||
# and this warning's own remedy, "check that it applies its floor",
|
||||
# cannot be answered from it. Those arms get `fixed_query_never_clears`
|
||||
# below, which asks the question that IS answerable for them.
|
||||
if (
|
||||
calls >= min_calls
|
||||
and (b.get("zero_result_calls") or 0) == 0
|
||||
and point is not None
|
||||
and point.kind == UNBIDDEN
|
||||
and point.logs_unconditionally
|
||||
and not point.fixed_query
|
||||
):
|
||||
out.append(_warn(
|
||||
"cannot_decline",
|
||||
@@ -572,6 +582,46 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict,
|
||||
source=name, calls=calls, zero_result_calls=0,
|
||||
))
|
||||
|
||||
# ── A fixed-query arm that never clears its bar ──────────────────
|
||||
#
|
||||
# The mirror image of `cannot_decline`, and the state that actually
|
||||
# threatens these arms. `reply_preferences` names it in its own
|
||||
# docstring: "a fixed query makes this arm's score a constant and a
|
||||
# floor a hair above it produces a dead arm no amount of traffic will
|
||||
# ever reveal."
|
||||
#
|
||||
# For an arm whose score can vary, a window of all-empty calls is
|
||||
# ordinary — it means nothing matched, which is an answer. For one
|
||||
# whose score is a constant it means the bar is above that constant,
|
||||
# and no volume of further calls will ever produce a different result.
|
||||
# The arm is not quiet; it is switched off, and nothing else in this
|
||||
# readout would say so.
|
||||
#
|
||||
# It has happened: `report_preference` logged 69 consecutive declines
|
||||
# at 0.0006 under the bar. Note what that incident also proves — the
|
||||
# fix is NOT automatically to lower the floor. Reading the refused
|
||||
# record showed the refusal was correct, so this warning sends the
|
||||
# reader to `near_miss_samples` rather than to the dial.
|
||||
if (
|
||||
calls >= min_calls
|
||||
and point is not None
|
||||
and point.fixed_query
|
||||
and point.logs_unconditionally
|
||||
and (b.get("zero_result_calls") or 0) == calls
|
||||
):
|
||||
out.append(_warn(
|
||||
"fixed_query_never_clears",
|
||||
f"{calls} calls, every one of them empty — and this arm always "
|
||||
f"searches the same query, so its score is a constant. That "
|
||||
f"means the bar sits above it and no amount of further traffic "
|
||||
f"will change the result: the arm is off, not quiet. Read the "
|
||||
f"record it refused (`near_miss_samples`) before touching the "
|
||||
f"floor — the last time this arm sat here, every percentile "
|
||||
f"said lower it and the refused record showed the refusal was "
|
||||
f"right.",
|
||||
source=name, calls=calls, zero_result_calls=calls,
|
||||
))
|
||||
|
||||
# ── Band hugs its floor ──────────────────────────────────────────
|
||||
#
|
||||
# Read on p10, the WEAKEST tenth of what the arm returned. If even
|
||||
|
||||
@@ -114,6 +114,38 @@ async def seed_standard_systems(user_id: int, project_id: int) -> list[System]:
|
||||
return out
|
||||
|
||||
|
||||
def embed_system(system) -> None:
|
||||
"""Refresh a System's charter vectors, fire-and-forget (#4251).
|
||||
|
||||
The twin of `notes.embed_note`, and here for the same reason (#2056): at
|
||||
the service, so every door gets it by construction rather than each route
|
||||
and tool remembering. A charter edited through one door and not another
|
||||
would stay findable by what it used to say, and nothing would report it.
|
||||
|
||||
Not called on delete. `delete_system` is a SOFT delete and the search joins
|
||||
through `System`, so a deleted System's vectors are already unreachable —
|
||||
and leaving them means a restore is findable again immediately instead of
|
||||
waiting for the next startup backfill.
|
||||
|
||||
Import is lazy so importing this module doesn't pull in the embedding
|
||||
model; a missing event loop (unit tests, scripts) is ordinary, not an
|
||||
error; exceptions are swallowed because a System that saved must not fail
|
||||
on its index refresh.
|
||||
"""
|
||||
try:
|
||||
import asyncio
|
||||
|
||||
from scribe.services.embeddings import upsert_system_embedding
|
||||
|
||||
asyncio.create_task(
|
||||
upsert_system_embedding(system.id, system.name, system.description)
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # no running loop — a sync caller, not a failure
|
||||
except Exception: # noqa: BLE001 - never let indexing break a write
|
||||
logger.exception("embedding refresh failed for system %s", system.id)
|
||||
|
||||
|
||||
async def create_system(
|
||||
user_id: int,
|
||||
project_id: int,
|
||||
@@ -144,6 +176,7 @@ async def create_system(
|
||||
session.add(system)
|
||||
await session.commit()
|
||||
await session.refresh(system)
|
||||
embed_system(system)
|
||||
return system
|
||||
|
||||
|
||||
@@ -195,6 +228,7 @@ async def update_system(user_id: int, system_id: int, **fields: object) -> Syste
|
||||
system.updated_at = datetime.now(timezone.utc)
|
||||
await session.commit()
|
||||
await session.refresh(system)
|
||||
embed_system(system)
|
||||
return system
|
||||
|
||||
|
||||
|
||||
@@ -2,17 +2,48 @@
|
||||
import logging
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy import func, select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.task_log import TaskLog
|
||||
from scribe.models.note import Note
|
||||
from scribe.services.access import can_read_note, readable_notes_clause
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_UNSET = object()
|
||||
|
||||
|
||||
async def _refresh_task_document(session, task_id: int) -> None:
|
||||
"""Re-embed the task whose work log just changed (#4251).
|
||||
|
||||
A task's embedded document carries its logs, so a log written and not
|
||||
indexed is #4241's half-surface wearing different clothes: the entry is
|
||||
readable and unfindable, and the next session rebuilds what this one ruled
|
||||
out. Create, edit and delete all go through here — an edited log that keeps
|
||||
matching its old wording is the stale-vector problem `upsert_note_embedding`
|
||||
already refuses to leave behind for a body.
|
||||
|
||||
Loads the NOTE rather than synthesising one. `embed_note` reads
|
||||
`title`, `body` and the OWNER's `user_id` off what it is handed, so a
|
||||
stand-in row would index the logs under an empty title — throwing away the
|
||||
per-chunk topical anchor that makes any of this discriminative — and file
|
||||
the vectors under the wrong user.
|
||||
|
||||
Failure is swallowed the way `embed_note`'s own is: a log that saved must
|
||||
not fail on its index refresh. The startup backfill is the backstop.
|
||||
"""
|
||||
from scribe.services.notes import embed_note
|
||||
|
||||
try:
|
||||
result = await session.execute(select(Note).where(Note.id == task_id))
|
||||
note = result.scalars().first()
|
||||
if note is not None:
|
||||
embed_note(note)
|
||||
except Exception: # noqa: BLE001 - indexing never breaks a write
|
||||
logger.exception("embedding refresh failed for task %s", task_id)
|
||||
|
||||
|
||||
async def create_log(
|
||||
user_id: int,
|
||||
task_id: int,
|
||||
@@ -35,6 +66,7 @@ async def create_log(
|
||||
session.add(log)
|
||||
await session.commit()
|
||||
await session.refresh(log)
|
||||
await _refresh_task_document(session, task_id)
|
||||
return log
|
||||
|
||||
|
||||
@@ -48,6 +80,81 @@ async def list_logs(user_id: int, task_id: int) -> list[TaskLog]:
|
||||
return list(result.scalars().all())
|
||||
|
||||
|
||||
async def logs_for_task(
|
||||
user_id: int, task_id: int, limit: int = 0
|
||||
) -> list[TaskLog]:
|
||||
"""Every work log on a task, NEWEST FIRST, whoever wrote it.
|
||||
|
||||
Two differences from `list_logs`, both deliberate:
|
||||
|
||||
1. Scoped by who may read the TASK, not by who wrote each entry. A work
|
||||
log belongs to the task, and on a shared task the owner's record is
|
||||
exactly what a collaborator needs — `list_logs`' `TaskLog.user_id ==
|
||||
user_id` returns them an empty list, which reads as "no work has been
|
||||
done" rather than "not yours". The permission question is asked of the
|
||||
note through `can_read_note` (rule #78), so this is not an unscoped
|
||||
read with the check left to the caller.
|
||||
2. Newest first, because the question a reader asks of a work log is
|
||||
"where does this actually stand", and the answer is the last entry.
|
||||
`list_logs` stays ascending — the web UI renders a narrative.
|
||||
|
||||
`limit` of 0 means all of them. An unreadable or missing task returns []
|
||||
the same way an empty one does: a work log is not a channel for proving a
|
||||
record exists.
|
||||
"""
|
||||
if not await can_read_note(user_id, task_id):
|
||||
return []
|
||||
async with async_session() as session:
|
||||
stmt = (
|
||||
select(TaskLog)
|
||||
.where(TaskLog.task_id == task_id)
|
||||
.order_by(TaskLog.created_at.desc(), TaskLog.id.desc())
|
||||
)
|
||||
if limit > 0:
|
||||
stmt = stmt.limit(limit)
|
||||
result = await session.execute(stmt)
|
||||
return list(result.scalars().all())
|
||||
|
||||
|
||||
async def count_logs_for_task(user_id: int, task_id: int) -> int:
|
||||
"""How many work logs a task carries. Same scoping as logs_for_task."""
|
||||
if not await can_read_note(user_id, task_id):
|
||||
return 0
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(func.count(TaskLog.id)).where(TaskLog.task_id == task_id)
|
||||
)
|
||||
return int(result.scalar() or 0)
|
||||
|
||||
|
||||
async def log_counts_for_tasks(
|
||||
user_id: int, task_ids: list[int]
|
||||
) -> dict[int, int]:
|
||||
"""Log counts for a whole page of tasks in ONE query.
|
||||
|
||||
A per-row lookup would be N+1 by construction — and so would a per-row
|
||||
`can_read_note` — which is the reason a list surface would otherwise keep
|
||||
omitting this and leave a reader to guess which rows carry a record. So
|
||||
the permission is expressed as set membership with
|
||||
`readable_notes_clause` and folded into the same statement, the pattern
|
||||
that function exists for.
|
||||
|
||||
Tasks with no logs are absent from the mapping; callers zero-fill, so
|
||||
"none" reads as a count rather than a missing key.
|
||||
"""
|
||||
ids = [int(t) for t in task_ids if t]
|
||||
if not ids:
|
||||
return {}
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(TaskLog.task_id, func.count(TaskLog.id))
|
||||
.join(Note, Note.id == TaskLog.task_id)
|
||||
.where(TaskLog.task_id.in_(ids), readable_notes_clause(user_id))
|
||||
.group_by(TaskLog.task_id)
|
||||
)
|
||||
return {int(tid): int(n) for tid, n in result.all()}
|
||||
|
||||
|
||||
async def update_log(
|
||||
user_id: int,
|
||||
log_id: int,
|
||||
@@ -68,6 +175,7 @@ async def update_log(
|
||||
log.updated_at = datetime.now(timezone.utc)
|
||||
await session.commit()
|
||||
await session.refresh(log)
|
||||
await _refresh_task_document(session, log.task_id)
|
||||
return log
|
||||
|
||||
|
||||
@@ -79,6 +187,8 @@ async def delete_log(user_id: int, log_id: int) -> bool:
|
||||
log = result.scalars().first()
|
||||
if log is None:
|
||||
return False
|
||||
task_id = log.task_id
|
||||
await session.delete(log)
|
||||
await session.commit()
|
||||
await _refresh_task_document(session, task_id)
|
||||
return True
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
"""Text shortening for surfaces that cannot show a record whole.
|
||||
|
||||
One function, in one place, because both doors shorten and two copies would
|
||||
drift — and because the reasoning below is the part that matters and should
|
||||
not have to be re-derived at each call site.
|
||||
"""
|
||||
|
||||
|
||||
def elide(text: str, budget: int) -> tuple[str, bool]:
|
||||
"""Cut to `budget` characters from the MIDDLE, keeping both ends.
|
||||
|
||||
A head-only cut — `text[:800]` — decides what a reader sees by character
|
||||
position, which is uncorrelated with what matters. Prose does not put its
|
||||
conclusion first: a passage that opens with what was attempted and closes
|
||||
with "so this shipped in 04775c3" loses exactly the sentence that answers
|
||||
the question. Worse, the reader cannot tell: a truncation marker says that
|
||||
something was removed, never whether it mattered, so the decision "should
|
||||
I look deeper?" gets made on evidence selected by length.
|
||||
|
||||
So keep the opening (what this is about) AND the closing (where it landed),
|
||||
and state in between how much went. Two thirds to the head because that is
|
||||
where the subject is established; a conclusion needs less room to carry.
|
||||
|
||||
Returns (text, was_cut). A `budget` of 0 or less means no cut.
|
||||
|
||||
This is the fallback, not the goal. Where the system knows WHICH span of a
|
||||
record is the relevant one — a semantic search knows exactly that, and
|
||||
stores it (#4243) — show that span and say so. Reach for this only when
|
||||
nothing identifies a better part than "all of it".
|
||||
"""
|
||||
if budget <= 0 or len(text) <= budget:
|
||||
return text, False
|
||||
head_len = max(1, budget * 2 // 3)
|
||||
tail_len = max(1, budget - head_len)
|
||||
omitted = len(text) - head_len - tail_len
|
||||
head = text[:head_len].rstrip()
|
||||
tail = text[-tail_len:].lstrip()
|
||||
return f"{head}\n\n[… {omitted} characters omitted …]\n\n{tail}", True
|
||||
|
||||
|
||||
# What a search result shows a reader, as one decision made in one place.
|
||||
#
|
||||
# Every door onto a semantic search faces the same question — which span of a
|
||||
# record do I show someone deciding whether to open it? — and each used to
|
||||
# answer it separately with a bare head cut of a different length: 240
|
||||
# characters in the MCP search, 200 in the web's knowledge search. Both showed
|
||||
# the document's OPENING, which is not the span that matched and not the span
|
||||
# the ranking was built on.
|
||||
#
|
||||
# Doors keep their own field names (the web UI renders `snippet`, the MCP
|
||||
# surface returns `excerpt`), because renaming a field a frontend consumes is
|
||||
# a separate change from fixing which text goes in it. What they share is this
|
||||
# function: the choice of span, and the obligation to say which span it is.
|
||||
|
||||
MATCHED_PASSAGE = "matched_passage"
|
||||
BODY_OPENING = "body_opening"
|
||||
|
||||
|
||||
def matched_excerpt(
|
||||
body: str, chunk: dict | None, budget: int
|
||||
) -> tuple[str, str, bool]:
|
||||
"""Pick the span to show, and say which span it is.
|
||||
|
||||
`chunk` is a `report["best_chunk"]` entry from one of the semantic
|
||||
searches — `{"index": int, "text": str}` — or None when the caller ran no
|
||||
semantic search, passed no report, or the record has no embedding row.
|
||||
|
||||
Returns `(text, kind, was_cut)` where `kind` is MATCHED_PASSAGE or
|
||||
BODY_OPENING. The kind is not decoration: a reader who cannot tell the
|
||||
passage that earned the hit from the first paragraph of the document
|
||||
cannot tell whether a thin-looking result is genuinely thin, and the whole
|
||||
point of the excerpt is to support exactly that judgement.
|
||||
|
||||
A short record comes back whole either way — fragmenting a 200-character
|
||||
note serves nobody, and its opening IS its content.
|
||||
"""
|
||||
passage = (chunk or {}).get("text") or ""
|
||||
if passage.strip():
|
||||
text, cut = elide(passage.strip(), budget)
|
||||
return text, MATCHED_PASSAGE, cut
|
||||
text, cut = elide(body or "", budget)
|
||||
return text, BODY_OPENING, cut
|
||||
|
||||
|
||||
def excerpt_fields(
|
||||
body: str, chunk: dict | None, budget: int, *, key: str = "excerpt"
|
||||
) -> dict:
|
||||
"""`matched_excerpt` as the block a result row carries.
|
||||
|
||||
`key` names the text field so a door can keep the name its consumers
|
||||
already read. The companion keys are derived from it, so a row never ends
|
||||
up with an excerpt under one name and its label under another.
|
||||
"""
|
||||
text, kind, cut = matched_excerpt(body, chunk, budget)
|
||||
out = {key: text, f"{key}_is": kind, "body_length": len(body or "")}
|
||||
if cut or (kind == MATCHED_PASSAGE and len(text) < len(body or "")):
|
||||
out["read_full"] = (
|
||||
"This is one span of a longer record. Open it by id for the whole "
|
||||
"thing rather than judging from what is here."
|
||||
)
|
||||
return out
|
||||
@@ -102,6 +102,32 @@ def _no_supersession():
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_task_log_arm():
|
||||
"""Stub the task-log read arm that get_task / list_tasks / get_milestone
|
||||
grew in #4241.
|
||||
|
||||
Autouse for the reason _no_rule_arm is: those three tools now read work
|
||||
logs, and the reads go through the access layer to Postgres. Every unit
|
||||
test that opens a task — and most of them do, because a task is what this
|
||||
codebase is mostly about — would otherwise try to reach the fake
|
||||
DATABASE_URL this file sets, to learn that a fake task has no logs.
|
||||
|
||||
The arm's own behaviour is covered where it belongs: the payload shape and
|
||||
the tool wiring in tests/test_task_work_log_surface.py, which re-patches
|
||||
these explicitly, and the ACL scoping against real Postgres in
|
||||
tests/test_integration_task_work_log.py. A test that wants the arm live
|
||||
re-patches it, same as the rules arm.
|
||||
"""
|
||||
with patch("scribe.services.task_logs.logs_for_task",
|
||||
AsyncMock(return_value=[])), \
|
||||
patch("scribe.services.task_logs.count_logs_for_task",
|
||||
AsyncMock(return_value=0)), \
|
||||
patch("scribe.services.task_logs.log_counts_for_tasks",
|
||||
AsyncMock(return_value={})):
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_rule_arm():
|
||||
"""Stub the write-path hint's standing-RULES arm (milestone 307).
|
||||
|
||||
@@ -89,6 +89,24 @@ def make_mock_session() -> AsyncMock:
|
||||
return s
|
||||
|
||||
|
||||
def session_returning(note):
|
||||
"""``make_mock_session`` whose every ``execute()`` yields `note` (or None).
|
||||
|
||||
The commonest shape above the bare session: a service that loads one
|
||||
record by id and acts on it. Two files had spelled this out identically
|
||||
before a third was about to (#3207 — derive before the third copy), and
|
||||
it belongs beside `make_mock_session` for the same reason that one does.
|
||||
|
||||
For a service making SEVERAL different reads, set
|
||||
``session.execute.side_effect`` to a list of results instead.
|
||||
"""
|
||||
session = make_mock_session()
|
||||
result = MagicMock()
|
||||
result.scalars.return_value.first.return_value = note
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
return session
|
||||
|
||||
|
||||
async def ensure_user(session, username: str, role: str = "user"):
|
||||
"""Get-or-create a User by username inside an open session (flushed, not
|
||||
committed).
|
||||
|
||||
+52
-4
@@ -148,32 +148,54 @@ async def test_search_collapses_chunk_rows_to_best_chunk_per_note():
|
||||
from scribe.services import embeddings as emb
|
||||
|
||||
note_a, note_b = MagicMock(id=1), MagicMock(id=2)
|
||||
rows = [(note_a, 0.10), (note_b, 0.20), (note_a, 0.25), (note_a, 0.30)]
|
||||
# Rows are (Note, distance, chunk_index, chunk_text) — the chunk columns
|
||||
# ride along so the collapse can report WHICH passage won (#4243).
|
||||
rows = [
|
||||
(note_a, 0.10, 3, "the passage that actually matched"),
|
||||
(note_b, 0.20, 0, "b's best"),
|
||||
(note_a, 0.25, 7, "a worse chunk of a"),
|
||||
(note_a, 0.30, 1, "a worse chunk of a"),
|
||||
]
|
||||
result = MagicMock()
|
||||
result.all.return_value = rows
|
||||
session, ctx = _session_ctx()
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
|
||||
report: dict = {}
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=ctx),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
):
|
||||
out = await emb.semantic_search_notes(
|
||||
1, "a query", limit=8, demote_superseded=False
|
||||
1, "a query", limit=8, demote_superseded=False, report=report
|
||||
)
|
||||
|
||||
assert [note.id for _s, note in out] == [1, 2]
|
||||
assert out[0][0] == 1.0 - 0.10 # the BEST chunk's score, not a later one
|
||||
|
||||
# And the winning chunk is reported, not merely used for scoring. Without
|
||||
# this a caller can only preview the head of the body — a span this query
|
||||
# has already ranked lower than the one that won (#4243).
|
||||
assert report["best_chunk"][1] == {
|
||||
"index": 3, "text": "the passage that actually matched",
|
||||
}
|
||||
assert report["best_chunk"][2]["index"] == 0
|
||||
|
||||
|
||||
# --- the write path: one row per chunk (#280 step 3) -------------------------
|
||||
|
||||
|
||||
def _session_ctx():
|
||||
def _session_ctx(log_rows=()):
|
||||
"""A session stand-in. `log_rows` answers the work-log read a task's
|
||||
document now needs (#4251) — answered explicitly rather than left to
|
||||
autovivify, because `list(result.all())` on a bare MagicMock raises and is
|
||||
swallowed, which would quietly make every one of these a no-log test."""
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
session = MagicMock()
|
||||
session.execute = AsyncMock()
|
||||
result = MagicMock()
|
||||
result.all.return_value = list(log_rows)
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
session.commit = AsyncMock()
|
||||
ctx = MagicMock()
|
||||
ctx.__aenter__ = AsyncMock(return_value=session)
|
||||
@@ -210,6 +232,32 @@ async def test_upsert_stores_one_versioned_row_per_chunk():
|
||||
session.execute.assert_awaited() # the delete that makes replacement atomic
|
||||
|
||||
|
||||
async def test_a_tasks_work_logs_reach_the_rows_it_is_stored_as():
|
||||
"""The wiring half of #4251: the shaper is pure and tested next door, so
|
||||
what is pinned here is that the WRITER actually asks for the logs and
|
||||
embeds what comes back. A log that is written, readable (#4241) and absent
|
||||
from the index is the same half-surface one layer down."""
|
||||
import datetime
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
from scribe.services import embeddings as emb
|
||||
|
||||
session, ctx = _session_ctx(
|
||||
log_rows=[(datetime.datetime(2026, 9, 20), "ruled out the cache theory")]
|
||||
)
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=ctx),
|
||||
patch.object(emb, "get_embeddings", AsyncMock(return_value=[[0.0] * 384])),
|
||||
):
|
||||
await emb.upsert_note_embedding(7, 42, "A task", "Its own prose.")
|
||||
|
||||
rows = [call.args[0] for call in session.add.call_args_list]
|
||||
stored = "\n".join(r.chunk_text for r in rows)
|
||||
assert "ruled out the cache theory" in stored
|
||||
assert "Its own prose." in stored
|
||||
assert emb.WORK_LOG_HEADING in stored
|
||||
|
||||
|
||||
async def test_upsert_of_an_emptied_record_clears_rows_instead_of_embedding():
|
||||
"""An empty embedding is worse than none, and a STALE one is worse than
|
||||
that — a record emptied of content must stop being findable by what it no
|
||||
|
||||
@@ -32,6 +32,11 @@ def _mock_session(lock_result: object = 7, execute_side_effect=None):
|
||||
session = MagicMock()
|
||||
claimed = MagicMock()
|
||||
claimed.scalar_one_or_none.return_value = lock_result
|
||||
# A task's document carries its work logs (#4251), so the refresh reads
|
||||
# task_logs before it builds chunks. Answered explicitly — left to
|
||||
# autovivify, `list(result.all())` would raise and be swallowed, and these
|
||||
# tests would be exercising the failure path without saying so.
|
||||
claimed.all.return_value = []
|
||||
if execute_side_effect is not None:
|
||||
session.execute = AsyncMock(side_effect=execute_side_effect)
|
||||
else:
|
||||
@@ -65,10 +70,19 @@ async def test_a_note_refresh_claims_the_row_before_rewriting_its_vectors():
|
||||
):
|
||||
await emb.upsert_note_embedding(7, 42, "T", "a short body")
|
||||
|
||||
claim, replace = [c.args[0] for c in session.execute.call_args_list][:2]
|
||||
assert compiled_sql(claim, dialect=PG).startswith("SELECT notes.id")
|
||||
assert "FOR KEY SHARE NOWAIT" in compiled_sql(claim, dialect=PG)
|
||||
assert compiled_sql(replace, dialect=PG).startswith("DELETE FROM note_embeddings")
|
||||
sqls = [compiled_sql(c.args[0], dialect=PG) for c in session.execute.call_args_list]
|
||||
# Located by what they ARE rather than by position: the work-log read a
|
||||
# task's document needs (#4251) runs before any of this, and an index is
|
||||
# not what the claim is about.
|
||||
claim = next(i for i, sql in enumerate(sqls) if "FOR KEY SHARE NOWAIT" in sql)
|
||||
replace = next(
|
||||
i for i, sql in enumerate(sqls) if sql.startswith("DELETE FROM note_embeddings")
|
||||
)
|
||||
assert sqls[claim].startswith("SELECT notes.id")
|
||||
assert claim < replace, "the claim goes first — that is the whole fix"
|
||||
assert not any("note_embeddings" in sql for sql in sqls[:claim]), (
|
||||
"nothing may touch the chunk rows before the parent is claimed"
|
||||
)
|
||||
session.add.assert_called()
|
||||
|
||||
|
||||
@@ -113,6 +127,8 @@ async def test_a_record_already_gone_is_not_re_embedded():
|
||||
):
|
||||
await emb.upsert_note_embedding(7, 42, "T", "a short body")
|
||||
|
||||
assert session.execute.await_count == 1
|
||||
sqls = [compiled_sql(c.args[0], dialect=PG) for c in session.execute.call_args_list]
|
||||
assert any("FOR KEY SHARE NOWAIT" in sql for sql in sqls), "it did reach the claim"
|
||||
assert not any("note_embeddings" in sql for sql in sqls), "and stopped there"
|
||||
session.add.assert_not_called()
|
||||
session.commit.assert_not_awaited()
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
"""Every semantic search hands on the passage that matched (#4243, #4250).
|
||||
|
||||
Three searches collapse chunk rows to the best one per record, so each of them
|
||||
KNOWS which passage earned the hit. Each used to drop it, leaving every door to
|
||||
preview the head of the document instead — a span the search had already scored
|
||||
lower, with nothing saying so.
|
||||
|
||||
These pin the mechanism (`report["best_chunk"]` from all three searches) and
|
||||
each surface that reads it, because the failure mode is silent: a door that
|
||||
quietly reverts to the body's opening still returns a plausible-looking string
|
||||
and no test that only checks "a preview exists" would notice.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import embeddings as emb
|
||||
from scribe.services.text import (
|
||||
BODY_OPENING,
|
||||
MATCHED_PASSAGE,
|
||||
excerpt_fields,
|
||||
matched_excerpt,
|
||||
)
|
||||
from tests.helpers import make_mock_session
|
||||
|
||||
|
||||
def _searching(rows):
|
||||
"""A patched session whose one query returns `rows` — the shared
|
||||
make_mock_session (#2834) rather than a third local copy of the
|
||||
__aenter__/__aexit__ dance."""
|
||||
session = make_mock_session()
|
||||
result = MagicMock()
|
||||
result.all.return_value = rows
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
return session
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The shared choice of span
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_the_matched_passage_wins_over_the_opening():
|
||||
body = "An opening about nothing much. " * 20 + " THE ANSWER."
|
||||
text, kind, _cut = matched_excerpt(body, {"index": 4, "text": "THE ANSWER."}, 1000)
|
||||
assert text == "THE ANSWER."
|
||||
assert kind == MATCHED_PASSAGE
|
||||
|
||||
|
||||
def test_without_a_chunk_the_opening_is_named_as_the_opening():
|
||||
"""The fallback is legitimate — a plain listing matched nothing — but it
|
||||
must not pass for the passage that matched."""
|
||||
text, kind, _cut = matched_excerpt("just a body", None, 1000)
|
||||
assert kind == BODY_OPENING
|
||||
|
||||
|
||||
def test_an_empty_chunk_is_not_mistaken_for_a_passage():
|
||||
"""A record embedded from its title alone stores an empty body chunk;
|
||||
rendering that as "the passage that matched" would be a blank line
|
||||
presented as evidence."""
|
||||
_t, kind, _c = matched_excerpt("real body", {"index": 0, "text": " "}, 1000)
|
||||
assert kind == BODY_OPENING
|
||||
|
||||
|
||||
def test_the_field_names_travel_together():
|
||||
"""A door renames the text field to keep its consumers working; the label
|
||||
has to follow it, or a row carries an excerpt under one name and its
|
||||
meaning under another."""
|
||||
out = excerpt_fields("b" * 500, {"index": 1, "text": "hit"}, 100, key="snippet")
|
||||
assert out["snippet"] == "hit"
|
||||
assert out["snippet_is"] == MATCHED_PASSAGE
|
||||
assert out["body_length"] == 500
|
||||
assert "read_full" in out
|
||||
|
||||
|
||||
def test_a_record_shown_whole_advertises_nothing_further():
|
||||
out = excerpt_fields("short", None, 1000)
|
||||
assert out["excerpt"] == "short"
|
||||
assert "read_full" not in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# All three searches publish the winning chunk
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rule_search_reports_the_clause_that_matched():
|
||||
"""A rule's `why` and `how_to_apply` run long. A caller shown only the head
|
||||
cannot see the clause the query actually hit."""
|
||||
r1, r2 = MagicMock(id=1), MagicMock(id=2)
|
||||
session = _searching([
|
||||
(r1, 0.10, 2, "the clause that matched"),
|
||||
(r2, 0.20, 0, "r2 best"),
|
||||
(r1, 0.40, 9, "a worse clause of r1"),
|
||||
])
|
||||
report: dict = {}
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=session),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
patch.object(emb, "can_read_project", AsyncMock(return_value=True)),
|
||||
):
|
||||
out = await emb.semantic_search_rules(1, "q", limit=5, threshold=0.0,
|
||||
report=report)
|
||||
assert [r.id for _s, r in out] == [1, 2]
|
||||
assert report["best_chunk"][1] == {"index": 2, "text": "the clause that matched"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_milestone_search_reports_the_passage_of_the_plan_that_matched():
|
||||
"""A milestone's body IS the plan and search shows its short description,
|
||||
which need not mention the part the query was about."""
|
||||
m1 = MagicMock(id=7)
|
||||
session = _searching([(m1, 0.15, 5, "step 6 — the acceptance case")])
|
||||
report: dict = {}
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=session),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
patch.object(emb, "can_read_project", AsyncMock(return_value=True)),
|
||||
):
|
||||
out = await emb.semantic_search_milestones(
|
||||
1, "acceptance", limit=5, threshold=0.0, report=report,
|
||||
)
|
||||
assert [m.id for _s, m in out] == [7]
|
||||
assert report["best_chunk"][7]["text"] == "step 6 — the acceptance case"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_caller_that_passes_no_report_still_works():
|
||||
"""Every one of these searches fails open by design — a recall aid must
|
||||
never break the call it serves — and that includes the chunk channel."""
|
||||
session = _searching([(MagicMock(id=1), 0.1, 0, "text")])
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=session),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
patch.object(emb, "can_read_project", AsyncMock(return_value=True)),
|
||||
):
|
||||
out = await emb.semantic_search_milestones(1, "q", limit=5, threshold=0.0)
|
||||
assert len(out) == 1
|
||||
|
||||
|
||||
def test_record_best_chunk_on_no_report_is_a_no_op():
|
||||
emb.record_best_chunk(None, {1: {"index": 0, "text": "x"}}) # must not raise
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Only what survived the bar is published
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_chunks_are_published_only_for_results_that_came_back():
|
||||
"""Keyed off the returned rows, so a caller can look up every result it has
|
||||
and never holds passages for records it was not shown."""
|
||||
keep, drop = MagicMock(id=1), MagicMock(id=2)
|
||||
session = _searching([
|
||||
(keep, 0.10, 0, "kept"),
|
||||
(drop, 0.95, 0, "below the bar"),
|
||||
])
|
||||
report: dict = {}
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=session),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
patch.object(emb, "can_read_project", AsyncMock(return_value=True)),
|
||||
):
|
||||
await emb.semantic_search_milestones(
|
||||
1, "q", limit=5, threshold=0.5, report=report,
|
||||
)
|
||||
assert set(report["best_chunk"]) == {1}
|
||||
+259
-11
@@ -1,7 +1,7 @@
|
||||
"""search tool — proves the tool pattern (context + service call + dict shape).
|
||||
|
||||
Service call is mocked; no DB needed."""
|
||||
from unittest.mock import AsyncMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -39,43 +39,194 @@ async def test_fable_search_returns_repackaged_results():
|
||||
r = out["results"][0]
|
||||
assert r["id"] == 1
|
||||
assert r["title"] == "kafka rebalance"
|
||||
assert r["body"] == "HPA details"
|
||||
# The whole body, because it fits — and named as the opening, not as the
|
||||
# passage that matched, since this call patched the search and so carries
|
||||
# no chunk.
|
||||
assert r["excerpt"] == "HPA details"
|
||||
assert r["excerpt_is"] == "body_opening"
|
||||
assert r["body_length"] == len("HPA details")
|
||||
assert r["is_task"] is False
|
||||
assert r["tags"] == ["ops"]
|
||||
assert r["similarity"] == pytest.approx(0.93)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fable_search_body_is_truncated_to_240_chars():
|
||||
async def test_search_shows_the_passage_that_matched_not_the_opening():
|
||||
"""The point of #4243. A record can rank on its sixth paragraph; showing
|
||||
its first is showing the caller a span the search already judged less
|
||||
relevant, and letting them decide from it."""
|
||||
_user_id_ctx.set(7)
|
||||
long_body = "x" * 500
|
||||
fake = fake_note(id=1, title="t", body=long_body)
|
||||
body = "Chapter one, about nothing. " * 40 + " THE ANSWER IS 04775c3."
|
||||
fake = fake_note(id=1, title="t", body=body)
|
||||
|
||||
async def _search(*a, **kw):
|
||||
kw["report"]["best_chunk"] = {
|
||||
1: {"index": 3, "text": "THE ANSWER IS 04775c3."}
|
||||
}
|
||||
return [(0.5, fake)]
|
||||
|
||||
with patch("scribe.mcp.tools.search.semantic_search_notes", _search):
|
||||
out = await search(q="answer")
|
||||
r = out["results"][0]
|
||||
assert r["excerpt"] == "THE ANSWER IS 04775c3."
|
||||
assert r["excerpt_is"] == "matched_passage"
|
||||
assert r["chunk_index"] == 3
|
||||
# And the caller is told there is more record behind the passage.
|
||||
assert r["body_length"] == len(body)
|
||||
assert "read_full" in r
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_result_says_whether_its_excerpt_is_the_match_or_the_opening():
|
||||
"""A caller that cannot tell the two apart cannot judge whether looking
|
||||
deeper is worth it, which is the only decision this field supports."""
|
||||
_user_id_ctx.set(7)
|
||||
fake = fake_note(id=1, title="t", body="short body")
|
||||
with patch(
|
||||
"scribe.mcp.tools.search.semantic_search_notes",
|
||||
AsyncMock(return_value=[(0.5, fake)]),
|
||||
):
|
||||
out = await search(q="x")
|
||||
assert len(out["results"][0]["body"]) == 240
|
||||
assert out["results"][0]["excerpt_is"] == "body_opening"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_long_excerpt_keeps_both_ends_and_says_how_much_went():
|
||||
"""The fallback is still an elision, and an elision that drops the tail
|
||||
drops wherever the conclusion was."""
|
||||
_user_id_ctx.set(7)
|
||||
body = "OPENING. " + ("m" * 3000) + " CLOSING."
|
||||
fake = fake_note(id=1, title="t", body=body)
|
||||
with patch(
|
||||
"scribe.mcp.tools.search.semantic_search_notes",
|
||||
AsyncMock(return_value=[(0.5, fake)]),
|
||||
):
|
||||
out = await search(q="x")
|
||||
excerpt = out["results"][0]["excerpt"]
|
||||
assert excerpt.startswith("OPENING.")
|
||||
assert excerpt.rstrip().endswith("CLOSING.")
|
||||
assert "characters omitted" in excerpt
|
||||
assert out["results"][0]["body_length"] == len(body)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_short_record_arrives_whole_and_unmarked():
|
||||
"""Fragmenting a 200-character note serves nobody."""
|
||||
_user_id_ctx.set(7)
|
||||
fake = fake_note(id=1, title="t", body="all of it")
|
||||
with patch(
|
||||
"scribe.mcp.tools.search.semantic_search_notes",
|
||||
AsyncMock(return_value=[(0.5, fake)]),
|
||||
):
|
||||
out = await search(q="x")
|
||||
assert out["results"][0]["excerpt"] == "all of it"
|
||||
assert "read_full" not in out["results"][0]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fable_search_content_type_filters_at_service_layer():
|
||||
"""content_type maps to the is_task kwarg passed to the service."""
|
||||
"""The three historical values keep meaning exactly what they meant.
|
||||
|
||||
`note` is the one that could plausibly have been tightened when the
|
||||
specific kinds arrived — it is BROAD here (any non-task, snippets and
|
||||
lessons included) while the web facet of the same name is narrow. Pinning
|
||||
it stops a later tidy-up from silently removing snippets from every caller
|
||||
that already asks this way (#4250)."""
|
||||
_user_id_ctx.set(7)
|
||||
mock_search = AsyncMock(return_value=[])
|
||||
with patch("scribe.mcp.tools.search.semantic_search_notes", mock_search):
|
||||
await search(q="x", content_type="task")
|
||||
assert mock_search.call_args.kwargs["is_task"] is True
|
||||
assert mock_search.call_args.kwargs.get("task_kind") is None
|
||||
|
||||
mock_search.reset_mock()
|
||||
await search(q="x", content_type="note")
|
||||
assert mock_search.call_args.kwargs["is_task"] is False
|
||||
assert mock_search.call_args.kwargs.get("note_type") is None
|
||||
|
||||
mock_search.reset_mock()
|
||||
await search(q="x", content_type="all")
|
||||
assert mock_search.call_args.kwargs["is_task"] is None
|
||||
|
||||
|
||||
# --- the specific kinds: the engine already supported them (#4250) -----------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"content_type,expected",
|
||||
[
|
||||
("snippet", {"is_task": False, "note_type": "snippet"}),
|
||||
("lesson", {"is_task": False, "note_type": "lesson"}),
|
||||
("process", {"is_task": False, "note_type": "process"}),
|
||||
("issue", {"is_task": True, "task_kind": "issue"}),
|
||||
("spike", {"is_task": True, "task_kind": "spike"}),
|
||||
("work", {"is_task": True, "task_kind": "work"}),
|
||||
("plan", {"is_task": True, "task_kind": "plan"}),
|
||||
],
|
||||
)
|
||||
async def test_each_specific_kind_reaches_the_engine_filter_it_names(
|
||||
content_type, expected
|
||||
):
|
||||
"""`note_type` and `task_kind` were parameters of the search all along —
|
||||
what was missing was a way to ask for them from the agent's door. Asking
|
||||
for a snippet must narrow to snippets, not merely to non-tasks."""
|
||||
_user_id_ctx.set(7)
|
||||
mock_search = AsyncMock(return_value=[])
|
||||
with patch("scribe.mcp.tools.search.semantic_search_notes", mock_search):
|
||||
await search(q="x", content_type=content_type)
|
||||
kwargs = mock_search.call_args.kwargs
|
||||
for key, value in expected.items():
|
||||
assert kwargs[key] == value, f"{content_type}: {key}"
|
||||
|
||||
|
||||
def test_the_vocabulary_is_derived_from_the_facet_table_not_recopied():
|
||||
"""#3161's property, held at this door too: adding a kind to `_FACETS` is
|
||||
one edit. A hand-kept list here is exactly how the agent's search came to
|
||||
offer two kinds while the web's offered nine."""
|
||||
from scribe.services.knowledge import FACET_TYPES, content_type_filters
|
||||
|
||||
for facet in FACET_TYPES:
|
||||
filters = content_type_filters(facet) # raises if a kind is unreachable
|
||||
assert "is_task" in filters, facet
|
||||
|
||||
# The guard can fail: a name absent from the table is refused, so this is
|
||||
# membership in the table and not "every string works" (#167).
|
||||
with pytest.raises(ValueError):
|
||||
content_type_filters("a-kind-that-is-not-in-the-facet-table")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_unknown_content_type_is_refused_rather_than_returning_nothing():
|
||||
"""An empty result set is a CLAIM — "the corpus holds nothing like this" —
|
||||
and an agent acts on it by writing the thing it could not find. A typo must
|
||||
not be able to make that claim, so the door raises with the vocabulary
|
||||
instead of falling through to a filter that matches no row."""
|
||||
_user_id_ctx.set(7)
|
||||
mock_search = AsyncMock(return_value=[])
|
||||
with patch("scribe.mcp.tools.search.semantic_search_notes", mock_search):
|
||||
with pytest.raises(ValueError) as err:
|
||||
await search(q="x", content_type="snippets") # plural typo
|
||||
mock_search.assert_not_awaited()
|
||||
message = str(err.value)
|
||||
assert "snippets" in message
|
||||
assert "snippet" in message and "lesson" in message # names the valid ones
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_docstring_names_every_kind_the_tool_accepts():
|
||||
"""The docstring IS the agent-facing contract (#2846) — a filter an agent
|
||||
has not been told about is unreachable however well it is wired."""
|
||||
from scribe.mcp.tools.search import search as search_tool
|
||||
from scribe.services.knowledge import FACET_TYPES
|
||||
|
||||
doc = search_tool.__doc__ or ""
|
||||
for facet in FACET_TYPES:
|
||||
assert f"'{facet}'" in doc, f"{facet} is accepted but never documented"
|
||||
for own in ("'rule'", "'milestone'", "'system'", "'all'"):
|
||||
assert own in doc, f"{own} dispatches somewhere and is never documented"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_explicit_search_reaches_a_lesson_from_any_project():
|
||||
"""The wiring half of milestone 385 step 3.
|
||||
@@ -146,16 +297,113 @@ async def test_milestone_search_is_its_own_shape_and_scopes_to_the_project():
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
_user_id_ctx.set(7)
|
||||
# `body` is a REAL string, not left to MagicMock's attribute autovivication:
|
||||
# the result now reads it, and a mock body would make `matched` a mock
|
||||
# object and `body_length` zero while the assertion still looked green
|
||||
# (lesson #2833).
|
||||
ms = MagicMock(id=339, title="M3 — Metadata", description="works, editions",
|
||||
status="active", project_id=30)
|
||||
found = AsyncMock(return_value=[(0.81, ms)])
|
||||
status="active", project_id=30,
|
||||
body="Step 4 — the metadata editions table.")
|
||||
|
||||
summary = AsyncMock(return_value=[{"id": 339, "total": 0, "completed": 0}])
|
||||
with patch("scribe.mcp.tools.search.semantic_search_milestones", found), \
|
||||
seen: dict = {}
|
||||
|
||||
async def _milestone_search(*_a, **kw):
|
||||
seen.update(kw)
|
||||
kw["report"]["best_chunk"] = {
|
||||
339: {"index": 1, "text": "Step 4 — the metadata editions table."}
|
||||
}
|
||||
return [(0.81, ms)]
|
||||
|
||||
with patch("scribe.mcp.tools.search.semantic_search_milestones",
|
||||
_milestone_search), \
|
||||
patch("scribe.services.milestones.get_project_milestone_summary", summary):
|
||||
out = await search(q="book metadata", content_type="milestone", project_id=30)
|
||||
assert found.await_args.kwargs["project_id"] == 30
|
||||
assert seen["project_id"] == 30
|
||||
assert out["results"] == [{
|
||||
"id": 339, "title": "M3 — Metadata", "description": "works, editions",
|
||||
# The plan body stays out; the passage that matched comes along, and
|
||||
# says which of the two it is (#4243).
|
||||
"matched": "Step 4 — the metadata editions table.",
|
||||
"matched_is": "matched_passage",
|
||||
"body_length": len("Step 4 — the metadata editions table."),
|
||||
"status": "active", "project_id": 30, "total": 0, "completed": 0,
|
||||
"similarity": 0.81,
|
||||
}]
|
||||
|
||||
|
||||
# --- systems: the charter as an answer, not a filter (#4251) -----------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_system_search_is_its_own_shape_and_carries_the_matched_passage():
|
||||
"""A System's charter runs to several hundred words and a result shows its
|
||||
NAME — so a match on the paragraph that actually decides where a record
|
||||
belongs would be previewed by two words that cannot. The passage comes
|
||||
along, marked as the passage (#4243)."""
|
||||
_user_id_ctx.set(7)
|
||||
charter = (
|
||||
"Opening sentence about the area in general. "
|
||||
+ "x" * 400
|
||||
+ " Retrieval telemetry and the floors it judges belong here."
|
||||
)
|
||||
fake = MagicMock(id=3, project_id=2, description=charter)
|
||||
# `name` is MagicMock's own constructor kwarg — passed in, it names the
|
||||
# mock and leaves `.name` a mock object, which is note #2833's whole point.
|
||||
fake.name = "Retrieval & recall"
|
||||
|
||||
async def _system_search(uid, q, **kwargs):
|
||||
kwargs["report"]["best_chunk"] = {
|
||||
3: {"index": 2, "text": "Retrieval telemetry and the floors it judges belong here."}
|
||||
}
|
||||
return [(0.81, fake)]
|
||||
|
||||
with patch("scribe.mcp.tools.search.semantic_search_systems", _system_search):
|
||||
out = await search(q="where does retrieval telemetry go?",
|
||||
content_type="system", project_id=2)
|
||||
|
||||
assert out["total"] == 1
|
||||
row = out["results"][0]
|
||||
assert row["id"] == 3
|
||||
assert row["name"] == "Retrieval & recall"
|
||||
assert row["project_id"] == 2
|
||||
assert row["similarity"] == 0.81
|
||||
# The passage that won, not the charter's opening — and SAID to be that.
|
||||
assert row["matched"] == (
|
||||
"Retrieval telemetry and the floors it judges belong here."
|
||||
)
|
||||
assert row["matched_is"] == "matched_passage"
|
||||
assert row["body_length"] == len(charter)
|
||||
assert "read_full" in row, "a 56-character span of a 500-character charter"
|
||||
# The charter itself does not ride along: get_system reads it.
|
||||
assert "description" not in row
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_system_search_scopes_to_the_project_it_was_given():
|
||||
"""Systems are per-project and a charter from another project is not an
|
||||
answer to "where does this belong here?"."""
|
||||
_user_id_ctx.set(7)
|
||||
mock = AsyncMock(return_value=[])
|
||||
with patch("scribe.mcp.tools.search.semantic_search_systems", mock):
|
||||
await search(q="x", content_type="system", project_id=30)
|
||||
assert mock.call_args.kwargs["project_id"] == 30
|
||||
|
||||
mock.reset_mock()
|
||||
with patch("scribe.mcp.tools.search.semantic_search_systems", mock):
|
||||
await search(q="x", content_type="system")
|
||||
assert mock.call_args.kwargs["project_id"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_system_search_does_not_go_through_the_note_search():
|
||||
"""Its own search, not a note_type. A charter competing with the whole note
|
||||
corpus for one top-k is outranked by the records filed under it."""
|
||||
_user_id_ctx.set(7)
|
||||
notes = AsyncMock(return_value=[])
|
||||
with (
|
||||
patch("scribe.mcp.tools.search.semantic_search_notes", notes),
|
||||
patch("scribe.mcp.tools.search.semantic_search_systems", AsyncMock(return_value=[])),
|
||||
):
|
||||
await search(q="x", content_type="system")
|
||||
notes.assert_not_awaited()
|
||||
|
||||
@@ -224,13 +224,34 @@ def _pulling_getters():
|
||||
yield module, name, body
|
||||
|
||||
|
||||
def _takes_reading_project(body: str) -> bool:
|
||||
"""Does this function declare `project_id: int = 0`?
|
||||
|
||||
Parsed, not string-matched against the first line. The first version read
|
||||
`body.split("\n")[0]`, which sees only as far as the first newline — so
|
||||
adding a parameter to `get_task` wrapped its signature over four lines and
|
||||
the guard reported a function that DOES take the project as one that does
|
||||
not. A guard that fails on formatting is a guard that gets appeased by
|
||||
reflowing the code it was meant to check.
|
||||
"""
|
||||
node = ast.parse(body).body[0]
|
||||
args = node.args
|
||||
params = list(args.posonlyargs) + list(args.args) + list(args.kwonlyargs)
|
||||
for arg in params:
|
||||
if arg.arg != "project_id":
|
||||
continue
|
||||
ann = getattr(arg, "annotation", None)
|
||||
return isinstance(ann, ast.Name) and ann.id == "int"
|
||||
return False
|
||||
|
||||
|
||||
def test_every_getter_that_pulls_takes_the_reading_project():
|
||||
"""Asserted on structure (rule 167): a behavioural test cannot see a
|
||||
parameter that was never threaded through."""
|
||||
missing = [
|
||||
f"{module}.{name}"
|
||||
for module, name, body in _pulling_getters()
|
||||
if "project_id: int = 0" not in body.split("\n")[0]
|
||||
if not _takes_reading_project(body)
|
||||
]
|
||||
assert not missing, (
|
||||
f"these getters record a pull but cannot say where the reader was: "
|
||||
|
||||
@@ -446,3 +446,80 @@ def test_a_quiet_arm_is_not_suspended_either_way() -> None:
|
||||
ws = warn({"auto_inject": src(calls=1, zero_result_calls=0, p10=0.705)},
|
||||
floors={"auto_inject": 0.70}, floor_moves={"auto_inject": MOVED})
|
||||
assert "floor_moved_mid_window" not in codes(ws)
|
||||
|
||||
|
||||
# ── a fixed-query arm: decline rate is arithmetic, not evidence (#4232) ─────
|
||||
#
|
||||
# `report_preference` searches one constant string (COMPLETION_QUERY), so it
|
||||
# scores against one number on every call. Its decline rate is therefore 0% or
|
||||
# 100% and never in between, and which one depends only on where the bar sits
|
||||
# relative to that constant.
|
||||
#
|
||||
# So the two warnings swap roles for these arms. "Never declined" stops being
|
||||
# evidence about the floor — `cannot_decline`'s own remedy, "check that it
|
||||
# applies its floor", is unanswerable from it. "Always declined" starts being
|
||||
# evidence, because for a constant score it means the bar is above it and no
|
||||
# further traffic will ever say otherwise.
|
||||
|
||||
|
||||
def test_cannot_decline_is_silent_on_a_fixed_query_arm():
|
||||
"""The live readout fired this on `report_preference` at 45 calls, 0
|
||||
empty, with p10 = p50 = p90 = min = max = 0.791 — five identical
|
||||
percentiles, which is one record at one score rather than a ranking."""
|
||||
ws = warn({"report_preference": src(calls=45, zero_result_calls=0, p10=0.791)})
|
||||
assert "cannot_decline" not in codes(ws, "report_preference")
|
||||
|
||||
|
||||
def test_cannot_decline_still_fires_where_the_rate_means_something():
|
||||
"""The falsifier for the case above (rule 167). If this passes only
|
||||
because the check was disabled rather than narrowed, this fails."""
|
||||
ws = warn({"auto_inject": src(calls=N, zero_result_calls=0)})
|
||||
assert "cannot_decline" in codes(ws, "auto_inject")
|
||||
|
||||
|
||||
def test_a_fixed_query_arm_that_never_clears_its_bar_is_named():
|
||||
"""69 consecutive declines at 0.0006 under the bar is a state this arm has
|
||||
actually been in. Nothing else in the readout would have said so: it looks
|
||||
exactly like an arm with nothing to report."""
|
||||
ws = warn({"report_preference": src(calls=45, zero_result_calls=45)})
|
||||
assert "fixed_query_never_clears" in codes(ws, "report_preference")
|
||||
|
||||
detail = next(w["detail"] for w in ws if w["code"] == "fixed_query_never_clears")
|
||||
assert "near_miss_samples" in detail, (
|
||||
"the last time this fired, every percentile said lower the floor and "
|
||||
"the refused record showed the refusal was right — so the warning has "
|
||||
"to send the reader to the record, not to the dial"
|
||||
)
|
||||
|
||||
|
||||
def test_an_ordinary_arm_returning_nothing_all_window_is_not_dead():
|
||||
"""For an arm whose score can vary, an empty window means nothing matched,
|
||||
which is an answer rather than a fault."""
|
||||
ws = warn({"auto_inject": src(calls=N, zero_result_calls=N)})
|
||||
assert "fixed_query_never_clears" not in codes(ws, "auto_inject")
|
||||
|
||||
|
||||
def test_a_fixed_query_arm_that_sometimes_clears_is_not_dead():
|
||||
"""Only ALL-empty says the bar is above the constant. Anything in between
|
||||
means the score is not actually constant, and the premise is wrong."""
|
||||
ws = warn({"report_preference": src(calls=45, zero_result_calls=44)})
|
||||
assert "fixed_query_never_clears" not in codes(ws, "report_preference")
|
||||
|
||||
|
||||
def test_the_dead_arm_warning_still_needs_volume():
|
||||
ws = warn({"report_preference": src(calls=N - 1, zero_result_calls=N - 1)})
|
||||
assert "fixed_query_never_clears" not in codes(ws)
|
||||
|
||||
|
||||
def test_the_registry_declares_which_arms_ask_a_fixed_question():
|
||||
"""Asserted on structure (rule 167), and able to fail: if `fixed_query`
|
||||
is dropped or defaults to True, one of these two halves breaks."""
|
||||
from scribe.services.retrieval_registry import POINTS
|
||||
|
||||
assert POINTS["report_preference"].fixed_query is True, (
|
||||
"services/reply_preferences.py::COMPLETION_QUERY is a module constant"
|
||||
)
|
||||
# An arm whose query is built from the prompt, the file or the command is
|
||||
# not fixed, and marking one would silence a warning that works there.
|
||||
for varying in ("auto_inject", "write_path", "pre_tool_rule", "prompt_rule"):
|
||||
assert POINTS[varying].fixed_query is False, varying
|
||||
|
||||
+42
-10
@@ -1,18 +1,50 @@
|
||||
"""Unit tests for the search route parameter mapping."""
|
||||
from scribe.routes.search import _content_type_to_is_task
|
||||
"""/api/search — the kind filter it offers, and what it does with a bad one.
|
||||
|
||||
This route used to carry its own two-value map (`note`/`task`/everything else
|
||||
is `all`), which made it the third hand-kept copy of a vocabulary that lives in
|
||||
one table. Both halves of #4250 are pinned here: every declared kind is
|
||||
reachable, and a kind that does not exist is refused rather than widened.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from scribe.services.knowledge import FACET_TYPES, content_type_filters
|
||||
|
||||
|
||||
def test_content_type_note():
|
||||
assert _content_type_to_is_task("note") is False
|
||||
def test_the_three_historical_values_still_mean_what_they_meant():
|
||||
assert content_type_filters("all") == {"is_task": None}
|
||||
assert content_type_filters("task")["is_task"] is True
|
||||
# BROAD on purpose: any non-task, so snippets and lessons stay in scope.
|
||||
assert content_type_filters("note") == {"is_task": False}
|
||||
|
||||
|
||||
def test_content_type_task():
|
||||
assert _content_type_to_is_task("task") is True
|
||||
@pytest.mark.parametrize("facet", sorted(FACET_TYPES))
|
||||
def test_every_declared_kind_is_reachable_through_this_door(facet):
|
||||
"""The engine took `note_type` and `task_kind` all along — the route was
|
||||
simply unable to say them."""
|
||||
filters = content_type_filters(facet)
|
||||
assert "is_task" in filters
|
||||
if facet not in ("all", "note", "task"):
|
||||
assert filters.get("note_type") == facet or filters.get("task_kind") == facet
|
||||
|
||||
|
||||
def test_content_type_all():
|
||||
assert _content_type_to_is_task("all") is None
|
||||
def test_an_unknown_kind_is_refused_instead_of_silently_widening():
|
||||
"""The old map read anything unrecognised as "no filter", so
|
||||
`?content_type=snippets` returned the whole corpus while looking like a
|
||||
narrowed search — the failure mode that answers a question nobody asked."""
|
||||
with pytest.raises(ValueError) as err:
|
||||
content_type_filters("snippets")
|
||||
assert "snippets" in str(err.value)
|
||||
assert "snippet" in str(err.value) # names the real ones
|
||||
|
||||
|
||||
def test_content_type_unknown_defaults_to_all():
|
||||
assert _content_type_to_is_task("unknown") is None
|
||||
def test_a_doors_refusal_lists_only_what_that_door_accepts():
|
||||
"""`rule` and `milestone` are the MCP tool's own searches. This route has
|
||||
neither, so offering them in its error would send a caller at a parameter
|
||||
that does not work here."""
|
||||
with pytest.raises(ValueError) as plain:
|
||||
content_type_filters("nonsense")
|
||||
assert "rule" not in str(plain.value)
|
||||
|
||||
with pytest.raises(ValueError) as extra:
|
||||
content_type_filters("nonsense", extra=("rule", "milestone"))
|
||||
assert "rule" in str(extra.value) and "milestone" in str(extra.value)
|
||||
|
||||
@@ -14,23 +14,14 @@ from scribe.services.dedup import (
|
||||
plan_candidate_text,
|
||||
plan_match_response,
|
||||
)
|
||||
from tests.helpers import fake_note, make_mock_session
|
||||
|
||||
|
||||
def _session_returning(note):
|
||||
"""A mocked async_session() whose single execute() yields `note` (or None)."""
|
||||
s = make_mock_session()
|
||||
result = MagicMock()
|
||||
result.scalars.return_value.first.return_value = note
|
||||
s.execute = AsyncMock(return_value=result)
|
||||
return s
|
||||
from tests.helpers import fake_note, make_mock_session, session_returning
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_title_exact_match_returns_title_duplicate():
|
||||
note = fake_note(id=10, title="Setup CI")
|
||||
with patch("scribe.services.dedup.async_session",
|
||||
return_value=_session_returning(note)):
|
||||
return_value=session_returning(note)):
|
||||
# whitespace/case differences are normalized away
|
||||
dup = await find_duplicate_note(7, " setup ci ", project_id=2, is_task=True)
|
||||
assert dup is not None
|
||||
@@ -43,7 +34,7 @@ async def test_title_exact_match_returns_title_duplicate():
|
||||
async def test_short_body_skips_semantic_check():
|
||||
sem = AsyncMock()
|
||||
with patch("scribe.services.dedup.async_session",
|
||||
return_value=_session_returning(None)), \
|
||||
return_value=session_returning(None)), \
|
||||
patch("scribe.services.dedup.embeddings_svc.semantic_search_notes", sem):
|
||||
dup = await find_duplicate_note(7, "Unique", body="too short", project_id=2)
|
||||
assert dup is None
|
||||
@@ -55,7 +46,7 @@ async def test_semantic_match_when_body_substantial():
|
||||
hit = fake_note(id=20, title="Existing", note_type="note")
|
||||
sem = AsyncMock(return_value=[(0.93, hit)])
|
||||
with patch("scribe.services.dedup.async_session",
|
||||
return_value=_session_returning(None)), \
|
||||
return_value=session_returning(None)), \
|
||||
patch("scribe.services.dedup.embeddings_svc.semantic_search_notes", sem):
|
||||
dup = await find_duplicate_note(
|
||||
7, "Title", body="x" * 250, project_id=2, is_task=False, note_type="note",
|
||||
@@ -84,7 +75,7 @@ async def test_gate_catches_a_duplicate_hiding_in_a_later_chunk():
|
||||
# Every chunk misses except the LAST one the gate will ask about.
|
||||
sem = AsyncMock(side_effect=[[] for _ in range(n_chunks - 1)] + [[(0.94, hit)]])
|
||||
with patch("scribe.services.dedup.async_session",
|
||||
return_value=_session_returning(None)), \
|
||||
return_value=session_returning(None)), \
|
||||
patch("scribe.services.dedup.embeddings_svc.semantic_search_notes", sem):
|
||||
dup = await find_duplicate_note(
|
||||
7, "Title", body=body, project_id=2, is_task=False, note_type="note",
|
||||
@@ -98,7 +89,7 @@ async def test_semantic_match_of_other_note_type_is_ignored():
|
||||
other = fake_note(id=21, title="X", note_type="process")
|
||||
sem = AsyncMock(return_value=[(0.97, other)])
|
||||
with patch("scribe.services.dedup.async_session",
|
||||
return_value=_session_returning(None)), \
|
||||
return_value=session_returning(None)), \
|
||||
patch("scribe.services.dedup.embeddings_svc.semantic_search_notes", sem):
|
||||
dup = await find_duplicate_note(7, "Title", body="x" * 250, note_type="note")
|
||||
assert dup is None # type mismatch must not block
|
||||
@@ -108,7 +99,7 @@ async def test_semantic_match_of_other_note_type_is_ignored():
|
||||
async def test_rule_title_match_in_topic():
|
||||
rule = fake_note(id=47, title="Honor the multi-user sharing ACL")
|
||||
with patch("scribe.services.dedup.async_session",
|
||||
return_value=_session_returning(rule)):
|
||||
return_value=session_returning(rule)):
|
||||
dup = await find_duplicate_rule(
|
||||
"honor the multi-user sharing acl", topic_id=7,
|
||||
)
|
||||
@@ -361,7 +352,7 @@ async def test_plan_title_match_short_circuits_the_semantic_arm():
|
||||
ms = MagicMock(id=415, title="Plan gate")
|
||||
sem = AsyncMock()
|
||||
with patch("scribe.services.dedup.can_read_project", AsyncMock(return_value=True)), \
|
||||
patch("scribe.services.dedup.async_session", return_value=_session_returning(ms)), \
|
||||
patch("scribe.services.dedup.async_session", return_value=session_returning(ms)), \
|
||||
patch("scribe.services.dedup.embeddings_svc.semantic_search_milestones", sem):
|
||||
dup = await find_matching_plan(7, 2, " plan GATE", "x" * 300)
|
||||
assert (dup.id, dup.reason, dup.similarity) == (415, "title", 1.0)
|
||||
@@ -373,7 +364,7 @@ async def test_plan_semantic_arm_asks_for_active_plans_in_the_project_at_the_set
|
||||
ms = MagicMock(id=9, title="Metadata")
|
||||
sem = AsyncMock(return_value=[(0.912345, ms)])
|
||||
with patch("scribe.services.dedup.can_read_project", AsyncMock(return_value=True)), \
|
||||
patch("scribe.services.dedup.async_session", return_value=_session_returning(None)), \
|
||||
patch("scribe.services.dedup.async_session", return_value=session_returning(None)), \
|
||||
patch("scribe.services.dedup.embeddings_svc.semantic_search_milestones", sem), \
|
||||
patch("scribe.services.settings.get_setting", AsyncMock(return_value="0.8")):
|
||||
dup = await find_matching_plan(7, 2, "Book metadata", "x" * 300)
|
||||
@@ -396,7 +387,7 @@ async def test_plan_gate_fails_open():
|
||||
@pytest.mark.asyncio
|
||||
async def test_plan_gate_says_nothing_about_a_project_the_caller_cannot_read():
|
||||
ms = MagicMock(id=415, title="Their plan")
|
||||
session = MagicMock(return_value=_session_returning(ms))
|
||||
session = MagicMock(return_value=session_returning(ms))
|
||||
with patch("scribe.services.dedup.can_read_project", AsyncMock(return_value=False)), \
|
||||
patch("scribe.services.dedup.async_session", session):
|
||||
assert await find_matching_plan(8, 2, "Their plan") is None
|
||||
|
||||
@@ -78,8 +78,12 @@ async def test_build_autoinject_hint_titles_only_with_margin_gate():
|
||||
assert out["note_ids"] == [11, 22]
|
||||
assert '#11 [note] "Pool sizing decision" (0.80)' in out["context"]
|
||||
assert "#33" not in out["context"]
|
||||
# Title-first: no body text, ever.
|
||||
# Title-first when the search reports no matched passage — which is this
|
||||
# test, whose mock returns bare (score, note) pairs and fills no report.
|
||||
# A record whose passage IS known gets it on a second line; that is
|
||||
# test_the_menu_shows_the_passage_that_matched below.
|
||||
assert "get_note(id)" in out["context"]
|
||||
assert "↳" not in out["context"]
|
||||
# Telemetry fired for BOTH retrievals this path runs: the scored menu and
|
||||
# the reuse-slot query competing against it. The slot's query used to be
|
||||
# the one unlogged retrieval on this path — the hit it displaced was in
|
||||
@@ -552,3 +556,83 @@ async def test_the_config_stand_in_carries_every_key_the_real_one_does():
|
||||
"a key the real config has and the stand-in does not turns an arm "
|
||||
"into a silent no-op under test"
|
||||
)
|
||||
|
||||
|
||||
# ─── the passage that matched travels onto the menu (#4243, #4250) ───────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_menu_shows_the_passage_that_matched():
|
||||
"""A title is a headline. For a lesson or a snippet it carries the trigger
|
||||
and answers "does this apply to me"; for an issue or a dev-log the reason
|
||||
this record matched is a sentence somewhere inside it, and the reader was
|
||||
being handed the one part guaranteed not to say so."""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
hits = [(0.80, fake_note(id=11, title="Pool sizing decision", user_id=1))]
|
||||
|
||||
# Only the FIRST call is the menu's own search; the reuse and lesson slots
|
||||
# run their own queries afterwards and must not contribute chunks, which is
|
||||
# also what makes the count assertion below deterministic. `report` is
|
||||
# optional on this interface, so it is written only when one was passed.
|
||||
calls: list[int] = []
|
||||
|
||||
async def _menu_search(*_a, **kw):
|
||||
calls.append(1)
|
||||
if len(calls) > 1:
|
||||
return []
|
||||
if kw.get("report") is not None:
|
||||
kw["report"]["best_chunk"] = {
|
||||
11: {"index": 3, "text": "we set max_overflow to 5 after the leak"}
|
||||
}
|
||||
return hits
|
||||
|
||||
with patch.object(pc, "get_autoinject_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.55,
|
||||
"top_k": 3})), \
|
||||
patch.object(pc, "semantic_search_notes", _menu_search), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()):
|
||||
out = await pc.build_autoinject_hint(1, "pool", project_id=2)
|
||||
|
||||
assert "we set max_overflow to 5 after the leak" in out["context"]
|
||||
# Indented under its line, so the menu still reads as a list of records
|
||||
# rather than a wall of prose.
|
||||
assert "> ↳ we set max_overflow" in out["context"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_record_with_no_stored_chunk_gets_no_invented_passage():
|
||||
"""The reserved lesson and reuse slots are fetched by their own queries, so
|
||||
they are absent from this search's report. Falling back to the body's
|
||||
opening would put a line of preamble under them dressed as the reason they
|
||||
matched — and once indented identically, a reader cannot tell the two
|
||||
apart."""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
hits = [(0.80, fake_note(id=11, title="Has a chunk", user_id=1)),
|
||||
(0.78, fake_note(id=22, title="Has none", user_id=1,
|
||||
body="A long body whose opening says nothing."))]
|
||||
|
||||
calls: list[int] = []
|
||||
|
||||
async def _menu_search(*_a, **kw):
|
||||
calls.append(1)
|
||||
if len(calls) > 1:
|
||||
return []
|
||||
if kw.get("report") is not None:
|
||||
kw["report"]["best_chunk"] = {
|
||||
11: {"index": 0, "text": "the real reason"}
|
||||
}
|
||||
return hits
|
||||
|
||||
with patch.object(pc, "get_autoinject_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.55,
|
||||
"top_k": 3})), \
|
||||
patch.object(pc, "semantic_search_notes", _menu_search), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()):
|
||||
out = await pc.build_autoinject_hint(1, "q", project_id=2)
|
||||
|
||||
assert "the real reason" in out["context"]
|
||||
assert "A long body whose opening" not in out["context"]
|
||||
# Exactly one passage line, for the one record that had a passage.
|
||||
assert out["context"].count("↳") == 1
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
"""Systems become findable by meaning — the charter as an ANSWER (#4251).
|
||||
|
||||
A System's `description` is a charter: several hundred words saying what
|
||||
belongs in that area and what does not. It is the answer to "where does this
|
||||
go?", and there was no semantic path to it — `list_systems` enumerates and
|
||||
`search(system_id=…)` uses a System as a FILTER over notes, so a System could
|
||||
narrow a search and could never be the answer to one.
|
||||
|
||||
These pin the three halves: the document shape, the search's scoping and
|
||||
best-chunk contract, and that a charter edited through any door is re-indexed.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import embeddings as emb
|
||||
from scribe.services import systems as systems_svc
|
||||
from tests.helpers import make_mock_session
|
||||
|
||||
|
||||
def _system(system_id=3, name="Retrieval & recall", description="the charter"):
|
||||
s = MagicMock(id=system_id, project_id=2, description=description)
|
||||
s.name = name # never via the constructor — see note #2833
|
||||
return s
|
||||
|
||||
|
||||
# --- the document shape ------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_document_is_the_name_and_the_charter():
|
||||
assert emb.system_document("Retrieval", "what belongs here") == (
|
||||
"Retrieval", "what belongs here"
|
||||
)
|
||||
|
||||
|
||||
def test_a_system_with_no_charter_yet_degrades_to_its_name():
|
||||
"""It still embeds, just weakly. That is an argument for writing the
|
||||
charter, not for padding the document with whatever is to hand."""
|
||||
assert emb.system_document("Retrieval", None) == ("Retrieval", None)
|
||||
assert emb.system_document("Retrieval", " ") == ("Retrieval", None)
|
||||
assert emb.chunk_document(*emb.system_document("Retrieval", None)) == ["Retrieval"]
|
||||
|
||||
|
||||
def test_an_empty_system_produces_no_document_at_all():
|
||||
"""Callers gate on falsiness to clear vectors rather than embed nothing."""
|
||||
assert emb.system_document("", "") == (None, None)
|
||||
assert emb.chunk_document(*emb.system_document("", "")) == []
|
||||
|
||||
|
||||
# --- the search --------------------------------------------------------------
|
||||
|
||||
|
||||
def _rows_ctx(rows):
|
||||
session = MagicMock()
|
||||
result = MagicMock()
|
||||
result.all.return_value = rows
|
||||
session.execute = AsyncMock(return_value=result)
|
||||
ctx = MagicMock()
|
||||
ctx.__aenter__ = AsyncMock(return_value=session)
|
||||
ctx.__aexit__ = AsyncMock(return_value=False)
|
||||
return session, ctx
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_search_collapses_to_best_chunk_and_reports_which_one_won():
|
||||
"""A charter is long, and a result shows its NAME — so the paragraph that
|
||||
actually decides where a record belongs has to come back with it, or the
|
||||
caller judges from two words that cannot say (#4243). Published from the
|
||||
start here rather than retrofitted, which is what #4251 asked for."""
|
||||
a, b = _system(3), _system(4, name="Data Model")
|
||||
rows = [
|
||||
(a, 0.10, 2, "retrieval telemetry belongs here"),
|
||||
(b, 0.22, 0, "the persistence layer"),
|
||||
(a, 0.40, 0, "a weaker paragraph of the same charter"),
|
||||
]
|
||||
_session, ctx = _rows_ctx(rows)
|
||||
report: dict = {}
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=ctx),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
patch.object(emb, "can_read_project", AsyncMock(return_value=True)),
|
||||
):
|
||||
out = await emb.semantic_search_systems(
|
||||
7, "where does telemetry go?",
|
||||
project_id=2, threshold=0.0, report=report,
|
||||
)
|
||||
|
||||
assert [s.id for _score, s in out] == [3, 4]
|
||||
assert out[0][0] == 1.0 - 0.10 # the BEST chunk's score
|
||||
assert report["best_chunk"][3] == {
|
||||
"index": 2, "text": "retrieval telemetry belongs here",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_project_the_caller_cannot_read_returns_nothing():
|
||||
"""Rule 78 — the charter of a project someone was not given is not an
|
||||
answer to any question they are entitled to ask."""
|
||||
_session, ctx = _rows_ctx([])
|
||||
with (
|
||||
patch.object(emb, "async_session", return_value=ctx),
|
||||
patch.object(emb, "get_embedding", AsyncMock(return_value=[0.0] * 384)),
|
||||
patch.object(emb, "can_read_project", AsyncMock(return_value=False)),
|
||||
):
|
||||
assert await emb.semantic_search_systems(7, "q", project_id=99) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_empty_query_never_reaches_the_embedder():
|
||||
with patch.object(emb, "get_embedding", AsyncMock()) as embed:
|
||||
assert await emb.semantic_search_systems(7, " ") == []
|
||||
embed.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_search_that_fails_returns_nothing_rather_than_raising():
|
||||
"""A recall aid must never break the call it serves."""
|
||||
with patch.object(emb, "get_embedding", AsyncMock(side_effect=RuntimeError)):
|
||||
assert await emb.semantic_search_systems(7, "q") == []
|
||||
|
||||
|
||||
# --- staying current ---------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_creating_a_system_indexes_its_charter():
|
||||
session = make_mock_session()
|
||||
with (
|
||||
patch.object(systems_svc, "async_session", return_value=session),
|
||||
patch.object(systems_svc.access, "can_write_project", AsyncMock(return_value=True)),
|
||||
patch.object(systems_svc, "embed_system") as embed,
|
||||
):
|
||||
await systems_svc.create_system(7, 2, "Retrieval", description="the charter")
|
||||
embed.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_editing_a_charter_re_indexes_it():
|
||||
"""A charter edited and not re-indexed stays findable by what it used to
|
||||
say — the stale-vector case, and the one that matters most for a record
|
||||
whose whole job is to say where things belong now."""
|
||||
system = _system()
|
||||
session = make_mock_session()
|
||||
session.get = AsyncMock(return_value=system)
|
||||
system.deleted_at = None
|
||||
with (
|
||||
patch.object(systems_svc, "async_session", return_value=session),
|
||||
patch.object(systems_svc.access, "can_write_project", AsyncMock(return_value=True)),
|
||||
patch.object(systems_svc, "embed_system") as embed,
|
||||
):
|
||||
await systems_svc.update_system(7, 3, description="a new charter")
|
||||
embed.assert_called_once_with(system)
|
||||
|
||||
|
||||
def test_embedding_a_system_never_breaks_the_write_that_saved_it():
|
||||
"""A System that saved must not fail on its index refresh. ValueError
|
||||
rather than RuntimeError deliberately: RuntimeError is also what "no
|
||||
running loop" raises, which is an ordinary sync caller and has its own
|
||||
branch — this has to land in the general swallow to prove it exists."""
|
||||
with patch(
|
||||
"scribe.services.embeddings.upsert_system_embedding",
|
||||
side_effect=ValueError("boom"),
|
||||
):
|
||||
systems_svc.embed_system(_system()) # returns quietly, does not raise
|
||||
@@ -0,0 +1,200 @@
|
||||
"""task_document — the document a TASK is embedded as, work logs included (#4251).
|
||||
|
||||
A work log is the richest record Scribe holds of *why* something is the way it
|
||||
is: prose written during the work, saying what was tried and ruled out. #4241
|
||||
made it readable. It was still not findable, so "has anyone tried this
|
||||
approach?" — the question a log answers — could not reach one, and #4208 was
|
||||
rebuilt in this very session because its logs were unreachable.
|
||||
|
||||
These tests pin the shape and the two properties the decision rested on: a task
|
||||
without logs embeds EXACTLY as it always did, and a task with them stays as
|
||||
discriminative as it was, because each log is its own title-anchored chunk
|
||||
rather than prose averaged into one vector.
|
||||
"""
|
||||
import datetime
|
||||
|
||||
from scribe.services.embeddings import (
|
||||
WORK_LOG_HEADING,
|
||||
chunk_document,
|
||||
embedding_text,
|
||||
task_document,
|
||||
)
|
||||
|
||||
D1 = datetime.datetime(2026, 9, 20, 10, 0)
|
||||
D2 = datetime.datetime(2026, 9, 21, 11, 0)
|
||||
|
||||
|
||||
# --- the identity half: a task with no logs is untouched ---------------------
|
||||
|
||||
|
||||
def test_a_task_with_no_logs_embeds_exactly_as_it_did_before():
|
||||
"""Most notes are not tasks and most tasks carry no log. Their vectors are
|
||||
the corpus this instance's thresholds were measured against, and changing
|
||||
them for nothing would move every tuned number underneath itself (#4225)."""
|
||||
assert task_document("A title", "A body") == ("A title", "A body")
|
||||
assert task_document("A title", "A body", []) == ("A title", "A body")
|
||||
assert task_document("T", None) == ("T", None)
|
||||
|
||||
|
||||
def test_an_entry_with_no_content_is_skipped_rather_than_emitted_empty():
|
||||
"""A bare heading is a vector containing nothing but the task's title — it
|
||||
would compete with the task's real chunk and say nothing."""
|
||||
_t, body = task_document("T", "prose", [(D1, " "), (D2, None), (D1, "real")])
|
||||
assert body.count(WORK_LOG_HEADING) == 1
|
||||
assert "real" in body
|
||||
|
||||
|
||||
def test_a_task_that_is_only_logs_still_yields_a_document():
|
||||
"""A task opened with a title and filled in entirely through its log — the
|
||||
shape every step of a milestone starts as."""
|
||||
title, body = task_document("T", "", [(D1, "the only content")])
|
||||
assert title == "T"
|
||||
assert body.startswith(WORK_LOG_HEADING)
|
||||
assert "the only content" in body
|
||||
|
||||
|
||||
# --- the shape: the logs are sections, oldest first --------------------------
|
||||
|
||||
|
||||
def test_logs_are_appended_as_dated_sections_after_the_tasks_own_prose():
|
||||
title, body = task_document(
|
||||
"Fix the thing", "The task prose.",
|
||||
[(D1, "Tried X, ruled out."), (D2, "Y worked.")],
|
||||
)
|
||||
assert title == "Fix the thing"
|
||||
assert body.index("The task prose.") < body.index("Tried X") < body.index("Y worked.")
|
||||
assert f"{WORK_LOG_HEADING} — 2026-09-20" in body
|
||||
assert f"{WORK_LOG_HEADING} — 2026-09-21" in body
|
||||
|
||||
|
||||
def test_an_entry_with_no_timestamp_still_gets_its_own_section():
|
||||
"""Degrades to an undated heading rather than dropping the entry or
|
||||
crashing — a restored row or a hand-built one is not a reason to lose the
|
||||
richest prose in the record."""
|
||||
_t, body = task_document("T", "p", [(None, "content from nowhere")])
|
||||
assert WORK_LOG_HEADING in body
|
||||
assert "content from nowhere" in body
|
||||
|
||||
|
||||
# --- why folding them in is safe: chunking, not averaging --------------------
|
||||
|
||||
|
||||
def test_each_log_becomes_its_own_title_anchored_chunk():
|
||||
"""THE decision this build turns on. The objection to putting logs in the
|
||||
task's document is that a long log drowns a short title — true before #280,
|
||||
when one vector per record meant a 2,000-word log averaged the task's
|
||||
subject away and everything past ~400 words was truncated unread. Chunking
|
||||
answers both: separate vectors, each carrying the title as its anchor."""
|
||||
def _entry(subject: str) -> str:
|
||||
para = " ".join([f"This log entry discusses the {subject} in detail."] * 8)
|
||||
return "\n\n".join(f"{para} (p{i})" for i in range(6))
|
||||
|
||||
title, body = task_document(
|
||||
"Short task title", "Short body.",
|
||||
[(D1, _entry("approach")), (D2, _entry("alternative"))],
|
||||
)
|
||||
chunks = chunk_document(title, body)
|
||||
assert len(chunks) > 1, "a long log must split rather than truncate"
|
||||
for chunk in chunks:
|
||||
assert chunk.startswith("Short task title\n")
|
||||
|
||||
# The task's own prose still has a chunk of its own — it is not merged into
|
||||
# a log section and scored as part of it.
|
||||
assert any("Short body." in c and "alternative" not in c for c in chunks)
|
||||
|
||||
# And nothing is lost: this is what #280 exists for. Paragraph-shaped,
|
||||
# because that is what a log is and because a single unbroken 3,000-char
|
||||
# line is hard-split mid-line by design — `test_chunking` owns that case.
|
||||
joined = "\n".join(chunks)
|
||||
for line in body.splitlines():
|
||||
if line.strip():
|
||||
assert line.strip() in joined, f"content dropped: {line[:60]!r}"
|
||||
|
||||
|
||||
def test_a_short_task_with_a_short_log_is_still_one_sharp_chunk():
|
||||
"""Over-splitting is not the goal either. A task and one short log stay a
|
||||
single document, the shape note #2485 measured as the sharpest."""
|
||||
title, body = task_document("T", "body", [(D1, "a short note about it")])
|
||||
assert chunk_document(title, body) == [embedding_text(title, body)]
|
||||
|
||||
|
||||
# --- keeping it current: a log write refreshes the task's vectors ------------
|
||||
#
|
||||
# The shaper above is pure. These pin that the writers actually call it — a
|
||||
# task whose logs are in the document but never re-indexed after one is written
|
||||
# is #4241's half-surface one layer down: the entry is readable, and the search
|
||||
# still answers as though it were never written.
|
||||
|
||||
from unittest.mock import MagicMock, patch # noqa: E402
|
||||
|
||||
import pytest # noqa: E402
|
||||
|
||||
from scribe.services import task_logs as svc # noqa: E402
|
||||
from tests.helpers import ( # noqa: E402
|
||||
fake_note, make_mock_session, session_returning,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_writing_a_work_log_refreshes_the_tasks_embedding():
|
||||
note = fake_note(id=7, title="A task", body="prose", is_task=True)
|
||||
session = session_returning(note)
|
||||
with (
|
||||
patch.object(svc, "async_session", return_value=session),
|
||||
patch("scribe.services.notes.embed_note") as embed,
|
||||
):
|
||||
await svc.create_log(42, 7, "what I tried")
|
||||
embed.assert_called_once()
|
||||
assert embed.call_args.args[0] is note
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_editing_a_work_log_refreshes_it_too():
|
||||
"""An edited log whose vectors still carry the old wording keeps matching
|
||||
what it no longer says — the stale-vector case `upsert_note_embedding`
|
||||
already refuses to leave behind for a body."""
|
||||
note = fake_note(id=7, title="A task", body="prose", is_task=True)
|
||||
log = MagicMock(id=3, task_id=7)
|
||||
session = make_mock_session()
|
||||
found_log, found_note = MagicMock(), MagicMock()
|
||||
found_log.scalars.return_value.first.return_value = log
|
||||
found_note.scalars.return_value.first.return_value = note
|
||||
session.execute.side_effect = [found_log, found_note]
|
||||
with (
|
||||
patch.object(svc, "async_session", return_value=session),
|
||||
patch("scribe.services.notes.embed_note") as embed,
|
||||
):
|
||||
await svc.update_log(42, 3, content="reworded")
|
||||
embed.assert_called_once_with(note)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deleting_a_work_log_refreshes_it_too():
|
||||
"""And reads the task id BEFORE the row goes — after the delete there is
|
||||
nothing left to ask which task it belonged to."""
|
||||
note = fake_note(id=7, title="A task", body="prose", is_task=True)
|
||||
log = MagicMock(id=3, task_id=7)
|
||||
session = make_mock_session()
|
||||
found_log, found_note = MagicMock(), MagicMock()
|
||||
found_log.scalars.return_value.first.return_value = log
|
||||
found_note.scalars.return_value.first.return_value = note
|
||||
session.execute.side_effect = [found_log, found_note]
|
||||
with (
|
||||
patch.object(svc, "async_session", return_value=session),
|
||||
patch("scribe.services.notes.embed_note") as embed,
|
||||
):
|
||||
assert await svc.delete_log(42, 3) is True
|
||||
embed.assert_called_once_with(note)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_failed_refresh_does_not_fail_the_log_that_saved():
|
||||
"""Indexing never breaks a write. The startup backfill is the backstop."""
|
||||
session = session_returning(fake_note(id=7, title="T", body="b", is_task=True))
|
||||
with (
|
||||
patch.object(svc, "async_session", return_value=session),
|
||||
patch("scribe.services.notes.embed_note", side_effect=RuntimeError("boom")),
|
||||
):
|
||||
log = await svc.create_log(42, 7, "what I tried")
|
||||
assert log is not None
|
||||
session.commit.assert_awaited()
|
||||
@@ -0,0 +1,379 @@
|
||||
"""The work log is READABLE from the tools an agent opens a task with (#4241).
|
||||
|
||||
`add_task_log` wrote to a surface no agent could read back: the entries
|
||||
reached the web UI through routes/task_logs.py and nothing else. So a session
|
||||
opening a task saw only the body — a claim written once, before the work —
|
||||
with the record written during it invisible beside it. A stale body had
|
||||
nothing to contradict it, and shipped work got rebuilt.
|
||||
|
||||
These pin the read, not the write. The payload shape is tested against the
|
||||
real `work_log_payload`; the three tools are tested by re-patching the arm
|
||||
that tests/conftest.py stubs autouse, so each one is exercised live here and
|
||||
nowhere else.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.mcp.tools.milestones import get_milestone
|
||||
from scribe.mcp.tools.tasks import (
|
||||
elide, get_task, list_tasks, work_log_payload,
|
||||
)
|
||||
from scribe.services.notes import brief_row
|
||||
# Bound at IMPORT time, which is before any fixture runs — so these names
|
||||
# keep pointing at the real implementations even though conftest's autouse
|
||||
# _no_task_log_arm replaces the module attributes for every test. Reaching
|
||||
# them as `task_logs.logs_for_task` would get the stub and test nothing.
|
||||
from scribe.services.task_logs import (
|
||||
count_logs_for_task,
|
||||
log_counts_for_tasks,
|
||||
logs_for_task,
|
||||
)
|
||||
from tests.helpers import fake_task, make_mock_session
|
||||
|
||||
pytestmark = pytest.mark.usefixtures("_bind_user")
|
||||
|
||||
|
||||
def fake_log(log_id: int = 1, content: str = "did the thing", **attrs):
|
||||
"""A stand-in TaskLog. `to_dict` is what the payload builder consumes."""
|
||||
row = {
|
||||
"id": log_id,
|
||||
"task_id": 1,
|
||||
"user_id": 7,
|
||||
"content": content,
|
||||
"duration_minutes": None,
|
||||
"created_at": "2026-09-21T12:00:00+00:00",
|
||||
"updated_at": "2026-09-21T12:00:00+00:00",
|
||||
}
|
||||
row.update(attrs)
|
||||
log = MagicMock()
|
||||
log.to_dict = MagicMock(return_value=row)
|
||||
log.content = row["content"]
|
||||
log.id = row["id"]
|
||||
return log
|
||||
|
||||
|
||||
def _live_arm(logs=(), total=0, counts=None):
|
||||
"""Re-patch conftest's autouse stub so the arm under test is live."""
|
||||
return (
|
||||
patch("scribe.services.task_logs.logs_for_task",
|
||||
AsyncMock(return_value=list(logs))),
|
||||
patch("scribe.services.task_logs.count_logs_for_task",
|
||||
AsyncMock(return_value=total)),
|
||||
patch("scribe.services.task_logs.log_counts_for_tasks",
|
||||
AsyncMock(return_value=counts or {})),
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The payload shape
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_no_logs_reads_as_a_count_not_a_missing_key():
|
||||
""""This task has no record" and "you were not shown the record" are
|
||||
different answers, and a caller has to be able to tell them apart."""
|
||||
out = work_log_payload([], 0, 800)
|
||||
assert out == {"total": 0, "entries": []}
|
||||
# No advice on an empty log: there is no body/record disagreement to warn
|
||||
# about, and a standing paragraph on every task is noise.
|
||||
assert "advice" not in out
|
||||
|
||||
|
||||
def test_entries_carry_the_advice_that_the_log_outranks_the_body():
|
||||
out = work_log_payload([fake_log()], 1, 800)
|
||||
assert "advice" in out
|
||||
advice = out["advice"].lower()
|
||||
assert "claim" in advice and "record" in advice
|
||||
# The operative instruction: which one to believe when they disagree.
|
||||
assert "later" in advice
|
||||
|
||||
|
||||
def test_elision_keeps_the_end_where_the_conclusion_lives():
|
||||
"""The whole reason this is not `text[:n]`.
|
||||
|
||||
Prose does not put its conclusion first. An entry that opens with what was
|
||||
attempted and closes with "so this shipped in 04775c3" loses the one
|
||||
sentence that answers the question if the cut is taken from the head — and
|
||||
a `truncated: true` flag tells a reader that something went, never whether
|
||||
it mattered. Both ends survive, and the gap says how much is missing.
|
||||
"""
|
||||
text = "Tried the form column. " + ("m" * 2000) + " CONCLUSION: shipped in 04775c3."
|
||||
out, cut = elide(text, 300)
|
||||
assert cut is True
|
||||
assert out.startswith("Tried the form column.")
|
||||
assert out.rstrip().endswith("shipped in 04775c3.")
|
||||
assert "characters omitted" in out
|
||||
|
||||
|
||||
def test_elision_states_how_much_went():
|
||||
out, _ = elide("a" * 1000, 100)
|
||||
assert "900 characters omitted" in out
|
||||
|
||||
|
||||
def test_short_text_is_returned_untouched():
|
||||
out, cut = elide("brief", 300)
|
||||
assert out == "brief"
|
||||
assert cut is False
|
||||
|
||||
|
||||
def test_the_newest_entry_arrives_whole_and_older_ones_are_headlines():
|
||||
"""The newest entry answers "where does this stand", which is the question
|
||||
the block exists for — so it is not competing for budget with history."""
|
||||
newest = fake_log(9, content="N" * 3000)
|
||||
older = fake_log(8, content="O" * 3000)
|
||||
out = work_log_payload([newest, older], 2, chars=800, latest_chars=4000)
|
||||
assert out["entries"][0]["content"] == "N" * 3000
|
||||
assert "truncated" not in out["entries"][0]
|
||||
assert out["entries"][1]["truncated"] is True
|
||||
assert out["entries"][1]["full_length"] == 3000
|
||||
|
||||
|
||||
def test_even_the_newest_entry_is_capped_somewhere():
|
||||
out = work_log_payload([fake_log(content="x" * 9000)], 1, chars=800,
|
||||
latest_chars=4000)
|
||||
entry = out["entries"][0]
|
||||
assert entry["truncated"] is True
|
||||
assert entry["full_length"] == 9000
|
||||
assert "read_all" in out
|
||||
|
||||
|
||||
def test_read_all_says_the_window_was_chosen_by_recency_not_relevance():
|
||||
"""A pointer to the fuller read is only useful if the reader knows the
|
||||
shown part was not selected for being the pertinent part."""
|
||||
out = work_log_payload([fake_log(content="x" * 9000)], 1, chars=800,
|
||||
latest_chars=4000)
|
||||
assert "relevance" in out["read_all"]
|
||||
|
||||
|
||||
def test_total_is_the_record_and_entries_are_the_window():
|
||||
"""The bug this guards: reporting len(entries) as the total, so "the last
|
||||
three of nine" is indistinguishable from "three"."""
|
||||
out = work_log_payload([fake_log(1), fake_log(2), fake_log(3)], 9, 800)
|
||||
assert out["total"] == 9
|
||||
assert len(out["entries"]) == 3
|
||||
assert out["not_shown"] == 6
|
||||
assert "get_task" in out["read_all"]
|
||||
|
||||
|
||||
def test_an_untruncated_full_window_advertises_nothing_further():
|
||||
out = work_log_payload([fake_log(1), fake_log(2)], 2, 800)
|
||||
assert "not_shown" not in out
|
||||
assert "read_all" not in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# get_task — the tool that failed
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_task_returns_the_work_log():
|
||||
fake = fake_task(id=4208, title="stale body", parent_id=None)
|
||||
p1, p2, p3 = _live_arm(logs=[fake_log(content="partially shipped in 04775c3")],
|
||||
total=1)
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user",
|
||||
AsyncMock(return_value=(fake, "owner"))), p1, p2, p3:
|
||||
out = await get_task(task_id=4208)
|
||||
assert "work_log" in out
|
||||
assert out["work_log"]["total"] == 1
|
||||
assert "04775c3" in out["work_log"]["entries"][0]["content"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_task_says_a_task_has_no_log_rather_than_omitting_the_key():
|
||||
fake = fake_task(id=1, parent_id=None)
|
||||
p1, p2, p3 = _live_arm(logs=[], total=0)
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user",
|
||||
AsyncMock(return_value=(fake, "owner"))), p1, p2, p3:
|
||||
out = await get_task(task_id=1)
|
||||
assert out["work_log"] == {"total": 0, "entries": []}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_task_counts_every_entry_not_just_the_window():
|
||||
fake = fake_task(id=1, parent_id=None)
|
||||
window = [fake_log(i) for i in (9, 8, 7)]
|
||||
p1, p2, p3 = _live_arm(logs=window, total=9)
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user",
|
||||
AsyncMock(return_value=(fake, "owner"))), p1, p2, p3:
|
||||
out = await get_task(task_id=1)
|
||||
assert out["work_log"]["total"] == 9
|
||||
assert out["work_log"]["not_shown"] == 6
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_task_default_window_is_three_entries():
|
||||
fake = fake_task(id=1, parent_id=None)
|
||||
limit_seen = {}
|
||||
|
||||
async def _capture_limit(uid, task_id, limit=0):
|
||||
limit_seen["limit"] = limit
|
||||
return []
|
||||
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user",
|
||||
AsyncMock(return_value=(fake, "owner"))), \
|
||||
patch("scribe.services.task_logs.logs_for_task", _capture_limit), \
|
||||
patch("scribe.services.task_logs.count_logs_for_task",
|
||||
AsyncMock(return_value=0)):
|
||||
await get_task(task_id=1)
|
||||
assert limit_seen["limit"] == 3
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_log_limit_zero_asks_for_every_entry_and_skips_the_count_query():
|
||||
"""0 means "all" here because it means "no cap" in the service it calls.
|
||||
With no cap the rows ARE the total, so a second query would be waste."""
|
||||
fake = fake_task(id=1, parent_id=None)
|
||||
counter = AsyncMock(return_value=99)
|
||||
p1, _, p3 = _live_arm(logs=[fake_log(1), fake_log(2)])
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user",
|
||||
AsyncMock(return_value=(fake, "owner"))), \
|
||||
p1, p3, patch("scribe.services.task_logs.count_logs_for_task", counter):
|
||||
out = await get_task(task_id=1, log_limit=0)
|
||||
assert out["work_log"]["total"] == 2
|
||||
assert counter.await_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_task_docstring_tells_a_reader_the_log_outranks_the_body():
|
||||
"""#2846: the docstring IS the agent-facing contract. A `work_log` key
|
||||
that nothing tells the reader to prefer over a stale body is the same
|
||||
failure one layer up."""
|
||||
doc = (get_task.__doc__ or "").lower()
|
||||
assert "work_log" in doc
|
||||
assert "body" in doc
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The list surfaces — which rows carry a record
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_tasks_rows_carry_a_zero_filled_log_count():
|
||||
rows = [fake_task(id=1, milestone_id=None), fake_task(id=2, milestone_id=None)]
|
||||
_, _, p3 = _live_arm(counts={1: 4})
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.list_notes",
|
||||
AsyncMock(return_value=(rows, 2))), p3:
|
||||
out = await list_tasks()
|
||||
assert out["tasks"][0]["log_count"] == 4
|
||||
# Zero-filled, not omitted: a missing key would read as "no logs" on every
|
||||
# row rather than on the rows that have none.
|
||||
assert out["tasks"][1]["log_count"] == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_tasks_counts_the_page_in_one_query():
|
||||
rows = [fake_task(id=i, milestone_id=None) for i in range(1, 6)]
|
||||
counter = AsyncMock(return_value={})
|
||||
with patch("scribe.mcp.tools.tasks.notes_svc.list_notes",
|
||||
AsyncMock(return_value=(rows, 5))), \
|
||||
patch("scribe.services.task_logs.log_counts_for_tasks", counter):
|
||||
await list_tasks()
|
||||
assert counter.await_count == 1
|
||||
assert sorted(counter.await_args.args[1]) == [1, 2, 3, 4, 5]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_milestone_steps_carry_a_log_count():
|
||||
"""A step's status is set by hand, and a plan is exactly where that goes
|
||||
stale — so a plan reader needs to see which steps have a record."""
|
||||
milestone = MagicMock(id=385, title="Lessons", project_id=2)
|
||||
milestone.to_dict = MagicMock(return_value={"id": 385, "title": "Lessons"})
|
||||
steps = [fake_task(id=3735, milestone_id=385),
|
||||
fake_task(id=3736, milestone_id=385)]
|
||||
counter = AsyncMock(return_value={3735: 2})
|
||||
with patch("scribe.mcp.tools.milestones.milestones_svc.get_milestone",
|
||||
AsyncMock(return_value=milestone)), \
|
||||
patch("scribe.mcp.tools.milestones.milestones_svc.get_milestone_progress",
|
||||
AsyncMock(return_value={})), \
|
||||
patch("scribe.mcp.tools.milestones.notes_svc.list_notes",
|
||||
AsyncMock(return_value=(steps, 2))), \
|
||||
patch("scribe.mcp.tools.milestones.rulebooks_svc.get_applicable_rules",
|
||||
AsyncMock(return_value=[])), \
|
||||
patch("scribe.mcp.tools.milestones.rulebooks_svc.rules_payload",
|
||||
MagicMock(return_value={})), \
|
||||
patch("scribe.services.task_logs.log_counts_for_tasks", counter):
|
||||
out = await get_milestone(milestone_id=385)
|
||||
assert out["steps"][0]["log_count"] == 2
|
||||
assert out["steps"][1]["log_count"] == 0
|
||||
|
||||
|
||||
def test_brief_row_without_a_mapping_is_unchanged():
|
||||
"""The other three brief_row callers (list_notes, list_system_records)
|
||||
pass no counts and must not grow a misleading zero."""
|
||||
row = brief_row(fake_task(id=1, milestone_id=None, due_date=None))
|
||||
assert "log_count" not in row
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The service read — scoped by who may read the TASK (rule #78)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_logs_for_task_is_not_filtered_by_who_wrote_the_entry():
|
||||
"""`list_logs` filters TaskLog.user_id == user_id, which hands a shared
|
||||
collaborator an empty list that reads as "no work has been done". The work
|
||||
log belongs to the task."""
|
||||
session = make_mock_session()
|
||||
session.execute = AsyncMock(return_value=MagicMock(
|
||||
scalars=MagicMock(return_value=MagicMock(all=MagicMock(return_value=[])))
|
||||
))
|
||||
with patch("scribe.services.task_logs.can_read_note",
|
||||
AsyncMock(return_value=True)), \
|
||||
patch("scribe.services.task_logs.async_session",
|
||||
MagicMock(return_value=session)):
|
||||
await logs_for_task(7, 42)
|
||||
# The WHERE clause specifically — `user_id` is a selected COLUMN of the
|
||||
# row and appears in every SELECT, so asserting against the whole compiled
|
||||
# statement tests the projection rather than the scoping.
|
||||
where = str(session.execute.await_args.args[0].whereclause)
|
||||
assert "task_logs.task_id" in where
|
||||
assert "task_logs.user_id" not in where
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_logs_for_task_refuses_a_task_the_caller_cannot_read():
|
||||
"""Unscoped would have been the mirror-image hole: rule #78 is about
|
||||
routing the question through the access layer, in both directions."""
|
||||
opened = MagicMock()
|
||||
with patch("scribe.services.task_logs.can_read_note",
|
||||
AsyncMock(return_value=False)), \
|
||||
patch("scribe.services.task_logs.async_session", opened):
|
||||
out = await logs_for_task(7, 42)
|
||||
assert out == []
|
||||
assert opened.call_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_count_for_an_unreadable_task_is_zero_not_a_leak():
|
||||
opened = MagicMock()
|
||||
with patch("scribe.services.task_logs.can_read_note",
|
||||
AsyncMock(return_value=False)), \
|
||||
patch("scribe.services.task_logs.async_session", opened):
|
||||
assert await count_logs_for_task(7, 42) == 0
|
||||
assert opened.call_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_log_counts_for_tasks_scopes_by_readability_in_the_same_query():
|
||||
"""A per-row can_read_note would be the N+1 this function exists to avoid,
|
||||
so the permission goes in as set membership instead."""
|
||||
session = make_mock_session()
|
||||
session.execute = AsyncMock(return_value=MagicMock(
|
||||
all=MagicMock(return_value=[(1, 3)])
|
||||
))
|
||||
with patch("scribe.services.task_logs.async_session",
|
||||
MagicMock(return_value=session)):
|
||||
out = await log_counts_for_tasks(7, [1, 2])
|
||||
assert out == {1: 3}
|
||||
stmt = str(session.execute.await_args.args[0])
|
||||
assert "notes" in stmt.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_ids_asks_the_database_nothing():
|
||||
from scribe.services import task_logs as svc
|
||||
|
||||
opened = MagicMock()
|
||||
with patch("scribe.services.task_logs.async_session", opened):
|
||||
assert await log_counts_for_tasks(7, []) == {}
|
||||
assert opened.call_count == 0
|
||||
Reference in New Issue
Block a user