Writing Python

---
name: writing-python
description: How Python is written in this repo — free functions over classes, structured returns instead of mutated arguments, comprehensions, no module state, and the stdlib-only, annotation-free, Python 3.8 dialect the AST lints enforce. Covers modular structure and the import-layer rule, DRY without copying a helper into a second file, what makes a function testable from a `--selftest`, readability rules (guard clauses, named predicates, why-not-what docstrings), and the anti-patterns that have actually bitten here. Use when planning, writing, reading, reviewing or refactoring any `.py` under plugins/audit/, when adding a helper or a new script, when deciding where a function belongs, or when the same logic is about to exist in two places.
---

# Writing Python here

Not a general Python style guide. This repo bans things most guides mandate, and the bans are
read out of the AST rather than trusted — `_output.house_style_violations()` fails the build, it
does not warn. Everything below is either enforced or measured from the tree as it stands.

## The dialect, in one paragraph

Stdlib only. Python 3.8 floor. **No `typing`, no `dataclasses`, no annotations, no walrus, no
`from __future__`** — banned by AST, because hooks start on every tool call and the import and
parse cost is real. `%`-style formatting; there is not one f-string in the tree. Reach for
`os.path`, `json`, `re`, `subprocess` — not for a dependency.

3.8 also rules out things that read as ordinary today: no `X | Y` unions, no `match`, no
`dict1 | dict2` merge, no `list[str]`. `vermin -t=3.8-` catches these; it will not catch the
banned imports, which is why the AST lint exists alongside it.

## Free functions, structured returns

The measured shape of this codebase is **top-level functions by the hundred against a
handful of classes**, comprehensions everywhere, and **`global` statements you can count on one
hand**. The ratio is the pattern to keep; the figures are not, so re-derive them rather than
trusting a sentence:

```bash
grep -rhc '^def ' plugins/audit/scripts plugins/audit/hooks --include='*.py' | paste -sd+ | bc
grep -rn '^class \|^ *global ' plugins/audit/scripts plugins/audit/hooks --include='*.py' | wc -l
```

- **Write a function, not a class.** A class here needs a reason you can say out loud — the six
  that exist are all genuine (a lock, a launcher). Grouping related functions is what a module
  prefix is for.
- **Return a new value; do not mutate an argument.** 181 functions return a dict or tuple. A
  function that edits the caller's dict in place and also returns it is two contracts and the
  caller will rely on the wrong one.
- **No module state.** Constants at module level are fine (52 of them, uppercase, frozen tables).
  Mutable module state that a function writes to is how two callers start disagreeing about the
  same run. Where a cache is genuinely needed the existing pattern is an explicit
  `_cache`/`cache=True` argument, not a hidden global.
- **Comprehensions over accumulate loops** when the result is a list or dict and the body is one
  expression. Past that, a named function reads better than a nested comprehension.
- **Push I/O to the edges.** A function that takes values and returns values can be tested by a
  `--selftest` case with no temp directory and no `git init`; one that reads a file and prints
  cannot. Split the read from the decision — `decide()` taking parsed input is the shape every
  hook already uses, and it is why their logic has cases at all.

## Where code lives

- **Name by role.** `_underscore.py` is an importable helper; `hyphen-name.py` is an entry point
  something invokes. The name is the contract.
- **A basename must be unique across all of `scripts/`.** A file may sit in a subdirectory — every
  scanner walks recursively — but `import`, `_loader` and `_deps.LAYERS` all resolve by basename, so
  two files sharing one would be a single node in the layer graph wearing both files' edges.
  `layer_violations()` fails the build by name on it. Directories are labels, not namespaces.
- **Never derive a directory from your own `__file__`.** `_output.py` is the anchor:
  `SCRIPTS_DIR`, `PLUGIN_ROOT`, `HOOKS_DIR`, `TESTS_DIR`, `REPO_ROOT`. The pinned
  `PATH_PREAMBLE` is the only place under `scripts/` allowed to touch `__file__` at all, and
  `depth_sensitive_paths()` enforces it — the rule is "none", not "not a parent", because the
  sites it replaced were written as a two-step that any nesting-only lint waves through.
