From f80401d58e41ddf7c924b747fcdb59deabdc9886 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 28 Aug 2026 12:06:43 -0400 Subject: [PATCH] fix(knowledge): the browse vocabulary catches up three kinds, and a snippet's mirror survives the generic door (#3128 recs 2-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spike #3128 found the storage sound and the retrieval vocabulary frozen before `issue` shipped (0065). Five things, in the order they had to land. **The mirror (rec 5, the data-integrity one).** `notes.data` is DERIVED from a snippet's body, but only `update_snippet` knew that. `update_note` is a hasattr loop with no snippet awareness, and both doors reach it — so PATCH /api/notes/ {body} rewrote the body and left the mirror behind. `snippet_fields` PREFERS the mirror, so the row went on reporting its old repo/path/symbol to the location reverse lookup and to prior-art recall while displaying its new body: surfaced with full authority, and wrong. `snippets.recompose_data` rebuilds it from the body, carrying `verification` and `provenance` (neither is in the body to parse). An explicit `data` still wins, so every snippet-service write is untouched. **One facet table (rec 3), before adding any facet.** The type predicate was written three times — SQL, Python over semantic candidates, and a ternary computing the `is_task` pre-filter — and agreed only by luck. Adding `issue` to the SQL arm alone would have set the pre-filter to is_task=False, handed the Python arm a candidate set with no tasks in it, and returned an empty semantic half for the Issues facet forever with nothing red. `_FACETS` now generates all three. The Python arm also regains the `status IS NULL` half its SQL twin always had. **Issue and spike become facets (rec 2).** 435 issues — 17% of every task — were filterable nowhere on the human surface, while retired `plan` (90 rows) had a chip of its own. `_VALID_TYPES` was a hand-kept copy and is now derived. `plan` stays a valid facet for its legacy rows; it loses its chip. **Snippets stop being half-present in the feed (rec 4).** All 90 were in the All list, in no count, wearing an empty badge, and opening in the note editor. Counts now group by task_kind — every kind for the same two round-trips, which is why `issue` had no number — and total includes snippets, so the All chip matches the list it labels. Snippet cards route to /snippets/:id. **The prose that excused it (rec 6).** `snippet_fields` and the `data` column both still said pre-0070 rows were "never backfilled". True when 0070 landed, false since `backfill_snippet_data` shipped, and it read as licence for a stale mirror. Tests: the pre-filter can never exclude a row its own facet accepts (the regression, parameterised over every facet); both dialects select exactly their own rows; an unknown facet matches nothing; the mirror follows a body or title write, carries the verdict, and yields to an explicit `data`. `compiled_sql` moves to tests/helpers rather than becoming a third copy. Write-up: note #3161. --- frontend/src/views/KnowledgeView.vue | 67 +++++++-- src/scribe/models/note.py | 7 +- src/scribe/routes/knowledge.py | 16 +- src/scribe/services/knowledge.py | 175 ++++++++++++++-------- src/scribe/services/notes.py | 24 +++ src/scribe/services/snippets.py | 54 ++++++- tests/helpers.py | 10 ++ tests/test_knowledge_facets.py | 120 +++++++++++++++ tests/test_services_access_visibility.py | 5 +- tests/test_services_knowledge_counts.py | 91 ++++++++--- tests/test_snippet_mirror_generic_door.py | 111 ++++++++++++++ 11 files changed, 576 insertions(+), 104 deletions(-) create mode 100644 tests/test_knowledge_facets.py create mode 100644 tests/test_snippet_mirror_generic_door.py diff --git a/frontend/src/views/KnowledgeView.vue b/frontend/src/views/KnowledgeView.vue index 46ad3ed..281da8b 100644 --- a/frontend/src/views/KnowledgeView.vue +++ b/frontend/src/views/KnowledgeView.vue @@ -24,7 +24,7 @@ const router = useRouter(); interface KnowledgeItem { id: number; - note_type: "note" | "task" | "process"; + note_type: "note" | "task" | "process" | "snippet"; title: string; snippet: string; tags: string[]; @@ -42,9 +42,34 @@ interface KnowledgeItem { task_kind?: TaskKind; } +// ─── The facet vocabulary ───────────────────────────────────────────────────── +// Mirrors services/knowledge._FACETS, which is where it is defined for real. +// A facet spans BOTH typing axes — a record TYPE (note / process / snippet) or +// a task KIND (`task` for any, else issue / spike) — because that is what this +// feed actually holds. +// +// `plan` is still a valid facet at the API, for the 90 legacy plan-tasks, but +// it has no chip: retired in 0066, it kept a chip of its own for longer than +// `issue` — 17% of every task here — went without one (#3128). Those rows are +// still reachable under Tasks, wearing a Plan badge. +type Facet = "" | "note" | "task" | "issue" | "spike" | "snippet" | "process"; + +// The facets that select TASKS. Kinds are subsets of `task`, so any of them +// means the duplicate report should be comparing tasks. +const TASK_FACETS = new Set(["task", "issue", "spike"]); + +const FACET_CHIPS: [Exclude, string][] = [ + ["note", "Notes"], + ["task", "Tasks"], + ["issue", "Issues"], + ["spike", "Spikes"], + ["snippet", "Snippets"], + ["process", "Processes"], +]; + // ─── Filter state ───────────────────────────────────────────────────────────── -const activeType = ref<"" | "note" | "task" | "plan" | "process">(""); +const activeType = ref(""); const activeTag = ref(""); const sortMode = ref<"modified" | "created" | "alpha" | "type">("modified"); const searchQuery = ref(""); @@ -70,9 +95,10 @@ const dupGroups = ref([]); const dupSuggestion = ref(""); const dupLoading = ref(false); const dupChecked = ref(false); -// The report follows the type filter: viewing tasks checks tasks. Anything -// else (all / plan / process) checks notes — the kind with the most to find. -const dupKind = computed(() => (activeType.value === "task" ? "task" : "note")); +// The report follows the type filter: viewing tasks — under ANY task facet, +// including a single kind — checks tasks. Everything else checks notes, the +// kind with the most to find. +const dupKind = computed(() => (TASK_FACETS.has(activeType.value) ? "task" : "note")); async function loadDuplicates() { dupLoading.value = true; @@ -96,8 +122,11 @@ watch(dupKind, () => { dupChecked.value = false; dupGroups.value = []; }); // ─── Type counts ────────────────────────────────────────────────────────────── -interface KnowledgeCounts { note: number; task: number; plan: number; process: number; total: number } -const typeCounts = ref({ note: 0, task: 0, plan: 0, process: 0, total: 0 }); +// One number per facet, plus the grand total. Partial because the server sends +// a key only for a facet it has rows for. Kinds are subsets of `task` and are +// deliberately absent from `total` — including them would count an issue twice. +type KnowledgeCounts = Partial, number>> & { total: number }; +const typeCounts = ref({ total: 0 }); async function fetchCounts() { try { @@ -274,9 +303,18 @@ function isOverdue(item: KnowledgeItem): boolean { return new Date(item.due_date) < new Date(new Date().toDateString()); } +// Each record kind opens in ITS OWN editor. A snippet used to fall through to +// /notes/:id, whose save is a plain PATCH of the body — which left the snippet's +// derived `data` mirror describing the previous version (#3128). The service now +// recomposes the mirror either way, so this is no longer the guard; it is simply +// that the note editor cannot edit a snippet's signature, language or locations, +// and offering it as the way in was always wrong. Processes stay here on +// purpose: they have no editor of their own and the note editor knows the type. function openItem(item: KnowledgeItem) { if (item.note_type === 'task') { router.push(`/tasks/${item.id}`); + } else if (item.note_type === 'snippet') { + router.push(`/snippets/${item.id}`); } else { router.push(`/notes/${item.id}`); } @@ -380,14 +418,14 @@ onUnmounted(() => { {{ typeCounts.total }} @@ -501,6 +539,7 @@ onUnmounted(() => { Note {{ item.task_kind === 'plan' ? 'Plan' : 'Task' }} Process + Snippet