[Peeja]

Written by Claude.

Part of FIL-1145.

Follow-up to #273, which corrected the alert comments' account of the telemetry labels but left two kinds of claim behind.

The Scrapes panel on the Regions dashboard still carried the unqualified sentence #273 removed from alerts.tf — "two boxes in one stage and region share service_name and are told apart by node" — in the one place a person actually reads it. It now says what the panel can and cannot show: two boxes in one stage and region share service_name, and no label there would separate them, since the only label telling those two rows apart is node and every node value today names a stage and at most a region.

The comments asserted that node is the label meant to name a box. They no longer name the label that will close the gap, only the gap. Which label should be per-box is still being settled — infra-nodes' design doc and runbook call node the fleet's per-box identity, while the migration work treats it as a choice between node and appliance — and nothing here depends on the answer: every appliance rule but the node-label one groups by appliance, region and node together, so whichever one gains a per-box value splits the groups with no edit here.

That also covers the node-label rule's description, which an on-call reads and which called node "the label every per-box rule needs".

One unrelated correction rides along: the same rule's comment said node's value is "per-region today", which is untrue of dev, whose node is plain dev. It now says what both have in common — the value does not name a machine.

Verification

• scripts/normalise-dashboard.sh --check over both dashboards and scripts/check-stage-picker.sh pass. tofu fmt -check and shellcheck are in make check but neither tool is installed in this environment, and scripts/check-alert-routing.sh needs a plan JSON and runs only in plan-grafana.
• No rule, query, threshold or grouping changes, and no panel query changes. The only annotation change is the wording of the node-label rule's description. The dashboard edit is one string inside an existing description, so the normaliser has nothing to reformat.
• A grep over terraform/ and docs/ for the superseded phrasings comes back empty.

Reviewed under pr-review-rounds; the rounds are in the comments.

✴️

[github-actions]

Previews for this pull request — in Dashboard PR previews, deleted when it closes. Read-only, and tagged preview.

• Forge Regions — preview · current

[Peeja]

Claude reviewer says… (first round, 16e7940)

Clean. Both statements are true of the code, the rewrap is faithful, nothing stale survives, and every description claim checks out. Two nits, no more.

Verified:

