docs: add execution-model page (autocommit-each-step, committed prefix)

[Kiran01bm]

Summary

Adds docs/execution-model.md — a human-first answer to the question every online-DDL tool must answer before anyone runs it against a table that matters: "if a multi-step schema change fails halfway, what state is my table in?" — and points to it from the README.

Why

The autocommit-each-step contract, the committed prefix, and the per-sequence partial-failure contracts are all documented today, but only in machine-consumer form scattered across plan-report.md, suggest-report.md, low-level-design.md, and the invalid-index runbook. A new user asking the plain failure-state question has no single page to read; this matters more as multi-statement surfaces grow.

What

• docs/execution-model.md: why PostgreSQL forbids a wrapping transaction for the online forms (and the two mechanical shapes behind autocommit-each-step), the committed prefix with diagrams, how the verdict/JSON/exit codes report the boundary (including the empty-executed_sql cases and the failed_step discriminator), the per-sequence partial-state → retry-path table with a safe-to-automate column, and a worked recovery example for the mid-sequence failure.
• README: a short Q&A paragraph under Commands linking the doc.
• docs/README.md: index row.

Before / after

before                                     after
┌──────────────────────────────┐           ┌──────────────────────────────┐
│ "what if it fails halfway?"  │           │ README: Q&A paragraph        │
│                              │           │   └─▶ docs/execution-model.md│
│ answer scattered across:     │           │        one page:             │
│  plan-report.md (contract)   │           │        - why no wrapping tx  │
│  suggest-report.md (caveats) │           │        - committed prefix    │
│  low-level-design.md (JSON)  │           │        - failure reporting   │
│  invalid-index-recovery.md   │           │        - partial states +    │
│                              │           │          retry paths, linking│
│ no human-first entry point   │           │          the contract docs   │
└──────────────────────────────┘           └──────────────────────────────┘

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 Adversarial correctness review, requested by @aparajon and performed by his agent. Reviewed at head c1f6d30, in a worktree, with the page's claims checked against the source that produces them and the CLI run against a live PostgreSQL 16 to compare its real output byte-for-byte with the page's examples.

Verdict: this is an excellent page and the mechanism half of it is accurate — I ran the exact scenario and the rendered verdict, JSON keys, exit code, error line, step SQL and constraint name all match what the page shows. The findings are all in one place: the page's reassurance about leftover state doesn't account for state the failed step itself leaves, and the repo's own invalid-index-recovery.md contradicts it. Nothing here is a code defect and none of it blocks merge, but finding 1 points in the unsafe direction on the page a reader reaches during an incident.

Findings

1. The opening promise is false for the INVALID index, and the page it links to says so. The lede says every state pg-sprite can leave behind is "documented, harmless to live traffic, and a step toward the desired schema — not debris." Two sections later the partial-state table lists an INVALID index (pg_index.indisvalid = false) as a reachable leftover and links invalid-index-recovery.md, which opens with the opposite claim: an invalid index is "pure cost: every write still maintains it, no query ever uses it." Both halves of the lede fail on that row — it is not harmless to live traffic (every write pays to maintain an index no read can use, and an invalid unique index still enforces uniqueness against new writes), and it is not a step toward the desired schema, since the table's own retry column says the path is DROP INDEX and rebuild, i.e. start over. The direction matters: a reader who arrives at this page mid-incident and reads the lede concludes there is nothing urgent to clean up, which is exactly wrong for the one leftover that costs them continuously. The fix is small — scope the reassurance to the committed prefix (where it is entirely true) and say plainly that a failed step can leave state of its own, with the invalid index as the named example.

2. "An empty executed_sql means … there is no partial state at all" contradicts the code's own wording, deliberately. failureVerdict sets the step-1 detail to "sequence step 1 of 4 failed; no earlier steps had committed — Code names the outcome and any state the failed step itself left". That trailing clause is doing real work: a CREATE INDEX CONCURRENTLY failing at step 1 of the USING INDEX sequence leaves an INVALID index with an empty committed prefix — precisely the shape the page tells you means "no partial state at all". I confirmed the rendering by staging the failure against a live database (competing lock held across a 4M-row table):

