docs: document pull and add the baseline export + zero-diff demo step

[Kiran01bm]

Document pg-sprite pull and add a baseline-export + zero-diff verification step to the demo tour.

Why

The pull CLI landed without a user-facing doc, so the intended onboarding loop — export a live schema, then prove diff against the export is empty — existed only in the PR that added the command. Users adopting pg-sprite on an existing database need that loop spelled out and demonstrated, not inferred from --help.

What

• docs/pull.md: what pull enumerates, per-table results (not fail-fast), the refusal set, and the export → diff --json zero-statement verification loop; linked from README.md and docs/README.md.
• docs/cli-output-examples.md: an executed pull example.
• demo/tour.sh: run_pull runs pull, asserts the file count, then asserts diff --json returns zero statements; exposed as a pull tour mode and described in demo/README.md.

Before / after

Before
  pull CLI ──> --help only; demo tour: lint ──> diff ──> apply

After
  docs/pull.md ──> README / docs index
  demo tour:  lint ──> diff ──> apply ──> pull ──> file count ──> diff --json == 0 statements

[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 Armand and performed by his agent. Reviewed at head 00be9ab4.

Verdict: docs/pull.md is accurate everywhere I could check it against the code — every exit-code claim, flag default, and scoping statement holds — but the one snippet an adopting operator will copy-paste does not do what the sentence under it says it does. The verification loop reports success when a non-final table diverges, and git add / git commit follow immediately. Nothing else here is more than a nit; the executable version of the same check in demo/tour.sh gets it right, which is what makes the doc version worth fixing rather than reasoning about.

#	Finding	Severity
1	The documented zero-diff loop exits 0 when a non-final table produces changes	medium
2	pull's text output is now the only documented CLI output with nothing pinning it, and the intro implies it is a contract	low
3	The demo's file-count assertion silently encodes seed.sql's table count	nit
4	A fourth pin of the plan report's format_version, now in bash	nit

Findings

1. "The loop exits non-zero if any exported table produces a schema change plan" is not true of the loop as written. A for loop's exit status is the status of its last iteration, so a divergent first table followed by a clean second table leaves the loop at 0. Probed directly: a two-file loop whose first iteration returns false and whose second returns true exits 0 under both sh and bash. The snippet has no set -e — the only preceding line is the export, so it reads as a paste-and-run sequence — and git add schema / git commit -m 'Add declarative schema baseline' are the next two lines, which means an operator who follows the doc verbatim on a schema with one unrenderable-as-written table commits a baseline that does not round-trip and gets no signal at all. This is the doc's headline safety property ("verify every file against the same database before committing the baseline"), and it is the one thing in the file that a reader cannot check for themselves. demo/tour.sh handles the identical check correctly — assert_eq accumulates into failures and the script exits 1 at the end — so the fix is to give the doc the same property.

2. The intro edit promises a contract that the same PR removes the check for. docs/cli-output-examples.md now opens with "Where JSON is available, the human text rendering of the same reports is display only", whose plain reading is that where JSON is not available — i.e. pull — the text rendering is not merely display. But docs_test.go is narrowed in this PR from every console block to only --json-terminated ones, and the demo's check mode asserts pull's exit code and exported file count and nothing about its printed lines. So PULLED orders -> … and Summary: 2 pulled, 0 refused, 0 errors are captured output with no test behind them, and they will rot the first time the report's wording changes. Narrowing the regex was the right call over bumping the count to 10 and hand-writing a text comparator — the finding is only that the prose now points the other way.

3. (nit) assert_eq "pulled file count" 2 "$file_count" encodes demo/seed.sql having exactly two tables. Adding a third demo table fails with expected '2', got '3' and no pointer at the cause. One clause naming seed.sql as the source of the number turns a puzzle into a one-line fix.

4. (nit) assert_eq "diff format_version of [$desired]" 2 is a fourth place the plan report's version is pinned — after pgplan.FormatVersion, docs/plan-report.md, and the doc examples — and the first one in a shell script. Correct as a contract pin; worth knowing it is there when the plan contract next bumps, since it will not turn up in a Go-only search.

Action items

1. (Finding 1) Add set -euo pipefail to the snippet in docs/pull.md, or make the loop accumulate (ok=0; for … do … || ok=1; done; [ "$ok" = 0 ]) so the sentence beneath it is true as written.
2. (Finding 2) Either say plainly in docs/cli-output-examples.md that pull's text report is display-only and unpinned like every other text rendering, or assert its two output lines in the demo's check mode — it already captures pull's stdout there, so it is a couple of assert_eqs.
3. (Finding 3) Name demo/seed.sql in the assertion label or a comment above it.
4. (optional) (Finding 4) No change needed; a # plan report contract version comment on the line would make the pin findable.

Verified (tried to break these, couldn't)

Every checkable claim in docs/pull.md holds. The exit-code contract matches pullResultsError: it returns ErrPullFailed on the first pullStatusError before the refusal accumulator is consulted, so "if both refusals and operational errors occur, the operational error takes precedence" is true by construction, not by luck. Flag claims verified empirically from a locally built binary's pull --help: --schema="public", -o, --out="schema", $PGSPRITE_URL — all three exactly as documented. The per-table scoping question I expected to be the bug is stated correctly and explicitly ("each desired file is single-table scoped, while --schema tells diff where to find that live table"), which is why the zero-diff proof does not report a spurious drop for the tables absent from a given file. Both cross-doc anchors resolve (limitations.md § Declarative model boundaries, capabilities.md § The declarative model (desired files, diff, pull)). The docs_test.go regex narrowing is exact and fails loudly rather than silently: the doc holds 10 console blocks of which 9 end in --json, require.Len(blocks, 9) therefore pins precisely the JSON set, reverting the regex to its main form turns the test red on the count, and a future example written as --json --schema public would drop the count to 8 rather than pass quietly. fail() accumulates instead of exiting, so run_pull's rm -rf "$out_dir" still runs after a failed assertion and the tour still exits 1 on any failure — no temp-directory leak and no swallowed check. run_pull is ordered before run_exec in the all path, so the write-bearing section cannot perturb its file count. go build ./... clean at head; ./internal/cli/ doc tests green locally (1.1s). I could not run the container-backed suites — no Docker on this machine — so those rest on CI, which is fully green including the built-artifact smoke test that runs this tour.

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

[aparajon]

🤖 Two-lens product review (adoption + integration), same head 00be9ab4. Shorter than the correctness review above and separate from it on purpose.

Lens 1 — OSS adoption ease

This is the highest-leverage doc the repo was missing. "I have an existing database and I want it under declarative management" is the first question a stranger asks, and until now the answer lived in the PR that added the command. The shape is right too: what pull enumerates, what it refuses and why, the per-table (not fail-fast) result model, exit codes, and then the loop that proves the export round-trips. The refusal list in particular is what makes the tool trustworthy on first contact — it says out loud which tables it will not pretend to model.

The loop bug in finding 1 above matters more here than as a correctness issue. This audience is defined by the fact that they will paste the block rather than read it. A verification step that reports success on a divergent baseline is worse for a newcomer than no verification step, because it converts "I should check this" into "I checked this".

rm -rf schema in a paste-and-run block deserves a second look. --out defaults to schema, so the directory the doc deletes is also the conventional one, and the line sits immediately after an export the reader is expected to edit — i.e. exactly the moment attention is on the DSN and not on the working directory. For a tool whose pitch is that it fails closed, leading the onboarding recipe with an untargeted recursive delete is off-brand. pg-sprite pull --out schema.new and a rename, or just "start with an empty destination directory" as prose without the command, gets the same result without the footgun.

pull having no --json is documented honestly and is the obvious next ask. The doc is upfront about it, which is the right call over pretending otherwise. But the loop it describes is precisely what someone would want to run in CI to keep a committed baseline honest, and that consumer needs a machine-readable per-table result rather than PULLED/REFUSED/ERROR lines to grep. Worth a line in the doc pointing at that gap so a reader knows it is known.

Lens 2 — SchemaBot integration

This doc is deliberately not on SchemaBot's path, and that is the correct division. SchemaBot's Postgres schema puller is built over schemadiff.Introspect / Render directly rather than by shelling out to the CLI, so nothing here changes what the engine consumes and the missing pull --json is not an integration gap for the same reason.

The useful integration contribution is the named guarantee, not the command. Stating introspect → render → parse → diff = zero changes at the command level, and pointing at the integration tests that prove it, gives both sides one phrase for the property SchemaBot's puller actually depends on — which is the thing worth citing in a cross-repo discussion. Naming it in a user-facing doc rather than only in package comments is the part I would keep doing.

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

[aparajon]

🤖 Approving on Armand's behalf after the adversarial correctness review above. The findings there are yours to pick up as follow-ups — flagging them, not gating on them.

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