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." + )