Skip to content

Make alert severities match their definitions - #249

Merged
Peeja merged 24 commits into
mainfrom
claude/happy-bell-cfh5qb-severity-warnings
Oct 20, 2026
Merged

Peeja merged 24 commits into
mainfrom
claude/happy-bell-cfh5qb-severity-warnings

Conversation

@Peeja

@Peeja Peeja commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Written by Claude.

Some rules alerted as critical don't meet the definition of critical in #248. That definition says a rule pages when waiting until the next day could make a loss permanent. This PR makes those rules warning, which notifies rather than pages, splitting one rule to do it. It also makes every Piri proving rule say how soon to act, and links each to its own section of the runbook added in fil-forge/infra-nodes#178. That PR should merge first, so the links resolve.

Piri's proving rules

The question for each rule is whether, while its condition holds, customers are affected or anything is lost for good. From Piri's code, uploads and reads never wait on the chain: blob/accept queues on-chain work through a harmonydb task, and Ingot reads through its own blob_locations table and Piri's blob store. So the answer turns on what is permanent.

Rule Severity Act Why
Piri's chain head is stale critical (unchanged) within about 3 h AddRoots gives up on each batch after 50 retries, about 3.5 h (6.9 h if Lotus answers but is stale), and nothing claims it again (FIL-1508). Becomes a warning once that's fixed.
Piri is reporting but has no chain head critical (unchanged) within about 3 h Same.
Piri proving is failing critical (unchanged) now Curio gives up on a set for good on its fifth rejection, and the 100/200/400/800-epoch backoffs let that come 12.5 h after the first.
Piri's proving period has not advanced warning (was critical) within a day Rejections are excluded by the rule; what's left is a stall, which costs about one period's payment a day and catches up once fixed (Curio takes the next window from the contract). See the AddRoots exception in the next row.
Piri's Prove task has not run in 1.5 proving periods warning (was critical) within a day Same. Most of its permanent-risk causes page through the rules above; the exception is an AddRoots failure with the head still moving, such as an empty wallet, which strands batches (FIL-1508) while probably only these two warnings fire (whether "proving is failing" fires depends on Curio's revert patterns, unconfirmed). This rule stays paused for FIL-1222, so today only the row above fires. Both descriptions say to check the wallet first.
Piri proof set is unrecoverable warning (was critical) within a day See below.

Each description now ends with how soon to act and why, and each runbook_url points at the rule's own section instead of "When something is wrong".

"Piri proof set is unrecoverable"

Curio gives up on a proof set in two ways. Both are in tasks/pdpv0/failure_handling.go at the version Piri pins, curio@38ca280c43a2.

  • The set's payment has ended or been terminated. The send fails with DataSetPaymentBeyondEndEpoch or DataSetPaymentAlreadyTerminated, and Curio marks the set unrecoverable at once. There's no revenue left to lose.
  • Five consecutive contract reverts. Curio backs off 100, 200, 400 and 800 epochs between attempts, at least 12.5 h in all, before the fifth revert; a prove revert happens at most once a proving period, so it can take days. "Piri proving is failing" (critical) pages for each rejection before the last.

A cause that threatens the node's other sets, such as a broken prover, shows up in their own rejections. Those page through "Piri proving is failing" hours before any set reaches five.

"Appliance container telemetry has stopped"

This means cAdvisor is dark, so nothing is watching the containers; it doesn't mean a container has stopped. If a container on the request path stops in the meantime, its site's responses still show it, and "Appliance site is failing most requests" pages on that. In practice the label matters little: the rule only fires where cAdvisor runs, and that's staging only for now, where nothing pages.

Other changes

  • Piri's own monitoring.md recommends Critical for an unrecoverable proof set. The rule's comment now says it departs from that, and why.
  • The comment on "Appliance metrics scrape is failing" said the disk rule's no_data_state = "NoData" adds a third notification. The disk rule has been OK since that rule took over, so that clause is gone.
  • The comment on "Appliance container is not running" said nothing covers a dead cAdvisor. It now points to the next rule, which does.
  • The comment above the Piri rules said "All of these page (critical)". It now states the test, says which three page, and replaces "a fault on chain, which costs the provider" with what a missed proof actually costs: no payment for its period.
  • "Piri's Prove task has not run" said it was critical "as the piri docs recommend". It now says it departs from them, and why, as the unrecoverable rule does.
  • A few comments said "page" where they now mean any alert.

