docs(contributing): state the bar — documentation standard, gates, tests-first

CONTRIBUTING.md

@@ -1,5 +1,13 @@
 # Contributing to keel
 
+Thank you for considering it. One thing before anything else, because it is the single
+biggest surprise contributors hit here: **the documentation bar is unusually high, on
+purpose.** Decisions carry their reasoning, rejected alternatives stay recorded, and comments
+name what was *measured* rather than what was assumed. A stated bar can be met; an unstated
+one can only be resented. The standard is spelled out with a worked example under
+[The documentation standard](#the-documentation-standard) — read that section before your
+first PR and reviews will feel fair instead of heavy.
+
 ## Governance: rulings vs. machinery
 
 **keel is not a fatwa engine. It is an enforcement engine for a ruling you supply.**
@@ -29,6 +37,100 @@ it — and run the enforcement engine under it. The disagreement then costs nobo
 upstream stays neutral, your deployment follows your ruling, and the audit trail records
 exactly who said what.
 
+## Development setup and the gates a PR must pass
+
+Python **3.14+** is the floor (see `.python-version`; `uv python install 3.14` gets you one).
+Then:
+
+```bash
+uv sync --all-extras --dev   # everything: the workspace, dev deps, the conformance extra
+uv run ruff check            # lint — must pass clean
+uv run mypy                  # types — must pass clean
+uv run pytest -q             # the full suite — must pass (CI runs exactly this)
+```
+
+All four must be green before you ask for review. CI runs the same commands, so anything red
+locally is red everywhere. The suite is fast (tens of seconds) — run it freely.
+
+## The documentation standard
+
+A comment or docstring here is acceptable when it does three things: **it says why** (the
+constraint the code cannot show), **it names what was measured** (numbers, incidents, the
+specific input that broke — not "this is faster"), and **it says what it would take to change
+the decision** (which assumption, overturned, reverses it). Code reviews enforce this; the
+point of writing it down is that you can enforce it on yourself first.
+
+A worked example, from `_open_exposure_by_asset` in `keel/execution/guards.py` — the net
+at-risk figure that the exposure and concentration caps read:
+
+> ⚠️ **An unparseable `product_id` is handled by SIDE, and always logged at WARNING.** A
+> malformed **BUY** is COUNTED, under whatever key `_asset` gives it; a malformed **SELL** is
+> SKIPPED. Both choices are the same choice — never let a row nobody can read make this
+> figure SMALLER — and it is the sign of the row, not the fact of the row, that decides which
+> action achieves that.
+
+What makes this acceptable:
+
+- **It says why, at the level the code cannot.** The code shows a branch on side; the comment
+  shows the *invariant* the branch serves — the figure feeds caps, and a smaller figure is a
+  looser cap, so unreadable rows may only ever err towards refusing an order.
+- **It names what was measured.** Not "SELLs are risky" but the arithmetic: a counted
+  `$800` unreadable SELL against a `$900` BUY measures `$100`, and past the BUY total the
+  `if amt > 0` filter *deletes the bucket entirely*. Someone checked; the comment shows the
+  check.
+- **It records the decision's history and its reversal conditions.** It supersedes an earlier
+  unconditional rule and its unconditional opposite, names which evidence would flip it again
+  ("neither survives a SELL"), and stays honest about the impossible case it still guards.
+
+You do not need to write a essay per function. You need those three properties wherever a
+choice was made that a reader could plausibly make differently.
+
+## Tests come first
+
+Changes land test-first: write the failing test, run it, watch it fail **for the right
+reason** — the assertion you meant to assert, not an import error or a typo — then make it
+pass. Your PR should carry that evidence: a test whose failure message is the bug being
+described, in the PR description or the diff's story. A test that has never been seen red is
+a test that may be green for no reason.
+
+The suite's own docstrings follow the documentation standard too; `tests/test_packaging.py`
+is a good read for how a test argues its own existence.
+
+## Commit convention
+
+[Conventional Commits](https://www.conventionalcommits.org/), matching the existing history:
+`fix(strategy): fill entries at the next bar's open`, `docs(experiments): ...`,
+`chore(release): ...`. The prefix is load-bearing — releases and changelogs are cut from it
+(see `docs/RELEASING.md`) — so an untyped commit is not a style nit, it is a missing record.
+
+## Scope: what is welcome, what needs discussion first, what is out
+
+**Welcome, PRs directly:**
+
+- Bug fixes with a failing test that reproduces them.
+- Documentation for something that deserves it and lacks it (a rail, a command, a failure
+  mode) — written to the standard above.
+- Tooling, packaging, and test-quality improvements.
+- New broker adapters behind the port (`packages/keel-broker-*`), discussed in an issue first
+  only if they need port changes.
+
+**Needs an issue and agreement BEFORE the PR:**
+
+- Anything touching a **rail** (`keel/execution/guards.py`): a rail's semantics are the
+  product, and a subtle change distributes itself to every operator who upgrades.
+- Anything changing a **default classification** — see the governance section above for the
+  source-and-discussion bar those carry.
+- New dependencies, and anything that widens the public surface of `keel-broker-api` (every
+  adapter codes against it).
+
+**Out of scope:**
+
+- Shariah rulings as code defaults — attested locally, never merged (see governance).
+- "Make the bot profitable" — the honest measured state of the rules is recorded in
+  `docs/experiments/`, and that record is the project's posture, not a to-do list.
+- Anything that weakens a fails-closed path to make an operational annoyance go away; the
+  annoyance is the smaller problem.
+
 ## Licence: why Apache-2.0
 
 keel is licensed under [Apache-2.0](LICENSE). That was a decision, not a default, and the

tests/test_contributing.py

@@ -0,0 +1,142 @@
+"""CONTRIBUTING.md: the bar, stated, so nobody has to guess it.
+
+This repository's signature is documentation that argues: decisions carry their reasoning,
+comments name what was MEASURED rather than what was assumed, and rejected alternatives stay
+recorded. It is also the single biggest barrier to contributing, because nobody will guess a
+standard they have never seen written down (#282). An unwritten bar does not filter for
+quality -- it filters for clairvoyance: PRs arrive at ordinary quality, get heavy review, and
+the contributor quietly leaves. That is the most common way a promising project loses its
+first ten contributors.
+
+These tests pin CONTRIBUTING.md to the specific things #282 asks it to state: the gates a PR
+must pass, the documentation standard WITH a worked example lifted from the code, the
+tests-first expectation, the commit convention, and the scope rules. The governance boundary
+and licence rationale already have their own guards in `test_governance.py` and
+`test_licensing.py`; this file covers the parts a first-time contributor reads before opening
+a PR.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+_ROOT = Path(__file__).resolve().parents[1]
+
+#: The sentence from `_open_exposure_by_asset`'s docstring (`keel/execution/guards.py`) used as
+#: the worked example. Quoted in CONTRIBUTING.md, asserted against the source below, so the
+#: example cannot drift from the code it explains.
+_WORKED_EXAMPLE_SNIPPET = "never let a row nobody can read make this figure SMALLER"
+
+
+def _contributing() -> str:
+    return (_ROOT / "CONTRIBUTING.md").read_text()
+
+
+def _unwrapped(text: str) -> str:
+    """Join markdown wrapping: drop blockquote markers, then collapse all whitespace.
+
+    The worked example is quoted inside a `>` block, where a sentence can break as
+    `...make this\\n> figure SMALLER...`; a plain whitespace-collapse leaves the `>` in the
+    way of the match.
+    """
+    return " ".join(re.sub(r"(?m)^\s*>\s?", "", text).split())
+
+
+def test_the_gates_a_pr_must_pass_are_stated():
+    """Dev setup plus the three gates, as runnable commands.
+
+    'Run the tests' is not an instruction; `uv run pytest -q` is. The commands are pinned
+    verbatim because a gate nobody can paste is a gate nobody runs.
+    """
+    text = _contributing()
+    for command in (
+        "uv sync --all-extras --dev",
+        "uv run ruff check",
+        "uv run mypy",
+        "uv run pytest -q",
+    ):
+        assert command in text, f"CONTRIBUTING.md must state the gate command {command!r}"
+
+
+def test_the_documentation_standard_has_a_worked_example_from_the_code():
+    """The standard is taught from a real comment, not described in the abstract.
+
+    The example must be QUOTED from the codebase and must remain there: CONTRIBUTING.md
+    reproduces a distinctive line from `guards.py`'s `_open_exposure_by_asset` docstring, and
+    this test asserts both halves -- the quote in the doc, and the quote's continued existence
+    in the source. If the source comment is ever rewritten, this fails rather than letting the
+    doc teach from an example that no longer exists.
+    """
+    text = _contributing()
+    # Wrap-normalised: the quote is markdown, and markdown wraps wherever the line ends.
+    assert _WORKED_EXAMPLE_SNIPPET in _unwrapped(text), (
+        "CONTRIBUTING.md's documentation-standard section must quote the worked example "
+        f"(from keel/execution/guards.py): expected {_WORKED_EXAMPLE_SNIPPET!r}"
+    )
+    guards = (_ROOT / "keel" / "execution" / "guards.py").read_text()
+    assert _WORKED_EXAMPLE_SNIPPET in _unwrapped(guards), (
+        "the comment CONTRIBUTING.md quotes as its worked example no longer exists in "
+        "keel/execution/guards.py -- update the doc to quote a comment that does"
+    )
+
+
+def test_the_standard_names_what_makes_a_comment_acceptable():
+    """The reader must be told the RULE, not only shown the example.
+
+    Three properties, so a contributor can check their own writing before anyone else has to:
+    it says WHY, it names what was MEASURED, and it says what it would take to change the
+    decision.
+    """
+    text = _contributing().lower()
+    assert "why" in text
+    assert "measured" in text
+    assert "decision" in text
+
+
+def test_tests_first_is_expected_with_evidence():
+    """The TDD expectation, including the evidence that makes it checkable.
+
+    Tests written before the fix, and shown in the PR to have failed FOR THE RIGHT REASON --
+    an assertion message, not an import error -- is the house style. Stated, it is a bar
+    contributors can meet; unstated, it is a review surprise.
+    """
+    text = _unwrapped(_contributing()).lower()
+    phrases = ("tests first", "test-first", "tests before", "tests come first")
+    assert any(phrase in text for phrase in phrases), (
+        "CONTRIBUTING.md must state that tests are written first"
+    )
+    assert "fail" in text and "right reason" in text, (
+        "CONTRIBUTING.md must ask for evidence the failing tests failed for the right reason"
+    )
+
+
+def test_the_commit_convention_is_stated():
+    """Conventional Commits, because that is what the history already does.
+
+    The point is not aesthetics: `fix(strategy):` vs `feat(engine):` is the changelog, and a
+    release process that reads it (see docs/RELEASING.md) silently degrades when a commit
+    arrives untyped.
+    """
+    assert re.search(r"conventional commits", _contributing(), re.IGNORECASE), (
+        "CONTRIBUTING.md must name Conventional Commits as the commit convention"
+    )
+
+
+def test_scope_guidance_separates_welcome_from_needs_discussion_from_out():
+    """Three tiers of scope, with the guarded kinds named.
+
+    Rails and default classifications are the two surfaces where a casual PR does quiet,
+    distributed harm, so they are the two that must be called out as discuss-first. The
+    governance section above already carries the fiqh reasoning; this pins that the SCOPE
+    list exists and points at them.
+    """
+    text = _contributing().lower()
+    assert "rail" in text, "scope guidance must name rails as discuss-first territory"
+    assert "classification" in text, (
+        "scope guidance must name default classifications as discuss-first territory"
+    )
+    assert "discussion" in text or "discuss" in text, (
+        "scope guidance must say these need discussion BEFORE a PR, not review during one"
+    )
+    assert "out of scope" in text, "scope guidance must state what is out of scope entirely"