Plugin reply shape, preferences UI, cited-record status, and the lesson kind #166

Merged
bvandeusen merged 7 commits from dev into main 2026-09-18 16:33:57 -04:00
38 changed files with 2507 additions and 63 deletions
+68 -1
View File
@@ -39,12 +39,27 @@ export interface RulebookTopic {
updated_at: string | null; updated_at: string | null;
} }
/**
* What kind of instruction this is, and it is about FORCE, not importance.
*
* A `rule` must be FOLLOWED — ignoring it breaks something. It is the
* operator's decision, so it changes when they change it. A `preference` is
* how they want work DONE — ignoring it costs consistency, not correctness —
* and the agent rewrites it in the ordinary course of working, which is what
* makes the drift surface necessary.
*
* Always sent by the server, never inferred from an absent key: "no kind
* field" and "kind is rule" must not be the same payload.
*/
export type RuleKind = "rule" | "preference";
export interface Rule { export interface Rule {
id: number; id: number;
topic_id: number | null; topic_id: number | null;
project_id: number | null; project_id: number | null;
title: string; title: string;
statement: string; statement: string;
kind: RuleKind;
/** WHEN this rule fires — the trigger, not the instruction. */ /** WHEN this rule fires — the trigger, not the instruction. */
when_to_apply: string; when_to_apply: string;
why: string; why: string;
@@ -79,6 +94,8 @@ export interface RuleHeader {
title: string; title: string;
statement: string; statement: string;
topic_id: number | null; topic_id: number | null;
/** Unconditional on the wire (services.rulebooks.rule_brief). */
kind: RuleKind;
/** A date (YYYY-MM-DD), not a timestamp. */ /** A date (YYYY-MM-DD), not a timestamp. */
updated_at: string | null; updated_at: string | null;
when_to_apply?: string; when_to_apply?: string;
@@ -180,6 +197,9 @@ export async function getRule(id: number): Promise<Rule> {
export interface RuleWrite { export interface RuleWrite {
title: string; title: string;
statement: string; statement: string;
/** Writable from the editor: a preference is not a lesser rule, it is a
* different force, and the person writing it is the one who knows which. */
kind: RuleKind;
when_to_apply: string; when_to_apply: string;
why: string; why: string;
how_to_apply: string; how_to_apply: string;
@@ -256,9 +276,56 @@ export async function getRuleVersion(
return apiGet<RuleVersion>(`/api/rules/${ruleId}/versions/${versionId}`); return apiGet<RuleVersion>(`/api/rules/${ruleId}/versions/${versionId}`);
} }
// No restoreRuleVersion, deliberately (milestone 323). Putting an old wording // No restore FOR A RULE, deliberately (milestone 323). Putting an old wording
// back goes through updateRule, which snapshots what it replaces — so the // back goes through updateRule, which snapshots what it replaces — so the
// undo stays visible in the history like any other edit. // undo stays visible in the history like any other edit.
//
// A preference is the exception and the server refuses anything else (409).
// 323's reasoning is that a rewrite is the operator's own decision; a
// preference's rewrite is the agent's, made mid-work without asking, so
// putting it back is a veto rather than an undo — and a veto that costs more
// than shrugging is not really supervision. Nothing is erased either way:
// the restore snapshots too, so the history GAINS the revert.
export async function restoreRuleVersion(
ruleId: number, versionId: number,
): Promise<Rule> {
return apiPost<Rule>(`/api/rules/${ruleId}/versions/${versionId}/restore`, {});
}
/**
* One preference that has been rewritten: what it said, what it says now, and
* what taught the change.
*
* Both texts ride along so the list can show the diff without a follow-up call
* per row — a listing that needs N round-trips to say what it means is one
* nobody scrolls, which would leave the drift as unsupervised as before.
*/
export interface PreferenceDrift {
rule: RuleHeader;
/** What it said BEFORE the latest rewrite. */
previous: {
id: number;
created_at: string | null;
title: string;
statement: string;
when_to_apply: string;
};
current: { title: string; statement: string; when_to_apply: string };
/**
* The record named by `arose_from_id` — what the change was learned from.
* Absent when the preference carries none, which is every one written
* before that field was required.
*/
taught_by?: { id: number; title: string };
}
export async function listPreferenceDrift(
limit?: number,
): Promise<PreferenceDrift[]> {
const qs = limit ? `?limit=${limit}` : "";
const data = await apiGet<{ drift: PreferenceDrift[] }>(`/api/rules/drift${qs}`);
return data.drift;
}
export async function deleteRule(id: number): Promise<void> { export async function deleteRule(id: number): Promise<void> {
return apiDelete(`/api/rules/${id}`); return apiDelete(`/api/rules/${id}`);
+13
View File
@@ -40,3 +40,16 @@
padding: 0.05rem 0.4rem; padding: 0.05rem 0.4rem;
vertical-align: middle; vertical-align: middle;
} }
/* A PREFERENCE, marked because force is the one thing a list of instructions
must not leave the reader to infer. A preference does not bind — ignoring it
costs consistency, not correctness — and it is the one kind the agent
rewrites on its own, so a row that renders identically to a rule teaches the
opposite of both facts.
The accent, not the warning colour: nothing is wrong with a preference. It
is a different KIND, and the marker says which. */
.rule-chip-preference {
color: var(--fs-accent);
background: var(--fs-accent-soft);
}
@@ -0,0 +1,223 @@
<script setup lang="ts">
/**
* What Scribe has changed about how it works with you.
*
* THE RISK THIS EXISTS FOR (milestone 399). A preference is the one record
* kind the agent rewrites on its own, mid-work, without asking — which is
* what keeps it current and what makes it dangerous. An agent misreads one
* session, rewrites a preference, and follows the rewritten version forever
* while the operator never sees the moment it changed. That is worse than
* having no preference at all: a confident wrong answer wearing the
* operator's own authority.
*
* `rule_versions` already recorded every rewrite. What it could not do is
* ARRIVE. A history you open one rule at a time, having first suspected that
* rule, is not oversight — so this pane is the PUSH half, and it sits beside
* the staleness sweep for the same reason that does: drift belongs to no one
* rulebook.
*
* Cross-cutting, and deliberately not a filter on the per-topic rule list —
* that list shows one topic of one rulebook, so filtering it would silently
* under-report, which is the exact failure this surface exists to catch.
*/
import { onMounted, ref } from "vue";
import DiffView from "@/components/DiffView.vue";
import { computeDiff } from "@/utils/diff";
import { useRulebooksStore } from "@/stores/rulebooks";
import { useToastStore } from "@/stores/toast";
import type { PreferenceDrift } from "@/api/rulebooks";
const emit = defineEmits<{ "open-rule": [id: number] }>();
const store = useRulebooksStore();
const toast = useToastStore();
const openId = ref<number | null>(null);
const busyId = ref<number | null>(null);
/** Old on the left, new on the right — the direction a reader expects of
* "what changed", and the opposite of the rule history panel, which is
* answering "what did it used to say" from the current text backwards. */
function diffFor(row: PreferenceDrift) {
return computeDiff(row.previous.statement, row.current.statement);
}
function triggerChanged(row: PreferenceDrift): boolean {
return row.previous.when_to_apply !== row.current.when_to_apply;
}
function stamp(iso: string | null): string {
return iso ? iso.slice(0, 10) : "";
}
function toggle(row: PreferenceDrift) {
openId.value = openId.value === row.rule.id ? null : row.rule.id;
}
/** The veto. One action, because a veto that costs more than shrugging is
* not really supervision — and nothing is lost either way: the restore is
* itself an edit, so the rewrite stays in the preference's history with the
* revert recorded after it. */
async function restore(row: PreferenceDrift) {
busyId.value = row.rule.id;
try {
await store.restoreVersion(row.rule.id, row.previous.id);
toast.show(`Put “${row.previous.title}” back`, "success");
openId.value = null;
} catch {
toast.show("Could not put that wording back", "error");
} finally {
busyId.value = null;
}
}
onMounted(() => store.fetchDrift());
</script>
<template>
<section class="pane drift">
<header>
<h2>Recent changes</h2>
<p class="lede">
Preferences Scribe rewrote while working, most recently changed first. A
preference is how you want work done, so sessions keep it current
without asking this is where you see what they decided. Rules are not
here: those change when you change them.
</p>
</header>
<p v-if="store.loading" class="state">Loading</p>
<!-- Nothing changed is the ordinary state and must not read as a fault. -->
<p v-else-if="!store.drift.length" class="state empty">
Nothing has been rewritten. A preference appears here the first time a
session changes one until then there is nothing to review.
</p>
<ol v-else class="rows">
<li v-for="row in store.drift" :key="row.rule.id" class="row">
<div class="row-head">
<button class="row-title" @click="emit('open-rule', row.rule.id)">
{{ row.rule.title }}
</button>
<span class="when">{{ stamp(row.previous.created_at) }}</span>
</div>
<!-- The provenance, named rather than numbered: a bare id reads as
complete to the writer and as homework to the reader. -->
<p v-if="row.taught_by" class="taught">
Learned from <em>{{ row.taught_by.title }}</em>
<span class="taught-id">#{{ row.taught_by.id }}</span>
</p>
<p v-else class="taught untaught">
Nothing recorded what taught this change.
</p>
<button class="expand" :aria-expanded="openId === row.rule.id" @click="toggle(row)">
{{ openId === row.rule.id ? "Hide what changed" : "See what changed" }}
</button>
<div v-if="openId === row.rule.id" class="detail">
<p v-if="triggerChanged(row)" class="trigger-moved">
Its trigger changed too, so it now arrives at a different moment.
<span class="was">Was:</span> {{ row.previous.when_to_apply || "nothing" }}
</p>
<DiffView v-if="diffFor(row).length" :diff="diffFor(row)" />
<p v-else class="state">
The statement is unchanged this edit moved another field.
</p>
<div class="actions">
<button
:disabled="busyId === row.rule.id"
@click="restore(row)"
>Put the old wording back</button>
<span class="actions-note">
Kept, not erased: this is recorded as another edit, so both
wordings stay in the preference's history.
</span>
</div>
</div>
</li>
</ol>
<p v-if="store.drift.length" class="footnote">
One row per preference, carrying its latest rewrite. A preference changed
several times shows the most recent one its full history is in its
editor, under <strong>Edit history</strong>.
</p>
</section>
</template>
<style src="@/assets/rules-shared.css" />
<style scoped>
.drift { display: flex; flex-direction: column; gap: var(--fs-space-3); }
.lede {
margin: 0; max-width: 62ch; font-size: var(--fs-size-body-sm);
color: var(--fs-text-secondary); line-height: var(--fs-leading-body);
}
.state { margin: 0; font-size: var(--fs-size-body-sm); color: var(--fs-text-secondary); }
.state.empty { color: var(--fs-text-tertiary); }
.rows { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: var(--fs-space-3); }
.row {
background: var(--fs-surface-raised);
border-radius: var(--fs-radius-md);
padding: var(--fs-space-3);
}
.row-head { display: flex; align-items: baseline; gap: var(--fs-space-2); flex-wrap: wrap; }
.row-title {
background: none; border: none; padding: 0; cursor: pointer;
font-family: Fraunces, serif; font-style: italic; font-size: 1.02rem;
color: var(--fs-text-primary); text-align: left;
}
.row-title:hover { text-decoration: underline; }
.when {
margin-left: auto; font-size: var(--fs-size-tiny);
color: var(--fs-text-secondary); font-variant-numeric: tabular-nums;
}
.taught {
margin: var(--fs-space-2) 0 0; font-size: var(--fs-size-tiny);
color: var(--fs-text-secondary); line-height: var(--fs-leading-body);
}
.taught em { font-style: italic; color: var(--fs-text-primary); }
.taught-id { margin-left: 0.35rem; color: var(--fs-text-tertiary); font-variant-numeric: tabular-nums; }
/* Not a warning: every preference written before provenance was required has
none, and marking those as faults would cry wolf on the whole backlog. */
.taught.untaught { color: var(--fs-text-tertiary); font-style: italic; }
.expand {
align-self: flex-start; margin-top: var(--fs-space-2);
background: none; border: none; padding: 0; cursor: pointer;
font: inherit; font-size: var(--fs-size-tiny); color: var(--fs-text-secondary);
}
.expand:hover { color: var(--fs-text-primary); text-decoration: underline; }
.detail { margin-top: var(--fs-space-2); display: flex; flex-direction: column; gap: var(--fs-space-2); }
/* A TINT, not the solid token — `--fs-warning-fg` is defined as warning text
ON a warning tint, and painting it over solid `--fs-warning` is the
same-hue contrast failure #3141 records. */
.trigger-moved {
margin: 0; font-size: var(--fs-size-tiny); line-height: var(--fs-leading-body);
color: var(--fs-warning-fg);
background: color-mix(in srgb, var(--fs-warning) 12%, transparent);
border-radius: var(--fs-radius-sm); padding: var(--fs-space-2);
}
.was { color: var(--fs-text-tertiary); }
.actions { display: flex; align-items: baseline; gap: var(--fs-space-3); flex-wrap: wrap; }
.actions button {
cursor: pointer; font: inherit; font-size: 0.78rem;
background: var(--fs-surface-page); color: var(--fs-text-primary);
border: 1px solid var(--fs-border-color); border-radius: var(--fs-radius-md);
padding: 0.25rem 0.6rem;
}
.actions button:hover:not(:disabled) { background: var(--fs-surface-hover); }
.actions button:disabled { opacity: var(--fs-disabled-opacity); cursor: default; }
.actions-note {
flex: 1; min-width: 18ch; font-size: var(--fs-size-tiny);
color: var(--fs-text-tertiary); line-height: var(--fs-leading-body);
}
.footnote { margin: 0; max-width: 62ch; font-size: var(--fs-size-tiny); color: var(--fs-text-tertiary); line-height: var(--fs-leading-body); }
</style>
@@ -4,7 +4,7 @@ import { useRulebooksStore } from "@/stores/rulebooks";
import { useCanonicalSystemsStore } from "@/stores/canonicalSystems"; import { useCanonicalSystemsStore } from "@/stores/canonicalSystems";
import RuleHistoryPanel from "@/components/rules/RuleHistoryPanel.vue"; import RuleHistoryPanel from "@/components/rules/RuleHistoryPanel.vue";
import RuleHomePicker from "@/components/rules/RuleHomePicker.vue"; import RuleHomePicker from "@/components/rules/RuleHomePicker.vue";
import type { Rule } from "@/api/rulebooks"; import type { Rule, RuleKind } from "@/api/rulebooks";
const props = defineProps<{ ruleId: number | null; topicId: number | null }>(); const props = defineProps<{ ruleId: number | null; topicId: number | null }>();
const emit = defineEmits<{ close: [] }>(); const emit = defineEmits<{ close: [] }>();
@@ -14,6 +14,11 @@ const canon = useCanonicalSystemsStore();
const title = ref(""); const title = ref("");
const statement = ref(""); const statement = ref("");
const whenToApply = ref(""); const whenToApply = ref("");
// Defaults to `rule`, matching the server's column default. The safe
// direction is the one that binds: a preference mislabelled as a rule is
// followed too faithfully, where a rule mislabelled as a preference is one a
// session may quietly rewrite.
const kind = ref<RuleKind>("rule");
const systemIds = ref<number[]>([]); const systemIds = ref<number[]>([]);
const why = ref(""); const why = ref("");
const howToApply = ref(""); const howToApply = ref("");
@@ -70,6 +75,7 @@ async function load() {
title.value = r.title; title.value = r.title;
statement.value = r.statement; statement.value = r.statement;
whenToApply.value = r.when_to_apply || ""; whenToApply.value = r.when_to_apply || "";
kind.value = r.kind;
systemIds.value = (r.systems ?? []).map((sys) => sys.id); systemIds.value = (r.systems ?? []).map((sys) => sys.id);
why.value = r.why || ""; why.value = r.why || "";
howToApply.value = r.how_to_apply || ""; howToApply.value = r.how_to_apply || "";
@@ -80,6 +86,7 @@ async function load() {
title.value = ""; title.value = "";
statement.value = ""; statement.value = "";
whenToApply.value = ""; whenToApply.value = "";
kind.value = "rule";
systemIds.value = []; systemIds.value = [];
why.value = ""; why.value = "";
howToApply.value = ""; howToApply.value = "";
@@ -98,6 +105,7 @@ async function save() {
title: title.value, title: title.value,
statement: statement.value, statement: statement.value,
when_to_apply: whenToApply.value, when_to_apply: whenToApply.value,
kind: kind.value,
// Always sent, so clearing the last area actually clears it — the server // Always sent, so clearing the last area actually clears it — the server
// reads a list as "these ARE the areas now". // reads a list as "these ARE the areas now".
system_ids: systemIds.value, system_ids: systemIds.value,
@@ -140,7 +148,7 @@ watch(() => props.ruleId, load);
<div class="backdrop" @click="save"> <div class="backdrop" @click="save">
<aside class="slide-over" @click.stop> <aside class="slide-over" @click.stop>
<header> <header>
<h2>{{ isCreating ? "New rule" : "Edit rule" }}</h2> <h2>{{ `${isCreating ? "New" : "Edit"} ${kind === "preference" ? "preference" : "rule"}` }}</h2>
<button v-if="!isCreating" class="trash" @click="remove" aria-label="Delete">🗑</button> <button v-if="!isCreating" class="trash" @click="remove" aria-label="Delete">🗑</button>
<button class="close" @click="save" aria-label="Close">×</button> <button class="close" @click="save" aria-label="Close">×</button>
</header> </header>
@@ -148,6 +156,34 @@ watch(() => props.ruleId, load);
Title Title
<input v-model="title" placeholder="e.g. dev is home" /> <input v-model="title" placeholder="e.g. dev is home" />
</label> </label>
<fieldset class="kind">
<legend>What kind of instruction is this?</legend>
<label class="kind-opt">
<input v-model="kind" type="radio" value="rule" />
<span>
<strong>Rule</strong> must be followed. Ignoring it breaks
something or crosses a boundary. It changes when you change it.
</span>
</label>
<label class="kind-opt">
<input v-model="kind" type="radio" value="preference" />
<span>
<strong>Preference</strong> how you want work done. Ignoring it
costs consistency, not correctness, and <em>Scribe updates it as
the work teaches it</em>.
</span>
</label>
<p class="field-note">
The test is what happens when someone does not do it. Something
breaks a rule. The tenth time goes differently from the ninth a
preference.
</p>
</fieldset>
<p v-if="kind === 'preference'" class="kind-drift">
Sessions rewrite preferences without asking, which is what makes them
stay current. Every rewrite is kept, and
<strong>Recent changes</strong> lists them with what taught each one.
</p>
<label> <label>
Statement <span class="required">*</span> Statement <span class="required">*</span>
<textarea v-model="statement" rows="3" placeholder="The actionable instruction (1-2 sentences)." /> <textarea v-model="statement" rows="3" placeholder="The actionable instruction (1-2 sentences)." />
@@ -267,6 +303,17 @@ watch(() => props.ruleId, load);
</template> </template>
<style scoped> <style scoped>
.kind { border: 1px solid var(--fs-border-color); border-radius: var(--fs-radius-md); padding: var(--fs-space-3); margin: var(--fs-space-3) 0; }
.kind legend { font-size: var(--fs-size-tiny); text-transform: uppercase; letter-spacing: var(--fs-tracking-tiny); color: var(--fs-text-tertiary); padding: 0 var(--fs-space-2); }
.kind-opt { display: flex; align-items: flex-start; gap: var(--fs-space-2); margin-bottom: var(--fs-space-2); font-size: var(--fs-size-body-sm); line-height: var(--fs-leading-body); }
.kind-opt input { margin-top: 0.2rem; accent-color: var(--fs-accent); flex: none; }
.kind-drift {
margin: 0 0 var(--fs-space-3); padding: var(--fs-space-2);
font-size: var(--fs-size-tiny); line-height: var(--fs-leading-body);
color: var(--fs-text-secondary);
background: var(--fs-accent-soft); border-radius: var(--fs-radius-sm);
}
.backdrop { .backdrop {
position: fixed; inset: 0; position: fixed; inset: 0;
background: rgba(0, 0, 0, 0.4); background: rgba(0, 0, 0, 0.4);
+17 -1
View File
@@ -24,8 +24,18 @@ const emit = defineEmits<{
<header><h2>Rules</h2></header> <header><h2>Rules</h2></header>
<ul> <ul>
<li v-for="r in rules" :key="r.id" @click="emit('open-rule', r.id)"> <li v-for="r in rules" :key="r.id" @click="emit('open-rule', r.id)">
<div class="title"> <div class="title" :class="{ 'is-preference': r.kind === 'preference' }">
{{ r.title }} {{ r.title }}
<!-- Force is stated, never inferred. A preference does not bind and
is the one kind a session rewrites on its own, so an unmarked
row would teach the opposite of both. Rules carry no chip:
they are the default reading of a rulebook, and marking every
row marks nothing. -->
<span
v-if="r.kind === 'preference'"
class="rule-chip rule-chip-preference"
title="How you want work done. It does not bind, and Scribe updates it as the work teaches it."
>preference</span>
<!-- Marked only when something is WRONG: every rule arrives by <!-- Marked only when something is WRONG: every rule arrives by
retrieval now, so "conditional" stopped distinguishing anything. retrieval now, so "conditional" stopped distinguishing anything.
A missing trigger does it means nothing can retrieve this. --> A missing trigger does it means nothing can retrieve this. -->
@@ -70,6 +80,12 @@ li {
} }
li:hover { background: var(--fs-surface-hover); } li:hover { background: var(--fs-surface-hover); }
.title { font-family: Fraunces, serif; font-style: italic; font-size: 1.05em; } .title { font-family: Fraunces, serif; font-style: italic; font-size: 1.05em; }
/* The chip says which kind; this says it again at a glance, for scanning a
long topic rather than reading one row. Weight, not colour — the chip
already carries the accent, and a second coloured thing would compete
with it for the same job. */
.title.is-preference { font-weight: 500; }
.statement { font-size: 0.9em; opacity: 0.8; margin-top: 0.25rem; } .statement { font-size: 0.9em; opacity: 0.8; margin-top: 0.25rem; }
.meta { display: flex; align-items: baseline; gap: 0.5rem; margin-top: 0.35rem; font-size: 0.75em; } .meta { display: flex; align-items: baseline; gap: 0.5rem; margin-top: 0.35rem; font-size: 0.75em; }
.trigger { flex: 1; min-width: 0; color: var(--fs-text-secondary); overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } .trigger { flex: 1; min-width: 0; color: var(--fs-text-secondary); overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }
@@ -3,8 +3,17 @@ import { ref } from "vue";
import { useRulebooksStore } from "@/stores/rulebooks"; import { useRulebooksStore } from "@/stores/rulebooks";
import type { Rulebook } from "@/api/rulebooks"; import type { Rulebook } from "@/api/rulebooks";
defineProps<{ rulebooks: Rulebook[]; selectedId: number | null; sweepActive: boolean }>(); defineProps<{
const emit = defineEmits<{ select: [id: number]; "select-sweep": [] }>(); rulebooks: Rulebook[];
selectedId: number | null;
sweepActive: boolean;
driftActive: boolean;
}>();
const emit = defineEmits<{
select: [id: number];
"select-sweep": [];
"select-drift": [];
}>();
const store = useRulebooksStore(); const store = useRulebooksStore();
const isCreating = ref(false); const isCreating = ref(false);
@@ -44,6 +53,17 @@ async function submitNew() {
> >
Due for verification Due for verification
</button> </button>
<!-- The other cross-cutting view, and the one the operator would not think
to ask for: a preference is rewritten by the agent rather than by them,
so "what changed" has no rulebook to look in and no reason to be
suspected in the first place. -->
<button
class="sweep-entry"
:class="{ active: driftActive }"
@click="emit('select-drift')"
>
Recent changes
</button>
<div class="new-rulebook"> <div class="new-rulebook">
<button v-if="!isCreating" @click="isCreating = true">+ New rulebook</button> <button v-if="!isCreating" @click="isCreating = true">+ New rulebook</button>
+47 -1
View File
@@ -10,6 +10,7 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
const rulesByTopic = ref<Record<number, RuleHeader[]>>({}); const rulesByTopic = ref<Record<number, RuleHeader[]>>({});
const currentRule = ref<Rule | null>(null); const currentRule = ref<Rule | null>(null);
const rulesDue = ref<api.RuleVerificationRow[]>([]); const rulesDue = ref<api.RuleVerificationRow[]>([]);
const drift = ref<api.PreferenceDrift[]>([]);
// Kept so a verify re-reads the sweep with the SAME filters the operator is // Kept so a verify re-reads the sweep with the SAME filters the operator is
// looking at — re-fetching unfiltered would silently widen the list under // looking at — re-fetching unfiltered would silently widen the list under
// them at the moment they acted on it. // them at the moment they acted on it.
@@ -106,6 +107,10 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
title: rule.title, title: rule.title,
statement: rule.statement, statement: rule.statement,
topic_id: rule.topic_id, topic_id: rule.topic_id,
// Carried, not defaulted: a row written here must render with the same
// force as the same row re-fetched, or a preference the operator just
// created would sit in the list looking like a rule until a reload.
kind: rule.kind,
updated_at: rule.updated_at, updated_at: rule.updated_at,
when_to_apply: rule.when_to_apply || undefined, when_to_apply: rule.when_to_apply || undefined,
arose_from_id: rule.arose_from_id ?? undefined, arose_from_id: rule.arose_from_id ?? undefined,
@@ -202,6 +207,46 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
return rule; return rule;
} }
/**
* What Scribe has changed about how it works with the operator.
*
* Preferences only — a rule changes when its author changes it, so
* including them would bury the unreviewed rows under the operator's own
* edits, which is the failure this surface exists to prevent.
*/
async function fetchDrift(limit?: number) {
loading.value = true;
try {
drift.value = await api.listPreferenceDrift(limit);
} catch (e) {
useToastStore().show("Failed to load recent changes", "error");
throw e;
} finally {
loading.value = false;
}
}
/**
* Put a preference back to what a version said — the operator's veto.
*
* The drift list is RE-READ rather than patched, for the reason the sweep
* is: this surface is an ORDER (most recently changed first) and the
* restore is itself an edit, so the row's place in that order has just
* changed. A locally-mutated row would sit in its old position describing
* a rewrite that is no longer the latest one.
*/
async function restoreVersion(ruleId: number, versionId: number) {
const rule = await api.restoreRuleVersion(ruleId, versionId);
if (currentRule.value?.id === ruleId) currentRule.value = rule;
for (const tid of Object.keys(rulesByTopic.value)) {
const list = rulesByTopic.value[Number(tid)];
const idx = list.findIndex((r) => r.id === ruleId);
if (idx >= 0) list[idx] = toHeader(rule);
}
if (drift.value.length) await fetchDrift();
return rule;
}
async function deleteRule(id: number) { async function deleteRule(id: number) {
await api.deleteRule(id); await api.deleteRule(id);
if (currentRule.value?.id === id) currentRule.value = null; if (currentRule.value?.id === id) currentRule.value = null;
@@ -211,12 +256,13 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
} }
return { return {
rulebooks, topicsByRulebook, rulesByTopic, currentRule, rulesDue, lastSweepOpts, loading, rulebooks, topicsByRulebook, rulesByTopic, currentRule, rulesDue, drift, lastSweepOpts, loading,
placeMovedRule, placeMovedRule,
fetchRulebooks, fetchTopics, fetchRules, fetchRule, fetchRulebooks, fetchTopics, fetchRules, fetchRule,
createRulebook, updateRulebook, deleteRulebook, createRulebook, updateRulebook, deleteRulebook,
createTopic, updateTopic, deleteTopic, createTopic, updateTopic, deleteTopic,
createRule, updateRule, deleteRule, relateRules, unrelateRules, createRule, updateRule, deleteRule, relateRules, unrelateRules,
fetchRulesDue, verifyRule, fetchRulesDue, verifyRule,
fetchDrift, restoreVersion,
}; };
}); });
+20 -10
View File
@@ -7,6 +7,7 @@ import RulebookDetailPane from "@/components/rules/RulebookDetailPane.vue";
import RuleListPane from "@/components/rules/RuleListPane.vue"; import RuleListPane from "@/components/rules/RuleListPane.vue";
import RuleEditorSlideOver from "@/components/rules/RuleEditorSlideOver.vue"; import RuleEditorSlideOver from "@/components/rules/RuleEditorSlideOver.vue";
import RuleSweepPane from "@/components/rules/RuleSweepPane.vue"; import RuleSweepPane from "@/components/rules/RuleSweepPane.vue";
import PreferenceDriftPane from "@/components/rules/PreferenceDriftPane.vue";
const store = useRulebooksStore(); const store = useRulebooksStore();
const route = useRoute(); const route = useRoute();
@@ -17,6 +18,7 @@ const selectedTopicId = ref<number | null>(null);
const editingRuleId = ref<number | null>(null); const editingRuleId = ref<number | null>(null);
const creatingRuleForTopic = ref<number | null>(null); const creatingRuleForTopic = ref<number | null>(null);
const sweepActive = ref(false); const sweepActive = ref(false);
const driftActive = ref(false);
function syncFromRoute() { function syncFromRoute() {
const rb = route.query.rb ? Number(route.query.rb) : null; const rb = route.query.rb ? Number(route.query.rb) : null;
@@ -26,19 +28,23 @@ function syncFromRoute() {
selectedTopicId.value = topic; selectedTopicId.value = topic;
editingRuleId.value = rule; editingRuleId.value = rule;
sweepActive.value = route.query.view === "due"; sweepActive.value = route.query.view === "due";
driftActive.value = route.query.view === "drift";
} }
function selectSweep() { /** The two cross-cutting views share one `view` query key, so entering either
sweepActive.value = true; * leaves the other — and keeps `?rule=…` so an open editor survives the
// Keeps ?rule=… so the editor survives the mode switch, and drops the * switch, the way the sweep already did. */
// rulebook/topic selection the sweep does not use. function selectCrossCutting(view: "due" | "drift") {
sweepActive.value = view === "due";
driftActive.value = view === "drift";
const { rb, topic, ...rest } = route.query; const { rb, topic, ...rest } = route.query;
void rb; void topic; void rb; void topic;
router.replace({ query: { ...rest, view: "due" } }); router.replace({ query: { ...rest, view } });
} }
function selectRulebook(id: number) { function selectRulebook(id: number) {
sweepActive.value = false; sweepActive.value = false;
driftActive.value = false;
selectedRulebookId.value = id; selectedRulebookId.value = id;
selectedTopicId.value = null; selectedTopicId.value = null;
router.replace({ query: { rb: String(id) } }); router.replace({ query: { rb: String(id) } });
@@ -84,10 +90,13 @@ watch(() => route.query, syncFromRoute);
:rulebooks="store.rulebooks" :rulebooks="store.rulebooks"
:selected-id="selectedRulebookId" :selected-id="selectedRulebookId"
:sweep-active="sweepActive" :sweep-active="sweepActive"
:drift-active="driftActive"
@select="selectRulebook" @select="selectRulebook"
@select-sweep="selectSweep" @select-sweep="selectCrossCutting('due')"
@select-drift="selectCrossCutting('drift')"
/> />
<RuleSweepPane v-if="sweepActive" class="sweep-span" @open-rule="openRule" /> <RuleSweepPane v-if="sweepActive" class="sweep-span" @open-rule="openRule" />
<PreferenceDriftPane v-else-if="driftActive" class="sweep-span" @open-rule="openRule" />
<RulebookDetailPane <RulebookDetailPane
v-else-if="selectedRulebookId !== null" v-else-if="selectedRulebookId !== null"
:rulebook-id="selectedRulebookId" :rulebook-id="selectedRulebookId"
@@ -99,13 +108,13 @@ watch(() => route.query, syncFromRoute);
<p>Select a rulebook to view its topics.</p> <p>Select a rulebook to view its topics.</p>
</div> </div>
<RuleListPane <RuleListPane
v-if="!sweepActive && selectedTopicId !== null" v-if="!sweepActive && !driftActive && selectedTopicId !== null"
:topic-id="selectedTopicId" :topic-id="selectedTopicId"
:rules="store.rulesByTopic[selectedTopicId] || []" :rules="store.rulesByTopic[selectedTopicId] || []"
@open-rule="openRule" @open-rule="openRule"
@create-rule="startCreatingRule" @create-rule="startCreatingRule"
/> />
<div v-else-if="!sweepActive" class="pane empty"> <div v-else-if="!sweepActive && !driftActive" class="pane empty">
<p>Select a topic to view its rules.</p> <p>Select a topic to view its rules.</p>
</div> </div>
<RuleEditorSlideOver <RuleEditorSlideOver
@@ -125,8 +134,9 @@ watch(() => route.query, syncFromRoute);
gap: 1px; gap: 1px;
background: var(--fs-border-color); background: var(--fs-border-color);
} }
/* The sweep is cross-cutting, so it takes the width the rulebook + topic /* A cross-cutting pane — the sweep, or recent changes — takes the width the
panes would have used rather than being squeezed into one column. */ rulebook + topic panes would have used rather than being squeezed into one
column. */
.sweep-span { grid-column: 2 / -1; } .sweep-span { grid-column: 2 / -1; }
.pane.empty { .pane.empty {
background: var(--fs-surface-hover); background: var(--fs-surface-hover);
+1 -1
View File
@@ -1,7 +1,7 @@
{ {
"name": "scribe", "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).", "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.18.0436", "version": "2026.09.18.1606",
"author": { "author": {
"name": "Bryan Van Deusen" "name": "Bryan Van Deusen"
}, },
+51 -4
View File
@@ -10,9 +10,25 @@ while you worked: they don't hold the files you read, the names you used or
the order you did things in. A reply that follows *your* path is accurate and the order you did things in. A reply that follows *your* path is accurate and
still unreadable to them. Shape it around **where the work stands**. still unreadable to them. Shape it around **where the work stands**.
Pick the kind of reply first (the tables below), then fill its sections. The Pick the kind of reply first (the tables below). Its sections are **what to
sections are what lets the operator find things at a glance, so keep them even consider including, not a form to complete.**
when one is short — "**Needs you:** nothing" is an answer they were looking for.
Two kinds of section, and they behave differently:
- **A section that answers a standing question** — *does anything need me? what
happens next?* — is always answered, even when the answer is nothing.
"**Needs you:** nothing" is what they were looking for.
- **A section that explains** — how it was done, why that way, what else you
noticed — earns its place only when it changes what the operator does or
decides. When it would not, leave it out: that detail belongs in the record's
log, where it is available and not in the way.
**Write the shortest reply that carries the answer.** To someone reading
quickly, length is not thoroughness — it is work handed back to them. A reply
that fills every heading faithfully and runs a full screen is worse than four
lines naming the two things that changed their position. Extra length has to be
earned: a comparison they asked for, options that need laying side by side, a
measurement whose numbers are the point.
## Every reply ## Every reply
@@ -33,6 +49,13 @@ when one is short — "**Needs you:** nothing" is an answer they were looking fo
working. If a term has to appear, explain it once. working. If a term has to appear, explain it once.
- **Place the work in Scribe.** Name the task, issue or milestone it belongs to, - **Place the work in Scribe.** Name the task, issue or milestone it belongs to,
by id *and* title (using-scribe: "Name the record, never just its number"). by id *and* title (using-scribe: "Name the record, never just its number").
- **A decision already made gets acted on, and the reply says what you did with
it.** Once the operator has chosen, that is the input to the work, not a topic
to revisit. If something you have since learned genuinely overturns the
choice, say so once and plainly — name the new evidence and what it changes —
and otherwise let the decision stand. Laying out the trade-offs of a settled
question again reads as contradicting yourself rather than as being careful,
and it costs the operator the decision twice.
## Take the placement from the record ## Take the placement from the record
@@ -43,7 +66,15 @@ it is wrong. So take placement from Scribe:
the milestone, `position` (step N of M), `progress`, and `next` (the next open the milestone, `position` (step N of M), `progress`, and `next` (the next open
step). Use those values as they came back. step). Use those values as they came back.
- For a wider view, `get_milestone` (a plan and its steps) or `enter_project` - For a wider view, `get_milestone` (a plan and its steps) or `enter_project`
(the whole project). (the whole project). Every milestone they list carries **`next_step`** — the
earliest step still open, or null when none is — so a reply that says what
comes next takes it from the listing it already read. Progress alone says a
plan has an open step and not which one, and that is the gap recall fills.
- **A record you only mention is a record to read.** `placement` rides the
write that changed a task, so a task you cite without touching arrives with
nothing vouching for it. A retrieval hint carries an id, a kind and a title;
where a task stands is in the line's kind marker — `[task (done)]` — and a
line you are working from memory has no marker at all.
- Work with no task behind it: say so plainly — "this wasn't tracked as a - Work with no task behind it: say so plainly — "this wasn't tracked as a
task" — and offer to record it. An honest "untracked" is a placement too. task" — and offer to record it. An honest "untracked" is a placement too.
@@ -137,6 +168,15 @@ Notes on each section:
- **Needs you** — an action, an approval, a decision, or "nothing". If it's an - **Needs you** — an action, an approval, a decision, or "nothing". If it's an
action, give the reason with it. An approval you are holding for also gets action, give the reason with it. An approval you are holding for also gets
its own **Approval requested** section, and this line points at it. its own **Approval requested** section, and this line points at it.
**Two tests, and it takes both: is this theirs to decide, and is work waiting
on it?** A choice that is genuinely theirs — a priority, a trade-off only they
can price, something they have to live with afterwards — belongs here. A
question you could settle by reading something, by taking a measurement you
already have access to, or by choosing the obvious default does not: that is
work not yet done, and sending it moves your uncertainty onto them. Settle it,
say which way you went and why, and leave them free to overrule you. This
section is for what blocks them, not for what you are unsure about.
- **Next** — from `placement.next`, or say the milestone is finished. If you - **Next** — from `placement.next`, or say the milestone is finished. If you
found something you didn't fix, the offer to fix it goes here. found something you didn't fix, the offer to fix it goes here.
@@ -146,3 +186,10 @@ Read the reply as the operator will: someone who wasn't there, reading
quickly. Can they tell **what was done, whether anything needs them, and what quickly. Can they tell **what was done, whether anything needs them, and what
happens next** without asking a follow-up? If not, the sections are what's happens next** without asking a follow-up? If not, the sections are what's
missing — not more detail. missing — not more detail.
Then read it once more for **what can go**. A section filled because it was in
the table, reasoning supporting a conclusion nobody is going to dispute, a
finding already written to the record — none of it changes what the operator
does, so none of it belongs in the reply. Cutting is not hiding: the log holds
it, and the reply stays readable. A reply that has been cut twice is the one
they can act on.
+8 -3
View File
@@ -24,9 +24,14 @@ async def list_milestones(project_id: int) -> dict:
"""List milestones for a Scribe project, ordered by order_index. """List milestones for a Scribe project, ordered by order_index.
Returns every milestone, done ones included: id, title, description, Returns every milestone, done ones included: id, title, description,
status (active/done), order_index and progress (total, completed, pct, status (active/done), order_index, progress (total, completed, pct,
status_counts). The plan itself is not listed: get_milestone(id) returns a status_counts) and `next_step`. The plan itself is not listed:
milestone's body and its steps. get_milestone(id) returns a milestone's body and its steps.
`next_step` is the earliest step still open — {id, title, status} — or
null when the plan has none left. Use it as it came back. "7 of 9 done"
invites naming the open one from memory, and a remembered id reads exactly
like a read one while being a step that closed days ago.
""" """
uid = current_user_id() uid = current_user_id()
rows = await milestones_svc.get_project_milestone_summary(uid, project_id) rows = await milestones_svc.get_project_milestone_summary(uid, project_id)
+12 -5
View File
@@ -77,9 +77,15 @@ async def enter_project(project_id: int) -> dict:
`milestone_summary` is the 5 most recently touched milestones, any status, `milestone_summary` is the 5 most recently touched milestones, any status,
most recent first. Touched counts a step changing, not only the milestone most recent first. Touched counts a step changing, not only the milestone
itself. Each carries its description and progress but NOT its plan: itself. Each carries its description, its progress and its `next_step` —
get_milestone(id) reads a plan and its steps. `milestone_summary_omitted` the earliest step still open, {id, title, status}, or null when none is —
says how many others exist; list_milestones lists them all. but NOT its plan: get_milestone(id) reads a plan and its steps.
`milestone_summary_omitted` says how many others exist; list_milestones
lists them all.
Name the next step from `next_step`, not from recall: progress alone says
a plan has an open step and not which, and a step named from memory reads
exactly like one that was read.
`unplanned_milestones` is the active milestones that have NO steps yet, `unplanned_milestones` is the active milestones that have NO steps yet,
in roadmap order (up to 10; `unplanned_milestones_omitted` counts the in roadmap order (up to 10; `unplanned_milestones_omitted` counts the
@@ -290,8 +296,9 @@ async def get_project(project_id: int) -> dict:
"""Fetch a Scribe project by ID. """Fetch a Scribe project by ID.
Returns full project fields, a milestone_summary list (every milestone, Returns full project fields, a milestone_summary list (every milestone,
with description and progress but no plan body; get_milestone reads a with description, progress and `next_step` — the earliest still-open step,
plan), the project's own rules (project_rules), and applicable_rules: the or null — but no plan body; get_milestone reads a plan), the project's own
rules (project_rules), and applicable_rules: the
global rules tagged to an area this project works in. Every other global global rules tagged to an area this project works in. Every other global
rule applies too and arrives by retrieval when the work matches it. rule applies too and arrives by retrieval when the work matches it.
""" """
+11
View File
@@ -140,6 +140,9 @@ async def search(
enter_project) — otherwise this searches across ALL projects and enter_project) — otherwise this searches across ALL projects and
bleeds unrelated work into the result set. 0 = search everything bleeds unrelated work into the result set. 0 = search everything
(use only when you genuinely want a cross-project sweep). (use only when you genuinely want a cross-project sweep).
A LESSON is the exception and arrives whatever the scope: the kind
records an insight that transfers, so it is reachable from a
project it was not written on.
system_id: Narrow to records tagged to one System (a named system_id: Narrow to records tagged to one System (a named
subsystem/area — enter_project lists them). Use when investigating subsystem/area — enter_project lists them). Use when investigating
a specific subsystem: it cuts the candidates to records someone a specific subsystem: it cuts the candidates to records someone
@@ -167,6 +170,14 @@ async def search(
uid, q, limit=limit, is_task=is_task, uid, q, limit=limit, is_task=is_task,
project_id=project_id or None, project_id=project_id or None,
system_id=system_id or None, system_id=system_id or None,
# A LESSON is reachable from any project (milestone 385). The kind
# exists to carry an insight to the next project, so a project filter
# that hid it would hide it precisely where it is worth having. Only
# the project filter widens — everything else about the scoping holds,
# and a caller narrowing by `content_type` still gets what it asked
# for. This is the explicit search, where the operator asked; the
# unasked-for arms decide their own budget separately.
include_global_kinds=True,
# An explicit search reaches everything the operator may read, including # An explicit search reaches everything the operator may read, including
# records shared with them one-to-one. # records shared with them one-to-one.
scope="read", scope="read",
+21 -2
View File
@@ -76,8 +76,27 @@ class Note(Base, TimestampMixin, SoftDeleteMixin):
recurrence_next_spawn_at: Mapped[datetime | None] = mapped_column( recurrence_next_spawn_at: Mapped[datetime | None] = mapped_column(
DateTime(timezone=True), nullable=True DateTime(timezone=True), nullable=True
) )
# Note type — 'note' (default) or 'process' (a stored process). Task-ness is # WHAT KIND of record this is, on the note/entity axis. Task-ness is tracked
# tracked by `status`, not here. (person/place/list entity types removed 2026-07.) # by `status`, not here (person/place/list entity types removed 2026-07):
# note (default) — authored prose, findable by what it is ABOUT
# process — a stored procedure, synced to a client as a skill
# snippet — a reusable shape, with its structured fields mirrored in
# `data` and its trigger composed into the title (0070)
# lesson — a transferable insight, findable by WHEN IT APPLIES rather
# than by topic (milestone 385). Same mirror discipline as a
# snippet: the trigger lives in `data.when_to_apply` and is
# composed into the title and the head of the body, which is
# what puts it in the embedded document. A lesson is never a
# task — see `status` — because a lesson that acquired one
# would start appearing in open-work listings.
#
# DELIBERATELY UNGATED. Unlike `task_kind` there is no CHECK on this column:
# migration 0036 added it as plain Text with a server default and nothing
# has constrained it since, so rule 36 has no whitelist to expand when a
# kind is added. The vocabulary that actually decides what a reader can
# reach is `services.knowledge._FACETS`; an unrecognised value there
# resolves to a filter matching nothing, which is the intended answer to a
# typo.
note_type: Mapped[str] = mapped_column(Text, default="note", server_default="note") note_type: Mapped[str] = mapped_column(Text, default="note", server_default="note")
# Task sub-kind — what KIND of work this is, not how it is going: # Task sub-kind — what KIND of work this is, not how it is going:
# work (default) — ships a change # work (default) — ships a change
+49 -3
View File
@@ -258,11 +258,57 @@ async def get_rule_version(rule_id: int, version_id: int):
return jsonify(version.to_dict(include_text=True)) return jsonify(version.to_dict(include_text=True))
# NO restore route, deliberately (milestone 323). A note version can be # NO restore route FOR A RULE, deliberately (milestone 323). A note version
# restored; a binding instruction should not be revertible in one click. # can be restored; a binding instruction should not be revertible in one
# Putting a rewrite back goes through update_rule, which takes its own # click. Putting a rewrite back goes through update_rule, which takes its own
# snapshot and leaves the undo in the history like any other edit — a silent # snapshot and leaves the undo in the history like any other edit — a silent
# revert would erase the only record of why the rewrite happened. # revert would erase the only record of why the rewrite happened.
#
# A PREFERENCE IS THE EXCEPTION, and the route below refuses anything else.
# 323's reasoning turns on the rewrite being the operator's own decision.
# A preference's rewrite is not: the agent makes it mid-work without asking,
# which is what the kind is for. Reverting one is a veto over someone else's
# edit rather than an undo of your own, and milestone 399 named the cost of
# making that veto expensive — drift supervised in name only. The safeguard
# 323 actually wanted survives intact, because the restore goes through
# update_rule too: the rewrite stays in the history with the revert recorded
# after it, so the history gains an entry rather than losing one.
@rulebooks_bp.get("/rules/drift")
@login_required
async def preference_drift():
"""Preferences that have been rewritten, most recently changed first.
One row per preference carrying its latest rewrite, what it said before,
and the record that taught the change. `?limit=` caps the list.
"""
uid = get_current_user_id()
try:
limit = int(request.args.get("limit", 20))
except (TypeError, ValueError):
limit = 20
rows = await rulebooks_svc.recent_preference_drift(uid, limit=limit)
return jsonify({"drift": rows})
@rulebooks_bp.post("/rules/<int:rule_id>/versions/<int:version_id>/restore")
@login_required
async def restore_rule_version(rule_id: int, version_id: int):
"""Put a preference back to what that version said. Preferences only.
409 rather than 400 on a rule: the request is well-formed and the caller
is not wrong to have asked — this rule is simply in a state where the
action does not apply, and the message says which state and why.
"""
uid = get_current_user_id()
try:
rule = await rulebooks_svc.restore_rule_version(rule_id, version_id, uid)
except ValueError as exc:
return jsonify({"error": str(exc)}), 409
if rule is None:
return jsonify({"error": "rule or version not found"}), 404
return jsonify(await rulebooks_svc.rule_detail(uid, rule))
@rulebooks_bp.post("/rules/<int:rule_id>/relations") @rulebooks_bp.post("/rules/<int:rule_id>/relations")
+67 -2
View File
@@ -203,6 +203,33 @@ def embedding_text(title: str | None, body: str | None) -> str:
return f"{title}\n{body}".strip() if body else title return f"{title}\n{body}".strip() if body else title
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.
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
0.0100.023 for everything else — and the cause was this title plus the
same trigger repeated in the body, so purpose appears twice in a short
document and dominates the vector. Every kind that must be findable by WHEN
IT APPLIES rather than what it is about is built on this line.
Either side alone is returned as-is: a record with no trigger yet degrades
to its subject and still embeds, just less sharply — which is an argument
for backfilling triggers, not for padding the title with whatever text is
to hand.
"""
subject = (subject or "").strip()
trigger = (trigger or "").strip()
if subject and trigger:
return f"{subject}{trigger}"
return subject or trigger
# --- chunking (#280): the document shape ------------------------------------ # --- chunking (#280): the document shape ------------------------------------
# #
# bge-small reads at most 512 tokens and fastembed silently truncates the rest, # bge-small reads at most 512 tokens and fastembed silently truncates the rest,
@@ -481,6 +508,25 @@ async def upsert_note_embedding(
logger.warning("Failed to persist embedding for note %d", note_id, exc_info=True) logger.warning("Failed to persist embedding for note %d", note_id, exc_info=True)
# Kinds that belong to no single project, and are therefore reachable from a
# project-scoped search of a DIFFERENT project when a caller asks for them
# (milestone 385 step 3).
#
# A lesson is the whole reason this exists. "A better way to think about this
# problem" is not true only where it was learned, and a lesson confined to its
# origin project would be unreachable exactly where it is most useful — on the
# next project, which is the case the kind was created for (#3727).
# `semantic_search_rules` has always had this property: it scopes by OWNERSHIP
# rather than by what binds a given project, because "is there a rule about
# this" is a question asked across a whole rulebook. A lesson asks the same
# kind of question.
#
# Spelled as a literal rather than imported from `services.lessons`, which
# imports `trigger_title` from this module and would close a cycle. A guard
# pins the two equal instead.
GLOBAL_NOTE_TYPES: tuple[str, ...] = ("lesson",)
# Both searches rank WITHOUT the threshold and apply it in Python, so the best # Both searches rank WITHOUT the threshold and apply it in Python, so the best
# rejected score stays observable (#3670). The qualifying set is provably # rejected score stays observable (#3670). The qualifying set is provably
# unchanged: rows arrive ordered by distance ascending, so every above-bar row # unchanged: rows arrive ordered by distance ascending, so every above-bar row
@@ -509,6 +555,7 @@ async def semantic_search_notes(
note_type: str | Sequence[str] | None = None, note_type: str | Sequence[str] | None = None,
task_kind: str | Sequence[str] | None = None, task_kind: str | Sequence[str] | None = None,
orphan_only: bool = False, orphan_only: bool = False,
include_global_kinds: bool = False,
scope: str = "own", scope: str = "own",
demote_superseded: bool = True, demote_superseded: bool = True,
system_id: int | None = None, system_id: int | None = None,
@@ -543,6 +590,19 @@ async def semantic_search_notes(
alone can express it. With `note_type="note", task_kind="issue"` a caller alone can express it. With `note_type="note", task_kind="issue"` a caller
gets fixed problems and durable notes without the open to-do list. gets fixed problems and durable notes without the open to-do list.
`include_global_kinds` lets a project-scoped search ALSO reach the kinds in
GLOBAL_NOTE_TYPES — records that belong to no single project — so a lesson
written on one project is found from another. It widens the PROJECT filter
only: a caller that also passes `note_type` still gets exactly the kinds it
asked for, so narrowing to snippets does not quietly acquire lessons.
Off by default, because two callers depend on the project filter holding.
The near-duplicate gate compares a record only against its own project on
purpose, and a globally-visible kind would let a lesson block an unrelated
note's create on a project its author never touched. Ordinary note recall
is project-scoped for the same reason — the point of the carve-out is that
ONE kind escapes, not that scoping is weaker.
`scope` ("own" | "browse" | "read", see access.notes_visibility_clause) `scope` ("own" | "browse" | "read", see access.notes_visibility_clause)
decides how far this may see. It exists because this one function serves decides how far this may see. It exists because this one function serves
three different kinds of act: an explicit search, which should reach three different kinds of act: an explicit search, which should reach
@@ -604,7 +664,12 @@ async def semantic_search_notes(
if orphan_only: if orphan_only:
stmt = stmt.where(Note.project_id.is_(None)) stmt = stmt.where(Note.project_id.is_(None))
elif project_id is not None: elif project_id is not None:
stmt = stmt.where(Note.project_id == project_id) in_project = Note.project_id == project_id
if include_global_kinds:
in_project = or_(
in_project, Note.note_type.in_(GLOBAL_NOTE_TYPES)
)
stmt = stmt.where(in_project)
# Narrow to records tagged to one System (subsystem/area). An # Narrow to records tagged to one System (subsystem/area). An
# association filter, not a ranking signal — membership in the # association filter, not a ranking signal — membership in the
# candidate set, decided before scoring, like project_id above. # candidate set, decided before scoring, like project_id above.
@@ -785,7 +850,7 @@ def rule_document(
if not trigger: if not trigger:
return name or None, body or None return name or None, body or None
return ( return (
f"{name}{trigger}" if name else trigger, trigger_title(name, trigger),
f"When to apply: {trigger}\n\n{body}" if body else f"When to apply: {trigger}", f"When to apply: {trigger}\n\n{body}" if body else f"When to apply: {trigger}",
) )
+7
View File
@@ -292,6 +292,13 @@ _FACETS: dict[str, tuple[bool, str | None]] = {
"note": (False, "note"), "note": (False, "note"),
"process": (False, "process"), "process": (False, "process"),
"snippet": (False, "snippet"), "snippet": (False, "snippet"),
# A transferable insight, keyed by the situation it applies to rather than
# by its topic (milestone 385). It reaches the browse surface through this
# one entry: the door's validation, the counts and both dialects of the
# type filter are all generated from this table, which is the property
# #3161 asked for so that adding a kind is a single edit rather than four
# that must agree.
"lesson": (False, "lesson"),
} }
# The non-task record types, for the counts query. Derived so it cannot drift # The non-task record types, for the counts query. Derived so it cannot drift
+196
View File
@@ -0,0 +1,196 @@
"""Lesson service — a transferable insight, retrievable by situation.
A *lesson* is a Note with ``note_type='lesson'``: a better way to think about a
problem, or a solution that transfers, recorded so a later session meets it at
the moment it applies — and **without binding the reader**.
WHY THE KIND EXISTS (milestone 385, from note #3727)
Rules were the only surface that is global AND situation-keyed, so an agent
holding a transferable insight had one door, and that door binds. The observed
symptom was sessions offering rule proposals for things that should not be
rules.
The gap is a document-shape fact, not a threshold:
- a note is embedded as ``title\\nbody`` and is findable by **what it is
about**;
- a rule is embedded as ``{title}{trigger}`` with ``When to apply:``
repeated at the head of the body, so the trigger appears twice in a short
document and dominates the vector — findable by **when it applies**.
No tuning reaches across that: the field the query would match on is simply not
in a note's document. So a lesson carries a trigger and is embedded like a rule,
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``.
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
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.
WHAT A LESSON INHERITS, AND THE CELLS LEFT EMPTY ON PURPOSE (#3163)
A new kind inherits the note machinery wholesale, and #3163 asks which parts it
should NOT get — so that an empty cell is a decision rather than an oversight.
Inherited, all deliberately:
- **versions** — a lesson is reworded as understanding improves, and what it
used to say is worth as much as any note's history.
- **supersession** — the event this most needs. A lesson replaced by a better
lesson is precisely what ``note_supersessions`` models, and the demotion
penalty already exists.
- **trash**, **the share ACL**, **tags**, **project and System tagging**,
**chunked embeddings**, **the near-duplicate gate**.
NOT inherited, and each for a stated reason:
- **status / task_kind / milestone_id** — a lesson is not work. ``is_task`` is
``status is not None``, so a lesson that acquired a status would become a
task and appear in open-work listings. This is the one cell where filling it
in by accident silently changes what the record IS.
- **recurrence** — task-only, and a lesson does not recur.
- **verify_with / expires_when** — available, because they are generic note
fields, but not part of a lesson's contract and not asked for on create. The
milestone-312 distinction is why: those mark a record that asserts a FACT
about someone else's software and can go false unwatched. A lesson is closer
to a norm — "a better way to think about this" has no truth value that rots
on its own. A lesson that does assert such a fact can still carry them.
WHY THERE IS NO MIGRATION
``note_type`` carries **no CHECK constraint** — only ``task_kind`` does
(``notes_task_kind_check``, migrations 0056 / 0065). Migration 0036 added
``note_type`` as plain ``Text`` with a server default and nothing has gated it
since. So rule 36 has nothing to expand here, and the failure it guards against
— a value the database refuses on an instance predating its migration — cannot
arise for this column.
The real vocabulary is ``services.knowledge._FACETS``, which is where a kind
becomes reachable on the browse surface and validated at the door. That is one
table feeding both dialects of the type filter, so adding a kind there is a
single edit — a property #3161 recommended and that landed before this.
"""
from __future__ import annotations
import re
LESSON_NOTE_TYPE = "lesson"
# The key in `notes.data`. Named for the field it mirrors on `rules`, because it
# answers the same question and a reader who knows one should not have to learn
# a second word for it.
TRIGGER_KEY = "when_to_apply"
# The body's trigger line, and the pattern that reads it back. The body is the
# readable form and the thing that gets embedded; `data` is the queryable
# mirror. Reads prefer the mirror and fall back to this, which is the discipline
# `snippet_fields` follows and the reason a row written before the mirror
# existed is still readable.
_BODY_TRIGGER_RE = re.compile(r"^\*\*When to apply:\*\*\s*(.+?)\s*$", re.M)
def lesson_trigger(note) -> str:
"""When this lesson applies, or "" — the mirror first, then the body.
Prefers `data` for the same reason every snippet read does: it is indexed,
and parsing a body to answer a question the database can answer is how a
hot path ends up regexing markdown. The fallback is not dead code — it is
what makes a lesson readable if the mirror is ever absent, and an absent
mirror must degrade to the right answer rather than to silence.
"""
data = getattr(note, "data", None) or {}
from_mirror = (data.get(TRIGGER_KEY) or "").strip() if isinstance(data, dict) else ""
if from_mirror:
return from_mirror
match = _BODY_TRIGGER_RE.search(getattr(note, "body", None) or "")
return match.group(1).strip() if match else ""
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 = "") -> 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.
`**When to apply:**` rather than plain text: the body is the READABLE
form, `data` is the queryable mirror, and `_BODY_TRIGGER_RE` reads this
line back when the mirror is missing. Its markdown must therefore match
what that pattern expects, which is why neither is written by hand
anywhere else.
The insight goes in the body rather than being held out of the document.
`rule_document` excludes a rule's `why` because long dated narrative made
sixteen dev-logs land on the centroid of "development" — but that finding
predates chunking (#280). A body over the budget is now split into several
chunks, EACH prefixed with the title, so a lesson's story no longer
averages itself into its trigger: it occupies its own vectors, and every
one of them still carries the trigger in its prefix. Holding it out would
cost the reader the only part that explains the insight and would buy a
sharpness the chunker already provides.
"""
lines = []
trigger = (when_to_apply or "").strip()
if trigger:
lines.append(f"**When to apply:** {trigger}")
insight = (insight or "").strip()
if insight:
lines.append(insight)
return "\n\n".join(lines)
def lesson_document(
what: str, when_to_apply: str = "", insight: str = "",
) -> tuple[str, str]:
"""The (title, body) a lesson is STORED — and therefore embedded — 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.
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.
"""
return compose_title(what, when_to_apply), compose_body(insight, when_to_apply)
+59 -8
View File
@@ -7,9 +7,16 @@ from sqlalchemy import func, select
from scribe.models import async_session from scribe.models import async_session
from scribe.models.milestone import Milestone from scribe.models.milestone import Milestone
from scribe.models.note import Note from scribe.models.note import Note
from scribe.services import access as access_svc
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
# The statuses that make a step OPEN work, defined here because this is the
# lower layer: services/placement.py imports it rather than restating it. Two
# surfaces that both answer "what is next" and disagree about what counts as
# next is worse than one of them staying silent.
OPEN_STEP_STATUSES = ("todo", "in_progress")
def embed_milestone(milestone: Milestone) -> None: def embed_milestone(milestone: Milestone) -> None:
"""Refresh a milestone's vectors, fire-and-forget (milestone 415). """Refresh a milestone's vectors, fire-and-forget (milestone 415).
@@ -239,13 +246,21 @@ def _progress_from_counts(status_counts: dict[str, int]) -> dict:
async def get_project_milestone_summaries( async def get_project_milestone_summaries(
user_id: int, project_ids: list[int] user_id: int, project_ids: list[int]
) -> dict[int, list[dict]]: ) -> dict[int, list[dict]]:
"""Milestone summaries for MANY projects in two queries total. """Milestone summaries for MANY projects in three queries total.
The per-project version below is a nested fan-out: one query to list a The per-project version below is a nested fan-out: one query to list a
project's milestones, then one more per milestone for its progress. Called project's milestones, then one more per milestone for its progress. Called
for 25 projects concurrently it asked for ~250 pooled connections against a for 25 projects concurrently it asked for ~250 pooled connections against a
pool of 15, and every one of them waited out the 30-second checkout timeout pool of 15, and every one of them waited out the 30-second checkout timeout
(#2384). This does the same work in two queries and one session. (#2384). This does the same work in a fixed number of queries and one
session: the milestones, their step counts, and their open steps.
Each row carries `next_step` — the earliest open step, or None (#4154).
That is NOT the same question `services/placement.py` answers: placement
knows which step you are on and names the next one AFTER it, while a
listing has no current step, so the earliest open one is the whole answer.
The two share `OPEN_STEP_STATUSES` and the creation ordering so they can
never disagree about which steps are candidates.
""" """
if not project_ids: if not project_ids:
return {} return {}
@@ -261,16 +276,24 @@ async def get_project_milestone_summaries(
counts: dict[int, dict[str, int]] = {} counts: dict[int, dict[str, int]] = {}
step_touched: dict[int, datetime] = {} step_touched: dict[int, datetime] = {}
next_step: dict[int, dict] = {}
if milestones: if milestones:
milestone_ids = [m.id for m in milestones]
# Both step queries below take the SAME visibility clause (rule 78).
# They have to: `next_step` names a step and the counts beside it
# say how many there are, so a row that could name a step its own
# progress excludes would be reporting two different milestones.
readable = access_svc.readable_notes_clause(user_id)
rows = await session.execute( rows = await session.execute(
select( select(
Note.milestone_id, Note.status, func.count(Note.id), Note.milestone_id, Note.status, func.count(Note.id),
func.max(Note.updated_at), func.max(Note.updated_at),
) )
.where( .where(
Note.milestone_id.in_([m.id for m in milestones]), Note.milestone_id.in_(milestone_ids),
Note.status.isnot(None), Note.status.isnot(None),
Note.deleted_at.is_(None), Note.deleted_at.is_(None),
readable,
) )
.group_by(Note.milestone_id, Note.status) .group_by(Note.milestone_id, Note.status)
) )
@@ -280,6 +303,28 @@ async def get_project_milestone_summaries(
or latest > step_touched[milestone_id]): or latest > step_touched[milestone_id]):
step_touched[milestone_id] = latest step_touched[milestone_id] = latest
# ONE query for every milestone in the batch, not one per row.
# #2384 was exactly this listing fanned out per milestone, and it
# drained the connection pool; a third flat query keeps the cost
# constant in the number of plans. Ordered the way
# services/placement.py orders steps — creation order, the order a
# plan is written and a batch create inserts — so the first row per
# milestone IS its next open step.
open_rows = await session.execute(
select(Note.milestone_id, Note.id, Note.title, Note.status)
.where(
Note.milestone_id.in_(milestone_ids),
Note.status.in_(OPEN_STEP_STATUSES),
Note.deleted_at.is_(None),
readable,
)
.order_by(Note.created_at.asc(), Note.id.asc())
)
for milestone_id, note_id, title, status in open_rows.fetchall():
next_step.setdefault(
milestone_id, {"id": note_id, "title": title, "status": status},
)
out: dict[int, list[dict]] = {pid: [] for pid in project_ids} out: dict[int, list[dict]] = {pid: [] for pid in project_ids}
for m in milestones: for m in milestones:
entry = m.to_dict() entry = m.to_dict()
@@ -289,6 +334,12 @@ async def get_project_milestone_summaries(
# written (#4045). Touched is the later of the two. # written (#4045). Touched is the later of the two.
touched = [t for t in (m.updated_at, step_touched.get(m.id)) if t] touched = [t for t in (m.updated_at, step_touched.get(m.id)) if t]
entry["last_touched_at"] = max(touched).isoformat() if touched else None entry["last_touched_at"] = max(touched).isoformat() if touched else None
# Always present, None included. "8 of 9 done" tells a reader there is
# an open step and not WHICH, and a gap that shape gets filled from
# whatever id is nearest to hand — a retrieval hint carries an id and a
# title and no status, and that is how a done step was reported as the
# open one (#4154). A listing that names it leaves nothing to guess.
entry["next_step"] = next_step.get(m.id)
out.setdefault(m.project_id, []).append(entry) out.setdefault(m.project_id, []).append(entry)
return out return out
@@ -299,16 +350,16 @@ async def get_project_milestone_summary(user_id: int, project_id: int) -> list[d
return (await get_project_milestone_summaries(user_id, [project_id])).get(project_id, []) return (await get_project_milestone_summaries(user_id, [project_id])).get(project_id, [])
# What a milestone LISTING needs: enough to say what each plan is and how far # What a milestone LISTING needs: enough to say what each plan is, how far
# along it is. The plan itself (`body`) is get_milestone's job. Summaries once # along it is, and which step is next. The plan itself (`body`) is
# carried it, and on a project with 39 milestones enter_project came to ~222k # get_milestone's job. Summaries once carried it, and on a project with 39
# characters, 168k of them plan bodies. That is past what an MCP client will # milestones enter_project came to ~222k characters, 168k of them plan bodies. That is past what an MCP client will
# accept as a tool result, so the session handshake arrived as a file to page # accept as a tool result, so the session handshake arrived as a file to page
# through (#4045). user_id / project_id / timestamps repeat what the caller # through (#4045). user_id / project_id / timestamps repeat what the caller
# already knows. # already knows.
_BRIEF_FIELDS = ( _BRIEF_FIELDS = (
"id", "title", "description", "status", "order_index", "id", "title", "description", "status", "order_index",
"total", "completed", "pct", "status_counts", "total", "completed", "pct", "status_counts", "next_step",
) )
+4 -2
View File
@@ -54,9 +54,11 @@ from scribe.models.milestone import Milestone
from scribe.models.note import Note from scribe.models.note import Note
from scribe.models.project import Project from scribe.models.project import Project
from scribe.services import access as access_svc from scribe.services import access as access_svc
from scribe.services.milestones import _progress_from_counts from scribe.services.milestones import OPEN_STEP_STATUSES, _progress_from_counts
_OPEN = ("todo", "in_progress") # Imported, not restated. The milestone summary names a plan's next open step
# too (#4154), and the two answers must agree about what "open" means.
_OPEN = OPEN_STEP_STATUSES
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
+17 -2
View File
@@ -640,7 +640,7 @@ async def get_autoinject_config(user_id: int) -> dict:
def _record_kind(note) -> str: def _record_kind(note) -> str:
"""The one-word kind marker for an injected menu line. """The kind marker for an injected menu line — and, for a task, its status.
The menu is drawn from every record that carries an embedding, so a snippet, The menu is drawn from every record that carries an embedding, so a snippet,
a stored process, an issue and a stray dev-log all arrive looking identical. a stored process, an issue and a stray dev-log all arrive looking identical.
@@ -649,9 +649,24 @@ def _record_kind(note) -> str:
Task-ness wins over `note_type` because it's the more useful distinction at a Task-ness wins over `note_type` because it's the more useful distinction at a
glance: "there's an open issue about this" beats "there's a note about this". glance: "there's an open issue about this" beats "there's a note about this".
A TASK ALSO CARRIES ITS STATUS, because for that kind alone the line is
read as a claim about live work. A finished step and an open one rendered
identically is not a cosmetic gap: a done step was cited as a milestone's
open one on the strength of a line exactly like this, which carries an id,
a kind and a title and said nothing about where the work stood (#4154).
Only for tasks — a note or a snippet has no status to be wrong about.
""" """
if note.is_task: if note.is_task:
return "issue" if note.task_kind == "issue" else "task" kind = "issue" if note.task_kind == "issue" else "task"
# No fallback for a missing status: `is_task` IS `status is not None`
# (models/note.py), so a branch for a task without one could never be
# taken, and a dead branch is a claim about the data that isn't true.
#
# Parenthesised rather than dot-joined: the write-path prior-art line
# joins its own fields with " · ", so a dotted status would read as
# another flag beside `seen` instead of as part of the kind.
return f"{kind} ({note.status})"
return note.note_type or "note" return note.note_type or "note"
+209 -1
View File
@@ -11,9 +11,10 @@ import logging
from collections.abc import Iterable from collections.abc import Iterable
from typing import Optional from typing import Optional
from sqlalchemy import and_, delete as sql_delete, insert, or_, select from sqlalchemy import and_, delete as sql_delete, func, insert, or_, select
from scribe.models import async_session from scribe.models import async_session
from scribe.models.base import iso
from scribe.models.system import System from scribe.models.system import System
from scribe.models.rulebook import Rulebook from scribe.models.rulebook import Rulebook
from scribe.services.verification import ( from scribe.services.verification import (
@@ -846,6 +847,213 @@ async def get_rule_version(rule_id: int, version_id: int, user_id: int):
)).scalar_one_or_none() )).scalar_one_or_none()
# ── Drift: what Scribe changed about how it works with you (#3895) ─────
#
# A preference is the one record kind the AGENT rewrites in the ordinary
# course of working, which is the property that makes it useful and the
# property that makes it dangerous. Milestone 399 named the risk up front:
# an agent misreads one session, rewrites a preference, and follows the
# rewritten version forever while the operator never sees the moment it
# changed — a confident wrong answer wearing the operator's own authority.
#
# `rule_versions` already records every rewrite. What it does not do is
# ARRIVE: a history you have to open one rule at a time, having first
# suspected that rule, is not oversight. These two functions are the push
# half — one read that answers "what changed lately", and one write that
# puts it back.
_DRIFT_LIMIT_MAX = 50
def _owned_rules_clause(user_id: int):
"""Rules whose rulebook or project this user owns — the LISTING form of
`_fetch_owned_rule`.
Deliberately the same two paths in the same order, because a listing that
admits a rule the per-row fetch would refuse is a leak, and one that
refuses a rule the fetch admits is a row the operator cannot act on. The
per-row version stays the authority: everything reached through this is
re-checked by `_fetch_owned_rule` before it is written to.
"""
from scribe.models.project import Project
via_rulebook = (
select(RulebookTopic.id)
.join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id)
.where(
Rulebook.owner_user_id == user_id,
RulebookTopic.deleted_at.is_(None),
Rulebook.deleted_at.is_(None),
)
)
via_project = select(Project.id).where(
Project.user_id == user_id, Project.deleted_at.is_(None),
)
return and_(
Rule.deleted_at.is_(None),
or_(Rule.topic_id.in_(via_rulebook), Rule.project_id.in_(via_project)),
)
async def recent_preference_drift(user_id: int, limit: int = 20) -> list[dict]:
"""Preferences that have been rewritten, most recently changed first.
ONE ROW PER PREFERENCE, carrying its LATEST rewrite — not every version of
every preference. The question this answers is "what has Scribe changed
about how it works with me lately", and a preference rewritten eight times
this week is one answer to that, not eight. The full history of any one
preference is still a click away in its editor, which is where "how did
this get here" belongs.
Each row carries what it said before, what it says now, and `taught_by` —
the record named by `arose_from_id`, which step 2 made required on every
agent-written preference. That is the provenance: not who typed it (the
agent acts as the operator's own user, so the actor column cannot tell
them apart) but what the change was learned from.
Rules are excluded, and not as a filter that could be relaxed. A rule
changes when its author changes it, so "what changed without me" is not a
question about rules — including them would bury the few rows that are
actually unreviewed under every edit the operator made themselves.
"""
limit = max(1, min(int(limit), _DRIFT_LIMIT_MAX))
# The newest version per rule, by id rather than by created_at: a version
# is written once and never updated, so id order IS time order, and two
# versions written in the same clock tick still have a defined winner.
newest = (
select(
RuleVersion.rule_id.label("rule_id"),
func.max(RuleVersion.id).label("version_id"),
)
.group_by(RuleVersion.rule_id)
.subquery()
)
async with async_session() as session:
rows = (await session.execute(
select(Rule, RuleVersion)
.join(newest, newest.c.rule_id == Rule.id)
.join(RuleVersion, RuleVersion.id == newest.c.version_id)
.where(Rule.kind == "preference", _owned_rules_clause(user_id))
.order_by(RuleVersion.created_at.desc(), RuleVersion.id.desc())
.limit(limit)
)).all()
# One query for every provenance record, not one per row. These are
# titles for rows the caller can already see, so they are read without
# a further ownership filter — the same reasoning as
# milestones.titles_for, and blanking them would leave the operator a
# bare id where the whole point is naming the record.
taught_ids = {r.arose_from_id for r, _v in rows if r.arose_from_id}
titles: dict[int, str] = {}
if taught_ids:
from scribe.models.note import Note
titles = dict((await session.execute(
select(Note.id, Note.title).where(
Note.id.in_(taught_ids), Note.deleted_at.is_(None),
)
)).all())
out: list[dict] = []
for rule, version in rows:
row: dict = {
# rule_brief, not a hand-written dict: the drift pane links
# straight into the editor, so the row has to be the same shape
# every other rule listing is (#3313's three-copies lesson).
"rule": rule_brief(rule),
# What it said BEFORE this rewrite. `statement` and
# `when_to_apply` only — those are the two fields that change how
# a session behaves, and a diff of `why` is reading rather than
# reviewing.
"previous": {
"id": version.id,
"created_at": iso(version.created_at),
"title": version.title or "",
"statement": version.statement or "",
"when_to_apply": version.when_to_apply or "",
},
# The text it changed TO, beside the text it changed from, so the
# client can render the diff without a second call per row. A
# listing that needs N follow-ups to say what it means is one
# nobody scrolls.
"current": {
"title": rule.title,
"statement": rule.statement or "",
"when_to_apply": rule.when_to_apply or "",
},
}
if rule.arose_from_id and rule.arose_from_id in titles:
row["taught_by"] = {
"id": rule.arose_from_id, "title": titles[rule.arose_from_id],
}
out.append(row)
return out
async def restore_rule_version(
rule_id: int, version_id: int, user_id: int,
) -> Optional[Rule]:
"""Put a preference back to what it said. None when it is not readable.
MILESTONE 323 DECIDED THE OPPOSITE FOR RULES, and that decision stands —
`routes/rulebooks.py` still has no restore for them. Its reasoning:
"a binding instruction should not be revertible in one click", because a
silent revert erases the only record of why the rewrite happened.
A preference inverts both halves of that. The rewrite was not the
operator's — the agent makes it mid-work, without asking, which is the
whole design — so reverting is not undoing their own decision but
exercising a veto over someone else's. And the veto has to be cheaper
than shrugging, or drift is only nominally supervised.
What makes it safe is that nothing is erased. This goes through
`update_rule` like any other edit, so the restore takes its own snapshot:
the rewrite stays in the history, with the revert recorded after it. The
history gains an entry rather than losing one.
Raises ValueError on a rule, so the kind check cannot be forgotten by a
caller that reaches this directly.
"""
async with async_session() as session:
rule = await _fetch_owned_rule(session, rule_id, user_id)
if rule is None:
return None
if (rule.kind or "rule") != "preference":
raise ValueError(
"only a preference can be restored in one action. A rule is "
"the operator's own decision and reverting it goes through an "
"ordinary edit, so the reason for the rewrite stays visible "
"(milestone 323)."
)
version = (await session.execute(
select(RuleVersion).where(
RuleVersion.id == version_id, RuleVersion.rule_id == rule_id,
)
)).scalar_one_or_none()
if version is None:
return None
fields = {
"title": version.title,
"statement": version.statement,
"when_to_apply": version.when_to_apply,
"why": version.why,
"how_to_apply": version.how_to_apply,
}
# Outside the session above, because update_rule opens its own — and
# through it rather than beside it, so the snapshot, the trigger guard and
# the embedding refresh all happen exactly as they do for a hand edit.
# A `None` field in the snapshot means the preference had nothing there,
# so it is CLEARED rather than left at today's value; passing None to
# update_rule means "leave alone", which would half-restore it.
clear = [k for k, v in fields.items() if not (v or "").strip()]
return await update_rule(
rule_id, user_id,
clear=[k for k in clear if k != "statement"],
**{k: v for k, v in fields.items() if v},
)
# ── Canon tags + typed edges (milestone 307) ─────────────────────────── # ── Canon tags + typed edges (milestone 307) ───────────────────────────
async def set_rule_systems( async def set_rule_systems(
+9 -4
View File
@@ -55,10 +55,15 @@ UNSET: object = object()
# --- serialize: structured fields -> note (title/body/tags) ------------------ # --- serialize: structured fields -> note (title/body/tags) ------------------
def compose_title(name: str, when_to_use: str = "") -> str: def compose_title(name: str, when_to_use: str = "") -> str:
"""`name — when to use` (or just `name` when no usage note is given).""" """`name — when to use` (or just `name` when no usage note is given).
name = (name or "").strip()
when = (when_to_use or "").strip() The join itself lives in `embeddings.trigger_title`, which rules and
return f"{name}{when}" if when else name 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]: def compose_tags(language: str = "", tags: list[str] | None = None) -> list[str]:
+11 -2
View File
@@ -147,11 +147,20 @@ def _now():
def fake_note(**attrs) -> MagicMock: def fake_note(**attrs) -> MagicMock:
"""A stand-in Note: own (user_id=7, the caller `_bind_user` binds), live, """A stand-in Note: own (user_id=7, the caller `_bind_user` binds), live,
not a task, no structured data. The injected menu reads is_task / not a task, no structured data. The injected menu reads is_task /
task_kind / note_type for its kind marker, user_id for the "shared by …" task_kind / note_type / status for its kind marker, user_id for the
attribution, data for a snippet's language, deleted_at for trash.""" "shared by …" attribution, data for a snippet's language, deleted_at for
trash.
`status` follows `is_task`, because on the real model it DEFINES it —
`Note.is_task` is `status is not None`. A stand-in task with no status is
a row the database cannot hold, and code that reads both would be tested
against a shape it will never meet.
"""
is_task = attrs.get("is_task", False)
return _with_defaults({ return _with_defaults({
"id": 1, "title": "t", "body": "", "tags": [], "user_id": 7, "id": 1, "title": "t", "body": "", "tags": [], "user_id": 7,
"note_type": "note", "is_task": False, "task_kind": "work", "note_type": "note", "is_task": False, "task_kind": "work",
"status": "todo" if is_task else None,
"data": None, "deleted_at": None, "data": None, "deleted_at": None,
# Milestone 317: a truthy mock here reads as "this note carries a # Milestone 317: a truthy mock here reads as "this note carries a
# check", which trips the guard on records that may not have one. # check", which trips the guard on records that may not have one.
+301
View File
@@ -0,0 +1,301 @@
"""A record you merely CITE carries its status (#4154, milestone 409 step 8).
WHY THIS EXISTS
Step 1 (#4010) made placement cheap for a task whose status CHANGES: the write
returns where it sits, and the report is written from that. It does nothing for
a task a reply only mentions. In this milestone's own step-6 review the session
reported "#4014 is the open step of milestone 409". #4014 had been done for
four days; the open step was #4015. The id did not come from a read — it came
from a retrieval hint, which carries an id, a kind and a title and says nothing
about status, while `list_milestones` said "8 of 9" and would not say which one.
Two surfaces, one principle: the status arrives with the id.
1. A milestone summary row names its next open step, so the listing that
prompts the question also answers it.
2. An injected menu line renders a task's status, so a finished step cannot
read as live work.
THE ONE THAT MATTERS MOST
`test_the_listing_and_placement_agree_on_what_open_means` — two surfaces now
answer "what is next" and they must not drift. It is written as a behavioural
cross-check rather than `assert milestones.OPEN_STEP_STATUSES is placement._OPEN`,
which shares one object today and would therefore pass no matter what either
side did with it (rule 167: a guard has to be able to fail). This one fails if
either side changes its ordering or its notion of "open" alone.
"""
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from sqlalchemy import true
from scribe.services import milestones as ms
from scribe.services import placement as pl
from tests.helpers import fake_note, make_mock_session
def _milestone_row(mid: int, project_id: int = 5):
"""A Milestone as the first query returns it — .to_dict() feeds the entry."""
row = MagicMock()
row.id, row.project_id, row.user_id = mid, project_id, 7
row.updated_at = None
row.to_dict.return_value = {"id": mid, "title": f"M{mid}", "project_id": project_id}
return row
def _sessions(results: list, counter: list[int], seen_sql: list | None = None):
"""A patched `async_session` handing out queued results and counting queries.
Every query in `get_project_milestone_summaries` pulls the next entry, so
the ORDER of `results` pins the order of the queries — which is what makes
the query-count assertion meaningful rather than incidental.
Counts rows are 4-tuples — (milestone_id, status, count, max(updated_at)) —
because the counts query also carries the touched-at that `last_touched_at`
is computed from. A 3-tuple unpacks as a ValueError, not as a wrong answer.
`seen_sql` collects the rendered statements. The ordering these surfaces
have to agree on lives in an ORDER BY, which no amount of feeding rows to a
mock can exercise — a stand-in hands back whatever order the test chose.
"""
session = make_mock_session()
async def _execute(stmt=None, *_a, **_kw):
counter[0] += 1
if seen_sql is not None:
seen_sql.append(str(stmt))
rows = results.pop(0) if results else []
r = MagicMock()
r.fetchall = MagicMock(return_value=rows)
r.scalars = MagicMock(return_value=MagicMock(all=lambda: rows))
return r
session.execute = _execute
return MagicMock(return_value=session)
def _order_by(sql: str) -> str:
"""The ORDER BY tail of a rendered statement, normalised."""
_, _, tail = sql.upper().partition("ORDER BY")
return " ".join(tail.split())
async def _summaries(milestones, counts, open_steps, counter=None, seen_sql=None):
counter = counter if counter is not None else [0]
results = [milestones, counts, open_steps]
with patch.object(ms, "async_session", _sessions(results, counter, seen_sql)), \
patch.object(ms.access_svc, "readable_notes_clause",
MagicMock(return_value=true())):
return await ms.get_project_milestone_summaries(7, [5])
async def _placement_sql(seen_sql: list):
"""Run `task_placement` far enough to render its sibling-steps query."""
session = make_mock_session()
milestone = MagicMock(id=409, project_id=5, user_id=7, title="M", status="active")
async def _execute(stmt=None, *_a, **_kw):
seen_sql.append(str(stmt))
r = MagicMock()
r.scalars = MagicMock(return_value=MagicMock(
first=lambda: milestone, all=lambda: [],
))
return r
session.execute = _execute
with patch.object(pl, "async_session", MagicMock(return_value=session)), \
patch.object(pl.access_svc, "can_read_project", AsyncMock(return_value=False)), \
patch.object(pl.access_svc, "readable_notes_clause",
MagicMock(return_value=true())):
await pl.task_placement(7, SimpleNamespace(
id=1, project_id=5, milestone_id=409, status="todo",
))
@pytest.mark.asyncio
async def test_a_milestone_row_names_its_next_open_step():
"""The listing that says "8 of 9" now says WHICH one, in the same read."""
rows = await _summaries(
[_milestone_row(409)],
[(409, "done", 8, None), (409, "todo", 1, None)],
[(409, 4154, "Step 8 — a cited record carries its status", "todo")],
)
assert rows[5][0]["next_step"] == {
"id": 4154,
"title": "Step 8 — a cited record carries its status",
"status": "todo",
}
@pytest.mark.asyncio
async def test_a_finished_plan_names_no_next_step_rather_than_omitting_the_key():
"""Always present, None included.
A key that disappears when the answer is "nothing left" makes a reader
test for its absence to learn the answer, and a reader who forgets is back
to guessing — which is the failure this step exists for.
"""
rows = await _summaries([_milestone_row(416)], [(416, "done", 9, None)], [])
assert rows[5][0]["next_step"] is None
assert "next_step" in rows[5][0]
@pytest.mark.asyncio
async def test_the_earliest_open_step_wins_not_the_earliest_step():
"""The done-first case, which is the shape every part-finished plan has.
Steps arrive in creation order, so a plan whose first two are closed must
name the third. Naming the first would reproduce the exact misreport: a
step that IS in the milestone, that IS plausible, and that is finished.
"""
rows = await _summaries(
[_milestone_row(409)],
[(409, "done", 2, None), (409, "todo", 2, None)],
# The query filters to open steps, so the closed ones never appear —
# this asserts the ORDER of what does: earliest open, not last written.
[(409, 4015, "Step 6", "in_progress"), (409, 4154, "Step 8", "todo")],
)
assert rows[5][0]["next_step"]["id"] == 4015
assert rows[5][0]["next_step"]["status"] == "in_progress"
@pytest.mark.asyncio
async def test_the_batch_does_not_fan_out_per_milestone():
"""#2384's shape must not come back through the new query.
Three queries for one milestone and three for forty — the cost is in the
number of QUERIES, not the number of plans. A per-milestone "what's next"
lookup would produce identical output and reproduce the pool exhaustion.
"""
counter = [0]
await _summaries(
[_milestone_row(i) for i in range(40)],
[(i, "todo", 1, None) for i in range(40)],
[(i, 1000 + i, f"S{i}", "todo") for i in range(40)],
counter=counter,
)
assert counter[0] == 3, f"{counter[0]} queries for 40 milestones"
@pytest.mark.asyncio
async def test_both_step_queries_take_the_same_visibility_clause():
"""Rule 78, and a correctness point on top of it.
`next_step` names a step; the counts beside it say how many there are. Read
through different clauses, a row could name a step its own progress numbers
exclude — one row describing two different milestones.
"""
seen = []
def _clause(uid):
seen.append(uid)
return true()
results = [[_milestone_row(409)], [(409, "todo", 1, None)], [(409, 1, "S", "todo")]]
with patch.object(ms, "async_session", _sessions(results, [0])), \
patch.object(ms.access_svc, "readable_notes_clause", _clause):
await ms.get_project_milestone_summaries(7, [5])
# Built ONCE and reused, so the two queries cannot be given different ones.
assert seen == [7]
@pytest.mark.asyncio
async def test_the_listing_and_placement_agree_on_what_open_means():
"""THE GUARD. Two surfaces answer "what is next"; they must not drift.
Behavioural on purpose — see the module docstring. `placement` answers for
a session that knows which step it is on, the listing for one that does
not, so the two are only equal when the current step is the first. That is
the case checked here, and it fails if either side's ordering or its open
set moves without the other.
"""
steps = [
SimpleNamespace(id=1, title="Step 1", status="done"),
SimpleNamespace(id=2, title="Step 2", status="done"),
SimpleNamespace(id=3, title="Step 3", status="todo"),
SimpleNamespace(id=4, title="Step 4", status="todo"),
]
# From the first step, placement's "the next open one after this" and the
# listing's "the earliest open one" are the same question.
from_placement = pl._next_open(steps, current_id=1)
listing_sql: list = []
rows = await _summaries(
[_milestone_row(409)],
[(409, "done", 2, None), (409, "todo", 2, None)],
[(409, s.id, s.title, s.status) for s in steps if s.status in ms.OPEN_STEP_STATUSES],
seen_sql=listing_sql,
)
assert rows[5][0]["next_step"] == from_placement
# And they agree on ORDERING, which the rows above cannot show: a stand-in
# session hands back whatever order this test chose, so "the first row
# wins" would pass against any ORDER BY at all. The database does the
# sorting in production, so the assertion belongs on the statement.
placement_sql: list = []
await _placement_sql(placement_sql)
steps_query = next(q for q in placement_sql if "ORDER BY" in q.upper())
assert _order_by(listing_sql[-1]) == _order_by(steps_query)
assert _order_by(listing_sql[-1]), "the open-steps query has no ORDER BY at all"
def test_a_task_line_in_the_menu_says_where_the_work_stands():
"""The other half: an id and a title with no status is what got cited."""
from scribe.services.plugin_context import _record_kind
assert _record_kind(fake_note(is_task=True, status="done")) == "task (done)"
assert _record_kind(fake_note(is_task=True, status="todo")) == "task (todo)"
assert _record_kind(
fake_note(is_task=True, task_kind="issue", status="in_progress")
) == "issue (in_progress)"
def test_a_record_with_no_status_gains_no_parenthesis():
"""Only tasks. A note or a snippet has no status to be wrong about, and a
marker that appeared on every line would stop being read."""
from scribe.services.plugin_context import _record_kind
assert _record_kind(fake_note(note_type="snippet")) == "snippet"
assert _record_kind(fake_note(note_type="process")) == "process"
assert _record_kind(fake_note()) == "note"
def test_the_listing_tools_say_what_next_step_is_and_how_to_use_it():
"""The contract every MCP client reads (rule 119, decision #4027).
The field is only worth adding if a caller knows it is there. Without this
the docstring can be tidied to a parameter list and the one surface that
reaches a non-Claude-Code client goes quiet about it.
"""
from tests.helpers import tool_doc
for module, name in (
("scribe.mcp.tools.milestones", "list_milestones"),
("scribe.mcp.tools.projects", "enter_project"),
("scribe.mcp.tools.projects", "get_project"),
):
doc = tool_doc(module, name).lower()
assert "next_step" in doc, f"{name} does not mention next_step"
# Naming the field is not enough — a reader has to be told that a null
# is an answer, or an absent next step reads as data not yet loaded.
assert "null" in doc or "none" in doc, f"{name} does not say when it is empty"
def test_the_reply_skill_says_a_cited_record_still_gets_read():
"""The other half of the fix is a practice, and it has one home.
Presence, not absence — the skill legitimately discusses recall in order to
warn against it, so an absence check here would be satisfied by the warning
itself (snippet #3352).
"""
import pathlib
skill = pathlib.Path(__file__).resolve().parents[1] / "plugin/skills/reporting-back/SKILL.md"
text = " ".join(skill.read_text().split()).lower()
assert "a record you only mention is a record to read" in text
# And it points at the field rather than restating how to compute it.
assert "next_step" in text
+18
View File
@@ -170,6 +170,24 @@ TOPICS: tuple[Topic, ...] = (
Topic("the operator's own reply shapes come first", "skill:reporting-back", Topic("the operator's own reply shapes come first", "skill:reporting-back",
("reply_preferences", 'content_type="rule"'), ("reply_preferences", 'content_type="rule"'),
"the operator's own shapes come first"), "the operator's own shapes come first"),
# Milestone 409 step 7: the sections were being FILLED rather than chosen,
# so a reply could satisfy every heading and still be unreadable. These
# three are the discipline around the scaffold, not the scaffold itself.
Topic("sections are chosen, not filled, and the reply is cut twice",
"skill:reporting-back", ("not a form to complete", "what can go"),
"write the shortest reply that carries the answer"),
Topic("a needs-you item is theirs to decide and blocks work",
"skill:reporting-back", ("is this theirs to decide",),
"this section is for what blocks them, not for what you are unsure about"),
Topic("a settled decision is acted on, not re-opened",
"skill:reporting-back", ("already made",),
"reads as contradicting yourself rather than as being careful"),
# Milestone 409 step 8: `placement` rides a WRITE, so a task the reply only
# cites arrived with nothing vouching for it — which is how a step finished
# four days earlier was reported as the open one (#4154).
Topic("a record you only cite still gets read",
"skill:reporting-back", ("next_step", "only mention"),
"a record you only mention is a record to read"),
# ── per-tool contracts and in-band behaviour — owned by the server ── # ── per-tool contracts and in-band behaviour — owned by the server ──
Topic("closing a task cues the report", "docstrings", ("report_back",), "reporting this to the operator?"), Topic("closing a task cues the report", "docstrings", ("report_back",), "reporting this to the operator?"),
Topic("a note that asserts a fact carries its check", "docstrings", ("verify_with", "expires_when"), Topic("a note that asserts a fact carries its check", "docstrings", ("verify_with", "expires_when"),
+112
View File
@@ -0,0 +1,112 @@
"""A lesson is a row the database actually accepts (milestone 385 step 2).
WHY THIS IS THE REAL GUARD, AND WHY IT IS HERE
The step asked for "a guard that the CHECK actually accepts `lesson` and
rejects a typo", citing #3128 — a `spike` kind that could not be written on an
instance predating its migration. That failure mode belongs to `task_kind`,
which IS gated (`notes_task_kind_check`, migrations 0056 and 0065).
`note_type` is not gated at all: migration 0036 added it as plain Text with a
server default and nothing has constrained it since.
So there is no whitelist to expand and no rejection to assert. Writing the
guard as "the constraint admits lesson" would have pinned a constraint that
does not exist; writing it as "no constraint exists" would pin today's schema
rather than the behaviour that matters, and would go red on a change that is
perfectly fine.
Asserting the WRITE covers both worlds. It passes today, it keeps passing if a
CHECK is added that admits `lesson`, and it goes red the day one is added that
does not — which is the only outcome anyone needs to be told about.
The second test is the cell #3163 asks to be left empty on purpose: a lesson
must not be a task. `is_task` is a read-only property over `status`, so this
cannot be asserted by setting a flag — it has to be observed on a real row.
"""
import pytest
import pytest_asyncio
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.note import Note
from scribe.services import lessons as lessons_svc
from scribe.services import notes as notes_svc
from tests.helpers import ensure_user
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
OWNER_USERNAME = "lesson_kind_owner"
TRIGGER = "the operator pasted a stack trace and said it is still broken"
SUBJECT = "Change one thing, then look"
@pytest_asyncio.fixture
async def owner_id():
async with async_session() as s:
owner = await ensure_user(s, OWNER_USERNAME)
await s.commit()
uid = owner.id
# Cleaned at SETUP rather than teardown: create_note fires a detached
# embedding refresh that opens its own connection and writes the row,
# and a teardown delete would race it. A fresh loop has already
# cancelled whatever the previous test left in flight.
for note in (await s.execute(
select(Note).where(
Note.user_id == uid,
Note.note_type == lessons_svc.LESSON_NOTE_TYPE,
)
)).scalars().all():
await s.delete(note)
await s.commit()
return uid
async def test_a_lesson_is_a_row_the_database_accepts(owner_id):
"""The whole point. Goes red if `note_type` is ever gated without this
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),
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},
)
async with async_session() as s:
stored = (await s.execute(
select(Note).where(Note.id == lesson.id)
)).scalars().one()
assert stored.note_type == "lesson"
# The trigger survives the round trip on both halves — the indexed mirror
# 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}"
assert "**When to apply:**" in (stored.body or "")
async def test_a_lesson_is_not_a_task(owner_id):
"""#3163's cell left empty on purpose.
`is_task` is `status is not None` and is read-only, so this is only
observable on a stored row. A lesson that arrived with a status would join
the open-work listings — the one way an unfilled field changes what the
record IS rather than what it says.
"""
lesson = await notes_svc.create_note(
owner_id,
title=lessons_svc.compose_title(SUBJECT, TRIGGER),
body="One change at a time.",
note_type=lessons_svc.LESSON_NOTE_TYPE,
)
async with async_session() as s:
stored = (await s.execute(
select(Note).where(Note.id == lesson.id)
)).scalars().one()
assert stored.status is None
assert stored.is_task is False
assert stored.milestone_id is None
+143
View File
@@ -0,0 +1,143 @@
"""A lesson is reachable from a project it was not written on (step 3).
WHY THIS IS AN INTEGRATION TEST
The carve-out is one `OR` inside the query's project filter, and what has to be
proved is which ROWS come back — a mock session returns whatever it was told to
and would pass with the predicate inverted. Every note here embeds identically
to the query, so the only thing that can separate them is the scoping: a leak
and a correct result are otherwise indistinguishable.
The embedder is stubbed, as in the other pgvector tests, so this depends on
Postgres and the query rather than on a downloaded model. No similarity number
is asserted — only membership.
"""
import uuid
from unittest.mock import AsyncMock, patch
import pytest
import pytest_asyncio
from scribe.models import async_session
from scribe.models.embedding import EMBEDDING_DIM, NoteEmbedding
from scribe.models.note import Note
from scribe.models.project import Project
from scribe.services import lessons as lessons_svc
from scribe.services.embeddings import CHUNKER_VERSION, semantic_search_notes
from tests.helpers import ensure_user
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
QUERY_VEC = [1.0] + [0.0] * (EMBEDDING_DIM - 1)
@pytest_asyncio.fixture
async def corpus():
"""A lesson and a plain note on project A, plus a lesson on project B.
Fresh users per run: every row matches the query equally, so a record left
behind by another test would read exactly like a scoping leak.
"""
tag = uuid.uuid4().hex[:8]
async with async_session() as s:
owner = await ensure_user(s, f"lesson_reach_owner_{tag}")
await s.flush()
a = Project(user_id=owner.id, title="Where it was learned")
b = Project(user_id=owner.id, title="Somewhere else entirely")
s.add_all([a, b])
await s.flush()
rows = {
"lesson_on_a": Note(
user_id=owner.id, project_id=a.id,
note_type=lessons_svc.LESSON_NOTE_TYPE,
title="Suspect the guard — a test fails on correct code",
body="**When to apply:** a test fails on correct code",
),
"note_on_a": Note(
user_id=owner.id, project_id=a.id, note_type="note",
title="An ordinary note", body="ordinary body",
),
"lesson_on_b": Note(
user_id=owner.id, project_id=b.id,
note_type=lessons_svc.LESSON_NOTE_TYPE,
title="A lesson that lives on B", body="**When to apply:** on B",
),
}
s.add_all(rows.values())
await s.flush()
for note in rows.values():
s.add(NoteEmbedding(
note_id=note.id, chunk_index=0, user_id=owner.id,
embedding=QUERY_VEC, chunk_text=note.title,
chunker_version=CHUNKER_VERSION,
))
ids = {k: n.id for k, n in rows.items()}
ids["owner"], ids["a"], ids["b"] = owner.id, a.id, b.id
await s.commit()
return ids
async def _search(uid, **kw):
with patch(
"scribe.services.embeddings.get_embedding", AsyncMock(return_value=QUERY_VEC)
):
hits = await semantic_search_notes(uid, "when does this apply", limit=20, **kw)
return {note.id for _score, note in hits}
async def test_a_lesson_is_found_from_another_project(corpus):
"""THE acceptance this step exists for. Searching project B reaches the
lesson written on project A — the case the kind was created for, because a
transferable insight is most useful on the project that has not learned it
yet."""
found = await _search(
corpus["owner"], project_id=corpus["b"], include_global_kinds=True,
)
assert corpus["lesson_on_a"] in found
assert corpus["lesson_on_b"] in found
async def test_an_ordinary_note_stays_where_it_was_written(corpus):
"""The other half, and the one that would make this change a bug. Project
scoping is deliberate for every other kind; the carve-out admits ONE kind
rather than weakening the filter."""
found = await _search(
corpus["owner"], project_id=corpus["b"], include_global_kinds=True,
)
assert corpus["note_on_a"] not in found
async def test_the_carve_out_is_off_unless_asked_for(corpus):
"""Default off, because the near-duplicate gate and ordinary recall both
depend on the project filter holding. A globally visible kind arriving
there would let a lesson block an unrelated note's create on a project its
author never touched."""
found = await _search(corpus["owner"], project_id=corpus["b"])
assert found == {corpus["lesson_on_b"]}
async def test_the_home_project_is_unchanged(corpus):
"""Searching the project a lesson was written on returns it either way —
the carve-out adds reach, it does not move anything."""
for flag in (False, True):
found = await _search(
corpus["owner"], project_id=corpus["a"], include_global_kinds=flag,
)
assert corpus["lesson_on_a"] in found
assert corpus["note_on_a"] in found
async def test_a_kind_filter_still_means_what_it_says(corpus):
"""The carve-out widens the PROJECT filter only. A caller narrowing to
snippets asked for snippets, and quietly handing it lessons would make
`note_type` mean something different depending on a flag it did not set."""
found = await _search(
corpus["owner"], project_id=corpus["b"],
include_global_kinds=True, note_type="snippet",
)
assert found == set()
+291
View File
@@ -0,0 +1,291 @@
"""Real-Postgres tests for the drift surface (milestone 399 step 5, #3895).
WHY THIS EXISTS
A preference is the one record kind the AGENT rewrites, mid-work, without
asking. That is what keeps it current and it is also the risk the milestone
named up front: an agent misreads one session, rewrites a preference, and
follows the rewritten version forever while the operator never sees the
moment it changed — a confident wrong answer wearing the operator's own
authority.
`rule_versions` already recorded every rewrite. What it did not do is arrive.
`recent_preference_drift` is the read that makes drift PUSHED rather than
pulled, and `restore_rule_version` is the veto.
WHY NOT MOCKS
Every claim here is about which rows come back and in what order — a grouped
subquery picking the newest version per rule, an ownership clause spanning two
paths, and a write that has to leave the history longer than it found it. A
stand-in session returns whatever the test handed it, so it could confirm none
of those. `test_the_restore_is_recorded_as_an_edit` is the one that matters
most: it is the whole basis for allowing a one-click revert here when
milestone 323 refused one for rules, and if it were ever to pass while the
restore silently overwrote history, the carve-out would be indefensible.
"""
import pytest
import pytest_asyncio
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.note import Note
from scribe.models.rulebook import Rulebook
from scribe.models.rule_version import RuleVersion
from scribe.services import rulebooks as rulebooks_svc
from tests.helpers import ensure_user
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
OWNER_USERNAME = "drift_owner"
STRANGER_USERNAME = "drift_stranger"
# The moment a preference names, in the words a session actually produces.
# Required on a preference, so every fixture below carries one.
TRIGGER = "the operator pasted a stack trace and said it is still broken"
async def _purge(uid: int) -> None:
"""Clear this user's rulebooks. At SETUP, never teardown.
`update_rule` fires a detached embedding refresh that opens its own
connection and UPDATEs the rule row. A teardown delete would race it into
a deadlock — the same reasoning test_integration_rule_versions records.
"""
async with async_session() as s:
for book in (await s.execute(
select(Rulebook).where(Rulebook.owner_user_id == uid)
)).scalars().all():
await s.delete(book)
await s.commit()
@pytest_asyncio.fixture
async def world():
async with async_session() as s:
owner = await ensure_user(s, OWNER_USERNAME)
stranger = await ensure_user(s, STRANGER_USERNAME)
await s.commit()
uid, sid = owner.id, stranger.id
# The record a preference points at as what taught it. A plain note:
# `arose_from_id` is a FK to notes, and the drift row names it so the
# operator reads a title rather than a bare number.
taught = Note(user_id=uid, title="Paced the debugging one step at a time")
s.add(taught)
await s.commit()
await s.refresh(taught)
taught_id = taught.id
await _purge(uid)
await _purge(sid)
book = await rulebooks_svc.create_rulebook(uid, "Drift fixtures")
topic = await rulebooks_svc.create_topic(book.id, uid, "collaboration")
pref = await rulebooks_svc.create_rule(
topic.id, uid, "Pace hard debugging",
"Change one thing, then look.",
when_to_apply=TRIGGER, kind="preference", arose_from_id=taught_id,
)
rule = await rulebooks_svc.create_rule(
topic.id, uid, "dev is home",
"Ordinary work lands on dev.",
when_to_apply="about to commit", kind="rule",
)
return {
"uid": uid, "stranger_id": sid, "topic_id": topic.id,
"pref_id": pref.id, "rule_id": rule.id, "taught_id": taught_id,
}
async def test_a_preference_nobody_rewrote_is_not_drift(world):
"""The empty state is a REAL state, not a not-yet-loaded one.
A preference that has never been touched has nothing for the operator to
review, and listing it would bury the rows that do under the ones that
don't — which is how a supervision surface stops being read.
"""
rows = await rulebooks_svc.recent_preference_drift(world["uid"])
assert rows == []
async def test_a_rewritten_preference_carries_both_wordings_and_what_taught_it(world):
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"],
statement="Change one thing, then look. Say what you expect first.",
)
[row] = await rulebooks_svc.recent_preference_drift(world["uid"])
assert row["rule"]["id"] == world["pref_id"]
# Both texts ride along, so the list can show the diff without a
# follow-up call per row.
assert row["previous"]["statement"] == "Change one thing, then look."
assert row["current"]["statement"].endswith("Say what you expect first.")
# The provenance is NAMED, not numbered: a bare id reads as complete to
# the writer and as homework to the reader.
assert row["taught_by"] == {
"id": world["taught_id"],
"title": "Paced the debugging one step at a time",
}
async def test_a_rewritten_rule_is_not_listed(world):
"""Not a filter that could be relaxed — the question is what changed
WITHOUT the operator, and a rule changes when they change it. Including
rules would bury the few unreviewed rows under every edit they made."""
await rulebooks_svc.update_rule(
world["rule_id"], world["uid"], statement="Ordinary work lands on dev, always.",
)
assert await rulebooks_svc.recent_preference_drift(world["uid"]) == []
async def test_one_row_per_preference_carrying_its_LATEST_rewrite(world):
"""A preference rewritten three times this week is one answer to "what
changed lately", not three. The full history stays in its editor."""
for text in ("second wording.", "third wording.", "fourth wording."):
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement=text,
)
rows = await rulebooks_svc.recent_preference_drift(world["uid"])
assert len(rows) == 1
# The newest version holds what the LAST edit replaced.
assert rows[0]["previous"]["statement"] == "third wording."
assert rows[0]["current"]["statement"] == "fourth wording."
async def test_most_recently_changed_first(world):
"""The ORDER is the answer: the top of this list is what changed last."""
second = await rulebooks_svc.create_rule(
world["topic_id"], world["uid"], "Name the record",
"Say the title beside the id.",
when_to_apply="about to cite a record by number", kind="preference",
)
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement="edited first.",
)
await rulebooks_svc.update_rule(
second.id, world["uid"], statement="edited second.",
)
rows = await rulebooks_svc.recent_preference_drift(world["uid"])
assert [r["rule"]["id"] for r in rows] == [second.id, world["pref_id"]]
async def test_another_users_preferences_are_not_listed(world):
"""The listing clause has to agree with the per-row fetch. A drift pane
that reached across owners would leak the wording of someone else's
preference — and the wording is the whole payload."""
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement="mine, rewritten.",
)
assert await rulebooks_svc.recent_preference_drift(world["stranger_id"]) == []
async def test_the_limit_bounds_the_list_and_keeps_the_newest(world):
"""A cap that dropped the wrong end would hide the change just made.
Bounded because the payload is two full statements per row — so the list
has to stay short — and the end it keeps has to be the recent one, which
is the only end this surface is about.
"""
second = await rulebooks_svc.create_rule(
world["topic_id"], world["uid"], "Name the record",
"Say the title beside the id.",
when_to_apply="about to cite a record by number", kind="preference",
)
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement="edited first.",
)
await rulebooks_svc.update_rule(
second.id, world["uid"], statement="edited second.",
)
rows = await rulebooks_svc.recent_preference_drift(world["uid"], limit=1)
assert [r["rule"]["id"] for r in rows] == [second.id]
# A caller asking for everything gets a page, not the table — and the
# clamp is silent rather than an error, because "too many" is a request
# to serve, not a mistake to report.
assert len(await rulebooks_svc.recent_preference_drift(
world["uid"], limit=10_000,
)) == 2
async def test_restoring_puts_the_old_wording_back(world):
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement="the agent's rewrite.",
)
[row] = await rulebooks_svc.recent_preference_drift(world["uid"])
restored = await rulebooks_svc.restore_rule_version(
world["pref_id"], row["previous"]["id"], world["uid"],
)
assert restored is not None
assert restored.statement == "Change one thing, then look."
async def test_the_restore_is_recorded_as_an_edit(world):
"""THE ONE THAT MATTERS MOST — it is why this carve-out is defensible.
Milestone 323 refused a one-click restore for rules because "a silent
revert would erase the only record of why the rewrite happened". The
preference carve-out survives that objection only while nothing is
erased: the restore goes through `update_rule`, so it takes its own
snapshot and the history GAINS an entry.
A restore implemented as a direct write would satisfy the test above and
fail this one, which is the point of having both.
"""
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement="the agent's rewrite.",
)
async with async_session() as s:
before = len((await s.execute(
select(RuleVersion).where(RuleVersion.rule_id == world["pref_id"])
)).scalars().all())
[row] = await rulebooks_svc.recent_preference_drift(world["uid"])
await rulebooks_svc.restore_rule_version(
world["pref_id"], row["previous"]["id"], world["uid"],
)
async with async_session() as s:
versions = list((await s.execute(
select(RuleVersion).where(RuleVersion.rule_id == world["pref_id"])
.order_by(RuleVersion.id)
)).scalars().all())
assert len(versions) == before + 1
# The rewrite is still there, and the newest entry is what the restore
# replaced — the rewrite itself. Nothing was overwritten.
assert versions[-1].statement == "the agent's rewrite."
async def test_a_rule_refuses_the_one_click_restore(world):
"""Milestone 323 stands for rules, and the kind check is in the service
rather than only in the route — so a caller reaching this directly cannot
forget it."""
await rulebooks_svc.update_rule(
world["rule_id"], world["uid"], statement="rewritten by hand.",
)
async with async_session() as s:
version = (await s.execute(
select(RuleVersion).where(RuleVersion.rule_id == world["rule_id"])
)).scalars().first()
with pytest.raises(ValueError, match="preference"):
await rulebooks_svc.restore_rule_version(
world["rule_id"], version.id, world["uid"],
)
async def test_a_stranger_cannot_restore_a_preference_they_cannot_read(world):
"""None, not a raise: "not yours" and "not a preference" are different
answers, and the route turns them into 404 and 409 respectively."""
await rulebooks_svc.update_rule(
world["pref_id"], world["uid"], statement="mine, rewritten.",
)
[row] = await rulebooks_svc.recent_preference_drift(world["uid"])
assert await rulebooks_svc.restore_rule_version(
world["pref_id"], row["previous"]["id"], world["stranger_id"],
) is None
+7 -1
View File
@@ -38,6 +38,7 @@ ROWS = {
"plain note": fake_note(note_type="note"), "plain note": fake_note(note_type="note"),
"process": fake_note(note_type="process"), "process": fake_note(note_type="process"),
"snippet": fake_snippet(), "snippet": fake_snippet(),
"lesson": fake_note(note_type="lesson"),
"work task": fake_task(task_kind="work", note_type="note"), "work task": fake_task(task_kind="work", note_type="note"),
"issue": fake_task(task_kind="issue", note_type="note"), "issue": fake_task(task_kind="issue", note_type="note"),
"spike": fake_task(task_kind="spike", note_type="note"), "spike": fake_task(task_kind="spike", note_type="note"),
@@ -72,6 +73,7 @@ def test_pre_filter_never_excludes_a_row_the_facet_wants(facet):
("note", {"plain note"}), ("note", {"plain note"}),
("process", {"process"}), ("process", {"process"}),
("snippet", {"snippet"}), ("snippet", {"snippet"}),
("lesson", {"lesson"}),
("", set(ROWS)), ("", set(ROWS)),
], ],
) )
@@ -101,7 +103,11 @@ def test_the_live_task_kinds_are_all_facets():
def test_non_task_facets_are_the_note_types_and_only_those(): def test_non_task_facets_are_the_note_types_and_only_those():
assert set(NON_TASK_FACETS) == {"note", "process", "snippet"} """Spelled out rather than derived, so adding a kind to `_FACETS` has to
be a deliberate edit in two places. `lesson` joined in milestone 385 step
2 (#3729) and is non-task on purpose: `is_task` IS `status is not None`,
so a lesson that acquired a status would stop being a lesson."""
assert set(NON_TASK_FACETS) == {"note", "process", "snippet", "lesson"}
@pytest.mark.parametrize("facet", sorted(FACET_TYPES)) @pytest.mark.parametrize("facet", sorted(FACET_TYPES))
+139
View File
@@ -0,0 +1,139 @@
"""The document a lesson is embedded as (milestone 385 step 3).
WHY THIS IS THE STEP THAT DECIDES THE MILESTONE
Everything before this is storage. A lesson stored with a trigger but embedded
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()`
The step anticipated one. There isn't, and the difference is where the sharp
shape LIVES rather than whether it exists.
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.0100.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.
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.
"""
from __future__ import annotations
from scribe.services import lessons as lessons_svc
from scribe.services.embeddings import chunk_document, 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 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)
document = embedding_text(title, body)
assert document.count(TRIGGER) == 2
# Once in each half, not twice in one of them.
assert TRIGGER in title
assert TRIGGER in body
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)
assert title == f"{SUBJECT}{TRIGGER}"
assert body.splitlines()[0] == f"**When to apply:** {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)
chunks = chunk_document(title, body)
assert len(chunks) == 1
assert chunks[0].count(TRIGGER) == 2
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.
"""
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)
chunks = chunk_document(title, body)
assert len(chunks) > 1, "the fixture must actually exceed the chunk budget"
assert all(TRIGGER in chunk for chunk in chunks)
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)
assert title == SUBJECT
assert body == INSIGHT
assert chunk_document(title, body) == [f"{SUBJECT}\n{INSIGHT}"]
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."""
from types import SimpleNamespace
_, body = lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT)
no_mirror = SimpleNamespace(data=None, body=body)
assert lessons_svc.lesson_trigger(no_mirror) == TRIGGER
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."""
assert lessons_svc.lesson_document(SUBJECT, TRIGGER, INSIGHT) == (
lessons_svc.compose_title(SUBJECT, TRIGGER),
lessons_svc.compose_body(INSIGHT, TRIGGER),
)
+126
View File
@@ -0,0 +1,126 @@
"""The `lesson` kind — its trigger, its title, and its place in the vocabulary.
WHY THIS EXISTS (milestone 385 step 2, #3729)
A lesson is a note that must be findable by WHEN IT APPLIES rather than by what
it is about. That is a document-shape fact: the trigger has to reach the
embedded text, and decision #4157 put it in `notes.data` with a mirror in the
title and the head of the body — the shape snippets already use.
THE ONE THAT MATTERS MOST
`test_one_join_builds_every_trigger_title`. Three kinds now rank on a
`{subject}{trigger}` title: rules, snippets and lessons. That join had three
implementations before this step and would have had four; #3207 records what
that costs. The guard is behavioural rather than `assert a is b`, because the
three are reached through different public names and a test that compared
identities would pass on a re-implementation that merely re-exported.
WHAT IS DELIBERATELY NOT TESTED HERE
That the database accepts `note_type='lesson'`. `note_type` carries no CHECK —
only `task_kind` does — so there is nothing to assert against in a unit test,
and asserting the absence would pin the schema's current shape rather than the
behaviour that matters. The real guard is in the integration lane, where a
lesson is written and read back: it holds whether or not a constraint exists,
and goes red the day one is added without this value.
"""
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
def _note(data=None, body=""):
return SimpleNamespace(data=data, body=body)
def test_the_trigger_is_read_from_the_indexed_mirror():
note = _note(data={"when_to_apply": "a stack trace, and it is still broken"})
assert lessons_svc.lesson_trigger(note) == "a stack trace, and it is still broken"
def test_the_body_answers_when_the_mirror_is_absent():
"""Not dead code. A row written before the mirror existed is still readable,
and an absent mirror has to degrade to the right answer rather than to
silence — the discipline `snippet_fields` follows for the same reason."""
note = _note(body="**When to apply:** about to reach for a second hypothesis\n\nbody")
assert lessons_svc.lesson_trigger(note) == "about to reach for a second hypothesis"
def test_the_mirror_wins_when_both_are_present():
note = _note(
data={"when_to_apply": "from the mirror"},
body="**When to apply:** from the body",
)
assert lessons_svc.lesson_trigger(note) == "from the mirror"
def test_no_trigger_reads_as_empty_rather_than_raising():
"""A lesson with no trigger is unreachable, not broken. Whatever refuses to
write one belongs on the write path; a reader's job is to say so plainly."""
assert lessons_svc.lesson_trigger(_note()) == ""
assert lessons_svc.lesson_trigger(_note(data={}, body="no trigger line here")) == ""
def test_one_join_builds_every_trigger_title():
"""THE GUARD. Rules, snippets and lessons rank on the same title shape.
Behavioural on purpose — see the module docstring. Each of the three is
called through the name its own callers use, so a fourth hand-rolled copy
fails here even though it would look correct in isolation.
"""
subject, trigger = "Pace hard debugging", "a stack trace and it is still broken"
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
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 trigger_title("", "when it applies") == "when it applies"
def test_the_kind_is_in_the_browse_vocabulary():
"""A kind the browse surface does not know is a record nobody can filter to.
Asserted through the public table rather than a literal, because the door's
validation, the counts and both dialects of the type filter are all
generated from it (#3161) — so this is the one place that decides.
"""
assert lessons_svc.LESSON_NOTE_TYPE in knowledge_svc.FACET_TYPES
assert lessons_svc.LESSON_NOTE_TYPE in knowledge_svc.NON_TASK_FACETS
def test_the_global_kinds_list_names_the_lesson_and_nothing_else():
"""`embeddings.GLOBAL_NOTE_TYPES` spells "lesson" as a literal because
`services.lessons` imports `trigger_title` from that module, so importing
it back would close a cycle (milestone 385 step 3). The copy is pinned
here instead — the one thing a literal costs is that it can drift, and
this is what stops it.
"""
from scribe.services.embeddings import GLOBAL_NOTE_TYPES
assert GLOBAL_NOTE_TYPES == (lessons_svc.LESSON_NOTE_TYPE,)
def test_a_lesson_is_not_a_task_on_either_arm_of_the_filter():
"""The cell left empty on purpose (#3163).
`is_task` IS `status is not None`, so a lesson that acquired a status would
become a task and start appearing in open-work listings — the one way
filling a field in by accident changes what the record IS.
"""
assert knowledge_svc.facet_is_task(lessons_svc.LESSON_NOTE_TYPE) is False
lesson = SimpleNamespace(note_type="lesson", is_task=False, task_kind="work")
assert knowledge_svc.matches_facet(lesson, "lesson") is True
assert knowledge_svc.matches_facet(lesson, "task") is False
# And it does not answer to another kind's facet.
assert knowledge_svc.matches_facet(lesson, "note") is False
+22
View File
@@ -76,6 +76,28 @@ async def test_fable_search_content_type_filters_at_service_layer():
assert mock_search.call_args.kwargs["is_task"] is None assert mock_search.call_args.kwargs["is_task"] is None
@pytest.mark.asyncio
async def test_an_explicit_search_reaches_a_lesson_from_any_project():
"""The wiring half of milestone 385 step 3.
A lesson records an insight that transfers, so a project filter that hid it
would hide it precisely on the project that has not learned it yet. This is
the EXPLICIT search — the operator asked — so it opts in; the unasked-for
injection arms decide their own budget separately (step 5).
Asserted on the kwarg rather than on results, because what can regress here
is the wiring: the service grew the capability and a call site that never
passes it leaves the whole kind unreachable, with every unit test still
green.
"""
_user_id_ctx.set(7)
mock_search = AsyncMock(return_value=[])
with patch("scribe.mcp.tools.search.semantic_search_notes", mock_search):
await search(q="x", project_id=3)
assert mock_search.call_args.kwargs["include_global_kinds"] is True
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_fable_search_limit_is_clamped(): async def test_fable_search_limit_is_clamped():
_user_id_ctx.set(7) _user_id_ctx.set(7)
+5
View File
@@ -34,6 +34,7 @@ def _milestone(mid: int, status: str, touched_day: int) -> dict:
"updated_at": "2026-01-01T00:00:00+00:00", "last_touched_at": touched, "updated_at": "2026-01-01T00:00:00+00:00", "last_touched_at": touched,
"total": 4, "completed": 2, "pct": 50.0, "total": 4, "completed": 2, "pct": 50.0,
"status_counts": {"todo": 2, "in_progress": 0, "done": 2, "cancelled": 0}, "status_counts": {"todo": 2, "in_progress": 0, "done": 2, "cancelled": 0},
"next_step": {"id": 900 + mid, "title": f"M{mid} step 3", "status": "todo"},
} }
@@ -52,6 +53,10 @@ def test_brief_rows_leave_out_the_plan_and_what_the_caller_already_knows():
"id": 1, "title": "M1", "description": "what M1 is for", "status": "active", "id": 1, "title": "M1", "description": "what M1 is for", "status": "active",
"order_index": 1, "total": 4, "completed": 2, "pct": 50.0, "order_index": 1, "total": 4, "completed": 2, "pct": 50.0,
"status_counts": {"todo": 2, "in_progress": 0, "done": 2, "cancelled": 0}, "status_counts": {"todo": 2, "in_progress": 0, "done": 2, "cancelled": 0},
# Trimmed of the plan body, but NOT of which step is next (#4154): the
# listing is where "2 of 4 done" gets read, and that number is exactly
# what invites a reader to name the open step from memory.
"next_step": {"id": 901, "title": "M1 step 3", "status": "todo"},
}] }]
+137
View File
@@ -0,0 +1,137 @@
"""A reply's sections are chosen, not filled (#4153, milestone 409 step 7).
WHY THIS EXISTS
Step 6 measured the scaffold on live sessions and found the two halves
disagreeing: **adherence passed and the read test failed.** Completion replies
carried every section the table asks for — placement, what changed, what needs
the operator, what comes next — and the operator still could not read them.
The cause was in the skill rather than in compliance with it. It said to pick a
kind of reply "then fill its sections… keep them even when one is short", which
is an instruction to complete a form. Nothing anywhere set a ceiling, and a
section with a heading and nothing to say gets filled rather than dropped. So a
faithful reply and an unreadable one were the same reply.
WHAT IS PINNED
The discipline, not the scaffold. The fifteen categories and their sections are
unchanged and are not this file's subject:
1. Sections are chosen — a standing question is always answered, an
explanation earns its place.
2. The reply is as short as the answer allows, and gets a second pass for
what can go.
3. "Needs you" takes BOTH tests: theirs to decide, and blocking.
4. A decision already made is acted on rather than re-argued.
RULE 167, AND WHY THERE IS NO ABSENCE CHECK HERE
The obvious guard — assert the skill no longer tells anyone to "fill" a
section — is the trap snippet #3352 names. This skill legitimately discusses
filling in order to warn against it ("not a form to complete", "a section
filled because it was in the table"), so an absence check would fail on the
corrected text: a false alarm about the very thing it protects.
So every assertion here is a PRESENCE check, and one is POSITIONAL —
`test_the_needs_you_test_sits_with_the_needs_you_section` pins where the test
lives, not merely that the words occur somewhere in the file. All four were
falsified against the pre-#4153 text before being committed.
"""
from __future__ import annotations
import pathlib
ROOT = pathlib.Path(__file__).resolve().parents[1]
SKILL = ROOT / "plugin/skills/reporting-back/SKILL.md"
def _flat() -> str:
"""Whitespace-flattened: the file is hard-wrapped, so phrases straddle lines."""
return " ".join(SKILL.read_text().split()).lower()
def test_sections_are_chosen_rather_than_completed():
"""The framing the operator reacted to was "fill its sections"."""
text = _flat()
assert "not a form to complete" in text
# Both halves of the distinction, or "choose" collapses back into "fill":
# a standing question is answered even when the answer is nothing, an
# explanation is dropped when it changes nothing.
assert "always answered, even when the answer is nothing" in text
assert "changes what the operator does or decides" in text
def test_the_reply_carries_a_length_discipline():
"""Without a ceiling, every section is an invitation to keep writing."""
text = _flat()
assert "write the shortest reply that carries the answer" in text
# And the ceiling has named exceptions, so this cannot be read as
# "always be terse" — a measurement the operator asked for still earns room.
assert "extra length has to be earned" in text
def test_the_reply_gets_a_second_pass_for_what_can_go():
"""Cutting is a separate act from writing, and needs saying separately.
The pre-existing check asked whether anything was MISSING, which a bloated
reply passes.
"""
text = _flat()
assert "what can go" in text
# Cutting must not read as withholding, or it will not be done.
assert "cutting is not hiding" in text
def test_the_needs_you_test_sits_with_the_needs_you_section():
"""POSITIONAL. The test has to be where the section is defined.
A reader reaches this while writing that section; stated anywhere else it
is a paragraph nobody is reading at the moment it applies. Asserted by
offset rather than by presence, so moving it away fails here.
"""
raw = " ".join(SKILL.read_text().split())
lowered = raw.lower()
needs_you = lowered.index("- **needs you** — an action")
the_test = lowered.index("is this theirs to decide")
next_bullet = lowered.index("- **next** —", needs_you)
assert needs_you < the_test < next_bullet, (
"the needs-you test has moved out of the Needs you bullet; a reader "
"writing that section will not meet it"
)
# Both halves are load-bearing: "theirs" alone still admits a question the
# session could have answered, "blocking" alone admits one that is not
# theirs to make.
assert "work waiting on it" in lowered
assert "not for what you are unsure about" in lowered
def test_a_settled_decision_is_acted_on_rather_than_re_argued():
"""Re-opening a decision reads as self-contradiction, not as diligence.
Phrased as a practice rather than a prohibition (rule 165): the instruction
is what to DO with a decision, with the failure named after it.
"""
text = _flat()
assert "a decision already made gets acted on" in text
assert "reads as contradicting yourself rather than as being careful" in text
# The escape hatch stays open, or this becomes a rule against ever
# correcting anything — which is the opposite of what is wanted.
assert "genuinely overturns the choice" in text
def test_the_scaffold_itself_is_untouched():
"""This step changed the discipline AROUND the sections, not the sections.
If a future edit deletes a category while tightening the prose, that is a
change to #4009's subject and should not ride along silently here.
"""
text = _flat()
for section in ("where this sits", "what now works", "how / why",
"needs you", "next"):
assert section in text, f"completion-report section {section!r} is gone"
for kind in ("completion", "finding", "blocked / failed", "progress",
"decision", "clarification", "handoff", "approval", "conflict"):
assert kind in text, f"reply kind {kind!r} is gone"
+8 -4
View File
@@ -148,8 +148,10 @@ async def test_injected_menu_labels_the_record_kind():
hits = [ hits = [
(0.92, fake_note(id=1, title="debounce — rate-limit a callback", note_type="snippet")), (0.92, fake_note(id=1, title="debounce — rate-limit a callback", note_type="snippet")),
(0.91, fake_note(id=2, title="Release checklist", note_type="process")), (0.91, fake_note(id=2, title="Release checklist", note_type="process")),
(0.90, fake_note(id=3, title="Auth token expiry", is_task=True, task_kind="issue")), (0.90, fake_note(id=3, title="Auth token expiry", is_task=True,
(0.89, fake_note(id=4, title="Ship the drafter", is_task=True)), task_kind="issue", status="todo")),
(0.89, fake_note(id=4, title="Ship the drafter", is_task=True,
status="done")),
(0.88, fake_note(id=5, title="Why we dropped CalDAV")), (0.88, fake_note(id=5, title="Why we dropped CalDAV")),
] ]
with patch.object(plugin_context, "semantic_search_notes", with patch.object(plugin_context, "semantic_search_notes",
@@ -165,8 +167,10 @@ async def test_injected_menu_labels_the_record_kind():
assert "[snippet]" in by_id[1] assert "[snippet]" in by_id[1]
assert "[process]" in by_id[2] assert "[process]" in by_id[2]
# Task-ness wins over note_type, and an issue says so rather than "task". # Task-ness wins over note_type, and an issue says so rather than "task".
assert "[issue]" in by_id[3] # A task also carries its status (#4154): #4 is DONE, and a line that said
assert "[task]" in by_id[4] # only "[task]" is what let a finished step be cited as an open one.
assert "[issue (todo)]" in by_id[3]
assert "[task (done)]" in by_id[4]
assert "[note]" in by_id[5] assert "[note]" in by_id[5]
# Still title-first: the marker is metadata, not an excuse to carry bodies. # Still title-first: the marker is metadata, not an excuse to carry bodies.
assert "body" not in out["context"] assert "body" not in out["context"]
+4 -1
View File
@@ -71,7 +71,10 @@ async def test_total_counts_snippets_and_counts_no_task_twice():
async def test_absent_facets_report_zero_rather_than_missing(): async def test_absent_facets_report_zero_rather_than_missing():
counts, _ = await _counts([("note", 1)], []) counts, _ = await _counts([("note", 1)], [])
assert counts["note"] == 1 assert counts["note"] == 1
for key in ("process", "snippet", "task", "work", "issue", "spike", "plan"): # `lesson` is here from milestone 385 step 2: a kind added to the facet
# table and not to the counts would show an empty chip beside a feed that
# has rows in it, which is defect 3b in #3161 repeating itself.
for key in ("process", "snippet", "lesson", "task", "work", "issue", "spike", "plan"):
assert counts[key] == 0, key assert counts[key] == 0, key
assert counts["total"] == 1 assert counts["total"] == 1
+3 -1
View File
@@ -487,7 +487,9 @@ async def test_write_path_labels_a_non_snippet_hit_with_its_kind():
) )
ctx = out["context"] ctx = out["context"]
assert "· issue]" in ctx # the issue says what it is # The issue says what it is — and, since #4154, where it stands: a piece of
# prior art offered as "already tried" reads differently when it is still open.
assert "· issue (todo)]" in ctx
assert '[similar 0.72] "debounce helper"' in ctx # the snippet does not assert '[similar 0.72] "debounce helper"' in ctx # the snippet does not
# The header now names the right opener for each kind. # The header now names the right opener for each kind.
assert "get_task(id)" in ctx and "get_snippet(id)" in ctx assert "get_task(id)" in ctx and "get_snippet(id)" in ctx