feat(github): surface engine throttle pauses and checkpoint resumes

[aparajon]

Consumes the resume and throttle fields Spirit now reports through its status API.

Throttle backpressure. A table held back by the engine's throttler now renders the throttle and its trigger on the PR comment and CLI progress surfaces, so a slow progress bar reads as deliberate backpressure — slowed, not stopped — rather than a hang. The table's header line carries a (throttled) annotation right where the eye checks progress, and the trigger renders as a tooltip bullet using the same idiom as the estimate-exceeded note. The flag and display reason ride the stored task row through the durable read-model pipeline — readers never poll the engine:

engine TableProgress ──> drive write-through ──> task row ──> proto/API ──> PR comment + CLI
 (paced phases only)      (clears when the       (throttled,
                           throttle lifts)        throttle_reason)

Rendered on the PR comment (full scenario in TEMPLATES.md, "Second Table Throttled"):

users: 🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦⬜⬜⬜⬜⬜⬜⬜⬜ 62% (throttled)

ALTER TABLE `users` ADD INDEX `idx_email`(`email`);

• Rows: 914,707 / 1,466,232 · ETA: 3m 15s
• ℹ️ Throttled: commit-latency 112.4ms >= 100ms

The engine stamps the runner-wide throttle status only on tables participating in a paced phase — a table still copying, or every table during the checksum verify — so a completed table is never rendered as throttled by another table's copy. The stored pair behaves as a point-in-time signal: it refreshes on every progress tick (a lifted throttle clears promptly, including through the remote-progress mirror), a reason never travels without the flag, and it clears when the task comes to rest. Reasons are sanitized at every ingest boundary (whitespace collapse, | neutralized, rune-safe clamp) and the PR comment escapes inline markdown at render, so an engine-produced reason can never break the comment layout. When the engine reports throttled without a reason, the header annotation stands alone. Table detail lines (rows, ETA, verified counts) now render as a uniform bulleted list on the PR comment, matching the CLI.

Checkpoint resumes. A drive claim that reattaches to the engine's durable checkpoint records one timeline event, so operators can tell a resumed copy from a fresh start after a pod restart or lease handover. One event per drive claim is intentional: every claim that resumes is a real engine resume.

Includes the spirit bump that introduces both status fields, the two new task columns in both schema dialects, and new preview scenarios in TEMPLATES.md. The bump also rides along Spirit's other recent main work beyond the status API — binlog watermark visibility, checksum repair-path hardening, and flush/drain improvements — which lands here as part of the dependency update rather than as SchemaBot behavior changes.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR extends SchemaBot’s end-to-end progress pipeline (engine → durable storage → proto/API → PR comment + CLI) to surface two new Spirit status signals: per-table throttle pauses (with a human-readable trigger) and a “resumed from durable checkpoint” indicator that becomes a single timeline event per drive claim.

Changes:

• Plumbs throttled / throttle_reason from Spirit through engine progress, storage task rows, proto/API types, and renders it in PR comments + CLI progress output.
• Records a one-time “resumed from checkpoint” timeline event per drive claim when Spirit reports it reattached to a durable checkpoint.
• Updates schema (MySQL + Postgres), templates/previews, and bumps the Spirit dependency (and Go patch version).

Reviewed changes

