Add agent instructions and writing rules

.claude/skills/design-documents/SKILL.md

@@ -0,0 +1,53 @@
+---
+name: design-documents
+description: Write or revise a design document under docs/design_docs/. Use when proposing a new capability, not when fixing a bug.
+---
+
+# Design documents
+
+Long is fine. A design document is scanned and returned to, not read
+straight through. What must be scannable is the specification.
+
+- Normative statements come first in a section and stand alone. Rationale
+  goes in a marked block below, which a reader can skip.
+- Rejected alternatives and superseded drafts go in one `Decisions`
+  section, cited from the places they affect. Never re-argued in place.
+- A principle is stated once. Later sections cite it by name.
+- Do not pre-empt objections. Drop "worth noting", "not an accident",
+  "deliberately", "this is not a stylistic preference". State the decision
+  and let it stand.
+- Open questions go at the top or in their own section, never
+  mid-paragraph.
+
+## Calibration
+
+The two design documents in `docs/design_docs/` run 2,574 and 12,390 words,
+both at 28 words per sentence, with no hedging phrases in the first and one
+in the second. Sentence length is the thing to fix; aim for twenty words.
+Do not import the hedging habit that these documents do not have.
+
+`docs/design_docs/template.rst` is the prescribed structure: a
+`Requirement`, `Algorithm Design`, `Implementation` and `Testing` section
+per topic, with the same topic name used in each. It also says requirements
+"should not discuss technical software issues, but rather focus on model
+capability", which is the rule most often broken.
+
+## Enough
+
+From `docs/design_docs/cached_outputs.rst`. The heading states the
+requirement and the body is one or two normative sentences.
+
+> Requirement: updating cached outputs
+> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>
+> There should be a documented process for creating cached outputs for steps
+> and uploading them.
+>
+> Requirement: either "normal" or "cached" versions of a step
+> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+>
+> We **do not** require the ability to set up a "normal" and a "cached"
+> version of the same step within a ``compass`` test case or suite.
+
+Five requirements in that document take about 250 words between them, and
+saying which capability is *not* required is a requirement worth writing.

.claude/skills/plan-documents/SKILL.md

@@ -0,0 +1,28 @@
+---
+name: plan-documents
+description: Write a plan for work not yet started, for a colleague or the user to approve before implementation begins.
+---
+
+# Plan documents
+
+The reader is deciding whether to let you proceed.
+
+- Open questions and anything needing a decision go at the top.
+- The steps, in order, one line each.
+- Do not justify each step. Do not list the files you will touch. Do not
+  restate the codebase back.
+- If a step needs a paragraph to explain, it belongs in a design document.
+
+## Enough
+
+Plans are approved in conversation rather than committed, so there is no
+human example in this repository to copy. The following is constructed.
+
+> **Open:** should the `anvil` rows come out of the machines table now, or
+> wait until the config file is restored?
+>
+> 1. Fix the land-ice draft used for pressure in the thin-film cases.
+> 2. Restore the culled thin-film region in `initial_state`.
+> 3. Take the smoothing from ice thickness, not draft, in both cases.
+> 4. Update the `isomip_plus` docs in both guides.
+> 5. Rerun the `pr` suite on Chrysalis against a `main` baseline.

.claude/skills/pr-descriptions/SKILL.md