failed (budget-lock-exceeded)
  table:     public.users
  statement: ALTER TABLE public.users ALTER COLUMN email SET NOT NULL
  detail:    sequence step 1 of 4 failed; no earlier steps had committed — Code names the outcome and any state the failed step itself left
  failed at: step 1: ALTER TABLE "public"."users" ADD CONSTRAINT "users_email_not_null" CHECK ("email" IS NOT NULL) NOT VALID

The verdict says "check code for what the failed step left"; the page says "there is no partial state at all". The page should carry the verdict's caveat rather than round it off.

3. The same bullet's cause attribution is incomplete, which matters because the audience is automation. An empty executed_sql on a failed verdict has a second shape: failureVerdict returns early when the error is not a *SequenceStepError (a blind bounded attempt that failed for an operational reason rather than a budget), and that verdict carries detail: "execution failed; nothing committed — a started bounded attempt rolls back" with no failed_step at all. So "the failure hit step 1" is one of two readings, and the field that actually discriminates them is failed_step: absent means no sequence ran, 1 means the sequence stopped at its first step. Worth one clause, since this section is explicitly the instruction manual for consumers branching on the JSON.

4. (nit) The library bullet omits the one field its own quoted error string displays. It lists *executor.SequenceStepError's Step, Total, SQL and the underlying cause, then quotes sequence step 3 of 4 (brief) failed; …. That (brief) is Kind (StepKind) — an exported field a library caller can branch on to tell a brief catalog step from a concurrent build, which is exactly the distinction that decides whether an invalid index is possible. Naming it alongside the others closes the gap between the field list and the example directly under it.

Action items

1. (Finding 1) Scope the lede's reassurance to the committed prefix and state that a failed step can leave state of its own, naming the INVALID index — so the page and invalid-index-recovery.md agree on whether that leftover is harmless.
2. (Finding 2) Carry the verdict's own caveat into the empty-executed_sql bullet: no earlier step committed, and code names any state the failed step itself left.
3. (Finding 3) Note the second empty-executed_sql shape and give consumers the discriminator: failed_step absent means no sequence ran.
4. (optional) (Finding 4) Add Kind to the SequenceStepError field list so the quoted (brief) in the example is accounted for.