"Appliance has stopped reporting", split in two

The rule fired on two different failures, so it is now two rules.

  • "Appliance has stopped reconciling" (new, warning). This is the stale-stamp branch: the reconcile stamp is more than 15 minutes old while the node is still reporting. This is the common case: the box keeps serving what it already runs, and only deploys go stale.
  • "Appliance has stopped reporting" (still critical). This is the absence branch: nothing from the node's host exporter for 15 minutes, though it reported in the last day. It's the only rule that catches a whole appliance going dark, since the other appliance rules read data that vanishes with the node and stay quiet when it does.

The two don't overlap: one needs the series present and the other needs it absent. Keeping the old name on the absence branch keeps the three comments elsewhere in alerts.tf that cite it accurate; each describes node death or a dark exporter. docs/observability.md's rule table gains the new row.

Both are instant queries now. The old rule used a range query reduced with last, which can hold an alert open for up to an hour after recovery. The new rule is appended to the end of the Forge Regions group rather than placed next to its pair. The provider keeps each rule's uid by its position in the list, so inserting it mid-group would shift every later rule's uid.

Checks

  • make check passes.
  • git diff --check is clean.
  • The plan-grafana CI job runs the alert-routing check.

✴️

🤖 Generated with Claude Code

https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ


Generated by Claude Code