- **Imports go down, never sideways or up.** `_deps.LAYERS` assigns every module a layer and
  `layer_violations()` fails an unplaced or wrongly-layered file **by name**. `hooks/` may import
  nothing from `scripts/` at all. If a new helper does not fit a layer, that is information about
  the design, not an obstacle to route around.
- **Section markers every 400 lines.** Past that a flat scroll stops being a map;
  `navigability_violations()` enforces it, and `ui_navigability_violations()` does the same for
  the front end.

## DRY, and the way it fails here

The rule is not "never repeat a line". It is **one fact, one home**.

- If two modules need the same logic, it belongs in a **lower layer** both can import — not
  copied into the second file, and not re-derived.
- **The cautionary tale is real and still in the tree**: the token formatter exists in Python and
  is hand-written again in each of the two front-end surfaces. Two of the three now disagree —
  `uTok(2.6)` rounds to `"3"` while `fmtTokens(2.6, 1)` truncates to `"2"` — and both carry a
  comment claiming to mirror the Python. A copy with a comment saying it is a copy is still a
  copy.
- When a value must exist in two languages, **pin the agreement with a case** rather than a
  comment. A claim that two implementations agree is testable; test it.
- Do not extract for its own sake. Two similar-looking blocks that answer different questions
  should stay apart — the wrong abstraction costs more than the duplication.

## Readability

- **Guard clauses over nesting.** Return early; do not build an arrow of `if`/`else`.
- **Name the predicate.** `if _is_ready(task):` beats an inlined three-term `and` chain, and the
  name is where the reasoning goes.
- **Module docstring says *why the file exists*, not what it does.** That is a hard rule here and
  the reason the tree is navigable at 42k lines.
- **Comment the non-obvious constraint, never the mechanism.** The house comments that earn their
  place read like postmortems: what broke, and what would break again. A comment restating the
  next line is noise.
- **No `*Utils`, `*Helper`, `*Manager`, `*Impl`.** Name a module for the thing it owns.
- Keep functions short enough to hold. 39 functions in the tree exceed McCabe 15 — that is the
  known debt, not the target.

## Anti-patterns and pitfalls

- **Mutable default arguments** (`def f(items=[])`) — the default is created once and shared.
  Use `None` and build inside.
- **`x = x or default` when `0`, `False` or `""` are meaningful.** Use
  `x if x is not None else default`. This is the single most common silent-corruption bug in code
  that reduces data to a number.
- **Bare `except:` or a broad `except Exception:` that returns a normal-shaped value.** Either
  recover meaningfully or make the failure visible. See `no-silent-pass`.
- **Iterating a `set` into serialized output** — the order varies per process and the real diff
  drowns in noise. Impose a total order first.
- **Catching an exception to skip an item, with no count and no name.** Silent skips are how a
  job reports success over half the work.
- **Reaching around the layer rule** by importing inside a function to dodge the lint. The lint
  reads the AST; more importantly, the layering is the design.
- **Adding a file without its five obligations** — `--selftest`, `safe_stdio()` first in
  `__main__`, a `_deps.LAYERS` entry, a `PLUGIN-BUILD-GUIDE.md` tree line and section, and
  (under `scripts/`) `_output.PATH_PREAMBLE` pasted byte for byte above the first sibling
  import. Four of the five fail CI by name.

## Testability

A `--selftest` is not an afterthought here; it is the test suite, and CI fails a file without
one. Design for it:

- a function whose inputs are arguments and whose output is a return value needs no fixture;
- where a fixture is unavoidable, build it in a `tempfile.mkdtemp()` and delete it in `finally` —
  never commit one;
- **new behaviour means new cases**, and a case is only trusted once it has been seen red.

`no-silent-pass` owns that discipline — mutate the fix, mutate it the other way too, count rather
than assert presence, and choose fixture values that tell the two versions apart.