Verified (tried to break, couldn't)

Every mechanism claim on the page holds, and I checked the load-bearing ones by running the tool rather than reading it. The console block is byte-accurate against verdict.String(): the failed (<code>) headline, the table: / statement: / detail: / failed at: / committed before the failure (their state remains): labels, their column alignment, and their order all match, including the non-obvious detail that a failed sequence verdict carries no attempts: line even for a lock-budget failure — Attempts is only populated on the refusal path, so the example is right to omit it, and the JSON block is right to omit "attempts" for the same reason. I tried to break the SQL rendering by invoking the documented command exactly as written, with an unqualified users, on the theory that the page's schema-qualified "public"."users" steps could only come from a qualified input — the statement is resolved before the sequence is built, so unqualified and qualified inputs produce identical output and the page's strings match character-for-character, constraint name users_email_not_null included. The four-step SET NOT NULL sequence is exactly the four statements shown, in that order, from setNotNullSequence; the committed prefix genuinely cannot have holes because rep.Steps is appended only after a step returns without error, so ExecutedSQL is the committed prefix by construction; the failure exits 1 and a refusal exits 2 (ExitCodeRefused), which I confirmed both ways; the quoted library error line matches SequenceStepError.Error()'s template verbatim, including the step-1 variant; and all three retry-path rows match the planner's partial-failure contracts, SQLSTATEs (42710 duplicate_object, 42P07 duplicate_table) included. The admission list is accurate — unsupported shapes, partitioned parents, a defective retry policy and a too-small pool are all decided before step 1 — as is the two-shape explanation of autocommit-each-step, where the brief path really is BEGIN → SET LOCAL lock_timeout/statement_timeout → statement → COMMIT and the concurrent path really does refuse a transaction block. Both cross-doc anchors resolve, the docs/README.md index row is in the right place, and the README paragraph does not overclaim relative to the page. No leaks. All 8 checks green.

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

[aparajon]

🤖 Second pass, same head (c1f6d30), through the two lenses @aparajon asks pg-sprite changes to be judged on: how easily an outside team adopts this, and the seam an orchestrator embedding the engine consumes. Correctness findings are in the comment above; nothing here blocks.

Lens 1 — OSS adoption

This is the highest-leverage page the project has shipped, and the PR body undersells it as a consolidation. "If it fails halfway, what state is my table in?" is not one question among many — it is the objection every online-DDL tool has to answer before anyone will run it against a table that matters, and it is the question an evaluator asks in the first ten minutes and a responder asks at 2am. Until now the answer existed only as contract fragments a reader had to assemble from four documents, which in practice means the answer did not exist: nobody assembles four documents while deciding whether to trust a tool. Putting it on one page, phrased as the question a human actually asks, is worth more than the sum of the fragments.

Naming the "committed prefix" is the part that will outlive the page. A concept with a name is repeatable — it survives into the reader's own design review, their runbook, the summary they paste to their team. "Steps 1 through N−1 committed, step N rolled back, N+1 onward never attempted, no holes" is a precise invariant, and giving it a short handle is what lets someone advocate for the tool internally without re-deriving it. That is the same move the compiler-diagnostic grammar makes on the output side: borrow a vocabulary the reader can carry.

Where the page stops short of what its own audience needs: it says what state you are in, never what to type. The partial-state table's retry column is one clause per row — "Resume at VALIDATE", "Drop the invalid index, re-run the build". For the invalid-index row that is fine, because invalid-index-recovery.md is a real runbook one link away. The other two rows have no such landing. A reader who just hit a failed SET NOT NULL at step 3 knows from this page exactly where the boundary is and still has to work out for themselves that resuming means re-issuing the original --alter (which replays steps 1–2 harmlessly, per the duplicate_object contract) rather than hand-writing step 3. One worked example under the table — the failure above, then the literal command that finishes it — turns the page from a reference into the thing someone reaches for while it is happening.

And the honest sentence that is missing: nothing resumes automatically. The retry column reads as though a retry path is a feature; it is a description of what a human must do. An evaluator will ask whether the tool resumes on its own, and "no, deliberately — the engine never touches state it did not create, and here is the one-line recovery" is a much stronger answer than leaving them to infer it. Stating a limitation before it is discovered is the cheapest trust the project can buy, and this page is where it belongs.

Finally, this is the page that makes the contention demo worth building — the one I have wanted since #39 and #42. There is now a written answer for what a lock pileup leaves behind, and a GIF of pg-sprite meeting a held lock, backing off, and naming the boundary is the same story told in the medium evaluators actually watch. The lede question is the caption.

Lens 2 — the seam an orchestrator consumes

The closing paragraph is aimed squarely at embedders and it states the right contract. "Anything that executes more than one statement around the engine must report the statements already committed, the one that failed, and the ones never attempted" is exactly the obligation that propagates upward, and saying it explicitly means an orchestrator author reads it as a requirement on their surface rather than a property of pg-sprite's. That paragraph is the most reusable thing on the page.

The mapping the page gives is complete on the machine side and I checked it against the real output: executed_sql is the committed prefix by construction, failed_step / failed_step_sql the boundary, code the stable branch key, *executor.SequenceStepError the typed equivalent for in-process callers. An embedder can build correct reporting from this page alone, which is the bar.

The one thing an embedder can get wrong from this page is auto-retry, and the table hides the distinction rather than showing it. The three rows differ in a way that decides whether a retry loop is safe: re-running VALIDATE is idempotent and safe to automate; re-issuing the SET NOT NULL sequence is safe because the committed steps fail with duplicate_object and the run resumes; but the invalid-index row is emphatically not auto-retryable — the recovery starts with a DROP the engine deliberately refuses to perform, because PostgreSQL drops by name rather than identity. Those three sit in one column reading like variations on "try again". An orchestrator author who builds one retry policy from this table gets it right twice and destructive once. Making that structural — an "automatable / needs an operator" column, or a sentence naming the invalid-index row as the exception — costs a line and removes the only way I found to misuse the page.

Worth stating plainly, since the page is otherwise careful to keep consumers off prose: the partial-state table is keyed on the sequence, and no field on the verdict names which sequence ran. That is not a defect — an embedder submitted the statement, so it knows which row applies — but the page tells consumers to branch on code and never on prose, and then hands them a table that code alone does not index. One clause ("identify the row from the statement you submitted, not from the verdict") keeps the discipline intact instead of leaving a reader to notice the seam and wonder whether they are missing a field.

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

[aparajon]

🤖 Approving on @aparajon's behalf after the adversarial correctness review above. No blocking findings — every mechanism claim on the page checks out against the source and against a live run, and the four items are doc-accuracy fixes the author is trusted to fold in before merge. Action item 1 is the one worth not losing: the lede's "harmless to live traffic … not debris" is contradicted by docs/invalid-index-recovery.md for the INVALID-index row, and it points in the reassuring direction.

This stamp was left by Claude Code (claude-opus-5).

[Kiran01bm]

Review response from Kiran's (@Kiran01bm) code review assessment agent (Amp / Claude Opus 4.5)

Summary: all four correctness findings and every actionable lens item are fixed in 4109531; the contention demo is tracked as an internal follow-up; nothing rejected.

#	Finding	Status	Explanation
C1	Lede's "harmless, not debris" promise is false for the INVALID index and contradicts invalid-index-recovery.md	fixed	Reassurance is now scoped to the committed prefix; the lede states a failed step can leave state of its own, naming the INVALID index.
L2	Retry column hides the auto-retry distinction — one row is destructive to automate	fixed	Table gains a "Safe to automate?" column; the invalid-index row is named operator-only.
C2	"Empty executed_sql means no partial state at all" rounds off the verdict's own caveat	fixed	Bullet now carries the verdict's wording: no earlier step committed, and code names any state the failed step itself left.
C3	Second empty-executed_sql shape (non-sequence failure) undocumented; failed_step is the discriminator	fixed	Bullet notes both shapes: failed_step absent = no sequence ran, 1 = sequence stopped at its first step.
L1	Page says what state you're in, never what to type — no worked recovery example	fixed	Worked example added under the table: the step-3 SET NOT NULL failure, then the literal commands that finish it (--force past the re-derived sequence; duplicate_object is the double-apply guard, not a resume).
L1	Missing honest sentence: nothing resumes automatically	fixed	Stated plainly — deliberately, the engine never picks up state it did not create.
L2	Partial-state table is keyed on the sequence, which no verdict field names	fixed	Added the clause: identify the row from the statement you submitted, not from the verdict.
C4	(nit) SequenceStepError field list omits Kind, which the quoted example displays	fixed	Kind documented alongside Step/Total/SQL — it is the field that decides whether an invalid index is possible.
L1	PR body undersells the page as a consolidation	fixed	PR body reframed around the question the page answers, not the fragments it merges.
L1	Contention demo — the written answer now exists; the GIF should follow	deferred	Tracked as an internal follow-up (lock-contention demo tape).
—	"Verified (tried to break, couldn't)" + Lens 2 mapping/closing-paragraph confirmations	no action	Confirmations only — noted with thanks; the byte-for-byte live-run check is exactly the bar this page needed.