From 0e10f6bb8a47fcd995caa9ec1156ea2b7eb6691f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 11 Sep 2026 16:22:17 -0400 Subject: [PATCH] =?UTF-8?q?feat(rules)!:=20retire=20the=20always-on=20tier?= =?UTF-8?q?=20=E2=80=94=20every=20rule=20arrives=20by=20retrieval=20(#394)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Milestone 394, steps 5-8. Operator: "remove the always on rule functionality as the goal was to not have it at all since it didn't seem to work as expected." Unconditional preload had three failures the retrieval arms do not. It could not be MEASURED — a resident rule is in the context whether or not it mattered, so nothing distinguished "this governed the act" from "this was scenery", and it was the one surface structurally exempt from the scoreboard judging every other. It was SUMMARISED AWAY by compaction while the session went on believing it held the rules. And it CROWDED OUT the few rules that applied with the thirty that did not. WHAT GOES Schema (0100): rules.tier + ck_rules_tier, rule_versions.tier, rulebooks.always_on, and project_rulebook_exclusions — a table recording a project's opt-out of something that no longer binds it unasked. Tools: list_always_on_rules, exclude_always_on_rulebook, include_always_on_rulebook. Service: the same three plus rules_etag_for, _valid_tier and the whole etag family. The SessionStart preload and the write-path staleness arm go with them: nothing is resident, so nothing can have drifted since a session loaded it. THREE CALLS WORTH REVIEWING enter_project got NARROWER, not wider. Its filter was `always_on OR area-tagged`; dropping the tier arm leaves the deterministic half, so a project with no canonical-tagged Systems gets no bulk rules and reaches them by retrieval instead. Dropping the whole clause would have made that payload bigger than the preload this milestone deletes. Backups import tolerantly. A pre-394 archive carries tier, always_on and the retired inception choice; none is read, and the exclusion key is DROPPED rather than remapped, because restoring it would write data that validate_inception now rejects as unknown. The migration is irreversible in the way that matters and says so: downgrade recreates the columns at their defaults and cannot restore which rules were always-on. A value invented to fill a hole is not a measurement. THE INSTRUCTION SURFACES SAY THE HARDER THING Deleting "call list_always_on_rules()" is easy; replacing it is not, because the new model asks a session to trust something it cannot see. All three surfaces now say a session holds nothing, that rules arrive when work matches them, and — the half that got dangerous — that "no rule arrived" means "nothing matched", never "there is no rule". Under residency an empty session was rare and suspicious; it is now the ordinary state of most turns, so reading it as permission is wrong on nearly every turn rather than occasionally. That is #3720's defect at session scale. test_instruction_surfaces_agree is repointed rather than retired: its two halves collapsed into one instruction, and it gains a guard that every surface states what absence means. _INSTRUCTIONS is back at 1999/2000 — the inception clause paid for the longer HOW line. UI (rule 27, and the opportunity step 8 named) The tier selector is gone, and what replaces it is the point: `when_to_apply` is now the field that decides whether a rule is ever seen, so the editor marks it required, warns while it is empty, and both rule lists badge a trigger-less rule "never surfaces". A rule without one is not quiet, it is unreachable. TESTS Two files deleted outright — test_rules_etag.py and test_inception_rules.py tested subsystems that no longer exist. Elsewhere obsolete cases were removed and the rest repointed. One deserves naming: the wiring test asserted the act arms pass no `tier`, which had become an assertion that could not fail. It is repointed onto `kind`, which does still exist and where the same claim is live — a preference must reach a write exactly as a rule does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ --- alembic/versions/0100_drop_always_on_tier.py | 96 +++++++ frontend/src/api/inception.ts | 7 +- frontend/src/api/rulebooks.ts | 27 +- frontend/src/assets/rules-shared.css | 9 + frontend/src/components/InceptionCard.vue | 28 +- .../src/components/rules/ProjectRulesTab.vue | 48 +--- .../components/rules/RuleEditorSlideOver.vue | 63 ++--- .../src/components/rules/RuleHistoryPanel.vue | 3 +- .../src/components/rules/RuleListPane.vue | 11 +- .../src/components/rules/RuleSweepPane.vue | 19 +- .../components/rules/RulebookDetailPane.vue | 14 - .../src/components/rules/RulebookListPane.vue | 11 - frontend/src/stores/rulebooks.ts | 15 +- frontend/src/views/ProjectView.vue | 3 - plugin/hooks/scribe_session_context.sh | 26 +- src/scribe/models/rulebook.py | 17 +- src/scribe/services/backup.py | 43 +-- src/scribe/services/plugin_context.py | 12 - tests/helpers.py | 2 +- tests/test_inception.py | 19 +- tests/test_inception_rules.py | 69 ----- tests/test_instruction_surfaces_agree.py | 143 ++++++---- ...tegration_backup_rule_version_roundtrip.py | 3 - tests/test_integration_inception.py | 44 ++- tests/test_integration_rule_surfacing.py | 76 ++--- tests/test_integration_rule_verification.py | 9 +- tests/test_mcp_tool_projects.py | 8 +- tests/test_mcp_tool_rulebooks.py | 55 +--- tests/test_routes_rulebooks.py | 18 +- tests/test_rule_ledger_ageing.py | 5 +- tests/test_rule_usage_wiring.py | 23 +- tests/test_rules_etag.py | 267 ------------------ tests/test_services_backup.py | 8 +- tests/test_services_plugin_context.py | 99 ++----- tests/test_services_rulebooks.py | 27 +- 35 files changed, 388 insertions(+), 939 deletions(-) create mode 100644 alembic/versions/0100_drop_always_on_tier.py delete mode 100644 tests/test_inception_rules.py delete mode 100644 tests/test_rules_etag.py diff --git a/alembic/versions/0100_drop_always_on_tier.py b/alembic/versions/0100_drop_always_on_tier.py new file mode 100644 index 0000000..a524281 --- /dev/null +++ b/alembic/versions/0100_drop_always_on_tier.py @@ -0,0 +1,96 @@ +"""drop the always-on tier: rules.tier, rulebooks.always_on, the exclusions table + +Revision ID: 0100 +Revises: 0099 +Create Date: 2026-09-11 + +Milestone 394. Every rule now reaches a session by retrieval — because +something it is about to do made the rule relevant — and the machinery that +delivered rules unconditionally goes with it. + +WHAT GOES, AND WHERE IT CAME FROM + + - ``rules.tier`` and its ``ck_rules_tier`` CHECK (migration 0088). Dropping + the column takes the constraint with it. Rule 36 is about ADDING a value to + a live whitelist, which needs DROP + ADD in the same migration; it does not + speak to removing the column outright, and saying so here is cheaper than + the next reader wondering whether it was forgotten. + - ``rule_versions.tier`` (migration 0098). A version records what a rule + SAID; with no tier on a rule there is nothing for a snapshot to carry. + - ``rulebooks.always_on`` (migration 0058). A rulebook reaches a project by + subscription now, and by nothing else. + - ``project_rulebook_exclusions`` (migration 0085). It recorded a project's + opt-out of an always-on rulebook. Opting out of something that no longer + binds you is not a state that can exist — declining a rulebook is + expressed by not subscribing to it. + +IRREVERSIBLE, AND THE DOWNGRADE SAYS SO RATHER THAN PRETENDING + +The downgrade recreates the columns and the table with their DEFAULTS. It +cannot restore WHICH rules were always-on, which rulebooks bound every project, +or which projects had opted out — that information is in what this drops. + +That distinction is the one this repo keeps insisting on: a value invented to +fill a hole is not a measurement. So a downgraded database is structurally able +to run the old code and is NOT the database the old code was running against — +every rule comes back at the ``always_on`` default, which for the tier column +happens to mean "binding", the safe direction to be wrong in. + +Anyone who needs the real prior state restores a backup taken before this ran. +""" +import sqlalchemy as sa +from alembic import op + +revision = "0100" +down_revision = "0099" +branch_labels = None +depends_on = None + +_TIERS = ("always_on", "conditional") + + +def _in_list(column: str, values: tuple[str, ...]) -> str: + return f"{column} IN (" + ", ".join(f"'{v}'" for v in values) + ")" + + +def upgrade() -> None: + op.drop_table("project_rulebook_exclusions") + op.drop_column("rulebooks", "always_on") + op.drop_column("rule_versions", "tier") + # The CHECK goes with the column it constrains; naming it here would be a + # second drop of the same object. + op.drop_column("rules", "tier") + + +def downgrade() -> None: + """Structure only. See the module docstring — the values are gone.""" + op.add_column( + "rules", + sa.Column("tier", sa.Text(), nullable=False, server_default="always_on"), + ) + op.create_check_constraint("ck_rules_tier", "rules", _in_list("tier", _TIERS)) + op.add_column("rule_versions", sa.Column("tier", sa.Text(), nullable=True)) + op.add_column( + "rulebooks", + sa.Column( + "always_on", sa.Boolean(), nullable=False, + server_default=sa.text("false"), + ), + ) + op.create_table( + "project_rulebook_exclusions", + sa.Column( + "project_id", sa.BigInteger(), + sa.ForeignKey("projects.id", ondelete="CASCADE"), + primary_key=True, nullable=False, + ), + sa.Column( + "rulebook_id", sa.BigInteger(), + sa.ForeignKey("rulebooks.id", ondelete="CASCADE"), + primary_key=True, nullable=False, + ), + sa.Column( + "created_at", sa.DateTime(timezone=True), + server_default=sa.text("now()"), nullable=True, + ), + ) diff --git a/frontend/src/api/inception.ts b/frontend/src/api/inception.ts index 05773e1..0321c78 100644 --- a/frontend/src/api/inception.ts +++ b/frontend/src/api/inception.ts @@ -2,7 +2,6 @@ import { apiGet, apiPost } from "@/api/client"; export interface InceptionChoices { - exclude_always_on_rulebooks: number[]; subscribe_rulebooks: number[]; design_system_id: number | null; seed_systems: boolean; @@ -16,9 +15,7 @@ export interface InceptionRecord { } export interface InceptionDefaults { - always_on_rulebooks: { id: number; title: string }[]; - other_rulebooks: { id: number; title: string }[]; - excluded_always_on: { id: number; title: string }[]; + rulebooks: { id: number; title: string }[]; subscribed_rulebooks: { id: number; title: string }[]; design_system_id: number | null; design_systems: { id: number; title: string }[]; @@ -32,7 +29,7 @@ export interface InceptionDecision { } export const emptyChoices = (): InceptionChoices => ({ - exclude_always_on_rulebooks: [], subscribe_rulebooks: [], design_system_id: null, seed_systems: false, + subscribe_rulebooks: [], design_system_id: null, seed_systems: false, }); export const fetchInceptionDefaults = (projectId: number) => diff --git a/frontend/src/api/rulebooks.ts b/frontend/src/api/rulebooks.ts index 1a8c175..7b0fca1 100644 --- a/frontend/src/api/rulebooks.ts +++ b/frontend/src/api/rulebooks.ts @@ -3,7 +3,6 @@ import type { RecordUsage } from "@/types/usage"; import { apiGet, apiPost, apiPatch, apiDelete } from "@/api/client"; /** How a rule reaches a session (milestone 307). */ -export type RuleTier = "always_on" | "conditional"; /** * A typed edge between two rules. Each kind exists because its absence forced @@ -26,7 +25,6 @@ export interface Rulebook { owner_user_id: number; title: string; description: string; - always_on: boolean; created_at: string | null; updated_at: string | null; } @@ -49,12 +47,6 @@ export interface Rule { statement: string; /** WHEN this rule fires — the trigger, not the instruction. */ when_to_apply: string; - /** - * always_on preloads into every session; conditional is reachable and - * surfaced when its trigger fires. A rule with no tier set behaves as - * always_on, which is how every rule behaved before this existed. - */ - tier: RuleTier; why: string; how_to_apply: string; /** @@ -87,7 +79,6 @@ export interface RuleHeader { title: string; statement: string; topic_id: number | null; - tier: RuleTier; /** A date (YYYY-MM-DD), not a timestamp. */ updated_at: string | null; when_to_apply?: string; @@ -134,7 +125,6 @@ export interface ApplicableRules { truncated: boolean; subscribed_rulebooks: { id: number; title: string }[]; /** Always-on rulebooks this project opted out of at inception (milestone 297). */ - excluded_always_on: { id: number; title: string }[]; } // ── Rulebooks ─────────────────────────────────────────────────────── @@ -152,7 +142,7 @@ export async function createRulebook(data: { title: string; description?: string return apiPost("/api/rulebooks", data); } -export async function updateRulebook(id: number, data: Partial<{ title: string; description: string; always_on: boolean }>): Promise { +export async function updateRulebook(id: number, data: Partial<{ title: string; description: string }>): Promise { return apiPatch(`/api/rulebooks/${id}`, data); } @@ -207,7 +197,6 @@ export interface RuleWrite { title: string; statement: string; when_to_apply: string; - tier: RuleTier; why: string; how_to_apply: string; order_index: number; @@ -258,7 +247,6 @@ export interface RuleVersion { why?: string; how_to_apply?: string; when_to_apply?: string; - tier?: string; verify_with?: string; expires_when?: string; } @@ -323,16 +311,6 @@ export async function unsuppressTopicForProject(projectId: number, topicId: numb return apiDelete(`/api/projects/${projectId}/suppressions/topics/${topicId}`); } -// ── Always-on exclusions (milestone 297) ──────────────────────────────────── - -export async function excludeAlwaysOnRulebook(projectId: number, rulebookId: number): Promise { - await apiPost(`/api/projects/${projectId}/exclusions/rulebooks/${rulebookId}`, {}); -} - -export async function includeAlwaysOnRulebook(projectId: number, rulebookId: number): Promise { - await apiDelete(`/api/projects/${projectId}/exclusions/rulebooks/${rulebookId}`); -} - /** * One row of the staleness sweep. Unlike RuleHeader this carries the CHECK @@ -343,7 +321,6 @@ export interface RuleVerificationRow { id: number; title: string; statement: string; - tier: RuleTier; topic_id: number | null; project_id: number | null; when_to_apply: string; @@ -366,12 +343,10 @@ export interface RuleVerificationRow { */ export async function listRulesDueForVerification(opts: { olderThanDays?: number; - tier?: RuleTier; neverOnly?: boolean; } = {}): Promise<{ rules: RuleVerificationRow[]; total: number }> { const q = new URLSearchParams(); if (opts.olderThanDays) q.set("older_than_days", String(opts.olderThanDays)); - if (opts.tier) q.set("tier", opts.tier); if (opts.neverOnly) q.set("never_only", "true"); const qs = q.toString(); return apiGet(`/api/rules-due-for-verification${qs ? `?${qs}` : ""}`); diff --git a/frontend/src/assets/rules-shared.css b/frontend/src/assets/rules-shared.css index 72c1448..f2345f3 100644 --- a/frontend/src/assets/rules-shared.css +++ b/frontend/src/assets/rules-shared.css @@ -20,6 +20,15 @@ milestone (tier, then verification) and were byte-identical; a third would have drifted. The pane's italic serif title is inherited by anything inside it, so the chip resets family and style explicitly. */ +/* A rule with no trigger cannot be retrieved, and since milestone 394 + retrieval is the only delivery — so this marks a rule that will never + reach a session. Warning rather than error: the rule is not broken, it is + unreachable, and the fix is one field away. */ +.rule-chip-inert { + color: var(--fs-warning-fg); + background: color-mix(in srgb, var(--fs-warning) 12%, var(--fs-surface-raised)); +} + .rule-chip { margin-left: 0.4rem; font-family: var(--fs-font-body); diff --git a/frontend/src/components/InceptionCard.vue b/frontend/src/components/InceptionCard.vue index e33f842..f775f6f 100644 --- a/frontend/src/components/InceptionCard.vue +++ b/frontend/src/components/InceptionCard.vue @@ -28,7 +28,6 @@ const emit = defineEmits<{ }>(); const local = ref(props.choices ? { ...props.choices } : emptyChoices()); -const alwaysOn = ref<{ id: number; title: string }[]>([]); const others = ref<{ id: number; title: string }[]>([]); const designSystems = ref<{ id: number; title: string }[]>([]); const systemsCount = ref(0); @@ -47,21 +46,18 @@ async function load() { try { if (props.mode === "decide" && props.projectId) { const d: InceptionDefaults = await fetchInceptionDefaults(props.projectId); - alwaysOn.value = d.always_on_rulebooks; - others.value = d.other_rulebooks; + others.value = d.rulebooks; designSystems.value = d.design_systems; systemsCount.value = d.systems; // Start from what stands today so "record" without changes is a true inherit-all. local.value = { - exclude_always_on_rulebooks: d.excluded_always_on.map((r) => r.id), subscribe_rulebooks: d.subscribed_rulebooks.map((r) => r.id), design_system_id: d.design_system_id, seed_systems: false, }; } else { const [rulebooks, ds] = await Promise.all([listRulebooks(), fetchDesignSystems()]); - alwaysOn.value = rulebooks.filter((r) => r.always_on).map((r) => ({ id: r.id, title: r.title })); - others.value = rulebooks.filter((r) => !r.always_on).map((r) => ({ id: r.id, title: r.title })); + others.value = rulebooks.map((r) => ({ id: r.id, title: r.title })); designSystems.value = ds.design_systems.map((d) => ({ id: d.id, title: d.title })); } } catch (e: unknown) { @@ -71,13 +67,6 @@ async function load() { } } -function inherits(id: number): boolean { - return !local.value.exclude_always_on_rulebooks.includes(id); -} -function toggleInherit(id: number) { - const list = local.value.exclude_always_on_rulebooks; - local.value.exclude_always_on_rulebooks = list.includes(id) ? list.filter((x) => x !== id) : [...list, id]; -} function subscribed(id: number): boolean { return local.value.subscribe_rulebooks.includes(id); } @@ -87,7 +76,7 @@ function toggleSubscribe(id: number) { } const nothingToDecide = computed( - () => !alwaysOn.value.length && !others.value.length && !designSystems.value.length, + () => !others.value.length && !designSystems.value.length, ); async function record() { @@ -118,16 +107,11 @@ onMounted(load);

Loading…

{{ error }}