Merge pull request #166 from bvandeusen/dev
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 52s
CI & Build / Python tests (push) Successful in 1m32s
CI & Build / Build & push image (push) Successful in 13s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 52s
CI & Build / Python tests (push) Successful in 1m32s
CI & Build / Build & push image (push) Successful in 13s
Plugin reply shape, preferences UI, cited-record status, and the lesson kind
Merged to ship the plugin: the marketplace checkout tracks main, so
#4153's reply-shape rewrite and #4154's "a record you only mention is a
record to read" could not reach a session from dev. Manifest moves
2026.09.18.0436 -> 2026.09.18.1606, so the executing cache refreshes.
Riding along: #4154 (next_step on milestone summaries, status in the
injected kind marker), #3895 (preferences writable + drift visible), and
milestone 385 steps 2-3 (#3729, #3730) which are inert until a lesson can
be written in step 4.
CI 7023 green on 1361ed7, all six jobs.
This commit was merged in pull request #166.
This commit is contained in:
@@ -39,12 +39,27 @@ export interface RulebookTopic {
|
||||
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 {
|
||||
id: number;
|
||||
topic_id: number | null;
|
||||
project_id: number | null;
|
||||
title: string;
|
||||
statement: string;
|
||||
kind: RuleKind;
|
||||
/** WHEN this rule fires — the trigger, not the instruction. */
|
||||
when_to_apply: string;
|
||||
why: string;
|
||||
@@ -79,6 +94,8 @@ export interface RuleHeader {
|
||||
title: string;
|
||||
statement: string;
|
||||
topic_id: number | null;
|
||||
/** Unconditional on the wire (services.rulebooks.rule_brief). */
|
||||
kind: RuleKind;
|
||||
/** A date (YYYY-MM-DD), not a timestamp. */
|
||||
updated_at: string | null;
|
||||
when_to_apply?: string;
|
||||
@@ -180,6 +197,9 @@ export async function getRule(id: number): Promise<Rule> {
|
||||
export interface RuleWrite {
|
||||
title: 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;
|
||||
why: string;
|
||||
how_to_apply: string;
|
||||
@@ -256,9 +276,56 @@ export async function getRuleVersion(
|
||||
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
|
||||
// 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> {
|
||||
return apiDelete(`/api/rules/${id}`);
|
||||
|
||||
@@ -40,3 +40,16 @@
|
||||
padding: 0.05rem 0.4rem;
|
||||
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 RuleHistoryPanel from "@/components/rules/RuleHistoryPanel.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 emit = defineEmits<{ close: [] }>();
|
||||
@@ -14,6 +14,11 @@ const canon = useCanonicalSystemsStore();
|
||||
const title = ref("");
|
||||
const statement = 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 why = ref("");
|
||||
const howToApply = ref("");
|
||||
@@ -70,6 +75,7 @@ async function load() {
|
||||
title.value = r.title;
|
||||
statement.value = r.statement;
|
||||
whenToApply.value = r.when_to_apply || "";
|
||||
kind.value = r.kind;
|
||||
systemIds.value = (r.systems ?? []).map((sys) => sys.id);
|
||||
why.value = r.why || "";
|
||||
howToApply.value = r.how_to_apply || "";
|
||||
@@ -80,6 +86,7 @@ async function load() {
|
||||
title.value = "";
|
||||
statement.value = "";
|
||||
whenToApply.value = "";
|
||||
kind.value = "rule";
|
||||
systemIds.value = [];
|
||||
why.value = "";
|
||||
howToApply.value = "";
|
||||
@@ -98,6 +105,7 @@ async function save() {
|
||||
title: title.value,
|
||||
statement: statement.value,
|
||||
when_to_apply: whenToApply.value,
|
||||
kind: kind.value,
|
||||
// Always sent, so clearing the last area actually clears it — the server
|
||||
// reads a list as "these ARE the areas now".
|
||||
system_ids: systemIds.value,
|
||||
@@ -140,7 +148,7 @@ watch(() => props.ruleId, load);
|
||||
<div class="backdrop" @click="save">
|
||||
<aside class="slide-over" @click.stop>
|
||||
<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 class="close" @click="save" aria-label="Close">×</button>
|
||||
</header>
|
||||
@@ -148,6 +156,34 @@ watch(() => props.ruleId, load);
|
||||
Title
|
||||
<input v-model="title" placeholder="e.g. dev is home" />
|
||||
</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>
|
||||
Statement <span class="required">*</span>
|
||||
<textarea v-model="statement" rows="3" placeholder="The actionable instruction (1-2 sentences)." />
|
||||
@@ -267,6 +303,17 @@ watch(() => props.ruleId, load);
|
||||
</template>
|
||||
|
||||
<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 {
|
||||
position: fixed; inset: 0;
|
||||
background: rgba(0, 0, 0, 0.4);
|
||||
|
||||
@@ -24,8 +24,18 @@ const emit = defineEmits<{
|
||||
<header><h2>Rules</h2></header>
|
||||
<ul>
|
||||
<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 }}
|
||||
<!-- 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
|
||||
retrieval now, so "conditional" stopped distinguishing anything.
|
||||
A missing trigger does — it means nothing can retrieve this. -->
|
||||
@@ -70,6 +80,12 @@ li {
|
||||
}
|
||||
li:hover { background: var(--fs-surface-hover); }
|
||||
.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; }
|
||||
.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; }
|
||||
|
||||
@@ -3,8 +3,17 @@ import { ref } from "vue";
|
||||
import { useRulebooksStore } from "@/stores/rulebooks";
|
||||
import type { Rulebook } from "@/api/rulebooks";
|
||||
|
||||
defineProps<{ rulebooks: Rulebook[]; selectedId: number | null; sweepActive: boolean }>();
|
||||
const emit = defineEmits<{ select: [id: number]; "select-sweep": [] }>();
|
||||
defineProps<{
|
||||
rulebooks: Rulebook[];
|
||||
selectedId: number | null;
|
||||
sweepActive: boolean;
|
||||
driftActive: boolean;
|
||||
}>();
|
||||
const emit = defineEmits<{
|
||||
select: [id: number];
|
||||
"select-sweep": [];
|
||||
"select-drift": [];
|
||||
}>();
|
||||
|
||||
const store = useRulebooksStore();
|
||||
const isCreating = ref(false);
|
||||
@@ -44,6 +53,17 @@ async function submitNew() {
|
||||
>
|
||||
Due for verification
|
||||
</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">
|
||||
<button v-if="!isCreating" @click="isCreating = true">+ New rulebook</button>
|
||||
|
||||
@@ -10,6 +10,7 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
|
||||
const rulesByTopic = ref<Record<number, RuleHeader[]>>({});
|
||||
const currentRule = ref<Rule | null>(null);
|
||||
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
|
||||
// looking at — re-fetching unfiltered would silently widen the list under
|
||||
// them at the moment they acted on it.
|
||||
@@ -106,6 +107,10 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
|
||||
title: rule.title,
|
||||
statement: rule.statement,
|
||||
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,
|
||||
when_to_apply: rule.when_to_apply || undefined,
|
||||
arose_from_id: rule.arose_from_id ?? undefined,
|
||||
@@ -202,6 +207,46 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
|
||||
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) {
|
||||
await api.deleteRule(id);
|
||||
if (currentRule.value?.id === id) currentRule.value = null;
|
||||
@@ -211,12 +256,13 @@ export const useRulebooksStore = defineStore("rulebooks", () => {
|
||||
}
|
||||
|
||||
return {
|
||||
rulebooks, topicsByRulebook, rulesByTopic, currentRule, rulesDue, lastSweepOpts, loading,
|
||||
rulebooks, topicsByRulebook, rulesByTopic, currentRule, rulesDue, drift, lastSweepOpts, loading,
|
||||
placeMovedRule,
|
||||
fetchRulebooks, fetchTopics, fetchRules, fetchRule,
|
||||
createRulebook, updateRulebook, deleteRulebook,
|
||||
createTopic, updateTopic, deleteTopic,
|
||||
createRule, updateRule, deleteRule, relateRules, unrelateRules,
|
||||
fetchRulesDue, verifyRule,
|
||||
fetchDrift, restoreVersion,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -7,6 +7,7 @@ import RulebookDetailPane from "@/components/rules/RulebookDetailPane.vue";
|
||||
import RuleListPane from "@/components/rules/RuleListPane.vue";
|
||||
import RuleEditorSlideOver from "@/components/rules/RuleEditorSlideOver.vue";
|
||||
import RuleSweepPane from "@/components/rules/RuleSweepPane.vue";
|
||||
import PreferenceDriftPane from "@/components/rules/PreferenceDriftPane.vue";
|
||||
|
||||
const store = useRulebooksStore();
|
||||
const route = useRoute();
|
||||
@@ -17,6 +18,7 @@ const selectedTopicId = ref<number | null>(null);
|
||||
const editingRuleId = ref<number | null>(null);
|
||||
const creatingRuleForTopic = ref<number | null>(null);
|
||||
const sweepActive = ref(false);
|
||||
const driftActive = ref(false);
|
||||
|
||||
function syncFromRoute() {
|
||||
const rb = route.query.rb ? Number(route.query.rb) : null;
|
||||
@@ -26,19 +28,23 @@ function syncFromRoute() {
|
||||
selectedTopicId.value = topic;
|
||||
editingRuleId.value = rule;
|
||||
sweepActive.value = route.query.view === "due";
|
||||
driftActive.value = route.query.view === "drift";
|
||||
}
|
||||
|
||||
function selectSweep() {
|
||||
sweepActive.value = true;
|
||||
// Keeps ?rule=… so the editor survives the mode switch, and drops the
|
||||
// rulebook/topic selection the sweep does not use.
|
||||
/** The two cross-cutting views share one `view` query key, so entering either
|
||||
* leaves the other — and keeps `?rule=…` so an open editor survives the
|
||||
* switch, the way the sweep already did. */
|
||||
function selectCrossCutting(view: "due" | "drift") {
|
||||
sweepActive.value = view === "due";
|
||||
driftActive.value = view === "drift";
|
||||
const { rb, topic, ...rest } = route.query;
|
||||
void rb; void topic;
|
||||
router.replace({ query: { ...rest, view: "due" } });
|
||||
router.replace({ query: { ...rest, view } });
|
||||
}
|
||||
|
||||
function selectRulebook(id: number) {
|
||||
sweepActive.value = false;
|
||||
driftActive.value = false;
|
||||
selectedRulebookId.value = id;
|
||||
selectedTopicId.value = null;
|
||||
router.replace({ query: { rb: String(id) } });
|
||||
@@ -84,10 +90,13 @@ watch(() => route.query, syncFromRoute);
|
||||
:rulebooks="store.rulebooks"
|
||||
:selected-id="selectedRulebookId"
|
||||
:sweep-active="sweepActive"
|
||||
:drift-active="driftActive"
|
||||
@select="selectRulebook"
|
||||
@select-sweep="selectSweep"
|
||||
@select-sweep="selectCrossCutting('due')"
|
||||
@select-drift="selectCrossCutting('drift')"
|
||||
/>
|
||||
<RuleSweepPane v-if="sweepActive" class="sweep-span" @open-rule="openRule" />
|
||||
<PreferenceDriftPane v-else-if="driftActive" class="sweep-span" @open-rule="openRule" />
|
||||
<RulebookDetailPane
|
||||
v-else-if="selectedRulebookId !== null"
|
||||
:rulebook-id="selectedRulebookId"
|
||||
@@ -99,13 +108,13 @@ watch(() => route.query, syncFromRoute);
|
||||
<p>Select a rulebook to view its topics.</p>
|
||||
</div>
|
||||
<RuleListPane
|
||||
v-if="!sweepActive && selectedTopicId !== null"
|
||||
v-if="!sweepActive && !driftActive && selectedTopicId !== null"
|
||||
:topic-id="selectedTopicId"
|
||||
:rules="store.rulesByTopic[selectedTopicId] || []"
|
||||
@open-rule="openRule"
|
||||
@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>
|
||||
</div>
|
||||
<RuleEditorSlideOver
|
||||
@@ -125,8 +134,9 @@ watch(() => route.query, syncFromRoute);
|
||||
gap: 1px;
|
||||
background: var(--fs-border-color);
|
||||
}
|
||||
/* The sweep is cross-cutting, so it takes the width the rulebook + topic
|
||||
panes would have used rather than being squeezed into one column. */
|
||||
/* A cross-cutting pane — the sweep, or recent changes — takes the width the
|
||||
rulebook + topic panes would have used rather than being squeezed into one
|
||||
column. */
|
||||
.sweep-span { grid-column: 2 / -1; }
|
||||
.pane.empty {
|
||||
background: var(--fs-surface-hover);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "scribe",
|
||||
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
||||
"version": "2026.09.18.0436",
|
||||
"version": "2026.09.18.1606",
|
||||
"author": {
|
||||
"name": "Bryan Van Deusen"
|
||||
},
|
||||
|
||||
@@ -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
|
||||
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
|
||||
sections are what lets the operator find things at a glance, so keep them even
|
||||
when one is short — "**Needs you:** nothing" is an answer they were looking for.
|
||||
Pick the kind of reply first (the tables below). Its sections are **what to
|
||||
consider including, not a form to complete.**
|
||||
|
||||
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
|
||||
|
||||
@@ -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.
|
||||
- **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").
|
||||
- **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
|
||||
|
||||
@@ -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
|
||||
step). Use those values as they came back.
|
||||
- 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
|
||||
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
|
||||
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.
|
||||
|
||||
**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
|
||||
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
|
||||
happens next** without asking a follow-up? If not, the sections are what's
|
||||
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.
|
||||
|
||||
@@ -24,9 +24,14 @@ async def list_milestones(project_id: int) -> dict:
|
||||
"""List milestones for a Scribe project, ordered by order_index.
|
||||
|
||||
Returns every milestone, done ones included: id, title, description,
|
||||
status (active/done), order_index and progress (total, completed, pct,
|
||||
status_counts). The plan itself is not listed: get_milestone(id) returns a
|
||||
milestone's body and its steps.
|
||||
status (active/done), order_index, progress (total, completed, pct,
|
||||
status_counts) and `next_step`. The plan itself is not listed:
|
||||
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()
|
||||
rows = await milestones_svc.get_project_milestone_summary(uid, project_id)
|
||||
|
||||
@@ -77,9 +77,15 @@ async def enter_project(project_id: int) -> dict:
|
||||
|
||||
`milestone_summary` is the 5 most recently touched milestones, any status,
|
||||
most recent first. Touched counts a step changing, not only the milestone
|
||||
itself. Each carries its description and progress 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.
|
||||
itself. Each carries its description, its progress and its `next_step` —
|
||||
the earliest step still open, {id, title, status}, or null when none is —
|
||||
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,
|
||||
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.
|
||||
|
||||
Returns full project fields, a milestone_summary list (every milestone,
|
||||
with description and progress but no plan body; get_milestone reads a
|
||||
plan), the project's own rules (project_rules), and applicable_rules: the
|
||||
with description, progress and `next_step` — the earliest still-open step,
|
||||
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
|
||||
rule applies too and arrives by retrieval when the work matches it.
|
||||
"""
|
||||
|
||||
@@ -140,6 +140,9 @@ async def search(
|
||||
enter_project) — otherwise this searches across ALL projects and
|
||||
bleeds unrelated work into the result set. 0 = search everything
|
||||
(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
|
||||
subsystem/area — enter_project lists them). Use when investigating
|
||||
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,
|
||||
project_id=project_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
|
||||
# records shared with them one-to-one.
|
||||
scope="read",
|
||||
|
||||
@@ -76,8 +76,27 @@ class Note(Base, TimestampMixin, SoftDeleteMixin):
|
||||
recurrence_next_spawn_at: Mapped[datetime | None] = mapped_column(
|
||||
DateTime(timezone=True), nullable=True
|
||||
)
|
||||
# Note type — 'note' (default) or 'process' (a stored process). Task-ness is
|
||||
# tracked by `status`, not here. (person/place/list entity types removed 2026-07.)
|
||||
# WHAT KIND of record this is, on the note/entity axis. Task-ness is tracked
|
||||
# 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")
|
||||
# Task sub-kind — what KIND of work this is, not how it is going:
|
||||
# work (default) — ships a change
|
||||
|
||||
@@ -258,11 +258,57 @@ async def get_rule_version(rule_id: int, version_id: int):
|
||||
return jsonify(version.to_dict(include_text=True))
|
||||
|
||||
|
||||
# NO restore route, deliberately (milestone 323). A note version can be
|
||||
# restored; a binding instruction should not be revertible in one click.
|
||||
# Putting a rewrite back goes through update_rule, which takes its own
|
||||
# NO restore route FOR A RULE, deliberately (milestone 323). A note version
|
||||
# can be restored; a binding instruction should not be revertible in one
|
||||
# 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
|
||||
# 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")
|
||||
|
||||
@@ -203,6 +203,33 @@ def embedding_text(title: str | None, body: str | None) -> str:
|
||||
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.010–0.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 ------------------------------------
|
||||
#
|
||||
# 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)
|
||||
|
||||
|
||||
# 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
|
||||
# rejected score stays observable (#3670). The qualifying set is provably
|
||||
# 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,
|
||||
task_kind: str | Sequence[str] | None = None,
|
||||
orphan_only: bool = False,
|
||||
include_global_kinds: bool = False,
|
||||
scope: str = "own",
|
||||
demote_superseded: bool = True,
|
||||
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
|
||||
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)
|
||||
decides how far this may see. It exists because this one function serves
|
||||
three different kinds of act: an explicit search, which should reach
|
||||
@@ -604,7 +664,12 @@ async def semantic_search_notes(
|
||||
if orphan_only:
|
||||
stmt = stmt.where(Note.project_id.is_(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
|
||||
# association filter, not a ranking signal — membership in the
|
||||
# candidate set, decided before scoring, like project_id above.
|
||||
@@ -785,7 +850,7 @@ def rule_document(
|
||||
if not trigger:
|
||||
return name or None, body or None
|
||||
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}",
|
||||
)
|
||||
|
||||
|
||||
@@ -292,6 +292,13 @@ _FACETS: dict[str, tuple[bool, str | None]] = {
|
||||
"note": (False, "note"),
|
||||
"process": (False, "process"),
|
||||
"snippet": (False, "snippet"),
|
||||
# A transferable insight, keyed by the situation it applies to rather than
|
||||
# by its topic (milestone 385). It reaches the browse surface through this
|
||||
# one entry: the door's validation, the counts and both dialects of the
|
||||
# type filter are all generated from this table, which is the property
|
||||
# #3161 asked for so that adding a kind is a single edit rather than four
|
||||
# that must agree.
|
||||
"lesson": (False, "lesson"),
|
||||
}
|
||||
|
||||
# The non-task record types, for the counts query. Derived so it cannot drift
|
||||
|
||||
@@ -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)
|
||||
@@ -7,9 +7,16 @@ from sqlalchemy import func, select
|
||||
from scribe.models import async_session
|
||||
from scribe.models.milestone import Milestone
|
||||
from scribe.models.note import Note
|
||||
from scribe.services import access as access_svc
|
||||
|
||||
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:
|
||||
"""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(
|
||||
user_id: int, project_ids: list[int]
|
||||
) -> 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
|
||||
project's milestones, then one more per milestone for its progress. Called
|
||||
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
|
||||
(#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:
|
||||
return {}
|
||||
@@ -261,16 +276,24 @@ async def get_project_milestone_summaries(
|
||||
|
||||
counts: dict[int, dict[str, int]] = {}
|
||||
step_touched: dict[int, datetime] = {}
|
||||
next_step: dict[int, dict] = {}
|
||||
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(
|
||||
select(
|
||||
Note.milestone_id, Note.status, func.count(Note.id),
|
||||
func.max(Note.updated_at),
|
||||
)
|
||||
.where(
|
||||
Note.milestone_id.in_([m.id for m in milestones]),
|
||||
Note.milestone_id.in_(milestone_ids),
|
||||
Note.status.isnot(None),
|
||||
Note.deleted_at.is_(None),
|
||||
readable,
|
||||
)
|
||||
.group_by(Note.milestone_id, Note.status)
|
||||
)
|
||||
@@ -280,6 +303,28 @@ async def get_project_milestone_summaries(
|
||||
or latest > step_touched[milestone_id]):
|
||||
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}
|
||||
for m in milestones:
|
||||
entry = m.to_dict()
|
||||
@@ -289,6 +334,12 @@ async def get_project_milestone_summaries(
|
||||
# written (#4045). Touched is the later of the two.
|
||||
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
|
||||
# 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)
|
||||
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, [])
|
||||
|
||||
|
||||
# What a milestone LISTING needs: enough to say what each plan is and how far
|
||||
# along it is. The plan itself (`body`) is get_milestone's job. Summaries once
|
||||
# carried it, and on a project with 39 milestones enter_project came to ~222k
|
||||
# characters, 168k of them plan bodies. That is past what an MCP client will
|
||||
# What a milestone LISTING needs: enough to say what each plan is, how far
|
||||
# along it is, and which step is next. The plan itself (`body`) is
|
||||
# get_milestone's job. Summaries once carried it, and on a project with 39
|
||||
# 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
|
||||
# through (#4045). user_id / project_id / timestamps repeat what the caller
|
||||
# already knows.
|
||||
_BRIEF_FIELDS = (
|
||||
"id", "title", "description", "status", "order_index",
|
||||
"total", "completed", "pct", "status_counts",
|
||||
"total", "completed", "pct", "status_counts", "next_step",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -54,9 +54,11 @@ from scribe.models.milestone import Milestone
|
||||
from scribe.models.note import Note
|
||||
from scribe.models.project import Project
|
||||
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__)
|
||||
|
||||
|
||||
@@ -640,7 +640,7 @@ async def get_autoinject_config(user_id: int) -> dict:
|
||||
|
||||
|
||||
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,
|
||||
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
|
||||
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:
|
||||
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"
|
||||
|
||||
|
||||
|
||||
@@ -11,9 +11,10 @@ import logging
|
||||
from collections.abc import Iterable
|
||||
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.base import iso
|
||||
from scribe.models.system import System
|
||||
from scribe.models.rulebook import Rulebook
|
||||
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()
|
||||
|
||||
|
||||
# ── 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) ───────────────────────────
|
||||
|
||||
async def set_rule_systems(
|
||||
|
||||
@@ -55,10 +55,15 @@ UNSET: object = object()
|
||||
# --- serialize: structured fields -> note (title/body/tags) ------------------
|
||||
|
||||
def compose_title(name: str, when_to_use: str = "") -> str:
|
||||
"""`name — when to use` (or just `name` when no usage note is given)."""
|
||||
name = (name or "").strip()
|
||||
when = (when_to_use or "").strip()
|
||||
return f"{name} — {when}" if when else name
|
||||
"""`name — when to use` (or just `name` when no usage note is given).
|
||||
|
||||
The join itself lives in `embeddings.trigger_title`, which rules and
|
||||
lessons build their titles from too. Kept as a named function here because
|
||||
it is this module's public vocabulary and callers say `compose_title`.
|
||||
"""
|
||||
from scribe.services.embeddings import trigger_title
|
||||
|
||||
return trigger_title(name, when_to_use)
|
||||
|
||||
|
||||
def compose_tags(language: str = "", tags: list[str] | None = None) -> list[str]:
|
||||
|
||||
+11
-2
@@ -147,11 +147,20 @@ def _now():
|
||||
def fake_note(**attrs) -> MagicMock:
|
||||
"""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 /
|
||||
task_kind / note_type for its kind marker, user_id for the "shared by …"
|
||||
attribution, data for a snippet's language, deleted_at for trash."""
|
||||
task_kind / note_type / status for its kind marker, user_id for the
|
||||
"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({
|
||||
"id": 1, "title": "t", "body": "", "tags": [], "user_id": 7,
|
||||
"note_type": "note", "is_task": False, "task_kind": "work",
|
||||
"status": "todo" if is_task else None,
|
||||
"data": None, "deleted_at": None,
|
||||
# Milestone 317: a truthy mock here reads as "this note carries a
|
||||
# check", which trips the guard on records that may not have one.
|
||||
|
||||
@@ -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
|
||||
@@ -170,6 +170,24 @@ TOPICS: tuple[Topic, ...] = (
|
||||
Topic("the operator's own reply shapes come first", "skill:reporting-back",
|
||||
("reply_preferences", 'content_type="rule"'),
|
||||
"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 ──
|
||||
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"),
|
||||
|
||||
@@ -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
|
||||
@@ -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()
|
||||
@@ -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
|
||||
@@ -38,6 +38,7 @@ ROWS = {
|
||||
"plain note": fake_note(note_type="note"),
|
||||
"process": fake_note(note_type="process"),
|
||||
"snippet": fake_snippet(),
|
||||
"lesson": fake_note(note_type="lesson"),
|
||||
"work task": fake_task(task_kind="work", note_type="note"),
|
||||
"issue": fake_task(task_kind="issue", 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"}),
|
||||
("process", {"process"}),
|
||||
("snippet", {"snippet"}),
|
||||
("lesson", {"lesson"}),
|
||||
("", 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():
|
||||
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))
|
||||
|
||||
@@ -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.010–0.023 for everything else), gets there the other way: its STORED
|
||||
title is already the join and its stored body already opens with the trigger,
|
||||
so the ordinary `title\\nbody` join is the sharp document. A lesson follows the
|
||||
snippet, which is what step 1 decided and step 2 built.
|
||||
|
||||
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),
|
||||
)
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
|
||||
@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
|
||||
async def test_fable_search_limit_is_clamped():
|
||||
_user_id_ctx.set(7)
|
||||
|
||||
@@ -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,
|
||||
"total": 4, "completed": 2, "pct": 50.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",
|
||||
"order_index": 1, "total": 4, "completed": 2, "pct": 50.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"},
|
||||
}]
|
||||
|
||||
|
||||
|
||||
@@ -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"
|
||||
@@ -148,8 +148,10 @@ async def test_injected_menu_labels_the_record_kind():
|
||||
hits = [
|
||||
(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.90, fake_note(id=3, title="Auth token expiry", is_task=True, task_kind="issue")),
|
||||
(0.89, fake_note(id=4, title="Ship the drafter", is_task=True)),
|
||||
(0.90, fake_note(id=3, title="Auth token expiry", 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")),
|
||||
]
|
||||
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 "[process]" in by_id[2]
|
||||
# Task-ness wins over note_type, and an issue says so rather than "task".
|
||||
assert "[issue]" in by_id[3]
|
||||
assert "[task]" in by_id[4]
|
||||
# A task also carries its status (#4154): #4 is DONE, and a line that said
|
||||
# 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]
|
||||
# Still title-first: the marker is metadata, not an excuse to carry bodies.
|
||||
assert "body" not in out["context"]
|
||||
|
||||
@@ -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():
|
||||
counts, _ = await _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["total"] == 1
|
||||
|
||||
|
||||
@@ -487,7 +487,9 @@ async def test_write_path_labels_a_non_snippet_hit_with_its_kind():
|
||||
)
|
||||
|
||||
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
|
||||
# The header now names the right opener for each kind.
|
||||
assert "get_task(id)" in ctx and "get_snippet(id)" in ctx
|
||||
|
||||
Reference in New Issue
Block a user