docs: state that DROP TABLE is out of scope and who owns undeclared tables

[Kiran01bm]

Adds the missing DROP TABLE row to the support matrix and documents that reconciling live tables with no desired file is the whole-schema owner's job, not the per-table diff's — including how the owner enumerates those tables and why blocking rather than quarantining is a policy choice, not an engine deduction.

Why

docs/capabilities.md is the canonical matrix for every operation, but it listed DROP COLUMN, DROP CONSTRAINT and DROP INDEX with no table-level drop, so an orchestrator author could not confirm from pg-sprite's docs that the engine never plans or executes one. The gap matters for anyone using diff per file over a directory: a live table that has no desired file is silently absent from every diff, and under a declarative model its only convergence is a drop — so whoever owns the whole schema has to enumerate live tables and surface the divergence rather than report the schema as up to date.

Review of the first cut surfaced three more gaps: "block" read as forced by the model when an orchestrator may legitimately quarantine instead; the adapter guidance borrowed a verdict vocabulary pg-sprite has no type for (there is no verdict.Verdict for a table pg-sprite never saw); and a standalone user hitting this was routed to the SchemaBot doc for a job that is theirs.

What

• capabilities.md: DROP TABLE row (⚪ out of scope, owner tooling) under whole-table operations; matrix count and ⚪ legend corrected. New Deliberately operator-owned bullet, "Dropping a table nobody declares any more": the reasoning, the two remedies (pull/export a file, or a reviewed drop), block-vs-quarantine as the owner's policy, onboarding (pull first, or scope owned schemas), and the catalog recipe with its exclusions — pg_catalog-qualified, relkind r/p, no pg_inherits children, no extension-owned relations — plus why the rolled-back pgsprite_scratch_<random> schema never appears in it.
• limitations.md: "Live tables with no desired file" row in the declarative boundaries, pointing at the operator-owned section.
• schemabot-integration.md: the fan-out bullet splits out "So is the set of tables", written in SchemaBot's own types — the adapter synthesizes an engine.TableChange (ExecutionModeBlocked, IsUnsafe, ModeReason naming the remedies; DDL may carry the canonical DROP TABLE for display only; verdict.ReasonDestructiveChange not reused) — and states the deliberate divergence from SchemaBot's MySQL path, which plans the drop and quarantines behind its unsafe acknowledgment: PostgreSQL gets the block because pg-sprite offers no whole-schema convergence and the drop is outside its statement set, so the orchestrator holds the only view of the table set; a PG quarantine (SET SCHEMA) would be the orchestrator's mechanism to add.
• README.md: "Dropping a table" bullet under What pg-sprite does not do yet, per the capability-statement rule.

Docs only; the imperative-door refusal it describes is already pinned by the existing Gate("DROP TABLE t") test. The catalog recipe was run against PostgreSQL 16 with a partition, an INHERITS child, a view and an in-schema extension present — it lists exactly the declarable tables.

Before / after

before                                    after
------                                    -----
directory of desired files                directory of desired files
  users.sql  ──diff──▶ plan                 users.sql  ──diff──▶ plan
  orders.sql ──diff──▶ plan                 orders.sql ──diff──▶ plan
  (live: legacy)   ──▶ (nothing)            (live: legacy)   ──▶ owner enumerates live tables
                                                                 (recipe in capabilities.md)
                                                                 ▶ declare it (pull/export), or
                                                                 ▶ drop through a reviewed process
                                                                 block vs quarantine = owner's policy
capabilities.md: no DROP TABLE row         capabilities.md: DROP TABLE ⚪ owner tooling
                                                           + operator-owned enumeration
README: no pointer                         README: "Dropping a table" → operator-owned

