"""A rule cannot be created, or edited into, having no trigger (#4099). WHY THIS EXISTS `when_to_apply` is not metadata. `rule_document` embeds a rule as `{title} — {trigger}` / `When to apply: {trigger}\n\n{statement}`, with the trigger appearing TWICE so purpose dominates a short vector — the shape note 2485 measured. Drop the trigger and the document silently becomes title + statement: a DIFFERENT shape, ranked against a corpus it does not match, with nothing to report it. Every bar and every rank in the system assumes one shape. `create_preference` has always refused an empty trigger. `create_rule` and `create_project_rule` defaulted it to `""` — so the shape was enforced for the record kind that merely guides and optional for the kind that binds. WHAT THIS PINS The guard lives in the SERVICE, because both doors reach it: the MCP tools and the frontend's fast path in `routes/rulebooks.py`. A guard written in either one alone would leave the other able to create a rule that never fires, which is the failure mode this whole change exists to remove. These call the service directly and assert on the refusal, so an empty guard cannot make them pass by accident (rule 167). """ from __future__ import annotations import re import pytest from scribe.services import rulebooks as rulebooks_svc TRIGGER_RE = re.compile(r"when_to_apply", re.I) @pytest.mark.parametrize("blank", ["", " ", "\n\t "]) def test_a_rulebook_rule_needs_a_trigger(blank): """Whitespace is not a trigger — it embeds exactly like an empty one.""" with pytest.raises(ValueError, match=TRIGGER_RE): rulebooks_svc._require_trigger(blank) def test_a_real_trigger_passes(): """The guard has to let the normal case through, or the parametrised refusals above would pass for a guard that rejects everything.""" assert rulebooks_svc._require_trigger("before any git push") is None @pytest.mark.asyncio async def test_create_rule_refuses_before_it_touches_the_database(): """The guard runs ahead of the ownership check, so a missing trigger is reported as such rather than as whatever the session lookup says. A bogus topic_id proves no database was reached: were the guard absent, this would fail on the topic instead, with a different message.""" with pytest.raises(ValueError, match=TRIGGER_RE): await rulebooks_svc.create_rule( topic_id=999_999, user_id=1, title="t", statement="s", ) @pytest.mark.asyncio async def test_create_project_rule_refuses_too(): """The second door into the same defect. Named separately because the two functions are separate code paths that have drifted apart before.""" with pytest.raises(ValueError, match=TRIGGER_RE): await rulebooks_svc.create_project_rule( project_id=999_999, user_id=1, title="t", statement="s", ) def test_both_creators_actually_call_the_guard(): """Structural, and deliberately not satisfied by the behavioural tests above: those would still pass if a future edit inlined a second copy of the check. One definition is the point — a rule shape enforced in two places is a rule shape that will be enforced differently in two places.""" import inspect for fn in (rulebooks_svc.create_rule, rulebooks_svc.create_project_rule): src = inspect.getsource(fn) assert "_require_trigger(when_to_apply)" in src, ( f"{fn.__name__} no longer delegates to the shared guard" ) def test_update_cannot_empty_a_trigger_that_exists(): """The create guard is worth nothing if an edit can undo it, and both doors can try: `clear=['when_to_apply']` from the MCP side and an emptied form input from the REST side. Asserted on the source because the check sits mid-transaction, after the mutation and before the commit — reaching it needs a database. What must not silently vanish is the pairing: the before-value captured, and the raise that reads it. """ import inspect src = inspect.getsource(rulebooks_svc.update_rule) assert "trigger_before" in src, "the pre-edit trigger is no longer captured" assert re.search(r"if trigger_before and not", src), ( "update_rule no longer refuses to empty an existing trigger" ) def test_a_rule_that_never_had_one_is_still_editable(): """The deliberate asymmetry, and the reason the check asks 'did this edit REMOVE a trigger' rather than 'does one exist'. A rule predating the guard has no trigger. Refusing to save it would freeze exactly the records that most need fixing — the unreachable ones — so the guard must not fire when there was nothing to remove. """ import inspect src = inspect.getsource(rulebooks_svc.update_rule) assert "trigger_before and not" in src, ( "the guard no longer keys on the BEFORE value, so a legacy rule with " "no trigger can no longer be edited at all" )