diff --git a/src/scribe/mcp/tools/notes.py b/src/scribe/mcp/tools/notes.py index c9a0aa8..f1de8b5 100644 --- a/src/scribe/mcp/tools/notes.py +++ b/src/scribe/mcp/tools/notes.py @@ -32,10 +32,15 @@ async def list_notes( ) -> dict: """List notes (non-task documents) stored in Scribe. - Optionally filter by a single tag (plain string, no # prefix) or a keyword - search against title and body. Results are ordered by last-updated descending. + Optionally filter by a single tag (plain string, no # prefix) or by + `search_text`, which matches on MEANING — the same semantic match every + search surface uses (#2462; keyword ILIKE is only the fallback when the + embedder is down). With `search_text`, results order by relevance; without + it, by last-updated descending. Reaches your records plus shared-project + records, like every other list. - Use search for semantic/meaning-based lookup instead of exact keyword search. + Prefer `search` for a pure lookup — it returns scores and reaches + everything readable; this adds the lifecycle filters on top. Args: project_id: Scope to one project. PASS THE ACTIVE PROJECT'S ID whenever a diff --git a/src/scribe/routes/search.py b/src/scribe/routes/search.py index bdfe6b5..cba492c 100644 --- a/src/scribe/routes/search.py +++ b/src/scribe/routes/search.py @@ -4,12 +4,14 @@ from quart import Blueprint, jsonify, request from scribe.auth import login_required, get_current_user_id from scribe.services.access import owner_names_for +from scribe.services.embeddings import ( + INTERACTIVE_SEARCH_THRESHOLD as _REST_SEARCH_THRESHOLD, +) from scribe.services.embeddings import semantic_search_notes from scribe.services.retrieval_telemetry import record_retrieval -# This route searches with a looser floor than the MCP tool default — it powers -# an interactive feed where loosely-related hits still have value. -_REST_SEARCH_THRESHOLD = 0.3 +# The interactive floor lives in embeddings.py now, shared with Browse search +# and the list views' semantic `q` — one number, one rationale (#2463). search_bp = Blueprint("search", __name__, url_prefix="/api/search") diff --git a/src/scribe/services/embeddings.py b/src/scribe/services/embeddings.py index 13b576a..21e1483 100644 --- a/src/scribe/services/embeddings.py +++ b/src/scribe/services/embeddings.py @@ -31,6 +31,15 @@ logger = logging.getLogger(__name__) # loosely-related results that pad the sidebar without adding real value. _SIMILARITY_THRESHOLD = 0.45 +# The floor for INTERACTIVE, human-facing feeds — REST /api/search, Browse +# search, and the list views' semantic `q`. Deliberately looser than the agent +# default above: a human scanning a result list gets value from loosely-related +# hits an agent would be misled by. One constant, because this number was +# written as `0.3` in two files with the reasoning attached to only one of +# them — the exact shape where a later tuner moves one and not the other +# (#2463 finding 3). +INTERACTIVE_SEARCH_THRESHOLD = 0.3 + # Public alias so callers (and telemetry) can record the effective default # threshold without reaching for the underscored name. DEFAULT_SIMILARITY_THRESHOLD = _SIMILARITY_THRESHOLD diff --git a/src/scribe/services/knowledge.py b/src/scribe/services/knowledge.py index f62cbba..dea6bbc 100644 --- a/src/scribe/services/knowledge.py +++ b/src/scribe/services/knowledge.py @@ -396,14 +396,20 @@ async def _semantic_knowledge_search( # case a semantic search exists to serve. semantic_notes: list[Note] = [] try: - from scribe.services.embeddings import semantic_search_notes + from scribe.services.embeddings import ( + INTERACTIVE_SEARCH_THRESHOLD, + semantic_search_notes, + ) is_task_filter = True if note_type in ("task", "plan") else (False if note_type else None) candidates = await semantic_search_notes( user_id=user_id, scope="read", query=q, limit=min(200, limit * 4), - threshold=0.3, + # The shared interactive floor — this was a bare `0.3` while + # routes/search.py had the same number as a commented constant, the + # exact pair where one moves and the other doesn't (#2463). + threshold=INTERACTIVE_SEARCH_THRESHOLD, is_task=is_task_filter, project_id=project_id, ) diff --git a/src/scribe/services/notes.py b/src/scribe/services/notes.py index 649a28d..ee9b130 100644 --- a/src/scribe/services/notes.py +++ b/src/scribe/services/notes.py @@ -188,10 +188,32 @@ async def list_notes( limit: int = 50, offset: int = 0, ) -> tuple[list[Note], int]: + """Lifecycle-shaped listing. Two contracts worth knowing: + + VISIBILITY is the shared browse clause (#47, #2462): the caller's own + records plus anything in a project they can reach — the same reach + query_knowledge has always had. Before this, list_notes was silently + owner-only, so a task in a shared project was invisible in list_tasks, + enter_project's open-task list and the web UI while the same project's + notes appeared. Browse, not read, per decision #2094: an ambient list must + never surface a record someone shared one-to-one — those stay search-only. + + `q` is SEMANTIC (operator decision, 2026-08-06: "make it match") — the same + meaning-based match as Browse search, at the interactive floor. When `q` is + present, relevance ordering wins and `sort` is ignored; a query is a + relevance claim and sorting its results by date would shuffle the answer. + Falls back to ILIKE substring match only when the embedder is unavailable — + degraded but never empty. Superseded records are not demoted here: the + penalty reorders a top-k, and reordering a paginated, counted list would + make page boundaries lie. The search surfaces carry the demotion. + """ + from scribe.services.access import notes_visibility_clause + + visible = notes_visibility_clause(user_id, "browse") async with async_session() as session: - query = select(Note).where(Note.user_id == user_id, Note.deleted_at.is_(None)) + query = select(Note).where(visible, Note.deleted_at.is_(None)) count_query = select(func.count(Note.id)).where( - Note.user_id == user_id, Note.deleted_at.is_(None) + visible, Note.deleted_at.is_(None) ) # Filter by task vs note @@ -202,14 +224,34 @@ async def list_notes( query = query.where(Note.status.is_(None)) count_query = count_query.where(Note.status.is_(None)) + semantic_order = None if q: - terms = _strip_type_nouns(q) - for term in terms: - escaped_term = term.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") - pattern = f"%{escaped_term}%" - term_filter = or_(Note.title.ilike(pattern), Note.body.ilike(pattern)) - query = query.where(term_filter) - count_query = count_query.where(term_filter) + query_vec = None + try: + from scribe.services.embeddings import get_embedding + query_vec = await get_embedding(q) + except Exception: + query_vec = None # embedder down → keyword fallback below + if query_vec is not None: + from scribe.models.embedding import NoteEmbedding + from scribe.services.embeddings import INTERACTIVE_SEARCH_THRESHOLD + distance = NoteEmbedding.embedding.cosine_distance(query_vec) + sem_filter = distance <= (1.0 - INTERACTIVE_SEARCH_THRESHOLD) + query = query.join( + NoteEmbedding, NoteEmbedding.note_id == Note.id + ).where(sem_filter) + count_query = count_query.join( + NoteEmbedding, NoteEmbedding.note_id == Note.id + ).where(sem_filter) + semantic_order = distance.asc() + else: + terms = _strip_type_nouns(q) + for term in terms: + escaped_term = term.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + pattern = f"%{escaped_term}%" + term_filter = or_(Note.title.ilike(pattern), Note.body.ilike(pattern)) + query = query.where(term_filter) + count_query = count_query.where(term_filter) if tags: for i, tag in enumerate(tags): @@ -275,11 +317,15 @@ async def list_notes( query = query.where(paused_filter) count_query = count_query.where(paused_filter) - sort_col = getattr(Note, sort, Note.updated_at) - if order == "asc": - query = query.order_by(sort_col.asc()) + if semantic_order is not None: + # A query is a relevance claim — see the docstring. + query = query.order_by(semantic_order) else: - query = query.order_by(sort_col.desc()) + sort_col = getattr(Note, sort, Note.updated_at) + if order == "asc": + query = query.order_by(sort_col.asc()) + else: + query = query.order_by(sort_col.desc()) query = query.limit(limit).offset(offset) diff --git a/tests/test_retrieval_scopes.py b/tests/test_retrieval_scopes.py index 3f4fa6b..5e11394 100644 --- a/tests/test_retrieval_scopes.py +++ b/tests/test_retrieval_scopes.py @@ -202,3 +202,46 @@ async def test_injected_menu_labels_the_record_kind(): assert "body" not in out["context"] # The header can't claim they're all notes when the markers say otherwise. assert "Scribe records" in lines[0] + + +def test_list_shaped_services_use_the_shared_visibility_clause(): + """Every service that LISTS notes must declare its reach through + notes_visibility_clause / browsable_notes_clause — never a bare owner + filter (#47, #2462). + + Source inspection, because this is the shape no behavioural test catches: + an owner-only list returns correct-looking rows and simply omits the shared + ones, which is how list_notes shipped owner-only for months while + query_knowledge beside it was deliberately browse-scoped. A task in a + shared project was invisible in list_tasks, enter_project and the web UI + while the same project's notes appeared. + """ + import ast + import inspect + import textwrap + + from scribe.services import knowledge, notes + + for fn, label in ((notes.list_notes, "notes.list_notes"), + (knowledge.query_knowledge, "knowledge.query_knowledge")): + source = textwrap.dedent(inspect.getsource(fn)) + assert ( + "notes_visibility_clause" in source + or "browsable_notes_clause" in source + ), ( + f"{label} does not reference a shared visibility clause — a bare " + f"owner filter silently hides shared-project records (#2462)" + ) + # And it must not ALSO carry a bare owner filter on the main query, + # which would quietly re-narrow whatever the clause granted. + tree = ast.parse(source) + for node in ast.walk(tree): + if (isinstance(node, ast.Compare) + and isinstance(node.left, ast.Attribute) + and node.left.attr == "user_id" + and isinstance(node.left.value, ast.Name) + and node.left.value.id == "Note"): + raise AssertionError( + f"{label} compares Note.user_id directly — reach must come " + f"from the shared clause, not a bare owner filter" + )