From 1b22261377188ffe0802c23cdf88d1e3aaee8f90 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 23 Aug 2026 13:30:58 +0300 Subject: [PATCH 1/5] Convert planning/decisions to numbered ADRs in docs/adr --- .github/PULL_REQUEST_TEMPLATE.md | 8 +- .github/workflows/main.yml | 1 + CLAUDE.md | 17 +-- .../adr/0001-sequence-ids-not-snowflakes.md | 4 - .../adr/0002-cookie-auth-not-bearer.md | 6 +- .../adr/0003-explicit-cookie-secure-flag.md | 4 - .../0004-anonymous-doc-and-metrics-paths.md | 4 - .../adr/0005-domain-error-vocabulary.md | 4 - .../adr/0006-mutation-requires-membership.md | 4 - .../0007-upsert-via-duplicate-key-recovery.md | 4 - .../adr/0008-idempotency-scoped-per-chat.md | 4 - .../adr/0009-read-marker-integrity.md | 4 - .../0010-repoint-last-message-on-delete.md | 4 - .../adr/0011-per-user-channel-topology.md | 4 - .../adr/0012-no-server-side-event-replay.md | 4 - ...013-coverage-exclusions-structural-only.md | 4 - docs/adr/README.md | 89 +++++++++++++++ docs/adr/check.py | 107 ++++++++++++++++++ Justfile => justfile | 8 +- planning/README.md | 78 +++++++------ planning/_templates/decision.md | 25 ---- ...6-08-21-message-id-existence-404-vs-403.md | 2 +- planning/index.py | 74 +++--------- pyproject.toml | 1 + readme.md | 2 +- 25 files changed, 281 insertions(+), 185 deletions(-) rename planning/decisions/2026-08-21-sequence-ids-not-snowflakes.md => docs/adr/0001-sequence-ids-not-snowflakes.md (92%) rename planning/decisions/2026-08-21-cookie-auth-not-bearer.md => docs/adr/0002-cookie-auth-not-bearer.md (85%) rename planning/decisions/2026-08-21-explicit-cookie-secure-flag.md => docs/adr/0003-explicit-cookie-secure-flag.md (91%) rename planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md => docs/adr/0004-anonymous-doc-and-metrics-paths.md (92%) rename planning/decisions/2026-08-21-domain-error-vocabulary.md => docs/adr/0005-domain-error-vocabulary.md (92%) rename planning/decisions/2026-08-21-mutation-requires-membership.md => docs/adr/0006-mutation-requires-membership.md (92%) rename planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md => docs/adr/0007-upsert-via-duplicate-key-recovery.md (92%) rename planning/decisions/2026-08-21-idempotency-scoped-per-chat.md => docs/adr/0008-idempotency-scoped-per-chat.md (93%) rename planning/decisions/2026-08-21-read-marker-integrity.md => docs/adr/0009-read-marker-integrity.md (91%) rename planning/decisions/2026-08-21-repoint-last-message-on-delete.md => docs/adr/0010-repoint-last-message-on-delete.md (88%) rename planning/decisions/2026-08-21-per-user-channel-topology.md => docs/adr/0011-per-user-channel-topology.md (92%) rename planning/decisions/2026-08-21-no-server-side-event-replay.md => docs/adr/0012-no-server-side-event-replay.md (91%) rename planning/decisions/2026-08-21-coverage-exclusions-structural-only.md => docs/adr/0013-coverage-exclusions-structural-only.md (91%) create mode 100644 docs/adr/README.md create mode 100644 docs/adr/check.py rename Justfile => justfile (86%) delete mode 100644 planning/_templates/decision.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 30e5ce1..0a54b41 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -39,10 +39,10 @@ coverage your run reported — 100% line coverage is the gate), - [ ] **Adding a fact anywhere?** Run the admission check: derivable from `app/` → don't write it; enforceable → a test; otherwise it does not get written. - [ ] **Rejected an alternative** with reasoning that would otherwise be - re-litigated? File it in [`planning/decisions/`](../planning/decisions/) - with a revisit trigger — not here. + re-litigated? File it in [`docs/adr/`](../docs/adr/) as the next numbered + ADR, with a revisit trigger — not here. - [ ] **Found real work you are not doing now?** File it in [`planning/deferred/`](../planning/deferred/), self-contained, with a revisit trigger — not here. -- [ ] `just lint`, `just check-planning`, `just check-links`, `just test` and - `just test-migrations` all pass. +- [ ] `just lint`, `just check-planning`, `just check-adrs`, `just check-links`, + `just test` and `just test-migrations` all pass. diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ee43506..5332a77 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -24,6 +24,7 @@ jobs: uv run ruff check . --no-fix uv run ty check uv run python planning/index.py --check + uv run python docs/adr/check.py uv run python planning/links.py pytest: diff --git a/CLAUDE.md b/CLAUDE.md index 7594165..ae3ceba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,10 +66,11 @@ run alembic ...`. - `just lint` runs `eof-fixer`, `ruff format`, `ruff check --fix`, then `ty check` — this project uses `ty`, not mypy; suppress with `# ty: ignore[]` (not `# type: ignore`). -- `just index` prints the deferred/decision listing; `just check-planning` - validates `planning/deferred/` and `planning/decisions/` frontmatter (and - that every deferred item carries a revisit trigger); `just check-links` - validates every relative Markdown link and heading anchor in the repo. +- `just index` prints the deferred listing; `just check-planning` validates + `planning/deferred/` frontmatter (and that every item carries a revisit + trigger); `just check-adrs` validates `docs/adr/` numbering, naming and + revisit triggers; `just check-links` validates every relative Markdown link + and heading anchor in the repo. Python is 3.14, dependencies managed by `uv`. The API is exposed on `:8000`. @@ -81,8 +82,8 @@ verification); it is reviewed with the diff. There is no change file and no lane to choose. A trivial PR (typo, dep bump, formatter) deletes the template and ships a conventional-commit title. -Two things outlive the PR and are committed under `planning/`: an alternative -**rejected** with reasoning goes to `planning/decisions/`, and real work **not +Two things outlive the PR and are committed: an alternative **rejected** with +reasoning goes to `docs/adr/` as a numbered ADR, and real work **not scheduled** goes to `planning/deferred/` (self-contained, with a revisit trigger). There is no capability-page home — the living truth about behaviour is the code and its `INVARIANT:`-marked tests, and a behaviour change is reviewed @@ -195,10 +196,10 @@ env vars (see `docker-compose.yml`). `api_bootstrapper_config` builds the exceptions (`NotFoundError`, `DuplicateKeyError`, `ForeignKeyError`). Every mapping, and why login's `401` deliberately uses Litestar's own `NotAuthorizedException` instead, is described in - `planning/decisions/2026-08-21-domain-error-vocabulary.md`. + `docs/adr/0005-domain-error-vocabulary.md`. - **Comments.** None, unless the code would read as a bug without one; then a single line. Rationale, design decisions and "why not X" belong in - `planning/decisions/` and the PR body, never in the source — those are where + `docs/adr/` and the PR body, never in the source — those are where such reasoning is reviewed and kept, and a comment restating it goes stale in place. What survives in `app/` today is the whole permitted category: a setting that diff --git a/planning/decisions/2026-08-21-sequence-ids-not-snowflakes.md b/docs/adr/0001-sequence-ids-not-snowflakes.md similarity index 92% rename from planning/decisions/2026-08-21-sequence-ids-not-snowflakes.md rename to docs/adr/0001-sequence-ids-not-snowflakes.md index ddc29fd..0c2b464 100644 --- a/planning/decisions/2026-08-21-sequence-ids-not-snowflakes.md +++ b/docs/adr/0001-sequence-ids-not-snowflakes.md @@ -1,7 +1,3 @@ ---- -summary: Message ids are a Postgres BigInt identity sequence, not snowflake ids. ---- - # Sequence ids, not snowflakes `messages.id` is a plain BigInt identity column. Postgres assigns values diff --git a/planning/decisions/2026-08-21-cookie-auth-not-bearer.md b/docs/adr/0002-cookie-auth-not-bearer.md similarity index 85% rename from planning/decisions/2026-08-21-cookie-auth-not-bearer.md rename to docs/adr/0002-cookie-auth-not-bearer.md index 19f6d2c..e0ad5d5 100644 --- a/planning/decisions/2026-08-21-cookie-auth-not-bearer.md +++ b/docs/adr/0002-cookie-auth-not-bearer.md @@ -1,7 +1,3 @@ ---- -summary: Authentication is a JWT in a cookie rather than a bearer header, because EventSource cannot set an Authorization header. ---- - # Cookie auth, not a bearer header **Decision:** The JWT travels in a cookie (`JWTCookieAuth[UsersTable]`), not in @@ -30,7 +26,7 @@ realistic shape. The cost is accepted deliberately: cookie auth needs CSRF consideration on state-changing endpoints, and `jwt_cookie_secure` must be `True` behind HTTPS — see -[`2026-08-21-explicit-cookie-secure-flag.md`](2026-08-21-explicit-cookie-secure-flag.md). +[`0003-explicit-cookie-secure-flag.md`](0003-explicit-cookie-secure-flag.md). ## Revisit trigger diff --git a/planning/decisions/2026-08-21-explicit-cookie-secure-flag.md b/docs/adr/0003-explicit-cookie-secure-flag.md similarity index 91% rename from planning/decisions/2026-08-21-explicit-cookie-secure-flag.md rename to docs/adr/0003-explicit-cookie-secure-flag.md index 5a052dd..4254b92 100644 --- a/planning/decisions/2026-08-21-explicit-cookie-secure-flag.md +++ b/docs/adr/0003-explicit-cookie-secure-flag.md @@ -1,7 +1,3 @@ ---- -summary: The session cookie's Secure flag comes from an explicit jwt_cookie_secure setting, not from inspecting service_environment. ---- - # Cookie security is an explicit setting `Settings.jwt_cookie_secure` defaults to `False` and is passed straight to diff --git a/planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md b/docs/adr/0004-anonymous-doc-and-metrics-paths.md similarity index 92% rename from planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md rename to docs/adr/0004-anonymous-doc-and-metrics-paths.md index dcb2d83..0ed8117 100644 --- a/planning/decisions/2026-08-21-anonymous-doc-and-metrics-paths.md +++ b/docs/adr/0004-anonymous-doc-and-metrics-paths.md @@ -1,7 +1,3 @@ ---- -summary: The auth exclude list carries four anchored prefixes — /docs, /health, /static and /metrics — and nothing else. ---- - # The anonymous surface is four prefixes `jwt_cookie_auth`'s `exclude` list holds `^/docs`, `^/health`, `^/static` and diff --git a/planning/decisions/2026-08-21-domain-error-vocabulary.md b/docs/adr/0005-domain-error-vocabulary.md similarity index 92% rename from planning/decisions/2026-08-21-domain-error-vocabulary.md rename to docs/adr/0005-domain-error-vocabulary.md index 0060eff..258ac0a 100644 --- a/planning/decisions/2026-08-21-domain-error-vocabulary.md +++ b/docs/adr/0005-domain-error-vocabulary.md @@ -1,7 +1,3 @@ ---- -summary: Domain failures are split across PermissionDeniedError (403), ValidationError (400) and ConflictError (409) rather than expressed as authorization failures. ---- - # Three domain exceptions, not one `app/exceptions.py` defines `ChatAppError` and three subclasses, each with a diff --git a/planning/decisions/2026-08-21-mutation-requires-membership.md b/docs/adr/0006-mutation-requires-membership.md similarity index 92% rename from planning/decisions/2026-08-21-mutation-requires-membership.md rename to docs/adr/0006-mutation-requires-membership.md index 9a2a7e9..04771b4 100644 --- a/planning/decisions/2026-08-21-mutation-requires-membership.md +++ b/docs/adr/0006-mutation-requires-membership.md @@ -1,7 +1,3 @@ ---- -summary: Editing and deleting a message requires chat membership as well as authorship; the check order is existence, membership, authorship. ---- - # Mutation requires membership, not just authorship `fetch_message_for_author` (`app/use_cases/message_authorization.py`) is the diff --git a/planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md b/docs/adr/0007-upsert-via-duplicate-key-recovery.md similarity index 92% rename from planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md rename to docs/adr/0007-upsert-via-duplicate-key-recovery.md index 1210517..cb9cd44 100644 --- a/planning/decisions/2026-08-21-upsert-via-duplicate-key-recovery.md +++ b/docs/adr/0007-upsert-via-duplicate-key-recovery.md @@ -1,7 +1,3 @@ ---- -summary: Direct-chat creation and message send recover from a unique-constraint violation and re-read, rather than trusting a pre-check. ---- - # Upsert by recovering from DuplicateKeyError Both `CreateChatUseCase` and `CreateMessageUseCase` read first to see whether diff --git a/planning/decisions/2026-08-21-idempotency-scoped-per-chat.md b/docs/adr/0008-idempotency-scoped-per-chat.md similarity index 93% rename from planning/decisions/2026-08-21-idempotency-scoped-per-chat.md rename to docs/adr/0008-idempotency-scoped-per-chat.md index 7a5f612..348e7df 100644 --- a/planning/decisions/2026-08-21-idempotency-scoped-per-chat.md +++ b/docs/adr/0008-idempotency-scoped-per-chat.md @@ -1,7 +1,3 @@ ---- -summary: Message idempotency is scoped to (chat_id, idempotency_key), not to the key alone. ---- - # Idempotency is scoped per chat `messages` carries `UniqueConstraint("chat_id", "idempotency_key")`. The diff --git a/planning/decisions/2026-08-21-read-marker-integrity.md b/docs/adr/0009-read-marker-integrity.md similarity index 91% rename from planning/decisions/2026-08-21-read-marker-integrity.md rename to docs/adr/0009-read-marker-integrity.md index 45f42e8..eb79ade 100644 --- a/planning/decisions/2026-08-21-read-marker-integrity.md +++ b/docs/adr/0009-read-marker-integrity.md @@ -1,7 +1,3 @@ ---- -summary: The read marker advances only to a message in its own chat, and advances atomically via GREATEST so it can never move backwards. ---- - # Read-marker integrity `MarkReadUseCase` does three things in order: verify membership, verify that diff --git a/planning/decisions/2026-08-21-repoint-last-message-on-delete.md b/docs/adr/0010-repoint-last-message-on-delete.md similarity index 88% rename from planning/decisions/2026-08-21-repoint-last-message-on-delete.md rename to docs/adr/0010-repoint-last-message-on-delete.md index 0f6e40e..3ebd71a 100644 --- a/planning/decisions/2026-08-21-repoint-last-message-on-delete.md +++ b/docs/adr/0010-repoint-last-message-on-delete.md @@ -1,7 +1,3 @@ ---- -summary: Soft-deleting a chat's newest message repoints chats.last_message_id in the same transaction, rather than filtering the deleted row out of the listing preview. ---- - # Repoint last_message_id on delete `DeleteMessageUseCase` soft-deletes the message and, if it was the chat's diff --git a/planning/decisions/2026-08-21-per-user-channel-topology.md b/docs/adr/0011-per-user-channel-topology.md similarity index 92% rename from planning/decisions/2026-08-21-per-user-channel-topology.md rename to docs/adr/0011-per-user-channel-topology.md index 39c053e..08206c3 100644 --- a/planning/decisions/2026-08-21-per-user-channel-topology.md +++ b/docs/adr/0011-per-user-channel-topology.md @@ -1,7 +1,3 @@ ---- -summary: Event fan-out uses one Redis channel per user; per-chat and hybrid topologies rejected. ---- - # Per-user channel topology Every connected client subscribes to exactly one channel, `user:{user_id}`. A diff --git a/planning/decisions/2026-08-21-no-server-side-event-replay.md b/docs/adr/0012-no-server-side-event-replay.md similarity index 91% rename from planning/decisions/2026-08-21-no-server-side-event-replay.md rename to docs/adr/0012-no-server-side-event-replay.md index efa1a8e..0af1252 100644 --- a/planning/decisions/2026-08-21-no-server-side-event-replay.md +++ b/docs/adr/0012-no-server-side-event-replay.md @@ -1,7 +1,3 @@ ---- -summary: Reconnect recovery uses channel history plus a REST resync; no durable per-user event log honouring Last-Event-ID. ---- - # No server-side event replay On reconnect the client recovers in two layers. Fast path: diff --git a/planning/decisions/2026-08-21-coverage-exclusions-structural-only.md b/docs/adr/0013-coverage-exclusions-structural-only.md similarity index 91% rename from planning/decisions/2026-08-21-coverage-exclusions-structural-only.md rename to docs/adr/0013-coverage-exclusions-structural-only.md index ab7f3ef..79d659a 100644 --- a/planning/decisions/2026-08-21-coverage-exclusions-structural-only.md +++ b/docs/adr/0013-coverage-exclusions-structural-only.md @@ -1,7 +1,3 @@ ---- -summary: Coverage exclusions are reserved for code pytest structurally cannot execute; unreachable-in-production branches are tested through repository seams instead. ---- - # Coverage exclusions are structural only The suite runs at `--cov-fail-under=100`. Two mechanisms can exempt code, and diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..1396b25 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,89 @@ +# Architecture decision records + +One file per decision taken, especially the options **rejected**, so reviews do +not re-litigate them. `just check-adrs` validates the set; CI runs it. + +The directory listing is the index. There is no generated listing and no +`summary` field: the number, the slug and the `# ` title already say what a file +is, and a fourth telling would be the copy nobody edits. + +## Numbering is reading order, not chronology + +Numbers run contiguously from `0001` and are permanent. They are assigned in +**dependency order**, so reading `0001` upward introduces the system in the +order its decisions build on each other: identity, then auth, then the error and +authorization vocabulary, then write patterns, then chat state, then events, +then testing policy. + +A new ADR takes the next free number, which puts it at the end regardless of +where it belongs conceptually. That is the cost of permanence and it is +accepted: renumbering to preserve the reading order would break every existing +reference. + +## Status lives in the frontmatter, or nowhere + +An ADR with no frontmatter is **accepted**. There is no exit from this +directory: an ADR is never deleted or edited into reversal, because a superseded +decision that stays readable is one that does not get re-argued. + +When a later ADR supersedes an earlier one, add to the earlier file: + +```yaml +--- +superseded_by: 0014-its-slug +--- +``` + +`check.py` fails if that pointer does not name a real ADR. + +## The admission test + +All three must be true, or it is not an ADR: + +1. **Hard to reverse.** Changing your mind later carries a real cost. +2. **Surprising without context.** A reader will look at the code and wonder why + it was done this way. +3. **A real trade-off.** There were genuine alternatives and one was picked for + specific reasons. + +If a decision is easy to reverse, you will just reverse it. If it is not +surprising, nobody will wonder. If there was no alternative, there is nothing to +record beyond doing the obvious thing. + +This is stricter than the stock format the `/domain-modeling` skill writes, which +treats rejected alternatives and consequences as optional and expects a body of +one to three sentences. Here they are the point of the file. + +## Template + +```md +# One-line capitalized title + +**Decision:** What was decided, in a sentence. + +What the code actually does, and the constraint that forced it. + +## Rejected: + +Why it was not taken. Enough that a future explorer does not re-litigate it. + +## Consequence + +The non-obvious downstream effect, including what this deliberately leaves +uncovered. + +## Revisit trigger + +The concrete signal that should reopen this decision. +``` + +`## Rejected:` repeats once per alternative worth remembering. `## Consequence` +is optional. `## Revisit trigger` is **required** and `check.py` enforces it: a +decision with no trigger is never revisited, only rediscovered. + +## Where other facts go + +This directory is one of four homes, and the narrowest. See +[`../../planning/README.md`](../../planning/README.md#where-a-fact-goes) for the +admission check that decides between them: code, an `INVARIANT:`-marked test, an +ADR here, or a deferred item in [`../../planning/deferred/`](../../planning/deferred/). diff --git a/docs/adr/check.py b/docs/adr/check.py new file mode 100644 index 0000000..db8d759 --- /dev/null +++ b/docs/adr/check.py @@ -0,0 +1,107 @@ +"""Validate the ADR set: numbering, naming, revisit triggers, supersession pointers. + +Run via ``just check-adrs``. Globs ``docs/adr/*.md`` and reports every violation +at once rather than failing on the first. + +ADRs carry no ``summary`` frontmatter: the number, the slug and the ``# `` title +already say what the file is, and a fourth telling would be the copy nobody +edits. Frontmatter appears only on a superseded ADR, so *no frontmatter* means +accepted. + +Numbers must be contiguous from ``0001``. An ADR is never deleted — it is +superseded — so a gap is a mistake worth failing on, not a deliberate state. +""" + +import pathlib +import re +import sys + + +ROOT = pathlib.Path(__file__).parent +ADR_RE = re.compile(r"^(?P\d{4})-(?P[a-z0-9]+(?:-[a-z0-9]+)*)$") +REVISIT_HEADING = "## Revisit trigger" + + +def parse_frontmatter(text: str) -> dict[str, str]: + """Parse a single-line-scalar YAML frontmatter block into a dict.""" + lines = text.splitlines() + if not lines or lines[0].strip() != "---": + return {} + fields: dict[str, str] = {} + for line in lines[1:]: + if line.strip() == "---": + break + if line[:1] in (" ", "\t"): + continue + key, sep, value = line.partition(": ") + if not sep: + continue + fields[key.strip()] = value.strip().strip('"').strip("'") + return fields + + +def adr_paths(root: pathlib.Path) -> list[pathlib.Path]: + """Every ADR file, sorted by name; README and underscore-prefixed files are not ADRs.""" + return [path for path in sorted(root.glob("*.md")) if path.name != "README.md" and not path.name.startswith("_")] + + +def _check_numbering(paths: list[pathlib.Path], violations: list[str]) -> None: + """Require each name to be `NNNN-slug.md`, numbered contiguously from 0001.""" + numbers: dict[int, str] = {} + for path in paths: + match = ADR_RE.match(path.stem) + if match is None: + violations.append(f"{path.name}: file name is not 'NNNN-slug.md' with a lowercase hyphenated slug") + continue + number = int(match.group("number")) + if number in numbers: + violations.append(f"{path.name}: number {number:04d} is already taken by {numbers[number]}") + continue + numbers[number] = path.name + expected = set(range(1, len(numbers) + 1)) + violations.extend( + f"ADR {number:04d} is missing — numbers run contiguously from 0001" for number in expected - numbers.keys() + ) + + +def _check_body(path: pathlib.Path, stems: set[str], violations: list[str]) -> None: + """Require a revisit trigger, and a `superseded_by` that names a real ADR.""" + text = path.read_text(encoding="utf-8") + if REVISIT_HEADING not in text: + violations.append( + f"{path.name}: no '{REVISIT_HEADING}' section — a decision with no trigger is never revisited" + ) + superseded_by = parse_frontmatter(text).get("superseded_by") + if superseded_by is None: + return + if superseded_by == path.stem: + violations.append(f"{path.name}: superseded_by points at itself") + elif superseded_by not in stems: + violations.append(f"{path.name}: superseded_by '{superseded_by}' does not name an ADR in docs/adr/") + + +def check(root: pathlib.Path) -> list[str]: + """Validate every ADR; return the list of violation strings.""" + violations: list[str] = [] + paths = adr_paths(root) + _check_numbering(paths, violations) + stems = {path.stem for path in paths} + for path in paths: + _check_body(path, stems, violations) + return violations + + +def main(root: pathlib.Path | None = None) -> int: + """Report every violation on stderr, or confirm the set is clean on stdout.""" + violations = check(ROOT if root is None else root) + if violations: + sys.stderr.write(f"adr: {len(violations)} violation(s)\n") + for violation in violations: + sys.stderr.write(f" - {violation}\n") + return 1 + sys.stdout.write("adr: OK\n") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/Justfile b/justfile similarity index 86% rename from Justfile rename to justfile index fe359c5..15e6b79 100644 --- a/Justfile +++ b/justfile @@ -35,14 +35,18 @@ lint: uv run ruff check . --fix uv run ty check -# Print the planning index (deferred, then decisions) to stdout. +# Print the planning index (the deferred queue) to stdout. index: uv run python planning/index.py -# Validate planning/deferred/ + planning/decisions/ frontmatter and naming; CI runs this. +# Validate planning/deferred/ frontmatter and naming; CI runs this. check-planning: uv run python planning/index.py --check +# Validate docs/adr/ numbering, naming and revisit triggers; CI runs this. +check-adrs: + uv run python docs/adr/check.py + # Check every relative Markdown link and heading anchor in the repo. check-links: uv run python planning/links.py diff --git a/planning/README.md b/planning/README.md index 2544c3d..fd67162 100644 --- a/planning/README.md +++ b/planning/README.md @@ -1,16 +1,18 @@ # Planning The standing record for `chat-app`. The living truth about *what the system -does now* lives in the code itself and in its tests. This directory holds what -code and tests cannot: the decisions taken (especially the options rejected) -and the work deliberately not scheduled. +does now* lives in the code itself and in its tests. This directory holds the +work deliberately not scheduled. The decisions taken, especially the options +rejected, live in [`../docs/adr/`](../docs/adr/) as numbered ADRs. > **Local deviation.** This repo tracks the portable convention from > [`lesnik512/planning-convention`](https://github.com/lesnik512/planning-convention) > (applied version in `.convention-version`, beside this file), but **deviates -> from it** on six counts, listed under [Deviations](#deviations) below. The -> lean shape follows `modern-di`, which runs the same deviation; if it holds -> across both repos it goes upstream as convention 3.0.0. +> from it** on seven counts, listed under [Deviations](#deviations) below. The +> lean shape follows `modern-di`, which runs deviations 1-5; if it holds across +> both repos it goes upstream as convention 3.0.0. Deviation 7, which moves +> decisions out of this directory entirely, runs here alone until it has been +> lived with. ## Quick path (start here) @@ -23,10 +25,10 @@ delete the template and ship a conventional-commit title. **2. File what outlives the PR:** -- an alternative you **rejected** with reasoning → [`decisions/`](decisions/) +- an alternative you **rejected** with reasoning → [`../docs/adr/`](../docs/adr/) - work that is real but **not scheduled** → [`deferred/`](deferred/) -**3. Run `just check-planning` and `just check-links` before pushing.** +**3. Run `just check-planning`, `just check-adrs` and `just check-links` before pushing.** ## Where a fact goes @@ -36,7 +38,7 @@ Four homes, one owner each: |---|---| | `app/` | anything readable from the module — the default | | a named test | an **invariant**: must stay true, and a change could silently break it | -| `decisions/` | a rejected alternative, with the reasoning that would otherwise be re-litigated | +| `../docs/adr/` | a rejected alternative, with the reasoning that would otherwise be re-litigated | | `deferred/` | real work, not scheduled, with a revisit trigger | Before writing a line anywhere: @@ -48,8 +50,8 @@ Before writing a line anywhere: **Prose about mechanism has no home. There is no file to add a paragraph to.** This repo kept an `architecture/` directory of capability pages until 2026-08-22 -and removed it. The pages had become a second telling of `decisions/` — the -decision files referenced them zero times, while the pages re-narrated the +and removed it. The pages had become a second telling of the decision +records — those files referenced them zero times, while the pages re-narrated the decisions at length — and one had gone silently wrong: `chats.md` still described `chat_type` as a non-native enum after #4 converted it to a native Postgres enum, and nothing caught it, because the convention's promotion rule @@ -57,7 +59,7 @@ was a habit with nothing enforcing it. A prose copy of a fact the code already owns goes stale in the copy nobody edits. The absence of the directory is the mechanism. -`decisions/` and `INVARIANT:` docstrings inherit the same risk from the other +The ADRs and `INVARIANT:` docstrings inherit the same risk from the other direction: nothing prunes a record once its call is settled. Keeping both lean is a habit this repo owes them, not a one-time fix earned by deleting a directory. @@ -69,41 +71,44 @@ of what this one test happens to catch. ## Artifacts -- **[`decisions/-.md`](decisions/)** — one file per design - decision taken, especially options *rejected*, each with a revisit trigger, so - reviews don't re-litigate them. Frontmatter: `summary`, plus `superseded_by` - once something supersedes it. - **[`deferred/-.md`](deferred/)** — one file per open item, each **self-contained**: it inlines the evidence and reasoning needed to pick it up cold. Frontmatter: `summary`. A required `**Revisit trigger:**` section — an item with no trigger is abandoned, not deferred. -- **[`_templates/`](_templates/)** — `decision.md`, `deferred.md`. +- **[`_templates/`](_templates/)** — `deferred.md`. -### Location is status +Decisions are not an artifact of this directory. They are numbered ADRs in +[`../docs/adr/`](../docs/adr/), where [`../docs/adr/README.md`](../docs/adr/README.md) +carries their standard and template. -Neither artifact carries a `status:` field. Where a file sits, and which keys it -has, is what its state means. +### Location is status -A **deferred item's presence in `deferred/` is its status**. When it resolves: +A deferred item carries no `status:` field. Where the file sits is what its +state means, and **its presence in `deferred/` is its status**. When it +resolves: - **it ships** → delete the file. Its truth is now in the code and its tests. -- **it is declined** → move it to `decisions/`, so the refusal is on record. - -A **decision is accepted unless it says otherwise**. There is no exit from -`decisions/` — a superseded decision stays readable, or it gets re-litigated — -so the one state worth recording is marked by adding `superseded_by: `, -which `just index` renders. +- **it is declined** → write it up in [`../docs/adr/`](../docs/adr/), so the + refusal is on record, and delete the deferred item. `date` and `slug` are derived from the file name and never repeated in frontmatter. `summary` is one line; it is the only field the index renders. +ADRs run the same principle with a different mechanism: no frontmatter means +accepted, and `superseded_by` is the one state worth recording. See +[`../docs/adr/README.md`](../docs/adr/README.md#status-lives-in-the-frontmatter-or-nowhere). + ## Index -The listing is **generated**, not maintained — run `just index` to print it: -deferred first (the open queue), then decisions, newest-first. The frontmatter -in each file is the single source of truth; there is no committed copy to drift. -`just check-planning` validates it, and `just check-links` validates every -relative Markdown link and heading anchor in the repo. +The listing is **generated**, not maintained — run `just index` to print the +deferred queue, newest-first. The frontmatter in each file is the single source +of truth; there is no committed copy to drift. `just check-planning` validates +it, and `just check-links` validates every relative Markdown link and heading +anchor in the repo. + +ADRs have no generated listing: they are numbered, so the directory listing is +the index. `just check-adrs` validates their numbering, naming and revisit +triggers. ## Deviations @@ -115,15 +120,20 @@ Against upstream convention 2.2.0: rule. Enforceable claims are `INVARIANT:`-marked tests; the ubiquitous language lives in [`../CLAUDE.md`](../CLAUDE.md)'s Vocabulary section. 3. `deferred.md` is a `deferred/` directory of indexed, trigger-bearing items. -4. Decision frontmatter drops `status` and `supersedes`. +4. Decision frontmatter drops `status` and `supersedes`. Largely subsumed by 7: + decisions are no longer a `planning/` artifact at all. 5. `index.py` is edited to match that schema, and both `index.py` and `links.py` drop the canonical `# ruff: noqa: INP001` line — this repo ignores `INP` globally, so the directive is an unused `noqa` and fails `RUF100`. 6. There is no `lint-ci` recipe; CI inlines its lint steps, so `links.py` runs as a step in the workflow's `lint` job rather than via a recipe CI calls. `just check-links` exists for running it locally. +7. `decisions/` is removed. Design decisions are numbered ADRs in + [`../docs/adr/`](../docs/adr/), validated by `docs/adr/check.py`, and carry no + `summary` frontmatter: the number, the slug and the title already identify the + file. `index.py` indexes the deferred queue alone. -Deviations 1–5 match `modern-di`'s practice. Applying a future convention +Deviations 1–5 match `modern-di`'s practice; deviation 7 does not yet. Applying a future convention version runs upstream's `APPLY.md`, which copies `index.py` and `links.py` over any local version by design — that reverts the edits in 5, so re-apply them afterwards. diff --git a/planning/_templates/decision.md b/planning/_templates/decision.md deleted file mode 100644 index 23f2a54..0000000 --- a/planning/_templates/decision.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -summary: One line — shown in `just index`. ---- - - - - -# One-line capitalized title - -**Decision:** What was decided, in a sentence. - -## Context - -Why this came up; the options that were on the table. - -## Decision & rationale - -The call and why — including why the alternatives were rejected. Enough that a -future explorer doesn't re-litigate it. - -## Revisit trigger - -The concrete signal that should reopen this decision. diff --git a/planning/deferred/2026-08-21-message-id-existence-404-vs-403.md b/planning/deferred/2026-08-21-message-id-existence-404-vs-403.md index 60cc7b4..52e189e 100644 --- a/planning/deferred/2026-08-21-message-id-existence-404-vs-403.md +++ b/planning/deferred/2026-08-21-message-id-existence-404-vs-403.md @@ -11,7 +11,7 @@ id that doesn't exist and `403` for one that does but belongs to a chat they're not in — the two status codes leak whether the id is real. Accepted deliberately: it mirrors the spec's own decision that `FetchChatUseCase` returns `403` for a chat the actor isn't a member of rather than pretending -the chat doesn't exist (see `planning/decisions/2026-08-21-mutation-requires-membership.md`), and checking membership +the chat doesn't exist (see `../../docs/adr/0006-mutation-requires-membership.md`), and checking membership before authorship on every message use case keeps that posture consistent rather than making message mutation the one place that hides existence. diff --git a/planning/index.py b/planning/index.py index cb630b6..5c7d74b 100644 --- a/planning/index.py +++ b/planning/index.py @@ -1,18 +1,16 @@ # planning/ is not a Python package (this file is vendored into consumers' planning/) """Generate the planning index from frontmatter. -Run via ``just index``. Globs ``planning/deferred/*.md`` and -``planning/decisions/*.md``, reads their frontmatter, and prints a Markdown -listing to stdout — deferred (the open queue) then decisions, newest-first. -Never writes a file: the listing is a query over the files, not a committed -artifact. - -``date`` and ``slug`` are derived from the file name, not -frontmatter — the name is the single source of truth for both. - -Both artifact kinds carry ``summary`` and nothing else required. A decision has -no ``status`` field: absent ``superseded_by`` means accepted, and its presence -means superseded. +Run via ``just index``. Globs ``planning/deferred/*.md``, reads their +frontmatter, and prints a Markdown listing of the open queue to stdout, +newest-first. Never writes a file: the listing is a query over the files, not a +committed artifact. + +``date`` and ``slug`` are derived from the file name, not frontmatter — the name +is the single source of truth for both. + +Decisions are not indexed here. They live in ``docs/adr/`` as numbered ADRs, +where the directory listing is the index and ``docs/adr/check.py`` is the gate. """ import pathlib @@ -22,9 +20,7 @@ ROOT = pathlib.Path(__file__).parent DEFERRED_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") -DECISION_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") DEFERRED_REQUIRED = ("summary",) -DECISION_REQUIRED = ("summary",) def parse_frontmatter(text: str) -> dict[str, str]: @@ -71,42 +67,20 @@ def load_deferred(root: pathlib.Path) -> list[dict[str, str]]: return deferred -def load_decisions(root: pathlib.Path) -> list[dict[str, str]]: - """Read each decision's frontmatter; derive date/slug from the file name.""" - decisions_dir = root / "decisions" - decisions: list[dict[str, str]] = [] - if not decisions_dir.is_dir(): - return decisions - for path in sorted(decisions_dir.glob("*.md")): - if path.name == "README.md" or path.name.startswith("_"): - continue - fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, DECISION_RE) - fields["path"] = f"decisions/{path.name}" - fields["name"] = path.stem - decisions.append(fields) - return decisions - - def format_row(row: dict[str, str]) -> str: - """Render one deferred item or decision as a Markdown list item.""" + """Render one deferred item as a Markdown list item.""" slug = row.get("slug", "?") path = row.get("path", "") date = row.get("date", "") summary = row.get("summary") or "(no summary)" - line = f"- **[{slug}]({path})** ({date}) — {summary}" - if row.get("superseded_by"): - line += f" _(superseded by {row['superseded_by']})_" - return line + return f"- **[{slug}]({path})** ({date}) — {summary}" -def render(deferred: list[dict[str, str]], decisions: list[dict[str, str]]) -> str: - """Render the full Markdown listing: deferred then decisions, newest-first.""" +def render(deferred: list[dict[str, str]]) -> str: + """Render the Markdown listing of the deferred queue, newest-first.""" out = ["# Planning index", "", "_Generated by `just index` — do not edit._", "", "## Deferred", ""] deferred_rows = sorted(deferred, key=lambda b: b.get("name", ""), reverse=True) out += [format_row(b) for b in deferred_rows] if deferred_rows else ["_None._"] - out += ["", "## Decisions", ""] - decision_rows = sorted(decisions, key=lambda d: d.get("name", ""), reverse=True) - out += [format_row(d) for d in decision_rows] if decision_rows else ["_None._"] out.append("") return "\n".join(out).rstrip() + "\n" @@ -129,19 +103,10 @@ def _check_deferred(path: pathlib.Path, violations: list[str]) -> None: ) -def _check_decision(path: pathlib.Path, violations: list[str]) -> None: - """Validate one decision file (requires `summary`).""" - rel = f"decisions/{path.name}" - if DECISION_RE.match(path.stem) is None: - violations.append(f"{rel}: file name is not 'YYYY-MM-DD-slug.md'") - _require(parse_frontmatter(path.read_text(encoding="utf-8")), DECISION_REQUIRED, rel, violations) - - def check(root: pathlib.Path) -> list[str]: - """Validate every deferred item and decision; return the list of violation strings.""" + """Validate every deferred item; return the list of violation strings.""" violations: list[str] = [] deferred_dir = root / "deferred" - decisions_dir = root / "decisions" if deferred_dir.is_dir(): for path in sorted(deferred_dir.iterdir()): if path.name == "README.md" or path.name.startswith(("_", ".")): @@ -150,16 +115,11 @@ def check(root: pathlib.Path) -> list[str]: violations.append(f"deferred/{path.name}: unexpected non-md file in deferred/") else: _check_deferred(path, violations) - if decisions_dir.is_dir(): - for path in sorted(decisions_dir.glob("*.md")): - if path.name == "README.md" or path.name.startswith("_"): - continue - _check_decision(path, violations) return violations def main(argv: list[str] | None = None, root: pathlib.Path | None = None) -> int: - """Print the listing to stdout, or validate deferred items and decisions with --check.""" + """Print the listing to stdout, or validate deferred items with --check.""" argv = sys.argv[1:] if argv is None else argv root = ROOT if root is None else root if "--check" in argv: @@ -171,7 +131,7 @@ def main(argv: list[str] | None = None, root: pathlib.Path | None = None) -> int return 1 sys.stdout.write("planning: OK\n") return 0 - sys.stdout.write(render(load_deferred(root), load_decisions(root))) + sys.stdout.write(render(load_deferred(root))) return 0 diff --git a/pyproject.toml b/pyproject.toml index 7086185..545ac77 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,4 +98,5 @@ omit = [ # on that as an accident of discovery. "planning/index.py", "planning/links.py", + "docs/adr/check.py", ] diff --git a/readme.md b/readme.md index 498d8b1..1409061 100644 --- a/readme.md +++ b/readme.md @@ -48,7 +48,7 @@ transaction that must span two writes, a unique constraint that two concurrent requests can both hit, a count that must not cost a row per event. This repo answers that with a domain that actually needs it. See [PR #1](https://github.com/modern-python/chat-app/pull/1) for the full design -and `planning/decisions/` for the calls taken along the way. +and `docs/adr/` for the calls taken along the way. | Pattern | Where to look | |---|---| From c5459b2822af590098e92ace45b770710666f299 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 23 Aug 2026 13:38:32 +0300 Subject: [PATCH 2/5] Stop claiming ADR numbering is a reading order --- docs/adr/README.md | 23 +++++++++++------------ docs/agents/domain.md | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/docs/adr/README.md b/docs/adr/README.md index 1396b25..d6ee5bf 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -7,18 +7,17 @@ The directory listing is the index. There is no generated listing and no `summary` field: the number, the slug and the `# ` title already say what a file is, and a fourth telling would be the copy nobody edits. -## Numbering is reading order, not chronology - -Numbers run contiguously from `0001` and are permanent. They are assigned in -**dependency order**, so reading `0001` upward introduces the system in the -order its decisions build on each other: identity, then auth, then the error and -authorization vocabulary, then write patterns, then chat state, then events, -then testing policy. - -A new ADR takes the next free number, which puts it at the end regardless of -where it belongs conceptually. That is the cost of permanence and it is -accepted: renumbering to preserve the reading order would break every existing -reference. +## Numbering + +Numbers run contiguously from `0001`, are permanent, and mean nothing beyond +identity. A new ADR takes the next free number, so numbering is the order +decisions were adopted. Nothing is ever renumbered: renumbering would break +every reference for a property no reader should rely on. + +The first thirteen are the exception that proves this. They were written on one +day, so their dates could not order them, and they were seeded in dependency +order instead. That is a one-off property of the seed set, not a rule, and it +stops being true at `0014`. Do not read the sequence as a curriculum. ## Status lives in the frontmatter, or nowhere diff --git a/docs/agents/domain.md b/docs/agents/domain.md index 32c32d5..5815d59 100644 --- a/docs/agents/domain.md +++ b/docs/agents/domain.md @@ -24,7 +24,7 @@ If either doesn't exist yet, **proceed silently**. Don't flag its absence; don't └── tests/ ``` -ADR numbers are permanent and assigned in dependency order, so reading from `0001` upward introduces the system in the order its decisions build on each other. A new ADR takes the next free number. +ADR numbers are permanent and mean nothing beyond identity: a new ADR takes the next free number, so the sequence is the order decisions were adopted. Read the ones relevant to your area, not the sequence front to back. ## Use the glossary's vocabulary From 1ceabc3e3af72f09109d2da0488b52fd6ef66da3 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 23 Aug 2026 13:40:40 +0300 Subject: [PATCH 3/5] Trim docs/adr/README.md to what a writer acts on --- docs/adr/README.md | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/docs/adr/README.md b/docs/adr/README.md index d6ee5bf..bfc0355 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -1,29 +1,19 @@ # Architecture decision records One file per decision taken, especially the options **rejected**, so reviews do -not re-litigate them. `just check-adrs` validates the set; CI runs it. - -The directory listing is the index. There is no generated listing and no -`summary` field: the number, the slug and the `# ` title already say what a file -is, and a fourth telling would be the copy nobody edits. +not re-litigate them. The directory listing is the index: there is no generated +listing and no `summary` frontmatter. `just check-adrs` validates the set, and +CI runs it. ## Numbering Numbers run contiguously from `0001`, are permanent, and mean nothing beyond -identity. A new ADR takes the next free number, so numbering is the order -decisions were adopted. Nothing is ever renumbered: renumbering would break -every reference for a property no reader should rely on. - -The first thirteen are the exception that proves this. They were written on one -day, so their dates could not order them, and they were seeded in dependency -order instead. That is a one-off property of the seed set, not a rule, and it -stops being true at `0014`. Do not read the sequence as a curriculum. +identity. A new ADR takes the next free number. Nothing is ever renumbered. ## Status lives in the frontmatter, or nowhere An ADR with no frontmatter is **accepted**. There is no exit from this -directory: an ADR is never deleted or edited into reversal, because a superseded -decision that stays readable is one that does not get re-argued. +directory: a superseded decision stays readable, or it gets re-argued. When a later ADR supersedes an earlier one, add to the earlier file: @@ -33,8 +23,6 @@ superseded_by: 0014-its-slug --- ``` -`check.py` fails if that pointer does not name a real ADR. - ## The admission test All three must be true, or it is not an ADR: @@ -45,10 +33,6 @@ All three must be true, or it is not an ADR: 3. **A real trade-off.** There were genuine alternatives and one was picked for specific reasons. -If a decision is easy to reverse, you will just reverse it. If it is not -surprising, nobody will wonder. If there was no alternative, there is nothing to -record beyond doing the obvious thing. - This is stricter than the stock format the `/domain-modeling` skill writes, which treats rejected alternatives and consequences as optional and expects a body of one to three sentences. Here they are the point of the file. @@ -77,12 +61,11 @@ The concrete signal that should reopen this decision. ``` `## Rejected:` repeats once per alternative worth remembering. `## Consequence` -is optional. `## Revisit trigger` is **required** and `check.py` enforces it: a -decision with no trigger is never revisited, only rediscovered. +is optional. `## Revisit trigger` is required and enforced. ## Where other facts go -This directory is one of four homes, and the narrowest. See +This is one of four homes, and the narrowest. See [`../../planning/README.md`](../../planning/README.md#where-a-fact-goes) for the -admission check that decides between them: code, an `INVARIANT:`-marked test, an -ADR here, or a deferred item in [`../../planning/deferred/`](../../planning/deferred/). +admission check that decides between code, an `INVARIANT:`-marked test, an ADR +here, and a deferred item in [`../../planning/deferred/`](../../planning/deferred/). From 204a351765ac7d35f71d790b497b65552c92bd49 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 23 Aug 2026 13:52:34 +0300 Subject: [PATCH 4/5] Drop the comparison to the stock ADR format --- docs/adr/README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/adr/README.md b/docs/adr/README.md index bfc0355..42e5f7d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -33,10 +33,6 @@ All three must be true, or it is not an ADR: 3. **A real trade-off.** There were genuine alternatives and one was picked for specific reasons. -This is stricter than the stock format the `/domain-modeling` skill writes, which -treats rejected alternatives and consequences as optional and expects a body of -one to three sentences. Here they are the point of the file. - ## Template ```md From 188127a54fa6fc81fd6a8f076740e2dbaf8f2df0 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 23 Aug 2026 14:01:42 +0300 Subject: [PATCH 5/5] Show repeated Rejected headings in the template --- docs/adr/README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/adr/README.md b/docs/adr/README.md index 42e5f7d..c8955ce 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -42,10 +42,14 @@ All three must be true, or it is not an ADR: What the code actually does, and the constraint that forced it. -## Rejected: +## Rejected: deriving it from the environment Why it was not taken. Enough that a future explorer does not re-litigate it. +## Rejected: defaulting to True + +One heading per alternative, named in the heading so it gets its own anchor. + ## Consequence The non-obvious downstream effect, including what this deliberately leaves @@ -56,8 +60,7 @@ uncovered. The concrete signal that should reopen this decision. ``` -`## Rejected:` repeats once per alternative worth remembering. `## Consequence` -is optional. `## Revisit trigger` is required and enforced. +`## Consequence` is optional. `## Revisit trigger` is required and enforced. ## Where other facts go