From 8c9f947f096820df40e6c9d177beff5b9f71b49f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 11:39:00 -0400 Subject: [PATCH 01/13] feat(rules): the update surfaces teach the trigger shape, not just the create ones (#3855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A trigger is two-thirds of a rule's embedded document, so one naming a CATEGORY rather than a moment collapses the record toward its title and it never arrives. #3835 measured that across 113 rules; #3855 hit it again on the eight preferences, where six named a category and two did not. The split was not carelessness, it was an uneven contract. create_rule has carried the full argument since c61925b (2026-08-27) and the two preferences authored that day got good triggers; the six written weeks earlier got categories. The guidance worked wherever it existed — and it existed on three of five write surfaces. Both update_* tools were silent, and the update path is where every RETROFITTED trigger is written, which is most of them: a trigger that already reads fine as English is the one nobody rewrites. So: - update_rule gains the retrofit case, which is a different trap from the create case. There the field is empty and the instruction is "write one". Here one exists, reads perfectly well, and the honest-looking verdict is that it is fine. - update_preference gains it too, plus why the field is load-bearing there specifically: preferences get a reserved slot filled by a kind-filtered query at limit=1, so the corpus ranks against ITSELF and the trigger is nearly all that separates one from the next. - create_preference and create_project_rule now SHOW a moment instead of describing one. Advice about being concrete that is not itself concrete is the shape that was already on file while the corpus filled up. The guard pins one property: a tool taking when_to_apply mentions it. That is exactly what update_preference failed. The surface list is derived from register() rather than hand-kept, so a write tool added later is in scope the day it lands. Two stronger predicates were written for the softer regression — guidance kept but abstracted — and both were discarded after falsification: counting quoted phrases measured ambient quotation and passed the broken version, and scoping that count to a window failed create_preference while correct. Rule 167 settles it; the discarded attempts are recorded in the test docstring so the next author does not repeat them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/mcp/tools/rulebooks.py | 52 +++++++++++- tests/test_trigger_shape_is_taught.py | 116 ++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 tests/test_trigger_shape_is_taught.py diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index 87b1569..3d5acf1 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -518,8 +518,12 @@ async def create_project_rule( characters of statement. when_to_apply: WHEN this rule fires — the trigger, not the instruction, and the rule's retrieval surface: name the SYMPTOM, - the words someone would type while stuck. See create_rule for the - full argument. It informs the tier below rather than deciding it, + the words someone would type while stuck. Show the moment rather + than classifying it — "the migration failed with a check + violation on a column we just extended" retrieves; "when working + on migrations" does not, because it names a category no session + ever types. See create_rule for the full argument. It informs the + tier below rather than deciding it, since a project rule's tier turns on area-scope, not on whether the trigger can be named. tier: "always_on" (default) or "conditional". The SAME two values as @@ -597,6 +601,26 @@ async def update_rule( a rule stops being preloaded into every session and starts arriving when it is relevant. `system_ids` REPLACES the rule's areas (pass [] to clear). + RETROFITTING A TRIGGER HAS ITS OWN TRAP, and it is not the one create_rule + warns about. There the field is empty and the instruction is "write one". + Here a trigger usually already EXISTS and reads perfectly well as English — + "during hard debugging", "when reading any request from the operator", + "before starting an action while a previous one is still settling" — so the + honest-looking verdict is that it is fine. It is not. Those three named a + CATEGORY rather than a moment, and a category is not a thing any session + ever types. `rule_document()` puts this field in twice, as the title's + other half and again above the body, so it dominates the vector: a trigger + describing the abstraction collapses the record toward its title and the + rule never arrives. Measured in #3835 across 113 rules, and again in #3855 + on six preferences written before this was understood. + + So when you touch a rule with an old trigger, re-read it against the query + that would have to match it — the command about to run, the code being + written, the operator's actual message — and rewrite it in that vocabulary + if it does not. Prefer the words someone produces while the rule applies, + including the rationalisation they would be drafting to talk themselves out + of it. See create_rule for the full argument and the measurement behind it. + TO EMPTY A FIELD, NAME IT: clear_fields=["verify_with"]. Passing "" cannot do it — "" means "leave this alone" here, which is what lets you update two fields without wiping the other six. Clearable: why, how_to_apply, @@ -692,6 +716,14 @@ async def create_preference( session would actually be producing then: the command it is about to run, the code it is writing, the thing the operator just asked for. + Show the moment rather than classifying it. Write "the operator pasted a stack + trace and said it is still broken" rather than "during hard debugging"; + write "about to `git push` with an earlier CI run still unread" rather + than "when pacing actions". The second spelling of each is a category, and + no session ever produces a category — it produces the command, the error + text, the half-formed ask. A trigger naming the abstraction collapses the + record toward its title and it never arrives. + `arose_from_id` IS REQUIRED for the same kind of reason. A preference is expected to change as the work teaches it, and a corpus that drifts with no record of what taught each change is one nobody can audit. Point it at @@ -781,12 +813,28 @@ async def update_preference( create_rule rather than hardening a preference in place. Softening in the other direction is equally an edit worth flagging out loud. + EDITING `when_to_apply` IS THE HIGHEST-LEVERAGE EDIT HERE, and the easiest + to skip, because a preference's trigger is load-bearing in a way a rule's + is not. Preferences get a RESERVED slot at the prompt boundary, filled by a + kind-filtered query at limit=1 — so the corpus does not merely rank against + rules, it ranks against ITSELF, and the trigger is almost all of what + separates one preference from the next. Six preferences whose triggers all + named a category ("during hard debugging", "when reading any request from + the operator") made that slot pick close to arbitrarily on every prompt. + + So whenever you are here for any reason, read the trigger against the + operator's message that should have summoned it. If it describes a + situation rather than quoting the moment, rewrite it in the words they + actually type — and in the words YOU would be producing while about to get + this wrong. update_rule carries the full argument. + Empty strings leave fields unchanged; clear_fields empties them by name, exactly as update_rule does. Args: rule_id: The preference to update. arose_from_id: What taught this change. Required; see above. + when_to_apply: The moment it applies, in session vocabulary. See above. """ uid = current_user_id() if not arose_from_id: diff --git a/tests/test_trigger_shape_is_taught.py b/tests/test_trigger_shape_is_taught.py new file mode 100644 index 0000000..6d7dd17 --- /dev/null +++ b/tests/test_trigger_shape_is_taught.py @@ -0,0 +1,116 @@ +"""Every rule-write surface documents the trigger it can write (#3855). + +WHY THIS EXISTS + +`when_to_apply` is not documentation. `rule_document()` uses it twice — as +the embedded title's second half and again above the body — so it dominates +the vector, and a rule whose trigger names a CATEGORY rather than a moment +collapses toward its title and never arrives. Measured in #3835 across 113 +rules, then again in #3855 on eight preferences. + +The corpus damage traced to an uneven contract rather than to careless +authors. `create_rule` had carried the full argument since 2026-08-27, and +the two preferences written that day got good triggers; the six written +before it — "during hard debugging", "when reading any request from the +operator" — got categories. The guidance worked wherever it existed. It +simply did not exist on either `update_*` surface, which is where every +RETROFITTED trigger is written, and retrofitting is most of the work: a +trigger that already reads fine as English is the one nobody rewrites. + +WHAT THIS PINS + +One structural property, no wording: a tool that accepts `when_to_apply` +mentions it in its docstring. That is exactly what `update_preference` +failed before #3855 — the parameter existed and the docstring never said so, +which is how six triggers got written against no contract at all. + +THE SURFACE LIST IS DERIVED, NEVER HAND-KEPT. It comes from what +`rulebooks.register()` actually hands the server, filtered to signatures +taking `when_to_apply`, so a rule-write tool added later is in scope on the +day it is added. A hand-kept list has to be remembered by the person least +likely to know it exists — the argument `RANKED_SOURCES` makes in +services/rule_usage.py, and the reason #3855 happened at all. + +WHAT THIS DELIBERATELY DOES NOT PIN, AND WHY IT IS NOT HERE + +The regression worth catching is the one above's *softer* twin: guidance +kept but abstracted back to "name the moment in session vocabulary", which +is advice about being concrete that is not itself concrete — the shape that +was already on file while the corpus filled with categories. + +Two predicates for it were written and both were discarded, because each was +falsified and each failed: + + - Counting quoted multi-word phrases anywhere in the docstring measured + ambient quotation, not demonstrated triggers. It passed the broken + version by scoring unrelated prose ("what happens if someone doesn't do + this"), and prose with an odd number of quote characters produced + matches spanning the gap BETWEEN two unrelated phrases. + - Scoping that count to a window after each "trigger"/"when_to_apply" + mention then failed `create_preference` in its CORRECT state, because + its examples sit further from the first mention than any defensible + window reaches. + +Rule 167's standard settles it: a check that passes on the broken code reads +as coverage and stops anyone looking again, so no check is the better of the +two. Whether a docstring teaches the shape WELL stays a reading judgment, +and the record of that decision lives here so the next author does not spend +the same two rounds discovering it. +""" +import inspect + +import pytest + +from tests.helpers import tool_doc as _doc + +_MODULE = "scribe.mcp.tools.rulebooks" + + +def _registered_tools() -> list: + """Every function `rulebooks.register()` actually hands to the server. + + Collected by handing `register` a stand-in that records what it is given, + rather than by reading the source or repeating the tuple here. The point + is that this cannot drift from what ships. + """ + from scribe.mcp.tools import rulebooks + + collected: list = [] + + class _Collector: + def tool(self, name=None): + def register_one(fn): + collected.append(fn) + return fn + return register_one + + rulebooks.register(_Collector()) + return collected + + +def _trigger_writers() -> list[str]: + """The registered tools that can write a rule's trigger.""" + names = [ + fn.__name__ + for fn in _registered_tools() + if "when_to_apply" in inspect.signature(fn).parameters + ] + # An empty list would make every case below vacuous and the guard would + # pass by having nothing to check — absence read as non-existence, the + # #3720 shape. Fail loudly instead. + assert names, "no rule-write surface takes when_to_apply; the guard is blind" + return sorted(names) + + +@pytest.mark.parametrize("name", _trigger_writers()) +def test_a_trigger_writing_surface_documents_the_field(name): + """A tool that can write a trigger must say what the field is for.""" + doc = _doc(_MODULE, name) + assert "when_to_apply" in doc, ( + f"{name} accepts when_to_apply but never mentions it in its " + "docstring. The tool docstring is the agent-facing contract " + "(rule 119), and a trigger written against no contract is the #3855 " + "defect: six preferences named a category instead of a moment, and " + "a category is not a thing any session ever types. Say what the " + "field is for, and show a moment — see create_rule." + ) -- 2.54.0 From 8e06cdf74994ea61cb15a6d6ca15b333128577a5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 11:48:14 -0400 Subject: [PATCH 02/13] feat(rules): the trigger contract is shown as a worked contrast, and pinned (#3855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows 8c9f947, which taught the trigger shape on the two update_* surfaces but left the softer regression unguarded: guidance kept and abstracted back to "name the moment in session vocabulary" — advice about being concrete that is not itself concrete, which is the shape that was already on file while the corpus filled with categories. Two attempts to detect that in free prose were written and discarded: - Counting quoted multi-word phrases anywhere in a docstring measured ambient quotation rather than demonstrated triggers. It PASSED the abstracted version by scoring unrelated prose, and text with an odd number of quote characters produced matches spanning the gap BETWEEN two unrelated phrases. - Scoping that count to a window after each trigger mention then FAILED create_preference in its CORRECT state, its examples sitting further from the first mention than any defensible window reaches. Both were proxies inferring demonstration from prose. Where a property cannot be measured, changing the shape of the thing is cheaper than a cleverer measurement — so all five trigger-writing surfaces now carry a two-line labelled contrast: RETRIEVES: "the migration failed with a check violation on a column we just extended" COLLAPSES: "when working on migrations" Unambiguous to parse, free in its wording, and a better teaching form than the sentences it replaces: the labels name the mechanism, so they do work for the reader rather than only for the test. The guard now pins both halves — the field is documented, and the contrast is present, complete and non-identical. Falsified against three regressions before committing: the paragraph stripped, the examples abstracted away, and one half of the pair removed. All three fail; the current tree passes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/mcp/tools/rulebooks.py | 46 +++++++---- tests/test_trigger_shape_is_taught.py | 106 ++++++++++++++++++-------- 2 files changed, 106 insertions(+), 46 deletions(-) diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index 3d5acf1..cb1352e 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -420,6 +420,14 @@ async def create_rule( brought it back as the top hit. Where a rule prevents a specific failure, put that failure's vocabulary here — the error text, the wrong behaviour, the dead end. + The two spellings, side by side: + RETRIEVES: "the migration failed with a check violation on a + column we just extended" + COLLAPSES: "when working on migrations" + The second names a CATEGORY. No session ever produces a + category — it produces the command, the error, the half-formed + ask — so a trigger written that way leaves the embedded + document to be carried by the title alone. tier: "always_on" (default) or "conditional". The test: can you name the trigger WITHOUT naming a system, an artifact type or a moment? If the honest answer is "whenever you @@ -519,10 +527,11 @@ async def create_project_rule( when_to_apply: WHEN this rule fires — the trigger, not the instruction, and the rule's retrieval surface: name the SYMPTOM, the words someone would type while stuck. Show the moment rather - than classifying it — "the migration failed with a check - violation on a column we just extended" retrieves; "when working - on migrations" does not, because it names a category no session - ever types. See create_rule for the full argument. It informs the + than classifying it: + RETRIEVES: "the CI job passed locally and fails on the runner + with a permission error" + COLLAPSES: "when touching CI config" + See create_rule for the full argument. It informs the tier below rather than deciding it, since a project rule's tier turns on area-scope, not on whether the trigger can be named. @@ -619,7 +628,12 @@ async def update_rule( written, the operator's actual message — and rewrite it in that vocabulary if it does not. Prefer the words someone produces while the rule applies, including the rationalisation they would be drafting to talk themselves out - of it. See create_rule for the full argument and the measurement behind it. + of it — that rationalisation is often the only text in existence at the + moment the rule should fire: + RETRIEVES: "catching yourself drafting 'this is small enough to not + count' about a rule you have already read" + COLLAPSES: "when the next action would conflict with a standing rule" + See create_rule for the full argument and the measurement behind it. TO EMPTY A FIELD, NAME IT: clear_fields=["verify_with"]. Passing "" cannot do it — "" means "leave this alone" here, which is what lets you update @@ -716,13 +730,15 @@ async def create_preference( session would actually be producing then: the command it is about to run, the code it is writing, the thing the operator just asked for. - Show the moment rather than classifying it. Write "the operator pasted a stack - trace and said it is still broken" rather than "during hard debugging"; - write "about to `git push` with an earlier CI run still unread" rather - than "when pacing actions". The second spelling of each is a category, and - no session ever produces a category — it produces the command, the error - text, the half-formed ask. A trigger naming the abstraction collapses the - record toward its title and it never arrives. + Show the moment rather than classifying it: + RETRIEVES: "the operator pasted a stack trace and said it is still + broken" + COLLAPSES: "during hard debugging" + The second is a category, and no session ever produces a category — it + produces the command, the error text, the half-formed ask. A trigger + naming the abstraction collapses the record toward its title and it + never arrives. Both of those describe the same preference; only one of + them can be found at the moment it applies. `arose_from_id` IS REQUIRED for the same kind of reason. A preference is expected to change as the work teaches it, and a corpus that drifts with @@ -826,7 +842,11 @@ async def update_preference( operator's message that should have summoned it. If it describes a situation rather than quoting the moment, rewrite it in the words they actually type — and in the words YOU would be producing while about to get - this wrong. update_rule carries the full argument. + this wrong: + RETRIEVES: "the operator said 'clean this up' or 'make it work like', + naming an outcome rather than a change" + COLLAPSES: "when reading any request from the operator" + update_rule carries the full argument. Empty strings leave fields unchanged; clear_fields empties them by name, exactly as update_rule does. diff --git a/tests/test_trigger_shape_is_taught.py b/tests/test_trigger_shape_is_taught.py index 6d7dd17..74d68ef 100644 --- a/tests/test_trigger_shape_is_taught.py +++ b/tests/test_trigger_shape_is_taught.py @@ -1,4 +1,4 @@ -"""Every rule-write surface documents the trigger it can write (#3855). +"""Every rule-write surface SHOWS what a trigger looks like (#3855). WHY THIS EXISTS @@ -19,10 +19,45 @@ trigger that already reads fine as English is the one nobody rewrites. WHAT THIS PINS -One structural property, no wording: a tool that accepts `when_to_apply` -mentions it in its docstring. That is exactly what `update_preference` -failed before #3855 — the parameter existed and the docstring never said so, -which is how six triggers got written against no contract at all. +Two properties, neither of them wording: + + 1. A tool accepting `when_to_apply` mentions it. Exactly what + `update_preference` failed before #3855 — the parameter existed and + the docstring never said so. + 2. It carries a worked CONTRAST: one `RETRIEVES:` example and one + `COLLAPSES:` example, quoted, and different from each other. + +WHY A CONVENTION RATHER THAN A PROSE HEURISTIC + +The second property guards the softer regression — guidance kept but +abstracted back to "name the moment in session vocabulary", which is advice +about being concrete that is not itself concrete, and is the shape that was +already on file while the corpus filled with categories. + +Two attempts to detect that in free prose were written and discarded, and +the reason is worth keeping because it generalises: + + - Counting quoted multi-word phrases anywhere in the docstring measured + ambient quotation, not demonstrated triggers. It PASSED the abstracted + version by scoring unrelated prose, and text with an odd number of + quote characters produced matches spanning the gap BETWEEN two + unrelated phrases. + - Scoping that count to a window after each "trigger"/"when_to_apply" + mention then FAILED `create_preference` in its correct state, because + its examples sit further from the first mention than any defensible + window reaches. + +Both were proxies trying to infer demonstration from prose. The fix was to +stop inferring: a two-line labelled contrast is unambiguous to parse, free +in its wording, and a better teaching form than the sentences it replaced — +`RETRIEVES:`/`COLLAPSES:` names the mechanism, so the label does work for +the reader instead of only for the test. Where a property cannot be +measured, changing the shape of the thing is cheaper than a cleverer +measurement. + +What it still cannot do is judge whether a GOOD example is good. It catches +what actually happened: a write surface shipping with the contrast absent, +half-present, or tidied back into abstract advice. THE SURFACE LIST IS DERIVED, NEVER HAND-KEPT. It comes from what `rulebooks.register()` actually hands the server, filtered to signatures @@ -30,34 +65,9 @@ taking `when_to_apply`, so a rule-write tool added later is in scope on the day it is added. A hand-kept list has to be remembered by the person least likely to know it exists — the argument `RANKED_SOURCES` makes in services/rule_usage.py, and the reason #3855 happened at all. - -WHAT THIS DELIBERATELY DOES NOT PIN, AND WHY IT IS NOT HERE - -The regression worth catching is the one above's *softer* twin: guidance -kept but abstracted back to "name the moment in session vocabulary", which -is advice about being concrete that is not itself concrete — the shape that -was already on file while the corpus filled with categories. - -Two predicates for it were written and both were discarded, because each was -falsified and each failed: - - - Counting quoted multi-word phrases anywhere in the docstring measured - ambient quotation, not demonstrated triggers. It passed the broken - version by scoring unrelated prose ("what happens if someone doesn't do - this"), and prose with an odd number of quote characters produced - matches spanning the gap BETWEEN two unrelated phrases. - - Scoping that count to a window after each "trigger"/"when_to_apply" - mention then failed `create_preference` in its CORRECT state, because - its examples sit further from the first mention than any defensible - window reaches. - -Rule 167's standard settles it: a check that passes on the broken code reads -as coverage and stops anyone looking again, so no check is the better of the -two. Whether a docstring teaches the shape WELL stays a reading judgment, -and the record of that decision lives here so the next author does not spend -the same two rounds discovering it. """ import inspect +import re import pytest @@ -65,6 +75,11 @@ from tests.helpers import tool_doc as _doc _MODULE = "scribe.mcp.tools.rulebooks" +# The worked contrast. Quoted so the example's own punctuation stays inside +# it, labelled so no proximity guess is needed to find it. +_RETRIEVES = re.compile(r'RETRIEVES:\s*"([^"]+)"') +_COLLAPSES = re.compile(r'COLLAPSES:\s*"([^"]+)"') + def _registered_tools() -> list: """Every function `rulebooks.register()` actually hands to the server. @@ -111,6 +126,31 @@ def test_a_trigger_writing_surface_documents_the_field(name): "docstring. The tool docstring is the agent-facing contract " "(rule 119), and a trigger written against no contract is the #3855 " "defect: six preferences named a category instead of a moment, and " - "a category is not a thing any session ever types. Say what the " - "field is for, and show a moment — see create_rule." + "a category is not a thing any session ever types." + ) + + +@pytest.mark.parametrize("name", _trigger_writers()) +def test_a_trigger_writing_surface_shows_the_contrast(name): + """Advice about being concrete has to be concrete itself.""" + doc = _doc(_MODULE, name) + retrieves, collapses = _RETRIEVES.findall(doc), _COLLAPSES.findall(doc) + + assert retrieves and collapses, ( + f"{name} is missing its worked contrast — found " + f"{len(retrieves)} RETRIEVES and {len(collapses)} COLLAPSES example(s), " + "and one of each is required. Telling an author to 'name the moment " + "in session vocabulary' without showing one is the guidance that was " + "already on file while the corpus filled with categories. Add two " + 'quoted lines: RETRIEVES: "" and COLLAPSES: "".' + ) + + same = set(retrieves) & set(collapses) + assert not same, ( + f"{name} shows the same text as both RETRIEVES and COLLAPSES " + f"({next(iter(same))!r}). The pair teaches by DIFFERING — one moment " + "written the way a session would produce it, and the same moment " + "written as a category. Identical halves demonstrate nothing." ) -- 2.54.0 From 10343a601903a192721f0fdad964aaf581e31cd4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 14:00:36 -0400 Subject: [PATCH 03/13] feat(rules): an act surfaces a banded SET of rules, quieter after the first (#3851) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RULEHINT_LIMIT was 1. That was correct while retrieval SUPPLEMENTED a 33-rule resident set — one salient rule beside everything already loaded. Milestone 394 removes residency, and then this arm is the whole delivery: `git push origin dev` is governed by rules 1, 2, 9 and 140 simultaneously, and each alone permits the mistake the others catch. A cap plus a band, not a bigger cap. The old argument's real content is that a fixed k invents lines — it fills slots whether or not anything deserves them. A band keeps only what scored close to the top, so one clearly relevant rule still shows one and four competing rules show four. The corpus decides; the cap is a ceiling on the worst case, not the usual answer. MEASURED, AND IT CORRECTED THE PREDICTION. The expectation was that rules would rank sharply, since rule_document() shapes them like snippets and note 2485 measured snippets separating their top hit by 0.153 against 0.010-0.023 for every other kind. Three probes against real act queries say otherwise: `git push origin dev` top 0.757, gap 0.022 `docker compose up -d` top 0.685, gap 0.016 a bare-owner-filter query top 0.656, gap 0.020 Dev-log territory, not snippet territory — rules arrive as a packed block, so shaping alone did not buy separation. The band is therefore narrow: at 0.10 (the notes menu's value) every one of the top eight on the push probe falls inside, including a CI-registry rule and another project's branch policy. 0.05 admits about three ranks. COST, MEASURED RATHER THAN ASSUMED. The old comment claimed a line costs ~40 tokens. It is ~143 once the trigger is rendered, and #3855 roughly tripled trigger lengths, so five full lines are ~646 tokens before EVERY Bash call. Hence rank decides volume: the top hit keeps the full rendering, later hits are cited (~198 tokens total, 1.4x the old single line, for four more rules). The old paragraph's instinct — a fourth voice at full volume is where a reader stops reading — is answered by making later lines quieter rather than by refusing to have them. Band before dedup, deliberately. The band is a statement about scores; letting the ledger reorder it would make "you were told this already" change what counts as relevant. Same axis independence the renderer already keeps between `kind` and `seen`, and `rule_ids` stays fresh-only (#3752) so #3668's identity between logged results and surfacing rows survives. `suppressed` now covers both causes and says so. Both act arms take the same band: their score distributions are the same shape, and only the query differs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/services/plugin_context.py | 204 ++++++++++++++++++++------ tests/test_rule_hint_band.py | 169 +++++++++++++++++++++ 2 files changed, 331 insertions(+), 42 deletions(-) create mode 100644 tests/test_rule_hint_band.py diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 1e7d329..46a4f0b 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -129,34 +129,76 @@ WRITEPATH_DEFAULT_THRESHOLD = 0.68 RULEHINT_THRESHOLD_KEY = "kb_rulehint_threshold" RULEHINT_DEFAULT_THRESHOLD = 0.72 -# ONE rule per write, not two — and this is deliberately NOT a knob. +# A SET OF RULES PER ACT, NOT THE SINGLE BEST ONE (#3851). # -# With a corpus this small, top-k does as much damage as the threshold: k=2 -# over a few dozen candidates means the second line is almost always the -# second-best noise, arriving with the same confident framing as the first. -# Halving k halves that regardless of where the bar sits. +# This was 1, and the reasoning for that is kept below rather than deleted +# because it was correct for the world it was written in and the half of it +# that still holds is what shapes the replacement. # -# It also BOUNDS the blast radius of widening the pool (below): with k=1 a -# wider corpus can change WHICH rule surfaces and how often one does, but it -# can never make a single hint longer. The loudness of one hint and the -# eligibility of a rule are separate controls, and only one of them moved. +# THE OLD ARGUMENT. "With a corpus this small, top-k does as much damage as +# the threshold: k=2 over a few dozen candidates means the second line is +# almost always the second-best noise, arriving with the same confident +# framing as the first." True — and note the premise. Retrieval was then a +# SUPPLEMENT to a 33-rule resident set, so the arm's job was to add one +# salient rule beside everything the session already held. One was the right +# number for an accent. # -# It stays a constant because it is a decision about how LOUD one hint may be, -# not a per-install tuning question. The hint already carries prior art, shape -# signals and staleness; rules are the fourth voice in it, and a fourth voice -# that speaks twice is where a reader stops reading. Nothing suggests an -# operator wants this different, and a knob nobody turns is a knob that only -# adds a way to misconfigure the surface (rule 25 cuts both ways). -RULEHINT_LIMIT = 1 +# WHAT CHANGED. Milestone 394 removes residency, and then this arm is not the +# accent, it is the whole delivery. Moments genuinely need several rules at +# once: `git push origin dev` is governed by 1 (`dev` is home), 2 (never +# `main` unasked), 9 (poll CI) and 140 (let each action land) simultaneously, +# and each alone permits the mistake the others catch. One slot cannot serve +# that, and "the single best" is not a coherent answer when four rules bind. +# +# WHY A CAP PLUS A BAND, RATHER THAN A BIGGER CAP. The old argument's real +# content is that a fixed k invents lines — it fills slots whether or not +# anything deserves them. A band does not: it keeps what is close to the top +# and nothing else, so a moment with one clearly-relevant rule still shows +# one, and a moment with four shows four. The corpus decides, not a constant. +# The cap survives as a ceiling on the worst case, not as the usual answer. +RULEHINT_LIMIT = 5 -# AND A REPEAT COMPETES FOR THAT ONE SLOT ON RANK ALONE (#3750). +# MEASURED, NOT REASONED — and the reasoning it replaced was wrong (#3851). +# +# The prediction was that rules would rank SHARPLY, because `rule_document()` +# shapes them the way snippets are shaped — trigger in the embedded title and +# again above the body — and note 2485 measured snippets separating their top +# hit by 0.153 while every other kind managed 0.010–0.023. +# +# They do not. Three probes against real act queries, scored by the same +# embedding the arms use: +# +# `git push origin dev` top 0.757, gap to second 0.022 +# `docker compose up -d` top 0.685, gap to second 0.016 +# a bare-owner-filter query top 0.656, gap to second 0.020 +# +# That is dev-log territory, not snippet territory: rules arrive as a +# tightly-packed block. Shaping alone did not buy separation, which is worth +# recording because the opposite was the natural inference from 2485. +# +# So the band is narrow BECAUSE the corpus is flat. At 0.10 — the notes +# menu's value — every one of the top eight on the push probe falls inside, +# including a CI-registry rule and another project's branch policy. At 0.05 +# it admits roughly three ranks, which is the span where the scores are still +# saying something. 2485's Finding 3 is the standing caveat: no band value +# fixes a tie, and if rules ever rank as flat as dev-logs did this control +# stops working and the answer is a reranker (#1038), not a smaller number. +_RULEHINT_BAND = 0.05 + +# 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 # 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? +# take its place, or step aside for a fresh rule behind it? # -# It takes the slot, and nothing is fetched behind it. Two reasons. +# It keeps its place, and nothing is promoted past it. #3851 widened the arm +# from one slot to a banded set and did NOT reopen this: a repeat still ranks +# where it ranks, and the band is applied to scores with no regard for what +# the session has seen. The two are independent, exactly as `kind` and `seen` +# are independent in the renderer — rank answers "what is relevant now" and +# the ledger answers "have you been told", and neither is evidence about the +# other. Two reasons, both unchanged by the widening. # # 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 @@ -166,13 +208,21 @@ RULEHINT_LIMIT = 1 # 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. +# AND THE COST OF A SECOND LINE IS NOW PAID DIFFERENTLY, NOT WISHED AWAY. +# This paragraph used to read "a second line is the one thing the limit above +# forbids", on the strength of "a reference costs the same ~40 tokens as a +# first surfacing". Both halves are now wrong and the second was already +# wrong when written: a full line is ~143 tokens once the trigger is rendered, +# and #3855's rewrite of the corpus roughly tripled trigger length, so five +# full lines on a push probe measure ~646 tokens before EVERY Bash call. +# +# That number is why the widening pairs with a compact rendering rather than +# arriving alone (see `_rule_hint_line`). The old paragraph's instinct — that +# a fourth voice which speaks at full volume twice is where a reader stops +# reading — is the half worth keeping, and it is answered by making the +# later lines quieter rather than by refusing to have them. Top hit full, +# the rest as references: ~198 tokens, 1.4x the old single line, for four +# more rules. # # The consequence is deliberate and worth naming: a rule that keeps ranking # first for a recurring situation keeps being referenced, every time the @@ -1123,9 +1173,48 @@ async def get_writepath_config(user_id: int) -> dict: "rule_threshold": rule_threshold, } -def _rule_hint_line(rule, *, where: str, seen: bool) -> str: +def _rule_band(hits: list) -> list: + """The top hit, plus every hit within `_RULEHINT_BAND` of it (#3851). + + The instrument that lets an act surface a SET without inventing one: a + fixed k fills its slots whether or not anything deserves them, while this + keeps only what the scores say is close, so one clearly-relevant rule + still shows one and four competing rules show four. + + Sync and pure, and deliberately its own function rather than a comparison + written twice — the two act arms are the pair #3497 records drifting apart + by being modelled on each other instead of sharing. + + Takes `(score, rule)` pairs already ordered best-first, as both + `semantic_search_*` helpers return them. + """ + if not hits: + return [] + top = hits[0][0] + return [(s, r) for s, r in hits if s >= top - _RULEHINT_BAND] + + +def _rule_hint_line(rule, *, where: str, seen: bool, compact: bool = False) -> str: """One rule hint line — both arms, both tails, both kinds (#3750, #3849). + THREE INDEPENDENT AXES SINCE #3851. `compact` joins `kind` and `seen`, and + like them it reads none of the others: it says how much ROOM this line + gets, which is a fact about its rank among today's hits rather than about + the rule. A compact line is still a full claim that the rule may apply — + it simply cites the rule instead of quoting its trigger. + + WHY THE LATER LINES ARE QUIETER. Measured at #3851: a full line runs ~143 + tokens once the trigger is rendered, and #3855 tripled trigger lengths + across the corpus, so five full lines cost ~646 tokens before every Bash + call. Top-full-plus-references costs ~198 — 1.4x the old single line, for + four more rules. The budget argument that once justified a single slot was + real; what it actually forbids is four voices at full volume, not four + voices. + + The top hit keeps the full rendering because it is the one the ranker is + most confident about, and a reader who acts on exactly one line should + have acted on that one. + TWO INDEPENDENT AXES. `kind` decides the head, `seen` decides the tail, and neither reads the other. A preference and a rule differ in force; a repeat and a first surfacing differ in whether the session already holds @@ -1192,6 +1281,14 @@ def _rule_hint_line(rule, *, where: str, seen: bool) -> str: f"Read it with get_rule({rule.id}) {reason}; it is not in this " "session's loaded set." ) + if compact: + # No trigger, and a shortened tail. What survives is everything a + # reader needs in order to DECIDE to pull it: the force (noun), the + # identity (title) and the pointer. What goes is the trigger, which is + # the expensive half and the half the top line already demonstrates. + return ( + f"Also \u2014 {noun.lower()} \u201c{rule.title}\u201d: get_rule({rule.id})." + ) return ( f"{noun} that may apply {where} \u2014 \u201c{rule.title}\u201d" + (f" ({trigger})" if trigger else "") @@ -1682,11 +1779,21 @@ async def build_write_path_hint( report=_rep_wpr, ) rule_ms = (time.perf_counter() - rule_t0) * 1000.0 - fresh = [(score, rule) for score, rule in hits if rule.id not in already] - # EVERY hit gets a line; `already` only changes the tail (#3750). - for _score, rule in hits: + # BAND FIRST, dedup second, and the order is the whole point (#3851). + # The band is a statement about the SCORES — what the ranker thinks is + # close to the best match — so letting the ledger reorder it would let + # "you were told this already" change what counts as relevant. Those + # are the independent axes the renderer keeps apart. + kept = _rule_band(hits) + fresh = [(score, rule) for score, rule in kept if rule.id not in already] + # EVERY kept hit gets a line; `already` only changes the tail (#3750), + # and rank only changes how much room it gets (#3851). + for idx, (_score, rule) in enumerate(kept): lines.append( - _rule_hint_line(rule, where="here", seen=rule.id in already) + _rule_hint_line( + rule, where="here", seen=rule.id in already, + compact=idx > 0, + ) ) # `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 @@ -1733,10 +1840,15 @@ async def build_write_path_hint( best_available=_rep_wpr.get("best_available_score"), best_available_id=_rep_wpr.get("best_available_id"), searched=bool(_rep_wpr.get("searched", True)), - # What the ranker found and this session had already been told. - # Without it a zero row cannot say whether the bar was too high or - # the reader was simply ahead of it — and only the first is a - # reason to move the threshold. + # FOUND BUT NOT SHOWN, which since #3851 has TWO causes: the + # session had already been told (the ledger), or the score fell + # outside `_RULEHINT_BAND` of the top hit. Both are counted here + # because the question this answers is unchanged — a zero row must + # be able to say whether the bar was too high or whether the arm + # simply chose not to speak, and only the first is a reason to + # move the threshold. Splitting the two causes needs its own + # column and is worth doing only if the band turns out to be + # dropping rules anyone wanted. suppressed=len(hits) - len(fresh), ) if fresh: @@ -1824,7 +1936,10 @@ async def build_tool_rule_hint( duration_ms = (time.perf_counter() - t0) * 1000.0 already = set(exclude_rule_ids or []) - fresh = [(score, rule) for score, rule in hits if rule.id not in already] + # Band first, dedup second — see the sibling arm for why that order is + # load-bearing rather than incidental. + kept = _rule_band(hits) + fresh = [(score, rule) for score, rule in kept if rule.id not in already] # Logged BEFORE the early return, for the reason spelled out at length # on the write-path arm above: a call that found nothing is the only @@ -1842,22 +1957,27 @@ async def build_tool_rule_hint( best_available=_rep_ptr.get("best_available_score"), best_available_id=_rep_ptr.get("best_available_id"), searched=bool(_rep_ptr.get("searched", True)), - # See the sibling arm. It matters more here: this arm fires on every - # Bash call, so a long session excludes its way to an all-zero row - # and the threshold looks wrong when nothing about it is. + # See the sibling arm, including why this now counts BOTH the + # ledger and the band. It matters more here: this arm fires on + # every Bash call, so a long session excludes its way to an + # all-zero row and the threshold looks wrong when nothing about it + # is. suppressed=len(hits) - len(fresh), ) - # `hits`, not `fresh` (#3750). A call whose only hit is a repeat still + # `kept`, 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: + if not kept: return out lines = [ _rule_hint_line( rule, where=f"to this {tool_name} call", seen=rule.id in already, + # Rank decides volume (#3851): the ranker's best guess gets the + # trigger, the rest get cited. + compact=idx > 0, ) - for _score, rule in hits + for idx, (_score, rule) in enumerate(kept) ] # FRESH-ONLY, for the reason given on the sibling arm: a reference is a # rendering decision, not a retrieval outcome, and counting it here diff --git a/tests/test_rule_hint_band.py b/tests/test_rule_hint_band.py new file mode 100644 index 0000000..af52253 --- /dev/null +++ b/tests/test_rule_hint_band.py @@ -0,0 +1,169 @@ +"""An act surfaces a SET of rules, and rank decides how loudly (#3851). + +WHY THIS EXISTS + +`RULEHINT_LIMIT` was 1. That was right while retrieval merely SUPPLEMENTED a +33-rule resident set — one salient rule beside everything already loaded. It +stops being right the moment milestone 394 removes residency, because then +this arm is the whole delivery, and `git push origin dev` is governed by +rules 1, 2, 9 and 140 at once, each of which alone permits the mistake the +others catch. + +Two instruments, and they answer different questions: + + - `_rule_band` decides HOW MANY. A fixed k fills its slots whether or not + anything deserves them; a band keeps only what scored close to the top, + so one clearly-relevant rule still shows one. + - `compact` decides HOW LOUD. Measured at #3851: a full line is ~143 tokens + once the trigger is rendered, so five of them cost ~646 before every Bash + call. Top-full-plus-references costs ~198. + +WHAT THIS PINS + +Structure, never wording — the lines are prose and will be rewritten: + + 1. The band keeps the top hit and everything within `_RULEHINT_BAND`, and + drops what falls outside. Falsified below from both sides: a hit just + inside survives, a hit just outside does not. + 2. Rank decides volume — the first line carries the trigger, later lines do + not, and every line names its rule's id so any of them can be pulled. + 3. The band reads SCORES ONLY. A top hit the session has already seen still + anchors the band, and its score still sets the cutoff. This is the axis + independence the renderer already keeps between `kind` and `seen`, and + the regression it prevents is subtle: letting the ledger reorder the + band would make "you were told this" change what counts as relevant. + +The band width itself is deliberately NOT pinned. It is a tuning value with +a comment recording the measurement behind it, and a test asserting 0.05 +would fail on every future retune while proving nothing about behaviour — +so the cases below express their scores as offsets from the constant. +""" +import pytest + +from scribe.services.plugin_context import ( + _RULEHINT_BAND, + _rule_band, + _rule_hint_line, +) +from tests.helpers import fake_rule + +_TRIGGER = "about to run git push with an earlier CI run still unread" + + +def _hit(score: float, rule_id: int): + return (score, fake_rule(id=rule_id, title=f"rule {rule_id}", + when_to_apply=_TRIGGER)) + + +def test_an_empty_result_stays_empty(): + """No hits is not a crash and not a phantom line.""" + assert _rule_band([]) == [] + + +def test_the_band_keeps_a_hit_just_inside_it(): + """The whole point: a close second rule reaches the agent.""" + top = 0.75 + hits = [_hit(top, 1), _hit(top - _RULEHINT_BAND + 0.01, 2)] + assert [r.id for _s, r in _rule_band(hits)] == [1, 2] + + +def test_the_band_drops_a_hit_just_outside_it(): + """And the band must actually BIND, or it is a fixed k wearing a hat.""" + top = 0.75 + hits = [_hit(top, 1), _hit(top - _RULEHINT_BAND - 0.01, 2)] + assert [r.id for _s, r in _rule_band(hits)] == [1] + + +def test_one_clearly_better_rule_still_surfaces_alone(): + """The behaviour the old limit of 1 got right, which must not regress. + + A moment with a single relevant rule shows one line, because the corpus + said so — not because a constant capped it. + """ + hits = [_hit(0.80, 1), _hit(0.55, 2), _hit(0.54, 3)] + assert [r.id for _s, r in _rule_band(hits)] == [1] + + +def test_a_flat_cluster_surfaces_together(): + """Measured shape of this corpus: adjacent rules sit ~0.02 apart. + + Four rules governing one act is the `git push` case the step exists for, + and at the measured spacing they must arrive together rather than the + ranker picking one of four near-ties. + """ + hits = [_hit(0.757, 2), _hit(0.735, 7), _hit(0.726, 1), _hit(0.711, 9)] + assert [r.id for _s, r in _rule_band(hits)] == [2, 7, 1, 9] + + +def test_the_band_is_computed_from_scores_not_from_the_ledger(): + """A seen top hit still anchors the band (#3750 x #3851). + + `_rule_band` never learns what the session has seen — dedup happens after + it, in the arms. Pinned here because the tempting "reorder so a fresh rule + leads" would silently change the cutoff, and the failure is invisible: the + arm would still emit lines, just the wrong set. + """ + hits = [_hit(0.80, 1), _hit(0.78, 2), _hit(0.60, 3)] + kept = _rule_band(hits) + # Independent of any `already` set, because it is not consulted. + assert [r.id for _s, r in kept] == [1, 2] + + +@pytest.mark.parametrize("seen", [False, True]) +def test_the_leading_line_carries_the_trigger(seen): + """Rank 0 gets the full rendering, on either tail.""" + line = _rule_hint_line( + fake_rule(id=4, title="dev is home", when_to_apply=_TRIGGER), + where="to this Bash call", seen=seen, compact=False, + ) + assert _TRIGGER in line + assert "get_rule(4)" in line + + +@pytest.mark.parametrize("seen", [False, True]) +def test_a_later_line_cites_its_rule_without_quoting_the_trigger(seen): + """Rank > 0 is a reference: identity and pointer, no trigger. + + The trigger is the expensive half — it is what made a full line ~143 + tokens — and the leading line has already demonstrated the shape. Both + assertions matter: dropping the trigger is the saving, and keeping + `get_rule(id)` is what makes the saving safe, because a cited rule the + reader cannot pull is just noise. + """ + line = _rule_hint_line( + fake_rule(id=4, title="dev is home", when_to_apply=_TRIGGER), + where="to this Bash call", seen=seen, compact=True, + ) + assert _TRIGGER not in line + assert "dev is home" in line + assert "get_rule(4)" in line + + +def test_a_compact_line_is_materially_shorter_than_a_full_one(): + """The cost claim, asserted rather than asserted-in-a-comment. + + Not a token count — that would pin the tokenizer. Half the characters is + the property that makes widening the arm affordable, and it is what fails + if a later edit reintroduces the trigger into the compact branch. + """ + rule = fake_rule(id=4, title="dev is home", when_to_apply=_TRIGGER) + full = _rule_hint_line(rule, where="here", seen=False, compact=False) + compact = _rule_hint_line(rule, where="here", seen=False, compact=True) + assert len(compact) * 2 < len(full) + + +def test_a_preference_keeps_its_noun_when_compact(): + """Force survives the shortening (#3849). + + `kind` and `compact` are independent axes. A preference rendered as a + reference must still not read as a rule — the noun is the whole of the + visual difference, so losing it in the compact branch would make every + cited preference bind. + """ + line = _rule_hint_line( + fake_rule(id=5, title="pace debugging", kind="preference", + when_to_apply=_TRIGGER), + where="here", seen=False, compact=True, + ) + assert "preference" in line.lower() + assert "standing rule" not in line.lower() -- 2.54.0 From 40189147d2bb3f85cc6895c7b38b6580b7188ec4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 14:07:32 -0400 Subject: [PATCH 04/13] fix(rules): a shortened rule line must not decide what it says about holding (#3851) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI run 6485 was red. Six failures, three causes, and only one of them was a stale test. THE REAL DEFECT. The compact branch dropped the `seen` TAIL along with the trigger, so a rule the session had already been told rendered exactly like one it had not. #3750's whole argument is that those are different claims — a repeat is rendered precisely because the session may no longer HOLD what it was told — and the tail is the entire difference a reader can act on. test_a_rule_the_session_already_holds_is_referenced_not_re_offered caught it within one commit, which is that guard working as intended. Fixed by keeping the tail and dropping only the trigger, which is both the cheaper and the safer cut: a trigger runs 300-400 characters after #3855, a tail about 100. Re-measured on the real renderer — top-full-plus-references is ~299 tokens against ~568 for five full lines, so about 2x the old single line rather than the 1.4x claimed before, for four more rules and no lost information. The comments carrying the old figure are corrected rather than left to read as a decision nobody made. THE FIXTURE THAT STRADDLED THE BAND. `_THREE_HITS` spanned 0.81-0.74 against a 0.05 band, so the act arms dropped its lowest hit and four cases of test_both_recorders_report_the_same_rules_for_one_call failed reporting a count mismatch — under a message blaming the exclusion filter. A guard pointing confidently at the wrong subsystem costs more than no guard, because it is believed. Scores retightened to 0.81/0.80/0.79 and the precondition is now asserted by a named test, so a future band change is told where the problem is instead of through four confusing failures. THE STALE CONSTANT GUARD. test_the_rule_arm_asks_for_one_rule_not_two pinned RULEHINT_LIMIT == 1 — a real decision, correctly guarded, for a world with a resident set. Rewritten to pin what replaced it, as relationships rather than values (rule 115): the arm can return several, and rules are narrowed HARDER than the notes menu because they measured flatter, not sharper. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/services/plugin_context.py | 39 ++++++++++++++------- tests/test_rule_hint_band.py | 50 ++++++++++++++++++++++----- tests/test_rule_usage_wiring.py | 31 +++++++++++++++-- tests/test_write_path_trigger.py | 28 ++++++++++++--- 4 files changed, 121 insertions(+), 27 deletions(-) diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 46a4f0b..67457d7 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -221,8 +221,12 @@ _RULEHINT_BAND = 0.05 # a fourth voice which speaks at full volume twice is where a reader stops # reading — is the half worth keeping, and it is answered by making the # later lines quieter rather than by refusing to have them. Top hit full, -# the rest as references: ~198 tokens, 1.4x the old single line, for four -# more rules. +# the rest as references: ~299 tokens against ~568 for five full lines, so +# roughly 2x the old single line for four more rules. +# +# The reference keeps its TAIL and loses only its trigger, which is both the +# cheaper and the safer cut — see `_rule_hint_line`, where the first attempt +# dropped the tail as well and #3750's guard caught it within one commit. # # The consequence is deliberate and worth naming: a rule that keeps ranking # first for a recurring situation keeps being referenced, every time the @@ -1201,14 +1205,17 @@ def _rule_hint_line(rule, *, where: str, seen: bool, compact: bool = False) -> s like them it reads none of the others: it says how much ROOM this line gets, which is a fact about its rank among today's hits rather than about the rule. A compact line is still a full claim that the rule may apply — - it simply cites the rule instead of quoting its trigger. + it simply cites the rule instead of quoting its trigger. Crucially it + still carries the `seen` tail, so the three axes stay genuinely + independent: shortening a line must not decide what it says about whether + the session is holding the rule. WHY THE LATER LINES ARE QUIETER. Measured at #3851: a full line runs ~143 tokens once the trigger is rendered, and #3855 tripled trigger lengths - across the corpus, so five full lines cost ~646 tokens before every Bash - call. Top-full-plus-references costs ~198 — 1.4x the old single line, for - four more rules. The budget argument that once justified a single slot was - real; what it actually forbids is four voices at full volume, not four + across the corpus, so five full lines cost ~568 tokens before every Bash + call. Top-full-plus-references costs ~299 — about 2x the old single line, + for four more rules. The budget argument that once justified a single slot + was real; what it actually forbids is four voices at full volume, not four voices. The top hit keeps the full rendering because it is the one the ranker is @@ -1282,12 +1289,20 @@ def _rule_hint_line(rule, *, where: str, seen: bool, compact: bool = False) -> s "session's loaded set." ) if compact: - # No trigger, and a shortened tail. What survives is everything a - # reader needs in order to DECIDE to pull it: the force (noun), the - # identity (title) and the pointer. What goes is the trigger, which is - # the expensive half and the half the top line already demonstrates. + # THE TRIGGER GOES; THE TAIL STAYS. Only one of the two is expensive \u2014 + # a trigger runs 300-400 characters after #3855, the tail about 100 \u2014 + # so dropping the trigger is nearly the whole saving and dropping the + # tail would be mostly sacrifice. + # + # It would also destroy the one thing #3750 exists to say. The tail is + # what tells a reader whether they were already told this and may no + # longer be holding it, and that is the entire difference they can act + # on; a reference with no tail reads as a first surfacing whether it is + # one or not. This branch shipped without it for one commit and + # test_a_rule_the_session_already_holds_is_referenced_not_re_offered + # caught it, which is the guard working exactly as #3750 intended. return ( - f"Also \u2014 {noun.lower()} \u201c{rule.title}\u201d: get_rule({rule.id})." + f"Also \u2014 {noun.lower()} \u201c{rule.title}\u201d. {tail}" ) return ( f"{noun} that may apply {where} \u2014 \u201c{rule.title}\u201d" diff --git a/tests/test_rule_hint_band.py b/tests/test_rule_hint_band.py index af52253..c86ffdd 100644 --- a/tests/test_rule_hint_band.py +++ b/tests/test_rule_hint_band.py @@ -124,11 +124,11 @@ def test_the_leading_line_carries_the_trigger(seen): def test_a_later_line_cites_its_rule_without_quoting_the_trigger(seen): """Rank > 0 is a reference: identity and pointer, no trigger. - The trigger is the expensive half — it is what made a full line ~143 - tokens — and the leading line has already demonstrated the shape. Both - assertions matter: dropping the trigger is the saving, and keeping - `get_rule(id)` is what makes the saving safe, because a cited rule the - reader cannot pull is just noise. + The trigger is the expensive half — 300-400 characters after #3855 — and + the leading line has already demonstrated the shape. Both assertions + matter: dropping the trigger is the saving, and keeping `get_rule(id)` is + what makes the saving safe, because a cited rule the reader cannot pull is + just noise. """ line = _rule_hint_line( fake_rule(id=4, title="dev is home", when_to_apply=_TRIGGER), @@ -139,14 +139,48 @@ def test_a_later_line_cites_its_rule_without_quoting_the_trigger(seen): assert "get_rule(4)" in line +def test_shortening_a_line_does_not_decide_what_it_says_about_holding(): + """A reference still tells a repeat from a first surfacing (#3750 x #3851). + + This is the regression the first cut of #3851 actually shipped: the + compact branch dropped the tail along with the trigger, so a rule the + session had already been told read exactly like one it had not. #3750's + whole argument is that the two are different claims — a repeat is rendered + precisely because the session may no longer HOLD what it was told — and + the tail is the entire difference a reader can act on. + + `compact` and `seen` are independent axes. How much room a line gets is a + fact about its rank; whether the session holds it is a fact about the + ledger; and neither may be allowed to answer the other's question. + """ + rule = fake_rule(id=4, title="dev is home", when_to_apply=_TRIGGER) + seen = _rule_hint_line(rule, where="here", seen=True, compact=True) + fresh = _rule_hint_line(rule, where="here", seen=False, compact=True) + assert seen != fresh + assert "no longer hold it" in seen + assert "not in this session's loaded set" in fresh + + def test_a_compact_line_is_materially_shorter_than_a_full_one(): - """The cost claim, asserted rather than asserted-in-a-comment. + """The cost claim, asserted rather than left in a comment. Not a token count — that would pin the tokenizer. Half the characters is the property that makes widening the arm affordable, and it is what fails - if a later edit reintroduces the trigger into the compact branch. + if a later edit puts the trigger back into the compact branch. + + Measured against a REALISTIC trigger, because that is where the saving + lives: the rules this arm carries run 300-400 characters of trigger after + #3855, and a toy one-line trigger would make this pass on a compact branch + that had stopped saving anything. """ - rule = fake_rule(id=4, title="dev is home", when_to_apply=_TRIGGER) + long_trigger = ( + "Opening or merging a `dev`->`main` pull request, running " + "`git push origin main`, `git tag`, or minting a release, image tag " + "or other public artifact. Also whenever CI has just gone green and " + "the next step feels like shipping it, and whenever an earlier merge " + "this session reads like standing permission for the next one." + ) + rule = fake_rule(id=4, title="dev is home", when_to_apply=long_trigger) full = _rule_hint_line(rule, where="here", seen=False, compact=False) compact = _rule_hint_line(rule, where="here", seen=False, compact=True) assert len(compact) * 2 < len(full) diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index 82b7976..981c480 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -1026,10 +1026,37 @@ async def test_a_shown_hit_is_not_counted_as_suppressed(): _THREE_HITS = [ (0.81, fake_rule(id=156, title="A wait with no deadline is a bug")), - (0.77, fake_rule(id=157, title="A loop re-arms in a finally")), - (0.74, fake_rule(id=161, title="Reach the forge through its MCP tools")), + (0.80, fake_rule(id=157, title="A loop re-arms in a finally")), + (0.79, fake_rule(id=161, title="Reach the forge through its MCP tools")), ] +def test_the_three_hit_fixture_sits_inside_the_rule_band(): + """The fixture's own precondition, asserted rather than commented (#3851). + + The act arms band before they dedup, so a fixture whose spread straddles + `_RULEHINT_BAND` loses its lowest hit to the BAND and then reports a count + mismatch — under a message blaming the exclusion filter. That is the + failure this file is least able to survive: a guard pointing confidently + at the wrong subsystem costs more than no guard, because it is believed. + + Not hypothetical. The spread was 0.07 against a 0.05 band, and four cases + of `test_both_recorders_report_the_same_rules_for_one_call` failed that + way the moment the band shipped. + + Widening the band leaves this alone; narrowing it past the spread must + retighten these scores, and says so here rather than through four + confusing failures elsewhere. + """ + from scribe.services import plugin_context as pc + + spread = _THREE_HITS[0][0] - _THREE_HITS[-1][0] + assert spread < pc._RULEHINT_BAND, ( + f"the rule-arm fixture spans {spread:.3f} against a band of " + f"{pc._RULEHINT_BAND}: the act arms will drop its lowest hit as " + "out-of-band, and every count assertion below will blame the " + "exclusion filter for it" + ) + _ARMS = [ ("write_path_rule", _run_arm), ("pre_tool_rule", _run_tool_arm), diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 5b6f359..264980e 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -499,13 +499,31 @@ def test_the_rule_bar_defaults_above_the_code_bar(): assert pc.RULEHINT_DEFAULT_THRESHOLD > pc.WRITEPATH_DEFAULT_THRESHOLD -def test_the_rule_arm_asks_for_one_rule_not_two(): - """With a corpus this small, top-k does as much damage as the threshold: - k=2 over a few dozen candidates means the second line is almost always the - second-best noise, carrying the same confident framing as the first.""" +def test_the_rule_arm_asks_for_a_set_and_lets_the_band_narrow_it(): + """WAS `..._asks_for_one_rule_not_two`, pinning `RULEHINT_LIMIT == 1`. + + That guarded a real decision: with retrieval SUPPLEMENTING a 33-rule + resident set, k=2 over a few dozen candidates made the second line the + second-best noise wearing the first line's confident framing. Milestone + 394 removes residency, so this arm becomes the whole delivery and a + `git push` governed by four rules cannot be served by one slot (#3851). + + What replaces it is not simply a bigger k — that is the thing the old + test was right to fear, because a fixed k fills its slots whether or not + anything deserves them. The cap is a ceiling and the BAND is the control, + so both must exist for the arm to be shaped as intended. + + Pinned as relationships rather than values, like the threshold test above + and for the same rule-115 reason: the band is a tuning number measured + against one corpus, and a test asserting 0.05 would fail on every retune + while proving nothing. What must not silently invert is that the arm can + return several, and that rules — which measured FLATTER than notes, not + sharper — are narrowed harder than the notes menu is. + """ from scribe.services import plugin_context as pc - assert pc.RULEHINT_LIMIT == 1 + assert pc.RULEHINT_LIMIT > 1 + assert 0 < pc._RULEHINT_BAND < pc._AUTOINJECT_BAND # --- the minimum-substance floor on the semantic arm (#2223) ------------------ -- 2.54.0 From 690ca0306e855f19788cee912cba31af48fddaaf Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 14:36:18 -0400 Subject: [PATCH 05/13] feat(rules): the command arm gets its own bar, measured (#3853) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One threshold served both act arms. The telemetry says they are not the same problem: write_path_rule 2,325 calls, speaks on 37%, near-miss p50 0.6989 pre_tool_rule 11,768 calls, speaks on 2%, near-miss p50 0.6794 The second is not quiet, it is mute — 11,530 of 11,768 calls said nothing, with near-miss p90 at 0.7097 against a 0.72 bar. Refused mass piled one hundredth under the line is what a bar set too high leaves behind, and the note arms are the control: auto_inject refuses at p90 0.5463, write_path at 0.6738, both far below theirs. The cause is query shape, not corpus. A write-path query is a code payload, long and rich — the case 0.72 was calibrated on. A pre-tool query is a shell command, often under a dozen words: less text, less signal, lower scores for the same relevance. MEASURED. Eight replayed queries against the post-#3855 corpus, consequential acts against innocuous ones: 0.7571 git push origin dev consequential 0.7245 cd ...; git fetch; git add -A consequential 0.7193 git pull --rebase origin dev consequential 0.6850 docker compose up -d consequential ------------------------------------- 0.68 0.6735 wc -l src/*.py && date innocuous 0.6544 grep -rn useState src/ innocuous 0.6099 sed -n '120,160p' package.json innocuous 0.6056 ls -la && cat README.md innocuous At 0.72 three of four consequential acts retrieved nothing, including `git pull --rebase origin dev`, where rules 153, 1 and 2 all ranked correctly between 0.7126 and 0.7193 and were all refused. The separation is 0.0115 wide. That is a direction, not a settled number, and the comment says so — near_miss_samples on a few days of post-#3855 traffic is what settles it. This also corrects an assumption the old comment stated: it argued 0.68 sat "below where this corpus's noise sits", inferring a higher floor from the corpus being homogeneous. Measured, the command arm's noise ceiling is 0.6735, so 0.68 clears it barely rather than sitting under it. Lowering is safer now than it would have been. Until #3851 this arm had one slot, so the bar was the only noise control; the band now filters downstream, so the bar's job shrank and the bar can. write_path_rule is unchanged — healthy at 0.72 on its own evidence. Guards: the two bars parse independently, garbage falls back to its OWN default rather than to the sibling's (which would silently re-merge them), the command default stays below the write-path default as a direction check, and each arm both SEARCHES and REPORTS at its own bar. That last one is a failure the single-bar code could not have had: retrieval_logs.threshold is what near-miss analysis is read against, so an arm searching at one number and logging another misreports the refusal and invites moving the bar that was already right. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/services/plugin_context.py | 82 ++++++++++++++++++++++++++- tests/test_rule_usage_wiring.py | 79 ++++++++++++++++++++++++++ tests/test_write_path_trigger.py | 57 +++++++++++++++++++ 3 files changed, 216 insertions(+), 2 deletions(-) diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 67457d7..e37bf57 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -126,9 +126,75 @@ WRITEPATH_DEFAULT_THRESHOLD = 0.68 # arrive unread; lower it if rules you needed never arrived. What would RETIRE # it: a cross-encoder rerank (#1038), which would make a similarity bar the # wrong control entirely. +# SCOPED TO THE WRITE-PATH ARM SINCE #3853. The command arm has its own bar +# below, and the measurement that separated them is recorded there. Everything +# above still holds for THIS arm: a code payload is long and rich, which is the +# case 0.72 was calibrated on, and the telemetry says it is working — the +# write-path rule arm speaks on 37% of its calls and its refused mass sits at +# p50 0.6989, comfortably under the bar rather than piled against it. RULEHINT_THRESHOLD_KEY = "kb_rulehint_threshold" RULEHINT_DEFAULT_THRESHOLD = 0.72 +# THE COMMAND ARM'S OWN BAR, AND WHY IT IS NOT THE WRITE PATH'S (#3853). +# +# One bar served both act arms until this. They are not the same problem: a +# write-path query is a code payload, long and rich, while a pre-tool query is +# a shell command — often under a dozen words. Less text, less signal, lower +# scores for the same relevance. At a shared 0.72 the two arms measured like +# different subsystems: +# +# write_path_rule 2,325 calls, speaks on 37%, near-miss p50 0.6989 +# pre_tool_rule 11,768 calls, speaks on 2%, near-miss p50 0.6794 +# +# The second is not a quiet surface, it is a mute one: 11,530 of 11,768 calls +# said nothing, with near-miss p90 at 0.7097 — refused mass piled one +# hundredth under the line, which is the shape a bar set too high leaves. The +# note arms are the control and look nothing like it (auto_inject refuses at +# p90 0.5463, write_path at 0.6738, both far below their bars). +# +# WHAT 0.68 IS MEASURED AGAINST. Eight replayed queries, consequential acts +# against innocuous ones, scored on the post-#3855 corpus: +# +# 0.7571 git push origin dev consequential +# 0.7245 cd ...; git fetch; git add -A consequential +# 0.7193 git pull --rebase origin dev consequential +# 0.6850 docker compose up -d consequential +# ---------------------------------------- 0.68 +# 0.6735 wc -l src/*.py && date innocuous +# 0.6544 grep -rn useState src/ innocuous +# 0.6099 sed -n '120,160p' package.json innocuous +# 0.6056 ls -la && cat README.md innocuous +# +# At 0.72 three of the four consequential acts retrieved NOTHING, including +# `git pull --rebase origin dev`, where rules 153, 1 and 2 all ranked +# correctly and all sat between 0.7126 and 0.7193. +# +# THE SEPARATION IS 0.0115 WIDE, and that is a caveat, not a result. Eight +# probes set a direction; they do not settle a number. `near_miss_samples` on +# a few days of post-#3855 traffic is what settles it, and this is the bar to +# re-read first. +# +# This also CORRECTS an assumption stated above. That comment argued 0.68 was +# "below where this corpus's noise sits", inferring a higher floor from the +# corpus being homogeneous. Measured, the command arm's noise ceiling is +# 0.6735 — so 0.68 clears it, barely, rather than sitting under it. The +# inference was reasonable and the measurement disagrees. +# +# WHY LOWERING IS SAFER NOW THAN IT WOULD HAVE BEEN. Until #3851 this arm had +# a single slot, so its one line had to be right and a high bar was the only +# control. The band now does noise control downstream: a marginal hit that +# clears the bar still has to score within `_RULEHINT_BAND` of the top to be +# rendered. The bar's job shrank, so the bar can. +# +# The noise floor above is set by CROSS-PROJECT BLEED rather than bad ranking +# — 0.6735 is another project's shell-command rule matching a shell command in +# this one, which is a correct match to a rule that should never have been +# eligible. Retrieval is ownership-scoped, not project-scoped. Scoping it +# would drop that ceiling and widen the 0.0115, which is the larger fix and +# the reason to settle project scoping before tuning this number twice. +TOOLRULE_THRESHOLD_KEY = "kb_toolrule_threshold" +TOOLRULE_DEFAULT_THRESHOLD = 0.68 + # A SET OF RULES PER ACT, NOT THE SINGLE BEST ONE (#3851). # # This was 1, and the reasoning for that is kept below rather than deleted @@ -1169,12 +1235,24 @@ async def get_writepath_config(user_id: int) -> dict: rule_threshold = RULEHINT_DEFAULT_THRESHOLD rule_threshold = min(1.0, max(0.0, rule_threshold)) + try: + tool_rule_threshold = float(await get_setting( + user_id, TOOLRULE_THRESHOLD_KEY, str(TOOLRULE_DEFAULT_THRESHOLD))) + except (TypeError, ValueError): + tool_rule_threshold = TOOLRULE_DEFAULT_THRESHOLD + tool_rule_threshold = min(1.0, max(0.0, tool_rule_threshold)) + return { **cfg, "enabled": enabled_raw.strip().lower() in ("true", "1", "yes", "on"), "threshold": threshold, # Its own bar, for a third corpus — see RULEHINT_DEFAULT_THRESHOLD. "rule_threshold": rule_threshold, + # And the COMMAND arm's own bar again, for the same reason one level + # down: a shell command is a different query shape from a code payload + # and scores lower for the same relevance (#3853). Separate keys, so an + # install can move one without the other — which is the whole finding. + "tool_rule_threshold": tool_rule_threshold, } def _rule_band(hits: list) -> list: @@ -1945,7 +2023,7 @@ async def build_tool_rule_hint( _rep_ptr: dict = {} hits = await semantic_search_rules( user_id, query, limit=RULEHINT_LIMIT, - threshold=cfg["rule_threshold"], + threshold=cfg["tool_rule_threshold"], report=_rep_ptr, ) duration_ms = (time.perf_counter() - t0) * 1000.0 @@ -1966,7 +2044,7 @@ async def build_tool_rule_hint( # failure the arm was built to stop. record_retrieval( user_id=user_id, source="pre_tool_rule", query=query, - threshold=cfg["rule_threshold"], limit=RULEHINT_LIMIT, + threshold=cfg["tool_rule_threshold"], limit=RULEHINT_LIMIT, project_id=project_id, is_task=None, results=fresh, duration_ms=duration_ms, best_available=_rep_ptr.get("best_available_score"), diff --git a/tests/test_rule_usage_wiring.py b/tests/test_rule_usage_wiring.py index 981c480..c8b06b4 100644 --- a/tests/test_rule_usage_wiring.py +++ b/tests/test_rule_usage_wiring.py @@ -62,6 +62,12 @@ def _arm_patches(pc, hits, recorder, prior_art=None, cfg=None, rule_search=None, AsyncMock(return_value=cfg or { "enabled": True, "threshold": 0.6, "top_k": 3, "rule_threshold": 0.6, + # The command arm reads its OWN bar since #3853, and + # a stub missing this key does not fail where a + # reader would see it: the arm fails open, so the + # KeyError becomes an empty hint and every case in + # _ARMS reports the arm went silent instead. + "tool_rule_threshold": 0.6, })), patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), patch.object(pc, "semantic_search_notes", @@ -428,6 +434,12 @@ def _tool_patches(pc, hits, recorder, cfg=None, retrieval_log=None): AsyncMock(return_value=cfg or { "enabled": True, "threshold": 0.6, "top_k": 3, "rule_threshold": 0.6, + # The command arm reads its OWN bar since #3853, and + # a stub missing this key does not fail where a + # reader would see it: the arm fails open, so the + # KeyError becomes an empty hint and every case in + # _ARMS reports the arm went silent instead. + "tool_rule_threshold": 0.6, })), patch.object(pc, "semantic_search_rules", AsyncMock(return_value=hits)), patch.object(pc, "record_retrieval", retrieval_log or MagicMock()), @@ -1581,3 +1593,70 @@ async def test_a_preference_on_the_ledger_keeps_the_slot_and_is_not_recounted(): "a repeat was written back to the hook's ledger, which would keep " "pushing its stamp forward so it never aged out (#3751)" ) + + +# ── each act arm uses its OWN bar, end to end (#3853) ─────────────────── +# +# The two act arms shared one threshold until the telemetry showed them +# behaving like different subsystems at the same number: write_path_rule +# speaking on 37% of 2,325 calls, pre_tool_rule on 2% of 11,768, because a +# code payload is long and rich where a shell command is short and carries +# less signal for the same relevance. +# +# Splitting the bar creates a failure the old single-bar code could not have: +# an arm can now search at one threshold and REPORT another. That row is what +# near-miss analysis is read against, so a mismatch does not look like a bug — +# it looks like a corpus whose scores sit somewhere they do not, and it would +# be acted on by moving the very bar it is misreporting. + +@pytest.mark.asyncio +async def test_each_act_arm_searches_at_its_own_bar(): + """The split, where it actually takes effect.""" + from scribe.services import plugin_context as pc + + cfg = {"enabled": True, "threshold": 0.6, "top_k": 3, + "rule_threshold": 0.77, "tool_rule_threshold": 0.61} + + search = AsyncMock(return_value=list(_THREE_HITS)) + with ExitStack() as stack: + stack.enter_context(patch.object( + pc, "get_writepath_config", AsyncMock(return_value=cfg))) + stack.enter_context(patch.object(pc, "semantic_search_rules", search)) + stack.enter_context(patch.object(pc, "record_retrieval", MagicMock())) + stack.enter_context(patch.object(pc, "record_rule_surfaced", MagicMock())) + await pc.build_tool_rule_hint(1, "Bash", "git push origin dev") + + assert search.await_args.kwargs["threshold"] == 0.61, ( + "the command arm searched at the write-path arm's bar; the two were " + "split at #3853 precisely because one number cannot serve both" + ) + + +@pytest.mark.asyncio +async def test_an_act_arm_reports_the_bar_it_actually_searched_at(): + """Search and log must agree, or the telemetry lies about the refusal. + + `retrieval_logs.threshold` is what `near_miss_samples` is read against. + An arm searching at 0.61 and logging 0.72 reports every hit between them + as having cleared a bar it never faced — and the reader's conclusion would + be to move the bar that was already right. + """ + from scribe.services import plugin_context as pc + + cfg = {"enabled": True, "threshold": 0.6, "top_k": 3, + "rule_threshold": 0.77, "tool_rule_threshold": 0.61} + + search = AsyncMock(return_value=list(_THREE_HITS)) + log = MagicMock() + with ExitStack() as stack: + stack.enter_context(patch.object( + pc, "get_writepath_config", AsyncMock(return_value=cfg))) + stack.enter_context(patch.object(pc, "semantic_search_rules", search)) + stack.enter_context(patch.object(pc, "record_retrieval", log)) + stack.enter_context(patch.object(pc, "record_rule_surfaced", MagicMock())) + await pc.build_tool_rule_hint(1, "Bash", "git push origin dev") + + rows = [c for c in log.call_args_list + if c.kwargs.get("source") == "pre_tool_rule"] + assert len(rows) == 1 + assert rows[0].kwargs["threshold"] == search.await_args.kwargs["threshold"] diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 264980e..d6dd323 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -481,6 +481,63 @@ async def test_the_two_write_path_bars_are_independent(): assert cfg["rule_threshold"] == 0.61 +@pytest.mark.asyncio +async def test_the_two_act_arms_read_independent_rule_bars(): + """#3853's split: the command arm's bar moves without the write path's. + + The two act arms shared one key until the telemetry showed them behaving + like different subsystems at the same number — the write-path arm speaking + on 37% of calls against the command arm's 2%, because a code payload is + long and rich where a shell command is short. A config assembler that + reads one key into both fields would silently undo that, and the symptom + would be invisible: both arms would simply agree again. + """ + from scribe.services import plugin_context as pc + + stored = {pc.RULEHINT_THRESHOLD_KEY: "0.75", pc.TOOLRULE_THRESHOLD_KEY: "0.61"} + with patch.object(pc, "get_setting", + AsyncMock(side_effect=lambda uid, k, d: stored.get(k, d))): + cfg = await pc.get_writepath_config(1) + + assert cfg["rule_threshold"] == 0.75 + assert cfg["tool_rule_threshold"] == 0.61 + + +@pytest.mark.asyncio +async def test_a_garbage_command_bar_falls_back_to_its_own_default(): + """Not to 0.0, and not to the write path's default. + + Falling back to 0.0 would attach a rule to every Bash call in the session; + falling back to the sibling's default would quietly re-merge the two bars + that #3853 separated, which is the harder failure to see because the arm + keeps working. + """ + from scribe.services import plugin_context as pc + + stored = {pc.TOOLRULE_THRESHOLD_KEY: "banana"} + with patch.object(pc, "get_setting", + AsyncMock(side_effect=lambda uid, k, d: stored.get(k, d))): + cfg = await pc.get_writepath_config(1) + + assert cfg["tool_rule_threshold"] == pc.TOOLRULE_DEFAULT_THRESHOLD + + +def test_the_command_bar_defaults_below_the_write_path_bar(): + """A DIRECTION check, like its sibling above, and for the same rule-115 + reason: the value is measured against one corpus, the relationship is not. + + A shell command carries less text than a code payload and therefore scores + lower for the same relevance — measured at #3853, where three of four + consequential commands retrieved nothing at the shared bar while the + write-path arm was healthy at it. Tuning either value stays free; inverting + the relationship would reinstate the mute arm that spoke on 2% of 11,768 + calls. + """ + from scribe.services import plugin_context as pc + + assert pc.TOOLRULE_DEFAULT_THRESHOLD < pc.RULEHINT_DEFAULT_THRESHOLD + + def test_the_rule_bar_defaults_above_the_code_bar(): """Not a number check — a DIRECTION check, and the only part of the default that is defensible without one instance's histogram (rule 115). -- 2.54.0 From c149ef31a312f0358a5585d7af7b7b212e1c5239 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 15:15:33 -0400 Subject: [PATCH 06/13] =?UTF-8?q?wip(394):=20steps=206+7=20=E2=80=94=20bac?= =?UTF-8?q?kend=20path=20and=20instruction=20surfaces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/api-keys-and-mcp.md | 2 +- docs/api-reference.md | 5 +- docs/features.md | 8 +- plugin/.claude-plugin/plugin.json | 2 +- plugin/README.md | 2 +- plugin/hooks/scribe_after_write.sh | 13 +- plugin/hooks/scribe_session_context.sh | 12 +- plugin/hooks/scribe_static_context.md | 24 +- plugin/hooks/scribe_tool_rules.sh | 2 +- plugin/skills/using-scribe/SKILL.md | 65 +++-- src/scribe/mcp/server.py | 34 +-- src/scribe/mcp/tools/projects.py | 20 +- src/scribe/mcp/tools/rulebooks.py | 166 ++---------- src/scribe/mcp/tools/search.py | 3 +- src/scribe/models/project.py | 2 +- src/scribe/models/rule_version.py | 2 - src/scribe/models/rulebook.py | 24 +- src/scribe/routes/plugin.py | 7 - src/scribe/routes/projects.py | 2 +- src/scribe/routes/rulebooks.py | 35 +-- src/scribe/services/backup.py | 16 +- src/scribe/services/embeddings.py | 20 +- src/scribe/services/inception.py | 77 +++--- src/scribe/services/plugin_context.py | 147 +++------- src/scribe/services/retrieval_telemetry.py | 2 +- src/scribe/services/rule_usage.py | 6 +- src/scribe/services/rule_versions.py | 2 +- src/scribe/services/rulebooks.py | 298 +++------------------ 28 files changed, 260 insertions(+), 738 deletions(-) diff --git a/docs/api-keys-and-mcp.md b/docs/api-keys-and-mcp.md index d950d4b..4157fe0 100644 --- a/docs/api-keys-and-mcp.md +++ b/docs/api-keys-and-mcp.md @@ -89,7 +89,7 @@ table here. The tools are grouped by family: | Projects / Milestones | `enter_project`, `get_project`, `create_milestone`, … | Containers and outcomes | | Search / Recall | `search`, `get_recent`, `list_tags`, `retrieval_telemetry` | Semantic + structured recall, and the readout its thresholds are tuned from | | Systems | `create_system`, `list_systems`, `list_system_records` | Reusable per-project subsystems/areas | -| Rulebooks | `list_always_on_rules`, `list_rules`, `create_rule`, `create_project_rule`, `subscribe_project_to_rulebook`, … | Engineering/workflow rules | +| Rulebooks | `list_rules`, `create_rule`, `create_project_rule`, `subscribe_project_to_rulebook`, … | Engineering/workflow rules | | Processes | `list_processes`, `get_process`, `create_process` | Saved prompts/workflows | | Trash | `list_trash`, `restore`, `purge_trash` | Recoverable deletes | | Admin | `get_app_logs` (write/admin key) | Diagnostics | diff --git a/docs/api-reference.md b/docs/api-reference.md index fbf7c0a..1679231 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -77,7 +77,7 @@ endpoint at `/mcp`, not these REST routes. |--------|------|-------------| | GET / POST | `/api/projects` | List (owned + shared) / create | | GET / PATCH / DELETE | `/api/projects/:id` | Read (with `milestone_summary`, `inception`) / update / delete | -| POST | `/api/projects/:id/inception` | Record what the project inherits `{choices: {exclude_always_on_rulebooks, subscribe_rulebooks, design_system_id, seed_systems}}` (owner-only; `POST /api/projects` accepts the same under `inception`) | +| POST | `/api/projects/:id/inception` | Record what the project inherits `{choices: {subscribe_rulebooks, design_system_id, seed_systems}}` (owner-only; `POST /api/projects` accepts the same under `inception`) | | GET | `/api/projects/:id/inception/defaults` | What binds if nobody decides — the inception card's payload | | GET | `/api/projects/:id/notes` | Notes + tasks in this project | | GET / POST | `/api/projects/:id/milestones` | List / create milestones | @@ -120,7 +120,6 @@ endpoint at `/mcp`, not these REST routes. | POST | `/api/projects/:id/rules` | Create a project-scoped rule | | POST / DELETE | `/api/projects/:id/suppressions/rules/:rid` | Suppress / unsuppress a rule | | POST / DELETE | `/api/projects/:id/suppressions/topics/:tid` | Suppress / unsuppress a topic | -| POST / DELETE | `/api/projects/:id/exclusions/rulebooks/:rid` | Exclude / include an always-on rulebook for this project (inception) | ## Sharing @@ -206,6 +205,6 @@ endpoint at `/mcp`, not these REST routes. Claude clients connect to the built-in MCP server at `POST /mcp` (streamable HTTP, Bearer auth with an `fmcp_` key), served by `src/scribe/mcp/`. It is not a REST surface — it exposes the same data as typed tools (`create_note`, `create_task`, -`start_planning`, `search`, `enter_project`, `list_always_on_rules`, …) with +`start_planning`, `search`, `enter_project`, …) with server-level usage guidance delivered in the MCP `instructions` block. See [API Keys & MCP](api-keys-and-mcp.md). diff --git a/docs/features.md b/docs/features.md index 694223a..d45fcfa 100644 --- a/docs/features.md +++ b/docs/features.md @@ -60,8 +60,10 @@ Scribe stores the operator's engineering and workflow **rules** so Claude follow across sessions. - **Rulebooks → topics → rules** — Rules are grouped by topic inside a rulebook. -- **Always-on rules** — A rulebook can be flagged always-on; its rules load at the - start of every session through the plugin's push channel. +- **Rules arrive by retrieval** — Nothing is preloaded. A rule reaches a + session when what the agent is about to do matches its trigger: a command, + a file being written, or the operator's own message. `when_to_apply` is + therefore the field that decides whether a rule is ever seen. - **Per-project scope** — A project subscribes to rulebooks, and can add project-scoped rules or suppress individual inherited rules/topics. @@ -94,7 +96,7 @@ The whole store is reachable by Claude through a built-in **MCP endpoint at `/mc (Bearer-auth with an API key). The **Scribe Claude Code plugin** (shipped in this repo) wires it up: -- a `SessionStart` hook that injects the operator's always-on rules + active-project +- a `SessionStart` hook that injects active-project context so Scribe surfaces without being asked (fail-open if Scribe is unreachable); - universal process-skills — writing-plans, systematic-debugging, verification, brainstorming — that route their output into Scribe; diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index a66849f..20301e4 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "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 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.11.1154", "author": { "name": "Bryan Van Deusen" diff --git a/plugin/README.md b/plugin/README.md index f7e9a1a..68830e6 100644 --- a/plugin/README.md +++ b/plugin/README.md @@ -5,7 +5,7 @@ instance into a first-class Claude Code extension: - **MCP tools** over your notes, tasks, projects, milestones, systems, and rulebook (the `scribe` server). -- **Session-start push channel** — a `SessionStart` hook injects your always-on +- **Session-start push channel** — a `SessionStart` hook injects your rules + active-project context so Scribe surfaces *without being asked*. - **Prior-art recall on writes** — a `PreToolUse` hook on Write/Edit checks the file about to be written against your recorded snippets (what's kept at that diff --git a/plugin/hooks/scribe_after_write.sh b/plugin/hooks/scribe_after_write.sh index 10857d2..b474098 100644 --- a/plugin/hooks/scribe_after_write.sh +++ b/plugin/hooks/scribe_after_write.sh @@ -175,16 +175,11 @@ while IFS= read -r rel_path; do derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen="" [ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}" fi - # The rules marker the SessionStart hook stored, handed back so the server - # can say whether those rules moved since (milestone 323). Nothing stored - # means nothing sent, which the server reads as silence rather than as a - # mismatch — an install that never reached /api/plugin/context must not - # start claiming its rules changed. + # The rules marker is gone with the resident set it aged (milestone 394). + # A session no longer holds a fixed set of rules from turn zero, so there + # is nothing that can have drifted since it loaded them — each rule is + # retrieved at the moment it applies. etag_q="" - if [ -f "$state_dir/${safe_sid}.rules_etag" ]; then - held=$(jq -sRr '@uri' < "$state_dir/${safe_sid}.rules_etag" 2>/dev/null) || held="" - [ -n "$held" ] && etag_q="&rules_etag=${held}" - fi if [ -n "$path_enc" ]; then # 8s, not the pre-write hook's 5: this hook runs AFTER the tool, so it # gates nothing the session is waiting on, and the first prior-art call diff --git a/plugin/hooks/scribe_session_context.sh b/plugin/hooks/scribe_session_context.sh index 1274c21..a2617c9 100755 --- a/plugin/hooks/scribe_session_context.sh +++ b/plugin/hooks/scribe_session_context.sh @@ -8,7 +8,7 @@ # does not depend on the key or the network. # # Tier 2 (DYNAMIC, best-effort enrichment): curls the operator's Scribe instance -# for always-on rules + active-project context and appends it. Config comes from +# for active-project context and appends it. Config comes from # the plugin's userConfig, exported to hooks as: # CLAUDE_PLUGIN_OPTION_API_ENDPOINT base URL, no trailing slash # CLAUDE_PLUGIN_OPTION_API_TOKEN fmcp_ API key (sensitive) @@ -158,7 +158,7 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then [ -n "$body" ] && dyn=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) # Stash the rules marker for the write-path hook (milestone 323). THIS is # where it has to be captured: the model receives one from - # list_always_on_rules too, but a hook cannot see an MCP tool's result. Stored + # an MCP tool too, but a hook cannot see a tool's result. Stored # under the same state dir the prior-art hook already uses, keyed by session, # so "changed since" means since THIS session loaded its rules. # @@ -178,9 +178,9 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then && printf '%s' "$etag" > "$etag_dir/${safe_sid}.rules_etag" 2>/dev/null || true fi fi - [ -z "$dyn" ] && status="> ⚠️ Scribe: live rules/project context could not be loaded this session (instance unreachable or request failed). The standing guidance above still applies — pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\` as needed." + [ -z "$dyn" ] && status="> ⚠️ Scribe: live project context could not be loaded this session (instance unreachable or request failed). The standing guidance above still applies — ask for rules with \`search(content_type=\"rule\")\` and project context with \`enter_project()\` as needed." elif [ -n "$url" ] && [ -z "$token" ]; then - status="> ⚠️ Scribe: live context disabled this session — the API key is not configured (Scribe base URL is). Set it with \`/plugin\` → Scribe → configure, or export SCRIBE_TOKEN. Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`." + status="> ⚠️ Scribe: live context disabled this session — the API key is not configured (Scribe base URL is). Set it with \`/plugin\` → Scribe → configure, or export SCRIBE_TOKEN. Tools still work; ask for rules with \`search(content_type=\"rule\")\` and project context with \`enter_project()\`." elif [ -z "$url" ] && [ -z "$token" ]; then # NEITHER value arrived. Previously this case stayed silent as "an unconfigured # install", which made issue #2198 invisible for weeks: a *casing* bug here @@ -189,7 +189,7 @@ elif [ -z "$url" ] && [ -z "$token" ]; then # silently disabled auto-inject and the write-path trigger too. It is not a # benign state — the plugin prompts for both values at enable time, so if # neither reached the hook, something is wrong. Say so. - status="> ⚠️ Scribe: live context disabled this session — neither the Scribe base URL nor the API key reached this hook. Configure the plugin (\`/plugin\` → Scribe), or export SCRIBE_URL + SCRIBE_TOKEN. Note this also disables prompt auto-inject and the write-path prior-art trigger. Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`." + status="> ⚠️ Scribe: live context disabled this session — neither the Scribe base URL nor the API key reached this hook. Configure the plugin (\`/plugin\` → Scribe), or export SCRIBE_URL + SCRIBE_TOKEN. Note this also disables prompt auto-inject and the write-path prior-art trigger. Tools still work; ask for rules with \`search(content_type=\"rule\")\` and project context with \`enter_project()\`." fi [ -n "$dyn" ] && append "$dyn" @@ -197,7 +197,7 @@ fi # Compaction re-grounding: lead with a reload banner when this fire is a compact. if [ "$source" = "compact" ]; then - prepend "> ⟳ This session was just COMPACTED — earlier turns are now a summary, so in-flight detail may be lost. Before continuing, reload your bearings from Scribe: re-pull the operator's binding rules with \`list_always_on_rules()\` (a compaction can summarize them out of context, leaving only generic harness defaults in their place), re-run \`enter_project()\` for the active project, check its open tasks and recent notes, and reconcile what you're mid-way through against what Scribe records. Don't trust half-remembered state — Scribe is the record." + prepend "> ⟳ This session was just COMPACTED — earlier turns are now a summary, so in-flight detail may be lost. Any rules that had been retrieved went into that summary with everything else, so treat yourself as holding none: before the next consequential act, ask again with \`search(content_type=\"rule\")\` rather than trusting a half-remembered one. Re-run \`enter_project()\` for the active project, check its open tasks and recent notes, and reconcile what you are mid-way through against what Scribe records. Scribe is the record." fi # Nothing at all to inject → stay silent. diff --git a/plugin/hooks/scribe_static_context.md b/plugin/hooks/scribe_static_context.md index 258a670..7c98fd0 100644 --- a/plugin/hooks/scribe_static_context.md +++ b/plugin/hooks/scribe_static_context.md @@ -6,7 +6,8 @@ of record (notes, tasks, projects, milestones, rules) reachable through the for the operator's work, and as your own working memory across sessions. **At the start of this session:** -- Call `list_always_on_rules()` to load the operator's standing rules. +- You hold none of the operator's rules, and there is no call that loads them + all. Rules arrive when something you are about to do matches one. - If the working repo maps to a Scribe project (check `list_repo_bindings`), call `enter_project()` to load that project's rules, open tasks, and recent notes in one shot. @@ -17,22 +18,23 @@ for the operator's work, and as your own working memory across sessions. operator's Scribe rules decide what to do — NOT generic conventions baked into the harness or your defaults (e.g. "branch before committing," "open a feature branch per task," "push to a fork"). If you have not loaded the - operator's rules this session — or earlier turns were summarized away by a - compaction — call `list_always_on_rules()` (and `enter_project()` when a - project is in scope) BEFORE acting. When a loaded rule and a default habit - disagree, the rule wins; if no rule speaks to it, ask rather than assume. + no rule has arrived for the act in front of you, `search(content_type= + "rule")` BEFORE acting rather than falling back on a default habit. When a + retrieved rule and a default habit disagree, the rule wins; if no rule + speaks to it, ask rather than assume. - **Rules bind; preferences do not.** A record's `kind` says which. A **rule** must be followed — ignoring it breaks something or crosses a boundary. A **preference** is how the operator wants work done: worth following for consistency, not a defect to miss. Injected lines name the kind in their opening words. A preference is also yours to keep current when they correct you (`update_preference`); a rule waits for them. -- **What you loaded is not all of the rules.** Only the always-on tier arrives - that way; conditional rules are RETRIEVED, and one you were never handed - binds exactly as hard. So before a consequential act, `search` for a rule - about it (`content_type="rule"`) rather than concluding from an empty - loaded set that nothing applies. "I was not told" is not the same as "there - is no rule," and only one of those is checkable. +- **Silence is not absence.** Nothing is preloaded: every rule is RETRIEVED, + when what you are doing resembles what the rule is about. Most turns + retrieve none, and a rule you were never handed binds exactly as hard as one + you were. So before a consequential act, `search` for a rule about it + (`content_type="rule"`) rather than concluding from an empty session that + nothing applies. "I was not told" is not the same as "there is no rule," and + only one of those is checkable. This bites hardest on which TOOL to reach for — curling an API that has an MCP client, standing up a local stack, running a suite CI owns. Those feel like mechanics rather than decisions, so they raise no doubt and generate no diff --git a/plugin/hooks/scribe_tool_rules.sh b/plugin/hooks/scribe_tool_rules.sh index 2d68977..16a1bd4 100644 --- a/plugin/hooks/scribe_tool_rules.sh +++ b/plugin/hooks/scribe_tool_rules.sh @@ -5,7 +5,7 @@ # asks "what is recorded about the file being written". This one asks "does a # standing rule speak to the command about to be run" — the question nothing # could ask before, and the reason every rule about which tool to reach for had -# to live in the always-on preload instead. +# to live in the preload instead, back when there was one. # # WHY A HOOK AND NOT AN INSTRUCTION. A reflex generates no query (note #3089): # you reach for `curl` confidently, with no moment of doubt, so a surface that diff --git a/plugin/skills/using-scribe/SKILL.md b/plugin/skills/using-scribe/SKILL.md index 93d0611..7206511 100644 --- a/plugin/skills/using-scribe/SKILL.md +++ b/plugin/skills/using-scribe/SKILL.md @@ -1,6 +1,6 @@ --- name: using-scribe -description: Use at the START of every session, and before answering anything about the operator's work or starting any task — establishes the Scribe-first reflex. FIRST ACTION of a session: call list_always_on_rules() (and enter_project when a repo/project is in scope) to load the operator's binding rules. Then recall before acting, update over duplicate, plan in Scribe not in files. +description: Use at the START of every session, and before answering anything about the operator's work or starting any task — establishes the Scribe-first reflex. You hold none of the operator's rules: they arrive by retrieval when your work matches one, and search(content_type="rule") is how you ask before a consequential act. Call enter_project when a repo/project is in scope. Then recall before acting, update over duplicate, plan in Scribe not in files. --- # Using Scribe @@ -13,12 +13,19 @@ asked for. ## Do this first (every session) -**Pull the standing rules yourself — do not wait for them to be handed to you.** -At the start of a session, before substantive work, call -`list_always_on_rules()` to load the operator's always-on rules. If the working -repo maps to a Scribe project (you're in a known repo, or `list_repo_bindings` -shows a binding), call `enter_project(id)` instead/as-well — it returns the -project plus its applicable rules, open tasks, and recent notes in one shot. +**You are not holding the operator's rules, and no call loads them all.** +There is no standing set to pull. A rule reaches you when what you are about to +do matches it — a command, code you are writing, or what the operator just +asked for — and on most turns none will. That is the surface working. + +**So the reflex is to ASK, not to load.** Before a consequential act — anything +hard to reverse or outward-facing — `search(content_type="rule")` for the thing +you are about to do. An empty session is not evidence of an empty rulebook. + +If the working repo maps to a Scribe project (you're in a known repo, or +`list_repo_bindings` shows a binding), call `enter_project(id)` — it returns the +project plus the rules bound to the areas it works in, open tasks, and recent +notes in one shot. Do this actively. A SessionStart hook *may* also inject a rule index, but treat that as a bonus, not a precondition: it can be absent (e.g. when the instance is @@ -56,11 +63,12 @@ Two constraints on *how* that's achieved: re-deriving it or opening a duplicate. When a project is in scope, pass its `project_id` so results stay scoped. -2. **Standing rules are binding — and the ones you were handed are not all of - them.** Load the resident set via `list_always_on_rules()` at session start - (see "Do this first"). Pull a record's full statement with `get_rule(id)` - when it's about to bite. When a project is in scope, `enter_project(id)` - also returns its applicable rules. +2. **Rules are binding, and silence does not mean there are none.** Nothing + is preloaded, so "no rule arrived" means "nothing matched" — never "no rule + exists". Ask with `search(content_type="rule")` before a consequential act, + and pull a record's full statement with `get_rule(id)` when it is about to + bite. When a project is in scope, `enter_project(id)` also returns the rules + bound to its areas. **`kind` says how much force a record carries, and it is never something to infer.** A **rule** must be followed: ignoring it breaks something or @@ -222,13 +230,12 @@ bound — confine the session to it: ## Starting a project: decide what it inherits A project's inheritance is a **decision, not a default**. Before -`create_project`, ask the operator the four inception questions and pass the +`create_project`, ask the operator the three inception questions and pass the answers — never create a project bare by default: -- which **always-on rulebooks** it should NOT inherit (`list_rulebooks` shows - which are always_on; default: inherit them all) → - `exclude_always_on_rulebooks=[...]` -- which other rulebooks to **subscribe** → `subscribe_rulebooks=[...]` +- which rulebooks to **subscribe** (`list_rulebooks` shows them; default: none + — a rulebook binds a project only when it opts in) → + `subscribe_rulebooks=[...]` - which **design system** its UI is built from (`list_design_systems`; or none) → `design_system_id=` - whether to **seed the standard starter Systems** so records can be tagged @@ -246,19 +253,23 @@ inception is the moment they are decided together, and the record of why. When codifying a rule, pick its home by **who it should bind** — and keep shared homes general: -- **Always-on rulebook** (`create_rule` in an `always_on` rulebook) — universal - norms that bind *every* project. Cross-project standards only. -- **Subscribed rulebook** (`create_rule` + `subscribe_project_to_rulebook`) — a - reusable, *themed* module of general rules that binds only projects that opt - in (e.g. a review checklist → every service). Themed, but project-agnostic. +- **Rulebook** (`create_rule` + `subscribe_project_to_rulebook`) — a reusable, + *themed* module of general rules that binds the projects which opt in (e.g. a + review checklist → every service). Themed, but project-agnostic. - **Project rule** (`create_project_rule`) — anything specific to one project (its files, paths, quirks). -Both rulebook tiers are shared, so their rules stay general; they differ in -**reach** (all vs opt-in), not generality. Names one project's specifics → -project rule; a standard a category shares → subscribed rulebook; a universal -norm → always-on rulebook. Never put project-specific detail in a shared -rulebook — it leaks to every other project that gets it. +There used to be a third home — an `always_on` rulebook that bound every +project automatically. It is gone: subscription is the only reach a rulebook +has. Names one project's specifics → project rule; anything a category of +projects shares → rulebook. Never put project-specific detail in a rulebook — +it leaks to every other project that subscribes. + +**Whichever home it gets, a rule needs `when_to_apply`.** It is the only thing +that decides whether the rule is ever seen: nothing is preloaded, so a rule +with no trigger is not a quiet rule, it is an unreachable one. Write the moment +in the words a session actually produces — the command, the error, the +half-formed ask — not the category it belongs to. **First ask whether it's a rule at all.** A rule is prose you have to remember and apply; Scribe's other entities are structure a tool can resolve and check. diff --git a/src/scribe/mcp/server.py b/src/scribe/mcp/server.py index 0f9ee1b..9992884 100644 --- a/src/scribe/mcp/server.py +++ b/src/scribe/mcp/server.py @@ -38,8 +38,8 @@ from quart import Quart # them) was DECLINED a line, deliberately, by the operator — not overlooked. # The reasoning, so it is not re-litigated blind: this is a map, and its own # closing line says each tool's description carries the full contract. The -# sweep is a curation act, not a session-start reflex like enter_project or -# list_always_on_rules. Spending the last of the budget on it would leave the +# sweep is a curation act, not a session-start reflex like enter_project. +# Spending the last of the budget on it would leave the # map unable to grow for something more central later. # # The accepted cost: an agent that never opens create_note's docstring never @@ -59,15 +59,15 @@ from quart import Quart # - What it bought is not per-tool guidance and has nowhere else to live at # session-start altitude. Rules were retrievable only by RESIDENCY: the # always-on preload put them in front of the agent, and nothing told a -# session to go looking for one it had not been handed. The tier split is -# therefore load-bearing on ANY install (rule 115): a delivered rule costs -# tokens in every session forever, so a rulebook that only delivers cannot -# grow past what one session can hold, and every rule worth keeping has to -# become resident to bind at all. Retrieval is what lets it keep growing — -# and retrieval fires only if something asks, which nothing told a session -# to do. A tool-choice reflex asks least of all (#3476, #161). -# - This states the PULL for conditional rules, exactly as the surrounding -# line states it for always-on ones. Rule 119 makes these surfaces the +# session to go looking for one it had not been handed. That preload is +# gone (milestone 394), which makes this line LOAD-BEARING rather than +# supplementary: retrieval is now the only delivery, and retrieval fires +# only if something asks. A session that waits to be handed a rule is +# handed nothing. A tool-choice reflex asks least of all (#3476, #161). +# - It also has to carry what absence MEANS. "No rule arrived" is now the +# ordinary state rather than the exceptional one, and reading it as +# "there is no rule" is the #3720 defect at session scale. Rule 119 makes +# these surfaces the # specification, so the same sentence lands on all three session-start # surfaces, and test_instruction_surfaces_agree pins it. _INSTRUCTIONS = """ @@ -77,8 +77,8 @@ in local files (CLAUDE.md, auto-memory); Scribe holds the single copy. Hierarchy: Project -> Milestone -> Task/Note. The map, by purpose: - ORIENT: enter_project(id) at session start — rules, open tasks, recent - notes, Systems, design system. `inception` key: ask what the project - inherits, decide_project_inception (create_project takes the same). + notes, Systems, design system. `inception`: ask what the project + inherits, then decide_project_inception. - DO: create_task. Fixed a problem? kind="issue" (symptom -> root cause -> fix), never a work-log line on an unrelated task. Log with add_task_log; keep status honest — in_progress on start, done on finish. @@ -88,9 +88,9 @@ Hierarchy: Project -> Milestone -> Task/Note. The map, by purpose: active project_id to stay in scope. - WHERE work happens: Systems. Tag records with system_ids as you write; create_system when the area is unmodelled. -- HOW: rules bind; preferences guide. list_always_on_rules() at start; - before a consequential act, search(content_type="rule") — the resident - set is not all of them. +- HOW: rules bind; preferences guide. Nothing preloads — a rule arrives + when your work matches it. Before a consequential act, + search(content_type="rule"); silence means nothing matched, not none. - UI: the project's design system is binding — resolve_design_system / get_design_system_stylesheet before hand-writing a value. - REUSE: search snippets before writing a helper; record what you build with @@ -130,7 +130,7 @@ _READ_ONLY_TOOLS = frozenset({ "get_task", "get_milestone", "get_recent", "enter_project", "list_milestones", "list_notes", "list_projects", "list_rulebooks", "list_rules", "list_tags", "list_tasks", "list_topics", "list_trash", - "list_always_on_rules", "search", + "search", "get_system", "list_systems", "list_system_records", # The global area catalog and its mapping REPORT — propose writes nothing; # map_system_to_canonical is the separate, explicitly-called write. diff --git a/src/scribe/mcp/tools/projects.py b/src/scribe/mcp/tools/projects.py index 8f90d57..e1e8c98 100644 --- a/src/scribe/mcp/tools/projects.py +++ b/src/scribe/mcp/tools/projects.py @@ -256,17 +256,16 @@ async def get_project(project_id: int) -> dict: def _inception_choices( - exclude_always_on_rulebooks, subscribe_rulebooks, design_system_id, seed_systems, + subscribe_rulebooks, design_system_id, seed_systems, ) -> dict | None: """The tool args → an inception choices object, or None when no inception arg was given at all (a bare create stays undecided and enter_project asks). design_system_id: 0 = not stated, -1 = explicitly none, n = that system.""" - if (exclude_always_on_rulebooks is None and subscribe_rulebooks is None + if (subscribe_rulebooks is None and not design_system_id and seed_systems is None): return None return { - "exclude_always_on_rulebooks": list(exclude_always_on_rulebooks or []), "subscribe_rulebooks": list(subscribe_rulebooks or []), "design_system_id": None if design_system_id in (0, -1) else design_system_id, "seed_systems": bool(seed_systems), @@ -279,7 +278,6 @@ async def create_project( goal: str = "", status: str = "active", color: str = "", - exclude_always_on_rulebooks: list[int] | None = None, subscribe_rulebooks: list[int] | None = None, design_system_id: int = 0, seed_systems: bool | None = None, @@ -299,9 +297,10 @@ async def create_project( goal: The desired outcome or definition of done for the project. status: one of active (default), paused, completed, archived. color: Optional hex colour for the project card (e.g. "#6366f1"). - exclude_always_on_rulebooks: always-on rulebook ids this project does + subscribe_rulebooks: rulebook ids this project opts into. Since + milestone 394 subscription is the only way a rulebook binds a + project, so there is no automatic tier left to decline. Was NOT inherit ([] = inherit them all). list_rulebooks shows which are - always_on. subscribe_rulebooks: rulebook ids to subscribe (the non-always-on ones). design_system_id: the design system this project's UI is built from (list_design_systems); -1 = explicitly none; 0 = not stated. @@ -319,7 +318,7 @@ async def create_project( ) data = project.to_dict() choices = _inception_choices( - exclude_always_on_rulebooks, subscribe_rulebooks, design_system_id, seed_systems, + subscribe_rulebooks, design_system_id, seed_systems, ) if choices is not None: decided = await inception_svc.decide(uid, project.id, choices=choices, via="mcp") @@ -336,7 +335,6 @@ async def create_project( async def decide_project_inception( project_id: int, - exclude_always_on_rulebooks: list[int] | None = None, subscribe_rulebooks: list[int] | None = None, design_system_id: int = 0, seed_systems: bool | None = None, @@ -345,11 +343,11 @@ async def decide_project_inception( or re-decide later (milestone 297). Owner-only. Applies the effects through the ordinary tools' paths — - exclude_always_on_rulebook, subscribe_project_to_rulebook, + subscribe_project_to_rulebook, set_project_design_system, the standard Systems seed — and writes the decision on the project last, so get_project/enter_project can say why the project has the rules, design and Systems it has. Re-deciding is - additive for exclusions/subscriptions (use include_always_on_rulebook / + additive for subscriptions (use unsubscribe_project_from_rulebook to undo one), replaces the design system, and never re-seeds Systems a project already has. @@ -359,7 +357,7 @@ async def decide_project_inception( """ uid = current_user_id() choices = _inception_choices( - exclude_always_on_rulebooks, subscribe_rulebooks, design_system_id, seed_systems, + subscribe_rulebooks, design_system_id, seed_systems, ) or {} decided = await inception_svc.decide(uid, project_id, choices=choices, via="mcp") return {"project_id": project_id, **decided} diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index cb1352e..45d0e55 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -48,16 +48,13 @@ async def get_rulebook(rulebook_id: int) -> dict: async def create_rulebook(title: str, description: str = "") -> dict: """Create a new rulebook (a shared, reusable module of general rules). - Two ways a rulebook reaches projects, set by its always_on flag (toggle via - update_rulebook): - - always_on = true -> binds EVERY one of your projects automatically. - Use for universal cross-project norms that apply across every - project, not just one. - - always_on = false -> binds only projects that subscribe - (subscribe_project_to_rulebook). Use for a THEMED body of rules a - category of projects shares (e.g. a design system that visual apps - opt into). - Either way a rulebook is SHARED, so its rules must stay general — agnostic + A rulebook reaches a project ONE way: the project subscribes to it + (subscribe_project_to_rulebook). There was a second until milestone 394 — + an `always_on` flag that bound every project automatically — and it is + gone with the tier it belonged to. Opt-in is now the whole model, so a + rulebook binds what asked for it and nothing else. + + A rulebook is SHARED, so its rules must stay general — agnostic to any single project. Project-specific rules go in create_project_rule. Args: @@ -73,7 +70,6 @@ async def create_rulebook(title: str, description: str = "") -> dict: async def update_rulebook( rulebook_id: int, title: str = "", description: str = "", - always_on: bool | None = None, ) -> dict: """Update an existing rulebook. Only non-empty fields are changed. @@ -81,9 +77,6 @@ async def update_rulebook( rulebook_id: Rulebook to update. title: New title. Empty string leaves unchanged. description: New description. Empty string leaves unchanged. - always_on: When True, rules in this rulebook are loaded at session - start by list_always_on_rules regardless of project context. - Pass None to leave unchanged. """ uid = current_user_id() fields: dict = {} @@ -91,8 +84,6 @@ async def update_rulebook( fields["title"] = title if description: fields["description"] = description - if always_on is not None: - fields["always_on"] = always_on rb = await rulebooks_svc.update_rulebook(rulebook_id, uid, **fields) if rb is None: raise ValueError(f"rulebook {rulebook_id} not found") @@ -234,58 +225,6 @@ async def list_rules( return {"rules": [_rule_summary(r) for r in rows], "total": len(rows)} -async def list_always_on_rules(project_id: int = 0) -> dict: - """Return all rules from rulebooks flagged always_on for the current user. - - Call this at session start. Treat the returned rules as binding for the - session — they apply regardless of which project (if any) is in scope. - - Returns the ALWAYS-ON tier only (milestone 307). A `conditional` rule is - still binding when it applies; it just is not resident — it reaches a - session through enter_project (when the project works in an area the rule - is tagged to) or through search(content_type="rule"). Nothing here is a - behaviour change until rules are actually re-tiered: `tier` defaults to - always_on, so an existing rulebook returns exactly what it always did. - Pair with get_project(id).applicable_rules when working on a specific - project to also load that project's subscription-derived rules. - - A rule carrying `last_verified` asserts a FACT about something outside the - operator's control — a runner's shell, a tool's existence, a setting - somewhere. It is still binding; the field says how long ago anyone - confirmed it, and "never" means nobody has. Follow the rule, and if you - are already standing where the check could be made, make it: get_rule - gives you its `verify_with`. Most rules have no such field, which means - they are decisions and there is nothing to check. - - Args: - project_id: 0 (default) = the user-wide set. Inside a project, pass - its id: an always-on rulebook the project EXCLUDED at inception - (see enter_project's `excluded_always_on`) is left out — the - project decided not to inherit it. - """ - uid = current_user_id() - rules = await rulebooks_svc.list_always_on_rules(uid, project_id=project_id) - # AMBIENT source: the resident set, handed over whole. No ranker chose - # these, so they must not land in the pull-through numerator's denominator - # — but they must land SOMEWHERE, or the largest rule surface in the - # product stays the one surface its own scoreboard cannot see (#3473). - record_rule_surfaced( - user_id=uid, - rule_ids=[r.id for r in rules], - source="list_always_on_rules", - ) - return { - "rules": [_rule_summary(r) for r in rules], - "total": len(rules), - # A marker for the set you are now holding. It is not for you to read: - # the write-path hook carries it back and is told if these rules have - # moved since. Deliberately NOT on rules_payload's applicable_rules — - # that is a DIFFERENT set (subscription-derived), and one key name - # over two sets is how a comparison starts reporting phantom changes. - "rules_etag": rulebooks_svc.rules_etag(rules), - } - - async def get_rule(rule_id: int) -> dict: """Fetch a rule by id — full statement + why + how_to_apply. @@ -309,7 +248,6 @@ async def get_rule(rule_id: int) -> dict: 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, - tier: str = "always_on", system_ids: list[int] | None = None, arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", force: bool = False, ) -> dict: @@ -357,7 +295,7 @@ async def create_rule( * "Approve it AS WRITTEN" — you create it with the statement exactly as shown. This is what makes element 1 load-bearing: they approved TEXT, so that text is what gets stored, verbatim. - * "LET'S TALK ABOUT IT" — the wording, the scope, the tier, whether it + * "LET'S TALK ABOUT IT" — the wording, the scope, whether it wants to be a rule at all. Most good rules arrive this way, so treat this answer as the expected one rather than a setback. * "NO" — let it go. If the observation is still worth keeping, it is a @@ -371,7 +309,7 @@ async def create_rule( into existence, which is the thing this whole loop exists to prevent. A rulebook rule is shared by every project that gets the rulebook: an - always_on rulebook binds ALL your projects; a subscribed rulebook binds the + A subscribed rulebook binds the projects that opt in. So a rulebook rule must read as a general standard — never pin it to one project's files, paths, or quirks. For a rule that applies to a single project only, use create_project_rule instead (no @@ -408,7 +346,7 @@ async def create_rule( instruction. State the moment or the material: "before any git push", "when adding a value to a CHECK-gated column", "when a release is being cut". Write it even though the parameter is - optional: it decides the tier below, it is how the rule is found + optional: it is how the rule is found when it matters, and a rule nobody can place is a rule nobody applies. This field is also the rule's RETRIEVAL SURFACE — it and the @@ -428,12 +366,6 @@ async def create_rule( category — it produces the command, the error, the half-formed ask — so a trigger written that way leaves the embedded document to be carried by the title alone. - tier: "always_on" (default) or "conditional". - The test: can you name the trigger WITHOUT naming a system, an - artifact type or a moment? If the honest answer is "whenever you - are working", it is always_on. If you had to name something, it is - conditional — and conditional costs nothing when it is irrelevant, - which is what lets it be as long as it needs to be. system_ids: Ids from list_canonical_systems — the global AREAS this rule is about. This is what lets a rule reach a project that is working in that area, so a CI rule surfaces on a CI change. @@ -472,7 +404,7 @@ async def create_rule( rule = await rulebooks_svc.create_rule( topic_id=topic_id, user_id=uid, title=title, statement=statement, when_to_apply=when_to_apply, - tier=tier, arose_from_id=arose_from_id, + arose_from_id=arose_from_id, why=why, how_to_apply=how_to_apply, order_index=order_index, verify_with=verify_with, expires_when=expires_when, ) @@ -482,7 +414,6 @@ async def create_rule( async def create_project_rule( project_id: int, statement: str, title: str = "", when_to_apply: str = "", why: str = "", how_to_apply: str = "", order_index: int = 0, - tier: str = "always_on", system_ids: list[int] | None = None, arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", force: bool = False, ) -> dict: @@ -531,25 +462,7 @@ async def create_project_rule( RETRIEVES: "the CI job passed locally and fails on the runner with a permission error" COLLAPSES: "when touching CI config" - See create_rule for the full argument. It informs the - tier below rather than deciding it, - since a project rule's tier turns on area-scope, not on whether - the trigger can be named. - tier: "always_on" (default) or "conditional". The SAME two values as - create_rule, judged against a different cost — do not import that - tool's test wholesale. There, always_on means every session in - every project, so the bar is high: the trigger must be nameless - ("whenever you are working"). Here the rule is already scoped to - one project by construction, so always_on costs only that - project's sessions and the bar is correspondingly lower. A - project rule that names something specific is still ordinarily - always_on — being specific is what project rules are FOR. - Reach for conditional when the rule is about one AREA of a large - project — a CI quirk, a migration gotcha, one subsystem's - convention — so it arrives with that area instead of resident in - every session. The failure to avoid is local: forty always-on - rules on one project reproduces, inside that project, exactly the - preload bloat that made every rule compete for the same budget. + See create_rule for the full argument. system_ids: Ids from list_canonical_systems — the global AREAS this rule is about. Worth setting even on a project rule: it is what lets a conditional one surface when the project is working in @@ -583,7 +496,7 @@ async def create_project_rule( rule = await rulebooks_svc.create_project_rule( project_id=project_id, user_id=uid, title=derived_title, statement=statement, when_to_apply=when_to_apply, - tier=tier, arose_from_id=arose_from_id, + arose_from_id=arose_from_id, why=why, how_to_apply=how_to_apply, order_index=order_index, verify_with=verify_with, expires_when=expires_when, ) @@ -593,7 +506,7 @@ async def create_project_rule( async def update_rule( rule_id: int, title: str = "", statement: str = "", when_to_apply: str = "", why: str = "", how_to_apply: str = "", order_index: int = -1, - tier: str = "", system_ids: list[int] | None = None, arose_from_id: int = 0, + system_ids: list[int] | None = None, arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", kind: str = "", clear_fields: list[str] | None = None, ) -> dict: @@ -606,9 +519,10 @@ async def update_rule( correct. Ordinary edits to an existing preference belong in update_preference, which asks for what taught the change. - Adding `when_to_apply` and a `tier` to an existing rule is the ordinary way - a rule stops being preloaded into every session and starts arriving when it - is relevant. `system_ids` REPLACES the rule's areas (pass [] to clear). + `when_to_apply` IS HOW A RULE ARRIVES AT ALL. Nothing is preloaded since + milestone 394, so a rule with no trigger is not a quiet rule — it is one + no session will ever be shown. `system_ids` REPLACES the rule's areas + (pass [] to clear), and they decide which PROJECTS a rule binds by area. RETROFITTING A TRIGGER HAS ITS OWN TRAP, and it is not the one create_rule warns about. There the field is empty and the instruction is "write one". @@ -662,8 +576,6 @@ async def update_rule( fields["statement"] = statement if when_to_apply: fields["when_to_apply"] = when_to_apply - if tier: - fields["tier"] = tier if kind: fields["kind"] = kind if arose_from_id: @@ -978,7 +890,7 @@ async def subscribe_project_to_rulebook( ) -> dict: """Subscribe a project to a rulebook — its rules then bind that project. - Subscription is the opt-in path for a non-always_on rulebook: a reusable, + Subscription is the ONLY path for a rulebook (milestone 394): a reusable, themed module of GENERAL rules shared across the projects that subscribe. Subscribe a project because it fits the rulebook's theme (e.g. a visual app -> the design-system rulebook), not to host rules about this one project — @@ -1004,34 +916,6 @@ async def unsubscribe_project_from_rulebook( # ── Suppressions — project-level mute of rulebook rules / topics ──────── -async def exclude_always_on_rulebook(project_id: int, rulebook_id: int) -> dict: - """Opt a project OUT of a whole always-on rulebook (milestone 297). - - Always-on rulebooks bind every project implicitly; an inception decision - can say "not this one, not here". The exclusion is total for that project - — list_always_on_rules(project_id), enter_project/get_project rules and - the session-start context all leave it out and name it under - `excluded_always_on`. Owner-only; the rulebook must be always_on (a - subscribed rulebook is left with unsubscribe_project_from_rulebook). - Idempotent; include_always_on_rulebook reverses it. Normally reached via - decide_project_inception, not by hand. - """ - uid = current_user_id() - await rulebooks_svc.exclude_always_on_rulebook_for_project( - project_id=project_id, rulebook_id=rulebook_id, user_id=uid, - ) - return {"project_id": project_id, "rulebook_id": rulebook_id, "excluded": True} - - -async def include_always_on_rulebook(project_id: int, rulebook_id: int) -> dict: - """Reverse exclude_always_on_rulebook: the always-on rulebook binds this - project again. Idempotent.""" - uid = current_user_id() - await rulebooks_svc.include_always_on_rulebook_for_project( - project_id=project_id, rulebook_id=rulebook_id, user_id=uid, - ) - return {"project_id": project_id, "rulebook_id": rulebook_id, "excluded": False} - async def suppress_rule_for_project( project_id: int, rule_id: int, @@ -1087,8 +971,6 @@ async def unsuppress_topic_for_project( return {"project_id": project_id, "topic_id": topic_id, "suppressed": False} - - async def relate_rules( from_rule_id: int, to_rule_id: int, kind: str, note: str = "", ) -> dict: @@ -1136,7 +1018,7 @@ async def unrelate_rules(relation_id: int) -> dict: # ── The staleness sweep (milestone 312) ──────────────────────────────── async def rules_due_for_verification( - older_than_days: int = 0, tier: str = "", never_only: bool = False, + older_than_days: int = 0, never_only: bool = False, ) -> dict: """Which standing rules assert a FACT that nobody has confirmed lately. @@ -1163,9 +1045,6 @@ async def rules_due_for_verification( Args: older_than_days: only rules last verified longer ago than this. Never-checked rules always qualify. 0 = no age filter. - tier: "always_on" or "conditional" to narrow. An always-on constraint - that has gone false is the expensive kind — it is preloaded into - every session, so a wrong one is wrong everywhere at once. never_only: only rules nobody has ever verified. NOT filterable by project, deliberately: a project reaches rules through @@ -1175,7 +1054,7 @@ async def rules_due_for_verification( """ uid = current_user_id() rules = await rulebooks_svc.rules_due_for_verification( - uid, older_than_days=older_than_days, tier=tier, never_only=never_only, + uid, older_than_days=older_than_days, never_only=never_only, ) return { "rules": [rulebooks_svc.verification_row(r) for r in rules], @@ -1228,14 +1107,13 @@ 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, list_always_on_rules, get_rule, + list_rules, get_rule, create_rule, create_project_rule, update_rule, delete_rule, create_preference, update_preference, relate_rules, unrelate_rules, subscribe_project_to_rulebook, unsubscribe_project_from_rulebook, suppress_rule_for_project, unsuppress_rule_for_project, suppress_topic_for_project, unsuppress_topic_for_project, - exclude_always_on_rulebook, include_always_on_rulebook, rules_due_for_verification, mark_rule_verified, rule_history, ): diff --git a/src/scribe/mcp/tools/search.py b/src/scribe/mcp/tools/search.py index 612f4ca..df89e19 100644 --- a/src/scribe/mcp/tools/search.py +++ b/src/scribe/mcp/tools/search.py @@ -40,7 +40,6 @@ async def _search_rules(uid: int, q: str, limit: int) -> dict: "title": rule.title, "statement": rule.statement, "when_to_apply": rule.when_to_apply or "", - "tier": rule.tier, "why": rule.why or "", "how_to_apply": rule.how_to_apply or "", "verify_with": rule.verify_with or "", @@ -282,7 +281,7 @@ It is an UPPER BOUND per surface: a pull records the door it came `surfaced` VS `ambient` IS THE READING THAT MATTERS HERE. `surfaced` counts rules a ranker chose — today only the write-path arm — and those are claims a pull can settle. `ambient` counts BULK DELIVERIES: the SessionStart - preload, `list_always_on_rules`, and the `rules_payload` surfaces + preload and the `rules_payload` surfaces (`enter_project`, `get_project`, `get_milestone`, `start_planning`, `get_task`), which hand over the whole applicable set at once with nobody choosing anything. A large `ambient` says the resident set is big and diff --git a/src/scribe/models/project.py b/src/scribe/models/project.py index f9e4869..54a04e3 100644 --- a/src/scribe/models/project.py +++ b/src/scribe/models/project.py @@ -39,7 +39,7 @@ class Project(Base, TimestampMixin, SoftDeleteMixin): ) # The inception record (milestone 297): what this project was decided to # inherit, when, and through which door — {decided_at, decided_by, via, - # choices: {exclude_always_on_rulebooks, subscribe_rulebooks, + # choices: {subscribe_rulebooks, # design_system_id, seed_systems}}. NULL means nobody has decided yet, # and enter_project asks; the effects themselves live in the subscription # / exclusion tables, design_system_id and the project's Systems — this is diff --git a/src/scribe/models/rule_version.py b/src/scribe/models/rule_version.py index 477a186..63543bd 100644 --- a/src/scribe/models/rule_version.py +++ b/src/scribe/models/rule_version.py @@ -58,7 +58,6 @@ class RuleVersion(Base, CreatedAtMixin): why: Mapped[str | None] = mapped_column(Text, nullable=True) how_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True) when_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True) - tier: Mapped[str | None] = mapped_column(Text, nullable=True) # Carried so that a change of FORCE leaves a trace. `record_if_changed` # snapshots only the fields a version holds, so a kind omitted here would # make "this stopped binding" the one edit with no history behind it. @@ -90,7 +89,6 @@ class RuleVersion(Base, CreatedAtMixin): "why": self.why or "", "how_to_apply": self.how_to_apply or "", "when_to_apply": self.when_to_apply or "", - "tier": self.tier or "", "kind": self.kind or "", "verify_with": self.verify_with or "", "expires_when": self.expires_when or "", diff --git a/src/scribe/models/rulebook.py b/src/scribe/models/rulebook.py index 74dfc8e..b11a804 100644 --- a/src/scribe/models/rulebook.py +++ b/src/scribe/models/rulebook.py @@ -19,9 +19,6 @@ class Rulebook(Base, TimestampMixin, SoftDeleteMixin): ) title: Mapped[str] = mapped_column(Text) description: Mapped[str | None] = mapped_column(Text, nullable=True) - always_on: Mapped[bool] = mapped_column( - Boolean, default=False, nullable=False, server_default="false" - ) def to_dict(self) -> dict: return { @@ -29,7 +26,6 @@ class Rulebook(Base, TimestampMixin, SoftDeleteMixin): "owner_user_id": self.owner_user_id, "title": self.title, "description": self.description or "", - "always_on": self.always_on, "created_at": iso(self.created_at), "updated_at": iso(self.updated_at), } @@ -96,16 +92,15 @@ class Rule(Base, TimestampMixin, SoftDeleteMixin): # WHEN this rule applies — the trigger, not the instruction. Required of # new rules at the service layer and nullable here, because rules written # before migration 0088 have none and a migration cannot invent one. - # It carries three jobs at once (note 3026): it is the tier test made - # concrete, the readable form of the canon tag, and the half of the - # document that makes a rule findable by meaning. + # It carries three jobs at once (note 3026): it is the readable form of + # the canon tag, the half of the document that makes a rule findable by + # meaning, and — since milestone 394 removed the always-on tier — the ONLY + # thing that decides whether a rule ever reaches a session at all. A rule + # with no trigger is not a quiet rule, it is an unreachable one. when_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True) - # always_on = preloaded into every session, as every rule is today. - # conditional = reachable, and surfaced when its trigger fires. The - # default preserves existing behaviour exactly: nothing stops binding - # because of an upgrade. CHECK ck_rules_tier (migration 0088, rule 36). - tier: Mapped[str] = mapped_column(Text, default="always_on", server_default="always_on") - # WHAT KIND of instruction this is — force, where `tier` is delivery. + # WHAT KIND of instruction this is. `tier` used to sit beside this and + # carry delivery; milestone 394 removed it, so kind is now the only axis + # on a rule and delivery belongs entirely to retrieval. # `rule` must be FOLLOWED: ignoring it breaks something or crosses a # boundary. `preference` is how this person wants work DONE: ignoring it # costs consistency, not correctness. @@ -161,7 +156,6 @@ class Rule(Base, TimestampMixin, SoftDeleteMixin): "title": self.title, "statement": self.statement, "when_to_apply": self.when_to_apply or "", - "tier": self.tier, # Unconditional, unlike the `if present` keys below. A reader # deciding how much force a record carries must never infer it # from an ABSENT key: "no kind field" and "kind is rule" would be @@ -266,7 +260,7 @@ project_rule_suppressions = Table( # sibling of the two suppression tables below, one level up. Always-on # rulebooks bind every project implicitly; an inception decision can exclude # specific ones for this project, and get_applicable_rules / -# list_always_on_rules(project_id) skip them. FKs CASCADE like the others. +# get_applicable_rules(project_id) skips them. FKs CASCADE like the others. project_rulebook_exclusions = Table( "project_rulebook_exclusions", Base.metadata, diff --git a/src/scribe/routes/plugin.py b/src/scribe/routes/plugin.py index 0467b76..a0b1d6d 100644 --- a/src/scribe/routes/plugin.py +++ b/src/scribe/routes/plugin.py @@ -202,11 +202,6 @@ async def write_path_prior_art(): or `canon:`) already named this session by the ledger arm (#2900); its own channel, like the two above. - rules_etag (opt) — the marker the session was given when it loaded - its always-on rules (milestone 323). Sent back - so the server can say whether those rules have - MOVED since. Absent means the hook has nothing - stored, which is silence, not a mismatch. shapes (opt) — comma-separated `kind:name` definitions the hook found in (or enclosing) the payload, kind being css|sym. The shape ledger's write-path feed @@ -226,7 +221,6 @@ async def write_path_prior_art(): p.strip() for p in (request.args.get("exclude_derive") or "").split(",") if p.strip() ] exclude_rule_ids = _int_list(request.args.get("exclude_rule_ids")) - rules_etag = (request.args.get("rules_etag") or "").strip() shapes = _parse_shapes(request.args.get("shapes") or "") api_key = getattr(g, "api_key", None) may_stamp = api_key is None or getattr(api_key, "scope", "") == "write" @@ -238,7 +232,6 @@ async def write_path_prior_art(): repo_key=repo_bindings_svc.normalize_repo_key(repo) if repo else "", exclude_derive=exclude_derive, exclude_rule_ids=exclude_rule_ids, - rules_etag=rules_etag, ) return jsonify(result) diff --git a/src/scribe/routes/projects.py b/src/scribe/routes/projects.py index fd9b954..b266987 100644 --- a/src/scribe/routes/projects.py +++ b/src/scribe/routes/projects.py @@ -99,7 +99,7 @@ async def create_project_route(): @login_required async def decide_inception_route(project_id: int): """Record (or re-record) what a project inherits — milestone 297. - Body: the choices object {exclude_always_on_rulebooks, subscribe_rulebooks, + Body: the choices object {subscribe_rulebooks, design_system_id, seed_systems}; owner-only.""" uid = get_current_user_id() data = await request.get_json() or {} diff --git a/src/scribe/routes/rulebooks.py b/src/scribe/routes/rulebooks.py index 7574f5d..d8b8e4f 100644 --- a/src/scribe/routes/rulebooks.py +++ b/src/scribe/routes/rulebooks.py @@ -55,7 +55,7 @@ async def get_rulebook(rulebook_id: int): @login_required async def update_rulebook(rulebook_id: int): data = await request.get_json() or {} - fields = {k: v for k, v in data.items() if k in ("title", "description", "always_on")} + fields = {k: v for k, v in data.items() if k in ("title", "description")} rb = await rulebooks_svc.update_rulebook(rulebook_id, get_current_user_id(), **fields) if rb is None: return jsonify({"error": "rulebook not found"}), 404 @@ -177,7 +177,6 @@ async def create_rule(topic_id: int): how_to_apply=data.get("how_to_apply", ""), order_index=data.get("order_index", 0), when_to_apply=data.get("when_to_apply", ""), - tier=data.get("tier", "always_on"), # The human door carries `kind` too, and without the MCP door's # required provenance: an operator editing their own preference # owes nobody an explanation. That requirement is about auditing @@ -217,7 +216,7 @@ async def update_rule(rule_id: int): fields = { k: v for k, v in data.items() if k in ("title", "statement", "why", "how_to_apply", "order_index", - "when_to_apply", "tier", "kind", "arose_from_id", + "when_to_apply", "kind", "arose_from_id", "verify_with", "expires_when") } # No clear_fields here: a form sends "" for an emptied input, and the @@ -395,32 +394,6 @@ async def unsuppress_project_topic(project_id: int, topic_id: int): return "", 204 -@rulebooks_bp.post("/projects//exclusions/rulebooks/") -@login_required -async def exclude_project_rulebook(project_id: int, rulebook_id: int): - """Opt the project out of a whole always-on rulebook (milestone 297).""" - try: - await rulebooks_svc.exclude_always_on_rulebook_for_project( - project_id=project_id, rulebook_id=rulebook_id, user_id=get_current_user_id(), - ) - except ValueError as exc: - msg = str(exc) - return jsonify({"error": msg}), (400 if "not always-on" in msg else 404) - return "", 204 - - -@rulebooks_bp.delete("/projects//exclusions/rulebooks/") -@login_required -async def include_project_rulebook(project_id: int, rulebook_id: int): - try: - await rulebooks_svc.include_always_on_rulebook_for_project( - project_id=project_id, rulebook_id=rulebook_id, user_id=get_current_user_id(), - ) - except ValueError as exc: - return jsonify({"error": str(exc)}), 404 - return "", 204 - - @rulebooks_bp.post("/projects//rules") @login_required async def create_project_rule(project_id: int): @@ -440,7 +413,6 @@ async def create_project_rule(project_id: int): how_to_apply=data.get("how_to_apply", ""), order_index=data.get("order_index", 0), when_to_apply=data.get("when_to_apply", ""), - tier=data.get("tier", "always_on"), # The human door carries `kind` too, and without the MCP door's # required provenance: an operator editing their own preference # owes nobody an explanation. That requirement is about auditing @@ -464,7 +436,7 @@ async def create_project_rule(project_id: int): async def rules_due_for_verification(): """Rules that carry a check, oldest verification first, never-checked top. - Query params: older_than_days, tier, never_only. A rule with no + Query params: older_than_days, never_only. A rule with no `verify_with` never appears — it is a decision, not a fact. """ uid = get_current_user_id() @@ -477,7 +449,6 @@ async def rules_due_for_verification(): rules = await rulebooks_svc.rules_due_for_verification( uid, older_than_days=older, - tier=args.get("tier", ""), never_only=args.get("never_only", "").lower() in ("1", "true", "yes"), ) except ValueError as exc: diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index 1a8f8d8..4ef7ea7 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -513,7 +513,7 @@ def _rule_version_rows(rows) -> list[dict]: "id": rv.id, "rule_id": rv.rule_id, "user_id": rv.user_id, "title": rv.title, "statement": rv.statement, "why": rv.why, "how_to_apply": rv.how_to_apply, "when_to_apply": rv.when_to_apply, - "tier": rv.tier, "kind": rv.kind, "verify_with": rv.verify_with, + "kind": rv.kind, "verify_with": rv.verify_with, "expires_when": rv.expires_when, "created_at": rv.created_at.isoformat(), } @@ -529,7 +529,7 @@ def _rulebook_rows(rows) -> list[dict]: return [ { "id": rb.id, "owner_user_id": rb.owner_user_id, "title": rb.title, - "description": rb.description, "always_on": rb.always_on, + "description": rb.description, "created_at": rb.created_at.isoformat(), "updated_at": rb.updated_at.isoformat(), } @@ -575,7 +575,7 @@ def _rule_rows(rows) -> list[dict]: "id": r.id, "topic_id": r.topic_id, "project_id": r.project_id, "title": r.title, "statement": r.statement, "why": r.why, "how_to_apply": r.how_to_apply, "order_index": r.order_index, - "when_to_apply": r.when_to_apply, "tier": r.tier, "kind": r.kind, + "when_to_apply": r.when_to_apply, "kind": r.kind, "verify_with": r.verify_with, "expires_when": r.expires_when, "verified_at": r.verified_at.isoformat() if r.verified_at else None, "arose_from_id": r.arose_from_id, @@ -1238,7 +1238,6 @@ async def _restore_v2(data: dict) -> dict: owner_user_id=mapped_uid, title=rb_data.get("title", ""), description=rb_data.get("description", ""), - always_on=rb_data.get("always_on", False), created_at=_dt(rb_data.get("created_at")), updated_at=_dt(rb_data.get("updated_at")), ) @@ -1279,10 +1278,14 @@ async def _restore_v2(data: dict) -> dict: why=r_data.get("why") or None, how_to_apply=r_data.get("how_to_apply") or None, when_to_apply=r_data.get("when_to_apply") or None, - # A file written before migration 0088 has no tier. always_on + # A file written before milestone 394 carries `tier` and + # `always_on`; neither is read. Dropping a field the schema + # no longer has is the tolerant direction — an archive + # records what WAS, and refusing it because it remembers a + # deleted column would make every pre-394 backup + # unrestorable. Previously: always_on # is the pre-0088 behaviour, so an old backup restores rules # that bind exactly as they did when it was taken. - tier=r_data.get("tier") or "always_on", # Same shape, same reason: a file written before 0098 has no # kind, and every rule in it was a rule. Defaulting the other # way would restore an old backup with things that had always @@ -1427,7 +1430,6 @@ async def _restore_v2(data: dict) -> dict: why=rv.get("why"), how_to_apply=rv.get("how_to_apply"), when_to_apply=rv.get("when_to_apply"), - tier=rv.get("tier"), # NOT defaulted, unlike the rule above. A version records what # was; absent means nobody wrote it down, and inventing "rule" # here would put an artifact where a measurement belongs. diff --git a/src/scribe/services/embeddings.py b/src/scribe/services/embeddings.py index 6aa5c32..b1f1c90 100644 --- a/src/scribe/services/embeddings.py +++ b/src/scribe/services/embeddings.py @@ -817,7 +817,6 @@ async def semantic_search_rules( query: str, limit: int = 5, threshold: float = _SIMILARITY_THRESHOLD, - tier: str | None = None, kind: str | None = None, report: dict | None = None, ) -> list[tuple[float, "Rule"]]: @@ -844,17 +843,13 @@ async def semantic_search_rules( is the surfacing question, and it has its own machinery (get_applicable_rules) rather than a second, subtly different copy here. - `tier` narrows to one tier, and NONE is the ordinary case. The write-path - and pre-tool hints deliberately pass nothing: an always-on rule is already - in the session, but being in a list from turn zero is not the same as being - in front of the reader when the action it governs is taken, and filtering - on tier made a whole class of rules permanently ineligible for the one - mechanism that surfaces a rule AT the moment. Relevance is the threshold's - job; see the block above RULEHINT_LIMIT in services/plugin_context.py for - the argument and for what the resulting scores are being read against. - - Pass a tier when a caller genuinely wants one class — a listing, an audit, - a UI that renders the tiers apart. Not to approximate relevance. + THERE IS NO TIER TO NARROW BY ANY MORE (milestone 394). This carried a + `tier` parameter, and the arms deliberately passed nothing: filtering on it + made a whole class of rules permanently ineligible for the one mechanism + that surfaces a rule AT the moment it applies. The tier is now gone + entirely, so every rule is eligible for every arm and relevance is the + threshold's job alone — see the block above RULEHINT_LIMIT in + services/plugin_context.py for what those scores are read against. `kind` narrows to `rule` or `preference`, and NONE is likewise the ordinary case: a caller asking "what governs this" wants both, because the reader @@ -905,7 +900,6 @@ async def semantic_search_rules( Rulebook.owner_user_id == user_id, Project.user_id == user_id, ), - *( [Rule.tier == tier] if tier else [] ), *( [Rule.kind == kind] if kind else [] ), ) # Overfetch so collapsing chunks to their best row still fills diff --git a/src/scribe/services/inception.py b/src/scribe/services/inception.py index 65d18ee..eec2b2b 100644 --- a/src/scribe/services/inception.py +++ b/src/scribe/services/inception.py @@ -7,7 +7,6 @@ A project's inheritance is a decision, not a default. The record lives on "decided_at": "", "decided_by": | null, "via": "mcp" | "ui" | "legacy", "choices": { - "exclude_always_on_rulebooks": [rulebook ids], "subscribe_rulebooks": [rulebook ids], "design_system_id": | null, "seed_systems": bool @@ -18,9 +17,14 @@ NULL = undecided → enter_project asks. ``legacy`` is the migration's stamp on projects that existed before the step did (inherit-all / no design system / no seed), so the ask fires only for projects created after this shipped. +``exclude_always_on_rulebooks`` was a fourth choice until milestone 394. It +let a project decline to inherit an always-on rulebook, and with no always-on +tier there is nothing to decline — a rulebook now reaches a project by +subscription, which is opt-IN, so declining is expressed by not subscribing. + The shape and its validator are pure; ``decide`` composes the existing -services — always-on exclusions, subscriptions, set_project_design_system, -the standard Systems seed — checks every target BEFORE touching anything, +services — subscriptions, set_project_design_system, the standard Systems +seed — checks every target BEFORE touching anything, applies the effects (each idempotent), and writes the record LAST, so a half-applied decision is re-runnable rather than recorded as done. ``current_defaults`` is what the enter_project ask shows: what binds today @@ -37,7 +41,7 @@ from scribe.models.project import Project from scribe.models.rulebook import Rulebook INCEPTION_VIAS = ("mcp", "ui", "legacy") -CHOICE_KEYS = ("exclude_always_on_rulebooks", "subscribe_rulebooks", "design_system_id", "seed_systems") +CHOICE_KEYS = ("subscribe_rulebooks", "design_system_id", "seed_systems") def _is_id_list(value) -> bool: @@ -60,15 +64,9 @@ def validate_inception(choices) -> str | None: unknown = sorted(set(choices) - set(CHOICE_KEYS)) if unknown: return f"unknown inception choice(s): {', '.join(unknown)} (one of: {', '.join(CHOICE_KEYS)})" - excl = choices.get("exclude_always_on_rulebooks") or [] subs = choices.get("subscribe_rulebooks") or [] - if not _is_id_list(excl): - return "exclude_always_on_rulebooks must be a list of rulebook ids" if not _is_id_list(subs): return "subscribe_rulebooks must be a list of rulebook ids" - both = sorted(set(excl) & set(subs)) - if both: - return f"rulebook(s) {both} cannot be both excluded and subscribed" ds = choices.get("design_system_id") if ds is not None and (isinstance(ds, bool) or not isinstance(ds, int) or ds <= 0): return "design_system_id must be a positive id or null" @@ -79,11 +77,10 @@ def validate_inception(choices) -> str | None: def normalize_choices(choices: dict | None) -> dict: - """The four keys, always present, in canonical form — what gets stored + """The three keys, always present, in canonical form — what gets stored and what the UI/agent reads back. Call after validate_inception.""" choices = choices or {} return { - "exclude_always_on_rulebooks": sorted(set(choices.get("exclude_always_on_rulebooks") or [])), "subscribe_rulebooks": sorted(set(choices.get("subscribe_rulebooks") or [])), "design_system_id": choices.get("design_system_id"), "seed_systems": bool(choices.get("seed_systems", False)), @@ -98,8 +95,7 @@ def is_decided(project) -> bool: async def current_defaults(user_id: int, project_id: int) -> dict: """What the project inherits if nobody decides — the ask's payload. - {always_on_rulebooks: [{id,title}], other_rulebooks: [{id,title}], - excluded_always_on: [...], subscribed_rulebooks: [...], + {rulebooks: [{id,title}], subscribed_rulebooks: [...], design_system_id, design_systems: [{id,title}], systems: }. Instance-agnostic: an install with no rulebooks / design systems shows empty lists, and the ask says so rather than inventing a default. @@ -115,7 +111,7 @@ async def current_defaults(user_id: int, project_id: int) -> dict: async with async_session() as session: rows = ( await session.execute( - select(Rulebook.id, Rulebook.title, Rulebook.always_on) + select(Rulebook.id, Rulebook.title) .where(Rulebook.owner_user_id == user_id, Rulebook.deleted_at.is_(None)) .order_by(Rulebook.title) ) @@ -124,9 +120,11 @@ async def current_defaults(user_id: int, project_id: int) -> dict: designs = await design_systems_svc.list_design_systems(user_id) systems = await systems_svc.list_systems(user_id, project_id, include_archived=True) return { - "always_on_rulebooks": [{"id": i, "title": t} for i, t, on in rows if on], - "other_rulebooks": [{"id": i, "title": t} for i, t, on in rows if not on], - "excluded_always_on": applicable.get("excluded_always_on", []), + # ONE list since milestone 394. This was split into always-on and + # "other" because the first bound the project whether it asked or not; + # with the tier gone every rulebook is opt-in, so the split named a + # difference that no longer exists. + "rulebooks": [{"id": i, "title": t} for i, t in rows], "subscribed_rulebooks": applicable.get("subscribed_rulebooks", []), "design_system_id": project.design_system_id, "design_systems": [{"id": d.id, "title": d.title} for d in designs], @@ -139,28 +137,22 @@ async def _check_targets(user_id: int, choices: dict) -> None: effect lands — a decision applies whole or errors whole.""" from scribe.services import access - wanted = set(choices["exclude_always_on_rulebooks"]) | set(choices["subscribe_rulebooks"]) + wanted = set(choices["subscribe_rulebooks"]) if wanted: async with async_session() as session: rows = ( await session.execute( - select(Rulebook.id, Rulebook.always_on).where( + select(Rulebook.id).where( Rulebook.id.in_(wanted), Rulebook.owner_user_id == user_id, Rulebook.deleted_at.is_(None), ) ) ).all() - found = {rid: on for rid, on in rows} - missing = sorted(wanted - set(found)) + found = {rid for (rid,) in rows} + missing = sorted(wanted - found) if missing: raise ValueError(f"rulebook(s) {missing} not found (or not yours)") - not_always = sorted(r for r in choices["exclude_always_on_rulebooks"] if not found[r]) - if not_always: - raise ValueError( - f"rulebook(s) {not_always} are not always-on — only always-on rulebooks " - "can be excluded; a subscribed rulebook is simply not subscribed" - ) ds = choices["design_system_id"] if ds is not None and not await access.can_read_design_system(user_id, ds): raise ValueError(f"design system {ds} not found (or not readable)") @@ -176,13 +168,12 @@ async def decide( """Record a project's inception decision and apply it (milestone 297). Owner-only. Validates the choices (pure) and every target (owned / - readable) first; then, each idempotent: exclude the named always-on - rulebooks, subscribe the named rulebooks, point the project at the design - system (None = explicitly none), seed the standard Systems if asked and - the project has none; then write ``projects.inception`` LAST. Re-deciding - is additive for exclusions/subscriptions (nothing is silently dropped — - include/unsubscribe are explicit calls), replaces the design system, and - re-seeds nothing a project already has. + readable) first; then, each idempotent: subscribe the named rulebooks, + point the project at the design system (None = explicitly none), seed the + standard Systems if asked and the project has none; then write + ``projects.inception`` LAST. Re-deciding is additive for subscriptions + (nothing is silently dropped — unsubscribe is an explicit call), replaces + the design system, and re-seeds nothing a project already has. Returns {"inception": , "effects": {excluded, subscribed, design_system_id, systems_seeded}}. @@ -203,8 +194,6 @@ async def decide( raise ValueError(f"project {project_id} not found (or not yours)") await _check_targets(user_id, choices) - for rb in choices["exclude_always_on_rulebooks"]: - await rulebooks_svc.exclude_always_on_rulebook_for_project(project_id, rb, user_id) for rb in choices["subscribe_rulebooks"]: await rulebooks_svc.subscribe_project(project_id, rb, user_id) if not await design_systems_svc.set_project_design_system( @@ -230,7 +219,6 @@ async def decide( return { "inception": record, "effects": { - "excluded": choices["exclude_always_on_rulebooks"], "subscribed": choices["subscribe_rulebooks"], "design_system_id": choices["design_system_id"], "systems_seeded": [sy.name for sy in seeded], @@ -247,25 +235,24 @@ async def inception_ask(user_id: int, project_id: int) -> dict: defaults = await current_defaults(user_id, project_id) except Exception: return {} - always = ", ".join(f"{r['title']} (#{r['id']})" for r in defaults["always_on_rulebooks"]) or "none" - others = ", ".join(f"{r['title']} (#{r['id']})" for r in defaults["other_rulebooks"]) or "none" + books = ", ".join(f"{r['title']} (#{r['id']})" for r in defaults["rulebooks"]) or "none" designs = ", ".join(f"{d['title']} (#{d['id']})" for d in defaults["design_systems"]) or "none" return { "defaults": defaults, "ask": ( "This project has no inception decision: nobody has said what it " - f"inherits. Today, by default: always-on rulebooks binding it — {always}; " - f"rulebooks it could subscribe to — {others}; design system — " + f"inherits. Rulebooks it could subscribe to — {books}; design system — " f"{'#' + str(defaults['design_system_id']) if defaults['design_system_id'] else 'none'} " f"(available: {designs}); Systems — {defaults['systems']}. Ask the operator, " - "once: which always-on rulebooks to EXCLUDE here (default: none), which " - "rulebooks to subscribe, which design system (or none), and whether to seed " + "once: which rulebooks to subscribe (default: none — a rulebook binds " + "a project only when it opts in), which design system (or none), and " + "whether to seed " "the standard starter Systems — then record the answers. This ask repeats on " "every enter_project until a decision is recorded." ), "call": ( f"decide_project_inception(project_id={project_id}, " - "exclude_always_on_rulebooks=[...], subscribe_rulebooks=[...], " + "subscribe_rulebooks=[...], " "design_system_id=, seed_systems=)" ), } diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index e37bf57..bdf6471 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -8,7 +8,7 @@ Design note — altitude: we inject rule *titles* grouped by topic (a compact index), NOT every rule's full statement. The 48 always-on statements run well past the 10k-char `additionalContext` cap, and the push channel's job is to make Claude *aware* the rules exist and *reach* for them — not to dump them. Full -text stays one `get_rule(id)` / `list_always_on_rules()` call away. Titles are +text stays one `get_rule(id)` / `search(content_type="rule")` call away. Titles are mostly self-describing ("`dev` is home", "No GitHub — Fabled-Git only"), so the index alone already steers behavior. """ @@ -1400,7 +1400,6 @@ async def build_write_path_hint( repo_key: str = "", exclude_derive: list[str] | None = None, exclude_rule_ids: list[int] | None = None, - rules_etag: str = "", ) -> dict: """Prior-art hint for the plugin's PreToolUse hook on Write/Edit. @@ -1686,60 +1685,22 @@ async def build_write_path_hint( staleness: list[str] = [] # ── Have the rules moved under this session? (milestone 323) ─────── # - # THE CARRIER IS THE POINT. This hook already fires before a write — the - # moment acting on a stale rule actually costs something — and the check - # is one comparison against a marker the session already holds. No - # payload, no extra round trip, and nothing said when nothing moved. + # THE RULES-ETAG STALENESS ARM IS GONE (milestone 394). # - # WHAT THIS CANNOT SEE, and a reader who finds an etag here will assume - # otherwise: + # It took a marker the session had been given at SessionStart, compared it + # against the resident set as it stood now, and said which rules had moved + # or fallen out of force. That was worth doing while a session held a + # fixed set of rules from turn zero and could be holding a stale copy of + # it hours later. # - # what goes wrong | caught? - # ---------------------------------------------------|-------- - # another session edits a rule mid-flight | yes - # the session is misremembering a rule read hours ago | yes - # compaction summarised the rules out of context | NO + # Nothing is resident now. A rule is retrieved at the moment it applies, + # so a session cannot be holding an out-of-date one — the next act that + # needs it fetches it again. The staleness this arm reported was an + # artifact of the delivery model rather than a fact about the corpus, and + # it goes with the model. # - # The third is the most common and this is blind to it: the etag was in - # context too and went with the rules. The SessionStart nudge is that - # case's only mechanism and must not be softened because this shipped. - # - # Fails open, like every other arm here: a staleness hint must never - # break a write. - if rules_etag: - try: - current = await rulebooks_svc.list_always_on_rules( - user_id, project_id=project_id or 0, - ) - if rulebooks_svc.rules_etag(current) != rules_etag: - moved = rulebooks_svc.rules_moved_since(current, rules_etag) - held = rulebooks_svc.etag_count(rules_etag) - bits = [] - if moved: - named = ", ".join( - f"#{r.id} \u201c{r.title}\u201d" for r in moved[:3] - ) - more = len(moved) - 3 - bits.append( - f"{named}" + (f", and {more} more" if more > 0 else "") - ) - # A DELETED rule moves no timestamp and leaves no row to name, - # so the count is the only thing that can report the one change - # that takes an instruction OUT of force. - if held is not None and held != len(current): - delta = len(current) - held - bits.append( - f"{abs(delta)} rule(s) {'added' if delta > 0 else 'no longer in force'}" - ) - if bits: - staleness.append( - "Your loaded rules have changed since this session " - "started — " + "; ".join(bits) + ". Re-read them with " - "list_always_on_rules() before relying on the set you " - "are holding." - ) - except Exception: - logger.debug("write-path rules-etag arm failed", exc_info=True) + # `staleness` survives as the list the arms below still append to. + # The guard sits BELOW the staleness arm on purpose. A rules change is # unconditional news — it does not become less true because this @@ -2208,67 +2169,37 @@ async def build_session_context( its normalized key — triggers a one-line "bind this repo" hint so the binding is self-healing. - Returns {"context": str, "rule_count": int, "project": dict | None, - "rules_etag": str}. The etag is for the HOOK, not for the model — the - hook stores it and hands it back on each write so the server can say - whether these rules have moved since the session loaded them. + Returns {"context": str, "project": dict | None}. + + It carried `rule_count` and `rules_etag` until milestone 394, when the + preload it described was removed. The etag let the hook hand a marker back + on each write so the server could say whether the resident rules had + moved; nothing is resident now, so nothing can have moved, and a rule is + re-retrieved at the moment it applies rather than held and aged. `context` is markdown ready to drop into `additionalContext`; it is capped at _MAX_CHARS with an explicit truncation note so the hook can pass it through verbatim. """ - # Inside a project, the always-on set is the project's: an inception - # exclusion (milestone 297) takes a rulebook out of this block, and is - # named below so the departure is visible rather than silent. - rules = await rulebooks_svc.list_always_on_rules(user_id, project_id=project_id) - # AMBIENT source, and the one that matters most: this is the preload — the - # block every session opens with, chosen by nobody, paid for every turn. - # - # It emitted nothing until 2026-09-03, which made the resident set's cost - # certain and its usefulness unfalsifiable at the same time (#3473). Note - # #3089 is the argument this measurement finally lets someone test: that a - # rule arriving with thirty others, none of them relevant, is read as - # preamble rather than as a claim — so presence is not surfacing, and a - # tier-1 set can grow without anybody noticing it stopped working. - # - # Recorded even when the hook truncates the block below: the rules WERE - # delivered, and counting only the untruncated ones would quietly shrink - # the denominator exactly where the set is too big to read. - record_rule_surfaced( - user_id=user_id, - rule_ids=[r.id for r in rules], - source="session_start", - ) - excluded = ( - await rulebooks_svc.excluded_always_on_rulebooks(user_id, project_id) - if project_id else [] - ) - topic_map = await _topic_titles({r.topic_id for r in rules if r.topic_id}) - lines: list[str] = [ "# Scribe — standing session context (auto-injected by the Scribe plugin)", "", - "You are working with Scribe, the operator's self-hosted second brain. " - "The always-on rules below are BINDING this session. Titles only — full " - "text via `list_always_on_rules()` or `get_rule(id)`.", + "You are working with Scribe, the operator's self-hosted second brain.", "", - "## Always-on rules (by topic)", + "## You are not holding the operator's rules", + "", + "No rule has been loaded into this session, and that is deliberate. " + "Rules arrive when something you are about to do makes one relevant — " + "a command you are about to run, code you are writing, or what the " + "operator just asked for. On most turns none will, and that is the " + "surface working rather than failing.", + "", + "**\"No rule arrived\" means \"nothing matched\" — never \"there is no " + "rule.\"** Before a consequential act, one that is hard to reverse or " + "outward-facing, `search(content_type=\"rule\")` is how you ask. " + "Retrieval runs on its own and is a convenience; asking is what you do " + "when it matters and nothing has spoken.", ] - # rules already arrive ordered by rulebook/topic/order, so grouping by - # consecutive topic_id preserves the intended sequence. - current_topic: int | None = object() # sentinel distinct from any id/None - for r in rules: - if r.topic_id != current_topic: - current_topic = r.topic_id - heading = topic_map.get(r.topic_id, "ungrouped") if r.topic_id else "ungrouped" - lines.append(f"### {heading}") - lines.append(f"- [{r.id}] {r.title}") - if excluded: - names = ", ".join(f"{e['title']} (#{e['id']})" for e in excluded) - lines += [ - "", - f"Excluded for this project by its inception decision (not binding here): {names}.", - ] project_dict: dict | None = None if project_id: @@ -2336,14 +2267,10 @@ async def build_session_context( context = "\n".join(line for line in lines if line is not None) if len(context) > _MAX_CHARS: - context = context[:_MAX_CHARS].rstrip() + "\n\n…(truncated — call list_always_on_rules())" + context = context[:_MAX_CHARS].rstrip() + \ + "\n\n…(truncated — ask with search(content_type=\"rule\"))" return { "context": context, - "rule_count": len(rules), "project": project_dict, - # Computed from the rules THIS payload was built from, not re-queried: - # the marker has to describe the set the session is actually holding, - # and a second query could disagree with the first. - "rules_etag": rulebooks_svc.rules_etag(rules), } diff --git a/src/scribe/services/retrieval_telemetry.py b/src/scribe/services/retrieval_telemetry.py index f44fa44..bffdfc1 100644 --- a/src/scribe/services/retrieval_telemetry.py +++ b/src/scribe/services/retrieval_telemetry.py @@ -852,7 +852,7 @@ async def retrieval_summary( # something else. # # `ambient` now carries the bulk deliveries — the SessionStart preload, - # `list_always_on_rules`, and every `rules_payload` surface (#3473). Before + # and every `rules_payload` surface (#3473). Before # they emitted, this block had no ambient key and said the absence was a # fact about the data. It was, and it was also the thing that made the # always-on set impossible to judge: the largest rule surface in the diff --git a/src/scribe/services/rule_usage.py b/src/scribe/services/rule_usage.py index a47a869..3162b41 100644 --- a/src/scribe/services/rule_usage.py +++ b/src/scribe/services/rule_usage.py @@ -46,7 +46,7 @@ AMBIENT VS RANKED. The note twin splits ranked surfacings from ambient ones because `enter_project` and the skill sync put records in front of the agent without choosing them, and counting those as surfacings makes recency read as popularity (#2477). Rules have exactly that shape: the SessionStart preload, -`list_always_on_rules`, and every `rules_payload` surface hand over the whole +and every `rules_payload` surface hand over the whole applicable set at once, chosen by nobody. Until 2026-09-03 those bulk surfaces emitted nothing, and this module said so — @@ -184,8 +184,8 @@ def record_rule_surfaced( def record_rule_pulled(*, user_id: int | None, rule_id: int, source: str) -> None: """Fire-and-forget: record that a rule was opened in full. - A PULL is somebody choosing to open one record. `list_always_on_rules` and - `enter_project` are NOT pulls — they are bulk resident loads that hand over + A PULL is somebody choosing to open one record. `enter_project` is NOT a + pull — they are bulk resident loads that hand over every applicable rule at once, and counting them would swamp the signal with the very ambient delivery the ratio exists to distinguish from. """ diff --git a/src/scribe/services/rule_versions.py b/src/scribe/services/rule_versions.py index 004d481..a13d09a 100644 --- a/src/scribe/services/rule_versions.py +++ b/src/scribe/services/rule_versions.py @@ -36,7 +36,7 @@ from scribe.models.rule_version import RuleVersion # snapshots would bury the edits somebody is actually looking for. SNAPSHOT_FIELDS = ( "title", "statement", "why", "how_to_apply", "when_to_apply", - "tier", "kind", "verify_with", "expires_when", + "kind", "verify_with", "expires_when", ) diff --git a/src/scribe/services/rulebooks.py b/src/scribe/services/rulebooks.py index 8b1a40e..5a8c208 100644 --- a/src/scribe/services/rulebooks.py +++ b/src/scribe/services/rulebooks.py @@ -12,7 +12,7 @@ from collections.abc import Iterable from datetime import datetime from typing import Optional -from sqlalchemy import and_, delete as sql_delete, insert, or_, select +from sqlalchemy import and_, delete as sql_delete, false as sa_false, insert, or_, select from scribe.models import async_session from scribe.models.system import System @@ -82,7 +82,7 @@ async def update_rulebook( rb = result.scalar_one_or_none() if rb is None: return None - allowed = {"title", "description", "always_on"} + allowed = {"title", "description"} for key, value in fields.items(): if key in allowed and value is not None: setattr(rb, key, value) @@ -293,7 +293,6 @@ async def _assert_rulebook_rule_owned(session, rule_id: int, user_id: int) -> No # The vocabularies migration 0088's CHECK constraints enforce. Named here so # a caller can be corrected before the database refuses it (rule 36 keeps the # two in step; this keeps the error readable). -TIERS = ("always_on", "conditional") RELATION_KINDS = ("co_surfaces", "overrides", "elaborates") # Migration 0098's CHECK. `rule` binds; `preference` is how the operator # wants work done — see the model comment for why both live on one table. @@ -311,21 +310,11 @@ NULLABLE_RULE_TEXT = ( ) -def _valid_tier(tier: str) -> str: - """An unrecognised tier falls back to always_on — the SAFE direction. - - Getting this wrong the other way would silently stop a rule binding, which - is the one failure this whole milestone exists to prevent. A rule that - preloads when it did not need to costs context; a rule that quietly stops - preloading costs the behaviour it was written for. - """ - return tier if tier in TIERS else "always_on" - - def _valid_kind(kind: str) -> str: """An unrecognised kind falls back to `rule` — the SAFE direction. - Same shape as _valid_tier and the same argument, pointed at force instead + The unrecognised value falls back to the binding one — the SAFE + direction, pointed at force instead of delivery. A preference wrongly treated as binding costs a little friction: the reader is told something is required that was only preferred. A rule wrongly treated as a preference costs the thing the rule @@ -369,7 +358,6 @@ def rule_brief(rule: Rule, **extra) -> dict: "title": rule.title, "statement": rule.statement, "topic_id": rule.topic_id, - "tier": rule.tier, # Unconditional, and the payload cost is accepted deliberately. Every # other optional key below is attached only when present, because an # absent key should never read as a capability the record lacks. Force @@ -503,7 +491,7 @@ async def rule_detail(user_id: int, rule: Rule, system_ids: list[int] | None = N async def create_rule( topic_id: int, user_id: int, title: str, statement: str, why: str = "", how_to_apply: str = "", order_index: int = 0, - when_to_apply: str = "", tier: str = "always_on", arose_from_id: int = 0, + when_to_apply: str = "", arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", kind: str = "rule", ) -> Rule: async with async_session() as session: @@ -513,7 +501,6 @@ async def create_rule( title=title, statement=statement, when_to_apply=when_to_apply or None, - tier=_valid_tier(tier), kind=_valid_kind(kind), why=why or None, how_to_apply=how_to_apply or None, @@ -532,7 +519,7 @@ async def create_rule( async def create_project_rule( project_id: int, user_id: int, title: str, statement: str, why: str = "", how_to_apply: str = "", order_index: int = 0, - when_to_apply: str = "", tier: str = "always_on", arose_from_id: int = 0, + when_to_apply: str = "", arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", kind: str = "rule", ) -> Rule: """Create a rule scoped to a single project (no rulebook ceremony). @@ -548,7 +535,6 @@ async def create_project_rule( title=title, statement=statement, when_to_apply=when_to_apply or None, - tier=_valid_tier(tier), kind=_valid_kind(kind), why=why or None, how_to_apply=how_to_apply or None, @@ -632,89 +618,6 @@ async def list_rules( return rulebook_rules + list(proj_result.scalars().all()) -def _excluded_rulebook_ids_q(project_id: int): - """Subquery: the always-on rulebooks this project opted out of at - inception (milestone 297) — used by every rule-resolution path so an - exclusion is total, not just cosmetic.""" - from scribe.models.rulebook import project_rulebook_exclusions - - return select(project_rulebook_exclusions.c.rulebook_id).where( - project_rulebook_exclusions.c.project_id == project_id - ) - - -async def excluded_always_on_rulebooks(user_id: int, project_id: int) -> list[dict]: - """[{id, title}] of the always-on rulebooks excluded for ``project_id`` - (owner-scoped). Empty for an undecided or inherit-all project.""" - from scribe.models.rulebook import project_rulebook_exclusions - - if not project_id: - return [] - async with async_session() as session: - rows = ( - await session.execute( - select(Rulebook.id, Rulebook.title) - .join(project_rulebook_exclusions, - project_rulebook_exclusions.c.rulebook_id == Rulebook.id) - .where( - project_rulebook_exclusions.c.project_id == project_id, - Rulebook.owner_user_id == user_id, - Rulebook.deleted_at.is_(None), - ) - .order_by(Rulebook.title) - ) - ).all() - return [{"id": rid, "title": title} for rid, title in rows] - - -async def list_always_on_rules( - user_id: int, limit: int = 100, project_id: int = 0, -) -> list[Rule]: - """Return all rules from rulebooks flagged always_on for the user. - - Called by the MCP tool of the same name at session start to load the - standing rules that apply regardless of which project (if any) is in - scope. Ordering matches list_rules so results are stable across calls. - - ``project_id`` (milestone 297): inside a project that excluded specific - always-on rulebooks at inception, those rulebooks' rules are NOT - returned — the project decided not to inherit them. 0 = the user-wide - set, which is what a session sees before a project is in scope. - """ - async with async_session() as session: - q = ( - select(Rule) - .join(RulebookTopic, Rule.topic_id == RulebookTopic.id) - .join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id) - .where( - Rulebook.owner_user_id == user_id, - Rulebook.always_on.is_(True), - Rule.deleted_at.is_(None), - RulebookTopic.deleted_at.is_(None), - Rulebook.deleted_at.is_(None), - # TIER (milestone 307). This is the SESSION-START call, made - # before any project is in scope — there is no area vocabulary - # to match a conditional rule against yet, so only the - # unconditional tier belongs here. A conditional rule reaches a - # session through enter_project (by area) or search (by - # meaning), not by being resident. - # - # Behaviour is unchanged until rules are actually re-tiered: - # `tier` defaults to always_on, so every existing rule still - # arrives exactly as it did. - Rule.tier == "always_on", - ) - ) - if project_id: - q = q.where(Rulebook.id.notin_(_excluded_rulebook_ids_q(project_id))) - result = await session.execute( - q.order_by( - Rulebook.id, RulebookTopic.order_index, Rule.order_index, Rule.title, - ).limit(limit) - ) - return list(result.scalars().all()) - - async def _fetch_owned_rule(session, rule_id: int, user_id: int) -> Optional[Rule]: """Fetch a rule by id, scoped to user owning either its rulebook (via topic) or its project (via project_id). Honors soft-delete. @@ -780,7 +683,7 @@ async def update_rule( return None allowed = { "title", "statement", "why", "how_to_apply", "order_index", - "when_to_apply", "tier", "kind", "arose_from_id", + "when_to_apply", "kind", "arose_from_id", "verify_with", "expires_when", } check_before = rule.verify_with @@ -796,9 +699,7 @@ async def update_rule( for key, value in fields.items(): if key not in allowed or value is None: continue - if key == "tier": - value = _valid_tier(value) - elif key == "kind": + if key == "kind": value = _valid_kind(value) elif key in NULLABLE_RULE_TEXT: value = value or None @@ -808,9 +709,8 @@ async def update_rule( # A verification stamp certifies A CHECK, not a rule. Rewrite or # remove the check and the old stamp certifies something that no # longer exists — so it is dropped, and the rule re-enters the sweep. - # The safe direction, for the same reason _valid_tier falls back to - # always_on: a rule wrongly listed as due costs one look, a rule - # wrongly vouched for costs the thing the sweep exists to catch. + # The safe direction: a rule wrongly listed as due costs one look, a + # rule wrongly vouched for costs the thing the sweep exists to catch. if rule.verify_with != check_before: rule.verified_at = None # Same session as the edit, so the two commit together. The snapshot @@ -1113,51 +1013,6 @@ async def unsuppress_rule_for_project( await session.commit() -async def exclude_always_on_rulebook_for_project( - project_id: int, rulebook_id: int, user_id: int, -) -> None: - """Opt one project out of a whole ALWAYS-ON rulebook (milestone 297). - Owner-only on both sides; the rulebook must be always_on — a subscribed - rulebook is left by unsubscribing, not excluding. Idempotent.""" - from scribe.models.rulebook import project_rulebook_exclusions - - async with async_session() as session: - await _assert_project_owned(session, project_id, user_id) - await _assert_rulebook_owned(session, rulebook_id, user_id) - rb = await session.get(Rulebook, rulebook_id) - if rb is None or not rb.always_on: - raise ValueError( - f"rulebook {rulebook_id} is not always-on — it binds only by " - "subscription; unsubscribe_project_from_rulebook instead" - ) - try: - await session.execute( - insert(project_rulebook_exclusions).values( - project_id=project_id, rulebook_id=rulebook_id, - ) - ) - await session.commit() - except IntegrityError: - await session.rollback() # already excluded — idempotent - - -async def include_always_on_rulebook_for_project( - project_id: int, rulebook_id: int, user_id: int, -) -> None: - """Undo exclude_always_on_rulebook_for_project. Idempotent.""" - from scribe.models.rulebook import project_rulebook_exclusions - - async with async_session() as session: - await _assert_project_owned(session, project_id, user_id) - await session.execute( - sql_delete(project_rulebook_exclusions).where( - project_rulebook_exclusions.c.project_id == project_id, - project_rulebook_exclusions.c.rulebook_id == rulebook_id, - ) - ) - await session.commit() - - async def suppress_topic_for_project( project_id: int, topic_id: int, user_id: int, ) -> None: @@ -1326,7 +1181,6 @@ async def get_applicable_rules( Rulebook.deleted_at.is_(None), # An inception exclusion is total (milestone 297): a rulebook the # project opted out of contributes nothing, subscribed or not. - Rulebook.id.notin_(_excluded_rulebook_ids_q(project_id)), ) .order_by( Rulebook.id, RulebookTopic.order_index, Rule.order_index, Rule.title, @@ -1337,11 +1191,10 @@ async def get_applicable_rules( rules_q = rules_q.where(Rule.id.notin_(suppressed_rule_ids)) if suppressed_topic_ids: rules_q = rules_q.where(Rule.topic_id.notin_(suppressed_topic_ids)) - # TIER (milestone 307). always_on rules are resident, as every rule was - # before tiers existed. A conditional rule is REACHABLE, and reaches - # this project only when it is tagged to an area this project actually - # works in — a deterministic tag match, never a similarity score, so - # bindingness never depends on a ranking (D7). + # AREA BINDING (milestone 307, narrowed by 394). A rule reaches this + # project when it is tagged to an area the project actually works in — + # a deterministic tag match, never a similarity score, so bindingness + # never depends on a ranking (D7). # # Applied in SQL rather than by filtering afterwards, so `limit` counts # the rules that will actually be surfaced instead of counting rules @@ -1357,10 +1210,21 @@ async def get_applicable_rules( reachable = select(rule_systems.c.rule_id).where( rule_systems.c.canonical_id.in_(project_area_ids) ) if project_area_ids else None - tier_clause = (Rule.tier == "always_on") - if reachable is not None: - tier_clause = or_(tier_clause, Rule.id.in_(reachable)) - rules_q = rules_q.where(tier_clause) + # AREA REACHABILITY IS NOW THE WHOLE TEST (milestone 394). This read + # `always_on OR reachable`, so a subscribed rulebook's resident rules + # arrived here whatever the project did. The tier is gone, and + # dropping its arm rather than the whole clause is the deliberate + # half: what survives is the DETERMINISTIC one — a rule binds this + # project because it is tagged to an area the project actually works + # in (D7), never because a similarity score cleared a bar. + # + # A project with no canonical-tagged Systems therefore gets no bulk + # rules here, and that is the reading rather than a gap: rules still + # reach it by retrieval, when something it is doing makes one + # relevant. Handing over every subscribed rule instead would make this + # payload BIGGER than the preload this milestone exists to remove. + rules_q = (rules_q.where(Rule.id.in_(reachable)) if reachable is not None + else rules_q.where(sa_false())) rule_rows = (await session.execute(rules_q)).all() truncated = len(rule_rows) > limit rules = [ @@ -1381,12 +1245,11 @@ async def get_applicable_rules( ) .order_by(Rule.order_index, Rule.title) ) - if reachable is not None: - proj_rules_q = proj_rules_q.where( - or_(Rule.tier == "always_on", Rule.id.in_(reachable)) - ) - else: - proj_rules_q = proj_rules_q.where(Rule.tier == "always_on") + # A PROJECT'S OWN RULES ARE NOT FILTERED BY AREA, and the asymmetry + # with the family query above is the point. A family rule has to earn + # its way into this project; a rule written ON this project is scoped + # to it by construction, and filtering it again would drop rules whose + # only fault is that nobody tagged them to a System. proj_rule_rows = (await session.execute(proj_rules_q)).all() project_rules = [rule_brief(rule) for (rule,) in proj_rule_rows] @@ -1422,7 +1285,6 @@ async def get_applicable_rules( "suppressed_topics": suppressed_topics, "truncated": truncated, "subscribed_rulebooks": subscribed_rulebooks, - "excluded_always_on": await excluded_always_on_rulebooks(user_id, project_id), } @@ -1434,9 +1296,6 @@ def rules_payload(applicable: dict, *, user_id: int | None, source: str) -> dict same seven keys under the same names — so a reader learns them once. One place renames `rules` → `applicable_rules` and `truncated` → `applicable_rules_truncated`; the tools merge this into their payloads. - `excluded_always_on` (milestone 297) names the always-on rulebooks this - project decided NOT to inherit, so the departure is visible wherever the - rules are. IT ALSO RECORDS THE SURFACING, which is why it now takes a caller and a source. Every one of those surfaces is a bulk delivery — the applicable set @@ -1453,7 +1312,7 @@ def rules_payload(applicable: dict, *, user_id: int | None, source: str) -> dict Emitting from here is safe in a way emitting from `get_applicable_rules` would not be: this function is only ever called to BUILD A REPLY. The two other callers of the rules machinery — the write-path etag arm - (`plugin_context`) and `rules_etag_for` — compute a marker and show nobody + (`plugin_context`) — computed a marker and showed nobody anything, and counting those would put rules in the denominator that no agent ever saw. """ @@ -1472,7 +1331,6 @@ def rules_payload(applicable: dict, *, user_id: int | None, source: str) -> dict "project_rules": applicable.get("project_rules", []), "suppressed_rules": applicable.get("suppressed_rules", []), "suppressed_topics": applicable.get("suppressed_topics", []), - "excluded_always_on": applicable.get("excluded_always_on", []), } @@ -1497,86 +1355,11 @@ def rules_payload(applicable: dict, *, user_id: int | None, source: str) -> dict _ETAG_EMPTY = "empty|0" -def rules_etag(rules: list) -> str: - """A marker for "is the set you are holding still the current one?". - - `max(updated_at)` alone is not enough: DELETING a rule moves no timestamp, - and that is the single change that takes an instruction OUT of force — - the one a session most needs to hear about. The count catches it. - - Instance-agnostic (rule 115): it knows nothing about any particular - rulebook, and an install with one rule or none produces a stable marker - rather than an error. "No rules" must read as a state, not as a change, - or every session on a fresh install would be told its rules had moved. - """ - if not rules: - return _ETAG_EMPTY - # A decoration must not be able to break what it decorates. This is - # computed on the SessionStart path, where raising would cost the whole - # context payload to save a hint — so a row with no usable timestamp is - # skipped rather than compared, and a set with none degrades to a - # count-only marker instead of failing. Count-only still catches a rule - # added or deleted; it just cannot see an edit, which is the right way - # round to lose information. - stamps = [ - r.updated_at for r in rules - if isinstance(getattr(r, "updated_at", None), datetime) - ] - if not stamps: - return f"unknown|{len(rules)}" - return f"{max(stamps).isoformat()}|{len(rules)}" - - -async def rules_etag_for(user_id: int, project_id: int = 0) -> str: - """The current marker for the set a session at this scope would hold. - - Deliberately built from `list_always_on_rules` rather than from a - `max()/count()` aggregate. An aggregate would be cheaper, and would have - to restate that function's definition of the set — the always_on flag, - the project's inception exclusions, the tier filter. Two definitions of - "the session's rules" is how the marker starts disagreeing with the - rules, which is worse than materialising a few dozen rows. - """ - rules = await list_always_on_rules(user_id, project_id=project_id) - return rules_etag(rules) - - -def rules_moved_since(rules: list, held_etag: str) -> list: - """The rules whose text changed after `held_etag` was issued. - - Returns [] when the marker matches, is unparseable, or is absent — a - caller cannot act on "something is different but I cannot say what", and - a garbled marker must not be reported as a change. - - A count difference is real news that this list cannot show: a rule - DELETED since the marker was issued has no row left to return. Callers - compare counts separately. - """ - if not held_etag or held_etag == _ETAG_EMPTY: - return [] - stamp, _, _count = held_etag.partition("|") - try: - held_at = datetime.fromisoformat(stamp) - except ValueError: - return [] - return [r for r in rules if r.updated_at and r.updated_at > held_at] - - -def etag_count(held_etag: str) -> int | None: - """How many rules the holder had. None when the marker cannot be read.""" - _stamp, _, count = (held_etag or "").partition("|") - try: - return int(count) - except ValueError: - return None - - # ── The staleness sweep (milestone 312) ──────────────────────────────── async def rules_due_for_verification( user_id: int, older_than_days: int = 0, - tier: str = "", never_only: bool = False, ) -> list[Rule]: """Rules that carry a check, oldest verification first, never-checked top. @@ -1605,20 +1388,12 @@ async def rules_due_for_verification( older_than_days: only rules last verified longer ago than this. Never-checked rules always qualify — they are the most overdue thing there is. 0 = no age filter. - tier: "always_on" or "conditional" to narrow. Raises on anything else - rather than falling back: _valid_tier's silent always_on default - is right for a WRITE (the safe direction is to keep binding), and - wrong for a FILTER, where it would quietly answer a different - question than the one asked. never_only: only rules that have never been verified. """ from datetime import datetime, timedelta, timezone from scribe.models.project import Project - if tier and tier not in TIERS: - raise ValueError(f"tier must be one of {TIERS}, got {tier!r}") - async with async_session() as session: stmt = ( select(Rule) @@ -1641,8 +1416,6 @@ async def rules_due_for_verification( ), ) ) - if tier: - stmt = stmt.where(Rule.tier == tier) if never_only: stmt = stmt.where(Rule.verified_at.is_(None)) elif older_than_days > 0: @@ -1668,7 +1441,6 @@ def verification_row(rule: Rule) -> dict: "id": rule.id, "title": rule.title, "statement": rule.statement, - "tier": rule.tier, "topic_id": rule.topic_id, "project_id": rule.project_id, "when_to_apply": rule.when_to_apply or "", -- 2.54.0 From bfa2d419f9304ff296c1c0988c1101bb8da0897b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 15:38:40 -0400 Subject: [PATCH 07/13] fix(telemetry): a logged query never carries a credential (#3925) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pre_tool_rule retrieves against the RAW COMMAND TEXT and write_path_rule against the code being written, so whatever was on the command line or in the buffer is what record_retrieval stored in retrieval_logs.query. A command that exported a token stored the token. Storing it was not the worst of it. near_miss_samples is the readout the threshold docs tell you to open before moving a bar, so the value came back OUT into an agent's context on the next tuning pass — which is exactly how this was found, mid-way through #3853's threshold spike. SCRUBBED ON WRITE, at _build_payload — the single seam every source reaches the column through. A read-side filter would leave the secret in the table where a backup or a debug query still reaches it, and a per-caller scrub would be three places for one to be forgotten by whoever adds the fourth arm. REDACTED VISIBLY. `[redacted:]` rather than a silent deletion: a reader who cannot tell a scrubbed query from a short one is being lied to by the readout itself. DELIBERATELY CONSERVATIVE — vendor-prefixed credentials, values assigned to secret-NAMED variables, auth headers, PEM blocks. Things that are secrets by construction. Entropy heuristics and long-opaque-string detection start eating real queries, and a query is evidence: missing an exotic secret costs one redaction nobody made, while eating a query costs the ability to tune the bar at all. The guard pins BOTH directions, and the second half is the one that matters. A scrubber that eats evidence fails silently — it keeps looking like it works while turning the one instrument for tuning a threshold into unreadable stubs, which is the #2663 shape in a new place. So nine REAL queries from this install's near-miss samples must survive byte for byte. If a future pattern touches one, the pattern is too greedy. Verified against the real shapes before commit: six credential formats redacted (fabricated values), nine real queries unchanged, and the payload seam confirmed to store "export API_TOKEN=[redacted:assigned] && git push". This does NOT scrub rows already written. Purging those is separate. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- src/scribe/services/retrieval_telemetry.py | 81 ++++++++++++- tests/test_retrieval_query_scrubbing.py | 128 +++++++++++++++++++++ 2 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 tests/test_retrieval_query_scrubbing.py diff --git a/src/scribe/services/retrieval_telemetry.py b/src/scribe/services/retrieval_telemetry.py index bffdfc1..d15c892 100644 --- a/src/scribe/services/retrieval_telemetry.py +++ b/src/scribe/services/retrieval_telemetry.py @@ -17,6 +17,7 @@ from __future__ import annotations import asyncio import logging +import re from typing import Any from datetime import datetime, timedelta, timezone @@ -44,6 +45,81 @@ _pending: set[asyncio.Task] = set() _reported = False + +# ── secrets never reach the query column (#3925) ─────────────────────── +# +# `pre_tool_rule` retrieves against the RAW COMMAND TEXT and `write_path_rule` +# against the code being written, so whatever was on the command line or in the +# buffer is what gets logged. A command that exports a token therefore stored +# the token — and worse than stored it: `near_miss_samples` is the readout the +# threshold docs tell you to open before moving a bar, so the value came back +# out into an agent's context on the next tuning pass. That is how this was +# found. +# +# SCRUBBED ON WRITE, NOT ON READ. A read-side filter leaves the secret in the +# table, where a backup, a debug query or a future readout still reaches it. +# The value must never land. +# +# REDACTED VISIBLY, AND THIS IS THE PART THAT KEEPS THE READOUT HONEST. The +# whole worth of a near-miss sample is reading the query that was actually +# refused; a scrubber that silently deleted spans would turn the one instrument +# for tuning a bar into unreadable stubs — the #2663 shape, where a surface +# looks fine and has quietly stopped saying anything. A `[redacted:]` +# marker keeps the sentence readable, keeps its shape and length roughly +# intact for the ranker's reader, and says plainly that something was removed. +# +# DELIBERATELY CONSERVATIVE. These patterns match things that are secrets by +# CONSTRUCTION — a vendor-prefixed credential, a value assigned to a +# secret-named variable, an auth header, a PEM header. Anything cleverer +# (entropy heuristics, long-opaque-string detection) starts eating real +# queries, and a query is evidence. Missing an exotic secret costs one +# redaction nobody made; eating a query costs the ability to tune the bar. +_SECRET_PATTERNS: tuple[tuple[str, "re.Pattern[str]"], ...] = ( + # Vendor-prefixed credentials. The prefix IS the tell, so no entropy + # guessing is needed — `fmcp_` is Scribe's own API key format. + ("token", re.compile( + r"\b(?:fmcp_|flt_|ghp_|gho_|ghs_|ghu_|github_pat_|glpat-|gitlab-ci-token:" + r"|xox[abprs]-|sk-[A-Za-z0-9]*-?|AKIA|ASIA)[A-Za-z0-9_\-]{12,}" + )), + # A value handed to a secret-NAMED variable, in shell, env files, YAML, + # JSON or a query string. The name is what identifies it, so the value can + # be anything. + ("assigned", re.compile( + r"(?i)\b([A-Za-z0-9_]*" + r"(?:token|secret|password|passwd|api[_-]?key|access[_-]?key|auth)" + r"[A-Za-z0-9_]*)" + r"(\s*[:=]\s*[\"']?)" + r"([^\s\"'&]{8,})" + )), + ("auth-header", re.compile( + r"(?i)(authorization\s*:\s*(?:bearer|basic|token)\s+)(\S+)" + )), + ("private-key", re.compile( + r"-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----" + )), +) + + +def scrub_secrets(text: str | None) -> str | None: + """Redact credential-shaped spans from a query before it is stored. + + Pure and synchronous, so it is unit-testable and safe to run inline on the + write path. Returns the input unchanged when nothing matches, which is the + overwhelmingly common case and the one the patterns are tuned to protect. + """ + if not text: + return text + for kind, pattern in _SECRET_PATTERNS: + if kind == "assigned": + text = pattern.sub( + lambda m: f"{m.group(1)}{m.group(2)}[redacted:{kind}]", text) + elif kind == "auth-header": + text = pattern.sub(lambda m: f"{m.group(1)}[redacted:{kind}]", text) + else: + text = pattern.sub(f"[redacted:{kind}]", text) + return text + + def _build_payload( *, user_id: int | None, @@ -85,7 +161,10 @@ def _build_payload( return { "user_id": user_id, "source": source, - "query": query, + # Scrubbed HERE rather than at each caller: this is the only path to + # the column, and a per-caller scrub is three places for one of them + # to be forgotten by whoever adds the fourth arm. + "query": scrub_secrets(query), "threshold": threshold, "limit_n": limit, "project_id": project_id, diff --git a/tests/test_retrieval_query_scrubbing.py b/tests/test_retrieval_query_scrubbing.py new file mode 100644 index 0000000..17c0807 --- /dev/null +++ b/tests/test_retrieval_query_scrubbing.py @@ -0,0 +1,128 @@ +"""A logged query never carries a credential (#3925). + +WHY THIS EXISTS + +`pre_tool_rule` retrieves against the RAW COMMAND TEXT and `write_path_rule` +against the code being written, so whatever was on the command line or in the +buffer is what `record_retrieval` stores in `retrieval_logs.query`. A command +that exported a token therefore stored the token. + +And storing it was not the worst of it. `near_miss_samples` is the readout the +threshold documentation tells you to open before moving a bar, so the value +came back OUT into an agent's context on the next tuning pass — which is +exactly how this was found, during #3853's threshold spike. + +WHAT THIS PINS, IN BOTH DIRECTIONS, AND WHY THE SECOND HALF IS THE HARD ONE + +A scrubber has two ways to fail and only one of them is obvious. + + 1. It misses a secret. Caught by the redaction cases below. + 2. It eats the EVIDENCE. This is the failure that would do more damage, + because it is silent: the whole worth of a near-miss sample is reading the + query that was actually refused, and a scrubber that chewed up ordinary + commands would turn the one instrument for tuning a bar into unreadable + stubs while still looking like it worked. That is the #2663 shape — a + surface that reads fine and has quietly stopped saying anything. + +So the second block is not padding. Its cases are REAL queries taken from this +install's `near_miss_samples` during #3853, and they must survive byte for +byte. If a future pattern is added and one of them changes, the pattern is too +greedy — tighten it rather than editing the expectation. + +The secret cases use FABRICATED values in the real formats. Nothing here is or +was a live credential. +""" +import pytest + +from scribe.services.retrieval_telemetry import scrub_secrets + +# Fabricated, in the shapes that actually occur. The first is the shape that +# was found stored: a shell assignment of a vendor-prefixed token. +_SECRETS = [ + ("vendor-prefixed token in a shell assignment", + "TOK=flt_AAAABBBBCCCCDDDDEEEEFFFF\npython3 - <<'PY'", + "flt_AAAABBBBCCCCDDDDEEEEFFFF"), + ("a Scribe fmcp_ key in an auth header", + "curl -H 'Authorization: Bearer fmcp_ZZZZYYYYXXXXWWWWVVVV' https://x", + "fmcp_ZZZZYYYYXXXXWWWWVVVV"), + ("a forge token in an export", + "export GITHUB_TOKEN=ghp_1234567890abcdefghijABCDEF", + "ghp_1234567890abcdefghijABCDEF"), + ("a value assigned to a secret-named variable", + 'REGISTRY_PASSWORD="hunter2-correct-horse"', + "hunter2-correct-horse"), + ("an api_key in a query string", + "curl 'https://api.example/v1/things?api_key=abcdef1234567890'", + "abcdef1234567890"), + ("a private key block", + "-----BEGIN RSA PRIVATE KEY-----\nMIIEowIBAAKC\n-----END RSA PRIVATE KEY-----", + "MIIEowIBAAKC"), +] + +# Real queries, from this install's near-miss samples during #3853. +_EVIDENCE = [ + "git push origin dev", + 'git pull --rebase origin dev 2>&1 | tail -3; echo "=== HEAD ==="; git log --oneline -2', + "python3 - <<'PY'\nimport pathlib\np = pathlib.Path(\"web/src/routes/admin/tuning/tuning.test.ts\")", + "docker compose up -d", + 'package library\n\nimport (\n\t"context"\n\t"fmt"\n)', + "import {\n fetchTransfers,\n retryTransfer,\n} from './api'", + 'grep -rn "useState" src/components/ | head -20', + # The word "token" in ordinary prose is not a token. + "explain how the token bucket rate limiter works", + "wc -l src/*.py && date", +] + + +@pytest.mark.parametrize(("label", "text", "secret"), _SECRETS, + ids=[c[0] for c in _SECRETS]) +def test_a_credential_never_survives_into_the_query_column(label, text, secret): + """The value goes; something visible stays in its place.""" + out = scrub_secrets(text) + assert secret not in out, ( + f"{label}: the credential is still in the text that would be stored" + ) + assert "[redacted" in out, ( + f"{label}: the span was removed without saying so. A silent deletion " + "leaves a reader unable to tell a scrubbed query from a short one, " + "which is the readout lying about itself rather than protecting you." + ) + + +@pytest.mark.parametrize("query", _EVIDENCE) +def test_an_ordinary_query_is_stored_exactly_as_it_was(query): + """Evidence survives byte for byte. + + These came out of real `near_miss_samples`. A threshold is tuned by reading + them, so a pattern greedy enough to touch one has destroyed the instrument + it was meant to make safe — tighten the pattern, never this expectation. + """ + assert scrub_secrets(query) == query + + +def test_empty_and_missing_queries_pass_through(): + """Some sources log no query at all; scrubbing must not invent one.""" + assert scrub_secrets(None) is None + assert scrub_secrets("") == "" + + +def test_the_write_path_scrubs_rather_than_the_read_path(): + """The payload built for storage carries the redacted text (#3925). + + Pinned on `_build_payload` because that is the single seam every source + reaches the column through. A per-caller scrub would be three places for + one of them to be forgotten by whoever adds the fourth arm — and the one + forgotten would be the one that stored a secret. + """ + from scribe.services.retrieval_telemetry import _build_payload + + payload = _build_payload( + user_id=1, source="pre_tool_rule", + query="export API_TOKEN=ghp_1234567890abcdefghijABCDEF && git push", + threshold=0.68, limit=5, project_id=0, is_task=None, + results=[], duration_ms=1.0, + ) + assert "ghp_1234567890abcdefghijABCDEF" not in payload["query"] + assert "[redacted" in payload["query"] + # The rest of the command survives, or the row stops being evidence. + assert "git push" in payload["query"] -- 2.54.0 From 18e3cf9f2f4ed28d4df2c8a119baf3930ed7d4b5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 15:43:31 -0400 Subject: [PATCH 08/13] fix(telemetry): scrub existing rows, and stop --author= being eaten (#3925) --- .../0099_scrub_secrets_from_retrieval_logs.py | 104 ++++++++++++++++++ src/scribe/services/retrieval_telemetry.py | 5 +- tests/test_retrieval_query_scrubbing.py | 7 ++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/0099_scrub_secrets_from_retrieval_logs.py diff --git a/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py b/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py new file mode 100644 index 0000000..c643d21 --- /dev/null +++ b/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py @@ -0,0 +1,104 @@ +"""scrub credential-shaped spans out of retrieval_logs.query + +Revision ID: 0099 +Revises: 0098 +Create Date: 2026-09-11 + +`pre_tool_rule` retrieves against the RAW COMMAND TEXT and `write_path_rule` +against the code being written, so whatever was on the command line or in the +buffer is what `record_retrieval` wrote into `retrieval_logs.query`. A command +that exported a token stored the token (#3925). + +`services/retrieval_telemetry.scrub_secrets` closes that going forward — the +value never reaches the column. It cannot reach BACKWARDS, and this does: it +rewrites the rows already written. + +REDACTED IN PLACE, NOT DELETED. The rest of the row — score, threshold, +result count, duration, the near-miss record id — is legitimate evidence, and +it is what a threshold is tuned from. Deleting the row would throw that away +to remove a secret that lives in one column, so the column is what gets +rewritten. Rows with no credential in them are not touched at all. + +THE PATTERNS ARE INLINED RATHER THAN IMPORTED, deliberately, against the DRY +instinct. A migration is a frozen record of a change that already happened on +every install that ran it; importing the live patterns would mean this +migration quietly does something different next year than it did when it ran, +and two installs at the same revision would no longer be in the same state. +The Python twin in `services/retrieval_telemetry.py` is free to grow — this is +what ran here, once. The one thing that must not drift is coverage, and the +guard for that is `test_retrieval_query_scrubbing.py`, which tests the live +function rather than this copy. + +POSIX regex, not Python's. Postgres ARE supports the non-greedy `*?` the PEM +pattern needs, and `\\s`/`\\S`, so the shapes port directly. The `'gi'` flags +are global + case-insensitive, matching `re.sub` with `(?i)`. + +NO BARE `auth` IN THE ASSIGNED PATTERN. It matches `--author=`, so a commit +naming an address would have had the address redacted — evidence eaten for a +word that only looks credential-shaped. `AUTH_TOKEN` is still caught, by +`token`. + +Downgrade is a no-op, and honestly so: the original text is gone and a +migration cannot invent it back. Saying that plainly is better than a +downgrade that appears to restore something and does not. +""" +from alembic import op + +revision = "0099" +down_revision = "0098" +branch_labels = None +depends_on = None + +# Vendor-prefixed credentials — the prefix IS the tell, so no entropy guessing. +_TOKEN = ( + r"(fmcp_|flt_|ghp_|gho_|ghs_|ghu_|github_pat_|glpat-|xox[abprs]-" + r"|sk-[A-Za-z0-9]*-?|AKIA|ASIA)[A-Za-z0-9_\-]{12,}" +) +# A value handed to a secret-NAMED variable, in shell, env, YAML, JSON or a +# query string. The NAME identifies it, so the value can be anything. +_ASSIGNED = ( + r"([A-Za-z0-9_]*(token|secret|password|passwd|api[_-]?key|access[_-]?key)" + r"[A-Za-z0-9_]*)(\s*[:=]\s*[\"']?)([^\s\"'&]{8,})" +) +_AUTH_HEADER = r"(authorization\s*:\s*(bearer|basic|token)\s+)(\S+)" +_PEM = ( + r"-----BEGIN [A-Z ]*PRIVATE KEY-----(.|\n)*?-----END [A-Z ]*PRIVATE KEY-----" +) + + + +def _lit(pattern: str) -> str: + """A regex as a SQL string literal. + + A single quote inside a single-quoted SQL literal has to be DOUBLED, and + the assigned-value pattern contains two of them (it allows an optional + quote around the value). Left unescaped they close the literal early and + the migration dies on a syntax error — which is the whole reason this + helper exists rather than the patterns being pasted in inline. + """ + return pattern.replace("'", "''") + + +_SCRUB_SQL = f""" +UPDATE retrieval_logs +SET query = regexp_replace( + regexp_replace( + regexp_replace( + regexp_replace(query, '{_lit(_TOKEN)}', '[redacted:token]', 'gi'), + '{_lit(_ASSIGNED)}', '\\1\\3[redacted:assigned]', 'gi'), + '{_lit(_AUTH_HEADER)}', '\\1[redacted:auth-header]', 'gi'), + '{_lit(_PEM)}', '[redacted:private-key]', 'gi') +WHERE query IS NOT NULL + AND (query ~* '{_lit(_TOKEN)}' + OR query ~* '{_lit(_ASSIGNED)}' + OR query ~* '{_lit(_AUTH_HEADER)}' + OR query ~* '{_lit(_PEM)}') +""" + + +def upgrade() -> None: + op.execute(_SCRUB_SQL) + + +def downgrade() -> None: + """Deliberately empty — the original text no longer exists to restore.""" diff --git a/src/scribe/services/retrieval_telemetry.py b/src/scribe/services/retrieval_telemetry.py index d15c892..f942329 100644 --- a/src/scribe/services/retrieval_telemetry.py +++ b/src/scribe/services/retrieval_telemetry.py @@ -86,7 +86,10 @@ _SECRET_PATTERNS: tuple[tuple[str, "re.Pattern[str]"], ...] = ( # be anything. ("assigned", re.compile( r"(?i)\b([A-Za-z0-9_]*" - r"(?:token|secret|password|passwd|api[_-]?key|access[_-]?key|auth)" + # NO BARE "auth" HERE. It matched `--author=`, so a commit naming an + # address redacted the address — evidence eaten for a word that only + # LOOKS credential-shaped. `AUTH_TOKEN` is still caught, by `token`. + r"(?:token|secret|password|passwd|api[_-]?key|access[_-]?key)" r"[A-Za-z0-9_]*)" r"(\s*[:=]\s*[\"']?)" r"([^\s\"'&]{8,})" diff --git a/tests/test_retrieval_query_scrubbing.py b/tests/test_retrieval_query_scrubbing.py index 17c0807..c4a13ed 100644 --- a/tests/test_retrieval_query_scrubbing.py +++ b/tests/test_retrieval_query_scrubbing.py @@ -71,6 +71,13 @@ _EVIDENCE = [ # The word "token" in ordinary prose is not a token. "explain how the token bucket rate limiter works", "wc -l src/*.py && date", + # `--author=` contains "auth". A bare `auth` keyword in the assigned + # pattern redacted the address here, which is the evidence-eating failure + # this block exists to catch — and it shipped for one commit because the + # set did not contain a case with it. `AUTH_TOKEN=` is still caught, via + # `token`. + "git commit --author=bvandeusen@example.com -m 'x'", + "git log --author=\"Bryan Van Deusen\" --oneline", ] -- 2.54.0 From 0e10f6bb8a47fcd995caa9ec1156ea2b7eb6691f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 16:22:17 -0400 Subject: [PATCH 09/13] =?UTF-8?q?feat(rules)!:=20retire=20the=20always-on?= =?UTF-8?q?=20tier=20=E2=80=94=20every=20rule=20arrives=20by=20retrieval?= =?UTF-8?q?=20(#394)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Milestone 394, steps 5-8. Operator: "remove the always on rule functionality as the goal was to not have it at all since it didn't seem to work as expected." Unconditional preload had three failures the retrieval arms do not. It could not be MEASURED — a resident rule is in the context whether or not it mattered, so nothing distinguished "this governed the act" from "this was scenery", and it was the one surface structurally exempt from the scoreboard judging every other. It was SUMMARISED AWAY by compaction while the session went on believing it held the rules. And it CROWDED OUT the few rules that applied with the thirty that did not. WHAT GOES Schema (0100): rules.tier + ck_rules_tier, rule_versions.tier, rulebooks.always_on, and project_rulebook_exclusions — a table recording a project's opt-out of something that no longer binds it unasked. Tools: list_always_on_rules, exclude_always_on_rulebook, include_always_on_rulebook. Service: the same three plus rules_etag_for, _valid_tier and the whole etag family. The SessionStart preload and the write-path staleness arm go with them: nothing is resident, so nothing can have drifted since a session loaded it. THREE CALLS WORTH REVIEWING enter_project got NARROWER, not wider. Its filter was `always_on OR area-tagged`; dropping the tier arm leaves the deterministic half, so a project with no canonical-tagged Systems gets no bulk rules and reaches them by retrieval instead. Dropping the whole clause would have made that payload bigger than the preload this milestone deletes. Backups import tolerantly. A pre-394 archive carries tier, always_on and the retired inception choice; none is read, and the exclusion key is DROPPED rather than remapped, because restoring it would write data that validate_inception now rejects as unknown. The migration is irreversible in the way that matters and says so: downgrade recreates the columns at their defaults and cannot restore which rules were always-on. A value invented to fill a hole is not a measurement. THE INSTRUCTION SURFACES SAY THE HARDER THING Deleting "call list_always_on_rules()" is easy; replacing it is not, because the new model asks a session to trust something it cannot see. All three surfaces now say a session holds nothing, that rules arrive when work matches them, and — the half that got dangerous — that "no rule arrived" means "nothing matched", never "there is no rule". Under residency an empty session was rare and suspicious; it is now the ordinary state of most turns, so reading it as permission is wrong on nearly every turn rather than occasionally. That is #3720's defect at session scale. test_instruction_surfaces_agree is repointed rather than retired: its two halves collapsed into one instruction, and it gains a guard that every surface states what absence means. _INSTRUCTIONS is back at 1999/2000 — the inception clause paid for the longer HOW line. UI (rule 27, and the opportunity step 8 named) The tier selector is gone, and what replaces it is the point: `when_to_apply` is now the field that decides whether a rule is ever seen, so the editor marks it required, warns while it is empty, and both rule lists badge a trigger-less rule "never surfaces". A rule without one is not quiet, it is unreachable. TESTS Two files deleted outright — test_rules_etag.py and test_inception_rules.py tested subsystems that no longer exist. Elsewhere obsolete cases were removed and the rest repointed. One deserves naming: the wiring test asserted the act arms pass no `tier`, which had become an assertion that could not fail. It is repointed onto `kind`, which does still exist and where the same claim is live — a preference must reach a write exactly as a rule does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- alembic/versions/0100_drop_always_on_tier.py | 96 +++++++ frontend/src/api/inception.ts | 7 +- frontend/src/api/rulebooks.ts | 27 +- frontend/src/assets/rules-shared.css | 9 + frontend/src/components/InceptionCard.vue | 28 +- .../src/components/rules/ProjectRulesTab.vue | 48 +--- .../components/rules/RuleEditorSlideOver.vue | 63 ++--- .../src/components/rules/RuleHistoryPanel.vue | 3 +- .../src/components/rules/RuleListPane.vue | 11 +- .../src/components/rules/RuleSweepPane.vue | 19 +- .../components/rules/RulebookDetailPane.vue | 14 - .../src/components/rules/RulebookListPane.vue | 11 - frontend/src/stores/rulebooks.ts | 15 +- frontend/src/views/ProjectView.vue | 3 - plugin/hooks/scribe_session_context.sh | 26 +- src/scribe/models/rulebook.py | 17 +- src/scribe/services/backup.py | 43 +-- src/scribe/services/plugin_context.py | 12 - tests/helpers.py | 2 +- tests/test_inception.py | 19 +- tests/test_inception_rules.py | 69 ----- tests/test_instruction_surfaces_agree.py | 143 ++++++---- ...tegration_backup_rule_version_roundtrip.py | 3 - tests/test_integration_inception.py | 44 ++- tests/test_integration_rule_surfacing.py | 76 ++--- tests/test_integration_rule_verification.py | 9 +- tests/test_mcp_tool_projects.py | 8 +- tests/test_mcp_tool_rulebooks.py | 55 +--- tests/test_routes_rulebooks.py | 18 +- tests/test_rule_ledger_ageing.py | 5 +- tests/test_rule_usage_wiring.py | 23 +- tests/test_rules_etag.py | 267 ------------------ tests/test_services_backup.py | 8 +- tests/test_services_plugin_context.py | 99 ++----- tests/test_services_rulebooks.py | 27 +- 35 files changed, 388 insertions(+), 939 deletions(-) create mode 100644 alembic/versions/0100_drop_always_on_tier.py delete mode 100644 tests/test_inception_rules.py delete mode 100644 tests/test_rules_etag.py diff --git a/alembic/versions/0100_drop_always_on_tier.py b/alembic/versions/0100_drop_always_on_tier.py new file mode 100644 index 0000000..a524281 --- /dev/null +++ b/alembic/versions/0100_drop_always_on_tier.py @@ -0,0 +1,96 @@ +"""drop the always-on tier: rules.tier, rulebooks.always_on, the exclusions table + +Revision ID: 0100 +Revises: 0099 +Create Date: 2026-09-11 + +Milestone 394. Every rule now reaches a session by retrieval — because +something it is about to do made the rule relevant — and the machinery that +delivered rules unconditionally goes with it. + +WHAT GOES, AND WHERE IT CAME FROM + + - ``rules.tier`` and its ``ck_rules_tier`` CHECK (migration 0088). Dropping + the column takes the constraint with it. Rule 36 is about ADDING a value to + a live whitelist, which needs DROP + ADD in the same migration; it does not + speak to removing the column outright, and saying so here is cheaper than + the next reader wondering whether it was forgotten. + - ``rule_versions.tier`` (migration 0098). A version records what a rule + SAID; with no tier on a rule there is nothing for a snapshot to carry. + - ``rulebooks.always_on`` (migration 0058). A rulebook reaches a project by + subscription now, and by nothing else. + - ``project_rulebook_exclusions`` (migration 0085). It recorded a project's + opt-out of an always-on rulebook. Opting out of something that no longer + binds you is not a state that can exist — declining a rulebook is + expressed by not subscribing to it. + +IRREVERSIBLE, AND THE DOWNGRADE SAYS SO RATHER THAN PRETENDING + +The downgrade recreates the columns and the table with their DEFAULTS. It +cannot restore WHICH rules were always-on, which rulebooks bound every project, +or which projects had opted out — that information is in what this drops. + +That distinction is the one this repo keeps insisting on: a value invented to +fill a hole is not a measurement. So a downgraded database is structurally able +to run the old code and is NOT the database the old code was running against — +every rule comes back at the ``always_on`` default, which for the tier column +happens to mean "binding", the safe direction to be wrong in. + +Anyone who needs the real prior state restores a backup taken before this ran. +""" +import sqlalchemy as sa +from alembic import op + +revision = "0100" +down_revision = "0099" +branch_labels = None +depends_on = None + +_TIERS = ("always_on", "conditional") + + +def _in_list(column: str, values: tuple[str, ...]) -> str: + return f"{column} IN (" + ", ".join(f"'{v}'" for v in values) + ")" + + +def upgrade() -> None: + op.drop_table("project_rulebook_exclusions") + op.drop_column("rulebooks", "always_on") + op.drop_column("rule_versions", "tier") + # The CHECK goes with the column it constrains; naming it here would be a + # second drop of the same object. + op.drop_column("rules", "tier") + + +def downgrade() -> None: + """Structure only. See the module docstring — the values are gone.""" + op.add_column( + "rules", + sa.Column("tier", sa.Text(), nullable=False, server_default="always_on"), + ) + op.create_check_constraint("ck_rules_tier", "rules", _in_list("tier", _TIERS)) + op.add_column("rule_versions", sa.Column("tier", sa.Text(), nullable=True)) + op.add_column( + "rulebooks", + sa.Column( + "always_on", sa.Boolean(), nullable=False, + server_default=sa.text("false"), + ), + ) + op.create_table( + "project_rulebook_exclusions", + sa.Column( + "project_id", sa.BigInteger(), + sa.ForeignKey("projects.id", ondelete="CASCADE"), + primary_key=True, nullable=False, + ), + sa.Column( + "rulebook_id", sa.BigInteger(), + sa.ForeignKey("rulebooks.id", ondelete="CASCADE"), + primary_key=True, nullable=False, + ), + sa.Column( + "created_at", sa.DateTime(timezone=True), + server_default=sa.text("now()"), nullable=True, + ), + ) diff --git a/frontend/src/api/inception.ts b/frontend/src/api/inception.ts index 05773e1..0321c78 100644 --- a/frontend/src/api/inception.ts +++ b/frontend/src/api/inception.ts @@ -2,7 +2,6 @@ import { apiGet, apiPost } from "@/api/client"; export interface InceptionChoices { - exclude_always_on_rulebooks: number[]; subscribe_rulebooks: number[]; design_system_id: number | null; seed_systems: boolean; @@ -16,9 +15,7 @@ export interface InceptionRecord { } export interface InceptionDefaults { - always_on_rulebooks: { id: number; title: string }[]; - other_rulebooks: { id: number; title: string }[]; - excluded_always_on: { id: number; title: string }[]; + rulebooks: { id: number; title: string }[]; subscribed_rulebooks: { id: number; title: string }[]; design_system_id: number | null; design_systems: { id: number; title: string }[]; @@ -32,7 +29,7 @@ export interface InceptionDecision { } export const emptyChoices = (): InceptionChoices => ({ - exclude_always_on_rulebooks: [], subscribe_rulebooks: [], design_system_id: null, seed_systems: false, + subscribe_rulebooks: [], design_system_id: null, seed_systems: false, }); export const fetchInceptionDefaults = (projectId: number) => diff --git a/frontend/src/api/rulebooks.ts b/frontend/src/api/rulebooks.ts index 1a8c175..7b0fca1 100644 --- a/frontend/src/api/rulebooks.ts +++ b/frontend/src/api/rulebooks.ts @@ -3,7 +3,6 @@ import type { RecordUsage } from "@/types/usage"; import { apiGet, apiPost, apiPatch, apiDelete } from "@/api/client"; /** How a rule reaches a session (milestone 307). */ -export type RuleTier = "always_on" | "conditional"; /** * A typed edge between two rules. Each kind exists because its absence forced @@ -26,7 +25,6 @@ export interface Rulebook { owner_user_id: number; title: string; description: string; - always_on: boolean; created_at: string | null; updated_at: string | null; } @@ -49,12 +47,6 @@ export interface Rule { statement: string; /** WHEN this rule fires — the trigger, not the instruction. */ when_to_apply: string; - /** - * always_on preloads into every session; conditional is reachable and - * surfaced when its trigger fires. A rule with no tier set behaves as - * always_on, which is how every rule behaved before this existed. - */ - tier: RuleTier; why: string; how_to_apply: string; /** @@ -87,7 +79,6 @@ export interface RuleHeader { title: string; statement: string; topic_id: number | null; - tier: RuleTier; /** A date (YYYY-MM-DD), not a timestamp. */ updated_at: string | null; when_to_apply?: string; @@ -134,7 +125,6 @@ export interface ApplicableRules { truncated: boolean; subscribed_rulebooks: { id: number; title: string }[]; /** Always-on rulebooks this project opted out of at inception (milestone 297). */ - excluded_always_on: { id: number; title: string }[]; } // ── Rulebooks ─────────────────────────────────────────────────────── @@ -152,7 +142,7 @@ export async function createRulebook(data: { title: string; description?: string return apiPost("/api/rulebooks", data); } -export async function updateRulebook(id: number, data: Partial<{ title: string; description: string; always_on: boolean }>): Promise { +export async function updateRulebook(id: number, data: Partial<{ title: string; description: string }>): Promise { return apiPatch(`/api/rulebooks/${id}`, data); } @@ -207,7 +197,6 @@ export interface RuleWrite { title: string; statement: string; when_to_apply: string; - tier: RuleTier; why: string; how_to_apply: string; order_index: number; @@ -258,7 +247,6 @@ export interface RuleVersion { why?: string; how_to_apply?: string; when_to_apply?: string; - tier?: string; verify_with?: string; expires_when?: string; } @@ -323,16 +311,6 @@ export async function unsuppressTopicForProject(projectId: number, topicId: numb return apiDelete(`/api/projects/${projectId}/suppressions/topics/${topicId}`); } -// ── Always-on exclusions (milestone 297) ──────────────────────────────────── - -export async function excludeAlwaysOnRulebook(projectId: number, rulebookId: number): Promise { - await apiPost(`/api/projects/${projectId}/exclusions/rulebooks/${rulebookId}`, {}); -} - -export async function includeAlwaysOnRulebook(projectId: number, rulebookId: number): Promise { - await apiDelete(`/api/projects/${projectId}/exclusions/rulebooks/${rulebookId}`); -} - /** * One row of the staleness sweep. Unlike RuleHeader this carries the CHECK @@ -343,7 +321,6 @@ export interface RuleVerificationRow { id: number; title: string; statement: string; - tier: RuleTier; topic_id: number | null; project_id: number | null; when_to_apply: string; @@ -366,12 +343,10 @@ export interface RuleVerificationRow { */ export async function listRulesDueForVerification(opts: { olderThanDays?: number; - tier?: RuleTier; neverOnly?: boolean; } = {}): Promise<{ rules: RuleVerificationRow[]; total: number }> { const q = new URLSearchParams(); if (opts.olderThanDays) q.set("older_than_days", String(opts.olderThanDays)); - if (opts.tier) q.set("tier", opts.tier); if (opts.neverOnly) q.set("never_only", "true"); const qs = q.toString(); return apiGet(`/api/rules-due-for-verification${qs ? `?${qs}` : ""}`); diff --git a/frontend/src/assets/rules-shared.css b/frontend/src/assets/rules-shared.css index 72c1448..f2345f3 100644 --- a/frontend/src/assets/rules-shared.css +++ b/frontend/src/assets/rules-shared.css @@ -20,6 +20,15 @@ milestone (tier, then verification) and were byte-identical; a third would have drifted. The pane's italic serif title is inherited by anything inside it, so the chip resets family and style explicitly. */ +/* A rule with no trigger cannot be retrieved, and since milestone 394 + retrieval is the only delivery — so this marks a rule that will never + reach a session. Warning rather than error: the rule is not broken, it is + unreachable, and the fix is one field away. */ +.rule-chip-inert { + color: var(--fs-warning-fg); + background: color-mix(in srgb, var(--fs-warning) 12%, var(--fs-surface-raised)); +} + .rule-chip { margin-left: 0.4rem; font-family: var(--fs-font-body); diff --git a/frontend/src/components/InceptionCard.vue b/frontend/src/components/InceptionCard.vue index e33f842..f775f6f 100644 --- a/frontend/src/components/InceptionCard.vue +++ b/frontend/src/components/InceptionCard.vue @@ -28,7 +28,6 @@ const emit = defineEmits<{ }>(); const local = ref(props.choices ? { ...props.choices } : emptyChoices()); -const alwaysOn = ref<{ id: number; title: string }[]>([]); const others = ref<{ id: number; title: string }[]>([]); const designSystems = ref<{ id: number; title: string }[]>([]); const systemsCount = ref(0); @@ -47,21 +46,18 @@ async function load() { try { if (props.mode === "decide" && props.projectId) { const d: InceptionDefaults = await fetchInceptionDefaults(props.projectId); - alwaysOn.value = d.always_on_rulebooks; - others.value = d.other_rulebooks; + others.value = d.rulebooks; designSystems.value = d.design_systems; systemsCount.value = d.systems; // Start from what stands today so "record" without changes is a true inherit-all. local.value = { - exclude_always_on_rulebooks: d.excluded_always_on.map((r) => r.id), subscribe_rulebooks: d.subscribed_rulebooks.map((r) => r.id), design_system_id: d.design_system_id, seed_systems: false, }; } else { const [rulebooks, ds] = await Promise.all([listRulebooks(), fetchDesignSystems()]); - alwaysOn.value = rulebooks.filter((r) => r.always_on).map((r) => ({ id: r.id, title: r.title })); - others.value = rulebooks.filter((r) => !r.always_on).map((r) => ({ id: r.id, title: r.title })); + others.value = rulebooks.map((r) => ({ id: r.id, title: r.title })); designSystems.value = ds.design_systems.map((d) => ({ id: d.id, title: d.title })); } } catch (e: unknown) { @@ -71,13 +67,6 @@ async function load() { } } -function inherits(id: number): boolean { - return !local.value.exclude_always_on_rulebooks.includes(id); -} -function toggleInherit(id: number) { - const list = local.value.exclude_always_on_rulebooks; - local.value.exclude_always_on_rulebooks = list.includes(id) ? list.filter((x) => x !== id) : [...list, id]; -} function subscribed(id: number): boolean { return local.value.subscribe_rulebooks.includes(id); } @@ -87,7 +76,7 @@ function toggleSubscribe(id: number) { } const nothingToDecide = computed( - () => !alwaysOn.value.length && !others.value.length && !designSystems.value.length, + () => !others.value.length && !designSystems.value.length, ); async function record() { @@ -118,16 +107,11 @@ onMounted(load);

Loading…

{{ error }}