@@ -0,0 +1,72 @@
+---
+name: pr-descriptions
+description: Write or update a pull request description, including drafting pr_description.md before opening a PR. Use when opening a pull request or editing its body.
+---
+
+# Pull request descriptions
+
+The reader is deciding whether to review.
+
+- What changed and why, in a few sentences. Not how.
+- Anything needing a reviewer decision goes in its own short list near the
+  top, never mid-paragraph.
+- A list of changed behaviours is fine. A trace of the mechanism is not.
+- No commit list. No testing; that goes in a separate `Testing` comment.
+- Link the issue or upstream pull request that gives context.
+- Several fixes usually means several pull requests.
+
+## Calibration
+
+Measured over merged pull requests from 2023 and 2024, before any agent
+wrote here, excluding bots. Descriptions run 32 to 36 words at the median,
+57 to 60 at the seventy-fifth percentile, 94 to 106 at the ninetieth, and
+230 at the longest. Roughly one description in ten is empty; a title that
+says it all is an acceptable description.
+
+## Enough
+
+A bug fix, stated and done, from #609:
+
+> The code for changing permissions was being given an incorrect directory
+> and then was failing silently. This merge fixes the path and also removes
+> the silent failure (an error will be raised if the status of the download
+> directory cannot be determined).
+>
+> This merge also adds the database root and the root for each affected core
+> to the list of directories to chmod/chown anytime any files get downloaded.
+>
+> closes #586
+> closes #608
+
+A port, in one sentence, from #519:
+
+> This PR moves the generation of an initial state for the ocean global
+> cosine bell test case from init mode to the initial state compass step.
+
+A change with a caveat the reviewer needs, from #716:
+
+> This PR replaces the `mode_init` MPAS-Ocean run in the `initial_state`
+> step with local computations. The initial state is unchanged for the
+> configuration that is currently used in the drying_slope cases. I did not
+> retain all of the configuration options that were previously present in
+> `mode_init`, e.g., the idealized transect.
+>
+> I added some buffer space in the y-dimension (the new config options `Ly`
+> as differentiated from `Ly_analysis`) because the 1km case did not include
+> the full 25km stretch of wetting and drying.
+
+## Too much
+
+Constructed, in the style of a real agent-written description elsewhere.
+Five fixes each got a section, and each section traced its mechanism:
+
+> `compass.ocean.tests.global_ocean` gained a module-scope import of
+> `compass.ocean.mesh.remap_topography`, which runs
+> `compass/ocean/mesh/__init__.py`, which imports the step classes, which
+> import `compass.ocean.tests.global_ocean` back. The module could no longer
+> be imported on its own; it worked only when something else imported
+> `compass.ocean.mesh` first.
+
+Someone deciding whether to review does not need the cycle traced. One
+sentence would do; the rest belongs in the commit message. Five fixes is
+also five pull requests.

.claude/skills/review-comments/SKILL.md

@@ -0,0 +1,74 @@
+---
+name: review-comments
+description: Write a review comment, review findings, or a reply to review feedback on a GitHub pull request. Use when reviewing code, reporting what testing someone else's branch turned up, or answering a reviewer's question.
+---
+
+# Review comments
+
+The reader is deciding what to change.
+
+- Put each finding as an inline comment on the line it concerns, one point
+  each. That is where colleagues put them, and it is why their review
+  bodies are short.
+- The review body summarizes: what you ran, and the verdict. Two or three
+  sentences.
+- Use a list in the body only for requests that span files.
+- No section on what already works. One line for all of it, if any.
+- Say what you could not check.
+
+## Calibration
+
+Measured over review comments from 2023 and 2024, before any agent wrote
+here. Review bodies run 21 to 23 words at the median, 80 to 92 at the
+ninetieth percentile, and 383 at the longest. Inline comments run 14 words
+at the median, 58 at the ninetieth percentile, and 517 at the longest.
+
+## Enough
+
+Inline, one point and a suggestion, from #547 and #738:
+
+> I'd be inclined to call this test group `BaroclinicGyre` rather than
+> `MitgcmBaroclinicGyre`. I think shorter is better, and I doubt we'll be
+> implementing multiple baroclinic gyre cases.
+
+> Ok. Why don't you leave a comment that mentions that they refer to the old
+> METIS partitioning so others know why they are there?
+
+Inline, a question rather than an assertion, from #817:
+
+> Is the tilde correct here? Won't this overwrite all the good data and
+> leave the missing/bad data in place?
+
+A body that is the verdict and nothing else, from #523:
+
+> Thanks for doing this, @xylar. I'm approving based on my testing on Cori,
+> in which I built spack with Albany and successfully ran the MALI
+> `full_integration` test suite. I also had a quick look through the code
+> diffs and nothing jumped out at me, although I can't pretend that I
+> understand all of it.
+
+In the body, when several requests span the whole change, from #637:
+
+> @scalandr, this is excellent!
+>
+> I have some small changes and then there are a few other things to do:
+> * the test group and all its classes and methods need to be added to the
+>   `ocean/api.rst` in the developer's guide
+> * It would be great if you could build the documentation locally (see
+>   checklist above) to make sure it looks right.
+> * Document the testing you did in a comment in the PR
+
+## Too much
+
+Constructed, in the style of a real agent-written review elsewhere, which
+spent its first 444 words on "How this was reviewed", "What the previous
+review asked for" and four paragraphs of "What works", then traced each
+finding's mechanism:
+
+> The bin-centre array is static: computed once in the step's `setup()` from
+> `nBins`, `minLat` and `maxLat`, and never updated. But it is added to the
+> output stream like any other field, so every output file in every run
+> carries a copy of the same 61 numbers.
+
+The finding is that a static array is written to every output file. Say
+that inline, on the line that adds it, and stop.

