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:

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.