Skip to content

Commit c7b1bba

Browse files
Merge branch 'main' into implementation/roc-curve/makie
2 parents 5ffd243 + 6e3ad28 commit c7b1bba

37 files changed

Lines changed: 6660 additions & 11 deletions

File tree

.claude/guardrails.md

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
# Guardrails — the long version
2+
3+
`CLAUDE.md` states each working rule as one line: the rule and its shortest
4+
reason. This file holds what does not fit there — the incident that produced
5+
the rule, the recipe it implies, the numbers that make it credible.
6+
7+
The split exists because `CLAUDE.md` is loaded into **every** session, so its
8+
cost is paid on every turn, while a retro narrative is needed only when
9+
someone actually hits the situation. Nothing here is a new rule. If this file
10+
and `CLAUDE.md` ever disagree, `CLAUDE.md` is the rule and this is the
11+
commentary that fell behind.
12+
13+
Sections follow the order in which `CLAUDE.md` states the rules.
14+
15+
---
16+
17+
## Delegated agents run on Opus by default
18+
19+
Owner directive, 2026-08-11, refined 2026-08-16.
20+
21+
Pass `model: opus` when spawning subagents or workflows — the tier the
22+
`agentic/README.md` model table already uses for large tasks. Escalate to
23+
Fable for genuinely hard reasoning, drop to Sonnet or Haiku for mechanical
24+
grinding, but Opus is the default answer.
25+
26+
Escalation is for judgment calls, not for every detail. Within its briefed
27+
scope a delegate decides routine matters itself and documents them; otherwise
28+
delegation gains nothing. What comes back to the main loop, which keeps the
29+
overview, is anything that changes scope, contradicts the brief or the docs,
30+
or would be expensive to redo. Say that split explicitly in the prompt:
31+
decide-and-document versus return-as-finding.
32+
33+
**Two lines every brief carries** (2026-09-05). A delegate reads `CLAUDE.md`
34+
like anyone else, but it also receives harness reminders that arrive LATER in
35+
its context and therefore read as the more recent instruction. In auto mode
36+
one of them prescribes editing files through `sed`, heredocs and short
37+
scripts. In the sibling repository three agents in one day followed it against
38+
the Edit/Write rule, two of them for the single-token `sed -i` that fills the
39+
PR number into a changelog fragment — the same fragment format this repository
40+
uses. A fourth ran `git checkout -b` in the SHARED checkout, which moves a ref
41+
the owner's own working tree is sitting on. So spell both out in the brief, in
42+
the brief's own words:
43+
44+
1. Repo files are modified ONLY with Edit/Write — `CLAUDE.md` wins over the
45+
auto-mode reminder.
46+
2. All `git` happens in the agent's own worktree; the shared checkout is left
47+
on the branch it was found on.
48+
49+
Neither is new policy. Both are precedence a brief has to make explicit,
50+
because an agent cannot infer precedence from the order messages arrived in.
51+
52+
## External-system writes need explicit, named authorization
53+
54+
The rule is the asking. This section is about what to hand over once the
55+
answer is yes and the agent still cannot run the command itself.
56+
57+
Some prod runbooks are refused by the harness permission classifier rather
58+
than by the owner — the `gcloud run services update` plus `update-traffic`
59+
pair that arms the site's origin gate is one, observed 2026-09-04. Reaching
60+
for a different phrasing of the same command is the wrong move: the classifier
61+
is not an obstacle to route around. Stopping half-way is not the danger
62+
either, when the runbook is built for it — the pair in
63+
`infra/cloudflare/README.md` § "Arming, in full" stages the new revision and
64+
promotes it by name, so a config that fails to render leaves the revision
65+
never ready and traffic on the old one, which the README calls a safe failure
66+
and the reason its step 4 is not optional. The real danger is reading the
67+
STAGED revision as the finished one and reporting an arm that never took
68+
traffic.
69+
70+
The pattern that works: write the runbook as ONE script in the scratchpad —
71+
never into the repo — and hand the owner a single line to paste:
72+
73+
```
74+
! bash /tmp/…/scratchpad/arm-origin-gate.sh
75+
```
76+
77+
The script does the whole pair under `set -euo pipefail`, echoes what it is
78+
about to change, and ends by reading the state back — for that gate, the
79+
`x-origin-gate` response header AND `status.traffic`, because only the second
80+
says which revision answered. Rules for it: one action per script, and no
81+
secret values in its output.
82+
83+
**The reverse direction is its own script, not the same one with a flag.**
84+
`infra/cloudflare/README.md` § "Rolling back" is explicit about why: a
85+
rollback has to run in the worst state the service can be in, which includes
86+
the secret having been disabled during the incident — so it never looks the
87+
SECRET up. It still resolves the serving revision and its image, because it
88+
has to; what it must not do is depend on anything the incident may have taken
89+
away.
90+
91+
Afterwards, VERIFY from the session with a read the agent is allowed to make,
92+
instead of trusting a "done" in chat. A runbook that shipped this way belongs
93+
in the owning README, so the next round starts from a reviewed text rather
94+
than a fresh improvisation.
95+
96+
## Modify repo files only with the Edit/Write tools
97+
98+
Appending with `>>` counts — appending at the end of a file is exactly the
99+
forbidden path, however little it feels like editing.
100+
101+
When a Bash command legitimately mutates a tracked file (a formatter, codegen,
102+
`git checkout`), read the file again before the next edit; stale-state errors
103+
cascade otherwise.
104+
105+
The moment this rule gets broken is when an edit ANCHOR fails — "string not
106+
found", "file modified since read". The answer is a fresh targeted read plus a
107+
longer anchor, never a regex rewrite from a heredoc. The other moment is a
108+
change small enough to feel exempt: a one-token substitution reads like a
109+
`sed` job, and `sed -i` on a changelog fragment is how three delegated agents
110+
in one day broke the rule in the sibling repository (2026-09-05). Size is not
111+
the criterion; the tool is.
112+
113+
The exemptions are narrow and stated in `CLAUDE.md`: GitHub Actions workflows
114+
and codegen scripts write files by design. An interactive session is never one
115+
of those.

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

