From 9dbe30c0ec8a6fc236da2bead775d19a4e613294 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sat, 31 Oct 2026 23:55:17 +0100 Subject: [PATCH 1/4] docs(agents): rules and skills from the 2026-10-31 session retro Applies the owner-approved findings of the retro of session 402df83f (2026-10-28 to 2026-10-31): - watch-deploys.sh ends with "no deploy for " after 6 empty polls, passes --project=anyplot, and exits 2 when gcloud fails; watch-checks.sh waits 90 polls and names the pending checks. - open-pr: hadolint gate for Dockerfiles, tests for new code before the PR opens (codecov triage folded in), the real check names, Copilot latency of about 5 minutes, the GraphQL requestReviews mechanic, the nvm prefix for site-smoke, a pull of the shared checkout after a merge, and a report of worktrees whose branch is gone. - zweitmeinung: attach a diff over about 60 KB, how --attach resolves paths, measured Gemini costs, cleanup of root-owned files. - CLAUDE.md: delegate stall check and post-compaction re-check, a classifier denial is final for every agent, first Copilot review at about 5 minutes, merges only on a named OK (mirrored in copilot-instructions.md). - guardrails.md: one builder per scope and one message per round, the denial incident, the Console-wizard pattern, the stalled-delegate incident and recipe. - Recipes: single-library impl-generate dispatch, eval-matrix forms, the second-generation trigger import route. - uv run --extra dev ruff wherever a skill, command or doc runs ruff. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01H7HSkGyea1k2v293XiAMo8 --- .claude/guardrails.md | 72 +++++++++- .claude/skills/gemini-task/SKILL.md | 4 +- .claude/skills/open-pr/SKILL.md | 130 ++++++++++++++---- .../skills/open-pr/scripts/watch-checks.sh | 19 ++- .../skills/open-pr/scripts/watch-deploys.sh | 22 ++- .claude/skills/verify-api/SKILL.md | 2 +- .claude/skills/verify-core/SKILL.md | 8 +- .claude/skills/zweitmeinung/SKILL.md | 45 +++++- .github/copilot-instructions.md | 16 +-- CLAUDE.md | 8 +- agentic/commands/agentic.md | 4 +- agentic/commands/audit.md | 6 +- agentic/commands/audit/backend-auditor.md | 2 +- agentic/commands/pull_request.md | 4 +- agentic/commands/refactor.md | 2 +- agentic/commands/regen.md | 4 +- agentic/commands/test.md | 8 +- agentic/commands/update.md | 8 +- agentic/docs/project-guide.md | 50 ++++++- agents/README.md | 17 ++- changelog.d/session-retro-rules.md | 29 ++++ docs/development.md | 14 +- docs/workflows/overview.md | 29 ++++ 23 files changed, 411 insertions(+), 92 deletions(-) create mode 100644 changelog.d/session-retro-rules.md diff --git a/.claude/guardrails.md b/.claude/guardrails.md index 3d3b26f748c..8903ef77f5d 100644 --- a/.claude/guardrails.md +++ b/.claude/guardrails.md @@ -31,8 +31,8 @@ work that Google's quota can do goes there first. | Reads or polls with credentials (CI, deploys, `gh`/`gcloud` state); creates Todoist tasks | Haiku | Cheap; never changes state | | High-risk reviews (security, prod data, sandbox, privacy); genuinely hard reasoning | Fable | About 2.5 times Opus per token at API prices, so only where it pays | | Larger steps and long refactors, and the main driver once Claude tokens run low late in the week | Gemini, in the owner's interactive agy session, through a brief file in `~/projects/anyplot-briefs/`, one package of at most 20 to 25 files per fresh agy subagent, independent packages in parallel (`/gemini-task`) | The owner decides what runs without asking; Gemini pushes a `gemini/*` branch and opens a draft PR; Claude reviews it, marks it ready, and follows it through | -| Check of every code PR Claude opens, before it opens; not for diffs only of docs, instruction files, changelog fragments, or `plots/`, under about 20 lines, or Dependabot (owner, 2026-10-29) | Gemini 3.8 Flash through Vertex AI in project `anyplot`, location `global`, on Flex PayGo (`/zweitmeinung`) | Copilot gets one round per PR, so its findings should already be fixed; half price on Flex, $0.07–0.28 per review | -| Second opinion, only when the owner asks for one | Fable and Gemini 4 Argon side by side, Argon on the same Vertex route with thinking high | Stronger across files; $0.57–1.16 per Argon run. Both Gemini lanes are paid per token and covered by the $100 monthly Google Cloud credit that comes with Google AI Ultra; Gemini reads tracked repository files, prefetched data, and the public API through read-only tools, with no shell and no credentials | +| Check of every code PR Claude opens, before it opens; not for diffs only of docs, instruction files, changelog fragments, or `plots/`, under about 20 lines, or Dependabot (owner, 2026-10-29) | Gemini 3.8 Flash through Vertex AI in project `anyplot`, location `global`, on Flex PayGo (`/zweitmeinung`) | Copilot gets one round per PR, so its findings should already be fixed; half price on Flex, median $0.27 per review ($0.07–1.14 over 40 runs to 2026-10-31) | +| Second opinion, only when the owner asks for one | Fable and Gemini 4 Argon side by side, Argon on the same Vertex route with thinking high | Stronger across files; median $1.21 per Argon run ($0.35–1.64 over 16 runs to 2026-10-31). Both Gemini lanes are paid per token and covered by the $100 monthly Google Cloud credit that comes with Google AI Ultra; Gemini reads tracked repository files, prefetched data, and the public API through read-only tools, with no shell and no credentials | The pipeline's model choices are separate. Sonnet's lane is narrow on purpose: it never runs `gh` writes, `git push`, or `gcloud`, and a task whose result no @@ -211,6 +211,19 @@ the brief's own words: Neither is new policy. Both are precedence a brief has to make explicit, because an agent cannot infer precedence from the order messages arrived in. +**One builder per scope, and one message per round** (2026-10-31 retro). The +review-fix rounds of a change stay with the builder that wrote it: it already +holds the context, and a fresh agent would spend its first hour rebuilding +it. New scope gets a new delegate on a fresh worktree instead of being fed to +a long-lived one instruction by instruction. In the session of 2026-10-28 to +2026-10-31, four builders that were steered that way (`ci-auth`, +`js-page-export`, `pb1-exec`, `pb2-serve`) grew to 11 to 19 MB of transcript +with two compactions each. Every message to an idle delegate also costs the +main loop a turn when the hand-back arrives: 67 messages drew 28 hand-backs +and at least 14 main-loop turns that only acknowledged an idle notice. So +collect what a delegate needs to hear, review findings and corrections +together, and send it as one message. + ## External-system writes need explicit, named authorization The rule is the asking. This section is about what to hand over once the @@ -255,6 +268,29 @@ instead of trusting a "done" in chat. A runbook that shipped this way belongs in the owning README, so the next round starts from a reviewed text rather than a fresh improvisation. +**A denial binds the whole session, delegates included** (2026-10-31). The +classifier refused a delegate's download of `actionlint`. The main loop then +asked the same delegate, in a batch of instructions, to try it through a +Docker image instead, "and if that is blocked too, say so and I run it". The +classifier refused that message itself as an auto-mode bypass, at 15:02 UTC. +The harness's own guidance speaks of denials "in your session", which reads +as if a peer were a fresh start; it is not, because the peer runs under the +same owner's permission decision. So a denial anywhere in the session is +final for every agent in it. The next step goes to the owner: the exact +command for a named yes, or a `!` line to run. Here the owner said yes and the +pinned image ran at 15:09. + +**Console wizards: a private artifact the owner's own browser agent follows** +(2026-10-31). Some setups exist only as clicks in a vendor console, such as +the Claude Console workload identity federation for CI. The WSL session has +no Chrome tools, so when the owner asked it to set this up through Claude in +Chrome, it could not. What worked: write the steps as a private artifact, +each field name copied from the vendor's documentation rather than recalled, +with the values to enter and the result to paste back. The owner handed the +link to Claude in Chrome in the owner's own browser, which clicked through +the wizard, and the result came back about 10 minutes after the request (14:37 to +14:48 UTC). + ## After every merge, check that the site renders Owner rule, 2026-10-22, after an outage nothing caught. @@ -292,6 +328,38 @@ break the site through the API, a Cloudflare rule, or a data change, and the run costs seconds. A `FAIL` stops further merges until the site renders, because each extra merge widens the set of suspects and the revert. +## Long-running work reports progress proactively + +**What happened (2026-10-30).** The `plot-all-pill` builder wrote "The gates +are still running (test and build); waiting for them to finish" at 12:09 UTC +and ended its turn on that line, without a commit and without a report; it +never woke again. The context compaction at 16:27 UTC summarised it as "still +working on the Flash fixes", and the main loop trusted the summary, which is +the only thing that survives a compaction. The main loop itself made no entry +between 17:48 and 19:46 UTC. At 19:46 the owner asked "läuft wirklich noch +was?" ("is anything really still running?"), and the answer at 19:50 was that +nothing had run since midday. The changes in the worktree were nearly +complete; the main loop finished them into #12478. On 2026-10-31 at 16:08 the +owner was again the first to say that work had gone idle ("es ist einiges +idle weiter"). + +**The check.** About 30 minutes with no new commit and no message from a +delegate is a suspected stall. So is any "still working" that a compaction +summary hands over. Before repeating one, read the delegate's real state: + +```bash +# ListAgents first: is it listed, busy or idle? +git -C log -1 --format='%ci %h %s' # its last commit, and when +git -C status --porcelain | head # uncommitted work it may have left +``` + +Idle, with no commit since the brief and changes left in the worktree, +means stopped, whatever the summary says. Report it to the owner at once, +with that evidence, and then either send the delegate one message that +resumes it or take the work over. A delegate that ended its turn "waiting +for" its own background job is the pattern to look for: nothing guarantees +it is woken when the job finishes. + ## Modify repo files only with the Edit/Write tools Appending with `>>` counts — appending at the end of a file is exactly the diff --git a/.claude/skills/gemini-task/SKILL.md b/.claude/skills/gemini-task/SKILL.md index cf7de97690b..2cfa8ca11cc 100644 --- a/.claude/skills/gemini-task/SKILL.md +++ b/.claude/skills/gemini-task/SKILL.md @@ -377,8 +377,8 @@ This section is for the Gemini agent in the owner's agy session. the owner every time (2026-10-30). ```bash - PYTHON_DOTENV_DISABLED=1 uv run ruff check . - PYTHON_DOTENV_DISABLED=1 uv run ruff format --check . + PYTHON_DOTENV_DISABLED=1 uv run --extra dev ruff check . + PYTHON_DOTENV_DISABLED=1 uv run --extra dev ruff format --check . PYTHON_DOTENV_DISABLED=1 uv run --extra typecheck mypy api core --pretty # Python changes PYTHON_DOTENV_DISABLED=1 uv run pytest tests/unit tests/integration -n auto # or the narrower set the task names PYTHON_DOTENV_DISABLED=1 uv run python -m tools.changelog check --base origin/main # after committing the fragment diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md index faea47700b6..afc5970ed44 100644 --- a/.claude/skills/open-pr/SKILL.md +++ b/.claude/skills/open-pr/SKILL.md @@ -46,7 +46,7 @@ PR/review/CI steps swap. The mapping: | List review threads | `gh api graphql … reviewThreads` (§3c) | `mcp__github__pull_request_read` (review-threads method) | | Reply on a thread | `gh pr comment` | `mcp__github__add_reply_to_pull_request_comment` (thread) / `mcp__github__add_issue_comment` (PR-level) | | Resolve a thread | `gh api graphql … resolveReviewThread` (§3d) | `mcp__github__resolve_review_thread` | -| Request Copilot review | `gh api -X POST … requested_reviewers` (§3b) | `mcp__github__request_copilot_review` | +| Request Copilot review | `gh api -X POST … requested_reviewers`; a further round through GraphQL `requestReviews` (§3b) | `mcp__github__request_copilot_review` | | Merge (only if authorized) | `gh pr merge` | `mcp__github__merge_pull_request` | In the cloud, prefer `mcp__github__subscribe_pr_activity` over any @@ -68,6 +68,7 @@ git diff --name-only origin/main... | `api/` | `/verify-api` (read sweep + the changed endpoint's payload) | | `core/`, `tests/` | `/verify-core` (pytest + direct smoke) | | `alembic/` | `/verify-migrations` (throwaway Postgres; the shared prod DB never sees an untested revision). CI verifies every migration on a throwaway PostgreSQL 18 (`Verify Migrations`, `.github/workflows/ci-migrations.yml`) for every PR touching `alembic/` or `core/database/`. | +| a Dockerfile: `api/`, `app/`, `agents/`, `agents/renderer/` (`Dockerfile`, `toolchain.Dockerfile`) | `docker run --rm -i hadolint/hadolint:v2.15.1 hadolint --failure-threshold warning - < `, CI's hadolint version and threshold (`ci-image.yml`, hadolint-action v3.5.0). It must exit 0 and print no `warning` or `error` line; `info` lines such as DL3059 sit below the threshold. DL4006 showed locally on 2026-10-28 and still failed the renderer image's CI on 2026-10-31 | | review rubric: `prompts/quality-criteria.md`, `prompts/workflow-prompts/ai-quality-review.md`, `prompts/default-style-guide.md`, review rules in `prompts/library/*.md`, review model routing | dispatch a candidate arm (`gh workflow run review-retest.yml -f rules_ref= -f label= -f compare_to=`) and paste its snippet in the PR (`docs/workflows/review-retest.md`) | | `.github/workflows/`, other `prompts/`, `automation/` | no live loop exists — reason through carefully, dry-run what's dry-runnable, and say so in the PR | | `docs/`, `CLAUDE.md`, instruction files | `/write-docs` (layer choice, `docs/index.md` + cross-file sync duties) | @@ -95,12 +96,23 @@ Skip it for a diff made only of docs, instruction files (`CLAUDE.md`, `.github/copilot-instructions.md`, skills, commands), changelog fragments, or `plots/`, for a diff under about 20 lines, and for Dependabot bumps. +**Tests for new code, before the PR opens.** Every new code path gets its unit +test in the branch before the PR opens: `codecov/patch/backend` and +`codecov/patch/frontend` are checks like any other, and on #12484 and #12487 +(2026-10-31) the patch check, not the review, was what kept the PR from being +ready. Judge like a reviewer, not by the percentage: uncovered NEW pure logic +that a unit test reaches cheaply gets a test — extracting a pure core from an +async/DB wrapper is the preferred move. Lines only a live DB/HTTP flow +exercises are the `/verify-api` sweep's job; leave those and say so in the PR +if the patch percentage looks alarming. + Then the local CI equivalents — the same commands the pipeline runs. **Hard gate: do not open the PR while any of these is red.** ```bash -# backend (CI job "Run Linting" + "Run Tests") -uv run ruff check . && uv run ruff format --check . +# backend (CI job "Run Linting" + "Run Tests"); ruff lives in the dev extra, +# which a fresh worktree's .venv lacks +uv run --extra dev ruff check . && uv run --extra dev ruff format --check . uv run --extra typecheck mypy api core --pretty uv run pytest tests/unit tests/integration -n auto # CI: -n 4 --dist worksteal @@ -123,7 +135,11 @@ Branch first (never commit on `main`), commit with a conventional message, then use the **`/pull_request` command** — it owns the body format (Summary / Plan / Test plan), the changelog gate, the push, and the PR-ref follow-up. English throughout, no -"Generated with..." lines in the body. +"Generated with..." lines in the body. That repository rule outranks the +harness's attribution reminder for the PR body: the body may end with the +session link the reminder names, but never with its "Generated with…" line. +The commit trailers the reminder asks for (`Co-Authored-By`, +`Claude-Session`) stay. **A multi-paragraph commit message goes through a file — and that file belongs to this branch alone.** `git commit -F` keeps the prose @@ -160,11 +176,18 @@ write a body file yourself. Repeat until **both** hold: all checks pass AND zero unresolved review threads. -**a. Watch the pipeline.** Check names: `Run Linting`, `Run Tests`, -`Run Frontend Tests`, `Analyze (python|javascript-typescript|actions)`, -`CodeQL`. Wait for checks to REGISTER before watching — `gh pr checks ---watch` right after creation can exit on an empty list. Use a -background poll so other work continues: +**a. Watch the pipeline.** #12487 (2026-10-31) registered these 16 +checks: `Run Linting`, `Run Tests`, `Run E2E Tests`, `Run Frontend Tests`, +`Changelog (fragment)`, `Verify Migrations`, `Build API image and smoke the +container`, `Build app image and smoke the origin gate`, `Build agents image +and smoke the service`, `Build renderer image and render through the +harness`, `Analyze (python|javascript-typescript|actions)`, `CodeQL`, +`codecov/patch/backend` and `codecov/patch/frontend`. Hadolint is not a +check of its own: each `Build … image` job runs it as a `Hadolint ()` +step, so a lint finding shows as that job failing. The codecov patch checks +report last, after the test jobs upload coverage. Wait for checks to +REGISTER before watching — `gh pr checks --watch` right after creation can +exit on an empty list. Use a background poll so other work continues: ```bash .claude/skills/open-pr/scripts/watch-checks.sh # run in the background @@ -175,20 +198,39 @@ check runs by name (newest id wins, §3e), prints each check as it finishes and exits once at least 10 runs exist and none is pending (pass a smaller minimum as the second argument for a PR whose paths start fewer checks). Its last line counts the checks that are not -green. On failure: `gh run view --log-failed`, fix, push to the same branch, -keep watching. +green. It gives up after 90 polls, about 45 minutes, because a PR that +builds the renderer image takes 30 to 40: it then exits 1 with a `TIMEOUT` +line that names the checks still pending. On failure: `gh run view +--log-failed`, fix, push to the same branch, keep watching. **b. Wait for the Copilot review.** Bot login: -`copilot-pull-request-reviewer[bot]`. It arrives a few minutes after -the PR is OPENED — not after each push; see "One review per PR" -below. If `gh pr view --json reviewRequests,reviews` shows -neither a request nor a review after the checks pass, request it -explicitly (verified working): +`copilot-pull-request-reviewer[bot]`. It arrives about 5 minutes after +the PR is OPENED (1 to 7 minutes on 29 PRs, 2026-10-28 to 31) — not after +each push; see "One review per PR" below. If `gh pr view --json +reviewRequests,reviews` shows neither a request nor a review after the +checks pass, request it explicitly: ```bash gh api -X POST repos/{owner}/{repo}/pulls//requested_reviewers -f "reviewers[]=copilot-pull-request-reviewer[bot]" ``` +For a further round, which the next paragraph allows only after a high or +critical fix, that REST call can return success and request nothing once +Copilot has reviewed the PR (#12200, 2026-10-15; it worked again on +2026-10-23). Request the round through GraphQL with Copilot's bot node id, +then wait for it with `watch-copilot.sh 2`: + +```bash +gh api graphql -F pr="$(gh pr view --json id --jq .id)" -f query=' + mutation($pr: ID!) { + requestReviews(input: {pullRequestId: $pr, botIds: ["BOT_kgDOCnlnWA"], union: true}) { pullRequest { number } } + }' +``` + +`BOT_kgDOCnlnWA` is the reviewer's node id; if the mutation refuses it, read +the id again from any of its reviews (`reviews { nodes { author { login ... +on Bot { id } } } }` in a `pullRequest` query). + **One review per PR is the normal case now.** The ruleset "Automated Copilot Code Review" (anyplot 10370785, kurrentschrift 18516317) carries `review_on_push: false` since 2026-09-03 — the owner asked @@ -216,13 +258,10 @@ gh api repos/{owner}/{repo}/pulls//comments # inline file/line comments gh api repos/{owner}/{repo}/issues//comments # conversation (codecov, bots, humans) ``` -**b2. Codecov triage** (comment arrives after coverage upload; the -repo uses flags/components). Judge like a reviewer, not a hard gate: -uncovered NEW pure logic that a unit test reaches cheaply gets a test -in the same PR — extracting a pure core from an async/DB wrapper is -the preferred move. Lines only a live DB/HTTP flow exercises are the -`/verify-api` sweep's job; leave those and say so in the PR if the -patch percentage looks alarming. +**b2. Codecov** (the comment arrives in the conversation after the +coverage upload; the repo uses flags/components). A red +`codecov/patch/*` check means §1's test rule was missed: add the test to +the same branch, judged the way §1 says. **c. List unresolved threads** (the id is needed for resolving): @@ -318,7 +357,13 @@ The watcher lists the regional builds whose `SHORT_SHA` (7 characters) matches the merge, prints each as it finishes, and exits once at least one build appeared and none has been queued or working for two polls in a row, since a merge starts one to four deploys -depending on its paths. +depending on its paths. When no build has appeared after 6 polls, about +3 minutes, it exits 0 with `no deploy for `: a triggered build is +queued within a minute, so the merge's paths triggered none (#12487, a +`.github/workflows/` merge, kept the old watcher waiting until it was +killed). It gives up with exit 1 after 60 polls (30 minutes), and exits +2 at once when `gcloud builds list` fails, so a lapsed login never reads +as "no deploy". The `--region` flag is load-bearing: the `deploy-app` / `deploy-api` triggers are **regional** (europe-west4), and the global `gcloud builds @@ -336,6 +381,7 @@ the merge's deploys have finished (the watcher above shows them `SUCCESS`; right after the merge when it triggered none), run: ```bash +source ~/.nvm/nvm.sh && nvm use 24 >/dev/null # harness shells may still start on Node 22 SCRATCH= [ -d "$SCRATCH/node_modules/playwright-core" ] || \ (cd "$SCRATCH" && PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 npm i --silent playwright-core@1.64.1) @@ -383,12 +429,37 @@ if [ -n "$MERGED" ] && [ "$MERGED" = "$(git rev-parse )" ]; then else echo "local tip differs from the merged head: keep both and ask the owner" fi +# keep the shared checkout current when it sits on main with no tracked changes +if [ "$(git -C "$MAIN" branch --show-current)" = main ] && git -C "$MAIN" diff --quiet HEAD; then + git -C "$MAIN" pull -q --ff-only +fi ``` +The pull matters because every agent of the session reads plans and +docs from the shared checkout: on 2026-10-28 a plan file read from it +did not exist yet, and it ended the session six commits behind +`origin/main`. + This covers merged PRs only. A branch whose PR is still open, a branch whose tip differs from the merged head, and someone else's worktree stay; deleting those still needs the owner's OK. +**Once per session, report the worktrees whose branch is gone from +`origin`** — the owner decides what goes; never delete one unasked. By +2026-10-31, 54 of 69 worktrees sat on branches that no longer existed +on `origin`, most of them left by older sessions: + +```bash +remote=$(git -C "$MAIN" ls-remote --heads origin | sed 's#.*refs/heads/##') +git -C "$MAIN" worktree list --porcelain | + awk '/^worktree /{sub(/^worktree /, ""); w=$0} /^branch /{sub(/^branch refs\/heads\//, ""); print $0 "\t" w}' | + while IFS=$'\t' read -r b w; do grep -qxF "$b" <<<"$remote" || echo "no origin/$b: $w"; done +``` + +`ls-remote` asks `origin` itself, so a remote-tracking ref that a fetch +without `--prune` left behind does not hide a deleted branch. A branch +that was never pushed shows up too; say which ones in the report. + ## Gotchas - **Never update a PR branch with `GITHUB_TOKEN`** (`update-branch` @@ -416,11 +487,12 @@ worktree stay; deleting those still needs the owner's OK. - **The three comment APIs really differ** — Copilot's overview only appears in `/reviews`; codecov only in `/issues//comments`. Checking just `gh pr view --comments` misses content. -- **Auto-review may not fire at all** on some PRs — don't wait past - ~5 min without checking `reviewRequests`; request explicitly (§3b). - But a missing review is never evidence the bot is inactive on this - repo — reviews have landed well after 5 min and carried real - findings. Do not declare the task done while the review is +- **Auto-review may not fire at all** on some PRs — the first review + usually lands 1 to 7 minutes after the PR opens (§3b), so don't wait + past about 8 minutes without checking `reviewRequests`; request + explicitly (§3b). But a missing review is never evidence the bot is + inactive on this repo — reviews have landed well after that and + carried real findings. Do not declare the task done while the review is outstanding; if the session ends first, say explicitly that the review is still pending. - **Background-poll mechanics:** if you use the Monitor tool, load diff --git a/.claude/skills/open-pr/scripts/watch-checks.sh b/.claude/skills/open-pr/scripts/watch-checks.sh index cb50c53085f..5a357688f4c 100755 --- a/.claude/skills/open-pr/scripts/watch-checks.sh +++ b/.claude/skills/open-pr/scripts/watch-checks.sh @@ -1,9 +1,11 @@ #!/usr/bin/env bash # Watch a PR's checks on its current head (§3a): print each check once it finishes, exit 0 -# when every check has finished, 1 after ~25 minutes. Reads the check-runs API, deduped -# by name with the newest id winning (§3e), and re-reads the head on every poll, so a -# push during the watch is followed. A check that has not registered yet cannot be told -# from one that never comes, so the watch also waits until at least MIN_CHECKS runs exist. +# when every check has finished, 1 after ~45 minutes (90 polls; a PR that builds the +# renderer image takes 30 to 40 minutes) with the names of the checks still pending. +# Reads the check-runs API, deduped by name with the newest id winning (§3e), and re-reads +# the head on every poll, so a push during the watch is followed. A check that has not +# registered yet cannot be told from one that never comes, so the watch also waits until +# at least MIN_CHECKS runs exist. # # Usage: watch-checks.sh [min-checks, default 10] set -uo pipefail @@ -11,7 +13,7 @@ PR=${1:?usage: watch-checks.sh [min-checks]} MIN_CHECKS=${2:-10} REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) seen="" -for _ in $(seq 1 50); do +for _ in $(seq 1 90); do head=$(gh pr view "$PR" -R "$REPO" --json headRefOid --jq .headRefOid) runs=$(gh api "repos/$REPO/commits/$head/check-runs?per_page=100" --jq \ '[.check_runs[]] | group_by(.name) | map(max_by(.id)) | .[] | "\(.name): \(.status) \(.conclusion // "")"') @@ -28,5 +30,10 @@ for _ in $(seq 1 50); do fi sleep 30 done -echo "TIMEOUT: $pending of $total checks still pending on ${head:0:9}" +names=$(grep . <<<"$runs" | grep -v ": completed" | sed -E 's/: [a-z_]+ ?$//' | paste -sd ',' - | sed 's/,/, /g') +if [ -n "$names" ]; then + echo "TIMEOUT: $pending of $total checks still pending on ${head:0:9}: $names" +else + echo "TIMEOUT: only $total checks registered on ${head:0:9}, none pending (waited for $MIN_CHECKS)" +fi exit 1 diff --git a/.claude/skills/open-pr/scripts/watch-deploys.sh b/.claude/skills/open-pr/scripts/watch-deploys.sh index a2517d9ecbf..599fba1b167 100755 --- a/.claude/skills/open-pr/scripts/watch-deploys.sh +++ b/.claude/skills/open-pr/scripts/watch-deploys.sh @@ -2,7 +2,10 @@ # Watch the Cloud Build deploys of one merge commit (§4): every regional build whose # SHORT_SHA matches. Prints each build once it finishes; exits 0 when at least one build # was seen and none has been queued or working for two polls in a row (a merge triggers -# one to four deploys, depending on the paths it touched), 1 after ~30 minutes. +# one to four deploys, depending on the paths it touched), 0 with "no deploy for " +# when no build has appeared after 6 polls (about 3 minutes; a triggered build is queued +# within a minute, so the merge's paths triggered none), 1 after ~30 minutes, and 2 at +# once when `gcloud builds list` fails, so a broken login never reads as "no deploy". # # Usage: watch-deploys.sh (the full SHA or its first 7 characters, Cloud Build's SHORT_SHA) set -uo pipefail @@ -10,10 +13,19 @@ SHA=${1:?usage: watch-deploys.sh } SHA=${SHA:0:7} seen="" quiet=0 -for _ in $(seq 1 60); do - rows=$(gcloud builds list --region=europe-west4 --limit 20 \ - --format='value(id,status,substitutions.TRIGGER_NAME,substitutions.SHORT_SHA)' 2>/dev/null | - awk -v sha="$SHA" '$4 == sha {print $3": "$2" ("$1")"}' | sort) +any=0 +for poll in $(seq 1 60); do + if ! list=$(gcloud builds list --region=europe-west4 --project=anyplot --limit 20 \ + --format='value(id,status,substitutions.TRIGGER_NAME,substitutions.SHORT_SHA)' 2>&1); then + echo "gcloud builds list failed: $(tail -1 <<<"$list")" + exit 2 + fi + rows=$(awk -v sha="$SHA" '$4 == sha {print $3": "$2" ("$1")"}' <<<"$list" | sort) + [ -n "$rows" ] && any=1 + if [ "$any" -eq 0 ] && [ "$poll" -ge 6 ]; then + echo "no deploy for $SHA" + exit 0 + fi finished=$(grep -E "SUCCESS|FAILURE|CANCELLED|TIMEOUT|INTERNAL_ERROR|EXPIRED" <<<"$rows") fresh=$(comm -13 <(echo "$seen") <(echo "$finished")) [ -n "$fresh" ] && echo "$fresh" diff --git a/.claude/skills/verify-api/SKILL.md b/.claude/skills/verify-api/SKILL.md index 2f22d47aefd..1cedd0dc0d0 100644 --- a/.claude/skills/verify-api/SKILL.md +++ b/.claude/skills/verify-api/SKILL.md @@ -94,7 +94,7 @@ and uses a separate `test` database (auto-created/dropped) — skipped when unset. Lint gate before commit, both halves: ```bash -uv run ruff check . && uv run ruff format --check . +uv run --extra dev ruff check . && uv run --extra dev ruff format --check . uv run --extra typecheck mypy api core --pretty ``` diff --git a/.claude/skills/verify-core/SKILL.md b/.claude/skills/verify-core/SKILL.md index 07c98493ccb..6a5c2cfacfd 100644 --- a/.claude/skills/verify-core/SKILL.md +++ b/.claude/skills/verify-core/SKILL.md @@ -35,7 +35,7 @@ that's an explicit acceptance criterion. Lint + type gates, the same three CI runs (`Run Linting` job): ```bash -uv run ruff check . && uv run ruff format --check . +uv run --extra dev ruff check . && uv run --extra dev ruff format --check . uv run --extra typecheck mypy api core --pretty ``` @@ -95,6 +95,12 @@ To check against **real** data, don't reach for the DB from a script gates use: `uv sync --extra test --extra dev --extra typecheck --extra agents` (CI's "Run Tests" syncs `--extra test --extra agents`); mypy always needs `--extra typecheck`. +- **The system `python3` is 3.12; the project and CI run 3.14** + (`requires-python = ">=3.14"`). A repo script with 3.14 syntax, such + as a PEP 758 `except A, B:` without parentheses, stops with + `SyntaxError: multiple exception types must be parenthesized` under + `python3` (2026-10-31, `python3 automation/scripts/regen_pick_pairs.py`). + Run repo scripts with `uv run python`, which uses the project's 3.14. ## Troubleshooting diff --git a/.claude/skills/zweitmeinung/SKILL.md b/.claude/skills/zweitmeinung/SKILL.md index 2fb6e483ab7..ce8a9966855 100644 --- a/.claude/skills/zweitmeinung/SKILL.md +++ b/.claude/skills/zweitmeinung/SKILL.md @@ -94,6 +94,30 @@ certainly read. Strip personal data (feedback messages, emails, IP hashes) and n a secret there; the script refuses obvious secret file names in `--data`, but the content is your responsibility. +**A diff over about 60 KB is attached, not inlined.** Check the size first +(`git diff origin/main... | wc -c`). Above about 60 KB, `--diff-base` puts so much +into every turn that the run can end on its input budget before it answers: on +2026-10-31 a 90,000-character diff cost $0.30 and returned "no answer: input budget" at +about 99k tokens per turn. Leave `--diff-base` out, write the diff without fixtures, docs +and changelog fragments to `$D/data/.diff`, attach it, and tell Gemini in the +briefing to review from it with at most six tool calls: + +```bash +git diff origin/main... -- . ':(exclude)docs' ':(exclude)changelog.d' ':(exclude,glob)**/fixtures/**' \ + > "$D/data/.diff" +``` + +One attachment holds at most 300,000 bytes; split a larger diff by directory. + +**`--attach` paths resolve against your current directory, not against `--root`.** The +script accepts a path only if it resolves to an existing file inside `--root` or +`--data`, and shows it to Gemini as `path/in/repo` or `data/`. Anything else ends +the run with exit 2 before the first model call, and the round trip is lost: on +2026-10-29 `data/pr3b.diff` ("attachment is not a file") and `agents/anyplot/data/kinds.py` +("attachment lies outside --root and --data") were both resolved in the shared checkout. +Pass absolute paths (`"$W/agents/…"`, `"$D/data/.diff"`), and write each file +before the run starts. + Gemini's safety filter refuses prompts that read like attacks (finding or using tokens, probing isolation); the script then exits 3 with a `block_reason`. Rephrase as a review question; for real security probing use Fable. @@ -117,6 +141,18 @@ git worktree remove --force "$W" `--model` goes before `review`. For a plan without code changes, `--root` is a worktree of `origin/main`, or the current branch's worktree, which the tools only read. +A container run against the review worktree, such as a renderer image a Fable reviewer +starts to reproduce a finding, writes as root unless told otherwise, and the root-owned +files it leaves (a `__pycache__` on 2026-10-31) make `git worktree remove --force` fail +with "Permission denied". Mount the worktree read-only (`-v "$W":/w:ro`) or run the +container with `--user "$(id -u):$(id -g)"`. If it already happened, hand the files back +through the same image, then remove the worktree: + +```bash +docker run --rm --network none --user 0 -v "$W":/w --entrypoint chown -R "$(id -u):$(id -g)" /w +git worktree remove --force "$W" +``` + - **Exit 0:** read `$D/gemini.md`. `$D/gemini.json` holds turns, tool calls, token counts (also per turn, to see the cache hit rate), `traffic_type` (`ON_DEMAND_FLEX` confirms Flex) and `est_cost_usd` at Vertex list price for `global`, per 1M tokens: Argon $4 / @@ -128,9 +164,12 @@ of `origin/main`, or the current branch's worktree, which the tools only read. - **Bounds:** 8 tool turns and 600k input tokens summed over all turns (`--max-turns`, `--max-input-tokens`). Then the tool responses tell Gemini to answer from what it has (`stopped_by`), and two more tool calls at most are refused before the run ends. -- **Cost, measured on 2026-10-21 and 2026-10-22:** Flash $0.07–0.28 per review, mostly - thinking; Argon $0.57–1.16. The owner's account merges about 8 PRs a day, so routine - reviews must stay cheap. +- **Cost, from the ledger's runs of 2026-10-21 to 2026-10-31:** Flash, 40 runs, $0.07 to + $1.14 per review, median $0.27, the middle half $0.13 to $0.32; Argon, 16 runs, $0.35 to + $1.64, median $1.21, the middle half $1.01 to $1.31. The six Flash runs above $0.70 all + summed 0.8 to 1.4 million tokens over their turns, so a large diff, not the model, sets + the price. The owner's account merges about 8 PRs a day, so routine reviews must stay + cheap. - **Flash is on probation.** The owner's decision on 2026-10-22: measure it on real PRs, with no extra test runs. On every PR Flash reviewed, once Copilot's review is in, add `copilot_findings` (the real ones) and `matched_copilot` (how many of those Flash also diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 77434de5607..3ca5e07570f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,7 +12,7 @@ A companion guide `CLAUDE.md` at the repo root carries the shared rules for Clau - **Never echo secret values into transcripts or logs** - Verify secrets by exit code or metadata, never by printing them. - **Fix small build/test blockers directly, even when out of scope** - If a typecheck error, failing test, lint failure, or other small pipeline blocker shows up while you work on something unrelated, fix it in the same PR or a tiny follow-up — never leave it parked as "pre-existing" or "out of scope". A latent error that doesn't surface locally silently breaks the next Cloud Build, which deploys nothing while every PR check looks green. The bar: if the fix is under ~20 lines and obviously correct, make it; if it would expand scope meaningfully, ask first rather than deferring silently. - **Structural fix over symptomatic fix** - When a cheap symptomatic fix and a correct structural fix compete, take the structural one: fix the cause, never mute the alarm. Never modify working code to make a broken test pass — fix the test or flag it. -- **Never manually merge a pipeline PR, and never bypass the pipeline** - Specifications and implementations go through the GitHub Actions workflows: `spec-create.yml` writes `plots/{spec-id}/specification.md` and `.yaml`, `impl-merge.yml` merges implementation PRs and creates their `metadata/*.yaml`, promotes the preview images to GCS and sets the `impl:{library}:done` label. Merging one by hand skips all of that and leaves `quality_score: null`, no review data, missing GCS images and the issue open. So: never write those files by hand, never run `gh pr merge` on a spec or implementation PR, and add the `approved` label to the **issue**, never to the PR. The full DON'T/DO table is in `CLAUDE.md`. Ordinary (non-pipeline) PRs are merged by the repository owner, not by an agent. +- **Never manually merge a pipeline PR, and never bypass the pipeline** - Specifications and implementations go through the GitHub Actions workflows: `spec-create.yml` writes `plots/{spec-id}/specification.md` and `.yaml`, `impl-merge.yml` merges implementation PRs and creates their `metadata/*.yaml`, promotes the preview images to GCS and sets the `impl:{library}:done` label. Merging one by hand skips all of that and leaves `quality_score: null`, no review data, missing GCS images and the issue open. So: never write those files by hand, never run `gh pr merge` on a spec or implementation PR, and add the `approved` label to the **issue**, never to the PR. The full DON'T/DO table is in `CLAUDE.md`. Ordinary (non-pipeline) PRs are merged only on the owner's explicit, named authorization in the session. ## Changelog + releases @@ -74,14 +74,14 @@ uv run pytest tests/integration # Run only E2E tests (requires DATABASE_URL) uv run pytest tests/e2e -# Linting (required for CI) -uv run ruff check . +# Linting (required for CI); ruff is in the dev extra +uv run --extra dev ruff check . # Auto-fix linting issues -uv run ruff check . --fix +uv run --extra dev ruff check . --fix # Formatting (required for CI) -uv run ruff format . +uv run --extra dev ruff format . ``` ### Frontend Development @@ -272,8 +272,8 @@ Automatic deployment on push to `main`: Before completing any task: 1. All tests pass: `uv run pytest tests/unit tests/integration` -2. Code passes linting: `uv run ruff check .` -3. Code is properly formatted: `uv run ruff format --check .` +2. Code passes linting: `uv run --extra dev ruff check .` +3. Code is properly formatted: `uv run --extra dev ruff format --check .` 4. Type hints are included for all new functions 5. Docstrings follow Google style for public functions 6. Add integration tests for database-related changes (repositories, models) @@ -330,5 +330,5 @@ uv run alembic upgrade head - **Import errors**: Run `uv sync --all-extras` to install dependencies - **Test failures**: Fix a small failure you run into even when your task did not cause it (see "Fix small build/test blockers directly" under Important Rules); ask before a fix that would expand the PR's scope meaningfully -- **Linting failures**: Run `uv run ruff check . --fix` to auto-fix common issues +- **Linting failures**: Run `uv run --extra dev ruff check . --fix` to auto-fix common issues - **Type checking**: Add type hints using standard Python typing (e.g., `list[str]`, `dict[str, int]`) diff --git a/CLAUDE.md b/CLAUDE.md index 7b3291d7641..6a16225f95e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,11 +18,11 @@ A companion guide `.github/copilot-instructions.md` carries the shared rules for - **Repository prose follows the Google developer documentation style guide** - `docs/`, `README.md`, `agentic/docs/`, changelog entries, and PR/issue text use [Google style](https://developers.google.com/style) (sentence-case headings, second person, numbered procedures); the concrete rules and the house-style exception (`docs/reference/style-guide.md` governs website/brand surfaces) live in the `write-docs` skill. Existing docs migrate on touch, not via bulk rewrites. - **Update documentation when making changes** - When adding new features, events, or modifying behavior, always check if related documentation needs updating (e.g., `docs/reference/plausible.md` for analytics events, `docs/workflows/` for workflow changes, `docs/contributing.md` for user-facing changes). - **Changelog and releases** - see [Changelog + releases](#changelog--releases) below: every PR adds a fragment under `changelog.d/` and adds nothing to `CHANGELOG.md` (correcting the wording of a bullet already under `[Unreleased]` is allowed), a release folds the fragments and bumps the version files via a PR, and the GitHub release is that section condensed, never copied. -- **External-system writes need explicit, named authorization** - Merging or closing PRs/issues this session did not create, bulk merges, and label changes on others' PRs are blocked by the permission classifier unless the user named that action; a generic "ok, sounds good" authorizes nothing. CI runs a Dependabot PR without secrets and without an OIDC token, keyed on the PR author: `ci-tests.yml` keeps every GCP step in the `test-e2e` job. The same discipline covers every prod-touching action in interactive sessions — Cloud SQL writes/DDL, GCS production-folder changes, Secret Manager access, Cloud Build config: name the exact action, resource, and id, and ask before acting (the automated `spec-*`/`impl-*` workflows write these by design and are exempt). For any change to `.claude/settings*.json`, use the built-in `/update-config` skill (a Claude Code harness skill, not a repo command) immediately — direct writes are blocked as self-modification and retrying variants just burns round trips. +- **External-system writes need explicit, named authorization** - Merging or closing PRs/issues this session did not create, bulk merges, and label changes on others' PRs are blocked by the permission classifier unless the user named that action; a generic "ok, sounds good" authorizes nothing. CI runs a Dependabot PR without secrets and without an OIDC token, keyed on the PR author: `ci-tests.yml` keeps every GCP step in the `test-e2e` job. The same discipline covers every prod-touching action in interactive sessions — Cloud SQL writes/DDL, GCS production-folder changes, Secret Manager access, Cloud Build config: name the exact action, resource, and id, and ask before acting (the automated `spec-*`/`impl-*` workflows write these by design and are exempt). For any change to `.claude/settings*.json`, use the built-in `/update-config` skill (a Claude Code harness skill, not a repo command) immediately — direct writes are blocked as self-modification and retrying variants just burns round trips. A permission-classifier denial anywhere in the session, in the main loop or in a delegate, is final: never ask a delegate or peer to retry it another way; hand the owner a `!` line (guardrails.md). - **Never echo secret values into the transcript** - Verify secrets by exit code or metadata, never by printing them; never create a Secret Manager version via `echo` (the trailing newline corrupts the value). - **Snapshot before destructive prod operations** - Before anything that can overwrite or delete shared prod DB data or GCS production objects (bulk UPDATE/DELETE, a data-rewriting migration, bulk GCS overwrite/delete): take a timestamped backup first (`pg_dump` to a new directory outside the working tree, `gsutil cp` to a backup prefix), sanity-check it (row/object counts — a silent empty snapshot is worse than none because it looks like safety), and never write into, delete, or rename an existing snapshot. - **After every merge, check that the site renders** - Yours, Dependabot's, or one the owner asked for: once the merge's deploys finish, run `scripts/site-smoke.mjs` (recipe in `/open-pr` §4); a `FAIL` is an incident, reported at once with the suspect merge and a revert proposal, and nothing else merges until the site renders. A green PR plus a green deploy blanked anyplot.ai for 45 minutes on 2026-10-22 (#12225). -- **Long-running work reports progress proactively** - During pipeline babysitting, bulk operations, or extended planning, post a one-line status roughly every 10 minutes and flag a suspected stall immediately with evidence; never let the user be the one to ask "still running?". Before starting a multi-item queue, confirm its extent and stop-point with the user. +- **Long-running work reports progress proactively** - During pipeline babysitting, bulk operations, or extended planning, post a one-line status roughly every 10 minutes and flag a suspected stall immediately with evidence; never let the user be the one to ask "still running?". A delegate with no new commit and no message for about 30 minutes is a suspected stall: check it and report it. After every context compaction, re-check each delegate's real state (`ListAgents`, `git -C log -1 --format=%ci`, `git -C status --porcelain`) before repeating a summary's "still working" (recipe and the 2026-10-30 incident in guardrails.md). Before starting a multi-item queue, confirm its extent and stop-point with the user. - **Manual user tasks go to Todoist** - Whenever a session identifies a step only the user can or should do (adding `approved` labels, merge authorization, console/billing/DNS actions, secret rotations), create a task in the user's Todoist project **Anyplot** (via the Todoist MCP tools) naming the concrete action and a context link (PR/issue/run URL) — instead of leaving it buried in a chat reply. Interactive sessions only; GitHub Actions workflows have no MCP access. - **Fix small build/test blockers directly, even when out of scope** - If a typecheck error, failing test, lint failure, or other small pipeline blocker shows up while working on something unrelated (incl. things the current PR was not meant to touch), fix it in the same PR or a tiny follow-up — never leave it parked under "out of scope". A latent `tsc` error that doesn't surface locally will silently break the next Cloud Build, which deploys nothing new and leaves production stale even though every PR check looks green (this is exactly how PR #6961's frontend fixes never reached anyplot.ai — the unrelated `prism/r` TS7016 from #6944 was deferred, then blocked the next `yarn build`). The bar: if the fix is < ~20 lines and obviously correct, just do it; if it would expand scope meaningfully, ask first rather than deferring silently. @@ -42,13 +42,13 @@ After opening a PR, the work is **not** complete. Stay with the PR until both th 1. **Watch the pipeline.** Poll `gh pr checks ` (and Cloud Build for triggered deploys) until every required check has finished. Use a background bash poll so other work can continue. Default: poll every 20 s, up to ~10 min per check. 2. **Fix CI failures.** If any check fails, read the relevant log (`gh run view --log-failed`, `gcloud builds log `), push a fix commit to the same branch, then keep watching. Repeat until green. -3. **Wait for the Copilot PR Reviewer bot — ONE review per PR, not one per push.** Review-on-push is off in the "Automated Copilot Code Review" ruleset (owner, 2026-09-03): the bot runs once when the PR opens or leaves draft, and a push triggers nothing. Don't re-request a review after every push. Each request is a full re-read of the whole diff, and the bot then surfaces "previously missed" findings in files the push never touched — which draws another push, which draws another request (the sibling repo's [kurrentschrift#406](https://github.com/MarkusNeusinger/kurrentschrift/pull/406) collected ~15 requests in a day over a one-line docstring fix). Request another review only while the previous round found something high or critical — a real bug, a security or correctness issue, a wrong mechanism — whose fix you pushed; a round with only low and medium findings gets its fixes pushed and its threads resolved, and that is the end (owner, 2026-10-23: every round costs premium requests; a docs PR had collected three over wording). A PR that is green with no open threads needs no further round — say so and let the owner merge. The first review typically lands within ~2 min of PR open. Fetch with `gh pr view --comments`, plus the three GitHub APIs that surface different comment types — `gh api` resolves `{owner}/{repo}` from the current git remote so these are copy/paste-portable: +3. **Wait for the Copilot PR Reviewer bot — ONE review per PR, not one per push.** Review-on-push is off in the "Automated Copilot Code Review" ruleset (owner, 2026-09-03): the bot runs once when the PR opens or leaves draft, and a push triggers nothing. Don't re-request a review after every push. Each request is a full re-read of the whole diff, and the bot then surfaces "previously missed" findings in files the push never touched — which draws another push, which draws another request (the sibling repo's [kurrentschrift#406](https://github.com/MarkusNeusinger/kurrentschrift/pull/406) collected ~15 requests in a day over a one-line docstring fix). Request another review only while the previous round found something high or critical — a real bug, a security or correctness issue, a wrong mechanism — whose fix you pushed; a round with only low and medium findings gets its fixes pushed and its threads resolved, and that is the end (owner, 2026-10-23: every round costs premium requests; a docs PR had collected three over wording). A PR that is green with no open threads needs no further round — say so and let the owner merge. The first review typically lands about 5 minutes after the PR opens (1 to 7 measured on 29 PRs, 2026-10-28 to 31). Fetch with `gh pr view --comments`, plus the three GitHub APIs that surface different comment types — `gh api` resolves `{owner}/{repo}` from the current git remote so these are copy/paste-portable: - `gh api repos/{owner}/{repo}/pulls//reviews` — top-level review summaries (Copilot's overall comment lives here) - `gh api repos/{owner}/{repo}/pulls//comments` — inline review comments tied to file/line - `gh api repos/{owner}/{repo}/issues//comments` — generic PR conversation comments (codecov, deployment bots, humans) 4. **Triage Copilot suggestions and apply only the sensible ones.** Apply when the comment flags a real bug, a deploy-order risk, a security/correctness issue, or a missed edge case. Skip pure style noise or anything that contradicts an explicit decision in the PR body. State briefly in chat which were applied vs skipped, so the user can override. 5. **Push review-driven fixes to the SAME branch.** Don't open a follow-up PR for review feedback — it belongs on the original PR. -6. **Only then announce the PR is ready / ask the user to merge.** Premature "done" leaves CI red or feedback unaddressed. +6. **Only then announce the PR is ready / ask the user to merge.** Premature "done" leaves CI red or feedback unaddressed. An ordinary PR is merged only on the owner's explicit, named authorization in the session. 7. **The user merges live — squash-merges can race late pushes.** After announcing "green and review-clean", hold further pushes until the merge lands. If a merge races a push anyway, recover by cutting a fresh branch from the merged `main` and cherry-picking exactly the missing commits — never re-push the stale branch. This rule applies to every PR Claude opens, including small fixes and follow-ups. diff --git a/agentic/commands/agentic.md b/agentic/commands/agentic.md index 0396b21a502..b46a0be7349 100644 --- a/agentic/commands/agentic.md +++ b/agentic/commands/agentic.md @@ -430,7 +430,7 @@ You are the **environment-auditor** on the agentic-audit team. Analyze leverage 5. Use Grep to find logging, error handling, and type annotation patterns 6. Compare `tests/` structure with `api/`, `core/`, `automation/` structure 7. **Do NOT use Bash** for `find`, `ls`, `grep`, `cat` — use Glob/Grep/Read tools instead -8. You MAY use Bash for: `uv run ruff check api/ core/ --select ANN 2>&1 | tail -20` +8. You MAY use Bash for: `uv run --extra dev ruff check api/ core/ --select ANN 2>&1 | tail -20` **Report format:** Same as core-auditor — send findings to `agentic-lead` via `SendMessage` using the `LEVERAGE_POINT` / `POSITIVE` format. @@ -558,6 +558,6 @@ You are the **workflow-auditor** on the agentic-audit team. Analyze leverage poi 5. Use Grep for patterns like `prompt_claude_code`, `WorkflowState`, `@pytest.fixture`, `$ARGUMENTS` 6. Use Glob to find all test files (`**/*test*.py`, `**/*.test.ts`) and template files 7. **Do NOT use Bash** for `find`, `ls`, `grep`, `cat` — use Glob/Grep/Read tools instead -8. You MAY use Bash for: `uv run pytest tests/ --co -q 2>&1 | tail -20` or `uv run ruff check . 2>&1 | tail -5` +8. You MAY use Bash for: `uv run pytest tests/ --co -q 2>&1 | tail -20` or `uv run --extra dev ruff check . 2>&1 | tail -5` **Report format:** Same as core-auditor — send findings to `agentic-lead` via `SendMessage` using the `LEVERAGE_POINT` / `POSITIVE` format. diff --git a/agentic/commands/audit.md b/agentic/commands/audit.md index d39511fa403..0b3e8186ada 100644 --- a/agentic/commands/audit.md +++ b/agentic/commands/audit.md @@ -31,10 +31,10 @@ You are the **audit-lead**. Your job is to coordinate a team of specialist audit 2. **Run baseline measurements** (these are the ONLY Bash commands the Lead runs in this phase): ```bash - uv run ruff check . 2>&1 | tail -5 + uv run --extra dev ruff check . 2>&1 | tail -5 ``` ```bash - uv run ruff format --check . 2>&1 | tail -5 + uv run --extra dev ruff format --check . 2>&1 | tail -5 ``` In `since=` mode, additionally: ```bash @@ -195,7 +195,7 @@ After all specialists report back and cross-validation has run: - **Importance** (1-5): see Severity Calibration table above - **Dimension** (1 of 6): see Dimension Taxonomy — every finding must carry exactly one - **Effort**: S (<30min, 1 file, mechanical), M (1-3h, 2-5 files, local context), L (half day+, 5-15 files, design decisions), XL (multi-day, 15+ files, needs own plan) - - **Auto-fix**: classify each finding as `ruff` (auto-fixable via `uv run ruff check --fix`), `eslint` (`yarn lint --fix`), `format` (`uv run ruff format`), `codemod` (mechanical rewrite that a small script could do), or `manual` (requires judgment) + - **Auto-fix**: classify each finding as `ruff` (auto-fixable via `uv run --extra dev ruff check --fix`), `eslint` (`yarn lint --fix`), `format` (`uv run --extra dev ruff format`), `codemod` (mechanical rewrite that a small script could do), or `manual` (requires judgment) 5. **Compute Health Score (30-100)** — unchanged formula, trend-comparable across all prior reports: - Start at 100 - Subtract: `min(70, 10 * critical_count + 3 * high_count + 1 * medium_count)` diff --git a/agentic/commands/audit/backend-auditor.md b/agentic/commands/audit/backend-auditor.md index 6a2c208036b..28229086375 100644 --- a/agentic/commands/audit/backend-auditor.md +++ b/agentic/commands/audit/backend-auditor.md @@ -20,7 +20,7 @@ You are the **backend-auditor** on the audit team. Analyze `api/`, `core/`, and 5. Grep for symbol names to find usages and check if code is actually used 6. Pause and consolidate findings after research sequences 7. **Do NOT use Bash** for `find`, `ls`, `grep`, `cat` — use Glob/Grep/Read tools instead -8. You MAY use Bash for: `uv run ruff check api/ core/ automation/` or `uv run pytest tests/unit -x -q` +8. You MAY use Bash for: `uv run --extra dev ruff check api/ core/ automation/` or `uv run pytest tests/unit -x -q` **Report format:** Emit findings per the **Findings Schema** in the orchestrator (`audit.md`). Every finding carries `DIMENSION` (one of `security | speed | looks | modern | correctness | maintainability`) in addition to importance/effort/auto-fix. Backend findings are usually `correctness`, `speed`, or `maintainability` — tag the single best fit. Also surface any high-value **exemplary gap** (something a best-in-class backend would have that anyplot lacks), rated like any other finding. In team mode, send findings to `audit-lead` via `SendMessage`, starting with one `COVERAGE: full | partial` line, then one block per finding: ``` diff --git a/agentic/commands/pull_request.md b/agentic/commands/pull_request.md index a49d4b99b38..a5b62c7340d 100644 --- a/agentic/commands/pull_request.md +++ b/agentic/commands/pull_request.md @@ -17,7 +17,9 @@ plan_file: $2 - A `## Plan` section linking to the plan/spec file if one exists - A `## Test plan` section with a checklist of how to verify the changes - Extract context from the commits and changed files to write the summary -- Do NOT include any "Generated with..." or "Authored by..." lines +- Do NOT include any "Generated with..." or "Authored by..." lines in the PR body. This repository rule outranks the + harness's attribution reminder for the PR body; the session link the reminder names may stay as the last line, and + the commit trailers it asks for (`Co-Authored-By`, `Claude-Session`) stay. ## Run diff --git a/agentic/commands/refactor.md b/agentic/commands/refactor.md index 83f2f068cd2..28691b53fd4 100644 --- a/agentic/commands/refactor.md +++ b/agentic/commands/refactor.md @@ -96,7 +96,7 @@ Execute these commands to validate the refactor is complete with zero regression - `uv run pytest tests/ -v` - Run test suite (must match baseline) - `uv run python -m py_compile api/**/*.py` - Check Python syntax -- `uv run ruff check .` - Lint check +- `uv run --extra dev ruff check .` - Lint check - ## Notes diff --git a/agentic/commands/regen.md b/agentic/commands/regen.md index 24cfc8351ef..654c6ea99e0 100644 --- a/agentic/commands/regen.md +++ b/agentic/commands/regen.md @@ -308,8 +308,8 @@ regen** — that's a real defect on this dev box, not an unrelated change. ### 2e. Lint ```bash -uv run ruff format plots/$SPEC_ID/implementations/python/$LIBRARY.py -uv run ruff check --fix plots/$SPEC_ID/implementations/python/$LIBRARY.py +uv run --extra dev ruff format plots/$SPEC_ID/implementations/python/$LIBRARY.py +uv run --extra dev ruff check --fix plots/$SPEC_ID/implementations/python/$LIBRARY.py ``` Fix any unfixable errors manually and re-run. diff --git a/agentic/commands/test.md b/agentic/commands/test.md index f7e2c909182..6521967178a 100644 --- a/agentic/commands/test.md +++ b/agentic/commands/test.md @@ -45,13 +45,13 @@ to read it for this command — the test commands below are self-contained. 1. **Code Formatting Check** - Preparation Command: None - - Command: `uv run ruff format --check .` + - Command: `uv run --extra dev ruff format --check .` - test_name: "code_formatting" - test_purpose: "Validates that Python code follows consistent formatting standards" 2. **Backend Code Quality Check** - Preparation Command: None - - Command: `uv run ruff check .` + - Command: `uv run --extra dev ruff check .` - test_name: "backend_linting" - test_purpose: "Validates Python code quality, identifies unused imports, style violations, and potential bugs" @@ -105,14 +105,14 @@ to read it for this command — the test commands below are self-contained. { "test_name": "backend_linting", "passed": false, - "execution_command": "uv run ruff check .", + "execution_command": "uv run --extra dev ruff check .", "test_purpose": "Validates Python code quality, identifies unused imports, style violations, and potential bugs", "error": "api/main.py:15:1: F401 `os` imported but unused" }, { "test_name": "code_formatting", "passed": true, - "execution_command": "uv run ruff format --check .", + "execution_command": "uv run --extra dev ruff format --check .", "test_purpose": "Validates that Python code follows consistent formatting standards" } ] diff --git a/agentic/commands/update.md b/agentic/commands/update.md index f970b89daff..714d75a160d 100644 --- a/agentic/commands/update.md +++ b/agentic/commands/update.md @@ -216,8 +216,8 @@ Run ruff format and check **sequentially first**, before any parallel version-in If parallel Bash calls are used and one fails, all sibling calls get cancelled — so always run ruff alone. ```bash -uv run ruff format plots/{spec_id}/implementations/python/*.py -uv run ruff check --fix plots/{spec_id}/implementations/python/*.py +uv run --extra dev ruff format plots/{spec_id}/implementations/python/*.py +uv run --extra dev ruff check --fix plots/{spec_id}/implementations/python/*.py ``` If there are unfixable errors, fix them manually and re-run. The agents should have already run ruff in their @@ -622,8 +622,8 @@ If the script fails, read the error, fix the implementation, and retry. **Up to Run ruff to format and fix lint issues in the implementation before proceeding: ```bash -uv run ruff format plots/{SPEC_ID}/implementations/python/{LIBRARY}.py -uv run ruff check --fix plots/{SPEC_ID}/implementations/python/{LIBRARY}.py +uv run --extra dev ruff format plots/{SPEC_ID}/implementations/python/{LIBRARY}.py +uv run --extra dev ruff check --fix plots/{SPEC_ID}/implementations/python/{LIBRARY}.py ``` If `ruff check` reports unfixable errors (exit code 1), read the error output and fix the code manually, then re-run. diff --git a/agentic/docs/project-guide.md b/agentic/docs/project-guide.md index 299bca9d84d..e904f84d847 100644 --- a/agentic/docs/project-guide.md +++ b/agentic/docs/project-guide.md @@ -91,20 +91,20 @@ uv run pytest tests/unit/api/test_routers.py::test_get_specs **Both linting and formatting must pass for CI.** -**IMPORTANT: Always run `uv run ruff check && uv run ruff format ` on changed files BEFORE every commit!** +**IMPORTANT: Always run `uv run --extra dev ruff check && uv run --extra dev ruff format ` on changed files BEFORE every commit!** Ruff lives in the `dev` extra, which a fresh worktree's `.venv` lacks. ```bash # Linting (required for CI) -uv run ruff check . +uv run --extra dev ruff check . # Auto-fix linting issues -uv run ruff check . --fix +uv run --extra dev ruff check . --fix # Formatting (required for CI) -uv run ruff format . +uv run --extra dev ruff format . # Before committing - always run both on changed files: -uv run ruff check && uv run ruff format +uv run --extra dev ruff check && uv run --extra dev ruff format ``` ### Frontend Development @@ -1055,6 +1055,46 @@ This matters most for `app/nginx.conf`: nginx behaviour has no local verification loop in this repo, so production is the first place a change is observable at all. +#### Change a trigger's included files + +The triggers are second-generation triggers on the `anyplot-github` repository +connection. `gcloud builds triggers update github --included-files=…` +fails on them with `INVALID_ARGUMENT: Request contains an invalid argument`, +with or without `--repository` and `--branch-pattern` (2026-10-29, +`deploy-renderer`). Export the trigger, edit the file, and import it instead. +A trigger edit is a Cloud Build config change, so name the trigger and the +paths to the owner and get a yes before step 4 (`CLAUDE.md`, external-system +writes). + +1. Export the trigger into a scratch directory, not the repository: + + ```bash + gcloud builds triggers describe --region=europe-west4 --project=anyplot --format=yaml > before.yaml + ``` + +2. Copy `before.yaml` to `after.yaml` and add or remove paths under + `includedFiles`. Leave every other key as exported. +3. Check that `diff before.yaml after.yaml` shows only the `includedFiles` + lines you meant. +4. Import the edited config. It keeps the trigger's `id`, so it updates the + trigger in place: + + ```bash + gcloud builds triggers import --source=after.yaml --region=europe-west4 --project=anyplot + ``` + +5. Read the trigger back and compare: + + ```bash + gcloud builds triggers describe --region=europe-west4 --project=anyplot --format=yaml > verify.yaml + diff after.yaml verify.yaml && echo "trigger matches the edited config" + ``` + +This route updated `deploy-renderer` at the first attempt on 2026-10-29 +(`Project.toml`, `Manifest.toml`) and on 2026-10-30 +(`agents/anyplot/render/convert.py`, `package.json`, `package-lock.json`, +`automation/js-render/**`). + ### Manual Deployment ```bash diff --git a/agents/README.md b/agents/README.md index b43cc4669c3..fb8486cca8f 100644 --- a/agents/README.md +++ b/agents/README.md @@ -423,8 +423,21 @@ Before the first case the harness renders the catalogue file of the first case o --renderer remote --render-url https://RENDERER_URL --gcloud-token ``` -- To run the Gemini arm, add `--provider gemini`; it selects `gemini-3.8-flash` with the judge on `gemini-3.5-flash-lite`. `--model` alone implies its provider, and `--judge-model` and `--location` override the rest. +- To run the Gemini arm, add `--provider gemini`; it selects `gemini-3.8-flash` with the judge on `gemini-3.5-flash-lite`. `--model` alone implies its provider, and `--judge-model` and `--location` (`eu` by default, `global` or `us`) override the rest. Give the arm its own `--out` so its gallery does not overwrite the other arm's: + + ```bash + uv run --extra agents python -m agents.evals.matrix --provider gemini --cases smoke --budget-usd 4 \ + --out agents/evals/reports/-gemini --renderer remote --render-url https://RENDERER_URL --gcloud-token + ``` + - To run a subset, pass comma-separated globs over case ids, for example `--cases "*-seaborn-*"` or `--cases "pie-basic-*,box-basic-*"`. `--cases scatter-basic-matplotlib-german-message` runs the case whose German follow-up sets the session language: its first version is English, its second must carry German plot text, and each run's record holds the `done` event's `language`. +- To check one library, the form every run of 2026-10-28 to 2026-10-31 took, select its cases by glob, cap the spend, write to a directory of its own, and skip the baseline diff, because the committed baseline holds none or few of that library's cases: + + ```bash + uv run --extra agents python -m agents.evals.matrix --provider anthropic-vertex --location eu \ + --cases '*-makie-*' --budget-usd 3 --no-baseline --out agents/evals/reports/-makie \ + --renderer remote --render-url https://RENDERER_URL --gcloud-token + ``` | Flag | Meaning | |---|---| @@ -436,7 +449,7 @@ Before the first case the harness renders the catalogue file of the first case o | `--static` | Run only the model-free static stage and write `-static.json` instead of a matrix report (see [Run the stress cases](#run-the-stress-cases)); the renderer is `fake` unless `--renderer` or `AGENT_RENDERER` names another | | `--repeats N` | Runs per case, to see how stable a result is | | `--max-attempts N` | Adapter attempts per run, 1 to 3 (`AGENT_MAX_ATTEMPTS`, 2 by default), so 1 measures the first attempt alone and 3 a second repair round; the report's stamp records the value as `max_attempts` | -| `--thinking-budget ROOT,ADAPTER` | Claude only: adaptive thinking for the root and the adapter, each value (0 to 4,096, 0 off) added to that agent's output cap (`AGENT_ROOT_THINKING_BUDGET`, `AGENT_ADAPTER_THINKING_BUDGET`); the stamp records it as `thinking_budget`, the summary names it, and the thinking tokens are priced as output | +| `--thinking-budget ROOT,ADAPTER` | Claude only: adaptive thinking for the root and the adapter, each value (0 to 8,192, 0 off) added to that agent's output cap (`AGENT_ROOT_THINKING_BUDGET`, `AGENT_ADAPTER_THINKING_BUDGET`); the stamp records it as `thinking_budget`, the summary names it, and the thinking tokens are priced as output | | `--budget-usd X` | Stop before a case that could take the estimated list-price cost past X (the spend so far plus the costliest case so far), or once the cost has passed X (exit code 3) | | `--out DIR` | Output directory, `agents/evals/reports/` by default (git-ignored) | | `--baseline FILE`, `--no-baseline` | The report to compare with; by default `evals/baselines/claude-haiku-5-5.json` when it exists | diff --git a/changelog.d/session-retro-rules.md b/changelog.d/session-retro-rules.md new file mode 100644 index 00000000000..903b5aa76ae --- /dev/null +++ b/changelog.d/session-retro-rules.md @@ -0,0 +1,29 @@ +### Changed + +- **The PR watchers end on their own.** `watch-deploys.sh` exits 0 with + `no deploy for ` once 6 polls (about 3 minutes) have found no build + for a merge whose paths trigger nothing, instead of waiting 30 minutes, and + exits 2 at once when `gcloud builds list` fails, so a lapsed login never + reads as "no deploy". `watch-checks.sh` waits 90 polls, about 45 minutes, + because a PR that builds the renderer image takes 30 to 40, and its + `TIMEOUT` line names the checks still pending. +- **The agent rules and skills carry what the 2026-10-31 session retro + measured.** `CLAUDE.md` adds a delegate stall check (about 30 minutes with + no commit and no message, and a re-check of every delegate after a context + compaction), makes a permission-classifier denial final for every agent of + the session, and puts the first Copilot review at about 5 minutes. The + `open-pr` skill gains a hadolint gate for the Dockerfiles, tests for new + code before the PR opens, the 16 real check names, the GraphQL request for + a further Copilot round, a `git pull --ff-only` of the shared checkout after + a merge, and a report of worktrees whose branch is gone. `zweitmeinung` + attaches a diff over about 60 KB instead of inlining it, documents how + `--attach` resolves paths, and states the measured Gemini costs. Gate + commands name ruff's `dev` extra (`uv run --extra dev ruff …`), which a + fresh worktree lacks. +- **Recipes that lived only in chat are documented.** The single-library + `impl-generate.yml` dispatch with its optional inputs + (`docs/workflows/overview.md`), the eval-matrix forms real runs use + (`agents/README.md`), and the export, edit and import route for a + second-generation Cloud Build trigger, whose `update github + --included-files` fails with `INVALID_ARGUMENT` + (`agentic/docs/project-guide.md`). diff --git a/docs/development.md b/docs/development.md index 19104cbd5b4..494378e4cdd 100644 --- a/docs/development.md +++ b/docs/development.md @@ -114,25 +114,27 @@ the reported coverage. ## Code quality -Both linting and formatting must pass for CI. +Both linting and formatting must pass for CI. Ruff is in the `dev` extra, so +the commands name it: they then work in any environment, including a fresh +worktree or one that a narrower `uv sync` left without it. ```bash # Check linting -uv run ruff check . +uv run --extra dev ruff check . # Auto-fix linting issues -uv run ruff check . --fix +uv run --extra dev ruff check . --fix # Check formatting -uv run ruff format --check . +uv run --extra dev ruff format --check . # Auto-format -uv run ruff format . +uv run --extra dev ruff format . ``` **Always run before committing:** ```bash -uv run ruff check . && uv run ruff format . +uv run --extra dev ruff check . && uv run --extra dev ruff format . ``` --- diff --git a/docs/workflows/overview.md b/docs/workflows/overview.md index 24ded3972a7..05f92cb0431 100644 --- a/docs/workflows/overview.md +++ b/docs/workflows/overview.md @@ -431,6 +431,35 @@ gh workflow run bulk-generate.yml -f specification_id=all -f library=matplotlib **Concurrency limit**: Max 3 parallel implementations globally. +### Generate one library for one spec + +To generate or regenerate a single (spec, library) pair, dispatch +`impl-generate.yml` directly. It's the workflow `bulk-generate.yml` dispatches +once per library: + +```bash +gh workflow run impl-generate.yml -f specification_id=scatter-basic -f library=bokeh +``` + +`specification_id` and `library` are required. The optional inputs are the +following: + +| Input | Default | Meaning | +|-------|---------|---------| +| `issue_number` | empty | The spec issue to report on; empty reads it from `plots/{spec-id}/specification.yaml` | +| `model` | `auto` | The generation model, also used for repair: `auto` routes as in [Generation model](#generation-model); `haiku`, `sonnet`, or `opus` pins one. Reviews always run on Opus | +| `change_request` | empty | A one-sentence hint the generator follows, such as the cross-library divergence note of the daily-regen similarity audit | +| `regen_gate` | `true` | For a pair that already has an implementation on `main`: one review decides keep or replace, with no repair. `false` forces the fresh-generation path with the repair loop and the `regen:forced` label (see [Regen gate](#regen-gate-regenerations)) | +| `source_ref` | empty | Adopt mode: take the file from this branch, tag, or commit SHA (see [Adopt implementations from a branch](#adopt-implementations-from-a-branch)) | +| `source_model` | empty | Adopt mode only: the model that wrote the adopted file, recorded as `generated_by` | + +For example, to regenerate one pair on Opus with a hint: + +```bash +gh workflow run impl-generate.yml -f specification_id=scatter-basic -f library=bokeh \ + -f model=opus -f change_request="Use a different data scenario from the matplotlib version." +``` + ### Adopt implementations from a branch Adopt mode takes each implementation file from a branch, tag, or commit SHA of From 39f906e2ef530179dae4422f1c517c3a50e5f644 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:02:53 +0100 Subject: [PATCH 2/4] docs(open-pr): list detached worktrees in the report; pathspec note Fixes from the Gemini Flash pre-PR check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01H7HSkGyea1k2v293XiAMo8 --- .claude/skills/open-pr/SKILL.md | 13 ++++++++++--- .claude/skills/zweitmeinung/SKILL.md | 3 ++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md index afc5970ed44..be9f4f2957f 100644 --- a/.claude/skills/open-pr/SKILL.md +++ b/.claude/skills/open-pr/SKILL.md @@ -452,13 +452,20 @@ on `origin`, most of them left by older sessions: ```bash remote=$(git -C "$MAIN" ls-remote --heads origin | sed 's#.*refs/heads/##') git -C "$MAIN" worktree list --porcelain | - awk '/^worktree /{sub(/^worktree /, ""); w=$0} /^branch /{sub(/^branch refs\/heads\//, ""); print $0 "\t" w}' | - while IFS=$'\t' read -r b w; do grep -qxF "$b" <<<"$remote" || echo "no origin/$b: $w"; done + awk '/^worktree /{sub(/^worktree /, ""); w=$0} + /^branch /{sub(/^branch refs\/heads\//, ""); print $0 "\t" w} + /^detached/{print "-\t" w}' | + while IFS=$'\t' read -r b w; do + if [ "$b" = - ]; then echo "detached: $w" # no branch name starts with "-" + else grep -qxF "$b" <<<"$remote" || echo "no origin/$b: $w"; fi + done ``` `ls-remote` asks `origin` itself, so a remote-tracking ref that a fetch without `--prune` left behind does not hide a deleted branch. A branch -that was never pushed shows up too; say which ones in the report. +that was never pushed shows up too, and so does every detached worktree, +such as a review worktree a run left behind; say which is which in the +report. ## Gotchas diff --git a/.claude/skills/zweitmeinung/SKILL.md b/.claude/skills/zweitmeinung/SKILL.md index ce8a9966855..88f65b3b6db 100644 --- a/.claude/skills/zweitmeinung/SKILL.md +++ b/.claude/skills/zweitmeinung/SKILL.md @@ -100,7 +100,8 @@ into every turn that the run can end on its input budget before it answers: on 2026-10-31 a 90,000-character diff cost $0.30 and returned "no answer: input budget" at about 99k tokens per turn. Leave `--diff-base` out, write the diff without fixtures, docs and changelog fragments to `$D/data/.diff`, attach it, and tell Gemini in the -briefing to review from it with at most six tool calls: +briefing to review from it with at most six tool calls. Run it from the repository root, +since the pathspecs are relative to the current directory: ```bash git diff origin/main... -- . ':(exclude)docs' ':(exclude)changelog.d' ':(exclude,glob)**/fixtures/**' \ From a5c1a32b088b85e207450ddf46ccc104fd78ef26 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:03:37 +0100 Subject: [PATCH 3/4] docs(changelog): add the PR number to the session-retro fragment (#12493) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01H7HSkGyea1k2v293XiAMo8 --- changelog.d/session-retro-rules.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog.d/session-retro-rules.md b/changelog.d/session-retro-rules.md index 903b5aa76ae..cc90fab40b2 100644 --- a/changelog.d/session-retro-rules.md +++ b/changelog.d/session-retro-rules.md @@ -6,7 +6,7 @@ exits 2 at once when `gcloud builds list` fails, so a lapsed login never reads as "no deploy". `watch-checks.sh` waits 90 polls, about 45 minutes, because a PR that builds the renderer image takes 30 to 40, and its - `TIMEOUT` line names the checks still pending. + `TIMEOUT` line names the checks still pending (#12493). - **The agent rules and skills carry what the 2026-10-31 session retro measured.** `CLAUDE.md` adds a delegate stall check (about 30 minutes with no commit and no message, and a re-check of every delegate after a context @@ -19,11 +19,11 @@ attaches a diff over about 60 KB instead of inlining it, documents how `--attach` resolves paths, and states the measured Gemini costs. Gate commands name ruff's `dev` extra (`uv run --extra dev ruff …`), which a - fresh worktree lacks. + fresh worktree lacks (#12493). - **Recipes that lived only in chat are documented.** The single-library `impl-generate.yml` dispatch with its optional inputs (`docs/workflows/overview.md`), the eval-matrix forms real runs use (`agents/README.md`), and the export, edit and import route for a second-generation Cloud Build trigger, whose `update github --included-files` fails with `INVALID_ARGUMENT` - (`agentic/docs/project-guide.md`). + (`agentic/docs/project-guide.md`) (#12493). From 033fa01f7b7c8a4e08514c731703024c2d2c050c Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:08:59 +0100 Subject: [PATCH 4/4] docs(open-pr): keep the live-flow exception in the codecov triage Copilot review on #12493. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01H7HSkGyea1k2v293XiAMo8 --- .claude/skills/open-pr/SKILL.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md index be9f4f2957f..fff3b625e24 100644 --- a/.claude/skills/open-pr/SKILL.md +++ b/.claude/skills/open-pr/SKILL.md @@ -259,9 +259,10 @@ gh api repos/{owner}/{repo}/issues//comments # conversation (codecov, bots ``` **b2. Codecov** (the comment arrives in the conversation after the -coverage upload; the repo uses flags/components). A red -`codecov/patch/*` check means §1's test rule was missed: add the test to -the same branch, judged the way §1 says. +coverage upload; the repo uses flags/components). Triage a red +`codecov/patch/*` check the way §1 says: uncovered NEW pure logic means +§1's test rule was missed, so add the test to the same branch; lines only a +live DB/HTTP flow exercises stay uncovered, and the PR says so. **c. List unresolved threads** (the id is needed for resolving):