Skip to content

docs: VHS terminal demos with committed tapes and make demos - #42

Merged
Kiran01bm merged 5 commits into
mainfrom
kiran01bm/readme-vhs-demos
Aug 18, 2026
Merged

docs: VHS terminal demos with committed tapes and make demos#42
Kiran01bm merged 5 commits into
mainfrom
kiran01bm/readme-vhs-demos

Conversation

@Kiran01bm

@Kiran01bm Kiran01bm commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Adds deterministic VHS terminal recordings for the four CLI stories and embeds the improve demo in the README.

Why

The README's verbatim console samples read well but don't show the tool moving — a short animated demo of the safer-sequence substitution lands the value proposition faster for new users. Committed tape sources (the same pattern charmbracelet/gum uses) keep recordings honest and cheap to refresh: when CLI output changes, make demos re-renders every GIF from scratch against the compose database instead of someone re-recording by hand.

What

  • docs/demos/: four VHS tapes plus their rendered GIFs — improve (dry-run → real run → \d catalog proof, then the declarative loop: diff --desired plans, migrate executes, diff confirms convergence), refuse (error[rewrite-required] with typed help and the exit-code contract), lint (offline, no database), and diff-greenfield (full desired schema planned as a diagnostic report with the greenfield note). Hidden setup reseeds database state, so every tape is rerunnable.
  • make demos: re-renders all GIFs from the tapes.
  • README: embeds the improve GIF at the top of "What it looks like", leads the samples with the declarative front door (diff → improve → refuse → lint), and links the rest.

Before / after

