bvandeusen/FabledGrimoire
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases 53 Wiki Activity

docs(6205): notes comments state the reason, not the instance that taught it

Browse Source 
Issue, decision, milestone and operator-decision citations are removed;
"previously each route…" and "list_tasks once returned…" stories become the
present-tense reasons they taught. The note about the removed delete_note
now says plainly that there is deliberately none, and why.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
bvandeusen
2026-10-21 20:18:30 -04:00
co-authored by Claude Opus 5.5
parent cdb49b2f9f
commit 5d48e23285
1 changed files with 55 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/scribe/services/notes.py
+55 -63
View File
@@ -21,11 +21,9 @@ _PARSED_FROM_BODY = frozenset({"title", "body", "tags"})
def _mirror_recomposers() -> dict[str, Callable[[Note], dict]]: def _mirror_recomposers() -> dict[str, Callable[[Note], dict]]:
"""note_type -> the function that rebuilds that kind's derived `data`. """note_type -> the function that rebuilds that kind's derived `data`.
A TABLE rather than a chain of `if note_type == ...`, because the previous A TABLE rather than a chain of `if note_type == ...`, because a chain
shape tested one constant and the next kind with a derived mirror was tests the constants someone remembered, and the next kind with a derived
silently not covered — which is exactly what happened: the snippet guard mirror (body is authority, `data` is mirror) is silently not covered.
(#3128) was hard-coded, and lessons arrived in milestone 385 with the same
body-is-authority/`data`-is-mirror design and none of the protection.
The failure is invisible from here. Nothing raises, nothing logs; the row The failure is invisible from here. Nothing raises, nothing logs; the row
simply keeps answering queries from a mirror that no longer matches its simply keeps answering queries from a mirror that no longer matches its
@@ -49,7 +47,7 @@ def _mirror_recomposers() -> dict[str, Callable[[Note], dict]]:
} }
# Text fields where EMPTY MEANS NULL (milestone 317). The sweep's whole signal # Text fields where EMPTY MEANS NULL. The sweep's whole signal
# is `verify_with IS NULL` = "this is a decision, there is nothing to go and # is `verify_with IS NULL` = "this is a decision, there is nothing to go and
# check". An empty string that is not NULL makes a norm look like a constraint # check". An empty string that is not NULL makes a norm look like a constraint
# nobody has verified, forever — and it would sit at the top of the sweep, # nobody has verified, forever — and it would sit at the top of the sweep,
@@ -64,7 +62,7 @@ def guard_check_fields(status: str | None, note_type: str | None) -> None:
which fields a caller passed, so it also catches the sideways route: a which fields a caller passed, so it also catches the sideways route: a
checked note being turned into a task, which no per-field gate would see. checked note being turned into a task, which no per-field gate would see.
Raises rather than dropping silently, for minted_kind's reason (#3129) — a Raises rather than dropping silently, for minted_kind's reason — a
silently-corrected write is the defect that reasoning exists to end, and a silently-corrected write is the defect that reasoning exists to end, and a
caller putting a check on the wrong record has an idea an error corrects caller putting a check on the wrong record has an idea an error corrects
and a default hides. Lives at the service, not either door, so REST and MCP and a default hides. Lives at the service, not either door, so REST and MCP
@@ -92,12 +90,11 @@ def embed_note(note) -> None:
"""Refresh a note's embedding, fire-and-forget. """Refresh a note's embedding, fire-and-forget.
Lives HERE — at the service, not the route — so every caller gets it by Lives HERE — at the service, not the route — so every caller gets it by
construction. Previously each REST route made this call itself and the MCP construction. A door that forgot the call would leave its records out of
tools did not, so a record created through MCP stayed out of semantic search semantic search and auto-inject until the next restart's backfill ran —
and auto-inject until the next restart's backfill ran (#2056). That is invisible on an instance that redeploys constantly and permanent on one
invisible on an instance that redeploys constantly and permanent on one that that doesn't, which is the worst shape a bug can have: it only appears
doesn't, which is the worst shape a bug can have: it only appears where where nobody is looking.
nobody is looking.
Uses `note.user_id` — the OWNER — rather than the caller. Embeddings belong Uses `note.user_id` — the OWNER — rather than the caller. Embeddings belong
to the record, and a collaborator editing a shared note must refresh the to the record, and a collaborator editing a shared note must refresh the
@@ -110,16 +107,16 @@ def embed_note(note) -> None:
Detaching also means this task races anything that deletes the note out Detaching also means this task races anything that deletes the note out
from under it. That is not handled here: `upsert_note_embedding` claims from under it. That is not handled here: `upsert_note_embedding` claims
the note's row before touching its vectors, and loses if it can't (#3262). the note's row before touching its vectors, and loses if it can't.
""" """
try: try:
import asyncio import asyncio
from scribe.services.embeddings import document_title, upsert_note_embedding from scribe.services.embeddings import document_title, upsert_note_embedding
# Chunking and the empty-record gate live inside upsert_note_embedding — # Chunking and the empty-record gate live inside upsert_note_embedding —
# one path for every writer (#280). The title is the EMBEDDED one: a # one path for every writer. The title is the EMBEDDED one: a
# snippet's or lesson's trigger joins its name here, not in the stored # snippet's or lesson's trigger joins its name here, not in the stored
# title (milestone 427). # title.
asyncio.create_task( asyncio.create_task(
upsert_note_embedding( upsert_note_embedding(
note.id, note.user_id, note.id, note.user_id,
@@ -305,7 +302,7 @@ def build_note(
expires_when=expires_when, expires_when=expires_when,
) )
# A create that names a status is the same transition an update to it is # A create that names a status is the same transition an update to it is
# (#3683) — otherwise `create_task(status="in_progress")` writes a row no # — otherwise `create_task(status="in_progress")` writes a row no
# update could produce: started, with no `started_at`. # update could produce: started, with no `started_at`.
if status is not None: if status is not None:
apply_status_transition(note, user_id) apply_status_transition(note, user_id)
@@ -318,11 +315,11 @@ def apply_status_transition(note: Note, user_id: int | None = None) -> None:
Called by the update path whenever `status` is written and by `build_note` Called by the update path whenever `status` is written and by `build_note`
whenever a create names one, so a task created at a status is whenever a create names one, so a task created at a status is
indistinguishable from one that reached it by update. Two copies of "what indistinguishable from one that reached it by update. Two copies of "what
a status implies" are where the two drift, and #3683 was that drift. a status implies" are where the two drift.
`user_id` is whoever is making the change: reaching `in_progress` is the `user_id` is whoever is making the change: reaching `in_progress` is the
moment a session takes the work on, so it stamps that user's claim, and a moment a session takes the work on, so it stamps that user's claim, and a
status that ends or un-starts the work releases it (milestone 381). status that ends or un-starts the work releases it.
""" """
from scribe.services.task_claims import release_claim, stamp_claim from scribe.services.task_claims import release_claim, stamp_claim
@@ -382,15 +379,14 @@ async def list_notes(
) -> tuple[list[Note], int]: ) -> tuple[list[Note], int]:
"""Lifecycle-shaped listing. Two contracts worth knowing: """Lifecycle-shaped listing. Two contracts worth knowing:
VISIBILITY is the shared browse clause (#47, #2462): the caller's own VISIBILITY is the shared browse clause: the caller's own records plus
records plus anything in a project they can reach — the same reach anything in a project they can reach — the same reach query_knowledge has.
query_knowledge has always had. Before this, list_notes was silently Owner-only here would hide a task in a shared project from list_tasks,
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 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 notes appeared. Browse, not read: an ambient list must never surface a
never surface a record someone shared one-to-one — those stay search-only. record someone shared one-to-one — those stay search-only.
`q` is SEMANTIC (operator decision, 2026-08-06: "make it match") — the same `q` is SEMANTIC — the same
meaning-based match as Browse search, at the interactive floor. When `q` is 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 present, relevance ordering wins and `sort` is ignored; a query is a
relevance claim and sorting its results by date would shuffle the answer. relevance claim and sorting its results by date would shuffle the answer.
@@ -430,9 +426,9 @@ async def list_notes(
if query_vec is not None: if query_vec is not None:
from scribe.models.embedding import NoteEmbedding from scribe.models.embedding import NoteEmbedding
from scribe.services.embeddings import INTERACTIVE_SEARCH_THRESHOLD from scribe.services.embeddings import INTERACTIVE_SEARCH_THRESHOLD
# Best-chunk-per-note as a correlated MIN, not a join (#280): # Best-chunk-per-note as a correlated MIN, not a join:
# a note stores one embedding row PER CHUNK, so the plain join # a note stores one embedding row PER CHUNK, so a plain join
# this used to be would repeat a long note once per matching # would repeat a long note once per matching
# chunk — duplicated list rows and a total that counts chunks. # chunk — duplicated list rows and a total that counts chunks.
# This query is filter-heavy and paginated, never HNSW-bound, # This query is filter-heavy and paginated, never HNSW-bound,
# so the scalar subquery costs what the join did. # so the scalar subquery costs what the join did.
@@ -528,7 +524,7 @@ async def list_notes(
elif sort == "touched": elif sort == "touched":
# Last touched: the later of the task's own edit and its newest # Last touched: the later of the task's own edit and its newest
# work-log. Logging doesn't bump updated_at, so a task worked # work-log. Logging doesn't bump updated_at, so a task worked
# through its logs would otherwise sort as untouched (#4045). # through its logs would otherwise sort as untouched.
# GREATEST skips a NULL, so a task with no logs sorts by its edit. # GREATEST skips a NULL, so a task with no logs sorts by its edit.
from scribe.models.task_log import TaskLog from scribe.models.task_log import TaskLog
last_log = ( last_log = (
@@ -589,8 +585,8 @@ def minted_kind(kind: str) -> str:
list is how the two doors would come to disagree. list is how the two doors would come to disagree.
Raises rather than falling back to 'work'. A silently-corrected kind is Raises rather than falling back to 'work'. A silently-corrected kind is
the defect this exists to end (#3129: the editor's Kind select reported the defect this exists to end (an editor that reports success and changes
success and changed nothing), and a caller naming a kind we do not know nothing), and a caller naming a kind we do not know
has a wrong idea that an error corrects and a default hides. has a wrong idea that an error corrects and a default hides.
""" """
if kind in MINTABLE_KINDS: if kind in MINTABLE_KINDS:
@@ -616,7 +612,7 @@ async def update_note(
the one form that cannot happen by accident. The REST door, where a the one form that cannot happen by accident. The REST door, where a
cleared form input arrives as "", reaches the same place through the cleared form input arrives as "", reaches the same place through the
NULLABLE_NOTE_TEXT normalisation below — two idioms, one outcome. NULLABLE_NOTE_TEXT normalisation below — two idioms, one outcome.
(Same shape as rulebooks.update_rule, milestone 312 step 2.) (Same shape as rulebooks.update_rule.)
""" """
async with async_session() as session: async with async_session() as session:
result = await session.execute( result = await session.execute(
@@ -650,9 +646,7 @@ async def update_note(
# Same shape as status/priority above, and for the same # Same shape as status/priority above, and for the same
# reason: a kind the column will refuse should fail here with # reason: a kind the column will refuse should fail here with
# a readable message, not as a CheckViolationError from the # a readable message, not as a CheckViolationError from the
# database. Before this, `task_kind` reached setattr through # database.
# the hasattr guard with no validation at all — but no door
# ever offered it, so a task's kind was write-once (#3129).
try: try:
value = TaskKind(value).value value = TaskKind(value).value
except ValueError: except ValueError:
@@ -684,12 +678,12 @@ async def update_note(
if note.verify_with != check_before: if note.verify_with != check_before:
note.verified_at = None note.verified_at = None
# Some kinds derive `data` from their body — so a write that moves the # Some kinds derive `data` from their body — so a write that moves the
# body through this generic door must move the mirror with it (#3128). # body through this generic door must move the mirror with it.
# Without this, PATCH /api/notes/<id> {body} left the mirror behind, # Without this, PATCH /api/notes/<id> {body} would leave the mirror
# and every read PREFERS the mirror: a snippet went on reporting its # behind, and every read PREFERS the mirror: a snippet would report its
# old repo/path/symbol to prior-art recall while showing its new body, # old repo/path/symbol to prior-art recall while showing its new body,
# and a lesson would go on being retrieved for the situation it used to # and a lesson would be retrieved for the situation it used to name.
# name. The kind's own updater composes the mirror itself and passes it # The kind's own updater composes the mirror itself and passes it
# explicitly, so an explicit `data` always wins — the caller that knows # explicitly, so an explicit `data` always wins — the caller that knows
# the field set beats the one that can only re-read the body. # the field set beats the one that can only re-read the body.
if "data" not in fields and not _PARSED_FROM_BODY.isdisjoint(fields): if "data" not in fields and not _PARSED_FROM_BODY.isdisjoint(fields):
@@ -715,22 +709,21 @@ async def update_note(
return note return note
# A hard `delete_note(user_id, note_id)` lived here with ZERO callers, and was # There is deliberately no hard `delete_note(user_id, note_id)` here. The
# removed with #278 step 1. It is recorded rather than silently dropped because # danger of one is not that it runs — it is that it is findable by name.
# the danger was never that it ran — it is that it was findable by name. Someone # Someone wanting to delete a note greps `delete_note`, finds a function in the
# wanting to delete a note greps `delete_note`, finds a function in the notes # notes service with exactly the right signature, and permanently destroys a
# service with exactly the right signature, and permanently destroys a record # record every path downstream expects to be recoverable.
# every path downstream expects to be recoverable.
# #
# The delete path is `trash_svc.delete`, which soft-deletes an entity AND its # The delete path is `trash_svc.delete`, which soft-deletes an entity AND its
# descendants under one batch_id so `restore(batch)` works. `purge_trash` owns # descendants under one batch_id so `restore(batch)` works. `purge_trash` owns
# permanent deletion. Both are reachable; neither is spelled `delete_note`. # permanent deletion. Both are reachable; neither is spelled `delete_note`.
# ── The sweep (milestone 317 step 3) ───────────────────────────────────────── # ── The sweep ────────────────────────────────────────────────────────────────
# #
# A SIBLING of rulebooks.rules_due_for_verification, not a shared # A SIBLING of rulebooks.rules_due_for_verification, not a shared
# implementation, and deliberately so (note 3163). The row could have been # implementation, and deliberately so. The row could have been
# shared; the QUERY cannot. That sweep scopes by rulebook ownership XOR project # shared; the QUERY cannot. That sweep scopes by rulebook ownership XOR project
# ownership because rules have no sharing ACL at all — no rule_shares, no # ownership because rules have no sharing ACL at all — no rule_shares, no
# can_read_rule. A note scopes by the note ACL, which is a different question # can_read_rule. A note scopes by the note ACL, which is a different question
@@ -760,12 +753,12 @@ async def notes_due_for_verification(
decisions, there is nothing to go and check, and listing them would dilute decisions, there is nothing to go and check, and listing them would dilute
the result until nobody reads it. the result until nobody reads it.
Scoped with `browsable_notes_clause`, NOT the read scope (decision note Scoped with `browsable_notes_clause`, NOT the read scope: a sweep is a
2094): a sweep is a passive surface, and a record shared one-to-one must passive surface, and a record shared one-to-one must
not arrive in one unasked. not arrive in one unasked.
Deliberately NOT filtered to non-task, non-snippet records even though the Deliberately NOT filtered to non-task, non-snippet records even though the
write path (step 2) permits a check on nothing else. A row in that state write path permits a check on nothing else. A row in that state
would be a row in an ILLEGAL state, and this is the one surface that could would be a row in an ILLEGAL state, and this is the one surface that could
tell somebody about it. Hiding it here to match the invariant would make tell somebody about it. Hiding it here to match the invariant would make
the sweep agree with a database it had stopped describing. the sweep agree with a database it had stopped describing.
@@ -942,10 +935,10 @@ async def resolve_process(user_id: int, name_or_id) -> tuple[Note | None, list[d
"""Resolve a stored process by id or name. """Resolve a stored process by id or name.
note_type='process', non-trashed, scoped to what this user may READ — owned note_type='process', non-trashed, scoped to what this user may READ — owned
plus shared (rule #78). Naming one is an explicit act, so a Process shared plus shared (services/access.py). Naming one is an explicit act, so a
with the caller resolves here even though it is deliberately absent from the Process shared with the caller resolves here even though it is
passive process list and the skill manifest (decision note 2094); without deliberately absent from the passive process list and the skill manifest;
this, a Process a search surfaced could not then be run — see #2093. without this, a Process a search surfaced could not then be run.
Precedence: numeric id → exact case-insensitive title → substring. Returns Precedence: numeric id → exact case-insensitive title → substring. Returns
(note, other_candidates); on a substring tie with no exact hit, `note` is the (note, other_candidates); on a substring tie with no exact hit, `note` is the
@@ -1118,16 +1111,15 @@ async def get_note_for_user(
return (note, perm) if note else None return (note, perm) if note else None
# What a LISTING row carries (#4061): what the record is and where it sits, # What a LISTING row carries: what the record is and where it sits, never what
# never what it says. list_tasks once returned every row's to_dict(), body # it says. Every row's to_dict(), body included, can bring a project's todo
# included, and a project's todo list came to 93-165k characters, past what an # list past what an MCP client accepts inline, so the list arrives as a file to
# MCP client accepts inline, so the list arrived as a file to page through. The # page through. get_task / get_note read a record in full; a list is for
# same failure #4045 fixed for enter_project, one call over. get_task / get_note # choosing which one to open.
# read a record in full; a list is for choosing which one to open.
# #
# A field most rows leave empty (a one-line description, a parent, a due date) # 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 # 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. # learn to skip the key, and the bytes are the thing being cut.
def brief_row( def brief_row(
note: Note, note: Note,
milestone_titles: dict[int, str] | None = None, milestone_titles: dict[int, str] | None = None,