feat(rules): a preference updates without asking, and says what taught it (#3849 step 2)
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 1m7s
CI & Build / integration (push) Successful in 1m8s
CI & Build / Python tests (push) Successful in 1m37s
CI & Build / Build & push image (push) Successful in 33s
CI & Build / Python lint (push) Successful in 5s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 1m7s
CI & Build / integration (push) Successful in 1m8s
CI & Build / Python tests (push) Successful in 1m37s
CI & Build / Build & push image (push) Successful in 33s
The write path, and the step where a preference stops being a relabelled rule. `create_preference` / `update_preference` on the MCP surface, plus `kind` on update_rule and both HTTP doors. SEPARATE TOOLS, NOT A `kind=` ARGUMENT. create_rule's docstring IS the approval gate (#3557): propose, offer three answers, wait. That is right for a rule — the person it binds should have agreed. A preference inverts it, and one reached through create_rule would be read through that prose, so the caller would hesitate over exactly the act this kind exists to make routine. Two doors, two contracts, one table. Reads stay shared: a preference IS a rule row, and "what governs this" wants both. Two required fields, each buying something: - `when_to_apply`, because the trigger is two-thirds of the embedded document. Without one the record is written, stored, and silently never delivered — indistinguishable from one nobody wrote. - `arose_from_id`, the price of the ungated write. A corpus that drifts with no record of what taught each change cannot be audited, and the operator's veto over drift is worth exactly as much as their ability to read why it happened. The near-duplicate gate is what lets this corpus be written freely and stay small: the second preference about a thing updates the first. It is title-scoped and kind-blind, so it also catches a preference restating a rule that already binds. The asymmetry is guarded as two PRESENCE facts — the rule door still asks, the preference door still says write it — never as an absence. An absence check passes against a docstring that was deleted or rewritten into something else, which is snippet #3352's warning and would read as coverage here while proving nothing. `_plain_detail` moved to tests/helpers on its second copy, per that module's own reason for existing (#2825). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ
This commit is contained in:
@@ -581,11 +581,18 @@ 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,
|
||||
verify_with: str = "", expires_when: str = "",
|
||||
verify_with: str = "", expires_when: str = "", kind: str = "",
|
||||
clear_fields: list[str] | None = None,
|
||||
) -> dict:
|
||||
"""Update a rule. Empty strings / order_index=-1 leave fields unchanged.
|
||||
|
||||
`kind` here is how a rule BECOMES a preference, and it is a real change of
|
||||
force rather than a relabelling — so make it deliberately and say so. The
|
||||
rule keeps its id, its history and its typed edges, which is why this is a
|
||||
field rather than a new record: everything that cites it by number stays
|
||||
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).
|
||||
@@ -619,6 +626,8 @@ async def update_rule(
|
||||
fields["when_to_apply"] = when_to_apply
|
||||
if tier:
|
||||
fields["tier"] = tier
|
||||
if kind:
|
||||
fields["kind"] = kind
|
||||
if arose_from_id:
|
||||
fields["arose_from_id"] = arose_from_id
|
||||
if why:
|
||||
@@ -639,6 +648,174 @@ async def update_rule(
|
||||
return await rulebooks_svc.rule_detail(uid, rule, system_ids)
|
||||
|
||||
|
||||
# ── Preferences ─────────────────────────────────────────────────────────
|
||||
#
|
||||
# Separate tools rather than a `kind=` argument on create_rule, and the reason
|
||||
# is the docstring rather than the data. create_rule's docstring IS the
|
||||
# approval gate: it tells its caller to propose, offer three answers, and
|
||||
# wait. A preference reached through that door would be read through that
|
||||
# prose, and the caller would hesitate over exactly the act this kind exists
|
||||
# to make routine. Two doors, two contracts, one table.
|
||||
#
|
||||
# Reads stay shared — get_rule and list_rules return preferences as they are,
|
||||
# because a preference IS a rule row and a reader asking "what governs this"
|
||||
# wants both. Only the WRITE contracts differ.
|
||||
|
||||
|
||||
async def create_preference(
|
||||
topic_id: int, title: str, statement: str, when_to_apply: str,
|
||||
arose_from_id: int, why: str = "", how_to_apply: str = "",
|
||||
order_index: int = 0, force: bool = False,
|
||||
) -> dict:
|
||||
"""Record how the operator wants work done. No approval loop — write it.
|
||||
|
||||
A PREFERENCE IS NOT A RULE, and the axis is force rather than importance:
|
||||
|
||||
* a RULE is what must be FOLLOWED — ignoring it breaks something or
|
||||
crosses a boundary. It is the operator's decision, so create_rule
|
||||
proposes and waits for them.
|
||||
* a PREFERENCE is how they want it DONE — ignoring it costs consistency,
|
||||
not correctness. Noticing one and recording it is ordinary work.
|
||||
|
||||
If the answer to "what happens if someone doesn't do this" is "something
|
||||
breaks", you are holding a rule: propose it with create_rule instead.
|
||||
|
||||
WHY IT IS WORTH RECORDING AT ALL. A preference stated in one session dies
|
||||
with that session, and the next one re-derives it or asks again. The point
|
||||
is consistency: the tenth time you do something it goes the way the ninth
|
||||
did, without the operator having to say so a tenth time.
|
||||
|
||||
`when_to_apply` IS REQUIRED, and not as ceremony. A rule's trigger is
|
||||
two-thirds of its embedded document, so a preference without one is a
|
||||
record that will never surface at the moment it applies — written,
|
||||
findable by nobody, and silently useless. Name the moment in the words a
|
||||
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.
|
||||
|
||||
`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
|
||||
the task or note where this became clear.
|
||||
|
||||
WHAT A PREFERENCE NEVER DOES: change what gets RECORDED. It shapes how
|
||||
work is done — pacing, phrasing, which tool to reach for, how much to
|
||||
check first. A dev-log, an issue and a snippet read the same whoever
|
||||
produced them, because the record has to outlive the person and their
|
||||
preferences.
|
||||
|
||||
A near-duplicate BLOCKS and returns the existing id. That is the whole
|
||||
reason this corpus can stay small while being written freely: the second
|
||||
preference about a thing UPDATES the first rather than sitting beside it,
|
||||
and two preferences that quietly disagree are worse than none — retrieval
|
||||
surfaces whichever scores higher and nobody learns the other exists. The
|
||||
gate is title-based within the topic and does not care about kind, so it
|
||||
also catches a preference restating a rule that already binds.
|
||||
|
||||
Args:
|
||||
topic_id: The rulebook topic to file it under. A preference is
|
||||
user-scoped: it follows the operator across every project, which
|
||||
is what separates it from a project rule.
|
||||
title: What the preference is about. Half the embedded document —
|
||||
worth as much care as the statement.
|
||||
statement: How the operator wants it done, in their terms.
|
||||
when_to_apply: The moment it applies. Required; see above.
|
||||
arose_from_id: The task or note that taught this. Required; see above.
|
||||
force: Bypass the near-duplicate gate. For a genuinely distinct
|
||||
preference, not for one that is "mostly" different — a mostly
|
||||
different preference is an update.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
if not when_to_apply.strip():
|
||||
raise ValueError(
|
||||
"when_to_apply is required: a preference with no trigger never "
|
||||
"surfaces at the moment it applies. Name that moment in the words "
|
||||
"a session would be producing then."
|
||||
)
|
||||
if not arose_from_id:
|
||||
raise ValueError(
|
||||
"arose_from_id is required: preferences change as the work teaches "
|
||||
"them, and a change with no record of what taught it cannot be "
|
||||
"audited. Pass the task or note where this became clear."
|
||||
)
|
||||
if not force:
|
||||
dup = await dedup_svc.find_duplicate_rule(title, topic_id=topic_id)
|
||||
if dup is not None:
|
||||
return dedup_svc.duplicate_response(dup, "rule")
|
||||
rule = await rulebooks_svc.create_rule(
|
||||
topic_id=topic_id, user_id=uid,
|
||||
title=title, statement=statement, when_to_apply=when_to_apply,
|
||||
kind="preference", arose_from_id=arose_from_id,
|
||||
why=why, how_to_apply=how_to_apply, order_index=order_index,
|
||||
)
|
||||
return await rulebooks_svc.rule_detail(uid, rule, None)
|
||||
|
||||
|
||||
async def update_preference(
|
||||
rule_id: int, arose_from_id: int, statement: str = "",
|
||||
when_to_apply: str = "", title: str = "", why: str = "",
|
||||
how_to_apply: str = "", order_index: int = -1,
|
||||
system_ids: list[int] | None = None, clear_fields: list[str] | None = None,
|
||||
) -> dict:
|
||||
"""Bring a preference up to date. Doing this mid-work is expected.
|
||||
|
||||
THIS IS THE TOOL THAT MAKES A PREFERENCE DIFFERENT FROM A RULE. A rule
|
||||
waits for its author; a preference is kept current by whoever is working.
|
||||
When the operator corrects you, or you notice the preference on file no
|
||||
longer matches how they actually want this done, edit it — that is the
|
||||
feature, not a liberty being taken. A preference nothing ever updates has
|
||||
become a rule nobody enforces.
|
||||
|
||||
So: no proposal, no three answers, no waiting. Update it and say in the
|
||||
conversation that you did, so the operator can disagree while it is still
|
||||
in front of them.
|
||||
|
||||
`arose_from_id` IS REQUIRED, and it is the price of the ungated write.
|
||||
Every edit here is versioned, and the operator can read what changed and
|
||||
put it back — but a diff with no reason attached leaves them deciding
|
||||
whether to trust a change they cannot account for. Point at the task or
|
||||
note that taught it.
|
||||
|
||||
WHEN NOT TO EDIT. If what you learned is that something MUST be done a
|
||||
certain way — that skipping it breaks something or crosses a boundary —
|
||||
that is a rule, and rules are the operator's call: propose it with
|
||||
create_rule rather than hardening a preference in place. Softening in the
|
||||
other direction is equally an edit worth flagging out loud.
|
||||
|
||||
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.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
if not arose_from_id:
|
||||
raise ValueError(
|
||||
"arose_from_id is required: this edit is the record of how the "
|
||||
"operator's preference changed, and a change with no reason "
|
||||
"attached cannot be judged. Pass the task or note that taught it."
|
||||
)
|
||||
fields: dict = {"arose_from_id": arose_from_id}
|
||||
if title:
|
||||
fields["title"] = title
|
||||
if statement:
|
||||
fields["statement"] = statement
|
||||
if when_to_apply:
|
||||
fields["when_to_apply"] = when_to_apply
|
||||
if why:
|
||||
fields["why"] = why
|
||||
if how_to_apply:
|
||||
fields["how_to_apply"] = how_to_apply
|
||||
if order_index >= 0:
|
||||
fields["order_index"] = order_index
|
||||
rule = await rulebooks_svc.update_rule(
|
||||
rule_id, uid, clear=clear_fields or (), **fields,
|
||||
)
|
||||
if rule is None:
|
||||
raise ValueError(f"rule {rule_id} not found")
|
||||
return await rulebooks_svc.rule_detail(uid, rule, system_ids)
|
||||
|
||||
|
||||
async def rule_history(rule_id: int, version_id: int = 0) -> dict:
|
||||
"""What a rule USED TO SAY, newest change first.
|
||||
|
||||
@@ -985,6 +1162,7 @@ def register(mcp) -> None:
|
||||
list_topics, create_topic, update_topic, delete_topic,
|
||||
list_rules, list_always_on_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,
|
||||
|
||||
@@ -178,6 +178,11 @@ async def create_rule(topic_id: int):
|
||||
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
|
||||
# what the AGENT changed, not what they did themselves.
|
||||
kind=data.get("kind", "rule"),
|
||||
arose_from_id=data.get("arose_from_id", 0) or 0,
|
||||
verify_with=data.get("verify_with", ""),
|
||||
expires_when=data.get("expires_when", ""),
|
||||
@@ -212,7 +217,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", "arose_from_id",
|
||||
"when_to_apply", "tier", "kind", "arose_from_id",
|
||||
"verify_with", "expires_when")
|
||||
}
|
||||
# No clear_fields here: a form sends "" for an emptied input, and the
|
||||
@@ -436,6 +441,11 @@ async def create_project_rule(project_id: int):
|
||||
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
|
||||
# what the AGENT changed, not what they did themselves.
|
||||
kind=data.get("kind", "rule"),
|
||||
arose_from_id=data.get("arose_from_id", 0) or 0,
|
||||
verify_with=data.get("verify_with", ""),
|
||||
expires_when=data.get("expires_when", ""),
|
||||
|
||||
@@ -226,6 +226,12 @@ def fake_rule(**attrs) -> MagicMock:
|
||||
# `when_to_apply` and `arose_from_id` would be truthy MagicMocks and
|
||||
# rule_brief would attach both keys on every stand-in.
|
||||
"when_to_apply": None, "tier": "always_on", "arose_from_id": None,
|
||||
# Named for the same reason one line up, and it bites harder here.
|
||||
# `rule_brief` and `to_dict` both emit `kind or "rule"`, and a
|
||||
# MagicMock is truthy — so an unnamed `kind` would put a MagicMock
|
||||
# where every payload promises a force, and every stand-in rule would
|
||||
# read as neither a rule nor a preference.
|
||||
"kind": "rule",
|
||||
# Same reason, and the same trap one field further on: an unnamed
|
||||
# `verify_with` is a truthy MagicMock, so every stand-in rule would
|
||||
# claim to carry a check and rule_brief would stamp a MagicMock date
|
||||
@@ -235,6 +241,25 @@ def fake_rule(**attrs) -> MagicMock:
|
||||
}, attrs)
|
||||
|
||||
|
||||
def plain_rule_detail():
|
||||
"""Stub `rulebooks_svc.rule_detail` down to the record's own dict.
|
||||
|
||||
Every rule-tool unit test needs it and none of them wants it: the real
|
||||
`rule_detail` reads the rule's Systems and its typed edges from the
|
||||
database, which a unit test has none of. What these tests assert is that
|
||||
the TOOL forwarded the right arguments, so the seam is stubbed the same
|
||||
way the create/update calls themselves already are.
|
||||
|
||||
Consolidated here on its second copy, per this module's own reason for
|
||||
existing (#2825) — two stubs for one seam drift apart quietly, and a test
|
||||
stubbing the seam slightly differently is a test asserting something
|
||||
slightly different than it appears to.
|
||||
"""
|
||||
async def _detail(_uid, rule, _system_ids=None):
|
||||
return rule.to_dict()
|
||||
return patch("scribe.mcp.tools.rulebooks.rulebooks_svc.rule_detail", _detail)
|
||||
|
||||
|
||||
class FakeMCP:
|
||||
"""Stand-in for the FastMCP server a tool module's ``register(mcp)`` is
|
||||
handed: records the ``name=`` of every ``@mcp.tool(...)`` registration in
|
||||
|
||||
@@ -3,6 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from tests.helpers import FakeMCP, fake_rule, fake_rulebook, fake_topic
|
||||
from tests.helpers import plain_rule_detail as _plain_detail
|
||||
|
||||
|
||||
pytestmark = pytest.mark.usefixtures("_bind_user")
|
||||
@@ -48,18 +49,10 @@ async def test_get_rulebook_raises_when_not_found():
|
||||
await get_rulebook(rulebook_id=999)
|
||||
|
||||
|
||||
def _plain_detail():
|
||||
"""Stub the rule_detail seam these tool tests are not about.
|
||||
|
||||
create/update/get_rule now return through services.rulebooks.rule_detail,
|
||||
which reads the rule's areas and edges from the database. These are unit
|
||||
tests with no database, and what they assert is that the TOOL forwards the
|
||||
right arguments — so the seam is stubbed to the plain record, the same way
|
||||
they already stub the create/update calls themselves.
|
||||
"""
|
||||
async def _detail(_uid, rule, _system_ids=None):
|
||||
return rule.to_dict()
|
||||
return patch("scribe.mcp.tools.rulebooks.rulebooks_svc.rule_detail", _detail)
|
||||
# _plain_detail moved to tests/helpers on its second copy (#2825's own
|
||||
# reason for existing): two stubs for one seam drift apart quietly, and a
|
||||
# test stubbing it slightly differently asserts something slightly different
|
||||
# than it appears to.
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -230,11 +223,18 @@ def test_register_attaches_every_tool():
|
||||
|
||||
register(mcp)
|
||||
# 26 through milestone 307, +2 for the staleness sweep (milestone 312),
|
||||
# +1 for a rule's edit history (milestone 323).
|
||||
assert len(mcp.names) == 29
|
||||
# +1 for a rule's edit history (milestone 323), +2 for preferences
|
||||
# (milestone 399).
|
||||
assert len(mcp.names) == 31
|
||||
# spot-check a few names
|
||||
assert "list_rulebooks" in mcp.names
|
||||
assert "create_rule" in mcp.names
|
||||
# Preferences get their own WRITE door — create_rule's docstring is the
|
||||
# approval gate, and a preference reached through it would be read
|
||||
# through that prose. Reads stay shared deliberately, so there is no
|
||||
# get_preference to look for here.
|
||||
assert "create_preference" in mcp.names
|
||||
assert "update_preference" in mcp.names
|
||||
assert "subscribe_project_to_rulebook" in mcp.names
|
||||
assert "list_always_on_rules" in mcp.names
|
||||
# milestone 297: a project's opt-out of a whole always-on rulebook
|
||||
|
||||
@@ -0,0 +1,225 @@
|
||||
"""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=...)` 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."
|
||||
)
|
||||
Reference in New Issue
Block a user