Copilot reviewed 29 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
TEMPLATES.md	Adds rendered preview examples showing throttled table output in PR comments and CLI.
pkg/webhook/templates/preview.go	Adds a new PR comment preview scenario for a throttled table.
pkg/webhook/templates/apply.go	Adds throttling fields to template data and renders the “Paused by throttler” line.
pkg/webhook/templates/apply_test.go	Adds unit tests asserting throttling text rendering in apply comments.
pkg/webhook/apply.go	Maps stored task throttling fields into webhook comment template data.
pkg/tern/local_client.go	Includes throttling fields in the local progress response mapping.
pkg/tern/local_client_test.go	Adds tests for checkpoint-resume timeline logging and verifies throttling fields in progress.
pkg/tern/local_apply.go	Adds helper to log a resume-from-checkpoint timeline event once per claim.
pkg/tern/local_apply_sequential.go	Logs resume-from-checkpoint once per sequential drive claim; persists throttling fields to tasks.
pkg/tern/local_apply_grouped.go	Logs resume-from-checkpoint once per atomic/grouped drive claim; persists throttling fields to tasks.
pkg/tern/grpc_client.go	Mirrors throttling fields from remote progress into stored tasks.
pkg/tern/grpc_client_test.go	Adds coverage verifying throttling fields mirror from remote progress snapshots.
pkg/storage/types.go	Extends storage.Task with Throttled and ThrottleReason.
pkg/storage/internal/sqlstore/tasks.go	Adds throttling columns to task CRUD SQL and scanning.
pkg/storage/internal/sqlstore/tasks_test.go	Adds round-trip test for task throttling columns.
pkg/schema/postgres/tasks.sql	Adds throttled and throttle_reason columns to Postgres tasks schema.
pkg/schema/mysql/tasks.sql	Adds throttled and throttle_reason columns to MySQL tasks schema.
pkg/proto/ternv1/tern.pb.go	Regenerates tern proto bindings with throttling fields on TableProgress.
pkg/proto/tern.proto	Adds throttling fields to TableProgress in the tern API.
pkg/engine/spirit/spirit.go	Reads Spirit throttle + resume status; sanitizes throttle reason for safe rendering.
pkg/engine/spirit/spirit_test.go	Adds tests for throttle scoping behavior and reason sanitization.
pkg/engine/engine.go	Extends engine progress types with ResumedFromCheckpoint and throttling fields.
pkg/cmd/internal/templates/progress.go	Renders throttling line in CLI progress templates.
pkg/cmd/internal/templates/progress_states_test.go	Adds CLI rendering tests for throttling across copy/checksum states.
pkg/cmd/internal/templates/progress_parse.go	Plumbs throttling fields from API types into CLI template data.
pkg/cmd/internal/templates/preview_sequential.go	Adds CLI preview scenario for a throttled sequential apply.
pkg/cmd/internal/templates/preview_comment.go	Adds preview selectors for the new comment + CLI throttle scenarios.
pkg/apitypes/apitypes.go	Adds throttling fields to the JSON API progress table response type.
pkg/api/progress_handlers.go	Maps throttling fields from proto and local storage into API responses; syncs from tern into tasks.
go.sum	Updates checksums for the Spirit dependency bump (and related module resolution).
go.mod	Bumps Go patch version and updates Spirit dependency version.

Files not reviewed (1)

• pkg/proto/ternv1/tern.pb.go: Generated file

---

💡 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 pull/1039, ae354db.

Verdict: 8 findings — 6 non-blocking, 2 suggestions, none blocking; CI 33/33 pass — all checks green at verification time (Build, Unit, Lint x4, Integration x3, E2E MySQL/K8s/Vitess/gRPC suites, LocalScale x3, build-and-push, DCO).

Non-blocking

1. Throttle mirror is skipped by the row-totals retention branch in gRPC sync. The Throttled/ThrottleReason assignments at pkg/tern/grpc_client.go:3297-3298 live only in the else of the remoteTaskOmittedRowTotals guard (:3279), the sole deviation from the PR's clear-on-empty write-through contract. If a remote engine restart briefly reports RowsTotal<=0 with Throttled=false, the stored throttled=true survives and the PR comment renders a stale (throttled) until row totals reappear (transient, self-healing). Mirror the flags unconditionally, like the three local-drive sync sites do.

2. Remote-supplied throttle_reason is stored unclamped and rendered raw. pkg/tern/grpc_client.go:3298 trusts that the data plane already sanitized the reason, and pkg/webhook/templates/apply.go:991 prints it without the sanitize+escape treatment error_message gets. A version-skewed data plane sending >255 chars makes Tasks().Update fail with data-too-long on every reconcile tick, stalling progress mirroring for that apply; raw HTML/markdown also renders unescaped. Defense-in-depth gap: clamp/escape at the control-plane or render boundary.

3. New test bisects an existing test's doc comment. TestLocalClient_LogEngineResumeOnce was inserted mid-comment: pkg/tern/local_client_test.go:1997-1999 describe the auto-cutover test but sit atop the resume test, and the auto-cutover comment resumes mid-sentence at :2032. Both tests now carry garbled godoc; move the three orphaned lines down to rejoin their test.

4. Already-bootstrapped PostgreSQL deployments need a manual ALTER before this ships. The two NOT NULL columns at pkg/schema/postgres/tasks.sql:28-29 will fail startup on existing PG instances, since the bootstrapper verifies columns but never alters tables (MySQL self-heals via Spirit). Sibling fix(operator): pace the automatic retries of an interrupted apply, and skip them when the target's verdict is final #1041 adds a PG applies column with the same requirement, so coordinate one pre-deploy ALTER pass for the batch.

5. The spirit bump rides in 23 commits beyond the two advertised status fields. go.mod:13 moves 864cf2ee -> fc3efbddfa34, which includes engine-behavior work (KeyAboveHighWatermark binlog visibility, checksum repair-path, flush-drain) the body's "introduces both status fields" undersells. A later engine regression could be misattributed to this "UI-only" PR; note the ride-alongs in the PR body.

6. The canonical progress-pipeline doc is now stale on exactly the surfaces this PR extends. docs/spirit_progress.md:52 (Spirit-exposes table), the persisted-fields table (:207-219), and the write-trigger table (:223-229) all omit Throttle/Resume and the new columns. An engineer using the doc would conclude throttle state is not persisted; update it in a follow-up.

