A repeat is not a rejection, and a compaction is not knowledge #145

Merged
bvandeusen merged 3 commits from dev into main 2026-09-09 00:13:02 -04:00
6 changed files with 275 additions and 21 deletions
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "scribe",
"description": "Scribe system-of-record for Claude Code: MCP tools over your notes/tasks/projects/rules, a session-start push channel that surfaces your always-on rules + active-project context, process-skills (writing-plans, systematic-debugging, verification, brainstorming, reusing-code), and your saved Scribe Processes auto-surfaced as skills (/scribe:sync). Replaces superpowers + file-memory with one app-backed plugin.",
"version": "2026.09.04.0140",
"version": "2026.09.09.0408",
"author": {
"name": "Bryan Van Deusen"
},
+47
View File
@@ -55,6 +55,53 @@ here=$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) || exit 0
event=$(cat 2>/dev/null || true)
source=$(printf '%s' "$event" | jq -r '.source // empty' 2>/dev/null) || source=""
# --- The rule ledger outlives the context it describes (#3749) ---
#
# scribe_prior_art.sh and scribe_tool_rules.sh record every rule id they have
# named in <state>/<sid>.rules.ids and hand it back as exclude_rule_ids, so a
# rule is named once per session and then goes quiet. That is right while the
# session still HOLDS what it was told, and wrong the moment it does not.
#
# A compaction summarizes the earlier injections away and does not touch the
# filesystem, so the rule ends up absent from context AND still excluded —
# unreachable for the rest of the session. The banner below tells the model to
# re-pull its ALWAYS-ON rules, but a rule an arm surfaced is conditional and is
# not in that set, so it has no other way back. The rules most likely to be in
# this state are the ones that fire most often, which is to say the ones that
# apply most.
#
# The session id survives a compaction — the etag marker further down is
# rewritten on `compact` and keyed by session_id, which is only meaningful if
# the id is stable — so the stale ledger is genuinely found again, not orphaned.
#
# CLEARED ON THE SOURCES THAT DESTROY CONTEXT, AND ONLY THOSE:
#
# compact CLEAR — summarized away; the file survived.
# clear CLEAR — context wiped.
# startup nothing to do: a new session id means a new, empty file.
# resume KEEP. The context was genuinely restored, so the ledger still
# describes what the session holds. Clearing here would re-surface
# every rule after a restore that lost nothing — the mirror error.
# fork KEEP, and the answer is the same whichever way forks are keyed: a
# fork carries the conversation, so if it inherits the id the ledger
# is accurate, and if it gets a new one the file is empty anyway.
#
# ONLY the rules ledger. The same directory holds .ids / .sync.ids /
# .derive.ids for the note arms. Whether a surfaced NOTE should return after a
# compaction is a different question with a different answer, and leaving those
# alone is a decision rather than an oversight.
case "$source" in
compact|clear)
sid=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || sid=""
if [ -n "$sid" ]; then
safe_sid=$(printf '%s' "$sid" | tr -c 'A-Za-z0-9._-' '_')
# Best-effort, like every other filesystem touch in these hooks: a ledger
# that cannot be removed costs a repeated exclusion, never a session.
rm -f "${TMPDIR:-/tmp}/scribe-priorart/${safe_sid}.rules.ids" 2>/dev/null || true
fi
;;
esac
out=""
# Append $1 to $out, separated by a horizontal rule when $out already has content.
append() { if [ -n "$out" ]; then out="${out}"$'\n\n---\n\n'"$1"; else out="$1"; fi; }
+6 -4
View File
@@ -169,10 +169,12 @@ async def retrieval_telemetry(days: int = 30) -> dict:
`avg_result_count` and `p90_duration_ms`.
THE NUMBER TO READ FIRST IS `near_misses.p90`, AGAINST THE THRESHOLD IN
FORCE FOR THAT SURFACE. It is measured only on the calls that returned
NOTHING, on the best score the ranker reached before the bar rejected it —
so it is the one figure here that says something the bar cannot make true
by construction. A bar at 0.72 turning away a stream of 0.71s is set too
FORCE FOR THAT SURFACE. It is measured on the calls the BAR turned away —
zero-result calls, minus the ones whose zero was a repeat the reader had
already been shown — using the best score the ranker reached before the bar
rejected it. So it is the one figure here that says something the bar
cannot make true by construction, and `max` is always below the threshold:
an above-bar candidate nobody excluded would have been returned. A bar at 0.72 turning away a stream of 0.71s is set too
high by a hair and the surface is losing hits it should have had. The same
bar turning away 0.30s is working, and the corpus simply had nothing. Both
render as a zero-result call, and nothing else in this readout tells them
+39 -9
View File
@@ -360,15 +360,45 @@ async def retrieval_summary(user_id: int | None, *, days: int = 30) -> dict:
}
zero = case((RetrievalLog.result_count == 0, 1), else_=0)
# THE NEAR-MISS POPULATION: calls that returned nothing AND recorded what
# the bar turned away. Both conditions matter. Restricting to zero-result
# calls is what makes the number say something the bar cannot fix by
# construction — on a call that returned something, `best_available_score`
# equals `top_score` and adds nothing. Requiring the column to be non-null
# keeps rows written before #3670 out of the sample rather than letting
# them read as scoreless declines.
declined = (RetrievalLog.result_count == 0) & (
RetrievalLog.best_available_score.isnot(None)
# THE NEAR-MISS POPULATION: calls that returned nothing BECAUSE THE BAR
# TURNED SOMETHING AWAY, and recorded what it was. Three conditions, and
# the third was missing for one deploy (#3739).
#
# Zero-result only: on a call that returned something,
# `best_available_score` equals `top_score` and adds nothing.
#
# Non-null only: rows written before #3670 genuinely do not know, and must
# not read as scoreless declines.
#
# AND NOT A REPEAT. A zero-result call is two unrelated events — the ranker
# found nothing above the bar, or it found only what this session had
# already been shown — and just the first says anything about the bar. That
# is the whole of #3497, and #3670 reintroduced the conflation one level up:
# the rule arms filter exclusions in PYTHON, after the search, so a rule
# that cleared the bar and was dropped as a repeat still reported a high
# `best_available_score` on a zero-result row. Live proof, first read after
# deploy: pre_tool_rule's near-miss max was 0.7457 while the lowest score it
# ever RETURNED was 0.7204 — a "rejection" that outscored acceptances.
#
# The NULL arm is principled, not permissive: `suppressed_count IS NULL`
# means the caller passed its exclusions INTO the search, which is exactly
# the case where the reported score is already post-exclusion and cannot be
# contaminated. Note arms stay measured; rule arms get cleaned.
#
# Deliberately conservative: a call carrying both a repeat and a lower
# genuine miss is dropped whole, losing that point. It undercounts; it
# cannot corrupt — the right way round for a number read against a bar.
#
# This also makes `near_misses.max < threshold` true BY CONSTRUCTION. An
# above-bar candidate that was not excluded would have been returned, so
# its call is not in this population at all.
declined = (
(RetrievalLog.result_count == 0)
& (RetrievalLog.best_available_score.isnot(None))
& (
RetrievalLog.suppressed_count.is_(None)
| (RetrievalLog.suppressed_count == 0)
)
)
miss = case((declined, 1), else_=0)
# `best_available_score` only for those rows; NULL elsewhere, and
+24 -7
View File
@@ -994,6 +994,17 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
user_id=UID, source="pre_tool_rule", query="ls", threshold=0.72,
limit=1, project_id=None, is_task=None, results=[], duration_ms=4.0,
))
# THE CASE WHOSE ABSENCE LET THIS GUARD PASS OVER BROKEN CODE (#3739).
# A zero-result call whose zero was a REPEAT, not a rejection: the ranker
# cleared the bar at 0.9 and the session had already been shown that rule,
# so the arm dropped it in Python after the search. Without the suppression
# arm of the predicate this row lands in the near-miss population and drags
# `max` to 0.9 — above the very threshold the field is read against.
await _insert_retrieval_log(_build_payload(
user_id=UID, source="pre_tool_rule", query="git commit", threshold=0.72,
limit=1, project_id=None, is_task=None, results=[], duration_ms=4.0,
best_available=0.9, suppressed=1,
))
try:
out = await retrieval_summary(UID, days=30)
@@ -1002,22 +1013,28 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e
"zeros everywhere, which is #2663 exactly"
)
src = out["sources"]["pre_tool_rule"]
assert src["calls"] == 5
assert src["zero_result_calls"] == 4
assert src["calls"] == 6
assert src["zero_result_calls"] == 5
nm = src["near_misses"]
assert nm is not None, "the near-miss block did not survive the query"
assert nm["measured_calls"] == 3, (
"the population is declines that RECORDED a score: three measured, "
"one unmeasured (excluded, not counted as a scoreless decline), and "
"one call that showed something (excluded — its best-available is "
"just its top score and says nothing about the bar)"
"the population is declines the BAR caused, that recorded a score. "
"Three qualify. Excluded: the unmeasured row (predates the column, "
"not a scoreless decline), the call that showed something (its "
"best-available is just its top score), and the REPEAT — a zero "
"the reader caused, not the bar (#3739)"
)
assert nm["max"] == pytest.approx(0.7189, abs=1e-4), (
"the closest thing the bar turned away — 0.7189 against a 0.72 "
"threshold, which is the reading the whole field exists to give"
)
assert nm["max"] < 0.72, "a near miss that cleared the bar is not a miss"
assert nm["max"] < 0.72, (
"a rejection that outscores the bar is not a rejection. This is "
"structural once the suppression arm is in the predicate: an "
"above-bar candidate that was not excluded would have been "
"RETURNED, so its call cannot be in this population (#3739)"
)
assert 0.70 <= nm["p50"] <= 0.7189
finally:
async with async_session() as s:
+158
View File
@@ -0,0 +1,158 @@
"""The SessionStart hook clears the rule ledger exactly when context dies (#3749).
WHY THIS EXISTS
The prior-art and tool-rule hooks record every rule id they have named in
`<state>/<sid>.rules.ids` and hand it back as `exclude_rule_ids`, so a rule is
surfaced once per session and then goes quiet. That is correct while the
session still holds what it was told.
A compaction breaks that assumption in the worst available way: it summarizes
the earlier injections out of context and does not touch the filesystem. The
rule ends up absent from context AND still excluded — unreachable for the rest
of the session. The compaction banner tells the model to re-pull its
*always-on* rules, but a rule an arm surfaced is conditional and is not in that
set, so it has no other way back. The rules most likely to be in that state are
the ones that fire most often.
WHAT THIS PINS
Not "the ledger is cleared" — that would pass against a hook which deletes it
on every source, and deleting on `resume` is its own defect: the context was
genuinely restored there, so re-surfacing every rule is the mirror error.
What is pinned is the DISCRIMINATION. The whole source table is asserted in one
statement, so a blanket delete (all False) and a no-op (all True) both fail,
and neither can be made to pass by editing one case.
Runs the real shell against a temp TMPDIR, like the after-write hook's tests.
Deliberately with no SCRIBE_URL/SCRIBE_TOKEN in the environment: clearing the
ledger is local, keyless and networkless, and must still happen on an instance
that is unreachable or unconfigured.
"""
from __future__ import annotations
import json
import os
import shutil
import subprocess
from pathlib import Path
import pytest
PLUGIN = Path(__file__).resolve().parents[1] / "plugin"
HOOK = PLUGIN / "hooks" / "scribe_session_context.sh"
def _env(tmp_path):
"""Near-namesake of test_after_write_hook's `_env`, and deliberately not it:
that one needs git and curl and SUPPLIES credentials, because the behaviour
it tests is a network round-trip. This one must prove the opposite — that
the clear happens with no credentials and no network at all — so sharing a
helper would mean testing this case in an environment that cannot show it.
"""
for tool in ("jq", "bash"):
if shutil.which(tool) is None:
pytest.skip(f"hook runtime tool {tool!r} not installed")
# No SCRIBE_URL / SCRIBE_TOKEN on purpose — see the module docstring.
return {"PATH": os.environ["PATH"], "TMPDIR": str(tmp_path),
"HOME": str(tmp_path)}
def _ledger(tmp_path, sid: str, name: str = "rules.ids") -> Path:
d = tmp_path / "scribe-priorart"
d.mkdir(exist_ok=True)
f = d / f"{sid}.{name}"
f.write_text("156\n168\n")
return f
def _fire(source: str, sid: str, env) -> None:
subprocess.run(
["bash", str(HOOK)],
input=json.dumps({"source": source, "session_id": sid}),
capture_output=True, text=True, env=env, timeout=30,
)
def test_the_rules_ledger_survives_exactly_when_the_context_does(tmp_path):
"""The whole source table, in one assertion, so it cannot be half-satisfied.
`startup` is listed even though it is a no-op against a session id that has
never been seen: it is asserted here so that a future change which starts
clearing indiscriminately fails on a case somebody would otherwise call
harmless.
"""
env = _env(tmp_path)
survived = {}
for source in ("compact", "clear", "resume", "startup"):
sid = f"sess-{source}"
ledger = _ledger(tmp_path, sid)
_fire(source, sid, env)
survived[source] = ledger.exists()
assert survived == {
"compact": False,
"clear": False,
"resume": True,
"startup": True,
}, (
f"got {survived}. A rule surfaced before a compaction is summarized "
f"out of context while its id stays on the exclusion ledger, so it "
f"becomes unreachable for the rest of the session — that is what the "
f"compact/clear cases prevent. The resume case is the other half: the "
f"context came back intact there, and re-surfacing every rule after a "
f"restore that lost nothing is the same defect from the other side. "
f"All-False means something is deleting unconditionally; all-True "
f"means the clear never runs."
)
def test_only_the_rule_ledger_is_cleared_and_the_note_ledgers_are_left(tmp_path):
"""Scope, asserted rather than described.
The same directory holds `.ids`, `.sync.ids` and `.derive.ids` for the note
arms. Whether a surfaced NOTE should come back after a compaction is a
different question with a different answer, and it is not being answered.
A `rm` glob over `<sid>.*` would pass every assertion in the test above
while silently deciding it.
"""
env = _env(tmp_path)
sid = "sess-scope"
rules = _ledger(tmp_path, sid, "rules.ids")
notes = _ledger(tmp_path, sid, "ids")
sync = _ledger(tmp_path, sid, "sync.ids")
derive = _ledger(tmp_path, sid, "derive.ids")
_fire("compact", sid, env)
assert not rules.exists(), "the rule ledger should have been cleared"
assert notes.exists() and sync.exists() and derive.exists(), (
"a note ledger was cleared too. The note arms were deliberately left "
"out of #3749 — clearing them is a decision about a different surface, "
"and a glob that takes them along makes it by accident."
)
def test_a_compact_without_a_session_id_is_survivable(tmp_path):
"""Defensive, because this hook's contract is fail-open.
An event with no `session_id` names no ledger. The hook must not error, and
must not fall back to a wildcard — clearing every session's ledger on the
machine because this one event was malformed is the worst available
reading of "best effort".
"""
env = _env(tmp_path)
other = _ledger(tmp_path, "someone-elses-session")
proc = subprocess.run(
["bash", str(HOOK)],
input=json.dumps({"source": "compact"}),
capture_output=True, text=True, env=env, timeout=30,
)
assert proc.returncode == 0, proc.stderr
assert other.exists(), (
"an event with no session id cleared a ledger belonging to a different "
"session"
)