chore(database): delete the migrations nothing applies - #185
Merged
Conversation
`drizzle/` held one generated migration and its snapshot, and `drizzle.config.ts` told `drizzle-kit` where to write the next one. Nothing has ever run either. Tables are created at boot by `createTables` in `src/database/tables.ts` — hand-written `CREATE TABLE IF NOT EXISTS` — and then reconciled column by column by `reconcileColumns`. No module under `src` imports drizzle's migrator. Nine months of that is visible in the drift, and it is total. The committed SQL creates `chats`, `messages`, and `providers`: three tables that exist nowhere else in the repository, left over from an API that was added the same week and has since gone. It creates none of the five that are real — `databases`, `queries`, `settings`, `spans`, `worksheets`. There is no overlap at all. Anyone reading `drizzle/0000_graceful_whizzer.sql` for the schema learns nothing true. `CONTRIBUTING.md` sent them there. Its "Database Migrations" section said to run `npx drizzle-kit generate` after a schema change, which produces another file nothing applies — so following the documented process moved the tree further from the truth than ignoring it did. That section is struck. It is not replaced here: CLAUDE.md already describes how DDL actually works, and #69 tracks saying so in CONTRIBUTING.md, which is a different change from this one. Deleted: drizzle.config.ts drizzle/0000_graceful_whizzer.sql drizzle/meta/0000_snapshot.json drizzle/meta/_journal.json along with the `drizzle-kit` devDependency, which existed only to write them. `drizzle-orm` stays — it is the query builder the whole backend runs on, and deleting the generator is not deleting the ORM. `src/database/path.ts` keeps its fallback for not running under Electron; only the sentence naming the `drizzle-kit` CLI as the caller is gone, because it is not one any more. The backend test project is, and now the comment says so. The guard is `unappliedMigrationTooling` in `src/database/migration-tooling.ts`. It answers one question — which artifacts and generator packages are kept with nothing that applies them — and the repository case feeds it what it finds in the tree rather than a list written today: tracked top-level entries matching `migrationPatterns.artifact`, manifest entries matching `.generator`, source files importing `.migrator`. A migration system is fine and no migration tooling is fine; one half without the other is the defect, so that is the only shape it fails on. The three patterns live in the module, not inline in the test, because they decide whether the guard ever fires and the guard cannot check them. On a tree with nothing left to find, a pattern that recognises too much and one that recognises too little produce the same empty, passing answer. So they are checked against the lines they have to tell apart: `drizzle.config.ts` is an artifact and `src` is not, `drizzle-kit` is a generator and `drizzle-orm` is not, an import of `drizzle-orm/libsql/migrator` is an applier and `migrateRows` is not. That is not caution after the fact. The first version had the patterns in the test and no such cases, and mutation testing found it inert: this very file quotes `'drizzle-orm/libsql/migrator'` as a fixture, and it sits under `src`, so the guard's own test was the applier that excused every artifact in the tree. Restoring the whole `drizzle/` folder left it green. The scan now skips test files — running a migration in a test is not running one at boot — and `leavesItsOwnFixtureOut` asserts it. Red first: with the artifacts still in place the repository case named `drizzle`, `drizzle.config.ts`, and `drizzle-kit`; deleting them turned it green. 14 mutants, no survivors. Restoring each artifact, restoring the devDependency, ignoring the appliers, returning either half of the answer alone, returning it unsorted, widening or narrowing each of the three patterns, emptying either walk — and the compound one, restoring the config while counting test files again, which reproduces the hole above. Closes #104.
The previous commit claimed "14 mutants, no survivors". That is wrong, and this corrects it. A fresh-context review reproduced four survivors, one of which made the guard inert against the exact thing it exists to catch. The count after this commit is 22 mutants, no survivors. **The applier pattern matched prose.** It was the bare path, so any line mentioning `drizzle-orm/libsql/migrator` counted as something that runs one — and an applier excuses every artifact in the tree. A comment in `src/database/index.ts` reading "nothing here runs drizzle-orm/libsql/migrator", or a sentence in the guard module's own documentation explaining what an applier looks like, switched the check off for the whole repository. Both are lines this codebase is likely to grow, and the second is one this module was one edit away from having. The pattern now requires `from` or `require(` and a quoted specifier; two cases pin the difference, and a `require` case pins that tightening it did not lose the other real spelling. **Nothing asserted that there was no applier.** The check passes when `unappliedMigrationTooling` finds nothing, and that has two causes: an empty tree, which is this repository's, or an applier that excuses everything without saying so. Only the first is worth passing on. The precondition is now its own case — `has nothing applying a migration to excuse what it finds` — so anything under `src` that registers as an applier fails loudly, with a message saying which half broke, instead of quietly satisfying the check below it. **Artifacts were only recognised at the top of the tree.** `drizzle-kit` writes where its config's `out` points it, and pointing it under `src` is ordinary; a folder generated there was invisible. `migrationArtifactPath` is a named, tested function now, matching a path component at any depth and returning the artifact the file belongs to. A folder whose name says nothing about drizzle — `out: './migrations'` — is still invisible, and deliberately so: reading every `.sql` file in the repository to guess whether it was generated is worse than the gap, and the pair is caught anyway, because nothing generates into that folder without a `drizzle.config.*`. **The non-vacuity check asserted a filename, not a property.** `leavesItsOwnFixtureOut` named this one test file, so narrowing the scan's exclusion to `migration-tooling.test.ts` alone survived — and the next test to quote a migrator import would have been free to make the guard inert again. It is `leavesEveryTestFileOut` now. The other half of that case walked the tree twice, which let the artifact walk be narrowed to nothing while the check went on proving the tree was read; both now read the same list. Also here: the two comments that outlived the files they named. CLAUDE.md's DDL paragraph explained how not to use a tool that is no longer in the tree, and `scripts/seed-config.ts` cited `drizzle.config.ts` as precedent for its `dotenv/config` import. Mutation testing, the four above included: 22 mutants, no survivors. Restoring each artifact, restoring the devDependency, committing a generated folder under `src/database/`, ignoring the appliers, returning either half of the answer alone, returning it unsorted, widening or narrowing each of the three patterns, flattening `migrationArtifactPath` to the top of the tree or to nothing, emptying each of the three walks — and four compound ones that restore the config while switching the applier scan off: counting test files again, prose in an ordinary module, prose in this module's own documentation, and a pattern that reads the path anywhere in a file.
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
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.
Closes #104.
drizzle/held one generated migration and its snapshot, anddrizzle.config.tstolddrizzle-kitwhere to write the next one. Nothing has ever run either — tables are created at boot bycreateTablesinsrc/database/tables.tsand then reconciled column by column byreconcileColumns, and no module undersrcimports drizzle's migrator.Nine months of that is visible in the drift, and it is total. The committed SQL creates
chats,messages, andproviders: three tables that exist nowhere else in the repository, left over from an API that was added the same week and has since gone. It creates none of the five that are real —databases,queries,settings,spans,worksheets. There is no overlap at all. Anyone readingdrizzle/0000_graceful_whizzer.sqlfor the schema learns nothing true.CONTRIBUTING.mdsent them there: its "Database Migrations" section said to runnpx drizzle-kit generateafter a schema change, so following the documented process moved the tree further from the truth than ignoring it did. That section is struck rather than rewritten — CLAUDE.md already describes how DDL actually works, and #69 tracks saying so in CONTRIBUTING.md.Deleted:
drizzle.config.ts,drizzle/0000_graceful_whizzer.sql,drizzle/meta/0000_snapshot.json,drizzle/meta/_journal.json, and thedrizzle-kitdevDependency that existed only to write them.drizzle-ormstays — it is the query builder the whole backend runs on, and deleting the generator is not deleting the ORM. Three comments that named the deleted files are corrected (src/database/path.ts, CLAUDE.md's DDL paragraph,scripts/seed-config.ts).The guard
unappliedMigrationToolinginsrc/database/migration-tooling.tsanswers one question: which artifacts and generator packages are kept with nothing that applies them. A migration system is fine and no migration tooling is fine; one half without the other is the defect, so that is the only shape it fails on. The repository case feeds it what it finds in the tree rather than a list written today — tracked paths whose components matchmigrationPatterns.artifact, manifest entries matching.generator, source files importing.migrator.Artifacts are recognised at any depth, because
drizzle-kitwrites where its config'soutpoints it and that is routinely somewhere undersrc. A folder whose name says nothing about drizzle —out: './migrations'— stays invisible, deliberately: reading every.sqlfile in the repository to guess whether it was generated is worse than the gap, and the pair is caught anyway, because nothing generates into that folder without adrizzle.config.*.The three patterns live in the module rather than inline in the test, because they decide whether the guard ever fires and the guard cannot check them. On a tree with nothing left to find, a pattern that recognises too much and one that recognises too little produce the same empty, passing answer. So they are checked against the lines they have to tell apart:
drizzle.config.tsis an artifact andsrcis not,drizzle-kitis a generator anddrizzle-ormis not, an import orrequireofdrizzle-orm/libsql/migratoris an applier andmigrateRowsis not — and neither is a comment that merely names the path.What review changed
The first commit's message claimed "14 mutants, no survivors". That was wrong, and the second commit says so. A fresh-context review reproduced four survivors, and one of them made the guard inert against exactly what it exists to catch:
drizzle-orm/libsql/migratorcounted as something that runs one — and an applier excuses every artifact in the tree. A comment insrc/database/index.tsreading "nothing here runs drizzle-orm/libsql/migrator", or a sentence in the guard module's own documentation explaining what an applier looks like, switched the check off for the whole repository. The pattern now requiresfromorrequire(and a quoted specifier.srcwas invisible.migrationArtifactPathis a named, tested function now.leavesItsOwnFixtureOutnamed this one test file, so narrowing the exclusion to it alone survived. It isleavesEveryTestFileOutnow, and both halves of that case read one tracked-file list rather than walking twice.That last shape is why the caution is here at all: the very first version of this test quoted
'drizzle-orm/libsql/migrator'as a fixture while sitting undersrc, so the guard's own test was the applier that excused every artifact in the tree — it passed with the wholedrizzle/folder restored.Mutation testing after the fixes: 22 mutants, no survivors — restoring each artifact, restoring the devDependency, committing a generated folder under
src/database/, ignoring the appliers, returning either half of the answer alone, returning it unsorted, widening or narrowing each of the three patterns, flatteningmigrationArtifactPathto the top of the tree or to nothing, emptying each of the three walks, and four compound ones that restore the config while switching the applier scan off.Two things the review raised that are deliberately not here
todo.md:115names the deleted config. It is a checked-off item whose "Fixed:" line already records what actually happened; editing it rewrites a record of the past rather than correcting a live instruction..claude/settings.local.jsonholds adrizzle-kitpermission entry. It is a local, gitignored-by-intent allowlist belonging to whoever's checkout it is, not repository state.Two NITs are noted and not taken: the collection-error message wording, and whether this file belongs in the
backend(node) project rather thanrenderer(jsdom) given it shells out togit.Gates
tsc -p tsconfig.backend.jsontsc -p tsconfig.renderer.json--checksqlite-adapter.test.tsWindows path, pre-existing onmain