From 4cd2d71d213f4dc30ce31d8c5930dfaafdb8e1da Mon Sep 17 00:00:00 2001 From: Pablo Brubeck Date: Sat, 8 Aug 2026 12:14:20 +0100 Subject: [PATCH 1/2] Add the prose rules to AGENTS.md FIAT's style section already matches Firedrake's on class attributes, the hasattr prohibition, type hints and numpydoc. It carries neither prose rule. Add the ASD-STE100 rule for docstrings and comments, and the rule against documenting code that is not there. Give each an anti-pattern with a WRONG/RIGHT pair, since that is the form these rules get followed in. Companion to firedrakeproject/firedrake#5338. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index af595278..6bc9a87e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,3 +97,59 @@ When writing Python code for FIAT, maintain the ecosystem's structural and styli * CI enforces `pydocstyle` (see the `[pydocstyle]` section of `setup.cfg` for the active ignore list) in addition to `flake8`; run `pydocstyle ` locally before finishing a change, since a clean `flake8` pass does not imply a clean `pydocstyle` pass. +* Every docstring or comment you write or touch must follow Simplified Technical English + (ASD-STE100): short sentences, one idea per sentence, active voice, subject named up front instead + of buried in a relative clause. Avoid the clause-stacking, inverted phrasing typical of unedited + AI-generated prose. +* When fixing code that was wrong, do not leave comments or prose explaining what the removed, + incorrect approach used to do or why it was wrong. Keep comments and documentation focused on the + current, correct code. The test to apply: a reader who never saw the diff must not be able to tell + that anything was removed. + +## Anti-Patterns + +These must be avoided when writing code, and flagged when reviewing it. + +### Clause-Stacked Docstrings And Comments + +WRONG — the subject hides inside a relative clause the reader must unwind before finding the verb: + +```python +def scale_boundary_nodes(u, factor): + """Give the nodes a boundary condition constrains their scaled values.""" +``` + +RIGHT — subject named up front, one short sentence, active voice: + +```python +def scale_boundary_nodes(u, factor): + """Scale the values of the nodes that a boundary condition constrains.""" +``` + +### Documenting Code That Is Not There + +A reader has only the file in front of them. A comment that describes a removed approach, or that +argues against a branch the code does not take, sends them looking for something that is not there. + +WRONG — the first sentence describes deleted code, and the second argues with an absent branch: + +```python +def barycentric_weights(points): + # This no longer normalises the weights, which was wrong when the points + # were not symmetric. A test for a repeated point here would divide by + # zero. + return 1.0 / numpy.prod(points[:, None] - points[None, :] + numpy.eye(len(points)), axis=1) +``` + +RIGHT — say what the present code does, and state the condition it relies on: + +```python +def barycentric_weights(points): + # The identity keeps the diagonal out of the product. The caller passes + # distinct points. + return 1.0 / numpy.prod(points[:, None] - points[None, :] + numpy.eye(len(points)), axis=1) +``` + +Some words give this away on sight: "used to", "previously", "no longer", "instead of", "we removed", +"this replaces". Watch equally for "would" when its subject is code that does not exist. An argument +against a branch that nobody can see is still a description of the past. From 24bb6dc17536b10f6213208a18439e140f33dbae Mon Sep 17 00:00:00 2001 From: Pablo Brubeck Date: Sat, 8 Aug 2026 12:20:51 +0100 Subject: [PATCH 2/2] AGENTS.md: shorten an over-long sentence Three sentences, one idea each, in place of one of 27 words. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6bc9a87e..653d8693 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,8 +128,9 @@ def scale_boundary_nodes(u, factor): ### Documenting Code That Is Not There -A reader has only the file in front of them. A comment that describes a removed approach, or that -argues against a branch the code does not take, sends them looking for something that is not there. +A reader has only the file in front of them. A comment can describe a removed approach. It can also +argue against a branch the code does not take. Either one sends the reader looking for something +that is not there. WRONG — the first sentence describes deleted code, and the second argues with an absent branch: