feat(github): explain throttle reasons with inline tips and a reference doc

[aparajon]

Follows up on the throttle surfacing in #1039. A raw engine reason like redo-aware 4 > 3 names a signal only its author recognizes; an operator watching a paused bar should not need to read the throttler source to know what the pause protects.

Inline tips. The throttle tooltip now appends a short tip keyed on the reason's leading signal token:

• ℹ️ Throttled: redo-aware 4 > 3 · backing off while the database's active threads exceed its budget (docs)

The full mapping:

Signal	Tip
redo-aware	backing off while the database's active threads exceed its budget
threads-running	backing off while the database's active threads exceed its budget
commit-latency	backing off while database writes commit slowly
anything else	no tip; the raw reason renders alone

Signals that read the same to a user share a tip (redo-aware and threads-running, the same budget measured two ways), composite reasons split on ; and join their distinct tips, and a reason containing any unrecognized signal renders raw with no tip at all — a partial explanation must never bind to a signal it does not cover. The thread-budget tip deliberately does not blame application load: the engine counts its own copy threads toward the budget, so the pause can occur on an idle instance. The CLI renders the same reason · tip line dimmed, without the link.

Reference doc. New docs/throttle.md carries the remediation prose the one-line tip cannot: what each signal measures, why the engine pauses on it, and when a sustained throttle warrants action (performance_schema access to upgrade the thread signal, storage pressure, instance sizing). It covers exactly the signals SchemaBot's engine configuration can emit. The PR comment links it so the tip is a pointer, not the whole story.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds operator-facing explanations for engine throttling so a slowed progress bar reads as intentional backpressure (and what it protects), with a linked reference doc for deeper remediation guidance.

Changes:

• Render throttle tooltips as reason · tip (and add a docs link in PR comment markdown) when the reason’s leading signal token is recognized.
• Introduce pkg/ui mapping logic for throttle-reason → operator tip, including composite reasons.
• Add docs/throttle.md as the canonical reference for throttle signals and operator actions.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
TEMPLATES.md	Updates rendered-output examples to include the new reason · tip throttle text (and docs link in PR comment example).
pkg/webhook/templates/apply.go	Appends an inline tip + docs link to the markdown throttle tooltip when a tip is available; falls back to raw reason when unknown.
pkg/webhook/templates/apply_test.go	Updates PR-comment rendering assertions for the new throttle tooltip format.
pkg/ui/throttle.go	Adds ThrottleTip and ThrottleDocURL to translate engine throttle reasons into short operator tips.
pkg/ui/throttle_test.go	Tests throttle tip mapping, composite reasons, de-duping, and unknown-signal behavior.
pkg/cmd/internal/templates/progress.go	Adds the same reason · tip rendering to the CLI (without a docs link).
pkg/cmd/internal/templates/progress_states_test.go	Updates CLI-format assertions for the new throttle tooltip text.
docs/throttle.md	New reference documentation for throttle signals, what they mean, and when to act.

Suppressed comments (1)

pkg/webhook/templates/apply_test.go:328

• This assertion also hardcodes the docs URL; prefer ui.ThrottleDocURL to avoid duplicating the link string.

	assert.Contains(t, result, "- ℹ️ _Throttled: threads-running 130 > 128 · yielding to application query load on the database ([docs](https://github.com/block/schemabot/blob/main/docs/throttle.md))_")

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1072, 4ff2f65.

Verdict: 9 findings — 2 blocking (the most likely tip names the wrong culprit, and a third of the doc covers a signal SchemaBot cannot emit), 5 non-blocking, 2 suggestions.

Blocking

1. The redo-aware / threads-running tip blames "application query load" for a threshold the copy's own threads are counted against — and the doc's own example is exactly the self-throttle case. Spirit deliberately includes them: "The copy's own read/apply threads are deliberately NOT excluded — those are genuine CPU load that should count toward saturation; only spirit's monitoring footprint is" (aurora_threads.go:129). On a 2-vCPU Aurora the threshold is vCPUs+1 = 3, while DefaultThreads = 2 read threads plus auto-sized write threads (execution.go:45) already put 4 Query threads on the box — producing redo-aware 4 > 3, the literal example at throttle.md:51. The operator is told to "reduce application load on the instance" (:63) when there may be none.

2. The two replica-lag sections — 30 of the doc's 103 lines, and 2 of the 5 tips — document a signal SchemaBot can never emit, and send operators to flags it does not expose. Spirit builds the replica throttler only if r.migration.ReplicaDSN != "" (runner.go:1051), and grep -rn ReplicaDSN pkg/ cmd/ returns zero hits in this repo — nothing sets it. So the remediation "remove --replica-dsn to proceed without lag protection" (:45) and the --replica-max-lag / --max-commit-latency references name a CLI surface SchemaBot users do not have; the commit-latency threshold in particular is the hardcoded const maxCommitLatency = 100 * time.Millisecond.

Non-blocking

3. In a composite reason, an unrecognized segment is dropped and the surviving tip silently binds to the whole pause. if tip == "" ... continue (throttle.go:25) means disk-usage 95% > 90%; commit-latency 101ms >= 100ms renders one tip across two signals, so the operator reads a storage-pressure pause as a commit-latency one and follows a docs link that never mentions the signal actually holding the copy. This directly contradicts the guarantee both call-site comments state ("a new engine signal degrades to raw text rather than a wrong explanation") — true only for single-segment reasons. Returning "" when any segment is unrecognized would restore it.

