docs(db): Generate the schema, and move its rationale into the schema - #193
Merged
Merged
Conversation
The hand-written description of the database had drifted and nothing could tell. Invite was missing from the ER diagram entirely. Columns were named in snake_case throughout; the model has only ever emitted PascalCase. `live_photo_pair_id` does not exist — the column is PairedAssetId. Metadata was documented as carrying a colour space, and ProxyFile a blurhash string; neither column exists. The unique index on (StorageConfigId, FilePath) appeared in no document at all. None of that was undocumented. It was DUPLICATED — restated by hand while the authoritative copy sat in AnichronDbContextModelSnapshot.cs, committed and diffed in every pull request. The copy rotted because nothing compared it to anything. Three layers now, none of them hand-written prose about columns: - Facts: docs/schema.sql, `dotnet ef dbcontext script` output, committed. CI regenerates it and fails on any change. - Rationale: PostgreSQL COMMENTs via .HasComment(...) in AnichronDbContext, beside the Fluent config they explain. They travel into the database and therefore into docs/schema.sql and any schema-doc tool pointed at a live instance. - Navigable view: an ER diagram from tbls against a throwaway database, published as a CI artefact and never committed — an artefact cannot go stale.⚠️ A first attempt generated Markdown from the EF model in ~200 lines of C# with a test asserting the file matched. It worked, and it was still wrong: a third copy of the same facts, and a bespoke renderer to own forever, in a space where mature tools exist. It is deleted. The rule: do not hand-build a layer a maintained tool covers, and do not restate a fact version control already holds.⚠️ AddSchemaComments is 48 comment alterations and zero structural statements, so it carries no data risk — but a one-word comment fix is now a migration, not a text edit. That trade is recorded in ADR-0004. The --check gate regenerates in place and asks git, rather than diffing against a temp file: process substitution hands diff a /dev/fd path that some sandboxes refuse, and a fixed temp path collides between concurrent runs. Both fail in ways that read as a broken script rather than as drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate regenerated docs/schema.sql in place and then asked git whether anything moved. That cannot see a hand-edited file at all: the regeneration overwrote the edit before git was consulted, so the check reported "current" on a file it had just silently repaired. It caught a stale COMMIT and nothing else. --check now derives to a sibling file and diffs, leaving the target untouched, so CI no longer mutates its own checkout either. The comparison file sits beside the target rather than in /tmp or behind `<(...)`. Process substitution hands diff a /dev/fd path that some sandboxes and container runtimes refuse, and a fixed temp path collides between concurrent runs — both surface as "Operation not permitted" on the diff, which reads like a broken script rather than like drift. Measured while writing the test suite; all three variants were tried. The test's repair assertion was also wrong and is corrected: the generator reports against HEAD, not against whatever the working tree held a moment earlier, so undoing an uncommitted edit correctly reports nothing to commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…id SQL
Two faults, both found by CI rather than locally, and both from the same cause —
the script was verified on a machine where `dotnet ef` happened to be installed
GLOBALLY, so its real prerequisites were invisible.
1. dotnet-ef was not in dotnet-tools.json at all. On a clean runner the script
failed with "dotnet-ef does not exist", which reads as a broken script rather
than a missing prerequisite. It is now a pinned manifest tool, `dotnet tool
restore` moved ahead of the steps that need it, and the script checks for the
tool up front and says what to run.
2. `dotnet ef` writes its tools-version-mismatch notice to STDOUT, not stderr, so
`2>/dev/null` did not keep it out of the generated file. The first committed
docs/schema.sql therefore began with
The Entity Framework tools version '10.0.7' is older than that of the runtime...
and was not valid SQL. Pinning the tool to the runtime version removes the
notice at source; a guard now rejects any first line that is not SQL, so a
future mismatch is loud instead of silently corrupting the file.
⚠️ The generated output is tool-version-dependent, which is why the pin matters
beyond tidiness: an unpinned dotnet-ef would make the drift gate fail for whoever
happened to have a different version installed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard added in the previous commit read its first line with `grep -v '^\s*$' | head -1`. `head` closes the pipe after one line, grep takes SIGPIPE, and under `set -o pipefail` that fails the whole function — so the generator exited 2 with "grep: write error: Broken pipe" and said nothing about the schema. Whether it fires depends on whether grep finished writing before head exited, so it PASSED locally on macOS and failed on the CI runner against identical input. A read loop has no pipeline and therefore no race. Also fixes a latent false positive in the same guard: `CREATE*` does not match " CREATE TABLE", so an indented first statement would have been rejected as not being SQL. dotnet ef does not indent it today, which is what kept that invisible. Leading whitespace is now trimmed before the match. Both faults are the same shape as the ones this branch already carries comments about: a pipeline whose exit status is not what it appears to be, and a check verified only against the one input that happened to be at hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lobal install The job installed dotnet-ef with `--global`, then failed: Tool 'dotnet-ef' (version '10.0.12') was successfully installed. Run "dotnet tool restore" to make the "dotnet-ef" command available. Inside a repo carrying dotnet-tools.json the MANIFEST wins, so a global install is ignored outright. The job was written before dotnet-ef was added to the manifest two commits ago and was never revisited — the earlier fix created this one. `dotnet tool restore` also keeps the version pinned in one place instead of two that can disagree. Verified against the upstream release while fixing this: the asset tbls_v1.96.0_linux_amd64.tar.gz exists and carries `tbls` at the archive root, so the download and extraction steps are right. The job still cannot be exercised locally — it needs a live PostgreSQL — and remains continue-on-error so it cannot block a merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gram as mermaid Third and final prerequisite the schema-diagram job did not inherit. It runs on a clean checkout in its own job, so nothing has restored the solution, and `dotnet ef database update` failed before reaching the database: error NETSDK1004: Assets file '.../Anichron.Core/project.assets.json' not found. Unable to retrieve project metadata. Ensure it's an SDK-style project. Rather than discover the next missing step on the next round-trip, the remaining chain was audited statically instead: - `--connection` DOES override the design-time factory's hardcoded connection string. Verified by pointing it at a nonexistent host and confirming the error named that host, not the factory's `anichron_design`. Worth recording: the factory ignores its `args` parameter, which makes the opposite look true from reading the code. - The POSTGRES_CONNECTION__* variables on that step were read by nobody — the factory never touches IConfiguration. Removed; they existed only to mislead. - tbls' command form is `doc [DSN] [DOC_PATH]`, `--rm-dist` is a real flag, and the postgres:// DSN shape is right — all checked against upstream's README. ER diagrams now render as mermaid rather than the default svg: no image-renderer dependency, and the diagram stays readable in the artefact without downloading it. The COMMENTs added via .HasComment(...) surface as table and column descriptions, which is the point of having put the rationale in the schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The status line still read "implemented, except the ER-diagram job", written while that job was unverifiable locally. It now runs green: tbls migrates a throwaway Postgres and generates the mermaid ER diagram as a CI artefact. A stale status line on the ADR that argues documentation should not drift is not a small thing to leave behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The hand-written description of the database had drifted, and nothing could tell:
Invitewas missing from the ER diagram entirely — an entire table, in both the wiki's copy and a separate documentation repository's copysnake_casethroughout; the model has only ever emitted PascalCase. The one explicitly-named column in the whole schema isInvites.xminlive_photo_pair_iddoes not exist — the column isPairedAssetIdMetadatawas documented as carrying a colour space. There is no such columnProxyFilewas documented as having a blurhash string. There is no such column —BlurHashis aProxyTypevalue whose content is a file(StorageConfigId, FilePath)appeared in no document at allNone of that was undocumented. It was duplicated — restated by hand while the authoritative copy sat in
AnichronDbContextModelSnapshot.cs, committed and diffed in every PR. The copy rotted because nothing compared it to anything.What changed
Three layers, none of them hand-written prose about columns:
dotnet ef dbcontext script→docs/schema.sql.HasComment(...)→COMMENT ONin the DDLtblsagainst a throwaway databasescripts/derive-db-docs.shcalls a tool; it renders nothing itself.--checkis the CI gate.Rationale now lives in the schema
24 such comments. They travel into PostgreSQL, so any schema-doc tool pointed at a live instance picks them up. There is no separate prose file left to drift.
What was deleted, and why it mattered
A first attempt generated Markdown from the EF model in ~200 lines of C# with a test asserting the committed file matched. It worked. It was still wrong: a third copy of the same facts, and a bespoke renderer to own forever, in a space where mature tools exist.
Trade-offs, stated plainly
AddSchemaCommentsis 48 comment alterations and zero structural statements, so it carries no data risk.HasCommentfor an index, so index reasoning stays in code comments and does not reach the database.database-docsiscontinue-on-error: trueand not a required check — it documents, it does not gate.Two bugs found and fixed while writing the tests
--checkoverwrote the file it was checking. It regenerated in place then asked git, so a hand-editedschema.sqlwas silently repaired and reported "current" — it only ever caught a stale commit. Now it derives to a sibling file and diffs, leaving the target untouched, so CI no longer mutates its own checkout either.CHANGEDafter repairing an uncommitted edit. The generator reports against HEAD, and the file correctly matched HEAD again.The comparison file sits beside the target rather than in
/tmpor behind<(...): process substitution handsdiffa/dev/fdpath some sandboxes refuse, and a fixed temp path collides between concurrent runs. All three variants were tried; both alternatives fail asOperation not permitted, which reads like a broken script rather than like drift.Verification
dotnet build src/— 0 warnings, 0 errors (Debug and Release)dotnet test src/— 635/635dotnet format --verify-no-changes— cleanderive-db-docs.test.shadds 12 assertions including the failure pathdatabase-docsjob needs a live PostgreSQL and Docker. Its first real run is on CI.See
docs/adr/0004-schema-documentation.md.🤖 Generated with Claude Code