CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 43s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m33s
CI & Build / Build & push image (push) Successful in 21s
CI run 6937 red — 3 collection errors, `positional argument follows keyword argument`, in the three integration fixtures that call the service positionally (`create_rule(topic.id, uid, "title", "statement")`). The script that added `when_to_apply=` to 15 fixtures inserted it as the FIRST argument, which is valid only where every other argument is already a keyword. Moved to the last argument in every call, which is legal in both styles, and the continuation indent now matches the surrounding arguments. Also repairs self-inflicted damage: the same script added a trigger to the two tests in test_rule_trigger_required.py whose whole purpose is to call the creators WITHOUT one. They would have stopped raising and the guard's own proof would have inverted — a test that passes for the opposite reason than the one it names, which is worse than a failing one. Caught by ast.parse across tests/ rather than by the next CI round trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
226 lines
9.7 KiB
Python
226 lines
9.7 KiB
Python
"""The preference write path, and how it differs from a rule's (milestone 399).
|
|
|
|
WHY TWO DOORS AT ALL
|
|
|
|
`create_rule`'s docstring IS the approval gate (#3557): it tells its caller to
|
|
propose, offer three answers, and wait. That is right for a rule — the person
|
|
a rule binds should have agreed to be bound.
|
|
|
|
A preference inverts it. The operator's framing: *"preferences are rules that
|
|
scribe can and should update during use."* A preference that asks every time
|
|
never drifts, and drifting is the whole feature. Reaching one through
|
|
`create_rule(kind=..., when_to_apply="when the moment this fixture stands in for arises")` would mean reading it through the gate's prose, and
|
|
the caller would hesitate over exactly the act this kind exists to make
|
|
routine.
|
|
|
|
So the asymmetry is the product, and these tests pin it.
|
|
|
|
TWO PRESENCE CHECKS, NEVER AN ABSENCE
|
|
|
|
The tempting guard is "create_preference's docstring does NOT run the approval
|
|
loop". That is the shape snippet #3352 warns against: an absence check passes
|
|
against a docstring that has been deleted, emptied, or rewritten into
|
|
something else entirely, and it reads as coverage while proving nothing.
|
|
|
|
So the asymmetry is asserted as two PRESENCE facts — the rule door still asks,
|
|
the preference door still says write it — and each fails if its own side is
|
|
tidied away. Synonym families, structure not wording, the same bargain
|
|
test_rule_creation_asks_first strikes.
|
|
"""
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
|
|
from tests.helpers import fake_rule, plain_rule_detail as _plain_detail
|
|
from tests.helpers import tool_doc as _doc
|
|
|
|
# The tool layer reads its caller from a ContextVar the HTTP transport sets.
|
|
# With no request in flight, the module binds it itself (snippet #2836).
|
|
pytestmark = pytest.mark.usefixtures("_bind_user")
|
|
|
|
MODULE = "scribe.mcp.tools.rulebooks"
|
|
|
|
|
|
# ── the required fields, and why each is required ───────────────────────
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_preference_without_a_trigger_is_refused():
|
|
"""A preference with no `when_to_apply` is inert, not merely incomplete.
|
|
|
|
The trigger is two-thirds of the embedded document, so a record without
|
|
one never surfaces at the moment it applies. Refusing at the tool is the
|
|
difference between an error the writer can fix and a preference that is
|
|
written, stored, and silently never delivered — which looks identical to
|
|
one nobody wrote.
|
|
"""
|
|
create_mock = AsyncMock()
|
|
with patch(f"{MODULE}.rulebooks_svc.create_rule", create_mock):
|
|
from scribe.mcp.tools.rulebooks import create_preference
|
|
with pytest.raises(ValueError, match="when_to_apply is required"):
|
|
await create_preference(
|
|
topic_id=10, title="t", statement="s",
|
|
when_to_apply=" ", arose_from_id=42,
|
|
)
|
|
create_mock.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_preference_without_provenance_is_refused():
|
|
"""Provenance is the price of the ungated write.
|
|
|
|
A preference is expected to change as the work teaches it. A corpus that
|
|
drifts with no record of what taught each change is one nobody can audit —
|
|
and the operator's veto over drift depends entirely on being able to read
|
|
why it happened.
|
|
"""
|
|
create_mock = AsyncMock()
|
|
with patch(f"{MODULE}.rulebooks_svc.create_rule", create_mock):
|
|
from scribe.mcp.tools.rulebooks import create_preference
|
|
with pytest.raises(ValueError, match="arose_from_id is required"):
|
|
await create_preference(
|
|
topic_id=10, title="t", statement="s",
|
|
when_to_apply="when x", arose_from_id=0,
|
|
)
|
|
create_mock.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_preference_stores_the_preference_kind():
|
|
"""The tool's one irreducible job.
|
|
|
|
Asserted on the kwarg reaching the service rather than on the returned
|
|
payload: a tool that accepted the call and wrote a plain rule would
|
|
return something that reads correctly, and the force would be wrong.
|
|
"""
|
|
rule = fake_rule(id=100, kind="preference")
|
|
create_mock = AsyncMock(return_value=rule)
|
|
with patch(f"{MODULE}.rulebooks_svc.create_rule", create_mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_preference
|
|
await create_preference(
|
|
topic_id=10, title="Pace hard debugging",
|
|
statement="One step per turn.",
|
|
when_to_apply="during hard debugging",
|
|
arose_from_id=42,
|
|
)
|
|
kwargs = create_mock.call_args.kwargs
|
|
assert kwargs["kind"] == "preference"
|
|
assert kwargs["arose_from_id"] == 42
|
|
assert kwargs["when_to_apply"] == "during hard debugging"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_near_duplicate_preference_blocks():
|
|
"""The gate is what lets this corpus be written freely and stay small.
|
|
|
|
The second preference about a thing must UPDATE the first. Two that
|
|
quietly disagree are worse than none: retrieval surfaces whichever scores
|
|
higher, and nobody learns the other exists.
|
|
"""
|
|
from scribe.services.dedup import DuplicateMatch
|
|
dup = DuplicateMatch(id=47, title="Pace hard debugging", similarity=1.0, reason="title")
|
|
create_mock = AsyncMock()
|
|
with patch(f"{MODULE}.dedup_svc.find_duplicate_rule", AsyncMock(return_value=dup)), \
|
|
patch(f"{MODULE}.rulebooks_svc.create_rule", create_mock):
|
|
from scribe.mcp.tools.rulebooks import create_preference
|
|
out = await create_preference(
|
|
topic_id=10, title="Pace hard debugging", statement="s",
|
|
when_to_apply="when", arose_from_id=42,
|
|
)
|
|
assert out["duplicate"] is True
|
|
assert out["existing_id"] == 47
|
|
create_mock.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_updating_a_preference_without_provenance_is_refused():
|
|
update_mock = AsyncMock()
|
|
with patch(f"{MODULE}.rulebooks_svc.update_rule", update_mock):
|
|
from scribe.mcp.tools.rulebooks import update_preference
|
|
with pytest.raises(ValueError, match="arose_from_id is required"):
|
|
await update_preference(rule_id=5, arose_from_id=0, statement="new")
|
|
update_mock.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_update_preference_forwards_what_taught_the_change():
|
|
rule = fake_rule(id=5, kind="preference")
|
|
update_mock = AsyncMock(return_value=rule)
|
|
with patch(f"{MODULE}.rulebooks_svc.update_rule", update_mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import update_preference
|
|
await update_preference(
|
|
rule_id=5, arose_from_id=99, statement="the new way",
|
|
)
|
|
kwargs = update_mock.call_args.kwargs
|
|
assert kwargs["arose_from_id"] == 99
|
|
assert kwargs["statement"] == "the new way"
|
|
|
|
|
|
# ── the asymmetry, as two presence facts ────────────────────────────────
|
|
|
|
|
|
def test_the_rule_door_still_asks_before_writing():
|
|
"""Half one of the asymmetry. If this fails, the gate was tidied away and
|
|
preferences are no longer the exception — they are just the same thing.
|
|
"""
|
|
doc = _doc(MODULE, "create_rule").lower()
|
|
asks = ("approve", "propose", "ask", "question")
|
|
assert any(w in doc for w in asks), (
|
|
"create_rule's docstring no longer runs the propose-then-approve loop. "
|
|
"The preference path's whole justification is that it is the exception "
|
|
"to this; with the gate gone there is no asymmetry left to justify."
|
|
)
|
|
|
|
|
|
def test_the_preference_door_says_to_write_it():
|
|
"""Half two. The inverting instruction has to be PRESENT, not merely
|
|
unaccompanied by a gate.
|
|
|
|
An agent that has internalised #3557 will hesitate to write or rewrite a
|
|
preference unless told plainly that this door is different. Silence here
|
|
does not read as permission — it reads as an omission, and the caller
|
|
falls back on the behaviour it already knows.
|
|
"""
|
|
for tool in ("create_preference", "update_preference"):
|
|
doc = _doc(MODULE, tool).lower()
|
|
permits = ("expected", "no approval", "without asking", "ordinary work",
|
|
"write it", "not a liberty", "no proposal")
|
|
assert any(w in doc for w in permits), (
|
|
f"{tool}'s docstring no longer tells its caller that writing "
|
|
"without an approval loop is expected. A caller carrying "
|
|
"create_rule's gate will default to asking, and a preference "
|
|
"nothing ever updates is a rule nobody enforces."
|
|
)
|
|
|
|
|
|
def test_the_preference_door_names_the_force_distinction():
|
|
"""The routing test, stated positively (rule 165).
|
|
|
|
The confusion this milestone exists to fix is that a session cannot tell
|
|
which kind it is holding. If the docstring stops drawing the line, the
|
|
tool becomes a second way to write rules.
|
|
"""
|
|
doc = _doc(MODULE, "create_preference").lower()
|
|
assert "rule" in doc and any(
|
|
w in doc for w in ("followed", "binds", "breaks", "consistency")
|
|
), (
|
|
"create_preference's docstring no longer distinguishes a preference "
|
|
"from a rule by force. Without that line the tool is a second door "
|
|
"onto the rulebook with a weaker gate."
|
|
)
|
|
|
|
|
|
def test_the_preference_door_keeps_the_record_out_of_scope():
|
|
"""Preferences shape HOW work is done, never WHAT is recorded.
|
|
|
|
Worth pinning because "record it the way I like it" is the natural next
|
|
reach, and it would make dev-logs and issues idiosyncratic per author —
|
|
while the record is the one thing that has to outlive the person.
|
|
"""
|
|
doc = _doc(MODULE, "create_preference").lower()
|
|
assert "record" in doc, (
|
|
"create_preference's docstring no longer says that a preference does "
|
|
"not change what gets recorded. That boundary is the one a reader "
|
|
"would cross without noticing."
|
|
)
|