4. threads-running 130 > 128 implies a 126-vCPU instance, which is not an Aurora size. globalStatusMode carries headroom: selfMonitoringHeadroom = 2 and the threshold is vCPUs + headroom, so a real 128-vCPU box renders 131 > 130. This is the same class of self-inconsistent example commit 466fad7 already fixed once for replica-lag, and the doc teaches the arithmetic 10 lines earlier.

5. "On Aurora this signal often pairs with redo-aware" (:94) inverts Spirit's design. redo-aware subtracts exactly the threads parked on redo-log flush, so when the log saturates its count falls: "it will not self-limit if the extra write threads saturate the log, and commit-latency is then the only signal that would notice" (aurora_threads.go:192). An operator who looks for a co-occurring redo-aware reason, finds none, and concludes "not a load problem" skips the resize the throttle is asking for.

6. Three more doc claims do not hold. :9 promises the tooltip carries "a link to this document", but the CLI renders no link (progress.go:795 vs apply.go:996) — the PR body says the opposite of the doc. The asserted grammar <signal> <observed> <op> <threshold> is contradicted by the doc's own replica-lag unobservable for 45s (failing closed) example 25 lines later. And :6's "no operator action is required" is contradicted by the fail-closed section's own remediation.

7. Two mutations survive green. Deleting the CLI's unknown-signal fallback render (progress.go:798) leaves all three packages passing — no CLI test ever passes an unrecognized signal, so a refactor could drop the reason from CLI output entirely. Dropping escapeInlineMarkdown from the new webhook tip branch also survives, because the only escaping case (apply_test.go:302) uses an unrecognized signal and so exercises the old branch instead. AGENTS.md's "tests must prove documented behavior" applies to the first.

General suggestions

8. Test precision. progress_states_test.go:419 was left at its pre-PR prefix while its sibling on :403 was updated, so the checksum-phase tooltip passes with or without the tip — its webhook twin (apply_test.go:328) was updated. No render-layer test uses a composite reason. And SanitizeThrottleReason's 200-char clamp exists "so a reason can never break a PR comment table", but the tips and docs link are appended after it — a three-signal line runs ~300 chars.

9. docs/throttle.md has no docs-toc block. make docs-toc inserts one (13 of 15 docs/*.md carry it), so the next contributor to run the target gets an unrelated hunk. docs/release.md has the same gap from #1066, so this is drift rather than a break.

The one thing that could have broken, verified

A wrong tip is worse than no tip, so the question was whether the mapping can mis-key. Token extraction is sound — strings.Cut(part, " ") plus an exact switch means replica-lag-foo and replica-lagX correctly get no tip, with no prefix bleed, and a;;b, trailing ;, and all-unrecognized composites produce no dangling · or empty tail. The one fragility is the unobservable check: it scans the whole segment rather than the value token, so a reason carrying that word in a hostname would report the fail-closed tip and send the operator to remove working lag protection. Not reachable today (Spirit's non-stale form is numeric), and Cut already computes the remainder the code discards — HasPrefix(rest, "unobservable") is a free fix. The real failure is structural, not lexical: finding 3.

Verified correct

• The tip set covers every production signal Spirit can emit — replica-lag (both forms), redo-aware, threads-running, commit-latency; only the test-only Mock reason is unmapped.
• The redo-aware 4 > 3 example arithmetic is right: redoAwareMode headroom is 1, so a 2-vCPU instance thresholds at 3.
• Both surfaces call the same ui.ThrottleTip, so they cannot diverge on the mapping; the empty-reason case returns early on both with no orphaned ℹ️.
• Escaping, dedup, the ; split, the · separator and the docs URL are all pinned — 10 of 12 mutations were killed.
• TEMPLATES.md is generated and the generator was run: both changed lines match a fresh preview run byte-for-byte.
• All 6 deletions are replacements with strictly stronger assertions; no guarantee was dropped.
• The four-commit shape is correct — AGENTS.md forbids squashing after human review, and this PR already has three reviewers.
• All 34 CI checks pass; go build, go vet and the three touched-package suites are green.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 All nine addressed in 21a92ac. Both blocking findings were confirmed against source: the copy's own threads deliberately count toward the thread budget, so the tip now stays neutral ("backing off while the database's active threads exceed its budget") and the doc calls out that an idle instance can throttle on the copy's own footprint; and ReplicaDSN has zero references in this repo, so the replica-lag tips and doc sections are removed rather than corrected. Composite tips are now all-or-nothing (finding 3's fix, which also removes the unobservable substring fragility), the threads example is 21 > 18, the commit-latency section now warns redo-aware falls silent on redo-log saturation, the intro's link/no-action claims are fixed, both surviving mutations have killer tests, the stale CLI assertion is tightened, and the doc carries its docs-toc block. Skipped one suggestion: the post-clamp tip length — tips are bounded compile-time constants, so the 200-char clamp on the untrusted segment still bounds the only variable part.

This reply was generated by Claude Code (claude-fable-5).