Lines changed: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,36 @@ format (Summary / Plan / Test plan), the changelog gate, the push,
116116
and the PR-ref follow-up. English throughout, no
117117
"Generated with..." lines in the body.
118118

119+
**A multi-paragraph commit message goes through a file — and that
120+
file belongs to this branch alone.** `git commit -F` keeps the prose
121+
out of shell quoting, but the scratchpad is shared by every agent of
122+
one session, so a generic `commitmsg.txt` gets overwritten by a
123+
parallel agent and a later re-read commits someone else's text
124+
(sibling repo, 2026-09-05: both a message and a body file were
125+
clobbered mid-run). Take a private directory, which needs no
126+
sanitising at all:
127+
128+
```bash
129+
D=$(mktemp -d) # or, if you name it: BRANCH=$(git branch --show-current)
130+
MSG="$D/commitmsg.txt" # SLUG=${BRANCH//\//-}; MSG="$D/commitmsg-$SLUG.txt"
131+
```
132+
133+
A branch name is not a filename — `release/v1.2.3` turns the slash
134+
into a directory that does not exist — so substitute the separators
135+
if you derive the name, and never write into a `$SCRATCH` you have
136+
not set yourself.
137+
138+
Write the file with the Write tool, then `git commit -F "$MSG"` in
139+
the SAME step that wrote it — never re-read one a turn later to reuse
140+
it, because between the two it may belong to another agent. These are
141+
scratch input to one command, not a record; the record is the commit.
142+
143+
The same holds for a PR body you pass as `--body-file`. The mandated
144+
`/pull_request` command does not take that path — it builds the body
145+
inline with a quoted heredoc (`agentic/commands/pull_request.md`
146+
step 6), which has no collision to avoid — so this applies when you
147+
write a body file yourself.
148+
119149
## 3 · After opening: pipeline + review loop (do not skip)
120150

121151
Repeat until **both** hold: all checks pass AND zero unresolved
@@ -135,15 +165,32 @@ On failure: `gh run view --log-failed`, fix, push to the same branch,
135165
keep watching.
136166

137167
**b. Wait for the Copilot review.** Bot login:
138-
`copilot-pull-request-reviewer[bot]`. It usually lands within ~2 min;
139-
if `gh pr view <num> --json reviewRequests,reviews` shows neither a
140-
request nor a review after the checks pass, request it explicitly
141-
(verified working):
168+
`copilot-pull-request-reviewer[bot]`. It arrives a few minutes after
169+
the PR is OPENED — not after each push; see "One review per PR"
170+
below. If `gh pr view <num> --json reviewRequests,reviews` shows
171+
neither a request nor a review after the checks pass, request it
172+
explicitly (verified working):
142173

