bvandeusen/FabledScribe
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases 53 Wiki Activity

Rule outcomes, the contract hint, and four extractor/backup fixes #174

Merged
bvandeusen merged 18 commits from dev into main 2026-09-21 06:31:10 -04:00
Conversation 0 Commits 18 Files Changed 51
+5802 -587
51 changed files with 5802 additions and 587 deletions
Download Patch File Download Diff File Expand all files Collapse all files
alembic/versions/0106_rule_usage_events_carry_an_outcome.py
+65
View File
@@ -0,0 +1,65 @@
"""rule_usage_events carry an outcome, not just a read (#4212, milestone 419)
Revision ID: 0106
Revises: 0105
Create Date: 2026-09-20
Milestone 419's first step. `rule_usage_events` can say a rule was SURFACED
and that it was PULLED. It cannot say what happened next, so these two
sessions leave identical telemetry:
- a rule surfaced, opened, and followed;
- a rule surfaced, opened, and silently ignored.
The second is the more urgent by a distance, and it is the one the readout
cannot name. That is the whole of what this milestone is about, measured on a
session where three of seven misses were caught by the operator and none by
the system.
TWO NEW EVENT VALUES, AND NO CHECK MIGRATION. `event` was created in 0094 as
plain `sa.Text()` with no constraint — checked in the migration itself, not
assumed from the model — so `applied` and `departed` join `surfaced` and
`pulled` without a DROP/ADD pair. Rule 36 governs CHECK-whitelisted columns
and this is not one; noted explicitly because the next reader will reach for
rule 36 here, and should be able to see in one place why it does not bite.
THE THIRD STATE IS DERIVED, AND THAT IS NOT A SHORTCUT. Read-and-silently-
unchanged is the absence of an outcome, and it has to be: an agent that knew
it was ignoring a rule would not be ignoring it. There is no honest way to ask
for that event, so nothing here tries. `applied` and `departed` are reported;
the third state is what is left over when a rule was pulled and neither
arrived. A schema that offered an `ignored` value would collect nothing and
read as though it had measured something, which is the #3311 failure — a
statistic that cannot vary being mistaken for a finding.
`detail` CARRIES THE WHY OF A DEPARTURE, and is the reason this is a column
rather than two more bare event strings. A departure without its reason is
indistinguishable from a miss when someone reads the table back, so the two
states the milestone wants to tell apart would collapse again one layer down.
Nullable because `applied` needs no argument — following a rule is the
unremarkable case, and demanding prose for it would make the cheap event
expensive and stop it being recorded at all.
NO NEW INDEX. Every outcome readout starts from a set of rule ids and narrows
by event, which is exactly `ix_rule_usage_rule_event` (rule_id, event) from
0094. Adding a `detail` index would serve no query anyone has — the column is
read, never filtered on.
"""
import sqlalchemy as sa
from alembic import op
revision = "0106"
down_revision = "0105"
branch_labels = None
depends_on = None
def upgrade() -> None:
op.add_column(
"rule_usage_events",
sa.Column("detail", sa.Text(), nullable=True),
)
def downgrade() -> None:
op.drop_column("rule_usage_events", "detail")
frontend/src/api/lessons.ts
+4
View File
@@ -65,6 +65,10 @@ export interface LessonListRow {
tags: string[];
when_to_apply?: string;
snippet?: string;
/** Surfaced-vs-opened for this lesson. Always present on a listing from
* this door, zero-filled where nothing has been recorded — absent only
* when a row came from somewhere else. */
usage?: RecordUsage;
shared?: boolean;
owner?: string | null;
}
frontend/src/views/LessonDetailView.vue
+21 -1
View File
@@ -24,6 +24,7 @@ import { apiErrorMessage } from "@/api/client";
import { deleteLesson, getLesson, type Lesson } from "@/api/lessons";
import ConfirmDialog from "@/components/ConfirmDialog.vue";
import TagPill from "@/components/TagPill.vue";
import UsageBadge from "@/components/UsageBadge.vue";
import { useToastStore } from "@/stores/toast";
import { renderMarkdown } from "@/utils/markdown";
@@ -99,7 +100,19 @@ onMounted(load);
<p class="ld-trigger-text">{{ lesson.when_to_apply }}</p>
</section>
<h1 class="ld-what">{{ lesson.what }}</h1>
<div class="ld-what-row">
<h1 class="ld-what">{{ lesson.what }}</h1>
<!-- Lessons collected surfaced-vs-opened from the day the slot shipped
and showed it nowhere (#4196). It belongs here above anywhere
else: the reading that matters is almost never "delete this", it
is "the trigger is keyed to a situation nobody is in", and the
place to act on that is the lesson you are already looking at. -->
<UsageBadge
:usage="lesson.usage"
noun="lesson"
dead-weight-advice="Repeatedly offered and never opened usually means the trigger fires on the wrong situation — re-key `when_to_apply` rather than deleting the claim."
/>
</div>
<div
v-if="insightHtml"
@@ -210,6 +223,13 @@ onMounted(load);
}
.ld-trigger-text { margin: 0; font-size: 0.98rem; line-height: 1.5; }
.ld-what-row {
display: flex;
align-items: baseline;
gap: var(--fs-space-2);
flex-wrap: wrap;
}
.ld-what {
margin: 0 0 1.25rem;
font-size: 1.25rem;
frontend/src/views/SettingsView.vue
+69 -2
View File
@@ -87,6 +87,7 @@ const kbWritePathEnabled = ref(true);
// unrelated code through (#2223). Shares top-k, not the threshold.
const kbWritePathThreshold = ref("0.68");
const kbRuleHintThreshold = ref("0.72");
const kbCheckpointThreshold = ref("0.80");
// The two ACT arms no longer share a bar (#3853). A write-path query is a code
// payload; a pre-tool query is a shell command, often under a dozen words —
// less text, less signal, lower scores for the same relevance. Measured at one
@@ -252,6 +253,21 @@ async function loadTuningHistory() {
}
}
/** Who moved a dial. Three answers, not two (#4225).
*
* `release` means the shipped default itself changed between versions — no
* user turned anything. Folding that into the `human ? 'you' : 'Claude'`
* fallback labelled it "Claude", which tells the operator the session moved a
* floor it never touched. Misattribution is the one failure the `actor`
* column exists to prevent, so an unknown value says so rather than guessing.
*/
function actorLabel(actor: string): string {
if (actor === "human") return "you";
if (actor === "release") return "this release";
if (actor === "model") return "Claude";
return actor;
}
async function saveKbInject() {
const t = Math.min(1, Math.max(0, Number(kbInjectThreshold.value) || 0));
const k = Math.min(10, Math.max(1, Math.floor(Number(kbInjectTopK.value) || 1)));
@@ -275,6 +291,10 @@ async function saveKbInject() {
// Bash call, so a fallback of 0 would put a rule in front of every command.
const trT = Math.min(1, Math.max(0, Number(kbToolRuleThreshold.value) || 0.68));
const prT = Math.min(1, Math.max(0, Number(kbPromptRuleThreshold.value) || 0.72));
// The checkpoint bar, and the `|| default` guard matters most here of all:
// this is the only number that can STOP a call, so a fallback of 0 would
// hold the first command of every session behind whatever ranked first.
const cpT = Math.min(1, Math.max(0, Number(kbCheckpointThreshold.value) || 0.8));
const rpT = Math.min(1, Math.max(0, Number(kbReportPrefThreshold.value) || 0.72));
// The budgets, clamped the way the server clamps them: a whole number in
// [1, 10]. Never 0 — an arm turned off is turned off by its switch, and a
@@ -300,6 +320,7 @@ async function saveKbInject() {
kbPlanMatchThreshold.value = String(planT);
kbWritePathThreshold.value = String(wpT);
kbRuleHintThreshold.value = String(rhT);
kbCheckpointThreshold.value = String(cpT);
kbToolRuleThreshold.value = String(trT);
kbPromptRuleThreshold.value = String(prT);
kbReportPrefThreshold.value = String(rpT);
@@ -319,6 +340,12 @@ async function saveKbInject() {
// in services/plugin_context.py for why rules cannot share the
// code threshold any more than code could share the prose one.
kb_rulehint_threshold: String(rhT),
// The one bar that stops a call rather than annotating it. Its own key,
// never derived from the two rule bars above: it answers a different
// question of the same scores — not "is this worth showing" but "is the
// corpus confident enough to be read first" — and a number that moves
// when another moves is a number nobody can reason about.
kb_checkpoint_threshold: String(cpT),
// A FOURTH and FIFTH bar, and they are separate keys on purpose: the
// whole finding of #3853 is that one number cannot serve arms whose
// queries are different shapes. Moving one must not move the others.
@@ -790,6 +817,9 @@ onMounted(async () => {
kbInjectTopK.value = allSettings.kb_autoinject_top_k;
}
kbWritePathEnabled.value = allSettings.kb_writepath_enabled !== "false";
if (allSettings.kb_checkpoint_threshold !== undefined) {
kbCheckpointThreshold.value = allSettings.kb_checkpoint_threshold;
}
if (allSettings.kb_rulehint_threshold !== undefined) {
kbRuleHintThreshold.value = allSettings.kb_rulehint_threshold;
}
@@ -1723,6 +1753,32 @@ async function deleteUser(userId: number) {
/>
<p class="field-hint">How many standing rules one edit may be shown (1–10).</p>
</div>
<div class="field">
<label for="kb-checkpoint-threshold">Hold-before-acting threshold (0–1)</label>
<input
id="kb-checkpoint-threshold"
v-model="kbCheckpointThreshold"
type="number"
min="0"
max="1"
step="0.01"
class="fs-input input"
style="max-width: 8rem"
/>
<p class="field-hint">
Every hint above arrives <em>beside</em> the result of the action it
was about — useful to read afterwards, too late to change what the
action was. Above this bar a standing rule is instead put
<em>in front</em> of the command: Claude is asked to read the rule
first, and then runs the command anyway if the rule does not apply.
Set well above the thresholds above, because it costs a round trip
rather than a line. Only standing rules can hold a command, never
preferences; only a rule this session has not already read; and
never more than once per rule or five times per session, so a bar
set too low is a chatty session rather than a stuck one. Commands
only — file edits are never held.
</p>
</div>
<div class="field">
<label for="kb-toolrule-threshold">Command confidence threshold (0–1)</label>
<input
@@ -1884,8 +1940,11 @@ async function deleteUser(userId: number) {
<template v-else>set to</template>
{{ ev.new_value }}
</span>
<span class="tuning-actor" :class="{ 'is-human': ev.actor === 'human' }">
{{ ev.actor === 'human' ? 'you' : 'Claude' }}
<span
class="tuning-actor"
:class="{ 'is-human': ev.actor === 'human', 'is-release': ev.actor === 'release' }"
>
{{ actorLabel(ev.actor) }}
</span>
<span v-if="ev.created_at" class="tuning-when">{{ fmtDate(ev.created_at) }}</span>
</div>
@@ -4195,6 +4254,14 @@ async function deleteUser(userId: number) {
color: var(--fs-accent);
border-color: var(--fs-accent);
}
/* A shipped default that moved between releases (#4225). Muted rather than
accented: it is the answer to "did I do this?" being NO for both of the
other two, and it wants to be legible without competing with the changes
somebody actually made. */
.tuning-actor.is-release {
color: var(--fs-text-secondary);
border-style: dashed;
}
.tuning-when { margin-left: auto; color: var(--fs-text-tertiary); }
.tuning-reason {
margin: var(--fs-space-2) 0 0;
plugin/.claude-plugin/plugin.json
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "scribe",
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
"version": "2026.09.21.0300",
"version": "2026.09.21.0503",
"author": {
"name": "Bryan Van Deusen"
},
plugin/hooks/hooks.json
+9
View File
@@ -62,6 +62,15 @@
"command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/scribe_record_opened.sh\""
}
]
},
{
"matcher": "mcp__.*__rule_outcome",
"hooks": [
{
"type": "command",
"command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/scribe_record_outcome.sh\""
}
]
}
],
"PreCompact": [
plugin/hooks/scribe_defs.sh
+560 -23
View File
@@ -303,15 +303,118 @@ scribe_urlenc() {
# or the one enclosing an Edit). Rule-for-rule mirrored by the server's
# services/coverage.py extract_shapes — ledger rows are keyed by what THAT
# sees, so the two must agree on what counts as a definition.
#
# THE WHOLE INPUT IS BUFFERED (#4222) so the span scan below can look ahead.
# The matchers are line-oriented and know nothing about what a line is INSIDE:
# a wrapped docstring beginning "class AND the …" announces a shape called
# `AND`, which reaches the session mid-edit as a divergence prompt about a
# symbol that does not exist. blank_spans() replaces every comment and string
# span with its own newlines before a matcher sees a line — the same scan, in
# the same order, as coverage.py::_blank_spans. Change one, change both.
scribe_defs() {
awk '
{
# CSS class definition: .name { or .name,
if (match($0, /^[[:space:]]*\.[A-Za-z][A-Za-z0-9_-]*[[:space:]]*[,{]/)) {
t = $0; sub(/^[[:space:]]*\./, "", t); sub(/[[:space:]]*[,{].*$/, "", t)
if (t != "") print "css\t" t; next
BEGIN {
SQ = sprintf("%c", 39)
SQ3 = SQ SQ SQ
DQ = "\""
DQ3 = DQ DQ DQ
# The only characters that can begin a span, a line comment or a
# string. Everything between two of them is copied in one go rather
# than a character at a time.
MARKERS = "[" DQ SQ "/#]"
}
# Offset just past the one-line string opening at c, or c+1 when it does
# not close before the end of the line — so an apostrophe in prose costs
# one character rather than everything up to the next quote.
function string_end(L, c, q, i, n, ch) {
n = length(L); i = c + 1
while (i <= n) {
ch = substr(L, i, 1)
if (ch == "\\") { i = i + 2; continue }
if (ch == q) return i + 1
i++
}
line = $0; sub(/^[[:space:]]+/, "", line)
return c + 1
}
# Does the "#" at c open a comment, or is it a CSS colour or id? An
# alphanumeric straight after it is #fff or #app; anything else is a
# comment in every language that has one.
function hash_comment(L, c) {
return substr(L, c + 1, 1) !~ /^[A-Za-z0-9]$/
}
# raw[1..n] -> msk[1..n] with comment and string spans emptied. Line
# COUNT is preserved and column positions are not; the matchers lstrip.
# ONLY CLOSED SPANS ARE BLANKED: an opener with no closer is rewound past
# and scanning resumes, so a stray marker costs one span rather than
# every definition below it.
function blank_spans(raw, n, msk,
i, c, L, len, state, closer, oplen, sl, sc, sprefix,
t3, t2, ch, e, k, rest, m) {
for (i = 1; i <= n; i++) msk[i] = ""
i = 1; c = 1; state = 0; closer = ""
while (1) {
while (i <= n) {
L = raw[i]; len = length(L)
if (c > len) { i++; c = 1; continue }
if (state) {
e = index(substr(L, c), closer)
if (e == 0) { i++; c = 1; continue }
c = c + e - 1 + length(closer)
state = 0; closer = ""
continue
}
rest = substr(L, c)
m = match(rest, MARKERS)
if (m == 0) { msk[i] = msk[i] rest; i++; c = 1; continue }
if (m > 1) {
msk[i] = msk[i] substr(rest, 1, m - 1)
c = c + m - 1
continue
}
t3 = substr(L, c, 3); t2 = substr(L, c, 2); ch = substr(L, c, 1)
if (t3 == DQ3 || t3 == SQ3) {
sl = i; sc = c; sprefix = msk[i]
state = 1; closer = t3; oplen = 3; c = c + 3
continue
}
if (t2 == "/*") {
sl = i; sc = c; sprefix = msk[i]
state = 1; closer = "*/"; oplen = 2; c = c + 2
continue
}
if (t2 == "//" || (ch == "#" && hash_comment(L, c))) {
# A line comment is COPIED, not blanked: its continuation lines
# carry their own marker, so none can read as a definition alone.
msk[i] = msk[i] substr(L, c)
i++; c = 1
continue
}
if (ch == DQ || ch == SQ) {
k = string_end(L, c, ch)
msk[i] = msk[i] substr(L, c, k - c)
c = k
continue
}
msk[i] = msk[i] ch
c++
}
if (!state) return
for (k = sl; k <= n; k++) msk[k] = ""
msk[sl] = sprefix
i = sl; c = sc + oplen; state = 0; closer = ""
}
}
function emit(line, t, rest) {
# CSS class definition: .name { or .name,
if (match(line, /^[[:space:]]*\.[A-Za-z][A-Za-z0-9_-]*[[:space:]]*[,{]/)) {
t = line; sub(/^[[:space:]]*\./, "", t); sub(/[[:space:]]*[,{].*$/, "", t)
if (t != "") print "css\t" t; return
}
sub(/^[[:space:]]+/, "", line)
# Strip leading declaration modifiers so the definition keyword is the
# first word regardless of language (export/pub/private/suspend/...).
sub(/^((pub(\([a-z]+\))?|export|default|private|internal|protected|public|static|suspend|async|open|sealed|data|abstract|final|inline|unsafe|extern|override)[[:space:]]+)*/, "", line)
@@ -319,7 +422,7 @@ scribe_defs() {
if (match(line, /^func[[:space:]]*\([^)]*\)[[:space:]]*[A-Za-z_]/)) {
t = line; sub(/^func[[:space:]]*\([^)]*\)[[:space:]]*/, "", t)
sub(/[^A-Za-z0-9_].*$/, "", t)
if (t != "") print "sym\t" t; next
if (t != "") print "sym\t" t; return
}
# Keyword-announced definitions, functions and named types alike.
# Dunders are skipped: every class defines __init__, so "already defined
@@ -333,17 +436,24 @@ scribe_defs() {
# nothing (mirror of coverage.py, #2904).
if (line ~ /^type[[:space:]]/) {
rest = line; sub(/^type[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*/, "", rest)
if (rest !~ /[={]/) next
if (rest !~ /[={]/) return
}
if (t != "" && t !~ /^__.*__$/) print "sym\t" t; next
if (t != "" && t !~ /^__.*__$/) print "sym\t" t; return
}
# Arrow/expression assignment: const name = (…) / let name = async (
if (match(line, /^(const|let)[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*[[:space:]]*=[[:space:]]*(async[[:space:]]*)?[(<]/)) {
t = line; sub(/^(const|let)[[:space:]]+/, "", t)
sub(/[^A-Za-z0-9_$].*$/, "", t)
if (t != "") print "sym\t" t; next
if (t != "") print "sym\t" t; return
}
}
{ raw[NR] = $0 }
END {
blank_spans(raw, NR, msk)
for (r = 1; r <= NR; r++) emit(msk[r])
}
' 2>/dev/null
}
@@ -367,8 +477,280 @@ scribe_defs() {
# $1 repo root, $2 repo-relative path of the file being written (excluded from
# the grep — it would always match itself on an Edit). Definitions on stdin.
# Prints one "> - `name` is already defined in N other file(s): …" per hit.
# ── The contract around a change (#4215, milestone 419) ───────────────────
#
# WHAT THIS ANSWERS. "You altered the arity, name or shape of something — here
# is everything that reads it." Rule 33's interface-contract check, one scope
# down: not between layers but between a definition and its callers.
#
# WHY IT IS A CHECK AND NOT A LESSON. #4207 was written — "widening a tuple is
# an interface change to every unpack site, and the compiler will not tell
# you" — hours before a structurally identical mistake was made by its author,
# and it was surfaced twice in the turns before. Text delivered at the moment
# of acting is too weak a carrier for a reflex that has to change what the act
# IS. This looks it up instead.
#
# `scribe_exposed` — the names a CALLER can depend on, from a blob of code.
# Three kinds, because a contract breaks three ways and they look nothing
# alike in the source:
#
# sym what is defined rename / removal
# arg its parameter names arity and order
# key quoted keys of dict literals the shape of what it RETURNS
#
# The third is here because of the miss that produced this step. A config
# function gained one dict key; three arms read that dict inside a fail-open
# `except`, so every one of them silently became a no-op and ten tests went
# red at once with nothing pointing at the cause. No signature changed. A
# check that only watched signatures would have watched the wrong thing.
scribe_exposed() {
# The `sym` half DELEGATES to scribe_defs rather than repeating its patterns.
# Those patterns cover nine languages and have been corrected several times
# (the Go receiver form, the `type` import-specifier false positive, the
# dunder skip); a second copy here would inherit today's version and then
# quietly stop agreeing with it, which is #3497's history for the two rule
# arms. One reader, called twice.
local blob
blob=$(cat)
{
printf '%s' "$blob" | scribe_defs
printf '%s' "$blob" | awk '
function emit(kind, name) {
if (name != "" && name !~ /^__.*__$/) print kind "\t" name
}
{
line = $0; sub(/^[[:space:]]+/, "", line)
sub(/^((pub(\([a-z]+\))?|export|default|private|internal|protected|public|static|suspend|async|open|sealed|data|abstract|final|inline|unsafe|extern|override)[[:space:]]+)*/, "", line)
# Parameter names, from whatever announces a definition. Taken from the
# FIRST parenthesis only: a default value can itself contain parens and
# a greedy match would swallow the body of a one-liner.
if (match(line, /^(function|def|func|fun|fn|sub)[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*[[:space:]]*\(/) \
|| match(line, /^(const|let)[[:space:]]+[A-Za-z_$][A-Za-z0-9_$]*[[:space:]]*=[[:space:]]*(async[[:space:]]*)?\(/)) {
args = line
sub(/^[^(]*\(/, "", args)
sub(/\).*$/, "", args)
n = split(args, parts, ",")
for (i = 1; i <= n; i++) {
a = parts[i]
gsub(/^[[:space:]]+|[[:space:]]+$/, "", a)
# Strip a type annotation, a default, and the * / ** / & markers.
sub(/[:=].*$/, "", a)
gsub(/^[*&]+/, "", a)
gsub(/[[:space:]]/, "", a)
# `self` and `cls` are not part of anything a caller passes.
if (a != "" && a != "self" && a != "cls" && a ~ /^[A-Za-z_$][A-Za-z0-9_$]*$/)
emit("arg", a)
}
}
# Quoted keys of a dict / object literal. Anchored on the quote so a
# dictionary ACCESS (`cfg["k"]`) does not read as a definition of one —
# only `"k":` counts, which is the writing position.
rest = $0
while (match(rest, /["'\''][A-Za-z_][A-Za-z0-9_]*["'\''][[:space:]]*:/)) {
tok = substr(rest, RSTART, RLENGTH)
rest = substr(rest, RSTART + RLENGTH)
gsub(/["'\'']/, "", tok); sub(/[[:space:]]*:$/, "", tok)
emit("key", tok)
}
}
' 2>/dev/null
} | sort -u
}
# Who references `name` anywhere else in the repo. Word-bounded, so `cfg` does
# not match `cfg_path`, and the defining file is excluded — a definition is
# always its own first mention and listing it says nothing.
#
# `|| true` INSIDE the substitution for the reason #4042 records at
# scribe_local_dups: under `pipefail` a `head` that exits early kills the
# still-writing git grep, and an outer fallback then wipes the hits head had
# already printed.
scribe_contract_readers() {
local root="$1" rel="$2" name="$3"
[ -n "$root" ] && [ -n "$name" ] || return 0
git -C "$root" grep -I -l -w -e "$name" -- . ":(exclude)${rel}" 2>/dev/null \
| head -6 || true
}
# The whole check, rendered. Kept here rather than inline in the hook so it
# can be exercised against a pair of blobs with no event, no server and no
# session — which is how every case in test_contract_around_the_change.py is
# written.
#
# Arguments: root, repo-relative path, the subject definition, the old text,
# the new text, and the session ledger (may be empty).
#
# TWO GATES, AND THE SECOND IS WHAT KEEPS THIS QUIET. A change to the exposed
# set is necessary but not sufficient: a definition NOTHING else references
# has no contract to break, so the readers lookup runs second and an empty
# result ends it silently. On a repo of any size most edits touch something
# local, so most edits say nothing here — and a hint that fires on everything
# is one that gets skipped.
scribe_contract_block() {
local root="$1" rel="$2" subject="$3" old_text="$4" new_text="$5" ledger="$6"
local changed gained lost readers count
[ -n "$root" ] && [ -n "$subject" ] || return 0
[ -n "$old_text" ] && [ -n "$new_text" ] || return 0
# Named once per session per subject. A second edit to the same definition
# is the SAME contract question, and answering it again would punish the
# ordinary rhythm of getting a change right over several passes.
if [ -n "$ledger" ] && [ -f "$ledger" ]; then
grep -qxF "$subject" "$ledger" 2>/dev/null && return 0
fi
# One pass, no process substitution: `comm` would need /dev/fd, and this
# also keeps the two sides' extraction visibly identical.
changed=$(
{
printf '%s' "$old_text" | scribe_exposed | sed 's/^/O\t/'
printf '%s' "$new_text" | scribe_exposed | sed 's/^/N\t/'
} | awk -F'\t' '
NF >= 3 { k = $2 "\t" $3; side[k] = side[k] $1 }
END {
for (k in side)
if (side[k] == "O") print "lost\t" k
else if (side[k] == "N") print "gained\t" k
}
' 2>/dev/null
)
[ -n "$changed" ] || return 0
readers=$(scribe_contract_readers "$root" "$rel" "$subject")
[ -n "$readers" ] || return 0
count=$(printf '%s\n' "$readers" | grep -c . 2>/dev/null || printf '0')
gained=$(printf '%s\n' "$changed" | awk -F'\t' '$1=="gained" {printf "%s%s %s", (n++?", ":""), $2, $3}')
lost=$(printf '%s\n' "$changed" | awk -F'\t' '$1=="lost" {printf "%s%s %s", (n++?", ":""), $2, $3}')
printf '> The contract around `%s` changed, and %s other file(s) reference it (`git grep -w`; a nudge, not a gate):\n' \
"$subject" "$count"
[ -n "$gained" ] && printf '> gained: %s\n' "$gained"
[ -n "$lost" ] && printf '> lost: %s\n' "$lost"
printf '> read by: %s\n' "$(printf '%s' "$readers" | tr '\n' ' ' | sed 's/ $//')"
printf '> A caller that passes or reads the old shape keeps compiling and fails only when that line runs (lesson #4207). Read them before moving on.\n'
[ -n "$ledger" ] && printf '%s\n' "$subject" >> "$ledger" 2>/dev/null
return 0
}
scribe_slippage_lines() {
# $1 state dir, $2 sanitised session id.
#
# SILENT ONLY WHEN NO RULE TOUCHED THE SESSION AT ALL. Traffic that did
# happen is always reported, because "which rules governed this work" is
# what the static instructions above already ask the summariser to preserve
# in prose — these lines are the measured version of that, and they are
# three short lines.
#
# What is conditional is the ACCUSATION. Each subtraction prints only when
# it has members, so a session that opened everything it was shown and
# resolved everything it opened gets the traffic and no more. "0 rules
# unresolved" on every compaction is how a readout teaches its reader to
# skip it.
local dir="$1" sid="$2" named opened acted held
[ -n "$dir" ] && [ -n "$sid" ] || return 0
# THE TWINS, not the exclusion ledgers. This runs at the compaction, which
# is the moment the exclusion ledgers are about to be cleared and the moment
# their TTL has usually already eaten the early part of a long session. A
# readout built on them would report only the last stretch of the session
# and read as though it had reported all of it — the #3311 shape, and the
# one this milestone exists to stop producing.
named=$(scribe_ledger_kept "$dir/${sid}.rules.keep.ids")
opened=$(scribe_ledger_kept "$dir/${sid}.opened.keep.ids")
acted=$(scribe_ledger_kept "$dir/${sid}.acted.keep.ids")
held=$(scribe_ledger_kept "$dir/${sid}.checkpoint.keep.ids")
[ -n "$named$opened" ] || return 0
local unread unresolved
unread=$(scribe_ids_minus "$named" "$opened")
unresolved=$(scribe_ids_minus "$opened" "$acted")
printf -- '- This session'"'"'s rule traffic, from what actually happened rather than from recollection:\n'
[ -n "$opened" ] && printf -- ' read: %s\n' "$opened"
[ -n "$held" ] && printf -- ' held an act before it ran: %s\n' "$held"
[ -n "$unread" ] && printf -- ' named by an arm and never opened: %s\n' "$unread"
if [ -n "$unresolved" ]; then
printf -- ' READ WITH NO OUTCOME RECORDED: %s. Carry these over as outstanding. A rule read and left unresolved looks exactly like one that worked, and the summary is where that difference is lost for good — say `rule_outcome(id, "applied")`, or `rule_outcome(id, "departed", why=...)` where you deliberately went another way.\n' "$unresolved"
fi
return 0
}
scribe_ids_minus() {
# Set difference over two space-separated id lists, order preserved. Written
# as one awk pass rather than a nested shell loop because the ledgers can
# hold a few dozen ids by the end of a long session and this runs inside the
# compaction path, where a slow hook delays the thing it is decorating.
local a="$1" b="$2"
[ -n "$a" ] || return 0
awk -v a="$a" -v b="$b" '
BEGIN {
n = split(b, drop, " ")
for (i = 1; i <= n; i++) if (drop[i] != "") skip[drop[i]] = 1
m = split(a, keep, " ")
out = ""
for (i = 1; i <= m; i++) {
id = keep[i]
if (id == "" || (id in skip) || (id in done)) continue
done[id] = 1
out = out (out == "" ? "" : " ") id
}
if (out != "") print out
}
' </dev/null 2>/dev/null
}
# ARM 1, BY NAME (#2280) — and the confirmation pass that makes it mean
# something (#4227).
#
# WHY A GREP IS NOT ENOUGH. The pattern below looks for a definition keyword
# followed by the name. A grep sees LINES, not spans, so the sentence
#
# class with only modifier rules is a deletion that went half-way.
#
# — real prose, from the module docstring of scripts/check_dangling_styles.py —
# matches it for `name=with`. #4222 fixed the other end of this same defect, in
# the extractors that decide what a payload DEFINES; this is the end that
# decides which other files already define it, and it was still a plain grep.
#
# SO EVERY HIT IS CONFIRMED by running the real extractor over the candidate
# file and keeping only names it actually reports. That is the honest check and
# the only one that cannot disagree with the other end of the pipe.
#
# TIGHTENING THE PATTERN WOULD HAVE BEEN CHEAPER AND WRONG. Requiring `(` or
# `{` or `:` after the name rejects `class with only…` — and also rejects
# `class Foo extends Bar {`, `class Foo : Base()` and `type Foo struct {`. This
# arm's whole justification (#2280, #2682) is that it works with no server, no
# index and no binding, which makes a miss here invisible. Trading a visible
# false positive for an invisible false negative is a bad trade.
#
# ONCE PER DISTINCT FILE, not once per (name, file) pair: the same file is
# usually a candidate for several names at once, and the extractor reads the
# whole file either way. Measured on this repo, a deliberately pathological
# payload — nine names that are ordinary English words — produced 28 distinct
# candidate files totalling 947KB, and `scribe_defs` runs at roughly 33ms per
# 250KB, so the confirmation costs about 200ms in the worst case anyone has
# been able to construct here. The per-(name, file) shape would have paid that
# several times over for the same answer.
_SCRIBE_DUP_CANDIDATES=12
_SCRIBE_DUP_SHOWN=4
_SCRIBE_DUP_BUDGET=48
scribe_local_dups() {
local root="$1" rel="$2" kind name pat hits count label files
local root="$1" rel="$2" kind name pat hits
local records="" cands="" idx=$'\n' seen=0
local f defs line record rest files shown
# PASS 1 — candidates. One grep per name, unchanged except for the cap.
#
# THE CAP IS RAISED FROM FOUR, and that is not a detail. Confirmation REMOVES
# hits, so capping before it runs lets three phantom matches crowd out a real
# definition in the fourth file — hits dropped before anyone looked at them,
# which is #4042's bug wearing a different hat. The display cap stays at four
# (_SCRIBE_DUP_SHOWN); it now applies to CONFIRMED hits, which is where a cap
# belongs.
while IFS=$'\t' read -r kind name; do
[ -n "${name:-}" ] || continue
case "$kind" in
@@ -377,17 +759,63 @@ scribe_local_dups() {
esac
# -I skips binaries; :(exclude) drops the file being written.
# `|| true` INSIDE the substitution, not `|| hits=""` outside it (#4042):
# under the hooks' `pipefail`, `head` exiting after four lines kills a
# git grep that is still writing, the pipeline reports SIGPIPE, and an
# outer fallback then wipes the four hits head already printed. The name
# most duplicated — the one this arm exists for — was the one it dropped.
hits=$(git -C "$root" grep -I -l -E -e "$pat" -- . ":(exclude)${rel}" 2>/dev/null | head -4 || true)
# under the hooks' `pipefail`, `head` exiting early kills a git grep that
# is still writing, the pipeline reports SIGPIPE, and an outer fallback
# then wipes the hits head had already printed. The name most duplicated —
# the one this arm exists for — was the one it dropped.
hits=$(git -C "$root" grep -I -l -E -e "$pat" -- . ":(exclude)${rel}" 2>/dev/null \
| head -"$_SCRIBE_DUP_CANDIDATES" || true)
[ -n "$hits" ] || continue
count=$(printf '%s\n' "$hits" | grep -c . 2>/dev/null || echo 0)
label=$([ "$kind" = css ] && printf '.%s' "$name" || printf '%s' "$name")
files=$(printf '%s' "$hits" | tr '\n' ' ' | sed 's/ $//')
printf '> - `%s` is already defined in %s other file(s): %s\n' "$label" "$count" "$files"
records+="${kind}"$'\t'"${name}"$'\t'"${hits//$'\n'/$'\t'}"$'\n'
cands+="${hits}"$'\n'
done
[ -n "$records" ] || return 0
# PASS 2 — what each candidate file actually defines, one extractor run per
# file. The budget is a floor under the worst case rather than a tuning knob:
# the upstream caps (twelve names, twelve candidates each) bound this at 144
# files, and a repo that reached that would be paying a second of hook time
# for a nudge. A file past the budget is DROPPED rather than passed through
# unconfirmed — this arm is a nudge and not a gate, so an unproven claim is
# worth less here than no claim.
while IFS= read -r f; do
[ -n "$f" ] || continue
[ "$seen" -ge "$_SCRIBE_DUP_BUDGET" ] && break
seen=$((seen + 1))
defs=$(scribe_defs < "$root/$f" 2>/dev/null | sort -u) || defs=""
[ -n "$defs" ] || continue
while IFS= read -r line; do
[ -n "$line" ] || continue
idx+="${f}"$'\t'"${line}"$'\n'
done <<< "$defs"
done <<< "$(printf '%s' "$cands" | sort -u)"
# PASS 3 — emit the confirmed hits, in the order the names arrived.
while IFS= read -r record; do
[ -n "$record" ] || continue
kind=${record%%$'\t'*}
rest=${record#*$'\t'}
name=${rest%%$'\t'*}
files=""
shown=0
while IFS= read -r f; do
[ -n "$f" ] || continue
# Delimited on both sides so `usage.ts` cannot satisfy a lookup for
# `.ts`, and `handler` cannot satisfy one for `handle`.
case "$idx" in
*$'\n'"${f}"$'\t'"${kind}"$'\t'"${name}"$'\n'*) ;;
*) continue ;;
esac
files+="${f} "
shown=$((shown + 1))
[ "$shown" -ge "$_SCRIBE_DUP_SHOWN" ] && break
done <<< "$(printf '%s' "${rest#*$'\t'}" | tr '\t' '\n')"
[ -n "$files" ] || continue
if [ "$kind" = css ]; then label=".$name"; else label="$name"; fi
printf '> - `%s` is already defined in %s other file(s): %s\n' \
"$label" "$shown" "${files% }"
done <<< "$records"
}
# ---------------------------------------------------------------------------
@@ -540,10 +968,42 @@ scribe_rules_live() {
# Append surfaced ids, stamped. Reads ids on stdin, one per line — the shape
# `scribe_json_list "$flat" '.rule_ids'` already produces at both call sites.
scribe_rules_append() {
# Writes TWO files, and the second one is the point (#4217).
#
# `$f` is an EXCLUSION ledger: it answers "does this context already hold
# this?", so it is aged by TTL and swept at every compaction — after a
# compaction the agent genuinely does not hold what it was shown, and
# forgetting is correct. Three hooks depend on exactly that.
#
# The twin is an EVIDENCE ledger: it answers "did this happen?", which no
# compaction can make untrue. It is never aged and never swept.
#
# These are opposite lifetimes and `.opened.ids` was serving both, which is
# how a session with 45 `get_rule` calls came to report none: the compaction
# cleared the ledger that was also the record. Measured across six sessions
# on this instance — 208 opens, 3 surviving — before the split.
#
# DERIVED HERE rather than listed at the call sites, because a list is what
# broke this before (see `scribe_clear_session_ledgers`). Every ledger
# written through this function gets its twin, including the next one
# somebody adds. The cost is a second small file per ledger per session.
local f="$1" now
[ -n "$f" ] || return 0
now=$(date +%s 2>/dev/null) || now=0
awk -v ts="$now" 'NF { print $1 "\t" ts }' >> "$f" 2>/dev/null || true
awk -v ts="$now" -v keep="${f%.ids}.keep.ids" '
NF { line = $1 "\t" ts; print line; print line >> keep }
' >> "$f" 2>/dev/null || true
}
scribe_ledger_kept() {
# Ids from an EVIDENCE twin: deduped, order preserved, NOT aged. A rule
# opened two hours ago was still opened, so the TTL that keeps an exclusion
# ledger honest would here delete the finding.
local f="$1"
[ -n "$f" ] && [ -f "$f" ] || return 0
awk -F'\t' '$1 != "" && !seen[$1]++ {
out = out (out == "" ? "" : " ") $1
} END { if (out != "") print out }' "$f" 2>/dev/null || true
}
# The OPENED ledger's contribution to a rule arm's query string (#4100).
@@ -558,6 +1018,74 @@ scribe_rules_append() {
#
# Same reader as the naming ledger on purpose, so ageing, the last-entry-wins
# rule and the bare-id format are defined once and cannot drift apart.
# ── The pre-act checkpoint's session ledger (#4214, milestone 419) ─────────
#
# A checkpoint STOPS an act rather than annotating it, so unlike every other
# ledger here its job is to make sure the same stop cannot happen twice. Two
# guards, and they fail in different directions:
#
# per rule — a rule may hold at most ONE act per session. Once it has, the
# remedy has been offered; repeating it on the next call would
# turn a reader who decided the rule does not apply into a
# reader who cannot proceed.
# per session — at most `_SCRIBE_CHECKPOINT_CAP` stops in total, whatever the
# corpus scores. A mis-set floor or a corpus that suddenly
# resembles everything must degrade to a noisy session, never
# to one that cannot make progress. This is the guard on the
# worst case, not a tuning value.
#
# NOT AGED, unlike the naming ledgers. Those age because they describe what a
# session is still HOLDING, and a context stops holding things. This one
# describes what already HAPPENED — a stop was raised and its remedy offered —
# and that does not become untrue an hour later. Ageing it would let a long
# session be stopped by the same rule repeatedly, which is the one outcome
# both guards exist to prevent.
_SCRIBE_CHECKPOINT_CAP=5
scribe_checkpoint_allowed() {
# $1 ledger file, $2 rule id. Returns 0 (and RECORDS the stop) when this act
# may be held; non-zero otherwise. Records on the way out rather than asking
# the caller to, because a caller that forgets is a session that can be
# stopped forever and the failure is invisible until it happens.
local f="$1" id="$2" n
[ -n "$f" ] || return 1
id=$(printf '%s' "$id" | tr -cd '0-9')
[ -n "$id" ] || return 1
if [ -f "$f" ]; then
# Already held an act for this rule — the remedy has been offered once.
grep -qx "$id" "$f" 2>/dev/null && return 1
n=$(grep -c '^[0-9][0-9]*$' "$f" 2>/dev/null || printf '0')
# `grep -c` over a missing file can print nothing; a bare arithmetic test
# on an empty string is a syntax error in some shells and silently true in
# others, which is how a cap comes to cap nothing (#3191's shape).
n=$(printf '%s' "$n" | tr -cd '0-9')
[ -n "$n" ] || n=0
[ "$n" -ge "$_SCRIBE_CHECKPOINT_CAP" ] && return 1
fi
printf '%s\n' "$id" >> "$f" 2>/dev/null || return 1
# The evidence twin, beside the cap rather than instead of it. `$f` is swept
# at a compaction and SHOULD be: after one, this context has not read the
# rule, so the budget to stop an act on it is honestly fresh. That a stop
# already happened is a different claim, and it stays true.
printf '%s\n' "$id" >> "${f%.ids}.keep.ids" 2>/dev/null || true
return 0
}
scribe_json_deny() {
# $1 hook event name, $2 the reason the agent reads INSTEAD of the result.
#
# The one place this plugin emits a decision on a tool call. `deny` returns
# the reason to the MODEL and the call does not run — it is not a prompt to
# the operator, costs them nothing, and is undone by the model simply
# submitting the call again. That is the whole difference from `ask`, which
# would hand a judgement that is the agent's to make to the person who asked
# for the work.
local esc
esc=$(printf '%s' "$2" | scribe_json_escape) || return 0
printf '{"hookSpecificOutput":{"hookEventName":"%s","permissionDecision":"deny","permissionDecisionReason":"%s"}}\n' \
"$1" "$esc"
}
scribe_held_query() {
local ids
ids=$(scribe_rules_live "$1")
@@ -601,10 +1129,19 @@ scribe_held_query() {
SCRIBE_LEDGER_DIRS="scribe-priorart scribe-autoinject"
scribe_clear_session_ledgers() {
local sid="$1" dir
# SPARES `*.keep.ids`, which are evidence rather than exclusions — see
# `scribe_rules_append`. Everything else still goes: the convention is
# unchanged and still covers a ledger added tomorrow, which is what the
# block above insists on. The twin opts OUT by its name, so a new ledger is
# born on the swept side unless somebody says otherwise.
local sid="$1" dir f
[ -n "$sid" ] || return 0
for dir in $SCRIBE_LEDGER_DIRS; do
rm -f "${TMPDIR:-/tmp}/$dir/$sid"*.ids 2>/dev/null || true
for f in "${TMPDIR:-/tmp}/$dir/$sid"*.ids; do
[ -f "$f" ] || continue
case "$f" in *.keep.ids) continue ;; esac
rm -f "$f" 2>/dev/null || true
done
done
return 0
}
plugin/hooks/scribe_precompact_preserve.sh
+41 -5
View File
@@ -46,11 +46,22 @@
# text is the receipt. (Auto-compaction suppresses that notification.)
set -uo pipefail
# Drain the event so the caller never sees a broken pipe. Nothing in it changes
# what we emit: the instruction is the same whether the operator typed
# `/compact` or the session hit its limit, and it composes with any custom
# instructions they gave, which are merged ahead of ours.
cat >/dev/null 2>&1 || true
# shellcheck source=plugin/hooks/scribe_defs.sh
. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"
# THE EVENT IS NOW READ, where it used to be drained. The old comment here said
# nothing in it changes what we emit, and that was true while every line was
# static. It stopped being true at #4216: the slippage readout below is
# specific to THIS session, and `session_id` is the key to the ledgers that
# hold it. The instruction is still the same whether the operator typed
# `/compact` or the session hit its limit — what varies is the measurement
# appended to it.
event=$(cat 2>/dev/null || true)
session_id=""
if [ -n "$event" ]; then
event_flat=$(printf '%s' "$event" | scribe_json_flat 2>/dev/null || true)
session_id=$(scribe_json_pick "$event_flat" '.session_id' 2>/dev/null || true)
fi
cat <<'EOF'
Preserve the following literally in the summary — copied through, not
@@ -71,4 +82,29 @@ paraphrased or counted:
Everything else here can be recovered from the repository or from Scribe. These
cannot: they are this session's only copy.
EOF
# ── The slippage readout (#4216, milestone 419) ───────────────────────────
#
# WHY IT BELONGS HERE RATHER THAN IN A REPLY. A rule read and left unresolved
# is invisible by construction: it looks exactly like a rule that worked. The
# compaction is where that invisibility becomes permanent — the turns holding
# the evidence are summarised away, and an unjudged thing that survives as
# nothing is how a decision quietly becomes nobody's.
#
# STDOUT HERE IS THE SUMMARISER'S INSTRUCTIONS, not a message to the model
# (the header records how that was established). So this does not say "you
# slipped"; it says WHICH ids must be carried through, which is the one thing
# the summary can do about it. The next turn then reads them in the summary
# with the work still attached.
#
# SILENT WHEN THERE IS NOTHING TO SAY. A session that opened everything it was
# shown gets no extra lines. "0 rules unresolved" on every compaction is how a
# readout teaches its reader to skip it.
if [ -n "$session_id" ]; then
safe_sid=$(printf '%s' "$session_id" | tr -c 'A-Za-z0-9._-' '_')
slippage=$(scribe_slippage_lines "${TMPDIR:-/tmp}/scribe-priorart" "$safe_sid" 2>/dev/null || true)
if [ -n "$slippage" ]; then
printf '\n%s\n' "$slippage"
fi
fi
exit 0
plugin/hooks/scribe_prior_art.sh
+43 -4
View File
@@ -281,10 +281,49 @@ if [ -n "$local_lines" ] && [ "$reached" = 1 ]; then
fi
fi
# Local first. It answers "this already EXISTS", which is a stronger claim than
# "this resembles something recorded" — and it is the one the recorded arms are
# structurally unable to make.
combined="$local_context"
# ---------------------------------------------------------------------------
# ARM 0 — THE CONTRACT AROUND THE CHANGE (#4215, milestone 419).
#
# "You altered the arity, name or shape of something — here is everything that
# reads it." Rule 33's interface-contract check one scope down: not between
# layers but between a definition and its callers.
#
# LOCAL AND SERVERLESS, like ARM 1. It needs the working tree and nothing
# else — the server has no checkout, so this is the only place the question
# can be asked at all.
#
# EDITS ONLY. The comparison is between what the definition exposed before and
# what it exposes now, so it needs both texts; a Write that creates a file has
# no "before" and nothing to break.
contract_context=""
if [ -n "$repo_root" ] && [ -n "$code" ]; then
old_code=$(scribe_json_pick "$event_flat" '.tool_input.old_string')
[ -n "$old_code" ] || old_code=$(scribe_json_pick "$event_flat" '.tool_input.old_str')
# The SUBJECT is the definition whose contract may have moved. `shapes`
# already holds either the definitions in the payload or — for an edit
# inside a function body — the one enclosing the edit, which is exactly the
# thing whose callers matter. CSS rows are skipped: a class has readers, but
# they are markup files and `scribe_local_dups` already speaks for those.
subject=$(printf '%s\n' "$shapes" \
| awk -F'\t' 'NF>=2 && $1!="css" {print $2; exit}')
if [ -n "$old_code" ] && [ -n "$subject" ]; then
contract_file=""
[ -n "${safe_sid:-}" ] && contract_file="$state_dir/${safe_sid}.contract.ids"
contract_context=$(scribe_contract_block \
"$repo_root" "$rel_path" "$subject" "$old_code" "$code" "$contract_file" \
2>/dev/null) || contract_context=""
fi
fi
# CONTRACT FIRST of the three, and the order is the strength of the claim.
# This one says something may already be BROKEN by the edit in hand. The local
# arm says a copy exists. The recorded arms say something resembles this. A
# reader who reads one line should read that one.
combined="$contract_context"
if [ -n "$local_context" ]; then
[ -n "$combined" ] && combined="${combined}"$'\n'
combined="${combined}${local_context}"
fi
if [ -n "$context" ]; then
[ -n "$combined" ] && combined="${combined}"$'\n'
combined="${combined}${context}"
plugin/hooks/scribe_record_outcome.sh
Executable
+57
View File
@@ -0,0 +1,57 @@
#!/usr/bin/env bash
# Scribe — record that this session said what a rule DID, not merely that it
# read one (#4216, milestone 419).
#
# THE THIRD LEDGER, AND WHY THE TWO THAT EXIST ARE NOT ENOUGH.
#
# `.rules.ids` says a rule was NAMED. `.opened.ids` says it was READ (#4100 —
# a PostToolUse hook watches the `get_rule` call, so it is a recorded event
# rather than a model's claim about its own context). Neither can say what
# happened next, and that is the whole of what milestone 419 is about: a rule
# read and followed and a rule read and forgotten leave identical traces.
#
# `rule_outcome` (#4212) is the call that closes that gap, and the server
# records it. But the server's row carries no session — `rule_usage_events` is
# per user over a window — so a SESSION-scoped readout cannot be asked of it.
# The question "which rules changed something in THIS session" has to be
# answered where a session is a thing that exists, which is here.
#
# SAME EVIDENCE CLASS AS `.opened.ids`, deliberately. A tool call happened or
# it did not, and the harness reports it either way; nothing here asks the
# model whether it followed anything. That is the distinction milestone 386
# drew when it ruled out self-report, and this stays on the right side of it.
#
# WHAT IT CANNOT SAY: that the rule was followed WELL, or that `applied` was
# honest. It records that an outcome was declared. The value of that is not the
# claim itself — it is that the absence of one becomes visible, which is the
# state nothing could previously name.
#
# EXIT 0, ALWAYS. This decorates a ledger; a bookkeeping failure must never
# turn a successful tool call into a hook error.
set -uo pipefail
# shellcheck source=plugin/hooks/scribe_defs.sh
. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"
event=$(cat 2>/dev/null || true)
[ -n "$event" ] || exit 0
event_flat=$(printf '%s' "$event" | scribe_json_flat)
session_id=$(scribe_json_pick "$event_flat" '.session_id')
[ -n "$session_id" ] || exit 0
# The matcher in hooks.json narrows to the rule_outcome tools, but the server
# segment of an MCP tool name varies with how the plugin was installed, so the
# id is read from the field rather than from an assumed tool name — the same
# reasoning scribe_record_opened.sh gives.
rule_id=$(scribe_json_pick "$event_flat" '.tool_input.rule_id')
rule_id=$(printf '%s' "$rule_id" | tr -cd '0-9')
[ -n "$rule_id" ] || exit 0
state_dir="${TMPDIR:-/tmp}/scribe-priorart"
mkdir -p "$state_dir" 2>/dev/null || true
safe_sid=$(printf '%s' "$session_id" | tr -c 'A-Za-z0-9._-' '_')
# Stamped and append-only like its two siblings, so one reader ages them all.
printf '%s\n' "$rule_id" | scribe_rules_append "$state_dir/${safe_sid}.acted.ids"
exit 0
plugin/hooks/scribe_tool_rules.sh
+36 -2
View File
@@ -102,12 +102,46 @@ body=$(curl -fsS --max-time 5 \
body_flat=$(printf '%s' "$body" | scribe_json_flat)
context=$(scribe_json_pick "$body_flat" '.context')
[ -n "$context" ] || exit 0
# Remember what was named so it is not repeated this session.
# Remember what was named so it is not repeated this session. BEFORE the
# checkpoint branch and before the empty-context exit: the ledger records what
# the server chose to surface, which happened whichever way this hook then
# renders it. Doing it inside one branch is how the two arms' ledgers came to
# disagree once already.
if [ -n "$rulefile" ]; then
scribe_json_list "$body_flat" '.rule_ids' | scribe_rules_append "$rulefile"
fi
# ── The pre-act checkpoint (#4214, milestone 419) ────────────────────────
#
# WHY THIS ARM AND NOT THE WRITE PATH. scribe_prior_art.sh carries an explicit,
# tested property that it never returns a permissionDecision — a recall aid may
# not stand in the way of a write, which is the operator's decision and is
# guarded by test_hook_never_returns_a_permission_decision. No equivalent
# decision covers this arm, and the misses that motivated the milestone on the
# command side are the ones a stop actually reaches: a commit message asserting
# what CI said, a verification script that checks nothing.
#
# WHAT THE STOP IS FOR. Everything else this plugin emits is additionalContext,
# which Claude Code delivers alongside the tool RESULT — so the rule is read
# after the call is written and reads as commentary on a decision already made.
# That is milestone 419's central finding. A deny returns the reason to the
# model with the call unrun, so the rule's own text can be read before the act
# exists. It costs the operator nothing: no prompt reaches them, and the model
# clears it by reading one record and re-submitting.
#
# It cannot recur. `scribe_checkpoint_allowed` holds at most one act per rule
# and at most five per session, so the worst case of a mis-set floor is a noisy
# session rather than one that cannot move.
cp_rule=$(scribe_json_pick "$body_flat" '.checkpoint.rule_id')
cp_reason=$(scribe_json_pick "$body_flat" '.checkpoint.reason')
if [ -n "$cp_rule" ] && [ -n "$cp_reason" ] && [ -n "$session_id" ]; then
if scribe_checkpoint_allowed "$state_dir/${safe_sid}.checkpoint.ids" "$cp_rule"; then
scribe_json_deny PreToolUse "$cp_reason"
exit 0
fi
fi
[ -n "$context" ] || exit 0
scribe_json_out PreToolUse "$context"
exit 0
scripts/check_plugin.py
+11
View File
@@ -375,6 +375,17 @@ SMOKE_EVENTS: dict[str, str] = {
"tool_name": "mcp__scribe__get_rule", "tool_input": {"rule_id": 1},
"tool_response": {}}
),
# The acted-ledger recorder (#4216). Same shape and same reasoning as its
# sibling above: TMPDIR only, and silent, because a PostToolUse hook that
# spoke would put a line after every rule_outcome call and turn recording
# an outcome into something with a cost. A skip here would have left the
# newest of the three ledgers as the only hook the lane never runs.
"scribe_record_outcome.sh": json.dumps(
{"session_id": "smoke", "cwd": ".",
"tool_name": "mcp__scribe__rule_outcome",
"tool_input": {"rule_id": 1, "outcome": "applied"},
"tool_response": {}}
),
# The shared library is sourced, never run; executed bare it defines
# functions and exits — silent by construction.
"scribe_defs.sh": "",
src/scribe/app.py
+27
View File
@@ -182,6 +182,33 @@ def create_app() -> Quart:
start_notification_loop()
start_auth_token_retention_loop()
# Write down any shipped default that moved in this release (#4225).
#
# INLINE AND AWAITED, deliberately, against the instinct the block
# below argues for. What #4181 cost three hours was CONCURRENCY — a
# background task created here racing this hook for the same pool.
# Running sequentially creates no such contention, and this is twelve
# single-row reads on an index against a table with a handful of rows.
#
# It has to finish before serving because the thing it protects is a
# telemetry read: `band_hugs_floor` compares a band against a floor,
# and across a release that changed the floor those describe different
# regimes. A readout served before the change was recorded is the very
# answer this exists to stop giving. Failure is logged and swallowed —
# provenance is worth a lot and never worth refusing to boot.
try:
from scribe.services.retrieval_tuning import record_release_defaults
moved = await record_release_defaults()
for row in moved:
if not row["baseline"]:
app.logger.info(
"release moved %s's shipped %s: %s -> %s",
row["surface"], row["dial"],
row["old_value"], row["new_value"],
)
except Exception:
app.logger.warning("could not record release defaults", exc_info=True)
# Backfill embeddings for any notes that don't have one. Runs in the
# background so it never blocks the server from accepting requests —
# and, since #4181, not until the rest of this hook has finished.
src/scribe/mcp/server.py
+8
View File
@@ -185,6 +185,14 @@ _WRITE_TOOLS = frozenset({
"create_rule", "create_project_rule", "update_rule", "move_rule", "delete_rule",
"create_preference", "update_preference",
"relate_rules", "unrelate_rules", "mark_rule_verified",
# rule_outcome writes only telemetry, which is the case _READ_ONLY_TOOLS
# above explicitly tolerates for getters that call record_pulled. It is
# classed as a WRITE anyway, on the difference that matters: those are
# reads that happen to leave a trace, while this tool's entire effect is
# the row — and the row carries `detail`, free prose the agent authored.
# A read-scoped key that can put text into the operator's database is not
# read-scoped, whatever table it lands in (#4212).
"rule_outcome",
# retrieval tuning — a write in both senses: it moves the number the arm
# reads, and it appends the reason to the audit trail (#4102).
"tune_retrieval",
src/scribe/mcp/tools/lessons.py
+13 -1
View File
@@ -17,7 +17,7 @@ from scribe.services import lessons as lessons_svc
from scribe.services import systems as systems_svc
from scribe.services import trash as trash_svc
from scribe.mcp.tools import systems as systems_tools
from scribe.services.note_usage import record_pulled
from scribe.services.note_usage import empty_usage, record_pulled, usage_for_notes
# The payload shape lives in the service (`lesson_to_dict`), shared with the
@@ -61,6 +61,11 @@ async def list_lessons(
project_id=project_id or None,
)
labelled = await access_svc.label_shared_items(uid, items)
# One aggregate for the page, like the snippet listing — surfaced-vs-opened
# per lesson (#4196). An agent listing lessons can see which of its own
# triggers are firing and which are not, which is the reading that leads to
# `update_lesson` rather than to a second lesson about the same failure.
usage = await usage_for_notes([int(it["id"]) for it in labelled])
rows = [
{
"id": it["id"], "title": it["title"], "tags": it.get("tags", []),
@@ -68,6 +73,7 @@ async def list_lessons(
# Projected by `_note_to_item` straight off the `data` mirror —
# absent when the row carries none, rather than an empty string.
"when_to_apply": it.get("when_to_apply", ""),
"usage": usage.get(int(it["id"]), empty_usage()),
**({"shared": True, "owner": it.get("owner")} if it.get("shared") else {}),
}
for it in labelled
@@ -203,6 +209,12 @@ async def get_lesson(lesson_id: int, project_id: int = 0) -> dict:
# retrieval as any other note, so a getter that records nothing would leave
# the kind permanently at zero pulls — reading as dead weight beside kinds
# that merely had a counter (#2476, the repeat of #2245).
# Read BEFORE the pull is recorded, so the number an agent is shown is the
# one that was true when it asked — otherwise every first read of a lesson
# reports a pull that is its own.
out["usage"] = (await usage_for_notes([int(note.id)])).get(
int(note.id), empty_usage()
)
record_pulled(
user_id=uid, note_id=int(note.id),
source="mcp_get_lesson", project_id=project_id,
src/scribe/mcp/tools/rulebooks.py
+64 -2
View File
@@ -18,7 +18,9 @@ from scribe.mcp._context import current_user_id
from scribe.services import dedup as dedup_svc
from scribe.services import rulebooks as rulebooks_svc
from scribe.services import trash as trash_svc
from scribe.services.rule_usage import record_rule_pulled
from scribe.services.rule_usage import (
record_rule_outcome, record_rule_pulled,
)
# ── Rulebook CRUD ───────────────────────────────────────────────────────
@@ -245,6 +247,66 @@ async def get_rule(rule_id: int) -> dict:
return await rulebooks_svc.rule_detail(uid, rule)
async def rule_outcome(rule_id: int, outcome: str, why: str = "") -> dict:
"""Record what a rule you read ACTUALLY CHANGED — applied, or departed from.
Call this after a rule has been surfaced to you and you have acted. It is
the only way the system can tell a rule that is working from a rule that
is being read and ignored: `surfaced` says it was offered, `get_rule` says
it was opened, and until this exists neither says whether it made any
difference. A rule obeyed every time and a rule ignored every time leave
identical telemetry, and the second is the one worth knowing about.
`outcome` is one of:
"applied" — it changed what you did, or it confirmed the approach you
were already taking. `why` is optional; following a rule is
the ordinary case and does not need an argument.
"departed" — you read it and deliberately did not follow it. `why` is
REQUIRED and is the whole value of the call: a departure
without its reason is indistinguishable from a miss when
somebody reads this back, and "somebody" is usually you, in
a later session, with none of today's context.
There is deliberately NO value for "read it and ignored it". That state is
real, and it is the one this measurement exists to expose — but it is not
something you can report, because noticing it is the same act as not doing
it. It is derived instead: a rule you opened and never came back to. The
honest way to keep yourself out of that bucket is to call this, not to
reach for a word that describes it.
A departure is a legitimate answer and is not a confession. Rules are
written for the common case; recording the edge you found is how the rule
gets better, and a corpus where nothing is ever departed from is a corpus
nobody is really reading.
"""
uid = current_user_id()
rule = await rulebooks_svc.get_rule(rule_id, uid)
if rule is None:
raise ValueError(f"rule {rule_id} not found")
choice = (outcome or "").strip().lower()
if choice not in ("applied", "departed"):
raise ValueError(
f"outcome must be 'applied' or 'departed', got {outcome!r}"
)
if choice == "departed" and not (why or "").strip():
raise ValueError(
"a departure needs its reason — pass `why`. Without it the record "
"cannot be told from a rule that was simply missed."
)
record_rule_outcome(
user_id=uid, rule_id=int(rule.id), outcome=choice,
source="mcp_rule_outcome", detail=why,
)
return {
"rule_id": int(rule.id),
"title": rule.title,
"outcome": choice,
"why": (why or "").strip() or None,
"recorded": True,
}
async def create_rule(
topic_id: int, title: str, statement: str, when_to_apply: str,
why: str = "", how_to_apply: str = "", order_index: int = 0,
@@ -1086,7 +1148,7 @@ def register(mcp) -> None:
for fn in (
list_rulebooks, get_rulebook, create_rulebook, update_rulebook, delete_rulebook,
list_topics, create_topic, update_topic, delete_topic,
list_rules, get_rule,
list_rules, get_rule, rule_outcome,
create_rule, create_project_rule, update_rule, move_rule, delete_rule,
create_preference, update_preference,
relate_rules, unrelate_rules,
src/scribe/mcp/tools/search.py
+33
View File
@@ -336,6 +336,18 @@ It is an UPPER BOUND per surface: a pull records the door it came
`pulled_by_human`, the distinct-rule counts, and `pull_through` on the same
definition (agent pulls over RANKED surfacings).
`applied`, `departed` AND `distinct_rules_acted` ARE WHAT HAPPENED AFTER
THE RULE WAS OPENED (#4213). A pull says the rule was read; these say it
changed something. `applied` counts rules followed, `departed` rules
deliberately not followed — kept apart rather than summed, because a
departure carries the reason the agent gave and is evidence about the
RULE, while an application is evidence about the agent. There is
deliberately no count of rules read and quietly ignored: that state is what
is left over when a rule was pulled and neither outcome arrived, and
`read_and_unacted` below is where it is reported. Asking an agent to
declare it would be asking it to notice an omission it is defined by not
noticing.
A SEPARATE BLOCK, not folded into `usage`, and reading it as one number
with that is the mistake to avoid. The corpora differ by orders of
magnitude — a few dozen eligible rules against thousands of notes — so a
@@ -413,11 +425,32 @@ It is an UPPER BOUND per surface: a pull records the door it came
- `band_hugs_floor` — the weakest tenth of what an arm returns sits on
its floor. The bar is doing the selecting and the score is not, so
moving that floor changes how MUCH you get, not how good it is.
- `floor_moved_mid_window` — that arm's floor CHANGED inside the window,
by a release or by a dial turn, so its calls were made under two bars
and the band check above is suspended for it rather than answered
wrongly. Ask again with a `days` starting after the named date. The
warning replaces `band_hugs_floor` for that arm; it never accompanies
it.
- `no_duration` — rows written without timings. A logging gap, not a slow
arm, and it devalues every other number from that source.
- `surfaced_never_pulled` — distinct records shown and never opened, per
corpus. Read their titles before touching a threshold: a record nobody
opens is usually one whose title does not say when it matters.
- `read_and_unacted` — distinct rules OPENED in the window that recorded
no outcome, against the ones that did. The failure milestone 419 was
opened on, and the worse sibling of `surfaced_never_pulled` above: a
rule nobody opens is cheap, while a rule read and silently unchanged is
indistinguishable from one that worked. It does not say which of the two
causes it is — a rule mis-triggering, arriving where it does not apply,
or a rule being ignored — and those want opposite fixes, so read the
rules before moving anything.
- `outcomes_never_recorded` — rules were opened and NOT ONE outcome exists
anywhere in the window. Deliberately a separate code, and not a
`read_and_unacted` with a zero in it: a window with no outcomes at all
cannot tell "every rule was ignored" from "nothing on this install calls
`rule_outcome` yet", and reporting the first would manufacture a finding
out of an unwired feature. Wire the outcome call before reading this as
a fact about the corpus.
- `unregistered_source` — rows under a source missing from
`retrieval_registry`. Its numbers are real; no verdict could be
computed, because nothing says whether it was asked or fired unbidden.
src/scribe/mcp/tools/shapes.py
+15 -5
View File
@@ -267,13 +267,23 @@ async def stamps_to_review(project_id: int, top: int = 10) -> dict:
score, signature and derived form so you can judge rather than trust the
threshold.
`incoherent` — canons whose judged rows do not agree on a form: the
`forms` histogram, how many members were hook-stamped weakly, and a
sample. A canon asserts that some shapes are the same sort of thing; when
its members are a class, three getters and a dozen tests, that assertion
has stopped being true and every base-rate reading built on it — the
`incoherent` — canons whose membership no longer agrees what the canon
is. A canon asserts that some shapes are the same sort of thing; when its
members are a class, three getters and a dozen tests, that assertion has
stopped being true and every base-rate reading built on it — the
divergence prompt included — is reading noise.
Disagreement is judged by FAMILY (a sync helper and an async one are one
family), and a method defined beside a member class counts as part of the
shape rather than against it — so a model convention that covers both the
class and its `to_dict` reads as coherent, which it is (#4220). Each
entry carries `families` and `forms`, the majority `family`, `attached`
(minority rows excused as methods of a member), `strangers` — THE ROWS
THAT DO NOT FIT, which is what you judge — and `unattended` / `weak_rows`,
the count written by the hook with nobody reading. An incoherence whose
rows are all judged is likelier this check being strict than a bad
ledger; one made of unattended rows is the real thing.
A row listed here is a question, not a verdict. Some will be correct.
Read-only; requires read access to the project.
src/scribe/models/rule_usage.py
+36 -2
View File
@@ -7,9 +7,24 @@ from scribe.models.base import CreatedAtMixin, iso
SURFACED = "surfaced"
PULLED = "pulled"
# The outcome half (#4212, milestone 419). A rule that was read and then
# ignored has always been indistinguishable from one that was read and
# obeyed; these are the two events that can tell them apart.
#
# There is deliberately NO third value for "read and ignored". That state is
# real and is the whole point of the milestone, but it cannot be reported:
# an agent that knew it was ignoring a rule would not be ignoring it. It is
# DERIVED — a pull with no outcome — and an enum member for it would collect
# nothing while reading as though it had measured something, which is #3311's
# failure exactly.
APPLIED = "applied"
DEPARTED = "departed"
OUTCOMES = (APPLIED, DEPARTED)
class RuleUsageEvent(Base, CreatedAtMixin):
"""One row per time a rule was SURFACED to the agent, or PULLED in full.
"""One row per time a rule was SURFACED to the agent, PULLED in full, or
ACTED ON — applied, or departed from with a stated reason.
The sibling `note_usage_events` has had since 2026-07, third in the line
after `rule_embeddings` and `rule_versions` — and, like those, it exists
@@ -58,7 +73,13 @@ class RuleUsageEvent(Base, CreatedAtMixin):
user_id: Mapped[int | None] = mapped_column(BigInteger, nullable=True)
rule_id: Mapped[int] = mapped_column(BigInteger, nullable=False)
# 'surfaced' | 'pulled'
# 'surfaced' | 'pulled' | 'applied' | 'departed'
#
# Plain Text with no CHECK, as created in 0094 — which is why 0106 added
# the outcome pair without a DROP/ADD migration. Rule 36 governs
# CHECK-whitelisted columns and this is not one. Said here as well as in
# the migration because this is where the next person adding a value will
# look first.
event: Mapped[str] = mapped_column(Text, nullable=False)
# Which surface produced it. A CONVENTION, not a fixed vocabulary, and the
@@ -76,6 +97,18 @@ class RuleUsageEvent(Base, CreatedAtMixin):
# without saying why.
source: Mapped[str] = mapped_column(Text, nullable=False)
# The WHY of a departure, and the reason the outcome pair is not simply
# two more bare event strings. A departure stripped of its reason reads
# back as a miss, so the two states this table exists to separate would
# collapse again one layer down — in the readout, where nobody would see
# it happen.
#
# Nullable because `applied` needs no argument. Following a rule is the
# unremarkable case; demanding prose for it would make the cheap event
# expensive, and an expensive event is one that stops being recorded.
# Empty on a `surfaced` or `pulled` row, which nobody asks a reason of.
detail: Mapped[str | None] = mapped_column(Text, nullable=True)
__table_args__ = (
# Every readout is "these rule ids, split by event" — a covering
# composite beats separate single-column indexes for it.
@@ -92,4 +125,5 @@ class RuleUsageEvent(Base, CreatedAtMixin):
"rule_id": self.rule_id,
"event": self.event,
"source": self.source,
"detail": self.detail,
}
src/scribe/routes/lessons.py
+15 -4
View File
@@ -72,10 +72,21 @@ async def list_lessons_route():
offset=offset,
project_id=project_id,
)
return jsonify({
"lessons": await label_shared_items(uid, items),
"total": total,
})
items = await label_shared_items(uid, items)
# One aggregate for the whole page — a per-row lookup would be N+1 by
# construction. Every row gets the key, zero-filled, so the UI renders
# "never surfaced" rather than treating a missing field as a state.
#
# Lessons were the one kind collecting this and showing it nowhere (#4196).
# The counts matter more here than on a snippet: the promotion question a
# lesson eventually raises — does this bind? — is answered by repeatedly
# surfaced AND repeatedly opened, and the far commoner reading of the same
# row is that the trigger fires on the wrong situation, which `update_lesson`
# exists to fix.
usage = await usage_for_notes([int(it["id"]) for it in items])
for it in items:
it["usage"] = usage.get(int(it["id"]), empty_usage())
return jsonify({"lessons": items, "total": total})
@lessons_bp.route("/taught-by/<int:record_id>", methods=["GET"])
src/scribe/routes/plugin.py
+26
View File
@@ -170,6 +170,24 @@ async def pre_tool_rules():
different claims about the reader's
context, so they get different lines
(#4100).
Returns `context`, `rule_ids`, and `checkpoint` (#4214, milestone 419).
`checkpoint` IS THE ONE PART OF THIS RESPONSE THAT IS NOT A HINT. It is
empty on almost every call. When present it carries `rule_id`, `title`,
`trigger`, `score` and a rendered `reason`, and it means the hook should
DENY the call rather than annotate it — because every other line this
endpoint returns is delivered by Claude Code alongside the tool RESULT,
so it reaches the reader after the act is composed and reads as
commentary on a decision already made.
It is raised only for a rule (never a preference, which claims no such
force), only for the ranker's top hit, only above the checkpoint
threshold — well above this arm's own floor — and only when the session
has NOT opened that rule. The remedy is one `get_rule` call, and the act
may then be re-submitted unchanged; the hook caps stops at one per rule
and five per session so a mis-set floor degrades to noise rather than to
a session that cannot proceed.
"""
tool = (request.args.get("tool") or "tool").strip()
command = request.args.get("command") or ""
@@ -223,6 +241,14 @@ 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.
(Returns a `checkpoint` block on the same contract as /tool-rules —
see that endpoint. The write-path HOOK deliberately does not act on
it: scribe_prior_art.sh carries a tested property that it never
returns a permissionDecision, on the operator's decision that a recall
aid may not stand in the way of a write. The block is computed and
returned so that decision can be revisited with evidence rather than
re-argued, and so a change of mind is a hook edit and not a feature.)
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
src/scribe/services/backup.py
+775 -431
View File
File diff suppressed because it is too large Load Diff
src/scribe/services/coverage.py
+129 -1
View File
@@ -93,6 +93,133 @@ _ARROW_RE = re.compile(
)
# --- comment and string spans (#4222) ----------------------------------------
#
# The matchers above are line-oriented and know nothing about what a line is
# INSIDE. A wrapped docstring whose line happens to begin "class AND the …"
# reads as a definition of a shape called `AND`, and that phantom reaches the
# agent mid-edit as a divergence prompt about a symbol that does not exist.
# This repo's own source minted twenty-two of them, measured by running the
# extractor over every scannable file with and without this scan. Two —
# `with` and `nobody`, out of one module docstring in
# scripts/check_dangling_styles.py — are persisted, JUDGED `code_shapes`
# rows, so the cost was never only noise in the moment. Those clear
# themselves: sync_shapes marks a row it no longer extracts as vanished.
#
# The honest tool for .py would be `ast`, which cannot be fooled by prose at
# all. It is not what this uses, because this extractor is mirrored rule for
# rule by an awk program in plugin/hooks/scribe_defs.sh, awk cannot parse
# Python, and a fix only one of the pair can run is the drift the mirror
# exists to prevent. What both can do is blank the SPANS: a triple-quoted
# string or a /* … */ comment is replaced by its own newlines before any
# matcher sees a line, so every line index still lines up and the signature,
# body and fingerprint keep reading the untouched original.
#
# ONLY CLOSED SPANS ARE BLANKED, and an unterminated opener is stepped over
# rather than bailed on, so a stray opener costs one mishandled span and never
# every definition below it in the file.
_SPANS = (('"""', '"""'), ("'''", "'''"), ("/*", "*/"))
def _string_end(text: str, at: int) -> int:
"""Offset just past the one-line string opening at ``at``.
``at`` + 1 when it does not close before the newline, so an apostrophe in
prose — `don't` in a Vue template, outside any comment — costs one
character rather than everything up to the next quote in the file.
"""
quote = text[at]
i = at + 1
while i < len(text):
c = text[i]
if c == "\\":
i += 2
elif c == "\n":
return at + 1
elif c == quote:
return i + 1
else:
i += 1
return at + 1
def _hash_comment(text: str, at: int) -> bool:
"""Does the `#` at ``at`` open a comment, or is it a CSS colour or id?
`#` is the one marker whose meaning depends on the language, and the
extractor is handed text with no path. An alphanumeric straight after it
is `#fff` or `#app`; anything else — a space, a `!`, a `-` — is a comment
in every language that has one.
"""
nxt = text[at + 1:at + 2]
return not nxt.isalnum()
def _blank_spans(text: str) -> str:
"""``text`` with comment and string spans replaced by their own newlines.
A single left-to-right pass, because the alternative — matching markers
wherever they appear — cannot tell a comment from a string that QUOTES
one. Both of those are in this repo: the docstring-matching regex in
plugin_context.py holds a triple quote inside a single-quoted literal,
and test_design_stylesheet.py asserts on the text `red /*` inside a
double-quoted one. Each cost every definition below it in its file before
the scan was written this way.
Line COUNT is preserved, column positions are not — the result is only
ever fed to the line matchers, which lstrip anyway.
"""
out: list[str] = []
i = cut = 0
n = len(text)
while i < n:
opener = closer = ""
for op, cl in _SPANS:
if text.startswith(op, i):
opener, closer = op, cl
break
if opener:
end = text.find(closer, i + len(opener))
if end < 0:
# Unterminated: step over the opener rather than bail, so a
# stray marker costs one span and not the rest of the file.
i += len(opener)
continue
end += len(closer)
out.append(text[cut:i])
out.append("\n" * text.count("\n", i, end))
i = cut = end
continue
if text.startswith("//", i) or (text[i] == "#" and _hash_comment(text, i)):
# A line comment is COPIED, not blanked: its continuation lines
# carry their own marker, so none of them can read as a
# definition on their own.
nl = text.find("\n", i)
i = n if nl < 0 else nl
continue
if text[i] in "\"'":
i = _string_end(text, i)
continue
i += 1
out.append(text[cut:])
return "".join(out)
def _masked_lines(text: str, count: int) -> list[str]:
"""``text`` blanked and split, reconciled to ``count`` lines.
Blanking preserves every ``\n``, but ``splitlines`` also breaks on a bare
``\r`` and on the vertical-tab family, which a blanked span drops. The
reconciliation is what keeps a span containing one of those from shifting
suppression onto the wrong lines — padding is short by a line, never
misaligned by one.
"""
masked = _blank_spans(text).splitlines()
if len(masked) < count:
masked += [""] * (count - len(masked))
return masked[:count]
class Definition(NamedTuple):
"""One extracted definition with its content fingerprint (#2792).
@@ -176,8 +303,9 @@ def extract_definitions(text: str) -> list[Definition]:
(an overload, a re-declaration) is the same shape to it.
"""
lines = text.splitlines()
masked = _masked_lines(text, len(lines))
starts: list[tuple[int, str, str]] = []
for i, raw in enumerate(lines):
for i, raw in enumerate(masked):
hit = _definition_on(raw)
if hit:
starts.append((i, hit[0], hit[1]))
src/scribe/services/plugin_context.py
+211 -1
View File
@@ -271,6 +271,66 @@ RULEHINT_LIMIT = SURFACES["write_path_rule"].budget_default
# stops working and the answer is a reranker (#1038), not a smaller number.
_RULEHINT_BAND = 0.05
# ── The pre-act checkpoint (#4214, milestone 419) ───────────────────────
#
# WHAT A CHECKPOINT IS, AND WHY IT IS NOT A LOUDER HINT. Every arm above
# appends text to an act the agent has already composed. Claude Code delivers
# `additionalContext` alongside the tool result, so by the time the line is
# read the call is written and the rule reads as commentary on a decision
# already made. That is milestone 419's central finding, measured over a
# session where three misses were caught by the operator and none by this
# system.
#
# A checkpoint is the same retrieval, spent differently: the hook returns a
# `deny` and the act does not run, so the rule's own text is read BEFORE the
# call exists. The remedy is `get_rule(id)` and nothing else — which is why
# the condition below is "the session has not opened it" rather than "the
# session has not recorded an outcome for it". An outcome can be satisfied
# with one cheap call that asserts compliance without producing any, and a
# checkpoint that can be dismissed that way manufactures exactly the
# compliance data step 2 exists to measure. Reading a rule cannot be faked in
# that direction: after `get_rule` the statement is in context, which is the
# whole of what was wanted.
#
# WHAT "CONSEQUENTIAL" MEANS HERE, AND WHY IT IS NOT A LIST. The obvious
# implementation enumerates act kinds — a write to product code, a schema
# change, a bulk classification, a merge. Every one of those is consequential
# because THIS operator wrote rules about it, and a shipped list would be this
# instance's corpus hard-coded into the product (rule 115). So the corpus
# decides: an act is consequential when the install's own rules speak to it
# with high confidence. A fresh install with no rules never stops anything,
# and an install whose rules are about something else entirely stops on that
# instead.
_CHECKPOINT_THRESHOLD_KEY = "kb_checkpoint_threshold"
# MEASURED, not chosen (2026-09-21, `retrieval_telemetry(days=30)` on the
# instance this was built on — recorded as provenance for the number, not as
# a defence of it under rule 115):
#
# write_path_rule floor 0.72 p10 0.6984 p50 0.7319 p90 0.7628 max 0.8817
# pre_tool_rule floor 0.68 p10 0.6838 p50 0.7018 p90 0.7373 max 0.8293
#
# 0.80 sits above p90 on BOTH arms and below max on both, so it selects from
# the top decile of an already-selective arm rather than from its bulk — and
# it is reachable, which a bar above 0.8293 would not be for the busier arm.
# Over that window the two arms returned ~3,584 non-empty calls between them,
# so an upper bound of one tenth of those is ~12 a day across a very heavy
# install, and the true rate is lower because 0.80 is not p90 but above it.
#
# A SETTING, because the number above is a distance in one embedding model's
# geometry over one corpus and cannot transfer (retrieval_surfaces' opening
# argument). The default ships as a starting point with the means to correct
# it, which is the only honest form for a number like this.
_CHECKPOINT_DEFAULT = 0.80
# A session cannot be stopped more than this many times, however the corpus
# scores. Not a tuning value — a guard on the worst case, like MAX_BUDGET: a
# mis-set floor or a corpus that suddenly resembles everything must degrade to
# a noisy session, never to one that cannot make progress. The hook enforces
# it, because only the hook knows what a session is.
CHECKPOINT_SESSION_CAP = 5
# AND A REPEAT COMPETES ON RANK ALONE (#3750), WHICH SURVIVES THE BAND.
#
# Since #3750 a hit already on the session's exclusion ledger is RENDERED
@@ -1524,8 +1584,35 @@ async def get_writepath_config(user_id: int) -> dict:
"rule_top_k": await budget_for(user_id, "write_path_rule"),
"tool_rule_threshold": await floor_for(user_id, "pre_tool_rule"),
"tool_rule_top_k": await budget_for(user_id, "pre_tool_rule"),
# NOT from the surfaces registry, and deliberately so. Everything in
# that table is a pair belonging to one QUERY — a floor saying what is
# worth ranking and a budget saying how many lines it may spend. The
# checkpoint runs no query of its own; it re-reads hits the two rule
# arms already produced and asks a different question of them. Putting
# it in the registry would give it a phantom budget and make the
# tuning tool offer to change how many checkpoints an act may raise,
# which is not a number anyone should have.
"checkpoint_threshold": await _checkpoint_floor(user_id),
}
async def _checkpoint_floor(user_id: int) -> float:
"""The confidence at which a hint becomes a stop. Clamped, never trusted.
A floor read out of settings reaches here as operator-typed text. Below
zero it would stop every act with a rule anywhere near it; above one it
can never fire and the feature is silently dead, which is the failure mode
#3430 found and the reason this clamps rather than validating at the door.
"""
raw = await get_setting(
user_id, _CHECKPOINT_THRESHOLD_KEY, str(_CHECKPOINT_DEFAULT),
)
try:
value = float(str(raw).strip())
except (TypeError, ValueError):
return _CHECKPOINT_DEFAULT
return min(1.0, max(0.0, value))
def _rule_band(hits: list) -> list:
"""The top hit, plus every hit within `_RULEHINT_BAND` of it (#3851).
@@ -1547,6 +1634,104 @@ def _rule_band(hits: list) -> list:
return [(s, r) for s, r in hits if s >= top - _RULEHINT_BAND]
def checkpoint_for(
kept: list, *, held: set[int], floor: float, where: str,
) -> dict:
"""The one rule, if any, that should STOP this act rather than annotate it.
Sync and pure so it can be read against a fixed list of hits without a
database — the two arms share it for the reason `_rule_band` is shared:
#3497 is the record of these two drifting apart by being modelled on each
other instead of sharing one function.
FOUR CONDITIONS, AND EACH IS A DIFFERENT KIND OF WRONG IT PREVENTS.
1. THE SCORE CLEARS `floor`. Not the arm's own floor — a much higher bar,
measured at `_CHECKPOINT_DEFAULT`. The hint arms keep nudging at their
floor; only a hit the corpus is confident about is allowed to stop
anything.
2. IT IS A RULE, NEVER A PREFERENCE. A preference says how something has
been done before and following it is what keeps work consistent; a rule
says what happens if you do not. Stopping an act over a preference
would assert a force the record explicitly does not claim, and
`_rule_hint_line` already keeps that distinction in the one word that
names it.
3. THE SESSION HAS NOT OPENED IT. `held` is observable — a PostToolUse
hook watches for the `get_rule` call (#4100) — so this is a recorded
event and not a model's self-report about its own context. A session
that read the rule has already had the thing the checkpoint exists to
produce, and stopping it again would be punishing the behaviour being
asked for.
4. IT IS THE TOP HIT. `kept` is a band, and a band's tail is there to let
an act surface a SET; the ranker's confidence claim attaches to its
first element only. A checkpoint raised on the fourth line of a band is
a stop justified by a score nobody claimed.
Returns a dict rather than a rule or a tuple. Widening a tuple is an
interface change to every unpack site that the compiler does not report
(#4207, learned the expensive way in this same milestone's first week), and
this value crosses a JSON boundary into a shell script where a missing
field is a silently empty variable.
"""
if not kept or floor <= 0:
return {}
score, rule = kept[0]
if score < floor:
return {}
if getattr(rule, "kind", "") == "preference":
return {}
if rule.id in held:
return {}
found = {
"rule_id": rule.id,
"title": rule.title,
"trigger": (rule.when_to_apply or "").strip(),
"score": round(float(score), 4),
"where": where,
}
# RENDERED HERE, at the one call site, rather than by each arm. Two arms
# that each remember to render it are two arms that can stop agreeing on
# what a stop says — which is #3497's history for this exact pair. The
# text stays a separate function so it can be read and tested without a
# rule object, but nothing outside this line decides whether to call it.
found["reason"] = checkpoint_reason(found)
return found
def checkpoint_reason(checkpoint: dict) -> str:
"""The text the agent reads INSTEAD of running the act.
Written as a practice rather than a prohibition (rule 165): it says what
to do and why it is worth doing, not what is forbidden. The act is not
wrong — nothing here knows whether it is — and saying so plainly is what
keeps the stop from reading as an accusation the system is in no position
to make.
It names the remedy as ONE call, because a stop whose remedy is vague
costs more than the miss it prevents. And it says the act may simply be
re-submitted afterwards, so a reader who finds the rule irrelevant is out
in two calls rather than negotiating with a hook.
"""
if not checkpoint:
return ""
trigger = checkpoint.get("trigger") or ""
return (
f"Held for one read. “{checkpoint['title']}” is a standing rule "
f"this session has not opened, and it scores {checkpoint['score']} "
f"against what you are about to do"
+ (f" ({trigger})" if trigger else "")
+ f". Read it with get_rule({checkpoint['rule_id']}), then go ahead — "
f"re-submit this call unchanged if the rule does not apply, which is a "
f"judgement only you can make. Nothing here has decided the act is "
f"wrong; the rule is being put in front of it rather than beside it, "
f"because a rule delivered alongside a result arrives after the "
f"decision it was meant to inform."
)
def _rule_hint_line(
rule, *, where: str, seen: bool, held: bool = False, compact: bool = False,
) -> str:
@@ -2203,6 +2388,7 @@ async def build_write_path_hint(
#
# Fails open like every other arm: a rule hint must never break a write.
rule_ids: list[int] = []
checkpoint: dict = {}
try:
already = set(exclude_rule_ids or [])
held = set(held_rule_ids or [])
@@ -2242,6 +2428,14 @@ async def build_write_path_hint(
# would inflate pull_through's denominator with a choice this arm never
# made. A reference is a RENDERING decision, not a retrieval outcome.
rule_ids.extend(rule.id for _score, rule in fresh)
# The stop, beside the lines rather than instead of them — see
# `checkpoint_for`. `kept` is passed, not `fresh`: whether a rule
# was named earlier this session says nothing about whether this
# act should wait for it to be READ, and those are the two axes
# #3750 exists to keep apart.
checkpoint = checkpoint_for(
kept, held=held, floor=cfg["checkpoint_threshold"], where="here",
)
# TWO tables, and the split is not arbitrary. retrieval_logs is one
# row per CALL, keyed on the score distribution a threshold is tuned
# from. rule_usage_events is one row per RULE per event, which is the
@@ -2312,6 +2506,7 @@ async def build_write_path_hint(
"derive": derive,
"derive_keys": [d["key"] for d in derive],
"rule_ids": rule_ids,
"checkpoint": checkpoint,
}
@@ -2351,7 +2546,14 @@ async def build_tool_rule_hint(
Fails open and returns an empty context on any error: a recall aid may
never break the operator's action.
"""
out: dict = {"context": "", "rule_ids": []}
# `checkpoint` is present on EVERY return, including the early ones. The
# two arms feed one shell reader, and a key that exists on some responses
# and not others is read there as an empty variable either way — so the
# difference is invisible at the point it would bite and only shows up in
# a test that asserts the contract. Same reason `warnings` is always a
# list in the telemetry readout: an absent key and an empty one must not
# be two ways of saying nothing.
out: dict = {"context": "", "rule_ids": [], "checkpoint": {}}
command = (command or "").strip()
if not command:
return out
@@ -2440,6 +2642,14 @@ async def build_tool_rule_hint(
)
out["context"] = "\n".join(lines)
out["rule_ids"] = rule_ids
# AFTER the lines, never instead of them. A checkpoint stops the act;
# it does not decide what the act should be told, and a reader who
# reads the rule and re-submits must find the same hint waiting. The
# two are independent renderings of one retrieval.
out["checkpoint"] = checkpoint_for(
kept, held=held, floor=cfg["checkpoint_threshold"],
where=f"this {tool_name} call",
)
except Exception:
logger.debug("pre-tool rule arm failed", exc_info=True)
return out
src/scribe/services/retrieval_telemetry.py
+130 -3
View File
@@ -28,6 +28,9 @@ from scribe.models import async_session
from scribe.models.base import iso
from scribe.models.note import Note
from scribe.models.note_usage import PULLED, SURFACED, NoteUsageEvent
from scribe.models.rule_usage import OUTCOMES as RULE_OUTCOMES
from scribe.models.rule_usage import APPLIED as RULE_APPLIED
from scribe.models.rule_usage import DEPARTED as RULE_DEPARTED
from scribe.models.rule_usage import PULLED as RULE_PULLED
from scribe.models.rule_usage import SURFACED as RULE_SURFACED
from scribe.models.rule_usage import RuleUsageEvent
@@ -37,6 +40,7 @@ from scribe.services.retrieval_registry import (
POINTS, UNBIDDEN, get_point, is_registered, sources_expected_to_emit,
)
from scribe.services.retrieval_surfaces import SURFACES, floor_for
from scribe.services.retrieval_tuning import floor_moves_since
from scribe.services.settings import get_setting
logger = logging.getLogger(__name__)
@@ -495,7 +499,8 @@ def _warn(code, detail, source=None, **numbers) -> dict:
def _compute_warnings(sources: dict, usage: dict, rule_usage: dict,
floors: dict, min_calls: int, epsilon: float) -> list[dict]:
floors: dict, min_calls: int, epsilon: float,
floor_moves: dict | None = None) -> list[dict]:
"""The four checks, over whatever sources the window actually contains.
DELIBERATELY NOT KEYED ON A HARD-CODED SOURCE LIST. An arm added next
@@ -578,9 +583,41 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict,
# invite tuning a dial that does not exist.
floor = floors.get(name)
p10 = (b.get("top_score") or {}).get("p10")
moved = (floor_moves or {}).get(name)
if calls >= min_calls and floor is not None and p10 is not None:
gap = p10 - floor
if gap < epsilon:
# ── The floor moved inside the window ────────────────────────
#
# THE CHECK IS SUSPENDED, NOT SOFTENED, and this is the whole of
# #4225. `band_hugs_floor` asks whether the scores are piled on
# the bar. That question needs the scores and the bar to come from
# the same regime; across a floor change they do not, and the
# comparison quietly becomes one between two populations.
#
# It announced itself when the change was a RAISE: p10 computed
# over calls made under the old, lower bar came out BELOW today's
# floor, and the warning reported a band "-0.0216 above" its
# floor. A negative distance above something is not a number
# anybody can act on. A LOWERED floor hides better — the gap comes
# out comfortably positive and reads as a clean bill of health on
# a sample that half predates the bar being judged.
#
# So the honest move is to say the sample cannot answer, and say
# when it will be able to, rather than print a figure with an
# asterisk. A reader who is told a number is unavailable goes and
# gets one; a reader handed a qualified number uses it.
if moved is not None:
out.append(_warn(
"floor_moved_mid_window",
f"this arm's floor changed at {moved}, inside the window, "
f"so its calls were made under two different bars and its "
f"band cannot be compared against the floor now in force "
f"({floor}). The band check is suspended for this arm "
f"until the window clears that date — ask again with a "
f"`days` that starts after it, or wait.",
source=name, floor=floor, p10=p10, moved_at=moved,
))
elif p10 - floor < epsilon:
gap = p10 - floor
out.append(_warn(
"band_hugs_floor",
f"the weakest tenth of what this arm returns scores "
@@ -636,6 +673,52 @@ def _compute_warnings(sources: dict, usage: dict, rule_usage: dict,
surfaced=int(shown), pulled=int(pulled or 0), never_pulled=never,
))
# ── A rule that was read and changed nothing (#4213, milestone 419) ──
#
# The failure this milestone was opened on, and the one number that could
# not previously be computed. `surfaced_never_pulled` above catches a rule
# nobody opens; this catches the worse case — a rule the agent DID open,
# deliberately, and then left no trace of having acted on. Until #4212
# those were arithmetically identical to compliance.
opened = int(rule_usage.get("distinct_rules_pulled") or 0)
acted = int(rule_usage.get("distinct_rules_acted") or 0)
applied = int(rule_usage.get("applied") or 0)
departed = int(rule_usage.get("departed") or 0)
if opened and not (applied or departed):
# THE HONEST ANSWER WHILE THE INSTRUMENT IS COLD, and the reason this
# is a separate code rather than a zero fed into the check below.
# Outcomes only started being recorded in milestone 419; a window
# containing none cannot tell "every rule was ignored" from "nothing
# reports outcomes yet". Emitting the ignored-rules warning here would
# manufacture a finding out of an unwired feature — #3311's mistake
# exactly, where a statistic that could not vary was read as a fact
# about the corpus.
out.append(_warn(
"outcomes_never_recorded",
f"{opened} distinct rules were opened in this window and not one "
f"recorded an outcome. This does NOT mean they were ignored — it "
f"means nothing is calling `rule_outcome`, so the difference "
f"between a rule that worked and a rule that was read and "
f"forgotten is still unmeasured here.",
source=None, opened=opened, applied=0, departed=0,
))
elif opened:
unacted = opened - acted
if unacted > 0:
out.append(_warn(
"read_and_unacted",
f"{unacted} of {opened} distinct rules were opened in this "
f"window and left no outcome, against {acted} that did. A rule "
f"read and silently unchanged looks exactly like one that "
f"worked; these are the ones where nobody can tell. Either the "
f"rule is mis-triggering — it arrives, gets read, and does not "
f"apply — or it is being ignored, and the two want opposite "
f"fixes.",
source=None, opened=opened, acted=acted, unacted=unacted,
applied=applied, departed=departed,
))
return out
@@ -1063,10 +1146,26 @@ async def retrieval_summary(
)
)
).scalar_one()
# Rules that were OPENED AND THEN ACTED ON (#4212). Its own
# distinct count for the same reason the two above have one:
# "how many rules did anything come of" cannot be summed from
# the per-source group without double-counting a rule that was
# applied once and departed from once.
distinct_rules_acted = (
await session.execute(
select(func.count(func.distinct(RuleUsageEvent.rule_id)))
.where(
RuleUsageEvent.created_at >= since,
RuleUsageEvent.user_id == user_id,
RuleUsageEvent.event.in_(RULE_OUTCOMES),
)
)
).scalar_one()
except Exception:
logger.warning("rule usage read failed", exc_info=True)
rule_rows = None
distinct_rules_surfaced = distinct_rules_pulled = 0
distinct_rules_acted = 0
except Exception:
logger.warning("retrieval summary read failed", exc_info=True)
out["read_failed"] = True
@@ -1175,6 +1274,13 @@ async def retrieval_summary(
"pulled": 0, "pulled_by_agent": 0, "pulled_by_human": 0,
"distinct_rules_surfaced": int(distinct_rules_surfaced or 0),
"distinct_rules_pulled": int(distinct_rules_pulled or 0),
# The outcome half (#4212, milestone 419). `pulled` says a rule was
# opened; these say whether anything came of it. Until this existed, a
# rule obeyed every time and a rule ignored every time produced
# identical rows, and the second is the one worth finding.
"applied": 0,
"departed": 0,
"distinct_rules_acted": int(distinct_rules_acted or 0),
}
if rule_rows is None:
# The FLAG is added, the shape is kept — matching `by_source_failed`
@@ -1205,6 +1311,15 @@ async def retrieval_summary(
rule_usage["pulled_by_agent"] += n
else:
rule_usage["pulled_by_human"] += n
elif event == RULE_APPLIED:
rule_usage["applied"] += n
elif event == RULE_DEPARTED:
# Kept apart from `applied` rather than summed into a single
# "acted" count. A departure is a rule someone ARGUED with,
# and an install where every outcome is a departure is telling
# you something quite different from one where none is —
# folding them together would hide exactly that.
rule_usage["departed"] += n
# None, not 0.0, when nothing was surfaced — matching the note block. A
# ratio of zero asserts "we showed rules and none were opened"; with an
@@ -1269,8 +1384,20 @@ async def retrieval_summary(
except Exception: # pragma: no cover - telemetry never raises
logger.warning("could not read floor for %s", name, exc_info=True)
# Floors that MOVED inside this window (#4225) — a release's shipped
# default or the operator's own dial, because the question is about the
# sample, not about who is answerable for it. Read unconditionally rather
# than under `if user_id`, unlike `floors` above: a release change belongs
# to no account, and it is exactly the case that used to go unrecorded.
floor_moves: dict[str, str] = {}
try:
floor_moves = await floor_moves_since(since)
except Exception: # pragma: no cover - telemetry never raises
logger.warning("could not read floor changes", exc_info=True)
out["warnings"] = _compute_warnings(
out["sources"], usage, rule_usage, floors, min_calls, epsilon,
floor_moves,
)
# "Active" means this window saw real traffic SOMEWHERE. Without that
src/scribe/services/retrieval_tuning.py
+147 -3
View File
@@ -43,7 +43,7 @@ from __future__ import annotations
import logging
from sqlalchemy import select
from sqlalchemy import or_, select
from scribe.models import async_session
from scribe.models.retrieval_tuning import RetrievalTuningEvent
@@ -61,6 +61,10 @@ logger = logging.getLogger(__name__)
DIALS = ("floor", "budget")
# The actor for a change nobody made by hand: the shipped default moved
# between releases. Beside "model" and "human" (#4225).
RELEASE_ACTOR = "release"
# Long enough to say what was read and what it showed; short enough that nobody
# pastes a telemetry dump in. The number is not a measurement — it is the point
# at which "0.66" stops being an acceptable answer to "why".
@@ -168,7 +172,21 @@ async def current_settings(user_id: int) -> list[dict]:
select(RetrievalTuningEvent)
.where(
RetrievalTuningEvent.surface == name,
RetrievalTuningEvent.user_id == user_id,
# THIS USER'S CHANGES *OR* A RELEASE'S (#4225).
# A release's rows carry no user id because no user
# made them, and taking the newest of the two is
# what makes the answer true: a dial the operator
# has tuned is explained by their change, and an
# untouched one by the release that last shipped
# its default. Filtering to the user alone reported
# "still on the shipped starting point" for a
# default that had in fact moved — the one state a
# reader would not think to check, which is the
# same sentence #4104 wrote about the bug above.
or_(
RetrievalTuningEvent.user_id == user_id,
RetrievalTuningEvent.user_id.is_(None),
),
RetrievalTuningEvent.dial == dial,
)
.order_by(RetrievalTuningEvent.created_at.desc())
@@ -282,6 +300,126 @@ async def set_dial(
}
async def record_release_defaults() -> list[dict]:
"""Write down that a RELEASE moved a shipped default (#4225).
THE HOLE THIS FILLS. `retrieval_tuning_events` records dial turns — a
person or a model choosing a number. It is silent about the other way a
floor moves, which is somebody editing `floor_default` in the registry and
shipping it. That change is invisible to every consumer of this table, and
one of those consumers is `band_hugs_floor`, which compares a percentile
against a floor and has no way to notice they come from different regimes.
Measured on the instance that found this: `write_path_rule` went 0.68 ->
0.72 on 2026-09-02 as a shipped default, and a 30-day window opening
2026-08-22 therefore held six days of calls made under the old bar. The
warning reported the band sitting "-0.0216 above" its floor — a negative
distance above something, which is what a two-population comparison looks
like when it finally says so out loud.
`user_id IS NULL`, because no user did this. A release acts on every
account that has not overridden the dial, and writing one row per user
would both multiply the row and misattribute it. Readers take the newest of
(this user's own change, the release's) — a dial the operator has tuned is
explained by their change, and an untouched one by the release.
`actor="release"`, a third value beside "model" and "human". The column is
Text with no CHECK precisely so a new kind of actor is not a migration —
the model's own comment says so, and this is the case it anticipated.
THE FIRST SIGHTING IS A BASELINE, NOT A CHANGE, and is written with
`old_value=None`. Nothing moved; the row exists so that the NEXT release
has something to be different from. That distinction is load-bearing
downstream: a warning suspends itself on a genuine move and must not
suspend itself on a fresh install's baseline, and it tells the two apart by
exactly this null.
Idempotent, and safe to call on every boot: a default that matches the last
recorded one writes nothing.
"""
written: list[dict] = []
stamp = calibration_stamp()
async with async_session() as session:
for name in surface_names():
s = get_surface(name)
for dial, shipped in (
("floor", float(s.floor_default)),
("budget", float(s.budget_default)),
):
last = (
await session.execute(
select(RetrievalTuningEvent)
.where(
RetrievalTuningEvent.surface == name,
RetrievalTuningEvent.user_id.is_(None),
RetrievalTuningEvent.actor == RELEASE_ACTOR,
RetrievalTuningEvent.dial == dial,
)
.order_by(RetrievalTuningEvent.created_at.desc())
.limit(1)
)
).scalars().first()
if last is not None and abs(float(last.new_value) - shipped) < 1e-9:
continue
old = None if last is None else float(last.new_value)
reason = (
f"Baseline: this release ships {name}'s {dial} at {shipped}. "
f"Recorded so a later change to the shipped value has a "
f"date and a predecessor to be measured against; nothing "
f"moved here."
if old is None else
f"A release moved {name}'s shipped {dial} from {old} to "
f"{shipped}. Not a dial turn — the default in the registry "
f"changed, so calls logged either side of this date were "
f"made under different bars and cannot be pooled."
)
session.add(RetrievalTuningEvent(
user_id=None, surface=name, dial=dial,
old_value=old, new_value=shipped,
actor=RELEASE_ACTOR, reason=reason,
embedding_model=stamp["embedding_model"],
shape_version=stamp["shape_version"],
))
written.append({
"surface": name, "dial": dial,
"old_value": old, "new_value": shipped,
"baseline": old is None,
})
if written:
await session.commit()
return written
async def floor_moves_since(since) -> dict[str, str]:
"""Surfaces whose floor genuinely MOVED at or after `since`.
Genuinely: `old_value IS NOT NULL`, so a baseline row — which records a
default without changing it — does not read as a change. Returns surface ->
ISO timestamp of the newest such move, for a reader deciding whether a
window's numbers can be pooled.
Covers both kinds of move, a release's and this user's, because the
question is about the SAMPLE rather than about who is responsible for it:
a floor that moved mid-window splits the calls either way round.
"""
out: dict[str, str] = {}
async with async_session() as session:
rows = (
await session.execute(
select(RetrievalTuningEvent)
.where(
RetrievalTuningEvent.dial == "floor",
RetrievalTuningEvent.old_value.isnot(None),
RetrievalTuningEvent.created_at >= since,
)
.order_by(RetrievalTuningEvent.created_at.desc())
)
).scalars().all()
for row in rows:
out.setdefault(row.surface, row.created_at.isoformat())
return out
async def tuning_history(
user_id: int, *, surface: str | None = None, limit: int = 20
) -> list[dict]:
@@ -296,7 +434,13 @@ async def tuning_history(
async with async_session() as session:
q = (
select(RetrievalTuningEvent)
.where(RetrievalTuningEvent.user_id == user_id)
.where(or_(
RetrievalTuningEvent.user_id == user_id,
# Shipped-default changes, which belong to no account and
# would otherwise be missing from the one surface built
# for asking why a number is where it is (#4225).
RetrievalTuningEvent.user_id.is_(None),
))
.order_by(RetrievalTuningEvent.created_at.desc())
.limit(max(1, min(int(limit), 200)))
)
src/scribe/services/rule_usage.py
+132 -2
View File
@@ -85,7 +85,9 @@ from sqlalchemy import case, func, select
from scribe.models import async_session
from scribe.models.base import iso
from scribe.models.rule_usage import PULLED, SURFACED, RuleUsageEvent
from scribe.models.rule_usage import (
APPLIED, DEPARTED, OUTCOMES, PULLED, SURFACED, RuleUsageEvent,
)
from scribe.services.background import report_telemetry_failure, spawn
logger = logging.getLogger(__name__)
@@ -207,6 +209,107 @@ def record_rule_pulled(*, user_id: int | None, rule_id: int, source: str) -> Non
_schedule(rows)
def record_rule_outcome(
*,
user_id: int | None,
rule_id: int,
outcome: str,
source: str,
detail: str = "",
) -> None:
"""Fire-and-forget: record what a rule ACTUALLY CHANGED (#4212).
The third stream, and the one milestone 419 exists for. `surfaced` says
the system offered a rule; `pulled` says somebody opened it. Neither says
whether it made any difference, so a rule that fires constantly and is
always obeyed and a rule that fires constantly and is never obeyed have,
until now, produced identical telemetry. The second is far the more
urgent and is precisely the one the readout could not name.
Two outcomes, because there are only two a judge can honestly report:
APPLIED — the rule changed what was done, or confirmed it. No `detail`
required: following a rule is the unremarkable case and
charging prose for it is how an event stops being recorded.
DEPARTED — read, and deliberately not followed. `detail` is REQUIRED
and is the entire value of the event. A departure without
its reason reads back as a miss, which collapses the two
states this exists to separate.
THERE IS NO THIRD CALL, and the absence is the design. Read-and-silently-
unchanged is real — it is the failure this milestone was opened on — but
it cannot be reported, because an agent that knew it was ignoring a rule
would not be ignoring it. It is derived: a rule pulled, with no outcome
behind it. See `outcome_state`.
Guarded rather than trusting: a bad outcome or a reasonless departure is
dropped and REPORTED, never written. Telemetry that lies is worse than
telemetry that is missing (#2663), and a `departed` row with an empty
reason is a lie the readout cannot detect.
"""
if outcome not in OUTCOMES:
logger.warning("rule outcome rejected: unknown outcome %r", outcome)
spawn(_report_failure("outcome_unknown"), site="rule_usage_outcome")
return
if outcome == DEPARTED and not (detail or "").strip():
logger.warning("rule outcome rejected: departure with no reason")
spawn(_report_failure("outcome_no_reason"), site="rule_usage_outcome")
return
try:
rows = [
{
"user_id": user_id,
"rule_id": int(rule_id),
"event": outcome,
"source": source,
"detail": (detail or "").strip() or None,
}
]
except Exception:
logger.debug("rule usage payload build failed", exc_info=True)
return
_schedule(rows)
# What a rule's usage says happened to it, in one word. The four states are
# ordered by how much the system actually knows, and only the last two are
# new — the point of the milestone is that UNACTED used to be invisible
# inside APPLIED.
UNREAD = "unread" # surfaced, never opened
UNACTED = "unacted" # opened, and nothing recorded after — the blind spot
FOLLOWED = "followed" # opened and applied
DEPARTED_FROM = "departed" # opened and deliberately not followed, with a why
def outcome_state(usage: dict) -> str:
"""The three states milestone 419 asked to be able to tell apart, plus
the one that already existed.
Pure, and reading only the aggregate `usage_for_rules` already returns —
so the readout, the badge and any later session summary all answer this
question the same way. Two callers computing "was this followed" from raw
counts is the drift #3246 found across the rules system, arriving again.
PRECEDENCE, and it is deliberate: a departure outranks an application.
A rule both applied and departed from in the same window is a rule
someone argued with, and the argument is the interesting half — reporting
it as plain compliance would hide the one row a reader most wants.
UNACTED is the derived state and the reason this function exists. It is
not "no data"; it is a rule that was surfaced, deliberately OPENED, and
then left no trace of having mattered. That is a much stronger signal
than never having been opened at all, and it is the signal that was
previously indistinguishable from compliance.
"""
if int(usage.get("departed_count") or 0):
return DEPARTED_FROM
if int(usage.get("applied_count") or 0):
return FOLLOWED
if int(usage.get("pull_count") or 0):
return UNACTED
return UNREAD
def empty_rule_usage() -> dict:
"""The zero readout — what a rule with no recorded events looks like.
@@ -227,6 +330,21 @@ def empty_rule_usage() -> dict:
"surfaced_count": 0,
"ambient_count": 0,
"pull_count": 0,
# The outcome half (#4212). Zero here means "nothing recorded", which
# for a rule that was also never pulled is simply silence — and for
# one that WAS pulled is the blind spot this milestone is named for.
# `outcome_state` is what tells those apart; no caller should be
# reading these counts raw to decide it.
#
# The REASON for a departure is on the row (`detail`), not here. One
# GROUP BY cannot carry the text of the latest departure without a
# DISTINCT ON alongside it, and a key that the aggregate could never
# fill would read as "no reason given" on every rule that has one —
# a permanently-null field that lies. The readout that needs the
# prose reads the rows (#4213).
"applied_count": 0,
"departed_count": 0,
"last_outcome_at": None,
"last_surfaced_at": None,
"last_pulled_at": None,
}
@@ -288,7 +406,19 @@ async def usage_for_rules(rule_ids: list[int]) -> dict[int, dict]:
slot = out.get(int(rule_id))
if slot is None:
continue
if event == SURFACED and is_amb:
# Outcomes first, and never split by ambient. `ambient` asks whether
# a RANKER chose to show the rule; an outcome is reported by a judge
# after the fact and has no ranker behind it, so the flag is noise
# here. Branching on it would silently drop every outcome row into a
# bucket nothing reads.
if event in OUTCOMES:
key = "applied_count" if event == APPLIED else "departed_count"
slot[key] = slot[key] + int(n)
prev = slot["last_outcome_at"]
now = iso(last_at)
if now and (prev is None or now > prev):
slot["last_outcome_at"] = now
elif event == SURFACED and is_amb:
slot["ambient_count"] = int(n)
elif event == SURFACED:
slot["surfaced_count"] = int(n)
src/scribe/services/shape_ledger.py
+259 -34
View File
@@ -918,6 +918,12 @@ FORM_UNKNOWN = ""
# a canon poisoned by loose stamping falls silent instead of flagging.
_FORM_SHARE = 0.6
# How much of a canon's membership may be shapes it cannot account for
# before the canon stops meaning anything. Not a majority test — see
# `canon_coherence` for why a majority test goes blind exactly when the
# ledger is worst.
_STRANGER_SHARE = 0.2
# Leading words that say nothing about the form of the thing being declared.
_FORM_NOISE = ("export ", "default ", "public ", "private ", "static ", "final ")
@@ -1069,6 +1075,124 @@ def canon_form(rows: Iterable, snippet_id: int) -> str:
return top if n / sum(forms.values()) >= _FORM_SHARE else FORM_UNKNOWN
# A judgment nobody read before it was written. "hook" is the write-path
# stamp — a similarity score and no reader. Every other value ("agent",
# "audit", "import", "mechanical") came from something that examined the
# shape and said so, which is a different kind of evidence, not a stronger
# score.
_UNATTENDED_BY = ("hook",)
def canon_coherence(members: Iterable) -> dict:
"""Does a canon's membership still agree on what the canon IS?
The verdict is NOT "do all the rows share a form". That was the first
version, and on this surface's first live day both canons it reported
were sound (#4220) while the one genuinely polluted canon had already
been cleaned by hand. Two things were wrong with it, and the fix for the
first nearly broke the second:
1. FAMILY, NOT FORM. A sync loop-starter and the async tick it schedules
are one shape written two ways; `shape_family` already collapses `fn`
and `async-fn` into `callable` for exactly this reason on the
divergence side, and this side never asked. Snippet #2849 — four
starters, three ticks, every row judged by an audit — scored 4/7 and
was called incoherent when nothing about it is.
2. A METHOD OF A MEMBER IS NOT A STRANGER. Snippet #2844 is the
SQLAlchemy model convention and its own text covers the class AND the
`to_dict` the class must carry. Its 37 classes and 25 serialisers
scored 37/62 = 0.597, missing the bar by three thousandths for
containing exactly what it says it contains.
THE TRAP, and it is why this is not simply a family histogram: grouping
by family and keeping a majority test makes the check BLIND. Before #2844
was cleaned it held 37 classes and 56 callables; as families that is
56/93 = 0.602, a clean pass, and the 31 rows that had no business being
there — Vue functions, route handlers, a dozen tests — would never have
been reported. A looser bar in the same shape is not a fix.
So the verdict is inverted. Instead of asking whether most rows agree, it
asks how many rows the canon CANNOT ACCOUNT FOR:
* a row in the majority family is accounted for;
* a callable defined in a FILE that also holds a majority-family `type`
row is accounted for — it is a method of a member;
* everything else is a STRANGER, and strangers above `_STRANGER_SHARE`
of the readable rows make the canon incoherent.
The majority vote abstains those methods, so a class's own serialisers
cannot outvote the classes and turn the members into the strangers. On
the real ledger the three populations separate completely: clean #2844
has 0 strangers in 62, #2849 has 0 in 7, and polluted #2844 had 31 in 93.
Scoped to the REVIEW surface on purpose. `canon_form` still answers at
the precise form level for stamping and divergence, where a sync helper
beside an async canon is a fair question; nothing here changes what the
ledger writes.
Returns a dict, deliberately. Widening a tuple return is the #4204 break
exactly — a 4-tuple grew a fifth field and one consumer went on unpacking
four, and Python said nothing until that line ran.
"""
forms: dict[str, int] = {}
families: dict[str, int] = {}
per_row: list[tuple[object, str]] = []
has_type: set[str] = set()
for r in members:
f = shape_form(getattr(r, "signature", "") or "", r.kind)
if not f:
continue # unreadable: never counts either way
fam = shape_family(f)
forms[f] = forms.get(f, 0) + 1
families[fam] = families.get(fam, 0) + 1
per_row.append((r, fam))
if fam == "type":
has_type.add(r.path)
out = {
"readable": len(per_row),
"forms": dict(sorted(forms.items(), key=lambda kv: -kv[1])),
"families": dict(sorted(families.items(), key=lambda kv: -kv[1])),
"family": FORM_UNKNOWN,
"attached": 0,
"strangers": [],
"coheres": True,
}
if not per_row:
return out # nothing legible: say nothing, not "broken"
# The vote, with methods abstaining. A callable sitting in a file that
# defines a class is presumed to belong to it and does not get to argue
# that the canon is really about callables.
def _attachable(row, fam: str) -> bool:
return fam == "callable" and row.path in has_type
votes: dict[str, int] = {}
for r, fam in per_row:
if _attachable(r, fam):
continue
votes[fam] = votes.get(fam, 0) + 1
if not votes: # every readable row is a method
votes = dict(families)
# Ties go to `type`: a canon that defines a class is about the class.
family = max(votes, key=lambda k: (votes[k], k == "type"))
attached, strangers = 0, []
for r, fam in per_row:
if fam == family:
continue
if family == "type" and _attachable(r, fam):
attached += 1
else:
strangers.append(r)
out["family"] = family
out["attached"] = attached
out["strangers"] = strangers
out["coheres"] = len(strangers) / len(per_row) <= _STRANGER_SHARE
return out
def signature_in(code: str, symbol: str, kind: str) -> str:
"""The line in ``code`` that DEFINES ``symbol``, or "" if none does.
@@ -2011,6 +2135,35 @@ _DENSITY_MIN_JUDGED = 3
_DENSITY_SHARE = 0.6
def comparable_siblings(rows: Iterable, form: str) -> list:
"""The siblings a candidate of ``form`` can honestly be counted against.
Excludes only a KNOWN family contradiction, via `families_conflict` — the
same predicate the divergence gate uses, so the denominator and the gate
cannot drift into disagreeing about what "comparable" means.
`form` is a FORM (`fn`, `async-fn`, `type`, …), never a family:
`families_conflict` coarsens both sides itself, and handing it a family
makes `shape_family("callable")` return "" so nothing is excluded — a
narrowing that silently becomes a no-op while still reading as applied.
AN UNREADABLE SIBLING STAYS COUNTED, and that direction is the point.
Dropping it would shrink `judged`, raise the dominant canon's share, and
make the check fire MORE on the directories it can read least. Every
unknown-form decision in this module goes the same way: quieter, never
more confident.
"""
if not form:
return list(rows)
return [
r for r in rows
if not families_conflict(
shape_form(getattr(r, "signature", "") or "", getattr(r, "kind", "sym")),
form,
)
]
def dominant_canon(rows: Iterable[CodeShape]) -> tuple[int, int, int] | None:
"""(snippet_id, its_count, judged_count) when one canon dominates these
sibling rows (same directory + kind), else None."""
@@ -2032,9 +2185,37 @@ def _dir_of(path: str) -> str:
return path.rsplit("/", 1)[0] if "/" in path else ""
async def canon_density(project_id: int, path: str, kind: str) -> tuple[int, int, int] | None:
async def canon_density(
project_id: int, path: str, kind: str, form: str = ""
) -> tuple[int, int, int, str] | None:
"""The dominant canon for the directory ``path`` sits in, for ``kind`` —
the write-time question "is this a canon-dense place?"."""
the write-time question "is this a canon-dense place?".
``form`` is the candidate's own FORM — `fn`, `async-fn`, `type`, … as
`shape_form` returns it, NOT a family. `families_conflict` coarsens both
sides itself, and handing it a family makes `shape_family("callable")`
return "" so nothing is ever excluded: the narrowing silently becomes a
no-op that still reads as applied. It narrows the DENOMINATOR
(#4208). Without it the base rate was computed over every code symbol in
the directory as one bucket: "372 judged siblings" counted a dataclass, a
CSS-less constant and an async service unit as three comparable things,
and the share that came out of that was a statement about a population
nobody had asked a question about.
EXCLUDES ONLY A KNOWN CONFLICT, using `families_conflict` — the same
predicate the divergence gate uses, so the two cannot drift apart. A
sibling whose form is unreadable STAYS COUNTED. That direction is
deliberate and it is the one that matters: dropping unknown rows would
shrink `judged`, raise the share, and make the check fire MORE on exactly
the directories it can read least. Every other unknown-form decision in
this module goes the same way — quieter, never more confident.
NOT narrowed to the candidate's exact form, for the reason `shape_family`
gives at length: `fn` beside `async-fn` is the acceptance case of
milestone #2793, not noise. Bucketing the denominator by form would take
the async canon out of a sync candidate's count and silence that flag —
the same inversion the first form gate made, one layer down.
"""
directory = _dir_of(path)
async with async_session() as session:
rows = (
@@ -2049,6 +2230,7 @@ async def canon_density(project_id: int, path: str, kind: str) -> tuple[int, int
)
).scalars().all()
siblings = [r for r in rows if _dir_of(r.path) == directory]
siblings = comparable_siblings(siblings, form)
dom = dominant_canon(siblings)
if dom is None:
return None
@@ -2071,10 +2253,28 @@ async def write_time_divergence(
edit. Returns [{symbol, kind, canon_snippet_id, instances, judged}]."""
just_stamped = {(s["symbol"], s["kind"]): s["snippet_id"] for s in stamped}
out: list[dict] = []
kinds = {k for k, _n in shapes}
density = {k: await canon_density(project_id, path, k) for k in kinds}
if not any(density.values()):
if not shapes:
return out
# DENSITY IS NOW PER CANDIDATE, not per kind (#4208): the denominator
# excludes siblings whose family contradicts what is being written, so it
# cannot be computed until the candidate's own form is known. Cached on
# (kind, family) — a write names a handful of shapes and they collapse to
# one or two buckets, so this is the same one-or-two queries as before.
#
# The cost is that the row load below no longer sits behind an early exit
# on "nothing is dense here". That is one indexed lookup on
# (project_id, path), and it has to happen first regardless: the
# candidate's signature comes from its stored row when the payload does
# not carry one.
_density: dict[tuple[str, str], tuple[int, int, int, str] | None] = {}
async def density_for(kind: str, form: str):
# Keyed and passed as a FORM, not a family — see `canon_density`.
key = (kind, form)
if key not in _density:
_density[key] = await canon_density(project_id, path, kind, form)
return _density[key]
async with async_session() as session:
rows = (
await session.execute(
@@ -2087,13 +2287,19 @@ async def write_time_divergence(
).scalars().all()
by_key = {(r.symbol, r.kind): r for r in rows}
for kind, name in shapes:
dom = density.get(kind)
row = by_key.get((name, kind))
# The candidate's own form, read before anything is counted. Signature
# from the payload first — a shape being written now may have no row
# yet — falling back to the stored row's.
mine = shape_form(
signature_in(code, name, kind) or getattr(row, "signature", "") or "", kind
)
dom = await density_for(kind, mine)
if not dom:
continue
sid, n, judged, cform = dom
if just_stamped.get((name, kind)) == sid:
continue
row = by_key.get((name, kind))
if row is not None and (
row.status != "unclassified" or row.proposed_snippet_id == sid
):
@@ -2107,12 +2313,10 @@ async def write_time_divergence(
# divergence prompt is ABOUT a mismatch, so requiring the candidate to
# match would silence the check precisely where it belongs.
#
# Signature from the payload first — a shape being written now may
# have no row yet — and an unreadable one produces a fair question
# rather than a guess, because `families_conflict` needs both sides.
mine = shape_form(
signature_in(code, name, kind) or getattr(row, "signature", "") or "", kind
)
# `mine` was read above, before the denominator was counted — the
# same value serves both, and they must not be able to disagree.
# An unreadable signature produces a fair question rather than a
# guess, because `families_conflict` needs both sides.
if families_conflict(mine, cform):
continue
out.append({"symbol": name, "kind": kind, "canon_snippet_id": sid,
@@ -2270,12 +2474,26 @@ def stamps_to_review(rows: Iterable[CodeShape], *, top: int = 10) -> dict:
person or an audit judged are never listed, however old: a human judgment
is not weak evidence, it is a different kind of evidence.
`incoherent` — canons whose judged rows do not agree on a form. A canon is
a claim that some set of shapes are the same sort of thing; when its own
members are a class, three getters and a dozen tests, that claim has
stopped being true, and every base-rate reading built on it is reading
noise. `canon_form` already makes such a canon fall silent — this is what
makes it VISIBLE, which is the half that was missing.
`incoherent` — canons whose membership no longer agrees what the canon
IS. A canon is a claim that some set of shapes are the same sort of
thing; when its own members are a class, three getters and a dozen tests,
that claim has stopped being true, and every base-rate reading built on
it is reading noise. `canon_form` already makes such a canon fall silent
— this is what makes it VISIBLE, which is the half that was missing.
What counts as disagreement is `canon_coherence`, and it is deliberately
looser than "one form": it compares FAMILIES, and it does not hold a
method against the class it is defined beside. Both corrections came from
this surface's first live day, when the only two canons it reported were
both sound (#4220). A review surface whose output is noise is one that
stops being read, which costs more than the check was ever worth.
`strangers` names the rows that do not fit, not the first dozen members:
the reader's question is which ones are wrong. `unattended` counts rows
written by the hook with nobody reading — an incoherence made entirely of
judged rows is far more likely to be this check being too strict than a
ledger full of junk, and the reader should be able to see that without
opening the canon.
"""
live = [r for r in rows if r.vanished_at is None]
weak = []
@@ -2300,32 +2518,39 @@ def stamps_to_review(rows: Iterable[CodeShape], *, top: int = 10) -> dict:
by_canon.setdefault(int(r.snippet_id), []).append(r)
incoherent = []
for sid, members in by_canon.items():
forms: dict[str, int] = {}
for r in members:
f = shape_form(r.signature or "", r.kind)
if f:
forms[f] = forms.get(f, 0) + 1
readable = sum(forms.values())
if readable < _DENSITY_MIN_JUDGED:
coh = canon_coherence(members)
if coh["readable"] < _DENSITY_MIN_JUDGED:
continue # too few to say anything either way
if canon_form(members, sid):
continue # a form holds the majority: coherent
if coh["coheres"]:
continue
unattended = sum(1 for r in members if r.classified_by in _UNATTENDED_BY)
incoherent.append({
"snippet_id": sid,
"judged": len(members),
"forms": dict(sorted(forms.items(), key=lambda kv: -kv[1])),
"forms": coh["forms"],
"families": coh["families"],
"family": coh["family"],
"attached": coh["attached"],
"unattended": unattended,
# The listing below is capped; without this you cannot tell a
# canon with twelve strangers from one with three hundred.
"stranger_count": len(coh["strangers"]),
"weak_rows": sum(
1 for r in members
if r.classified_by == "hook"
if r.classified_by in _UNATTENDED_BY
and (stamp_score(r.reason) or 1.0) < _RESEMBLE_MIN
),
"sample": [
# The rows that do NOT fit — not the first dozen members. The
# reader's question is "which ones are wrong", and a sample of
# the majority cannot answer it.
"strangers": [
{"path": r.path, "symbol": r.symbol,
"signature": r.signature or "", "by": r.classified_by}
for r in members[:_REVIEW_ROWS_SHOWN]
"signature": r.signature or "", "by": r.classified_by,
"form": shape_form(r.signature or "", r.kind)}
for r in coh["strangers"][:_REVIEW_ROWS_SHOWN]
],
})
incoherent.sort(key=lambda d: (-d["weak_rows"], -d["judged"]))
incoherent.sort(key=lambda d: (-d["weak_rows"], -d["unattended"], -d["judged"]))
return {
"weak_count": len(weak),
"weak": weak[:top],
tests/helpers.py
+12
View File
@@ -374,7 +374,18 @@ def writepath_cfg(**over):
So the keys come from `retrieval_surfaces.SURFACES`. A seventh surface, or a
rename, changes this helper for free and cannot quietly disable an arm in
ten hand-written dicts that each looked complete on the day they were typed.
NOT EVERY KEY IS A SURFACE, and that gap already bit once (#4214). The
checkpoint bar has no entry in SURFACES on purpose — everything in that
table is a floor/budget pair belonging to one QUERY, and the checkpoint
runs none, it re-reads hits the rule arms already produced. Adding it there
would give it a phantom budget. So it is taken from the module constant
instead, which keeps the "one literal, in the product" property even though
the derivation differs. `test_the_config_stand_in_carries_every_key_the_
real_one_does` is what makes the next addition fail loudly here rather than
silently no-op an arm, which is the whole claim this docstring makes.
"""
from scribe.services.plugin_context import _CHECKPOINT_DEFAULT
from scribe.services.retrieval_surfaces import SURFACES
cfg = {
@@ -385,6 +396,7 @@ def writepath_cfg(**over):
"rule_top_k": SURFACES["write_path_rule"].budget_default,
"tool_rule_threshold": SURFACES["pre_tool_rule"].floor_default,
"tool_rule_top_k": SURFACES["pre_tool_rule"].budget_default,
"checkpoint_threshold": _CHECKPOINT_DEFAULT,
}
cfg.update(over)
return cfg
tests/test_contract_around_the_change.py
+253
View File
@@ -0,0 +1,253 @@
"""You altered the shape of something — here is everything that reads it (#4215).
WHY THIS EXISTS
Milestone 419's most repeated miss, five of seven: acting on the thing in hand
without reading the contract around it. Lesson #4207 says it in words —
"widening a tuple is an interface change to every unpack site, and the
compiler will not tell you" — and was written by its author HOURS before a
structurally identical mistake, having been surfaced twice in the turns
between. Text delivered at the moment of acting is too weak a carrier for a
reflex that has to change what the act IS. This looks it up instead.
Rule 33 one scope down: its checks are between layers ("every parameter the
caller sends is read by the handler under the same name"), and the same
question exists between a definition and its callers.
THREE KINDS OF EXPOSED NAME, because a contract breaks three ways that look
nothing alike in source:
sym what is defined a rename or a removal
arg its parameter names arity and order
key quoted keys of dict literals the shape of what it RETURNS
The third is here because of the miss that produced this step, and it is the
one a signature-watcher would have missed: `get_writepath_config` gained one
dict key, three arms read that dict inside a fail-open `except`, every one
silently became a no-op, and ten tests went red at once with nothing pointing
at the cause. No signature changed.
WHAT IS PINNED: which edits speak and which stay silent, and that both gates
are load-bearing. NOT pinned: the wording, which is prose.
A FIXTURE REPO, NEVER THIS ONE. Asserting against Scribe's own files would
make the test a description of today's tree, failing the next time someone
renames something (rule 115's reasoning, one floor down).
"""
import shutil
import subprocess
from pathlib import Path
import pytest
DEFS = Path(__file__).resolve().parents[1] / "plugin" / "hooks" / "scribe_defs.sh"
HOOK = Path(__file__).resolve().parents[1] / "plugin" / "hooks" / "scribe_prior_art.sh"
def _need(*tools):
for t in tools:
if shutil.which(t) is None:
pytest.skip(f"hook runtime tool {t!r} not installed")
def run(script: str) -> str:
_need("bash", "awk", "git", "grep", "sed")
r = subprocess.run(
["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'],
capture_output=True, text=True,
)
assert r.returncode == 0, f"exit {r.returncode}: {r.stderr}"
return r.stdout
@pytest.fixture()
def repo(tmp_path):
"""A small git repo: a definition with a reader, and one without."""
_need("git")
# `other` lives in lib.py BESIDE widget, and nothing references it. That
# placement is the point of the no-readers case: putting it in its own
# file would leave that file as its reader, since only the file being
# edited is excluded — which is how the first version of this fixture
# quietly tested the opposite of what it claimed.
(tmp_path / "lib.py").write_text(
"def widget(size, colour):\n"
' return {"size": size, "colour": colour}\n\n'
"def other():\n return 1\n"
)
(tmp_path / "caller.py").write_text(
"from lib import widget\n\n"
"def render():\n"
' return widget(1, "red")["size"]\n'
)
(tmp_path / "unrelated.py").write_text("def gizmo():\n return 2\n")
subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True)
subprocess.run(["git", "add", "-A"], cwd=tmp_path, check=True)
return tmp_path
def q(text: str) -> str:
"""A multi-line blob as one bash word, newlines intact.
ANSI-C quoting (`$'...'`) and NOT Python's `repr`, which is what the first
version of this file used. Inside ordinary single quotes bash leaves `\\n`
as two characters and `printf %s` does not interpret it either, so every
fixture arrived as a SINGLE line. The extractor is line-oriented, so the
cases still passed — the test would have gone green while exercising input
no editor could produce. Escaping is explicit rather than delegated to
`repr`, whose quote character depends on the content.
"""
esc = (text.replace("\\", "\\\\").replace("'", "\\'")
.replace("\n", "\\n").replace("\t", "\\t"))
return "$'" + esc + "'"
def block(repo, subject, old, new, ledger="") -> str:
return run(
f'scribe_contract_block "{repo}" "lib.py" "{subject}" '
f'{q(old)} {q(new)} "{ledger}"'
)
# ── scribe_exposed: the three kinds ───────────────────────────────────────
def exposed(text: str) -> set[str]:
out = run(f'printf %s {q(text)} | scribe_exposed')
return {ln for ln in out.split("\n") if ln.strip()}
def test_a_definitions_name_is_exposed():
assert "sym\twidget" in exposed("def widget(size):\n pass\n")
def test_parameter_names_are_exposed():
got = exposed("def widget(size, colour):\n pass\n")
assert "arg\tsize" in got and "arg\tcolour" in got
def test_self_and_cls_are_not_part_of_the_contract():
"""Nothing a caller passes, so naming them would report a change on every
method that gains a keyword argument."""
got = exposed("def method(self, size):\n pass\n")
assert "arg\tself" not in got
assert "arg\tsize" in got
def test_a_type_annotation_and_a_default_are_stripped_from_a_parameter():
got = exposed('def f(user_id: int, days: int = 30, *, flag=False):\n pass\n')
assert {"arg\tuser_id", "arg\tdays", "arg\tflag"} <= got
def test_a_quoted_dict_key_is_exposed():
"""THE CASE A SIGNATURE-WATCHER MISSES, and the one that produced this
step — see the module docstring."""
assert 'key\tcheckpoint_threshold' in exposed(' "checkpoint_threshold": floor,\n')
def test_reading_a_dict_key_does_not_count_as_defining_one():
"""Anchored on the writing position (`"k":`), so `cfg["k"]` says nothing.
Without this every consumer of a config would report a contract change
the moment it read one."""
assert exposed('x = cfg["tool_rule_threshold"]\ny = d.get("other")\n') == set()
def test_a_dunder_is_never_exposed():
"""Every class defines __init__, so it would be a guaranteed false
positive on every class edit — and noise is what teaches a reader to skip
the block."""
assert "sym\t__init__" not in exposed(" def __init__(self, x):\n pass\n")
# ── The check: what speaks, and what stays quiet ──────────────────────────
def test_a_gained_dict_key_names_the_files_that_read_it(repo):
out = block(
repo, "widget",
'def widget(size, colour):\n return {"size": size}\n',
'def widget(size, colour):\n return {"size": size, "weight": 1}\n',
)
assert "widget" in out
assert "gained" in out and "weight" in out
assert "caller.py" in out
assert "unrelated.py" not in out, "only files that reference the subject"
def test_a_gained_parameter_names_the_files_that_read_it(repo):
out = block(repo, "widget", "def widget(size):\n", "def widget(size, colour):\n")
assert "colour" in out and "caller.py" in out
def test_a_rename_reports_both_halves(repo):
out = block(repo, "widget", "def widget(size):\n", "def gadget(size):\n")
assert "lost" in out and "widget" in out
assert "gained" in out and "gadget" in out
def test_a_body_only_edit_says_nothing(repo):
"""THE GATE THAT DECIDES WHETHER THIS IS USABLE. Most edits are bodies; a
check that spoke on all of them would be skipped by the third turn."""
out = block(
repo, "widget",
"def widget(size, colour):\n total = size\n return total\n",
"def widget(size, colour):\n total = size + 1\n return total\n",
)
assert out.strip() == ""
def test_a_definition_nothing_references_says_nothing(repo):
"""THE SECOND GATE, and it is not redundant. A shape change is necessary
but not sufficient — a definition with no readers has no contract to
break, and on any real repo this is what keeps the check quiet."""
out = block(repo, "other", "def other():\n", "def other(x):\n")
assert out.strip() == ""
def test_the_same_subject_is_named_once_per_session(repo, tmp_path):
"""Getting a change right takes several passes over the same definition,
and re-asking the same contract question at each one punishes exactly the
rhythm that gets it right."""
ledger = tmp_path / "sid.contract.ids"
first = block(repo, "widget", "def widget(size):\n",
"def widget(size, colour):\n", ledger=str(ledger))
assert first.strip()
second = block(repo, "widget", "def widget(size):\n",
"def widget(size, colour):\n", ledger=str(ledger))
assert second.strip() == ""
def test_a_missing_old_text_says_nothing(repo):
"""A Write that creates a file has no `before`, so there is no contract to
have changed. Silence, never a report against an empty string."""
assert block(repo, "widget", "", "def widget(size, colour):\n").strip() == ""
# ── The hook that carries it ──────────────────────────────────────────────
def test_the_write_hook_still_cannot_stop_a_write():
"""THE RECORDED DECISION THIS ARM MUST NOT ERODE. The operator's position
is that a recall aid may not stand in the way of a write, and this arm is
a nudge like the others — it names files, it does not gate. Asserted here
as well as in test_write_path_trigger.py because this is the change most
likely to tempt someone into making it a gate: it reports something that
may already be broken.
"""
code = [ln for ln in HOOK.read_text().splitlines()
if not ln.lstrip().startswith("#")]
assert not any("permissionDecision" in ln for ln in code)
assert not any("scribe_json_deny" in ln for ln in code)
def test_the_hook_asks_the_contract_question_first():
"""Order is the strength of the claim: this arm says something may already
be BROKEN, the local arm says a copy exists, the recorded arms say
something resembles this. A reader who reads one line should read that
one."""
code = [ln for ln in HOOK.read_text().splitlines()
if not ln.lstrip().startswith("#")]
combined = next(i for i, ln in enumerate(code) if ln.startswith("combined="))
assert "contract_context" in code[combined]
def test_the_hook_is_still_shell_valid():
_need("bash")
subprocess.run(["bash", "-n", str(HOOK)], check=True)
subprocess.run(["bash", "-n", str(DEFS)], check=True)
tests/test_hook_duplicate_confirmation.py
+224
View File
@@ -0,0 +1,224 @@
"""The by-name duplicate arm confirms its grep hits against the real extractor (#4227).
#4222 fixed one end of this defect: the extractors that decide what a payload
DEFINES now blank comment and string spans before any line matcher runs. This
is the other end — `scribe_local_dups`, which decides which OTHER files already
define that name — and it was a plain `git grep`.
A grep sees lines, not spans. The line
class with only modifier rules is a deletion that went half-way.
is prose from a module docstring in this repo, and it matches the arm's pattern
for `name=with`. So the arm would tell a writer their genuine symbol was
already defined, and point at a docstring.
WHAT THE FIX COSTS, measured rather than guessed, over 141 payload files of
this repo: the arm removed 15 of 210 report lines, and every one was a string
literal, a comment, an import statement, or Vue's `const emit = defineEmits()`
boilerplate. No real definition was lost — where a name had both a real
definition and a phantom, the phantom was dropped and the definition kept.
Timing: mean 193ms → 222ms, worst 569ms → 572ms. The arm was already dominated
by its twelve `git grep` calls, so confirming costs about 15% rather than the
second a per-(name, file) shape would have cost.
These cases are written against real git repositories rather than mocks,
because what is being pinned is the interaction between `git grep`, `head` and
the extractor — which is where #4042 lived too.
"""
from __future__ import annotations
import os
import shutil
import subprocess
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parents[1]
DEFS = ROOT / "plugin" / "hooks" / "scribe_defs.sh"
@pytest.fixture
def repo(tmp_path):
"""A committed git repo, plus a runner for the arm against it."""
for tool in ("git", "bash", "awk"):
if shutil.which(tool) is None:
pytest.skip(f"{tool!r} not installed")
env = {"PATH": os.environ["PATH"], "HOME": str(tmp_path),
"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@x",
"GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@x"}
root = tmp_path / "repo"
root.mkdir()
subprocess.run(["git", "init", "-q"], cwd=root, check=True, env=env)
def build(files: dict[str, str]):
for rel, text in files.items():
path = root / rel
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(text)
subprocess.run(["git", "add", "-A"], cwd=root, check=True, env=env,
capture_output=True)
subprocess.run(["git", "commit", "-q", "-m", "base"], cwd=root,
check=True, env=env, capture_output=True)
def run(names: str, writing: str = "new.py") -> str:
script = (f'set -uo pipefail\n. "{DEFS}"\n'
f'printf %s "$NAMES" | scribe_local_dups "{root}" {writing}\n')
out = subprocess.run(["bash", "-c", script], capture_output=True,
text=True, env={**env, "NAMES": names}, timeout=120)
assert out.returncode == 0, out.stderr
return out.stdout
return build, run
# ── the case the task names ──────────────────────────────────────────────────
def test_a_sentence_about_a_definition_is_not_a_definition(repo):
"""The verify case from #4227: one real `def change`, one docstring that
wraps onto a line beginning `function change`. The writer is told about
the first file and not the second."""
build, run = repo
build({
"real.py": "def change(a, b):\n return a\n",
"prose.py": (
'"""Some module.\n\n'
'When a caller moves, the enclosing\n'
'function change may have moved with it, which is the case\n'
'this paragraph exists to describe.\n'
'"""\n\n'
'X = 1\n'
),
})
out = run("sym\tchange\n")
assert "real.py" in out
assert "prose.py" not in out, (
"a wrapped docstring line still reads as a definition — the "
"confirmation pass did not run, or did not see the span"
)
assert "in 1 other file(s)" in out
def test_the_repos_own_prose_no_longer_defines_with():
"""Not a fixture — this repo's actual corpus, which is where the defect
was found.
Before the confirmation pass the arm named four files for `with`:
`…record type with no vector…`, `A class with no rules anywhere…` and two
more, all of them prose in docstrings and comments. It now names none,
and none is the correct answer: nothing here defines a symbol called
`with`, and in several of the languages in this tree it could not, because
it is a keyword.
Asserting on emptiness is only honest if the assertion could see a hit —
so it checks the arm ran, via a name that IS defined here.
"""
if shutil.which("bash") is None:
pytest.skip("bash not installed")
def arm(name: str) -> str:
script = (f'set -uo pipefail\n. "{DEFS}"\n'
f'printf "sym\\t{name}\\n" | scribe_local_dups "{ROOT}" nothing.py\n')
out = subprocess.run(["bash", "-c", script], capture_output=True,
text=True, timeout=180)
assert out.returncode == 0, out.stderr
return out.stdout
assert arm("extract_definitions").strip(), (
"the arm found nothing for a name this repo certainly defines, so a "
"silent result below would prove nothing"
)
assert arm("with").strip() == "", (
"`with` is prose in every file that matches the grep — a hit here is "
"a sentence being read as a definition"
)
def test_a_name_that_is_both_real_and_phantom_keeps_the_real_file(repo):
"""The narrowing case, and the one that would betray an over-eager filter:
the phantom file drops out and the genuine definition stays."""
build, run = repo
build({
"lib.py": "def create_note(user_id):\n return user_id\n",
"test_form.py": 'canon = shape_form("async def create_note(user_id: int, ...):", "sym")\n',
})
out = run("sym\tcreate_note\n")
assert "lib.py" in out
assert "test_form.py" not in out
assert "in 1 other file(s)" in out
def test_an_import_is_not_a_definition(repo):
"""TypeScript's `import { type Foo }` matched the `type\\s+NAME` arm, so a
new type was reported as a duplicate of the files importing it."""
build, run = repo
build({
"card.vue": 'import {\n type Choices, type Decision,\n} from "@/api/inception";\n',
"real.ts": "export type Choices = { a: number };\n",
})
out = run("sym\tChoices\n")
assert "card.vue" not in out, "an import site is not a definition site"
# ── the caps, which is where #4042 lived ─────────────────────────────────────
def test_the_candidate_cap_is_raised_above_the_display_cap():
"""Confirmation REMOVES hits, so a cap applied before it runs lets phantom
matches crowd real definitions out of the window — hits dropped before
anyone looked at them, which is #4042's bug in a new place. Asserted on
structure (rule 167) because the ordering is what matters, not the
numbers."""
src = DEFS.read_text()
for const in ("_SCRIBE_DUP_CANDIDATES=", "_SCRIBE_DUP_SHOWN="):
assert const in src, f"{const.rstrip(chr(61))} is gone — the two caps were folded back together"
cands = int(src.split("_SCRIBE_DUP_CANDIDATES=")[1].split("\n")[0])
shown = int(src.split("_SCRIBE_DUP_SHOWN=")[1].split("\n")[0])
assert cands > shown, (
f"candidates ({cands}) must exceed the display cap ({shown}), or "
"confirmation can only ever shrink an already-truncated window"
)
def test_phantoms_ahead_of_the_display_cap_do_not_hide_a_real_definition(repo):
"""Four phantom files sort before the one real definition. With the old
`head -4` on the grep, the real file never entered the window at all."""
build, run = repo
files = {f"a_phantom_{i}.py": f'BLURB = """\nclass Widget is described here.\n"""\n'
for i in range(4)}
files["z_real.py"] = "class Widget:\n pass\n"
build(files)
out = run("sym\tWidget\n")
assert "z_real.py" in out, "the real definition sorted behind four phantoms"
assert "a_phantom_0.py" not in out
def test_the_early_exiting_head_still_keeps_its_own_output(repo):
"""#4042, re-run through the new shape: under `pipefail` a `head` that
exits while git grep is still writing must not void the hits it printed.
Every file here holds a REAL definition, so confirmation keeps them all
and only the display cap applies."""
build, run = repo
build({f"module_with_a_fairly_long_descriptive_name_{i}.py": "def slug(t):\n return t\n"
for i in range(3000)})
out = run("sym\tslug\n")
assert "`slug` is already defined in 4 other file(s)" in out
# ── css ──────────────────────────────────────────────────────────────────────
def test_a_selector_named_inside_a_css_comment_is_not_a_rule(repo):
"""#2990's defect, on the lookup side rather than the extraction side."""
build, run = repo
build({
"real.css": ".badge { color: red; }\n",
# The selector must START the line, or the old anchored pattern
# never matched it and this case would pass without the fix.
"notes.css": "/*\n.badge, .chip and friends were retired here\n*/\na { color: blue; }\n",
})
out = run("css\tbadge\n")
assert "real.css" in out
assert "notes.css" not in out
tests/test_integration_backup_code_shape_roundtrip.py
+199
View File
@@ -0,0 +1,199 @@
"""Real-Postgres round trip for code_shapes (#4197).
**What this guards is a column that was simply not there.** `code_shapes`
exported `reason_code`, `recheck_at` and `diverges_from` and imported none of
them — a restore reported success and came back with every judgment's verdict
and no code for why, every recheck flag cleared, and every divergence pointer
gone. The unit-level column guard in `test_services_backup.py` is what found
it and is what stops it recurring; this file is the behavioural half, because
"the builder passes the kwarg" and "the value survives a real restore" are
different claims.
`diverges_from` gets the harder assertion, and it is the same shape as the one
`test_integration_backup_rule_usage_roundtrip.py` makes. It is a FK to
`notes.id`, so the tempting fix — carry the exported id across — produces a
row that points at whatever note happens to hold that number in the target
database. Not dropped, REATTACHED: the restore reports success and the
divergence is about the wrong snippet, with nothing downstream able to tell.
So the assertion is about WHOSE note the pointer landed on, not about which
integer it holds, and it fails if the answer ever becomes "the source's".
"""
import pytest
import pytest_asyncio
from datetime import datetime, timezone
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.code_shape import CodeShape
from scribe.models.note import Note
from scribe.models.project import Project
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 = "code_shape_roundtrip_owner"
RESTORED_USERNAME = "code_shape_roundtrip_restored"
REASON_CODE = "scoped-css"
RECHECK_AT = datetime(2026, 3, 4, 5, 6, tzinfo=timezone.utc)
async def _purge(username: str) -> None:
"""project -> code_shapes is ON DELETE CASCADE, so dropping the projects
clears the shapes this file made."""
async with async_session() as s:
for user in (await s.execute(
select(User).where(User.username == username)
)).scalars().all():
for proj in (await s.execute(
select(Project).where(Project.user_id == user.id)
)).scalars().all():
await s.delete(proj)
for note in (await s.execute(
select(Note).where(Note.user_id == user.id)
)).scalars().all():
await s.delete(note)
await s.commit()
async def _purge_restored() -> None:
await _purge(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():
"""SETUP ONLY — a database call after a `yield` here orphans a pooled
connection and breaks unrelated tests (see the sibling round-trip files)."""
await _purge_restored()
await _purge(OWNER_USERNAME)
@pytest_asyncio.fixture
async def source():
"""One judged shape carrying all three of the columns that went missing,
plus TWO notes: the snippet it is an instance of, and the one it diverges
from. Two, because a pointer that resolves to the same note as the
snippet would pass whether it was re-mapped or coincidentally right."""
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:
proj = Project(user_id=uid, title="Shapes round trip")
s.add(proj)
await s.flush()
canon = Note(user_id=uid, title="the canon", body="", note_type="snippet")
other = Note(user_id=uid, title="the one it diverges from", body="",
note_type="snippet")
s.add_all([canon, other])
await s.flush()
shape = CodeShape(
project_id=proj.id,
repo_key="git.example.com/x/y",
path="frontend/src/views/Thing.vue",
symbol="thing-row",
kind="css",
status="variant",
snippet_id=canon.id,
reason="deliberate departure, recorded",
reason_code=REASON_CODE,
classified_by="operator",
classified_at=datetime(2026, 2, 1, tzinfo=timezone.utc),
recheck_at=RECHECK_AT,
diverges_from=other.id,
)
s.add(shape)
await s.flush()
user_rows = backup._user_rows([owner])
user_rows[0]["username"] = RESTORED_USERNAME
payload = {
"version": backup.BACKUP_VERSION,
"users": user_rows,
"projects": backup._project_rows([proj]),
"notes": backup._note_rows([canon, other]),
"code_shapes": backup._code_shape_rows([shape]),
}
source_ids = {"other_note_id": other.id, "canon_note_id": canon.id}
await s.commit()
yield {"payload": payload, **source_ids}
await _purge(OWNER_USERNAME)
@pytest_asyncio.fixture
async def restored(source):
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"
proj = (await s.execute(
select(Project).where(Project.user_id == user.id)
)).scalars().one()
shape = (await s.execute(
select(CodeShape).where(CodeShape.project_id == proj.id)
)).scalars().one()
notes = {
n.title: n for n in (await s.execute(
select(Note).where(Note.user_id == user.id)
)).scalars().all()
}
# Dedented on purpose: a `yield` inside the session block holds a pooled
# connection open for the whole test, which is the failure the sibling
# round-trip files warn about in their own fixtures.
yield {"user": user, "shape": shape, "notes": notes, "source": source}
await _purge_restored()
async def test_the_reason_code_and_recheck_flag_survive_a_restore(restored):
"""Both were exported and both were dropped. A ledger restored without
`reason_code` keeps its verdicts and loses the argument for them, which is
the column the accounting reads to tell one kind of exemption from
another; without `recheck_at` it looks settled and is not."""
shape = restored["shape"]
assert shape.reason_code == REASON_CODE
assert shape.recheck_at is not None
assert shape.recheck_at.replace(tzinfo=timezone.utc) == RECHECK_AT
async def test_the_divergence_pointer_lands_on_the_restored_note(restored):
"""Not "does it hold a number" — WHOSE note it points at.
Carrying the exported id across would leave the pointer on the SOURCE
user's note, which still exists and still answers, so the row would read
as fine and be about the wrong snippet. Asserting on ownership fails in
that case even if the two ids happened to coincide.
"""
shape = restored["shape"]
expected = restored["notes"]["the one it diverges from"]
assert expected.id != restored["source"]["other_note_id"], (
"the restore reused the source note id, so this test cannot tell a "
"remap from a copy — the fixture is not proving what it claims"
)
assert shape.diverges_from is not None, "the pointer was dropped"
assert shape.diverges_from == expected.id
async with async_session() as s:
target = await s.get(Note, shape.diverges_from)
assert target is not None
assert target.user_id == restored["user"].id, (
"the divergence points at a note belonging to the SOURCE user — the "
"exported id was carried across instead of re-mapped"
)
assert shape.snippet_id == restored["notes"]["the canon"].id, (
"the two note pointers were resolved through different maps"
)
tests/test_integration_backup_rule_usage_roundtrip.py
+57 -4
View File
@@ -25,7 +25,9 @@ from sqlalchemy import select
from scribe.models import async_session
from scribe.models.note import Note
from scribe.models.rule_usage import PULLED, SURFACED, RuleUsageEvent
from scribe.models.rule_usage import (
APPLIED, DEPARTED, PULLED, SURFACED, RuleUsageEvent,
)
from scribe.models.rulebook import Rule, Rulebook, RulebookTopic
from scribe.models.user import User
from scribe.services import backup
@@ -141,6 +143,21 @@ async def source():
user_id=None, rule_id=rule.id,
event=SURFACED, source="write_path_rule",
),
# A departure and its reason (#4212). Seeded here because
# `detail` is the ONE field on this table that cannot be
# recomputed: a fresh install re-earns its counts by being used,
# but a stated reason exists once and is gone if a restore drops
# it — and a `departed` row that comes back reasonless reads as a
# rule that was simply missed.
RuleUsageEvent(
user_id=uid, rule_id=rule.id,
event=DEPARTED, source="mcp_rule_outcome",
detail="the integration lane has no registry credentials",
),
RuleUsageEvent(
user_id=uid, rule_id=rule.id,
event=APPLIED, source="mcp_rule_outcome",
),
])
await s.commit()
book_id, rule_id, note_id = book.id, rule.id, note.id
@@ -232,8 +249,8 @@ async def restored(source):
async def test_every_event_comes_back(restored):
"""The count first: every shape assertion below reads the same on an empty
list, so without this a restore that dropped all three would pass them."""
assert len(restored["events"]) == 3
list, so without this a restore that dropped all five would pass them."""
assert len(restored["events"]) == 5
async def test_the_events_attach_to_the_RESTORED_rule(restored):
@@ -272,7 +289,9 @@ async def test_the_actor_is_remapped_and_a_missing_one_survives(restored):
surfacings of exactly the surface being measured."""
attributed = [e for e in restored["events"] if e.user_id is not None]
orphaned = [e for e in restored["events"] if e.user_id is None]
assert len(attributed) == 2
# Four attributed: the surfacing, the pull, and the two outcome rows
# added with `detail` (#4212). One orphaned, deliberately.
assert len(attributed) == 4
assert len(orphaned) == 1, (
"the event with no actor did not come back. Telemetry outlives the "
"account it was recorded for; dropping it silently lowers the "
@@ -289,6 +308,40 @@ async def test_the_event_and_source_survive(restored):
assert pairs == {
(SURFACED, "write_path_rule"),
(PULLED, "mcp_get_rule"),
(DEPARTED, "mcp_rule_outcome"),
(APPLIED, "mcp_rule_outcome"),
}
assert sum(1 for e in restored["events"] if e.event == SURFACED) == 2
assert sum(1 for e in restored["events"] if e.event == PULLED) == 1
async def test_a_departures_reason_survives_the_round_trip(restored):
"""The one field here that a fresh install cannot re-earn.
Counts come back by being used again; a stated reason exists once. A
restore that kept the `departed` row and dropped its `detail` would turn
a deliberate, argued departure into something indistinguishable from a
rule that was read and missed — which is the exact distinction milestone
419 was opened to create, undone silently at the one moment nobody is
watching.
#4197 is the standing warning behind this test: the backup column guard
watches the export side only, so a column added to the model and to the
exporter and NOT to the importer round-trips as null with nothing to say
so.
"""
departures = [e for e in restored["events"] if e.event == DEPARTED]
assert len(departures) == 1
assert departures[0].detail == (
"the integration lane has no registry credentials"
)
async def test_an_application_carries_no_reason_and_that_is_not_a_loss(restored):
"""`applied` is the unremarkable case and is stored reasonless on
purpose. Asserted so that a later change making `detail` NOT NULL — or
backfilling it with a placeholder — has to argue with a test rather than
quietly make every application look like it had something to say."""
applications = [e for e in restored["events"] if e.event == APPLIED]
assert len(applications) == 1
assert applications[0].detail is None
tests/test_lesson_usage_surface.py
+189
View File
@@ -0,0 +1,189 @@
"""A lesson's surfaced-vs-opened counts reach a reader (#4196).
The lesson slot has recorded usage since it shipped, and until this nothing
showed it. `get_lesson`'s REST door attached it; the list did not, neither MCP
door did, and no view rendered it — so the one kind whose central question is
"is this trigger firing on the right situation" was the one kind whose answer
was unreadable.
WHY THE COUNTS MATTER MORE HERE THAN ON A SNIPPET. #4196 asks whether a lesson
repeatedly followed should become a rule, and names the trap: raw frequency
cannot tell "this should bind" from "this trigger is too broad", which is the
commoner reading. Surfaced-AND-opened can. Neither question is answerable by
anything that cannot see the numbers, which is why this is the first step and
not the threshold.
NO THRESHOLD IS ASSERTED ANYWHERE HERE, deliberately. At the time of writing
the corpus is 10 lessons with 7 recorded surfacings and 3 opens; a promotion
rule fitted to that would be fitting noise, and `UsageBadge` already declines
to render a verdict on fewer than three surfacings for the same reason.
"""
import inspect
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from scribe.mcp.tools import lessons as lesson_tools
from scribe.services.note_usage import empty_usage
def _rows():
return [
{"id": 7, "title": "a", "tags": [], "snippet": "", "when_to_apply": "x"},
{"id": 9, "title": "b", "tags": [], "snippet": "", "when_to_apply": "y"},
]
@pytest.mark.asyncio
async def test_the_mcp_listing_carries_usage_for_every_row():
"""Zero-filled, not omitted. A missing key would make the reader treat
"never surfaced" and "not reported" as the same thing, which is the
distinction the whole readout exists to keep."""
used = {7: {**empty_usage(), "surfaced_count": 5, "pull_count": 2}}
with (
patch.object(lesson_tools, "current_user_id", return_value=1),
patch.object(lesson_tools.knowledge_svc, "query_knowledge",
new=AsyncMock(return_value=(_rows(), 2))),
patch.object(lesson_tools.access_svc, "label_shared_items",
new=AsyncMock(side_effect=lambda _uid, items: items)),
patch.object(lesson_tools, "usage_for_notes",
new=AsyncMock(return_value=used)),
):
out = await lesson_tools.list_lessons()
by_id = {r["id"]: r for r in out["lessons"]}
assert by_id[7]["usage"]["surfaced_count"] == 5
assert by_id[7]["usage"]["pull_count"] == 2
# The row nobody has surfaced still carries the key.
assert by_id[9]["usage"] == empty_usage()
@pytest.mark.asyncio
async def test_the_listing_asks_for_usage_once_for_the_whole_page():
"""One aggregate, not one lookup per row — the snippet listing's own
comment calls a per-row read N+1 by construction, and a lesson list is the
same shape."""
reader = AsyncMock(return_value={})
with (
patch.object(lesson_tools, "current_user_id", return_value=1),
patch.object(lesson_tools.knowledge_svc, "query_knowledge",
new=AsyncMock(return_value=(_rows(), 2))),
patch.object(lesson_tools.access_svc, "label_shared_items",
new=AsyncMock(side_effect=lambda _uid, items: items)),
patch.object(lesson_tools, "usage_for_notes", new=reader),
):
await lesson_tools.list_lessons()
assert reader.await_count == 1
assert sorted(reader.await_args.args[0]) == [7, 9]
@pytest.mark.asyncio
async def test_get_lesson_reads_the_count_before_recording_its_own_pull():
"""Otherwise the first read of a lesson reports a pull that is its own.
`get_lesson` records a pull on every open — it has to, or the kind sits
permanently at zero and reads as dead weight. That makes the ORDER load
bearing in a way it is not for kinds that only count.
"""
order: list[str] = []
note = MagicMock(id=7, user_id=1)
async def _usage_read(ids):
order.append("read")
return {7: {**empty_usage(), "surfaced_count": 4, "pull_count": 1}}
with (
patch.object(lesson_tools, "current_user_id", return_value=1),
patch.object(lesson_tools.lessons_svc, "get_lesson",
new=AsyncMock(return_value=note)),
patch.object(lesson_tools, "_to_dict", return_value={"id": 7}),
patch.object(lesson_tools.access_svc, "describe_provenance",
new=AsyncMock(return_value={})),
patch.object(lesson_tools, "usage_for_notes", new=_usage_read),
patch.object(lesson_tools, "record_pulled",
side_effect=lambda **_kw: order.append("pull")),
):
out = await lesson_tools.get_lesson(7)
assert order == ["read", "pull"], (
"the pull was recorded before the count was read, so the number the "
"caller is shown includes the read that produced it"
)
assert out["usage"]["pull_count"] == 1
# ── the REST door, on structure (rule 167) ───────────────────────────────────
#
# Its siblings in test_lesson_rest_door.py are source guards for the same
# reason: the route is decorated and returns a Quart response, so driving it
# means standing up the app. What matters here is reachable from the source
# and falsifiable from it — an aggregate rather than a per-row read, and the
# same read-then-record order the MCP door is pinned to above.
def _route_source(name: str) -> str:
from scribe.routes import lessons as routes
src = inspect.getsource(routes)
start = src.index(f"async def {name}(")
nxt = src.find("\n@lessons_bp.route", start)
return src[start:nxt if nxt > 0 else len(src)]
def test_the_rest_listing_reads_usage_once_for_the_page():
"""A per-row lookup would be N+1 by construction — the listing's own
comment says so, and this is what makes that comment checkable."""
src = _route_source("list_lessons_route")
assert src.count("usage_for_notes(") == 1
# The one call is not inside the loop that assigns the rows.
call = src.index("usage_for_notes(")
assign = src.index('["usage"]')
assert call < assign
def test_every_rest_row_carries_the_key_even_at_zero():
""""Never surfaced" is a state the UI renders; a missing field is not."""
src = _route_source("list_lessons_route")
assert "empty_usage()" in src, (
"a row with no recorded usage would come back without the key, and a "
"reader cannot tell that from a reporting failure"
)
def test_the_rest_detail_door_also_reads_before_it_records():
"""Two doors that disagree about what the number counts are worse than
one door that is wrong, because only one of them looks wrong."""
src = _route_source("get_lesson_route")
assert src.index("usage_for_notes(") < src.index("record_pulled(")
def test_these_guards_can_fail():
"""Rule 167: falsify the shape they assert against, so a guard that has
quietly stopped describing anything cannot pass by describing nothing."""
src = _route_source("list_lessons_route")
assert "def list_lessons_route" in src
assert "def get_lesson_route" not in src, "the slice ran past its route"
def test_the_detail_view_renders_the_badge_rather_than_respelling_it():
"""Snippet #3460: reach for UsageBadge and pass the kind's own advice; a
view's scoped re-spelling of `.usage-tag` is what that component replaced.
The advice is the kind-specific half. For a lesson it points at the
trigger, not at deletion — a lesson nobody opens is usually keyed to a
situation nobody is in, which is `update_lesson`'s own words and the
reading #4196 calls the commoner one.
"""
view = (Path(__file__).resolve().parents[1]
/ "frontend/src/views/LessonDetailView.vue").read_text()
assert "UsageBadge" in view
assert "usage-tag" not in view, "re-spelled the chip instead of reusing it"
key = 'dead-weight-advice="'
start = view.index(key) + len(key)
advice = view[start:view.index('"', start)]
assert "when_to_apply" in advice, (
f"the dead-weight advice does not point at the trigger: {advice!r}"
)
tests/test_mcp_tool_rulebooks.py
+108 -2
View File
@@ -206,8 +206,10 @@ def test_register_attaches_every_tool():
# always-on exclusion tools with the tier they served.
# 22 since milestone 414 retired subscriptions and suppressions: the two
# subscribe tools and the four suppress/unsuppress tools. 23 with move_rule
# (milestone 414 step 3), the way a rule changes home.
assert len(mcp.names) == 23
# (milestone 414 step 3), the way a rule changes home. 24 with
# rule_outcome (milestone 419 step 1), the way a rule says what it
# actually changed.
assert len(mcp.names) == 24
# spot-check a few names
assert "list_rulebooks" in mcp.names
assert "create_rule" in mcp.names
@@ -478,3 +480,107 @@ async def test_move_rule_on_someone_elses_rule_is_not_found():
from scribe.mcp.tools.rulebooks import move_rule
with pytest.raises(ValueError, match="not found"):
await move_rule(rule_id=94, project_id=3)
# ── rule_outcome: what a rule actually changed (#4212, milestone 419) ─────
@pytest.mark.asyncio
async def test_rule_outcome_records_an_application():
rule = fake_rule(id=156, title="`dev` is home")
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=rule),
), patch(
"scribe.mcp.tools.rulebooks.record_rule_outcome", MagicMock()
) as rec:
from scribe.mcp.tools.rulebooks import rule_outcome
out = await rule_outcome(rule_id=156, outcome="applied")
assert out["outcome"] == "applied" and out["recorded"] is True
assert out["why"] is None
assert rec.call_args.kwargs["outcome"] == "applied"
assert rec.call_args.kwargs["source"] == "mcp_rule_outcome"
@pytest.mark.asyncio
async def test_rule_outcome_records_a_departure_with_its_reason():
rule = fake_rule(id=156, title="`dev` is home")
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=rule),
), patch(
"scribe.mcp.tools.rulebooks.record_rule_outcome", MagicMock()
) as rec:
from scribe.mcp.tools.rulebooks import rule_outcome
out = await rule_outcome(
rule_id=156, outcome="departed", why="the operator asked for main"
)
assert out["outcome"] == "departed"
assert rec.call_args.kwargs["detail"] == "the operator asked for main"
@pytest.mark.asyncio
async def test_a_departure_without_a_reason_is_refused_at_the_door():
"""Refused with a message that says WHY a reason is needed, not just
that one is missing — the caller is an agent deciding whether to bother,
and "it cannot be told from a miss" is the argument that lands."""
rule = fake_rule(id=156, title="`dev` is home")
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=rule),
), patch(
"scribe.mcp.tools.rulebooks.record_rule_outcome", MagicMock()
) as rec:
from scribe.mcp.tools.rulebooks import rule_outcome
with pytest.raises(ValueError, match="departure needs its reason"):
await rule_outcome(rule_id=156, outcome="departed", why=" ")
rec.assert_not_called()
@pytest.mark.asyncio
async def test_there_is_no_way_to_report_having_ignored_a_rule():
"""Deliberate, and the reason is in the tool's docstring: noticing that
you ignored a rule is the same act as not ignoring it, so the state is
derived rather than reported. A caller reaching for the word gets an
error rather than a row that would read as measurement."""
rule = fake_rule(id=156, title="`dev` is home")
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=rule),
), patch(
"scribe.mcp.tools.rulebooks.record_rule_outcome", MagicMock()
) as rec:
from scribe.mcp.tools.rulebooks import rule_outcome
for bogus in ("ignored", "skipped", "read", ""):
with pytest.raises(ValueError, match="must be 'applied' or 'departed'"):
await rule_outcome(rule_id=156, outcome=bogus)
rec.assert_not_called()
@pytest.mark.asyncio
async def test_rule_outcome_refuses_a_rule_the_caller_cannot_see():
"""The access check comes FIRST, so a miss cannot be used to probe for
the existence of someone else's rule, and nothing is recorded against
an id the caller has no claim on."""
with patch(
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
AsyncMock(return_value=None),
), patch(
"scribe.mcp.tools.rulebooks.record_rule_outcome", MagicMock()
) as rec:
from scribe.mcp.tools.rulebooks import rule_outcome
with pytest.raises(ValueError, match="rule 999 not found"):
await rule_outcome(rule_id=999, outcome="applied")
rec.assert_not_called()
def test_rule_outcome_is_registered_and_is_not_read_only():
"""Its whole effect is a write, and `why` is prose the agent authored —
a read-scoped key that can put text in the operator's database is not
read-scoped, whatever table it lands in."""
from scribe.mcp.server import _READ_ONLY_TOOLS, _WRITE_TOOLS
from scribe.mcp.tools import rulebooks as mod
mcp = FakeMCP()
mod.register(mcp)
assert "rule_outcome" in mcp.names
assert "rule_outcome" in _WRITE_TOOLS
assert "rule_outcome" not in _READ_ONLY_TOOLS
tests/test_pattern_coverage.py
+83
View File
@@ -10,7 +10,10 @@ tripwire.
"""
import io
import json
import shutil
import subprocess
import tarfile
from pathlib import Path
import pytest
import pytest_asyncio
@@ -28,6 +31,8 @@ from scribe.services.coverage import (
from scribe.services.shape_ledger import location_covers
from tests.helpers import ensure_user
PLUGIN = Path(__file__).resolve().parents[1] / "plugin"
# --- unit: the definition extractor (shared vectors with the hook) -----------
EXTRACTION_VECTORS = [
@@ -63,6 +68,53 @@ EXTRACTION_VECTORS = [
[]),
("dedup-within-file", "def f():\n pass\ndef f():\n pass\n",
[("sym", "f")]),
# --- comment and string spans (#4222) ------------------------------------
# Prose is not code. A wrapped docstring line beginning "class AND the"
# announced a shape called `AND` to a live session; `with` and `nobody`
# out of one module docstring in scripts/check_dangling_styles.py reached
# persisted, judged `code_shapes` rows.
("docstring-prose",
'def real_one():\n """Its own text is about the\n'
' class AND the to_dict, and is a def bar():\n'
' class Foo: lives here too.\n """\n pass\n',
[("sym", "real_one")]),
# An opener with no closer blanks NOTHING: the scan rewinds past it, so a
# stray marker costs one span rather than the rest of the file.
("unterminated-docstring",
'def before():\n """oops, never closed\n\ndef after():\n pass\n',
[("sym", "before"), ("sym", "after")]),
# The CSS half of the same defect (#2990): a wrapped comment line that
# happens to begin with a dotted token reads as a selector.
("css-comment-selector",
"/* A real base rule, not just descendants: the check reads a\n"
" .ghost, class that only ever appears as an ancestor */\n.check { }\n",
[("css", "check")]),
# The rest of #2990's verify list, which this scan satisfies as written:
# a comment holding a WHOLE rule defines nothing, and an unterminated
# comment does not swallow the file. The phantom it was filed for —
# `.editor-body` out of a wrapped comment in TaskEditorView.vue, a
# persisted row with a `used_by` count of zero inflating unused_css — is
# among the twenty-two this removes.
("comment-containing-a-whole-rule",
"/* .ghost {\n color: red;\n } */\n.real { color: blue; }\n",
[("css", "real")]),
("unterminated-block-comment",
"/* never closed\n.after { color: red; }\n",
[("css", "after")]),
# A string that HOLDS a comment marker is not a comment — the case that
# made the first draft of the scan eat 70 lines of live code.
("string-holds-a-marker",
'SAMPLE = "red /* "\ndef after_the_string():\n pass\n',
[("sym", "after_the_string")]),
# `#` is the one marker whose meaning is the language\'s: a colour here,
# a comment two lines down, and the extractor is handed no path.
("hash-is-a-colour-not-a-comment",
".a { color: #fff; } /* .ghost,\n class Phantom: */\n.b { }\n",
[("css", "a"), ("css", "b")]),
("line-comment-mentioning-a-docstring",
'def kept():\n pass\n# a stray """ in a comment\n'
'def also_kept():\n pass\n',
[("sym", "kept"), ("sym", "also_kept")]),
]
@@ -75,6 +127,37 @@ def test_extractor_agrees_with_the_hook_on_what_defines(text, expected):
assert extract_shapes(text) == expected
@pytest.mark.parametrize(
"text",
[t for _i, t, _e in EXTRACTION_VECTORS],
ids=[i for i, _t, _e in EXTRACTION_VECTORS],
)
def test_the_hook_extractor_runs_and_agrees_line_for_line(text):
"""The comment at the top of this module has been the only thing holding
the two extractors together, and a comment cannot fail. This RUNS the
hook's awk program over the same vectors.
The hook emits every definition in source order with no dedup — identity
there is per payload, not per file — so the comparison de-dupes its
output before matching, which is the one difference between the two that
is by design.
"""
if shutil.which("awk") is None: # pragma: no cover - env guard
pytest.skip("awk not available")
lib = PLUGIN / "hooks" / "scribe_defs.sh"
out = subprocess.run(
["bash", "-c", f'. "{lib}"; scribe_defs'],
input=text, capture_output=True, text=True,
)
assert out.returncode == 0, out.stderr
seen: list[tuple[str, str]] = []
for line in out.stdout.splitlines():
kind, _, name = line.partition("\t")
if name and (kind, name) not in seen:
seen.append((kind, name))
assert seen == extract_shapes(text)
def test_scannable_gates_prose_vendored_and_sourcemaps():
assert scannable("src/app.py")
assert scannable("web/button.css")
tests/test_pre_act_checkpoint.py
+307
View File
@@ -0,0 +1,307 @@
"""A rule put IN FRONT of an act, not beside its result (#4214, milestone 419).
WHY THIS EXISTS
Every other rule surface in this plugin returns `additionalContext`, which
Claude Code delivers alongside the tool RESULT. By the time the line is read
the call is written, so the rule reads as commentary on a decision already
made. Milestone 419 measured the cost over one session: seven misses, three
caught by the operator and none by this system, five of them the same move —
acting on the thing in hand without reading the contract around it.
A checkpoint spends the same retrieval differently. The hook returns a `deny`,
the act does not run, and the rule's own text can be read before the call
exists. The remedy is one `get_rule` call and the act may then be re-submitted
unchanged.
WHAT THIS PINS, AND WHAT IT DELIBERATELY DOES NOT
Pinned: the four conditions under which an act may be held, the two guards
that stop it recurring, and the shape of the envelope. Not pinned: the bar
itself (a tuning value, and a test asserting 0.80 would fail on every retune
while proving nothing) or the wording of the reason (prose, and it will be
rewritten). The cases below express scores relative to the constant.
THE ONE BOUNDARY THAT IS A RECORDED DECISION, NOT A DESIGN CHOICE. Only the
ACTION arm can hold. `scribe_prior_art.sh` carries a tested property that it
never returns a permissionDecision — the operator's decision that a recall aid
may not stand in the way of a write — and this milestone does not get to
quietly overturn it. The write-path arm computes the same block and returns
it, so the decision can be revisited with evidence; the hook ignores it. The
last test here asserts that boundary holds, because it is exactly the kind of
property that erodes when someone extends the feature later.
"""
import json
import shutil
import subprocess
from pathlib import Path
import pytest
from scribe.services.plugin_context import (
_CHECKPOINT_DEFAULT,
CHECKPOINT_SESSION_CAP,
_rule_band,
checkpoint_for,
)
from tests.helpers import fake_rule
ROOT = Path(__file__).resolve().parents[1]
HOOKS = ROOT / "plugin" / "hooks"
DEFS = HOOKS / "scribe_defs.sh"
ACTION_HOOK = HOOKS / "scribe_tool_rules.sh"
WRITE_HOOK = HOOKS / "scribe_prior_art.sh"
WHERE = "this Bash call"
TRIGGER = "about to assert in a commit message what CI said"
def hit(score: float, rule_id: int, **attrs):
# `attrs` overrides rather than duplicates: passing `when_to_apply=""` for
# the no-trigger case alongside a hard-coded default is a duplicate keyword
# and a TypeError, not a test.
fields = {"id": rule_id, "title": f"rule {rule_id}", "when_to_apply": TRIGGER}
fields.update(attrs)
return (score, fake_rule(**fields))
def hold(kept, held=(), floor=None):
return checkpoint_for(
kept, held=set(held), floor=_CHECKPOINT_DEFAULT if floor is None else floor,
where=WHERE,
)
# ── The four conditions ───────────────────────────────────────────────────
def test_nothing_retrieved_holds_nothing():
"""The common case by a wide margin, and the one that must be cheapest."""
assert hold([]) == {}
def test_a_hit_under_the_bar_is_a_hint_and_not_a_stop():
"""The hint arms keep working at their own floor; only a hit the corpus is
confident about is allowed to cost a round trip."""
assert hold([hit(_CHECKPOINT_DEFAULT - 0.001, 1)]) == {}
def test_a_hit_on_the_bar_holds_the_act():
got = hold([hit(_CHECKPOINT_DEFAULT, 1)])
assert got["rule_id"] == 1
assert got["where"] == WHERE
def test_a_preference_never_holds_an_act():
"""A preference says how something has been done and following it is what
keeps work consistent; a rule says what happens if you do not. Stopping an
act over a preference would assert a force the record does not claim, and
the renderer already keeps that distinction in the word that names it."""
assert hold([hit(0.99, 2, kind="preference")]) == {}
def test_a_rule_the_session_already_opened_never_holds_an_act():
"""`held` is observable — a PostToolUse hook watches for the `get_rule`
call (#4100) — so this is a recorded event, not a model's self-report
about its own context. A session that read the rule has already had the
thing the checkpoint exists to produce, and holding it again would punish
the behaviour being asked for."""
assert hold([hit(0.99, 3)], held={3}) == {}
def test_holding_one_rule_does_not_excuse_another():
assert hold([hit(0.99, 4)], held={3})["rule_id"] == 4
def test_only_the_bands_top_hit_may_hold():
"""`_rule_band` keeps a SET so an act can surface several rules, but the
ranker's confidence claim attaches to its first element only. A stop
raised on the fourth line of a band is a stop justified by a score nobody
claimed — so a preference on top ends the question rather than deferring
to the rule behind it."""
band = [hit(0.99, 5, kind="preference"), hit(0.985, 6)]
assert hold(band) == {}
def test_position_decides_not_score():
"""Deliberately falsifiable from the other side: hits arrive ordered, and
this reads `kept[0]` rather than re-maximising. A version that took the
highest score would pick 8 here."""
assert hold([hit(0.985, 7), hit(0.99, 8)])["rule_id"] == 7
def test_the_band_trims_before_the_checkpoint_sees_it():
"""The two instruments compose in one direction only: the band decides
what is close enough to show, and the checkpoint reads what survived."""
kept = _rule_band([hit(0.83, 11), hit(0.81, 12), hit(0.70, 13)])
assert len(kept) == 2
assert hold(kept, floor=0.80)["rule_id"] == 11
def test_a_floor_of_zero_disables_rather_than_holding_everything():
"""The failure direction that matters. A bar read as 0 — a cleared
setting, a bad parse — must turn the feature OFF, never hold the first
command of every session behind whatever ranked first."""
assert hold([hit(0.99, 9)], floor=0.0) == {}
# ── What the held act is told ─────────────────────────────────────────────
def test_the_reason_names_the_one_call_that_clears_it():
"""A stop whose remedy is vague costs more than the miss it prevents."""
reason = hold([hit(0.99, 42)])["reason"]
assert "get_rule(42)" in reason
assert "rule 42" in reason
assert TRIGGER in reason
def test_the_reason_says_the_act_may_proceed_unchanged():
"""Nothing here knows whether the act is wrong, and saying so is what
keeps the stop from reading as an accusation the system cannot support."""
assert "re-submit" in hold([hit(0.99, 42)])["reason"]
def test_a_rule_with_no_trigger_renders_without_an_empty_bracket():
assert "()" not in hold([hit(0.99, 10, when_to_apply="")])["reason"]
def test_every_held_act_carries_a_rendered_reason():
"""Rendered at the one call site inside `checkpoint_for`, never by each
arm: two arms that each remember to render it are two arms that can stop
agreeing on what a stop says, which is #3497's history for this pair."""
assert hold([hit(0.99, 1)])["reason"].strip()
# ── The two guards, in the shell that enforces them ───────────────────────
def sh(script: str) -> subprocess.CompletedProcess:
for tool in ("bash", "awk"):
if shutil.which(tool) is None:
pytest.skip(f"hook runtime tool {tool!r} not installed")
return subprocess.run(
["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'],
capture_output=True, text=True,
)
@pytest.fixture()
def ledger(tmp_path):
return tmp_path / "sid.checkpoint.ids"
def allowed(ledger, rule_id) -> bool:
r = sh(f'scribe_checkpoint_allowed "{ledger}" "{rule_id}" && echo YES || echo NO')
assert r.returncode == 0, r.stderr
return r.stdout.strip().endswith("YES")
def test_a_rule_may_hold_at_most_one_act_per_session(ledger):
"""Once the remedy has been offered, repeating it turns a reader who
decided the rule does not apply into a reader who cannot proceed."""
assert allowed(ledger, 101)
assert not allowed(ledger, 101)
assert allowed(ledger, 102), "a different rule is a different claim"
def test_a_session_cannot_be_held_more_than_the_cap(ledger):
"""The guard on the worst case, not a tuning value: a mis-set floor or a
corpus that suddenly resembles everything must degrade to a noisy session,
never to one that cannot make progress."""
for n in range(CHECKPOINT_SESSION_CAP):
assert allowed(ledger, 200 + n)
assert not allowed(ledger, 999)
def test_a_refused_hold_is_not_written_to_the_ledger(ledger):
"""Otherwise the cap eats itself: refusals would count toward it and the
ledger would grow without a single act ever being held."""
for n in range(CHECKPOINT_SESSION_CAP):
allowed(ledger, 200 + n)
allowed(ledger, 999)
assert len(ledger.read_text().split()) == CHECKPOINT_SESSION_CAP
@pytest.mark.parametrize("rule_id", ["", "abc", " "])
def test_an_unreadable_rule_id_refuses_rather_than_holding(ledger, rule_id):
"""Fails CLOSED in the direction that costs nothing. A garbled id cannot
be written to the ledger, so allowing it would be a stop that recurs
forever with no way to clear it."""
assert not allowed(ledger, rule_id)
def test_no_ledger_file_refuses_rather_than_holding(tmp_path):
"""A session with no id gets no ledger, and a stop that cannot be recorded
is a stop that cannot be capped."""
assert not allowed("", 101)
def test_the_deny_envelope_is_valid_json_carrying_the_reason():
r = sh('scribe_json_deny PreToolUse "read \\"rule 9\\" first — then re-submit"')
assert r.returncode == 0, r.stderr
out = json.loads(r.stdout)["hookSpecificOutput"]
assert out["hookEventName"] == "PreToolUse"
assert out["permissionDecision"] == "deny"
# Quotes and an em dash survive the escaper — the reason is prose and will
# contain both, and a broken envelope is silently ignored by the harness.
assert '"rule 9"' in out["permissionDecisionReason"]
assert "re-submit" in out["permissionDecisionReason"]
# ── The boundary that is a recorded decision ──────────────────────────────
def test_only_the_action_hook_can_hold_an_act():
"""THE GUARD ON THE RECORDED DECISION, and the reason it is here rather
than left to memory.
`scribe_prior_art.sh` runs before every Write and Edit and has never been
able to stop one. That is the operator's decision — a recall aid may not
stand in the way of the work — and `test_hook_never_returns_a_permission_
decision` in test_write_path_trigger.py holds the other half of it.
This milestone has a live argument for extending the checkpoint to writes:
three of its seven misses were file edits and none of them are reachable
from the command side. That argument is exactly why this assertion exists.
A feature with a good reason to spread is the kind that spreads without
anyone deciding to, and the decision here is the operator's to revisit.
"""
assert "scribe_json_deny" in ACTION_HOOK.read_text()
write_code = [
ln for ln in WRITE_HOOK.read_text().splitlines()
if not ln.lstrip().startswith("#")
]
assert not any("scribe_json_deny" in ln for ln in write_code)
assert not any("permissionDecision" in ln for ln in write_code)
def test_the_action_hook_caps_every_hold_it_emits():
"""Structural, and able to fail (rule 167): the deny and the ledger call
must appear together. A deny emitted outside the guard is a session that
can be held by the same rule on every command, which is the one outcome
both guards exist to prevent."""
text = ACTION_HOOK.read_text()
code = [ln for ln in text.splitlines() if not ln.lstrip().startswith("#")]
denies = [i for i, ln in enumerate(code) if "scribe_json_deny" in ln]
assert denies, "the action arm no longer emits a hold at all"
for i in denies:
window = "\n".join(code[max(0, i - 6):i])
assert "scribe_checkpoint_allowed" in window, (
"a hold is emitted without passing the per-rule and per-session "
"guards first"
)
def test_the_action_hook_records_what_was_surfaced_whichever_way_it_renders():
"""The ledger append sits BEFORE the checkpoint branch. What the server
chose to surface happened whichever way this hook then renders it, and
doing the bookkeeping inside one branch is how the two arms' ledgers came
to disagree once already."""
code = [
ln for ln in ACTION_HOOK.read_text().splitlines()
if not ln.lstrip().startswith("#")
]
append = next(i for i, ln in enumerate(code) if "scribe_rules_append" in ln)
deny = next(i for i, ln in enumerate(code) if "scribe_json_deny" in ln)
assert append < deny
def test_the_action_hook_is_still_shell_valid():
subprocess.run(["bash", "-n", str(ACTION_HOOK)], check=True)
tests/test_precompact_hook.py
+18 -3
View File
@@ -29,6 +29,8 @@ correct beside every other hook in this directory and would inject nothing.
from __future__ import annotations
import json
import tempfile
import os
import shutil
import subprocess
from pathlib import Path
@@ -44,11 +46,22 @@ EVENT = {"session_id": "s1", "transcript_path": "/tmp/t.jsonl", "cwd": "/repo",
"custom_instructions": None}
def _run(event: dict) -> subprocess.CompletedProcess:
def _run(event: dict, tmpdir: str | None = None) -> subprocess.CompletedProcess:
"""Run the hook with its ledger directory ISOLATED.
The hook reads this session's rule ledgers since #4216, and they live under
`$TMPDIR/scribe-priorart`. Without an override these tests would read the
machine's real /tmp: on a developer box mid-session that is not empty, and
the output would depend on what some other session happened to leave
behind. None of the assertions below would fail on it today, which is
exactly why it is worth closing now rather than after it starts flaking.
"""
if shutil.which("bash") is None:
pytest.skip("bash not installed")
env = dict(os.environ)
env["TMPDIR"] = tmpdir or tempfile.mkdtemp()
return subprocess.run(["bash", str(HOOK)], input=json.dumps(event),
capture_output=True, text=True, timeout=30)
capture_output=True, text=True, timeout=30, env=env)
def _code() -> str:
@@ -115,9 +128,11 @@ def test_it_never_blocks_the_compaction():
assert "decision" not in code, "a block decision would skip the compaction"
assert "exit 2" not in code
# A truncated or absent event must not turn into a non-zero exit either.
env = dict(os.environ)
env["TMPDIR"] = tempfile.mkdtemp()
for event in ("", "not json", "{}"):
out = subprocess.run(["bash", str(HOOK)], input=event,
capture_output=True, text=True, timeout=30)
capture_output=True, text=True, timeout=30, env=env)
assert out.returncode == 0, f"{event!r} → {out.returncode}: {out.stderr}"
tests/test_retrieval_surfaces.py
+5 -1
View File
@@ -183,5 +183,9 @@ async def test_the_write_path_config_carries_every_arm_it_drives():
cfg = await pc.get_writepath_config(1)
for key in ("threshold", "top_k", "rule_threshold", "rule_top_k",
"tool_rule_threshold", "tool_rule_top_k"):
"tool_rule_threshold", "tool_rule_top_k",
# Not a surface, and that is why it is spelled out here: it
# has no floor/budget pair to derive from, so nothing else in
# this file would notice it going missing (#4214).
"checkpoint_threshold"):
assert key in cfg, f"{key} missing — its arm will silently no-op"
tests/test_retrieval_tuning.py
+144
View File
@@ -196,3 +196,147 @@ def test_every_tool_in_the_module_is_registered():
"retrieval_surfaces", "migrate_retrieval_floor",
"tune_retrieval", "retrieval_tuning_history",
]
# ── record_release_defaults / floor_moves_since (#4225) ───────────────────
#
# THE HOLE THESE FILL. This table records dial turns — a person or a model
# choosing a number. It was silent about the other way a floor moves: somebody
# edits `floor_default` in the registry and ships it. That change is invisible
# to every consumer of this table, and one consumer is `band_hugs_floor`, which
# compares a band against a floor and could not notice they came from different
# regimes.
#
# Measured on the instance that found it: `write_path_rule` went 0.68 -> 0.72
# on 2026-09-02 as a shipped default, so a 30-day window opening 2026-08-22
# held six days of calls made under the old bar. The readout reported the band
# sitting "-0.0216 above" its floor — which is what a two-population comparison
# looks like when it finally says so out loud.
def _release_rows(rows):
"""Patch `async_session` so the recorder sees `rows` as what is on record.
`rows` maps (surface, dial) -> new_value already recorded by a release.
"""
session = make_mock_session()
seen = []
async def execute(stmt):
seen.append(stmt)
result = MagicMock()
# The recorder asks one question at a time, in registry order, so the
# answers are handed back in the order it asks them.
key = seen_keys.pop(0) if seen_keys else None
row = None
if key is not None and key in rows:
row = MagicMock()
row.new_value = rows[key]
result.scalars.return_value.first.return_value = row
return result
seen_keys = [(n, d) for n in rt.surface_names() for d in ("floor", "budget")]
session.execute = AsyncMock(side_effect=execute)
return session
@pytest.mark.asyncio
async def test_a_first_boot_records_a_baseline_and_calls_it_one():
"""Nothing moved. The row exists so the NEXT release has a predecessor.
Load-bearing downstream: the band check suspends itself on a genuine move
and must NOT suspend itself on a fresh install's baseline. It tells them
apart by `old_value` being null, so a baseline that claimed a change would
silently retire the check on every new install.
"""
session = _release_rows({})
with patch.object(rt, "async_session", MagicMock(return_value=session)):
written = await rt.record_release_defaults()
assert written, "a first boot records every dial"
assert all(w["baseline"] for w in written)
assert all(w["old_value"] is None for w in written)
@pytest.mark.asyncio
async def test_a_default_that_did_not_move_writes_nothing():
"""Called on every boot, so it has to be idempotent — otherwise the
history fills with rows saying the release shipped the same number again,
and a history nobody can skim is one nobody reads."""
current = {(n, d): (rt.get_surface(n).floor_default if d == "floor"
else float(rt.get_surface(n).budget_default))
for n in rt.surface_names() for d in ("floor", "budget")}
session = _release_rows(current)
with patch.object(rt, "async_session", MagicMock(return_value=session)):
written = await rt.record_release_defaults()
assert written == []
session.add.assert_not_called()
session.commit.assert_not_called()
@pytest.mark.asyncio
async def test_a_moved_default_is_recorded_with_both_values():
"""The event the whole task is about, and it carries what it moved FROM —
without that a reader knows a change happened and nothing about whether
the old sample can be pooled with the new one."""
name = rt.surface_names()[0]
s = rt.get_surface(name)
current = {(n, d): (rt.get_surface(n).floor_default if d == "floor"
else float(rt.get_surface(n).budget_default))
for n in rt.surface_names() for d in ("floor", "budget")}
current[(name, "floor")] = s.floor_default - 0.04 # what the last release shipped
session = _release_rows(current)
with patch.object(rt, "async_session", MagicMock(return_value=session)):
written = await rt.record_release_defaults()
moved = [w for w in written if w["surface"] == name and w["dial"] == "floor"]
assert len(moved) == 1
assert moved[0]["baseline"] is False
assert moved[0]["old_value"] == pytest.approx(s.floor_default - 0.04)
assert moved[0]["new_value"] == pytest.approx(s.floor_default)
@pytest.mark.asyncio
async def test_a_release_row_belongs_to_no_account():
"""`user_id IS NULL`, because no user did this.
A release acts on every account that has not overridden the dial. Writing
one row per user would both multiply the row and misattribute it, and the
readers take the newest of (this user's change, the release's) — which only
works if the release's is distinguishable.
"""
session = _release_rows({})
with patch.object(rt, "async_session", MagicMock(return_value=session)):
await rt.record_release_defaults()
added = [c.args[0] for c in session.add.call_args_list]
assert added
assert all(e.user_id is None for e in added)
assert all(e.actor == rt.RELEASE_ACTOR for e in added)
@pytest.mark.asyncio
async def test_every_release_row_states_why_it_exists():
"""`reason` is the guardrail on this table, and a row written by machinery
is the one most likely to arrive blank."""
session = _release_rows({})
with patch.object(rt, "async_session", MagicMock(return_value=session)):
await rt.record_release_defaults()
added = [c.args[0] for c in session.add.call_args_list]
assert all(e.reason and e.reason.strip() for e in added)
assert all(e.surface in e.reason for e in added)
@pytest.mark.asyncio
async def test_a_baseline_is_not_reported_as_a_floor_move():
"""`floor_moves_since` is what suspends the band check, so it must ask for
a genuine move — `old_value IS NOT NULL` — rather than for any row."""
from datetime import datetime, timezone
session = make_mock_session()
result = MagicMock()
result.scalars.return_value.all.return_value = []
session.execute = AsyncMock(return_value=result)
with patch.object(rt, "async_session", MagicMock(return_value=session)):
out = await rt.floor_moves_since(datetime.now(timezone.utc))
assert out == {}
# The filter is the whole correctness argument; assert it is in the query.
stmt = str(session.execute.call_args.args[0])
assert "old_value IS NOT NULL" in stmt
assert "dial" in stmt
tests/test_retrieval_warnings.py
+181 -1
View File
@@ -49,9 +49,10 @@ def src(**kw) -> dict:
def warn(sources, usage=None, rule_usage=None, floors=None,
min_calls=N, epsilon=EPS) -> list[dict]:
min_calls=N, epsilon=EPS, floor_moves=None) -> list[dict]:
return _compute_warnings(
sources, usage or {}, rule_usage or {}, floors or {}, min_calls, epsilon,
floor_moves or {},
)
@@ -266,3 +267,182 @@ def test_a_point_seen_only_in_usage_counts_as_having_emitted() -> None:
])
def test_a_setting_falls_back_rather_than_raising(raw, fallback, want) -> None:
assert _num(raw, fallback) == want
# ── read_and_unacted / outcomes_never_recorded (#4213, milestone 419) ─────
#
# The pair exists because ZERO OUTCOMES IS AMBIGUOUS, and getting that wrong
# would have been this milestone's own failure mode in miniature: a window
# with no outcome rows cannot tell "every rule was ignored" from "nothing
# reports outcomes yet". Reporting the first when the truth is the second
# manufactures a finding out of an unwired feature — #3311, where a statistic
# that could not vary was read as a fact about the corpus.
def ru(**kw) -> dict:
base = {
"distinct_rules_surfaced": 0, "distinct_rules_pulled": 0,
"distinct_rules_acted": 0, "applied": 0, "departed": 0,
}
base.update(kw)
return base
def test_rules_opened_with_no_outcome_machinery_running_says_so() -> None:
"""The cold-instrument case, which is what an install looks like the day
this ships. It must NOT read as "47 rules ignored"."""
ws = warn({}, rule_usage=ru(distinct_rules_pulled=47))
assert "outcomes_never_recorded" in codes(ws)
assert "read_and_unacted" not in codes(ws)
[w] = [w for w in ws if w["code"] == "outcomes_never_recorded"]
assert w["numbers"]["opened"] == 47
# The distinction is in the prose, because the prose is what gets read.
assert "does NOT mean they were ignored" in w["detail"]
def test_once_outcomes_exist_the_unacted_rules_are_named() -> None:
"""The instrument is live — some rules recorded an outcome — so the ones
that did not are a real finding rather than an artefact."""
ws = warn({}, rule_usage=ru(
distinct_rules_pulled=20, distinct_rules_acted=6, applied=5, departed=2,
))
assert "read_and_unacted" in codes(ws)
assert "outcomes_never_recorded" not in codes(ws)
[w] = [w for w in ws if w["code"] == "read_and_unacted"]
assert w["numbers"]["unacted"] == 14
assert w["numbers"]["opened"] == 20 and w["numbers"]["acted"] == 6
assert w["numbers"]["applied"] == 5 and w["numbers"]["departed"] == 2
def test_a_departure_alone_is_enough_to_warm_the_instrument() -> None:
"""Departures count as outcomes. An install whose every recorded outcome
is a departure is saying something loudly, and must not be mistaken for
one that records nothing."""
ws = warn({}, rule_usage=ru(
distinct_rules_pulled=9, distinct_rules_acted=2, departed=3,
))
assert "read_and_unacted" in codes(ws)
assert "outcomes_never_recorded" not in codes(ws)
def test_every_opened_rule_acted_on_reports_nothing() -> None:
ws = warn({}, rule_usage=ru(
distinct_rules_pulled=4, distinct_rules_acted=4, applied=4,
))
assert "read_and_unacted" not in codes(ws)
assert "outcomes_never_recorded" not in codes(ws)
def test_no_rules_opened_at_all_reports_neither() -> None:
"""Silence is not a finding. A window where nothing was opened has nothing
to say about outcomes, and saying it anyway would put a warning on every
fresh install (rule 115)."""
ws = warn({}, rule_usage=ru(distinct_rules_surfaced=12))
assert "read_and_unacted" not in codes(ws)
assert "outcomes_never_recorded" not in codes(ws)
def test_an_absent_rule_usage_block_is_not_a_finding() -> None:
"""A failed rule-usage read leaves the keys missing or zero. Neither may
become a warning, because a warning computed over rows that could not be
loaded describes the outage, not the corpus (#2663)."""
assert "read_and_unacted" not in codes(warn({}, rule_usage={}))
assert "outcomes_never_recorded" not in codes(warn({}, rule_usage={}))
assert "outcomes_never_recorded" not in codes(
warn({}, rule_usage={"rule_usage_failed": True})
)
# ── floor_moved_mid_window (#4225) ────────────────────────────────────────
#
# WHY THE BAND CHECK IS SUSPENDED RATHER THAN SOFTENED.
#
# `band_hugs_floor` asks whether the scores are piled on the bar. That needs
# the scores and the bar to come from the same regime, and across a floor
# change they do not — the comparison silently becomes one between two
# populations.
#
# It announced itself when the change was a RAISE: on the instance this was
# found on, `write_path_rule` went 0.68 -> 0.72 as a shipped default inside
# the window, and p10 computed over calls made under the old bar came out
# BELOW the new floor. The readout printed a band "-0.0216 above" its floor.
#
# A LOWERED floor is the dangerous one, because it hides: the gap comes out
# comfortably positive and reads as a clean bill of health on a sample that
# half predates the bar being judged. Both directions are pinned below.
MOVED = "2026-09-02T00:00:00+00:00"
def test_a_floor_that_moved_in_the_window_suspends_the_band_check() -> None:
ws = warn({"auto_inject": src(calls=100, zero_result_calls=5, p10=0.705)},
floors={"auto_inject": 0.70}, floor_moves={"auto_inject": MOVED})
assert "floor_moved_mid_window" in codes(ws, "auto_inject")
assert "band_hugs_floor" not in codes(ws), (
"a suspended check must not also answer — the two never accompany "
"each other, or the reader gets a number and a warning about it"
)
def test_the_impossible_negative_gap_is_not_printed_at_all() -> None:
"""The symptom that exposed this: p10 BELOW the floor that gates the arm.
Arithmetically impossible inside one population, and the sentence built
from it ("only -0.0216 above") is not one anybody can act on.
"""
ws = warn({"auto_inject": src(calls=100, zero_result_calls=5, p10=0.6984)},
floors={"auto_inject": 0.72}, floor_moves={"auto_inject": MOVED})
assert "band_hugs_floor" not in codes(ws)
assert not any(w.get("numbers", {}).get("gap", 0) < 0 for w in ws)
def test_a_lowered_floor_is_suspended_too_though_its_gap_looks_healthy() -> None:
"""The direction that does NOT announce itself.
A gap of 0.10 reads as a comfortable margin. It is computed over calls
half of which were made under a different bar, so it is not a margin at
all — and nothing in the number says so.
"""
ws = warn({"auto_inject": src(calls=100, zero_result_calls=5, p10=0.80)},
floors={"auto_inject": 0.70}, floor_moves={"auto_inject": MOVED})
assert "floor_moved_mid_window" in codes(ws, "auto_inject")
def test_a_floor_that_did_not_move_still_gets_judged() -> None:
"""The mirror error, and the expensive one: suspending on nothing would
retire a working check."""
ws = warn({"auto_inject": src(calls=100, zero_result_calls=5, p10=0.705)},
floors={"auto_inject": 0.70}, floor_moves={})
assert "band_hugs_floor" in codes(ws, "auto_inject")
assert "floor_moved_mid_window" not in codes(ws)
def test_only_the_arm_that_moved_is_suspended() -> None:
"""Surfaces are judged independently; one arm's release change says
nothing about another's sample."""
ws = warn(
{"auto_inject": src(calls=100, zero_result_calls=5, p10=0.705),
"write_path": src(calls=100, zero_result_calls=5, p10=0.705)},
floors={"auto_inject": 0.70, "write_path": 0.70},
floor_moves={"auto_inject": MOVED},
)
assert "floor_moved_mid_window" in codes(ws, "auto_inject")
assert "band_hugs_floor" in codes(ws, "write_path")
def test_the_warning_says_when_and_what_to_do_about_it() -> None:
"""A finding with no remedy is a complaint. The reader needs the date, so
they can ask again with a window that starts after it."""
w = next(w for w in warn(
{"auto_inject": src(calls=100, zero_result_calls=5, p10=0.705)},
floors={"auto_inject": 0.70}, floor_moves={"auto_inject": MOVED},
) if w["code"] == "floor_moved_mid_window")
assert w["numbers"]["moved_at"] == MOVED
assert MOVED in w["detail"] and "days" in w["detail"]
def test_a_quiet_arm_is_not_suspended_either_way() -> None:
"""Below `min_calls` neither check runs — a moved floor does not promote
an arm nobody used into something worth a line."""
ws = warn({"auto_inject": src(calls=1, zero_result_calls=0, p10=0.705)},
floors={"auto_inject": 0.70}, floor_moves={"auto_inject": MOVED})
assert "floor_moved_mid_window" not in codes(ws)
tests/test_rule_opened_ledger.py
+7
View File
@@ -138,6 +138,13 @@ def test_an_event_with_nothing_usable_records_nothing_and_still_exits_zero(event
# What a compaction clears — including `.opened.ids`, whose claim is the one
# that would be worst to get wrong — is asserted against the running hook in
# tests/test_session_ledger_clear.py, so there is one home for it.
#
# Since #4217 the recorder writes TWO files and only this one is cleared.
# `.opened.ids` says "this context holds rule 156", which a compaction makes
# false, and three hooks read it to decide whether to stay quiet. Its twin
# `.opened.keep.ids` says "rule 156 was opened", which a compaction does not
# touch, and the session-end readout is built on that. The split is tested
# where the sweep is.
def test_the_recorder_is_registered_on_the_get_rule_tool():
tests/test_rule_usage_wiring.py
+8 -3
View File
@@ -508,6 +508,11 @@ async def test_the_prompt_arm_says_nothing_when_asked_nothing():
stack.enter_context(patch.object(pc, "record_rule_surfaced", MagicMock()))
out = await pc.build_prompt_rule_hint(1, " ")
# NO `checkpoint` KEY, and that is the distinction rather than an
# oversight (#4214). The two ACT arms can hold a call because there is
# a composed act to hold; this arm fires on the operator's message,
# before anything has been decided, so there is nothing to put a rule
# in front of. A checkpoint here would have to guess at an act.
assert out == {"context": "", "rule_ids": []}
search.assert_not_called()
log.assert_not_called()
@@ -653,7 +658,7 @@ async def test_an_empty_command_asks_the_ranker_nothing():
stack.enter_context(patch.object(pc, "record_rule_surfaced", rec))
out = await pc.build_tool_rule_hint(1, "Bash", " ")
assert out == {"context": "", "rule_ids": []}
assert out == {"context": "", "rule_ids": [], "checkpoint": {}}
search.assert_not_called()
rec.assert_not_called()
@@ -669,7 +674,7 @@ async def test_the_tool_arm_fails_open():
AsyncMock(side_effect=RuntimeError("boom"))))
out = await pc.build_tool_rule_hint(1, "Bash", "docker compose up -d")
assert out == {"context": "", "rule_ids": []}
assert out == {"context": "", "rule_ids": [], "checkpoint": {}}
@pytest.mark.asyncio
@@ -833,7 +838,7 @@ async def test_the_tool_arm_logs_the_call_that_found_nothing():
log, rec = MagicMock(), MagicMock()
out = await _run_tool_arm([], rec, retrieval_log=log)
assert out == {"context": "", "rule_ids": []}
assert out == {"context": "", "rule_ids": [], "checkpoint": {}}
assert log.call_count == 1
assert log.call_args.kwargs["source"] == "pre_tool_rule"
assert log.call_args.kwargs["results"] == []
tests/test_services_backup.py
+184 -32
View File
@@ -2,10 +2,14 @@
This is the no-database lane, so these cover the parts that need none: the
version/coverage constants, the pure row helpers, and the export dict shape
(via a mocked session). The full FK-remapping round-trip needs real Postgres
and belongs in a `@pytest.mark.integration` module — it is not written yet,
which is why every row helper here is a plain function that can be tested
without a session.
(via a mocked session), plus the two COLUMN GUARDS — one per direction — which
need only a stand-in and a builder.
The full FK-remapping round-trip needs real Postgres and lives in the
`test_integration_backup_*_roundtrip.py` modules, which drive the real
`restore_full_backup`. That sentence used to read "it is not written yet";
four of those files exist now, the newest covering the three `code_shapes`
columns the import guard below found missing.
"""
from datetime import datetime, timezone
from types import SimpleNamespace
@@ -188,8 +192,17 @@ def _stand_in(model):
of a dict. Typed rather than a bare instance because the helpers call
`.isoformat()` on the timestamps, which `None` does not have.
"""
import itertools
import sqlalchemy as sa
# DISTINCT integers, not a constant 1. The import guard feeds this row's
# exported form to the builder, and several builders reject a self-edge —
# a supersession or a rule relation whose two ends are the same id is not
# a weaker claim, it is a row pointing at itself. With one value shared by
# every column those builders would refuse a legitimate stand-in and the
# guard would read as a fixture bug. Only the KEYS matter to either guard.
ints = itertools.count(1)
row = model()
for column in model.__table__.columns:
t = column.type
@@ -200,7 +213,7 @@ def _stand_in(model):
elif isinstance(t, sa.Boolean):
value = False
elif isinstance(t, sa.Integer):
value = 1
value = next(ints)
elif isinstance(t, sa.ARRAY) or isinstance(getattr(t, "impl", None), sa.ARRAY):
value = []
elif isinstance(t, (sa.Text, sa.String)):
@@ -245,40 +258,179 @@ def test_every_column_is_exported_or_declared_excluded(table):
)
def test_the_usage_importer_restores_the_reading_project():
"""The column guard above checks the EXPORT side only.
def _import_guard_targets():
"""table -> (model, export helper, import builder).
A column can be exported faithfully and then dropped on the way back in,
which restores a backup that reports success and has quietly lost a
dimension — #3182's failure mode, one direction over. There is no general
import-side guard yet; this covers the column #4196 added, by source
inspection, because the behavioural path needs Postgres.
It also pins the DEGRADE. `code_shape_events` skips a row whose project
will not map, because a shape event without its project says nothing. A
usage event is not like that: the project is optional by design and null
already means "not reported", so an unmappable one must restore as
unreported rather than vanish — dropping it would lose a real pull and
deflate the very pull-through this table exists to report.
Built from the export registry so the two cannot drift apart: a table with
a row helper and no builder shows up here as a KeyError with its own name
in it, rather than as a table nobody checks.
"""
import inspect
builders = {
"users": backup._build_user,
"projects": backup._build_project,
"milestones": backup._build_milestone,
"notes": backup._build_note,
"task_logs": backup._build_task_log,
"note_drafts": backup._build_note_draft,
"note_versions": backup._build_note_version,
"settings": backup._build_setting,
"rulebooks": backup._build_rulebook,
"rulebook_topics": backup._build_topic,
"rules": backup._build_rule,
"rule_versions": backup._build_rule_version,
"systems": backup._build_system,
"canonical_systems": backup._build_canonical_system,
"record_systems": backup._build_record_system,
"note_supersessions": backup._build_note_supersession,
"rule_relations": backup._build_rule_relation,
"note_usage_events": backup._build_usage_event,
"rule_usage_events": backup._build_rule_usage_event,
"retrieval_tuning_events": backup._build_retrieval_tuning_event,
"design_systems": backup._build_design_system,
"design_tokens": backup._build_design_token,
"repo_bindings": backup._build_repo_binding,
"code_shapes": backup._build_code_shape,
"code_shape_events": backup._build_code_shape_event,
"code_shape_uses": backup._build_code_shape_use,
}
return {
table: (model, helper, builders[table])
for table, (model, helper) in _column_guard_targets().items()
}
src = inspect.getsource(backup._restore_v2)
marker = 'for ev in data.get("note_usage_events", []):'
assert marker in src, "the usage import loop moved; this guard is blind"
block = src[src.index(marker):][:1200]
assert 'ev.get("project_id")' in block, (
"the usage importer drops project_id — a restore would report success "
"and come back without the reading project"
def _everything_maps(row: dict) -> "backup._Maps":
"""Id maps in which every id the row mentions resolves.
The guard is about which COLUMNS a builder sets, not about what it does
when a foreign key is missing — that is the skip/degrade question, which
the tests below ask directly. So every lookup succeeds here, and a builder
that returned None would be a bug in the fixture rather than a finding.
"""
maps = backup._Maps()
ids = {v for v in row.values() if isinstance(v, int)} | {0, 1}
for name in ("users", "projects", "milestones", "notes", "rulebooks",
"topics", "rules", "systems", "design_systems", "shapes"):
getattr(maps, name).update({i: i + 1000 for i in ids})
# `canonical_slug` only, never `slug`: a canonical_systems row is built
# exactly when its slug is NOT already known to the destination, so
# seeding it from the row's own slug would make that builder skip.
slug = row.get("canonical_slug")
if slug:
maps.canonical_by_slug[slug] = 7
return maps
@pytest.mark.parametrize("table", sorted(_import_guard_targets()))
def test_every_exported_column_is_imported_or_declared_excluded(table):
"""THE COLUMN GUARD, THE OTHER WAY (#4197).
The export guard above makes a dropped column unexpressible on the way
OUT. Nothing watched the way back IN, and that is the worse half: an
export gap leaves an obviously thin backup, an import gap means holding a
complete, correct file and restoring an incomplete database from it, with
a success message.
It was one-sided because the code was — `_restore_v2` built every model
inline, so there was no per-table unit to hand a stand-in to. There is
now, and this composes the two halves end to end: export a stand-in row,
feed THAT dict to the builder, and read which columns the constructed
model actually received.
What it caught on the first run: `code_shapes` was exporting `reason_code`,
`recheck_at` and `diverges_from` and importing none of them. Every judged
shape would have restored with its verdict and without the code for why,
every recheck flag cleared, and every divergence pointer gone.
"""
model, helper, builder = _import_guard_targets()[table]
[row] = helper([_stand_in(model)])
built = builder(row, _everything_maps(row))
assert built is not None, (
f"{table}: the builder skipped a row whose ids all resolve — "
"the guard fixture is wrong, or the builder is"
)
assert "project_id_map" in block, (
"project_id must be re-mapped; a raw id points at whatever project "
"happens to hold that number in the destination install"
received = set(built.__dict__) - {"_sa_instance_state"}
columns = {c.name for c in model.__table__.columns}
missing = columns - received
declared = backup._IMPORT_COLUMN_EXCLUSIONS[table]
assert missing == declared, (
f"{table}: imported columns and _IMPORT_COLUMN_EXCLUSIONS disagree.\n"
f" dropped but not declared: {sorted(missing - declared)}\n"
f" declared but imported anyway: {sorted(declared - missing)}"
)
assert "continue" not in block.split('project_id=')[1][:200], (
"an unmappable project must degrade to None, not skip the row"
def test_the_import_guard_covers_every_table_the_export_guard_does():
"""The two registries have to hold the same tables, or a table can be
guarded in one direction and silently unguarded in the other — which is
the state this whole pair of guards exists to end."""
assert set(_import_guard_targets()) == set(_column_guard_targets())
assert set(backup._IMPORT_COLUMN_EXCLUSIONS) == set(_column_guard_targets())
def test_an_unmappable_project_degrades_a_usage_event_and_skips_a_shape_event():
"""The skip-or-degrade choice is per table, and both answers are right.
`code_shape_events` SKIPS a row whose project will not map — a shape event
without its project says nothing. `note_usage_events` DEGRADES to None —
the project is optional by design, null already means "not reported", and
dropping the row would lose a real pull and deflate the very pull-through
the table exists to report.
Until #4197 this was asserted by reading the source of `_restore_v2` with
`inspect.getsource`, because there was no unit to call. Now there is.
"""
maps = backup._Maps()
maps.notes[5] = 55
maps.users[9] = 99
maps.shapes[3] = 33
# project 7 is deliberately absent from maps.projects
event = backup._build_usage_event(
{"note_id": 5, "user_id": 9, "project_id": 7,
"event": "pulled", "source": "search"},
maps,
)
assert event is not None, "an unmappable project must not drop a real pull"
assert event.project_id is None
assert event.note_id == 55
shape_event = backup._build_code_shape_event(
{"shape_id": 3, "project_id": 7, "path": "a.py", "symbol": "f"}, maps,
)
assert shape_event is None, (
"a shape event whose project did not map says nothing and must be "
"skipped, not restored project-less"
)
def test_a_shapes_divergence_pointer_is_remapped_not_carried():
"""`diverges_from` is a FK to notes.id, like `snippet_id`. Carrying the
source id would point at whatever snippet took that number in the
destination — wrong rather than missing, and nothing downstream could
tell. It was dropped entirely until #4197; restoring it raw would have
been the worse fix."""
maps = backup._Maps()
maps.projects[1] = 11
maps.notes[4] = 44
shape = backup._build_code_shape(
{"project_id": 1, "status": "exempt", "diverges_from": 4,
"reason_code": "scoped-css", "recheck_at": "2026-01-01T00:00:00+00:00"},
maps,
)
assert shape.diverges_from == 44, "not re-mapped through the note map"
assert shape.reason_code == "scoped-css"
assert shape.recheck_at is not None
# A pointer whose target did not survive lands NULL rather than dangling.
orphan = backup._build_code_shape(
{"project_id": 1, "status": "exempt", "diverges_from": 999}, maps,
)
assert orphan.diverges_from is None
def test_the_column_guard_covers_every_table_with_a_row_helper():
tests/test_services_plugin_context.py
+34
View File
@@ -518,3 +518,37 @@ async def test_write_path_labels_a_non_snippet_hit_with_its_kind():
assert '[similar 0.72] "debounce helper"' in ctx # the snippet does not
# The header now names the right opener for each kind.
assert "get_task(id)" in ctx and "get_snippet(id)" in ctx
# ── The config stand-in cannot fall behind the real one (#4214) ───────────
@pytest.mark.asyncio
async def test_the_config_stand_in_carries_every_key_the_real_one_does():
"""THE GUARD `writepath_cfg`'s DOCSTRING ALREADY CLAIMED AND DID NOT HAVE.
Three arms read their numbers out of that dict inside a fail-open
`except`, so a missing key does not raise where anyone can see it — the
arm silently becomes a no-op, which is indistinguishable from the arm
working and finding nothing. The helper derives its SURFACE keys from the
registry to prevent exactly that, and then #4214 added a key that is
deliberately not a surface: the whole derivation missed it, ten tests went
red at once, and the diagnosis cost a CI round.
Asserting the KEY SETS match, not the values: the stand-in exists to let a
test set different numbers.
"""
from unittest.mock import AsyncMock, patch
from scribe.services import plugin_context as pc
from tests.helpers import writepath_cfg
with patch.object(pc, "get_setting", AsyncMock(return_value="0.6")), \
patch.object(pc, "floor_for", AsyncMock(return_value=0.6)), \
patch.object(pc, "budget_for", AsyncMock(return_value=3)):
real = await pc.get_writepath_config(1)
assert set(writepath_cfg()) == set(real), (
"tests/helpers.writepath_cfg has fallen behind get_writepath_config; "
"a key the real config has and the stand-in does not turns an arm "
"into a silent no-op under test"
)
tests/test_services_retrieval_telemetry.py
+66
View File
@@ -643,6 +643,72 @@ async def test_ambient_alone_reports_no_ratio(_dispose_engine):
await cleanup()
@pytest.mark.integration
@pytest.mark.asyncio
async def test_the_outcome_counts_come_back_split(_dispose_engine):
"""A pull says the rule was read; an outcome says it changed something
(#4213). Integration rather than a mock because `distinct_rules_acted` is
a count(distinct) with an IN over a column that carries no CHECK — the
kind of SQL shape #2663 was, where a mock agrees with whatever the code
does including nothing.
`applied` and `departed` stay APART. Summed they would say "an outcome was
recorded", which is true of both and useful about neither: a departure is
evidence about the rule, an application is evidence about the agent.
"""
from scribe.services.retrieval_telemetry import retrieval_summary
cleanup = await _rule_events(990016, [
# Opened and followed.
(5301, "surfaced", "write_path_rule"),
(5301, "pulled", "mcp_get_rule"),
(5301, "applied", "mcp_rule_outcome"),
# Opened and deliberately departed from.
(5302, "surfaced", "write_path_rule"),
(5302, "pulled", "mcp_get_rule"),
(5302, "departed", "mcp_rule_outcome"),
# Opened, and nothing after it. The state the milestone exists for,
# and the one that is counted by its ABSENCE.
(5303, "surfaced", "write_path_rule"),
(5303, "pulled", "mcp_get_rule"),
])
try:
ru = (await retrieval_summary(990016, days=30))["rule_usage"]
assert ru["applied"] == 1
assert ru["departed"] == 1
assert ru["distinct_rules_acted"] == 2
assert ru["distinct_rules_pulled"] == 3
# An outcome is NOT a pull. If `applied` leaked into the pull counters
# the silently-unchanged rule would vanish into a compliant-looking
# total, which is the exact confusion #4212 was opened to end.
assert ru["pulled"] == 3
finally:
await cleanup()
@pytest.mark.integration
@pytest.mark.asyncio
async def test_an_outcome_is_never_split_by_ambient(_dispose_engine):
"""`source` on an outcome row names the door the outcome came through, not
a ranker, so the surfaced/ambient split has nothing to say about it. An
outcome recorded from an unranked source must still count."""
from scribe.services.retrieval_telemetry import retrieval_summary
cleanup = await _rule_events(990017, [
(5401, "surfaced", "session_start"),
(5401, "pulled", "mcp_get_rule"),
(5401, "applied", "some_door_nobody_has_ranked"),
])
try:
ru = (await retrieval_summary(990017, days=30))["rule_usage"]
assert ru["applied"] == 1
assert ru["distinct_rules_acted"] == 1
assert ru["ambient"] == 1, "the surfacing was ambient; the outcome is not"
finally:
await cleanup()
# ── Window coverage (#3712) ────────────────────────────────────────────
#
# A counter added last week, read over a 30-day window, reports a real count
tests/test_services_rule_usage.py
+137 -1
View File
@@ -6,7 +6,9 @@ where a mistake is silent rather than loud.
"""
import pytest
from scribe.models.rule_usage import PULLED, SURFACED, RuleUsageEvent
from scribe.models.rule_usage import (
APPLIED, DEPARTED, PULLED, SURFACED, RuleUsageEvent,
)
from scribe.services import rule_usage
@@ -101,6 +103,9 @@ def test_the_zero_readout_names_every_key():
"surfaced_count": 0,
"ambient_count": 0,
"pull_count": 0,
"applied_count": 0,
"departed_count": 0,
"last_outcome_at": None,
"last_surfaced_at": None,
"last_pulled_at": None,
}
@@ -255,3 +260,134 @@ async def test_a_preloaded_rule_does_not_read_as_a_ranked_surfacing(_dispose_eng
delete(RuleUsageEvent).where(RuleUsageEvent.user_id == 990021)
)
await s.commit()
# ── the outcome stream (#4212, milestone 419) ─────────────────────────────
#
# What these guard is a distinction, not a payload. Before this existed, a
# rule read and obeyed and a rule read and ignored left byte-identical
# telemetry, so the readout could not name the failure the whole milestone
# was opened on. The tests that matter most below are the ones asserting
# that a REASONLESS DEPARTURE IS NEVER WRITTEN, and that an unacted rule is
# a state in its own right rather than the absence of one.
def test_an_applied_outcome_needs_no_argument(captured):
"""Following a rule is the ordinary case. Charging prose for it would
make the cheap event expensive, and an expensive event stops being
recorded — which costs the whole measurement."""
rule_usage.record_rule_outcome(
user_id=7, rule_id=156, outcome=APPLIED, source="mcp_rule_outcome"
)
[batch] = captured
assert batch == [{
"user_id": 7, "rule_id": 156, "event": APPLIED,
"source": "mcp_rule_outcome", "detail": None,
}]
def test_a_departure_carries_its_reason(captured):
rule_usage.record_rule_outcome(
user_id=7, rule_id=156, outcome=DEPARTED, source="mcp_rule_outcome",
detail=" the integration lane has no registry credentials ",
)
[batch] = captured
assert batch[0]["event"] == DEPARTED
assert batch[0]["detail"] == "the integration lane has no registry credentials"
@pytest.mark.parametrize("reason", ["", " ", "\n", None])
def test_a_departure_with_no_reason_is_never_written(captured, reason):
"""THE ONE THAT MATTERS. A `departed` row without its why reads back as a
miss, so writing one would collapse the two states this table exists to
separate — silently, in the readout, where nobody would see it happen.
Dropped and reported, never stored: telemetry that lies is worse than
telemetry that is absent (#2663)."""
rule_usage.record_rule_outcome(
user_id=7, rule_id=156, outcome=DEPARTED,
source="mcp_rule_outcome", detail=reason or "",
)
assert captured == []
def test_an_unknown_outcome_is_never_written(captured):
"""Including the one somebody will reach for. There is no `ignored`
event by design — see `record_rule_outcome` — and a caller inventing one
must not get a row that reads as though the state were measurable."""
for bogus in ("ignored", "skipped", "surfaced", "", "APPLIED "):
rule_usage.record_rule_outcome(
user_id=7, rule_id=156, outcome=bogus, source="mcp_rule_outcome"
)
assert captured == []
def test_an_outcome_row_is_one_row(captured):
"""A judgement is about one rule. Unlike a surfacing, which delivers a
whole hint at once, there is no batch shape to get wrong here — asserted
so that a later 'helpful' bulk variant has to change a test that says
why."""
rule_usage.record_rule_outcome(
user_id=7, rule_id=1, outcome=APPLIED, source="mcp_rule_outcome"
)
[batch] = captured
assert len(batch) == 1
# ── the four states, read off the aggregate ───────────────────────────────
def _usage(**kw):
base = rule_usage.empty_rule_usage()
base.update(kw)
return base
def test_a_rule_surfaced_and_never_opened_is_unread():
assert rule_usage.outcome_state(_usage(surfaced_count=4)) == rule_usage.UNREAD
def test_a_rule_opened_and_acted_on_is_followed():
assert rule_usage.outcome_state(
_usage(surfaced_count=4, pull_count=1, applied_count=1)
) == rule_usage.FOLLOWED
def test_a_rule_opened_and_departed_from_is_departed():
assert rule_usage.outcome_state(
_usage(surfaced_count=4, pull_count=1, departed_count=1)
) == rule_usage.DEPARTED_FROM
def test_a_rule_opened_and_never_acted_on_is_unacted() -> None:
"""THE STATE THAT DID NOT EXIST, and the reason for the milestone. Not
"no data": the rule was surfaced, deliberately opened, and then left no
trace of having mattered. Until now that was arithmetically identical to
compliance, which is why nothing could report it."""
assert rule_usage.outcome_state(
_usage(surfaced_count=4, pull_count=2)
) == rule_usage.UNACTED
def test_unacted_and_followed_are_not_the_same_reading():
"""Stated as its own test because it IS the milestone in one line. If a
change ever makes these two agree, the measurement is gone and every
other test here would still pass."""
opened_only = _usage(surfaced_count=4, pull_count=2)
opened_and_applied = _usage(surfaced_count=4, pull_count=2, applied_count=1)
assert rule_usage.outcome_state(opened_only) != rule_usage.outcome_state(
opened_and_applied
)
def test_a_departure_outranks_an_application():
"""A rule both applied and argued with is a rule someone argued with, and
the argument is the half worth surfacing. Reporting it as plain
compliance would bury the one row a reader most wants to see."""
assert rule_usage.outcome_state(
_usage(pull_count=3, applied_count=5, departed_count=1)
) == rule_usage.DEPARTED_FROM
def test_the_state_reads_the_aggregate_the_readout_already_returns():
"""`outcome_state` takes `usage_for_rules`' own shape, so the badge, the
readout and any later session summary cannot disagree about what
"followed" means — the drift #3246 found across the rules system."""
assert rule_usage.outcome_state(rule_usage.empty_rule_usage()) == rule_usage.UNREAD
tests/test_session_ledger_clear.py
+33 -2
View File
@@ -75,8 +75,12 @@ def _swept_dirs() -> set[str]:
return set(line.group(1).split())
def _run_session_start(source: str, tmp: Path) -> Path:
"""Run the SessionStart hook for real, with the ledger directories filled."""
def _run_session_start(source: str, tmp: Path, extra: list[str] | None = None) -> Path:
"""Run the SessionStart hook for real, with the ledger directories filled.
`extra` names further files to plant in `scribe-priorart` — used to put
an evidence twin in front of the sweep.
"""
for tool in ("bash",):
if shutil.which(tool) is None:
pytest.skip(f"hook runtime tool {tool!r} not installed")
@@ -88,6 +92,8 @@ def _run_session_start(source: str, tmp: Path) -> Path:
(state / f"s1{suffix}").write_text("42\t1789600000\n")
# Not a ledger: an outage marker that must outlive the clear.
(tmp / "scribe-priorart" / "s1.unreached").write_text("1\n")
for name in extra or ():
(tmp / "scribe-priorart" / name).write_text("42\t1789600000\n")
env = {"PATH": os.environ["PATH"], "HOME": str(tmp), "TMPDIR": str(tmp)}
out = subprocess.run(
@@ -226,3 +232,28 @@ def test_the_clear_is_derived_and_not_a_list_of_names():
assert suffix not in block, (
f"the clear names {suffix} again — a list, not a convention"
)
def test_the_evidence_twin_is_not_swept_with_them(tmp_path):
"""The second thing that survives, and for the `.unreached` reason (#4217).
`scribe_rules_append` writes each ledger twice: `<sid>.<kind>.ids`, which
says what this context HOLDS and must be forgotten here, and
`<sid>.<kind>.keep.ids`, which says what HAPPENED and no compaction makes
untrue. The session-end readout is built on the second, and it runs AT the
compaction — so a sweep that took both would leave the readout reporting
only the stretch since the last one, while reading as though it had
reported the session.
Measured before the split, on the instance this was built on: six sessions,
208 `get_rule` calls, 3 surviving ledger entries.
The test above asserts every exclusion ledger dies; this asserts its twin
does not. Neither is complete alone, and the sweep is one glob away from
either mistake.
"""
root = _run_session_start("compact", tmp_path, extra=["s1.rules.keep.ids"])
assert (root / "scribe-priorart" / "s1.rules.keep.ids").exists(), (
"the evidence twin was swept with the exclusion ledgers — the readout "
"at this seam now reports a session it cannot see"
)
tests/test_session_slippage_readout.py
+334
View File
@@ -0,0 +1,334 @@
"""Which rules fired this session, which changed an action, which did not (#4216).
WHY THIS CANNOT BE ASKED OF THE SERVER
`rule_usage_events` has no session column — it is per user over a window — so
a SESSION-scoped answer has to be assembled where a session is a thing that
exists. That is the plugin, from four ledgers four hooks already write:
.rules.ids an arm NAMED the rule (a teaser was shown)
.opened.ids the session called get_rule (#4100)
.acted.ids the session called rule_outcome (#4216, new here)
.checkpoint.ids the rule HELD an act (#4214)
EVERY LINE IS AN OBSERVED TOOL CALL. Nothing asks the model what it followed;
milestone 386 ruled that out because a model asked "did you apply rule 156?"
will say yes. These record what happened.
WHY THE COMPACTION SEAM
A rule read and left unresolved is invisible by construction — it looks
exactly like a rule that worked. The compaction is where that invisibility
becomes permanent: the turns holding the evidence are summarised away, and an
unjudged thing that survives as nothing is how a decision quietly becomes
nobody's. A PreCompact hook's stdout becomes the summariser's instructions
(#3680), so this does not say "you slipped" — it says which ids must be
carried through, which is the one thing a summary can do about it.
WHAT IS PINNED: the arithmetic, and which conditions produce which lines. NOT
pinned: the wording.
"""
from __future__ import annotations
import json
import os
import shutil
import subprocess
import time
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parents[1]
HOOKS = ROOT / "plugin" / "hooks"
DEFS = HOOKS / "scribe_defs.sh"
PRECOMPACT = HOOKS / "scribe_precompact_preserve.sh"
RECORDER = HOOKS / "scribe_record_outcome.sh"
HOOKS_JSON = HOOKS / "hooks.json"
def _need(*tools):
for t in tools:
if shutil.which(t) is None:
pytest.skip(f"hook runtime tool {t!r} not installed")
def sh(script: str) -> str:
_need("bash", "awk")
r = subprocess.run(
["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'],
capture_output=True, text=True, timeout=30,
)
assert r.returncode == 0, f"exit {r.returncode}: {r.stderr}"
return r.stdout
def ledgers(tmp_path, *, named=(), opened=(), acted=(), held=()) -> Path:
"""The four ledgers, written THROUGH THE REAL WRITERS.
Not hand-rolled files: `scribe_rules_append` is what creates the evidence
twin these read from, so a fixture that wrote the bytes itself would keep
passing if the twin stopped being written — which is the whole defect
#4217 found.
"""
d = tmp_path / "scribe-priorart"
d.mkdir(parents=True, exist_ok=True)
script = []
for name, ids in (("rules", named), ("opened", opened), ("acted", acted)):
if ids:
body = "".join(f"{i}\n" for i in ids)
script.append(
f"printf '%s' '{body}' | scribe_rules_append \"{d}/s.{name}.ids\""
)
for i in held:
script.append(f'scribe_checkpoint_allowed "{d}/s.checkpoint.ids" {i} >/dev/null')
if script:
sh("\n".join(script))
return d
def readout(d: Path) -> str:
return sh(f'scribe_slippage_lines "{d}" "s"')
# ── The arithmetic ────────────────────────────────────────────────────────
def minus(a: str, b: str) -> str:
return sh(f'scribe_ids_minus "{a}" "{b}"').strip()
def test_the_difference_keeps_order_and_drops_members():
assert minus("1 9 34 156 173", "9 34 156") == "1 173"
def test_an_empty_difference_prints_nothing():
assert minus("9 34", "9 34") == ""
def test_an_empty_minuend_is_not_an_error():
assert minus("", "9") == ""
def test_a_repeated_id_is_counted_once():
"""The ledgers are append-only, so the same rule can appear many times."""
assert minus("9 9 34 9", "") == "9 34"
# ── What the readout says ─────────────────────────────────────────────────
def test_the_readout_names_what_was_read(tmp_path):
out = readout(ledgers(tmp_path, named=[1, 9], opened=[9]))
assert "read: 9" in out
def test_a_rule_named_and_never_opened_is_named_as_such(tmp_path):
"""The arm talking to nobody. Not an accusation — a teaser skimmed past
leaves nothing behind — but it is the number that says whether the arm is
earning its place."""
out = readout(ledgers(tmp_path, named=[1, 9, 173], opened=[9]))
assert "never opened" in out
line = next(ln for ln in out.splitlines() if "never opened" in ln)
assert "1" in line and "173" in line
assert " 9" not in line.split(":")[1], "an opened rule is not also unread"
def test_a_rule_read_with_no_outcome_is_the_headline(tmp_path):
"""THE MILESTONE'S WHOLE SUBJECT. Read and unresolved is arithmetically
identical to read and followed, and this is the only place that difference
gets carried across the seam."""
out = readout(ledgers(tmp_path, named=[9, 34], opened=[9, 34], acted=[34]))
assert "READ WITH NO OUTCOME RECORDED" in out
line = next(ln for ln in out.splitlines() if "NO OUTCOME" in ln)
assert "9" in line
assert "rule_outcome" in line, "a finding with no remedy is a complaint"
def test_a_rule_that_held_an_act_is_reported_separately(tmp_path):
"""The strongest evidence a rule changed something: it stopped a call
before it ran (#4214). Kept apart from `read` because reading a rule and
having it alter what you did are different claims."""
out = readout(ledgers(tmp_path, named=[156], opened=[156], held=[156]))
assert "held an act" in out and "156" in out
def test_a_session_that_resolved_everything_makes_no_accusation(tmp_path):
"""Traffic is always reported — it is what the static instructions above
already ask for in prose. The SUBTRACTIONS are conditional, so a clean
session gets no scolding. '0 rules unresolved' on every compaction is how
a readout teaches its reader to skip it."""
out = readout(ledgers(tmp_path, named=[7], opened=[7], acted=[7]))
assert "read: 7" in out
assert "NO OUTCOME" not in out
assert "never opened" not in out
def test_a_session_no_rule_touched_says_nothing_at_all(tmp_path):
"""Rule 115's reasoning: a fresh install must not be told something is
wrong when the truth is that nothing has happened yet."""
d = tmp_path / "scribe-priorart"
d.mkdir(parents=True)
assert readout(d).strip() == ""
# ── The hook that carries it ──────────────────────────────────────────────
def run_precompact(event: dict, tmpdir: Path) -> subprocess.CompletedProcess:
_need("bash")
env = dict(os.environ)
env["TMPDIR"] = str(tmpdir)
return subprocess.run(["bash", str(PRECOMPACT)], input=json.dumps(event),
capture_output=True, text=True, timeout=30, env=env)
def test_the_compaction_hook_appends_the_readout(tmp_path):
ledgers(tmp_path, named=[1, 9], opened=[9], acted=[])
r = run_precompact({"session_id": "s", "trigger": "manual"}, tmp_path)
assert r.returncode == 0
assert "Preserve the following literally" in r.stdout, "static half intact"
assert "READ WITH NO OUTCOME RECORDED" in r.stdout
def test_the_compaction_hook_still_exits_zero_with_no_session(tmp_path):
"""An event with no session_id has no ledgers to read. The static
instructions still go out — they are the part that matters most, and
losing them because a measurement was unavailable would be the worse
trade."""
r = run_precompact({"trigger": "auto"}, tmp_path)
assert r.returncode == 0
assert "Preserve the following literally" in r.stdout
def test_the_compaction_hook_never_emits_a_json_envelope(tmp_path):
"""Regression guard on the change that added the readout: for PreCompact
the envelope is pasted into the summariser's prompt rather than
unwrapped."""
ledgers(tmp_path, named=[1], opened=[1])
r = run_precompact({"session_id": "s", "trigger": "manual"}, tmp_path)
assert not r.stdout.lstrip().startswith("{")
# ── The recorder that makes `acted` mean anything ─────────────────────────
def run_recorder(event: dict, tmpdir: Path) -> subprocess.CompletedProcess:
_need("bash")
env = {"PATH": os.environ["PATH"], "HOME": str(tmpdir), "TMPDIR": str(tmpdir)}
return subprocess.run(["bash", str(RECORDER)], input=json.dumps(event),
capture_output=True, text=True, timeout=30, env=env)
def test_declaring_an_outcome_is_recorded(tmp_path):
r = run_recorder(
{"session_id": "s", "tool_input": {"rule_id": 156, "outcome": "applied"}},
tmp_path,
)
assert r.returncode == 0
assert (tmp_path / "scribe-priorart" / "s.acted.ids").read_text().startswith("156\t")
def test_the_entry_is_stamped_like_its_siblings(tmp_path):
"""One reader ages all three ledgers, so all three carry a timestamp."""
run_recorder({"session_id": "s", "tool_input": {"rule_id": 9}}, tmp_path)
line = (tmp_path / "scribe-priorart" / "s.acted.ids").read_text().strip()
assert "\t" in line and line.split("\t")[1].isdigit()
@pytest.mark.parametrize("event", [
{}, {"session_id": "s"}, {"tool_input": {"rule_id": 9}},
{"session_id": "s", "tool_input": {"rule_id": "not-a-number"}},
])
def test_an_unusable_event_records_nothing_and_still_exits_zero(event, tmp_path):
"""A bookkeeping failure must never turn a successful tool call into a
hook error."""
r = run_recorder(event, tmp_path)
assert r.returncode == 0
assert not (tmp_path / "scribe-priorart" / "s.acted.ids").exists()
def test_the_recorder_is_registered_on_the_rule_outcome_tool():
"""A ledger nothing writes to reads as 'nothing was acted on', which is
the exact false finding this milestone exists to avoid producing."""
cfg = json.loads(HOOKS_JSON.read_text())
entries = cfg["hooks"]["PostToolUse"]
assert any(
"rule_outcome" in e.get("matcher", "")
and any("scribe_record_outcome.sh" in h["command"] for h in e["hooks"])
for e in entries
)
def test_the_recorder_is_shell_valid():
_need("bash")
subprocess.run(["bash", "-n", str(RECORDER)], check=True)
# ── Evidence outlives the compaction; exclusions do not (#4217) ────────────
#
# THE DEFECT THIS PINS, measured before it was fixed: across six sessions on
# the instance this was built on, 208 `get_rule` calls produced 3 surviving
# ledger entries. `.opened.ids` was doing two jobs with opposite lifetimes —
# "this context holds the rule" (must be forgotten at a compaction, and three
# hooks depend on that) and "this was opened" (which no compaction makes
# untrue). The sweep, correct for the first, was deleting the second.
def test_the_readout_is_unchanged_by_the_sweep(tmp_path):
"""The readout runs AT the compaction. If the sweep took its inputs it
would report only the last stretch of a session and read as though it had
reported all of it."""
d = ledgers(tmp_path, named=[9, 34, 156], opened=[9, 156], acted=[156], held=[156])
before = readout(d)
sh(f'TMPDIR="{tmp_path}" scribe_clear_session_ledgers s')
assert readout(d) == before
assert "READ WITH NO OUTCOME RECORDED: 9" in readout(d)
def test_the_sweep_still_clears_what_the_context_no_longer_holds(tmp_path):
"""The other half, and it must keep working: after a compaction the agent
genuinely does not hold what it was shown, so an arm that stayed quiet on
the strength of the old ledger would be silent about a rule the context
has lost."""
d = ledgers(tmp_path, named=[9], opened=[9])
sh(f'TMPDIR="{tmp_path}" scribe_clear_session_ledgers s')
assert not (d / "s.rules.ids").exists()
assert not (d / "s.opened.ids").exists()
assert (d / "s.opened.keep.ids").exists()
def test_the_evidence_twin_is_not_aged_out(tmp_path):
"""An exclusion ledger ages by TTL, and should: a rule named two hours ago
is not in this context. A rule OPENED two hours ago was still opened."""
d = tmp_path / "scribe-priorart"
d.mkdir(parents=True, exist_ok=True)
stale = int(time.time()) - 99999
(d / "s.opened.keep.ids").write_text(f"9\t{stale}\n")
(d / "s.rules.keep.ids").write_text(f"9\t{stale}\n")
assert "read: 9" in readout(d)
def test_every_ledger_written_through_the_appender_gets_a_twin(tmp_path):
"""Derived in one place rather than listed at the call sites — a list is
what broke this before, and the next ledger somebody adds should be born
on the right side without anyone remembering to say so."""
d = tmp_path / "scribe-priorart"
d.mkdir(parents=True, exist_ok=True)
sh(f"""printf '7\n' | scribe_rules_append "{d}/s.brandnew.ids" """)
assert (d / "s.brandnew.keep.ids").read_text().startswith("7\t")
sh(f'TMPDIR="{tmp_path}" scribe_clear_session_ledgers s')
assert (d / "s.brandnew.keep.ids").exists()
def test_a_checkpoint_budget_is_restored_by_a_compaction(tmp_path):
"""The cap counts the SWEPT file on purpose. After a compaction this
context has not read the rule, so the budget to stop an act on it is
honestly fresh — while the record that a stop already happened stays."""
d = tmp_path / "scribe-priorart"
d.mkdir(parents=True, exist_ok=True)
f = d / "s.checkpoint.ids"
sh(f'scribe_checkpoint_allowed "{f}" 156 >/dev/null')
r = subprocess.run(
["bash", "-c", f'. "{DEFS}"\nscribe_checkpoint_allowed "{f}" 156'],
capture_output=True, text=True, timeout=30,
)
assert r.returncode != 0, "a rule does not get to stop the same session twice"
sh(f'TMPDIR="{tmp_path}" scribe_clear_session_ledgers s')
sh(f'scribe_checkpoint_allowed "{f}" 156 >/dev/null')
assert (d / "s.checkpoint.keep.ids").read_text().count("156") == 2
tests/test_shape_form_gate.py
+122 -4
View File
@@ -26,8 +26,8 @@ from __future__ import annotations
import pytest
from scribe.services.shape_ledger import (
FORM_UNKNOWN, canon_form, families_conflict, forms_agree, shape_family,
shape_form, signature_in,
FORM_UNKNOWN, canon_form, comparable_siblings, dominant_canon,
families_conflict, forms_agree, shape_family, shape_form, signature_in,
)
@@ -103,8 +103,17 @@ def test_how_many_of_the_five_the_divergence_gate_actually_silences() -> None:
That is not a shortcoming of the gate, it is the limit of the signature:
at this level those four are indistinguishable from #2793's acceptance
case, where a sync `confirmDanger` beside an async confirm helper SHOULD
be flagged. Separating them needs #4204 option 2 (widen `kind`) or a
comparison of meaning."""
be flagged.
CORRECTED 2026-09-21 (#4208). This used to say separating them needs
"widen `kind` or a comparison of meaning", offering the two as
alternatives. Widening `kind` does not separate them: it buys the silence
by bucketing `fn` apart from `async-fn`, which takes the async canon out
of a sync candidate's denominator and silences #2793's acceptance case by
the identical mechanism, one layer down. Whatever separates these four
has to distinguish a helper that does the canon's JOB from one that does
not, and no signature carries that. A comparison of meaning is the only
lever, not one of two."""
canon = shape_form("async def create_note(user_id: int, ...):", "sym")
silenced = {
sig: families_conflict(shape_form(sig, "sym"), canon)
@@ -323,3 +332,112 @@ def test_the_resemblance_floor_is_above_the_retrieval_floors() -> None:
from scribe.services.shape_ledger import _RESEMBLE_MIN
assert _RESEMBLE_MIN >= 0.80
# ── The denominator, not the gate (#4208) ─────────────────────────────────
#
# `dominant_canon` is a base rate, and a base rate is only a statement about
# something if its denominator is a population somebody asked a question
# about. It counted every code symbol in a directory as one bucket: a frozen
# dataclass, a module constant and an async service unit were three
# comparable things, and "372 judged siblings" was the authority the
# divergence line spoke with.
#
# `comparable_siblings` narrows it using the SAME predicate as the gate, so
# the count and the verdict cannot drift into disagreeing about what
# comparable means.
# `_Row` above is reused rather than redefined. A second class of the same
# name here shadowed the first — same fields, different `snippet_id` default —
# and silently broke three `canon_form` tests that had been passing, which is
# a neater demonstration of this file's subject than anything it asserts.
SERVICE = "async def get_note(user_id: int, note_id: int) -> Note | None:"
HELPER = "def is_registered(source: str) -> bool:"
def test_a_type_is_not_counted_against_a_directory_of_callables() -> None:
rows = [_Row(SERVICE) for _ in range(6)] + [_Row("class Point:", snippet_id=9)]
assert len(comparable_siblings(rows, shape_form("class Point:"))) == 1
def test_a_callable_is_not_counted_against_a_dataclass() -> None:
"""The honest-denominator half, and the one that changes reported numbers:
`judged` stops overstating how much of the directory was ever comparable."""
rows = [_Row(SERVICE) for _ in range(6)] + [_Row("class Point:", snippet_id=9)]
assert len(comparable_siblings(rows, shape_form(HELPER))) == 6
def test_the_async_canon_stays_in_a_sync_candidates_denominator() -> None:
"""THE ACCEPTANCE CASE OF #2793, and the reason this narrows by family
rather than by form.
A hand-rolled sync `confirmDanger` in a directory where an async confirm
helper is canon must still be flagged. Bucketing the denominator by exact
form — which is what widening `kind` to carry `fn` vs `async-fn` amounts
to — would take the canon out of this count and silence it.
"""
rows = [_Row("async def confirmDanger(message: str) -> bool:") for _ in range(5)]
kept = comparable_siblings(rows, shape_form("function confirmDanger(message) {"))
assert len(kept) == 5
assert dominant_canon(kept) is not None
def test_an_unreadable_sibling_stays_counted() -> None:
"""Quieter, never louder. Dropping unknown rows shrinks `judged`, raises
the dominant canon's share, and fires the check MORE on exactly the
directories it can read least."""
rows = [_Row(SERVICE) for _ in range(4)] + [_Row("")]
assert len(comparable_siblings(rows, shape_form(SERVICE))) == 5
def test_an_unreadable_candidate_narrows_nothing() -> None:
"""The other side of the same discipline: a candidate whose own signature
says nothing gets the full denominator, not a guessed one."""
rows = [_Row(SERVICE), _Row("class Point:")]
assert comparable_siblings(rows, FORM_UNKNOWN) == rows
def test_a_family_passed_where_a_form_belongs_would_be_a_silent_no_op() -> None:
"""A REGRESSION GUARD ON A BUG THIS CHANGE ACTUALLY HAD.
`families_conflict` coarsens both sides itself, so handing it a family
makes `shape_family("callable")` return "" and the whole narrowing becomes
a no-op — while every call site still reads as though it applied. Pinned
because the wrong value is the right TYPE and the failure is silent.
"""
# A CALLABLE candidate, deliberately: `fn`'s family is `callable`, a word
# that is not itself a form, so `shape_family("callable")` is "" and the
# exclusion never fires. Picking `type` here would prove nothing — `type`
# is both a form and a family name, so passing the family still narrows
# and the bug hides. That near-miss is why this test exists at all.
rows = [_Row(SERVICE) for _ in range(6)] + [_Row("class Point:", snippet_id=9)]
form = shape_form("def helper(x) -> bool:")
assert shape_family(form) != form, "this test needs a form whose family differs"
by_form = comparable_siblings(rows, form)
by_family = comparable_siblings(rows, shape_family(form))
assert len(by_form) == 6, "the dataclass is not comparable to a function"
assert len(by_family) == len(rows), "a family narrows nothing — that is the bug"
assert by_form != by_family
def test_the_four_survivors_still_prompt() -> None:
"""HONEST ACCOUNTING, matching the sibling test above.
The narrowed denominator does not silence #4204's four `def` helpers, and
was never going to: they are callables, the canon is a callable, so
nothing is excluded from their count. What changes is that the count is
now over comparable things. Asserted so the claim cannot quietly rot into
"this fixed it".
"""
rows = [_Row(SERVICE) for _ in range(6)] + [_Row("class Point:", snippet_id=9)]
for sig in (
"def _p(source, kind, what, **kw) -> tuple[str, Point]:",
"def get_point(source: str) -> Point | None:",
"def is_registered(source: str) -> bool:",
"def sources_expected_to_emit() -> list[str]:",
):
kept = comparable_siblings(rows, shape_form(sig))
dom = dominant_canon(kept)
assert dom is not None, sig
assert not families_conflict(shape_form(sig), canon_form(kept, dom[0])), sig
tests/test_stamps_to_review.py
+160 -7
View File
@@ -25,7 +25,8 @@ hook's own history is made of.
import pytest
from scribe.services.shape_ledger import (
_RESEMBLE_MIN, _RESEMBLE_REASON, stamp_score, stamps_to_review,
_RESEMBLE_MIN, _RESEMBLE_REASON, _REVIEW_ROWS_SHOWN, canon_coherence,
stamp_score, stamps_to_review,
)
@@ -135,17 +136,26 @@ def test_a_canon_whose_members_agree_is_not_listed() -> None:
def test_a_canon_whose_members_are_all_different_things_is_listed() -> None:
"""Portal's #3283, in miniature: a class, a getter, a test and a binding
recorded as instances of one pattern."""
rows = [_member("class SessionAbsent(RuntimeError):"),
_member("def build_channel() -> str:"),
_member("async def attach(self) -> None:"),
_member("MAX = 10")]
recorded as instances of one pattern — and in four different files, as
they really were. The paths matter now: a callable sharing a file with a
class is read as that class's method (#4220), so putting them all in one
file would test the excuse rather than the disagreement."""
rows = [_member("class SessionAbsent(RuntimeError):", path="src/errors.py",
symbol="SessionAbsent"),
_member("def build_channel() -> str:", path="src/channel.py",
symbol="build_channel"),
_member("async def attach(self) -> None:", path="src/attach.py",
symbol="attach"),
_member("MAX = 10", path="src/limits.py", symbol="MAX")]
out = stamps_to_review(rows)
assert out["incoherent_count"] == 1
entry = out["incoherent"][0]
assert entry["snippet_id"] == 7 and entry["judged"] == 4
assert set(entry["forms"]) == {"type", "fn", "async-fn", "binding"}
assert len(entry["sample"]) == 4
# The two callables carry the vote; the class and the binding are what
# the canon cannot account for, and they are what the reader is shown.
assert entry["family"] == "callable"
assert {s["symbol"] for s in entry["strangers"]} == {"SessionAbsent", "MAX"}
def test_too_few_readable_members_says_nothing_either_way() -> None:
@@ -197,3 +207,146 @@ def test_the_service_carries_no_machinery_for_bulk_withdrawal() -> None:
body = open(ledger.__file__).read()
for banned in ("def retire_weak", "def auto_unclassify", "def bulk_withdraw"):
assert banned not in body, banned
# ── coherence is about what a canon CANNOT account for (#4220) ────────────
#
# The first version of this check asked whether every member shared a form.
# On its first live day the only two canons it reported were both sound,
# while the genuinely polluted one had already been cleaned by hand — a
# surface whose whole output is noise is one that stops being read.
#
# The obvious repair — group by family, keep the majority test — is a trap,
# and `test_family_grouping_alone_would_have_gone_blind` is the guard that
# stops anyone walking back into it.
def _model(sym, path):
"""A SQLAlchemy model class, as #2844 holds one."""
return _member(f"class {sym}(Base, TimestampMixin):", path=path)
def _to_dict(path):
"""The serialiser that model is required to carry, beside it."""
return _member("def to_dict(self) -> dict:", path=path, symbol="to_dict")
def test_a_class_and_the_to_dict_beside_it_are_one_shape() -> None:
"""#2844 exactly: the SQLAlchemy model convention, whose own text is
about the class AND the to_dict it must carry. Live it scored
37/62 = 0.597 and was called incoherent for containing precisely what it
says it contains."""
rows = []
for i in range(6):
rows += [_model(f"M{i}", f"src/scribe/models/m{i}.py"),
_to_dict(f"src/scribe/models/m{i}.py")]
out = stamps_to_review(rows)
assert out["incoherent_count"] == 0
def test_a_sync_starter_and_the_async_tick_it_schedules_are_one_shape() -> None:
"""#2849: four loop-starters and three ticks. `shape_family` already
collapses fn and async-fn for the divergence gate; this surface was the
one place still asking at form level."""
rows = [_member("def start_notification_loop() -> None:", path="src/n.py"),
_member("def start_log_retention_loop() -> None:", path="src/l.py"),
_member("def start_auth_token_retention_loop() -> None:", path="src/a.py"),
_member("async def _notification_tick() -> None:", path="src/n.py"),
_member("async def _retention_tick() -> None:", path="src/l.py"),
_member("async def _auth_token_retention_tick() -> None:", path="src/a.py")]
assert stamps_to_review(rows)["incoherent_count"] == 0
def test_family_grouping_alone_would_have_gone_blind() -> None:
"""THE REGRESSION GUARD. Polluted #2844 as it stood this morning: 37
model classes, 25 to_dict methods beside them, and 31 rows that had no
business being there. Counted as families that is 56 callables to 37
types — 0.602, a clean pass under any majority test. Those 31 rows are
the ones the surface exists to find, so a change that lets this canon
read as coherent has broken the feature while keeping every other test
in this file green."""
rows = []
for i in range(37):
rows.append(_model(f"M{i}", f"src/scribe/models/m{i}.py"))
for i in range(25):
rows.append(_to_dict(f"src/scribe/models/m{i % 37}.py"))
strangers = (["src/scribe/routes/rulebooks.py"] * 2
+ ["src/scribe/services/embeddings.py"] * 7
+ ["frontend/src/stores/rulebooks.ts"] * 3
+ ["tests/test_services_rulebooks.py"] * 19)
for i, path in enumerate(strangers):
rows.append(_member(f"async def f{i}():", path=path, symbol=f"f{i}"))
out = stamps_to_review(rows)
assert out["incoherent_count"] == 1, "the polluted canon must still be caught"
entry = out["incoherent"][0]
assert entry["families"]["callable"] > entry["families"]["type"]
assert entry["family"] == "type", "methods must not outvote their classes"
assert entry["attached"] == 25
assert entry["stranger_count"] == len(strangers) == 31
# The listing is capped, the count is not — a reader must be able to
# tell twelve strangers from thirty-one.
assert len(entry["strangers"]) == _REVIEW_ROWS_SHOWN
assert all(not s["path"].startswith("src/scribe/models")
for s in entry["strangers"])
def test_a_callable_in_a_file_with_no_class_is_a_stranger() -> None:
"""The excuse is "method of a member", not "callable anywhere". Without
this, any function in the repo would be excused by the existence of a
class somewhere else in the canon."""
rows = [_model("A", "src/models/a.py"), _to_dict("src/models/a.py"),
_model("B", "src/models/b.py"), _to_dict("src/models/b.py"),
_member("def helper():", path="src/services/free.py", symbol="helper"),
_member("def other():", path="src/services/free.py", symbol="other")]
out = stamps_to_review(rows)
assert out["incoherent_count"] == 1
assert {s["symbol"] for s in out["incoherent"][0]["strangers"]} == {"helper", "other"}
def test_the_strangers_are_named_not_a_sample_of_the_majority() -> None:
"""The reader's question is which rows are wrong. A sample of the
agreeing majority cannot answer it, which is what the first version
returned."""
# Seven and two: 2 of 9 is over `_STRANGER_SHARE`, 2 of 11 is under it.
# The tolerance is real and the counts here sit deliberately on the far
# side of it — see the test below for the near side.
rows = [_member("async def a():", path=f"src/ok{i}.py", symbol=f"a{i}")
for i in range(7)]
rows.append(_member("class Odd:", path="src/odd.py", symbol="Odd"))
rows.append(_member("class Odder:", path="src/odder.py", symbol="Odder"))
out = stamps_to_review(rows)
assert out["incoherent_count"] == 1
assert {s["symbol"] for s in out["incoherent"][0]["strangers"]} == {"Odd", "Odder"}
def test_a_single_odd_row_in_a_large_canon_is_tolerated() -> None:
"""One stranger in twenty is a row to fix, not a canon that has stopped
meaning anything. The surface is for the second thing."""
rows = [_member("async def a():", path=f"src/ok{i}.py", symbol=f"a{i}")
for i in range(19)]
rows.append(_member("class Odd:", path="src/odd.py", symbol="Odd"))
assert stamps_to_review(rows)["incoherent_count"] == 0
def test_an_incoherence_made_of_judged_rows_says_so() -> None:
"""The discriminator that tells a too-strict check from a bad ledger.
Both canons flagged on the first live day were entirely audit-judged,
and a reader could not see that without opening each one."""
judged = [_member("async def a():", path=f"src/j{i}.py", symbol=f"a{i}")
for i in range(4)]
judged += [_member("class J:", path="src/jc.py", symbol="J"),
_member("class J2:", path="src/jc2.py", symbol="J2")]
out = stamps_to_review(judged)
assert out["incoherent_count"] == 1
assert out["incoherent"][0]["unattended"] == 0
assert out["incoherent"][0]["weak_rows"] == 0
def test_the_coherence_verdict_reads_rows_and_writes_nothing() -> None:
"""`canon_coherence` is called on live ORM rows; it must not touch them."""
rows = [_model("A", "src/models/a.py"), _to_dict("src/models/a.py"),
_member("def loose():", path="src/x.py", symbol="loose")]
before = [(r.path, r.symbol, r.status, r.snippet_id, r.signature) for r in rows]
canon_coherence(rows)
assert [(r.path, r.symbol, r.status, r.snippet_id, r.signature)
for r in rows] == before