From 3588340015a3afe4eafd70854282d1a5b67947a1 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 6 Sep 2026 21:32:32 +0300 Subject: [PATCH 1/2] docs: retire planning/ and route deferred work to GitHub Issues The nine items in planning/deferred/ become issues #16-#24, labelled needs-triage. Bodies are the files verbatim; the summary frontmatter is the opening sentence and relative links become absolute GitHub URLs. docs/agents/issue-tracker.md flips from local .scratch/ markdown to GitHub, so the tracker AGENTS.md now routes deferred work to is the one the repo documents. The five canonical triage roles become real labels rather than a Status: line in a file that no longer exists. links.py and just check-links go with planning/, on the same reasoning that retires the directory: the checker existed to guard a tree the repo no longer has. Closes modern-python/.github#70 --- .github/workflows/main.yml | 2 - AGENTS.md | 56 ++++--- docs/adr/0006-mutation-requires-membership.md | 2 +- docs/adr/0014-auth-carries-an-actor-id.md | 7 +- docs/adr/README.md | 6 +- docs/agents/issue-tracker.md | 55 +++++-- justfile | 12 -- planning/.convention-version | 1 - planning/README.md | 137 ---------------- planning/_templates/deferred.md | 18 --- ...dit-message-duplicates-text-constraints.md | 16 -- ...-21-isolation-test-pair-order-dependent.md | 21 --- ...itestar-channels-empty-entries-retained.md | 24 --- ...1-litestar-channels-subscriber-orphaned.md | 22 --- .../2026-08-21-logout-does-not-revoke-jwt.md | 17 -- ...6-08-21-message-id-existence-404-vs-403.md | 21 --- ...26-08-21-no-query-count-instrumentation.md | 17 -- ...026-08-21-per-test-rollback-fail-silent.md | 21 --- .../2026-08-21-presence-beyond-ttl-key.md | 15 -- planning/index.py | 139 ---------------- planning/links.py | 148 ------------------ pyproject.toml | 7 +- 22 files changed, 83 insertions(+), 681 deletions(-) delete mode 100644 planning/.convention-version delete mode 100644 planning/README.md delete mode 100644 planning/_templates/deferred.md delete mode 100644 planning/deferred/2026-08-21-edit-message-duplicates-text-constraints.md delete mode 100644 planning/deferred/2026-08-21-isolation-test-pair-order-dependent.md delete mode 100644 planning/deferred/2026-08-21-litestar-channels-empty-entries-retained.md delete mode 100644 planning/deferred/2026-08-21-litestar-channels-subscriber-orphaned.md delete mode 100644 planning/deferred/2026-08-21-logout-does-not-revoke-jwt.md delete mode 100644 planning/deferred/2026-08-21-message-id-existence-404-vs-403.md delete mode 100644 planning/deferred/2026-08-21-no-query-count-instrumentation.md delete mode 100644 planning/deferred/2026-08-21-per-test-rollback-fail-silent.md delete mode 100644 planning/deferred/2026-08-21-presence-beyond-ttl-key.md delete mode 100644 planning/index.py delete mode 100644 planning/links.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5332a77..b84567e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -23,9 +23,7 @@ jobs: uv run ruff format . --check 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: runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 8943165..f1a951b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,10 +38,9 @@ only covers what isn't obvious from the recipe names. Almost everything runs through Docker Compose: the app and Postgres come up together, and running tests/migrations outside Docker is **not** the -supported path (`just install`, `just lint`, `just index`, `just -check-planning` and `just check-links` are the exceptions — they run on the -host). Inside the container, raw commands look like `uv run pytest ...`, `uv -run alembic ...`. +supported path (`just install`, `just lint` and `just check-adrs` are the +exceptions — they run on the host). Inside the container, raw commands look +like `uv run pytest ...`, `uv run alembic ...`. - `just test` cycles the DB (downgrade to `base`, upgrade to `head`) before pytest and tears the stack down before and after. Pass pytest args through, @@ -66,24 +65,39 @@ 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 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. +- `just check-adrs` validates `docs/adr/` numbering, naming and revisit + triggers. CI runs it as a lint step. Python is 3.14, dependencies managed by `uv`. The API is exposed on `:8000`. ## Workflow -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 -with the diff, not promoted to a page. See `planning/README.md` for the full -convention, including the admission check that decides where a given fact -belongs. +**The spec for a change is its PR body**, not a committed file. Two things +outlive the PR, and there are exactly two places to put them: an alternative +**rejected** with reasoning becomes a numbered ADR in +[`docs/adr/`](docs/adr/), and real work **not scheduled** becomes a GitHub +issue. There is no third state and no capability-page home — the living truth +about behaviour is the code and its `INVARIANT:`-marked tests, and a behaviour +change is reviewed with the diff, not promoted to a page. + +### Where a fact goes + +Four homes, one owner each: + +| Home | Holds | +|---|---| +| `app/` | anything readable from the module — the default | +| a named test | an **invariant**: must stay true, and a change could silently break it | +| `docs/adr/` | a rejected alternative, with the reasoning that would otherwise be re-litigated | +| a GitHub issue | real work, not scheduled | + +Before writing a line anywhere: + +> Can an agent get this by reading `app/`? → **don't write it.** +> Would a wrong change here fail a test? → it belongs **in the test**, not in prose. +> Otherwise it does not get written. + +**Prose about mechanism has no home. There is no file to add a paragraph to.** An invariant is a test whose name is the claim, with a docstring opening `INVARIANT:` and a second paragraph naming what breaks it. Applied to new @@ -216,13 +230,13 @@ or a meaning subtle enough that code and docs must agree on it. ### Issue tracker -Issues and specs live as markdown files under `.scratch//` -(gitignored). See [`docs/agents/issue-tracker.md`](docs/agents/issue-tracker.md). +Issues and specs live as GitHub issues on `modern-python/chat-app`, driven with +`gh`. See [`docs/agents/issue-tracker.md`](docs/agents/issue-tracker.md). ### Triage labels -The five canonical triage roles, unchanged, recorded as a `Status:` line in each -issue file. See [`docs/agents/triage-labels.md`](docs/agents/triage-labels.md). +The five canonical triage roles, unchanged, applied as GitHub labels. See +[`docs/agents/triage-labels.md`](docs/agents/triage-labels.md). ### Domain docs diff --git a/docs/adr/0006-mutation-requires-membership.md b/docs/adr/0006-mutation-requires-membership.md index 04771b4..2e3f074 100644 --- a/docs/adr/0006-mutation-requires-membership.md +++ b/docs/adr/0006-mutation-requires-membership.md @@ -27,7 +27,7 @@ A non-member still learns whether a message id exists, because the message must be loaded before its chat is known. That residual is accepted deliberately and mirrors the decision that `FetchChatUseCase` returns `403` rather than pretending the chat does not exist. See -`planning/deferred/2026-08-21-message-id-existence-404-vs-403.md`. +[#21](https://github.com/modern-python/chat-app/issues/21). ## Revisit trigger diff --git a/docs/adr/0014-auth-carries-an-actor-id.md b/docs/adr/0014-auth-carries-an-actor-id.md index 0f410e5..cffcd29 100644 --- a/docs/adr/0014-auth-carries-an-actor-id.md +++ b/docs/adr/0014-auth-carries-an-actor-id.md @@ -47,11 +47,10 @@ Authentication no longer proves the user exists. A token whose row is gone authenticates: reads come back empty, writes hit the `messages.user_id` foreign key. Nothing can reach that state today — there is no delete-user or disable-user path — and the accepted cost is recorded in the invariant test's -docstring, not as a deferred item. +docstring, not as an open issue. ## Revisit trigger A delete-user or disable-user path being added. It meets the same problem as -[`../../planning/deferred/2026-08-21-logout-does-not-revoke-jwt.md`](../../planning/deferred/2026-08-21-logout-does-not-revoke-jwt.md) -— a credential outliving what it names — and both should be solved once, -together. +[#20](https://github.com/modern-python/chat-app/issues/20) — a credential +outliving what it names — and both should be solved once, together. diff --git a/docs/adr/README.md b/docs/adr/README.md index c8955ce..ed51497 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -65,6 +65,6 @@ The concrete signal that should reopen this decision. ## Where other facts go 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 code, an `INVARIANT:`-marked test, an ADR -here, and a deferred item in [`../../planning/deferred/`](../../planning/deferred/). +[`../../AGENTS.md`](../../AGENTS.md#where-a-fact-goes) for the admission check +that decides between code, an `INVARIANT:`-marked test, an ADR here, and a +GitHub issue for real work that is not scheduled. diff --git a/docs/agents/issue-tracker.md b/docs/agents/issue-tracker.md index 0209a19..3c2736c 100644 --- a/docs/agents/issue-tracker.md +++ b/docs/agents/issue-tracker.md @@ -1,30 +1,53 @@ -# Issue tracker: Local Markdown +# Issue tracker: GitHub -Issues and specs for this repo live as markdown files in `.scratch/`. +Issues and specs for this repo live as GitHub issues on `modern-python/chat-app`. Use the `gh` CLI +for all operations. ## Conventions -- One feature per directory: `.scratch//` -- The spec is `.scratch//spec.md` -- Implementation issues are one file per ticket at `.scratch//issues/-.md`, numbered from `01`, never a single combined tickets file -- Triage state is recorded as a `Status:` line near the top of each issue file (see `triage-labels.md` for the role strings) -- Comments and conversation history append to the bottom of the file under a `## Comments` heading +- **Create an issue**: `gh issue create --title "..." --body "..."`. Use a heredoc for multi-line bodies. +- **Read an issue**: `gh issue view --comments`, filtering comments by `jq` and also fetching labels. +- **List issues**: `gh issue list --state open --json number,title,body,labels,comments --jq '[.[] | {number, title, body, labels: [.labels[].name], comments: [.comments[].body]}]'` with appropriate `--label` and `--state` filters. +- **Comment on an issue**: `gh issue comment --body "..."` +- **Apply / remove labels**: `gh issue edit --add-label "..."` / `--remove-label "..."` +- **Close**: `gh issue close --comment "..."` + +Infer the repo from `git remote -v`; `gh` does this automatically when run inside a clone. + +## Pull requests as a triage surface + +**PRs as a request surface: no.** _(Set to `yes` if this repo treats external PRs as feature requests; `/triage` reads this flag.)_ + +When set to `yes`, PRs run through the same labels and states as issues, using the `gh pr` equivalents: + +- **Read a PR**: `gh pr view --comments` and `gh pr diff ` for the diff. +- **List external PRs for triage**: `gh pr list --json` has no `authorAssociation` field, so the association has to come from the REST API, where it is `author_association` (snake_case): + ``` + gh api "repos/{owner}/{repo}/pulls?state=open&per_page=100" \ + --jq '.[] | select(.author_association | IN("CONTRIBUTOR","FIRST_TIME_CONTRIBUTOR","NONE")) | {number, title, author: .user.login}' + ``` + That keeps only external authors; `OWNER`, `MEMBER` and `COLLABORATOR` are dropped. `gh api` substitutes `{owner}`/`{repo}` from the current clone. +- **Comment / label / close**: `gh pr comment`, `gh pr edit --add-label`/`--remove-label`, `gh pr close`. + +GitHub shares one number space across issues and PRs, so a bare `#42` may be either: resolve with `gh pr view 42` and fall back to `gh issue view 42`. ## When a skill says "publish to the issue tracker" -Create a new file under `.scratch//` (creating the directory if needed). +Create a GitHub issue. ## When a skill says "fetch the relevant ticket" -Read the file at the referenced path. The user will normally pass the path or the issue number directly. +Run `gh issue view --comments`. ## Wayfinding operations -Used by `/wayfinder`. The **map** is a file with one **child** file per ticket. +Used by `/wayfinder`. The **map** is a single issue with **child** issues as tickets. + +- **Map**: a single issue labelled `wayfinder:map`, holding the Notes / Decisions-so-far / Fog body. `gh issue create --label wayfinder:map`. +- **Child ticket**: an issue linked to the map as a GitHub sub-issue (`gh api` on the sub-issues endpoint). Where sub-issues aren't enabled, add the child to a task list in the map body and put `Part of #` at the top of the child body. Labels: `wayfinder:` (`research`/`prototype`/`grilling`/`task`). Once claimed, the ticket is assigned to the driving dev. +- **Blocking**: GitHub's **native issue dependencies**, the canonical, UI-visible representation. Add an edge with `gh api --method POST repos///issues//dependencies/blocked_by -F issue_id=`, where `` is the blocker's numeric **database id** (`gh api repos///issues/ --jq .id`, _not_ the `#number` or `node_id`). GitHub reports `issue_dependencies_summary.blocked_by` (open blockers only, the live gate). Where dependencies aren't available, fall back to a `Blocked by: #, #` line at the top of the child body. A ticket is unblocked when every blocker is closed. +- **Frontier query**: list the map's open children (`gh issue list --state open`, scoped to the map's sub-issues / task list), drop any with an open blocker (`issue_dependencies_summary.blocked_by > 0`, or an open issue in the `Blocked by` line) or an assignee; first in map order wins. +- **Claim**: `gh issue edit --add-assignee @me`, the session's first write. +- **Resolve**: `gh issue comment --body ""`, then `gh issue close `, then append a context pointer (gist + link) to the map's Decisions-so-far. -- **Map**: `.scratch//map.md` (the Notes / Decisions-so-far / Fog body). -- **Child ticket**: `.scratch//issues/NN-.md`, numbered from `01`, with the question in the body. A `Type:` line records the ticket type (`research`/`prototype`/`grilling`/`task`); a `Status:` line records `claimed`/`resolved`. -- **Blocking**: a `Blocked by: NN, NN` line near the top. A ticket is unblocked when every file it lists is `resolved`. -- **Frontier**: scan `.scratch//issues/` for files that are open, unblocked, and unclaimed; first by number wins. -- **Claim**: set `Status: claimed` and save before any work. -- **Resolve**: append the answer under an `## Answer` heading, set `Status: resolved`, then append a context pointer (gist + link) to the map's Decisions-so-far in `map.md`. +The `wayfinder:*` labels do not exist in this repo yet. Create them the first time `/wayfinder` runs. diff --git a/justfile b/justfile index 15e6b79..f165f39 100644 --- a/justfile +++ b/justfile @@ -35,18 +35,6 @@ lint: uv run ruff check . --fix uv run ty check -# Print the planning index (the deferred queue) to stdout. -index: - uv run python planning/index.py - -# 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/.convention-version b/planning/.convention-version deleted file mode 100644 index ccbccc3..0000000 --- a/planning/.convention-version +++ /dev/null @@ -1 +0,0 @@ -2.2.0 diff --git a/planning/README.md b/planning/README.md deleted file mode 100644 index 93a6230..0000000 --- a/planning/README.md +++ /dev/null @@ -1,137 +0,0 @@ -# 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 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 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) - -**1. Write the spec in the PR body.** There is no change file to write and nothing to -commit: the PR body *is* the spec, reviewed inline with the diff. A trivial PR (typo, -dep bump, formatter, mechanical rename) ships a conventional-commit title with no body -ceremony. - -**2. File what outlives the PR:** - -- an alternative you **rejected** with reasoning → [`../docs/adr/`](../docs/adr/) -- work that is real but **not scheduled** → [`deferred/`](deferred/) - -**3. Run `just check-planning`, `just check-adrs` and `just check-links` before pushing.** - -## Where a fact goes - -Four homes, one owner each: - -| Home | Holds | -|---|---| -| `app/` | anything readable from the module — the default | -| a named test | an **invariant**: must stay true, and a change could silently break it | -| `../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: - -> Can an agent get this by reading `app/`? → **don't write it.** -> Would a wrong change here fail a test? → it belongs **in the test**, not in prose. -> Otherwise it does not get written. - -**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 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 -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. - -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. - -An invariant is written as a test whose name is the claim, with a docstring -opening `INVARIANT:` and a second paragraph naming **what breaks it**. That -second paragraph is design rationale — an anti-refactor warning — not a report -of what this one test happens to catch. - -## Artifacts - -- **[`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/)** — `deferred.md`. - -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. - -### Location is status - -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** → 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 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 - -Against upstream convention 2.2.0: - -1. `changes/`, `audits/` and `retros/` are removed; the per-change spec is the - PR body. -2. `architecture/` is removed; there is no capability-page home and no promotion - rule. Enforceable claims are `INVARIANT:`-marked tests; the ubiquitous - language lives in [`../CONTEXT.md`](../CONTEXT.md). -3. `deferred.md` is a `deferred/` directory of indexed, trigger-bearing items. -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; 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/deferred.md b/planning/_templates/deferred.md deleted file mode 100644 index 43146f4..0000000 --- a/planning/_templates/deferred.md +++ /dev/null @@ -1,18 +0,0 @@ ---- -summary: One line — shown in `just index`. ---- - -# One-line capitalized title - -What the item is, in a sentence or two. - -## Why it is open - -The substance: the evidence, measurements, and reasoning needed to pick this up -cold. Inline it — a deferred item cites no report and no change file, because -this file is the only place the reasoning lives. - -## Revisit trigger - -The concrete signal that should make someone act on this. An item with no -trigger is not deferred, it is abandoned. diff --git a/planning/deferred/2026-08-21-edit-message-duplicates-text-constraints.md b/planning/deferred/2026-08-21-edit-message-duplicates-text-constraints.md deleted file mode 100644 index 1953205..0000000 --- a/planning/deferred/2026-08-21-edit-message-duplicates-text-constraints.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -summary: `SendMessageRequest.text` and `EditMessageRequest.text` independently declare the same length constraints, so a change to one silently doesn't affect the other. ---- - -# EditMessageRequest duplicates SendMessageRequest's text constraints - -## Why it is open - -Both `app/schemas/api.py::SendMessageRequest.text` and `EditMessageRequest.text` -independently declare `pydantic.Field(min_length=1, max_length=4000)`. A -change to one's bounds is silently not a change to the other's. - -## Revisit trigger - -The two are ever meant to diverge deliberately, or a bug report about edit -accepting/rejecting text that send doesn't (or vice versa). diff --git a/planning/deferred/2026-08-21-isolation-test-pair-order-dependent.md b/planning/deferred/2026-08-21-isolation-test-pair-order-dependent.md deleted file mode 100644 index 6030a6c..0000000 --- a/planning/deferred/2026-08-21-isolation-test-pair-order-dependent.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -summary: The two tests proving the per-test rollback fixture only prove it when run together in file order; run the second alone and it passes vacuously. ---- - -# Isolation test pair is order-dependent - -## Why it is open - -`tests/test_main.py::test_db_session_insert_is_visible_within_test` and -`test_db_session_rolls_back_between_tests` together prove the per-test -rollback fixture, but only when pytest runs them in file order: the first -inserts and commits, the second asserts the table is empty. Run the second -alone (e.g. `-k test_db_session_rolls_back_between_tests`) and it passes -vacuously — an empty table before any insert is indistinguishable from a -correctly rolled-back one. - -## Revisit trigger - -Test order ever becomes non-deterministic (parallel pytest execution, -`pytest-randomly`), or before trusting `-k` output from just this pair as -proof the fixture works. diff --git a/planning/deferred/2026-08-21-litestar-channels-empty-entries-retained.md b/planning/deferred/2026-08-21-litestar-channels-empty-entries-retained.md deleted file mode 100644 index 406fb4b..0000000 --- a/planning/deferred/2026-08-21-litestar-channels-empty-entries-retained.md +++ /dev/null @@ -1,24 +0,0 @@ ---- -summary: `unsubscribe` removes the subscriber but leaves the now-empty channel entry behind in `self._channels`, growing unboundedly with distinct users. ---- - -# Litestar channels: empty channel entries retained after unsubscribe - -## Why it is open - -`unsubscribe` removes the subscriber but leaves the now-empty `set()` and its key -in `self._channels`. With per-user channel names that dict grows by one entry per -distinct user that ever connects, in a singleton that lives for the whole -process. Upstream: -[litestar#4867](https://github.com/litestar-org/litestar/issues/4867). - -`rchat`'s `PruningChannelsPlugin` overrides the public `unsubscribe` to drop -empty entries, so this one needs no private access. Still not shipped, for -symmetry with -[2026-08-21-litestar-channels-subscriber-orphaned.md](2026-08-21-litestar-channels-subscriber-orphaned.md) -and because the growth is bounded by distinct users in a demo. - -## Revisit trigger - -Upstream fix released, or the app being run anywhere with a non-trivial user -population. diff --git a/planning/deferred/2026-08-21-litestar-channels-subscriber-orphaned.md b/planning/deferred/2026-08-21-litestar-channels-subscriber-orphaned.md deleted file mode 100644 index 7d83607..0000000 --- a/planning/deferred/2026-08-21-litestar-channels-subscriber-orphaned.md +++ /dev/null @@ -1,22 +0,0 @@ ---- -summary: `ChannelsPlugin.subscribe()` registers a subscriber before awaiting the history fetch, so a client disconnecting mid-subscribe leaves it orphaned and never unsubscribed. ---- - -# Litestar channels: subscriber orphaned on mid-subscribe disconnect - -## Why it is open - -`ChannelsPlugin.subscribe()` registers the subscriber into `_channels` before -awaiting the history fetch, so a client disconnecting mid-subscribe leaves a -registered subscriber that is never unsubscribed. Upstream: -[litestar#4871](https://github.com/litestar-org/litestar/issues/4871). - -`rchat` works around it by reordering the operations, which requires reaching -into `plugin._subscriber_class`, `plugin._channels`, and `plugin._backend`. Not -shipped here: a reference repository demonstrating private-attribute access -teaches the wrong lesson, and the leak is inert at demo scale. - -## Revisit trigger - -Upstream fix released, or a deployment where connection churn is high enough -for the leak to matter. diff --git a/planning/deferred/2026-08-21-logout-does-not-revoke-jwt.md b/planning/deferred/2026-08-21-logout-does-not-revoke-jwt.md deleted file mode 100644 index 5390b54..0000000 --- a/planning/deferred/2026-08-21-logout-does-not-revoke-jwt.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -summary: Logout deletes the cookie but does not revoke the JWT, which stays valid for the rest of its lifetime if it was copied beforehand. ---- - -# Logout does not revoke the JWT - -## Why it is open - -`POST /api/auth/logout/` deletes the cookie but the token itself stays valid -for the rest of its `jwt_lifetime_seconds` (7 days by default) if it was -copied out of the cookie beforehand — no `revoked_token_handler` is -configured on `jwt_cookie_auth`. - -## Revisit trigger - -Any deployment where a leaked/copied token is a realistic threat model, or -before shipping a "log out of all devices" feature. 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 deleted file mode 100644 index 52e189e..0000000 --- a/planning/deferred/2026-08-21-message-id-existence-404-vs-403.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -summary: A non-member issuing `PATCH`/`DELETE /api/messages/{id}/` gets `404` for a nonexistent id and `403` for one that exists but isn't theirs, leaking whether the id is real. ---- - -# Message id existence is distinguishable via 404-vs-403 - -## Why it is open - -A non-member issuing `PATCH`/`DELETE /api/messages/{id}/` gets `404` for an -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 `../../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. - -## Revisit trigger - -A threat model where message-id enumeration by a non-member is a real -concern (e.g. ids that encode something sensitive). diff --git a/planning/deferred/2026-08-21-no-query-count-instrumentation.md b/planning/deferred/2026-08-21-no-query-count-instrumentation.md deleted file mode 100644 index 1de7cb2..0000000 --- a/planning/deferred/2026-08-21-no-query-count-instrumentation.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -summary: Nothing in the test suite counts queries per request, so an N+1 regression in chat listing would keep `just test` green as long as the returned data stays correct. ---- - -# No query-count instrumentation - -## Why it is open - -Nothing in the test suite counts queries per request, so an N+1 regression in -the chat listing (e.g. `FetchChatsUseCase`'s bounded `last_message` lookup -regressing back to one query per chat) would keep `just test` green as long -as the returned data is still correct. - -## Revisit trigger - -A reported latency regression on `GET /api/chats/`, or before adding another -listing endpoint that joins per-row data. diff --git a/planning/deferred/2026-08-21-per-test-rollback-fail-silent.md b/planning/deferred/2026-08-21-per-test-rollback-fail-silent.md deleted file mode 100644 index ffe4d5c..0000000 --- a/planning/deferred/2026-08-21-per-test-rollback-fail-silent.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -summary: The `if connection.in_transaction():` guard in `tests/conftest.py`'s `db_session` teardown silently skips the rollback whenever the outer transaction is already closed. ---- - -# Per-test rollback is fail-silent on an unexpected commit - -## Why it is open - -The `if connection.in_transaction():` guard in `tests/conftest.py`'s -`db_session` teardown skips the rollback without error whenever the outer -transaction is already closed. It exists to tolerate tests that legitimately -closed their own transaction, but it can't distinguish that from a session -somewhere having committed the outer transaction instead of nesting a -savepoint under it — that failure mode would leak state into the next test -with no diagnostic. - -## Revisit trigger - -A test suite flake that looks like cross-test state leakage, or before adding -any code path that opens a session without going through -`database_resources.create_session`. diff --git a/planning/deferred/2026-08-21-presence-beyond-ttl-key.md b/planning/deferred/2026-08-21-presence-beyond-ttl-key.md deleted file mode 100644 index 884fde5..0000000 --- a/planning/deferred/2026-08-21-presence-beyond-ttl-key.md +++ /dev/null @@ -1,15 +0,0 @@ ---- -summary: Presence is planned as a Redis key with a TTL refreshed by the SSE heartbeat, which reports stream-open rather than actively-viewing. ---- - -# Presence beyond a TTL key - -## Why it is open - -Presence is planned as a Redis key with a TTL refreshed by the SSE heartbeat. -This reports "has an open stream", not "is looking at this chat", and a client -killed between heartbeats stays online until expiry. - -## Revisit trigger - -The demo needing per-chat presence or accurate last-seen. diff --git a/planning/index.py b/planning/index.py deleted file mode 100644 index 5c7d74b..0000000 --- a/planning/index.py +++ /dev/null @@ -1,139 +0,0 @@ -# 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``, 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 -import re -import sys - - -ROOT = pathlib.Path(__file__).parent -DEFERRED_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") -DEFERRED_REQUIRED = ("summary",) - - -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 - cleaned = value.strip().strip('"').strip("'") - fields[key.strip()] = "" if cleaned == "null" else cleaned - return fields - - -def _named(fields: dict[str, str], name: str, pattern: re.Pattern[str]) -> dict[str, str]: - """Inject ``date``/``slug`` derived from a file name into ``fields``.""" - match = pattern.match(name) - if match: - fields["date"] = match.group("date") - fields["slug"] = match.group("slug") - return fields - - -def load_deferred(root: pathlib.Path) -> list[dict[str, str]]: - """Read each deferred item's summary; derive date/slug from the file name.""" - deferred_dir = root / "deferred" - deferred: list[dict[str, str]] = [] - if not deferred_dir.is_dir(): - return deferred - for path in sorted(deferred_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, DEFERRED_RE) - fields["path"] = f"deferred/{path.name}" - fields["name"] = path.stem - deferred.append(fields) - return deferred - - -def format_row(row: dict[str, str]) -> str: - """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)" - return f"- **[{slug}]({path})** ({date}) — {summary}" - - -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.append("") - return "\n".join(out).rstrip() + "\n" - - -def _require(fields: dict[str, str], keys: tuple[str, ...], rel: str, violations: list[str]) -> None: - """Append a violation for each required key that is absent or empty.""" - violations.extend(f"{rel}: missing or empty frontmatter key '{key}'" for key in keys if not fields.get(key)) - - -def _check_deferred(path: pathlib.Path, violations: list[str]) -> None: - """Validate one deferred item (requires `summary` + a revisit trigger).""" - rel = f"deferred/{path.name}" - if DEFERRED_RE.match(path.stem) is None: - violations.append(f"{rel}: file name is not 'YYYY-MM-DD-slug.md'") - text = path.read_text(encoding="utf-8") - _require(parse_frontmatter(text), DEFERRED_REQUIRED, rel, violations) - if "Revisit trigger" not in text: - violations.append( - f"{rel}: no '**Revisit trigger:**' section — an item with no trigger is abandoned, not deferred" - ) - - -def check(root: pathlib.Path) -> list[str]: - """Validate every deferred item; return the list of violation strings.""" - violations: list[str] = [] - deferred_dir = root / "deferred" - if deferred_dir.is_dir(): - for path in sorted(deferred_dir.iterdir()): - if path.name == "README.md" or path.name.startswith(("_", ".")): - continue - if path.suffix != ".md": - violations.append(f"deferred/{path.name}: unexpected non-md file in deferred/") - else: - _check_deferred(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 with --check.""" - argv = sys.argv[1:] if argv is None else argv - root = ROOT if root is None else root - if "--check" in argv: - violations = check(root) - if violations: - sys.stderr.write(f"planning: {len(violations)} violation(s)\n") - for violation in violations: - sys.stderr.write(f" - {violation}\n") - return 1 - sys.stdout.write("planning: OK\n") - return 0 - sys.stdout.write(render(load_deferred(root))) - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/planning/links.py b/planning/links.py deleted file mode 100644 index a9d5df8..0000000 --- a/planning/links.py +++ /dev/null @@ -1,148 +0,0 @@ -# planning/ is not a Python package (this file is vendored into consumers' planning/) -"""Check every relative Markdown link and heading anchor in the repository. - -Run via ``just check-links``. Exists because a site builder only validates the -directory it publishes: a repo's ``architecture/`` and ``planning/`` trees usually -sit outside it, are read on GitHub, and rot silently. In the repo this convention -came from, anchors in ``architecture/`` broke three times in one week, each caught -only by a human re-deriving slugs by hand. - -Slugs follow **GitHub's** algorithm, because that is where these files are read — -including the ones a site builder also publishes. Where the two disagree, the fix -is to change the heading rather than to teach this checker both dialects: a heading -containing an em dash yields ``a--b`` on GitHub (the dash is dropped, both spaces -become hyphens) and ``a-b`` under python-markdown (the whitespace run collapses). - -External links are not fetched; this checks the repository's internal consistency. -A relative link that resolves outside the repository is reported rather than followed: -it is a 404 on GitHub, and whether it resolves on disk depends on what the author -happens to have cloned next to the repo — a verdict a lint gate must never depend on. -""" - -import argparse -import collections -import pathlib -import re -import sys - - -SKIP_DIRS = frozenset({".git", ".venv", ".tox", "site", "node_modules", "__pycache__", ".ruff_cache", ".superpowers"}) -FENCE = re.compile(r"^\s*(```|~~~)") -INLINE_CODE = re.compile(r"(`+).+?\1") # any run of backticks delimits a span: `x`, ``a`b`` -HEADING = re.compile(r"^(#{1,6})\s+(.+?)\s*$") -LINK = re.compile(r"\[[^\]]*\]\(\s*([^)\s]+)(?:\s+\"[^\"]*\")?\s*\)") -EXTERNAL = re.compile(r"^(?:[a-z][a-z0-9+.-]*:|//)", re.IGNORECASE) - - -def repo_root(start: pathlib.Path) -> pathlib.Path: - """Nearest ancestor holding ``.git``, else ``start``. - - Found rather than computed because this file has two homes: the canonical repo's - root, and a consumer's ``planning/`` — a fixed relative depth is wrong in one of them. - """ - for candidate in [start, *start.parents]: - if (candidate / ".git").exists(): - return candidate - return start - - -def strip_fences(text: str) -> str: - """Blank out fenced blocks, keeping line count, so code is never read as a heading.""" - out, fenced = [], False - for line in text.splitlines(): - if FENCE.match(line): - fenced = not fenced - out.append("") - continue - out.append("" if fenced else line) - return "\n".join(out) - - -def link_lines(text: str) -> list[str]: - """Lines with fenced blocks and inline spans removed — what to scan for real links. - - Only link scanning strips inline spans. A page documenting the markup an author should - copy is not linking anywhere, while a heading's backticked content is part of its slug. - """ - return [INLINE_CODE.sub("", line) for line in strip_fences(text).splitlines()] - - -def slugify(heading: str) -> str: - """GitHub's heading slug: drop formatting and punctuation, lowercase, spaces to hyphens.""" - text = re.sub(r"`([^`]*)`", r"\1", heading) - text = re.sub(r"\[([^\]]*)\]\([^)]*\)", r"\1", text) - # `*` and `~` are emphasis; `_` is kept because GitHub keeps it and headings name - # identifiers (`bound_type`) far more often than they use underscore-italics. - text = re.sub(r"[*~]", "", text) - text = "".join(ch for ch in text.lower() if ch.isalnum() or ch in " -_") - return text.strip().replace(" ", "-") - - -def anchors(text: str) -> set[str]: - """Every anchor a reader can target, including GitHub's ``-1``/``-2`` duplicate suffixes.""" - seen: collections.Counter[str] = collections.Counter() - found: set[str] = set() - for line in strip_fences(text).splitlines(): - match = HEADING.match(line) - if not match: - continue - base = slugify(match.group(2)) - found.add(base if not seen[base] else f"{base}-{seen[base]}") - seen[base] += 1 - return found - - -def check(root: pathlib.Path) -> list[str]: - """Return one message per broken link; empty means every internal link resolves.""" - root = root.resolve() - files = sorted(p for p in root.rglob("*.md") if not SKIP_DIRS & set(p.relative_to(root).parts)) - cache: dict[pathlib.Path, set[str]] = {} - violations: list[str] = [] - for path in files: - text = path.read_text(encoding="utf-8") - for line_no, line in enumerate(link_lines(text), 1): - for target in LINK.findall(line): - if EXTERNAL.match(target): - continue - rel, _, fragment = target.partition("#") - # A bare `#frag` targets this same file — the anchor is still checkable, - # and a same-page link rots exactly like a cross-page one. - dest = (path.parent / rel).resolve() if rel else path - where = f"{path.relative_to(root)}:{line_no}" - if dest != root and root not in dest.parents: - # Judged before existence: a sibling repo cloned alongside this one makes - # ../../../other-repo/… resolve on one machine and nowhere else, and it is - # a 404 on GitHub either way. The verdict must not depend on the checkout layout. - violations.append(f"{where}: leaves the repository -> {rel}") - continue - if not dest.exists(): - violations.append(f"{where}: no such file -> {rel}") - continue - if not fragment or dest.suffix != ".md": - continue - if dest not in cache: - cache[dest] = anchors(dest.read_text(encoding="utf-8")) - if fragment.lower() not in cache[dest]: - violations.append(f"{where}: no such anchor -> {target}") - return violations - - -def main(argv: list[str] | None = None, root: pathlib.Path | None = None) -> int: - """Report every broken link; return 1 if any, else 0.""" - parser = argparse.ArgumentParser(description="Check Markdown links and heading anchors.") - parser.add_argument("--root", type=pathlib.Path, default=None) - args = parser.parse_args(sys.argv[1:] if argv is None else argv) - - target = args.root or root or repo_root(pathlib.Path(__file__).resolve().parent) - violations = check(target) - if violations: - sys.stderr.write(f"links: {len(violations)} broken\n") - for violation in violations: - sys.stderr.write(f" - {violation}\n") - return 1 - sys.stdout.write("links: OK\n") - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/pyproject.toml b/pyproject.toml index 545ac77..c1c1a76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -93,10 +93,7 @@ omit = [ # run under `just test-migrations` instead, where coverage is off. "tests/migrations/*", "app/api/__main__.py", - # No __init__.py under planning/ (see the comment atop planning/index.py), so coverage's - # package walk never reaches these on its own - omit them explicitly rather than relying - # on that as an accident of discovery. - "planning/index.py", - "planning/links.py", + # No __init__.py under docs/, so coverage's package walk never reaches this on its own - + # omit it explicitly rather than relying on that as an accident of discovery. "docs/adr/check.py", ] From 16a719649123680336608927b67403b4c60c3323 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 6 Sep 2026 21:47:16 +0300 Subject: [PATCH 2/2] docs: drop the ADR checker check.py, `just check-adrs` and the CI step go. docs/adr/README.md no longer claims the set is validated or the revisit trigger enforced: a reviewer is what holds the standard up now. With links.py already gone the repo has no doc validators, so the coverage omit list drops its last non-app entry and the lint job is ruff and ty alone. --- .github/workflows/main.yml | 1 - AGENTS.md | 8 ++- docs/adr/README.md | 7 +-- docs/adr/check.py | 107 ------------------------------------- justfile | 4 -- pyproject.toml | 3 -- 6 files changed, 7 insertions(+), 123 deletions(-) delete mode 100644 docs/adr/check.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b84567e..c9d6784 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -23,7 +23,6 @@ jobs: uv run ruff format . --check uv run ruff check . --no-fix uv run ty check - uv run python docs/adr/check.py pytest: runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index f1a951b..054ca7c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,9 +38,9 @@ only covers what isn't obvious from the recipe names. Almost everything runs through Docker Compose: the app and Postgres come up together, and running tests/migrations outside Docker is **not** the -supported path (`just install`, `just lint` and `just check-adrs` are the -exceptions — they run on the host). Inside the container, raw commands look -like `uv run pytest ...`, `uv run alembic ...`. +supported path (`just install` and `just lint` are the exceptions — they run +on the host). Inside the container, raw commands look like `uv run pytest ...`, +`uv run alembic ...`. - `just test` cycles the DB (downgrade to `base`, upgrade to `head`) before pytest and tears the stack down before and after. Pass pytest args through, @@ -65,8 +65,6 @@ like `uv run pytest ...`, `uv 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 check-adrs` validates `docs/adr/` numbering, naming and revisit - triggers. CI runs it as a lint step. Python is 3.14, dependencies managed by `uv`. The API is exposed on `:8000`. diff --git a/docs/adr/README.md b/docs/adr/README.md index ed51497..a64411d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,8 +2,8 @@ One file per decision taken, especially the options **rejected**, so reviews do 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. +listing and no `summary` frontmatter. Nothing validates the set mechanically; +the standard below is held up by review. ## Numbering @@ -60,7 +60,8 @@ uncovered. The concrete signal that should reopen this decision. ``` -`## Consequence` is optional. `## Revisit trigger` is required and enforced. +`## Consequence` is optional. `## Revisit trigger` is required; a reviewer is +what enforces it. ## Where other facts go diff --git a/docs/adr/check.py b/docs/adr/check.py deleted file mode 100644 index db8d759..0000000 --- a/docs/adr/check.py +++ /dev/null @@ -1,107 +0,0 @@ -"""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 index f165f39..087c211 100644 --- a/justfile +++ b/justfile @@ -34,7 +34,3 @@ lint: uv run ruff format . uv run ruff check . --fix uv run ty check - -# Validate docs/adr/ numbering, naming and revisit triggers; CI runs this. -check-adrs: - uv run python docs/adr/check.py diff --git a/pyproject.toml b/pyproject.toml index c1c1a76..ee84d52 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -93,7 +93,4 @@ omit = [ # run under `just test-migrations` instead, where coverage is off. "tests/migrations/*", "app/api/__main__.py", - # No __init__.py under docs/, so coverage's package walk never reaches this on its own - - # omit it explicitly rather than relying on that as an accident of discovery. - "docs/adr/check.py", ]