General suggestions

7. At-rest tasks freeze the throttle pair the way ETASeconds deliberately isn't. pkg/tern/local_apply.go:359-361 clears ETASeconds when a task leaves in-flight state but leaves Throttled/ThrottleReason set, so the JSON API (pkg/api/progress_handlers.go:1153) reports throttled:true on a stopped task. Renderers mask it; consider clearing the pair in the same branch.

8. The ResumedFromCheckpoint wiring is untested end to end. Only the latch helper has a unit test (driven with literal bools), so dropping ResumedFromCheckpoint: spiritProgress.Resume at pkg/engine/spirit/spirit.go:803 or the call at pkg/tern/local_apply_sequential.go:537 would leave all tests green while the headline timeline event silently never fires. A pipeline-level test would pin it.

The one thing that could have broken, verified

The riskiest mechanism is the durable write-through clear semantics — the promise that (throttled) appears only while the engine is actually pacing the table and never lingers on an operator surface. All four storage write paths were traced (pkg/tern/local_apply_sequential.go:566, pkg/tern/local_apply_grouped.go:1269, pkg/api/progress_handlers.go:1222, pkg/tern/grpc_client.go:3297), confirming unconditional per-tick overwrite everywhere except the single row-totals retention branch reported as finding 1. Every terminal/at-rest render branch in the CLI and webhook returns before any throttle output, so even a stale stored flag is masked on visible surfaces (pinned by pkg/webhook/templates/progress_states_test.go:433). The engine-side predicate at pkg/engine/spirit/spirit.go:881 was checked against the bumped spirit source: it mirrors exactly the phases spirit's runner-wide throttleStatus paces, so a completed table can never be stamped by another table's copy. The mechanism holds; the residual exposures are the gRPC version-skew edges in findings 1-2.

Verified correct

• Clear-on-empty write-through is unconditional on all three local/control sync paths (pkg/tern/local_apply_sequential.go:566-567); a lifted throttle clears the stored flag.
• Engine stamping is scoped to paced phases only (pkg/engine/spirit/spirit.go:869-872), pinned by the three-case matrix in spirit_test.go and the bumped module source.
• sanitizeThrottleReason collapses whitespace, neutralizes |, and clamps rune-safely to <=200 bytes (pkg/engine/spirit/spirit.go:889-901), fitting varchar(255) in both dialects.
• Read-model fan-out is complete end to end: proto fields 17/18 collide with nothing (proto/tern.proto:639) -> gRPC server -> API -> CLI -> webhook; non-spirit engines render unthrottled.
• No stale (throttled) on any terminal render surface — every terminal branch returns before the throttle suffix (pkg/webhook/templates/apply.go:727), and no double-tooltip path exists.
• Checkpoint resume fires exactly one timeline event per drive claim via the per-claim latch (pkg/tern/local_apply.go:290-294); spirit's Resume flag is false on fresh start after checkpoint expiry.
• Schema/sqlstore sweep is consistent in both dialects with matching column order and a round-trip test (pkg/storage/sqlstore/tasks.go:956); TEMPLATES.md is byte-identical to output regenerated from the PR-head binary; composition with sibling fix(operator): pace the automatic retries of an interrupted apply, and skip them when the target's verdict is final #1041 is safe in both merge orders (disjoint storage, render states, and scenarios).
• Unit tests pass locally at head ae354db with -race -count=1 across all touched packages; CI 33/33 green (Build, Unit, Lint x4, Integration x3, E2E MySQL/K8s/Vitess/gRPC suites, LocalScale x3, build-and-push, DCO).

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

[aparajon]

🤖 Thanks for the thorough review — all eight findings are addressed in three follow-up commits (kept as new commits since the PR is approved):

• 1, 2, 7 → b53089d: the throttle pair is now mirrored on every gRPC tick (a lifted throttle clears even when stored row totals are kept), a reason never travels without the flag, and both clear when a task comes to rest. Sanitization moved to pkg/engine.SanitizeThrottleReason so the remote-ingest boundary shares the engine stamp site's bounds, and the PR-comment tooltip escapes inline markdown at render.
• 3, 8 → 330f9d8: the split auto-cutover comment is reunited, and a new pipeline-level test drives pollTaskToCompletion against an engine reporting the resume flag on every poll, asserting one timeline event per drive claim.
• 6 → 694e568: docs/spirit_progress.md now covers Resume and Throttle end to end (exposed fields, paced-phase scoping, persisted columns, write triggers).
• 5: noted the spirit bump's ride-along engine work in the PR description.
• 4: acknowledged — the column rollout ordering is handled operationally on the deployment side before this ships.

This reply was written by Claude Code (Claude Fable 5).