[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 — bacdcdf1 (+9/-2, 3 files)

The gap is real and worth closing: a matrix that lists DROP COLUMN, DROP CONSTRAINT and DROP INDEX with no table-level drop reads as an omission rather than a decision, and "an orchestrator author could not confirm from pg-sprite's docs that the engine never plans or executes one" is exactly the failure mode. I checked that claim rather than taking it — grep "DROP TABLE" across pkg/, internal/ and cmd/ returns only test fixtures and internal/testutil teardown; no production path emits one. And the imperative half is precisely stated: Gate(parse("DROP TABLE t")) returns verdict.ReasonUnsupportedStatement with an empty SaferIdiom (pkg/migrate/migrate_test.go:61), which is exactly "an unsupported statement kind."

The findings are about the second half — the who owns it claim, which is where the new prose makes assertions the code and the sibling repo don't support.

#	Sev	Where	What
1	med	schemabot-integration.md:118, capabilities.md:225	The doc presents "block it" as the only thing a declarative owner can do, but SchemaBot already resolves this exact case the opposite way on MySQL — and the doc never says it is choosing a divergence
2	med	schemabot-integration.md:121-123	"blocked destructive verdict" is pg-sprite's vocabulary for a row pg-sprite never produces, in a bullet list that is otherwise careful to speak SchemaBot's
3	low	capabilities.md:137, capabilities.md:292	The matrix headline this PR bumps is off by one, and the page's own operator-owned section still says "Two"

---

1 — the "only convergence" argument skips the step SchemaBot already took (med)

Both new passages make the same move:

Under a declarative model the only convergence for an undeclared table is a drop, so a whole-schema owner … must … treat each undeclared one as a blocked destructive change

The premise is right; the conclusion doesn't follow from it. "The only convergence is a drop" and "so the orchestrator blocks" are two claims, and the orchestrator this doc is written for already implements the other branch. SchemaBot's MySQL declarative path diffs the whole database as one unit and plans the drop — its own error text says so, in the repo this doc names:

ignore_namespaces is not supported for MySQL targets whose DSN names a database: the whole database is diffed as one unit, so ignored namespaces would have their live tables planned as DROP TABLE
— block/schemabot pkg/tern/local_client.go:1622

and it makes that safe rather than blocking it: pkg/pendingdrops renames the table into a _pending_drops schema with a timestamp prefix, keeps the data recoverable by RENAME TABLE for a retention period, and a background cleaner removes it after. So the same product, for the same situation, ships a reviewed process rather than deferring to one.

That makes the Postgres proposal a deliberate cross-engine divergence, and this doc is the only place it would be recorded — but it's written as a deduction, so a reader (or an adapter author) takes it as the forced answer. Two things follow:

• The divergence deserves a sentence of its own reasoning. It isn't a mechanism gap: ALTER TABLE … SET SCHEMA / RENAME TO gives Postgres the same quarantine primitive MySQL's rename does, so "no equivalent exists" isn't available as the argument. If the reason is that pg-sprite deliberately owns no whole-schema view, say that — it's a stronger reason and it's the one the existing Whole-schema convergence row (🔵, capabilities.md) already gives.
• The operator-facing consequence is worth naming. capabilities.md:225 says the drop goes "to a reviewed process" without noting that on the sibling engine the reviewed process is built in. An operator moving a root from MySQL to Postgres reads this row and reasonably expects the same behavior.

There's also a consequence of the rule as written that the doc doesn't scope: each undeclared table blocks. A schema being onboarded one table at a time is entirely undeclared tables, so the first PR declaring one table is blocked by the other N. Whether the declarative root is authoritative for the whole schema or only for what it declares is the actual design question underneath this bullet, and it's the one an adapter author will hit first.

2 — the instruction is in the wrong side's vocabulary, and hides that the row has to be invented (med)

The adapter owning the whole schema enumerates the live tables itself and surfaces each undeclared one as a blocked destructive verdict

Everywhere else this bullet list is scrupulous about which side owns which term — the sibling bullet spells it out: "DispositionRefuse maps to execution mode blocked. DispositionUnavailable … must also surface as blocked", and the verb→engine table says "map a not native-safe refusal to engine.ExecutionModeBlocked with the refusal reason as ModeReason". This sentence mixes both sides in three words, and the mix conceals the part that matters: there is no verdict to map. pg-sprite never sees the undeclared table — that's the whole premise — so no verdict.Verdict exists, and the adapter has to synthesize an engine.TableChange from nothing. That's a materially different instruction from every other mapping in this section, all of which transform something pg-sprite returned.

Synthesizing raises the question the sentence skips: what goes in the row's DDL? The engine.TableChange shape the table above names is {Table, Operation, DDL, IsUnsafe, UnsafeReason, ExecutionMode, ModeReason}. The candidate — emitting the DROP TABLE … text as the DDL so the operator sees what convergence would mean — is the one thing this PR insists pg-sprite never produces, and putting it in a blocked row is a plausible enough move that the doc should say yes or no. Spelling the instruction in SchemaBot terms answers it: the adapter synthesizes an engine.TableChange with ExecutionMode = ExecutionModeBlocked and a ModeReason naming the undeclared table; there is no pg-sprite verdict behind the row.

verdict.ReasonDestructiveChange is the nearest existing vocabulary and its doc comment scopes it narrowly — "the desired-state plan discards live structure" — to a plan pg-sprite produced. Reusing it for a table pg-sprite never planned against would widen it; worth saying explicitly that the adapter does not.

3 — the count the PR bumps is off by one, and a sibling count wasn't (low)

Counting the matrix rows between ## Support matrix and ## Peers share these limits, on this branch:

                    headline says      rows actually say
total                   52                    53
✅ T1                   17                    17  ✓
🟡 T2                   19                    20
⚪+🔵 out of scope       14                    14  ✓
❌ no mechanism          2                     2  ✓

The drift is pre-existing — origin/main says 51/19 against 52 rows and 20 🟡 — and this PR incremented both numbers correctly from a wrong base. But it's the line this PR edits, on the page whose closing rule is "any change that adds, lifts, or re-tiers a refusal must update this matrix in the same PR", so it's a one-character fix while the file is open: 53 operations: 17 … 20 planned …. (No duplicate of the count exists elsewhere in docs/, so this is the only line to touch — and a make-time assertion that recounts the rows would stop it drifting a third time.)

Two smaller ones on the same page:

• capabilities.md:292 still says "Two related jobs stay with humans on purpose." This PR adds a third — an undeclared table's drop, left to a reviewed process — and ## Deliberately operator-owned is its natural home, two bullets long and both about index ownership. A third bullet there would also give limitations.md:35 and the new matrix row a single place to point at.
• The ⚪ marker is right, but it's the one ⚪ row that qualifies the legend. ⚪ glosses as "no online-safety problem — run directly", and this row says the opposite about directly: run it through a reviewed process. The marker choice itself is correct (see Verified), so this is just a case for the legend absorbing it — "run directly, or through whatever review the object warrants".

---

Verified — the claims that hold, and an attack that dissolved

Every factual assertion in the three new passages checks out.

Claim	Check
"the imperative door [refuses it] as an unsupported statement kind"	Gate(parse("DROP TABLE t")) → verdict.ReasonUnsupportedStatement, empty SaferIdiom (pkg/migrate/migrate_test.go:61)
"pg-sprite never plans or executes it"	grep "DROP TABLE" over pkg/, internal/, cmd/: test fixtures and internal/testutil/postgres.go:146 teardown only — no production emitter
"the declarative diff is single-table scoped"	consistent with the existing Multiple tables per file row; a DROP TABLE inside a desired file is refused at parse (ErrDisallowedStatement, pkg/statement/desired_test.go:77)
link target schemabot-integration.md#the-proposed-schemabot-side-contract	resolves — docs/schemabot-integration.md:63
the arithmetic of the bumped headline	17 + 19 + 14 + 2 = 52, internally consistent (the row count is the problem, not the sum → finding 3)

⭐ Attack that dissolved: the ⚪/🔵 marker looked wrong. The new row's safety column reads "No — owner tooling, through a reviewed process", and "owner tooling" sounded like the 🔵 gloss (a different tool class owns it), which would have moved a row between buckets without changing the total. It doesn't: across the whole matrix, plain "No — owner tooling" is the ⚪ convention — 5 of the 6 pre-existing ⚪ rows use that exact phrase (enum/domain creation, views, function bodies, triggers, extensions, standalone sequences) — while every 🔵 row names a specific competing tool class ("data-change runners", "convergence planners (pg-schema-diff, pgschema, pgdelta)", "pgroll/reshape", "provisioning / IaC"). DROP TABLE has no competing tool class, so ⚪ is the right marker and the phrasing matches its neighbours.

The two new prose entries agree with each other, which is not automatic across three files: limitations.md:35 and capabilities.md:225 make the same four claims (not in diff's view · imperative refusal · only convergence is a drop · owner blocks it) with no drift in either direction, and schemabot-integration.md scopes its version to the adapter without contradicting them.

Nothing else in docs/ restates the operation count, so finding 3 is a single-line fix rather than a sweep.

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

[aparajon]

🤖 Two-lens product review — OSS adoption, then SchemaBot integration

Separate from the correctness comment above: how this reads to someone who has never heard of SchemaBot, and how it holds up as the contract pkg/engine/postgres gets built from.

Lens 1 — OSS adoption ease

The matrix row is a clear win here. "Does it drop tables?" is a first-five-minutes question for anyone evaluating a schema tool, and answering it in the canonical matrix — with the reasoning, not just a ⚪ — is better than the silence it replaces. limitations.md is also the right second home, since that page is where a reader goes after something surprised them.

Two things work against that reader, though, and both are in the routing rather than the content:

The standalone user is handed a SchemaBot doc. limitations.md:35 deliberately addresses them — "a whole-schema owner (an orchestrator, or an operator running diff per file)" — and the matrix row's only pointer for the same rule is schemabot-integration.md#the-proposed-schemabot-side-contract. That page opens with "pg-sprite does not implement or import this interface today" and is otherwise about engine.Engine, PlanResult and an adapter the reader will never write. So the one link offered for a rule that applies to every diff-per-file user lands them in someone else's integration spec. The rule is tool-agnostic; it deserves a tool-agnostic home — the ## Deliberately operator-owned section is one candidate (see finding 3 above), with the SchemaBot link kept as a for example, not as the destination.

The rule has no recipe. "Enumerate the live tables itself" is the actionable half, and nothing in docs/ shows how. That's out of character for this repo: the surrounding pages hand the reader the exact spelling — DROP INDEX CONCURRENTLY as a safer idiom, the precise GRANTs in engine-role.md, ALTER TABLE … DROP CONSTRAINT through the imperative door for the drop-and-recreate case. Four lines of pg_class SQL (with the exclusions from lens 2 below) would make this the same kind of instruction. Without it, the honest reading is "you're on your own for the part that actually takes work."

Smaller: the new limitations row is one 90-word sentence-plus-clause doing four jobs (what diff does, what the imperative door does, why a drop is the only convergence, what the owner must do). Its neighbours in that table are shorter and single-purpose. Splitting the owner's obligation into its own row — or into the operator-owned section — would keep the boundaries table scannable, which is what it's for.

Lens 2 — SchemaBot integration

The bullet assigns a job with no seam. Every other item in Adapter design notes for the Plan row names the exported entry point: diffplan.Plan, migrate.Run, migrate.RunDesired, statement.ParseDesired, dbconn.NewPool, plan.Fingerprint. This one says "enumerates the live tables itself" and there is nothing to call — schemadiff.Introspect(ctx, db, schema, table) requires a table name, and nothing in pkg/ lists a schema's relations. So the adapter writes its own catalog query, and it has to re-derive knowledge pg-sprite already holds about which relations are managed tables:

• partition children. relispartition rows are ordinary relkind = 'r' relations. A naive listing turns every partition of a declared parent into an "undeclared table" and blocks the plan — on exactly the tables the matrix already marks 🟡 for the declarative model, so the failure lands on users who are furthest from being able to fix it.
• inheritance children, same shape (INHERITS is 🟡 in the declarative model).
• diffplan.Plan's scratch objects. The doc notes the plan path "executes the desired DDL in an always-rolled-back scratch schema on the target database." A schema-scoped enumeration never sees them; a database-scoped one, run concurrently with a plan, does. That's a race an adapter author would have to know the internals to anticipate — which is the argument for the seam rather than the SQL snippet.
• non-table relkinds — matviews, sequences, foreign tables — all explicitly outside the model, all in the same catalog.

Each of those is a false positive that blocks, and blocking failures are the expensive kind: they stop a merge on a table nobody touched. An exported schemadiff.ListManagedTables(ctx, db, schema) ([]string, error) — the filter pg-sprite already has opinions about, in the package that already owns catalog reads — makes the adapter's job a call, keeps the definition of "a table pg-sprite would manage" in one place, and gives the rule somewhere concrete to point in both docs. It's also the seam the next orchestrator needs, not just this one.

One thing the bullet does well and should keep: stating that the divergence must "show on the plan and the apply is refused rather than the table lingering unreported" pins the fail-closed direction, which is the property that matters and the one an adapter would otherwise get wrong by omission. That sentence is doing real work — it's the how underneath it that's missing.

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

[aparajon]

🤖 Approving on Kiran'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).

[Kiran01bm]

🤖 Review response — created by Kiran's code review agent (Amp, Claude Opus 4.5) — pull/77, follow-up commit

All three correctness findings and lens 1 are fixed in follow-up commit c793d09d; the lens-2 library seam is a code change deferred to its own tracked follow-up.

#	Concern	Status
1 (med)	Doc presented "block" as the forced deduction from the declarative model; SchemaBot's MySQL path plans the drop and quarantines it (pending drops, behind the unsafe acknowledgment), so block-vs-quarantine is a policy choice the doc must own	fixed — schemabot-integration.md now states the divergence and its reason (pg-sprite offers no whole-schema convergence and DROP TABLE is outside its statement set, so the orchestrator holds the only view of the table set; a PG quarantine via ALTER TABLE … SET SCHEMA would be the orchestrator's mechanism to add), plus the onboarding consequence (pull first, or scope owned namespaces)
2 (med)	"blocked destructive verdict" mixed vocabularies — no verdict.Verdict exists for a table pg-sprite never saw	fixed — bullet rewritten in SchemaBot's own types: synthesize an engine.TableChange (ExecutionModeBlocked, IsUnsafe + data-loss UnsafeReason, ModeReason naming the two remedies), DDL may carry the canonical DROP TABLE for display only, verdict.ReasonDestructiveChange explicitly not reused (scoped to plans pg-sprite produced)
Lens 1	A standalone user hitting the undeclared-table gap was routed to the SchemaBot doc for guidance that is theirs	fixed — matrix row and limitations row now point to capabilities.md#deliberately-operator-owned, which gains a "Dropping a table nobody declares any more" bullet with the reasoning, the remedies, and the catalog recipe (pg_catalog-qualified, relkind r/p, excludes pg_inherits children and deptype='e' relations; notes on search_path shadowing, non-table relkinds, and why the rolled-back pgsprite_scratch_<random> schema never appears). Recipe run against PostgreSQL 16 with a partition, an INHERITS child, a view, and pg_stat_statements in-schema — lists exactly the four declarable tables. README "What pg-sprite does not do yet" gains the short-form bullet per the capability-statement rule
3 (low)	Matrix headline counts stale (52/19 vs. actual rows)	fixed — recounted: 53 rows / 17 ✅ / 20 🟡 / 14 out of scope (7 ⚪ + 7 🔵) / 2 ❌; headline and the "Two related jobs" → three in the operator-owned section corrected; ⚪ legend reworded
Lens 2	The enumeration is a natural library seam (schemadiff.ListManagedTables) rather than a SQL recipe every adapter re-derives	deferred — code change, out of scope for a docs PR. pg-sprite already has the predicate unexported in internal/cli/pull.go (listTables); the seam is to promote it, make pull call it, and have the SchemaBot adapter consume it with only its policy exemptions layered on top. Tracked as an internal follow-up (pg-sprite PR + tag first, adapter adoption after the pin bump)

Source: review comments 5550008208 and 5550008598 at head bacdcdf1.