Skip to content

chore: adopt the modern-di convention; drop planning/ and architecture/ - #159

Merged
lesnik512 merged 4 commits into
mainfrom
chore/adopt-modern-di-convention
Sep 5, 2026
Merged

chore: adopt the modern-di convention; drop planning/ and architecture/#159
lesnik512 merged 4 commits into
mainfrom
chore/adopt-modern-di-convention

Conversation

@lesnik512

@lesnik512 lesnik512 commented Sep 5, 2026

Copy link
Copy Markdown
Member

Why

Two documentation trees carried the cost of every change and neither paid it back.

architecture/ was 12 capability pages, 612 lines, restating what faststream_outbox/ already
says. The promotion rule — "update the matching capability file in the same PR" — asked did you
update the page?
and never should this paragraph exist?, so pages only ever grew.

planning/ was 63 files: 34 change files, 13 curated release notes, 6 closed audit reports, 5
decision records, 5 templates, and a validator wired into lint-ci. Of that, only some of the
decision records and one deferred item were still load-bearing. Every audit was fully closed (PRs
#61#95). The change files duplicated what the PRs they described already recorded.

CONTEXT.md did not exist, though docs/agents/domain.md had been pointing at it as "not yet
created" for months, so the glossary rule it stated was unenforceable.

Design

Adopt the convention modern-di arrived at over PRs #424–#450.

The spec for a change is its PR body. .github/PULL_REQUEST_TEMPLATE.md carries why / design /
non-goals / verification and is reviewed with the diff. No change file, no lane. A trivial PR
deletes the template and ships a conventional-commit title.

Two things outlive the PR, one home each: a rejected alternative becomes an ADR in docs/adr/
(NNNN-slug.md, with a revisit trigger); unscheduled work becomes a GitHub issue. CLAUDE.md now
opens with the admission check that decides where a fact goes at all — derivable from the package →
don't write it; enforceable → a test; a user needs it → docs/; otherwise it does not get written.

Mechanism moves into the tests that already pinned it. The invariants turned out to be
executable already: 32 existing tests now carry an INVARIANT: docstring — the claim on line 1,
what breaks it in the second paragraph — spanning all 11 capabilities the old CLAUDE.md table
listed. This was marking, not writing. tests/test_invariant_census.py guards the shape: an
INVARIANT: docstring without a "what breaks it" paragraph fails.

One test is genuinely new. Engine ownership — "the caller owns the AsyncEngine; the broker never
disposes it" — was asserted in CLAUDE.md and pinned by nothing, and every other test constructs a
dedicated engine, so a broker that disposed it would have passed the whole suite.

The ADR corpus is two records, not five. The migration first carried all five planning/decisions/
forward; review cut it back. The doc-code-fence record declined a CI feature that was never built.
The conn-union record's contract already lives in AbstractOutboxClient's docstring, which is
where a contract belongs. The two free-threading records were one decision split in two, and the
second largely restated docs/introduction/installation.md — they merge into one holding only the
declined options. What survives is the metrics-recorder record (trimmed) and free-threading.

The BrokerUsecase-invariance rationale that planning/lint-suppressions.md carried is now a
one-line comment at each of the four suppression sites, plus a separate note on the two
invalid-method-override suppressions, which are a deliberate signature divergence rather than the
same variance rule.

CONTEXT.md is written: 16 terms, each listed only because there is a synonym to reject.

Curated release notes go with the planning tree. release.yml drops the mandatory-notes gate and
falls back to GitHub's generated notes, rendered from the squashed PR titles since the previous tag
— which is what makes the conventional-commit title on a trivial PR carry its weight.

Non-goals

  • Not changing any runtime behaviour. The only faststream_outbox/ edits are five comments at
    existing suppression sites; no logic moves.
  • Not adopting modern-di's docs_slug / docs/troubleshooting/ scheme. That rests on exceptions
    owning every message, which this package does not do.
  • Not renaming CLAUDE.md to AGENTS.md. Worth doing, but org-wide rather than here alone —
    tracked separately.
  • Not moving the PR template to the org .github repo. That repo is still on convention 2.0.0,
    so inheriting its template would contradict this one. Tracked separately.
  • Not fixing the OutboxRouter routers gap. It was the only genuinely open item in
    planning/deferred.md and is now OutboxRouter does not forward routers to the inner broker #158 — it needs a design call on lifespan-start ordering and
    AsyncAPI composition, not a kwarg passthrough.

Verification

just test612 passed, 100.00% coverage (the gate). just lint-ci clean — ruff format,
ruff check --no-fix, ty. just docs-build clean under --strict, with docs/adr/ built so its
outgoing links are validated and not_in_nav keeping it out of the site menu.

A repo-wide sweep for planning/, architecture/, check-planning, just index, deferred.md,
and lint-suppressions returns one hit: the sentence in docs/agents/domain.md that says neither
directory exists any more. mkdocs --strict passing after the deletion is the completeness proof
for the docs side — those references resolve while the directories exist and only break once they
are gone.

The spec for a change is now its PR body. .github/PULL_REQUEST_TEMPLATE.md
carries the shape and is reviewed with the diff, so there is no change file to
write and no lane to pick. Two things outlive the PR and have exactly one home
each: a rejected alternative becomes an ADR under docs/adr/, and unscheduled
work becomes a GitHub issue.

architecture/ was 12 capability pages restating what faststream_outbox/ already
says. Its invariants turned out to be already executable: 32 existing tests now
carry an INVARIANT: docstring -- the claim on line 1, what breaks it in the
second paragraph -- and only one test is genuinely new. Engine ownership was
asserted in CLAUDE.md and pinned by nothing, so a broker that disposed the
caller's engine would have passed the whole suite.

tests/test_invariant_census.py guards the scheme: every INVARIANT: docstring
must state what breaks it, and every path::test_name citation must resolve to a
real test. Code comments cite tests, not pages.

planning/ took nothing forward but its five decision records, which became
docs/adr/0001-0005. planning/lint-suppressions.md was a rationale register with
no home under the admission check; its recurring half -- everything downstream
of BrokerUsecase's invariance on its config type -- is now ADR-0006, and the
rest is derivable from the suppression sites. The audits were all closed
(PRs #61-#95). The one genuinely open deferred item, the OutboxRouter routers
forwarding gap, is now #158.

CONTEXT.md is the glossary docs/agents/domain.md had been promising: 16 terms,
each listed only because there is a synonym to reject.

Curated release notes go with the planning tree -- release.yml falls back to
GitHub's generated notes, rendered from the squashed PR titles since the
previous tag.

Closes #158 is deliberately not written here: that issue records work not being
done in this PR.
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Benchmark gate

✅ gate passed

scenario msg/s delete/msg WALrec/msg WALB/msg fpi upd del dead_tup
consumer/w1/b10 918 1.000 6.79 918 242 5000 5000 10000
consumer/w1/b100 923 1.000 6.82 921 243 5000 5000 10000
consumer/w2/b10 1026 1.000 6.80 968 242 5000 5000 10000
consumer/w2/b100 1252 1.000 6.78 997 263 5000 5000 10000
consumer/w4/b10 919 1.000 6.74 947 243 5000 5000 10000
consumer/w4/b100 1419 1.000 6.87 1036 243 5000 5000 10000
consumer/w1/b100/tfbs100 4842 0.010 6.07 1116 243 5000 5000 10000
producer/w1/b100 2112 0.000 3.04 584 0 0 0 0

Gated (fails the build): delete_calls + tuple counters (upd/del/ins) + the producer's insert_calls/select_calls, exact; wal_records within a 10% band. msg/s, WAL bytes and total calls are informational (timing/FPI noise).

Comment thread .github/PULL_REQUEST_TEMPLATE.md
Comment thread docs/adr/0001-metrics-recorders-not-unified.md Outdated
Comment thread docs/adr/0002-no-doc-code-fence-validation.md Outdated
Comment thread docs/adr/0003-conn-union-is-deliberate.md Outdated
Comment thread docs/adr/0004-free-threading-is-compat-only.md Outdated
Comment thread docs/adr/0005-sqlalchemy-cyext-disables-gil.md Outdated
Comment thread docs/adr/0006-broker-config-invariance-suppressions.md Outdated
Comment thread tests/test_invariant_census.py Outdated
Comment thread CLAUDE.md
…ites

Review pass on the six ADRs the migration produced. Four are gone.

The doc-code-fence record declined a CI feature that was never built, with a
revisit trigger amounting to "if it starts hurting". The conn-union record's
contract already lives in AbstractOutboxClient's docstring. The free-threading
pair was one decision split across two files, and the second mostly restated
docs/introduction/installation.md, which already carries everything a user
needs -- they merge into one record holding only the declined options.

The BrokerUsecase-invariance record is replaced by a one-line comment at each
of the four suppression sites, which is where the cause belongs; the two
invalid-method-override suppressions get their own note, since the outbox's
added publish kwargs are a deliberate divergence rather than the same variance
rule.

The census keeps the INVARIANT: docstring-shape check and drops the citation
check. It guarded a single citation, and its scan covered only .py files, so
the one citation that mattered -- CLAUDE.md's -- was the one it missed.
A revisit trigger guesses at the condition that would reopen a decision, and a
guess that never fires is noise on every read. The reasoning that settled the
call is the durable part and it stays; docs/agents/domain.md now says to argue
against that reasoning rather than to check whether a predicted trigger fired.

CLAUDE.md is 124 -> 106 lines, cut by running the admission check over itself.
Gone: the architecture summary (CONTEXT.md's opening paragraph already says it),
the _scheduling/_time bullet (readable from two stdlib-only leaves), the Outcome
sum-type description (readable from message.py -- only the DLQFailureReason
public contract survives), the no-state-column restatement, and the parts of
Code Style that restate pyproject.toml: ruff's select/ignore, the coverage
threshold, and PLC0415, which select = ["ALL"] already enforces. What is left of
the import rule is the part the linter cannot say -- that a noqa is the wrong
fix.
The free-threading ADR's trigger is scheduled work with an external dependency,
not a prediction: DISABLE_SQLALCHEMY_CEXT_RUNTIME=1 comes out of CI and the docs
when SQLAlchemy's Cython extensions declare Py_MOD_GIL_NOT_USED. Verified still
required on CPython 3.14.7t against SQLAlchemy 2.0.52 -- importing sqlalchemy
re-enables the GIL, and the variable is what keeps it off.
@lesnik512
lesnik512 merged commit 8380559 into main Sep 5, 2026
8 of 9 checks passed
@lesnik512
lesnik512 deleted the chore/adopt-modern-di-convention branch September 5, 2026 15:05
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