Two milestones: a note can carry its own check (317), and a rule keeps what it used to say (323) #135

Merged
bvandeusen merged 23 commits from dev into main 2026-08-31 00:01:15 -04:00
78 changed files with 5269 additions and 371 deletions
@@ -0,0 +1,80 @@
"""a note can carry its own check — verify_with, expires_when, verified_at
(milestone 317 step 1)
Revision ID: 0092
Revises: 0091
Create Date: 2026-08-28
The sibling of 0090, which gave rules the same three columns. Same
distinction, one table over:
A NORM is a decision — no truth value, and it changes only when its author
changes it, which they know they did. A CONSTRAINT is a fact about someone
else's software, and nobody is present when it goes false.
Notes hold far more constraints than rules do, and hold them for longer. A
cross-project reference note asserting what a signing service does on a
duplicate upload, or how a forge numbers its CI runs, is believed by every
project that reads it, and there is nothing in the record that says when
anyone last looked. `note_supersessions` only fires once a human has read
the note, disagreed, and written the correction — which is the case where
the note was already believed.
Three nullable columns:
- `verify_with` — how to tell whether this is still true. A command, a path,
a URL, a query. Prose is allowed; something runnable is better.
- `expires_when` — the STATE under which it stops being true. Deliberately
not a date: constraints do not expire on a schedule, they expire when the
world underneath them moves.
- `verified_at` — when the check last passed. NULL means never checked, and
sorts FIRST in the sweep: unexamined outranks examined-long-ago.
WHICH ROWS THESE ARE FOR. `notes` is one table holding notes, tasks,
snippets and processes, so these columns land on all of them. Only non-task,
non-snippet records are OFFERED them (milestone 317 decisions 1 and 2, gated
at the service in step 2): a task's decay is its status, and a snippet
already carries a richer, location-aware verdict in `data.verification`. The
columns exist on the other rows and stay null there; a gate that lives in
the schema would have meant a partial index or a CHECK across three columns
to express something the write path can say in two lines.
All three optional, because most notes should set none of them — the whole
value of the sweep is that its output is short. A null `verify_with` is not
an omission; it is the honest marker of "this one is a decision, and there
is nothing to go and check."
No CHECK constraint is involved, so rule 36 does not apply. Nothing is
backfilled: a migration cannot invent a check.
"""
import sqlalchemy as sa
from alembic import op
revision = "0092"
down_revision = "0091"
branch_labels = None
depends_on = None
def upgrade() -> None:
op.add_column("notes", sa.Column("verify_with", sa.Text(), nullable=True))
op.add_column("notes", sa.Column("expires_when", sa.Text(), nullable=True))
op.add_column(
"notes",
sa.Column("verified_at", sa.DateTime(timezone=True), nullable=True),
)
# No index, for 0090's reason — the sweep runs when a human asks, never on
# a request path — but the margin is thinner here and worth naming. `rules`
# is hundreds of rows; `notes` is thousands and grows with every session.
#
# Still a sequential scan's job at this size, and an index on
# (verified_at) filtered to `verify_with IS NOT NULL` would be maintained
# on every note write to serve one operator-initiated query. If step 3's
# live acceptance measures otherwise, add it there against a real plan
# rather than guessing here.
def downgrade() -> None:
op.drop_column("notes", "verified_at")
op.drop_column("notes", "expires_when")
op.drop_column("notes", "verify_with")
+83
View File
@@ -0,0 +1,83 @@
"""rules gain an edit history — rule_versions (milestone 323 step 1)
Revision ID: 0093
Revises: 0092
Create Date: 2026-08-29
The sibling `note_versions` has had for a long time. A note's every meaningful
edit is snapshotted, and the design-system note calls that history "the
changelog". A RULE — which binds behaviour on every session that loads it —
had nothing: an edit destroyed what it used to say, with no record anywhere.
Rescoping rule 79 on 2026-08-29 is what surfaced it. The superseded statement
had to be hand-copied into a task log to survive the edit (#3237), which is
not a process, it is a person remembering. The more consequential record had
the weaker protection.
Three things are deliberately NOT copied from note_versions, and each is a
guard that exists there for a reason that does not hold here:
- **No pruning, and no MAX_VERSIONS.** That cap defends against note autosave
filling every slot. Rules have no autosave; every edit is a deliberate
update_rule. A rule is edited a handful of times in its life, and capping
invites losing the one edit somebody needed.
- **No pin columns.** `pin_kind`/`pin_label` exist so a note's version can
survive that pruning. With nothing pruning, a pin protects a row that was
never at risk.
- **No minimum interval.** 300 seconds between snapshots is also an autosave
defence; here it would only ever discard a second deliberate edit.
`user_id` is the ACTOR rather than the owner, and is SET NULL rather than
CASCADE: deleting a user must not erase the history of the rules they edited.
The edit still happened and the rule still binds because of it.
No CHECK constraint, so rule 36 does not apply. Nothing is backfilled — a
migration cannot invent the text a rule used to have, and inventing "the
current text, as of now" would be worse than an empty history, because it
would look like a record of an edit that never occurred.
"""
import sqlalchemy as sa
from alembic import op
revision = "0093"
down_revision = "0092"
branch_labels = None
depends_on = None
def upgrade() -> None:
op.create_table(
"rule_versions",
sa.Column("id", sa.BigInteger(), primary_key=True),
sa.Column(
"rule_id",
sa.BigInteger(),
sa.ForeignKey("rules.id", ondelete="CASCADE"),
nullable=False,
),
sa.Column(
"user_id",
sa.BigInteger(),
sa.ForeignKey("users.id", ondelete="SET NULL"),
nullable=True,
),
sa.Column("title", sa.Text(), nullable=False, server_default=""),
sa.Column("statement", sa.Text(), nullable=False, server_default=""),
sa.Column("why", sa.Text(), nullable=True),
sa.Column("how_to_apply", sa.Text(), nullable=True),
sa.Column("when_to_apply", sa.Text(), nullable=True),
sa.Column("tier", sa.Text(), nullable=True),
sa.Column("verify_with", sa.Text(), nullable=True),
sa.Column("expires_when", sa.Text(), nullable=True),
sa.Column("created_at", sa.DateTime(timezone=True), nullable=False),
)
# The only query this table serves is "the history of THIS rule, newest
# first" — unlike 0092's columns, which are read by an operator-initiated
# sweep over the whole set. Every read here is keyed on rule_id, so the
# index earns its write cost immediately rather than on a hunch.
op.create_index("ix_rule_versions_rule_id", "rule_versions", ["rule_id"])
def downgrade() -> None:
op.drop_index("ix_rule_versions_rule_id", table_name="rule_versions")
op.drop_table("rule_versions")
+43
View File
@@ -228,6 +228,49 @@ export async function unrelateRules(relationId: number): Promise<void> {
return apiDelete(`/api/rule-relations/${relationId}`);
}
/**
* One entry in a rule's edit history.
*
* Each entry holds the text the edit REPLACED, not the text it introduced —
* so the newest entry is what the rule said before its most recent change,
* and what that change produced is the rule as it stands now. Read the other
* way round, every diff comes out backwards.
*
* The listing form omits the long fields; open one to get them.
*/
export interface RuleVersion {
id: number;
rule_id: number;
/** Who made the edit. Null when that account has since been deleted. */
user_id: number | null;
title: string;
created_at: string;
statement?: string;
why?: string;
how_to_apply?: string;
when_to_apply?: string;
tier?: string;
verify_with?: string;
expires_when?: string;
}
export async function listRuleVersions(ruleId: number): Promise<RuleVersion[]> {
const data = await apiGet<{ versions: RuleVersion[] }>(
`/api/rules/${ruleId}/versions`,
);
return data.versions;
}
export async function getRuleVersion(
ruleId: number, versionId: number,
): Promise<RuleVersion> {
return apiGet<RuleVersion>(`/api/rules/${ruleId}/versions/${versionId}`);
}
// No restoreRuleVersion, 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.
export async function deleteRule(id: number): Promise<void> {
return apiDelete(`/api/rules/${id}`);
}
+3 -3
View File
@@ -127,7 +127,7 @@
border: 1px solid var(--fs-error);
border-radius: var(--fs-radius-sm);
font-size: 0.85rem;
color: var(--fs-error);
color: var(--fs-error-fg);
}
.diff-view {
border: 1px solid var(--fs-border-color);
@@ -147,11 +147,11 @@
}
.diff-delete {
background: color-mix(in srgb, var(--fs-error) 12%, transparent);
color: var(--fs-error);
color: var(--fs-error-fg);
}
.diff-insert {
background: color-mix(in srgb, var(--fs-success) 12%, transparent);
color: var(--fs-success);
color: var(--fs-success-fg);
}
.diff-equal {
color: var(--fs-text-tertiary);
+6
View File
@@ -31,6 +31,7 @@
--fs-accent-faint: color-mix(in srgb, var(--fs-accent) 8%, transparent); /* The faintest accent wash */
--fs-accent-deep: color-mix(in srgb, var(--fs-accent) 70%, black); /* The accent, darkened */
--fs-accent-wash: color-mix(in srgb, var(--fs-accent) 22%, transparent); /* Heaviest accent tint */
--fs-accent-fg: color-mix(in srgb, var(--fs-accent) 45%, var(--fs-text-primary)); /* Accent TEXT on an accent tint */
--fs-gradient-cta: linear-gradient(135deg, var(--fs-accent), var(--fs-accent-deep));
--fs-glow-cta: 0 2px 10px color-mix(in srgb, var(--fs-accent) 35%, transparent);
--fs-glow-cta-hover: 0 4px 24px color-mix(in srgb, var(--fs-accent) 65%, transparent);
@@ -102,8 +103,11 @@
/* semantic */
--fs-success: var(--fs-action-primary);
--fs-success-fg: color-mix(in srgb, var(--fs-success) 45%, var(--fs-text-primary)); /* Success TEXT on a success tint */
--fs-warning: #8B6F1E;
--fs-warning-fg: color-mix(in srgb, var(--fs-warning) 50%, var(--fs-text-primary)); /* Warning TEXT on a warning tint */
--fs-error: #C04A1F;
--fs-error-fg: color-mix(in srgb, var(--fs-error) 50%, var(--fs-text-primary)); /* Error TEXT on an error tint */
--fs-info: #3D5A6E;
--fs-destructive: #6B2118; /* irreversible — deliberately not the error colour */
@@ -148,7 +152,9 @@
/* text */
--fs-text-primary: #E8E4D8; /* body, headings, labels — inverts by mode */
--fs-text-secondary: #C2BFB4;
--fs-text-secondary-fg: color-mix(in srgb, var(--fs-text-secondary) 90%, var(--fs-text-primary)); /* Secondary TEXT on a secondary tint (barely moves; no exceptions) */
--fs-text-tertiary: #9C9A92;
--fs-text-tertiary-fg: color-mix(in srgb, var(--fs-text-tertiary) 55%, var(--fs-text-primary)); /* Tertiary TEXT on a tertiary tint */
--fs-text-on-action: #E8E4D8; /* text on a filled colour — NOT mode-dependent */
/* type */
+1 -1
View File
@@ -25,7 +25,7 @@
border-color: var(--fs-accent);
}
.ctx-crumb-project {
color: var(--fs-accent);
color: var(--fs-accent-fg);
background: color-mix(in srgb, var(--fs-accent) 10%, transparent);
border: 1px solid color-mix(in srgb, var(--fs-accent) 30%, transparent);
text-decoration: none;
+2 -2
View File
@@ -206,7 +206,7 @@ router.afterEach(() => {
background: var(--fs-accent-soft);
}
.nav-link.router-link-active {
color: var(--fs-accent);
color: var(--fs-accent-fg);
font-weight: 500;
background: color-mix(in srgb, var(--fs-accent) 25%, transparent);
box-shadow: 0 0 16px color-mix(in srgb, var(--fs-accent) 30%, transparent);
@@ -257,7 +257,7 @@ router.afterEach(() => {
font-weight: 500;
text-transform: uppercase;
letter-spacing: 0.05em;
color: var(--fs-accent);
color: var(--fs-accent-fg);
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
padding: 0.1rem 0.35rem;
border-radius: var(--fs-radius-sm);
+2 -2
View File
@@ -137,12 +137,12 @@ function markerFor(type: DiffLine['type']): string {
.diff-delete {
background: color-mix(in srgb, var(--fs-error) 12%, transparent);
color: var(--fs-error);
color: var(--fs-error-fg);
}
.diff-insert {
background: color-mix(in srgb, var(--fs-success) 12%, transparent);
color: var(--fs-success);
color: var(--fs-success-fg);
}
.diff-equal {
+2 -22
View File
@@ -2,7 +2,7 @@
import { ref, computed, onMounted } from "vue";
import { apiGet, pinNoteVersion, unpinNoteVersion } from "@/api/client";
import DiffView from "@/components/DiffView.vue";
import type { DiffLine } from "@/composables/useAssist";
import { computeDiff, type DiffLine } from "@/utils/diff";
import { fmtStamp } from "@/utils/dateFormat";
interface NoteVersion {
@@ -33,28 +33,8 @@ const loadingDetail = ref(false);
const diff = computed<DiffLine[]>(() => {
if (!selectedVersion.value?.body) return [];
const a = props.currentBody;
const b = selectedVersion.value.body;
const aLines = a.split('\n');
const bLines = b.split('\n');
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i+1][j+1] + 1
: Math.max(dp[i+1][j], dp[i][j+1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: 'equal', text: aLines[i++] }); j++; }
else if (dp[i+1][j] >= dp[i][j+1]) result.push({ type: 'delete', text: aLines[i++] });
else result.push({ type: 'insert', text: bLines[j++] });
}
while (i < m) result.push({ type: 'delete', text: aLines[i++] });
while (j < n) result.push({ type: 'insert', text: bLines[j++] });
return result;
return computeDiff(props.currentBody, selectedVersion.value.body);
});
async function loadVersions() {
@@ -227,11 +227,11 @@ const markers: Record<DiffLine["type"], string> = {
.iap-diff-equal { color: var(--fs-text-tertiary); }
.iap-diff-delete {
background: color-mix(in srgb, var(--fs-error) 10%, transparent);
color: var(--fs-error);
color: var(--fs-error-fg);
}
.iap-diff-insert {
background: color-mix(in srgb, var(--fs-success) 10%, transparent);
color: var(--fs-success);
color: var(--fs-success-fg);
}
.iap-diff-marker {
+1 -1
View File
@@ -156,7 +156,7 @@ const groups = [
.md-btn.active {
background: color-mix(in srgb, var(--fs-accent) 14%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
box-shadow: 0 0 0 1px color-mix(in srgb, var(--fs-accent) 35%, transparent);
}
+198
View File
@@ -0,0 +1,198 @@
<script setup lang="ts">
/**
* The staleness sweep for NOTES: notes that assert a fact, oldest first.
*
* Sibling of RuleSweepPane, not a shared component — the two read differently
* enough that merging them would mean a prop for every difference (a rule has
* a tier and a statement; a note has a project and opens at a route). What
* they share is the SHAPE of the judgement, and that is worth copying
* deliberately rather than abstracting: the ordering carries urgency, "never"
* is categorically different from a date, and a failed check writes nothing.
*
* Lives in the Knowledge view rather than beside the rules sweep (operator's
* call, milestone 317 step 4): notes stay where notes live. The cost, accepted
* knowingly, is that there is no single screen showing every record anyone has
* left unconfirmed — /rules keeps its own.
*/
import { onMounted, ref } from "vue";
import { apiGet, apiPost } from "@/api/client";
import { useToastStore } from "@/stores/toast";
interface DueNote {
id: number;
title: string;
project_id: number | null;
verify_with: string;
expires_when: string;
last_verified: string | null;
days_since_verified: number | null;
}
const emit = defineEmits<{ "open-note": [id: number] }>();
const toast = useToastStore();
const rows = ref<DueNote[]>([]);
const loading = ref(false);
const neverOnly = ref(false);
const busyId = ref<number | null>(null);
async function reload() {
loading.value = true;
try {
const p = new URLSearchParams();
if (neverOnly.value) p.set("never_only", "1");
const data = await apiGet<{ notes: DueNote[] }>(
`/api/notes/due-for-verification?${p}`,
);
rows.value = data.notes;
} catch {
toast.show("Could not load the sweep", "error");
} finally {
loading.value = false;
}
}
async function verify(id: number, stillTrue: boolean) {
busyId.value = id;
try {
await apiPost(`/api/notes/${id}/verify`, { still_true: stillTrue });
if (stillTrue) {
// It has been confirmed, so it leaves the list — the sweep shows what
// still needs looking at, and leaving it in place would invite a second
// stamp nobody earned.
rows.value = rows.value.filter((r) => r.id !== id);
toast.show("Recorded — checked today");
} else {
// It stays. A failed check writes nothing on purpose: the note is wrong
// rather than in a state worth recording, so it keeps its place until
// someone corrects, supersedes, or unhooks it.
toast.show("Recorded as no longer true — the note keeps its place here");
}
} catch {
toast.show("Could not record that", "error");
} finally {
busyId.value = null;
}
}
onMounted(reload);
defineExpose({ reload });
</script>
<template>
<section class="sweep">
<header>
<h2>Due for verification</h2>
<p class="lede">
Notes that assert a fact about something outside your control what a
service does, how a tool behaves. Most notes are decisions and never
appear here; they have no truth value to go stale.
</p>
</header>
<div class="filters">
<label class="filter">
<input v-model="neverOnly" type="checkbox" @change="reload" />
<span>Never checked only</span>
</label>
</div>
<p v-if="loading" class="state">Loading</p>
<!-- An empty sweep is GOOD NEWS and must not read like a broken page. -->
<p v-else-if="!rows.length" class="state empty">
Nothing to check.
{{ neverOnly
? "Every note that carries a check has been confirmed at least once."
: "No note carries a check yet add one to a note that asserts a fact." }}
</p>
<ol v-else class="rows">
<li v-for="n in rows" :key="n.id" class="row">
<div class="row-head">
<button class="row-title" @click="emit('open-note', n.id)">{{ n.title }}</button>
<span class="age" :class="{ unchecked: n.days_since_verified === null }">
{{ n.days_since_verified === null
? "never checked"
: `${n.days_since_verified}d ago` }}
</span>
</div>
<dl class="check">
<dt>Check</dt>
<dd>{{ n.verify_with }}</dd>
<template v-if="n.expires_when">
<dt>Ends when</dt>
<dd>{{ n.expires_when }}</dd>
</template>
</dl>
<div class="actions">
<button :disabled="busyId === n.id" @click="verify(n.id, true)">Still true</button>
<button :disabled="busyId === n.id" @click="verify(n.id, false)">No longer true</button>
</div>
</li>
</ol>
<p v-if="rows.length" class="footnote">
Record a result only after actually running the check. No longer true stores nothing
on purpose the note is wrong rather than in a state worth recording, so it keeps its
place here until you correct it, supersede it, or remove its check.
</p>
</section>
</template>
<style scoped>
.sweep { display: flex; flex-direction: column; gap: var(--fs-space-3); }
h2 { margin: 0; font-size: 1.05rem; }
.lede {
margin: 0.35rem 0 0;
max-width: 62ch;
font-size: 0.85rem;
color: var(--fs-text-secondary);
line-height: 1.5;
}
.filters { display: flex; gap: var(--fs-space-5); align-items: center; flex-wrap: wrap; }
.filter { display: flex; align-items: center; gap: var(--fs-space-2); font-size: 0.82rem; color: var(--fs-text-secondary); }
.filter input[type="checkbox"] { accent-color: var(--fs-accent); }
.state { margin: 0; font-size: 0.9rem; 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; }
/* The ORDER carries urgency — the top of this list is the least-confirmed
thing in the corpus. No red/amber ramp: it would restate the ordering and
force an invented "stale after N days" threshold. "Never" is marked because
it is categorically DIFFERENT from a date, not a worse one. */
.age { margin-left: auto; font-size: 0.78rem; color: var(--fs-text-secondary); font-variant-numeric: tabular-nums; }
.age.unchecked { font-style: italic; color: var(--fs-text-tertiary); }
.check { display: grid; grid-template-columns: auto 1fr; gap: 0.15rem var(--fs-space-3); margin: var(--fs-space-3) 0 0; }
.check dt { font-size: 0.7rem; text-transform: uppercase; letter-spacing: 0.05em; color: var(--fs-text-tertiary); }
.check dd { margin: 0; font-size: 0.82rem; color: var(--fs-text-primary); min-width: 0; overflow-wrap: anywhere; }
.actions { display: flex; gap: var(--fs-space-2); margin-top: var(--fs-space-3); }
.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; }
.footnote { margin: 0; max-width: 62ch; font-size: 0.78rem; color: var(--fs-text-tertiary); line-height: 1.45; }
</style>
+2 -2
View File
@@ -679,7 +679,7 @@ async function confirmDelete() {
font-weight: 500;
background: color-mix(in srgb, var(--fs-accent) 12%, transparent);
border: 1px solid color-mix(in srgb, var(--fs-accent) 30%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
border-radius: 999px;
padding: 0.05rem 0.45rem;
flex-shrink: 0;
@@ -689,7 +689,7 @@ async function confirmDelete() {
font-weight: 500;
text-transform: uppercase;
letter-spacing: 0.04em;
color: var(--fs-text-tertiary);
color: var(--fs-text-tertiary-fg);
background: color-mix(in srgb, var(--fs-text-tertiary) 12%, transparent);
border-radius: 999px;
padding: 0.05rem 0.45rem;
+1 -1
View File
@@ -168,7 +168,7 @@ function focusInput() {
border-radius: 999px;
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
border: 1px solid var(--fs-accent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
font-size: 0.8rem;
white-space: nowrap;
}
@@ -2,7 +2,7 @@
import { ref, computed } from "vue";
import { apiGet } from "@/api/client";
import DiffView from "@/components/DiffView.vue";
import type { DiffLine } from "@/composables/useAssist";
import { computeDiff, type DiffLine } from "@/utils/diff";
interface NoteVersion {
id: number;
@@ -31,25 +31,7 @@ const loadingDetail = ref(false);
const diff = computed<DiffLine[]>(() => {
if (!selectedVersion.value?.body) return [];
const aLines = props.currentBody.split("\n");
const bLines = selectedVersion.value.body.split("\n");
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i + 1][j + 1] + 1
: Math.max(dp[i + 1][j], dp[i][j + 1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: "equal", text: aLines[i++] }); j++; }
else if (dp[i + 1][j] >= dp[i][j + 1]) result.push({ type: "delete", text: aLines[i++] });
else result.push({ type: "insert", text: bLines[j++] });
}
while (i < m) result.push({ type: "delete", text: aLines[i++] });
while (j < n) result.push({ type: "insert", text: bLines[j++] });
return result;
return computeDiff(props.currentBody, selectedVersion.value.body);
});
function formatDate(iso: string): string {
@@ -548,7 +548,7 @@ defineExpose({ reload: loadProjectNotes });
.note-tag-pill {
font-size: 0.58rem;
color: var(--fs-accent);
color: var(--fs-accent-fg);
background: color-mix(in srgb, var(--fs-accent) 10%, transparent);
border-radius: 999px;
padding: 0 0.3rem;
@@ -645,7 +645,7 @@ defineExpose({ reload: loadProjectNotes });
.btn-tag-suggestion.applied {
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
border-color: var(--fs-accent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
}
.link-suggest-strip {
@@ -424,8 +424,8 @@ defineExpose({ reload: loadAll });
border-radius: 10px;
text-transform: capitalize;
}
.ms-status-active { background: color-mix(in srgb, var(--fs-accent) 15%, transparent); color: var(--fs-accent); }
.ms-status-completed { background: color-mix(in srgb, var(--fs-success) 15%, transparent); color: var(--fs-success); }
.ms-status-active { background: color-mix(in srgb, var(--fs-accent) 15%, transparent); color: var(--fs-accent-fg); }
.ms-status-completed { background: color-mix(in srgb, var(--fs-success) 15%, transparent); color: var(--fs-success-fg); }
.task-items {
list-style: none;
@@ -516,8 +516,8 @@ defineExpose({ reload: loadAll });
user-select: none;
margin-left: auto;
}
.status-cycler.status-in_progress { border-color: var(--fs-accent); color: var(--fs-accent); background: color-mix(in srgb, var(--fs-accent) 10%, transparent); }
.status-cycler.status-done { border-color: var(--fs-success); color: var(--fs-success); background: color-mix(in srgb, var(--fs-success) 10%, transparent); }
.status-cycler.status-in_progress { border-color: var(--fs-accent); color: var(--fs-accent-fg); background: color-mix(in srgb, var(--fs-accent) 10%, transparent); }
.status-cycler.status-done { border-color: var(--fs-success); color: var(--fs-success-fg); background: color-mix(in srgb, var(--fs-success) 10%, transparent); }
.btn-edit-task { margin-left: 0.25rem; }
.btn-edit-task:hover { text-decoration: underline; }
@@ -3,6 +3,7 @@ import { computed, ref, watch, onMounted } from "vue";
import { useRulebooksStore } from "@/stores/rulebooks";
import { useCanonicalSystemsStore } from "@/stores/canonicalSystems";
import type { RuleTier } from "@/api/rulebooks";
import RuleHistoryPanel from "@/components/rules/RuleHistoryPanel.vue";
const props = defineProps<{ ruleId: number | null; topicId: number | null }>();
const emit = defineEmits<{ close: [] }>();
@@ -256,6 +257,17 @@ watch(() => props.ruleId, load);
How to apply
<textarea v-model="howToApply" rows="4" placeholder="When / where this kicks in." />
</label>
<!-- Only on an existing rule: a rule being created has no past, and an
"Edit history — none" line on a blank form reads as a broken panel.
Keyed on ruleId so switching rules reloads rather than showing the
previous rule's history under the new one's text. -->
<RuleHistoryPanel
v-if="!isCreating && ruleId !== null"
:key="ruleId"
:rule-id="ruleId"
:current="store.currentRule"
/>
</aside>
</div>
</template>
@@ -0,0 +1,274 @@
<script setup lang="ts">
/**
* What a rule USED TO SAY — inside the slide-over, where a rule is read in
* full. Not on the list row: a history entry point there would compete with
* the row's actual job.
*
* A SIBLING OF HistoryPanel.vue, NOT A REUSE OF IT, and the reason is in its
* props: `noteId` + `currentBody`, a `NoteVersion` carrying tags and pin
* columns, a fetch of /api/notes/…, a `restore` emit, and pin/unpin buttons.
* Every one of those is note-shaped. Rules have no tags, no pins, and
* deliberately no restore, and a rule's text is EIGHT fields rather than one
* body — which changes the central question from "what changed" to "which
* fields moved".
*
* What was genuinely shared is shared: DiffView.vue takes DiffLine[] and
* nothing note-shaped, and the LCS walk now lives in utils/diff.ts, which
* this file uses rather than copying a fourth time (#3207).
*/
import { computed, onMounted, ref, watch } from "vue";
import DiffView from "@/components/DiffView.vue";
import { computeDiff } from "@/utils/diff";
import {
listRuleVersions, getRuleVersion, type Rule, type RuleVersion,
} from "@/api/rulebooks";
import { useToastStore } from "@/stores/toast";
const props = defineProps<{ ruleId: number; current: Rule | null }>();
const toast = useToastStore();
const versions = ref<RuleVersion[]>([]);
const selected = ref<RuleVersion | null>(null);
const expanded = ref(false);
const loading = ref(false);
const loadingDetail = ref(false);
// The eight TEXT fields a version carries, in the order the editor shows
// them. Narrowed to its own type rather than `keyof RuleVersion`, which would
// also admit id/rule_id/user_id/created_at — none of which is text a reader
// compares, and all of which would widen every lookup below to `number`.
// Labels rather than column names: a reader is deciding whether to open a
// row, and "How to apply" reads where "how_to_apply" has to be decoded.
type TextField =
| "title" | "statement" | "when_to_apply" | "tier"
| "why" | "how_to_apply" | "verify_with" | "expires_when";
const FIELDS: Array<[TextField, string]> = [
["title", "Title"],
["statement", "Statement"],
["when_to_apply", "When to apply"],
["tier", "Tier"],
["why", "Why"],
["how_to_apply", "How to apply"],
["verify_with", "Check"],
["expires_when", "Ends when"],
];
/**
* Which fields this edit moved.
*
* A version holds the text the edit REPLACED, so the edit is the step from
* this row to the NEXT NEWER state — the version above it in the list, or,
* for the newest row, the rule as it stands now. Comparing against the row
* below instead would attribute every change to the wrong edit.
*/
function changedFields(index: number): string[] {
const before = versions.value[index];
// `Rule` carries all eight as required strings; a RuleVersion carries them
// only once opened, which is what the undefined check below is about.
const after: Pick<Rule, TextField> | RuleVersion | null =
index === 0 ? props.current : versions.value[index - 1] ?? null;
if (!before || !after) return [];
return FIELDS
.filter(([key]) => {
// A listing row carries only the title; the rest arrive when opened.
// Undefined means NOT LOADED, which is not the same as unchanged — so a
// field nobody has fetched is claimed as neither.
const a = before[key];
const b = after[key];
if (a === undefined || b === undefined) return false;
return (a ?? "") !== (b ?? "");
})
.map(([, label]) => label);
}
/** True when this edit rewrote or removed the rule's check.
*
* Worth its own marker because editing `verify_with` silently drops
* `verified_at` (milestone 312) — the moment a rule re-entered the staleness
* sweep. That happens nowhere a reader can see it, and this row is the only
* surface that can say when it happened. */
function checkChanged(index: number): boolean {
return changedFields(index).includes("Check");
}
const diff = computed(() => {
if (!selected.value || selected.value.statement === undefined) return [];
const now = props.current?.statement ?? "";
return computeDiff(now, selected.value.statement);
});
function stamp(iso: string): string {
return iso.slice(0, 10);
}
async function load() {
loading.value = true;
try {
versions.value = await listRuleVersions(props.ruleId);
} catch {
toast.show("Could not load this rule's history", "error");
} finally {
loading.value = false;
}
}
async function open(v: RuleVersion) {
if (selected.value?.id === v.id) {
selected.value = null;
return;
}
loadingDetail.value = true;
try {
const full = await getRuleVersion(props.ruleId, v.id);
// Merged back into the list so `changedFields` can compare against real
// text once a neighbour has been opened, instead of staying blind.
const at = versions.value.findIndex((x) => x.id === v.id);
if (at >= 0) versions.value[at] = { ...versions.value[at], ...full };
selected.value = versions.value[at] ?? full;
} catch {
toast.show("Could not open that version", "error");
} finally {
loadingDetail.value = false;
}
}
onMounted(load);
watch(() => props.ruleId, () => { selected.value = null; load(); });
</script>
<template>
<section class="history">
<button class="toggle" :aria-expanded="expanded" @click="expanded = !expanded">
<span>Edit history</span>
<span class="count">{{ versions.length || "none" }}</span>
</button>
<div v-if="expanded" class="body">
<p v-if="loading" class="state">Loading</p>
<!-- Never reworded is the ordinary case, and must not read as a fault. -->
<p v-else-if="!versions.length" class="state empty">
This rule has never been reworded. Nothing was recorded before the history
existed, so an older rule starts empty too.
</p>
<template v-else>
<p class="lede">
Each entry is what the rule said <em>before</em> that edit. The wording it
was changed to is the rule as it stands above.
</p>
<ol class="rows">
<li v-for="(v, i) in versions" :key="v.id" class="row">
<button
class="row-head"
:class="{ open: selected?.id === v.id }"
@click="open(v)"
>
<span class="when">{{ stamp(v.created_at) }}</span>
<span class="fields">
{{ changedFields(i).join(", ") || "opened to compare" }}
</span>
<span v-if="checkChanged(i)" class="check-moved">check reset</span>
</button>
<div v-if="selected?.id === v.id" class="detail">
<p v-if="loadingDetail" class="state">Loading</p>
<template v-else>
<p v-if="checkChanged(i)" class="warn">
This edit changed the rule's check, which cleared its verification
stamp the rule went back to the top of the staleness sweep here.
</p>
<dl class="fields-list">
<template v-for="[key, label] in FIELDS" :key="key">
<template v-if="key !== 'statement' && v[key]">
<dt>{{ label }}</dt>
<dd>{{ v[key] }}</dd>
</template>
</template>
</dl>
<h4>Statement</h4>
<DiffView v-if="diff.length" :diff="diff" />
<p v-else class="state">The statement did not change in this edit.</p>
</template>
</div>
</li>
</ol>
</template>
</div>
</section>
</template>
<style scoped>
.history { border-top: 1px solid var(--fs-border-color); padding-top: var(--fs-space-3); }
.toggle {
display: flex; align-items: center; gap: var(--fs-space-2); width: 100%;
background: none; border: none; padding: 0; cursor: pointer;
font: inherit; font-size: var(--fs-size-body-sm); color: var(--fs-text-secondary);
}
.toggle:hover { color: var(--fs-text-primary); }
.count {
margin-left: auto; font-size: var(--fs-size-tiny); color: var(--fs-text-tertiary);
font-variant-numeric: tabular-nums;
}
.body { margin-top: var(--fs-space-3); display: flex; flex-direction: column; gap: var(--fs-space-3); }
.state { margin: 0; font-size: var(--fs-size-body-sm); color: var(--fs-text-secondary); }
.state.empty { color: var(--fs-text-tertiary); }
.lede {
margin: 0; max-width: 62ch; font-size: var(--fs-size-tiny);
color: var(--fs-text-tertiary); line-height: var(--fs-leading-body);
}
.rows { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: var(--fs-space-2); }
.row { background: var(--fs-surface-raised); border-radius: var(--fs-radius-md); }
.row-head {
display: flex; align-items: baseline; gap: var(--fs-space-3); width: 100%;
background: none; border: none; cursor: pointer; text-align: left;
padding: var(--fs-space-2) var(--fs-space-3);
font: inherit; font-size: var(--fs-size-body-sm); color: var(--fs-text-primary);
}
.row-head:hover { background: var(--fs-surface-hover); border-radius: var(--fs-radius-md); }
.when {
font-variant-numeric: tabular-nums; color: var(--fs-text-secondary);
font-size: var(--fs-size-tiny);
}
.fields { color: var(--fs-text-primary); min-width: 0; overflow-wrap: anywhere; }
/* A TINT, not the solid token. `--fs-warning-fg` is defined as "warning text
ON A WARNING TINT" — painting it over solid `--fs-warning` is the same-hue
contrast failure #3141 records. The 12% mix is how theme.css builds its own
`-bg` pairs, and it keeps the value a resolvable var() rather than a raw hex
that check_design_tokens.py cannot see at all. */
.check-moved {
margin-left: auto; flex: none;
background: color-mix(in srgb, var(--fs-warning) 12%, transparent);
color: var(--fs-warning-fg);
border-radius: var(--fs-radius-pill);
padding: 0.1rem 0.5rem;
font-size: var(--fs-size-tiny); letter-spacing: var(--fs-tracking-tiny);
}
.detail {
padding: 0 var(--fs-space-3) var(--fs-space-3);
display: flex; flex-direction: column; gap: var(--fs-space-2);
}
.warn {
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);
}
.fields-list { display: grid; grid-template-columns: auto 1fr; gap: 0.15rem var(--fs-space-3); margin: 0; }
.fields-list dt {
font-size: var(--fs-size-tiny); text-transform: uppercase;
letter-spacing: var(--fs-tracking-tiny); color: var(--fs-text-tertiary);
}
.fields-list dd {
margin: 0; font-size: var(--fs-size-body-sm);
color: var(--fs-text-primary); min-width: 0; overflow-wrap: anywhere;
}
h4 { margin: var(--fs-space-2) 0 0; font-size: var(--fs-size-tiny); color: var(--fs-text-tertiary); }
</style>
+5 -26
View File
@@ -1,4 +1,5 @@
import { ref, computed, watch, type Ref } from "vue";
import { computeDiff, type DiffLine } from "@/utils/diff";
import { apiPost, apiPut, apiDelete, apiSSEStream, type SSEStreamHandle } from "@/api/client";
import { useToastStore } from "@/stores/toast";
import {
@@ -9,17 +10,16 @@ import {
export type AssistState = "idle" | "streaming" | "review";
export type ScopeMode = "document" | "section";
// Re-exported: this composable was where DiffLine lived before the diff
// moved to a shared util, and every consumer still imports the type from here.
export type { DiffLine };
export interface AssistTarget {
text: string;
startOffset: number;
endOffset: number;
}
export interface DiffLine {
type: 'equal' | 'delete' | 'insert';
text: string;
}
export interface NoteDraft {
id: number;
note_id: number;
@@ -31,27 +31,6 @@ export interface NoteDraft {
updated_at: string;
}
function computeDiff(a: string, b: string): DiffLine[] {
const aLines = a.split('\n');
const bLines = b.split('\n');
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i+1][j+1] + 1
: Math.max(dp[i+1][j], dp[i][j+1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: 'equal', text: aLines[i++] }); j++; }
else if (dp[i+1][j] >= dp[i][j+1]) result.push({ type: 'delete', text: aLines[i++] });
else result.push({ type: 'insert', text: bLines[j++] });
}
while (i < m) result.push({ type: 'delete', text: aLines[i++] });
while (j < n) result.push({ type: 'insert', text: bLines[j++] });
return result;
}
export function useAssist(body: Ref<string>, noteId?: Ref<number | null>, projectId?: Ref<number | null>) {
const toast = useToastStore();
+7 -1
View File
@@ -31,6 +31,8 @@ export const useNotesStore = defineStore("notes", () => {
project_id?: number | null;
milestone_id?: number | null;
note_type?: string;
verify_with?: string;
expires_when?: string;
}): Promise<Note> {
try {
return await apiPost<Note>("/api/notes", data);
@@ -42,7 +44,11 @@ export const useNotesStore = defineStore("notes", () => {
async function updateNote(
id: number,
data: Partial<Pick<Note, "title" | "body" | "tags" | "project_id" | "milestone_id" | "note_type">>
data: Partial<Pick<
Note,
"title" | "body" | "tags" | "project_id" | "milestone_id" | "note_type"
| "verify_with" | "expires_when"
>>
): Promise<Note> {
try {
const note = await apiPut<Note>(`/api/notes/${id}`, data);
+9
View File
@@ -34,6 +34,15 @@ export interface Note {
is_task: boolean;
note_type: NoteType;
task_kind?: TaskKind;
// The note's own check (milestone 317). Empty on almost every note — that
// is the normal case: a note with no `verify_with` is a DECISION, and there
// is nothing to go and check. Only a note asserting a fact about something
// outside the operator's control carries one. `verified_at` null while
// `verify_with` is set means NOBODY HAS EVER CONFIRMED IT, which is the
// state the sweep ranks first.
verify_with?: string;
expires_when?: string;
verified_at?: string | null;
systems?: System[];
arose_from_id?: number | null;
created_at: string;
+50
View File
@@ -0,0 +1,50 @@
/**
* Line diff — one copy, for every surface that shows what changed.
*
* WHY THIS FILE EXISTS. The same LCS walk was written out three times:
* privately in `useAssist.ts`, and again inside `HistoryPanel.vue` and
* `VersionHistorySection.vue`. The three were character-identical apart from
* quote style — nobody had diverged them on purpose, they were simply copied
* because `computeDiff` was never exported. Milestone 323 needed a fourth
* consumer (a rule's edit history), and a fourth copy is the cost #3207
* records: a fix or an improvement now has to be found in N places by someone
* who does not know N.
*/
export interface DiffLine {
type: "equal" | "delete" | "insert";
text: string;
}
/**
* Diff `a` against `b`, line by line.
*
* `delete` lines come from `a`, `insert` lines from `b` — so the caller
* decides which side reads as "before" by which argument it passes. Every
* caller here passes the CURRENT text as `a` and the older text as `b`, so a
* deletion is what the old version had and an insertion is what replaced it.
*
* O(m·n) in time and memory: fine for a note or a rule, and deliberately not
* generalised further, since nothing here diffs a file of thousands of lines.
*/
export function computeDiff(a: string, b: string): DiffLine[] {
const aLines = a.split("\n");
const bLines = b.split("\n");
const m = aLines.length, n = bLines.length;
const dp: number[][] = Array.from({ length: m + 1 }, () => new Array(n + 1).fill(0));
for (let i = m - 1; i >= 0; i--)
for (let j = n - 1; j >= 0; j--)
dp[i][j] = aLines[i] === bLines[j]
? dp[i + 1][j + 1] + 1
: Math.max(dp[i + 1][j], dp[i][j + 1]);
const result: DiffLine[] = [];
let i = 0, j = 0;
while (i < m && j < n) {
if (aLines[i] === bLines[j]) { result.push({ type: "equal", text: aLines[i++] }); j++; }
else if (dp[i + 1][j] >= dp[i][j + 1]) result.push({ type: "delete", text: aLines[i++] });
else result.push({ type: "insert", text: bLines[j++] });
}
while (i < m) result.push({ type: "delete", text: aLines[i++] });
while (j < n) result.push({ type: "insert", text: bLines[j++] });
return result;
}
+2 -2
View File
@@ -774,7 +774,7 @@ onUnmounted(() => {
.tag-chip {
font-size: 0.7rem;
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
border-radius: 999px;
padding: 0.1rem 0.4rem;
}
@@ -928,7 +928,7 @@ onUnmounted(() => {
}
.peek-linked-item:hover {
background: color-mix(in srgb, var(--fs-accent) 8%, var(--fs-surface-raised));
color: var(--fs-accent);
color: var(--fs-accent-fg);
}
.peek-linked-type {
+94 -12
View File
@@ -4,6 +4,7 @@ import { useRouter } from "vue-router";
import { apiGet } from "@/api/client";
import type { TaskKind, TaskStatus, TaskPriority } from "@/types/note";
import KindBadge from "@/components/KindBadge.vue";
import NoteSweepPane from "@/components/NoteSweepPane.vue";
import StatusBadge from "@/components/StatusBadge.vue";
import PriorityBadge from "@/components/PriorityBadge.vue";
import GraphView from "@/views/GraphView.vue";
@@ -13,6 +14,7 @@ import {
Workflow,
Search,
Share2,
ShieldCheck,
ChevronLeft,
ChevronRight,
X,
@@ -24,7 +26,7 @@ const router = useRouter();
interface KnowledgeItem {
id: number;
note_type: "note" | "task" | "process";
note_type: "note" | "task" | "process" | "snippet";
title: string;
snippet: string;
tags: string[];
@@ -42,9 +44,41 @@ interface KnowledgeItem {
task_kind?: TaskKind;
}
// ─── The facet vocabulary ─────────────────────────────────────────────────────
// Mirrors services/knowledge._FACETS, which is where it is defined for real.
// A facet spans BOTH typing axes — a record TYPE (note / process / snippet) or
// a task KIND (`task` for any, else issue / spike) — because that is what this
// feed actually holds.
//
// `plan` is still a valid facet at the API, for the 90 legacy plan-tasks, but
// it has no chip: retired in 0066, it kept a chip of its own for longer than
// `issue` — 17% of every task here — went without one (#3128). Those rows are
// still reachable under Tasks, wearing a Plan badge.
type Facet = "" | "note" | "task" | "issue" | "spike" | "snippet" | "process";
// The facets that select TASKS. Kinds are subsets of `task`, so any of them
// means the duplicate report should be comparing tasks.
const TASK_FACETS = new Set<Facet>(["task", "issue", "spike"]);
const FACET_CHIPS: [Exclude<Facet, "">, string][] = [
["note", "Notes"],
["task", "Tasks"],
["issue", "Issues"],
["spike", "Spikes"],
["snippet", "Snippets"],
["process", "Processes"],
];
// ─── View mode ────────────────────────────────────────────────────────────────
// The sweep is cross-cutting — a note that has gone false does not care which
// facet it sits under — so it REPLACES the browse list rather than filtering
// it. Filtering would mean the answer depended on which chip was active, which
// is the under-reporting the sweep exists to prevent (milestone 317 step 4).
const sweepActive = ref(false);
// ─── Filter state ─────────────────────────────────────────────────────────────
const activeType = ref<"" | "note" | "task" | "plan" | "process">("");
const activeType = ref<Facet>("");
const activeTag = ref("");
const sortMode = ref<"modified" | "created" | "alpha" | "type">("modified");
const searchQuery = ref("");
@@ -70,9 +104,10 @@ const dupGroups = ref<DupGroup[]>([]);
const dupSuggestion = ref("");
const dupLoading = ref(false);
const dupChecked = ref(false);
// The report follows the type filter: viewing tasks checks tasks. Anything
// else (all / plan / process) checks notes — the kind with the most to find.
const dupKind = computed(() => (activeType.value === "task" ? "task" : "note"));
// The report follows the type filter: viewing tasks — under ANY task facet,
// including a single kind — checks tasks. Everything else checks notes, the
// kind with the most to find.
const dupKind = computed(() => (TASK_FACETS.has(activeType.value) ? "task" : "note"));
async function loadDuplicates() {
dupLoading.value = true;
@@ -96,8 +131,11 @@ watch(dupKind, () => { dupChecked.value = false; dupGroups.value = []; });
// ─── Type counts ──────────────────────────────────────────────────────────────
interface KnowledgeCounts { note: number; task: number; plan: number; process: number; total: number }
const typeCounts = ref<KnowledgeCounts>({ note: 0, task: 0, plan: 0, process: 0, total: 0 });
// One number per facet, plus the grand total. Partial because the server sends
// a key only for a facet it has rows for. Kinds are subsets of `task` and are
// deliberately absent from `total` — including them would count an issue twice.
type KnowledgeCounts = Partial<Record<Exclude<Facet, "">, number>> & { total: number };
const typeCounts = ref<KnowledgeCounts>({ total: 0 });
async function fetchCounts() {
try {
@@ -234,6 +272,10 @@ function onSearchInput() {
}
watch([activeType, sortMode], () => resetAndReobserve());
// Closing the sweep remounts the feed, and with it the scroll sentinel — a
// fresh element the old observer is not watching. Without this the list loads
// its first page and then never loads another.
watch(sweepActive, (open) => { if (!open) resetAndReobserve(); });
watch(activeTag, () => { fetchCounts(); resetAndReobserve(); });
// ─── Today bar ────────────────────────────────────────────────────────────────
@@ -274,9 +316,18 @@ function isOverdue(item: KnowledgeItem): boolean {
return new Date(item.due_date) < new Date(new Date().toDateString());
}
// Each record kind opens in ITS OWN editor. A snippet used to fall through to
// /notes/:id, whose save is a plain PATCH of the body — which left the snippet's
// derived `data` mirror describing the previous version (#3128). The service now
// recomposes the mirror either way, so this is no longer the guard; it is simply
// that the note editor cannot edit a snippet's signature, language or locations,
// and offering it as the way in was always wrong. Processes stay here on
// purpose: they have no editor of their own and the note editor knows the type.
function openItem(item: KnowledgeItem) {
if (item.note_type === 'task') {
router.push(`/tasks/${item.id}`);
} else if (item.note_type === 'snippet') {
router.push(`/snippets/${item.id}`);
} else {
router.push(`/notes/${item.id}`);
}
@@ -380,14 +431,14 @@ onUnmounted(() => {
<span v-if="typeCounts.total > 1" class="filter-count">{{ typeCounts.total }}</span>
</button>
<button
v-for="[val, label, key] in ([['note','Notes','note'],['task','Tasks','task'],['plan','Plans','plan'],['process','Processes','process']] as [string,string,string][])"
v-for="[val, label] in FACET_CHIPS"
:key="val"
class="filter-btn"
:class="{ active: activeType === val }"
@click="activeType = (val as '' | 'note' | 'task' | 'plan' | 'process')"
@click="activeType = val"
>
<span class="filter-btn-label">{{ label }}</span>
<span v-if="typeCounts[key as keyof KnowledgeCounts] > 1" class="filter-count">{{ typeCounts[key as keyof KnowledgeCounts] }}</span>
<span v-if="(typeCounts[val] ?? 0) > 1" class="filter-count">{{ typeCounts[val] }}</span>
</button>
</div>
@@ -433,6 +484,15 @@ onUnmounted(() => {
<Share2 :size="16" />
Graph
</button>
<button
class="btn-ghost btn-compact"
:class="{ active: sweepActive }"
title="Notes that assert a fact about something outside your control, least-recently-confirmed first. Most notes are decisions and never appear."
@click="sweepActive = !sweepActive"
>
<ShieldCheck :size="16" />
Due
</button>
<button
class="btn-ghost btn-compact"
:disabled="dupLoading"
@@ -443,6 +503,12 @@ onUnmounted(() => {
</button>
</div>
<!-- The sweep replaces the feed. It is not a facet: a facet answers
"show me this kind", and this answers "show me what nobody has
confirmed" a question the type chips cannot narrow without
under-reporting it. -->
<NoteSweepPane v-if="sweepActive" @open-note="(id) => router.push(`/notes/${id}`)" />
<!-- Near-duplicate report. A proposal surface only: unlike snippets
(which merge losslessly), notes are never merged the right fix is
supersession, extraction into a reference note, or leaving parallel
@@ -480,6 +546,12 @@ onUnmounted(() => {
</template>
</div>
<!-- The whole feed stands down while the sweep is open: two answers
to two different questions on one screen is neither. Wrapped
rather than given an extra v-if branch, because the scroll
sentinel lives inside the grid and the observer must not be left
holding a ref to something that never renders. -->
<template v-if="!sweepActive">
<!-- Loading / empty -->
<div v-if="loading && items.length === 0" class="knowledge-empty">Loading…</div>
<div v-else-if="!loading && items.length === 0" class="knowledge-empty">
@@ -501,6 +573,7 @@ onUnmounted(() => {
<span v-if="item.note_type === 'note'">Note</span>
<span v-else-if="item.note_type === 'task'">{{ item.task_kind === 'plan' ? 'Plan' : 'Task' }}</span>
<span v-else-if="item.note_type === 'process'">Process</span>
<span v-else-if="item.note_type === 'snippet'">Snippet</span>
</span>
<!-- Kind sits BESIDE the type badge, not inside it: the type badge
speaks the vocabulary of this view's type filter (note / task /
@@ -552,6 +625,7 @@ onUnmounted(() => {
<span v-if="contentFetching" class="sentinel-loading">Loading</span>
</div>
</div>
</template>
</div>
<!-- Graph panel -->
@@ -753,7 +827,7 @@ onUnmounted(() => {
}
.filter-btn.active .filter-count {
background: color-mix(in srgb, var(--fs-accent) 20%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
}
.filter-tag { font-size: 0.78rem; }
@@ -881,6 +955,14 @@ onUnmounted(() => {
.badge--note { background: color-mix(in srgb, var(--fs-accent) 15%, transparent); color: #7A6DA8; }
.badge--task { background: rgba(212,160,23,0.15); color: #fbbf24; }
.badge--plan { background: rgba(99,102,241,0.18); color: #818cf8; }
/* Snippet and process are NEUTRAL on purpose. Both were unstyled — and the
snippet had no label either, so all 90 of them rendered an empty chip in
this feed (#3128). Giving them hues would put a third and fourth colour
beside KindBadge's warm/cool pair on the same card; a record type that
isn't an alarm reads better as plain. Standard body pair, so the contrast
is the one the palette already guarantees. */
.badge--snippet,
.badge--process { background: var(--fs-surface-raised); color: var(--fs-text-secondary); }
.k-card-body { flex: 1; padding-right: 40px; }
.k-card-title {
@@ -926,7 +1008,7 @@ onUnmounted(() => {
border-radius: 4px;
white-space: nowrap;
background: color-mix(in srgb, var(--fs-text-secondary) 15%, transparent);
color: var(--fs-text-secondary);
color: var(--fs-text-secondary-fg);
}
/* ── Task card ──────────────────────────────────────────── */
+110 -44
View File
@@ -34,6 +34,14 @@ const tags = ref<string[]>([]);
const projectId = ref<number | null>(null);
const milestoneId = ref<number | null>(null);
const noteType = ref<NoteType>("note");
// The note's own check (milestone 317). Offered only for a plain note: a
// task's decay is its status, and a snippet has verify_snippet — the service
// refuses both, so the form must not ask for what the save would reject.
const verifyWith = ref("");
const expiresWhen = ref("");
const verifiedAt = ref<string | null>(null);
const canCarryCheck = computed(() => noteType.value === "note");
const dirty = ref(false);
const saving = ref(false);
const showPreview = ref(false);
@@ -198,6 +206,41 @@ let savedTags: string[] = [];
let savedProjectId: number | null = null;
let savedMilestoneId: number | null = null;
let savedNoteType: NoteType = "note";
let savedVerifyWith = "";
let savedExpiresWhen = "";
/** The write, in one place. Three call sites (save, create, auto-save) each
* spelled this out, so every new field had to be added three times — which is
* how one of them ends up not carrying it. */
function payload() {
return {
title: title.value,
body: body.value,
tags: tags.value,
project_id: projectId.value,
milestone_id: milestoneId.value,
note_type: noteType.value,
// "" clears the check: the REST door reads an empty string as NULL
// (NULLABLE_NOTE_TEXT), which is how a cleared form input says "remove
// this" without needing the MCP door's explicit `clear` list.
verify_with: canCarryCheck.value ? verifyWith.value : "",
expires_when: canCarryCheck.value ? expiresWhen.value : "",
};
}
/** What the form last agreed with the server about — the other half of the
* same list, and for the same reason. */
function snapshot() {
savedTitle = title.value;
savedBody = body.value;
savedTags = [...tags.value];
savedProjectId = projectId.value;
savedMilestoneId = milestoneId.value;
savedNoteType = noteType.value;
savedVerifyWith = verifyWith.value;
savedExpiresWhen = expiresWhen.value;
dirty.value = false;
}
function markDirty() {
dirty.value =
@@ -206,7 +249,9 @@ function markDirty() {
JSON.stringify(tags.value) !== JSON.stringify(savedTags) ||
projectId.value !== savedProjectId ||
milestoneId.value !== savedMilestoneId ||
noteType.value !== savedNoteType;
noteType.value !== savedNoteType ||
verifyWith.value !== savedVerifyWith ||
expiresWhen.value !== savedExpiresWhen;
}
function onBodyUpdate(newVal: string) {
@@ -224,12 +269,10 @@ onMounted(async () => {
projectId.value = store.currentNote.project_id ?? null;
milestoneId.value = store.currentNote.milestone_id ?? null;
noteType.value = (store.currentNote.note_type as NoteType) || "note";
savedTitle = title.value;
savedBody = body.value;
savedTags = [...tags.value];
savedProjectId = projectId.value;
savedMilestoneId = milestoneId.value;
savedNoteType = noteType.value;
verifyWith.value = store.currentNote.verify_with || "";
expiresWhen.value = store.currentNote.expires_when || "";
verifiedAt.value = store.currentNote.verified_at ?? null;
snapshot();
}
} else {
// New note: read type from query param
@@ -260,31 +303,11 @@ async function save() {
const finalBody = body.value;
try {
if (isEditing.value) {
await store.updateNote(noteId.value!, {
title: title.value,
body: finalBody,
tags: tags.value,
project_id: projectId.value,
milestone_id: milestoneId.value,
note_type: noteType.value,
});
savedTitle = title.value;
savedBody = body.value;
savedTags = [...tags.value];
savedProjectId = projectId.value;
savedMilestoneId = milestoneId.value;
savedNoteType = noteType.value;
dirty.value = false;
await store.updateNote(noteId.value!, { ...payload(), body: finalBody });
snapshot();
toast.show("Note saved");
} else {
const note = await store.createNote({
title: title.value,
body: finalBody,
tags: tags.value,
project_id: projectId.value,
milestone_id: milestoneId.value,
note_type: noteType.value,
});
const note = await store.createNote({ ...payload(), body: finalBody });
dirty.value = false;
toast.show("Note created");
router.push(`/notes/${note.id}`);
@@ -321,18 +344,8 @@ async function doAutoSave() {
saving.value = true;
const finalBody = body.value;
try {
await store.updateNote(noteId.value!, {
title: title.value, body: finalBody, tags: tags.value,
project_id: projectId.value, milestone_id: milestoneId.value,
note_type: noteType.value,
});
savedTitle = title.value;
savedBody = body.value;
savedTags = [...tags.value];
savedProjectId = projectId.value;
savedMilestoneId = milestoneId.value;
savedNoteType = noteType.value;
dirty.value = false;
await store.updateNote(noteId.value!, { ...payload(), body: finalBody });
snapshot();
toast.show("Auto-saved");
} catch {
// Silent
@@ -496,6 +509,41 @@ onUnmounted(() => assist.clearSelection());
</select>
</div>
<!-- The note's own check (milestone 317). Shown only for a plain
note: the service refuses a check on a task or a snippet, so
offering the fields there would be a form whose save fails. -->
<template v-if="canCarryCheck">
<div class="sb-field">
<div class="sb-label-row">
<span class="sb-label">Check</span>
<span v-if="verifyWith" class="check-age" :class="{ unchecked: !verifiedAt }">
{{ verifiedAt ? `checked ${verifiedAt.slice(0, 10)}` : "never checked" }}
</span>
</div>
<!-- Phrased as the question that decides, not as a field name.
"Verify with" would get filled in on every note; "could this
become false without anyone editing it?" gets filled in on
the few that can. -->
<textarea
v-model="verifyWith"
class="sb-textarea"
rows="2"
placeholder="How would someone check this is still true? Leave empty unless this note could become false without anyone editing it."
@input="markDirty"
></textarea>
</div>
<div v-if="verifyWith" class="sb-field">
<label class="sb-label">Ends when</label>
<textarea
v-model="expiresWhen"
class="sb-textarea"
rows="2"
placeholder="What state ends it? A state, not a date — “when the forge numbers runs per workflow”, not “in six months”."
@input="markDirty"
></textarea>
</div>
</template>
<!-- Link Suggestions -->
<div v-if="linkSuggestions.length > 0" class="sb-field link-suggest-field">
<div class="sb-label-row">
@@ -678,7 +726,7 @@ onUnmounted(() => assist.clearSelection());
flex-direction: column;
}
.sb-select, .sb-input {
.sb-select, .sb-input, .sb-textarea {
width: 100%;
padding: 5px 8px;
border-radius: var(--fs-radius-sm);
@@ -690,9 +738,27 @@ onUnmounted(() => assist.clearSelection());
outline: none;
transition: border-color 0.15s;
}
.sb-select:focus, .sb-input:focus {
.sb-select:focus, .sb-input:focus, .sb-textarea:focus {
border-color: var(--fs-accent);
}
.sb-textarea {
resize: vertical;
line-height: 1.4;
box-sizing: border-box;
}
/* No red/amber ramp, matching RuleSweepPane: a colour scale would restate the
sweep's ordering and force an invented "stale after N days" threshold.
"Never" is marked because it is categorically different from a date, not a
worse one it means nobody has ever confirmed the claim. */
.check-age {
font-size: 0.7rem;
color: var(--fs-text-secondary);
font-variant-numeric: tabular-nums;
}
.check-age.unchecked {
font-style: italic;
color: var(--fs-text-tertiary);
}
/* Link Suggestions */
.link-suggest-field { gap: 0.4rem; }
+1 -1
View File
@@ -416,7 +416,7 @@ async function convertToTask() {
}
.badge-note {
background: color-mix(in srgb, var(--fs-accent) 12%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
border: 1px solid color-mix(in srgb, var(--fs-accent) 25%, transparent);
}
.badge-task {
+4 -4
View File
@@ -1287,8 +1287,8 @@ async function confirmDelete() {
.stat-todo { background: color-mix(in srgb, var(--fs-text-tertiary) 8%, transparent); color: var(--fs-text-secondary); border-color: var(--fs-border-color); }
.stat-inprogress { background: color-mix(in srgb, #3b82f6 10%, transparent); color: #3b82f6; border-color: color-mix(in srgb, #3b82f6 28%, transparent); }
.stat-done { background: color-mix(in srgb, var(--fs-success) 10%, transparent); color: var(--fs-success); border-color: color-mix(in srgb, var(--fs-success) 28%, transparent); }
.stat-notes { background: color-mix(in srgb, var(--fs-accent) 8%, transparent); color: var(--fs-accent); border-color: color-mix(in srgb, var(--fs-accent) 22%, transparent); }
.stat-done { background: color-mix(in srgb, var(--fs-success) 10%, transparent); color: var(--fs-success-fg); border-color: color-mix(in srgb, var(--fs-success) 28%, transparent); }
.stat-notes { background: color-mix(in srgb, var(--fs-accent) 8%, transparent); color: var(--fs-accent-fg); border-color: color-mix(in srgb, var(--fs-accent) 22%, transparent); }
/* ── Pattern-library coverage card ───────────────────────────── */
.coverage-card {
@@ -1471,7 +1471,7 @@ async function confirmDelete() {
.tab-btn.active .tab-count {
background: color-mix(in srgb, var(--fs-accent) 12%, transparent);
border-color: color-mix(in srgb, var(--fs-accent) 30%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
}
/* ── Tasks view ──────────────────────────────────────────────── */
@@ -1707,7 +1707,7 @@ async function confirmDelete() {
border-radius: 3px;
margin-left: auto;
}
.col-add-btn:hover { color: var(--fs-accent); background: color-mix(in srgb, var(--fs-accent) 10%, transparent); }
.col-add-btn:hover { color: var(--fs-accent-fg); background: color-mix(in srgb, var(--fs-accent) 10%, transparent); }
.kanban-cards { display: flex; flex-direction: column; gap: 0.3rem; }
+8 -8
View File
@@ -2732,7 +2732,7 @@ async function deleteUser(userId: number) {
background: var(--fs-surface-raised);
}
.sidebar-item.active {
color: var(--fs-accent);
color: var(--fs-accent-fg);
background: color-mix(in srgb, var(--fs-accent) 8%, transparent);
border-left-color: var(--fs-accent);
font-weight: 500;
@@ -3099,7 +3099,7 @@ async function deleteUser(userId: number) {
border-radius: var(--fs-radius-sm);
}
.role-admin {
color: var(--fs-accent);
color: var(--fs-accent-fg);
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
}
.role-user {
@@ -3179,9 +3179,9 @@ async function deleteUser(userId: number) {
text-transform: uppercase; letter-spacing: 0.05em;
padding: 0.1rem 0.35rem; border-radius: var(--fs-radius-sm);
}
.cat-audit { color: var(--fs-accent); background: color-mix(in srgb, var(--fs-accent) 15%, transparent); }
.cat-usage { color: var(--fs-success); background: color-mix(in srgb, var(--fs-success) 15%, transparent); }
.cat-error { color: var(--fs-error); background: color-mix(in srgb, var(--fs-error) 15%, transparent); }
.cat-audit { color: var(--fs-accent-fg); background: color-mix(in srgb, var(--fs-accent) 15%, transparent); }
.cat-usage { color: var(--fs-success-fg); background: color-mix(in srgb, var(--fs-success) 15%, transparent); }
.cat-error { color: var(--fs-error-fg); background: color-mix(in srgb, var(--fs-error) 15%, transparent); }
.method-tag {
display: inline-block;
font-size: 0.65rem; font-weight: 500; font-family: monospace;
@@ -3346,8 +3346,8 @@ async function deleteUser(userId: number) {
padding: 0.15rem 0.4rem;
border-radius: 4px;
}
.role-owner { background: color-mix(in srgb, var(--fs-warning) 15%, transparent); color: var(--fs-warning); }
.role-member { background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent); color: var(--fs-text-tertiary); }
.role-owner { background: color-mix(in srgb, var(--fs-warning) 15%, transparent); color: var(--fs-warning-fg); }
.role-member { background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent); color: var(--fs-text-tertiary-fg); }
.members-empty {
color: var(--fs-text-tertiary);
@@ -3528,7 +3528,7 @@ async function deleteUser(userId: number) {
.day-btn.active {
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
border-color: var(--fs-accent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
font-weight: 500;
}
+3 -3
View File
@@ -242,9 +242,9 @@ onMounted(async () => {
border-radius: 4px;
white-space: nowrap;
}
.perm-viewer { background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent); color: var(--fs-text-tertiary); }
.perm-editor { background: color-mix(in srgb, var(--fs-accent) 15%, transparent); color: var(--fs-accent); }
.perm-admin { background: color-mix(in srgb, var(--fs-warning) 15%, transparent); color: var(--fs-warning); }
.perm-viewer { background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent); color: var(--fs-text-tertiary-fg); }
.perm-editor { background: color-mix(in srgb, var(--fs-accent) 15%, transparent); color: var(--fs-accent-fg); }
.perm-admin { background: color-mix(in srgb, var(--fs-warning) 15%, transparent); color: var(--fs-warning-fg); }
.empty-msg {
margin: 0;
+1 -1
View File
@@ -288,7 +288,7 @@ async function confirmDelete() {
font-family: var(--fs-font-mono);
font-size: 0.82rem;
background: color-mix(in srgb, var(--fs-accent) 12%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
padding: 0.08rem 0.35rem;
border-radius: var(--fs-radius-sm);
word-break: break-all;
+5 -5
View File
@@ -710,7 +710,7 @@ function usageTitle(s: SnippetListItem): string {
flex-shrink: 0;
white-space: nowrap;
background: color-mix(in srgb, var(--fs-accent) 15%, transparent);
color: var(--fs-accent);
color: var(--fs-accent-fg);
}
.snippet-when {
@@ -739,7 +739,7 @@ function usageTitle(s: SnippetListItem): string {
border-radius: 4px;
white-space: nowrap;
background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent);
color: var(--fs-text-tertiary);
color: var(--fs-text-tertiary-fg);
}
.dup-action {
@@ -754,7 +754,7 @@ function usageTitle(s: SnippetListItem): string {
border-radius: 4px;
white-space: nowrap;
background: color-mix(in srgb, var(--fs-error) 15%, transparent);
color: var(--fs-error);
color: var(--fs-error-fg);
}
.usage-tag {
@@ -764,14 +764,14 @@ function usageTitle(s: SnippetListItem): string {
white-space: nowrap;
font-variant-numeric: tabular-nums;
background: color-mix(in srgb, var(--fs-text-tertiary) 15%, transparent);
color: var(--fs-text-tertiary);
color: var(--fs-text-tertiary-fg);
}
/* Dead weight is a nudge, not an error — it warns in the warning colour rather
than the danger one, because the record isn't broken, just unearned. */
.usage-tag.usage-dead {
background: color-mix(in srgb, var(--fs-warning) 18%, transparent);
color: var(--fs-warning);
color: var(--fs-warning-fg);
}
/* Header + select-mode */
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "scribe",
"description": "Scribe system-of-record for Claude Code: MCP tools over your notes/tasks/projects/rules, a session-start push channel that surfaces your always-on rules + active-project context, process-skills (writing-plans, systematic-debugging, verification, brainstorming, reusing-code), and your saved Scribe Processes auto-surfaced as skills (/scribe:sync). Replaces superpowers + file-memory with one app-backed plugin.",
"version": "0.1.47",
"version": "0.1.48",
"author": {
"name": "Bryan Van Deusen"
},
+11 -1
View File
@@ -175,6 +175,16 @@ while IFS= read -r rel_path; do
derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen=""
[ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}"
fi
# The rules marker the SessionStart hook stored, handed back so the server
# can say whether those rules moved since (milestone 323). Nothing stored
# means nothing sent, which the server reads as silence rather than as a
# mismatch — an install that never reached /api/plugin/context must not
# start claiming its rules changed.
etag_q=""
if [ -f "$state_dir/${safe_sid}.rules_etag" ]; then
held=$(jq -sRr '@uri' < "$state_dir/${safe_sid}.rules_etag" 2>/dev/null) || held=""
[ -n "$held" ] && etag_q="&rules_etag=${held}"
fi
if [ -n "$path_enc" ]; then
# 8s, not the pre-write hook's 5: this hook runs AFTER the tool, so it
# gates nothing the session is waiting on, and the first prior-art call
@@ -184,7 +194,7 @@ while IFS= read -r rel_path; do
reached=1
body=$(curl -fsS --max-time 8 \
-H "Authorization: Bearer ${token}" \
"${url%/}/api/plugin/prior-art?path=${path_enc}&code=${code_enc}${repo_q}${exclude_q}${sync_exclude_q}${derive_exclude_q}${shapes_q}" 2>/dev/null) || { body=""; reached=0; }
"${url%/}/api/plugin/prior-art?path=${path_enc}&code=${code_enc}${repo_q}${exclude_q}${sync_exclude_q}${derive_exclude_q}${shapes_q}${etag_q}" 2>/dev/null) || { body=""; reached=0; }
# A call that was owed and didn't come back is said, once per outage
# (#2932) — shared marker with the pre-write hook, so one outage is one
# line however the code was written.
+22
View File
@@ -109,6 +109,28 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then
-H "Authorization: Bearer ${token}" \
"${url%/}/api/plugin/context${q}" 2>/dev/null) || body=""
[ -n "$body" ] && dyn=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null)
# Stash the rules marker for the write-path hook (milestone 323). THIS is
# where it has to be captured: the model receives one from
# list_always_on_rules too, but a hook cannot see an MCP tool's result. Stored
# under the same state dir the prior-art hook already uses, keyed by session,
# so "changed since" means since THIS session loaded its rules.
#
# Written on `compact` as well as `startup`, and that is correct rather than
# convenient: a compact tells the session to re-pull its rules, so the marker
# should describe the set it is about to hold. It is also why this cannot
# cover the compaction case — see the table in services/plugin_context.py.
if [ -n "$body" ]; then
etag=$(printf '%s' "$body" | jq -r '.rules_etag // empty' 2>/dev/null) || etag=""
if [ -n "$etag" ]; then
sid=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || sid=""
safe_sid=$(printf '%s' "${sid:-nosession}" | tr -c 'A-Za-z0-9._-' '_')
etag_dir="${TMPDIR:-/tmp}/scribe-priorart"
# Best-effort throughout: a marker that cannot be stored costs a hint,
# never the session.
mkdir -p "$etag_dir" 2>/dev/null \
&& printf '%s' "$etag" > "$etag_dir/${safe_sid}.rules_etag" 2>/dev/null || true
fi
fi
[ -z "$dyn" ] && status="> ⚠️ Scribe: live rules/project context could not be loaded this session (instance unreachable or request failed). The standing guidance above still applies — pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\` as needed."
elif [ -n "$url" ] && [ -z "$token" ]; then
status="> ⚠️ Scribe: live context disabled this session — the API key is not configured (Scribe base URL is). Set it with \`/plugin\` → Scribe → configure, or export SCRIBE_TOKEN. Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`."
+61 -1
View File
@@ -96,7 +96,25 @@ Two constraints on *how* that's achieved:
not restraint. Only a record genuinely about no particular area goes
untagged.
8. **State updates in place; chronicles don't.** A dev-log records what
8. **Name the record, never just its number.** Whenever you refer to a Scribe
record — in a message to the operator, a commit message, a task body, a
work-log — write the id *and* its title: `#3244 "the staleness signal"`,
`milestone 323 "rule versioning"`. Not `#3244`.
You have the record open; the operator does not. A bare id reads as
complete to you and as homework to them — they have to look it up to know
what their own conversation is about, or guess. Scribe's own duplicate gate
already writes `id 412: "debounce helper"` for exactly this reason; match
it everywhere else.
The first mention in a message carries the title; later mentions of the
same record can use the bare id. If you don't know the title, look it up
before citing the number — an id you can't name is one you haven't checked.
This matters most in the places read later by someone with even less
context than the operator has now: commit messages, task bodies, and any
record that cites another.
9. **State updates in place; chronicles don't.** A dev-log records what
*happened* — write it once, never rewrite it. A durable finding (how a
subsystem works, a measured number) lives in that System's **reference
note** ("«System» — reference"), which you UPDATE as facts change — safe,
@@ -106,6 +124,48 @@ Two constraints on *how* that's achieved:
re-measurement, a reversed decision), pass the old id in `supersedes` so the
stale record is demoted and labelled rather than left competing.
10. **A few notes assert a FACT, and those can carry their own check.**
Supersession only fires once somebody has read a note and disagreed — which
is the case where it was already believed. A note asserting something about
*someone else's* software — what a service does on a duplicate upload, how a
forge numbers its CI runs, what an updater compares — can instead carry
`verify_with` (how to check it) and `expires_when` (the STATE that ends it:
"when the forge numbers runs per workflow", never "in six months").
`notes_due_for_verification` lists them least-recently-confirmed first, with
never-checked at the top; `mark_note_verified` records what you found, and
`still_true=False` deliberately writes nothing — a note whose check failed
is wrong rather than in a state worth recording, so it keeps its place.
**The test is one question: could this note become false without anyone
editing it?** If no, leave both fields empty. That is the normal case, and
an empty `verify_with` is the positive marker for "this is a decision, there
is nothing to go and check" — not an unfinished record. The sweep is only
worth reading while almost nothing is on it, so a check added out of
tidiness costs the whole surface, not just that note.
**The sharper form of the same test: is the thing this note describes yours
to change?** If yes it is a decision — editing your own software is how it
changes, and you will know you did it. Measured against a real corpus, every
note that earned a check was about somebody ELSE's software: a signing
service, a forge, a hub, an SDK, a model, a dependency set.
**Three that look like candidates and are not:**
- **Resume pointers and "current state" notes.** They go stale fastest of
anything, which is exactly why they tempt — but the cure is to update or
delete them, not to schedule a check. A sweep full of pointers is a sweep
nobody reads.
- **Measurements of your own system.** They go false because you changed
something, and you knew. A measurement earns a check only when what it
measures is outside your control.
- **A decision that RESTS on somebody else's behaviour.** The decision is
still a decision. Put the check on the note asserting the fact, and link
the decision to it.
Not for tasks — a task's decay is its status, and a done issue records what
happened rather than asserting something that can go false. Not for snippets
either: `verify_snippet` compares the recorded location and code against the
repo, which is richer and already wired to drift detection.
## Stay inside the active project's scope
Once a project is in scope — you called `enter_project`, or the working repo is
+22 -15
View File
@@ -119,15 +119,20 @@ def same_hue_text_on_tint(css: str) -> tuple[list[str], list[str]]:
dangerous of the two — it does not even name a `-bg` token, so nothing
about it looks like the pattern until you measure it.
Returned separately because they are at different stages. The token form
is CLEAN and therefore gates. The inline form has a live backlog (48 sites
when this split was written, 26 of them --fs-accent), so it reports with a
count: a gate nobody can satisfy today gets switched off, and then it
guards nothing.
Returned separately because they were paid down separately — the token
form first (7 badge pairs), then the inline form (46 sites across 18
files, 26 of them --fs-accent). Both are clean now, so BOTH gate. The
split is kept because the two spellings need different error text: one
names a -bg token you can search for, the other names nothing at all.
"""
token_form, inline_form = [], []
for body in re.findall(r"\{([^{}]*)\}", css):
fg = set(re.findall(r"color\s*:\s*var\(\s*(--fs-[\w-]+?)\s*\)", body))
# (?<![-\w]) or `border-color`, `border-left-color` and `outline-color`
# all match as if they were text. They are not: a border is a non-text
# graphic and its floor is 3:1, not 4.5. Without this the check reported
# seven rules that were already correct — and a check that cries wolf on
# correct code is one that gets muted.
fg = set(re.findall(r"(?<![-\w])color\s*:\s*var\(\s*(--fs-[\w-]+?)\s*\)", body))
bg_tok = set(re.findall(r"background(?:-color)?\s*:\s*var\(\s*(--fs-[\w-]+?)-bg\s*\)", body))
bg_inl = set(re.findall(
r"background(?:-color)?\s*:\s*color-mix\([^;]*?var\(\s*(--fs-[\w-]+?)\s*\)[^;]*?\)",
@@ -232,21 +237,23 @@ def main() -> int:
print("OK — no text painted with a token on a tint of its own -bg.\n")
if inline_tint_hits:
by_tok: dict[str, int] = {}
for _p, tok in inline_tint_hits:
by_tok[tok] = by_tok.get(tok, 0) + 1
print(f"REPORT — {len(inline_tint_hits)} rule(s) paint text with a token on "
f"an INLINE color-mix tint of that same token.")
print(" Same defect, spelled without a -bg token so it does not gate yet.")
print(" Worst offenders: " + ", ".join(
f"{t} x{n}" for t, n in sorted(by_tok.items(), key=lambda kv: -kv[1])[:4]) + "\n")
print(f"FAIL — {len(inline_tint_hits)} rule(s) paint text with a token on an "
f"INLINE color-mix tint of that same token.")
print(" Identical defect to the block above, spelled without a -bg token —")
print(" which is what let it hide: nothing about it LOOKS like the pattern.")
print(" Use the token's -fg sibling.\n")
for path, tok in inline_tint_hits:
print(f" {path}: color: var({tok}) on an inline tint -> var({tok}-fg)")
print()
else:
print("OK — no text painted with a token on an inline tint of itself.\n")
if args.report_literals:
print(f"REPORT — {literal_count} raw colour literal(s) in component CSS.")
print(" Advisory: a literal is a value stated outside the system, so it "
"cannot follow a palette change.\n")
return 1 if (unresolved or same_hue_hits) else 0
return 1 if (unresolved or same_hue_hits or inline_tint_hits) else 0
if __name__ == "__main__":
+22
View File
@@ -30,6 +30,21 @@ from quart import Quart
# duplicate gate, the untagged-record systems_hint) act in-band in tool
# responses, at the moment they apply.
# Grow one of those, not this block.
# BUDGET: ~1980 of the client's ~2048-char cap (#2562). Everything below is
# competing for the last ~68 characters, so an addition here is a trade, never
# an append.
#
# Milestone 317 (a note's own verify_with / expires_when, and the sweep over
# them) was DECLINED a line, deliberately, by the operator — not overlooked.
# The reasoning, so it is not re-litigated blind: this is a map, and its own
# closing line says each tool's description carries the full contract. The
# sweep is a curation act, not a session-start reflex like enter_project or
# list_always_on_rules. Spending the last of the budget on it would leave the
# map unable to grow for something more central later.
#
# The accepted cost: an agent that never opens create_note's docstring never
# learns the field exists. Guidance lives in the create_note / update_note
# docstrings and the using-scribe skill instead.
_INSTRUCTIONS = """
Scribe is the operator's self-hosted second brain and system of record — and
yours: recall from it before acting, record as you go. Keep no parallel copy
@@ -120,6 +135,13 @@ _READ_ONLY_TOOLS = frozenset({
# prefix, so the completeness test below cannot derive it — the same
# reason `enter_project` is spelled out above.
"retrieval_telemetry",
# The note staleness sweep (milestone 317). A pure read — mark_note_verified
# is the write, and it is deliberately NOT here. Spelled out for
# retrieval_telemetry's reason: `notes_due_for_verification` matches none of
# the prefixes the completeness test derives from, so nothing would have
# prompted this decision. `rules_due_for_verification` is in the same
# position and is NOT listed — see #3191.
"notes_due_for_verification",
})
# Read-SHAPED tools that must NOT be reachable with a read key — a getter that
+14 -2
View File
@@ -137,11 +137,23 @@ async def delete_milestone(milestone_id: int) -> dict:
"""Move a milestone to the trash (recoverable). Its tasks go with it as one batch.
Restore via restore(batch_id)."""
uid = current_user_id()
# Read the title BEFORE the delete: afterwards the row is trashed and the
# confirmation could only echo the number back. A deletion the operator
# cannot recognise is one they cannot tell was the wrong one.
# Fail-open: the title is a COURTESY on top of the delete, so a lookup
# that errors must not stop the delete happening. Same posture the
# staleness marker takes — a decoration may never break its payload.
try:
doomed = await milestones_svc.get_milestone(uid, milestone_id)
title = getattr(doomed, "title", "") if doomed else ""
except Exception:
title = ""
batch = await trash_svc.delete(uid, "milestone", milestone_id)
if batch is None:
raise ValueError(f"milestone {milestone_id} not found")
return {"deleted_batch_id": batch,
"message": f"Milestone {milestone_id} + its tasks moved to trash. Restore with restore('{batch}')."}
return {"deleted": milestone_id, "title": title, "deleted_batch_id": batch,
"message": f'Milestone {milestone_id} ("{title}") and its tasks '
f"moved to trash. Restore with restore('{batch}')."}
def register(mcp) -> None:
+168 -3
View File
@@ -103,6 +103,8 @@ async def create_note(
project_id: int = 0,
system_ids: list[int] | None = None,
supersedes: list[int] | None = None,
verify_with: str = "",
expires_when: str = "",
force: bool = False,
) -> dict:
"""Create a new note in Scribe.
@@ -134,6 +136,33 @@ async def create_note(
arrives labelled when it does surface. This records a CLAIM, not a
verdict: it never says the older note was wrong, only that it is no
longer the current answer.
verify_with: HOW TO CHECK this note is still true. Leave empty for
almost every note — that is the normal case, not an unfinished
one.
A note is a NORM or a CONSTRAINT. A norm is a decision ("we derive
versions from commit time"): it has no truth value and changes only
when its author changes it, which they know they did. A CONSTRAINT
asserts a fact about someone else's software ("AMO refuses to
re-sign a version", "this forge numbers CI runs per repository"),
and it goes false with nobody watching. Only constraints get a
check.
The test, in one question: COULD THIS NOTE BECOME FALSE WITHOUT
ANYONE EDITING IT? If no, leave this empty. Sharper still: is the
thing this note describes YOURS TO CHANGE? If yes it is a
decision. Measured against a real corpus, every note that earned a
check was about somebody else's software.
Three that look like candidates and are not: a resume pointer or
"current state" note (goes stale fastest, but the cure is to
update it, not to check it); a measurement of your own system (it
goes false because you changed something, and you knew); and a
decision that RESTS on someone else's behaviour (check the note
asserting the fact, not the decision).
A command, a path, a URL, a query. Prose is allowed; something
runnable is better.
expires_when: The STATE that ends it — deliberately not a date.
"When Forgejo issues run numbers per workflow rather than per
repository", not "in six months". Constraints expire when the
ground moves, not on a schedule.
force: Bypass the near-duplicate gate. By default, if a title- or
meaning-similar note already exists in the same project, creation is
BLOCKED and the existing note's id is returned so you update it
@@ -161,6 +190,8 @@ async def create_note(
body=body,
tags=tags,
project_id=project_id or None,
verify_with=verify_with,
expires_when=expires_when,
)
if system_ids:
await systems_svc.set_record_systems(uid, note.id, system_ids)
@@ -185,6 +216,9 @@ async def update_note(
project_id: int = 0,
system_ids: list[int] | None = None,
supersedes: list[int] | None = None,
verify_with: str = "",
expires_when: str = "",
clear: list[str] | None = None,
) -> dict:
"""Update an existing Scribe note. Only explicitly provided fields are changed.
@@ -199,6 +233,27 @@ async def update_note(
supersedes: Replace the ids of earlier notes this one replaces
(set-semantics). None = leave unchanged; [] = clear all. See
create_note for when to reach for it.
verify_with: How to check this note is still true. Almost every note
should leave this empty — that is the normal, finished state, not
a gap: an empty check is the marker for "this is a decision, there
is nothing to go and check". See create_note for the full
norm-vs-constraint test; the short form is "could this become
false without anyone editing it?".
expires_when: The STATE that ends it, not a date.
clear: Names of fields to UNSET — "verify_with", "expires_when".
Needed because "" means "leave this alone" here, so there is no
value that removes a field: an agent updating a body must not
silently wipe a check it was not asked about. A note that stops
being a constraint is cleared by naming the field, which cannot
happen by accident.
Rewriting `verify_with` drops the note's verification stamp: a stamp
certifies a particular check, and carrying it across a rewrite would vouch
for something nobody has looked at.
A task and a snippet are both REFUSED a check, with a message saying where
to go instead — a task's decay is its status, and a snippet has
verify_snippet.
"""
uid = current_user_id()
fields: dict = {}
@@ -210,7 +265,13 @@ async def update_note(
fields["tags"] = tags
if project_id:
fields["project_id"] = project_id
note = await notes_svc.update_note(uid, note_id, **fields)
if verify_with:
fields["verify_with"] = verify_with
if expires_when:
fields["expires_when"] = expires_when
note = await notes_svc.update_note(
uid, note_id, clear=clear or (), **fields
)
if note is None:
raise ValueError(f"note {note_id} not found")
if system_ids is not None:
@@ -263,11 +324,113 @@ async def find_duplicate_records(kind: str = "note", threshold: float = 0.0) ->
async def delete_note(note_id: int) -> dict:
"""Move a Scribe note to the trash (recoverable). Restore via restore(batch_id)."""
uid = current_user_id()
# Read the title BEFORE the delete: afterwards the row is trashed and the
# confirmation could only echo the number back. A deletion the operator
# cannot recognise is one they cannot tell was the wrong one.
# Fail-open: the title is a COURTESY on top of the delete, so a lookup
# that errors must not stop the delete happening. Same posture the
# staleness marker takes — a decoration may never break its payload.
try:
loaded = await notes_svc.get_note_for_user(uid, note_id)
title = getattr(loaded[0], "title", "") if loaded else ""
except Exception:
title = ""
batch = await trash_svc.delete(uid, "note", note_id)
if batch is None:
raise ValueError(f"note {note_id} not found")
return {"deleted_batch_id": batch,
"message": f"Note {note_id} moved to trash. Restore with restore('{batch}')."}
return {"deleted": note_id, "title": title, "deleted_batch_id": batch,
"message": f'Note {note_id} ("{title}") moved to trash. '
f"Restore with restore('{batch}')."}
async def notes_due_for_verification(
older_than_days: int = 0, project_id: int = 0, never_only: bool = False,
) -> dict:
"""Which notes assert a FACT that nobody has confirmed lately.
A corpus of notes holds two kinds of thing. Most are DECISIONS or records
of what happened — they have no truth value and cannot rot. A few assert a
fact about someone else's software: what a signing service does on a
duplicate upload, how a forge numbers its CI runs, what an updater
compares. Those go false silently, with nobody present, and a
cross-project reference note keeps being read as current by every project
that cites it.
This lists the second kind, oldest verification first, NEVER-CHECKED AT
THE TOP — a note nobody has ever confirmed is a claim with no evidence
behind it at all. Each row carries `verify_with` in full, because you are
about to go and run it, plus `expires_when` and `days_since_verified`.
Reach for it when curating, when a note's claim just contradicted what you
observed, or periodically. Then, per row: run the check, and call
mark_note_verified with what you found.
Notes with no `verify_with` never appear, and that is correct — they are
decisions, and there is nothing to go and check. Do not "fix" their
absence by giving them checks: this list is only worth reading while
everything on it genuinely can go false.
Args:
older_than_days: only notes last verified longer ago than this.
Never-checked notes always qualify — they are the most overdue
thing there is. 0 = no age filter.
project_id: narrow to one project. 0 = every project. Unlike the rules
sweep, this filter is safe: a note belongs to at most one project
outright, with none of the subscription and always-on paths that
would make a project filter UNDER-report a rule.
never_only: only notes nobody has ever verified.
"""
uid = current_user_id()
notes = await notes_svc.notes_due_for_verification(
uid,
older_than_days=older_than_days,
project_id=project_id or None,
never_only=never_only,
)
return {
"notes": [notes_svc.verification_row(n) for n in notes],
"total": len(notes),
}
async def mark_note_verified(note_id: int, still_true: bool = True) -> dict:
"""Record that you ran a note's check — and what it said.
Call this AFTER actually running the note's `verify_with`, never on the
strength of the claim sounding plausible. A stamp nobody earned is worse
than no stamp: it moves the note to the bottom of the sweep and buys the
claim another long silence.
`still_true=False` writes NOTHING. A note whose check failed is not in a
special state to be recorded — it is WRONG, and the honest next moves are
to correct it, supersede it, or find out why. So it stays at the top of
the sweep until someone deals with it, and the response tells you what the
note said would end it.
Args:
note_id: the note whose check you ran.
still_true: True if the check passed. False if the fact it asserts is
no longer true — say so, that is the outcome worth having.
"""
uid = current_user_id()
note = await notes_svc.mark_note_verified(note_id, uid, still_true)
if note is None:
raise ValueError(
f"note {note_id} not found, not writable by you, or carries no "
f"verify_with (nothing to verify is not the same as verified)"
)
data = notes_svc.verification_row(note)
data["verified"] = bool(still_true)
if not still_true:
data["next"] = (
"This note is no longer true and is still being read as current "
"by anything that cites it. Correct it with update_note, write "
"the replacement with create_note(supersedes=[...]), or clear its "
"check with update_note(clear=[\"verify_with\"]) if it has stopped "
"asserting a fact at all. It stays at the top of "
"notes_due_for_verification until one of those happens."
)
return data
def register(mcp) -> None:
@@ -278,5 +441,7 @@ def register(mcp) -> None:
update_note,
find_duplicate_records,
delete_note,
notes_due_for_verification,
mark_note_verified,
):
mcp.tool(name=fn.__name__)(fn)
+87 -7
View File
@@ -122,8 +122,9 @@ async def delete_rulebook(rulebook_id: int, confirmed: bool = False) -> dict:
"confirmed_required": True,
}
batch = await trash_svc.delete(uid, "rulebook", rulebook_id)
return {"deleted": rulebook_id, "deleted_batch_id": batch,
"message": f"Moved to trash. Restore with restore('{batch}')."}
return {"deleted": rulebook_id, "title": rb.title, "deleted_batch_id": batch,
"message": f'Rulebook {rulebook_id} ("{rb.title}") moved to trash. '
f"Restore with restore('{batch}')."}
# ── Topic CRUD ─────────────────────────────────────────────────────────
@@ -192,8 +193,9 @@ async def delete_topic(topic_id: int, confirmed: bool = False) -> dict:
"confirmed_required": True,
}
batch = await trash_svc.delete(uid, "topic", topic_id)
return {"deleted": topic_id, "deleted_batch_id": batch,
"message": f"Moved to trash. Restore with restore('{batch}')."}
return {"deleted": topic_id, "title": topic.title, "deleted_batch_id": batch,
"message": f'Topic {topic_id} ("{topic.title}") moved to trash. '
f"Restore with restore('{batch}')."}
# ── Rule CRUD ──────────────────────────────────────────────────────────
@@ -262,7 +264,16 @@ async def list_always_on_rules(project_id: int = 0) -> dict:
"""
uid = current_user_id()
rules = await rulebooks_svc.list_always_on_rules(uid, project_id=project_id)
return {"rules": [_rule_summary(r) for r in rules], "total": len(rules)}
return {
"rules": [_rule_summary(r) for r in rules],
"total": len(rules),
# A marker for the set you are now holding. It is not for you to read:
# the write-path hook carries it back and is told if these rules have
# moved since. Deliberately NOT on rules_payload's applicable_rules —
# that is a DIFFERENT set (subscription-derived), and one key name
# over two sets is how a comparison starts reporting phantom changes.
"rules_etag": rulebooks_svc.rules_etag(rules),
}
async def get_rule(rule_id: int) -> dict:
@@ -548,6 +559,73 @@ async def update_rule(
return await rulebooks_svc.rule_detail(uid, rule, system_ids)
async def rule_history(rule_id: int, version_id: int = 0) -> dict:
"""What a rule USED TO SAY, newest change first.
Read this before you argue with a rule, and before you rewrite one. A
rule that has been reworded may have been reworded for a reason you are
about to rediscover the hard way — and the wording it replaced is often
the fastest way to see what the current one is guarding against. The
rescoping of rule 79 is the case this exists for: the superseded
statement had to be hand-copied into a task log to survive the edit.
EACH ENTRY HOLDS THE TEXT THE EDIT REPLACED, not the text it introduced.
So "what did this say before the most recent change?" is the first entry,
and the text the change PRODUCED is the rule as it stands now — read that
with get_rule. Pair the two and you have the diff.
An empty history is ordinary and means the rule has never been reworded,
not that its history was lost. Nothing is written before milestone 323,
so a rule edited before then starts empty too.
Args:
rule_id: The rule whose history to read.
version_id: 0 (default) lists the history — when each change
happened, by whom, and the title as it then stood. Pass an id
from that list to read that snapshot IN FULL. The list omits
statement and why on purpose: a rule's statement runs to
thousands of characters, and a history carrying every field would
cost more to read than the answer is worth.
There is deliberately no restore. Putting an old wording back is a
decision, so it goes through update_rule — which snapshots what it
replaces, leaving the undo visible in the history like any other edit. A
one-click revert would erase the only record of why the rewrite happened.
"""
uid = current_user_id()
if version_id:
version = await rulebooks_svc.get_rule_version(rule_id, version_id, uid)
if version is None:
raise ValueError(
f"version {version_id} not found on rule {rule_id}"
)
return version.to_dict(include_text=True)
versions = await rulebooks_svc.list_rule_versions(rule_id, uid)
if versions is None:
raise ValueError(f"rule {rule_id} not found")
# Fail-open, like the deletes: a missing title must not turn a readable
# history into an error.
try:
rule = await rulebooks_svc.get_rule(rule_id, uid)
except Exception:
rule = None
return {
"rule_id": rule_id,
"title": rule.title if rule else "",
"versions": [v.to_dict(include_text=False) for v in versions],
"total": len(versions),
# Said in-band because an empty list is the ordinary case and reads
# like a missing feature otherwise.
"note": (
"Each entry holds the text the edit REPLACED. The current wording "
"is on the rule itself — get_rule(%d)." % rule_id
if versions else
"This rule has never been reworded."
),
}
async def delete_rule(rule_id: int, confirmed: bool = False) -> dict:
"""Move a rule to the trash (recoverable). Requires confirmed=True."""
uid = current_user_id()
@@ -563,8 +641,9 @@ async def delete_rule(rule_id: int, confirmed: bool = False) -> dict:
"confirmed_required": True,
}
batch = await trash_svc.delete(uid, "rule", rule_id)
return {"deleted": rule_id, "deleted_batch_id": batch,
"message": f"Moved to trash. Restore with restore('{batch}')."}
return {"deleted": rule_id, "title": rule.title, "deleted_batch_id": batch,
"message": f'Rule {rule_id} ("{rule.title}") moved to trash. '
f"Restore with restore('{batch}')."}
# ── Subscriptions ──────────────────────────────────────────────────────
@@ -832,5 +911,6 @@ def register(mcp) -> None:
suppress_topic_for_project, unsuppress_topic_for_project,
exclude_always_on_rulebook, include_always_on_rulebook,
rules_due_for_verification, mark_rule_verified,
rule_history,
):
mcp.tool(name=fn.__name__)(fn)
+11 -1
View File
@@ -494,9 +494,19 @@ async def delete_snippet(snippet_id: int) -> dict:
that should survive, prefer merge_snippets — that keeps the call sites.
"""
uid = current_user_id()
# Read before deleting so the confirmation can NAME what went — an id
# alone leaves the operator unable to tell which snippet this was.
# Fail-open: the title is a COURTESY on top of the delete, so a lookup
# that errors must not stop the delete happening. Same posture the
# staleness marker takes — a decoration may never break its payload.
try:
doomed = await snippets_svc.get_snippet(uid, snippet_id)
title = getattr(doomed, "title", "") if doomed else ""
except Exception:
title = ""
if not await snippets_svc.delete_snippet(uid, snippet_id):
raise ValueError(f"snippet {snippet_id} not found")
return {"deleted": True, "id": snippet_id}
return {"deleted": True, "id": snippet_id, "title": title}
async def merge_snippets(target_id: int, source_ids: list[int]) -> dict:
+14 -2
View File
@@ -364,11 +364,23 @@ async def delete_task(task_id: int) -> dict:
"""Move a Scribe task (or plan) to the trash (recoverable). Sub-tasks go with it.
Restore via restore(batch_id)."""
uid = current_user_id()
# Read the title BEFORE the delete: afterwards the row is trashed and the
# confirmation could only echo the number back. A deletion the operator
# cannot recognise is one they cannot tell was the wrong one.
# Fail-open: the title is a COURTESY on top of the delete, so a lookup
# that errors must not stop the delete happening. Same posture the
# staleness marker takes — a decoration may never break its payload.
try:
loaded = await notes_svc.get_note_for_user(uid, task_id)
title = getattr(loaded[0], "title", "") if loaded else ""
except Exception:
title = ""
batch = await trash_svc.delete(uid, "task", task_id)
if batch is None:
raise ValueError(f"task {task_id} not found")
return {"deleted_batch_id": batch,
"message": f"Task {task_id} moved to trash. Restore with restore('{batch}')."}
return {"deleted": task_id, "title": title, "deleted_batch_id": batch,
"message": f'Task {task_id} ("{title}") moved to trash. '
f"Restore with restore('{batch}')."}
def register(mcp) -> None:
+1
View File
@@ -33,6 +33,7 @@ from scribe.models.milestone import Milestone # noqa: E402, F401
from scribe.models.task_log import TaskLog # noqa: E402, F401
from scribe.models.note_draft import NoteDraft # noqa: E402, F401
from scribe.models.note_version import NoteVersion # noqa: E402, F401
from scribe.models.rule_version import RuleVersion # noqa: E402, F401
from scribe.models.note_supersession import NoteSupersession # noqa: E402, F401
from scribe.models.group import Group, GroupMembership # noqa: E402, F401
from scribe.models.share import NoteShare, ProjectShare # noqa: E402, F401
+5 -1
View File
@@ -51,7 +51,11 @@ class RuleEmbedding(Base):
"""One embedding vector per CHUNK of a rule (milestone 307, note 3026).
A SIBLING of NoteEmbedding rather than a generalisation of it, decided
deliberately:
deliberately. The reasoning below turned out to be the only written
statement of a rule Scribe applies to every record type, so it is now also
NOTE 3163 — "When a record type earns its own table" — with the worked
cases and the cost of leaving `notes`. Read that before splitting a record
type off; this stays here because it is where the decision was made.
- The embedding ROW could have been made polymorphic. The SEARCH could not.
`semantic_search_notes` is a long function of Note-specific scoping —
+41 -2
View File
@@ -94,9 +94,40 @@ class Note(Base, TimestampMixin, SoftDeleteMixin):
# name/language/signature/locations live here so they can be INDEXED. The
# body keeps the same facts in readable markdown and remains what gets
# embedded; this is a mirror for querying, not the source of truth for
# display. NULL on every row written before migration 0070, so readers fall
# back to parsing the body (see services/snippets.snippet_fields).
# display — and it is DERIVED, so every path that writes a snippet's body
# rewrites it too (services/snippets.recompose_data, called from
# notes.update_note). 0070 left it NULL on existing rows and
# snippets.backfill_snippet_data filled them at startup; readers still fall
# back to parsing the body when it is absent (snippet_fields).
data: Mapped[dict | None] = mapped_column(JSONB, nullable=True)
# The three fields that tell a CONSTRAINT apart from a NORM (milestone
# 317, migration 0092) — the same trio `rules` carries, and for the same
# reason. A norm is a decision: no truth value, changes only when its
# author changes it. A constraint asserts a fact about someone else's
# software and goes false with nobody watching. Only constraints get a
# check.
#
# `verify_with` is how to check it is still true; `expires_when` is the
# STATE that ends it, deliberately not a date — constraints expire when
# the ground moves, not on a schedule. `verified_at` NULL means never
# checked and sorts FIRST in the sweep: unexamined outranks
# examined-long-ago.
#
# These sit on `notes`, so every kind of row in this table has them, but
# only non-task, non-snippet records are OFFERED them (gated in
# services/notes.py). A task's decay is its status — a done issue records
# what happened and cannot go false — and a snippet already carries a
# richer, location-aware verdict in `data.verification`. On those rows
# these stay null, which is also what they mean.
#
# Most notes should leave all three empty. A null `verify_with` is not a
# gap; it is the marker for "this is a decision, there is nothing to go
# and check", and the sweep is only worth reading while that holds.
verify_with: Mapped[str | None] = mapped_column(Text, nullable=True)
expires_when: Mapped[str | None] = mapped_column(Text, nullable=True)
verified_at: Mapped[datetime | None] = mapped_column(
DateTime(timezone=True), nullable=True
)
__table_args__ = (
Index("ix_notes_tags", "tags", postgresql_using="gin"),
@@ -137,6 +168,14 @@ class Note(Base, TimestampMixin, SoftDeleteMixin):
"is_task": self.is_task,
"note_type": self.note_type or "note",
"task_kind": self.task_kind,
# Serialized unconditionally, like every other field a given row
# kind may not use (recurrence, started_at, the task fields). The
# DERIVED "last_verified" label is the one that appears only when
# a check exists — a raw projection of the row should not make a
# client branch on which keys are present.
"verify_with": self.verify_with or "",
"expires_when": self.expires_when or "",
"verified_at": iso(self.verified_at),
"created_at": iso(self.created_at),
"updated_at": iso(self.updated_at),
}
+91
View File
@@ -0,0 +1,91 @@
from datetime import datetime
from sqlalchemy import BigInteger, ForeignKey, Text
from sqlalchemy.orm import Mapped, mapped_column
from scribe.models import Base
from scribe.models.base import CreatedAtMixin, iso
class RuleVersion(Base, CreatedAtMixin):
"""One snapshot of a rule's text, taken before an edit overwrote it.
THE SIBLING NOTES ALREADY HAD. `note_versions` has existed for a long
time, and the design-system note calls its history "the changelog". Rules
— which BIND BEHAVIOUR on every session that loads them — had nothing, so
an edit destroyed what the rule used to say. Rescoping rule 79 on
2026-08-29 meant hand-copying the superseded statement into a task log to
keep it (#3237). The more consequential record had the weaker protection.
`CreatedAtMixin`, not `TimestampMixin`: a version is an EVENT. It is
written once and never updated, so an `updated_at` on it would be a column
that can only ever lie.
WHAT IS DELIBERATELY DIFFERENT FROM NoteVersion (milestone 323):
- `user_id` is the ACTOR — who made the edit — where NoteVersion's is the
owner, because `update_note` passes an owner-scoped id. For an audit
trail over a binding instruction, "who changed this" is the question
being asked, and a rule is editable by anyone with rulebook access.
- No `pin_kind` / `pin_label`. Those exist so a note's version can survive
autosave pruning. Nothing prunes here, so a pin would protect a row that
was never at risk.
- TEXT ONLY. A rule's Systems and its typed relations are edges with their
own lifecycle; folding them in would make one word, "version", mean two
different things — the rule's wording, and the rule's place in the
graph. `verified_at` is likewise absent: it is a stamp about a check,
not a property of the text, and step 2's snapshot is taken before
update_rule clears it precisely so the history shows the check that was
in force when this wording was written.
"""
__tablename__ = "rule_versions"
id: Mapped[int] = mapped_column(BigInteger, primary_key=True)
rule_id: Mapped[int] = mapped_column(
BigInteger, ForeignKey("rules.id", ondelete="CASCADE"), index=True
)
# The actor. SET NULL rather than CASCADE: deleting a user must not erase
# the history of the rules they edited — the edit still happened, and the
# rule is still binding because of it. NoteVersion cascades because a
# note's versions belong to its owner; a rule's belong to the rule.
user_id: Mapped[int | None] = mapped_column(
BigInteger, ForeignKey("users.id", ondelete="SET NULL"), nullable=True
)
title: Mapped[str] = mapped_column(Text, default="")
statement: Mapped[str] = mapped_column(Text, default="")
why: Mapped[str | None] = mapped_column(Text, nullable=True)
how_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True)
when_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True)
tier: Mapped[str | None] = mapped_column(Text, nullable=True)
verify_with: Mapped[str | None] = mapped_column(Text, nullable=True)
expires_when: Mapped[str | None] = mapped_column(Text, nullable=True)
def to_dict(self, include_text: bool = True) -> dict:
"""The row. `include_text=False` gives the listing form.
A rule's `statement` and `why` run to thousands of characters — rule
149's `why` alone is longer than most notes — so a history LIST that
carried every field would be unreadable and expensive. The listing
answers "when, and by whom"; opening one answers "and what did it
say". Same split NoteVersion makes with `include_body`.
"""
out: dict = {
"id": self.id,
"rule_id": self.rule_id,
"user_id": self.user_id,
"title": self.title,
"created_at": iso(self.created_at),
}
if include_text:
out.update({
"statement": self.statement,
"why": self.why or "",
"how_to_apply": self.how_to_apply or "",
"when_to_apply": self.when_to_apply or "",
"tier": self.tier or "",
"verify_with": self.verify_with or "",
"expires_when": self.expires_when or "",
})
return out
+12 -4
View File
@@ -1,4 +1,4 @@
"""Unified Knowledge endpoint — notes, tasks, plans, and processes in one queryable feed."""
"""Unified Knowledge endpoint — every record kind in one queryable feed."""
import logging
from quart import Blueprint, jsonify, request
@@ -6,12 +6,18 @@ from quart import Blueprint, jsonify, request
from scribe.auth import get_current_user_id, login_required
from scribe.routes.utils import parse_pagination
from scribe.services.access import label_shared_items
from scribe.services.knowledge import FACET_TYPES
logger = logging.getLogger(__name__)
knowledge_bp = Blueprint("knowledge", __name__, url_prefix="/api/knowledge")
_VALID_TYPES = {"note", "task", "plan", "process"}
# Derived from the service's facet table, never re-listed here. This set was a
# hand-kept copy and had drifted three kinds behind it: it admitted `plan`
# (retired in 0066) and rejected `issue` (shipped in 0065, 435 rows) and
# `snippet` — so the browse surface could not filter to the kinds it was
# already rendering badges for (#3128).
_VALID_TYPES = FACET_TYPES
_VALID_SORTS = {"modified", "created", "alpha", "type"}
@@ -21,7 +27,9 @@ async def list_knowledge():
"""Return paginated knowledge objects with optional filtering.
Query params:
type — one of note|task|plan|process (omit for all)
type — a facet from services.knowledge._FACETS: a record type
(note|process|snippet) or a task kind (task for any,
else work|issue|spike|plan). Omit for all.
tags — comma-separated tag filter (AND logic)
sort — modified|created|alpha|type (default: modified)
q — search query (semantic when provided, keyword fallback)
@@ -127,7 +135,7 @@ async def get_knowledge_batch():
@knowledge_bp.route("/tags", methods=["GET"])
@login_required
async def list_knowledge_tags():
"""Return all tags used across knowledge objects (excludes tasks)."""
"""Return all tags used across knowledge objects, narrowed to one facet."""
uid = get_current_user_id()
note_type = request.args.get("type", "").strip().lower() or None
+72 -1
View File
@@ -19,7 +19,10 @@ from scribe.services.notes import (
get_note_for_user,
get_or_create_note_by_title,
list_notes,
mark_note_verified,
notes_due_for_verification,
update_note,
verification_row,
)
from scribe.services.note_drafts import upsert_draft, get_draft, delete_draft
from scribe.services import dedup as dedup_svc
@@ -112,6 +115,8 @@ async def create_note_route():
priority=priority,
due_date=due_date,
note_type=note_type,
verify_with=data.get("verify_with"),
expires_when=data.get("expires_when"),
)
except ValueError as e:
return jsonify({"error": str(e)}), 400
@@ -248,7 +253,14 @@ async def update_note_route(note_id: int):
owner_uid = note_obj.user_id
data = await request.get_json()
fields = {}
for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"):
for key in (
"title", "body", "description", "parent_id", "project_id",
"milestone_id", "status", "priority", "note_type",
# A cleared form input arrives as "" and the service reads that as
# NULL (NULLABLE_NOTE_TEXT), so this door expresses "remove the check"
# with its own idiom and needs no `clear` list (milestone 317).
"verify_with", "expires_when",
):
if key in data:
fields[key] = data[key]
if "due_date" in data:
@@ -490,3 +502,62 @@ async def graph_route():
shared_tags = request.args.get("shared_tags", "false").lower() == "true"
graph = await build_note_graph(uid, project_id=project_id, include_shared_tags=shared_tags)
return jsonify(graph)
# ── The staleness sweep (milestone 317) ──────────────────────────────────────
# The web half of notes_due_for_verification / mark_note_verified. Same
# contract as the MCP door and the rules routes beside it — the service holds
# the behaviour, these two just parse and serialise.
@notes_bp.route("/due-for-verification", methods=["GET"])
@login_required
async def notes_due_route():
"""Notes that carry a check, oldest verification first, never-checked top.
Query params: older_than_days, project_id, never_only. A note with no
`verify_with` never appears — it is a decision, not a fact.
"""
uid = get_current_user_id()
args = request.args
try:
older = int(args.get("older_than_days", 0) or 0)
project = int(args.get("project_id", 0) or 0)
except ValueError:
return jsonify({"error": "older_than_days and project_id must be integers"}), 400
try:
notes = await notes_due_for_verification(
uid,
older_than_days=older,
project_id=project or None,
never_only=args.get("never_only", "").lower() in ("1", "true", "yes"),
)
except ValueError as exc:
# A 400, not a silently narrowed result: a filter that quietly answers
# a different question is the failure this whole surface exists to
# catch.
return jsonify({"error": str(exc)}), 400
return jsonify({
"notes": [verification_row(n) for n in notes],
"total": len(notes),
})
@notes_bp.route("/<int:note_id>/verify", methods=["POST"])
@login_required
async def mark_note_verified_route(note_id: int):
"""Record that the note's check was run. Body: {"still_true": bool}.
`still_true: false` writes nothing — a note whose check failed is wrong,
not in a recordable state — so it keeps its place at the top of the sweep.
"""
data = await request.get_json() or {}
uid = get_current_user_id()
still_true = bool(data.get("still_true", True))
note = await mark_note_verified(note_id, uid, still_true)
if note is None:
return jsonify({
"error": "note not found, not writable by you, or carries no verify_with"
}), 404
payload = verification_row(note)
payload["verified"] = still_true
return jsonify(payload)
+7
View File
@@ -138,6 +138,11 @@ async def write_path_prior_art():
or `canon:<snippet_id>`) already named this
session by the ledger arm (#2900); its own
channel, like the two above.
rules_etag (opt) — the marker the session was given when it loaded
its always-on rules (milestone 323). Sent back
so the server can say whether those rules have
MOVED since. Absent means the hook has nothing
stored, which is silence, not a mismatch.
shapes (opt) — comma-separated `kind:name` definitions the hook
found in (or enclosing) the payload, kind being
css|sym. The shape ledger's write-path feed
@@ -157,6 +162,7 @@ async def write_path_prior_art():
p.strip() for p in (request.args.get("exclude_derive") or "").split(",") if p.strip()
]
exclude_rule_ids = _int_list(request.args.get("exclude_rule_ids"))
rules_etag = (request.args.get("rules_etag") or "").strip()
shapes = _parse_shapes(request.args.get("shapes") or "")
api_key = getattr(g, "api_key", None)
may_stamp = api_key is None or getattr(api_key, "scope", "") == "write"
@@ -168,6 +174,7 @@ async def write_path_prior_art():
repo_key=repo_bindings_svc.normalize_repo_key(repo) if repo else "",
exclude_derive=exclude_derive,
exclude_rule_ids=exclude_rule_ids,
rules_etag=rules_etag,
)
return jsonify(result)
+36
View File
@@ -206,6 +206,42 @@ async def update_rule(rule_id: int):
return jsonify(await rulebooks_svc.rule_detail(uid, rule, data.get("system_ids")))
@rulebooks_bp.get("/rules/<int:rule_id>/versions")
@login_required
async def list_rule_versions(rule_id: int):
"""A rule's edit history, newest first.
Listing form only — a rule's `statement` and `why` run to thousands of
characters, so a history list carrying every field would be unreadable
and expensive to send. Open one for the text.
"""
uid = get_current_user_id()
versions = await rulebooks_svc.list_rule_versions(rule_id, uid)
if versions is None:
return jsonify({"error": "rule not found"}), 404
return jsonify({
"versions": [v.to_dict(include_text=False) for v in versions],
})
@rulebooks_bp.get("/rules/<int:rule_id>/versions/<int:version_id>")
@login_required
async def get_rule_version(rule_id: int, version_id: int):
"""One snapshot in full — what the rule said before that edit."""
uid = get_current_user_id()
version = await rulebooks_svc.get_rule_version(rule_id, version_id, uid)
if version is None:
return jsonify({"error": "version not found"}), 404
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
# 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.
@rulebooks_bp.post("/rules/<int:rule_id>/relations")
@login_required
async def relate_rules(rule_id: int):
+261 -9
View File
@@ -9,6 +9,7 @@ from scribe.models.note import Note
from scribe.models.note_draft import NoteDraft
from scribe.models.note_supersession import NoteSupersession
from scribe.models.note_version import NoteVersion
from scribe.models.rule_version import RuleVersion
from scribe.models.design_system import DesignSystem, DesignToken
from scribe.models.note_usage import NoteUsageEvent
from scribe.models.canonical_system import CanonicalSystem
@@ -50,8 +51,19 @@ logger = logging.getLogger(__name__)
# ones (reference/hook) travel too, cheaply, and the next refresh refreshes them.
# v10 (2026-08) added projects.inception + project_rulebook_exclusions
# (milestone 297): the WHY a project inherits what it does, and its opt-outs.
# v11 (2026-08) added the note verification trio — notes.verify_with /
# expires_when / verified_at (milestone 317).
# v12 (2026-08) closed #3182: the nine Note columns that had been missing for
# years (note_type, task_kind, arose_from_id, data, description, the
# recurrence pair, started_at, completed_at), plus milestones.body — which IS
# the plan — and repo_bindings.ref. Until v12 a restore reported success and
# handed back a corpus with every snippet and process flattened into a plain
# note, every issue and spike into `work`, and every plan reduced to a title.
# _COLUMN_EXCLUSIONS and its guard landed with it, so the next such column
# fails the build instead.
# v13 (2026-08) added rule_versions — a rule's edit history (milestone 323).
# Bump when the serialized schema changes.
BACKUP_VERSION = 10
BACKUP_VERSION = 13
# Every table this backup carries, by its REAL name. Paired with _NOT_INCLUDED
# below, these two lists must together account for the entire schema — which is
@@ -77,6 +89,9 @@ _BACKED_UP = [
"canonical_systems",
# v10 (2026-08): a rule's area tag and its typed edges (milestone 307).
"rule_systems", "rule_relations",
# v13 (2026-08): a rule's edit history (milestone 323). note_versions has
# always travelled; its sibling has no excuse not to.
"rule_versions",
]
# Tables intentionally NOT in the backup, surfaced in the payload so the gap is
@@ -108,6 +123,86 @@ _NOT_INCLUDED = [
]
# Columns a backed-up table deliberately does NOT export, per table. Paired
# with the column-coverage guard in tests/test_services_backup.py, this is
# _NOT_INCLUDED's shape one level down — and it exists because the table guard
# could not see the failure it was written to stop.
#
# #2293 was six whole tables missing. #3182 was NINE COLUMNS missing from a
# table that had been "covered" for years: note_type and task_kind, so every
# snippet and process restored as a plain note and every issue and spike as
# `work`; arose_from_id, so every provenance edge vanished; the recurrence
# pair, so recurring tasks stopped recurring; milestones.body, which IS the
# plan; repo_bindings.ref, the branch a ledger follows. Every one arrived the
# same way — a column added to the model and the migration, both of which fail
# loudly, and then never added to the serialiser, which fails silently.
#
# So: a new column on a backed-up table now fails the build unless it is either
# exported or named here with a reason. "I forgot" is no longer expressible.
_COLUMN_EXCLUSIONS: dict[str, set[str]] = {
# Trash is not exported at all, so neither is the batch id that groups a
# deletion for restore(). Uniform across every soft-deletable table.
"users": set(),
"projects": {
"deleted_at", "deleted_batch_id",
# Credentials-adjacent, same reasoning as api_keys and
# forge_connections: a restored project falls back to keyring-by-host
# resolution, which is the documented unpinned behaviour (#2778).
"forge_connection_id",
},
"milestones": {"deleted_at", "deleted_batch_id"},
"notes": {"deleted_at", "deleted_batch_id"},
"task_logs": set(),
"note_drafts": set(),
"note_versions": set(),
"settings": set(),
"rulebooks": {"deleted_at", "deleted_batch_id"},
"rulebook_topics": {"deleted_at", "deleted_batch_id"},
"rules": {"deleted_at", "deleted_batch_id"},
"systems": {
"deleted_at", "deleted_batch_id",
# Travels as `canonical_slug`: the catalog is global and its ids are
# per-install, so an id would restore pointing at whatever area
# happened to land on that number.
"canonical_id",
"created_at", "updated_at",
},
"canonical_systems": {
"deleted_at", "deleted_batch_id",
# Matched on SLUG at restore, so a target install that already seeded
# the standard vocabulary reuses its own rows rather than colliding.
"id", "created_at", "updated_at",
},
# Edge tables: the id is regenerated on insert, and the pair IS the row.
"record_systems": {"id", "created_at"},
"note_supersessions": {"id", "created_at"},
"rule_relations": {"id", "created_at"},
"note_usage_events": {"id"},
"design_systems": {"deleted_at", "deleted_batch_id", "created_at", "updated_at"},
"design_tokens": {"deleted_at", "deleted_batch_id", "created_at", "updated_at"},
"repo_bindings": {"id", "created_at", "updated_at"},
# Everything travels. A version row IS the audit trail, so a column left
# behind is a fact about a binding instruction that no longer exists
# anywhere.
"rule_versions": set(),
# Serialised via the model's own to_dict(), so a column reaches the backup
# the moment it reaches that method — and the guard still catches one that
# reaches neither.
"code_shapes": {
# The PROPOSER's standing suggestion for an unclassified row, not a
# judgment: "looks like an instance of #N", or "repeats with no canon".
# Every refresh recomputes it and a judgment clears it, so carrying it
# would restore stale machine guesses over a tree the proposer has not
# seen. Same reasoning as code_shape_consumers in _NOT_INCLUDED —
# derived data is regenerated, never restored.
"proposed_snippet_id", "proposal_basis", "proposal_score",
"proposal_group", "proposed_at", "proposed_sha",
},
"code_shape_events": set(),
"code_shape_uses": set(),
}
def _dt(val: str | None) -> datetime:
return datetime.fromisoformat(val) if val else datetime.now(timezone.utc)
@@ -240,7 +335,13 @@ def _code_shape_use_rows(rows) -> list[dict]:
def _repo_binding_rows(rows) -> list[dict]:
return [
{"user_id": r.user_id, "project_id": r.project_id, "repo_key": r.repo_key}
# `ref` is the branch this binding's ledger follows (#2873). Without
# it a restored binding silently falls back to the default branch, and
# the shape ledger starts accounting for a different tree (#3182).
{
"user_id": r.user_id, "project_id": r.project_id,
"repo_key": r.repo_key, "ref": r.ref,
}
for r in rows
]
@@ -282,6 +383,11 @@ def _milestone_rows(rows) -> list[dict]:
{
"id": m.id, "user_id": m.user_id, "project_id": m.project_id,
"title": m.title, "description": m.description, "status": m.status,
# THE PLAN. A milestone IS the plan (0066) and `body` is its
# design and intent; `description` is only the one-line summary.
# Dropping this restored every plan as a title with no reasoning
# behind it (#3182).
"body": m.body,
"order_index": m.order_index,
"created_at": m.created_at.isoformat(),
"updated_at": m.updated_at.isoformat(),
@@ -294,12 +400,45 @@ def _note_rows(rows) -> list[dict]:
return [
{
"id": n.id, "user_id": n.user_id, "title": n.title, "body": n.body,
"description": n.description,
"tags": n.tags or [], "parent_id": n.parent_id,
"project_id": n.project_id, "milestone_id": n.milestone_id,
"status": n.status, "priority": n.priority,
"due_date": n.due_date.isoformat() if n.due_date else None,
"created_at": n.created_at.isoformat(),
"updated_at": n.updated_at.isoformat(),
# WHAT KIND OF RECORD THIS IS — both typing axes (#3182). Missing
# until now, which meant a restore reported success and handed back
# a corpus with every snippet and process flattened into a plain
# note and every issue and spike into `work`. Nothing recomputes
# these; the vocabulary is simply gone.
"note_type": n.note_type,
"task_kind": n.task_kind,
# Provenance — which record caused this one. Re-mapped in the
# second pass beside parent_id, never here: the value is an id in
# the SOURCE database.
"arose_from_id": n.arose_from_id,
# The queryable mirror. The only one of these that would self-heal
# (backfill_snippet_data rebuilds it from the body at startup), but
# a restore should not hand back a corpus that needs a restart to
# become searchable by location.
"data": n.data,
# Lifecycle: when the work actually started and finished, and the
# recurrence rule that makes a task come back. Without these a
# restored recurring task simply stops recurring.
"started_at": n.started_at.isoformat() if n.started_at else None,
"completed_at": n.completed_at.isoformat() if n.completed_at else None,
"recurrence_rule": n.recurrence_rule,
"recurrence_next_spawn_at": (
n.recurrence_next_spawn_at.isoformat()
if n.recurrence_next_spawn_at else None
),
# The verification trio (milestone 317, migration 0092). Operator
# judgment — "somebody checked this fact, and this is when" —
# which nothing can recompute.
"verify_with": n.verify_with,
"expires_when": n.expires_when,
"verified_at": n.verified_at.isoformat() if n.verified_at else None,
}
for n in rows
]
@@ -342,6 +481,23 @@ def _note_version_rows(rows) -> list[dict]:
]
def _rule_version_rows(rows) -> list[dict]:
"""A rule's edit history. Sibling of _note_version_rows, and it travels for
the same reason: a version is the only record of what a binding
instruction used to say, and nothing can recompute it."""
return [
{
"id": rv.id, "rule_id": rv.rule_id, "user_id": rv.user_id,
"title": rv.title, "statement": rv.statement, "why": rv.why,
"how_to_apply": rv.how_to_apply, "when_to_apply": rv.when_to_apply,
"tier": rv.tier, "verify_with": rv.verify_with,
"expires_when": rv.expires_when,
"created_at": rv.created_at.isoformat(),
}
for rv in rows
]
def _setting_rows(rows) -> list[dict]:
return [{"user_id": s.user_id, "key": s.key, "value": s.value} for s in rows]
@@ -423,6 +579,9 @@ async def export_full_backup() -> dict:
note_versions = (await session.execute(
select(NoteVersion).order_by(NoteVersion.note_id, NoteVersion.id)
)).scalars().all()
rule_versions = (await session.execute(
select(RuleVersion).order_by(RuleVersion.rule_id, RuleVersion.id)
)).scalars().all()
settings = (await session.execute(select(Setting))).scalars().all()
systems = (await session.execute(select(System))).scalars().all()
canonical_systems = (await session.execute(
@@ -487,6 +646,7 @@ async def export_full_backup() -> dict:
"task_logs": _task_log_rows(task_logs),
"note_drafts": _note_draft_rows(note_drafts),
"note_versions": _note_version_rows(note_versions),
"rule_versions": _rule_version_rows(rule_versions),
"settings": _setting_rows(settings),
"rulebooks": _rulebook_rows(rulebooks),
"rulebook_topics": _topic_rows(topics),
@@ -623,6 +783,14 @@ async def export_user_backup(user_id: int) -> dict:
.join(CanonicalSystem, CanonicalSystem.id == rule_systems_t.c.canonical_id)
.where(rule_systems_t.c.rule_id.in_(_rule_ids))
)).all() if _rule_ids else []
# Scoped through the RULE, not the version's user_id. That column is
# the ACTOR (milestone 323), so filtering on it would carry the
# versions this user wrote on someone ELSE's rule and drop the ones
# someone else wrote on theirs — the opposite of a per-user export.
rule_versions = (await session.execute(
select(RuleVersion).where(RuleVersion.rule_id.in_(_rule_ids))
.order_by(RuleVersion.rule_id, RuleVersion.id)
)).scalars().all() if _rule_ids else []
rule_relations = (await session.execute(
select(RuleRelation).where(
RuleRelation.from_rule_id.in_(_rule_ids),
@@ -671,6 +839,7 @@ async def export_user_backup(user_id: int) -> dict:
"task_logs": _task_log_rows(task_logs),
"note_drafts": _note_draft_rows(note_drafts),
"note_versions": _note_version_rows(note_versions),
"rule_versions": _rule_version_rows(rule_versions),
"settings": _setting_rows(settings),
"rulebooks": _rulebook_rows(rulebooks),
"rulebook_topics": _topic_rows(topics),
@@ -747,11 +916,29 @@ async def _restore_v1(data: dict) -> dict:
body=n_data.get("body", ""),
tags=n_data.get("tags", []),
parent_id=None, # patched below
arose_from_id=None, # patched below, same reason
description=n_data.get("description"),
note_type=n_data.get("note_type") or "note",
task_kind=n_data.get("task_kind") or "work",
data=n_data.get("data"),
started_at=_dt_or_none(n_data.get("started_at")),
completed_at=_dt_or_none(n_data.get("completed_at")),
recurrence_rule=n_data.get("recurrence_rule"),
recurrence_next_spawn_at=_dt_or_none(
n_data.get("recurrence_next_spawn_at")
),
status=n_data.get("status"),
priority=n_data.get("priority"),
due_date=_d(n_data.get("due_date")),
created_at=_dt(n_data.get("created_at")),
updated_at=_dt(n_data.get("updated_at")),
verify_with=n_data.get("verify_with"),
expires_when=n_data.get("expires_when"),
# _dt_or_none, NOT _dt: an absent stamp must stay absent. _dt
# substitutes now(), which would restore every never-checked
# note as checked at the moment of the restore — inverting the
# one signal the sweep reads.
verified_at=_dt_or_none(n_data.get("verified_at")),
)
session.add(note)
await session.flush()
@@ -759,14 +946,25 @@ async def _restore_v1(data: dict) -> dict:
note_id_map[old_id] = note.id
stats["notes"] += 1
# Patch parent_id now that all notes have new IDs
# Patch the two note->note edges now that every note has a new id.
# Both are ids in the SOURCE database, so writing either straight into
# the constructor would point at whatever record happens to hold that
# number here — a restore that succeeds and silently re-parents (#3182).
# An edge whose target did not survive the import is left NULL rather
# than guessed at.
for n_data in data.get("notes", []):
old_id = n_data.get("id")
if not old_id or old_id not in note_id_map:
continue
note_row = await session.get(Note, note_id_map[old_id])
if note_row is None:
continue
old_parent = n_data.get("parent_id")
if old_id and old_parent and old_id in note_id_map and old_parent in note_id_map:
note_row = await session.get(Note, note_id_map[old_id])
if note_row:
note_row.parent_id = note_id_map[old_parent]
if old_parent and old_parent in note_id_map:
note_row.parent_id = note_id_map[old_parent]
old_origin = n_data.get("arose_from_id")
if old_origin and old_origin in note_id_map:
note_row.arose_from_id = note_id_map[old_origin]
for s_data in data.get("settings", []):
mapped_user_id = user_id_map.get(s_data.get("user_id", 0))
@@ -799,7 +997,7 @@ async def _restore_v2(data: dict) -> dict:
"design_tokens": 0, "note_usage_events": 0, "repo_bindings": 0,
"note_supersessions": 0, "code_shapes": 0, "code_shape_events": 0,
"code_shape_uses": 0, "canonical_systems": 0,
"rule_systems": 0, "rule_relations": 0,
"rule_systems": 0, "rule_relations": 0, "rule_versions": 0,
}
async with async_session() as session:
@@ -859,6 +1057,7 @@ async def _restore_v2(data: dict) -> dict:
project_id=mapped_pid,
title=m_data.get("title", ""),
description=m_data.get("description"),
body=m_data.get("body"),
status=m_data.get("status", "active"),
order_index=m_data.get("order_index", 0),
created_at=_dt(m_data.get("created_at")),
@@ -871,6 +1070,7 @@ async def _restore_v2(data: dict) -> dict:
# 4a. Notes — first pass (no parent_id yet)
notes_with_parents: list[tuple[int, int]] = [] # (new_note_id, old_parent_id)
notes_with_origins: list[tuple[int, int]] = [] # (new_note_id, old_arose_from_id)
for n_data in data.get("notes", []):
mapped_uid = user_id_map.get(n_data.get("user_id", 0))
if mapped_uid is None:
@@ -881,6 +1081,17 @@ async def _restore_v2(data: dict) -> dict:
body=n_data.get("body", ""),
tags=n_data.get("tags", []),
parent_id=None,
arose_from_id=None, # patched below, same reason as parent_id
description=n_data.get("description"),
note_type=n_data.get("note_type") or "note",
task_kind=n_data.get("task_kind") or "work",
data=n_data.get("data"),
started_at=_dt_or_none(n_data.get("started_at")),
completed_at=_dt_or_none(n_data.get("completed_at")),
recurrence_rule=n_data.get("recurrence_rule"),
recurrence_next_spawn_at=_dt_or_none(
n_data.get("recurrence_next_spawn_at")
),
project_id=project_id_map.get(n_data["project_id"]) if n_data.get("project_id") else None,
milestone_id=milestone_id_map.get(n_data["milestone_id"]) if n_data.get("milestone_id") else None,
status=n_data.get("status"),
@@ -888,21 +1099,35 @@ async def _restore_v2(data: dict) -> dict:
due_date=_d(n_data.get("due_date")),
created_at=_dt(n_data.get("created_at")),
updated_at=_dt(n_data.get("updated_at")),
verify_with=n_data.get("verify_with"),
expires_when=n_data.get("expires_when"),
# _dt_or_none — see the note on the other restore path.
verified_at=_dt_or_none(n_data.get("verified_at")),
)
session.add(note)
await session.flush()
note_id_map[n_data["id"]] = note.id
if n_data.get("parent_id"):
notes_with_parents.append((note.id, n_data["parent_id"]))
if n_data.get("arose_from_id"):
notes_with_origins.append((note.id, n_data["arose_from_id"]))
stats["notes"] += 1
# 4b. Patch parent_id
# 4b. Patch the note->note edges. Deferred for the same reason
# parent_id always has been: these are ids in the SOURCE database
# (#3182). An edge whose target did not survive stays NULL.
for new_note_id, old_parent_id in notes_with_parents:
new_parent_id = note_id_map.get(old_parent_id)
if new_parent_id:
note_row = await session.get(Note, new_note_id)
if note_row:
note_row.parent_id = new_parent_id
for new_note_id, old_origin_id in notes_with_origins:
new_origin_id = note_id_map.get(old_origin_id)
if new_origin_id:
note_row = await session.get(Note, new_note_id)
if note_row:
note_row.arose_from_id = new_origin_id
# 5. TaskLogs
for tl_data in data.get("task_logs", []):
@@ -1141,6 +1366,32 @@ async def _restore_v2(data: dict) -> dict:
))
stats["rule_relations"] += 1
# A rule's edit history (milestone 323). Must come after the rules
# themselves — rule_id_map is only populated above — and both ids are
# ids in the SOURCE database, which is #3182's arose_from_id trap.
for rv in data.get("rule_versions", []):
mapped_rid = rule_id_map.get(rv.get("rule_id", 0))
if mapped_rid is None:
continue
# Unlike NoteVersion, an unmappable user does NOT drop the row.
# user_id is the ACTOR and is nullable by design: the column is
# SET NULL precisely so history outlives the account that wrote
# it. Skipping here would delete the record the FK preserves.
session.add(RuleVersion(
rule_id=mapped_rid,
user_id=user_id_map.get(rv.get("user_id") or 0),
title=rv.get("title", ""),
statement=rv.get("statement", ""),
why=rv.get("why"),
how_to_apply=rv.get("how_to_apply"),
when_to_apply=rv.get("when_to_apply"),
tier=rv.get("tier"),
verify_with=rv.get("verify_with"),
expires_when=rv.get("expires_when"),
created_at=_dt(rv.get("created_at")),
))
stats["rule_versions"] += 1
# 15. Systems
for sy_data in data.get("systems", []):
mapped_uid = user_id_map.get(sy_data.get("user_id", 0))
@@ -1255,6 +1506,7 @@ async def _restore_v2(data: dict) -> dict:
session.add(RepoBinding(
user_id=mapped_uid, project_id=mapped_pid,
repo_key=rb_data.get("repo_key", ""),
ref=rb_data.get("ref"),
))
stats["repo_bindings"] += 1
+116 -59
View File
@@ -1,4 +1,4 @@
"""Knowledge service — unified query across notes, tasks, plans, and processes.
"""Knowledge service — one query across every record kind Scribe holds.
ACL (rules #47/#78, decision note 2094): these queries were owner-only until
2026-07-25, which meant a record shared with you could be opened by id but never
@@ -265,22 +265,94 @@ def _note_to_item(note: Note) -> dict:
return item
def _apply_type_filter(stmt, note_type: str | None):
"""Apply the type facet to a Note select.
# What each type facet MEANS, once, for every arm that has to know.
#
# The vocabulary spans BOTH typing axes — `note_type` for non-task records and
# `task_kind` for tasks — so a facet cannot be a filter on one column, which is
# why this is a table rather than a chain of ifs. Each entry is
# (is_task, the value pinned on that axis); None pins nothing, i.e. every task.
#
# It is a table because the alternative had already gone wrong. The predicate
# was written three times — a SQL if-chain, a Python if-chain over semantic
# candidates, and a ternary computing the `is_task` pre-filter — and the three
# only agreed by luck. Adding `issue` to the SQL arm alone (the obvious edit,
# and the one #3128 was about to make) would have set the pre-filter to
# is_task=False, handed the Python arm a candidate set containing no tasks at
# all, and returned an empty semantic half for the Issues facet forever, with
# nothing red anywhere. A new facet is now one row here.
#
# `plan` is retired (0066) but kept: 90 legacy plan-tasks exist and a facet
# they answer to costs one line. It simply has no chip in the UI any more.
_FACETS: dict[str, tuple[bool, str | None]] = {
"task": (True, None),
"work": (True, "work"),
"issue": (True, "issue"),
"spike": (True, "spike"),
"plan": (True, "plan"),
"note": (False, "note"),
"process": (False, "process"),
"snippet": (False, "snippet"),
}
'task' = any task (status not null); 'plan' = a task with task_kind='plan';
any other non-empty type = a non-task note of that note_type; None = all.
# The non-task record types, for the counts query. Derived so it cannot drift
# from the table above.
NON_TASK_FACETS = tuple(
value for _is_task, value in _FACETS.values() if not _is_task and value
)
Trashed rows (deleted_at set) are always excluded.
# The whole vocabulary, for the door's request validation — public so the route
# validates against the same table the query reads instead of a hand-kept copy.
FACET_TYPES = frozenset(_FACETS)
# An unrecognised facet resolves to "a non-task note whose note_type is that
# string" — which matches nothing, since no row stores an unknown type. That is
# the behaviour the old if-chain had by falling through, and it is the right
# one: a typo should return an empty list, never the whole corpus.
def _facet(note_type: str) -> tuple[bool, str | None]:
return _FACETS.get(note_type, (False, note_type))
def facet_is_task(note_type: str | None) -> bool | None:
"""The `is_task` pre-filter a facet implies — None when it spans both.
Used to narrow the semantic candidate set before it is fetched. Reads the
same table `_apply_type_filter` and `matches_facet` read, so the pre-filter
can no longer disagree with the predicate it is meant to anticipate.
"""
if not note_type:
return None
return _facet(note_type)[0]
def matches_facet(note, note_type: str | None) -> bool:
"""The Python dialect of `_apply_type_filter`, for candidates the vector
search has already fetched — there is no query left to narrow.
Generated from the same table, so this is a translation rather than a
second implementation. Note the `not note.is_task` arm: the hand-written
version omitted it and was saved only by the upstream pre-filter.
"""
if not note_type:
return True
is_task, value = _facet(note_type)
if is_task:
return note.is_task and (value is None or note.task_kind == value)
return not note.is_task and note.note_type == value
def _apply_type_filter(stmt, note_type: str | None):
"""Apply the type facet to a Note select. Trashed rows are always excluded."""
stmt = stmt.where(Note.deleted_at.is_(None))
if note_type == "task":
return stmt.where(Note.status.isnot(None))
if note_type == "plan":
return stmt.where(Note.status.isnot(None)).where(Note.task_kind == "plan")
if note_type:
return stmt.where(Note.note_type == note_type).where(Note.status.is_(None))
return stmt
if not note_type:
return stmt
is_task, value = _facet(note_type)
if is_task:
stmt = stmt.where(Note.status.isnot(None))
if value is not None:
stmt = stmt.where(Note.task_kind == value)
return stmt
return stmt.where(Note.status.is_(None)).where(Note.note_type == value)
async def query_knowledge(
@@ -295,7 +367,7 @@ async def query_knowledge(
locations: dict[str, str] | None = None,
verification: str = "",
) -> tuple[list[dict], int]:
"""Query knowledge objects (non-task notes) with filters.
"""Query knowledge objects with filters.
`project_id` narrows to one project (None = every project).
@@ -424,7 +496,7 @@ async def _semantic_knowledge_search(
INTERACTIVE_SEARCH_THRESHOLD,
semantic_search_notes,
)
is_task_filter = True if note_type in ("task", "plan") else (False if note_type else None)
is_task_filter = facet_is_task(note_type)
import time as _time
_t0 = _time.perf_counter()
candidates = await semantic_search_notes(
@@ -454,11 +526,7 @@ async def _semantic_knowledge_search(
for _score, note in candidates:
if note.deleted_at is not None:
continue
if note_type == "task" and not note.is_task:
continue
elif note_type == "plan" and (not note.is_task or note.task_kind != "plan"):
continue
elif note_type and note_type not in ("task", "plan") and note.note_type != note_type:
if not matches_facet(note, note_type):
continue
if tags and not all(t in (note.tags or []) for t in tags):
continue
@@ -514,51 +582,40 @@ async def get_knowledge_counts(user_id: int, tags: list[str] | None = None) -> d
search would surface."""
visible = browsable_notes_clause(user_id)
async with async_session() as session:
# Count non-task types
stmt = (
select(Note.note_type, func.count(Note.id))
.where(visible)
.where(Note.status.is_(None))
.where(Note.deleted_at.is_(None))
.where(Note.note_type.in_(["note", "process"]))
.group_by(Note.note_type)
)
if tags:
for tag in tags:
def _scoped(stmt):
stmt = stmt.where(visible).where(Note.deleted_at.is_(None))
for tag in tags or []:
stmt = stmt.where(Note.tags.contains([tag]))
rows = list((await session.execute(stmt)).all())
counts = {row[0]: row[1] for row in rows}
return stmt
# Count tasks separately (is_task = status IS NOT NULL)
task_stmt = (
select(func.count(Note.id))
.where(visible)
# One grouped query per typing axis. The task half used to be a count
# for 'task' plus a second count for 'plan', which is why 'issue' —
# 17% of every task here — had no number to show: each kind needed its
# own query and nobody added one. Grouping by task_kind counts every
# kind, including ones added later, for the same two round-trips.
non_task = _scoped(
select(Note.note_type, func.count(Note.id))
.where(Note.status.is_(None))
.where(Note.note_type.in_(NON_TASK_FACETS))
).group_by(Note.note_type)
counts = {t: n for t, n in (await session.execute(non_task)).all()}
by_kind = _scoped(
select(Note.task_kind, func.count(Note.id))
.where(Note.status.isnot(None))
.where(Note.deleted_at.is_(None))
)
if tags:
for tag in tags:
task_stmt = task_stmt.where(Note.tags.contains([tag]))
task_count: int = (await session.execute(task_stmt)).scalar_one()
counts["task"] = task_count
).group_by(Note.task_kind)
kind_counts = {k: n for k, n in (await session.execute(by_kind)).all()}
# Plans are a subset of tasks (task_kind='plan'); counted for the facet
# but NOT added to total to avoid double-counting against "task".
plan_stmt = (
select(func.count(Note.id))
.where(visible)
.where(Note.status.isnot(None))
.where(Note.task_kind == "plan")
.where(Note.deleted_at.is_(None))
)
if tags:
for tag in tags:
plan_stmt = plan_stmt.where(Note.tags.contains([tag]))
counts["plan"] = (await session.execute(plan_stmt)).scalar_one()
# Kinds are SUBSETS of 'task' and are deliberately left out of the total —
# adding them would count every task twice.
counts["task"] = sum(kind_counts.values())
for kind, value in _FACETS.items():
if value[0] and value[1] is not None:
counts[kind] = kind_counts.get(kind, 0)
for t in ("note", "task", "plan", "process"):
for t in NON_TASK_FACETS:
counts.setdefault(t, 0)
counts["total"] = sum(counts[t] for t in ("note", "task", "process"))
counts["total"] = counts["task"] + sum(counts[t] for t in NON_TASK_FACETS)
return counts
+261 -1
View File
@@ -1,5 +1,6 @@
import logging
import re
from collections.abc import Iterable
from datetime import date, datetime, timezone
from sqlalchemy import func, or_, select, text
@@ -9,6 +10,52 @@ from scribe.models.note import Note, TaskKind, TaskPriority, TaskStatus
logger = logging.getLogger(__name__)
# The fields `snippets.parse_snippet_fields` reads. Writing any of them can
# change what a snippet's derived `data` mirror should say, so update_note
# recomposes the mirror when one moves. Kept here as a set of NAMES rather
# than imported, because it describes update_note's own `fields` dict, not the
# parser's signature.
_PARSED_FROM_BODY = frozenset({"title", "body", "tags"})
# Text fields where EMPTY MEANS NULL (milestone 317). The sweep's whole signal
# is `verify_with IS NULL` = "this is a decision, there is nothing to go and
# check". An empty string that is not NULL makes a norm look like a constraint
# nobody has verified, forever — and it would sit at the top of the sweep,
# since never-checked sorts first. Sibling of rulebooks.NULLABLE_RULE_TEXT.
NULLABLE_NOTE_TEXT = ("verify_with", "expires_when")
def guard_check_fields(status: str | None, note_type: str | None) -> None:
"""Raise unless a record in this shape may carry verify_with/expires_when.
Stated as an INVARIANT over the resulting record rather than a filter on
which fields a caller passed, so it also catches the sideways route: a
checked note being turned into a task, which no per-field gate would see.
Raises rather than dropping silently, for minted_kind's reason (#3129) — a
silently-corrected write is the defect that reasoning exists to end, and a
caller putting a check on the wrong record has an idea an error corrects
and a default hides. Lives at the service, not either door, so REST and MCP
cannot come to disagree about it.
"""
if status is not None:
raise ValueError(
"a task cannot carry verify_with/expires_when: a task's decay is "
"its status, and a done issue records what happened rather than "
"asserting something that can later go false. Put the check on the "
"note the fact lives in, or clear the check before making this a "
"task."
)
if note_type == "snippet":
raise ValueError(
"a snippet already has a check: verify_snippet(), which compares "
"the recorded location and code against the repo and expires its "
"own verdict when the code moves. verify_with/expires_when are the "
"free-text form, for prose notes that assert a fact about "
"something outside the repo."
)
def embed_note(note) -> None:
"""Refresh a note's embedding, fire-and-forget.
@@ -101,7 +148,16 @@ async def create_note(
task_kind: str = "work",
arose_from_id: int | None = None,
data: dict | None = None,
verify_with: str | None = None,
expires_when: str | None = None,
) -> Note:
# Empty means empty (NULLABLE_NOTE_TEXT), then the invariant. Both run
# before anything is written, so an illegal shape never reaches the table.
verify_with = verify_with or None
expires_when = expires_when or None
if verify_with or expires_when:
guard_check_fields(status, note_type)
# Validate status/priority here so the MCP create_task path (which passes
# them straight through) can't persist an out-of-enum value that the REST
# route would have rejected — there's no DB CHECK on notes.status.
@@ -145,6 +201,8 @@ async def create_note(
task_kind=task_kind,
arose_from_id=arose_from_id,
data=data,
verify_with=verify_with,
expires_when=expires_when,
)
session.add(note)
await session.commit()
@@ -397,7 +455,20 @@ def minted_kind(kind: str) -> str:
raise ValueError(f"kind must be one of {MINTABLE_KINDS}, got {kind!r}")
async def update_note(user_id: int, note_id: int, **fields: object) -> Note | None:
async def update_note(
user_id: int, note_id: int, clear: Iterable[str] = (), **fields: object,
) -> Note | None:
"""Partial update. `clear` names fields to UNSET; **fields carries values.
Clearing is explicit and separate because a nullable field cannot be
emptied by passing it: the MCP door reads "" as "leave this alone", so an
agent filling two fields does not wipe the others, and a note that stops
being a constraint genuinely needs its check removed. Naming the field is
the one form that cannot happen by accident. The REST door, where a
cleared form input arrives as "", reaches the same place through the
NULLABLE_NOTE_TEXT normalisation below — two idioms, one outcome.
(Same shape as rulebooks.update_rule, milestone 312 step 2.)
"""
async with async_session() as session:
result = await session.execute(
select(Note).where(Note.id == note_id, Note.user_id == user_id)
@@ -409,6 +480,10 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No
old_body = note.body
old_title = note.title
old_tags = list(note.tags or [])
check_before = note.verify_with
for key in clear:
if key in NULLABLE_NOTE_TEXT:
setattr(note, key, None)
for key, value in fields.items():
if not hasattr(note, key):
continue
@@ -438,7 +513,44 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No
)
elif key == "tags" and isinstance(value, list):
value = _normalize_tags(value)
elif key in NULLABLE_NOTE_TEXT:
value = value or None
elif key == "verified_at":
# Not settable here. A stamp says somebody performed THIS
# check, so it is written by the verification path and by a
# restore, never by an ordinary edit that could mint one for a
# check nobody ran.
continue
setattr(note, key, value)
# The invariant, over the RESULTING record rather than over what was
# passed — which is what catches a checked note being turned into a
# task. Raised before commit, so nothing is persisted.
if note.verify_with or note.expires_when:
guard_check_fields(note.status, note.note_type)
# A stamp certifies A CHECK, not a record. Rewrite or remove the check
# and the old stamp certifies something that no longer exists, so it is
# dropped and the note re-enters the sweep. The safe direction: a note
# wrongly listed as due costs one look; a note wrongly vouched for
# costs exactly what the sweep exists to catch.
if note.verify_with != check_before:
note.verified_at = None
# A snippet's `data` is DERIVED from its body — so a write that moves
# the body through this generic door must move the mirror with it
# (#3128). Without this, PATCH /api/notes/<snippet_id> {body} left the
# mirror behind, and snippet_fields PREFERS the mirror: the row went on
# reporting its old repo/path/symbol to prior-art recall while showing
# its new body. `update_snippet` composes the mirror itself and passes
# it explicitly, so an explicit `data` always wins — the caller that
# knows the field set beats the one that can only re-read the body.
if "data" not in fields and not _PARSED_FROM_BODY.isdisjoint(fields):
# Imported here, not at module scope: services/snippets.py calls
# back into this module (update_snippet -> update_note), so a
# top-level import is a cycle.
from scribe.services.snippets import (
SNIPPET_NOTE_TYPE, recompose_data,
)
if note.note_type == SNIPPET_NOTE_TYPE:
note.data = recompose_data(note)
# Auto-set lifecycle timestamps on status transitions
if "status" in fields:
_now = datetime.now(timezone.utc)
@@ -487,6 +599,154 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No
# permanent deletion. Both are reachable; neither is spelled `delete_note`.
# ── The sweep (milestone 317 step 3) ─────────────────────────────────────────
#
# A SIBLING of rulebooks.rules_due_for_verification, not a shared
# implementation, and deliberately so (note 3163). The row could have been
# shared; the QUERY cannot. That sweep scopes by rulebook ownership XOR project
# ownership because rules have no sharing ACL at all — no rule_shares, no
# can_read_rule. A note scopes by the note ACL, which is a different question
# with a different answer. What IS common — how a stamp reads, how old it is —
# lives in services/verification.py and is imported by both.
async def notes_due_for_verification(
user_id: int,
older_than_days: int = 0,
project_id: int | None = None,
never_only: bool = False,
) -> list[Note]:
"""Notes that carry a check, oldest verification first, never-checked top.
THE QUERY THE COLUMNS EXIST FOR. `verify_with` and `expires_when` are
storage; this is what turns them into something that gets acted on.
Without it, note decay is caught only when a human reads the note and
disagrees — which is the case where the note was already believed.
Ordered `verified_at` ASC **NULLS FIRST**: never-checked outranks
checked-long-ago, because a note nobody has ever confirmed is a claim with
no evidence behind it at all. Postgres sorts NULLs LAST on ASC by default,
so this is explicit — and getting it wrong would not error, it would
silently invert the one signal the sweep exists to carry.
Notes with no `verify_with` never appear. Not an omission: they are
decisions, there is nothing to go and check, and listing them would dilute
the result until nobody reads it.
Scoped with `browsable_notes_clause`, NOT the read scope (decision note
2094): a sweep is a passive surface, and a record shared one-to-one must
not arrive in one unasked.
Deliberately NOT filtered to non-task, non-snippet records even though the
write path (step 2) permits a check on nothing else. A row in that state
would be a row in an ILLEGAL state, and this is the one surface that could
tell somebody about it. Hiding it here to match the invariant would make
the sweep agree with a database it had stopped describing.
Args:
user_id: whose notes.
older_than_days: only notes last verified longer ago than this.
Never-checked notes always qualify — they are the most overdue
thing there is. 0 = no age filter. Negative raises: it would mean
"everything", which is a different question than the one asked,
answered silently.
project_id: narrow to one project. None = every project.
never_only: only notes that have never been verified.
"""
from datetime import timedelta
from scribe.services.access import browsable_notes_clause
if older_than_days < 0:
raise ValueError(
f"older_than_days must be >= 0, got {older_than_days}. A negative "
f"window silently means 'everything', which is not what any caller "
f"of a staleness sweep is asking."
)
async with async_session() as session:
# One statement, not a fetch-then-filter: the ordering below is the
# database's, so it cannot disagree with itself across two halves.
stmt = (
select(Note)
.where(
browsable_notes_clause(user_id),
Note.deleted_at.is_(None),
Note.verify_with.is_not(None),
)
)
if project_id is not None:
stmt = stmt.where(Note.project_id == project_id)
if never_only:
stmt = stmt.where(Note.verified_at.is_(None))
elif older_than_days > 0:
cutoff = datetime.now(timezone.utc) - timedelta(days=older_than_days)
stmt = stmt.where(
or_(Note.verified_at.is_(None), Note.verified_at < cutoff)
)
stmt = stmt.order_by(Note.verified_at.asc().nullsfirst(), Note.id)
return list((await session.execute(stmt)).scalars().all())
def verification_row(note: Note) -> dict:
"""One row of the sweep — the CHECK in full, unlike a listing.
The opposite call from a browse: here the caller is about to go and run the
check, so the text they need IS the payload rather than the bloat.
"""
from scribe.services.verification import (
days_since_verified,
last_verified_label,
)
return {
"id": note.id,
"title": note.title,
"project_id": note.project_id,
"verify_with": note.verify_with or "",
"expires_when": note.expires_when or "",
"last_verified": last_verified_label(note),
"days_since_verified": days_since_verified(note),
}
async def mark_note_verified(
note_id: int, user_id: int, still_true: bool = True,
) -> Note | None:
"""Stamp a note as verified — or, when the check FAILED, refuse to.
The asymmetry is the design: passing writes a stamp, failing writes
nothing. There is no "verified false" state, because a note whose check
failed is not a note in a special condition — it is a note that is WRONG,
and the honest resolutions are to correct it, supersede it, or find out
why. Recording the failure as a flag would let it sit there being false
with the sweep quietly satisfied that somebody had looked.
So a failed check leaves `verified_at` untouched and the note stays at the
top of the sweep until someone actually deals with it.
Write access, not read (rules 47/78): stamping is a mutation, and an
editor-share holder may make it while a viewer may not.
Returns None when the note is not found, not writable, or carries no
`verify_with` — nothing to verify is a different answer from verified.
"""
from scribe.services.access import can_write_note
async with async_session() as session:
note = (await session.execute(
select(Note).where(Note.id == note_id, Note.deleted_at.is_(None))
)).scalars().first()
if note is None or not note.verify_with:
return None
if not await can_write_note(user_id, note_id):
return None
if still_true:
note.verified_at = datetime.now(timezone.utc)
await session.commit()
await session.refresh(note)
return note
async def get_all_tags(user_id: int, q: str | None = None) -> list[str]:
async with async_session() as session:
if q:
+85 -4
View File
@@ -708,6 +708,7 @@ async def build_write_path_hint(
repo_key: str = "",
exclude_derive: list[str] | None = None,
exclude_rule_ids: list[int] | None = None,
rules_etag: str = "",
) -> dict:
"""Prior-art hint for the plugin's PreToolUse hook on Write/Edit.
@@ -951,7 +952,74 @@ async def build_write_path_hint(
derive = [d for d in found if d.get("key") not in skip]
except Exception:
logger.warning("write-time derive check failed", exc_info=True)
if not synced and not menu and not stamped and not divergence and not derive:
staleness: list[str] = []
# ── Have the rules moved under this session? (milestone 323) ───────
#
# THE CARRIER IS THE POINT. This hook already fires before a write — the
# moment acting on a stale rule actually costs something — and the check
# is one comparison against a marker the session already holds. No
# payload, no extra round trip, and nothing said when nothing moved.
#
# WHAT THIS CANNOT SEE, and a reader who finds an etag here will assume
# otherwise:
#
# what goes wrong | caught?
# ---------------------------------------------------|--------
# another session edits a rule mid-flight | yes
# the session is misremembering a rule read hours ago | yes
# compaction summarised the rules out of context | NO
#
# The third is the most common and this is blind to it: the etag was in
# context too and went with the rules. The SessionStart nudge is that
# case's only mechanism and must not be softened because this shipped.
#
# Fails open, like every other arm here: a staleness hint must never
# break a write.
if rules_etag:
try:
current = await rulebooks_svc.list_always_on_rules(
user_id, project_id=project_id or 0,
)
if rulebooks_svc.rules_etag(current) != rules_etag:
moved = rulebooks_svc.rules_moved_since(current, rules_etag)
held = rulebooks_svc.etag_count(rules_etag)
bits = []
if moved:
named = ", ".join(
f"#{r.id} \u201c{r.title}\u201d" for r in moved[:3]
)
more = len(moved) - 3
bits.append(
f"{named}" + (f", and {more} more" if more > 0 else "")
)
# A DELETED rule moves no timestamp and leaves no row to name,
# so the count is the only thing that can report the one change
# that takes an instruction OUT of force.
if held is not None and held != len(current):
delta = len(current) - held
bits.append(
f"{abs(delta)} rule(s) {'added' if delta > 0 else 'no longer in force'}"
)
if bits:
staleness.append(
"Your loaded rules have changed since this session "
"started — " + "; ".join(bits) + ". Re-read them with "
"list_always_on_rules() before relying on the set you "
"are holding."
)
except Exception:
logger.debug("write-path rules-etag arm failed", exc_info=True)
# The guard sits BELOW the staleness arm on purpose. A rules change is
# unconditional news — it does not become less true because this
# particular write happened to match no prior art — and this arm is one
# indexed query, only when the session actually sent a marker.
#
# The standing-rule arm further down is deliberately left on the far side
# of this guard: that one runs a SEMANTIC search, and moving it here would
# run an embedding query on every write in the session. Its gating is a
# separate question from this one (see the note on #3244).
if not staleness and not synced and not menu and not stamped and not divergence and not derive:
return empty
owners = await owner_names_for({
@@ -970,7 +1038,9 @@ async def build_write_path_hint(
for marker, item in menu:
rendered.append((item, marker, _owner_of(item), _foreign_language(item, target_lang)))
lines: list[str] = []
# Seeded with the staleness line, which is decided above the early
# return and so cannot wait for this list to exist.
lines: list[str] = list(staleness)
sync_note_ids: list[int] = []
if synced:
# The sync framing (#2708). Deliberately imperative about the record —
@@ -1208,7 +1278,10 @@ async def build_session_context(
its normalized key — triggers a one-line "bind this repo" hint so
the binding is self-healing.
Returns {"context": str, "rule_count": int, "project": dict | None}.
Returns {"context": str, "rule_count": int, "project": dict | None,
"rules_etag": str}. The etag is for the HOOK, not for the model — the
hook stores it and hands it back on each write so the server can say
whether these rules have moved since the session loaded them.
`context` is markdown ready to drop into `additionalContext`; it is capped
at _MAX_CHARS with an explicit truncation note so the hook can pass it
through verbatim.
@@ -1317,4 +1390,12 @@ async def build_session_context(
if len(context) > _MAX_CHARS:
context = context[:_MAX_CHARS].rstrip() + "\n\n…(truncated — call list_always_on_rules())"
return {"context": context, "rule_count": len(rules), "project": project_dict}
return {
"context": context,
"rule_count": len(rules),
"project": project_dict,
# Computed from the rules THIS payload was built from, not re-queried:
# the marker has to describe the set the session is actually holding,
# and a second query could disagree with the first.
"rules_etag": rulebooks_svc.rules_etag(rules),
}
+82
View File
@@ -0,0 +1,82 @@
"""A rule's edit history — the snapshot taken when an edit overwrites text.
Sibling of `note_versions`, and the differences are the whole design.
WHAT A VERSION IS FOR. It records what a rule USED TO SAY, because an update
overwrites that and nothing can recompute it. It is not a record of the rule's
lifecycle: a soft delete keeps the rule row whole and restorable, so there is
nothing to preserve at delete time, and anything that outlived a purge would be
data the operator explicitly asked to be gone.
WHAT IS DELIBERATELY NOT COPIED FROM note_versions (milestone 323). Both of its
guards defend against note autosave, which fires every 60 seconds and would
otherwise burn every slot. Rules have no autosave — every version here comes
from a deliberate `update_rule` — so both guards would only ever discard a real
edit:
- No `MIN_VERSION_INTERVAL_SECONDS`. Two deliberate edits four minutes apart
are two edits.
- No `MAX_VERSIONS` and no pruning. A rule is edited a handful of times in its
life; a cap invites losing the one edit somebody needed.
TEXT ONLY. A rule's topic, its Systems tags and its typed edges are not here.
They have their own lifecycle, and folding them in would make one word,
"version", mean two things — the rule's wording, and the rule's place in the
graph.
"""
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.rule_version import RuleVersion
# The columns a version carries, and therefore the ones whose change is worth
# a snapshot. `order_index` and `arose_from_id` are editable through
# update_rule and are deliberately absent: reordering a rulebook is not an
# edit to what any rule says, and a run of "moved rule 4 above rule 3"
# snapshots would bury the edits somebody is actually looking for.
SNAPSHOT_FIELDS = (
"title", "statement", "why", "how_to_apply", "when_to_apply",
"tier", "verify_with", "expires_when",
)
def snapshot(rule) -> dict:
"""The rule's text right now, as a plain dict.
Plain values rather than the ORM object because the caller has to hold
this ACROSS the mutation — a live `rule` would show the new text by the
time it was read, which is the one thing the snapshot must not do.
"""
return {f: getattr(rule, f) for f in SNAPSHOT_FIELDS}
def record_if_changed(session, rule, actor_user_id: int | None, before: dict):
"""Add a version holding `before`, unless the edit changed no text.
Git semantics, and not a nicety: `update_rule` is reached by callers that
resend every field, so without this a form saved twice would file a second
identical snapshot and the history would stop reading as a list of edits.
Session-BOUND, unlike `note_versions.create_version` which opens its own.
The version and the edit that caused it commit together or not at all;
a separate session could leave a snapshot behind after the update it
describes had failed, which is a history entry for an edit that never
happened.
"""
if snapshot(rule) == before:
return None
version = RuleVersion(
rule_id=rule.id, user_id=actor_user_id, **before,
)
session.add(version)
return version
async def list_versions(rule_id: int) -> list[RuleVersion]:
"""Newest first — the reader's question is "what did this just say?"."""
async with async_session() as session:
return list((await session.execute(
select(RuleVersion)
.where(RuleVersion.rule_id == rule_id)
.order_by(RuleVersion.created_at.desc(), RuleVersion.id.desc())
)).scalars().all())
+160 -21
View File
@@ -9,6 +9,7 @@ from __future__ import annotations
import logging
from collections.abc import Iterable
from datetime import datetime
from typing import Optional
from sqlalchemy import and_, delete as sql_delete, insert, or_, select
@@ -16,6 +17,12 @@ from sqlalchemy import and_, delete as sql_delete, insert, or_, select
from scribe.models import async_session
from scribe.models.system import System
from scribe.models.rulebook import Rulebook
from scribe.services.verification import (
days_since_verified as _days_since_verified,
last_verified_label as _last_verified_label,
)
from scribe.services import rule_versions
from scribe.models.rule_version import RuleVersion
logger = logging.getLogger(__name__)
@@ -311,19 +318,12 @@ def _valid_tier(tier: str) -> str:
return tier if tier in TIERS else "always_on"
def last_verified_label(rule: Rule) -> str | None:
"""How long ago the rule's check passed — None when it carries no check.
One helper because two surfaces need the same answer and the brief-dict
lesson in rule_brief's docstring is what happens otherwise: three copies
that had already drifted. `None` means "this rule is a decision, the
question does not apply"; "never" means "it is a fact and nobody has
confirmed it" — a distinction worth keeping, because the second is the
one worth acting on.
"""
if not rule.verify_with:
return None
return rule.verified_at.date().isoformat() if rule.verified_at else "never"
# Re-exported, not redefined. Notes gained the same trio in milestone 317 and
# this reading of it is genuinely common, so it moved to services/verification
# — the DRY win note 3163 names, as against sharing the QUERY, which the two
# record types cannot (a rule scopes by rulebook ownership, a note by the note
# ACL). Kept importable from here because callers already reach for it here.
last_verified_label = _last_verified_label
def rule_brief(rule: Rule, **extra) -> dict:
@@ -751,6 +751,10 @@ async def update_rule(
"verify_with", "expires_when",
}
check_before = rule.verify_with
# Captured BEFORE anything is written, and as plain values — this has
# to survive the mutation below. A rule's history is the only record
# of what it used to say; the edit itself destroys that.
text_before = rule_versions.snapshot(rule)
for key in clear:
if key in allowed and key in NULLABLE_RULE_TEXT:
setattr(rule, key, None)
@@ -774,12 +778,59 @@ async def update_rule(
# wrongly vouched for costs the thing the sweep exists to catch.
if rule.verify_with != check_before:
rule.verified_at = None
# Same session as the edit, so the two commit together. The snapshot
# holds the OLD verify_with — the check that was in force when that
# wording was written — which is why it is taken before the loop and
# not here.
rule_versions.record_if_changed(session, rule, user_id, text_before)
await session.commit()
await session.refresh(rule)
_refresh_rule_embedding(rule)
return rule
# ── Edit history (milestone 323) ───────────────────────────────────────
#
# The ACL-scoped reads live HERE rather than in services/rule_versions.py,
# and not by preference: rulebooks imports rule_versions for the write path,
# so the reverse import would be a cycle. The split is also the honest one —
# rule_versions owns what a version IS, this module owns who may read one.
async def list_rule_versions(rule_id: int, user_id: int):
"""A rule's history, newest first. None when the rule is not readable.
Scoped through the rule itself, never through the version's `user_id`:
that column is the ACTOR. Reading a rule's history is a question about
the RULE, so anyone who can read the rule can read what it used to say,
and anyone who cannot read the rule gets nothing — including the versions
they personally wrote, if the rule has since moved out of their reach.
"""
async with async_session() as session:
if await _fetch_owned_rule(session, rule_id, user_id) is None:
return None
return await rule_versions.list_versions(rule_id)
async def get_rule_version(rule_id: int, version_id: int, user_id: int):
"""One snapshot in full. None when the rule or the version is not found.
Takes the rule id as well as the version id so the ownership check has
something to run against BEFORE the version is read, and so a version id
from another rule cannot be read through a rule the caller does happen to
own — the check and the fetch have to agree about which rule is in play.
"""
async with async_session() as session:
if await _fetch_owned_rule(session, rule_id, user_id) is None:
return None
return (await session.execute(
select(RuleVersion).where(
RuleVersion.id == version_id,
RuleVersion.rule_id == rule_id,
)
)).scalar_one_or_none()
# ── Canon tags + typed edges (milestone 307) ───────────────────────────
async def set_rule_systems(
@@ -1363,6 +1414,101 @@ def rules_payload(applicable: dict) -> dict:
}
# ── The staleness marker (milestone 323 step 5) ────────────────────────
#
# WHAT THIS CAN AND CANNOT SEE. An etag catches a rule that MOVED after a
# session loaded it. It is not a general staleness check, and a reader who
# finds one here will assume it is:
#
# what goes wrong | caught?
# ---------------------------------------------------|--------
# another session edits a rule mid-flight | yes
# the session is misremembering a rule read hours ago | yes
# compaction summarised the rules out of context | NO
#
# The third is the most common and the marker is blind to it, because the
# etag was in context too and went with the rules. The SessionStart nudge is
# that case's only mechanism, and MUST NOT be softened because this exists —
# retiring something that covers the common case in favour of something that
# does not is the plausible mistake here.
_ETAG_EMPTY = "empty|0"
def rules_etag(rules: list) -> str:
"""A marker for "is the set you are holding still the current one?".
`max(updated_at)` alone is not enough: DELETING a rule moves no timestamp,
and that is the single change that takes an instruction OUT of force —
the one a session most needs to hear about. The count catches it.
Instance-agnostic (rule 115): it knows nothing about any particular
rulebook, and an install with one rule or none produces a stable marker
rather than an error. "No rules" must read as a state, not as a change,
or every session on a fresh install would be told its rules had moved.
"""
if not rules:
return _ETAG_EMPTY
# A decoration must not be able to break what it decorates. This is
# computed on the SessionStart path, where raising would cost the whole
# context payload to save a hint — so a row with no usable timestamp is
# skipped rather than compared, and a set with none degrades to a
# count-only marker instead of failing. Count-only still catches a rule
# added or deleted; it just cannot see an edit, which is the right way
# round to lose information.
stamps = [
r.updated_at for r in rules
if isinstance(getattr(r, "updated_at", None), datetime)
]
if not stamps:
return f"unknown|{len(rules)}"
return f"{max(stamps).isoformat()}|{len(rules)}"
async def rules_etag_for(user_id: int, project_id: int = 0) -> str:
"""The current marker for the set a session at this scope would hold.
Deliberately built from `list_always_on_rules` rather than from a
`max()/count()` aggregate. An aggregate would be cheaper, and would have
to restate that function's definition of the set — the always_on flag,
the project's inception exclusions, the tier filter. Two definitions of
"the session's rules" is how the marker starts disagreeing with the
rules, which is worse than materialising a few dozen rows.
"""
rules = await list_always_on_rules(user_id, project_id=project_id)
return rules_etag(rules)
def rules_moved_since(rules: list, held_etag: str) -> list:
"""The rules whose text changed after `held_etag` was issued.
Returns [] when the marker matches, is unparseable, or is absent — a
caller cannot act on "something is different but I cannot say what", and
a garbled marker must not be reported as a change.
A count difference is real news that this list cannot show: a rule
DELETED since the marker was issued has no row left to return. Callers
compare counts separately.
"""
if not held_etag or held_etag == _ETAG_EMPTY:
return []
stamp, _, _count = held_etag.partition("|")
try:
held_at = datetime.fromisoformat(stamp)
except ValueError:
return []
return [r for r in rules if r.updated_at and r.updated_at > held_at]
def etag_count(held_etag: str) -> int | None:
"""How many rules the holder had. None when the marker cannot be read."""
_stamp, _, count = (held_etag or "").partition("|")
try:
return int(count)
except ValueError:
return None
# ── The staleness sweep (milestone 312) ────────────────────────────────
async def rules_due_for_verification(
@@ -1455,14 +1601,7 @@ def verification_row(rule: Rule) -> dict:
because "2026-06-14" and "74 days" prompt different reactions and only
one of them is the question being asked.
"""
from datetime import datetime, timezone
days = None
if rule.verified_at is not None:
stamp = rule.verified_at
if stamp.tzinfo is None:
stamp = stamp.replace(tzinfo=timezone.utc)
days = (datetime.now(timezone.utc) - stamp).days
days = _days_since_verified(rule)
return {
"id": rule.id,
"title": rule.title,
+50 -4
View File
@@ -540,14 +540,60 @@ def compose_data(
return out
def recompose_data(note) -> dict:
"""Rebuild a snippet's `data` mirror from its own body, title and tags.
For the GENERIC note door. `update_snippet` composes the mirror itself from
the field set it just merged and never needs this; a plain
`update_note(body=...)` — which the Knowledge feed's editor issues, because
a snippet card there routes to /notes/:id — has no idea the mirror exists,
and left it stale. `snippet_fields` then PREFERS the stale mirror, so the
row reported its old repo/path/symbol to prior-art recall while displaying
its new body: confidently wrong, which is worse than no record (#3128).
The body is the authority; the mirror is derived. That is already the rule
this file states — it just had no enforcement on the path that bypasses
`update_snippet`.
`verification` and `provenance` are CARRIED, not recomposed, because
neither is in the body to parse — the same carry `compose_data` does for
the snippet service's own writes. Note that a verdict does not need
invalidating here: `code_sha` is recomputed from the new code, so a stale
verdict expires itself on read exactly as it does after any other edit.
"""
parsed = parse_snippet_fields(note.title, note.body, note.tags)
prior = note.data or {}
return compose_data(
name=parsed["name"],
when_to_use=parsed["when_to_use"],
signature=parsed["signature"],
language=parsed["language"],
code=parsed["code"],
locations=parsed["locations"],
merged_from=parsed["merged_from"],
verification=prior.get("verification"),
provenance=prior.get("provenance"),
)
def snippet_fields(note) -> dict:
"""Structured fields for a snippet, preferring the indexed `data` column and
falling back to parsing the body.
Both paths must agree, because rows written before migration 0070 have no
`data` and are never backfilled — a hand-edited body is the authority for
those, and there is no deadline by which they must be converted. `code` only
ever comes from the body, since `data` doesn't carry it.
THE BODY IS THE AUTHORITY; `data` is a mirror derived from it. Every writer
keeps them in step — the snippet service composes the mirror from the field
set it just merged, `update_note` recomposes it when a body reaches the
generic door (#3128), and `backfill_snippet_data` filled the pre-0070 rows
at startup. The fallback below is therefore a belt to that braces, not a
second source of truth: it is what a row looks like before the backfill has
run, and it must keep agreeing with the mirror.
(This docstring used to say those rows were "never backfilled". That was
true when 0070 landed and stopped being true when the backfill shipped; it
is corrected here because the sentence read as licence for a stale mirror,
which is exactly the bug #3128 found.)
`code` only ever comes from the body, since `data` doesn't carry it.
"""
parsed = parse_snippet_fields(note.title, note.body, note.tags)
stored = getattr(note, "data", None)
+57
View File
@@ -0,0 +1,57 @@
"""What a record's own check MEANS — shared by rules and notes.
Two record types carry `verify_with` / `expires_when` / `verified_at`: rules
(milestone 312) and notes (milestone 317). What they share is BEHAVIOUR — how
a stamp is read, how old it is, what "never" means — not storage and not the
query. Note 3163 is the rule this file is an instance of: `semantic_search_*`
could never have been shared between them because a rule scopes by rulebook
ownership and a note scopes by the note ACL, so the two sweeps are siblings.
These functions are the part that genuinely is common, factored so it exists
once rather than twice.
Everything here is pure, sync and DUCK-TYPED: it reads `verify_with` and
`verified_at` off whatever it is handed. That is deliberate. A shared base
class or a protocol would tie two SQLAlchemy models together to share four
lines of date arithmetic — the DRY costume, in the same note's words.
"""
from datetime import datetime, timezone
def last_verified_label(record) -> str | None:
"""How long ago this record's check passed — None when it carries none.
Three states, and the distinction between the last two is the whole point:
- `None` — this record is a DECISION. There is nothing to go and check,
and the question does not apply. Most records.
- "never" — it asserts a fact and NOBODY HAS EVER CONFIRMED IT. The one
worth acting on, and why the sweep sorts these first.
- a date — somebody checked, then.
Callers attach this to a payload only when it is not None, so "no key" and
"never verified" do not become two states every client has to tell apart.
"""
if not getattr(record, "verify_with", None):
return None
stamp = getattr(record, "verified_at", None)
return stamp.date().isoformat() if stamp else "never"
def days_since_verified(record) -> int | None:
"""Whole days since the check last passed; None if it never has.
Computed rather than left to the reader, because "2026-06-14" and "74
days" prompt different reactions and only one of them is the question
being asked.
Naive stamps are read as UTC. Postgres hands these back tz-aware, but a
restore, a fixture or a sqlite-backed test may not, and subtracting an
aware datetime from a naive one raises — which would make the sweep fail
on exactly the rows it exists to surface.
"""
stamp = getattr(record, "verified_at", None)
if stamp is None:
return None
if stamp.tzinfo is None:
stamp = stamp.replace(tzinfo=timezone.utc)
return (datetime.now(timezone.utc) - stamp).days
+64 -1
View File
@@ -9,7 +9,64 @@ from __future__ import annotations
from contextlib import contextmanager
from datetime import datetime, timezone
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock
from unittest.mock import AsyncMock, MagicMock, patch
async def drive_update_note(note, **kwargs):
"""Run `services/notes.update_note` against a stand-in row.
The patch stack is the point: update_note reaches for a version snapshot,
an embedding refresh and a project reactivation on its way out, none of
which a unit test has. Written twice — once for the snippet mirror
(#3128) and once for the verification fields (#3182/317) — before being
consolidated here.
Returns whatever update_note returned; assert on the `note` you passed in.
"""
from unittest.mock import AsyncMock as _AsyncMock
session = make_mock_session()
result = MagicMock()
result.scalars.return_value.first.return_value = note
session.execute = _AsyncMock(return_value=result)
with patch("scribe.services.notes.async_session") as cls, \
patch("scribe.services.notes.embed_note", MagicMock()), \
patch("scribe.services.notes._maybe_reactivate_project", _AsyncMock()), \
patch("scribe.services.note_versions.create_version", _AsyncMock()):
cls.return_value = session
from scribe.services.notes import update_note
return await update_note(user_id=7, note_id=note.id, **kwargs)
def tool_doc(module: str, name: str) -> str:
"""An MCP tool's docstring, whitespace-flattened.
Flattened because these are hard-wrapped at ~76 characters, so any phrase
worth asserting on is liable to straddle a line break — a property of the
formatter, not of the guidance. The disambiguator guard (#3123) learned
that on its own first run, matching raw text and reporting a phrase absent
that was plainly there.
Used by every test that pins the docstring CONTRACT rather than its
wording. The tool docstring is the agent-facing contract (rule 119), so
these guards exist to catch it being tidied down to a parameter list.
"""
import importlib
import re as _re
fn = getattr(importlib.import_module(module), name)
assert fn.__doc__, f"{name} has no docstring at all"
return _re.sub(r"\s+", " ", fn.__doc__)
def compiled_sql(element) -> str:
"""A SQLAlchemy clause or statement rendered as literal SQL text.
For asserting on the shape of a predicate without a database — which is how
the visibility clauses and the knowledge facets are both tested. Was a
private copy in each of those modules before #3128 needed a third.
"""
return str(element.compile(compile_kwargs={"literal_binds": True}))
def make_mock_session() -> AsyncMock:
@@ -92,6 +149,9 @@ def fake_note(**attrs) -> MagicMock:
"id": 1, "title": "t", "body": "", "tags": [], "user_id": 7,
"note_type": "note", "is_task": False, "task_kind": "work",
"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.
"verify_with": None, "expires_when": None, "verified_at": None,
}, attrs)
@@ -101,6 +161,7 @@ def fake_task(**attrs) -> MagicMock:
"id": 1, "title": "t", "body": "", "status": "todo", "priority": "none",
"tags": [], "parent_id": None, "project_id": None, "is_task": True,
"task_kind": "work", "user_id": 7, "deleted_at": None,
"verify_with": None, "expires_when": None, "verified_at": None,
}, attrs)
@@ -112,6 +173,8 @@ def fake_snippet(**attrs) -> MagicMock:
"body": "```js\nreturn 1\n```\n", "tags": ["js", "snippet"],
"note_type": "snippet", "is_task": False, "task_kind": "work",
"user_id": 7, "data": None, "deleted_at": None,
"status": None,
"verify_with": None, "expires_when": None, "verified_at": None,
}, attrs)
+1 -15
View File
@@ -29,6 +29,7 @@ parameters, with the "is this even the right tool" paragraph quietly gone.
import re
import pytest
from tests.helpers import tool_doc as _doc
# The create surfaces and where they live. `start_planning` is here because
# it is a create in everything but name — it is how a plan comes into being.
@@ -50,21 +51,6 @@ _ALTERNATIVES = [
]
def _doc(module: str, name: str) -> str:
"""The docstring with its whitespace flattened.
Flattened because a docstring is hard-wrapped: "design system" spans a
line break in at least one of these, and matching the raw text would
report it absent. The first draft of this check did exactly that, and
caught it on itself.
"""
import importlib
fn = getattr(importlib.import_module(module), name)
assert fn.__doc__, f"{name} has no docstring at all"
return re.sub(r"\s+", " ", fn.__doc__)
@pytest.mark.parametrize(("module", "name"), _SURFACES)
def test_a_create_surface_names_at_least_two_alternatives(module, name):
"""Reaching for the wrong tool must still put the right one in view."""
@@ -0,0 +1,207 @@
"""Real-Postgres round trip for the note fields #3182 restored.
The unit lane can prove a serialiser EMITS a key. It cannot prove a restore
puts the value back on the right row, and the note->note edges are exactly
where that distinction bites: `parent_id` and `arose_from_id` hold ids from
the SOURCE database, so a restore that writes them straight through succeeds,
reports success, and silently points every edge at whatever record happens to
hold that number here.
That is why #3182 was not fixed in passing.
These drive the REAL `restore_full_backup`, not a reimplementation of its
loop. A test that re-derives the remap it is checking would agree with
whatever the product does, including nothing.
"""
import pytest
import pytest_asyncio
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.note import Note
from scribe.models.user import User
from scribe.services import backup
from tests.helpers import ensure_user
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
RESTORED_USERNAME = "backup_roundtrip_restored"
@pytest_asyncio.fixture
async def source():
"""A snippet, a process, and an issue that arose from a work task — the
shapes whose identity the backup used to drop — exported as backup rows.
Deleted children-first: `arose_from_id` is a real FK, so removing the
origin while the issue still points at it is asking the database a
question the test has no reason to ask.
"""
async with async_session() as s:
owner = await ensure_user(s, "backup_roundtrip_owner")
uid = owner.id
await s.commit()
async with async_session() as s:
origin = Note(
user_id=uid, title="the work that broke", body="",
status="done", task_kind="work",
)
snippet = Note(
user_id=uid, title="debounce — rate-limit", body="```js\n1\n```",
note_type="snippet", data={"name": "debounce", "language": "js"},
)
process = Note(
user_id=uid, title="DRY pass", body="steps", note_type="process",
)
s.add_all([origin, snippet, process])
await s.commit()
origin_id = origin.id
issue = Note(
user_id=uid, title="the fix", body="symptom -> cause -> fix",
status="done", task_kind="issue", arose_from_id=origin_id,
description="one-liner",
)
s.add(issue)
await s.commit()
order = [issue.id, origin_id, snippet.id, process.id]
async with async_session() as s:
rows = (await s.execute(select(Note).where(Note.id.in_(order)))).scalars().all()
note_rows = backup._note_rows(list(rows))
user_rows = backup._user_rows(
[(await s.execute(select(User).where(User.id == uid))).scalars().one()]
)
# Restore mints a NEW user from the payload, so the restored corpus is
# entirely separate from the source — which is what makes the id
# assertions meaningful. Renamed here rather than in a sibling fixture:
# `restored` depends on this one, and a rename elsewhere might not have
# run by the time the restore does.
user_rows[0]["username"] = RESTORED_USERNAME
yield {
"payload": {
"version": backup.BACKUP_VERSION,
"users": user_rows,
"notes": note_rows,
},
"origin_id": origin_id,
"owner_id": uid,
}
async with async_session() as s:
for nid in order:
row = await s.get(Note, nid)
if row is not None:
await s.delete(row)
await s.commit()
@pytest_asyncio.fixture
async def restored(source):
"""Run the real restore, then hand back the new rows by title.
The restore mints a NEW user from the payload, so the restored corpus is
entirely separate from the source one — which is what makes the id
assertions below meaningful.
"""
await backup.restore_full_backup(source["payload"])
async with async_session() as s:
user = (await s.execute(
select(User).where(User.username == RESTORED_USERNAME)
)).scalars().first()
assert user is not None, "the payload's user was not restored"
rows = (await s.execute(
select(Note).where(Note.user_id == user.id)
)).scalars().all()
by_title = {n.title: n for n in rows}
new_user_id = user.id
yield by_title, source
async with async_session() as s:
fresh = [await s.get(Note, r.id) for r in by_title.values()]
for row in fresh:
if row is not None:
row.arose_from_id = None
row.parent_id = None
await s.flush()
for row in fresh:
if row is not None:
await s.delete(row)
await s.commit()
async with async_session() as s:
user = await s.get(User, new_user_id)
if user is not None:
await s.delete(user)
await s.commit()
@pytest_asyncio.fixture(autouse=True)
async def _no_leftover_restored_user():
"""The payload's username is fixed, so a previous failed run would leave a
row that makes `restored` pick the wrong user. Clear it first."""
async with async_session() as s:
stale = (await s.execute(
select(User).where(User.username == RESTORED_USERNAME)
)).scalars().all()
for user in stale:
notes = (await s.execute(
select(Note).where(Note.user_id == user.id)
)).scalars().all()
for n in notes:
n.arose_from_id = None
n.parent_id = None
await s.flush()
for n in notes:
await s.delete(n)
await s.delete(user)
await s.commit()
async def test_a_restored_record_keeps_what_it_IS(restored):
"""#3182's headline. Without note_type and task_kind a restore reported
success and handed back a corpus where every snippet and process was a
plain note and every issue and spike was `work` — the whole vocabulary
milestone 312 and #3128 were about, gone, with nothing to notice it by."""
by_title, _ = restored
assert by_title["debounce — rate-limit"].note_type == "snippet"
assert by_title["DRY pass"].note_type == "process"
assert by_title["the fix"].task_kind == "issue"
assert by_title["the work that broke"].task_kind == "work"
async def test_a_restored_snippet_keeps_its_queryable_mirror(restored):
"""The one field that would self-heal — backfill_snippet_data rebuilds it
from the body at startup — but a restore should not hand back a corpus
that needs a restart before it is findable by location."""
by_title, _ = restored
assert by_title["debounce — rate-limit"].data == {
"name": "debounce", "language": "js",
}
async def test_the_provenance_edge_is_remapped_not_copied(restored):
"""THE regression, and the reason this needed a real database.
The payload's `arose_from_id` is an id in the SOURCE database. Copying it
through would leave the restored issue pointing at whatever record happens
to hold that number — a restore that succeeds and silently rewires
history. The edge must land on the RESTORED origin instead.
"""
by_title, src = restored
issue = by_title["the fix"]
origin = by_title["the work that broke"]
assert issue.arose_from_id == origin.id
# ...and that is a different row from the one the payload named.
assert issue.arose_from_id != src["origin_id"]
async def test_description_and_status_survive(restored):
by_title, _ = restored
assert by_title["the fix"].description == "one-liner"
assert by_title["the fix"].status == "done"
@@ -0,0 +1,266 @@
"""Real-Postgres round trip for rule_versions (milestone 323 step 1).
Two things here cannot be shown with mocks, and both are the kind that fail
QUIETLY — a restore reports success and hands back history that is wrong.
1. **`rule_id` is remapped, not copied.** It is an id in the SOURCE database.
A restore that writes it straight through succeeds and attaches every
snapshot to whatever rule happens to hold that number here — an edit
history filed against the wrong binding instruction, which is worse than
no history at all. This is #3182's `arose_from_id` trap on a new table.
2. **A null actor does not drop the row.** `user_id` is SET NULL precisely so
history outlives the account that wrote it, so the restore deliberately
diverges from its `NoteVersion` sibling, which skips a version it cannot
map to a user. Nothing about the code says which of the two shapes is
intended; without this test, "make it match the sibling" reads as a tidy-up
and silently deletes the record the FK was chosen to preserve.
These drive the REAL `restore_full_backup`. A test that re-derived the remap
would agree with whatever the product does, including nothing.
"""
import pytest
import pytest_asyncio
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.rule_version import RuleVersion
from scribe.models.rulebook import Rule, Rulebook, RulebookTopic
from scribe.models.user import User
from scribe.services import backup
from tests.helpers import ensure_user
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
OWNER_USERNAME = "rule_version_roundtrip_owner"
RESTORED_USERNAME = "rule_version_roundtrip_restored"
async def _purge_books(username: str) -> None:
"""user -> rulebook -> topic -> rule -> rule_version is ON DELETE CASCADE
the whole way down, and no ORM relationships are configured, so dropping
the books clears every row this file made under them."""
async with async_session() as s:
users = (await s.execute(
select(User).where(User.username == username)
)).scalars().all()
for user in users:
books = (await s.execute(
select(Rulebook).where(Rulebook.owner_user_id == user.id)
)).scalars().all()
for book in books:
await s.delete(book)
await s.commit()
async def _purge_restored() -> None:
"""The restored user is minted by the restore itself, so it goes too."""
await _purge_books(RESTORED_USERNAME)
async with async_session() as s:
for user in (await s.execute(
select(User).where(User.username == RESTORED_USERNAME)
)).scalars().all():
await s.delete(user)
await s.commit()
@pytest_asyncio.fixture(autouse=True)
async def _no_leftovers():
"""The usernames are fixed, so a previous failed run would leave rows that
make the fixtures below pick the wrong user — or hit `.one()` with two.
SETUP ONLY, and that is not a stylistic choice. `_dispose_engine` is a
usefixtures entry, so it sets up AFTER this autouse one and therefore
tears down BEFORE it. Any database call here after a `yield` would open a
fresh pooled connection that the closing loop then orphans, and the next
test to touch Postgres dies on "Future attached to a different loop"
including tests in other files. Cleanup belongs in the fixtures below,
whose teardowns run while the engine is still live.
"""
await _purge_restored()
await _purge_books(OWNER_USERNAME)
@pytest_asyncio.fixture
async def source():
"""One rule with two snapshots: one written by a user who still exists,
one whose actor is already gone.
Both are needed. With only the attributed version, dropping unmappable
rows would pass; with only the orphaned one, so would dropping the actor
from every row.
"""
async with async_session() as s:
owner = await ensure_user(s, OWNER_USERNAME)
uid = owner.id
await s.commit()
async with async_session() as s:
book = Rulebook(owner_user_id=uid, title="Environment facts")
s.add(book)
await s.flush()
topic = RulebookTopic(rulebook_id=book.id, title="ci")
s.add(topic)
await s.flush()
rule = Rule(
topic_id=topic.id,
title="The runner has no bash",
statement="Write every `run:` step in POSIX sh.",
verify_with="read the workflow's shell setting",
)
s.add(rule)
await s.flush()
s.add_all([
RuleVersion(
rule_id=rule.id, user_id=uid,
title="The runner has no bash",
statement="Use sh.",
why="the image ships no bash",
verify_with="read the workflow's shell setting",
tier="always_on",
),
# The actor is already gone — what SET NULL leaves behind.
RuleVersion(
rule_id=rule.id, user_id=None,
title="The runner has no bash",
statement="Use POSIX sh in run steps.",
tier="always_on",
),
])
await s.commit()
book_id, rule_id = book.id, rule.id
async with async_session() as s:
user_rows = backup._user_rows(
[(await s.execute(select(User).where(User.id == uid))).scalars().one()]
)
book_rows = backup._rulebook_rows(
[(await s.execute(select(Rulebook).where(Rulebook.id == book_id)))
.scalars().one()]
)
topic_rows = backup._topic_rows(
(await s.execute(
select(RulebookTopic).where(RulebookTopic.rulebook_id == book_id)
)).scalars().all()
)
rule_rows = backup._rule_rows(
[(await s.execute(select(Rule).where(Rule.id == rule_id))).scalars().one()]
)
version_rows = backup._rule_version_rows(
(await s.execute(
select(RuleVersion).where(RuleVersion.rule_id == rule_id)
.order_by(RuleVersion.id)
)).scalars().all()
)
# The restore mints a NEW user from the payload, so the restored corpus is
# separate from the source one — which is what makes the id assertion
# below able to fail.
user_rows[0]["username"] = RESTORED_USERNAME
yield {
"payload": {
"version": backup.BACKUP_VERSION,
"users": user_rows,
"rulebooks": book_rows,
"rulebook_topics": topic_rows,
"rules": rule_rows,
"rule_versions": version_rows,
},
"source_rule_id": rule_id,
"source_user_id": uid,
}
async with async_session() as s:
book = await s.get(Rulebook, book_id)
if book is not None:
await s.delete(book)
await s.commit()
@pytest_asyncio.fixture
async def restored(source):
"""Runs the real restore, then hands back the new rows.
The restore mints a NEW user from the payload, so the restored corpus is
entirely separate from the source one — which is what makes the id
assertions below able to fail.
"""
await backup.restore_full_backup(source["payload"])
async with async_session() as s:
user = (await s.execute(
select(User).where(User.username == RESTORED_USERNAME)
)).scalars().first()
assert user is not None, "the payload's user was not restored"
book = (await s.execute(
select(Rulebook).where(Rulebook.owner_user_id == user.id)
)).scalars().one()
topic = (await s.execute(
select(RulebookTopic).where(RulebookTopic.rulebook_id == book.id)
)).scalars().one()
rule = (await s.execute(
select(Rule).where(Rule.topic_id == topic.id)
)).scalars().one()
versions = (await s.execute(
select(RuleVersion).where(RuleVersion.rule_id == rule.id)
.order_by(RuleVersion.id)
)).scalars().all()
yield {"user": user, "rule": rule, "versions": versions, "source": source}
# Here rather than in the autouse fixture: this teardown still runs while
# the engine is live. See _no_leftovers.
await _purge_restored()
async def test_both_snapshots_come_back(restored):
"""The count first: everything below reads the same on an empty list, so
without this a restore that silently dropped both would look like a pass
on the shape assertions."""
assert len(restored["versions"]) == 2
async def test_the_history_attaches_to_the_RESTORED_rule(restored):
"""#3182's trap. The source rule still exists and holds a different id, so
a straight-through copy would file this history against it — or against
whatever unrelated rule owns that number."""
new_rule_id = restored["rule"].id
source_rule_id = restored["source"]["source_rule_id"]
assert new_rule_id != source_rule_id, (
"the restore reused the source id, so this test cannot tell a remap "
"from a copy — the fixture is not proving what it claims"
)
assert {v.rule_id for v in restored["versions"]} == {new_rule_id}
async def test_the_actor_is_remapped_to_the_restored_user(restored):
"""`user_id` is an id in the source database too — the same trap, on the
column that answers "who changed this"."""
attributed = [v for v in restored["versions"] if v.user_id is not None]
assert len(attributed) == 1
assert attributed[0].user_id == restored["user"].id
assert attributed[0].user_id != restored["source"]["source_user_id"]
async def test_a_snapshot_with_no_actor_survives(restored):
"""The deliberate divergence from NoteVersion. `user_id` is SET NULL so
that history outlives the account that wrote it; skipping the row on an
unmappable user would throw away exactly what the FK preserves."""
orphaned = [v for v in restored["versions"] if v.user_id is None]
assert len(orphaned) == 1, (
"the version whose actor was already gone did not come back. A rule's "
"history is the only record of what it used to say — losing it "
"because nobody can be credited is the wrong trade."
)
assert orphaned[0].statement == "Use POSIX sh in run steps."
async def test_the_text_survives(restored):
"""The whole point of the table: what the rule USED to say. A restore that
kept the rows and lost their wording would preserve a changelog of empty
entries."""
by_statement = {v.statement: v for v in restored["versions"]}
assert set(by_statement) == {"Use sh.", "Use POSIX sh in run steps."}
assert by_statement["Use sh."].why == "the image ships no bash"
assert by_statement["Use sh."].verify_with == (
"read the workflow's shell setting"
)
assert by_statement["Use sh."].tier == "always_on"
+293
View File
@@ -0,0 +1,293 @@
"""Real-Postgres tests for the rule-history write path (milestone 323 step 2).
Every assertion here is about ORDERING or ABSENCE, which is why none of them
can be made with mocks.
What a version is for: it holds what the rule USED TO SAY. The edit destroys
that, and nothing can recompute it — the rescoping of rule 79 had to be
hand-copied into a task log to survive (#3237), which is not a process, it is
a person remembering.
Two of these pin a DELIBERATE ABSENCE. `note_versions` carries a 300-second
interval gate and a 50-version cap, both defences against note autosave. Rules
have no autosave, so here those guards would only ever discard a real edit.
Nothing in the code says so on its own, and "make it consistent with the
sibling" is a plausible-sounding change that would silently start dropping
history — so the absence is a test, not a comment.
"""
import pytest
import pytest_asyncio
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.rulebook import Rule, Rulebook
from scribe.models.rule_version import RuleVersion
from scribe.services import rule_versions as rv_svc
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 = "rule_history_owner"
@pytest_asyncio.fixture
async def constraint():
"""One rule carrying a check, with no history yet.
CLEANED UP AT SETUP, NOT TEARDOWN, and that is forced. `update_rule` fires
a detached `asyncio.create_task(upsert_rule_embedding(...))` that opens
its own connection and UPDATEs the rule row. A teardown that deleted the
rulebook would race it: the delete cascade-locks the rule the embedding
task is writing, and Postgres kills one of them with a deadlock. Purging
at setup instead runs on a fresh loop, after the previous test's loop
closed and cancelled whatever it left in flight.
"""
async with async_session() as s:
owner = await ensure_user(s, OWNER_USERNAME)
uid = owner.id
await s.commit()
# A previous failed run would leave a book whose topic title collides.
for book in (await s.execute(
select(Rulebook).where(Rulebook.owner_user_id == uid)
)).scalars().all():
await s.delete(book)
await s.commit()
book = await rulebooks_svc.create_rulebook(uid, "History fixtures")
topic = await rulebooks_svc.create_topic(book.id, uid, "ci")
rule = await rulebooks_svc.create_rule(
topic.id, uid, "The runner has no bash",
"Write every `run:` step in POSIX sh.",
why="the image ships no bash",
verify_with="read the workflow's shell setting",
)
return {"uid": uid, "rule_id": rule.id}
async def _versions(rule_id: int) -> list[RuleVersion]:
async with async_session() as s:
return list((await s.execute(
select(RuleVersion).where(RuleVersion.rule_id == rule_id)
.order_by(RuleVersion.id)
)).scalars().all())
async def test_an_edit_leaves_the_text_it_replaced(constraint):
"""The headline. The version holds the OLD statement — the rule row
already holds the new one, so a snapshot of the new text would record
nothing that was not already there."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"],
statement="Write every `run:` step in POSIX sh, and set shell: sh.",
)
[version] = await _versions(constraint["rule_id"])
assert version.statement == "Write every `run:` step in POSIX sh."
assert version.why == "the image ships no bash"
async def test_the_actor_is_recorded(constraint):
"""`user_id` is who made the edit — the question an audit trail over a
binding instruction is actually asked."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], title="The runner ships no bash",
)
[version] = await _versions(constraint["rule_id"])
assert version.user_id == constraint["uid"]
async def test_an_edit_that_changes_no_text_writes_nothing(constraint):
"""Git semantics. `update_rule` is reached by callers that resend every
field, so without the skip a form saved twice would file an identical
snapshot and the history would stop reading as a list of edits."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"],
statement="Write every `run:` step in POSIX sh.",
title="The runner has no bash",
)
assert await _versions(constraint["rule_id"]) == []
async def test_reordering_is_not_an_edit(constraint):
"""`order_index` goes through the same door but is not text. A run of
"moved rule 4 above rule 3" snapshots would bury the edits somebody is
looking for."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], order_index=7,
)
assert await _versions(constraint["rule_id"]) == []
async def test_the_snapshot_holds_the_CHECK_that_was_in_force(constraint):
"""The ordering trap. `update_rule` drops `verified_at` when `verify_with`
changes, and rewriting the check is exactly the edit whose history matters
most — so the snapshot has to be taken before the field loop, or it
records the new check against the old wording."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"],
verify_with="run `sh -c 'echo $0'` in a job step",
)
[version] = await _versions(constraint["rule_id"])
assert version.verify_with == "read the workflow's shell setting"
async with async_session() as s:
rule = await s.get(Rule, constraint["rule_id"])
assert rule.verify_with == "run `sh -c 'echo $0'` in a job step"
async def test_clearing_a_field_is_an_edit_worth_recording(constraint):
"""A rule that stops being a constraint loses its check entirely, and the
version is then the only place the retired check exists. `clear` is a
separate argument from the field loop — a snapshot wired to only one of
the two would lose exactly this case."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], clear=("verify_with",),
)
[version] = await _versions(constraint["rule_id"])
assert version.verify_with == "read the workflow's shell setting"
async with async_session() as s:
rule = await s.get(Rule, constraint["rule_id"])
assert rule.verify_with is None
async def test_rapid_successive_edits_are_all_kept(constraint):
"""DELIBERATE ABSENCE — no MIN_VERSION_INTERVAL_SECONDS.
`note_versions` skips a snapshot taken within 300 seconds of the last one,
because note autosave fires every 60 and would burn all 50 slots. Rules
have no autosave: every one of these is a deliberate update_rule, and
three edits in the same second are three edits. Porting the sibling's gate
here would silently keep only the first.
"""
for n in (1, 2, 3):
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], statement=f"revision {n}",
)
versions = await _versions(constraint["rule_id"])
assert [v.statement for v in versions] == [
"Write every `run:` step in POSIX sh.", "revision 1", "revision 2",
]
async def test_the_service_carries_no_pruning_machinery(constraint):
"""DELIBERATE ABSENCE — no MAX_VERSIONS and no pruning DELETE.
Asserted structurally because the alternative is 51 real edits to prove a
negative. The cap exists in `note_versions` to bound autosave volume; a
rule is edited a handful of times in its life, so a cap here could only
ever discard the one edit somebody went looking for.
"""
assert not hasattr(rv_svc, "MAX_VERSIONS")
assert not hasattr(rv_svc, "MIN_VERSION_INTERVAL_SECONDS")
async def test_history_reads_newest_first(constraint):
"""The reader's question is "what did this just say?", so the most recent
supersession has to be the first row."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], statement="first change",
)
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], statement="second change",
)
listed = await rv_svc.list_versions(constraint["rule_id"])
assert [v.statement for v in listed] == [
"first change", "Write every `run:` step in POSIX sh.",
]
# ── The read path is ACL-scoped (milestone 323 step 3) ─────────────────
#
# Rule 47: every read of user data is scoped by owner. A version carries a
# `user_id`, which makes it tempting to scope the history by it — that would
# be wrong in both directions, and these pin which way round it goes.
@pytest_asyncio.fixture
async def stranger():
"""A second user who owns nothing in the fixture above."""
async with async_session() as s:
other = await ensure_user(s, "rule_history_stranger")
uid = other.id
await s.commit()
return uid
async def test_the_owner_reads_the_history(constraint):
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], statement="reworded",
)
versions = await rulebooks_svc.list_rule_versions(
constraint["rule_id"], constraint["uid"],
)
assert [v.statement for v in versions] == [
"Write every `run:` step in POSIX sh.",
]
async def test_a_stranger_reads_nothing(constraint, stranger):
"""None, not an empty list. The two mean different things — "not your
rule" versus "this rule has never been reworded" — and collapsing them
would tell a caller the rule exists and is unedited."""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], statement="reworded",
)
assert await rulebooks_svc.list_rule_versions(
constraint["rule_id"], stranger,
) is None
async def test_a_version_cannot_be_read_through_a_DIFFERENT_rule(constraint):
"""The check and the fetch have to agree about which rule is in play.
The second rule is owned by the SAME user on purpose — an id that simply
does not exist would pass on the ownership check alone and prove nothing
about the `rule_id` clause. Here the caller genuinely owns the rule they
name and genuinely owns the version's rule, and the read must still
refuse, because scoping by version id alone is how a snapshot leaks
through whichever rule the caller happens to be able to see.
"""
await rulebooks_svc.update_rule(
constraint["rule_id"], constraint["uid"], statement="reworded",
)
[version] = await _versions(constraint["rule_id"])
async with async_session() as s:
rule = await s.get(Rule, constraint["rule_id"])
topic_id = rule.topic_id
sibling = await rulebooks_svc.create_rule(
topic_id, constraint["uid"], "A different rule", "Unrelated.",
)
assert await rulebooks_svc.get_rule_version(
sibling.id, version.id, constraint["uid"],
) is None
# And through its own rule it reads fine — or the assertion above would
# pass for the wrong reason.
assert await rulebooks_svc.get_rule_version(
constraint["rule_id"], version.id, constraint["uid"],
) is not None
async def test_the_actor_does_not_grant_the_read(constraint, stranger):
"""The inverse mistake. `user_id` on a version is the ACTOR, so scoping
the history by it would hand someone the snapshots they personally wrote
on a rule that has since moved out of their reach — and would hide, from
the rule's owner, every edit somebody else made."""
async with async_session() as s:
s.add(RuleVersion(
rule_id=constraint["rule_id"], user_id=stranger,
title="written by someone else", statement="a stranger's edit",
))
await s.commit()
assert await rulebooks_svc.list_rule_versions(
constraint["rule_id"], stranger,
) is None
owner_view = await rulebooks_svc.list_rule_versions(
constraint["rule_id"], constraint["uid"],
)
assert "a stranger's edit" in [v.statement for v in owner_view]
+120
View File
@@ -0,0 +1,120 @@
"""The type facet, in both of its dialects.
The facet predicate is written twice by necessity — as SQL for rows the
database hands back, and as Python for candidates the vector search has
already fetched — plus a third time as the `is_task` pre-filter that narrows
the candidate set before it exists. Those three used to be hand-written and
agreed only by luck: adding `issue` to the SQL arm alone would have set the
pre-filter to is_task=False, handed the Python arm a candidate set with no
tasks in it, and returned an empty semantic half for the Issues facet forever
with nothing red anywhere (#3128).
They are now generated from one table. These tests pin the property that made
the trap possible, so it cannot come back by a different route.
"""
import pytest
from scribe.services.knowledge import (
_FACETS,
FACET_TYPES,
NON_TASK_FACETS,
_apply_type_filter,
facet_is_task,
matches_facet,
)
from tests.helpers import compiled_sql, fake_note, fake_snippet, fake_task
def _sql(note_type):
from sqlalchemy import select
from scribe.models.note import Note
return compiled_sql(_apply_type_filter(select(Note.id), note_type))
# One representative row per shape the corpus actually holds.
ROWS = {
"plain note": fake_note(note_type="note"),
"process": fake_note(note_type="process"),
"snippet": fake_snippet(),
"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"),
"legacy plan": fake_task(task_kind="plan", note_type="note"),
}
@pytest.mark.parametrize("facet", sorted(FACET_TYPES))
def test_pre_filter_never_excludes_a_row_the_facet_wants(facet):
"""THE regression. `facet_is_task` narrows the semantic candidate set before
`matches_facet` ever sees it, so a pre-filter that disagrees with the
predicate doesn't return wrong rows — it returns NO rows, silently, on one
half of a hybrid search."""
want = facet_is_task(facet)
for label, row in ROWS.items():
if matches_facet(row, facet):
assert want is None or want == row.is_task, (
f"facet {facet!r} accepts the {label} row, but its pre-filter "
f"asks for is_task={want} while the row has is_task={row.is_task} "
f"— the semantic arm would never be handed this row"
)
@pytest.mark.parametrize(
"facet,expected",
[
("task", {"work task", "issue", "spike", "legacy plan"}),
("issue", {"issue"}),
("spike", {"spike"}),
("work", {"work task"}),
("plan", {"legacy plan"}),
("note", {"plain note"}),
("process", {"process"}),
("snippet", {"snippet"}),
("", set(ROWS)),
],
)
def test_each_facet_selects_exactly_its_own_rows(facet, expected):
assert {k for k, row in ROWS.items() if matches_facet(row, facet)} == expected
def test_a_plain_note_is_not_selected_by_its_own_type_when_it_is_a_task():
"""A task's `note_type` is 'note' — that column says nothing about task-ness.
The hand-written Python arm omitted the `status IS NULL` half its SQL twin
carried, so it only avoided returning every task under the Notes facet
because the pre-filter had already dropped them."""
assert matches_facet(fake_task(note_type="note"), "note") is False
def test_an_unknown_facet_matches_nothing_rather_than_everything():
"""A typo must return an empty list, never the whole corpus."""
assert all(not matches_facet(row, "wrok") for row in ROWS.values())
assert "notes.status IS NULL" in _sql("wrok")
def test_the_live_task_kinds_are_all_facets():
"""`issue` shipped in 0065 and `spike` in 0091; the browse vocabulary went
three kinds without noticing either."""
for kind in ("work", "issue", "spike"):
assert kind in FACET_TYPES and _FACETS[kind][0] is True
def test_non_task_facets_are_the_note_types_and_only_those():
assert set(NON_TASK_FACETS) == {"note", "process", "snippet"}
@pytest.mark.parametrize("facet", sorted(FACET_TYPES))
def test_sql_arm_constrains_the_axis_the_facet_lives_on(facet):
"""The SQL dialect of the same table. A task facet must pin `status` (and,
for a single kind, `task_kind`); a record-type facet must pin `note_type`
AND exclude tasks."""
sql = _sql(facet)
is_task, value = _FACETS[facet]
assert "notes.deleted_at IS NULL" in sql
if is_task:
assert "notes.status IS NOT NULL" in sql
assert (f"notes.task_kind = '{value}'" in sql) is (value is not None)
else:
assert "notes.status IS NULL" in sql
assert f"notes.note_type = '{value}'" in sql
+70 -4
View File
@@ -1,5 +1,5 @@
"""Tests for fable_*_note tools."""
from unittest.mock import AsyncMock, patch
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
@@ -214,17 +214,53 @@ async def test_create_note_project_zero_becomes_none():
@pytest.mark.asyncio
async def test_update_note_only_sends_non_default_fields():
"""Omitted (default) fields must NOT reach the service — otherwise they'd
overwrite real data with empty strings."""
overwrite real data with empty strings.
`clear` is always forwarded and is not a field: it is how this door says
"unset these", and an empty one says "unset nothing". Same shape the rules
door took in #3096, and the same test that had to learn about it there."""
fake = fake_note()
mock = AsyncMock(return_value=fake)
with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock):
await update_note(note_id=1, title="new title")
# Service got user_id, note_id positional + only the title kwarg
args, kwargs = mock.call_args
assert args == (7, 1)
assert kwargs.pop("clear") == (), "nothing was asked to be cleared"
assert kwargs == {"title": "new title"}
@pytest.mark.asyncio
async def test_update_note_forwards_a_check_but_not_an_empty_one():
""""" means "leave this alone" at this door — an agent updating a body must
not wipe a check it was never asked about (milestone 317)."""
mock = AsyncMock(return_value=fake_note())
with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock):
await update_note(note_id=1, verify_with="curl the docs")
assert mock.call_args.kwargs["verify_with"] == "curl the docs"
with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock):
await update_note(note_id=1, title="t")
assert "verify_with" not in mock.call_args.kwargs
@pytest.mark.asyncio
async def test_update_note_forwards_clear_so_a_check_can_be_removed():
"""The only way to unset a field at a door where "" means "leave alone"."""
mock = AsyncMock(return_value=fake_note())
with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock):
await update_note(note_id=1, clear=["verify_with"])
assert mock.call_args.kwargs["clear"] == ["verify_with"]
@pytest.mark.asyncio
async def test_create_note_forwards_the_check_fields():
mock = AsyncMock(return_value=fake_note())
with patch("scribe.mcp.tools.notes.notes_svc.create_note", mock):
await create_note(title="t", verify_with="curl", expires_when="AMO changes")
assert mock.call_args.kwargs["verify_with"] == "curl"
assert mock.call_args.kwargs["expires_when"] == "AMO changes"
@pytest.mark.asyncio
async def test_update_note_empty_tags_clears_explicitly():
"""tags=[] is an explicit clear, distinct from tags=None (omit)."""
@@ -232,7 +268,9 @@ async def test_update_note_empty_tags_clears_explicitly():
mock = AsyncMock(return_value=fake)
with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock):
await update_note(note_id=1, tags=[])
assert mock.call_args.kwargs == {"tags": []}
kwargs = dict(mock.call_args.kwargs)
kwargs.pop("clear")
assert kwargs == {"tags": []}
@pytest.mark.asyncio
@@ -256,12 +294,37 @@ async def test_update_note_raises_when_not_found():
@pytest.mark.asyncio
async def test_delete_note_soft_deletes_and_returns_batch():
doomed = MagicMock()
doomed.title = "the note that went"
with patch(
"scribe.mcp.tools.notes.trash_svc.delete",
AsyncMock(return_value="batch-1"),
), patch(
"scribe.mcp.tools.notes.notes_svc.get_note_for_user",
AsyncMock(return_value=(doomed, "owner")),
):
result = await delete_note(note_id=7)
assert result["deleted_batch_id"] == "batch-1"
# The confirmation NAMES what went (#3273). After the delete the row is
# trashed, so this line is the last chance to say which note it was.
assert result["title"] == "the note that went"
assert "the note that went" in result["message"]
@pytest.mark.asyncio
async def test_delete_note_still_deletes_when_the_title_lookup_fails():
"""The title is a courtesy on top of the delete, never a precondition for
it. A lookup that errors costs the name, not the operation."""
with patch(
"scribe.mcp.tools.notes.trash_svc.delete",
AsyncMock(return_value="batch-1"),
), patch(
"scribe.mcp.tools.notes.notes_svc.get_note_for_user",
AsyncMock(side_effect=RuntimeError("database down")),
):
result = await delete_note(note_id=7)
assert result["deleted_batch_id"] == "batch-1"
assert result["title"] == ""
@pytest.mark.asyncio
@@ -269,6 +332,9 @@ async def test_delete_note_raises_when_not_found():
with patch(
"scribe.mcp.tools.notes.trash_svc.delete",
AsyncMock(return_value=None),
), patch(
"scribe.mcp.tools.notes.notes_svc.get_note_for_user",
AsyncMock(return_value=None),
):
with pytest.raises(ValueError, match="note 999 not found"):
await delete_note(note_id=999)
+131 -3
View File
@@ -229,8 +229,9 @@ def test_register_attaches_every_tool():
mcp = FakeMCP()
register(mcp)
# 26 through milestone 307, +2 for the staleness sweep (milestone 312).
assert len(mcp.names) == 28
# 26 through milestone 307, +2 for the staleness sweep (milestone 312),
# +1 for a rule's edit history (milestone 323).
assert len(mcp.names) == 29
# spot-check a few names
assert "list_rulebooks" in mcp.names
assert "create_rule" in mcp.names
@@ -244,6 +245,8 @@ def test_register_attaches_every_tool():
# milestone 312: the sweep, and the stamp that answers it
assert "rules_due_for_verification" in mcp.names
assert "mark_rule_verified" in mcp.names
# milestone 323: what a rule used to say
assert "rule_history" in mcp.names
assert "unsuppress_rule_for_project" in mcp.names
assert "suppress_topic_for_project" in mcp.names
assert "unsuppress_topic_for_project" in mcp.names
@@ -257,7 +260,10 @@ async def test_list_always_on_rules_returns_empty_when_no_always_on_rulebooks():
):
from scribe.mcp.tools.rulebooks import list_always_on_rules
out = await list_always_on_rules()
assert out == {"rules": [], "total": 0}
# An install with no always-on rulebooks still gets a marker (milestone
# 323): "no rules" is a STATE, and a payload that omitted the key would
# make the write path read every session on a fresh install as a change.
assert out == {"rules": [], "total": 0, "rules_etag": "empty|0"}
@pytest.mark.asyncio
@@ -433,3 +439,125 @@ async def test_unrelate_rules_raises_when_the_edge_is_gone():
from scribe.mcp.tools.rulebooks import unrelate_rules
with pytest.raises(ValueError, match="not found"):
await unrelate_rules(relation_id=99)
# ── rule_history (milestone 323 step 3) ────────────────────────────────
def _fake_version(**over):
"""A RuleVersion-shaped stand-in. A real model instance rather than a
MagicMock, because the tool calls `to_dict` and a mock would hand back
another mock instead of failing."""
from scribe.models.rule_version import RuleVersion
from datetime import datetime, timezone
defaults = {
"id": 5, "rule_id": 100, "user_id": 1,
"title": "The runner has no bash", "statement": "Use sh.",
"why": "the image ships no bash", "how_to_apply": None,
"when_to_apply": None, "tier": "always_on",
"verify_with": "read the workflow's shell setting",
"expires_when": None,
"created_at": datetime(2026, 8, 29, tzinfo=timezone.utc),
}
return RuleVersion(**{**defaults, **over})
@pytest.mark.asyncio
async def test_rule_history_lists_without_the_heavy_text():
"""The listing answers "when, and by whom". A rule's statement runs to
thousands of characters, so a history carrying every field would cost
more to read than the answer is worth."""
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rule_versions",
AsyncMock(return_value=[_fake_version(), _fake_version(id=4)]),
), patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=fake_rule(
id=100, title="The runner has no bash", statement="s", topic_id=10,
)),
):
from scribe.mcp.tools.rulebooks import rule_history
out = await rule_history(rule_id=100)
assert out["total"] == 2
assert out["versions"][0]["title"] == "The runner has no bash"
assert "statement" not in out["versions"][0]
assert "why" not in out["versions"][0]
@pytest.mark.asyncio
async def test_rule_history_opens_one_version_in_full():
"""Passing a version id switches from the index to the text — which is
the whole reason the listing can afford to omit it."""
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule_version",
AsyncMock(return_value=_fake_version()),
):
from scribe.mcp.tools.rulebooks import rule_history
out = await rule_history(rule_id=100, version_id=5)
assert out["statement"] == "Use sh."
assert out["verify_with"] == "read the workflow's shell setting"
@pytest.mark.asyncio
async def test_rule_history_says_an_empty_history_is_ordinary():
"""Most rules have never been reworded, and nothing was written before
milestone 323. Without this line an empty list reads as a lost history or
a broken tool."""
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rule_versions",
AsyncMock(return_value=[]),
), patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=fake_rule(
id=100, title="The runner has no bash", statement="s", topic_id=10,
)),
):
from scribe.mcp.tools.rulebooks import rule_history
out = await rule_history(rule_id=100)
assert out["total"] == 0
assert "never been reworded" in out["note"]
@pytest.mark.asyncio
async def test_rule_history_raises_when_the_rule_is_not_yours():
"""None from the service means "not readable", and the tool must not
turn that into an empty history — which would read as "this rule has no
past" rather than "this is not your rule"."""
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rule_versions",
AsyncMock(return_value=None),
):
from scribe.mcp.tools.rulebooks import rule_history
with pytest.raises(ValueError, match="rule 100 not found"):
await rule_history(rule_id=100)
@pytest.mark.asyncio
async def test_rule_history_raises_for_a_version_on_another_rule():
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule_version",
AsyncMock(return_value=None),
):
from scribe.mcp.tools.rulebooks import rule_history
with pytest.raises(ValueError, match="version 5 not found"):
await rule_history(rule_id=100, version_id=5)
def test_rule_history_docstring_says_what_a_version_HOLDS():
"""The one thing a reader gets wrong unaided: an entry is the text the
edit REPLACED, not the text it introduced. Read the other way, every
diff comes out backwards — so the docstring has to say it, and this is
the guard against a later tidy-up dropping the line."""
from scribe.mcp.tools.rulebooks import rule_history
doc = rule_history.__doc__ or ""
assert "REPLACED" in doc
assert "no restore" in doc.lower(), (
"the docstring no longer explains that a rule version cannot be "
"restored. A caller who assumes a revert exists will look for one "
"and, not finding it, is likely to hand-copy the old text back with "
"no record of why."
)
+12 -3
View File
@@ -138,9 +138,18 @@ async def test_merge_snippets_read_only_target_says_why():
@pytest.mark.asyncio
async def test_delete_snippet_retires_or_raises():
from scribe.mcp.tools.snippets import delete_snippet
with patch("scribe.services.snippets.delete_snippet", AsyncMock(return_value=True)):
assert await delete_snippet(1) == {"deleted": True, "id": 1}
with patch("scribe.services.snippets.delete_snippet", AsyncMock(return_value=False)):
doomed = MagicMock()
doomed.title = "debounce helper"
# The confirmation names what went (#3273) — after the delete there is
# nothing left to look the title up from.
with patch("scribe.services.snippets.delete_snippet", AsyncMock(return_value=True)), \
patch("scribe.services.snippets.get_snippet", AsyncMock(return_value=doomed)):
assert await delete_snippet(1) == {
"deleted": True, "id": 1, "title": "debounce helper",
}
with patch("scribe.services.snippets.delete_snippet", AsyncMock(return_value=False)), \
patch("scribe.services.snippets.get_snippet", AsyncMock(return_value=None)):
with pytest.raises(ValueError):
await delete_snippet(404)
@@ -0,0 +1,106 @@
"""A record is cited by id AND title — never by number alone.
THE PROBLEM THIS IS ABOUT. The agent has the record open; the operator does
not. `#3244` reads as complete to the writer and as homework to the reader,
who has to look it up to know what their own conversation is about. The
operator's words: *"I don't know what a note, task, or milestone is by its ID
number."*
PRODUCT, NOT A RULE (rule 119). Every Scribe user hits this, so the fix is in
the surfaces the product ships the skill that shapes how an agent writes,
and the tool responses that hand a record back. A per-instance rule would fix
it for one operator and leave the behaviour wrong for everyone else.
Scribe's duplicate gate already had the right shape — `id 412: "debounce
helper"` — which is why these assert the CONVENTION reaches the other
surfaces rather than inventing a new one.
"""
import pathlib
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
SKILL = (
pathlib.Path(__file__).resolve().parents[1]
/ "plugin/skills/using-scribe/SKILL.md"
)
def test_the_skill_carries_the_convention():
"""The skill is read while deciding HOW to write, which is the only moment
this can be applied. A tool response can name one record; only the skill
can govern the prose around it."""
text = " ".join(SKILL.read_text().split())
assert "Name the record, never just its number" in text, (
"the using-scribe skill no longer tells an agent to write the title "
"alongside the id. Nothing else governs how records are cited in "
"prose, commit messages, or task bodies."
)
def test_the_skill_says_where_it_matters_most():
"""A convention stated only for chat messages gets applied only there —
and the places read LATER, by someone with even less context, are where a
bare id costs most."""
text = " ".join(SKILL.read_text().split()).lower()
assert "commit message" in text and "task bod" in text
def test_the_skill_says_to_look_up_a_title_it_does_not_know():
"""The escape hatch that would otherwise swallow the convention whole: an
agent that does not know the title will emit the number and move on."""
text = " ".join(SKILL.read_text().split()).lower()
assert "an id you can't name is one you haven't checked" in text
# ── The tool responses (the other half) ────────────────────────────────
#
# A deletion is the sharpest case: afterwards the row is trashed, so if the
# confirmation did not name it, nothing can. An operator who cannot recognise
# what was deleted cannot tell it was the wrong thing.
@pytest.mark.parametrize("module,fn,kind,rid", [
("tasks", "delete_task", "task", 3244),
("notes", "delete_note", "note", 2109),
("milestones", "delete_milestone", "milestone", 323),
])
@pytest.mark.asyncio
async def test_a_delete_confirmation_names_what_it_deleted(
module, fn, kind, rid, monkeypatch,
):
import importlib
mod = importlib.import_module(f"scribe.mcp.tools.{module}")
title = "the staleness signal"
row = MagicMock()
row.title = title
patches = [
patch.object(mod, "current_user_id", MagicMock(return_value=1)),
patch.object(mod.trash_svc, "delete", AsyncMock(return_value="batch-1")),
]
if module == "milestones":
patches.append(
patch.object(mod.milestones_svc, "get_milestone",
AsyncMock(return_value=row)))
else:
patches.append(
patch.object(mod.notes_svc, "get_note_for_user",
AsyncMock(return_value=(row, "owner"))))
for p in patches:
p.start()
try:
out = await getattr(mod, fn)(rid)
finally:
for p in patches:
p.stop()
assert out["title"] == title, f"{fn} returns no title for the record"
assert title in out["message"], (
f"{fn}'s message names only the id. After the delete the row is "
f"trashed, so this line is the last chance to say WHAT went."
)
assert str(rid) in out["message"], (
f"{fn} dropped the id — the title alone is not addressable, and the "
f"convention is id AND title, not one or the other."
)
+15 -6
View File
@@ -1,13 +1,23 @@
"""Tests for task lifecycle timestamps and recurrence logic."""
"""Tests for task lifecycle timestamps and recurrence logic.
The note stand-ins come from `fake_note`, not a bare MagicMock. These four
predated that helper and broke the moment `update_note` started reading a
column they did not set (milestone 317): a fresh MagicMock is truthy on every
attribute, so `verify_with` read as "this record carries a check" and the
guard refused the write. That is note 2109's lesson, and the reason fake_note
exists a stand-in has to be able to say NO.
"""
from datetime import date, datetime, timezone
from unittest.mock import AsyncMock, MagicMock, patch
from tests.helpers import fake_note, make_mock_session
# ── Timestamp side-effect tests ──────────────────────────────────────────────
async def test_update_note_sets_started_at_on_in_progress():
"""started_at is set when status transitions to in_progress."""
mock_note = MagicMock()
mock_note = fake_note()
mock_note.status = "in_progress"
mock_note.started_at = None
mock_note.recurrence_rule = None
@@ -28,7 +38,7 @@ async def test_update_note_sets_started_at_on_in_progress():
async def test_update_note_sets_completed_at_on_done():
"""completed_at is set when status transitions to done."""
mock_note = MagicMock()
mock_note = fake_note()
mock_note.status = "done"
mock_note.started_at = datetime(2026, 3, 1, tzinfo=timezone.utc)
mock_note.completed_at = None
@@ -50,7 +60,7 @@ async def test_update_note_sets_completed_at_on_done():
async def test_update_note_clears_timestamps_on_todo():
"""started_at and completed_at are cleared when status resets to todo."""
mock_note = MagicMock()
mock_note = fake_note()
mock_note.status = "todo"
mock_note.started_at = datetime(2026, 3, 1, tzinfo=timezone.utc)
mock_note.completed_at = datetime(2026, 3, 15, tzinfo=timezone.utc)
@@ -74,7 +84,7 @@ async def test_update_note_clears_timestamps_on_todo():
async def test_update_note_preserves_started_at_if_already_set():
"""started_at is not overwritten on a second transition to in_progress."""
original_start = datetime(2026, 3, 1, tzinfo=timezone.utc)
mock_note = MagicMock()
mock_note = fake_note()
mock_note.status = "in_progress"
mock_note.started_at = original_start
mock_note.recurrence_rule = None
@@ -96,7 +106,6 @@ async def test_update_note_preserves_started_at_if_already_set():
# ── Recurrence rule validation ────────────────────────────────────────────────
import pytest
from tests.helpers import make_mock_session
def test_validate_interval_rule_valid():
+267
View File
@@ -0,0 +1,267 @@
"""The staleness marker on the rules payload (milestone 323 step 5).
WHAT THE MARKER IS FOR: telling a session that the rules it is holding have
MOVED since it loaded them. Not general staleness the limitation is stated
in services/rulebooks.py and in the write-path arm, and two tests here pin the
cases that would otherwise be quietly lost.
The two that matter most are both about NOT crying wolf. A marker that reports
a change when nothing changed gets ignored within a day, and an ignored
staleness signal is worse than none: it trains a reader to skip the line that
will one day be true.
"""
from datetime import datetime, timedelta, timezone
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from scribe.services import rulebooks as svc
NOW = datetime(2026, 8, 30, 12, 0, tzinfo=timezone.utc)
LATER = NOW + timedelta(hours=1)
def _rule(updated_at=NOW, rid=1, title="a rule"):
"""A Rule-shaped stand-in. The marker only ever reads three attributes,
and a real model would need a session to build."""
return SimpleNamespace(id=rid, title=title, updated_at=updated_at)
def test_the_same_set_produces_the_same_marker():
"""The whole mechanism rests on this. If the marker moved on its own, every
write would report a change and the line would be noise by lunchtime."""
rules = [_rule(rid=1), _rule(rid=2, updated_at=LATER)]
assert svc.rules_etag(rules) == svc.rules_etag(list(reversed(rules))), (
"the marker depends on the ORDER rules come back in, so any query "
"whose sort changes would look like an edit"
)
def test_an_edit_moves_the_marker():
before = svc.rules_etag([_rule(rid=1), _rule(rid=2)])
after = svc.rules_etag([_rule(rid=1), _rule(rid=2, updated_at=LATER)])
assert before != after
def test_a_DELETED_rule_moves_the_marker():
"""THE CASE max(updated_at) ALONE CANNOT SEE, and the reason the count is
in there. Deleting a rule moves no timestamp and it is the single change
that takes an instruction OUT of force, which is the one a session most
needs to hear about."""
before = svc.rules_etag([_rule(rid=1), _rule(rid=2)])
after = svc.rules_etag([_rule(rid=1)])
assert before != after, (
"a deleted rule left the marker unchanged — the count is missing, and "
"the session would keep obeying an instruction that no longer exists"
)
def test_no_rules_is_a_state_not_a_change():
"""Rule 115: this has to behave on an install with no rules at all. `max()`
over an empty set raises; a marker that raised would take the whole write
path's hint down, and one that varied would tell every session on a fresh
install that its rules had changed."""
assert svc.rules_etag([]) == svc.rules_etag([])
assert svc.rules_etag([]) != svc.rules_etag([_rule()])
def test_moved_since_names_only_what_actually_moved():
held = svc.rules_etag([_rule(rid=1), _rule(rid=2)])
current = [_rule(rid=1), _rule(rid=2, updated_at=LATER, title="reworded")]
moved = svc.rules_moved_since(current, held)
assert [r.id for r in moved] == [2]
def test_a_matching_marker_names_nothing():
rules = [_rule(rid=1), _rule(rid=2)]
assert svc.rules_moved_since(rules, svc.rules_etag(rules)) == []
def test_an_unreadable_marker_reports_no_change():
"""A caller cannot act on "something differs but I cannot say what", and a
garbled marker must never be rendered as a change that is the shape of a
signal that gets ignored."""
assert svc.rules_moved_since([_rule(updated_at=LATER)], "not-an-etag") == []
assert svc.rules_moved_since([_rule(updated_at=LATER)], "") == []
assert svc.etag_count("garbled") is None
def test_the_empty_marker_reports_no_change():
"""An install that had no rules and now has some: the count says so, and
this function has no timestamp to reason from. Silence here, not a claim."""
assert svc.rules_moved_since([_rule()], svc.rules_etag([])) == []
def test_the_count_survives_the_round_trip():
assert svc.etag_count(svc.rules_etag([_rule(rid=1), _rule(rid=2)])) == 2
assert svc.etag_count(svc.rules_etag([])) == 0
def test_the_limitation_is_stated_where_a_reader_will_be():
"""A future reader who finds an etag will assume it covers staleness
generally. It does not it is blind to compaction, which is the most
common case and the SessionStart nudge is that case's only mechanism.
Pinned because the plausible mistake is retiring a nudge that works on the
strength of a signal that does not cover it, and the comment is the only
thing standing in the way.
"""
import inspect
from scribe.services import plugin_context
for module in (svc, plugin_context):
src = inspect.getsource(module).lower()
assert "compaction" in src and "etag" in src, (
f"{module.__name__} no longer explains what the rules marker "
f"cannot see. Without it the next reader will treat an etag as a "
f"general staleness check and soften the SessionStart nudge."
)
# ── The arm that delivers the message (milestone 323 step 5) ───────────
#
# The marker is worth nothing until a session is actually TOLD. These drive
# the real `build_write_path_hint`, because the feature IS a line arriving in
# a hook's output — a test of the helper alone would prove the arithmetic and
# nothing about the delivery.
def _quiet_write_path(pc, rules):
"""Every other arm stubbed to silent, so the only line that can appear is
the one under test."""
return (
patch.object(pc, "get_writepath_config",
AsyncMock(return_value={"enabled": True, "threshold": 0.6,
"top_k": 3})),
patch.object(pc.snippets_svc, "list_snippets",
AsyncMock(return_value=([], 0))),
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])),
patch.object(pc, "semantic_search_rules", AsyncMock(return_value=[])),
patch.object(pc, "record_retrieval", MagicMock()),
patch.object(pc, "record_surfaced", MagicMock()),
patch.object(pc, "owner_names_for", AsyncMock(return_value={})),
patch.object(pc.rulebooks_svc, "list_always_on_rules",
AsyncMock(return_value=rules)),
)
async def _hint(rules, held_etag):
from scribe.services import plugin_context as pc
patches = _quiet_write_path(pc, rules)
for p in patches:
p.start()
try:
out = await pc.build_write_path_hint(
1, "src/scribe/services/rulebooks.py", code="x" * 400,
rules_etag=held_etag,
)
finally:
for p in patches:
p.stop()
return out["context"]
@pytest.mark.asyncio
async def test_a_session_is_told_which_rule_moved():
"""The delivery, end to end through the real hint builder. Naming the rule
is the point "something changed" sends the reader to re-read everything,
which is the cost the marker was meant to avoid."""
held = svc.rules_etag([_rule(rid=1), _rule(rid=2, title="dev is home")])
current = [_rule(rid=1), _rule(rid=2, title="dev is home", updated_at=LATER)]
ctx = await _hint(current, held)
assert "changed since this session started" in ctx
assert "#2" in ctx and "dev is home" in ctx
@pytest.mark.asyncio
async def test_a_session_holding_the_current_rules_is_told_nothing():
"""The one that keeps the signal worth reading. A line on every write is a
line nobody reads."""
rules = [_rule(rid=1), _rule(rid=2)]
ctx = await _hint(rules, svc.rules_etag(rules))
assert "changed since this session started" not in ctx
@pytest.mark.asyncio
async def test_a_session_that_sent_no_marker_is_told_nothing():
"""An install whose hook never reached /api/plugin/context has nothing
stored. Absent must read as silence, not as a mismatch otherwise the
first thing a new install hears is that its rules changed."""
ctx = await _hint([_rule(updated_at=LATER)], "")
assert "changed since this session started" not in ctx
@pytest.mark.asyncio
async def test_a_DELETED_rule_is_reported_even_though_it_has_no_row():
"""The count arm. A deleted rule leaves nothing to name, and it is the
change that takes an instruction OUT of force so "no longer in force"
has to be sayable without a row to say it about."""
held = svc.rules_etag([_rule(rid=1), _rule(rid=2)])
ctx = await _hint([_rule(rid=1)], held)
assert "no longer in force" in ctx
@pytest.mark.asyncio
async def test_the_arm_fails_open():
"""A staleness hint must never break a write. Every other arm here fails
open for the same reason, and this one runs a query that can fail."""
from scribe.services import plugin_context as pc
patches = _quiet_write_path(pc, [])
for p in patches:
p.start()
try:
with patch.object(pc.rulebooks_svc, "list_always_on_rules",
AsyncMock(side_effect=RuntimeError("database down"))):
out = await pc.build_write_path_hint(
1, "src/x.py", code="x" * 400, rules_etag="2026-01-01T00:00:00+00:00|3",
)
finally:
for p in patches:
p.stop()
assert "changed since this session started" not in out["context"]
def test_the_marker_cannot_break_the_payload_it_decorates():
"""It is computed on the SessionStart path. Raising there would cost the
whole context payload every rule title, the project, the lot to save
a hint, which is the wrong trade in every case.
A row with no usable timestamp is skipped; a set with none degrades to a
count-only marker. Count-only still catches a rule ADDED or DELETED and
only loses edits, which is the right way round to lose information.
Found by CI: `build_session_context` tests hand it MagicMock rules, and
`max()` over those raises TypeError rather than returning anything.
"""
from unittest.mock import MagicMock
assert svc.rules_etag([MagicMock(), MagicMock()]) == "unknown|2"
assert svc.rules_etag([SimpleNamespace()]) == "unknown|1"
# A count-only marker still moves when the set does.
assert svc.rules_etag([MagicMock()]) != svc.rules_etag([MagicMock(), MagicMock()])
# One usable stamp is enough to keep the real thing.
assert svc.rules_etag([_rule(), MagicMock()]).startswith("2026-")
@pytest.mark.asyncio
async def test_the_signal_arrives_even_when_nothing_else_matched():
"""THE BUG CI CAUGHT, and the one the task's acceptance criterion was
written to catch.
`build_write_path_hint` returns early when no prior art, stamp, divergence
or derive matched which sat ABOVE this arm, so a session whose rules had
changed was told only if the file it happened to be editing also matched
something else. A staleness signal that fires on that coincidence is not a
staleness signal.
"""
held = svc.rules_etag([_rule(rid=1), _rule(rid=2, title="dev is home")])
current = [_rule(rid=1), _rule(rid=2, title="dev is home", updated_at=LATER)]
# Every other arm silent — which is exactly the case that used to return "".
ctx = await _hint(current, held)
assert "changed since this session started" in ctx
+2 -3
View File
@@ -20,10 +20,9 @@ from scribe.services.access import (
notes_visibility_clause,
readable_notes_clause,
)
from tests.helpers import compiled_sql
def _sql(clause) -> str:
return str(clause.compile(compile_kwargs={"literal_binds": True}))
_sql = compiled_sql
def _read(user_id: int = 7) -> str:
+240 -6
View File
@@ -16,12 +16,246 @@ import pytest
from scribe.services import backup
def test_backup_version_is_v8():
"""v7 added code_shapes (#2787), v8 its history (#2793). The bump is the
point of the test a payload section added without moving the version
produces backups that are structurally different and indistinguishable
by inspection."""
assert backup.BACKUP_VERSION == 10
def test_backup_version_is_current():
"""The bump is the point of the test — a payload section added without
moving the version produces backups that are structurally different and
indistinguishable by inspection.
(Named for the number it asserted until v10, which is exactly the drift a
name-carrying-a-value invites; it now says what it checks.)"""
assert backup.BACKUP_VERSION == 13
def _exportable_note(**over):
"""A Note-shaped stand-in for the pure row helper. SimpleNamespace, not a
MagicMock: `_note_rows` calls .isoformat() on the timestamps, and a mock
would happily return another mock instead of failing."""
base = dict(
id=1, user_id=7, title="t", body="b", description=None, tags=["x"],
parent_id=None, arose_from_id=None,
project_id=None, milestone_id=None, status=None, priority=None,
due_date=None, created_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
updated_at=datetime(2026, 1, 2, tzinfo=timezone.utc),
note_type="note", task_kind="work", data=None,
started_at=None, completed_at=None,
recurrence_rule=None, recurrence_next_spawn_at=None,
verify_with=None, expires_when=None, verified_at=None,
)
base.update(over)
return SimpleNamespace(**base)
def test_note_rows_carry_the_verification_trio():
"""Operator judgment — somebody checked this fact, and this is when —
which nothing downstream can recompute (milestone 317)."""
[row] = backup._note_rows([_exportable_note(
verify_with="curl the AMO docs",
expires_when="AMO starts allowing re-signing",
verified_at=datetime(2026, 8, 28, tzinfo=timezone.utc),
)])
assert row["verify_with"] == "curl the AMO docs"
assert row["expires_when"] == "AMO starts allowing re-signing"
assert row["verified_at"] == "2026-08-28T00:00:00+00:00"
def test_a_never_checked_note_exports_a_null_stamp_and_restores_as_one():
"""The round trip that matters. NULL `verified_at` means nobody has ever
looked, and it is what sorts FIRST in the sweep. Restoring it as now()
which is what `_dt` would do silently converts the sweep's top result
into its bottom one."""
[row] = backup._note_rows([_exportable_note(verify_with="check the runner shell")])
assert row["verified_at"] is None
assert backup._dt_or_none(row["verified_at"]) is None
# ...and the helper that must NOT be used here, for contrast.
assert backup._dt(row["verified_at"]) is not None
def test_the_record_type_and_kind_survive_the_export():
"""#3182's headline. Without these two columns a restore reported success
and handed back a corpus where all 90 snippets and 3 processes were plain
notes and all 435 issues and the spike were `work` the entire vocabulary
milestone 312 and #3128 were about, gone, with nothing to notice it by."""
[snippet] = backup._note_rows([_exportable_note(note_type="snippet")])
[issue] = backup._note_rows([_exportable_note(task_kind="issue", status="done")])
assert snippet["note_type"] == "snippet"
assert issue["task_kind"] == "issue"
def test_provenance_and_lifecycle_travel():
started = datetime(2026, 3, 1, tzinfo=timezone.utc)
[row] = backup._note_rows([_exportable_note(
arose_from_id=42,
description="one-liner",
started_at=started,
recurrence_rule={"freq": "weekly"},
)])
assert row["arose_from_id"] == 42
assert row["description"] == "one-liner"
assert row["started_at"] == started.isoformat()
assert row["recurrence_rule"] == {"freq": "weekly"}
# Absent lifecycle stamps stay absent — a note that never started must not
# restore as one that started at restore time.
assert row["completed_at"] is None
def test_a_milestone_carries_its_plan():
"""A milestone IS the plan (0066); `body` is its design and intent and
`description` is only the one-line summary. Dropping it restored every
plan as a title with no reasoning behind it (#3182)."""
m = SimpleNamespace(
id=1, user_id=7, project_id=2, title="t", description="d",
body="## Goal\n\nthe actual plan", status="active", order_index=0,
created_at=datetime(2026, 1, 1, tzinfo=timezone.utc),
updated_at=datetime(2026, 1, 2, tzinfo=timezone.utc),
)
[row] = backup._milestone_rows([m])
assert row["body"] == "## Goal\n\nthe actual plan"
def test_a_repo_binding_carries_the_branch_its_ledger_follows():
"""#2873. Without `ref` a restored binding silently falls back to the
default branch and the shape ledger starts accounting for a different
tree a wrong answer that looks like a working one."""
b = SimpleNamespace(user_id=7, project_id=2, repo_key="Scribe", ref="dev")
[row] = backup._repo_binding_rows([b])
assert row["ref"] == "dev"
# The table -> (model, row helper) registry the column guard walks. Kept here
# rather than in the service because it exists only to be introspected: the
# product code already knows these pairings by calling them.
def _column_guard_targets():
from scribe.models.canonical_system import CanonicalSystem
from scribe.models.code_shape import CodeShape, CodeShapeEvent, CodeShapeUse
from scribe.models.design_system import DesignSystem, DesignToken
from scribe.models.milestone import Milestone
from scribe.models.note import Note
from scribe.models.note_draft import NoteDraft
from scribe.models.note_supersession import NoteSupersession
from scribe.models.note_usage import NoteUsageEvent
from scribe.models.note_version import NoteVersion
from scribe.models.rule_version import RuleVersion
from scribe.models.project import Project
from scribe.models.repo_binding import RepoBinding
from scribe.models.rulebook import Rule, Rulebook, RulebookTopic, RuleRelation
from scribe.models.setting import Setting
from scribe.models.system import RecordSystem, System
from scribe.models.task_log import TaskLog
from scribe.models.user import User
return {
"users": (User, backup._user_rows),
"projects": (Project, backup._project_rows),
"milestones": (Milestone, backup._milestone_rows),
"notes": (Note, backup._note_rows),
"task_logs": (TaskLog, backup._task_log_rows),
"note_drafts": (NoteDraft, backup._note_draft_rows),
"note_versions": (NoteVersion, backup._note_version_rows),
"settings": (Setting, backup._setting_rows),
"rulebooks": (Rulebook, backup._rulebook_rows),
"rulebook_topics": (RulebookTopic, backup._topic_rows),
"rules": (Rule, backup._rule_rows),
"rule_versions": (RuleVersion, backup._rule_version_rows),
"systems": (System, lambda rows: backup._system_rows(rows, {})),
"canonical_systems": (CanonicalSystem, backup._canonical_system_rows),
"record_systems": (RecordSystem, backup._record_system_rows),
"note_supersessions": (NoteSupersession, backup._note_supersession_rows),
"rule_relations": (RuleRelation, backup._rule_relation_rows),
"note_usage_events": (NoteUsageEvent, backup._usage_event_rows),
"design_systems": (DesignSystem, backup._design_system_rows),
"design_tokens": (DesignToken, backup._design_token_rows),
"repo_bindings": (RepoBinding, backup._repo_binding_rows),
"code_shapes": (CodeShape, backup._code_shape_rows),
"code_shape_events": (CodeShapeEvent, backup._code_shape_event_rows),
"code_shape_uses": (CodeShapeUse, backup._code_shape_use_rows),
}
def _stand_in(model):
"""A real instance of `model` with every column set to a value of roughly
the right type, so the serialiser runs and we can read which KEYS it
produced. Values are meaningless; only the shape of the output dict is
under test.
A real instance rather than a MagicMock because several helpers delegate to
the model's own `to_dict()`, and a mock would return another mock instead
of a dict. Typed rather than a bare instance because the helpers call
`.isoformat()` on the timestamps, which `None` does not have.
"""
import sqlalchemy as sa
row = model()
for column in model.__table__.columns:
t = column.type
if isinstance(t, sa.DateTime):
value = datetime(2026, 1, 1, tzinfo=timezone.utc)
elif isinstance(t, sa.Date):
value = datetime(2026, 1, 1).date()
elif isinstance(t, sa.Boolean):
value = False
elif isinstance(t, sa.Integer):
value = 1
elif isinstance(t, sa.ARRAY) or isinstance(getattr(t, "impl", None), sa.ARRAY):
value = []
elif isinstance(t, (sa.Text, sa.String)):
value = "x"
else:
# JSON/JSONB and anything exotic. None is what these actually hold
# most of the time, and no serialiser calls a method on one.
value = None
setattr(row, column.name, value)
return row
@pytest.mark.parametrize("table", sorted(_column_guard_targets()))
def test_every_column_is_exported_or_declared_excluded(table):
"""THE COLUMN GUARD (#3182) — _NOT_INCLUDED's shape, one level down.
The table guard below catches a whole table going missing. It cannot catch
a COLUMN going missing from a table it already considers covered, which is
how nine of them vanished from `notes` alone: note_type and task_kind, so
every snippet and process restored as a plain note and every issue and
spike as `work`; arose_from_id, so every provenance edge went; the
recurrence pair, so recurring tasks stopped recurring. Plus milestones.body
which IS the plan and repo_bindings.ref.
Each arrived the same way: added to the model and the migration, both of
which fail loudly, and never to the serialiser, which fails silently.
A new column must now be exported or named in _COLUMN_EXCLUSIONS with a
reason. Forgetting is no longer expressible.
"""
model, helper = _column_guard_targets()[table]
[row] = helper([_stand_in(model)])
columns = {c.name for c in model.__table__.columns}
missing = columns - set(row)
declared = backup._COLUMN_EXCLUSIONS[table]
assert missing == declared, (
f"{table}: exported columns and _COLUMN_EXCLUSIONS disagree.\n"
f" dropped but not declared: {sorted(missing - declared)}\n"
f" declared but exported anyway: {sorted(declared - missing)}"
)
def test_the_column_guard_covers_every_table_with_a_row_helper():
"""The guard is only as good as its registry — a table added to _BACKED_UP
with a new helper, and not to the registry, would be unguarded and look
guarded. Join tables have no model class and carry both their columns by
construction, so they are the only permitted absences."""
# REAL table names, as _BACKED_UP holds them — not the shorter keys the
# payload uses for the same sections. Getting this wrong is what the guard
# caught on its own first run.
join_tables = {
"project_rulebook_subscriptions", "project_rule_suppressions",
"project_topic_suppressions", "project_rulebook_exclusions",
"rule_systems",
}
covered = set(_column_guard_targets()) | join_tables
assert set(backup._BACKED_UP) - covered == set()
# And no stale entries: every declaration must name a real target.
assert set(backup._COLUMN_EXCLUSIONS) == set(_column_guard_targets())
def test_not_included_lists_the_known_gaps():
+69 -22
View File
@@ -1,4 +1,12 @@
"""get_knowledge_counts includes the 'process' type and counts it in total."""
"""get_knowledge_counts — one number per facet, and an honest total.
Two grouped queries, one per typing axis. It used to be three: a grouped count
over note_type restricted to ("note", "process"), a scalar count of tasks, and
a second scalar just for plans. That shape is why `issue` had no number
every kind needed a query of its own and nobody added one and why the "All"
chip sat ~90 below the list it labelled, since snippets were in the feed but
in no count (#3128).
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
@@ -11,29 +19,68 @@ def _grouped(rows):
return r
def _scalar(n):
r = MagicMock()
r.scalar_one.return_value = n
return r
@pytest.mark.asyncio
async def test_counts_include_process_in_facet_and_total():
async def _counts(non_task_rows, kind_rows, **kwargs):
session = make_mock_session()
# 1) grouped non-task counts, 2) task count, 3) plan count
session.execute = AsyncMock(side_effect=[
_grouped([("note", 3), ("process", 2)]),
_scalar(1), # tasks
_scalar(0), # plans
])
# 1) non-task rows grouped by note_type, 2) task rows grouped by task_kind
session.execute = AsyncMock(
side_effect=[_grouped(non_task_rows), _grouped(kind_rows)]
)
with patch("scribe.services.knowledge.async_session") as cls:
cls.return_value = session
from scribe.services.knowledge import get_knowledge_counts
counts = await get_knowledge_counts(user_id=1)
return await get_knowledge_counts(user_id=1, **kwargs), session
assert counts["process"] == 2
# facet keys all present (setdefault)
for key in ("note", "task", "plan", "process"):
assert key in counts
# total = note(3) + task(1) + process(2)
assert counts["total"] == 6
@pytest.mark.asyncio
async def test_every_facet_gets_a_number_including_the_kinds():
counts, _ = await _counts(
[("note", 395), ("process", 3), ("snippet", 90)],
[("work", 2104), ("issue", 435), ("spike", 1), ("plan", 90)],
)
assert counts["note"] == 395
assert counts["process"] == 3
assert counts["snippet"] == 90
assert counts["issue"] == 435
assert counts["spike"] == 1
assert counts["plan"] == 90
assert counts["work"] == 2104
@pytest.mark.asyncio
async def test_task_is_the_sum_of_its_kinds():
"""`task` is not counted separately any more — it is what the kinds add up
to, so the two can't disagree."""
counts, _ = await _counts([], [("work", 2104), ("issue", 435), ("spike", 1)])
assert counts["task"] == 2540
@pytest.mark.asyncio
async def test_total_counts_snippets_and_counts_no_task_twice():
"""The All chip labels a feed that contains every kind, so it has to count
every kind and exactly once. Kinds are subsets of `task`; adding them
would count each issue a second time."""
counts, _ = await _counts(
[("note", 10), ("process", 2), ("snippet", 5)],
[("work", 20), ("issue", 4)],
)
assert counts["task"] == 24
assert counts["total"] == 10 + 2 + 5 + 24
@pytest.mark.asyncio
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"):
assert counts[key] == 0, key
assert counts["total"] == 1
@pytest.mark.asyncio
async def test_a_tag_filter_narrows_both_axes():
"""A tag has to reach both queries, or the chips would disagree with each
other under a filter tasks narrowed, notes not."""
_, session = await _counts([], [], tags=["python"])
assert session.execute.await_count == 2
for call in session.execute.await_args_list:
assert "notes.tags" in str(call.args[0])
+242
View File
@@ -0,0 +1,242 @@
"""The note staleness sweep (milestone 317 step 3).
The query is a sibling of the rules sweep, not a shared implementation a
rule scopes by rulebook ownership, a note by the note ACL, so only the
BEHAVIOUR is common (services/verification). These pin the parts that would
fail silently rather than loudly: the ordering, which rows are eligible, and
the asymmetry of a failed check.
"""
from datetime import datetime, timedelta, timezone
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from tests.helpers import compiled_sql, fake_note, make_mock_session
async def _sweep_sql(**kwargs):
"""The statement the sweep builds, as SQL text.
No database: the SHAPE of the query is what is under test, and the
ordering clause in particular cannot be checked any other way without one.
The await happens INSIDE the patch a coroutine created in the block and
awaited outside it would run against the real session.
"""
captured = {}
session = make_mock_session()
async def _execute(stmt):
try:
captured["sql"] = compiled_sql(stmt)
except Exception:
# Literal-rendering a datetime bind is dialect-dependent and can
# raise. Only the tests asserting on a literal value (user_id,
# project_id) need that form, and none of those build a cutoff.
captured["sql"] = str(stmt)
result = MagicMock()
result.scalars.return_value.all.return_value = []
return result
session.execute = _execute
with patch("scribe.services.notes.async_session") as cls:
cls.return_value = session
from scribe.services.notes import notes_due_for_verification
await notes_due_for_verification(user_id=7, **kwargs)
return captured["sql"]
def _where(sql: str) -> str:
"""Just the WHERE clause. `select(Note)` names every column, so searching
the whole statement for a column name always finds the SELECT list which
is how one of these tests first failed for the wrong reason.
Matched by regex rather than split on a literal, because the exact
whitespace SQLAlchemy emits around WHERE is not something a test should
depend on.
"""
import re
m = re.search(r"\bWHERE\b(.*?)(?:\bORDER BY\b|$)", sql, re.S)
assert m, "no WHERE clause — the sweep must never select the whole table"
return m.group(1)
# ── the ordering, which is the whole signal ──────────────────────────────────
@pytest.mark.asyncio
async def test_never_checked_sorts_first_not_last():
"""THE thing most likely to be got wrong, and it would not error.
Postgres sorts NULLs LAST on ASC by default, so the obvious `ORDER BY
verified_at ASC` sinks every never-checked note below every checked one
exactly inverting the signal the sweep exists to carry. A note nobody has
ever confirmed is a claim with no evidence behind it at all.
"""
sql = await _sweep_sql()
assert "ORDER BY notes.verified_at ASC NULLS FIRST" in sql
@pytest.mark.asyncio
async def test_the_order_is_total():
"""A tiebreak, so two notes verified in the same transaction do not swap
places between calls and make a page boundary lie.
Asserted on the END of the statement, not by searching it: `select(Note)`
names every column, so the first `notes.id` in the text is the SELECT
list, not the ORDER BY."""
sql = (await _sweep_sql()).rstrip()
assert sql.endswith("ORDER BY notes.verified_at ASC NULLS FIRST, notes.id")
# ── which rows are eligible ──────────────────────────────────────────────────
@pytest.mark.asyncio
async def test_only_notes_that_carry_a_check_appear():
"""A note with no `verify_with` is not overdue — it is a decision, and
listing it would dilute the result until nobody reads it."""
assert "notes.verify_with IS NOT NULL" in await _sweep_sql()
@pytest.mark.asyncio
async def test_trashed_notes_are_excluded():
assert "notes.deleted_at IS NULL" in await _sweep_sql()
@pytest.mark.asyncio
async def test_the_scope_is_browse_not_read():
"""Decision note 2094: a sweep is a PASSIVE surface, so a record shared
one-to-one must not arrive in one unasked. `note_shares` is the tell
its presence would mean the read scope leaked in."""
sql = await _sweep_sql()
assert "notes.user_id = 7" in sql
assert "project_shares" in sql
assert "note_shares" not in sql
@pytest.mark.asyncio
async def test_a_task_carrying_a_check_is_NOT_filtered_out():
"""Deliberate. The write path permits a check on nothing but a plain note,
so such a row would be in an ILLEGAL state and this is the one surface
that could tell somebody. Hiding it to match the invariant would make the
sweep agree with a database it had stopped describing."""
where = _where(await _sweep_sql())
assert "notes.status" not in where
assert "notes.note_type" not in where
# ── the filters ──────────────────────────────────────────────────────────────
@pytest.mark.asyncio
async def test_never_only_narrows_to_the_unexamined():
assert "notes.verified_at IS NULL" in await _sweep_sql(never_only=True)
@pytest.mark.asyncio
async def test_an_age_window_still_includes_the_never_checked():
"""They are the most overdue thing there is; a window that excluded them
would answer the opposite of the question."""
sql = await _sweep_sql(older_than_days=30)
assert "notes.verified_at IS NULL" in sql
assert "notes.verified_at <" in sql
@pytest.mark.asyncio
async def test_never_only_wins_over_an_age_window():
"""Both together is a caller contradicting themselves; the narrower one is
the safe reading, and it must not emit a cutoff as well."""
sql = await _sweep_sql(never_only=True, older_than_days=30)
assert "notes.verified_at <" not in sql
@pytest.mark.asyncio
async def test_a_project_filter_narrows():
assert "notes.project_id = 4" in await _sweep_sql(project_id=4)
@pytest.mark.asyncio
async def test_a_negative_window_raises_rather_than_meaning_everything():
"""Silently answering a different question is the failure this guards."""
with pytest.raises(ValueError, match="older_than_days"):
await _sweep_sql(older_than_days=-1)
# ── the stamp ────────────────────────────────────────────────────────────────
async def _mark(note, still_true=True, writable=True):
session = make_mock_session()
result = MagicMock()
result.scalars.return_value.first.return_value = note
session.execute = AsyncMock(return_value=result)
with patch("scribe.services.notes.async_session") as cls, \
patch("scribe.services.access.can_write_note",
AsyncMock(return_value=writable)):
cls.return_value = session
from scribe.services.notes import mark_note_verified
return await mark_note_verified(note_id=1, user_id=7,
still_true=still_true), session
@pytest.mark.asyncio
async def test_a_passing_check_stamps_the_note():
note = fake_note(verify_with="curl the docs", verified_at=None)
out, session = await _mark(note)
assert out is note
assert note.verified_at is not None
session.commit.assert_awaited()
@pytest.mark.asyncio
async def test_a_failing_check_writes_nothing():
"""The asymmetry IS the design. There is no "verified false" state,
because a note whose check failed is not in a special condition it is
WRONG. Recording the failure as a flag would let it sit there being false
with the sweep quietly satisfied that somebody had looked."""
note = fake_note(verify_with="curl the docs", verified_at=None)
out, session = await _mark(note, still_true=False)
assert out is note
assert note.verified_at is None, "a failed check must leave the stamp alone"
session.commit.assert_not_awaited()
@pytest.mark.asyncio
async def test_a_note_with_no_check_cannot_be_verified():
"""Nothing to verify is a different answer from verified."""
note = fake_note(verify_with=None)
out, _ = await _mark(note)
assert out is None
@pytest.mark.asyncio
async def test_a_reader_cannot_stamp():
"""Stamping is a mutation (rules 47/78) — an editor-share holder may make
it, a viewer may not."""
note = fake_note(verify_with="curl the docs", verified_at=None)
out, _ = await _mark(note, writable=False)
assert out is None
assert note.verified_at is None
# ── the row ──────────────────────────────────────────────────────────────────
def test_the_row_carries_the_check_in_full():
"""The opposite call from a listing: the caller is about to go and run it,
so the text IS the payload rather than the bloat."""
note = fake_note(
id=9, title="Versioning", project_id=2,
verify_with="curl the AMO docs", expires_when="AMO allows re-signing",
verified_at=datetime.now(timezone.utc) - timedelta(days=74),
)
from scribe.services.notes import verification_row
row = verification_row(note)
assert row["verify_with"] == "curl the AMO docs"
assert row["expires_when"] == "AMO allows re-signing"
assert row["days_since_verified"] == 74
def test_a_never_checked_row_says_never_not_none():
"""None means "this is a decision, the question does not apply"; "never"
means "it asserts a fact and nobody has confirmed it". The second is the
one worth acting on, and collapsing them loses the sweep's point."""
from scribe.services.notes import verification_row
row = verification_row(fake_note(verify_with="a check", verified_at=None))
assert row["last_verified"] == "never"
assert row["days_since_verified"] is None
+172
View File
@@ -0,0 +1,172 @@
"""A note's own check — verify_with / expires_when (milestone 317 step 2).
Three behaviours the rules path (#3096) had to get right and this one inherits:
empty means NULL, clearing is explicit, and rewriting the check drops the
stamp. Plus one this path adds: not every record may carry a check, and the
rule is an INVARIANT over the record rather than a filter on the write.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from tests.helpers import drive_update_note as _update
from tests.helpers import fake_note, make_mock_session
def _checkable(**over):
"""A plain note — no status, no snippet type, no check. `fake_note` is a
MagicMock, so every one of these must be set explicitly: an unset
attribute is a truthy mock, which would look like a check that is there."""
base = dict(
status=None, note_type="note",
verify_with=None, expires_when=None, verified_at=None,
project_id=None,
)
base.update(over)
return fake_note(**base)
async def _create(**kwargs):
session = make_mock_session()
captured = {}
session.add = MagicMock(side_effect=lambda obj: captured.update(
verify_with=getattr(obj, "verify_with", "MISSING"),
expires_when=getattr(obj, "expires_when", "MISSING"),
))
with patch("scribe.services.notes.async_session") as cls, \
patch("scribe.services.notes.embed_note", MagicMock()), \
patch("scribe.services.notes._maybe_reactivate_project", AsyncMock()):
cls.return_value = session
from scribe.services.notes import create_note
await create_note(user_id=1, title="t", **kwargs)
return captured
# ── empty means empty ────────────────────────────────────────────────────────
@pytest.mark.asyncio
async def test_an_empty_check_is_stored_as_null_not_as_a_blank():
"""The sweep's whole signal is `verify_with IS NULL` = "this is a decision,
there is nothing to check". A "" that is not NULL makes a norm look like a
constraint nobody has verified and never-checked sorts FIRST, so it would
sit at the top of the sweep forever."""
assert (await _create(verify_with="", expires_when=""))["verify_with"] is None
note = _checkable(verify_with="curl the docs")
await _update(note, verify_with="")
assert note.verify_with is None
@pytest.mark.asyncio
async def test_a_check_is_stored_when_given():
captured = await _create(verify_with="curl the AMO docs", expires_when="AMO allows re-signing")
assert captured["verify_with"] == "curl the AMO docs"
assert captured["expires_when"] == "AMO allows re-signing"
# ── clearing is explicit ─────────────────────────────────────────────────────
@pytest.mark.asyncio
async def test_clear_unsets_a_field_the_mcp_door_cannot_empty():
"""At the MCP door "" means "leave this alone", so an agent updating a body
does not wipe a check it was never asked about. That leaves no value
meaning "remove it" hence naming the field, which cannot happen by
accident."""
note = _checkable(verify_with="a check", expires_when="a state")
await _update(note, clear=["verify_with"])
assert note.verify_with is None
assert note.expires_when == "a state", "clearing one must not clear the other"
@pytest.mark.asyncio
async def test_clear_ignores_a_field_that_is_not_clearable():
note = _checkable(title="keep me", verify_with="a check")
await _update(note, clear=["title"])
assert note.title == "keep me"
# ── the stamp certifies a check, not a record ────────────────────────────────
@pytest.mark.asyncio
async def test_rewriting_the_check_drops_the_stamp():
note = _checkable(verify_with="the old check", verified_at="2026-01-01")
await _update(note, verify_with="a different check")
assert note.verified_at is None
@pytest.mark.asyncio
async def test_clearing_the_check_drops_the_stamp():
note = _checkable(verify_with="the old check", verified_at="2026-01-01")
await _update(note, clear=["verify_with"])
assert note.verified_at is None
@pytest.mark.asyncio
async def test_an_unchanged_check_keeps_its_stamp():
"""Only a CHANGE invalidates it — otherwise every unrelated edit would
re-enter the note into the sweep and the signal would mean nothing."""
note = _checkable(verify_with="the same check", verified_at="2026-01-01")
await _update(note, title="a new title", verify_with="the same check")
assert note.verified_at == "2026-01-01"
@pytest.mark.asyncio
async def test_a_stamp_cannot_be_set_through_an_ordinary_edit():
"""A stamp says somebody performed THIS check. Minting one from a write
that ran no check is the one thing that would make the sweep lie."""
note = _checkable(verify_with="a check", verified_at=None)
await _update(note, verified_at="2026-08-28")
assert note.verified_at is None
# ── the invariant: which records may carry a check ───────────────────────────
@pytest.mark.asyncio
async def test_a_task_is_refused_a_check():
with pytest.raises(ValueError, match="task"):
await _create(status="todo", verify_with="a check")
@pytest.mark.asyncio
async def test_a_snippet_is_refused_and_told_where_to_go():
"""The message has to name the alternative, or the caller is left with a
refusal and no route."""
with pytest.raises(ValueError, match="verify_snippet"):
await _create(note_type="snippet", verify_with="a check")
@pytest.mark.asyncio
async def test_turning_a_checked_note_into_a_task_is_refused():
"""THE reason this is an invariant over the resulting record and not a
filter on which fields were passed. This write names no check at all, and
would sail past any per-field gate."""
note = _checkable(verify_with="a check")
with pytest.raises(ValueError, match="task"):
await _update(note, status="todo")
@pytest.mark.asyncio
async def test_an_unchecked_note_can_still_become_a_task():
"""The invariant must not make ordinary promotion impossible."""
note = _checkable()
await _update(note, status="todo")
assert note.status == "todo"
@pytest.mark.asyncio
async def test_clearing_the_check_in_the_same_write_lets_it_become_a_task():
"""The error tells the caller to clear the check first; doing both at once
has to actually work, or the advice is wrong."""
note = _checkable(verify_with="a check")
await _update(note, status="todo", clear=["verify_with"])
assert note.status == "todo"
assert note.verify_with is None
@pytest.mark.asyncio
async def test_an_ordinary_note_write_is_untouched_by_any_of_this():
"""The common case: no check, nothing to guard, nothing to reset."""
note = _checkable(title="before")
await _update(note, title="after")
assert note.title == "after"
assert note.verify_with is None
assert note.verified_at is None
+95
View File
@@ -0,0 +1,95 @@
"""A snippet's `data` mirror survives the GENERIC note door.
`notes.data` is derived from the body. The snippet service always composed it
from the field set it had just merged, so `update_snippet` was never the
problem the problem was every other way a snippet's body could be written.
`update_note` is a `hasattr` loop with no snippet awareness, and both doors
reach it: PATCH /api/notes/<id> and the MCP update_note tool. The Knowledge
feed handed you that path, because a snippet card there routed to /notes/:id.
The failure was silent and the wrong way round: `snippet_fields` PREFERS the
mirror, so the row went on reporting its old repo/path/symbol to the location
reverse lookup and to prior-art recall while displaying its new body a record
surfaced with full authority and wrong, which the drift-check docstring calls
worse than having no record at all (#3128).
"""
import pytest
from tests.helpers import drive_update_note as _update
from tests.helpers import fake_note, fake_snippet
OLD_MIRROR = {
"name": "debounce",
"language": "javascript",
"locations": [{"repo": "Scribe", "path": "old/place.js", "symbol": "debounce"}],
"verification": {"status": "ok", "code_sha": "abc", "checked_at": "2026-01-01"},
"provenance": {"commit_sha": "deadbeef"},
}
MOVED_BODY = (
"**Locations:**\n"
"- `Scribe` · `new/place.ts` · `debounce`\n\n"
"```typescript\nexport const debounce = 1;\n```\n"
)
@pytest.mark.asyncio
async def test_a_body_write_moves_the_mirror_with_it():
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
await _update(note, body=MOVED_BODY)
assert note.data["locations"] == [
{"repo": "Scribe", "path": "new/place.ts", "symbol": "debounce"}
], "the mirror still describes where the snippet used to live"
assert note.data["language"] == "typescript"
@pytest.mark.asyncio
async def test_the_verdict_and_provenance_are_carried_not_dropped():
"""Neither is in the body to parse, so recomposing must carry them. An
ordinary edit must not erase the last drift check and it needs no
invalidation branch either: `code_sha` is recomputed from the new code, so
a verdict stamped against the old code expires itself on read."""
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
await _update(note, body=MOVED_BODY)
assert note.data["verification"] == OLD_MIRROR["verification"]
assert note.data["provenance"] == OLD_MIRROR["provenance"]
assert note.data["code_sha"] != OLD_MIRROR["verification"]["code_sha"]
@pytest.mark.asyncio
async def test_an_explicit_data_wins_over_recomposition():
"""`update_snippet` composes the mirror from the merged field set it holds
and passes it here. That caller knows things the body cannot be re-read for
which locations were replaced, whether provenance survives the edit so
an explicit mirror must not be recomputed out from under it."""
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
authoritative = {"name": "from the service", "locations": []}
await _update(note, body=MOVED_BODY, data=authoritative)
assert note.data == authoritative
@pytest.mark.asyncio
async def test_a_plain_note_is_left_alone():
"""Only snippets carry a mirror; a note's `data` must not be invented."""
note = fake_note(note_type="note", data=None, project_id=None)
await _update(note, body="just some prose")
assert note.data is None
@pytest.mark.asyncio
async def test_a_write_that_cannot_change_the_parse_does_not_touch_the_mirror():
"""Status, priority, project — none of them is an input to the body parser,
so recomposing on them would be work for nothing and would rebuild a mirror
from a body nobody claimed to have changed."""
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
await _update(note, project_id=4)
assert note.data == OLD_MIRROR
@pytest.mark.asyncio
async def test_a_title_change_reaches_the_mirror_too():
"""A snippet's NAME lives in its title, not its body — `parse_snippet_fields`
reads both, so both are triggers."""
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
await _update(note, title="throttle — cap a callback's rate")
assert note.data["name"] == "throttle"
assert note.data["when_to_use"] == "cap a callback's rate"
@@ -0,0 +1,108 @@
"""The write surfaces say WHEN a note earns a check — not just that it can.
WHY THIS EXISTS
`verify_with` is a free-text field on the highest-volume record kind in the
product. A field described only as "how to verify this note" gets filled in on
every note within a week, and at that point the sweep returns the whole corpus
and means nothing. The signal is not "has a check" it is "has a check AND
almost nothing else does".
So the guidance is not decoration on this feature, it IS the feature's
precondition, and rule 119 puts it in the tool docstrings and the skill rather
than in a Scribe rule. That makes it exactly the kind of prose a later
docstring tidy-up deletes without noticing what it was for.
WHAT THIS PINS, AND WHAT IT DOES NOT
Structure, never wording, for `test_create_tools_disambiguate`'s reason: a test
that punishes rewriting is a test that gets deleted. Each write surface must
still (a) draw the norm-vs-constraint distinction in some form, (b) say the
empty case is normal, and (c) name where NOT to reach for it.
It cannot tell whether the guidance is any good only that the paragraph
explaining when to leave the field alone has not quietly become a parameter
list.
"""
import re
import pytest
from tests.helpers import tool_doc as _doc
# The surfaces that OFFER the field. The read surfaces (the sweep, the stamp)
# explain what a RESULT means, which is a different job — they are deliberately
# not held to this.
_WRITE_SURFACES = [
("scribe.mcp.tools.notes", "create_note"),
("scribe.mcp.tools.notes", "update_note"),
]
@pytest.mark.parametrize("module,name", _WRITE_SURFACES)
def test_the_norm_versus_constraint_distinction_is_stated(module, name):
"""The whole discipline in one line: a decision cannot go stale, a claim
about someone else's software can. Without it, "how would you check this"
reads as a chore to complete rather than a question with a usual answer of
"you wouldn't"."""
doc = _doc(module, name).lower()
assert "norm" in doc and "constraint" in doc, (
f"{name} no longer draws the norm-vs-constraint distinction. Without "
f"it the field is just a box, and a box gets filled in."
)
@pytest.mark.parametrize("module,name", _WRITE_SURFACES)
def test_the_empty_case_is_stated_as_normal(module, name):
"""Said POSITIVELY, or an empty field reads as an unfinished record. This
is the single sentence standing between the sweep and irrelevance."""
doc = _doc(module, name).lower()
assert re.search(r"leave (it|both|them|this|these)? ?empty|empty for", doc), (
f"{name} no longer says that leaving the check empty is the normal "
f"case. Most notes are decisions; the field's default must read as a "
f"deliberate state, not a gap."
)
@pytest.mark.parametrize("module,name", _WRITE_SURFACES)
def test_the_wrong_places_to_reach_for_it_are_named(module, name):
"""A task's decay is its status; a snippet has verify_snippet. The service
refuses both this is what should mean nobody ever hits that error."""
doc = _doc(module, name).lower()
assert "task" in doc and "snippet" in doc, (
f"{name} no longer names the records that must NOT carry a check. The "
f"gate still refuses them, but a refusal the caller could have "
f"foreseen is a worse door than one that explained itself."
)
def test_expires_when_is_described_as_a_state_not_a_date():
"""The one field whose obvious reading is wrong. A date invents a staleness
schedule nobody can justify; a constraint expires when the ground moves,
which is a condition and not a time."""
doc = _doc("scribe.mcp.tools.notes", "create_note")
assert "STATE" in doc, (
"create_note no longer says expires_when is a STATE. Left to itself, "
'"expires" reads as a date, and every check would get an arbitrary one.'
)
def test_the_skill_carries_the_test_a_writer_can_actually_apply():
"""The docstrings are read by whatever is holding the tool; the skill is
read while deciding what to write. The one-question form has to be in the
second place too, or the guidance only reaches callers who already opened
the tool."""
import pathlib
skill = pathlib.Path(__file__).resolve().parents[1] / (
"plugin/skills/using-scribe/SKILL.md"
)
text = " ".join(skill.read_text().split())
assert "could this note become false without anyone editing it" in text.lower(), (
"the using-scribe skill no longer carries the one-question test. That "
"question is what makes the distinction applicable rather than merely "
"true."
)
assert "notes_due_for_verification" in text, (
"the skill names the fields but not the surface that reads them — "
"guidance for writing a check with no route to acting on one."
)