From f0be7ebef37fba2ab17483ac4e63ba5b5d3e155e Mon Sep 17 00:00:00 2001 From: jinhongkuan Date: Fri, 15 May 2026 22:54:12 -0700 Subject: [PATCH] =?UTF-8?q?docs(dev-cycle):=20add=20=C2=A74.7=20(expand-on?= =?UTF-8?q?ly=20+=20flag-gate)=20and=20amend=20=C2=A710.5.1=20(triage=20el?= =?UTF-8?q?igibility)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Encodes the three architectural decisions ratified 2026-05-15 (decision:cp25jfz1nt6h3u2gjzmu, decision:adklplvfhthkdch05pe9, decision:0ok1249n2tdrfud2a5j9): §4.7 — new subsection enforcing two complementary rules for any PR that touches ledger/schema.py or its _MIGRATIONS registry: §4.7.1 — schema migrations must be expand-only. Destructive operations (REMOVE / DROP / breaking ALTER / tightening ASSERT) live in their own commits and ship in a later release after the prior reader surface is validated as gone from prod. Includes an allowed/forbidden table for reviewer ease. CI lint planned via scripts/lint_schema_destructive.py. §4.7.2 — code paths that depend on new schema must be feature-flag gated and default OFF in prod (env var or .bicameral/config.yaml setting). Schema ships immediately; flag flips later in a separate release. If the experiment is killed, the flag never flips on and a follow-up cleanup migration drops the slot. Exception: invariant bugfixes (e.g. fixing a unique-index collision that breaks the dashboard for everyone) don't need flag-gating — that's not feature surface. §4.7.3 — concrete PR-review checklist for schema-touching PRs. §10.5.1 — triage eligibility rule rewritten. Previously: "schema-migrating changes are not triage-eligible" (blanket). Now: schema migrations CAN ride a triage release if they comply with §4.7 (expand-only AND feature code is flag-gated). The blanket ban is replaced by enumerated exclusions (destructive schema, flag-flip releases, breaking public-API changes, multi-PR epics, v1 patches). Motivation: the prior rule was correct under the implicit assumption that schema and feature ship together — then you can't ship one without the other. Once §4.7 decouples them, schema can drain to main on every triage instead of accumulating on dev waiting for a "real" release. The current v18→v24 backlog (drained by the v0.15.0 release PR #388) is the symptom the prior rule produced; this amendment prevents recurrence. Refs decision:cp25jfz1nt6h3u2gjzmu, decision:adklplvfhthkdch05pe9, decision:0ok1249n2tdrfud2a5j9. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/DEV_CYCLE.md | 107 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/docs/DEV_CYCLE.md b/docs/DEV_CYCLE.md index bdd5d879..e9162ab2 100644 --- a/docs/DEV_CYCLE.md +++ b/docs/DEV_CYCLE.md @@ -600,6 +600,84 @@ CodeRabbit, Devin, and human reviewers all leave comments. The author's job: their line anchors. Use `--force-with-lease` only after a `git fetch`, and call it out in a PR comment so reviewers re-fetch. +### 4.7 Schema-touching PRs — expand-only + flag-gate + +> **Why this rule exists.** Migration tools (Alembic, Rails, Django, Flyway, +> Liquibase, the bicameral `_MIGRATIONS` registry) all treat the schema chain +> as append-only — you can't cleanly skip a middle migration. When experimental +> features piggyback their schema onto feature commits, killed experiments +> leave schema fragments in the chain that prod still has to apply, and the +> only escape is a "next full release drains everything" model that pushes +> migrations to main in big lumps. The 2026-05-15 cross-tool research survey +> (`decision:cp25jfz1nt6h3u2gjzmu`) confirmed every major ecosystem has +> converged on the same answer: **decouple schema from features**. Ship the +> schema; gate the feature. This subsection encodes that doctrine. + +Two ratified architectural decisions (`decision:cp25jfz1nt6h3u2gjzmu` + +`decision:adklplvfhthkdch05pe9`) govern any PR that touches `ledger/schema.py` +or its `_MIGRATIONS` registry: + +#### 4.7.1 Migrations must be expand-only + +A migration commit may contain **only additive** schema operations: + +| ✅ Allowed | ❌ Forbidden in the same migration commit | +|---|---| +| `DEFINE FIELD x ON t TYPE option<…> DEFAULT …` (new optional field, has default) | `REMOVE FIELD x ON t` | +| `DEFINE TABLE t SCHEMAFULL …` (new table) | `REMOVE TABLE t` | +| `DEFINE INDEX i ON t FIELDS …` (new index) | `REMOVE INDEX i` | +| `DEFINE INDEX OVERWRITE i ON t FIELDS …` that *only relaxes* uniqueness or adds discriminator fields (rows valid pre-change remain valid post-change) | `DEFINE INDEX OVERWRITE i ON t FIELDS …` that tightens uniqueness or drops fields | +| `DEFINE FIELD x ON t TYPE string DEFAULT '' ASSERT $value != ''` (new field, but rejects empty so existing rows must already satisfy it — only allowed when paired with a backfill that runs *first* in the same migration function) | `ALTER` that breaks existing readers (changed TYPE, added required field without default, narrowed ASSERT) | + +Destructive operations are not banned outright — they live in their **own +dedicated commit** and ship in a **later release**, after every reader of the +deprecated surface has been validated as removed from prod. The kernel-style +deprecate-then-remove cycle. PR reviewers reject schema PRs that mix additive +and destructive ops in one commit. + +CI lint (planned, `scripts/lint_schema_destructive.py`) parses +`ledger/schema.py` diffs and flags `REMOVE …` / breaking `ALTER` / +tightening-ASSERT operations in any commit that *also* modifies non-migration +code. Until the lint ships, this rule is enforced by reviewer attention. + +#### 4.7.2 New-schema-dependent code must be feature-flag gated + +A code path that depends on a newly-added schema element must be wrapped in +a flag that defaults OFF in prod. Two acceptable shapes: + +- **Env var** — `BICAMERAL_FEATURE_X=1` to enable. Read in the handler via + `os.environ.get("BICAMERAL_FEATURE_X", "0") == "1"`. +- **Config setting** — surfaced through `.bicameral/config.yaml` with a + default-off entry in `setup_wizard`. + +The schema ships immediately in the next release. The flag stays off in prod +until a **separate** release flips its default. If the experiment is killed, +the flag never flips on, and a follow-up cleanup migration (destructive, +following §4.7.1's separate-commit rule) drops the schema slot in a later +release — never via history rewrite on the original migration. + +**This rule does not apply** to schema changes that the binary itself +unconditionally depends on (e.g. fixing a unique-index collision that breaks +the dashboard for everyone). Those are bugfixes, not features; the flag-gate +applies to *new feature surface*, not to invariants. + +#### 4.7.3 PR review checklist for schema-touching PRs + +Reviewers MUST verify, before approving any PR that bumps `SCHEMA_VERSION`: + +- [ ] Every operation in the new migration function is additive per §4.7.1. +- [ ] If the PR also adds feature code that reads/writes the new schema, + the feature path is wrapped in an env-var or config flag, defaulting OFF. +- [ ] If the PR adds destructive cleanup of an earlier feature's schema, that + destructive operation is in a separate commit *and* the migration + docstring names the prior decision/PR that introduced the surface being + cleaned up. +- [ ] The migration is exercised by at least one sociable test that exercises + a forward-migration path on `memory://` (`init_schema` + `migrate`, + then a read+write that validates the new shape). + +Failure on any of these = changes-requested. + --- ## 5. Merging to `dev` @@ -1027,10 +1105,31 @@ A commit is triage-eligible if **all** of: - It does not depend on `dev`-only refactors that haven't shipped to `main`. If it does, the prerequisites must be triage-eligible too, and they all cherry-pick as a coherent batch. - -**Not triage-eligible** by default: schema-migrating changes, breaking -public-API changes, multi-PR feature epics, "v1 patches" (the catch-all -`triage-from-dev` PR title uses for work explicitly held for the next major). +- If it touches `ledger/schema.py` or any migration function, it complies with + **§4.7 (expand-only + flag-gate)**. Schema migrations that are strictly + additive AND any feature code that reads the new schema is flag-gated off in + prod are triage-eligible. This amendment supersedes the prior blanket + "schema-migrating changes are not triage-eligible" rule + (`decision:0ok1249n2tdrfud2a5j9`, 2026-05-15). + +**Not triage-eligible** by default: + +- **Destructive schema changes** — any `REMOVE` / `DROP` / breaking `ALTER` / + tightening-ASSERT operation per §4.7.1. These ship only in full `dev → main` + releases, after the prior feature surface has been demonstrably removed from + prod readers. +- **Flag-flip releases** — a release whose code change is "set the default of + an existing feature flag from off to on." Flipping a flag is a behavior + change, not a fix; it goes through the normal `dev → main` release path so + CHANGELOG, eval gates, and rollback story are explicit. +- **Breaking public-API changes** — including MCP tool contract changes that + remove fields, change response shape, or rename tools. Migration shims + inside `bicameral.update` may carry across a triage but the breaking change + itself ships in a full release. +- **Multi-PR feature epics** — work that spans more than one PR and needs all + parts to ship together for coherence. +- **"v1 patches"** — the catch-all `triage-from-dev` PR title uses for work + explicitly held for the next major. When in doubt, the change waits for the next `dev → main` release.