• The grep claim is exact, and wider than it was asked to be: git grep "told apart by" with no pathspec returns two hits at d0da888 and one at 16e7940, the survivor being the already-qualified alerts.tf:1038. (docs/observability.md:83's "told apart" is about DatasourceError and stage, unrelated.)
• The panel sentence is true. Only two node values exist — dev, which names a stage and no region, and staging/eu-central-3, a stage and a region — so "names a stage and at most a region" is right, and matches the already-merged wording at alerts.tf:2535.
• "Nothing here would separate them" is right for this panel specifically: its two queries are up{service_name=~"appliance-.*-(host|caddy)", …} with legends host @ {{node}} and caddy @ {{node}}, so node is the only distinguishing label displayed, and two boxes in one stage and region would share service_name, appliance, region, node and instance. The "at most a region" hedge is load-bearing rather than decorative — the stage picker includes dev, so node=dev really does appear here.
• The alerts.tf wording is true of both nodes, where "per-region today" was false of dev, and the paragraph's argument is unchanged: the rule is about the label going missing, and the value being wrong stays explicitly parked.
• The rewrap is faithful. Reflowing alerts.tf:1605-1617 at both revisions into a single word stream and diffing gives exactly one change, is per-region today → does not name a machine. Every rewrapped line is 66-77 columns.
• No contradictions elsewhere: forge-central.json carries no node-bearing description, docs/observability.md mentions node only as a routing and grouping label, and the sibling Containers panel says nothing about node identity.
• Both local scripts pass; CI green on 16e7940 including check, deploy and plan-grafana. tofu fmt, shellcheck and check-alert-routing.sh were not run in the review environment.

Findings

1 — nit. alerts.tf:1612. "That node's value does not name a machine is a different problem…" puts a noun phrase immediately before the main verb, so "a machine is a different problem" garden-paths on first read. Not a regression — the phrasing it replaced had the same shape.

2 — nit, recommended against. forge-regions.json:1721. The replacement adds about twenty words to an already long tooltip. A shorter form would carry the same content, but the reviewer's own recommendation is to keep it as written: at a dashboard, "names a stage and at most a region" tells the on-call what they are looking at, where "names no box" would not.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude author says…

4f0224d.

1 — fixed. Now "No node value names a machine today, but that is a separate problem from the label going missing, and not the one this rule is about." Statement first, comment on it second, so there is no noun-clause subject to garden-path through.

2 — kept as written, per the finding's own recommendation. The length is real, but the explicit form is what makes the host @ staging/eu-central-3 row labels on that panel legible to someone who has not read alerts.tf, and that is the audience for a tooltip.

I checked the two load-bearing claims myself rather than taking them on trust. The whole-repo grep returns only alerts.tf:1038, the qualified copy. And the rewrap is provably faithful: reflowing the paragraph at both revisions into one word stream and diffing gives exactly is per-region today → does not name a machine, nothing else moved.

Checks on 4f0224d: scripts/normalise-dashboard.sh --check over both dashboards and scripts/check-stage-picker.sh pass; no line over 79 columns in the rewrapped paragraph. tofu fmt, shellcheck and check-alert-routing.sh run in CI.

This round found only nits, so there won't be another.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude reviewer says… (second round, e8ec508)

Blocking. The neutral rewrite mostly lands, but it introduces a false claim in a comment, and the description's "no annotation changes" bullet went stale on this very push. Three smaller findings besides.

Verified: every assertion that node is the per-box label is gone from terraform/ and docs/, including the node-label rule's user-visible description; the grep claim holds; forge-regions.json is untouched this round and forge-central.json, routing.tf, docs/observability.md and the Makefile carry nothing contradicting; the unless and up == bool 0 sentences are unchanged and still correct. "The grouping separates boxes as soon as any of the three carries a per-box value, with no edit here" is true — all four aggregations in that rule are max by (appliance, region, node) and the gate is a set operator, so a per-box value on any of the three splits the groups and the gate still matches. Both local scripts pass; CI green on e8ec508.

Findings

1 — blocking. PR description. "No rule, query, threshold, annotation or grouping changes" is false of this push: it rewrites annotations.description on Appliance telemetry is missing its node label. The body says so three paragraphs earlier. The bullet was true at round 1's head and went stale here, and it becomes the squash-merge message.

2 — blocking. alerts.tf:1003-1004. "node is the finest grouping these rules carry" is false three ways. The rule it annotates groups min by (appliance, region, node, mountpoint). Nine rules in the same group now group by instance, which is <node>/piri-<n> and strictly finer. And on dev appliance is dev-us-east-9 while FILONE_NODE=dev, so there appliance is the finer of the two. A fresh over-claim of exactly the kind this PR exists to delete.

3 — should-fix. alerts.tf:1002-1004. Removing that sentence leaves the comment opening "Grouped by node as well as region:" and then giving a fact that argues against including node. The sibling comment solves the same problem with a forward-looking clause; reuse it.

4 — should-fix. alerts.tf:2945-2946. "every value names a stage and at most a region" is offered after a colon as the proof that none of the three separates two boxes — but region's value is eu-central-3, a region and no stage, so the proof doesn't cover one of the three.

5 — nit. alerts.tf:1571-1572. "Every rule in this file that groups finer than the region needs node, since appliance is <stage>-<region> and goes no finer" implies node is finer than appliance, which the line four below then denies and which finding 2 shows is backwards on dev. The rule's real justification — the convention lives in an out-of-repo config and gets dropped on re-implementation — survives intact, so the rule still reads as worth having.

6 — nit, speculative and not applied. "Which label should name a box is still being settled" gives a reader no pointer to what the documents say today, and infra-nodes is unambiguous: the design doc's Telemetry section, docs/observability.md and nodes/dev/node.env all name node. Someone reading those may take alerts.tf for out of date rather than deliberately neutral. Reporting that state arguably isn't taking a position — but neutrality here was a deliberate instruction, so this is for the author to decide rather than for review to impose.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude author says…

Fixes in bcf96f4; description corrected.

1 — fixed. The bullet now reads "No rule, query, threshold or grouping changes, and no panel query changes. The only annotation change is the wording of the node-label rule's description."

2 — fixed, and it was an own-goal. All three counts check out: the disk rule groups min by (appliance, region, node, mountpoint); nine rules now group by instance since #270 merged; and nodes/dev/platform/compose.yml sets APPLIANCE_LABEL=${STAGE}-${REGION_LABEL} → dev-us-east-9 against FILONE_NODE=dev, so on dev appliance really is the finer label. I removed a set of over-claims and wrote a new one in the same push.

3 — fixed with the clause the finding suggested, which is the one the container rule already uses: grouping by all three means the groups split as soon as one of them carries a per-box value, with no edit here. That explains the grouping without predicting which label does it.

4 — fixed. "none of their values names more than a stage and a region", which covers region as well as the other two.

5 — fixed. The opening sentence no longer orders the labels by granularity. It now says what actually follows from the label being absent: the scrape lands in a group of its own and joins to nothing that has it. The out-of-repo-convention justification below it is untouched.

6 — not applied, raised with the author. The finding is right that a reader coming from infra-nodes' docs may read this as stale rather than deliberately neutral. But neutrality here was an explicit instruction, and re-introducing "the docs name node" is close to the sentence that instruction removed. Left as is pending the author's call.

While fixing 4 I rewrapped the paragraph and dropped the `unless` from "the unless in the second branch below" — the exact reference round 1's finding 5 asked for. Caught and restored before committing.

Checks on bcf96f4: both local scripts pass, no comment line over 79 columns, and the only non-comment line in the diff against main is the one description string.

Round 2 was blocking, so a round 3 follows on the new head.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude reviewer says… (third round, bcf96f4)

Blocking. Four of round 2's five actionable findings are genuinely fixed, but the replacement for one states something the very rule it annotates disproves, and the replacement for another was copied without the antecedent that made it true.

Verified: against the merge base, alerts.tf changes comment lines plus exactly one non-comment line, and forge-regions.json one string inside one panel description — so the corrected "only annotation change" bullet now matches. The grep claim is empty as described. "None of their values names more than a stage and a region" holds for all three (appliance = dev-us-east-9, region = eu-central-3, node = dev / staging/eu-central-3). The join claim holds: node="" matches nothing on the and/unless on (…, instance) joins, the - on (…) group_right(), or the and on (node) gate. The disk-rule split claim is true of that rule's actual aggregation. The `unless` reference is present and points at the right branch. No contradictions in forge-central.json, the other panel descriptions, docs/observability.md, routing.tf or the Makefile, and the cross-reference at alerts.tf:1675 still resolves. Both local scripts pass.

Findings

1 — blocking. alerts.tf:1572. "Every appliance rule groups by node" is false, and the counterexample is the rule the comment is attached to: Appliance telemetry is missing its node label groups count by (appliance, job, service_name) and holds node out deliberately, since it selects node="". Round 2's finding 5 replaced an ordering over-claim with a universal one — the same shape of defect this PR exists to remove.

2 — should-fix. alerts.tf:1003. "Grouping by all three" has no antecedent. The preceding sentence names node, region and service_name; the rule groups min by (appliance, region, node, mountpoint). service_name is not in the grouping at all, so a reader resolving "all three" to the labels on offer concludes the rule splits when service_name goes per-box, which it does not. The clause was lifted from the container comment, which establishes its three first; the copy didn't carry that over.

3 — nit. alerts.tf:1605, user-visible description. "the node label these rules group by" has the same unanchored referent, and the on-call reading it is looking at the one appliance rule that doesn't group by node. Vague rather than false, since "these rules" most naturally means the Forge Regions rules at large.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude author says…

All three held. Fixes in eb038e8.

1 — fixed, and I checked the replacement rather than assuming it. "Every other appliance rule groups by node" is true, verified across grafana_rule_group.appliance, .appliance_containers and .appliance_outages: every rule but this one carries node in its outer grouping. Three aggregations omit it and none is a counterexample — Appliance 5xx rate too high and Appliance site is failing most requests group by (appliance, region, node, host) and use by (code, handler) only as an inner term, and the container rule's max by (appliance) is the gate's right-hand side, which its own comment explains. I also added the clause the finding implied was missing: this rule is the exception because it selects node="" to find exactly those scrapes, which the comment should have said regardless.

2 — fixed by naming the labels rather than counting them: "Grouping by appliance, region and node together means the groups split as soon as one of them carries a per-box value."

3 — fixed in the same edit: "without the node label the other appliance rules group by".

Both paragraphs were rewrapped whole rather than patched. Two rounds running, a piecemeal rewrap of mine has either left a ragged line or silently dropped a word, so that stops being a method.

Checks on eb038e8: both local scripts pass, no comment line over 79 columns, and the only non-comment line in the diff against main is the one description string.

A pattern worth naming, since three rounds have now found the same shape: every blocking finding on this PR has been a quantifier or a pronoun — "the label meant to", "the finest grouping", "every value", "all three", "every appliance rule", "these rules". The content has been right each time; the scope attached to it has not. I ran a sweep for that class over the changed lines before this push, which is how the "every other" claim got verified rather than asserted.

Round 3 was blocking, so a round 4 follows on the new head.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude reviewer says… (fourth round, eb038e8)

No blocking findings. All three of round 3's are fixed correctly, including the load-bearing one, which this round re-enumerated independently. One should-fix in the description, three nits.

Verified independently: of the 22 rules across grafana_rule_group.appliance, .appliance_containers and .appliance_outages, all carry node in their outer grouping except Appliance telemetry is missing its node label — so "every other appliance rule" is exactly right. Only two aggregations anywhere in those groups omit node: that rule's own count by (appliance, job, service_name) and the container gate's max by (appliance). The sibling rule pairs and on (node) with max by (node), so it isn't a counterexample. "This rule is the exception: it selects node=\"\" to find those scrapes" matches the query. The disk-rule antecedent fix names all three labels the rule actually groups by, and the description fix is true of all 21 others. A word-stream diff of both rewrapped paragraphs is pure insertion — nothing dropped or reordered, which is the failure mode of the two rounds before. Longest added comment line is 79. Both local scripts pass.

Findings

1 — should-fix. PR description. "…and these rules behave the same either way, since they group by appliance, region and node together". "these rules" has no antecedent; the nearest resolvable referent is the rules the changed comments annotate, and the next paragraph names one of them — Appliance telemetry is missing its node label, which groups count by (appliance, job, service_name). The claim is true of all of them; the reason given fails for one. This text becomes the squash-merge message.

2 — nit. alerts.tf:1572-1573. "a scrape arriving without it lands in a group of its own" — every node-less scrape on one appliance lands in the same node="" group, not one each.

3 — nit. forge-regions.json:1721. "no label here would separate them: every node value today names a stage and at most a region" — the claim ranges over labels, the proof covers only node. True as written, since appliance, region, instance, service_name and job would all collide too, but the proof is narrower than the claim. This is the defect round 2's finding 4 fixed in alerts.tf and not here.

4 — nit, not in merged text; a correction to the author's round-3 reply. That reply said three aggregations omit node, citing by (code, handler) in the two 5xx rules. Those strings are in the rules' description annotations — an ad-hoc query suggested to the on-call — not in either rule's query. Only two aggregations omit it. The conclusion is unaffected, but the reply offered that accounting as its verification.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude author says…

All four held. Code fixes in c0fc254; description updated.

1 — fixed. The description now says "the rules that read those series group by appliance, region and node together", which is true of the rules the claim is about and doesn't sweep in the one rule that deliberately groups otherwise.

2 — fixed. "lands in a group apart from the labelled ones" — all node-less scrapes on an appliance share the node="" group rather than getting one each.

3 — fixed. The panel now names the label before giving the fact about its values: "node is the only label those rows are told apart by, and every node value today names a stage and at most a region." Same shape as the fix round 2 applied in alerts.tf; I'd missed carrying it across to the panel.

4 — correct, and my error. Verified: both by (code, handler) occurrences are inside description annotations, an ad-hoc query suggested to the on-call, not in either rule's query. Only two aggregations omit node. The conclusion stood, but I offered that accounting as the verification for it, and the accounting was wrong — my script scanned whole rule blocks rather than just the expr fields.

While rewrapping I hit a new way to damage a paragraph: textwrap split "re-implemented" across lines at the hyphen. Caught and re-wrapped with break_on_hyphens=False. That is the third distinct way a rewrap of mine has gone wrong on this PR, which is why each one now gets re-read rather than assumed.

Checks on c0fc254: both local scripts pass, no comment line over 79 columns, and the only non-comment line in the diff against main is the one description string.

Round 4 had a should-fix, so a round 5 follows on the new head. Worth saying plainly, though: four rounds in, the findings have converged on one family — a quantifier or a pronoun whose scope doesn't match what it ranges over — and none since round 2 has touched a fact about the system. If round 5 lands in the same family again, this is ready to merge on the author's call rather than worth a round 6.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude reviewer says… (fifth round, c0fc254)

No blocking findings. The three code and panel replacements are accurate — every quantifier in them resolves correctly against the code — but the same over-broad-universal defect has migrated into the PR description, where round 4's fix repaired the pronoun and left the counterexample standing. One should-fix, three nits.

Verified independently: of the 22 rules carrying component = "appliance" across the three appliance groups, all 21 others put node in their alerting query's grouping; the node-label rule alone groups by (appliance, job, service_name), and the two stray by (code, handler) clauses are example PromQL inside annotation prose. "This rule is the exception: it selects node=\"\"" matches the query, and "lands in a group apart from the labelled ones" no longer claims a group per scrape. The disk rule is min by (appliance, region, node, mountpoint) with instant = true, so its three claims hold. On the panel, both queries use legendFormat host @ {{node}} and caddy @ {{node}}, and Alloy sets instance to FILONE_NODE for the host and Caddy scrapes, so instance would not separate two boxes either. No word split in the rewraps, nothing dropped or reordered, longest changed line 79. Both local scripts pass.

Findings

1 — should-fix. PR description. "nothing here depends on the answer: the rules that read those series group by appliance, region and node together". Appliance telemetry is missing its node label reads exactly those series — up{appliance=~…, node="", …} — and groups by (appliance, job, service_name). It is the one rule of the 22 that doesn't, and it is a rule this PR edits. Round 4 flagged this sentence; the fix changed "these rules" to "the rules that read those series", repairing the dangling demonstrative and leaving the universal with its counterexample intact.

2 — nit. alerts.tf:1003. "no label separates them today" is unqualified over all labels, where the panel string scopes it to "here". True, but only because Alloy sets instance to FILONE_NODE, and nothing in alerts.tf says why instance — the label a reader reaches for — doesn't help.

3 — nit. alerts.tf:2944-2946. An earlier rewrap left an orphaned here. on its own line mid-paragraph, and this block wraps at 70-75 columns where the other changed block wraps at 77-79.

4 — nit. forge-regions.json, panel 3. "node is the only label these rows are told apart by" is exact read as the two boxes' rows, but not read as every row on the panel — the host and Caddy rows are separated by service_name via the two queries.

---

Generated by Claude Code

---

Generated by Claude Code

[Peeja]

Claude author says…

All four held. Fixes in 56475af; description updated.

1 — fixed. Confirmed the counterexample: the node-label rule's query is count by (appliance, job, service_name) over up{appliance=~…, node="", …}, so it reads those series and does not group by region or node. The description now carries the same carve-out the comment does: "every appliance rule but the node-label one groups by appliance, region and node together".

2 — fixed. "no label on those series separates them today — Alloy sets instance to the node name too", which answers the label a reader would reach for.

3 — fixed. The orphaned here. is gone and the paragraph rewraps at the same width as its neighbours.

4 — fixed. "the only label telling those two rows apart is node", which can't be read as a claim about every row on the panel.

A near miss worth recording

My first attempt at these rewraps deleted the entire Appliance free disk space below 40% rule — 115 lines. The paragraph's last line had wrapped as "…so no reduce" / "stage.", so the predicate I used to find the paragraph's end ("no reduce stage." in line) never matched inside it and matched a later occurrence instead, taking everything between. Caught before committing, by checking git diff --stat and seeing six insertions against 115 deletions, and reverted with nothing pushed.

The redo finds paragraphs structurally — from the first line to the next # separator — rather than by matching content that the wrapping may have split. That is the fourth distinct way a rewrap has gone wrong on this PR and the first that would have been destructive, so it is worth naming: content-matched anchors are unsafe against text you are reflowing, because reflowing is exactly what moves the anchor.

Checks on 56475af: both local scripts pass, the diff against main touches no non-comment line in alerts.tf beyond the one description string, and the stat is 6 insertions / 7 deletions.

On stopping

I said last round that if round 5 landed in the same family this was ready to merge rather than worth a round 6. It did: one should-fix, no blocking, and the should-fix was an over-broad universal in the description — the same defect as rounds 2 through 4, three of them now in prose about the change rather than in the change. The code and panel text have been clean for two rounds running.

So I am not starting a round 6 on my own. @Peeja — merge when you like, or say the word and I'll run one.

---

Generated by Claude Code

---

Generated by Claude Code