Before                              After
┌─────────────────────────┐        ┌─────────────────────────────────────┐
│ README                  │        │ README                              │
│  static console samples │        │  animated improve demo (GIF)        │
│  (pinned by tests)      │        │  + static console samples           │
└─────────────────────────┘        └──────────────┬──────────────────────┘
                                                  │ rendered from
                                   ┌──────────────▼──────────────────────┐
                                   │ docs/demos/*.tape  (committed)      │
                                   │   make demos → docs/demos/*.gif     │
                                   │   deterministic, reseeds its own DB │
                                   └─────────────────────────────────────┘

@Kiran01bm
Kiran01bm marked this pull request as ready for review August 18, 2026 11:07
@chatgpt-codex-connector

Copy link
Copy Markdown

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

Committed .tape sources keep the demos re-renderable after CLI output
changes (the samples are pinned by renderer tests); `make demos`
re-renders all GIFs against the compose database.
Committed .tape sources keep the demos re-renderable after CLI output
changes (the samples are pinned by renderer tests); `make demos`
re-renders all GIFs against the compose database.
…mples

Two empty prompts between successive commands so each command's output
reads as its own block in the rendered GIFs (improve.tape grows taller
to fit). The hero demo continues past the catalog proof into the
declarative loop: diff --desired plans the remaining change, migrate
executes it, diff confirms convergence. README samples and the demos
table now lead with the declarative front door: diff, improve, refuse,
lint.
@aparajon

Copy link
Copy Markdown
Collaborator

🤖 Adversarial correctness review, requested by @aparajon and performed by his agent. Reviewed at head 51b9875, with every command each tape types run against a live PostgreSQL 16.14 under both the main binary and the #43 binary, so the drift below is measured rather than predicted.

Verdict: the tapes are well-built and this is the right pattern — hidden reseeding setup, committed sources, a documented re-render path — and it is safe to land. The one thing to settle before merge is order: #43 invalidates three of these four GIFs, including the README hero. Nothing here is a safety issue; the risk is shipping animated documentation that shows output the tool no longer produces, which no test in this repo can catch.

Findings

1. #43 (cli: render diff, lint, and suggest as diagnostic reports) rewrites the output three of these tapes record. Measured, same command, both binaries:

tape command on camera main after #43
diff-greenfield pg-sprite diff --desired widgets.sql 5 lines (SQL script) 29 lines (diagnostic report)
lint pg-sprite lint changes.sql 7 lines 43 lines
improve pg-sprite diff --desired users.sql ×2 -- no changes: live table matches… plan:\n public.users (PostgreSQL 16.14) — no changes; …
refuse migrate --dry-run only unchanged ✅

The geometry was sized for the old output too: lint.tape declares Set Height 700 at Set FontSize 14, which is roughly 39 rows once padding is removed, and the new lint output alone is 43 lines before the prompt and the cat changes.sql section. I could not render locally to confirm the frame — vhs is not installed here — so treat the overflow as arithmetic against the declared height; the line counts are measured. Two of the tape comments and two rows of the docs/demos/README.md table also describe the pre-#43 output in prose ("planned with the migrate guard comment").

2. The README collision is textual, not only semantic. This PR moves the "Diff: declarative desired state in, executable plan out." block from below the lint sample up to just under the new GIF, carrying the old SQL-script sample with it. #43 rewrites that same block in place — new lead sentence ("classified plan out"), the diagnostic sample, the --sql pointer and the exit-2 sentence. Whichever lands second hits a conflict whose natural resolution is "keep the moved block," which silently reinstates the pre-#43 sample in the most-read file in the repo.

3. make demos has no prerequisites, so it renders against whatever is on PATH. The new target is bare demos:, while demo: eleven lines below it is demo: build db-up demo-seed. Every tape starts with export PATH="$(git rev-parse --show-toplevel)/bin:$PATH", so on a fresh clone — or after make clean, which deletes bin/make demos records pg-sprite: command not found into four committed GIFs and vhs still exits 0. This is the same fail-open shape as the smoke-test assertion on #39: the artifact is produced, and nothing checks that what it captured is the tool. demos: build db-up costs one line.

4. "the samples are pinned by renderer tests" does not hold for the README. docs/demos/README.md offers that parenthetical as the mechanism that tells a contributor a re-render is due. Tests do pin docs/cli-output-examples.md, docs/plan-report.md, docs/suggest-report.md and docs/postgres-online-ddl-reference.md — but grepping the test files for README.md returns nothing, and nothing pins a GIF at all. The distinction matters precisely here: a CLI output change breaks the pinned doc's test and gets fixed, while the README samples and the GIFs drift in silence. Worth saying plainly instead, so the reader knows re-rendering is a manual duty.

5. (nit) The recorded prompt bakes in the author's local path. export PS1='\n~/kiran01bm/github/pg-sprite main ' appears in all four tapes, so every GIF in the README shows one person's directory layout. A neutral ~/pg-sprite reads as the project's demo rather than someone's screen recording.

6. (nit) lint.tape writes into /tmp directly (cd /tmp, changes.sql) while the other three use mktemp -d. It is the one tape a contributor can run with no database, so it is the one most likely to be run repeatedly, and it is the one that leaves a file behind.

7. (nit) Repo weight compounds. 873 KB across the four GIFs today. Because they are binary, each re-render commits a whole new blob rather than a delta, and the stated workflow is to re-render whenever CLI output changes — which, between #39, #43 and this PR, is roughly monthly right now.

Action items

  1. (Finding 1) Decide the order explicitly and put it in the PR body: land cli: render diff, lint, and suggest as diagnostic reports; move the SQL script behind --sql #43 first, then rebase this and re-render with make demos; or land this first and add the re-render to cli: render diff, lint, and suggest as diagnostic reports; move the SQL script behind --sql #43. Either way diff-greenfield, lint and improve need re-recording, and lint.tape's Set Height needs raising for the longer report.
  2. (Finding 2) Rebase on whichever of the two lands first and re-check the moved "Diff:" block by eye — the conflict resolution that keeps this PR's version is the wrong one.
  3. (Finding 3) Make the target demos: build db-up.
  4. (Finding 4) Reword the parenthetical in docs/demos/README.md to say what is actually true: docs/cli-output-examples.md is pinned by tests, the README samples and the GIFs are not, so re-rendering is manual.
  5. (optional) Neutralize the recorded prompt (5); mktemp -d in lint.tape (6); note the compounding blob cost somewhere, or move the GIFs to a docs branch or release asset if it starts to bite (7).

Verified (tried to break, couldn't)

The tapes themselves are sound: PGSPRITE_URL is the real env tag on the shared --url flag, so the hidden export works and lint.tape's on-camera unset genuinely demonstrates the offline path rather than breaking the command; the compose container name compose-postgres-1 is stable because docker compose derives the project from the compose/ directory holding the file, not from the clone's directory name; the port matches the Makefile's PG_PORT ?= 5432 default; and the hidden setup in improve.tape and refuse.tape drops and reseeds users with a thousand rows before recording, so a re-render does not depend on what the previous one left behind — I re-ran that setup and the commands twice and got identical output both times. make demos's loop propagates failure correctly (vhs $$t || exit 1), so a broken tape stops the run rather than leaving a partial set. The desired-state file improve.tape writes line-by-line does parse — I fed the identical content through the real binary and it planned cleanly — which is the thing the tape-writing note about \n typing literally is guarding against. .PHONY gained demos. Nothing in this PR touches engine code, the JSON contracts, or any test; go build ./... and the full ./internal/cli/..., ./pkg/lint/..., ./pkg/suggest/..., ./pkg/plan/... suites pass at head, and CI is green across all 12 checks. The only claim I could not check first-hand is the rendered appearance of the GIFs, since vhs is not installed in this environment — I reviewed the tape sources and the recorded command output instead.

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

@aparajon

Copy link
Copy Markdown
Collaborator

🤖 Second pass, same head (51b9875), 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 README change available to this project, and the hero choice is right. The thing that makes someone try a schema-change tool is watching it take a statement they know is dangerous and quietly run something safer — not a feature list, not a table of dispositions. improve.tape puts exactly that above the fold: the dry run showing what would happen, the real run doing the substitution, then \d users proving it in the catalog. The catalog proof is the part most demos skip and the part a skeptical reader actually needs, because "the tool says it worked" is not evidence. Leading with it, and keeping the verbatim console samples underneath for the people who want to copy text, is the correct shape.

Moving the diff section above the improve sample is also right, and worth keeping through the rebase. Declarative desired-state-in is the story that distinguishes this from a linter, and burying it under three imperative samples undersold it.

Where I would push further: the demo you do not have is contention. Picking up the thread from #39 — every prospective adopter has personally been burned by a schema change that queued behind a lock and took the application down with it. None of the four tapes shows pg-sprite meeting a held lock and backing off rather than joining the queue. That is the one thing a reader cannot get from documentation, cannot get from a competitor's README, and cannot talk themselves out of. It is also very recordable: open a transaction in a second psql session, run the change, show the bounded lock_timeout giving up instead of piling on. If one more tape gets written, that is the one.

One consequence of finding 1 worth calling out separately. After #43, diff-greenfield.gif will show the greenfield report's apply: footer telling the viewer to run pg-sprite migrate --alter, which migrate refuses for a CREATE TABLE and answers by pointing back at pg-sprite diff --desired. That dead end is finding 1 on #43 and it has a one-line fix there — but it is worth knowing that this PR is what would put it on camera in the README, which raises the stakes on fixing it rather than deferring it.

Lens 2 — the seam an orchestrator consumes

Nothing in this PR touches the seam, which is exactly right for a docs change. No engine code, no JSON contract, no plan.Report field, no exit code. An orchestrator embedding pg-sprite is unaffected. Worth stating only because it is the property to keep true as the demo surface grows: tapes are documentation, and documentation should never become a reason to hold an interface still.

The structural gap is that the demo surface now has three tiers with three different guarantees. docs/cli-output-examples.md is pinned by tests and breaks CI on drift. demo/tour.sh (from #39) runs in CI as the smoke-test job and asserts on real output. The GIFs have nothing — and they are the tier the README leads with. That inversion is the thing to fix eventually, and there is a cheap way in: the commands the tapes type are very nearly a subset of what demo/tour.sh already exercises — dry-run, a native execution, a refusal, a diff plan. If the tapes were written to type the same commands the tour already asserts, then a CLI output change breaks the smoke test, the author is told, and re-rendering becomes a known chore attached to a failing check rather than something remembered. Rendering the GIFs in CI is the wrong answer — GIF encoding is not byte-deterministic and the job would flap — but pinning the text those commands produce is already solved in this repo twice over.

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

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Approving on @aparajon's behalf after the adversarial correctness review above (no blocking findings). One merge-order condition, not a change request: land this and #43 in a deliberate order and re-render — three of the four GIFs record output #43 rewrites. This stamp was left by Claude Code (claude-opus-5).

diff and lint now print diagnostic reports instead of the SQL script and
one-line findings, so the recorded GIFs showed output the binary no
longer produces. Tape heights grow to fit the taller reports and the
stale guard-comment references are updated.
@Kiran01bm
Kiran01bm force-pushed the kiran01bm/readme-vhs-demos branch from 51b9875 to aa45de2 Compare August 18, 2026 11:24
make demos now builds the binary and starts the compose database itself
so a fresh clone cannot record "command not found" into the GIFs; the
demos README states plainly that nothing pins the GIFs and re-rendering
is a manual duty; the recorded prompt is neutral; lint.tape cleans up
its /tmp file off camera. All four GIFs re-rendered for the new prompt.
@Kiran01bm

Copy link
Copy Markdown
Collaborator Author

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

Summary: all seven adversarial findings are resolved — the ordering question (findings 1–2) was settled by landing #43 first and rebasing/re-rendering this PR on top of it; findings 3–7 are fixed in the follow-up commit — and the two lens suggestions (a contention tape, tying the tapes to the tour-pinned command surface) are tracked as internal follow-ups.

# Finding Status Explanation
1 #43 rewrites the output three tapes record; lint.tape's height is sized for the old output fixed #43 landed first; this PR was rebased onto post-#43 main and all affected GIFs re-rendered with make demos. lint.tape height raised to 1160 for the 43-line report — the final frame shows the full report through the summary line with no clipping.
2 README "Diff:" block collision — the natural conflict resolution silently reinstates the pre-#43 SQL-script sample fixed The rebase kept #43's rewritten block ("classified plan out", the diagnostic sample, the --sql pointer and exit-2 sentence) in this PR's new position under the GIF — checked by eye per the review's warning.
3 make demos has no prerequisites, so a fresh clone records command not found into the GIFs fixed Now demos: build db-up, with a comment explaining why the prerequisites exist.
4 "the samples are pinned by renderer tests" does not hold for the README or the GIFs fixed docs/demos/README.md now says what is actually true: renderer tests pin docs/cli-output-examples.md and the contract docs, CI's smoke test asserts on demo/tour.sh, but the README samples and GIFs drift silently — re-rendering is a manual duty done in the same change that alters the output.
5 Recorded prompt bakes in the author's local path fixed All four tapes now record a neutral ~/pg-sprite main prompt; all four GIFs re-rendered.
6 lint.tape writes into /tmp and leaves a file behind fixed The tape now removes /tmp/changes.sql off camera after recording. It deliberately stays in /tmp rather than mktemp -d: lint prints the absolutized file path in its finding labels, so a mktemp dir would put /var/folders/... noise into the GIF (the path-anchoring itself is tracked as an internal follow-up, F9).
7 Repo weight compounds — each re-render commits whole new blobs fixed Documented in docs/demos/README.md: re-render only the tapes whose recorded output actually changed, and move the GIFs to release assets if the accumulated weight starts to bite.
L1 The demo you do not have is contention: a tape showing pg-sprite backing off from a held lock deferred Tracked as an internal follow-up (F8, which already carried the lock-contention demo section from the #39 review — now annotated with the tape/GIF re-raise).
L2 Greenfield apply: footer would put the migrate dead end on camera in the README fixed Fixed on the #43 side before it landed; the re-rendered diff-greenfield.gif ends at the sql: pointer with no apply: footer.
L3 Three demo tiers with three guarantees — the GIFs, the tier the README leads with, have nothing; type the tour-asserted commands so drift breaks a check deferred Tracked as an internal follow-up (F12): rewrite the tapes to type the commands demo/tour.sh already asserts on, pinning the text rather than rendering GIFs in CI.

Verified before pushing: make demos re-rendered all four GIFs from the new tapes against the compose database, final frames inspected for the neutral prompt, complete output, and no setup leakage; make demo-check passes.

Sources: adversarial review and adoption/orchestrator lenses, posted by Claude Code (claude-opus-5) on @aparajon's behalf.

@Kiran01bm
Kiran01bm merged commit 311ff5f into main Aug 18, 2026
12 checks passed
Kiran01bm added a commit that referenced this pull request Aug 18, 2026
Trims the README's duplicated console samples in favor of the animated
demos and inlines all four GIFs in the demos page.

## Why
After the demos landed (#42), the improve, refuse, and lint sections
carried both a GIF and a near-identical verbatim console block — two
renderings of the same output to keep in sync by hand. The demos page
described the recordings in a table without showing them.

## What
- README: improve/refuse/lint keep their exit-code/gating prose and
point at the GIF plus `docs/cli-output-examples.md`; the diff sample
stays as the one copyable text rendering, with
`cli-output-examples.md`'s back-pointer updated to match. The lint
section's exit-code claim now matches the code (error-severity findings
exit non-zero; warnings alone pass).
- `docs/demos/README.md`: the tape table becomes per-tape sections with
each GIF embedded.
- Tapes: all four GIFs re-rendered.

```
Before                                  After
┌───────────────────────────────┐      ┌───────────────────────────────┐
│ README                        │      │ README                        │
│  hero GIF (improve)           │      │  hero GIF (improve)           │
│  diff:    prose + console     │      │  diff:    prose + console     │
│  improve: prose + console     │      │  improve: prose ──┐           │
│  refuse:  prose + console     │      │  refuse:  prose ──┼─ links ─┐ │
│  lint:    prose + console     │      │  lint:    prose ──┘         │ │
└───────────────────────────────┘      └─────────────────────────────┼─┘
                                                                     │
┌───────────────────────────────┐      ┌─────────────────────────────▼─┐
│ docs/demos/README.md          │      │ docs/demos/README.md          │
│  table describing the tapes   │      │  per-tape sections, each GIF  │
│  (GIFs not shown)             │      │  embedded inline              │
└───────────────────────────────┘      └───────────────────────────────┘
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants