feat(rules): preferences are writable, and their drift arrives (#3895)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 52s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m32s
CI & Build / Build & push image (push) Successful in 34s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 52s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m32s
CI & Build / Build & push image (push) Successful in 34s
Milestone 399 step 5. Steps 1-4 put preferences into the backend: a kind column, an inverted write path, a third register in the injected block, a delivery slot. Nothing the operator could touch. Rule 27 forbids leaving it there, and here it matters more than usual, because the UI is the only guard against the risk the milestone named up front — an agent misreads one session, rewrites a preference, and follows the rewritten version forever while the operator never sees the moment it changed. Four things ship. A preference is DISTINGUISHABLE. `kind` reaches the client (the server has always sent it in rule_brief) and a preference carries a chip. Force is the one thing a list of instructions must not leave the reader to infer, and a row that renders identically to a rule teaches the opposite of both facts about a preference: it does not bind, and a session may rewrite it. A preference is WRITABLE. The editor gains the kind as a first-class choice with the test beside it — what happens when someone does not do this — and says plainly, when preference is chosen, that sessions rewrite these without asking and every rewrite is kept. DRIFT ARRIVES. `GET /api/rules/drift` returns one row per rewritten preference carrying its latest rewrite: what it said, what it says now, and the record named by `arose_from_id` that taught the change. Both texts ride along so the list shows the diff without a call per row. The new pane sits beside the staleness sweep, because drift belongs to no one rulebook, and it answers a question the operator would not have thought to ask. REVERSION IS ONE ACTION, and this is the carve-out worth arguing with. Milestone 323 refused a one-click restore for rules — "a binding instruction should not be revertible in one click", because a silent revert erases the only record of why the rewrite happened. That reasoning turns on the rewrite being the operator's own decision. A preference's is not: the agent makes it mid-work without asking, so reverting is a veto over someone else's edit rather than an undo of your own, and a veto costing more than a shrug is not supervision. The route refuses anything but a preference (409), and nothing is erased: the restore goes through update_rule, so it snapshots too and the history GAINS the revert. An integration test pins that, because it is the whole basis for the exception. Tested against real Postgres — every claim is about which rows come back and in what order, which a stand-in session cannot judge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -258,11 +258,57 @@ async def get_rule_version(rule_id: int, version_id: int):
|
||||
return jsonify(version.to_dict(include_text=True))
|
||||
|
||||
|
||||
# NO restore route, deliberately (milestone 323). A note version can be
|
||||
# restored; a binding instruction should not be revertible in one click.
|
||||
# Putting a rewrite back goes through update_rule, which takes its own
|
||||
# NO restore route FOR A RULE, deliberately (milestone 323). A note version
|
||||
# can be restored; a binding instruction should not be revertible in one
|
||||
# click. Putting a rewrite back goes through update_rule, which takes its own
|
||||
# snapshot and leaves the undo in the history like any other edit — a silent
|
||||
# revert would erase the only record of why the rewrite happened.
|
||||
#
|
||||
# A PREFERENCE IS THE EXCEPTION, and the route below refuses anything else.
|
||||
# 323's reasoning turns on the rewrite being the operator's own decision.
|
||||
# A preference's rewrite is not: the agent makes it mid-work without asking,
|
||||
# which is what the kind is for. Reverting one is a veto over someone else's
|
||||
# edit rather than an undo of your own, and milestone 399 named the cost of
|
||||
# making that veto expensive — drift supervised in name only. The safeguard
|
||||
# 323 actually wanted survives intact, because the restore goes through
|
||||
# update_rule too: the rewrite stays in the history with the revert recorded
|
||||
# after it, so the history gains an entry rather than losing one.
|
||||
|
||||
|
||||
@rulebooks_bp.get("/rules/drift")
|
||||
@login_required
|
||||
async def preference_drift():
|
||||
"""Preferences that have been rewritten, most recently changed first.
|
||||
|
||||
One row per preference carrying its latest rewrite, what it said before,
|
||||
and the record that taught the change. `?limit=` caps the list.
|
||||
"""
|
||||
uid = get_current_user_id()
|
||||
try:
|
||||
limit = int(request.args.get("limit", 20))
|
||||
except (TypeError, ValueError):
|
||||
limit = 20
|
||||
rows = await rulebooks_svc.recent_preference_drift(uid, limit=limit)
|
||||
return jsonify({"drift": rows})
|
||||
|
||||
|
||||
@rulebooks_bp.post("/rules/<int:rule_id>/versions/<int:version_id>/restore")
|
||||
@login_required
|
||||
async def restore_rule_version(rule_id: int, version_id: int):
|
||||
"""Put a preference back to what that version said. Preferences only.
|
||||
|
||||
409 rather than 400 on a rule: the request is well-formed and the caller
|
||||
is not wrong to have asked — this rule is simply in a state where the
|
||||
action does not apply, and the message says which state and why.
|
||||
"""
|
||||
uid = get_current_user_id()
|
||||
try:
|
||||
rule = await rulebooks_svc.restore_rule_version(rule_id, version_id, uid)
|
||||
except ValueError as exc:
|
||||
return jsonify({"error": str(exc)}), 409
|
||||
if rule is None:
|
||||
return jsonify({"error": "rule or version not found"}), 404
|
||||
return jsonify(await rulebooks_svc.rule_detail(uid, rule))
|
||||
|
||||
|
||||
@rulebooks_bp.post("/rules/<int:rule_id>/relations")
|
||||
|
||||
@@ -11,9 +11,10 @@ import logging
|
||||
from collections.abc import Iterable
|
||||
from typing import Optional
|
||||
|
||||
from sqlalchemy import and_, delete as sql_delete, insert, or_, select
|
||||
from sqlalchemy import and_, delete as sql_delete, func, insert, or_, select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.base import iso
|
||||
from scribe.models.system import System
|
||||
from scribe.models.rulebook import Rulebook
|
||||
from scribe.services.verification import (
|
||||
@@ -846,6 +847,213 @@ async def get_rule_version(rule_id: int, version_id: int, user_id: int):
|
||||
)).scalar_one_or_none()
|
||||
|
||||
|
||||
# ── Drift: what Scribe changed about how it works with you (#3895) ─────
|
||||
#
|
||||
# A preference is the one record kind the AGENT rewrites in the ordinary
|
||||
# course of working, which is the property that makes it useful and the
|
||||
# property that makes it dangerous. Milestone 399 named the risk up front:
|
||||
# an agent misreads one session, rewrites a preference, and follows the
|
||||
# rewritten version forever while the operator never sees the moment it
|
||||
# changed — a confident wrong answer wearing the operator's own authority.
|
||||
#
|
||||
# `rule_versions` already records every rewrite. What it does not do is
|
||||
# ARRIVE: a history you have to open one rule at a time, having first
|
||||
# suspected that rule, is not oversight. These two functions are the push
|
||||
# half — one read that answers "what changed lately", and one write that
|
||||
# puts it back.
|
||||
|
||||
_DRIFT_LIMIT_MAX = 50
|
||||
|
||||
|
||||
def _owned_rules_clause(user_id: int):
|
||||
"""Rules whose rulebook or project this user owns — the LISTING form of
|
||||
`_fetch_owned_rule`.
|
||||
|
||||
Deliberately the same two paths in the same order, because a listing that
|
||||
admits a rule the per-row fetch would refuse is a leak, and one that
|
||||
refuses a rule the fetch admits is a row the operator cannot act on. The
|
||||
per-row version stays the authority: everything reached through this is
|
||||
re-checked by `_fetch_owned_rule` before it is written to.
|
||||
"""
|
||||
from scribe.models.project import Project
|
||||
|
||||
via_rulebook = (
|
||||
select(RulebookTopic.id)
|
||||
.join(Rulebook, RulebookTopic.rulebook_id == Rulebook.id)
|
||||
.where(
|
||||
Rulebook.owner_user_id == user_id,
|
||||
RulebookTopic.deleted_at.is_(None),
|
||||
Rulebook.deleted_at.is_(None),
|
||||
)
|
||||
)
|
||||
via_project = select(Project.id).where(
|
||||
Project.user_id == user_id, Project.deleted_at.is_(None),
|
||||
)
|
||||
return and_(
|
||||
Rule.deleted_at.is_(None),
|
||||
or_(Rule.topic_id.in_(via_rulebook), Rule.project_id.in_(via_project)),
|
||||
)
|
||||
|
||||
|
||||
async def recent_preference_drift(user_id: int, limit: int = 20) -> list[dict]:
|
||||
"""Preferences that have been rewritten, most recently changed first.
|
||||
|
||||
ONE ROW PER PREFERENCE, carrying its LATEST rewrite — not every version of
|
||||
every preference. The question this answers is "what has Scribe changed
|
||||
about how it works with me lately", and a preference rewritten eight times
|
||||
this week is one answer to that, not eight. The full history of any one
|
||||
preference is still a click away in its editor, which is where "how did
|
||||
this get here" belongs.
|
||||
|
||||
Each row carries what it said before, what it says now, and `taught_by` —
|
||||
the record named by `arose_from_id`, which step 2 made required on every
|
||||
agent-written preference. That is the provenance: not who typed it (the
|
||||
agent acts as the operator's own user, so the actor column cannot tell
|
||||
them apart) but what the change was learned from.
|
||||
|
||||
Rules are excluded, and not as a filter that could be relaxed. A rule
|
||||
changes when its author changes it, so "what changed without me" is not a
|
||||
question about rules — including them would bury the few rows that are
|
||||
actually unreviewed under every edit the operator made themselves.
|
||||
"""
|
||||
limit = max(1, min(int(limit), _DRIFT_LIMIT_MAX))
|
||||
# The newest version per rule, by id rather than by created_at: a version
|
||||
# is written once and never updated, so id order IS time order, and two
|
||||
# versions written in the same clock tick still have a defined winner.
|
||||
newest = (
|
||||
select(
|
||||
RuleVersion.rule_id.label("rule_id"),
|
||||
func.max(RuleVersion.id).label("version_id"),
|
||||
)
|
||||
.group_by(RuleVersion.rule_id)
|
||||
.subquery()
|
||||
)
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(Rule, RuleVersion)
|
||||
.join(newest, newest.c.rule_id == Rule.id)
|
||||
.join(RuleVersion, RuleVersion.id == newest.c.version_id)
|
||||
.where(Rule.kind == "preference", _owned_rules_clause(user_id))
|
||||
.order_by(RuleVersion.created_at.desc(), RuleVersion.id.desc())
|
||||
.limit(limit)
|
||||
)).all()
|
||||
|
||||
# One query for every provenance record, not one per row. These are
|
||||
# titles for rows the caller can already see, so they are read without
|
||||
# a further ownership filter — the same reasoning as
|
||||
# milestones.titles_for, and blanking them would leave the operator a
|
||||
# bare id where the whole point is naming the record.
|
||||
taught_ids = {r.arose_from_id for r, _v in rows if r.arose_from_id}
|
||||
titles: dict[int, str] = {}
|
||||
if taught_ids:
|
||||
from scribe.models.note import Note
|
||||
|
||||
titles = dict((await session.execute(
|
||||
select(Note.id, Note.title).where(
|
||||
Note.id.in_(taught_ids), Note.deleted_at.is_(None),
|
||||
)
|
||||
)).all())
|
||||
|
||||
out: list[dict] = []
|
||||
for rule, version in rows:
|
||||
row: dict = {
|
||||
# rule_brief, not a hand-written dict: the drift pane links
|
||||
# straight into the editor, so the row has to be the same shape
|
||||
# every other rule listing is (#3313's three-copies lesson).
|
||||
"rule": rule_brief(rule),
|
||||
# What it said BEFORE this rewrite. `statement` and
|
||||
# `when_to_apply` only — those are the two fields that change how
|
||||
# a session behaves, and a diff of `why` is reading rather than
|
||||
# reviewing.
|
||||
"previous": {
|
||||
"id": version.id,
|
||||
"created_at": iso(version.created_at),
|
||||
"title": version.title or "",
|
||||
"statement": version.statement or "",
|
||||
"when_to_apply": version.when_to_apply or "",
|
||||
},
|
||||
# The text it changed TO, beside the text it changed from, so the
|
||||
# client can render the diff without a second call per row. A
|
||||
# listing that needs N follow-ups to say what it means is one
|
||||
# nobody scrolls.
|
||||
"current": {
|
||||
"title": rule.title,
|
||||
"statement": rule.statement or "",
|
||||
"when_to_apply": rule.when_to_apply or "",
|
||||
},
|
||||
}
|
||||
if rule.arose_from_id and rule.arose_from_id in titles:
|
||||
row["taught_by"] = {
|
||||
"id": rule.arose_from_id, "title": titles[rule.arose_from_id],
|
||||
}
|
||||
out.append(row)
|
||||
return out
|
||||
|
||||
|
||||
async def restore_rule_version(
|
||||
rule_id: int, version_id: int, user_id: int,
|
||||
) -> Optional[Rule]:
|
||||
"""Put a preference back to what it said. None when it is not readable.
|
||||
|
||||
MILESTONE 323 DECIDED THE OPPOSITE FOR RULES, and that decision stands —
|
||||
`routes/rulebooks.py` still has no restore for them. Its reasoning:
|
||||
"a binding instruction should not be revertible in one click", because a
|
||||
silent revert erases the only record of why the rewrite happened.
|
||||
|
||||
A preference inverts both halves of that. The rewrite was not the
|
||||
operator's — the agent makes it mid-work, without asking, which is the
|
||||
whole design — so reverting is not undoing their own decision but
|
||||
exercising a veto over someone else's. And the veto has to be cheaper
|
||||
than shrugging, or drift is only nominally supervised.
|
||||
|
||||
What makes it safe is that nothing is erased. This goes through
|
||||
`update_rule` like any other edit, so the restore takes its own snapshot:
|
||||
the rewrite stays in the history, with the revert recorded after it. The
|
||||
history gains an entry rather than losing one.
|
||||
|
||||
Raises ValueError on a rule, so the kind check cannot be forgotten by a
|
||||
caller that reaches this directly.
|
||||
"""
|
||||
async with async_session() as session:
|
||||
rule = await _fetch_owned_rule(session, rule_id, user_id)
|
||||
if rule is None:
|
||||
return None
|
||||
if (rule.kind or "rule") != "preference":
|
||||
raise ValueError(
|
||||
"only a preference can be restored in one action. A rule is "
|
||||
"the operator's own decision and reverting it goes through an "
|
||||
"ordinary edit, so the reason for the rewrite stays visible "
|
||||
"(milestone 323)."
|
||||
)
|
||||
version = (await session.execute(
|
||||
select(RuleVersion).where(
|
||||
RuleVersion.id == version_id, RuleVersion.rule_id == rule_id,
|
||||
)
|
||||
)).scalar_one_or_none()
|
||||
if version is None:
|
||||
return None
|
||||
fields = {
|
||||
"title": version.title,
|
||||
"statement": version.statement,
|
||||
"when_to_apply": version.when_to_apply,
|
||||
"why": version.why,
|
||||
"how_to_apply": version.how_to_apply,
|
||||
}
|
||||
|
||||
# Outside the session above, because update_rule opens its own — and
|
||||
# through it rather than beside it, so the snapshot, the trigger guard and
|
||||
# the embedding refresh all happen exactly as they do for a hand edit.
|
||||
# A `None` field in the snapshot means the preference had nothing there,
|
||||
# so it is CLEARED rather than left at today's value; passing None to
|
||||
# update_rule means "leave alone", which would half-restore it.
|
||||
clear = [k for k, v in fields.items() if not (v or "").strip()]
|
||||
return await update_rule(
|
||||
rule_id, user_id,
|
||||
clear=[k for k in clear if k != "statement"],
|
||||
**{k: v for k, v in fields.items() if v},
|
||||
)
|
||||
|
||||
|
||||
# ── Canon tags + typed edges (milestone 307) ───────────────────────────
|
||||
|
||||
async def set_rule_systems(
|
||||
|
||||
Reference in New Issue
Block a user