.claude/skills/testing-comments/SKILL.md

@@ -0,0 +1,82 @@
+---
+name: testing-comments
+description: Write a Testing comment on a pull request, recording what was run and what the results were. Use after running suites, test cases or linting for a PR.
+---
+
+# Testing comments
+
+What you ran, where, and whether it passed.
+
+- Name the suite or test case, the machine and the compiler. One sentence.
+- Give the work directory or the baseline you compared against.
+- Say the result. Bit-for-bit, passed, or the numbers if they matter.
+- Use a table or a list only when there are several runs to compare.
+- Do not restate in prose what a table or a pasted result already shows.
+- Failures unrelated to the branch go under their own heading at the end.
+
+## Calibration
+
+Measured over `Testing` comments from 2023 and 2024, before any agent wrote
+here. They run 38 to 40 words at the median and 92 to 152 at the ninetieth
+percentile. The shortest useful ones are under 25.
+
+## Enough
+
+From #663 and #816:
+
+> ## Testing
+>
+> I ran the `pr` suite on Chrysalis using `main` as a baseline and all tests
+> passed with BFB results.
+
+> ## Testing
+>
+> I ran the `pr` suite with these changes, using the current `master` as a
+> baseline. All tests passed.
+
+With the work directory, from #753:
+
+> ## Testing
+>
+> This mesh has been run through `files_for_e3sm` and the output is in:
+> ```
+> /lcrc/group/e3sm/ac.xylar/compass_1.2/chrysalis/e3smv3-meshes/sowisc12to30e3r2
+> ```
+>
+> I have verified that the land-ice mask includes "land-locked" cells that
+> were formerly a problem for sea-ice, based on the fix in #752
+
+Saying what you could not check, from #791:
+
+> ## Testing
+>
+> I successfully ran the `pr` and `nightly` suites on Chrysalis with Intel
+> and Open-MPI. I didn't compare with a baseline because none is available
+> for the Icos meshes.
+
+When the results are worth pasting, paste them and stop, from #771:
+
+> Tests are passing on Chrysalis (Intel/OpenMPI):
+> ```
+> Test Runtimes:
+> 06:22 PASS ocean_hurricane_DEQU120at30cr10rr2_mesh_fblts
+> 46:58 PASS ocean_hurricane_DEQU120at30cr10rr2_init_fblts
+> 06:21 PASS ocean_hurricane_DEQU120at30cr10rr2_sandy_fblts
+> Total runtime 59:43
+> PASS: All passed successfully!
+> ```
+> and the documentation looks good.
+
+## Too much
+
+Constructed, in the style of a real agent-written comment elsewhere, which
+pasted the results table and then said the same thing again in prose:
+
+> Every test case now runs to completion. The five diffs are all of the form
+> `File ... does not exist`: `main` crashed before writing those outputs, so
+> there is nothing to compare against. Every comparison that had a file on
+> both sides passed. A clean like-for-like comparison for those five cases
+> needs a fresh baseline once this lands.
+
+The table already showed the passes and the five missing files. The only
+new sentence is the last one.