fix(github): fold the progress comment into the terminal summary - #1082
fix(github): fold the progress comment into the terminal summary#1082aparajon wants to merge 1 commit into
Conversation
Every apply ended with two near-identical "Schema Change Status" comments on the PR: the tracked progress comment edited to its full final rendering, and the separate terminal summary posted seconds later. The terminal publish now posts the summary first, then freezes the progress comment as a collapsed details block pointing at the summary — one authoritative final record at the bottom of the PR, with the per-operation history preserved inside the fold. A repeated terminal publish (webhook redelivery, fresh observer) that loses the summary claim reads the recorded summary from the marker and folds toward it again instead of restoring the full rendering. Stopped applies keep the full frozen rendering — stopped returns to active, and the resume rotation folds the stop record itself. When the summary's location is unknown (publish failed, claim in flight, or a per-driver observer deferring to the aggregate publisher), the full rendering stands as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the GitHub PR timeline UX by ensuring that terminal applies end with exactly one authoritative “Schema Change Status” record at the bottom of the timeline: the terminal summary comment. The tracked progress comment is “frozen” by folding its final per-operation rendering into a collapsed <details> section that points at the summary, avoiding duplicate near-identical terminal status comments.
Changes:
- Add a dedicated terminal fold rendering (“Finished: Superseded Progress Comment”) that links to the terminal summary and preserves the final progress body inside a collapsed details block.
- Reorder terminal publishing to post/resolve the terminal summary first (returning the summary comment ID when known), then fold the tracked progress comment toward that summary (except for stopped applies).
- Add/update preview templates and integration/unit tests to cover terminal folding and repeated terminal publishes (refold behavior).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TEMPLATES.md | Adds a rendered documentation example for the “Finished: Superseded Progress Comment” fold shape. |
| pkg/webhook/templates/preview.go | Adds a preview renderer for the terminal-summary-superseded progress comment. |
| pkg/webhook/templates/issue_comment.go | Introduces a terminal-summary fold marker/prefix and a shared fold helper parameterized by successor marker. |
| pkg/webhook/templates/issue_comment_test.go | Extends frozen-body detection tests to include the new terminal-summary fold. |
| pkg/webhook/comment_observer.go | Posts terminal summary first (returning its comment ID when known) and folds the tracked progress comment toward it when applicable. |
| pkg/webhook/apply_comment_integration_test.go | Updates an existing flow test and adds new end-to-end integration coverage for terminal folding/refolding and the stopped no-fold invariant. |
| pkg/cmd/internal/templates/preview_comment.go | Adds the new preview output entry to the CLI preview listing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1082, 20854dd. Verdict: 7 findings — 2 blocking (the fold fires on Failed/Cancelled/Reverted, and the freeze it depends on is lease-gated with no durable intent), 3 non-blocking, 2 suggestions. Blocking1. A failed apply is headlined "🏁 Schema change finished" with the error collapsed out of view. The fold's only exclusion is Stopped (comment_observer.go:504), so Failed, Cancelled and Reverted all take it: the tracked comment becomes 2. The fold is owed but never recorded, and the freeze it depends on now runs after a network round-trip. The summary is published first, then Non-blocking3. The lost-claim branch can fold onto a superseded summary. 4. Two terminal paths were missed by the sweep and still end with the duplicate pair. The deferred-cutover finalizer ( 5. Three of eleven mutations survive, all of them the operator-facing copy. Rewriting the General suggestions6. The cancel test now documents the opposite of what it asserts. 7. Nested The one thing that could have broken, verified
Verified correct
This review was generated by Claude Code (claude-opus-5). |
Why this matters
Every apply currently ends with two near-identical "Schema Change Status" comments on the PR: the tracked progress comment edited to its full final rendering, and a same-titled terminal summary posted seconds later. Operators reading the bottom of the PR see the same status twice and have to work out which one is authoritative.
What it does
At terminal, the reader sees what they always did — the progress comment above, the summary as the last word at the bottom — but the progress comment now freezes into a collapsed details block pointing at the summary, the same fold shape every rotation flavor already uses, with the final per-operation status preserved inside. Internally the summary is written before the fold: the fold's body links to the summary comment (so its ID must exist first), and a crash between the two writes must leave the verdict visible on the PR — a briefly unfolded progress comment is safe and re-folds on redelivery, while a folded progress comment with no summary posted would show a finished-looking apply with no outcome anywhere.
Rendered previews:
Finished: Superseded Progress Comment (the fold)
🏁 Schema change finished — the final status is in the summary comment.
Progress while the schema change ran
Schema Change Status — Staging
Database:
testapp| Apply ID:apply-a1b2c3d4e5f6Applied by @jackjackbits at 2026-01-01 00:00:00 UTC
Status: Applied
users: 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ CompleteSummary: Completed (unchanged by this PR — shown for the pairing)
✅ Schema Change Applied — Staging
Database:
testappApply details (3 tables)
Apply ID:
apply-a1b2c3d4e5f6ordersusersproductspublishClaimedSummaryreturns the posted comment ID; on a lost claim it reads the summary marker, so a repeated terminal publish (webhook redelivery, fresh observer) re-folds toward the recorded summary instead of restoring the full rendering and undoing the earlier fold.How it moves us toward the northstar
The PR timeline is the operator's primary interface for a schema change. Ending every apply with exactly one live status record — the summary at the bottom, history folded above it — keeps that interface unambiguous as more engines and multi-operation applies land on the same comment lifecycle.
Opened by Claude (Fable 5).