fix(introspect): a serial column is reported, not imported as an ordinary default (#119)

[jryannel]

Fixes #119, and writes down the schema opinions the survey work kept leaning on.

The bug

Postgres expands serial into three separate things — a plain integer column, a sequence it owns, and a nextval default — and marks none of them as an identity. So attidentity is empty and the identity check in buildColumn did not see it. The default carries a regclass cast, which is not the column's own type, so stripCast had no reason to touch it either.

The column therefore arrived as an ordinary bigint whose default happened to name a sequence, the table reported clean, and the DDL it produced did not run:

FAIL create table coprocess_steps — relation "coprocess_steps_seq_seq" does not exist
FAIL index coprocess_steps_session_idx — relation "coprocess_steps" does not exist

Two failures from one cause, with no warning before the bootstrap broke. TestBuildReportsWhatItCannotRepresent already states the principle this violated: "The failure that matters is the quiet one."

The fix

Refuse the column and report it, exactly as an identity column is refused.

The column goes rather than only its default. Declaring it without the default the database has would leave every Diff proposing to add one back — the permanently-red gate stripCast was written to avoid, and the reason that function's comment says a gate red for a reason that is not real is worse than no gate.

Verified against the schema that found it

process-mining, the studio-apps deployment where this surfaced:

	before	after
DDL apply failures	2	0
fixpoint residual	1	0
coprocess_steps	clean	partial

The table moves from a false clean to an honest partial, and the schema reaches a fixpoint. Re-running the other nine deployments shows no other schema changes — 233 tables, 0 apply failures fleet-wide.

Tests cover the spellings that reach the catalog (bigserial, serial, schema-qualified, unquoted, uppercase) and, in the other direction, that a default merely mentioning a sequence-like name — 'nextval'::text, 'seq_' || id — still imports as an ordinary column.

Declaring sequences outright remains open. This fixes the lie, not the gap; identity columns are still refused too.

docs/best-practices.md

Three buckets, kept apart on purpose: opinions the DSL enforces (each citing its ADR), practices it only recommends, and refusals that are merely unbuilt. Conflating the first and third is how a missing feature gets rewritten as a principle, so the gaps are listed on the same page with their issue numbers.

Counts come from the two surveys — valiro-go (68 tables) and studio-apps (ten deployments, 233 tables).

Two things came out of writing it:

• ADR-0015 got corroboration. studio-apps documents the same prefixing rule and enforces it in CI, and it still drifted to 8 violations — whose tables are exactly the ones that collided when the survey built one database per app. A rule enforced by review drifts; a prefix applied by the registry cannot.
• ADR-0034's revisit trigger fired again. It asks for "a real schema wanting a composite key on a table that is not exposed, expandable or cursor-paged", which had one instance. studio-apps has composite primary keys in 4 of 10 apps, including agentdeploy's (deployment_id, key) configuration tables that no REST resource mounts. The ADR already names the fix — move the refusal into rest.Resource's mount check rather than lift it — and asked for the stance to be stated in the schema docs, which this does.

🤖 Generated with Claude Code