From 21a409eac1bd646589e312844ad1fb4bc5bc6fc5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 21 Oct 2026 20:22:42 -0400 Subject: [PATCH] docs(6205): knowledge comments state the reason, not the instance that taught it The ACL header states the share behaviour without its date and decision ids; the facet table, door map, floor constant and facet-count comments state why they are single sources instead of retelling how copies drifted; this instance's counts (legacy plan-tasks, issue share) are removed. Co-Authored-By: Claude Opus 5.5 --- src/scribe/services/knowledge.py | 94 ++++++++++++++------------------ 1 file changed, 42 insertions(+), 52 deletions(-) diff --git a/src/scribe/services/knowledge.py b/src/scribe/services/knowledge.py index 05a311e2..03246090 100644 --- a/src/scribe/services/knowledge.py +++ b/src/scribe/services/knowledge.py @@ -1,8 +1,7 @@ """Knowledge service — one query across every record kind Scribe holds. -ACL (rules #47/#78, decision note 2094): these queries were owner-only until -2026-07-25, which meant a record shared with you could be opened by id but never -*found*. They now honour shares — at two different widths: +ACL (services/access.py): these queries honour shares, so a record shared with +you can be *found* as well as opened by id — at two different widths: - **searching** (a `q` the caller typed) uses the full read scope, so a record shared directly with you is findable when you go looking for it. BOTH halves @@ -32,7 +31,7 @@ logger = logging.getLogger(__name__) _SNIPPET_LEN = 200 -# --- the location filter (reverse lookup, #2083) ------------------------------ +# --- the location filter (reverse lookup) ------------------------------------ # # ONE predicate in two dialects: "this record carries a location matching every # part asked for." The SQL form runs in the browse arm and the keyword arm, @@ -79,7 +78,7 @@ def location_matches(data: dict | None, parts: dict[str, str]) -> bool: _path_matches((loc.get(key) or "").strip(), want) if key == "path" # Repo names are recorded free-form ("Scribe" / "FabledScribe" / - # "fabledscribe") — case is never the distinguishing thing (#2874). + # "fabledscribe") — case is never the distinguishing thing. else (loc.get(key) or "").strip().lower() == want.lower() if key == "repo" else (loc.get(key) or "").strip() == want @@ -106,7 +105,7 @@ def location_jsonpath(parts: dict[str, str]) -> str: prefix = json.dumps(want.rstrip("/") + "/") filters.append(f"(@.path == {literal} || @.path starts with {prefix})") elif key == "repo": - # Case-insensitive, anchored, regex-escaped (#2874) — mirrors the + # Case-insensitive, anchored, regex-escaped — mirrors the # Python dialect's .lower() compare. pattern = json.dumps("^" + re.escape(want) + "$") filters.append(f'(@.repo like_regex {pattern} flag "i")') @@ -120,7 +119,7 @@ def _location_clause(parts: dict[str, str]): return Note.data.path_exists(location_jsonpath(parts)) -# --- drift-check filter (#2086) ---------------------------------------------- +# --- drift-check filter ------------------------------------------------------ # `verification` selects on the drift-check verdict stored in `data.verification` # (see services/snippets.py). Statuses are the service's own constants; the two # composite values are what the operator actually asks for. @@ -154,7 +153,7 @@ def verification_matches(data: dict | None, value: str) -> bool: if not status: return want == "unverified" expired = verdict.get("code_sha") != (data or {}).get("code_sha") - # A push touched the recorded location since the verdict (#2691): the repo + # A push touched the recorded location since the verdict: the repo # moved under it, so it needs a look even though it hasn't failed. invalidated = bool(verdict.get("invalidated_by")) if want == "unverified": @@ -171,7 +170,7 @@ def verification_matches(data: dict | None, value: str) -> bool: _VERIFY_DRIFTED_JSONPATH = '$.verification ? (@.status != "ok")' _VERIFY_EXPIRED_JSONPATH = "$ ? (@.verification.code_sha != @.code_sha)" _VERIFY_ANY_JSONPATH = "$.verification" -# A push touched the recorded location since the verdict (#2691). +# A push touched the recorded location since the verdict. _VERIFY_INVALIDATED_JSONPATH = "$.verification.invalidated_by" @@ -189,7 +188,7 @@ def _verification_clause(value: str): return Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH) if want == "attention": # Everything worth looking at: a failing verdict, an expired one, OR - # one whose recorded location a push has since touched (#2691). + # one whose recorded location a push has since touched. return or_( Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH), and_(has_verdict, Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH)), @@ -221,10 +220,9 @@ def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict: 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). + The head of the body on every row alike (`body[:_SNIPPET_LEN]`) would show + a record matched on its sixth paragraph its first, which the search had + already scored lower, with nothing marking the cut. """ item: dict = { "id": note.id, @@ -244,7 +242,7 @@ def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict: "created_at": iso(note.created_at), "updated_at": iso(note.updated_at), } - # Drift verdict (#2086), when one has been recorded. Included here rather + # Drift verdict, when one has been recorded. Included here rather # than decorated on by the snippet layer because `current` is derivable from # `data` alone — the verdict's code_sha against the row's — so this needs no # body parsing and stays a plain projection of the column. Omitted entirely @@ -253,7 +251,7 @@ def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict: # Snippet language, same reasoning as the verdict below: a plain projection of # the `data` mirror, no body parsing. Needed because a prior-art hit in a # DIFFERENT language than the file being written is useful as the shape of a - # solution but must not be mistaken for code to paste (#2244) — and the caller + # solution but must not be mistaken for code to paste — and the caller # can only say "different" if the language is on the item. language = (note.data or {}).get("language") if note.data else None if language: @@ -267,8 +265,8 @@ def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict: trigger = (note.data or {}).get("when_to_apply") if note.data else None if trigger: item["when_to_apply"] = trigger - # A snippet's, for the same reason — and since milestone 427 the title no - # longer carries it, so without this a list shows names with no situation. + # A snippet's, for the same reason — the stored title does not carry it, + # so without this a list shows names with no situation. usage = (note.data or {}).get("when_to_use") if note.data else None if usage: item["when_to_use"] = usage @@ -283,7 +281,7 @@ def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict: "path": verdict.get("path"), } # Present only when a push has touched the recorded location since the - # verdict (#2691) — the "recheck me" marker, cleared by re-verifying. + # verdict — the "recheck me" marker, cleared by re-verifying. if verdict.get("invalidated_by"): item["verification"]["invalidated_by"] = verdict["invalidated_by"] @@ -305,17 +303,15 @@ def _note_to_item(note: Note, chunks: dict[int, dict] | None = None) -> dict: # why this is a table rather than a chain of ifs. Each entry is # (is_task, the value pinned on that axis); None pins nothing, i.e. every task. # -# It is a table because the alternative had already gone wrong. The predicate -# was written three times — a SQL if-chain, a Python if-chain over semantic -# candidates, and a ternary computing the `is_task` pre-filter — and the three -# only agreed by luck. Adding `issue` to the SQL arm alone (the obvious edit, -# and the one #3128 was about to make) would have set the pre-filter to -# is_task=False, handed the Python arm a candidate set containing no tasks at -# all, and returned an empty semantic half for the Issues facet forever, with -# nothing red anywhere. A new facet is now one row here. +# It is a table because the predicate has three dialects — SQL, a Python test +# over semantic candidates, and the `is_task` pre-filter — and three hand-kept +# copies agree only by luck. Adding `issue` to the SQL arm alone would set the +# pre-filter to is_task=False, hand the Python arm a candidate set containing +# no tasks at all, and return an empty semantic half for the Issues facet +# forever, with nothing red anywhere. A new facet is one row here. # -# `plan` is retired (0066) but kept: 90 legacy plan-tasks exist and a facet -# they answer to costs one line. It simply has no chip in the UI any more. +# `plan` is retired but kept: legacy plan-tasks may exist, and a facet they +# answer to costs one line. It has no chip in the UI. _FACETS: dict[str, tuple[bool, str | None]] = { "task": (True, None), "work": (True, "work"), @@ -326,11 +322,10 @@ _FACETS: dict[str, tuple[bool, str | None]] = { "process": (False, "process"), "snippet": (False, "snippet"), # A transferable insight, keyed by the situation it applies to rather than - # by its topic (milestone 385). It reaches the browse surface through this - # one entry: the door's validation, the counts and both dialects of the - # type filter are all generated from this table, which is the property - # #3161 asked for so that adding a kind is a single edit rather than four - # that must agree. + # by its topic. It reaches the browse surface through this one entry: the + # door's validation, the counts and both dialects of the type filter are + # all generated from this table, so adding a kind is a single edit rather + # than four that must agree. "lesson": (False, "lesson"), } @@ -358,7 +353,7 @@ def facet_is_task(note_type: str | None) -> bool | None: Used to narrow the semantic candidate set before it is fetched. Reads the same table `_apply_type_filter` and `matches_facet` read, so the pre-filter - can no longer disagree with the predicate it is meant to anticipate. + cannot disagree with the predicate it is meant to anticipate. """ if not note_type: return None @@ -388,7 +383,7 @@ def search_filters_for(facet: str) -> dict: 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). + keeps "adding a kind" a single edit. 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. @@ -421,10 +416,9 @@ _CONTENT_TYPE_BROAD = { 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). + Lives here, beside `_FACETS`, because a door that keeps its own map drifts: + one door offers two kinds while another offers nine. Derived, so adding a + kind stays one edit. 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 @@ -611,18 +605,15 @@ async def _semantic_knowledge_search( 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 - # exact pair where one moves and the other doesn't (#2463). + # The shared interactive floor, one constant, so no copy of the + # number can move without the others. threshold=INTERACTIVE_SEARCH_THRESHOLD, is_task=is_task_filter, project_id=project_id, ) - # The human's MAIN search surface, and it logged nothing — so - # retrieval_logs claimed the web's search was /api/search, a narrower - # path with (measured) zero frontend consumers. Distinct source, so + # The human's MAIN search surface, logged under its own source so # threshold tuning can include or exclude human queries deliberately - # rather than by accident (#2463). + # rather than by accident. from scribe.services.retrieval_telemetry import record_retrieval record_retrieval( user_id=user_id, source="browse_search", query=q, @@ -698,11 +689,10 @@ async def get_knowledge_counts(user_id: int, tags: list[str] | None = None) -> d stmt = stmt.where(Note.tags.contains([tag])) return stmt - # One grouped query per typing axis. The task half used to be a count - # for 'task' plus a second count for 'plan', which is why 'issue' — - # 17% of every task here — had no number to show: each kind needed its - # own query and nobody added one. Grouping by task_kind counts every - # kind, including ones added later, for the same two round-trips. + # One grouped query per typing axis. A count per kind needs a new + # query for every kind and leaves a kind nobody added with no number; + # grouping by task_kind counts every kind, including ones added later, + # in two round-trips. non_task = _scoped( select(Note.note_type, func.count(Note.id)) .where(Note.status.is_(None))