Preferences — a rule kind that Scribe keeps up to date (#3849 steps 1–3) #150
@@ -0,0 +1,90 @@
|
|||||||
|
"""rules gain a `kind` — a preference is a rule that does not bind (#3849)
|
||||||
|
|
||||||
|
Revision ID: 0098
|
||||||
|
Revises: 0097
|
||||||
|
Create Date: 2026-09-10
|
||||||
|
|
||||||
|
A rule says what must be followed. There was no way to record the other
|
||||||
|
thing the operator kept writing rules for: **how they want work done**.
|
||||||
|
Several rules in a mature rulebook are not really rules — pace this kind of
|
||||||
|
debugging, hand off an action with its reason, end a finding with an offer.
|
||||||
|
Ignoring one of those does not break anything or cross a boundary; it costs
|
||||||
|
consistency. They were written as rules because a rule was the only record
|
||||||
|
that is global, keyed to a situation, and delivered when that situation
|
||||||
|
arrives.
|
||||||
|
|
||||||
|
So: `kind`. `rule` binds. `preference` describes how this person wants it
|
||||||
|
done, and — the part that makes it its own kind rather than a softer label —
|
||||||
|
**it is expected to change as the work teaches it.** The agent updates a
|
||||||
|
preference in the ordinary course of working, where a rule waits for its
|
||||||
|
author.
|
||||||
|
|
||||||
|
WHY A COLUMN AND NOT A TABLE. Everything a preference needs already exists on
|
||||||
|
`rules` and nowhere else: `when_to_apply` as a real column, a
|
||||||
|
trigger-dominated embedding document, ownership-scoped search that is
|
||||||
|
deliberately not filtered to one project, three retrieval arms with per-arm
|
||||||
|
telemetry, typed relations, and versioning. The two differ in exactly one
|
||||||
|
dimension — force — and one dimension is a field.
|
||||||
|
|
||||||
|
The drift machinery is the decisive part. `rule_versions` already snapshots
|
||||||
|
every write, and `rule_relations.overrides` already models "this supersedes
|
||||||
|
that for its scope". A separate table would have rebuilt both, and moving
|
||||||
|
existing rows into it would have changed their ids — silently invalidating
|
||||||
|
every record in the corpus that cites a rule by number.
|
||||||
|
|
||||||
|
DEFAULTS TO `rule`, so this migration changes NOTHING about what binds. An
|
||||||
|
install upgrades and every existing row keeps the force it had. That is the
|
||||||
|
same reasoning 0088 used for `tier`, and it is the reason both are safe to
|
||||||
|
apply without reading the data first.
|
||||||
|
|
||||||
|
The CHECK is created with the column (rule 36: there is no prior constraint,
|
||||||
|
so the pair is created together — a value added to it LATER does DROP + ADD
|
||||||
|
in one migration).
|
||||||
|
|
||||||
|
`rule_versions` GETS THE COLUMN TOO, and that half is not bookkeeping.
|
||||||
|
`record_if_changed` decides whether an edit is worth a snapshot by comparing
|
||||||
|
the fields a version carries; a field a version does not carry is a field
|
||||||
|
whose change records NO HISTORY AT ALL. Without this, turning a rule into a
|
||||||
|
preference — the single most consequential edit either kind can undergo,
|
||||||
|
because it is the moment something stops binding — would leave the history
|
||||||
|
silent. Nullable and no CHECK there, matching `tier`: a version is a record
|
||||||
|
of what was, and a constraint on it would refuse to store a kind later
|
||||||
|
dropped from the live whitelist.
|
||||||
|
|
||||||
|
Downgrade drops all three. Nothing reads `kind` for correctness; a rule that
|
||||||
|
was a preference simply becomes a rule again, which is the safe direction.
|
||||||
|
"""
|
||||||
|
import sqlalchemy as sa
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
|
||||||
|
revision = "0098"
|
||||||
|
down_revision = "0097"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
# Kept in one place so upgrade and the CHECK agree by construction — 0088's
|
||||||
|
# idiom, for the same reason.
|
||||||
|
_KINDS = ("rule", "preference")
|
||||||
|
|
||||||
|
|
||||||
|
def _in_list(column: str, values: tuple[str, ...]) -> str:
|
||||||
|
return f"{column} IN (" + ", ".join(f"'{v}'" for v in values) + ")"
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column(
|
||||||
|
"rules",
|
||||||
|
sa.Column("kind", sa.Text(), nullable=False, server_default="rule"),
|
||||||
|
)
|
||||||
|
op.create_check_constraint("ck_rules_kind", "rules", _in_list("kind", _KINDS))
|
||||||
|
# Nullable, no CHECK — see the module docstring. A version written before
|
||||||
|
# this migration genuinely does not know, and NULL there means "not
|
||||||
|
# recorded", never "was a rule".
|
||||||
|
op.add_column("rule_versions", sa.Column("kind", sa.Text(), nullable=True))
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_column("rule_versions", "kind")
|
||||||
|
op.drop_constraint("ck_rules_kind", "rules", type_="check")
|
||||||
|
op.drop_column("rules", "kind")
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "scribe",
|
"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 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.",
|
||||||
"version": "2026.09.10.0221",
|
"version": "2026.09.11.0319",
|
||||||
"author": {
|
"author": {
|
||||||
"name": "Bryan Van Deusen"
|
"name": "Bryan Van Deusen"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ of record (notes, tasks, projects, milestones, rules) reachable through the
|
|||||||
for the operator's work, and as your own working memory across sessions.
|
for the operator's work, and as your own working memory across sessions.
|
||||||
|
|
||||||
**At the start of this session:**
|
**At the start of this session:**
|
||||||
- Call `list_always_on_rules()` to load the operator's binding rules.
|
- Call `list_always_on_rules()` to load the operator's standing rules.
|
||||||
- If the working repo maps to a Scribe project (check `list_repo_bindings`),
|
- If the working repo maps to a Scribe project (check `list_repo_bindings`),
|
||||||
call `enter_project(<id>)` to load that project's rules, open tasks, and
|
call `enter_project(<id>)` to load that project's rules, open tasks, and
|
||||||
recent notes in one shot.
|
recent notes in one shot.
|
||||||
@@ -21,6 +21,12 @@ for the operator's work, and as your own working memory across sessions.
|
|||||||
compaction — call `list_always_on_rules()` (and `enter_project()` when 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
|
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.
|
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
|
- **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
|
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
|
binds exactly as hard. So before a consequential act, `search` for a rule
|
||||||
|
|||||||
@@ -58,9 +58,27 @@ Two constraints on *how* that's achieved:
|
|||||||
|
|
||||||
2. **Standing rules are binding — and the ones you were handed are not all of
|
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
|
them.** Load the resident set via `list_always_on_rules()` at session start
|
||||||
(see "Do this first"); treat every one as binding. Pull a rule's full
|
(see "Do this first"). Pull a record's full statement with `get_rule(id)`
|
||||||
statement with `get_rule(id)` when it's about to bite. When a project is in
|
when it's about to bite. When a project is in scope, `enter_project(id)`
|
||||||
scope, `enter_project(id)` also returns its applicable rules.
|
also returns its applicable rules.
|
||||||
|
|
||||||
|
**`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
|
||||||
|
crosses a boundary. A **preference** records how the operator wants work
|
||||||
|
done, and ignoring it costs consistency rather than correctness. Both are
|
||||||
|
worth following and both arrive the same way; only one is a mistake to
|
||||||
|
miss. An injected line names which in its opening words — *"Standing rule
|
||||||
|
that may apply…"* against *"Preference that may apply…"* — and every
|
||||||
|
payload carries `kind` outright.
|
||||||
|
|
||||||
|
**A preference is the one record you keep current yourself.** When the
|
||||||
|
operator corrects you, or the preference on file no longer matches how they
|
||||||
|
actually want something done, `update_preference` — that is expected, not a
|
||||||
|
liberty, and it wants the task or note that taught the change. Say in the
|
||||||
|
same turn that you did it, so they can disagree while it is in front of
|
||||||
|
them. A rule waits for the operator instead: `create_rule` proposes and
|
||||||
|
asks. If what you learned is that something MUST be done a certain way,
|
||||||
|
that is a rule to propose, not a preference to harden in place.
|
||||||
|
|
||||||
Rules come in two tiers. **Always-on** rules are delivered — they arrive
|
Rules come in two tiers. **Always-on** rules are delivered — they arrive
|
||||||
whether or not you ask. **Conditional** rules are RETRIEVED, and one binds
|
whether or not you ask. **Conditional** rules are RETRIEVED, and one binds
|
||||||
|
|||||||
@@ -88,8 +88,9 @@ Hierarchy: Project -> Milestone -> Task/Note. The map, by purpose:
|
|||||||
active project_id to stay in scope.
|
active project_id to stay in scope.
|
||||||
- WHERE work happens: Systems. Tag records with system_ids as you write;
|
- WHERE work happens: Systems. Tag records with system_ids as you write;
|
||||||
create_system when the area is unmodelled.
|
create_system when the area is unmodelled.
|
||||||
- HOW: rules bind. list_always_on_rules() at start; before a consequential
|
- HOW: rules bind; preferences guide. list_always_on_rules() at start;
|
||||||
act, search(content_type="rule") — the resident set is not all of them.
|
before a consequential act, search(content_type="rule") — the resident
|
||||||
|
set is not all of them.
|
||||||
- UI: the project's design system is binding — resolve_design_system /
|
- UI: the project's design system is binding — resolve_design_system /
|
||||||
get_design_system_stylesheet before hand-writing a value.
|
get_design_system_stylesheet before hand-writing a value.
|
||||||
- REUSE: search snippets before writing a helper; record what you build with
|
- REUSE: search snippets before writing a helper; record what you build with
|
||||||
|
|||||||
@@ -581,11 +581,18 @@ async def update_rule(
|
|||||||
rule_id: int, title: str = "", statement: str = "", when_to_apply: str = "",
|
rule_id: int, title: str = "", statement: str = "", when_to_apply: str = "",
|
||||||
why: str = "", how_to_apply: str = "", order_index: int = -1,
|
why: str = "", how_to_apply: str = "", order_index: int = -1,
|
||||||
tier: str = "", system_ids: list[int] | None = None, arose_from_id: int = 0,
|
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,
|
clear_fields: list[str] | None = None,
|
||||||
) -> dict:
|
) -> dict:
|
||||||
"""Update a rule. Empty strings / order_index=-1 leave fields unchanged.
|
"""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
|
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
|
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).
|
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
|
fields["when_to_apply"] = when_to_apply
|
||||||
if tier:
|
if tier:
|
||||||
fields["tier"] = tier
|
fields["tier"] = tier
|
||||||
|
if kind:
|
||||||
|
fields["kind"] = kind
|
||||||
if arose_from_id:
|
if arose_from_id:
|
||||||
fields["arose_from_id"] = arose_from_id
|
fields["arose_from_id"] = arose_from_id
|
||||||
if why:
|
if why:
|
||||||
@@ -639,6 +648,174 @@ async def update_rule(
|
|||||||
return await rulebooks_svc.rule_detail(uid, rule, system_ids)
|
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:
|
async def rule_history(rule_id: int, version_id: int = 0) -> dict:
|
||||||
"""What a rule USED TO SAY, newest change first.
|
"""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_topics, create_topic, update_topic, delete_topic,
|
||||||
list_rules, list_always_on_rules, get_rule,
|
list_rules, list_always_on_rules, get_rule,
|
||||||
create_rule, create_project_rule, update_rule, delete_rule,
|
create_rule, create_project_rule, update_rule, delete_rule,
|
||||||
|
create_preference, update_preference,
|
||||||
relate_rules, unrelate_rules,
|
relate_rules, unrelate_rules,
|
||||||
subscribe_project_to_rulebook, unsubscribe_project_from_rulebook,
|
subscribe_project_to_rulebook, unsubscribe_project_from_rulebook,
|
||||||
suppress_rule_for_project, unsuppress_rule_for_project,
|
suppress_rule_for_project, unsuppress_rule_for_project,
|
||||||
|
|||||||
@@ -59,6 +59,12 @@ class RuleVersion(Base, CreatedAtMixin):
|
|||||||
how_to_apply: 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)
|
when_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
tier: 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.
|
||||||
|
# NULL means a version older than migration 0098 — not recorded, never
|
||||||
|
# "was a rule".
|
||||||
|
kind: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
verify_with: Mapped[str | None] = mapped_column(Text, nullable=True)
|
verify_with: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
expires_when: Mapped[str | None] = mapped_column(Text, nullable=True)
|
expires_when: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
|
|
||||||
@@ -85,6 +91,7 @@ class RuleVersion(Base, CreatedAtMixin):
|
|||||||
"how_to_apply": self.how_to_apply or "",
|
"how_to_apply": self.how_to_apply or "",
|
||||||
"when_to_apply": self.when_to_apply or "",
|
"when_to_apply": self.when_to_apply or "",
|
||||||
"tier": self.tier or "",
|
"tier": self.tier or "",
|
||||||
|
"kind": self.kind or "",
|
||||||
"verify_with": self.verify_with or "",
|
"verify_with": self.verify_with or "",
|
||||||
"expires_when": self.expires_when or "",
|
"expires_when": self.expires_when or "",
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -105,6 +105,24 @@ class Rule(Base, TimestampMixin, SoftDeleteMixin):
|
|||||||
# default preserves existing behaviour exactly: nothing stops binding
|
# default preserves existing behaviour exactly: nothing stops binding
|
||||||
# because of an upgrade. CHECK ck_rules_tier (migration 0088, rule 36).
|
# 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")
|
tier: Mapped[str] = mapped_column(Text, default="always_on", server_default="always_on")
|
||||||
|
# WHAT KIND of instruction this is — force, where `tier` is delivery.
|
||||||
|
# `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.
|
||||||
|
#
|
||||||
|
# The second half is what makes it a kind rather than a softer label — a
|
||||||
|
# preference is expected to CHANGE as the work teaches it, and the agent
|
||||||
|
# updates it in the ordinary course of working, where a rule waits for its
|
||||||
|
# author. So one column decides two behaviours: whether create's approval
|
||||||
|
# gate fires, and which voice the injected line speaks in.
|
||||||
|
#
|
||||||
|
# Lives here and not in its own table because a preference needs exactly
|
||||||
|
# what a rule has and a note does not — a trigger column, a
|
||||||
|
# trigger-dominated document, ownership-scoped search, the retrieval arms,
|
||||||
|
# relations, and `rule_versions`, which is where its drift is recorded.
|
||||||
|
# Defaults to `rule` so nothing changes force on upgrade.
|
||||||
|
# CHECK ck_rules_kind (migration 0098, rule 36).
|
||||||
|
kind: Mapped[str] = mapped_column(Text, default="rule", server_default="rule")
|
||||||
why: Mapped[str | None] = mapped_column(Text, nullable=True)
|
why: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
how_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True)
|
how_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||||
# The three fields that tell a CONSTRAINT apart from a NORM (milestone
|
# The three fields that tell a CONSTRAINT apart from a NORM (milestone
|
||||||
@@ -144,6 +162,13 @@ class Rule(Base, TimestampMixin, SoftDeleteMixin):
|
|||||||
"statement": self.statement,
|
"statement": self.statement,
|
||||||
"when_to_apply": self.when_to_apply or "",
|
"when_to_apply": self.when_to_apply or "",
|
||||||
"tier": self.tier,
|
"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
|
||||||
|
# the same payload, and that equivalence is the defect shape this
|
||||||
|
# codebase keeps re-encountering. Twenty bytes buys an answer that
|
||||||
|
# cannot be misread.
|
||||||
|
"kind": self.kind or "rule",
|
||||||
"why": self.why or "",
|
"why": self.why or "",
|
||||||
"how_to_apply": self.how_to_apply or "",
|
"how_to_apply": self.how_to_apply or "",
|
||||||
"verify_with": self.verify_with or "",
|
"verify_with": self.verify_with or "",
|
||||||
|
|||||||
@@ -178,6 +178,11 @@ async def create_rule(topic_id: int):
|
|||||||
order_index=data.get("order_index", 0),
|
order_index=data.get("order_index", 0),
|
||||||
when_to_apply=data.get("when_to_apply", ""),
|
when_to_apply=data.get("when_to_apply", ""),
|
||||||
tier=data.get("tier", "always_on"),
|
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,
|
arose_from_id=data.get("arose_from_id", 0) or 0,
|
||||||
verify_with=data.get("verify_with", ""),
|
verify_with=data.get("verify_with", ""),
|
||||||
expires_when=data.get("expires_when", ""),
|
expires_when=data.get("expires_when", ""),
|
||||||
@@ -212,7 +217,7 @@ async def update_rule(rule_id: int):
|
|||||||
fields = {
|
fields = {
|
||||||
k: v for k, v in data.items()
|
k: v for k, v in data.items()
|
||||||
if k in ("title", "statement", "why", "how_to_apply", "order_index",
|
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")
|
"verify_with", "expires_when")
|
||||||
}
|
}
|
||||||
# No clear_fields here: a form sends "" for an emptied input, and the
|
# 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),
|
order_index=data.get("order_index", 0),
|
||||||
when_to_apply=data.get("when_to_apply", ""),
|
when_to_apply=data.get("when_to_apply", ""),
|
||||||
tier=data.get("tier", "always_on"),
|
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,
|
arose_from_id=data.get("arose_from_id", 0) or 0,
|
||||||
verify_with=data.get("verify_with", ""),
|
verify_with=data.get("verify_with", ""),
|
||||||
expires_when=data.get("expires_when", ""),
|
expires_when=data.get("expires_when", ""),
|
||||||
|
|||||||
@@ -513,7 +513,7 @@ def _rule_version_rows(rows) -> list[dict]:
|
|||||||
"id": rv.id, "rule_id": rv.rule_id, "user_id": rv.user_id,
|
"id": rv.id, "rule_id": rv.rule_id, "user_id": rv.user_id,
|
||||||
"title": rv.title, "statement": rv.statement, "why": rv.why,
|
"title": rv.title, "statement": rv.statement, "why": rv.why,
|
||||||
"how_to_apply": rv.how_to_apply, "when_to_apply": rv.when_to_apply,
|
"how_to_apply": rv.how_to_apply, "when_to_apply": rv.when_to_apply,
|
||||||
"tier": rv.tier, "verify_with": rv.verify_with,
|
"tier": rv.tier, "kind": rv.kind, "verify_with": rv.verify_with,
|
||||||
"expires_when": rv.expires_when,
|
"expires_when": rv.expires_when,
|
||||||
"created_at": rv.created_at.isoformat(),
|
"created_at": rv.created_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,
|
"id": r.id, "topic_id": r.topic_id, "project_id": r.project_id,
|
||||||
"title": r.title, "statement": r.statement, "why": r.why,
|
"title": r.title, "statement": r.statement, "why": r.why,
|
||||||
"how_to_apply": r.how_to_apply, "order_index": r.order_index,
|
"how_to_apply": r.how_to_apply, "order_index": r.order_index,
|
||||||
"when_to_apply": r.when_to_apply, "tier": r.tier,
|
"when_to_apply": r.when_to_apply, "tier": r.tier, "kind": r.kind,
|
||||||
"verify_with": r.verify_with, "expires_when": r.expires_when,
|
"verify_with": r.verify_with, "expires_when": r.expires_when,
|
||||||
"verified_at": r.verified_at.isoformat() if r.verified_at else None,
|
"verified_at": r.verified_at.isoformat() if r.verified_at else None,
|
||||||
"arose_from_id": r.arose_from_id,
|
"arose_from_id": r.arose_from_id,
|
||||||
@@ -1283,6 +1283,11 @@ async def _restore_v2(data: dict) -> dict:
|
|||||||
# is the pre-0088 behaviour, so an old backup restores rules
|
# is the pre-0088 behaviour, so an old backup restores rules
|
||||||
# that bind exactly as they did when it was taken.
|
# that bind exactly as they did when it was taken.
|
||||||
tier=r_data.get("tier") or "always_on",
|
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
|
||||||
|
# bound quietly no longer binding.
|
||||||
|
kind=r_data.get("kind") or "rule",
|
||||||
verify_with=r_data.get("verify_with") or None,
|
verify_with=r_data.get("verify_with") or None,
|
||||||
expires_when=r_data.get("expires_when") or None,
|
expires_when=r_data.get("expires_when") or None,
|
||||||
# Restored as-is, NOT reset to null. `verified_at` records
|
# Restored as-is, NOT reset to null. `verified_at` records
|
||||||
@@ -1423,6 +1428,10 @@ async def _restore_v2(data: dict) -> dict:
|
|||||||
how_to_apply=rv.get("how_to_apply"),
|
how_to_apply=rv.get("how_to_apply"),
|
||||||
when_to_apply=rv.get("when_to_apply"),
|
when_to_apply=rv.get("when_to_apply"),
|
||||||
tier=rv.get("tier"),
|
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.
|
||||||
|
kind=rv.get("kind"),
|
||||||
verify_with=rv.get("verify_with"),
|
verify_with=rv.get("verify_with"),
|
||||||
expires_when=rv.get("expires_when"),
|
expires_when=rv.get("expires_when"),
|
||||||
created_at=_dt(rv.get("created_at")),
|
created_at=_dt(rv.get("created_at")),
|
||||||
|
|||||||
@@ -859,7 +859,13 @@ async def get_writepath_config(user_id: int) -> dict:
|
|||||||
}
|
}
|
||||||
|
|
||||||
def _rule_hint_line(rule, *, where: str, seen: bool) -> str:
|
def _rule_hint_line(rule, *, where: str, seen: bool) -> str:
|
||||||
"""One rule hint line — both arms, both tails (#3750).
|
"""One rule hint line — both arms, both tails, both kinds (#3750, #3849).
|
||||||
|
|
||||||
|
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
|
||||||
|
the line. Those are unrelated facts, and keeping them unrelated in the
|
||||||
|
code is what stopped the second kind from reopening the repeat question.
|
||||||
|
|
||||||
ONE FUNCTION BECAUSE THE TAILS MUST NOT DRIFT. The two arms phrase their
|
ONE FUNCTION BECAUSE THE TAILS MUST NOT DRIFT. The two arms phrase their
|
||||||
heads differently ("may apply here" vs "may apply to this Bash call") and
|
heads differently ("may apply here" vs "may apply to this Bash call") and
|
||||||
@@ -891,15 +897,38 @@ def _rule_hint_line(rule, *, where: str, seen: bool) -> str:
|
|||||||
managing it.
|
managing it.
|
||||||
"""
|
"""
|
||||||
trigger = (rule.when_to_apply or "").strip()
|
trigger = (rule.when_to_apply or "").strip()
|
||||||
|
preference = rule.kind == "preference"
|
||||||
|
# KIND CHANGES THE HEAD; `seen` CHANGES THE TAIL. The two axes are
|
||||||
|
# independent and stay that way, which is what lets the repeat logic above
|
||||||
|
# survive a second kind without being reasoned about again: whether a
|
||||||
|
# record is already on the ledger has nothing to do with how much force it
|
||||||
|
# carries, so the seen-branch is shared verbatim.
|
||||||
|
#
|
||||||
|
# The noun is the whole of the visual difference, and that is deliberate.
|
||||||
|
# A reader skimming an injected block gets one word to place the register \u2014
|
||||||
|
# so it is the SECOND word that moves, and it is the word naming force.
|
||||||
|
# Everything structural after it is identical, so the three kinds read as
|
||||||
|
# one set rather than three formats (milestone 385's step 5 writes the
|
||||||
|
# lesson voice against these two; they are one paragraph, not three).
|
||||||
|
noun = "Preference" if preference else "Standing rule"
|
||||||
|
# The only other place force is asserted. A rule's line tells the reader
|
||||||
|
# not to dismiss it unread, because dismissing a rule unread is how the
|
||||||
|
# thing it prevents happens. A preference makes no such claim: it says
|
||||||
|
# where to find how this has been done, and following it is what keeps
|
||||||
|
# things consistent rather than what keeps them correct.
|
||||||
|
reason = (
|
||||||
|
"for how this has been done before" if preference
|
||||||
|
else "before deciding it does not apply"
|
||||||
|
)
|
||||||
tail = (
|
tail = (
|
||||||
f"You saw it earlier this session; pull it with get_rule({rule.id}) "
|
f"You saw it earlier this session; pull it with get_rule({rule.id}) "
|
||||||
"if you no longer hold it."
|
"if you no longer hold it."
|
||||||
if seen else
|
if seen else
|
||||||
f"Read it with get_rule({rule.id}) before deciding it does not "
|
f"Read it with get_rule({rule.id}) {reason}; it is not in this "
|
||||||
"apply; it is not in this session's loaded set."
|
"session's loaded set."
|
||||||
)
|
)
|
||||||
return (
|
return (
|
||||||
f"Standing rule that may apply {where} \u2014 \u201c{rule.title}\u201d"
|
f"{noun} that may apply {where} \u2014 \u201c{rule.title}\u201d"
|
||||||
+ (f" ({trigger})" if trigger else "")
|
+ (f" ({trigger})" if trigger else "")
|
||||||
+ f". {tail}"
|
+ f". {tail}"
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ from scribe.models.rule_version import RuleVersion
|
|||||||
# snapshots would bury the edits somebody is actually looking for.
|
# snapshots would bury the edits somebody is actually looking for.
|
||||||
SNAPSHOT_FIELDS = (
|
SNAPSHOT_FIELDS = (
|
||||||
"title", "statement", "why", "how_to_apply", "when_to_apply",
|
"title", "statement", "why", "how_to_apply", "when_to_apply",
|
||||||
"tier", "verify_with", "expires_when",
|
"tier", "kind", "verify_with", "expires_when",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -295,6 +295,9 @@ async def _assert_rulebook_rule_owned(session, rule_id: int, user_id: int) -> No
|
|||||||
# two in step; this keeps the error readable).
|
# two in step; this keeps the error readable).
|
||||||
TIERS = ("always_on", "conditional")
|
TIERS = ("always_on", "conditional")
|
||||||
RELATION_KINDS = ("co_surfaces", "overrides", "elaborates")
|
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.
|
||||||
|
KINDS = ("rule", "preference")
|
||||||
|
|
||||||
|
|
||||||
# The rule columns that are nullable, and therefore the ones where EMPTY has
|
# The rule columns that are nullable, and therefore the ones where EMPTY has
|
||||||
@@ -319,6 +322,22 @@ def _valid_tier(tier: str) -> str:
|
|||||||
return tier if tier in TIERS else "always_on"
|
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
|
||||||
|
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
|
||||||
|
was written to prevent, and costs it silently, because nothing downstream
|
||||||
|
can tell a softened rule from a preference that was always one.
|
||||||
|
|
||||||
|
Between a reader who is too careful and a reader who is not careful
|
||||||
|
enough, the typo should produce the first.
|
||||||
|
"""
|
||||||
|
return kind if kind in KINDS else "rule"
|
||||||
|
|
||||||
|
|
||||||
# Re-exported, not redefined. Notes gained the same trio in milestone 317 and
|
# Re-exported, not redefined. Notes gained the same trio in milestone 317 and
|
||||||
# this reading of it is genuinely common, so it moved to services/verification
|
# this reading of it is genuinely common, so it moved to services/verification
|
||||||
# — the DRY win note 3163 names, as against sharing the QUERY, which the two
|
# — the DRY win note 3163 names, as against sharing the QUERY, which the two
|
||||||
@@ -351,6 +370,13 @@ def rule_brief(rule: Rule, **extra) -> dict:
|
|||||||
"statement": rule.statement,
|
"statement": rule.statement,
|
||||||
"topic_id": rule.topic_id,
|
"topic_id": rule.topic_id,
|
||||||
"tier": rule.tier,
|
"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
|
||||||
|
# is the opposite case: a reader seeing no `kind` would have to assume
|
||||||
|
# one, and the assumption it would reach for — "this binds" — is the
|
||||||
|
# expensive one to get wrong in the other direction. Say it outright.
|
||||||
|
"kind": rule.kind or "rule",
|
||||||
"updated_at": rule.updated_at.date().isoformat() if rule.updated_at else None,
|
"updated_at": rule.updated_at.date().isoformat() if rule.updated_at else None,
|
||||||
}
|
}
|
||||||
# Attached only when present (#2483: never a null key that reads as a
|
# Attached only when present (#2483: never a null key that reads as a
|
||||||
@@ -478,7 +504,7 @@ async def create_rule(
|
|||||||
topic_id: int, user_id: int, title: str, statement: str,
|
topic_id: int, user_id: int, title: str, statement: str,
|
||||||
why: str = "", how_to_apply: str = "", order_index: int = 0,
|
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 = "", tier: str = "always_on", arose_from_id: int = 0,
|
||||||
verify_with: str = "", expires_when: str = "",
|
verify_with: str = "", expires_when: str = "", kind: str = "rule",
|
||||||
) -> Rule:
|
) -> Rule:
|
||||||
async with async_session() as session:
|
async with async_session() as session:
|
||||||
await _assert_topic_owned(session, topic_id, user_id)
|
await _assert_topic_owned(session, topic_id, user_id)
|
||||||
@@ -488,6 +514,7 @@ async def create_rule(
|
|||||||
statement=statement,
|
statement=statement,
|
||||||
when_to_apply=when_to_apply or None,
|
when_to_apply=when_to_apply or None,
|
||||||
tier=_valid_tier(tier),
|
tier=_valid_tier(tier),
|
||||||
|
kind=_valid_kind(kind),
|
||||||
why=why or None,
|
why=why or None,
|
||||||
how_to_apply=how_to_apply or None,
|
how_to_apply=how_to_apply or None,
|
||||||
verify_with=verify_with or None,
|
verify_with=verify_with or None,
|
||||||
@@ -506,7 +533,7 @@ async def create_project_rule(
|
|||||||
project_id: int, user_id: int, title: str, statement: str,
|
project_id: int, user_id: int, title: str, statement: str,
|
||||||
why: str = "", how_to_apply: str = "", order_index: int = 0,
|
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 = "", tier: str = "always_on", arose_from_id: int = 0,
|
||||||
verify_with: str = "", expires_when: str = "",
|
verify_with: str = "", expires_when: str = "", kind: str = "rule",
|
||||||
) -> Rule:
|
) -> Rule:
|
||||||
"""Create a rule scoped to a single project (no rulebook ceremony).
|
"""Create a rule scoped to a single project (no rulebook ceremony).
|
||||||
|
|
||||||
@@ -522,6 +549,7 @@ async def create_project_rule(
|
|||||||
statement=statement,
|
statement=statement,
|
||||||
when_to_apply=when_to_apply or None,
|
when_to_apply=when_to_apply or None,
|
||||||
tier=_valid_tier(tier),
|
tier=_valid_tier(tier),
|
||||||
|
kind=_valid_kind(kind),
|
||||||
why=why or None,
|
why=why or None,
|
||||||
how_to_apply=how_to_apply or None,
|
how_to_apply=how_to_apply or None,
|
||||||
verify_with=verify_with or None,
|
verify_with=verify_with or None,
|
||||||
@@ -752,7 +780,7 @@ async def update_rule(
|
|||||||
return None
|
return None
|
||||||
allowed = {
|
allowed = {
|
||||||
"title", "statement", "why", "how_to_apply", "order_index",
|
"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",
|
"verify_with", "expires_when",
|
||||||
}
|
}
|
||||||
check_before = rule.verify_with
|
check_before = rule.verify_with
|
||||||
@@ -770,6 +798,8 @@ async def update_rule(
|
|||||||
continue
|
continue
|
||||||
if key == "tier":
|
if key == "tier":
|
||||||
value = _valid_tier(value)
|
value = _valid_tier(value)
|
||||||
|
elif key == "kind":
|
||||||
|
value = _valid_kind(value)
|
||||||
elif key in NULLABLE_RULE_TEXT:
|
elif key in NULLABLE_RULE_TEXT:
|
||||||
value = value or None
|
value = value or None
|
||||||
elif key == "arose_from_id":
|
elif key == "arose_from_id":
|
||||||
|
|||||||
@@ -226,6 +226,12 @@ def fake_rule(**attrs) -> MagicMock:
|
|||||||
# `when_to_apply` and `arose_from_id` would be truthy MagicMocks and
|
# `when_to_apply` and `arose_from_id` would be truthy MagicMocks and
|
||||||
# rule_brief would attach both keys on every stand-in.
|
# rule_brief would attach both keys on every stand-in.
|
||||||
"when_to_apply": None, "tier": "always_on", "arose_from_id": None,
|
"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
|
# Same reason, and the same trap one field further on: an unnamed
|
||||||
# `verify_with` is a truthy MagicMock, so every stand-in rule would
|
# `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
|
# claim to carry a check and rule_brief would stamp a MagicMock date
|
||||||
@@ -235,6 +241,25 @@ def fake_rule(**attrs) -> MagicMock:
|
|||||||
}, attrs)
|
}, 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:
|
class FakeMCP:
|
||||||
"""Stand-in for the FastMCP server a tool module's ``register(mcp)`` is
|
"""Stand-in for the FastMCP server a tool module's ``register(mcp)`` is
|
||||||
handed: records the ``name=`` of every ``@mcp.tool(...)`` registration in
|
handed: records the ``name=`` of every ``@mcp.tool(...)`` registration in
|
||||||
|
|||||||
@@ -252,3 +252,50 @@ def test_no_surface_names_the_push_without_stating_the_pull():
|
|||||||
f"the bridge — it can be absent without saying so. Name it if it helps, "
|
f"the bridge — it can be absent without saying so. Name it if it helps, "
|
||||||
f"but say to call {PULL}() regardless."
|
f"but say to call {PULL}() regardless."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── force: a surface that says rules bind must say what does not ────────
|
||||||
|
#
|
||||||
|
# Added with the preference kind (milestone 399). Before it, "rules bind" was
|
||||||
|
# the whole truth and every surface said so flatly. It is now half of one, and
|
||||||
|
# the half that is missing is the dangerous half to omit: a session reading
|
||||||
|
# only "rules bind" and then receiving a preference has been told, by the most
|
||||||
|
# authoritative surface it has, to treat it as binding.
|
||||||
|
#
|
||||||
|
# That failure is silent in both directions. Treating a preference as a rule
|
||||||
|
# produces a session that refuses to proceed over something the operator only
|
||||||
|
# preferred; and it removes the reason preferences exist, which is that they
|
||||||
|
# can be brought up to date rather than obeyed.
|
||||||
|
#
|
||||||
|
# Same bargain as every test in this file: STRUCTURE, not wording. A surface
|
||||||
|
# passes by mentioning the other kind at all, so the prose stays free.
|
||||||
|
#
|
||||||
|
# PINNED ON THE CLAIM, NOT THE WORD "bind". A bare substring also matches
|
||||||
|
# `bind_repo`, `list_repo_bindings` and the DNS-rebinding comment in
|
||||||
|
# server.py — so it would one day fail a skill that mentions repo binding and
|
||||||
|
# has nothing to do with force, which is rule 167's named failure: a guard
|
||||||
|
# raising a false alarm about the very thing it protects. These phrases are
|
||||||
|
# the ones that actually assert bindingness to a reader.
|
||||||
|
BINDING_CLAIMS = (
|
||||||
|
"rules bind",
|
||||||
|
"binding rules",
|
||||||
|
"rules are binding",
|
||||||
|
"treat every one as binding",
|
||||||
|
)
|
||||||
|
OTHER_KIND = "preference"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_surface_claiming_rules_bind_also_names_what_does_not():
|
||||||
|
offenders = [
|
||||||
|
label for label, text in _all_surfaces()
|
||||||
|
if any(c in text.lower() for c in BINDING_CLAIMS)
|
||||||
|
and OTHER_KIND not in text.lower()
|
||||||
|
]
|
||||||
|
assert not offenders, (
|
||||||
|
f"these surfaces tell a session that rules bind and never mention "
|
||||||
|
f"preferences: {offenders}. A preference arrives through the same arms "
|
||||||
|
f"and renders in the same line shape, so a surface that describes only "
|
||||||
|
f"the binding kind is read as covering both — and the session treats "
|
||||||
|
f"'how the operator likes this done' as something it may not proceed "
|
||||||
|
f"past. Name the other kind, however briefly."
|
||||||
|
)
|
||||||
|
|||||||
@@ -0,0 +1,165 @@
|
|||||||
|
"""Real-Postgres tests for `rules.kind` and its CHECK (0098, #3849 step 1).
|
||||||
|
|
||||||
|
Rule 36 exists because a value and its constraint drift apart: the code
|
||||||
|
starts writing a new kind while the database still refuses it, and nothing
|
||||||
|
catches it until a write fails in front of someone. A mock cannot show that —
|
||||||
|
it has no CHECK — so the constraint gets a real-DB test, exactly as 0091's
|
||||||
|
`spike` did.
|
||||||
|
|
||||||
|
THREE HALVES, not one. The positive (a preference writes), the negative (a
|
||||||
|
typo is refused — without it every other assertion here would pass just as
|
||||||
|
happily against a table whose CHECK was dropped and never re-added), and the
|
||||||
|
DEFAULT.
|
||||||
|
|
||||||
|
The default is the one worth spelling out, because it is the half that has no
|
||||||
|
obvious failure. `kind` is NOT NULL with a server default, and the entire
|
||||||
|
safety argument for this migration is that every existing row keeps the force
|
||||||
|
it had. A row written without a kind must read back as `rule` — if the server
|
||||||
|
default did not apply, an upgraded install gets a NOT NULL violation on its
|
||||||
|
next rule write, or worse, a column that reads as something other than what
|
||||||
|
every rule in it has always been.
|
||||||
|
"""
|
||||||
|
import pytest
|
||||||
|
import pytest_asyncio
|
||||||
|
from sqlalchemy import select
|
||||||
|
from sqlalchemy.exc import IntegrityError
|
||||||
|
|
||||||
|
from scribe.models import async_session
|
||||||
|
from scribe.models.rulebook import Rule, Rulebook
|
||||||
|
from scribe.services import rulebooks as rulebooks_svc
|
||||||
|
from tests.helpers import ensure_user
|
||||||
|
|
||||||
|
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
|
||||||
|
|
||||||
|
OWNER_USERNAME = "rule_kind_owner"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest_asyncio.fixture
|
||||||
|
async def topic_id():
|
||||||
|
"""A topic to hang rules on.
|
||||||
|
|
||||||
|
CLEANED UP AT SETUP, NOT TEARDOWN, for the reason spelled out in
|
||||||
|
test_integration_rule_versions: the rule write path fires a detached
|
||||||
|
embedding task that opens its own connection and UPDATEs the row, and a
|
||||||
|
teardown deleting the rulebook deadlocks against it. Purging at setup runs
|
||||||
|
on a fresh loop, after the previous test's loop cancelled whatever it left
|
||||||
|
in flight.
|
||||||
|
"""
|
||||||
|
async with async_session() as s:
|
||||||
|
owner = await ensure_user(s, OWNER_USERNAME)
|
||||||
|
uid = owner.id
|
||||||
|
await s.commit()
|
||||||
|
for book in (await s.execute(
|
||||||
|
select(Rulebook).where(Rulebook.owner_user_id == uid)
|
||||||
|
)).scalars().all():
|
||||||
|
await s.delete(book)
|
||||||
|
await s.commit()
|
||||||
|
|
||||||
|
book = await rulebooks_svc.create_rulebook(uid, "Kind fixtures")
|
||||||
|
topic = await rulebooks_svc.create_topic(book.id, uid, "conventions")
|
||||||
|
return uid, topic.id
|
||||||
|
|
||||||
|
|
||||||
|
async def _write_raw(topic: int, **kw) -> int:
|
||||||
|
"""Straight to the model, bypassing `_valid_kind`.
|
||||||
|
|
||||||
|
The service coerces an unrecognised kind to `rule`, which is right for
|
||||||
|
callers and useless for testing the CHECK — it means no service call can
|
||||||
|
ever reach the database with a bad value. These tests are about the
|
||||||
|
constraint, so they go around the coercion.
|
||||||
|
"""
|
||||||
|
async with async_session() as s:
|
||||||
|
rule = Rule(topic_id=topic, title="t", statement="s", **kw)
|
||||||
|
s.add(rule)
|
||||||
|
await s.commit()
|
||||||
|
return rule.id
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_preference_can_be_written(topic_id):
|
||||||
|
_uid, topic = topic_id
|
||||||
|
rule_id = await _write_raw(topic, kind="preference")
|
||||||
|
async with async_session() as s:
|
||||||
|
assert (await s.get(Rule, rule_id)).kind == "preference"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_rule_still_writes(topic_id):
|
||||||
|
"""0098 widens nothing — it adds a column — but it must not narrow either."""
|
||||||
|
_uid, topic = topic_id
|
||||||
|
rule_id = await _write_raw(topic, kind="rule")
|
||||||
|
async with async_session() as s:
|
||||||
|
assert (await s.get(Rule, rule_id)).kind == "rule"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_an_unknown_kind_is_refused(topic_id):
|
||||||
|
"""The half that proves the constraint is there at all.
|
||||||
|
|
||||||
|
Without this, every other assertion in this file would pass against a
|
||||||
|
table whose CHECK had been dropped — the exact failure rule 36 is written
|
||||||
|
against.
|
||||||
|
"""
|
||||||
|
_uid, topic = topic_id
|
||||||
|
with pytest.raises(IntegrityError):
|
||||||
|
await _write_raw(topic, kind="suggestion")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_rule_written_without_a_kind_defaults_to_rule(topic_id):
|
||||||
|
"""The migration's whole safety claim, asserted rather than assumed.
|
||||||
|
|
||||||
|
Every row that existed before 0098 was a rule and must stay one. This is
|
||||||
|
the closest a test can get to that: write the way code predating the
|
||||||
|
column would, and read back the force it should have.
|
||||||
|
"""
|
||||||
|
_uid, topic = topic_id
|
||||||
|
rule_id = await _write_raw(topic)
|
||||||
|
async with async_session() as s:
|
||||||
|
assert (await s.get(Rule, rule_id)).kind == "rule"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_rule_can_become_a_preference_after_the_fact(topic_id):
|
||||||
|
"""The read-back is the whole test.
|
||||||
|
|
||||||
|
A version asserting only that the update call succeeded would pass against
|
||||||
|
code that accepted `kind` and dropped it — which is how the same bug
|
||||||
|
survived on `task_kind` long enough to be found by hand (#3129).
|
||||||
|
|
||||||
|
This is also the migration path the always-on triage needs: the rules that
|
||||||
|
turn out to be preferences change one column and keep their id, history
|
||||||
|
and relations.
|
||||||
|
"""
|
||||||
|
uid, topic = topic_id
|
||||||
|
rule_id = await _write_raw(topic)
|
||||||
|
await rulebooks_svc.update_rule(rule_id, uid, kind="preference")
|
||||||
|
async with async_session() as s:
|
||||||
|
assert (await s.get(Rule, rule_id)).kind == "preference"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_an_unrecognised_kind_falls_back_rather_than_raising(topic_id):
|
||||||
|
"""`_valid_kind` coerces, matching `_valid_tier`, and the direction matters.
|
||||||
|
|
||||||
|
Deliberately NOT the `task_kind` behaviour, which raises a readable
|
||||||
|
ValueError. A rule's kind falls back to the binding value, because between
|
||||||
|
a reader who is too careful and one who is not careful enough, a typo
|
||||||
|
should produce the first. Asserted here so a later "make it consistent
|
||||||
|
with task_kind" change has to argue with a test rather than a comment.
|
||||||
|
"""
|
||||||
|
uid, topic = topic_id
|
||||||
|
rule_id = await _write_raw(topic, kind="preference")
|
||||||
|
await rulebooks_svc.update_rule(rule_id, uid, kind="prefrence")
|
||||||
|
async with async_session() as s:
|
||||||
|
assert (await s.get(Rule, rule_id)).kind == "rule"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_kind_reaches_both_read_shapes(topic_id):
|
||||||
|
"""`to_dict` and `rule_brief` both carry it, unconditionally.
|
||||||
|
|
||||||
|
Force must never be inferred from an ABSENT key: "no kind field" and
|
||||||
|
"kind is rule" would be the same payload, and a reader would have to
|
||||||
|
assume one. Both shapes say it outright, so pin both — the two dicts are
|
||||||
|
built in different modules and have diverged before.
|
||||||
|
"""
|
||||||
|
_uid, topic = topic_id
|
||||||
|
rule_id = await _write_raw(topic, kind="preference")
|
||||||
|
async with async_session() as s:
|
||||||
|
rule = await s.get(Rule, rule_id)
|
||||||
|
assert rule.to_dict()["kind"] == "preference"
|
||||||
|
assert rulebooks_svc.rule_brief(rule)["kind"] == "preference"
|
||||||
@@ -3,6 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from tests.helpers import FakeMCP, fake_rule, fake_rulebook, fake_topic
|
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")
|
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)
|
await get_rulebook(rulebook_id=999)
|
||||||
|
|
||||||
|
|
||||||
def _plain_detail():
|
# _plain_detail moved to tests/helpers on its second copy (#2825's own
|
||||||
"""Stub the rule_detail seam these tool tests are not about.
|
# reason for existing): two stubs for one seam drift apart quietly, and a
|
||||||
|
# test stubbing it slightly differently asserts something slightly different
|
||||||
create/update/get_rule now return through services.rulebooks.rule_detail,
|
# than it appears to.
|
||||||
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)
|
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -230,11 +223,18 @@ def test_register_attaches_every_tool():
|
|||||||
|
|
||||||
register(mcp)
|
register(mcp)
|
||||||
# 26 through milestone 307, +2 for the staleness sweep (milestone 312),
|
# 26 through milestone 307, +2 for the staleness sweep (milestone 312),
|
||||||
# +1 for a rule's edit history (milestone 323).
|
# +1 for a rule's edit history (milestone 323), +2 for preferences
|
||||||
assert len(mcp.names) == 29
|
# (milestone 399).
|
||||||
|
assert len(mcp.names) == 31
|
||||||
# spot-check a few names
|
# spot-check a few names
|
||||||
assert "list_rulebooks" in mcp.names
|
assert "list_rulebooks" in mcp.names
|
||||||
assert "create_rule" 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 "subscribe_project_to_rulebook" in mcp.names
|
||||||
assert "list_always_on_rules" in mcp.names
|
assert "list_always_on_rules" in mcp.names
|
||||||
# milestone 297: a project's opt-out of a whole always-on rulebook
|
# 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."
|
||||||
|
)
|
||||||
@@ -1006,6 +1006,93 @@ async def test_the_two_tails_are_distinguishable_and_say_the_true_one(source, ru
|
|||||||
assert fresh_ctx != seen_ctx, "the two tails collapsed into one"
|
assert fresh_ctx != seen_ctx, "the two tails collapsed into one"
|
||||||
|
|
||||||
|
|
||||||
|
# ── the third axis: KIND (milestone 399) ────────────────────────────────
|
||||||
|
#
|
||||||
|
# A preference is a rule row that does not bind, so it rides the same arms and
|
||||||
|
# the same line. What must differ is the REGISTER: a rule's line tells the
|
||||||
|
# reader not to dismiss it unread, because dismissing a rule unread is how the
|
||||||
|
# thing it prevents happens. A preference makes no such claim — it says where
|
||||||
|
# to find how this has been done, and following it buys consistency rather
|
||||||
|
# than correctness.
|
||||||
|
#
|
||||||
|
# Rendered in the rule's voice, a preference becomes the thing milestone 399
|
||||||
|
# exists to avoid: a rule with a different column.
|
||||||
|
|
||||||
|
_PREF = fake_rule(
|
||||||
|
id=177,
|
||||||
|
kind="preference",
|
||||||
|
title="Pace hard debugging one step at a time",
|
||||||
|
statement="Advance one investigative step per turn.",
|
||||||
|
when_to_apply="during hard debugging",
|
||||||
|
)
|
||||||
|
_RULE_FORCE = "before deciding it does not apply"
|
||||||
|
_PREF_FORCE = "for how this has been done before"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_preference_does_not_speak_in_the_rules_voice(source, run):
|
||||||
|
"""The register, pinned on the two places force is actually asserted.
|
||||||
|
|
||||||
|
On BOTH the noun and the clause, because either alone is weak. A line
|
||||||
|
reading "Preference … before deciding it does not apply" has swapped the
|
||||||
|
label and kept the instruction, which is worse than not distinguishing
|
||||||
|
them at all: it looks handled.
|
||||||
|
"""
|
||||||
|
ctx = (await run([(0.81, _PREF)], MagicMock()))["context"]
|
||||||
|
|
||||||
|
assert "Preference that may apply" in ctx, (
|
||||||
|
f"{source} announced a preference as something else. The noun is the "
|
||||||
|
f"one word a skimming reader gets to place the register, so it is the "
|
||||||
|
f"word that has to move. Context was: {ctx!r}"
|
||||||
|
)
|
||||||
|
assert _PREF_FORCE in ctx and _RULE_FORCE not in ctx, (
|
||||||
|
f"{source} told the session to read a PREFERENCE before deciding it "
|
||||||
|
f"does not apply. That is a rule's claim: it is the sentence that "
|
||||||
|
f"makes a line bind, and a preference does not."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_kind_and_seen_do_not_read_each_other(source, run):
|
||||||
|
"""The structural claim the design rests on: two INDEPENDENT axes.
|
||||||
|
|
||||||
|
Whether a record is already on the exclusion ledger has nothing to do with
|
||||||
|
how much force it carries. Keeping the two unrelated in the code is what
|
||||||
|
let a second kind arrive without reopening #3750's repeat question — and
|
||||||
|
the way that silently breaks is a `seen` branch that grows a kind test,
|
||||||
|
or a kind branch that grows a `seen` test, leaving one of the four
|
||||||
|
combinations rendered by nobody's intention.
|
||||||
|
|
||||||
|
So: all four are exercised, and the seen tail must come out identical for
|
||||||
|
both kinds.
|
||||||
|
"""
|
||||||
|
pref_fresh = (await run([(0.81, _PREF)], MagicMock()))["context"]
|
||||||
|
pref_seen = (await run([(0.81, _PREF)], MagicMock(),
|
||||||
|
exclude_rule_ids=[177]))["context"]
|
||||||
|
rule_seen = (await run([(0.81, _HELD)], MagicMock(),
|
||||||
|
exclude_rule_ids=[156]))["context"]
|
||||||
|
|
||||||
|
assert _SEEN_TAIL in pref_seen and _FRESH_TAIL in pref_fresh, (
|
||||||
|
f"{source}: the seen/fresh split stopped working once kind was added — "
|
||||||
|
f"the tail axis is now reading the head axis"
|
||||||
|
)
|
||||||
|
# The tail is about the LEDGER, so it is shared verbatim. Compared as the
|
||||||
|
# tail alone rather than the whole line, since the heads differ by design.
|
||||||
|
assert _SEEN_TAIL in rule_seen, "the rule's seen tail changed"
|
||||||
|
assert pref_seen.count(_SEEN_TAIL) == rule_seen.count(_SEEN_TAIL) == 1, (
|
||||||
|
f"{source} rendered the repeat clause a different number of times for "
|
||||||
|
f"the two kinds; the tail is about the ledger and does not vary by force"
|
||||||
|
)
|
||||||
|
# And the head still differs in the seen case — a repeat of a preference
|
||||||
|
# is still a preference.
|
||||||
|
assert "Preference that may apply" in pref_seen, (
|
||||||
|
f"{source} lost the preference register on a repeat, so a preference "
|
||||||
|
f"seen twice reads as a rule the second time"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
@pytest.mark.parametrize(("source", "run"), _ARMS, ids=["write_path", "pre_tool"])
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_neither_tail_injects_the_rule_statement(source, run):
|
async def test_neither_tail_injects_the_rule_statement(source, run):
|
||||||
|
|||||||
@@ -388,7 +388,7 @@ def test_rule_rows_carry_the_verification_fields():
|
|||||||
row = SimpleNamespace(
|
row = SimpleNamespace(
|
||||||
id=1, topic_id=2, project_id=None, title="t", statement="s",
|
id=1, topic_id=2, project_id=None, title="t", statement="s",
|
||||||
why="w", how_to_apply="h", order_index=0,
|
why="w", how_to_apply="h", order_index=0,
|
||||||
when_to_apply="when", tier="conditional",
|
when_to_apply="when", tier="conditional", kind="rule",
|
||||||
verify_with="cat some/file", expires_when="the file grows a shell",
|
verify_with="cat some/file", expires_when="the file grows a shell",
|
||||||
verified_at=checked, arose_from_id=99,
|
verified_at=checked, arose_from_id=99,
|
||||||
created_at=checked, updated_at=checked,
|
created_at=checked, updated_at=checked,
|
||||||
@@ -415,7 +415,7 @@ def test_rule_rows_keep_an_unverified_rule_unverified():
|
|||||||
row = SimpleNamespace(
|
row = SimpleNamespace(
|
||||||
id=1, topic_id=2, project_id=None, title="t", statement="s",
|
id=1, topic_id=2, project_id=None, title="t", statement="s",
|
||||||
why=None, how_to_apply=None, order_index=0,
|
why=None, how_to_apply=None, order_index=0,
|
||||||
when_to_apply=None, tier="always_on",
|
when_to_apply=None, tier="always_on", kind="rule",
|
||||||
verify_with=None, expires_when=None, verified_at=None,
|
verify_with=None, expires_when=None, verified_at=None,
|
||||||
arose_from_id=None,
|
arose_from_id=None,
|
||||||
created_at=datetime(2026, 8, 27, tzinfo=timezone.utc),
|
created_at=datetime(2026, 8, 27, tzinfo=timezone.utc),
|
||||||
@@ -426,3 +426,27 @@ def test_rule_rows_keep_an_unverified_rule_unverified():
|
|||||||
assert backup._dt_or_none("2026-08-27T12:00:00+00:00") == datetime(
|
assert backup._dt_or_none("2026-08-27T12:00:00+00:00") == datetime(
|
||||||
2026, 8, 27, 12, 0, tzinfo=timezone.utc
|
2026, 8, 27, 12, 0, tzinfo=timezone.utc
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_rule_rows_carry_the_kind_so_a_preference_does_not_restore_as_a_rule():
|
||||||
|
"""FORCE has to survive a backup, and the failure would be silent.
|
||||||
|
|
||||||
|
A preference that comes back as a rule is not a missing field anyone would
|
||||||
|
notice — the rule reads fine, it simply binds when it was only ever meant
|
||||||
|
to be how the operator prefers things done. Nothing in the restored
|
||||||
|
rulebook says it used to be softer.
|
||||||
|
|
||||||
|
Asserted with `preference` rather than `rule` on purpose: a fixture
|
||||||
|
carrying the DEFAULT would pass just as happily against a `_rule_rows`
|
||||||
|
that dropped the field entirely and let the importer's `or "rule"` fill
|
||||||
|
the hole back in, which is precisely the bug this guards.
|
||||||
|
"""
|
||||||
|
stamp = datetime(2026, 9, 10, tzinfo=timezone.utc)
|
||||||
|
row = SimpleNamespace(
|
||||||
|
id=1, topic_id=2, project_id=None, title="t", statement="s",
|
||||||
|
why=None, how_to_apply=None, order_index=0,
|
||||||
|
when_to_apply="when", tier="conditional", kind="preference",
|
||||||
|
verify_with=None, expires_when=None, verified_at=None,
|
||||||
|
arose_from_id=None, created_at=stamp, updated_at=stamp,
|
||||||
|
)
|
||||||
|
assert backup._rule_rows([row])[0]["kind"] == "preference"
|
||||||
|
|||||||
Reference in New Issue
Block a user