diff --git a/alembic/versions/0108_trigger_leaves_the_stored_title.py b/alembic/versions/0108_trigger_leaves_the_stored_title.py new file mode 100644 index 0000000..869d04c --- /dev/null +++ b/alembic/versions/0108_trigger_leaves_the_stored_title.py @@ -0,0 +1,64 @@ +"""trigger_leaves_the_stored_title — a snippet's and lesson's title is its name (milestone 427) + +Revision ID: 0108 +Revises: 0107 +Create Date: 2026-09-23 + +Snippets and lessons stored their title as `subject — trigger`, because the +join is what makes these kinds rank on the situation they apply to (#2485) and +the stored title WAS the embedded one. Every surface that shows a title then +showed the trigger too: menu lines, lists and search rows ran to 1,500–3,000 +characters, and an injected repeat spent all of it again. + +The trigger's home is `data` (decision #4157), and since this milestone the +join happens at embed time (`embeddings.document_title`). This revision +rewrites the rows already stored. + +EXACT-SUFFIX, READ FROM THE ROW'S OWN MIRROR. A title is only rewritten when +it ends with `' — ' || `, so a subject that legitimately +contains an em dash is never cut, and a row whose title and mirror disagree — +hand-edited through the generic note door — is left alone. Such a row still +embeds correctly (`document_title` is idempotent) and merely shows its old +title; that is the right way round for a data migration to fail. + +NO RE-EMBED, AND `updated_at` IS NOT TOUCHED. The embedded text is identical +before and after, so the vectors are already right. The startup backfill +re-embeds any row whose `updated_at` is newer than its vectors, and a raw +UPDATE that leaves `updated_at` alone is what keeps this from queueing the +whole snippet corpus for work that would produce the same numbers. There is +no database trigger on `updated_at`; it is set by the ORM only. +""" +from alembic import op + +revision = "0108" +down_revision = "0107" +branch_labels = None +depends_on = None + +# kind → the `data` key its trigger is mirrored under (embeddings._TRIGGER_DATA_KEYS). +_KINDS = (("snippet", "when_to_use"), ("lesson", "when_to_apply")) +_SEP = " — " + + +def upgrade() -> None: + for note_type, key in _KINDS: + op.execute(f""" + UPDATE notes + SET title = btrim(left(title, length(title) - length('{_SEP}' || (data->>'{key}')))) + WHERE note_type = '{note_type}' + AND coalesce(btrim(data->>'{key}'), '') <> '' + AND right(title, length('{_SEP}' || (data->>'{key}'))) = '{_SEP}' || (data->>'{key}') + """) + + +def downgrade() -> None: + # Recompose, on the same condition inverted: only where the trigger is not + # already on the end, so a downgrade run twice cannot double it. + for note_type, key in _KINDS: + op.execute(f""" + UPDATE notes + SET title = title || '{_SEP}' || (data->>'{key}') + WHERE note_type = '{note_type}' + AND coalesce(btrim(data->>'{key}'), '') <> '' + AND right(title, length('{_SEP}' || (data->>'{key}'))) <> '{_SEP}' || (data->>'{key}') + """) diff --git a/frontend/src/api/snippets.ts b/frontend/src/api/snippets.ts index 0931e14..95274a8 100644 --- a/frontend/src/api/snippets.ts +++ b/frontend/src/api/snippets.ts @@ -91,6 +91,9 @@ export interface SnippetListItem { * hit can be flagged as being in a DIFFERENT language than the file being * written, which is a shape to adapt rather than code to paste. */ language?: string; + /** When to reach for it, projected from the `data` mirror. The title is the + * name alone since milestone 427, so this is where the situation lives. */ + when_to_use?: string; /** Always present from the backend, zero-filled for records with no events. */ usage?: SnippetUsage; /** Present on the detail record; the list feed carries it when a check has diff --git a/frontend/src/views/SnippetListView.vue b/frontend/src/views/SnippetListView.vue index 599448c..660d090 100644 --- a/frontend/src/views/SnippetListView.vue +++ b/frontend/src/views/SnippetListView.vue @@ -189,11 +189,18 @@ const onLocationInput = onSearchInput; onMounted(loadSnippets); -/** Titles are stored as "name — when to reach for it"; split for display. */ -function splitTitle(title: string): { name: string; when: string } { - const idx = title.indexOf(" — "); - if (idx === -1) return { name: title, when: "" }; - return { name: title.slice(0, idx), when: title.slice(idx + 3) }; +/** A row's name and when-to-use. The title is the name alone since milestone + * 427 and the situation arrives as `when_to_use`; a title composed before then + * ("name — when to reach for it") is split, so either shape reads the same. */ +function nameAndWhen(s: { title: string; when_to_use?: string }): { name: string; when: string } { + const when = s.when_to_use || ""; + if (when && s.title.endsWith(` — ${when}`)) { + return { name: s.title.slice(0, -(when.length + 3)), when }; + } + if (when) return { name: s.title, when }; + const idx = s.title.indexOf(" — "); + if (idx === -1) return { name: s.title, when: "" }; + return { name: s.title.slice(0, idx), when: s.title.slice(idx + 3) }; } function languageOf(tags: string[]): string { @@ -313,7 +320,7 @@ function driftTitle(s: SnippetListItem): string {
- {{ splitTitle(s.title).name }} + {{ nameAndWhen(s).name }}
{{ Math.round(g.top_score * 100) }}% alike @@ -413,11 +420,11 @@ function driftTitle(s: SnippetListItem): string { :class="{ on: selectedIds.has(s.id) }" aria-hidden="true" > - {{ splitTitle(s.title).name }} + {{ nameAndWhen(s).name }} {{ languageOf(s.tags) }}
-

- {{ splitTitle(s.title).when }} +

+ {{ nameAndWhen(s).when }}

Updated {{ new Date(s.updated_at).toLocaleDateString() }} @@ -458,7 +465,7 @@ function driftTitle(s: SnippetListItem): string { :class="{ chosen: canonicalId === s.id }" > - {{ splitTitle(s.title).name }} + {{ nameAndWhen(s).name }} {{ canonicalId === s.id ? "keep" : "fold in" }}
diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index f90277a..5ec389c 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.09.21.0503", + "version": "2026.09.23.2008", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_autoinject.sh b/plugin/hooks/scribe_autoinject.sh index 48193a9..c560aa3 100755 --- a/plugin/hooks/scribe_autoinject.sh +++ b/plugin/hooks/scribe_autoinject.sh @@ -49,6 +49,7 @@ event_flat=$(printf '%s' "$event" | scribe_json_flat) prompt=$(scribe_json_pick "$event_flat" '.prompt') session_id=$(scribe_json_pick "$event_flat" '.session_id') event_cwd=$(scribe_json_pick "$event_flat" '.cwd') +transcript=$(scribe_json_pick "$event_flat" '.transcript_path') # Nothing to retrieve against. [ -n "$prompt" ] || exit 0 @@ -79,6 +80,16 @@ q=$(printf '%s' "$prompt" | head -c 2000) q_enc=$(printf '%s' "$q" | scribe_urlenc) || exit 0 [ -n "$q_enc" ] || exit 0 +# What the conversation is about, for a prompt too short to say (#4364). Sent +# beside `q`, never folded into it: the server decides whether the prompt is +# thin enough to need it, for the notes and rule arms alike. Capped at +# 600 chars here as well as there, so the URL stays small whatever the reply. +ctx_q="" +ctx=$(scribe_recent_context "$transcript") +if [ -n "$ctx" ]; then + ctx_enc=$(printf '%s' "$ctx" | scribe_urlenc) && [ -n "$ctx_enc" ] && ctx_q="&ctx=${ctx_enc}" +fi + # Scope to this directory's project — a `.scribe` marker, else the git remote. repo_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}} scope=$(scribe_scope_query "$repo_dir") @@ -120,7 +131,7 @@ fi body=$(curl -fsS --max-time 5 \ -H "Authorization: Bearer ${token}" \ - "${url%/}/api/plugin/retrieve?q=${q_enc}${repo_q}${exclude_q}" 2>/dev/null) || exit 0 + "${url%/}/api/plugin/retrieve?q=${q_enc}${ctx_q}${repo_q}${exclude_q}" 2>/dev/null) || exit 0 [ -n "$body" ] || exit 0 body_flat=$(printf '%s' "$body" | scribe_json_flat) diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 5bb3a36..675cfab 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -127,6 +127,36 @@ scribe_json_flat_lines() { awk -v mode=lines -f "$SCRIBE_HOOK_DIR/scribe_json.awk" 2>/dev/null || true } +# $1 transcript path → the tail of the last assistant REPLY text, decoded, on +# one line; "" when there is none (#4364). The auto-inject query pairs it with +# a short prompt, so "yes do that" still says what it is about. +# +# Cheap by construction, because it runs on every prompt: the last 512 KB only, +# and grep narrows to assistant records carrying a text block BEFORE anything is +# parsed — a transcript is mostly tool results, and parsing those in awk is the +# multi-second cost scribe_report_check.sh already measured. Fixed strings with +# UNESCAPED quotes can only match at a record's own top level (see that hook). +# A sidechain is a subagent talking, not this session, so it is skipped. +scribe_recent_context() { + [ -n "${1:-}" ] && [ -f "$1" ] || return 0 + tail -c 524288 "$1" 2>/dev/null \ + | grep -F '"role":"assistant"' 2>/dev/null \ + | grep -F '"type":"text"' 2>/dev/null \ + | tail -n 4 \ + | scribe_json_flat_lines \ + | awk -F'\t' ' + $2 == ".isSidechain" { side[$1] = $3; next } + $2 ~ /^\.message\.content\[[0-9]+\]\.text$/ { txt[$1] = txt[$1] " " $3; if ($1 > last) last = $1 } + END { for (i = last; i >= 0; i--) if ((i in txt) && side[i] != "true") { print txt[i]; exit } } + ' 2>/dev/null \ + | scribe_json_unescape \ + | tr '\n\t' ' ' \ + | awk '{ sub(/[ ]+$/, ""); n = length($0); if (n) print (n > 600 ? substr($0, n - 599) : $0) }' + # A transcript with no reply yet is an answer ("nothing"), not a failure — + # under pipefail the grep that matched nothing would otherwise be the status. + return 0 +} + # $1 flat text, $2 exact path → the decoded scalar, or "" if absent. # # `null` READS AS ABSENT, matching the `// empty` every call site used to carry. diff --git a/plugin/hooks/scribe_session_context.sh b/plugin/hooks/scribe_session_context.sh index c95d721..d269c4d 100755 --- a/plugin/hooks/scribe_session_context.sh +++ b/plugin/hooks/scribe_session_context.sh @@ -189,11 +189,35 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then scope=$(scribe_scope_query "$repo_dir") q="" [ -n "$scope" ] && q="?${scope}" - body=$(curl -fsS --max-time 8 \ - -H "Authorization: Bearer ${token}" \ - "${url%/}/api/plugin/context${q}" 2>/dev/null) || body="" + # ONE FETCH, NAMED WHEN IT FAILS (#4366). This used to be `curl -f … || + # body=""`, which folded a timeout, an HTTP error and a refused key into one + # sentence — so a session that started blind could not say why, and neither + # could the operator afterwards. curl's write-out still prints on a failed + # transfer (as `000`), so the status and the elapsed time come back either way. + fetch_context() { + local resp meta + resp=$(curl -sS --max-time "$1" -w '\n%{http_code} %{time_total}' \ + -H "Authorization: Bearer ${token}" \ + "${url%/}/api/plugin/context${q}" 2>/dev/null) + ctx_rc=$? + meta=${resp##*$'\n'} + body=${resp%$'\n'*} + [ "$body" = "$resp" ] && body="" + ctx_code=${meta%% *} + ctx_took=${meta#* } + } + # Deadlines: 8s is the long-standing first try, sized for a cold instance. + # ONE retry at 6s, and only for failures a second try can change — a + # timeout, a dropped connection, a 5xx. A 4xx is the key or the scope and + # will say the same thing twice. Worst case is ~14s of startup, against a + # whole session run without its project. + fetch_context 8 + case "$ctx_rc:$ctx_code" in + 0:2*|0:4*) ;; + *) fetch_context 6 ;; + esac body_flat="" - if [ -n "$body" ]; then + if [ "$ctx_rc" = 0 ] && [ "${ctx_code#2}" != "$ctx_code" ] && [ -n "$body" ]; then body_flat=$(printf '%s' "$body" | scribe_json_flat) dyn=$(scribe_json_pick "$body_flat" '.context') fi @@ -201,7 +225,28 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then # (milestone 394). Nothing is preloaded, so there is no set whose # drift a later write could be told about — a rule is retrieved at # the moment it applies, which cannot be stale. - [ -z "$dyn" ] && status="> ⚠️ Scribe: live project context could not be loaded this session (instance unreachable or request failed). The using-scribe skill still applies — ask for rules with \`search(content_type=\"rule\")\` and project context with \`enter_project()\` as needed." + if [ -z "$dyn" ]; then + if [ "$ctx_rc" = 28 ]; then + why="the instance did not answer in time (8s, then ${ctx_took}s on a retry)" + elif [ "$ctx_rc" != 0 ]; then + why="the instance could not be reached (curl exit ${ctx_rc}, after a retry)" + elif [ "$ctx_code" = 401 ] || [ "$ctx_code" = 403 ]; then + why="the API key was refused (HTTP ${ctx_code})" + elif [ "${ctx_code#2}" = "$ctx_code" ]; then + why="the instance answered HTTP ${ctx_code}" + else + why="the instance answered but sent no context" + fi + # The first move, stated as one. "As needed" read as optional, and a + # session that skips it starts with no recent milestones or open tasks — + # so it cannot know what prior work exists to look for (#4366). + if [ -n "$marker_id" ] && [ -z "$marker_why" ]; then + first="Start by calling \`enter_project(${marker_id})\`" + else + first="Start by finding this repo's project with \`list_projects()\` and calling \`enter_project()\`" + fi + status="> ⚠️ Scribe: live project context was not loaded this session — ${why}. The tools may still answer. ${first} before any other work: it loads the recent milestones and open tasks this session would otherwise begin without, and prior work you cannot see is work you will redo. The using-scribe skill still applies." + fi elif [ -n "$url" ] && [ -z "$token" ]; then status="> ⚠️ Scribe: live context disabled this session — the API key is not configured (Scribe base URL is). Set it with \`/plugin\` → Scribe → configure, or export SCRIBE_TOKEN. Tools still work; ask for rules with \`search(content_type=\"rule\")\` and project context with \`enter_project()\`." elif [ -z "$url" ] && [ -z "$token" ]; then diff --git a/src/scribe/mcp/tools/lessons.py b/src/scribe/mcp/tools/lessons.py index 24d46c9..0a35b1f 100644 --- a/src/scribe/mcp/tools/lessons.py +++ b/src/scribe/mcp/tools/lessons.py @@ -168,6 +168,7 @@ async def create_lesson( dup = await dedup_svc.find_duplicate_note( uid, title, body, project_id=project_id or None, is_task=False, note_type=lessons_svc.LESSON_NOTE_TYPE, + data=lessons_svc.compose_data(what, when_to_apply), ) if dup is not None: return dedup_svc.duplicate_response(dup, "lesson") diff --git a/src/scribe/mcp/tools/snippets.py b/src/scribe/mcp/tools/snippets.py index f9a0b47..3b5b4b9 100644 --- a/src/scribe/mcp/tools/snippets.py +++ b/src/scribe/mcp/tools/snippets.py @@ -176,7 +176,9 @@ async def create_snippet( raise ValueError("create_snippet requires a non-empty name and code") uid = current_user_id() - title = snippets_svc.compose_title(name, when_to_use) + # The NAME is the title (milestone 427); the trigger rides in `data` and + # joins the title only in the embedded document. + title = name.strip() body = snippets_svc.compose_body( code=code, language=language, signature=signature, when_to_use=when_to_use, repo=repo, path=path, symbol=symbol, @@ -190,6 +192,7 @@ async def create_snippet( # location and code before it compares prose (#2518). code=code, locations=snippets_svc.resolve_locations(repo, path, symbol, locations), + data=snippets_svc.compose_data(name=name, when_to_use=when_to_use), ) if dup is not None: return dedup_svc.duplicate_response(dup, "snippet") diff --git a/src/scribe/routes/lessons.py b/src/scribe/routes/lessons.py index 9137796..c9241a3 100644 --- a/src/scribe/routes/lessons.py +++ b/src/scribe/routes/lessons.py @@ -149,6 +149,7 @@ async def create_lesson_route(): project_id=project_id, is_task=False, note_type=lessons_svc.LESSON_NOTE_TYPE, + data=lessons_svc.compose_data(what, when_to_apply), ) if dup is not None: return jsonify(dedup_svc.duplicate_response(dup, "lesson")), 409 diff --git a/src/scribe/routes/plugin.py b/src/scribe/routes/plugin.py index 0d63961..5768d32 100644 --- a/src/scribe/routes/plugin.py +++ b/src/scribe/routes/plugin.py @@ -95,6 +95,12 @@ async def autoinject_retrieve(): project_id (opt) — explicit project scope override (ad-hoc/testing). exclude_ids (opt) — comma-separated note ids already injected this session; skipped so each note injects at most once. + ctx (opt) — the tail of the last assistant reply (#4364). The + notes AND rule arms append it to a short prompt, + so a follow-up like "yes do that" still names + what it is about — and a rule or lesson arrives + while the work is under way, before the operator + has to call it out. exclude_rule_ids — comma-separated rule ids already surfaced this (opt) session. SHARED with /prior-art and /tool-rules on purpose: one session keeps ONE rule ledger, so a @@ -125,11 +131,13 @@ async def autoinject_retrieve(): exclude_rule_ids = _int_list(request.args.get("exclude_rule_ids")) held_rule_ids = _int_list(request.args.get("held_rule_ids")) + ctx = request.args.get("ctx") or "" rules = await plugin_ctx_svc.build_prompt_rule_hint( - g.user.id, q, project_id=project_id, exclude_rule_ids=exclude_rule_ids, held_rule_ids=held_rule_ids + g.user.id, q, project_id=project_id, exclude_rule_ids=exclude_rule_ids, held_rule_ids=held_rule_ids, + context=ctx, ) result = await plugin_ctx_svc.build_autoinject_hint( - g.user.id, q, project_id=project_id, exclude_ids=exclude_ids + g.user.id, q, project_id=project_id, exclude_ids=exclude_ids, context=ctx, ) blocks = [b for b in (rules["context"], result["context"]) if b] result["context"] = "\n\n".join(blocks) diff --git a/src/scribe/routes/snippets.py b/src/scribe/routes/snippets.py index 585d803..e748b94 100644 --- a/src/scribe/routes/snippets.py +++ b/src/scribe/routes/snippets.py @@ -96,7 +96,7 @@ async def create_snippet_route(): if not data.get("force"): dup = await dedup_svc.find_duplicate_note( uid, - snippets_svc.compose_title(name, data.get("when_to_use", "")), + name.strip(), snippets_svc.compose_body( code=data.get("code", ""), language=data.get("language", ""), @@ -119,6 +119,9 @@ async def create_snippet_route(): data.get("repo", ""), data.get("path", ""), data.get("symbol", ""), data.get("locations"), ), + data=snippets_svc.compose_data( + name=name, when_to_use=data.get("when_to_use", ""), + ), ) if dup is not None: return jsonify(dedup_svc.duplicate_response(dup, "snippet")), 409 diff --git a/src/scribe/services/dedup.py b/src/scribe/services/dedup.py index 90101e6..f03ef70 100644 --- a/src/scribe/services/dedup.py +++ b/src/scribe/services/dedup.py @@ -248,6 +248,7 @@ async def find_duplicate_note( note_type: str = "note", code: str = "", locations: list[dict] | None = None, + data: dict | None = None, ) -> DuplicateMatch | None: """Best near-duplicate of (title, body) within the same owner + project + kind, or None. Title match first (cheap, exact), then — for snippets — the @@ -258,6 +259,11 @@ async def find_duplicate_note( `code` and `locations` are the snippet's structured fields. They are ignored for every other kind, and passing them is what lets the gate compare ARTEFACTS rather than descriptions of artefacts (#2518). + + `data` is the candidate's structured mirror. For a snippet or lesson it + carries the trigger, which the TITLE no longer does (milestone 427): the + title check compares names, and the semantic check rebuilds the embedded + document from `data`. """ norm = " ".join((title or "").split()).lower() @@ -309,7 +315,11 @@ async def find_duplicate_note( # section. Capped so one pathological paste can't turn a save into # dozens of searches — a duplicate past the cap is the duplicate # report's job, not the gate's. - for query in embeddings_svc.chunk_document(title, body)[:_GATE_MAX_CHUNKS]: + # The EMBEDDED title (milestone 427): a snippet or lesson is stored + # under its name and embedded under `name — trigger`, so the query + # document is built the way the corpus was, from `data`. + doc_title = embeddings_svc.document_title(title, note_type, data, body) + for query in embeddings_svc.chunk_document(doc_title, body)[:_GATE_MAX_CHUNKS]: # Scope the semantic check the same way as the title check: a record # in project P compares only to P; a project-less (orphan) record # compares only to other orphans (orphan_only), NOT across every diff --git a/src/scribe/services/embeddings.py b/src/scribe/services/embeddings.py index 1361c18..fe1675e 100644 --- a/src/scribe/services/embeddings.py +++ b/src/scribe/services/embeddings.py @@ -214,10 +214,12 @@ TRIGGER_SEP = " — " def trigger_title(subject: str | None, trigger: str | None) -> str: """`{subject} — {trigger}` — the title half of a situation-keyed document. - ONE definition, because this join had three. `rule_document` built it for - rules, `snippets.compose_title` for snippets, and milestone 385 needed a - fourth for lessons — the shape #3207 records, where a fix or an improvement - then has to be found in N places by someone who does not know N. + ONE definition, because this join had three — rules, snippets, and a + fourth for lessons (milestone 385) — the shape #3207 records, where a fix + or an improvement then has to be found in N places by someone who does not + know N. Since milestone 427 it builds EMBEDDED titles only: `rule_document` + for rules and `document_title` for snippets and lessons. No stored title + carries it. WHY THE JOIN MATTERS AT ALL, measured in note #2485: the snippet was the only sharp record in the corpus — a 0.153 top-to-second gap against @@ -265,6 +267,51 @@ def untrigger_title(title: str | None, trigger: str | None) -> str: return title +# The `data` key each trigger-keyed note kind mirrors its trigger under. Rules +# are not here: they keep the trigger in a column and `rule_document` builds +# their document from it. +_TRIGGER_DATA_KEYS = {"snippet": "when_to_use", "lesson": "when_to_apply"} + + +def document_title( + title: str | None, note_type: str | None, data: dict | None = None, + body: str | None = None, +) -> str | None: + """The title a note is EMBEDDED under — its stored title, plus its trigger. + + Milestone 427. A snippet's or lesson's STORED title is its subject alone; + the trigger lives in `data` (decision #4157). It still has to reach the + vector — the `subject — trigger` join is what makes these kinds rank on + the situation they apply to (#2485) — so it is joined HERE, at embed time, + rather than being carried in a title every listing then has to show. + + IDEMPOTENT, and that is what makes the migration safe: a title that is + already composed (a row not yet migrated, an old backup restored) is + untriggered first, so it comes out the same and never doubled. The text is + byte-identical to what these kinds were embedded as before, so no vector + moves and the floors tuned against them stay calibrated. + + `body` is the fallback when the mirror is missing, read by the kind's own + parser — the same degrade-to-the-body each kind's reader already has. + Every other kind, and a record with no trigger, keeps its title as-is. + """ + key = _TRIGGER_DATA_KEYS.get(note_type or "") + if key is None: + return title + trigger = ((data or {}).get(key) or "").strip() if isinstance(data, dict) else "" + if not trigger and body: + from types import SimpleNamespace + if note_type == "lesson": + from scribe.services.lessons import lesson_trigger + trigger = lesson_trigger(SimpleNamespace(data=None, body=body)) + else: + from scribe.services.snippets import parse_snippet_fields + trigger = parse_snippet_fields(title or "", body).get("when_to_use", "") + if not trigger: + return title + return trigger_title(untrigger_title(title, trigger), trigger) + + # --- chunking (#280): the document shape ------------------------------------ # # bge-small reads at most 512 tokens and fastembed silently truncates the rest, @@ -1081,10 +1128,14 @@ async def backfill_note_embeddings() -> None: ) success = 0 for note_id in notes_to_embed: - row = await _current_row((Note.user_id, Note.title, Note.body), Note.id, note_id) + row = await _current_row( + (Note.user_id, Note.title, Note.body, Note.note_type, Note.data), Note.id, note_id, + ) if row is None: continue # deleted between the scan and here - user_id, title, body = row + user_id, title, body, note_type, data = row + # The EMBEDDED title, as the write path builds it (milestone 427). + title = document_title(title, note_type, data, body) if not chunk_document(title, body): continue await upsert_note_embedding(note_id, user_id, title, body) diff --git a/src/scribe/services/knowledge.py b/src/scribe/services/knowledge.py index efb0ea8..05a311e 100644 --- a/src/scribe/services/knowledge.py +++ b/src/scribe/services/knowledge.py @@ -267,6 +267,11 @@ 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. + usage = (note.data or {}).get("when_to_use") if note.data else None + if usage: + item["when_to_use"] = usage verdict = (note.data or {}).get("verification") if note.data else None if verdict and verdict.get("status"): diff --git a/src/scribe/services/lessons.py b/src/scribe/services/lessons.py index 0f8080f..9a5a5bd 100644 --- a/src/scribe/services/lessons.py +++ b/src/scribe/services/lessons.py @@ -26,20 +26,23 @@ while staying a note in every other respect. WHERE THE TRIGGER LIVES (decision #4157, milestone 385 step 1) In ``notes.data`` under ``when_to_apply``, written through a named parameter and -mirrored into the title and the head of the body — the shape snippets already -use for ``when_to_use``. Not a column on ``notes``. +mirrored into the head of the body — the shape snippets already use for +``when_to_use``. Not a column on ``notes``. Since milestone 427 it is NOT in the +stored title: the title is the lesson's subject, and the trigger joins it only +in the embedded document (``embeddings.document_title``). That decision was measured rather than assumed. The whole snippet corpus — 164 of 164 — carries a ``when_to_use`` with **no guard anywhere**, which refutes the premise that an unenforced field gets skipped. What it does NOT show is that -an agent types a title convention correctly: ``compose_title`` builds the title +an agent types a title convention correctly: the service composed the title from the parameter, so what is at 100% is a named structured field. A column would have bought enforceability at the price of deciding, for every note kind at once, a question nothing had measured. -The mirror is what makes the vector sharp, and it is why nothing re-embeds: -``chunk_document`` is untouched, so ``CHUNKER_VERSION`` does not move. The -trigger reaches the document by being in the text, exactly as a snippet's is. +The trigger in the document is what makes the vector sharp. It reaches it +twice — in the embedded title, joined at embed time, and in the body's first +line — which is the text these kinds were always embedded as, so nothing +re-embeds and ``CHUNKER_VERSION`` does not move. WHAT A LESSON INHERITS, AND THE CELLS LEFT EMPTY ON PURPOSE (#3163) @@ -207,36 +210,17 @@ def sole_source(sources: list[int] | None) -> int | None: return ids[0] if len(ids) == 1 else None -def compose_title(what: str, when_to_apply: str = "") -> str: - """`{what} — {when it applies}`, the half of the document that ranks. - - Built HERE rather than asked of the caller, and that distinction is the - whole evidence base for this design: the snippet corpus is at 100% on its - trigger because a service composes the title from a named parameter, not - because agents type separators reliably. A caller made to spell the - convention is the option milestone 385 step 1 rejected. - - The join is `embeddings.trigger_title` — shared with rules and snippets, so - the three kinds that rank on a trigger cannot drift apart in how they say - so. - """ - from scribe.services.embeddings import trigger_title - - return trigger_title(what, when_to_apply) - - def compose_body( insight: str, when_to_apply: str = "", learned_from: list[int] | None = None, ) -> str: """The lesson body — the trigger line first, the insight after. - The mirror of `compose_title` on the other half of the document, and the - reason the pair is what makes a lesson findable: `chunk_document` joins - them as `{title}\\n{body}`, so a lesson composed here states WHEN IT - APPLIES in the title and again in the first line of the body. That is the - twice-in-a-short-document shape note #2485 measured as the only sharp one - in the corpus, reached the way a snippet reaches it — by being in the text - — rather than by a second document builder at embed time. + The trigger's home in the text of the document, and half of what makes a + lesson findable: `chunk_document` joins `{embedded title}\\n{body}`, and + the embedded title is `what — when it applies` (`embeddings.document_title`, + milestone 427), so the document states WHEN IT APPLIES in the title and + again in the first line of the body. That is the twice-in-a-short-document + shape note #2485 measured as the only sharp one in the corpus. `**When to apply:**` rather than plain text: the body is the READABLE form, `data` is the queryable mirror, and `_BODY_TRIGGER_RE` reads this @@ -275,23 +259,23 @@ def lesson_document( what: str, when_to_apply: str = "", insight: str = "", learned_from: list[int] | None = None, ) -> tuple[str, str]: - """The (title, body) a lesson is STORED — and therefore embedded — as. + """The (title, body) a lesson is STORED as. - One call so the two halves cannot be composed apart. A lesson whose title - carried the trigger and whose body did not would embed as an ordinary - note wearing a label, and nothing would report it: the record would look - right in every listing and simply never be retrieved at the moment it - applies. + One call so the two halves cannot be composed apart. The body's first line + carries the trigger; a lesson without it (and without the `data` mirror) + would embed as an ordinary note wearing a label, and nothing would report + it: the record would look right in every listing and simply never be + retrieved at the moment it applies. - Deliberately returns what is STORED, not a separate embed-time shape. - Rules need `rule_document` because a rule keeps its trigger in a column - and its title is a plain name, so the sharp document has to be synthesised - for the ranker and exists nowhere else. A lesson follows the snippet - instead — the stored record IS the sharp document — which is why nothing - re-embeds and `CHUNKER_VERSION` does not move. + The title is the SUBJECT alone (milestone 427). It used to carry the + trigger too, so the stored record was itself the sharp document — and every + listing, menu and search row then showed a title that ran to kilobytes. + The trigger now joins the title at embed time (`embeddings.document_title`, + reading `data`), producing the same text as before, so nothing re-embeds + and `CHUNKER_VERSION` does not move. """ return ( - compose_title(what, when_to_apply), + (what or "").strip(), compose_body(insight, when_to_apply, learned_from), ) diff --git a/src/scribe/services/notes.py b/src/scribe/services/notes.py index 165eaee..fe3c1b6 100644 --- a/src/scribe/services/notes.py +++ b/src/scribe/services/notes.py @@ -115,11 +115,17 @@ def embed_note(note) -> None: try: import asyncio - from scribe.services.embeddings import upsert_note_embedding + from scribe.services.embeddings import document_title, upsert_note_embedding # Chunking and the empty-record gate live inside upsert_note_embedding — - # one path for every writer (#280). + # one path for every writer (#280). The title is the EMBEDDED one: a + # snippet's or lesson's trigger joins its name here, not in the stored + # title (milestone 427). asyncio.create_task( - upsert_note_embedding(note.id, note.user_id, note.title, note.body) + upsert_note_embedding( + note.id, note.user_id, + document_title(note.title, note.note_type, note.data, note.body), + note.body, + ) ) except RuntimeError: pass # no running loop — a sync caller, not a failure diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 7b6665d..cda7e84 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -27,7 +27,11 @@ from scribe.services import projects as projects_svc from scribe.services import shape_ledger as shape_ledger_svc from scribe.services import snippets as snippets_svc from scribe.services.access import label_shared_items, owner_names_for -from scribe.services.embeddings import semantic_search_notes, semantic_search_rules +from scribe.services.embeddings import ( + document_title, + semantic_search_notes, + semantic_search_rules, +) from scribe.services.lessons import LESSON_NOTE_TYPE from scribe.services.note_usage import record_surfaced from scribe.services.rule_usage import record_rule_surfaced @@ -40,6 +44,7 @@ from scribe.services.retrieval_surfaces import ( ) from scribe.services.retrieval_telemetry import record_retrieval from scribe.services.settings import get_setting +from scribe.services.systems import system_names_for from scribe.services.text import elide logger = logging.getLogger(__name__) @@ -47,23 +52,73 @@ 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. +# WHAT A MENU LINE CARRIES (#4364): the record's NAME, its kind and System, +# and the WHOLE passage that matched. Metadata plus the evidence, rather than a +# title asked to be both. # -# 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. +# The name, not the title. A snippet's or lesson's title is `name — when it +# applies` by construction (`embeddings.trigger_title`), because that join is +# what makes it rank on its situation. That is an EMBEDDING shape, and rendered +# as a menu line it ran to 1,500+ characters — the trigger paragraph spent +# again on every line, and again on every repeat. The trigger still arrives +# when it is what matched: it is in the chunk, and `_menu_passage` hands it +# over when the title was the whole match. # -# 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 +# The whole passage, not 200 characters of it. The search already chose the +# chunk that matched; the old cut kept its head and tail, and the head is the +# title every chunk is prefixed with — so the reader got the title twice and +# lost the middle, which is where the match was (lesson #4248). A chunk is at +# most ~1.4 KB (`embeddings._CHUNK_CHAR_BUDGET`), and it is shown once: a +# repeat is a one-line pointer (`_menu_seen_line`), not a second copy. + + +def _menu_name(title: str | None, note_type: str | None, data=None, body: str | None = "") -> str: + """The record's name — its title without the trigger composed into it.""" + title = (title or "(untitled)").replace("\n", " ").strip() + data = data if isinstance(data, dict) else {} + if note_type == "snippet": + from scribe.services.embeddings import TRIGGER_SEP + return (data.get("name") or title.partition(TRIGGER_SEP)[0]).strip() or title + if note_type == LESSON_NOTE_TYPE: + from types import SimpleNamespace + + from scribe.services.embeddings import untrigger_title + from scribe.services.lessons import lesson_trigger + trigger = lesson_trigger(SimpleNamespace(data=data, body=body or "")) + return untrigger_title(title, trigger).strip() or title + return title + + +def _menu_passage(title: str | None, chunk_text: str | None, name: str = "") -> str: + """The matched chunk on one line, without the title it was embedded under. + + Every chunk is `title\nsection` (`embeddings.embedding_text`), and `title` + here must be the EMBEDDED one (`embeddings.document_title`) — for a snippet + or lesson that is `name — trigger`, not the stored name — so the prefix is + stripped exactly. A chunk that WAS only the title — a short + record, or the head chunk of one — matched on the title, and for a + trigger-keyed kind the part of it the name line no longer shows is the + trigger: that is returned, because it is precisely what matched. + One line, so the menu's blockquote survives it. + """ + title = (title or "").strip() + text = (chunk_text or "").strip() + if title and text.startswith(title): + text = text[len(title):] + text = " ".join(text.split()) + if not text and name and title.startswith(name) and title != name: + text = " ".join(title[len(name):].lstrip(" —-").split()) + return text + + +def _menu_label(kind: str, systems: list[str] | None) -> str: + """`issue (done) · Plugin & hooks` — the kind, then where it belongs.""" + return " · ".join([kind, *systems]) if systems else kind + + +def _menu_seen_line(note_id: int, kind: str, name: str) -> str: + """A pointer to a record this session was already shown, not a copy of it.""" + return f"> - #{note_id} [{kind} · seen] {name}" # Max chars of a Process body to fold into the auto-surface description. _PROC_PREVIEW_CHARS = 200 @@ -537,6 +592,36 @@ _AUTOINJECT_BAND = 0.10 # was an accident of which arm got a configurable budget first. _AUTOINJECT_MAX_TOP_K = MAX_BUDGET +# THE CONVERSATION BESIDE THE PROMPT (#4364). The operator's message is the +# only query this arm had, and mid-session it is mostly a follow-up — "yes do +# that", "now fix the filter" — that names nothing a record could match. The +# hook now sends the tail of the last assistant reply as `context`, and it is +# appended to a SHORT prompt only: a prompt that already says what it is about +# is the better query on its own, and diluting it is the one way this can make +# retrieval worse. +# +# Neither number costs a model token. The query is embedding input; what the +# session pays for is the menu, and the budget bounds that unchanged. +# 280 — about two sentences. Above it a prompt carries its own subject. +# 600 — the context cap. Prompt + context stays well inside bge-small's +# 512-token window, prompt FIRST, so truncation can only ever cut +# context and never the operator's words. +_AUTOINJECT_CONTEXT_PROMPT_MAX = 280 +_AUTOINJECT_CONTEXT_MAX = 600 + + +def _autoinject_query(prompt: str, context: str) -> str: + """The prompt, with recent conversation appended when the prompt is thin. + + The prompt leads and context is its tail, cut from the END of the reply — + a reply's closing lines are where it says what it did and what is next, + which is what the operator's follow-up is answering. + """ + ctx = " ".join((context or "").split())[-_AUTOINJECT_CONTEXT_MAX:] + if not ctx or len(prompt) > _AUTOINJECT_CONTEXT_PROMPT_MAX: + return prompt + return f"{prompt}\n\n{ctx}" + # --- the prompt-boundary rule arm (#3852) ------------------------------------ # # Both existing rule arms are keyed on something the session is about to DO — @@ -961,6 +1046,7 @@ async def build_autoinject_hint( query: str, project_id: int = 0, exclude_ids: list[int] | None = None, + context: str = "", ) -> dict: """Title-first awareness hint for the plugin's UserPromptSubmit hook. @@ -982,6 +1068,9 @@ async def build_autoinject_hint( q = (query or "").strip() if not cfg["enabled"] or not q: return empty + # Everything below searches, logs and fills its slots on the ENRICHED + # query, so the telemetry row records what was actually asked (#4364). + q = _autoinject_query(q, context) # THE LEDGER LEAVES THE SEARCH (#4101). `exclude_ids` used to go into # `semantic_search_notes` itself, so a record this session had already been @@ -1081,8 +1170,9 @@ async def build_autoinject_hint( lines = [ "> Possibly relevant from your Scribe records — open any in full with " "`get_note(id)`, or `get_snippet` / `get_process` / `get_lesson` for " - "those kinds (titles only; a line marked `seen` was surfaced earlier " - "this session and may no longer be in context):", + "those kinds. Each line is a record's name, its kind and System, and " + "the passage that matched; a line marked `seen` is a pointer to one " + "already shown this session, so it is in your context:", ] # THE REGISTER, SAID ONCE AND ONLY WHEN IT APPLIES (milestone 385 step 5). # @@ -1118,34 +1208,45 @@ async def build_autoinject_hint( # with the query that actually matched it. menu_chunks = _rep_ai.get("best_chunk") or {} + systems = await system_names_for({int(n.id) for _s, n in kept if int(n.id) not in already}) note_ids: list[int] = [] for score, note in kept: - note_ids.append(int(note.id)) - title = (note.title or "(untitled)").replace("\n", " ").strip() - line = f"> - #{note.id} [{_record_kind(note)}] \"{title}\" ({score:.2f})" - # ONE WORD, NOT A SENTENCE, and deliberately not the rule arms' phrasing. - # A rule line says "before deciding it does not apply", which is the - # voice of a record that BINDS; a note binds nothing, and borrowing that - # tone would tell the reader a dev-log has authority it does not have. - # The header carries the meaning, so the line carries only the flag. - if int(note.id) in already: - line += " [seen]" - if int(note.id) in stale: + nid = int(note.id) + note_ids.append(nid) + kind = _record_kind(note) + # The NAME, not the title (#4364): a snippet's or lesson's title is its + # embedding shape, trigger and all, and ran past 1,500 characters here. + name = _menu_name(note.title, note.note_type, note.data, note.body) + if nid in already: + # A POINTER, not a copy (#4364). The record is in this session's + # context already — the ledger is cleared at compaction, so "seen" + # stays true — and re-rendering it spent its whole line again for + # nothing. What the reader needs is the reminder that it matched + # again, and the id to open it if it has scrolled out of mind. + line = _menu_seen_line(nid, kind, name) + if nid in stale: + line += " — SUPERSEDED" + lines.append(line) + continue + line = f"> - #{nid} [{_menu_label(kind, systems.get(nid))}] \"{name}\" ({score:.2f})" + if nid in stale: line += " — SUPERSEDED, a later record covers this; check that first" if note.user_id != user_id: who = owners.get(int(note.user_id)) or "another user" line += f" — shared by {who}, 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}") + # The passage that earned the line, WHOLE, indented under it (#4364). + # 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 indented alike. + passage = _menu_passage( + document_title(note.title, note.note_type, note.data, note.body), + (menu_chunks.get(nid) or {}).get("text"), name, + ) + if passage: + lines.append(f"> ↳ {passage}") # Records what SURVIVED the margin gate, not what the ranker returned — the # menu the agent actually saw. retrieval_logs already holds the full @@ -1286,6 +1387,7 @@ async def build_prompt_rule_hint( project_id: int = 0, exclude_rule_ids: list[int] | None = None, held_rule_ids: list[int] | None = None, + context: str = "", ) -> dict: """Rules and preferences that may apply to what the operator just asked. @@ -1319,6 +1421,11 @@ async def build_prompt_rule_hint( q = (query or "").strip() if not q: return out + # The same enrichment the notes arm gets (#4364). A rule is meant to + # arrive while the work it governs is under way, not once the operator + # names it — and "yes, go ahead" names nothing a trigger can match, while + # the reply it answers ("commit this to dev and push") does. + q = _autoinject_query(q, context) try: threshold = await floor_for(user_id, "prompt_rule") @@ -1429,7 +1536,12 @@ def _prior_art_line(item: dict, marker: str, owner: str | None, foreign_lang: st rather than appended after the title, so the reader sees it while still reading the score — the two together are the judgement being offered. """ - title = (item.get("title") or "(untitled)").replace("\n", " ").strip() + # The NAME, not the composed title (#4364) — a snippet's title carries its + # whole trigger and ran to kilobytes on this line, again on every repeat. + title = ( + item.get("name") or (item.get("snippet") or {}).get("name") + or (item.get("title") or "(untitled)") + ).replace("\n", " ").strip() mark = f"{marker} · {foreign_lang}" if foreign_lang else marker line = f"> - #{item['id']} [{mark}] \"{title}\"" if owner: @@ -2038,6 +2150,7 @@ async def build_write_path_hint( # dropped — decided by which arm happened to find them — is worse than # either rule applied consistently: the marker would read as a complete # account of what the session has met before, and it would not be one. + item["seen"] = nid in excluded placed.append(("nearby · seen" if nid in excluded else "nearby", item)) # The stamping feed's "actually pulled it" half (#2791). Read once, before @@ -2215,6 +2328,16 @@ async def build_write_path_hint( marker, { "id": int(note.id), "title": note.title, "user_id": note.user_id, + # The name the line shows, and whether this session has + # it already — carried as data for the reason `kind` is + # (#4364): the line is built from facts, not from + # re-reading its own marker. + "name": _menu_name(note.title, note.note_type, note.data, note.body), + # What its chunks are prefixed with, for stripping. + "doc_title": document_title( + note.title, note.note_type, note.data, note.body, + ), + "seen": int(note.id) in excluded, # Carried, not re-read off the rendered marker. The # marker is prose assembled for a human and it already # varies by kind, language and the `seen` flag — a @@ -2359,8 +2482,9 @@ async def build_write_path_hint( "`get_lesson(id)` for a lesson, `get_note(id)` otherwise. Reuse a " "snippet rather than writing a fresh one-off; read an issue before " "repeating what it records " - "(titles only; a line marked `seen` was surfaced earlier this " - "session and may no longer be in context):" + "(each line is a record's name and kind, with the passage that " + "matched; a line marked `seen` is a pointer to one already shown " + "this session, so it is in your context):" ) # The same clause the prompt menu carries, on the same condition and for # the same reason: this menu's three other kinds are all things that WERE @@ -2394,10 +2518,17 @@ async def build_write_path_hint( # 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}") + # WHOLE, and only on first sight (#4364): a `seen` line is a pointer to + # a record already in context, and its passage is already there too. + if item.get("seen"): + continue + passage = _menu_passage( + item.get("doc_title") or item.get("title"), + (wp_chunks.get(int(item["id"])) or {}).get("text"), + item.get("name") or "", + ) + if passage: + lines.append(f"> ↳ {passage}") if stamped: lines.append(_stamp_line(path, stamped)) diff --git a/src/scribe/services/snippets.py b/src/scribe/services/snippets.py index 2873f5d..fbc4d7a 100644 --- a/src/scribe/services/snippets.py +++ b/src/scribe/services/snippets.py @@ -54,18 +54,6 @@ UNSET: object = object() # --- serialize: structured fields -> note (title/body/tags) ------------------ -def compose_title(name: str, when_to_use: str = "") -> str: - """`name — when to use` (or just `name` when no usage note is given). - - The join itself lives in `embeddings.trigger_title`, which rules and - lessons build their titles from too. Kept as a named function here because - it is this module's public vocabulary and callers say `compose_title`. - """ - from scribe.services.embeddings import trigger_title - - return trigger_title(name, when_to_use) - - def compose_tags(language: str = "", tags: list[str] | None = None) -> list[str]: """Language (lowercased) first, then the `snippet` marker, then caller tags — de-duplicated, order preserved.""" @@ -741,7 +729,7 @@ async def create_snippet( locations = resolve_locations(repo, path, symbol, locations) note = await notes_svc.create_note( user_id, - title=compose_title(name, when_to_use), + title=name.strip(), body=compose_body( code=code, language=language, signature=signature, when_to_use=when_to_use, locations=locations, @@ -898,7 +886,7 @@ async def update_snippet( provenance = cur.get("provenance") fields: dict = { - "title": compose_title(merged["name"], merged["when_to_use"]), + "title": (merged["name"] or "").strip(), "body": compose_body( code=merged["code"], language=merged["language"], signature=merged["signature"], when_to_use=merged["when_to_use"], diff --git a/src/scribe/services/systems.py b/src/scribe/services/systems.py index 7f2f0bc..293cd7a 100644 --- a/src/scribe/services/systems.py +++ b/src/scribe/services/systems.py @@ -310,6 +310,39 @@ async def list_record_systems(user_id: int, note_id: int) -> list[System]: return list(result.scalars().all()) +async def system_names_for(note_ids: set[int]) -> dict[int, list[str]]: + """{note_id: [system name, …]} in one query, for records ALREADY read. + + For decorating a result set the caller was allowed to see — an injected + menu line says which part of the project a record is about, so the reader + can place it without opening it (#4364). No access check here for that + reason: the ids come from a search that applied one, and a system name is + metadata of the record, not a record of its own. + + Fails soft, like `access.owner_names_for`: a menu without its system labels + is a cosmetic downgrade, and failing the whole injection over one is not. + """ + if not note_ids: + return {} + try: + async with async_session() as session: + rows = ( + await session.execute( + select(RecordSystem.note_id, System.name) + .join(System, System.id == RecordSystem.system_id) + .where(RecordSystem.note_id.in_(note_ids), System.deleted_at.is_(None)) + .order_by(System.order_index.asc(), System.name.asc()) + ) + ).all() + except Exception: + logger.warning("System-name lookup failed; menu lines go unlabelled", exc_info=True) + return {} + out: dict[int, list[str]] = {} + for note_id, name in rows: + out.setdefault(int(note_id), []).append(name) + return out + + async def list_records_for_system( user_id: int, system_id: int, kind: str | None = None, open_only: bool = False ) -> list[Note]: diff --git a/tests/conftest.py b/tests/conftest.py index 3058aa3..79c6b29 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -102,6 +102,20 @@ def _no_supersession(): yield +@pytest.fixture(autouse=True) +def _no_system_labels(): + """Stub the menu's "which System is each line about?" lookup (#4364). + + Autouse because every test that renders an injected menu reaches it, and + it is a real database call on a path those tests run without one. Stubbed + to "no labels", the state of any untagged record. Tests of the label + itself patch it with a value. + """ + with patch("scribe.services.plugin_context.system_names_for", + AsyncMock(return_value={})): + yield + + @pytest.fixture(autouse=True) def _no_task_log_arm(): """Stub the task-log read arm that get_task / list_tasks / get_milestone diff --git a/tests/helpers.py b/tests/helpers.py index f3b6090..54d7271 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -212,10 +212,10 @@ def fake_snippet(**attrs) -> MagicMock: def fake_lesson(**attrs) -> MagicMock: """A stand-in lesson: a note whose `note_type` is what makes it one. - The title carries the trigger because `compose_title` builds it that way — - `{what} — {when it applies}` — so a menu line rendering only the title is - already showing the reader when this lesson applies. Tests that used a bare - title here would be testing a record the product cannot create. + The title is the subject alone and the trigger lives in `data`, because + that is the record the product creates (milestone 427) — the trigger joins + the title only in the embedded document. A default whose title carried the + trigger would be testing a row only an un-migrated database holds. The check fields and `arose_from_id` are explicitly None for the reason `fake_snippet`'s `data` is: `update_note` reads `verify_with` and @@ -223,12 +223,11 @@ def fake_lesson(**attrs) -> MagicMock: auto-created MagicMock attribute is truthy — so a default lesson driven through the update path would take a branch no real record takes. """ - attrs.setdefault( - "title", - "Give absolutely-positioned siblings an explicit stacking order — " - "placing two absolutely-positioned elements in the same area", - ) - attrs.setdefault("data", {"when_to_apply": "two absolute siblings overlap"}) + attrs.setdefault("title", "Give absolutely-positioned siblings an explicit stacking order") + attrs.setdefault("data", { + "what": "Give absolutely-positioned siblings an explicit stacking order", + "when_to_apply": "placing two absolutely-positioned elements in the same area", + }) attrs.setdefault("status", None) attrs.setdefault("arose_from_id", None) attrs.setdefault("verify_with", None) diff --git a/tests/test_autoinject_context.py b/tests/test_autoinject_context.py new file mode 100644 index 0000000..63dab32 --- /dev/null +++ b/tests/test_autoinject_context.py @@ -0,0 +1,190 @@ +"""The auto-inject query carries the conversation beside a thin prompt (#4364). + +The notes arm used to retrieve against the operator's typed words alone, and a +mid-session follow-up — "yes do that", "now fix the filter" — names nothing a +record can match. The hook now reads the tail of the last assistant reply out +of the transcript and sends it as `ctx`; the server appends it to a SHORT +prompt only. Two halves, pinned separately: what the server builds, and what +the hook extracts. +""" +from __future__ import annotations + +import json +import subprocess +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from scribe.services.plugin_context import ( + _AUTOINJECT_CONTEXT_MAX, + _AUTOINJECT_CONTEXT_PROMPT_MAX, + _autoinject_query, + _menu_name, + _menu_passage, +) +from tests.helpers import fake_note, need_tools + +DEFS = Path(__file__).resolve().parents[1] / "plugin" / "hooks" / "scribe_defs.sh" + + +# --- the query the server builds --------------------------------------------- + +def test_a_short_prompt_is_followed_by_the_context(): + q = _autoinject_query("yes do that", "I'll move the library filters into the side column.") + assert q.startswith("yes do that\n\n") + assert "library filters" in q + + +def test_a_prompt_that_says_what_it_is_about_is_left_alone(): + long_prompt = "x" * (_AUTOINJECT_CONTEXT_PROMPT_MAX + 1) + assert _autoinject_query(long_prompt, "anything at all") == long_prompt + + +def test_no_context_is_the_prompt_unchanged(): + assert _autoinject_query("yes do that", "") == "yes do that" + assert _autoinject_query("yes do that", " \n ") == "yes do that" + + +def test_context_is_cut_from_the_end_of_the_reply(): + # The reply's close is where it says what it did and what is next — the + # part the operator's follow-up is answering. + ctx = "HEAD " + "m" * 2000 + " TAIL" + q = _autoinject_query("ok", ctx) + tail = q.split("\n\n", 1)[1] + assert len(tail) == _AUTOINJECT_CONTEXT_MAX + assert tail.endswith("TAIL") and "HEAD" not in tail + + +# --- what the hook extracts -------------------------------------------------- + +def _rec(role: str, blocks: list[dict], sidechain: bool = False) -> str: + return json.dumps({ + "type": role, "isSidechain": sidechain, + "message": {"role": role, "content": blocks}, + }, separators=(",", ":")) + + +def recent(tmp_path: Path, lines: list[str]) -> str: + need_tools("bash", "awk", "grep", "tail") + t = tmp_path / "t.jsonl" + t.write_text("\n".join(lines) + "\n") + r = subprocess.run( + ["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\nscribe_recent_context "{t}"'], + capture_output=True, + ) + assert r.returncode == 0, r.stderr.decode() + return r.stdout.decode().strip() + + +def test_the_hook_reads_the_last_assistant_text(tmp_path): + out = recent(tmp_path, [ + _rec("assistant", [{"type": "text", "text": "an older reply"}]), + _rec("assistant", [{"type": "tool_use", "id": "t1", "name": "Bash", "input": {}}]), + _rec("assistant", [{"type": "text", "text": "Shall I file it as an issue?\nNext: the hook."}]), + ]) + assert out == "Shall I file it as an issue? Next: the hook." + + +def test_a_subagent_reply_is_not_this_session(tmp_path): + out = recent(tmp_path, [ + _rec("assistant", [{"type": "text", "text": "the session's reply"}]), + _rec("assistant", [{"type": "text", "text": "a subagent's report"}], sidechain=True), + ]) + assert out == "the session's reply" + + +def test_the_hook_caps_what_it_sends(tmp_path): + out = recent(tmp_path, [_rec("assistant", [{"type": "text", "text": "a" * 5000 + "END"}])]) + assert len(out) == 600 and out.endswith("END") + + +def test_no_transcript_is_silence_not_failure(tmp_path): + assert recent(tmp_path, []) == "" + + +# --- what a menu line carries (#4364) ---------------------------------------- +# +# The name, the kind and System, and the WHOLE matched passage — once. A repeat +# is a pointer. These pin the shape against the three ways it had gone wrong: +# a trigger-composed title rendered as the line (1,500+ chars), the passage cut +# to its head (which was the title again), and a `seen` repeat re-rendered whole. + +TRIGGER = "Adding a record type that is semantically searchable. " * 20 + + +def test_a_snippet_line_shows_its_name_not_its_trigger(): + assert _menu_name(f"embed_x — {TRIGGER}", "snippet", {"name": "embed_x"}) == "embed_x" + # With no mirror, the first separator is the seam (snippets.py's inverse). + assert _menu_name(f"embed_x — {TRIGGER}", "snippet", None) == "embed_x" + + +def test_a_lesson_line_shows_its_subject_not_its_trigger(): + title = f"A guard does not undo a stored value — {TRIGGER.strip()}" + name = _menu_name(title, "lesson", {"when_to_apply": TRIGGER.strip()}) + assert name == "A guard does not undo a stored value" + + +def test_a_plain_note_keeps_its_title_dashes_and_all(): + t = "Dev-log 2026-07-29 — milestone #232 closed" + assert _menu_name(t, "note", None) == t + + +def test_the_passage_is_the_whole_chunk_without_its_title_prefix(): + body = "section " * 150 # ~1.2 KB: nothing of it is cut + out = _menu_passage("Pool sizing", f"Pool sizing\n{body}\nsecond line") + assert not out.startswith("Pool sizing") + assert out.endswith("second line") and "\n" not in out + assert len(out) > 1100 + + +def test_a_title_only_match_hands_over_the_trigger_it_matched_on(): + title = "embed_x — when adding a searchable record" + assert _menu_passage(title, title, "embed_x") == "when adding a searchable record" + + +async def _menu(hits, seen, chunks, systems=None): + from scribe.services import plugin_context as pc + + calls: list[int] = [] + + async def _search(*_a, **kw): + calls.append(1) + if len(calls) > 1: + return [] + if kw.get("report") is not None: + kw["report"]["best_chunk"] = chunks + 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", _search), \ + patch.object(pc, "superseded_ids", AsyncMock(return_value=set())), \ + patch.object(pc, "system_names_for", AsyncMock(return_value=systems or {})), \ + patch.object(pc, "record_retrieval", MagicMock()), \ + patch.object(pc, "record_surfaced", MagicMock()): + return (await pc.build_autoinject_hint(1, "q", project_id=2, exclude_ids=seen))["context"] + + +@pytest.mark.asyncio +async def test_a_first_sighting_carries_name_system_and_passage(): + title = f"embed_x — {TRIGGER}" + hits = [(0.8, fake_note(id=11, title=title, note_type="snippet", + data={"name": "embed_x"}, user_id=1))] + out = await _menu(hits, [], {11: {"index": 1, "text": f"{title}\nthe matched section"}}, + systems={11: ["Retrieval & recall"]}) + line = next(ln for ln in out.splitlines() if "#11" in ln) + assert '[snippet · Retrieval & recall] "embed_x"' in line + assert TRIGGER[:40] not in line + assert "> ↳ the matched section" in out + + +@pytest.mark.asyncio +async def test_a_seen_record_is_a_pointer_not_a_copy(): + title = f"embed_x — {TRIGGER}" + hits = [(0.8, fake_note(id=11, title=title, note_type="snippet", + data={"name": "embed_x"}, user_id=1))] + out = await _menu(hits, [11], {11: {"index": 1, "text": f"{title}\nthe matched section"}}) + line = next(ln for ln in out.splitlines() if "#11" in ln) + assert line == "> - #11 [snippet · seen] embed_x" + assert "↳" not in out diff --git a/tests/test_backfill_reads_current_text.py b/tests/test_backfill_reads_current_text.py index 999ca5d..6668a2a 100644 --- a/tests/test_backfill_reads_current_text.py +++ b/tests/test_backfill_reads_current_text.py @@ -56,7 +56,7 @@ async def test_the_backfill_embeds_the_text_as_it_is_now_not_as_it_was_scanned() with ( patch.object(emb, "async_session", return_value=_ctx(scan)), patch.object(emb, "_current_row", - AsyncMock(return_value=(42, "T", *edited))), + AsyncMock(return_value=(42, "T", *edited, "note", None))), patch.object(emb, "upsert_note_embedding", AsyncMock()) as upsert, patch.object(emb.asyncio, "sleep", AsyncMock()), ): @@ -75,7 +75,7 @@ async def test_a_record_deleted_between_the_scan_and_the_loop_is_skipped(): with ( patch.object(emb, "async_session", return_value=_ctx(scan)), patch.object(emb, "_current_row", - AsyncMock(side_effect=[None, (42, "T", "body")])), + AsyncMock(side_effect=[None, (42, "T", "body", "note", None)])), patch.object(emb, "upsert_note_embedding", AsyncMock()) as upsert, patch.object(emb.asyncio, "sleep", AsyncMock()), ): @@ -99,7 +99,7 @@ async def test_a_record_whose_text_outran_its_vectors_is_re_embedded(): with ( patch.object(emb, "async_session", return_value=_ctx(scan)), - patch.object(emb, "_current_row", AsyncMock(return_value=(42, "T", "b"))), + patch.object(emb, "_current_row", AsyncMock(return_value=(42, "T", "b", "note", None))), patch.object(emb, "upsert_note_embedding", AsyncMock()) as upsert, patch.object(emb.asyncio, "sleep", AsyncMock()), ): @@ -119,7 +119,7 @@ async def test_a_task_logged_since_its_vectors_is_re_embedded(): with ( patch.object(emb, "async_session", return_value=_ctx(scan)), - patch.object(emb, "_current_row", AsyncMock(return_value=(42, "T", "b"))), + patch.object(emb, "_current_row", AsyncMock(return_value=(42, "T", "b", "note", None))), patch.object(emb, "upsert_note_embedding", AsyncMock()) as upsert, patch.object(emb.asyncio, "sleep", AsyncMock()), ): diff --git a/tests/test_chunking.py b/tests/test_chunking.py index e775119..10d881a 100644 --- a/tests/test_chunking.py +++ b/tests/test_chunking.py @@ -306,7 +306,7 @@ async def test_backfill_reembeds_notes_with_a_stale_chunker_version(): with ( patch.object(emb, "async_session", return_value=ctx), patch.object(emb, "_current_row", - AsyncMock(return_value=(42, "stale-version", "body"))), + AsyncMock(return_value=(42, "stale-version", "body", "note", None))), patch.object(emb, "upsert_note_embedding", AsyncMock()) as upsert, patch.object(emb.asyncio, "sleep", AsyncMock()), ): diff --git a/tests/test_derived_mirror_generic_door.py b/tests/test_derived_mirror_generic_door.py index 7d70e68..95af1c5 100644 --- a/tests/test_derived_mirror_generic_door.py +++ b/tests/test_derived_mirror_generic_door.py @@ -164,11 +164,11 @@ def _lesson_body(trigger, insight="Read the job log.", sources=None): @pytest.mark.asyncio async def test_a_body_write_moves_a_lessons_trigger_with_it(): - from scribe.services.lessons import TRIGGER_KEY, compose_title + from scribe.services.lessons import TRIGGER_KEY what = "Read the job log before waiting longer" note = fake_lesson( - title=compose_title(what, NEW_TRIGGER), + title=what, data={TRIGGER_KEY: "a CI run is slow", "what": what}, project_id=None, ) @@ -183,13 +183,16 @@ async def test_a_body_write_moves_a_lessons_trigger_with_it(): @pytest.mark.asyncio async def test_a_subject_containing_an_em_dash_still_splits(): """Why `untrigger_title` is given the trigger instead of splitting on the - separator: a subject may legitimately contain one.""" - from scribe.services.lessons import TRIGGER_KEY, compose_title + separator: a subject may legitimately contain one. The title here is an + UN-MIGRATED one, still carrying its trigger (milestone 427), because that + is the row the inverse still has to read correctly.""" + from scribe.services.embeddings import trigger_title + from scribe.services.lessons import TRIGGER_KEY what = "A wait with no deadline — the shape, not the symptom" trigger = "you are about to await something crossing a process boundary" note = fake_lesson( - title=compose_title(what, trigger), data=None, project_id=None, + title=trigger_title(what, trigger), data=None, project_id=None, ) await _update(note, body=_lesson_body(trigger)) assert note.data["what"] == what @@ -202,10 +205,10 @@ async def test_dropping_the_provenance_line_drops_it_from_the_mirror(): the failure this recompose exists to prevent, not a courtesy — the opposite call from a snippet's `verification`, which is carried because it was never in the body to delete.""" - from scribe.services.lessons import SOURCES_KEY, compose_title + from scribe.services.lessons import SOURCES_KEY note = fake_lesson( - title=compose_title("Something learned", "a situation"), + title="Something learned", data={SOURCES_KEY: [999]}, project_id=None, ) @@ -229,7 +232,7 @@ async def test_an_explicit_data_wins_for_a_lesson_too(): async def test_a_lesson_title_change_reaches_the_mirror(): """A lesson's subject lives in its title, so a title edit is a trigger for recomposition exactly as it is for a snippet's name.""" - from scribe.services.lessons import TRIGGER_KEY, compose_title + from scribe.services.lessons import TRIGGER_KEY trigger = "two absolute siblings overlap" note = fake_lesson( @@ -237,7 +240,7 @@ async def test_a_lesson_title_change_reaches_the_mirror(): data={TRIGGER_KEY: trigger, "what": "the old subject"}, project_id=None, ) - await _update(note, title=compose_title("the new subject", trigger)) + await _update(note, title="the new subject") assert note.data["what"] == "the new subject" assert note.data[TRIGGER_KEY] == trigger diff --git a/tests/test_integration_lesson_kind.py b/tests/test_integration_lesson_kind.py index ef2162f..9d907cf 100644 --- a/tests/test_integration_lesson_kind.py +++ b/tests/test_integration_lesson_kind.py @@ -67,7 +67,7 @@ async def test_a_lesson_is_a_row_the_database_accepts(owner_id): value, and stays correct if it is gated with it.""" lesson = await notes_svc.create_note( owner_id, - title=lessons_svc.compose_title(SUBJECT, TRIGGER), + title=SUBJECT, body=f"**When to apply:** {TRIGGER}\n\nOne change at a time.", note_type=lessons_svc.LESSON_NOTE_TYPE, data={lessons_svc.TRIGGER_KEY: TRIGGER}, @@ -83,7 +83,8 @@ async def test_a_lesson_is_a_row_the_database_accepts(owner_id): # and the readable body — because the vector is built from the text and # the queries are built from the mirror. assert lessons_svc.lesson_trigger(stored) == TRIGGER - assert stored.title == f"{SUBJECT} — {TRIGGER}" + # The subject alone (milestone 427): the trigger is in `data` and the body. + assert stored.title == SUBJECT assert "**When to apply:**" in (stored.body or "") @@ -97,7 +98,7 @@ async def test_a_lesson_is_not_a_task(owner_id): """ lesson = await notes_svc.create_note( owner_id, - title=lessons_svc.compose_title(SUBJECT, TRIGGER), + title=SUBJECT, body="One change at a time.", note_type=lessons_svc.LESSON_NOTE_TYPE, ) diff --git a/tests/test_lesson_document_shape.py b/tests/test_lesson_document_shape.py index b868aad..21af5de 100644 --- a/tests/test_lesson_document_shape.py +++ b/tests/test_lesson_document_shape.py @@ -1,4 +1,4 @@ -"""The document a lesson is embedded as (milestone 385 step 3). +"""The document a lesson is embedded as (milestone 385 step 3; milestone 427). WHY THIS IS THE STEP THAT DECIDES THE MILESTONE @@ -7,26 +7,20 @@ as ordinary prose is a note wearing a label: it would look right in every listing and simply never be retrieved at the moment it applies, and nothing anywhere would report that. -WHY THERE IS NO `lesson_document()` BESIDE `rule_document()` +WHERE THE SHARP SHAPE LIVES -The step anticipated one. There isn't, and the difference is where the sharp -shape LIVES rather than whether it exists. +A snippet, which note #2485 measured as the only sharp record in the corpus (a +0.153 top-to-second gap against 0.010–0.023 for everything else), is sharp +because its document states its purpose twice: `name — when to use` as the +title, and again as the body's first line. A lesson follows the snippet. -A rule keeps its trigger in a column and its title is a plain name, so the -`{title} — {trigger}` document has to be synthesised at embed time and exists -nowhere else — that is what `rule_document` is for. A snippet, which note #2485 -measured as the only sharp record in the corpus (a 0.153 top-to-second gap -against 0.010–0.023 for everything else), gets there the other way: its STORED -title is already the join and its stored body already opens with the trigger, -so the ordinary `title\\nbody` join is the sharp document. A lesson follows the -snippet, which is what step 1 decided and step 2 built. +Until milestone 427 that `subject — trigger` title was also the STORED title, +so every listing, menu and search row showed the trigger too — kilobytes of it. +Now the stored title is the subject, and `embeddings.document_title` joins the +trigger back from `data` at embed time. The embedded TEXT is what it always +was, which is the property these guards pin: nothing re-embeds, and the floors +tuned against these vectors stay calibrated. -The consequence worth stating: `chunk_document` is untouched, so -`CHUNKER_VERSION` does not move and nothing re-embeds. The step's "Re-embed" -section describes a change this design does not make. - -These guards therefore assert the composed record, then assert that the generic -chunker turns it into the intended document — the two halves of the same claim. No similarity number is asserted anywhere: a threshold pins the embedder's behaviour rather than this code's, and breaks on a model change that is not a regression. @@ -34,18 +28,46 @@ regression. from __future__ import annotations from scribe.services import lessons as lessons_svc -from scribe.services.embeddings import chunk_document, embedding_text +from scribe.services.embeddings import chunk_document, document_title, embedding_text TRIGGER = "a test fails on code you believe is correct" SUBJECT = "Suspect the guard before the code" INSIGHT = "Check whether the assertion still describes the property it was written for." +def _embedded(what: str, trigger: str, insight: str) -> tuple[str, str]: + """The (title, body) the write path EMBEDS a lesson as — built the way + `notes.embed_note` builds it, from what `create_lesson` stores.""" + title, body = lessons_svc.lesson_document(what, trigger, insight) + data = lessons_svc.compose_data(what, trigger) + return document_title(title, lessons_svc.LESSON_NOTE_TYPE, data, body), body + + +def test_the_stored_title_is_the_subject_alone(): + """Milestone 427. The trigger lives in `data` and the body's first line; the + title a listing shows is what the lesson is ABOUT.""" + title, _ = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) + assert title == SUBJECT + + +def test_the_embedded_document_is_the_one_it_always_was(): + """THE no-re-embed guard. The document text must be byte-identical to what + a lesson embedded as when its stored title carried the trigger — and an + un-migrated row, whose stored title still does, must come out the same + rather than with the trigger twice.""" + legacy_title = f"{SUBJECT} — {TRIGGER}" + title, body = _embedded(SUBJECT, TRIGGER, INSIGHT) + assert title == legacy_title + + data = lessons_svc.compose_data(SUBJECT, TRIGGER) + assert document_title(legacy_title, lessons_svc.LESSON_NOTE_TYPE, data, body) == legacy_title + + def test_the_trigger_appears_twice_in_the_document(): - """THE guard. Purpose stated twice in a short document is the entire - measured cause of a snippet's sharpness, and it is the one property that - distinguishes a lesson's vector from a plain note's.""" - title, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) + """Purpose stated twice in a short document is the entire measured cause of + a snippet's sharpness, and it is the one property that distinguishes a + lesson's vector from a plain note's.""" + title, body = _embedded(SUBJECT, TRIGGER, INSIGHT) document = embedding_text(title, body) assert document.count(TRIGGER) == 2 @@ -55,22 +77,28 @@ def test_the_trigger_appears_twice_in_the_document(): def test_the_document_leads_with_when_it_applies(): - """The title is `{what} — {when}` and the body's FIRST line restates it, so - the opening of the document is about the situation rather than the topic. - A lesson buried behind a paragraph of narrative would rank on the - narrative.""" - title, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) + """The embedded title is `{what} — {when}` and the body's FIRST line + restates it, so the opening of the document is about the situation rather + than the topic.""" + title, body = _embedded(SUBJECT, TRIGGER, INSIGHT) assert title == f"{SUBJECT} — {TRIGGER}" assert body.splitlines()[0] == f"**When to apply:** {TRIGGER}" +def test_the_trigger_reaches_the_document_even_without_the_mirror(): + """A row whose `data` lost its mirror still embeds sharply: the trigger is + read back from the body, the same fallback `lesson_trigger` has.""" + title, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) + assert document_title(title, lessons_svc.LESSON_NOTE_TYPE, None, body) == ( + f"{SUBJECT} — {TRIGGER}" + ) + + def test_a_short_lesson_is_exactly_one_chunk(): """`chunk_document`'s first contract line: a record inside the window - yields one chunk identical to the historical `title\\nbody`. A lesson that - split into several would spread the trigger's weight across vectors that - each carry less of it.""" - title, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) + yields one chunk identical to the historical `title\\nbody`.""" + title, body = _embedded(SUBJECT, TRIGGER, INSIGHT) chunks = chunk_document(title, body) assert len(chunks) == 1 @@ -78,26 +106,15 @@ def test_a_short_lesson_is_exactly_one_chunk(): def test_a_long_lesson_keeps_the_trigger_on_every_chunk(): - """The narrative question, answered by the chunker rather than by holding - the story out of the record. - - `rule_document` excludes a rule's `why` because long dated narrative made - sixteen dev-logs land on the centroid of "development". That finding - predates chunking (#280): a body over budget is now split, and EVERY chunk - is prefixed with the title — which for a lesson carries the trigger. So the - story occupies its own vectors instead of averaging itself into the - trigger's, and each of those vectors is still anchored to when the lesson - applies. - - This is why the insight stays in the body where a reader can see it. Holding - it out would cost the reader the only part that explains the lesson, to buy - a sharpness the chunker already provides. + """Every chunk is prefixed with the embedded title, which carries the + trigger — so a long story occupies its own vectors instead of averaging + itself into the trigger's, and each is still anchored to when it applies. """ narrative = "\n\n".join( f"## Section {i}\n" + ("An unrelated sentence about deployment. " * 40) for i in range(6) ) - title, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, narrative) + title, body = _embedded(SUBJECT, TRIGGER, narrative) chunks = chunk_document(title, body) assert len(chunks) > 1, "the fixture must actually exceed the chunk budget" @@ -106,10 +123,8 @@ def test_a_long_lesson_keeps_the_trigger_on_every_chunk(): def test_a_lesson_with_no_trigger_still_embeds(): """Degrades to title + insight, the way a rule with no trigger does — less - sharply, and still findable. That is an argument for prompting hard for a - trigger at write time, not for padding the document with whatever text is - to hand.""" - title, body = lessons_svc.lesson_document(SUBJECT, "", INSIGHT) + sharply, and still findable.""" + title, body = _embedded(SUBJECT, "", INSIGHT) assert title == SUBJECT assert body == INSIGHT @@ -119,8 +134,7 @@ def test_a_lesson_with_no_trigger_still_embeds(): def test_the_composed_body_is_the_one_the_reader_is_parsed_back_from(): """`compose_body` writes the trigger line and `lesson_trigger` reads it. A lesson whose mirror in `data` is missing still answers correctly, so the - two must agree on the exact markdown — which is why neither is written by - hand at a call site.""" + two must agree on the exact markdown.""" from types import SimpleNamespace _, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) @@ -130,10 +144,8 @@ def test_the_composed_body_is_the_one_the_reader_is_parsed_back_from(): def test_the_title_and_body_are_composed_by_one_call(): - """`lesson_document` returns both halves so they cannot be built apart. A - title carrying the trigger over a body that does not would embed as an - ordinary note, and every listing would still look correct.""" + """`lesson_document` returns both halves so they cannot be built apart.""" assert lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) == ( - lessons_svc.compose_title(SUBJECT, TRIGGER), + SUBJECT, lessons_svc.compose_body(INSIGHT, TRIGGER), ) diff --git a/tests/test_lesson_kind.py b/tests/test_lesson_kind.py index 25fdd2a..7753af7 100644 --- a/tests/test_lesson_kind.py +++ b/tests/test_lesson_kind.py @@ -29,7 +29,6 @@ from types import SimpleNamespace from scribe.services import knowledge as knowledge_svc from scribe.services import lessons as lessons_svc -from scribe.services import snippets as snippets_svc from scribe.services.embeddings import trigger_title @@ -76,15 +75,18 @@ def test_one_join_builds_every_trigger_title(): expected = f"{subject} — {trigger}" assert trigger_title(subject, trigger) == expected - assert lessons_svc.compose_title(subject, trigger) == expected - assert snippets_svc.compose_title(subject, trigger) == expected + # The two note kinds reach it through the EMBEDDED title (milestone 427), + # from their own mirror key — never a hand-rolled join. + from scribe.services.embeddings import document_title + assert document_title(subject, "lesson", {"when_to_apply": trigger}) == expected + assert document_title(subject, "snippet", {"when_to_use": trigger}) == expected def test_a_subject_with_no_trigger_degrades_to_the_subject(): """It still embeds, just less sharply — an argument for backfilling triggers, not for padding the title with whatever text is to hand.""" assert trigger_title("debounce", "") == "debounce" - assert lessons_svc.compose_title(" debounce ") == "debounce" + assert lessons_svc.lesson_document(" debounce ")[0] == "debounce" assert trigger_title("", "when it applies") == "when it applies" diff --git a/tests/test_lesson_rest_door.py b/tests/test_lesson_rest_door.py index d853696..f2f8301 100644 --- a/tests/test_lesson_rest_door.py +++ b/tests/test_lesson_rest_door.py @@ -214,13 +214,13 @@ def test_the_payload_reads_back_the_composed_fields(): """A caller that wrote `when_to_apply` reads `when_to_apply` back, not a body it has to parse.""" from tests.helpers import fake_lesson - from scribe.services.lessons import compose_body, compose_title, lesson_to_dict + from scribe.services.lessons import compose_body, lesson_to_dict what = "Read the job log before waiting longer" trigger = "a CI run has sat in_progress longer than its suite takes" note = fake_lesson( id=7, - title=compose_title(what, trigger), + title=what, body=compose_body("The work is usually done.", trigger, [4181]), data={"what": what, "when_to_apply": trigger, "taught_by": [4181]}, project_id=None, diff --git a/tests/test_lesson_write_path.py b/tests/test_lesson_write_path.py index 19d8b96..03a7159 100644 --- a/tests/test_lesson_write_path.py +++ b/tests/test_lesson_write_path.py @@ -117,7 +117,8 @@ async def test_the_duplicate_gate_runs_before_anything_is_created(): @pytest.mark.asyncio async def test_the_gate_compares_the_composed_document_not_the_raw_fields(): """What reaches the gate is the title and body a lesson will actually be - stored as. Comparing `what` alone would miss that the trigger is half the + stored as, plus the `data` its EMBEDDED title is built from (milestone + 427). Comparing `what` alone would miss that the trigger is half the document, and would judge two lessons alike that rank nothing alike.""" _user_id_ctx.set(7) gate = AsyncMock(return_value=None) @@ -128,8 +129,9 @@ async def test_the_gate_compares_the_composed_document_not_the_raw_fields(): await create_lesson(what=SUBJECT, when_to_apply=TRIGGER, insight="Look.") title, body = gate.await_args.args[1], gate.await_args.args[2] - assert title == f"{SUBJECT} — {TRIGGER}" + assert title == SUBJECT assert body.startswith(f"**When to apply:** {TRIGGER}") + assert gate.await_args.kwargs["data"]["when_to_apply"] == TRIGGER assert gate.await_args.kwargs["note_type"] == "lesson" @@ -157,9 +159,11 @@ def test_a_lesson_is_judged_at_the_trigger_dominated_bar(): @pytest.mark.asyncio async def test_an_update_recomposes_both_halves_of_the_document(): - """A new trigger has to reach the title AND the head of the body. Patching - one would leave a lesson that reads correctly and ranks on the old - situation — the failure mode with no symptom.""" + """A new trigger has to reach the mirror AND the head of the body — the two + places the embedded document reads it from (milestone 427). Patching one + would leave a lesson that reads correctly and ranks on the old situation — + the failure mode with no symptom. The title stays the subject, even when + the stored one was an un-migrated composed title.""" _user_id_ctx.set(7) stored = _stub_note( title=f"{SUBJECT} — {TRIGGER}", @@ -172,7 +176,7 @@ async def test_an_update_recomposes_both_halves_of_the_document(): await lessons_svc.update_lesson(7, 1, when_to_apply="a guard goes red") fields = updated.await_args.kwargs - assert fields["title"] == f"{SUBJECT} — a guard goes red" + assert fields["title"] == SUBJECT assert fields["body"].startswith("**When to apply:** a guard goes red") assert fields["data"]["when_to_apply"] == "a guard goes red" diff --git a/tests/test_services_snippets.py b/tests/test_services_snippets.py index 357c677..10ab94f 100644 --- a/tests/test_services_snippets.py +++ b/tests/test_services_snippets.py @@ -2,10 +2,18 @@ from scribe.services import snippets as s -def test_compose_title_with_and_without_usage(): - assert s.compose_title("debounce", "rate-limit a callback") == "debounce — rate-limit a callback" - assert s.compose_title(" debounce ", "") == "debounce" - assert s.compose_title("debounce") == "debounce" +def test_the_embedded_title_joins_the_trigger_the_stored_one_does_not_carry(): + """Milestone 427: stored title = name; the trigger joins it at embed time, + idempotently, so an old composed title comes out the same.""" + from scribe.services.embeddings import document_title + + data = {"name": "debounce", "when_to_use": "rate-limit a callback"} + assert document_title("debounce", "snippet", data) == "debounce — rate-limit a callback" + assert document_title("debounce — rate-limit a callback", "snippet", data) == ( + "debounce — rate-limit a callback" + ) + assert document_title("debounce", "snippet", {"name": "debounce"}) == "debounce" + assert document_title("a — note", "note", data) == "a — note" def test_compose_tags_lowercases_language_and_dedups(): @@ -32,7 +40,7 @@ def test_compose_body_bare_code_only(): def test_parse_round_trips_a_composed_snippet(): - title = s.compose_title("useDebouncedRef", "debounce a reactive ref") + title = "useDebouncedRef" body = s.compose_body( code="const x = 1", language="ts", signature="useDebouncedRef(v, ms)", when_to_use="debounce a reactive ref", repo="scribe", @@ -241,9 +249,9 @@ def test_data_and_body_round_trip_to_the_same_fields(): name, when, sig, lang = ("debounce", "rate-limit a callback", "debounce(fn, ms)", "ts") locs = [{"repo": "web", "path": "src/util.ts", "symbol": "debounce"}] - # compose_body takes no `name` — the name lives in the title — so the two - # serializers get their own argument lists rather than a shared spread. - title = s.compose_title(name, when) + # compose_body takes no `name` — the name IS the title (milestone 427) — so + # the two serializers get their own argument lists rather than a shared spread. + title = name body = s.compose_body(code="const x = 1", language=lang, signature=sig, when_to_use=when, locations=locs, merged_from=[41, 42]) tags = s.compose_tags(lang) diff --git a/tests/test_session_context_failure.py b/tests/test_session_context_failure.py new file mode 100644 index 0000000..da68727 --- /dev/null +++ b/tests/test_session_context_failure.py @@ -0,0 +1,54 @@ +"""A SessionStart that loads no project says why, and names the first move (#4366). + +The fetch used to be `curl -f … || body=""`: a timeout, an HTTP error and a +refused key all produced one sentence, which ended by suggesting +`enter_project()` "as needed". A session that read it as optional started with +no recent milestones or open tasks, and so had no way to know which prior work +existed to look for. These pin the two halves of the fix on the path that needs +no server: the cause is named, and the fallback is a concrete first step. +""" +from __future__ import annotations + +import json +import os +import subprocess +from pathlib import Path + +from tests.helpers import need_tools + +HOOK = Path(__file__).resolve().parents[1] / "plugin" / "hooks" / "scribe_session_context.sh" + + +def run_hook(tmp_path: Path, url: str, marker_project: int | None = None) -> str: + need_tools("bash", "curl", "awk") + if marker_project is not None: + (tmp_path / ".scribe").write_text(json.dumps({ + "instance": url, "project_id": marker_project, "project": "P", + })) + env = {**os.environ, "SCRIBE_URL": url, "SCRIBE_TOKEN": "t", + "CLAUDE_PROJECT_DIR": str(tmp_path)} + r = subprocess.run(["bash", str(HOOK)], input=b'{"source":"startup"}', + capture_output=True, env=env, timeout=60) + assert r.returncode == 0, r.stderr.decode() + return json.loads(r.stdout)["hookSpecificOutput"]["additionalContext"] + + +# Port 9 (discard) on loopback: refused at once, so both tries fail fast. +DEAD = "http://127.0.0.1:9" + + +def test_an_unreachable_instance_is_named_as_one(tmp_path): + out = run_hook(tmp_path, DEAD) + assert "could not be reached (curl exit 7, after a retry)" in out + + +def test_the_fallback_is_a_first_step_not_an_option(tmp_path): + out = run_hook(tmp_path, DEAD) + assert "as needed" not in out + assert "Start by finding this repo's project" in out + assert "before any other work" in out + + +def test_a_marker_names_the_exact_call(tmp_path): + out = run_hook(tmp_path, DEAD, marker_project=31) + assert "Start by calling `enter_project(31)`" in out