"Piri proof set is unrecoverable" fires after proving has already paged
for the earlier rejections, or after the set's payment ended. "Appliance
container telemetry has stopped" means the containers are unwatched,
not that one has stopped. Neither meets the critical definition in
docs/observability.md (#248).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Copilot AI balanced review requested due to automatic review settings October 14, 2026 22:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The rule fired on two different failures: a reconcile timer that stopped
on a live box, and a node that stopped reporting altogether. The first
leaves deploys stale and can wait for working hours; the second is the
only rule that catches an appliance going dark. "Appliance has stopped
reconciling" now takes the stale-stamp branch as a warning, and
"Appliance has stopped reporting" keeps the absence branch as critical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
@Peeja Peeja changed the title Make two alert rules warnings Make three alert conditions warnings Oct 14, 2026
@Peeja
Peeja requested a review from bajtos October 14, 2026 22:07

Peeja commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (1st round, 5c9bc4b)

Approve after the follow-ups below: one should-fix and three nits. The severity changes are right, and every factual claim checks out against the code. This round reviewed 5c9bc4b, before the split in f25e950. The split gets its own round.

Verified

  • Piri's Curio pin: Piri's go.mod on main pins curio v1.28.3-0.20260717015646-38ca280c43a2.
  • Two ways to become unrecoverable. The only ones are the two HandleProvingSendError tiers: payment ended or terminated (at once), or MaxConsecutiveFailures = 5 reverts. Backoff totals 100+200+400+800 = 1500 epochs. Nothing else sets unrecoverable_proving_failure_epoch.
  • "Piri proving is failing" sees rejections 1-4. metrics.go skips unrecoverable sets, so the fifth rejection removes the series rather than raising it. Gaps of at least 100 epochs, with 30 s pushes, mean increase(...[1h]) > 0 catches each earlier rise.
  • Piri runs only part of Curio. pkg/curiopdp/module.go registers no terminate, delete or chain-sync task.
  • The telemetry rule is staging-only today. Every branch, and its gate, needs a job="cadvisor" series, and only staging runs cAdvisor.
  • Site rule. It catches a stopped container on the request path through 502s, given traffic; the PR's wording is scoped to that.
  • Dead node. "Appliance has stopped reporting" is the only critical rule that sees one. The other rules evaluate to no data, treated as OK; the disk rule's NoData is only a warning.
  • Checks: make check passes, git diff --check is clean, and CI plan-grafana and check are green on 5c9bc4b. tofu plan was not run locally.

Findings

  1. should-fix, alerts.tf Piri group comment and the new rationale. The group says it follows Piri's docs/content/operator-guide/monitoring.md, whose table (L259) lists "Proof set unrecoverable" as Critical. The PR departs from that without saying so, so a reader comparing the two could "fix" it back.
  2. nit, telemetry rule comment. It still says "keeps a dead node from paging twice", two paragraphs above the "page" → "alert" edits.
  3. nit. "one failure for each rule to page on" (Piri group) and "A rule on the level would page forever" (unrecoverable rule) still say page.
  4. nit. "five times in a row over about half a day of backoff": 12.5 h is the minimum, since a prove revert happens at most once per period. Separately, the PR body's "broken … wallet" is speculation. An out-of-gas send may not match IsContractRevert and would be retried as transient, so it might never be counted.

Outside this diff: the comment on "Appliance container is not running" says "nothing else covers" a dead cAdvisor, but the very next rule does.

✴️


Generated by Claude Code

claude and others added 2 commits October 14, 2026 22:09
Round 1 review of #249.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
The disk rule's no_data_state has been OK since the scrape rule took
over reporting a dark exporter, so it adds no third notification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

Round 1's findings are fixed in 89e5ee0, and the PR body is updated.

  1. Piri's docs (should-fix). The unrecoverable rule's rationale now says it departs from the Critical in Piri's monitoring.md table, and why. I haven't changed Piri's docs from here.
  2. Telemetry comment. "keeps a dead node from paging twice" now reads "from alerting twice".
  3. "page" wording. It now reads "one failure for each rule to alert on" and "A rule on the level would fire forever".
  4. Backoff time. The comment now says "at least half a day", and the body says it can take days. "or wallet" is gone from the body, since an out-of-gas send may never count as a revert.

Two more changes:

  • Outside this diff. I fixed the "nothing else covers" comment on "Appliance container is not running" too: it now points to the scrape rule that does cover it.
  • Stale comment. Separately, 74fa654 drops a clause from the scrape rule's comment that relied on the disk rule's old no_data_state = "NoData", which is now OK.

f25e950, which landed before this round reported, splits "Appliance has stopped reporting" into a critical absence rule and a new warning, "Appliance has stopped reconciling". This round didn't cover it.

make check, git diff --check and an offline tofu validate pass. Round 1 had a should-fix, and the split is new behavior, so round 2 follows.

✴️


Generated by Claude Code

A range query reduced with last keeps the last point it fired on for
the window's length, so both rules could stay firing for up to an hour
after recovery. They are instant queries now, as most rules here are.

"Appliance has stopped reconciling" moves to the end of its group, so
that no existing rule changes position and so none changes uid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (2nd round, 74fa654)

Approve after one should-fix and two nits. The split is correct, and round 1's fixes are right.

Verified

  • Absence rule: present_over_time[24h] unless present_over_time[15m] gives 1, and the threshold gt 0 fires on it. The 3600 s relative range doesn't limit the [24h] selector.
  • Age rule: time() - stamp > 900, with the same stages, for, no_data_state and exec_err_state.
  • No overlap: one rule needs the series present and the other needs it absent.
  • Dark node: every other rule in the three Forge Regions groups is no_data_state = "OK", and the absence-shaped container rules are gated on a live scrape. So "Appliance has stopped reporting" is the one rule that sees an appliance go dark.
  • Cross-references: the three comments that cite "Appliance has stopped reporting", and the "node rule" mentions, all describe a dark node or a dark exporter. That is now the critical absence rule.
  • Dashboard links: viewPanel=13 is on both rules. docs/observability.md has the new row.
  • Checks: make check, git diff --check and an offline tofu validate pass. CI check and plan-grafana are green. tofu plan was not run against Grafana.

Findings

  1. should-fix, new rule's query (also on main for the absence rule). Both rules use a range query (from = 3600) reduced with last. A Prometheus range result holds no points where the expression returned nothing, so last returns the final point the rule fired on. The rule would then stay Alerting for up to an hour after recovery, which contradicts the comment's "no_data_state = OK is again what clears it". Make it an instant query; the expression has no use for a range.
  2. nit, scrape rule comment.
    • "a wedged exporter … is the stamp going stale" now reads as the warning's case. The stamp actually vanishes, so the critical rule fires.
    • "a summary that reads as though the box were gone" is out of date: the new description names the exporter and Alloy.
  3. nit, rule order. In provider v4.49.0, rule is a TypeList with uid Optional+Computed (resource_alerting_rule_group.go L77-88). The new rule, inserted mid-group, probably hands each later rule its predecessor's uid. Grafana would then likely reset those rules' state, and old notification links would point at the wrong rule. That effect is speculation; the plan would show it. Append the rule at the end of the group, or pin uids.

✴️


Generated by Claude Code

Peeja commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

All three findings are fixed in 7c220ed.

  1. Range query. Both "Appliance has stopped reporting" and "Appliance has stopped reconciling" are now instant queries with the file's usual shape: query A over 600 s, then threshold B (gt 0) as the condition. The reduce stage is gone. The absence rule's comment says why a range query with last would hold the alert open. The two container rules on main have the same range shape. I left them alone, since this PR doesn't touch them.
  2. Scrape comment. It now reads "a wedged exporter on a live box takes the stamp with it" and names "Appliance has stopped reporting" instead of the node rule.
  3. Rule order. "Appliance has stopped reconciling" is now appended to the end of the Forge Regions group, so every existing rule keeps its position, and its comment says why. I checked group by group against main: every group's rule list is unchanged except Forge Regions, which only gains the new rule at the end.

make check, git diff --check and an offline tofu validate pass. Round 2 had a should-fix, so round 3 follows.

✴️


Generated by Claude Code

Round 3 review nit on #249.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (3rd round, 7c220ed)

Approve. I found one nit and nothing blocking or should-fix. All three round 2 findings are fixed correctly.

Verified

  • Query shape. Both rules match the file's instant shape: A is instant over 600 s, and the condition is B, a threshold with expression = "A", params = ["B"] and gt 0. That is the same as "Provision Lambda errors", with nothing dangling.
  • Absence expression. It works as an instant query, because its [24h] and [15m] selectors look back on their own.
  • Uid by position. Confirmed in provider v4.49.0. rule is a TypeList, uid is Optional+Computed, there is no CustomizeDiff, and each list index's uid is sent (resource_alerting_rule_group.go L77-88, L632).
  • Rule order. It is unchanged in every group. Forge Regions gains "Appliance has stopped reconciling" as rule 15, and the other 14 keep their places.
  • Comments. The moved rule's comment names its pair, and no "above/next rule" reference elsewhere broke. The scrape comment's "takes the stamp with it" is accurate.
  • PR body. Accurate.
  • Checks. make check, git diff --check and an offline tofu validate pass. CI check and plan-grafana are green. The plan log couldn't be read here, so the uid stability isn't confirmed from the plan itself.

Findings

  1. nit, absence rule comment. It no longer points to its pair, which now sits about 1,400 lines away. A reader of this rule alone could think a live node with a stale stamp goes unwatched.

✴️


Generated by Claude Code

Peeja commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

The nit is fixed in e5d5a12. The absence rule's comment now ends: "A live node whose reconcile stamp has gone stale is "Appliance has stopped reconciling", at the end of this group."

make check and git diff --check pass. This round found only a nit, so there won't be another.

When this applies, the plan-grafana output is worth a glance. The rule uids in Forge Regions should not change; the new rule is the only one with a new uid.

✴️


Generated by Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Claude says…

plan-dev-platform failed on 229f724 at the tofu plan step, about 20 s into the run (job). I don't think this PR caused it:

I can't read the job log from here, so I can't say what failed. All three PRs ran their plans at once against the same state, so lock contention or a transient AWS error is possible, but that's speculation. No fix exists to port. I'm re-running the job once.

✴️


Generated by Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Apply the severity test from the definitions: a rule pages when waiting
until the next day could make a loss permanent, and is a warning when
the loss is a delay that stops once the cause is fixed.

- "Piri's proving period has not advanced" and "Piri's Prove task has
  not run in 1.5 proving periods" become warnings. A stall costs about
  one proving period's payment a day and catches up once fixed; the
  causes that can lose more page through the other rules.
- "Piri proving is failing" stays critical: Curio gives up on a set
  for good on its fifth rejection, as soon as 12.5 hours after the
  first.
- Both chain-head rules stay critical while AddRoots strands batches
  after a few hours without a head (FIL-1508).

Each proving rule's description now says how soon to act, and its
runbook_url points at its own section of infra-nodes' RUNBOOK.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
An AddRoots failure with the head still moving, such as an empty wallet,
strands batches without any rule paging, so the two warning rules no
longer claim every costlier cause pages on its own. The retry totals
are 3.5 and 6.9 hours: harmony waits only from the second attempt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
@Peeja Peeja changed the title Make three alert conditions warnings Make alert severities match their definitions Oct 15, 2026

Peeja commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (4th round, 3d06263)

Approve after one should-fix. This round covered #248, #249 and fil-forge/infra-nodes#178 together.

Verified

  • AddRoots stranding (FIL-1508): MaxFailures: 50 (task_addroots.go:139-140), harmony's delete after the last retry, no foreign key on add_task_id, and IAmBored claiming only IS NULL rows. Each attempt waits 4 min for confirmation.
  • Proving failures: a proof set is given up on at the fifth counted failure, after 1500 epochs (12.5 h) of backoff.
  • Runbook links: all six runbook_url anchors match the headings in infra-nodes#178.
  • Wording: severities and timeframes agree across descriptions, comments, runbook and PR body; nothing still claims to page when it doesn't.
  • Checks: make check and git diff --check pass.

Findings

  1. should-fix, alerts.tf comments on "proving period has not advanced" and "Prove task has not run" (and the runbook's matching sections): "its causes that can lose more page on their own" misses one. AddRoots counts any persistent error toward its 50 retries. An owner wallet without FIL, for example, strands batches while the head keeps moving, so neither chain-head rule fires. Speculation: an out-of-funds send probably doesn't match Curio's revert patterns, so "proving is failing" stays quiet too.
  2. nit: the title "Make three alert conditions warnings" no longer matches the PR.
  3. nit: the retry sums are off by one. Harmony waits only from the second attempt, so the totals are about 3.5 h and 6.9 h, not 3.4 h and 6.7 h. "Act within about three hours" still holds.

✴️


Generated by Claude Code

Peeja commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

Fixed in c4cd073.

  1. AddRoots exception: confirmed. AddRoots counts every error toward its 50 retries, while Curio's limit counts only errors matching IsContractRevert (tasks/pdpv0/error_detection.go:81-94). Both comments now say that a cause which also stops AddRoots with the head still moving, such as an empty wallet, strands batches with nothing paging. The PR table says so too. The runbook's warning sections, in Add a runbook section for each Piri proving alert infra-nodes#178, now say to check the wallet and look for stranded batches. FIL-1508's fix closes this gap along with the chain-head one.
  2. Title: now "Make alert severities match their definitions".
  3. Sums: confirmed from harmonytask.go:424-427, which waits only when Retries > 0. The comment, the PR body, the runbook and FIL-1508 now say 3.5 h and 6.9 h.

Checks: make check, git diff --check.

Round 4 had a should-fix, so a round 5 follows.

✴️


Generated by Claude Code

Round 5 found the AddRoots exception only in the comments. The two
warnings' descriptions now say to check the owner wallet first, since a
wallet out of FIL strands pending batches after about 3.5 hours. The
proving-failing description drops the wallet check, which an
underfunded send probably doesn't reach, for the revert reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (5th round, c4cd073)

Approve after one should-fix. This round covered this PR and fil-forge/infra-nodes#178 together.

Verified

  • Retry totals: Harmony skips the wait when Retries == 0 (harmonytask.go:424-427), and AddRoots waits 10 + 10r seconds (task_addroots.go:139-140). Over r = 1…49 that's 12,740 s, about 3.5 h. Adding 50 × 240 s of confirmation waits gives about 6.9 h.
  • Counting: HandleProvingSendError counts only errors matching IsContractRevert toward the backoff. Whether an out-of-funds send matches is still unconfirmed.
  • Runbook links: all six runbook_url anchors match the headings in infra-nodes#178.
  • Checks: make check and git diff --check pass.

Findings

  1. should-fix, the descriptions of "proving period has not advanced" (~L1918) and "Prove task has not run" (~L2248): round 4's AddRoots/wallet exception is in the comments only. The descriptions, which are what the notification shows, still say "nothing more is lost once scheduling runs again".
  2. nit, PR body: "with only these two warnings firing" overlooks that the Prove-task rule is paused, and "proving is failing" staying quiet is speculation.
  3. nit, alerts.tf L2225-2227: "with the AddRoots exception "proving period has not advanced" gives" reads awkwardly.
  4. nit, the "proving is failing" description still says to check the wallet for gas, which the runbook dropped because an underfunded send probably isn't counted as a rejection.

✴️


Generated by Claude Code

Peeja commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

Fixed in edce2ba, on top of the merge from main at 46a92d5.

  1. Descriptions: both now end "…unless Piri is also failing to add pieces, for example because the owner wallet is out of FIL. Then its pending batches strand after about 3.5 hours, so check the wallet first and see the runbook."
  2. PR body: the row now says "probably", explains that it depends on Curio's revert patterns, and notes that the Prove-task rule is paused, so today only the proving-period warning fires.
  3. Comment: now "except the AddRoots case described under "proving period has not advanced"".
  4. Wallet: the "proving is failing" description now says to read the revert reason instead, and the wallet check lives in the two warnings.

Checks: make check, git diff --check.

Round 5 had a should-fix, so a round 6 follows.

✴️


Generated by Claude Code

Round 6 nits: both warnings fire long after an empty wallet starts
failing AddRoots, so the 3.5 hours is past, not ahead. The wallet joins
the check list, and the description ends on what the runbook frees.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (6th round, edce2ba)

Approve. Nits only.

Verified

  • Round 5's should-fix is fixed: both warning descriptions name the wallet case, and they match the runbook in Add a runbook section for each Piri proving alert infra-nodes#178 at 1b2b13e.
  • "Proving is failing" now points to the revert reason, which matches the runbook word for word.
  • The PR body's claims are now true.
  • "Owner wallet" and 3.5 h are used consistently across descriptions, comments and runbook.
  • make check and git diff --check pass.

Findings

  1. nit, alerts.tf:1919 and :2249: "strand after about 3.5 hours" reads as a countdown from the notification. Both rules fire long after AddRoots starts failing: up to about 24 h for the first, and about 36 h for the Prove-task rule. So the batches have probably stranded already.
  2. nit, same lines: the descriptions are 835 and 729 characters, and "check the wallet first" contradicts the check list earlier in the same description.
  3. nit, alerts.tf:2228: one comment line is 89 columns.

✴️


Generated by Claude Code

Peeja commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

Fixed in 101bdbb.

  1. Timing: the descriptions now say the pending piece additions "may already have stranded, about 3.5 hours after they began failing" ("have probably stranded" for the Prove-task rule), and that "the runbook says how to free them".
  2. Order and length: the wallet is now first in each check list, and the trailing clause is shorter. The descriptions are now 760 and 674 characters.
  3. Wrap: rewrapped.

Checks: make check, git diff --check.

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

✴️


Generated by Claude Code

claude and others added 3 commits October 16, 2026 15:22
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
claude and others added 3 commits October 16, 2026 20:54
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Claude says…

plan-dev-platform and plan-staging-platform failed at the tofu plan step on 108f413, the merge of main at 23:00Z. Both failed about 12 seconds in. I don't think the failure is this PR's:

I can't read the job log from my session, so the cause is unknown. A state lock, or an AWS error while three PRs planned at once, is my guess. I've re-run the two failed jobs once.

✴️


Generated by Claude Code

claude and others added 5 commits October 20, 2026 10:36
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
@Peeja
Peeja merged commit c96a817 into main Oct 20, 2026
25 checks passed
@Peeja
Peeja deleted the claude/happy-bell-cfh5qb-severity-warnings branch October 20, 2026 20:21
Peeja added a commit to fil-forge/piri that referenced this pull request Oct 27, 2026
*Written by Claude.*

The operator guide's "Alerts to Configure" table recommends Critical for
"Proof set unrecoverable". This PR changes that to Warning, and the
alert's comment in the PromQL block now explains why.

## Why

Curio, at the version Piri pins (`curio@38ca280c43a2`), marks a proof
set unrecoverable in only two ways. Both are in
`tasks/pdpv0/failure_handling.go`, `HandleProvingSendError`:

- **The set's payment has ended or been terminated.** This is
`DataSetPaymentBeyondEndEpoch` or `DataSetPaymentAlreadyTerminated`,
marked at once. There's no revenue left to protect.
- **Five consecutive contract reverts** (`MaxConsecutiveFailures = 5`).
The backoff of 100, 200, 400 and 800 epochs means this takes at least
about 12.5 h. The "Proving failing" alert in the same guide, which stays
Critical, fires on each rise in
`piri_pdp_proofset_consecutive_prove_failures` before the last. Piri
drops an unrecoverable set's per-set series (`pkg/curiopdp/metrics.go`),
so the fifth rejection doesn't register there.

When this alert fires, the urgent part has already been reported. What's
left is cleanup.

This matches fil-forge/infra-central#249, which makes Forge's Grafana
rule for this condition a warning. That PR's comment notes that it
departs from this guide; once this lands, the two agree.

## Checks

Docs only. `git diff --check` is clean. The site wasn't built locally.

[✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

---
_Generated by [Claude
Code](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants