Merge pull request 'A surfaced rule stops going permanently quiet (#3750, #3751, #3752)' (#149) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 43s
CI & Build / integration (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m18s
CI & Build / Build & push image (push) Successful in 24s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 43s
CI & Build / integration (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m18s
CI & Build / Build & push image (push) Successful in 24s
This commit was merged in pull request #149.
This commit is contained in:
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "scribe",
|
"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.",
|
"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.09.0408",
|
"version": "2026.09.10.0221",
|
||||||
"author": {
|
"author": {
|
||||||
"name": "Bryan Van Deusen"
|
"name": "Bryan Van Deusen"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -16,6 +16,9 @@
|
|||||||
# scribe_reached STATE SID the server answered: the next outage speaks again
|
# scribe_reached STATE SID the server answered: the next outage speaks again
|
||||||
# scribe_config sets `url` + `token` from the env, returns 0
|
# scribe_config sets `url` + `token` from the env, returns 0
|
||||||
# only if BOTH are usable (#2278)
|
# only if BOTH are usable (#2278)
|
||||||
|
# scribe_rules_live FILE live rule ids from the exclusion ledger,
|
||||||
|
# comma-joined; entries age out (#3751)
|
||||||
|
# scribe_rules_append FILE stdin ids -> the ledger, timestamped
|
||||||
#
|
#
|
||||||
# Sourced, not executed: `. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"`.
|
# Sourced, not executed: `. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"`.
|
||||||
|
|
||||||
@@ -175,3 +178,103 @@ scribe_unreached() {
|
|||||||
scribe_reached() {
|
scribe_reached() {
|
||||||
rm -f "$1/$2.unreached" 2>/dev/null || true
|
rm -f "$1/$2.unreached" 2>/dev/null || true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# THE RULE EXCLUSION LEDGER, and how entries in it AGE (#3751).
|
||||||
|
#
|
||||||
|
# Two hooks write this file and two read it, which is the whole reason the
|
||||||
|
# parsing lives here: `scribe_prior_art.sh` and `scribe_tool_rules.sh` share
|
||||||
|
# one ledger so a rule named by one arm is not re-offered by the other, and a
|
||||||
|
# format only one of them understood would break that on the first read.
|
||||||
|
#
|
||||||
|
# WHAT PROBLEM AGEING SOLVES. #3749 clears the ledger when an EVENT destroys
|
||||||
|
# context — a compaction or a /clear. This is the case with no event at all: a
|
||||||
|
# long session where the rule was named two hundred turns ago and has simply
|
||||||
|
# fallen out of attention. It is #3702's argument at the tier level (present in
|
||||||
|
# context and salient at the moment are different properties) applied to time
|
||||||
|
# instead of to tier.
|
||||||
|
#
|
||||||
|
# PER-ENTRY TIMESTAMPS, NOT A FILE MTIME. Clearing the whole ledger when the
|
||||||
|
# file is old is one line of shell and wrong in exactly the session that needs
|
||||||
|
# it: a single recent write keeps every stale id alive, and the ids that go
|
||||||
|
# stale first are the ones from the rules that fire most.
|
||||||
|
#
|
||||||
|
# WALL TIME, NOT A TURN COUNT, and the trade is real rather than dismissed. A
|
||||||
|
# turn count is a truer model of salience — an idle session does not forget —
|
||||||
|
# but a hook has no turn number without keeping its own counter, which is a
|
||||||
|
# second piece of session state to write, read, clear on compaction and get
|
||||||
|
# wrong. Wall time is available from `date` and costs nothing. The failure mode
|
||||||
|
# it accepts is a session left idle over lunch treating its rules as forgotten,
|
||||||
|
# which produces one extra full line per rule and no other harm.
|
||||||
|
_SCRIBE_RULE_TTL=2700
|
||||||
|
|
||||||
|
# 45 MINUTES, and the reasoning rather than the number (rule 32).
|
||||||
|
#
|
||||||
|
# There is no data on this yet, so it is a judgement made to be revised — the
|
||||||
|
# telemetry that would settle it is the one #3807 just built, and a reading of
|
||||||
|
# how often an aged-out rule gets PULLED after it returns is what should move
|
||||||
|
# this.
|
||||||
|
#
|
||||||
|
# Too short and the exclusion stops existing and the repetition it prevents
|
||||||
|
# comes back. Too long and it never fires at all in a session short enough to
|
||||||
|
# matter. 45 minutes is about one working stretch on a single task: long enough
|
||||||
|
# that a rule does not re-announce itself while you are still doing the thing
|
||||||
|
# it governs, short enough that a multi-hour session gets a genuine refresh
|
||||||
|
# rather than one 9am mention.
|
||||||
|
#
|
||||||
|
# Being wrong on the short side is now the cheaper error, which is why this
|
||||||
|
# leans short. Since #3750 an excluded rule is REFERENCED rather than withheld,
|
||||||
|
# so the ledger is no longer the only thing standing between a session and a
|
||||||
|
# rule it has forgotten — an expired entry costs one full line instead of one
|
||||||
|
# short one, and the exclusion re-arms the moment it is spent.
|
||||||
|
|
||||||
|
# Live ids from a ledger, comma-joined for `exclude_rule_ids`. Empty output for
|
||||||
|
# a missing, empty or fully-aged file — the callers already treat "" as "send
|
||||||
|
# no exclusions".
|
||||||
|
#
|
||||||
|
# THE LAST ENTRY FOR AN ID WINS, and this is what stops a rule ping-ponging.
|
||||||
|
# The file is append-only, so a rule that ages out, gets surfaced fresh and is
|
||||||
|
# appended again has TWO lines. Reading the first would leave it permanently
|
||||||
|
# expired and it would re-announce itself on every single call from then on —
|
||||||
|
# the loudest possible failure, from the mechanism meant to quieten things.
|
||||||
|
# Appends are chronological, so the last line for an id is its most recent.
|
||||||
|
#
|
||||||
|
# A BARE ID — no tab, no timestamp — IS LIVE. That is the pre-#3751 format, and
|
||||||
|
# a session in flight when this ships has a ledger full of them. Treating
|
||||||
|
# unknown as expired would make every one of those sessions re-announce every
|
||||||
|
# rule it had already been told, all at once, which is precisely the noise this
|
||||||
|
# exists to prevent. Unknown means "not measured" and never "old" — the same
|
||||||
|
# null discipline the retrieval_logs columns use. Those entries simply never
|
||||||
|
# age, which is bounded: the session ends.
|
||||||
|
scribe_rules_live() {
|
||||||
|
local f="$1" now
|
||||||
|
[ -n "$f" ] && [ -f "$f" ] || return 0
|
||||||
|
now=$(date +%s 2>/dev/null) || now=0
|
||||||
|
awk -F'\t' -v now="$now" -v ttl="$_SCRIBE_RULE_TTL" '
|
||||||
|
{
|
||||||
|
id = $1
|
||||||
|
gsub(/[^0-9]/, "", id)
|
||||||
|
if (id == "") next
|
||||||
|
if (!(id in seen)) { seen[id] = 1; seq[++n] = id }
|
||||||
|
stamp[id] = ($2 ~ /^[0-9]+$/) ? $2 : ""
|
||||||
|
}
|
||||||
|
END {
|
||||||
|
out = ""
|
||||||
|
for (i = 1; i <= n; i++) {
|
||||||
|
id = seq[i]
|
||||||
|
if (stamp[id] != "" && now > 0 && (now - stamp[id]) > ttl) continue
|
||||||
|
out = out (out == "" ? "" : ",") id
|
||||||
|
}
|
||||||
|
print out
|
||||||
|
}
|
||||||
|
' "$f" 2>/dev/null || true
|
||||||
|
}
|
||||||
|
|
||||||
|
# Append surfaced ids, stamped. Reads ids on stdin, one per line — the shape
|
||||||
|
# `jq -r '(.rule_ids // [])[]?'` already produces at both call sites.
|
||||||
|
scribe_rules_append() {
|
||||||
|
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
|
||||||
|
}
|
||||||
|
|||||||
@@ -204,11 +204,12 @@ if [ -n "$session_id" ]; then
|
|||||||
derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen=""
|
derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen=""
|
||||||
[ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}"
|
[ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}"
|
||||||
fi
|
fi
|
||||||
if [ -f "$rulefile" ]; then
|
# Ageing, not a flat read (#3751), and the ONLY ledger here that ages: the
|
||||||
rule_seen=$(tr '\n' ',' < "$rulefile" 2>/dev/null | sed 's/,$//')
|
# note channels above are a different question with a different answer, and
|
||||||
|
# this arm's sibling hook reads the same rule file through the same helper.
|
||||||
|
rule_seen=$(scribe_rules_live "$rulefile")
|
||||||
[ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}"
|
[ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}"
|
||||||
fi
|
fi
|
||||||
fi
|
|
||||||
|
|
||||||
# Not `|| exit 0`: an unreachable instance must not discard a local finding
|
# Not `|| exit 0`: an unreachable instance must not discard a local finding
|
||||||
# that needed no instance to produce. And not silence either (#2932): a call
|
# that needed no instance to produce. And not silence either (#2932): a call
|
||||||
@@ -239,7 +240,8 @@ if [ -n "$body" ]; then
|
|||||||
printf '%s' "$body" | jq -r '(.sync_note_ids // [])[]?' 2>/dev/null >> "$syncfile" || true
|
printf '%s' "$body" | jq -r '(.sync_note_ids // [])[]?' 2>/dev/null >> "$syncfile" || true
|
||||||
fi
|
fi
|
||||||
if [ -n "$rulefile" ]; then
|
if [ -n "$rulefile" ]; then
|
||||||
printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null >> "$rulefile" || true
|
printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null \
|
||||||
|
| scribe_rules_append "$rulefile"
|
||||||
fi
|
fi
|
||||||
if [ -n "$derivefile" ]; then
|
if [ -n "$derivefile" ]; then
|
||||||
printf '%s' "$body" | jq -r '(.derive_keys // [])[]?' 2>/dev/null >> "$derivefile" || true
|
printf '%s' "$body" | jq -r '(.derive_keys // [])[]?' 2>/dev/null >> "$derivefile" || true
|
||||||
|
|||||||
@@ -86,11 +86,11 @@ rule_exclude_q=""
|
|||||||
if [ -n "$session_id" ]; then
|
if [ -n "$session_id" ]; then
|
||||||
safe_sid=$(printf '%s' "$session_id" | tr -c 'A-Za-z0-9._-' '_')
|
safe_sid=$(printf '%s' "$session_id" | tr -c 'A-Za-z0-9._-' '_')
|
||||||
rulefile="$state_dir/${safe_sid}.rules.ids"
|
rulefile="$state_dir/${safe_sid}.rules.ids"
|
||||||
if [ -f "$rulefile" ]; then
|
# Ageing, not a flat read (#3751): an id named two hours ago is not one the
|
||||||
rule_seen=$(tr '\n' ',' < "$rulefile" 2>/dev/null | sed 's/,$//')
|
# session is still holding. scribe_rules_live carries the reasoning.
|
||||||
|
rule_seen=$(scribe_rules_live "$rulefile")
|
||||||
[ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}"
|
[ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}"
|
||||||
fi
|
fi
|
||||||
fi
|
|
||||||
|
|
||||||
# `|| exit 0` here, unlike the prior-art hook: there is no local arm whose
|
# `|| exit 0` here, unlike the prior-art hook: there is no local arm whose
|
||||||
# finding would be discarded, and an outage line before every command is worse
|
# finding would be discarded, and an outage line before every command is worse
|
||||||
@@ -104,7 +104,8 @@ context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || exit 0
|
|||||||
|
|
||||||
# Remember what was named so it is not repeated this session.
|
# Remember what was named so it is not repeated this session.
|
||||||
if [ -n "$rulefile" ]; then
|
if [ -n "$rulefile" ]; then
|
||||||
printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null >> "$rulefile" || true
|
printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null \
|
||||||
|
| scribe_rules_append "$rulefile"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
jq -cn --arg ctx "$context" '{
|
jq -cn --arg ctx "$context" '{
|
||||||
|
|||||||
@@ -149,6 +149,37 @@ RULEHINT_DEFAULT_THRESHOLD = 0.72
|
|||||||
# adds a way to misconfigure the surface (rule 25 cuts both ways).
|
# adds a way to misconfigure the surface (rule 25 cuts both ways).
|
||||||
RULEHINT_LIMIT = 1
|
RULEHINT_LIMIT = 1
|
||||||
|
|
||||||
|
# AND A REPEAT COMPETES FOR THAT ONE SLOT ON RANK ALONE (#3750).
|
||||||
|
#
|
||||||
|
# Since #3750 a hit already on the session's exclusion ledger is RENDERED
|
||||||
|
# rather than dropped, which raises a question the old behaviour never had to
|
||||||
|
# answer: when the top-ranked hit is one the session has already seen, does it
|
||||||
|
# take the slot, or step aside for a fresh rule behind it?
|
||||||
|
#
|
||||||
|
# It takes the slot, and nothing is fetched behind it. Two reasons.
|
||||||
|
#
|
||||||
|
# RANK IS THE ANSWER TO "WHAT IS RELEVANT NOW". If the repeat scores 0.85 and
|
||||||
|
# the best fresh candidate 0.73, the repeat is the better match for the action
|
||||||
|
# actually being taken. Overfetching in order to promote the fresh one past it
|
||||||
|
# would reinstate exactly the withholding this milestone exists to remove, one
|
||||||
|
# rank deeper and harder to see — recency is not a reason to show a worse
|
||||||
|
# match, and "you have seen this" is not the same claim as "you are holding
|
||||||
|
# this".
|
||||||
|
#
|
||||||
|
# AND A SECOND LINE IS THE ONE THING THE LIMIT ABOVE FORBIDS. Letting a repeat
|
||||||
|
# ride alongside a fresh rule means two hint lines, and the paragraph above is
|
||||||
|
# entirely about why a fourth voice that speaks twice is where a reader stops
|
||||||
|
# reading. A reference costs the same ~40 tokens as a first surfacing, so
|
||||||
|
# "it is only a short extra line" is not available as an argument: the budget
|
||||||
|
# is one line because of what a second line does to the whole hint, not
|
||||||
|
# because of what it costs.
|
||||||
|
#
|
||||||
|
# The consequence is deliberate and worth naming: a rule that keeps ranking
|
||||||
|
# first for a recurring situation keeps being referenced, every time the
|
||||||
|
# situation recurs. That is the intended behaviour — the situation recurring
|
||||||
|
# IS the trigger — and its decay belongs to exclusion ageing (#3751), not to
|
||||||
|
# a rule that ranks first being quietly demoted for having won before.
|
||||||
|
|
||||||
# WHY THE ARMS NO LONGER FILTER TO ONE TIER (#3702).
|
# WHY THE ARMS NO LONGER FILTER TO ONE TIER (#3702).
|
||||||
#
|
#
|
||||||
# Both arms used to pass `tier="conditional"`, on the reasoning that an
|
# Both arms used to pass `tier="conditional"`, on the reasoning that an
|
||||||
@@ -827,6 +858,52 @@ async def get_writepath_config(user_id: int) -> dict:
|
|||||||
"rule_threshold": rule_threshold,
|
"rule_threshold": rule_threshold,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
def _rule_hint_line(rule, *, where: str, seen: bool) -> str:
|
||||||
|
"""One rule hint line — both arms, both tails (#3750).
|
||||||
|
|
||||||
|
ONE FUNCTION BECAUSE THE TAILS MUST NOT DRIFT. The two arms phrase their
|
||||||
|
heads differently ("may apply here" vs "may apply to this Bash call") and
|
||||||
|
that difference is deliberate. Everything after it must not differ, and
|
||||||
|
#3497's history is that the pre-tool arm inherited a defect from its
|
||||||
|
sibling by being modelled on it rather than sharing with it. Two copies of
|
||||||
|
a two-branch string is how one branch gets fixed and the other does not.
|
||||||
|
|
||||||
|
WHY A REPEAT GETS A LINE AT ALL. Both arms used to drop a hit whose id was
|
||||||
|
already on the session's exclusion ledger and emit nothing. That is correct
|
||||||
|
only while the session still HOLDS what it was told, and a compaction
|
||||||
|
breaks exactly that: the earlier injection is summarized away while the id
|
||||||
|
stays on the ledger, so the rule is absent from context AND unreachable for
|
||||||
|
the rest of the session (#3749 closes the compaction half; this closes the
|
||||||
|
ordinary half, where a session simply stops holding a line it read an hour
|
||||||
|
ago).
|
||||||
|
|
||||||
|
Only ONE CLAUSE of the original line is false on a repeat — the claim that
|
||||||
|
the rule is not in the session's loaded set. So only that clause changes.
|
||||||
|
Title, trigger and pull pointer are identical either way, the statement is
|
||||||
|
never injected either way, and a repeat therefore costs the same ~40 tokens
|
||||||
|
as a first surfacing and no more.
|
||||||
|
|
||||||
|
DELIBERATELY NOT ASKING THE SESSION WHETHER IT HOLDS THE RULE. A model
|
||||||
|
asked "do you still hold rule 156?" will say yes, and the claim is
|
||||||
|
unverifiable self-report about its own context. The answer is also not
|
||||||
|
needed: the line is cheap enough to always emit and carries its own remedy
|
||||||
|
in both branches. Removing the question removes the fragility rather than
|
||||||
|
managing it.
|
||||||
|
"""
|
||||||
|
trigger = (rule.when_to_apply or "").strip()
|
||||||
|
tail = (
|
||||||
|
f"You saw it earlier this session; pull it with get_rule({rule.id}) "
|
||||||
|
"if you no longer hold it."
|
||||||
|
if seen else
|
||||||
|
f"Read it with get_rule({rule.id}) before deciding it does not "
|
||||||
|
"apply; it is not in this session's loaded set."
|
||||||
|
)
|
||||||
|
return (
|
||||||
|
f"Standing rule that may apply {where} \u2014 \u201c{rule.title}\u201d"
|
||||||
|
+ (f" ({trigger})" if trigger else "")
|
||||||
|
+ f". {tail}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def build_write_path_hint(
|
async def build_write_path_hint(
|
||||||
user_id: int,
|
user_id: int,
|
||||||
@@ -1287,9 +1364,13 @@ async def build_write_path_hint(
|
|||||||
# resembles what is being written, noticed at the moment it is relevant
|
# resembles what is being written, noticed at the moment it is relevant
|
||||||
# rather than by being resident in every session.
|
# rather than by being resident in every session.
|
||||||
#
|
#
|
||||||
# CONDITIONAL ONLY. An always-on rule is already in the session; repeating
|
# EVERY TIER, since #3702 — see the note at RULEHINT_LIMIT. This comment
|
||||||
# it here would be noise, and noise on a hint that fires on every write is
|
# used to read "CONDITIONAL ONLY: an always-on rule is already in the
|
||||||
# how a hint gets ignored.
|
# session, so repeating it here would be noise". That conflated being
|
||||||
|
# PRESENT in context with being SALIENT at the moment the action is taken,
|
||||||
|
# and it is the same conflation #3750 corrects one layer up: a rule the
|
||||||
|
# session was told about an hour ago is not a rule in front of the reader
|
||||||
|
# now.
|
||||||
#
|
#
|
||||||
# Fails open like every other arm: a rule hint must never break a write.
|
# Fails open like every other arm: a rule hint must never break a write.
|
||||||
rule_ids: list[int] = []
|
rule_ids: list[int] = []
|
||||||
@@ -1308,15 +1389,18 @@ async def build_write_path_hint(
|
|||||||
)
|
)
|
||||||
rule_ms = (time.perf_counter() - rule_t0) * 1000.0
|
rule_ms = (time.perf_counter() - rule_t0) * 1000.0
|
||||||
fresh = [(score, rule) for score, rule in hits if rule.id not in already]
|
fresh = [(score, rule) for score, rule in hits if rule.id not in already]
|
||||||
for _score, rule in fresh:
|
# EVERY hit gets a line; `already` only changes the tail (#3750).
|
||||||
trigger = (rule.when_to_apply or "").strip()
|
for _score, rule in hits:
|
||||||
lines.append(
|
lines.append(
|
||||||
f"Standing rule that may apply here — \u201c{rule.title}\u201d"
|
_rule_hint_line(rule, where="here", seen=rule.id in already)
|
||||||
+ (f" ({trigger})" if trigger else "")
|
|
||||||
+ f". Read it with get_rule({rule.id}) before deciding it "
|
|
||||||
"does not apply; it is not in this session's loaded set."
|
|
||||||
)
|
)
|
||||||
rule_ids.append(rule.id)
|
# `rule_ids` stays FRESH-ONLY, and that is the whole telemetry story of
|
||||||
|
# this change (#3752). It is what the hook writes to the exclusion
|
||||||
|
# ledger and what `record_rule_surfaced` counts; a referenced rule is
|
||||||
|
# already on the ledger by definition, and counting it as a surfacing
|
||||||
|
# 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)
|
||||||
# TWO tables, and the split is not arbitrary. retrieval_logs is one
|
# TWO tables, and the split is not arbitrary. retrieval_logs is one
|
||||||
# row per CALL, keyed on the score distribution a threshold is tuned
|
# 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
|
# from. rule_usage_events is one row per RULE per event, which is the
|
||||||
@@ -1412,10 +1496,10 @@ async def build_tool_rule_hint(
|
|||||||
which tools it watches, so widening the matcher is a `hooks.json` edit with
|
which tools it watches, so widening the matcher is a `hooks.json` edit with
|
||||||
no change here.
|
no change here.
|
||||||
|
|
||||||
CONDITIONAL ONLY, exactly as the write-path arm — an always-on rule is
|
EVERY TIER, since #3702 — the tier filter this docstring used to describe
|
||||||
already resident and repeating it is noise. That filter is also the
|
is gone from both arms, for the reason recorded at RULEHINT_LIMIT: present
|
||||||
transition this arm exists to enable: re-tier a rule to `conditional` and
|
in context and salient at the moment are different properties, and only the
|
||||||
it starts arriving here instead of in every session's preamble.
|
second is what this arm is for.
|
||||||
|
|
||||||
Fails open and returns an empty context on any error: a recall aid may
|
Fails open and returns an empty context on any error: a recall aid may
|
||||||
never break the operator's action.
|
never break the operator's action.
|
||||||
@@ -1469,25 +1553,31 @@ async def build_tool_rule_hint(
|
|||||||
# and the threshold looks wrong when nothing about it is.
|
# and the threshold looks wrong when nothing about it is.
|
||||||
suppressed=len(hits) - len(fresh),
|
suppressed=len(hits) - len(fresh),
|
||||||
)
|
)
|
||||||
if not fresh:
|
# `hits`, not `fresh` (#3750). A call whose only hit is a repeat still
|
||||||
|
# has something to say — the arm just says it differently.
|
||||||
|
if not hits:
|
||||||
return out
|
return out
|
||||||
|
|
||||||
lines: list[str] = []
|
lines = [
|
||||||
rule_ids: list[int] = []
|
_rule_hint_line(
|
||||||
for _score, rule in fresh:
|
rule, where=f"to this {tool_name} call",
|
||||||
trigger = (rule.when_to_apply or "").strip()
|
seen=rule.id in already,
|
||||||
lines.append(
|
|
||||||
f"Standing rule that may apply to this {tool_name} call — "
|
|
||||||
f"“{rule.title}”"
|
|
||||||
+ (f" ({trigger})" if trigger else "")
|
|
||||||
+ f". Read it with get_rule({rule.id}) before deciding it "
|
|
||||||
"does not apply; it is not in this session's loaded set."
|
|
||||||
)
|
)
|
||||||
rule_ids.append(rule.id)
|
for _score, rule in hits
|
||||||
|
]
|
||||||
|
# FRESH-ONLY, for the reason given on the sibling arm: a reference is a
|
||||||
|
# rendering decision, not a retrieval outcome, and counting it here
|
||||||
|
# would inflate the denominator pull_through is read from.
|
||||||
|
rule_ids = [rule.id for _score, rule in fresh]
|
||||||
|
|
||||||
# RANKED, not ambient: this arm chose what it showed, so a pull can
|
# RANKED, not ambient: this arm chose what it showed, so a pull can
|
||||||
# settle whether the choice was any good. `rule_usage.RANKED_SOURCES`
|
# settle whether the choice was any good. `rule_usage.RANKED_SOURCES`
|
||||||
# carries the same name.
|
# carries the same name.
|
||||||
|
#
|
||||||
|
# GUARDED, which it did not need to be before #3750: `fresh` can now be
|
||||||
|
# empty on a call that still emitted a line, and recording a surfacing
|
||||||
|
# of nothing would write an event with no rules in it.
|
||||||
|
if rule_ids:
|
||||||
record_rule_surfaced(
|
record_rule_surfaced(
|
||||||
user_id=user_id, rule_ids=rule_ids, source="pre_tool_rule",
|
user_id=user_id, rule_ids=rule_ids, source="pre_tool_rule",
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -0,0 +1,216 @@
|
|||||||
|
"""Entries in the rule exclusion ledger age out (#3751).
|
||||||
|
|
||||||
|
WHY THIS EXISTS
|
||||||
|
|
||||||
|
#3749 clears the ledger when an EVENT destroys context — a compaction, a
|
||||||
|
/clear. This covers the case with no event at all: a long session where a rule
|
||||||
|
was named two hundred turns ago and has simply fallen out of attention. Same
|
||||||
|
argument #3702 made at the tier level (present in context and salient at the
|
||||||
|
moment are different properties), applied to time instead of to tier.
|
||||||
|
|
||||||
|
WHAT IS PINNED, AND WHAT DELIBERATELY IS NOT
|
||||||
|
|
||||||
|
Not the TTL's value. 2700 is a judgement made to be revised once there is
|
||||||
|
telemetry to revise it with, and a test asserting `== 2700` would turn a
|
||||||
|
legitimate tuning change into a red build — which is how a number nobody may
|
||||||
|
touch gets one. The tests read the constant out of the shell and assert the
|
||||||
|
PROPERTY around it, so any TTL works and only a broken comparison fails.
|
||||||
|
|
||||||
|
Runs the real shell, like `test_session_context_ledger` and the after-write
|
||||||
|
hook's tests, and deliberately with no SCRIBE_URL/SCRIBE_TOKEN: ageing is
|
||||||
|
local, keyless and networkless, and must work on an instance that is
|
||||||
|
unreachable or was never configured.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import re
|
||||||
|
import shutil
|
||||||
|
import subprocess
|
||||||
|
import time
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
PLUGIN = Path(__file__).resolve().parents[1] / "plugin" / "hooks"
|
||||||
|
DEFS = PLUGIN / "scribe_defs.sh"
|
||||||
|
|
||||||
|
|
||||||
|
def _ttl() -> int:
|
||||||
|
"""The TTL as the shell defines it — never a copy of the number."""
|
||||||
|
m = re.search(r"^_SCRIBE_RULE_TTL=(\d+)", DEFS.read_text(), re.M)
|
||||||
|
assert m, "_SCRIBE_RULE_TTL is gone from scribe_defs.sh"
|
||||||
|
return int(m.group(1))
|
||||||
|
|
||||||
|
|
||||||
|
def _live(ledger: Path) -> list[str]:
|
||||||
|
"""`scribe_rules_live` against a real ledger, as the hooks call it."""
|
||||||
|
for tool in ("bash", "awk"):
|
||||||
|
if shutil.which(tool) is None:
|
||||||
|
pytest.skip(f"hook runtime tool {tool!r} not installed")
|
||||||
|
proc = subprocess.run(
|
||||||
|
["bash", "-c", f'. "{DEFS}"; scribe_rules_live "{ledger}"'],
|
||||||
|
capture_output=True, text=True, timeout=30,
|
||||||
|
env={"PATH": os.environ["PATH"]}, # no credentials — see the docstring
|
||||||
|
)
|
||||||
|
assert proc.returncode == 0, proc.stderr
|
||||||
|
out = proc.stdout.strip()
|
||||||
|
return out.split(",") if out else []
|
||||||
|
|
||||||
|
|
||||||
|
def _write_ledger(ledger: Path, entries: list[tuple[str, int | None]]) -> None:
|
||||||
|
"""A ledger written in AGES rather than absolute stamps.
|
||||||
|
|
||||||
|
Named for what it is: `tests/helpers._now` is a datetime for record
|
||||||
|
fixtures and this file wants the shell's epoch, so they are deliberately
|
||||||
|
not the same helper and deliberately not the same name. `time.time()`
|
||||||
|
rather than shelling out to `date` — both read the one system clock, so
|
||||||
|
the subprocess bought nothing.
|
||||||
|
"""
|
||||||
|
now = int(time.time())
|
||||||
|
ledger.write_text("".join(
|
||||||
|
f"{rid}\n" if ago is None else f"{rid}\t{now - ago}\n"
|
||||||
|
for rid, ago in entries
|
||||||
|
))
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_old_entry_ages_out_and_a_recent_one_does_not(tmp_path):
|
||||||
|
"""BOTH DIRECTIONS IN ONE ASSERTION (rule 167), because either half alone
|
||||||
|
passes against a broken implementation.
|
||||||
|
|
||||||
|
A test that only checks the old id is gone passes against a helper that
|
||||||
|
returns nothing at all — which would drop every exclusion and bring back
|
||||||
|
the repetition the ledger exists to prevent. A test that only checks the
|
||||||
|
recent id survives passes against the flat `tr '\\n' ','` read this
|
||||||
|
replaces, i.e. against the defect itself.
|
||||||
|
"""
|
||||||
|
ttl = _ttl()
|
||||||
|
ledger = tmp_path / "s.rules.ids"
|
||||||
|
_write_ledger(ledger, [("156", ttl + 600), ("168", 60)])
|
||||||
|
|
||||||
|
assert _live(ledger) == ["168"], (
|
||||||
|
f"expected the entry {ttl + 600}s old to age out and the 60s-old one "
|
||||||
|
f"to survive (TTL {ttl}s). An empty list means nothing is excluded any "
|
||||||
|
f"more; both ids means nothing ages."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_repeatedly_surfaced_rule_appears_once_in_the_exclusion_list(tmp_path):
|
||||||
|
"""List hygiene, and it is not cosmetic.
|
||||||
|
|
||||||
|
The ledger is append-only, so a rule surfaced five times has five lines.
|
||||||
|
The output is spliced straight into `&exclude_rule_ids=` and parsed by the
|
||||||
|
server as a list; duplicates make that list grow without bound over a long
|
||||||
|
session, and a trailing or doubled comma is an empty element the parser has
|
||||||
|
to decide what to do with. Neither is visible from the hint.
|
||||||
|
|
||||||
|
(There was a boundary test here — `ttl` vs `ttl + 1`. It was racy by
|
||||||
|
construction: a ledger written at T is read at T+n, so the entry ages by
|
||||||
|
however long the subprocess took and the two cases swap. A one-second
|
||||||
|
distinction on a 45-minute window is also not a behaviour anyone can
|
||||||
|
observe, so it was pinning a flake rather than a property.)
|
||||||
|
"""
|
||||||
|
ledger = tmp_path / "s.rules.ids"
|
||||||
|
_write_ledger(ledger, [("156", 60), ("168", 30), ("156", 10), ("156", 5)])
|
||||||
|
|
||||||
|
live = _live(ledger)
|
||||||
|
assert live == ["156", "168"], (
|
||||||
|
f"got {live}: an id repeated in the ledger must appear once in the "
|
||||||
|
f"exclusion list, in first-seen order"
|
||||||
|
)
|
||||||
|
raw = subprocess.run(
|
||||||
|
["bash", "-c", f'. "{DEFS}"; scribe_rules_live "{ledger}"'],
|
||||||
|
capture_output=True, text=True, timeout=30,
|
||||||
|
env={"PATH": os.environ["PATH"]},
|
||||||
|
).stdout.strip()
|
||||||
|
assert ",," not in raw and not raw.endswith(","), (
|
||||||
|
f"the exclusion list has an empty element: {raw!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_rule_that_ages_out_and_returns_does_not_ping_pong(tmp_path):
|
||||||
|
"""THE FAILURE THIS COSTS THE MOST TO GET WRONG.
|
||||||
|
|
||||||
|
The ledger is append-only, so a rule that ages out, gets surfaced fresh and
|
||||||
|
is appended again has TWO lines. An implementation reading the FIRST leaves
|
||||||
|
it permanently expired — and it then re-announces itself on every single
|
||||||
|
call for the rest of the session. The mechanism meant to quieten things
|
||||||
|
becomes the loudest thing in the hint.
|
||||||
|
"""
|
||||||
|
ttl = _ttl()
|
||||||
|
ledger = tmp_path / "s.rules.ids"
|
||||||
|
_write_ledger(ledger, [("156", ttl + 600)])
|
||||||
|
|
||||||
|
subprocess.run(
|
||||||
|
["bash", "-c", f'. "{DEFS}"; printf \'156\\n\' | scribe_rules_append "{ledger}"'],
|
||||||
|
capture_output=True, text=True, timeout=30,
|
||||||
|
env={"PATH": os.environ["PATH"]},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert _live(ledger) == ["156"], (
|
||||||
|
"a re-surfaced rule read as still expired; the most recent entry for "
|
||||||
|
"an id must win, or it re-announces itself on every call from here on"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_bare_id_from_the_old_format_is_treated_as_live(tmp_path):
|
||||||
|
"""The compatibility decision, asserted rather than described.
|
||||||
|
|
||||||
|
Every session in flight when this ships has a ledger of bare ids. Reading
|
||||||
|
unknown as EXPIRED would make all of them re-announce every rule they had
|
||||||
|
already been told, all at once — the exact noise this feature exists to
|
||||||
|
prevent, delivered by the feature itself on the day it ships.
|
||||||
|
|
||||||
|
Unknown means "not measured", never "old" — the same discipline the
|
||||||
|
nullable retrieval_logs columns use. Those entries never age, which is
|
||||||
|
bounded, because the session ends.
|
||||||
|
"""
|
||||||
|
ttl = _ttl()
|
||||||
|
ledger = tmp_path / "s.rules.ids"
|
||||||
|
ledger.write_text("156\n168\n")
|
||||||
|
assert _live(ledger) == ["156", "168"]
|
||||||
|
|
||||||
|
# And a bare id keeps its own meaning next to a stale stamped one, rather
|
||||||
|
# than the file falling back to one format or the other wholesale.
|
||||||
|
_write_ledger(ledger, [("9", None), ("156", ttl + 600)])
|
||||||
|
assert _live(ledger) == ["9"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_missing_or_empty_ledger_excludes_nothing(tmp_path):
|
||||||
|
"""Fail-open, like every other path in these hooks: no ledger means no
|
||||||
|
exclusions, not an error and not a stray comma the server would parse as
|
||||||
|
an id."""
|
||||||
|
assert _live(tmp_path / "never-written.rules.ids") == []
|
||||||
|
empty = tmp_path / "empty.rules.ids"
|
||||||
|
empty.write_text("")
|
||||||
|
assert _live(empty) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_both_hooks_read_the_ledger_through_the_shared_helper():
|
||||||
|
"""Structural, because the defect this would reintroduce is invisible.
|
||||||
|
|
||||||
|
Two hooks share one ledger so a rule named by one arm is not re-offered by
|
||||||
|
the other. If either goes back to reading the file flat, that hook stops
|
||||||
|
ageing while its sibling keeps ageing — the two disagree about which rules
|
||||||
|
are live, and nothing in the output says so.
|
||||||
|
|
||||||
|
Pinned on the flat-read SHAPE rather than on a helper name: a hook that
|
||||||
|
stops calling the helper but still ages correctly some other way is not a
|
||||||
|
defect, and a rename of the helper is not one either.
|
||||||
|
"""
|
||||||
|
for name in ("scribe_prior_art.sh", "scribe_tool_rules.sh"):
|
||||||
|
src = (PLUGIN / name).read_text()
|
||||||
|
rule_reads = [
|
||||||
|
ln for ln in src.splitlines()
|
||||||
|
if "rulefile" in ln and "tr " in ln and r"'\n' ','" in ln
|
||||||
|
]
|
||||||
|
assert not rule_reads, (
|
||||||
|
f"{name} reads the rule ledger flat again: {rule_reads}. That "
|
||||||
|
f"hook's exclusions would stop ageing while its sibling's keep "
|
||||||
|
f"ageing, and the two would disagree silently about which rules "
|
||||||
|
f"this session still holds."
|
||||||
|
)
|
||||||
|
assert "scribe_rules_live" in src, (
|
||||||
|
f"{name} no longer reads the rule ledger through the shared "
|
||||||
|
f"helper, so the two hooks can drift on the ledger format"
|
||||||
|
)
|
||||||
@@ -10,6 +10,7 @@ end in two different doors, and the property under test is that they meet. Split
|
|||||||
across three module-shaped files, "both ends are wired" is a thing no single
|
across three module-shaped files, "both ends are wired" is a thing no single
|
||||||
test asserts.
|
test asserts.
|
||||||
"""
|
"""
|
||||||
|
import ast
|
||||||
from contextlib import ExitStack
|
from contextlib import ExitStack
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
@@ -472,14 +473,40 @@ async def test_the_tool_arm_is_a_ranked_source():
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_a_rule_the_session_already_holds_is_not_re_offered():
|
async def test_a_rule_the_session_already_holds_is_referenced_not_re_offered():
|
||||||
|
"""WAS `..._is_not_re_offered`, asserting `"161" not in out["context"]`.
|
||||||
|
|
||||||
|
That assertion was the old contract and #3750 deliberately reverses half of
|
||||||
|
it: a rule already on the ledger now gets a line with a different tail
|
||||||
|
instead of being dropped in silence. Withholding was only ever right while
|
||||||
|
the session still HELD what it was told, and a compaction breaks exactly
|
||||||
|
that while leaving the id excluded.
|
||||||
|
|
||||||
|
The half that survives is the half that was always about telemetry rather
|
||||||
|
than rendering: an already-held rule stays out of `rule_ids`, so it reaches
|
||||||
|
neither the exclusion ledger (where it already is) nor the surfacing count
|
||||||
|
(which a reference must not inflate).
|
||||||
|
"""
|
||||||
rec = MagicMock()
|
rec = MagicMock()
|
||||||
hits = [(0.71, fake_rule(id=161, title="Reach the forge through its MCP tools")),
|
hits = [(0.71, fake_rule(id=161, title="Reach the forge through its MCP tools")),
|
||||||
(0.70, fake_rule(id=12, title="Don't run a local stack unless asked"))]
|
(0.70, fake_rule(id=12, title="Don't run a local stack unless asked"))]
|
||||||
out = await _run_tool_arm(hits, rec, exclude_rule_ids=[161])
|
out = await _run_tool_arm(hits, rec, exclude_rule_ids=[161])
|
||||||
|
|
||||||
assert out["rule_ids"] == [12]
|
assert out["rule_ids"] == [12], (
|
||||||
assert "161" not in out["context"]
|
"a referenced rule was counted as surfaced; only the fresh one was "
|
||||||
|
"actually chosen by this arm"
|
||||||
|
)
|
||||||
|
assert "get_rule(161)" in out["context"], (
|
||||||
|
"the held rule vanished instead of being referenced (#3750)"
|
||||||
|
)
|
||||||
|
assert "get_rule(12)" in out["context"], (
|
||||||
|
"the fresh rule was lost while adding the reference — both belong in "
|
||||||
|
"the hint, and the reference must not displace the surfacing"
|
||||||
|
)
|
||||||
|
assert _SEEN_TAIL in out["context"] and _FRESH_TAIL in out["context"], (
|
||||||
|
"one call rendered two hits in different states and gave them the same "
|
||||||
|
"tail; the tails are the entire difference a reader can act on"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -705,10 +732,46 @@ def test_neither_rule_arm_logs_its_call_behind_a_results_guard():
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Pre-tool arm: the call log comes BEFORE the early return.
|
# Pre-tool arm: the call log comes BEFORE the early return.
|
||||||
body = pc_src.split("async def build_tool_rule_hint")[1]
|
#
|
||||||
assert body.index('source="pre_tool_rule"') < body.index("if not fresh:"), (
|
# WALKED, NOT SUBSTRING-MATCHED (rule 167). This assertion used to read
|
||||||
"the pre-tool arm returns before logging its call — a surface with no "
|
# `body.index("if not fresh:")`, which pinned the name of a local variable
|
||||||
"rows at all cannot be told apart from a hook that never fired"
|
# rather than the property. #3750 changed that guard to `if not hits:` —
|
||||||
|
# the arm still logs before returning, so the property held perfectly, and
|
||||||
|
# a name-matching assertion would have raised ValueError and reported the
|
||||||
|
# #3497 defect as back. A guard that cries regression when the thing it
|
||||||
|
# protects is intact is the failure mode rule 167 names.
|
||||||
|
#
|
||||||
|
# The property is positional: between the search and the first guard that
|
||||||
|
# can return early, the call row has already been written.
|
||||||
|
fn = next(
|
||||||
|
n for n in ast.walk(ast.parse(pc_src))
|
||||||
|
if isinstance(n, ast.AsyncFunctionDef) and n.name == "build_tool_rule_hint"
|
||||||
|
)
|
||||||
|
search_at = min(
|
||||||
|
n.lineno for n in ast.walk(fn)
|
||||||
|
if isinstance(n, ast.Call)
|
||||||
|
and getattr(n.func, "id", None) == "semantic_search_rules"
|
||||||
|
)
|
||||||
|
logged_at = min(
|
||||||
|
n.lineno for n in ast.walk(fn)
|
||||||
|
if isinstance(n, ast.Call)
|
||||||
|
and getattr(n.func, "id", None) == "record_retrieval"
|
||||||
|
)
|
||||||
|
# Every `if <cond>: return ...` after the search — whatever it tests.
|
||||||
|
bailouts = [
|
||||||
|
n.lineno for n in ast.walk(fn)
|
||||||
|
if isinstance(n, ast.If) and n.lineno > search_at
|
||||||
|
and any(isinstance(b, ast.Return) for b in n.body)
|
||||||
|
]
|
||||||
|
assert bailouts, (
|
||||||
|
"no early return found after the search in build_tool_rule_hint — the "
|
||||||
|
"guard has nothing left to protect, which means this test is now "
|
||||||
|
"passing vacuously rather than the arm being correct"
|
||||||
|
)
|
||||||
|
assert logged_at < min(bailouts), (
|
||||||
|
f"the pre-tool arm returns at line {min(bailouts)} before logging its "
|
||||||
|
f"call at line {logged_at} — a surface with no rows at all cannot be "
|
||||||
|
f"told apart from a hook that never fired (#3497)"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -866,3 +929,161 @@ async def test_both_recorders_report_the_same_rules_for_one_call(
|
|||||||
"the fixture stopped exercising what it claims to; check the exclusion "
|
"the fixture stopped exercising what it claims to; check the exclusion "
|
||||||
"filter still runs before both recorders"
|
"filter still runs before both recorders"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── A repeat is REFERENCED, not withheld (#3750) ──────────────────────────
|
||||||
|
#
|
||||||
|
# Both arms used to drop a hit already on the session's exclusion ledger and
|
||||||
|
# emit nothing at all. That is right only while the session still HOLDS what it
|
||||||
|
# was told — and it stops being right the moment a compaction summarizes the
|
||||||
|
# earlier injection away while the id stays on the ledger, which leaves the
|
||||||
|
# rule absent from context AND unreachable for the rest of the session.
|
||||||
|
#
|
||||||
|
# The tests below pin the emitted LINE, not the prose describing it, and both
|
||||||
|
# arms are parametrized through one body so the two tails cannot drift apart —
|
||||||
|
# #3497's history is that the pre-tool arm inherited a defect from its sibling
|
||||||
|
# by being modelled on it rather than sharing with it.
|
||||||
|
|
||||||
|
_HELD = fake_rule(
|
||||||
|
id=156,
|
||||||
|
title="A wait with no deadline is a bug",
|
||||||
|
statement="Every wait on something that can fail to answer carries a deadline.",
|
||||||
|
when_to_apply="writing any call that crosses a process boundary",
|
||||||
|
)
|
||||||
|
_FRESH_TAIL = "not in this session's loaded set"
|
||||||
|
_SEEN_TAIL = "You saw it earlier this session"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_rule_already_on_the_ledger_still_produces_a_line(source, run):
|
||||||
|
"""THE REGRESSION, stated as the thing that used to be absent.
|
||||||
|
|
||||||
|
Falsified against the old behaviour: before #3750 both arms filtered to
|
||||||
|
`fresh` before rendering, so an all-excluded call returned an empty
|
||||||
|
context and this assertion fails on `context == ""`.
|
||||||
|
"""
|
||||||
|
out = await run([(0.81, _HELD)], MagicMock(), exclude_rule_ids=[156])
|
||||||
|
|
||||||
|
# ON `get_rule(156)` RATHER THAN A TRUTHY CONTEXT. The write-path arm's
|
||||||
|
# context also carries the prior-art menu, the staleness line and the shape
|
||||||
|
# signals, so `assert out["context"]` is TRUE under the old behaviour and
|
||||||
|
# would pin nothing on that arm while looking identical to a real check on
|
||||||
|
# the other. The rule line is the only part of the string this changes.
|
||||||
|
assert "get_rule(156)" in out["context"], (
|
||||||
|
f"{source} emitted no rule line for a rule the session had already "
|
||||||
|
f"been shown. Silence is only correct while the session still holds "
|
||||||
|
f"the line — after a compaction it does not, and the id is still on "
|
||||||
|
f"the ledger, so the rule is unreachable for the rest of the session. "
|
||||||
|
f"Context was: {out['context']!r}"
|
||||||
|
)
|
||||||
|
assert _HELD.title in out["context"], (
|
||||||
|
"the reference names no rule; a pull pointer with nothing attached "
|
||||||
|
"gives a reader no way to judge whether it is worth pulling"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_the_two_tails_are_distinguishable_and_say_the_true_one(source, run):
|
||||||
|
"""One clause differs, and it is the clause that would otherwise be false.
|
||||||
|
|
||||||
|
A repeat rendered with the FRESH tail would assert "it is not in this
|
||||||
|
session's loaded set" about a rule this session was handed twenty minutes
|
||||||
|
ago — a line that is wrong in the one way a reader cannot check.
|
||||||
|
"""
|
||||||
|
fresh_ctx = (await run([(0.81, _HELD)], MagicMock()))["context"]
|
||||||
|
seen_ctx = (await run([(0.81, _HELD)], MagicMock(),
|
||||||
|
exclude_rule_ids=[156]))["context"]
|
||||||
|
|
||||||
|
assert _FRESH_TAIL in fresh_ctx and _SEEN_TAIL not in fresh_ctx, (
|
||||||
|
f"{source} rendered a first surfacing with the repeat tail"
|
||||||
|
)
|
||||||
|
assert _SEEN_TAIL in seen_ctx and _FRESH_TAIL not in seen_ctx, (
|
||||||
|
f"{source} told the session a rule it has already been shown is not "
|
||||||
|
f"in its loaded set"
|
||||||
|
)
|
||||||
|
assert fresh_ctx != seen_ctx, "the two tails collapsed into one"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_neither_tail_injects_the_rule_statement(source, run):
|
||||||
|
"""The budget, pinned on both branches.
|
||||||
|
|
||||||
|
A reference costs the same ~40 tokens as a first surfacing precisely
|
||||||
|
because neither carries the statement. If a repeat ever starts inlining the
|
||||||
|
body "since we are re-showing it anyway", this arm stops being cheap enough
|
||||||
|
to always emit — and always emitting is the whole mechanism.
|
||||||
|
"""
|
||||||
|
for excluded in ([], [156]):
|
||||||
|
ctx = (await run([(0.81, _HELD)], MagicMock(),
|
||||||
|
exclude_rule_ids=excluded))["context"]
|
||||||
|
assert _HELD.statement not in ctx, (
|
||||||
|
f"{source} inlined the rule statement (excluded={excluded!r}); the "
|
||||||
|
f"line carries title, trigger and a pull pointer and nothing more"
|
||||||
|
)
|
||||||
|
assert _HELD.title in ctx and "process boundary" in ctx, (
|
||||||
|
"the line dropped the title or the trigger — those are what let a "
|
||||||
|
"reader decide whether to pull without pulling"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── What a reference IS in the telemetry: nothing new (#3752) ─────────────
|
||||||
|
#
|
||||||
|
# THE RELATION, STATED BEFORE IT SHIPS. #3750 changes what is RENDERED and
|
||||||
|
# nothing about what is COUNTED:
|
||||||
|
#
|
||||||
|
# result_count counts fresh surfacings — unchanged
|
||||||
|
# suppressed_count counts repeats — unchanged
|
||||||
|
# rule_usage counts fresh surfacings — unchanged
|
||||||
|
#
|
||||||
|
# A reference is a rendering decision, not a retrieval outcome. That answer is
|
||||||
|
# not free: the naive implementation renders repeats by dropping the `fresh`
|
||||||
|
# filter, which takes `suppressed_count` to zero everywhere — and #3739's
|
||||||
|
# near-miss fix identifies repeat-caused zeros by `suppressed_count > 0`, so
|
||||||
|
# the contamination corrected on 2026-09-08 would return by a different route,
|
||||||
|
# in the same field, with the fix still sitting in the code not working.
|
||||||
|
#
|
||||||
|
# #3712 gave a reader `complete_from` for a counter that started late. Nothing
|
||||||
|
# tells a reader a counter's DEFINITION moved. This test is the cheap version
|
||||||
|
# of that guarantee: the claim "nothing moved" is only worth anything if it is
|
||||||
|
# checkable, so it is asserted rather than described.
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_reference_is_rendered_but_not_counted(source, run):
|
||||||
|
"""The counters must read exactly as they did before #3750."""
|
||||||
|
log, rec = MagicMock(), MagicMock()
|
||||||
|
out = await run([(0.81, _HELD)], rec, retrieval_log=log,
|
||||||
|
exclude_rule_ids=[156])
|
||||||
|
|
||||||
|
row = next(c for c in log.call_args_list
|
||||||
|
if c.kwargs.get("source") == source).kwargs
|
||||||
|
assert row["results"] == [], (
|
||||||
|
f"{source} counted a referenced rule as a result. `result_count` "
|
||||||
|
f"drives zero_result_calls and the whole threshold picture; a repeat "
|
||||||
|
f"is not evidence the bar is set correctly."
|
||||||
|
)
|
||||||
|
assert row["suppressed"] == 1, (
|
||||||
|
f"{source} stopped reporting the repeat as suppressed. #3739's "
|
||||||
|
f"near_misses predicate excludes declines with suppressed_count > 0 — "
|
||||||
|
f"if this reads 0, every repeat-caused zero is re-counted as a genuine "
|
||||||
|
f"ranker rejection and the near-miss contamination returns."
|
||||||
|
)
|
||||||
|
assert rec.call_count == 0, (
|
||||||
|
f"{source} recorded a surfacing for a rule it only referenced, which "
|
||||||
|
f"inflates pull_through's denominator with a choice the arm never made"
|
||||||
|
)
|
||||||
|
assert out["rule_ids"] == [], (
|
||||||
|
"a referenced id went back to the hook for the exclusion ledger; it is "
|
||||||
|
"already there by definition, and returning it conflates 'shown fresh' "
|
||||||
|
"with 'mentioned again'"
|
||||||
|
)
|
||||||
|
assert "get_rule(156)" in out["context"], (
|
||||||
|
"guard is passing vacuously — no rule line was rendered, so 'rendered "
|
||||||
|
"but not counted' is not what this run demonstrated. Truthiness of the "
|
||||||
|
"whole context will not do: the write-path arm fills it from four "
|
||||||
|
"other sources."
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user