143174
```bash
144175
gh api -X POST repos/{owner}/{repo}/pulls/<num>/requested_reviewers -f "reviewers[]=copilot-pull-request-reviewer[bot]"
145176
```
146177

178+
**One review per PR is the normal case now.** The ruleset "Automated
179+
Copilot Code Review" (anyplot 10370785, kurrentschrift 18516317)
180+
carries `review_on_push: false` since 2026-09-03 — the owner asked
181+
for the churn to stop, and the setting, not any skill, was what
182+
re-reviewed. Two consequences for this loop. A FIX push starts no new
183+
Copilot run, so a `copilot-*` check on the new head SHA is
184+
legitimately ABSENT; waiting for one that will never come is the
185+
failure mode to avoid — see §3e for what to require instead. And a
186+
fresh review is requested only after a SUBSTANTIVE rework (new
187+
behaviour, a reworked mechanism), never after every push: each
188+
request re-reads the whole diff and surfaces "previously missed"
189+
findings in files the push never touched, which draws another push
190+
(kurrentschrift#406 collected ~15 requests in a day over a one-line
191+
docstring fix). Stop once a round yields no new inline comments but
192+
only carried-over items.
193+
147194
Fetch all three comment surfaces — they carry different content:
148195

149196
```bash
@@ -180,6 +227,57 @@ gh api graphql -f query='mutation($id:ID!){resolveReviewThread(input:{threadId:$
180227
report the PR URL and final state. **Do not merge unless explicitly
181228
authorized.**
182229

230+
**Merging on request: wait for the review, not just for green.**
231+
When the owner does ask for the merge in this session, four
232+
conditions, all read on the CURRENT head SHA — re-read it after every
233+
push, `gh pr view <num> --json headRefOid`:
234+
235+
1. A draft is not reviewable — `gh pr ready <num>` first. Copilot
236+
does not review a draft, so a draft merged "green" was never
237+
reviewed, and `gh pr merge` on a draft fails anyway. Check
238+
`isDraft`.
239+
2. Every non-Copilot check on the head SHA is `completed` and green.
240+
Dedupe the check runs **by name, newest wins**: a superseded run
241+
(a label re-trigger, a cancelled first attempt) stays beside the
242+
current one and reads as a red check that is not there any more.
243+
Dedupe on `.id`, which grows with creation and is always set — a
244+
check run carries no `created_at`, and `started_at` stays null
245+
until the run begins, so a `max_by(.started_at)` would hand the
246+
row to the OLD completed attempt while the new one is still
247+
queued, which is the failure this step exists to prevent.
248+
```bash
249+
gh api repos/{owner}/{repo}/commits/$(gh pr view <num> --json headRefOid --jq .headRefOid)/check-runs \
250+
--jq '[.check_runs[]] | group_by(.name) | map(max_by(.id)) | .[] | "\(.name): \(.status) \(.conclusion // "")"'
251+
```
252+
3. **A Copilot review actually exists on the PR** — `gh pr view <num>
253+
--json reviews`, author `copilot-pull-request-reviewer`. The
254+
head-SHA check run does not prove one: a run reaches `completed`
255+
with conclusion `cancelled` and delivers nothing. So read the
256+
check run only to learn whether a round is still RUNNING
257+
(`queued`/`in_progress` means wait) and read the review list to
258+
learn whether one was ever delivered. Since `review_on_push` is
259+
off (§3b), the normal state after a fix push is no run on the head
260+
at all with the first round's review standing — that is reviewed,
261+
not unreviewed. If no review exists and the run was cancelled, one
262+
re-request is the whole budget; after that report
263+
green-and-unreviewed and let the owner decide, never loop.
264+
4. Zero unresolved review threads (step c), outdated ones included.
265+
266+
**Merge state is two different fields; read each by its own name.**
267+
`mergeable` (`gh pr view --json mergeable`) is `MERGEABLE`,
268+
`CONFLICTING` or `UNKNOWN``UNKNOWN` right after another merge is
269+
GitHub still computing, so keep polling. `mergeStateStatus` is the
270+
richer enum, where the conflicting case is `DIRTY`. A conflict is not
271+
transient and has a symptom worth knowing: GitHub starts no CI at
272+
all, so the PR shows no red check, just none (#11212 and
273+
kurrentschrift#524, 2026-09-04, both read as "checks pending" for a
274+
while). Report it and merge `origin/main` into the branch instead of
275+
waiting it out.
276+
277+
Poll all of this from ONE script rather than by hand, and kill a
278+
stale wait loop with the bracket trick (`pkill -f "x[.]y"`), or
279+
`pkill` matches its own calling shell.
280+
183281
## 4 · After merge (when it happens): watch the deploy
184282

185283
Merges to `main` touching `api/**`, `core/**`, or `pyproject.toml`
@@ -206,9 +304,12 @@ a 20-minute poll on the global list never saw the builds). Match the
206304
- **`isOutdated``isResolved`.** A fix-push can outdate a Copilot
207305
thread while it stays unresolved; outdated threads still count
208306
against review-clean — resolve them explicitly.
209-
- **Copilot reviews every push round.** New threads on changed lines
210-
are the loop working, not noise — but don't chase cosmetic nits
211-
past a couple of rounds; surface stalemates to the user.
307+
- **A fix push no longer starts a review round.** `review_on_push` is
308+
`false` since 2026-09-03 (§3b), so only an explicit — and
309+
substantive — re-request opens another one. When a round does run,
310+
new threads on the changed lines are the loop working, not noise —
311+
but don't chase cosmetic nits past a couple of rounds; surface
312+
stalemates to the user.
212313
- **Stacked PRs die when their base squash-merges.** Don't stack; if
213314
work depends on an unmerged PR, wait for its merge (or do the work
214315
and rebase before opening).

.claude/skills/verify-frontend/SKILL.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,27 @@ than restating values here. Method:
156156
Style questions are **findings to report**, not things to silently
157157
fix — palette and typography decisions are settled in the style guide.
158158

159+
## 3b · Numeric rules: measure the result, not the plan
160+
161+
**A numeric UI rule is verified against the MEASURED result in the
162+
browser, never against the planned one.** A floor, a minimum size, a
163+
cap, a hit target, the ~13 px legibility floor above — the
164+
verification names the rule and the number it measured, on every
165+
surface the rule reaches, at both viewports.
166+
167+
The case for it is a sibling-repo PR that shipped a 14 px x-height
168+
floor for rendered lines whose planner sized them from the average
169+
advance per character. The plan met the floor and the widest real
170+
line did not, because the frame's own padding scales with the content
171+
and was never in the budget: the page came out at **13.9 px** — a
172+
rule broken by the code that enforces it, and only the measurement on
173+
the page could say so.
174+
175+
So read the number off the element (`getBoundingClientRect()`,
176+
`getComputedStyle`), not off the code that computed it, and quote the
177+
measurement in the PR. When the fix is to re-plan from the measured
178+
value, say plainly which cases still fall outside the rule.
179+
159180
## 4 · Performance
160181

161182
Only when the change can plausibly move performance (data loading,

.claude/skills/write-docs/SKILL.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,35 @@ the house guide, repository docs follow Google style. Existing docs
8888
migrate **on touch** (the "fix formatting" rule below), no bulk
8989
rewrites required.
9090

91+
## Shortening a text that carries claims
92+
93+
Applies wherever the text lives — a `docs/` reference, `README.md`,
94+
the site's legal and about copy (`app/src/pages/LegalPage.tsx`,
95+
`app/src/pages/AboutPage.tsx`) — whenever the edit makes an existing
96+
text SHORTER: a legal section, a licensing paragraph, a factsheet,
97+
any prose a reader may rely on.
98+
99+
**Diff claim by claim against the previous version, not paragraph by
100+
paragraph.** Shortening drops qualifiers before it drops sentences,
101+
and a qualifier is what makes a claim true: "only", "unless", "up
102+
to", a condition on a right, the last item of a list. In the sibling
103+
repository on 2026-09-03 a privacy section lost exactly three that
104+
way — an overstated retention period for the rate-limit counters, the
105+
condition on the right to object, and part of a list of rights —
106+
inside an edit that was otherwise a genuine improvement. The check
107+
that catches it is mechanical: list the claims of the old text, then
108+
tick each one off against the new text as kept, deliberately dropped,
109+
or narrowed.
110+
111+
Two rules on top:
112+
113+
- **The owner's own sentences stay verbatim.** Where he supplied the
114+
wording, it is quoted, not paraphrased and not tightened. Say in
115+
the PR body that it is his sentence.
116+
- **A claim you cannot support is removed, never softened** into a
117+
vaguer version of itself: an unproven statement made fuzzy is still
118+
an unproven statement.
119+
91120
## Formatting
92121

93122
- Actually FIX formatting issues while editing (headings, lists, code

0 commit comments

Comments
 (0)