♻️ Single-home the actionable rules and the rationale

development/ had restated the actionables that CONTRIBUTING.md owns: the
branching step list, the script prefix list, the feedback-tier rule of thumb,
the commit convention and the type-driven test loop. Those now live only in
CONTRIBUTING.md; development/ keeps the decision blocks and links to the rule.
development/README.md, CONTRIBUTING.md and AGENTS.md state the 'write each fact
once' principle explicitly.
This commit is contained in:
tmu committed 2026-09-15 13:46:31 +00:00
1 parent fe02317fc8
commit 95d73d11b6
5 files changed
+89 -126

No files matched your search

+1 -1
View File
@@ -12,7 +12,7 @@ first-action facts. Do not restate evolving prose here — it will drift.
- **Optional code intelligence:** this repo installs `@spences10/pi-lsp` (pinned in `.pi/settings.json`) as a project-local pi extension. It talks to the repo's own TypeScript 7 via `tsc --lsp --stdio` and exposes **read-only** tools — `lsp_hover`, `lsp_definition`, `lsp_references`, `lsp_find_symbol`, `lsp_document_symbols`, `lsp_diagnostics(_many)`. Prefer `lsp_references` over `grep -w` for widely-colliding identifiers (`matches`, `type`, …); use `lsp_hover` to read inferred types on generic-heavy code. It has no rename / code-action / apply-edit surface — the write side is pi's `edit` tool + `check:tsc`. Treat empty LSP output as _inconclusive_, not success: **`npm run test` / `npm run verify` remain the sole authoritative gate** (see the next bullet). The server keeps running across that gate with a ~5 min idle timeout and registers no file watchers, so if you change `tsconfig.json` / `package.json` mid-session its diagnostics can be stale — when LSP output disagrees with `check:tsc`, trust `check:tsc` and restart pi (or wait out the idle timeout) before concluding the LSP is wrong. - **Optional code intelligence:** this repo installs `@spences10/pi-lsp` (pinned in `.pi/settings.json`) as a project-local pi extension. It talks to the repo's own TypeScript 7 via `tsc --lsp --stdio` and exposes **read-only** tools — `lsp_hover`, `lsp_definition`, `lsp_references`, `lsp_find_symbol`, `lsp_document_symbols`, `lsp_diagnostics(_many)`. Prefer `lsp_references` over `grep -w` for widely-colliding identifiers (`matches`, `type`, …); use `lsp_hover` to read inferred types on generic-heavy code. It has no rename / code-action / apply-edit surface — the write side is pi's `edit` tool + `check:tsc`. Treat empty LSP output as _inconclusive_, not success: **`npm run test` / `npm run verify` remain the sole authoritative gate** (see the next bullet). The server keeps running across that gate with a ~5 min idle timeout and registers no file watchers, so if you change `tsconfig.json` / `package.json` mid-session its diagnostics can be stale — when LSP output disagrees with `check:tsc`, trust `check:tsc` and restart pi (or wait out the idle timeout) before concluding the LSP is wrong.
- **Definition of done — run this before you call the work finished:** `npm run verify`. If all green, commit. If red, look at the output, fix the root cause, and re-run. - **Definition of done — run this before you call the work finished:** `npm run verify`. If all green, commit. If red, look at the output, fix the root cause, and re-run.
- **On commit:** write a good message (see [CONTRIBUTING.md § Commit messages](./CONTRIBUTING.md#commit-messages)). Lefthook's pre-commit hook already runs the fast, offline, staged-file checks — don't run them by hand. If the hook fails on style, `npm run fix`, restage, recommit. - **On commit:** write a good message (see [CONTRIBUTING.md § Commit messages](./CONTRIBUTING.md#commit-messages)). Lefthook's pre-commit hook already runs the fast, offline, staged-file checks — don't run them by hand. If the hook fails on style, `npm run fix`, restage, recommit.
- **Document decisions where the next maintainer will look:** rationale, rejected alternatives and known issues go in `development/<category>.md` (see [development/README.md](./development/README.md)); the actionable rule stays in [CONTRIBUTING.md](./CONTRIBUTING.md) and links to it. Change both in the same commit. - **Document decisions where the next maintainer will look:** rationale, rejected alternatives and known issues go in `development/<category>.md` (see [development/README.md](./development/README.md)); the actionable rule stays in [CONTRIBUTING.md](./CONTRIBUTING.md) and links to it. Write each fact once — never copy the rule into `development/` or the reason into `CONTRIBUTING.md` — and change both in the same commit when a rule changes.
- **`npm run maintain` is NOT part of the feature loop.** Its scans are advisory, never a gate; run them only on an explicit maintenance / update-deps branch. - **`npm run maintain` is NOT part of the feature loop.** Its scans are advisory, never a gate; run them only on an explicit maintenance / update-deps branch.
```sh ```sh
+56 -15
View File
@@ -51,14 +51,27 @@ are where they are: [development/workflow.md § Feedback tiers](./development/wo
## Testing discipline (type-driven) ## Testing discipline (type-driven)
For this library the types _are_ the feature, so the loop is **type → red → For this library the types _are_ the feature, so development is **type-driven**:
green → refactor**: write the `expectTypeOf(...)` assertion first, then the the compile-time expectation is written before the runtime assertion, and both
runtime `assert.*`, then the implementation. Every test pairs the two; keep before the implementation. The loop is **type → red → green → refactor**:
them together. Type-first is enforced structurally: `npm test` runs
`check:tsc` before the test runner, so a wrong type can never be papered over by 1. **Type** — write the compile-time expectation first
a passing assertion. Per [AGENTS.md § Never do](./AGENTS.md#never-do), reach (`expectTypeOf(...).toEqualTypeOf<…>()`) and let `npm run check:tsc` fail on
green honestly — fix the types, never suppress the checks you can't make pass. the _type_. The type error is the spec you want to hit before the runtime
Full rationale: [development/testing.md](./development/testing.md). logic exists.
2. **Red** — add the matching runtime assertion (`assert.*`) so
`npm run test:unit` now fails on behavior.
3. **Green** — implement in `src/*.ts` until both the type check and the test
pass.
4. **Refactor** — with the type system and the tests as the safety net, then
`npm run verify` as the definition-of-done gate.
Every test pairs an `expectTypeOf(...)` with an `assert.*`; keep them together.
Type-first is enforced structurally: `npm test` runs `check:tsc` before the
test runner, so a wrong type can never be papered over by a passing assertion.
Per [AGENTS.md § Never do](./AGENTS.md#never-do), reach green honestly — fix the
types, never suppress the checks you can't make pass. Full rationale:
[development/testing.md](./development/testing.md).
## Code style and formatting ## Code style and formatting
@@ -83,13 +96,39 @@ Gitmoji subject, imperative mood, 50/72 wrapping. The template is
## Script prefix convention ## Script prefix convention
A new `npm run` script must reuse an existing prefix: `create:` / `check:` / Script names in `package.json` use a prefix that signals _when_ the script is
`fix:` / `test:` / `watch:` / `maintain:` / `publish:` / `setup:`. If none fits, intended to run. A `<prefix>:<name>` script is implicitly aggregated by a
that's a signal the script doesn't belong in the pipeline — not a reason to `<prefix>` script (if one exists) and run by the corresponding lefthook hook or
invent a new prefix. If it genuinely does belong, add the prefix to the list CI step. Pick the prefix that matches the script's lifecycle:
here in the same commit as its first member; an undocumented prefix becomes
invisible and quietly accrues members. Full convention and why `create:` exists: - `create:*` — front doors of the repo's own workflow; these mutate git state
[development/workflow.md § Script prefix convention](./development/workflow.md#script-prefix-convention). rather than the source. `create:branch` opens a unit of work, `create:finish`
closes the branch half, `create:release` closes the release half
(maintainer-only). No bare `create` aggregator on purpose.
- `check:*` — read-only verification; never modifies files. Aggregated by
`npm run check`.
- `fix:*` — mutating counterpart of a `check:*` script. Aggregated by
`npm run fix`; the diff is the review surface.
- `test:*` — test scripts. `test` is the canonical entry point (`check:tsc` +
unit tests); `test:unit` skips the typecheck for fast local iteration;
`test:ci` adds c8 coverage.
- `watch:*` — long-running watchers for the manual inner dev loop. Aggregated by
`watch`.
- `maintain:*` — advisory repo-maintenance scans: read-only, but whole-project
and/or network-bound, so never a correctness gate. Aggregated by
`npm run maintain`.
- `publish:*` — validates the _publishable artifact_ (e.g. `dist/`) rather than
the source, so it needs a fresh build.
- `setup:*` — one-time configuration of a fresh clone; mutates the local
environment rather than the repo source, so it is never part of a hook or CI
step. Aggregated by `npm run setup`, run once after cloning.
A new script must reuse an existing prefix. If none fits, that's a signal the
script doesn't belong in the pipeline — not a reason to invent a new prefix. If
it genuinely does belong, add the prefix to this list in the same commit as its
first member; an undocumented prefix becomes invisible and quietly accrues
members. Why `create:` exists, the rejected names, and the design of the bare
scripts: [development/workflow.md § Script prefix convention](./development/workflow.md#script-prefix-convention).
## Rules the tools don't enforce ## Rules the tools don't enforce
@@ -150,6 +189,8 @@ request workflow on Gitea yet.
merge stays local and reviewable. merge stays local and reviewable.
- CI runs on every push to `main` — see [Feedback tiers](#feedback-tiers) and - CI runs on every push to `main` — see [Feedback tiers](#feedback-tiers) and
[.gitea/workflows/ci.yml](./.gitea/workflows/ci.yml). [.gitea/workflows/ci.yml](./.gitea/workflows/ci.yml).
- **Releases are NOT triggered by pushes.** Only the maintainer triggers a
release; see [Publishing](#publishing).
Full rationale, including the front-door decisions and a known issue about Full rationale, including the front-door decisions and a known issue about
`main` being ahead of its upstream between a merge and the next push: `main` being ahead of its upstream between a merge and the next push:
+3
View File
@@ -10,6 +10,9 @@ rules: read the relevant file here before changing an area, and update it when
a decision changes, rather than leaving an obsolete reason behind. A rule and a decision changes, rather than leaving an obsolete reason behind. A rule and
its reason drift apart when they live in one place and are maintained in two; its reason drift apart when they live in one place and are maintained in two;
keeping the rule in CONTRIBUTING.md and the reason here keeps each single-homed. keeping the rule in CONTRIBUTING.md and the reason here keeps each single-homed.
**Each fact is written once**: the actionable rule in CONTRIBUTING.md, the
reason here. Neither restates the other — when the same fact would be useful in
both, one links to the other instead of copying it.
User-facing documentation lives in [README.md](../README.md). A `docs/` folder User-facing documentation lives in [README.md](../README.md). A `docs/` folder
is deliberately not used yet: the convention is that `docs/` is the future is deliberately not used yet: the convention is that `docs/` is the future
+10 -31
View File
@@ -8,26 +8,9 @@ the way it is.
## Type-driven development ## Type-driven development
New behavior follows **type-driven development** (in Edwin Brady's sense): The rules — the loop and the pairing rule — are in
_treat the type as the plan for a program, and use the compiler and type checker [CONTRIBUTING.md § Testing discipline (type-driven)](../CONTRIBUTING.md#testing-discipline-type-driven).
as your assistant, guiding you to a complete program that satisfies the type_ What follows is why the loop is type-driven and what was rejected.
([idris-lang.org](https://www.idris-lang.org/)). Here that plan is the
`expectTypeOf` assertion, written first. The loop is **type → red → green →
refactor**:
1. **Type** — write the compile-time expectation first
(`expectTypeOf(...).toEqualTypeOf<…>()`) and let `npm run check:tsc` fail on
the _type_. The type error is the spec you want to hit before the runtime
logic exists.
2. **Red** — add the matching runtime assertion (`assert.*`) so
`npm run test:unit` now fails on behavior.
3. **Green** — implement in `src/*.ts` until both the type check and the test
pass.
4. **Refactor** — with the type system and the tests as the safety net, then
`npm run verify` as the definition-of-done gate.
This is why every test in the suite pairs an `expectTypeOf(...)` with an
`assert.*` — keep them together.
#### Decision (2026-09) #### Decision (2026-09)
@@ -56,18 +39,14 @@ Per [AGENTS.md § Never do](../AGENTS.md#never-do), reach green honestly — fix
types so both the type check and the runtime assertion pass, never suppress the types so both the type check and the runtime assertion pass, never suppress the
ones you can't make pass. ones you can't make pass.
## Test tiers The runner is `node --test --strip-types "src/**/*.test.ts"` and the tiers are
listed in
[CONTRIBUTING.md § Development commands](../CONTRIBUTING.md#development-commands).
`c8` uses V8 coverage, so the `--strip-types` source is instrumented without a
build step. The runner relies on the `.ts` import-extension convention (see
[tooling.md](./tooling.md#source-imports-use-ts-extensions)).
- `npm run test:unit` — the test runner alone, for the fast local loop. ## Known issues
- `npm test` — `check:tsc` + the unit suite; this is what pre-push and the
baseline check run.
- `npm run test:ci` — adds c8 coverage; used by CI. `c8` uses V8 coverage, so
the `--strip-types` source is instrumented without a build step.
The runner is `node --test --strip-types "src/**/*.test.ts"`. It relies on the
`.ts` import-extension convention (see [tooling.md](./tooling.md#source-imports-use-ts-extensions)).
#### Known issue
- The type-aware linter misidentifies `expectTypeOf()` as a floating promise, - The type-aware linter misidentifies `expectTypeOf()` as a floating promise,
so `src/index.test.ts` carries a file-level `oxlint-disable so `src/index.test.ts` carries a file-level `oxlint-disable
+19 -79
View File
@@ -13,28 +13,9 @@ request workflow on Gitea yet: collaborative review through the Gitea UI is not
in place, so Gitea is the lab. When something is tested and ready for in place, so Gitea is the lab. When something is tested and ready for
production it will be promoted to GitHub. production it will be promoted to GitHub.
- **Base branch:** `main` The contributor-facing steps are in
- **Branch naming:** `feature/<desc>` / `fix/<desc>` / `chore/<desc>` [CONTRIBUTING.md § Branching model](../CONTRIBUTING.md#branching-model); what
- **Starting work:** `npm run create:branch -- <prefix>/<desc>`. It refuses, follows is why the front doors exist and what was rejected.
without changing anything, unless the working tree is clean (untracked files
included), no merge/rebase/cherry-pick is in progress, `main` matches its
upstream, and `npm run test` is green on `main` — so a later failure is always
attributable to your edits. The prefix is still _your_ call, inferred from the
task; the script validates it rather than guessing it.
- **Merging:** `npm run create:finish` (on the branch). It asserts the same
clean-tree / no-operation / current-`main` preconditions, fast-forwards a stale
`main` (a true divergence is refused), merges the branch `--no-ff`, runs
`npm run verify`, and deletes the branch only after the merge is green. The
push is deliberately left to `create:release`, so the merge stays local and
reviewable — read the diff yourself before finishing.
- CI runs `npm run check` + `npm run test:ci` on every push to `main` — this is
the authoritative gate. The one exception: a push headed by a release commit
(`:rocket: Release x.y.z`) skips the full `build`/`maintain` jobs, because
`create:release` pushes the tag for that exact commit right after and the tag
run is the authoritative one (see `release-gate` in
[.gitea/workflows/ci.yml](../.gitea/workflows/ci.yml)).
- **Releases are NOT triggered by pushes.** Only the maintainer triggers a
release (see [publishing.md](./publishing.md)).
#### Decision (2026-09) #### Decision (2026-09)
@@ -79,51 +60,18 @@ prose plus hand-written `git` commands.
## Script prefix convention ## Script prefix convention
Script names in `package.json` use a prefix that signals _when_ the script is The prefix taxonomy is the rule, and it lives in
intended to run. A `<prefix>:<name>` script is implicitly aggregated by a [CONTRIBUTING.md § Script prefix convention](../CONTRIBUTING.md#script-prefix-convention).
`<prefix>` script (if one exists) and run by the corresponding lefthook hook or What follows is the design rationale and the `create:` decision.
CI step. Picking the right prefix documents the script's intended lifecycle:
- `create:*` — front doors of the repo's own workflow; these mutate git state One part of the taxonomy is not a prefix rule: some top-level scripts are
rather than the source. `create:branch` opens a unit of work, `create:finish` **bare** (no prefix): the entry points that either run a single tool (`build`,
closes the branch half, `create:release` closes the release half `clean`) or aggregate a `prefix:*` family (`check`, `fix`, `test`, `watch`,
(maintainer-only). No bare `create` aggregator on purpose — see `publish:*` `maintain`, `setup`), plus one convenience that composes across tiers: `verify`
for the precedent. — composing `check` + `test:unit` into one whole-project correctness gate (it
- `check:*` — read-only verification; never modifies files. Aggregated by deliberately uses `test:unit` rather than `test` because `check` already runs
`npm run check`. `check:tsc`, so the type checker runs exactly once). Bare commands are how you
- `fix:*` — mutating counterpart of a `check:*` script. Aggregated by invoke a tier; the `prefix:*` scripts are what those tiers are made of.
`npm run fix`; the diff is the review surface.
- `test:*` — test scripts. `test` is the canonical entry point (`check:tsc` +
unit tests); `test:unit` skips the typecheck for fast local iteration;
`test:ci` adds c8 coverage.
- `watch:*` — long-running watchers for the manual inner dev loop. Aggregated by
`watch`; currently a single child (`watch:test`), and a future
`watch:oxlint` / `watch:tsc` would run concurrently under that umbrella.
- `maintain:*` — advisory repo-maintenance scans: read-only, but whole-project
and/or network-bound, so never a correctness gate. Aggregated by
`npm run maintain`.
- `publish:*` — validates the _publishable artifact_ (e.g. `dist/`) rather than
the source, so it needs a fresh build.
- `setup:*` — one-time configuration of a fresh clone; mutates the local
environment (git config, editor settings) rather than the repo source, so it
is never part of a hook or CI step. Aggregated by `npm run setup` (the
umbrella), run once after cloning.
A new script should pick the prefix that matches its lifecycle, not invent a new
one. If no existing prefix fits, that's a signal the script doesn't belong in
the standard pipeline. When it genuinely does belong, a new prefix is allowed —
but it enters both lists in [CONTRIBUTING.md](../CONTRIBUTING.md) in the same
commit as its first member, otherwise the rule "reuse an existing prefix"
silently develops an exception.
Separately, some top-level scripts are **bare** (no prefix): the entry points
that either run a single tool (`build`, `clean`) or aggregate a `prefix:*` family
(`check`, `fix`, `test`, `watch`, `maintain`, `setup`), plus one convenience that
composes across tiers: `verify` — composing `check` + `test:unit` into one
whole-project correctness gate (it deliberately uses `test:unit` rather than
`test` because `check` already runs `check:tsc`, so the type checker runs
exactly once). Bare commands are how you invoke a tier; the `prefix:*` scripts
are what those tiers are made of.
#### Decision (2026-09) #### Decision (2026-09)
@@ -155,11 +103,9 @@ aggregator.
## Feedback tiers ## Feedback tiers
The tools are organized into a feedback ladder. Each tier catches different The tier table and the rules for invoking it are in
things at different costs; the rule of thumb is "earlier tiers fire more often, [CONTRIBUTING.md § Feedback tiers](../CONTRIBUTING.md#feedback-tiers); this
faster tiers catch less, slower tiers are more thorough". The tier table itself section explains why the split is where it is.
lives in [CONTRIBUTING.md](../CONTRIBUTING.md#feedback-tiers); this section
explains why the split is where it is.
#### Decision (2026-09) #### Decision (2026-09)
@@ -193,16 +139,10 @@ into pre-push and `verify`; and slow or network-bound scans into `maintain`.
tier; `verify` is run by hand because the push is where the whole project is tier; `verify` is run by hand because the push is where the whole project is
already checked. already checked.
Before pushing, run `npm run verify` — the one-shot correctness gate. Run
`npm run maintain` only on a maintenance / update-deps branch.
## Commit messages ## Commit messages
Gitmoji subject, imperative mood, 50/72 wrapping. The template is The convention is in
[commit-message-template](../commit-message-template); run [CONTRIBUTING.md § Commit messages](../CONTRIBUTING.md#commit-messages).
`npm run setup:git-commit-message` once after cloning to register it as git's
`commit.template` (or `npm run setup` to run every one-time clone step).
Examples from history: `:sparkles: Add watch tier with watch:test child`, Examples from history: `:sparkles: Add watch tier with watch:test child`,
`:recycle: Move type-aware config to .oxlintrc.json; use source-level disable `:recycle: Move type-aware config to .oxlintrc.json; use source-level disable
directives`, `:memo: Restore unique maintainer content as CONTRIBUTING.md`. The directives`, `:memo: Restore unique maintainer content as CONTRIBUTING.md`. The