Skip to content

docs(architecture): say who decides what is open source, and why a feature's switch never touches its schema - #1405

Open
peteski22 wants to merge 2 commits into
mainfrom
docs/who-decides-what-is-open-source
Open

peteski22 wants to merge 2 commits into
mainfrom
docs/who-decides-what-is-open-source

Conversation

@peteski22

@peteski22 peteski22 commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Description

Two additions to ARCHITECTURE.md. Neither changes any code or any behavior. Both write down something the document already assumed.

1. Who decides whether a feature is part of the open-source product. "Where new code goes" picks a home by what is being added, and every row of it assumes someone has already classified the thing: "an optional feature any deployment may run" goes to core, "only an overlay ships" goes to an overlay. Nothing said who makes that call, so the table could be read as making it. The new paragraph says the classification comes first, that it belongs to the project's product and engineering leads together, and that a feature not yet confirmed starts in an overlay. The reason for that default is that the move only works one way: a feature proven in an overlay can move here later, and a feature released here can never be withdrawn.

2. Why a feature's switch never touches its schema. Step 3 of "How to add a core feature" states the rule (switching a feature off leaves its tables in place, switching it on changes no schema) with no reasoning, so it reads as arbitrary and open to renegotiation. The new subsection gives the four reasons, each checked against the code as it is on main:

  • Migrations run in-process at startup with no lock around them (_run_migrations in core/database.py), so a switch that created tables would have every replica run the same DDL at once.
  • With one chain and one head, alembic_version is a function of the release. With a branch per feature it depends on the order switches were flipped.
  • alembic/env.py refuses a database stamped with any revision the image does not know, so a feature switched on once would expose every later rollback.
  • One chain means the suite tests the schema every deployment has. N independent features means 2^N schemas in the field.

The rule this adds, per the checklist: a feature's place in the open-source product is confirmed by the project's product and engineering leads before "Where new code goes" applies. The schema rule is not new. It has been in step 3 since #1279 and in #1173's acceptance criteria; this PR adds only its reasoning.

How to test it locally

Documentation only. make lint passes. The new in-page link from step 3 resolves to the new subsection's heading, and the existing links into this section from CONTRIBUTING.md and src/gateway/AGENTS.md still resolve, because the heading they target is unchanged.

Worth reading rather than running: the four claims in the new subsection, against src/gateway/core/database.py and alembic/env.py.

PR Type

  • New Feature
  • Bug Fix
  • Refactor
  • Documentation
  • Infrastructure / CI

Relevant issues

Refs #1173, where the schema rule was set, and #1363, which proposes changing it and is the reason to put the reasoning on the record.

Checklist

  • I understand the code I am submitting.
  • I have added or updated tests that cover my change (tests/unit, tests/integration).
  • I ran the Definition of Done checks locally (make lint, make typecheck, make test).
  • Documentation was updated where necessary.
  • If the API contract changed, I regenerated the OpenAPI spec (uv run python scripts/generate_openapi.py).
  • If this changes a rule in ARCHITECTURE.md or scripts/check_architecture.py, the description names the rule and says why.

AI Usage

  • No AI was used.
  • AI was used for drafting/refactoring.
  • This is fully AI-generated.

AI Model/Tool used:

Claude, via Claude Code.

Any additional AI details you'd like to share:

The wording is AI-written. The decisions in it are not: who decides, and the overlay-first default, were settled by a person before anything was drafted. Each technical claim was checked against main first, and one sentence in the draft was corrected as a result (the gateway does not inspect the schema when auto_migrate is off, so it would not refuse to start on its own).

  • I am an AI Agent filling out this form (check box if true)

Summary

  • Added guidance in ARCHITECTURE.md on who decides whether a feature belongs in the open-source product.
  • Clarified that unconfirmed features start in an overlay and may move to core later.
  • Explained why feature switches do not change database schemas.
  • No code or behavior changes.

Validation

  • The author reports that make lint passes and relevant documentation links resolve.

@peteski22
peteski22 requested a review from a team as a code owner September 18, 2026 20:18
@peteski22
peteski22 requested review from njbrake and tbille and removed request for njbrake and tbille September 18, 2026 20:18
@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: mozilla-ai/otari/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e0ca182f-46c3-49fa-b60b-a3a5ac225e58

📥 Commits

Reviewing files that changed from the base of the PR and between 444cc54 and b1afc3c.

📒 Files selected for processing (1)
  • ARCHITECTURE.md

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

ARCHITECTURE.md adds guidance on deciding whether features belong in core or an overlay. It also explains why feature switches remain configuration and do not alter database schemas.

Changes

Architecture guidance

Layer / File(s) Summary
Feature placement guidance
ARCHITECTURE.md
States that product and engineering leads confirm whether a feature belongs in the open-source product. Unconfirmed features belong in an overlay and may later move to core.
Schema and switch boundary
ARCHITECTURE.md
Links the core feature steps to an explanation of why feature switches do not alter schemas, including migration, rollback, and test coverage constraints.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: njbrake

Merge Risk: ⚪ Minimal · up to b1afc

The guidance is ready to merge after normal checks; no actionable issue remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title accurately describes the documentation changes and uses an appropriate documentation prefix, but it does not start with the required exact prefix docs: and is 108 characters long, exceedin… Shorten the title to about 70 characters and start it with the exact docs: prefix. Move the detailed rationale into the pull request description.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description is complete and relevant. It explains both documentation changes, testing and link checks, PR type, related issues, checklist status, and AI usage. It also names the architecture rule …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title accurately describes the documentation changes and uses an appropriate documentation prefix, but it does not start with the required exact prefix docs: and is 108 characters long, exceeding the approximate 70-character limit.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@peteski22
peteski22 force-pushed the docs/who-decides-what-is-open-source branch 2 times, most recently from 444cc54 to b1afc3c Compare September 24, 2026 13:06

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant