feat(nextly): rewrite the field group vocabulary stored inside rows - #430
Conversation
|
@codex please review this PR |
|
@coderabbitai review |
|
@greptileai review |
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR adds field-group storage migration support. It introduces vocabulary and content rewrite helpers, resumable batch processing with persisted cursors, ordered registry and document migration steps, verification, rollback behavior, and comprehensive Vitest coverage. ChangesField-group migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MigrationRunner
participant MigrationSession
participant MetaService
participant RegistryTables
participant ContentTables
MigrationRunner->>MigrationSession: run ordered migration steps
MigrationSession->>RegistryTables: rewrite and verify registry rows
MigrationSession->>ContentTables: rewrite schema-event scope values
MigrationSession->>MetaService: read and persist batch cursor
MigrationSession->>ContentTables: rewrite version and event documents
MigrationSession->>ContentTables: verify all documents use target vocabulary
MigrationSession->>MetaService: clear completed cursor
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
create-nextly-app
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 031ff36d14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex please review this PR |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/nextly/src/domains/field-groups/migration/__tests__/data-steps.test.ts (2)
232-234: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the whole document rather than the presence of the new key.
toContain("_fieldGroupType")passes when the new key is present. It also passes when the old key survives beside it. A rewrite that added the key instead of renaming it would not be caught here. Theverifyassertion on line 231 uses the same rewrite function as the code under test, so it does not close the gap.Compare the full rewritten document against the expected value.
💚 Proposed assertion
- expect(JSON.stringify(target.rows(table)[0]?.[property])).toContain( - "_fieldGroupType" - ); + const expected = + table === "nextly_versions" + ? { _fieldGroupType: "hero" } + : { data: { _fieldGroupType: "cta" } }; + expect(target.rows(table)[0]?.[property]).toEqual(expected);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextly/src/domains/field-groups/migration/__tests__/data-steps.test.ts` around lines 232 - 234, Replace the partial toContain assertion in the data-steps migration test with a full-document comparison against the expected rewritten value. Assert the complete target.rows(table)[0]?.[property] structure so the test verifies the old key is renamed rather than merely retaining it alongside _fieldGroupType, while leaving the existing verify flow unchanged.
174-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for the refusal path in
registryPatch.
registryPatchreturns a refusal whenreadPropertycannot readfieldsorconfigPath, andregistryDefinitionsSteprethrows it outside the transaction on line 202 ofdata-steps.ts. No test covers that path. A regression that swallows the refusal, or that throws it inside the callback and loses the context naming the table, would pass this suite.Add a world whose registry row omits
fields, then assert thatrunrejects and that the error names the table and property.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextly/src/domains/field-groups/migration/__tests__/data-steps.test.ts` around lines 174 - 203, Add a test covering the refusal path from registryPatch by creating a world whose registry row omits the fields property. Invoke registryDefinitionsStep through the existing stepNamed/run flow and assert that run rejects with an error naming both the registry table and the missing property; preserve the existing successful and unknown-table cases.packages/nextly/src/domains/field-groups/migration/__tests__/rewrite-field-definitions.test.ts (1)
54-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRewrite this comment as behavior and rationale.
The comment states what the preceding test does not cover. That is review narrative about the test suite, not a description of the behavior under test. Keep only the rule the test pins: the reference keys are property names, so a rule keyed on the property name alone would rewrite another field type's configuration.
✏️ Proposed comment
- // 🔴 The trap the whole design exists for, and the one the test above does - // NOT cover: the reference keys are property names, so a field of some other - // type carrying a property called `component` or `components` — a plugin - // type's own option, say — would be rewritten by a rule keyed on the property - // name alone. That silently rewrites another type's configuration into - // something it cannot read. + // The reference keys are property names. A field of another type may carry a + // property called `component` or `components` as its own option, so every + // rename is anchored to a node already identified as a field group.Based on learnings: test comments must describe the code and its rationale, and must not reference surrounding work such as plans, conversations, or review findings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextly/src/domains/field-groups/migration/__tests__/rewrite-field-definitions.test.ts` around lines 54 - 59, Rewrite the comment to describe only the behavior and rationale: reference keys are property names, so matching on a property name alone could rewrite a different field type’s configuration, such as a plugin option named component or components. Remove references to test coverage, preceding tests, or the broader design context.Sources: Coding guidelines, Learnings
packages/nextly/src/domains/field-groups/migration/__tests__/helpers/table-world.ts (1)
94-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider refusing clause keys this double does not model.
matchesreadswhere.andonly. AWhereClausethat carriesorornotyields an empty clause list, so every row matches. That silently widens the filter, which is the behaviour the file header argues against at lines 89-91. The unsupported-operator branch at line 114 already fails loudly, so the same treatment for unsupported clause keys keeps the double consistent.No current caller passes
orornot, so this is future-proofing rather than a live defect.♻️ Proposed guard
if (where === undefined) return true; + // Only `and` is modelled. Refuse the rest rather than treat an unmodelled + // clause as "no filter", which would match every row. + for (const key of Object.keys(where)) { + if (key !== "and") { + throw new Error(`unsupported where clause "${key}" in this double`); + } + } const clauses = where.and ?? [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextly/src/domains/field-groups/migration/__tests__/helpers/table-world.ts` around lines 94 - 116, Update matches to reject WhereClause instances containing unsupported or/not keys instead of defaulting to an empty and-match list; preserve the existing where.and matching behavior and throw a clear error consistent with the unsupported-operator branch.packages/nextly/src/domains/field-groups/migration/__tests__/rewrite-rows.test.ts (1)
230-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this test to match what it asserts.
The name says the scan "cannot see" the inserted row. The assertion at lines 248-250 says the scan returns that row's id. The comment explains the difference, but the name alone tells a reader the opposite of the verified behaviour.
The untested case is an insert that lands behind the scan while the scan is running. Keep that statement in the comment and name the test after the behaviour it proves.
♻️ Proposed rename
- // 🔴 The boundary of what the rescan can promise, made executable so it is not - // mistaken for a guarantee. Ids are random, so a row inserted behind the point - // the scan has already passed sorts before the cursor and is never visited. - // No cursor closes this - a row lock does not prevent an insert - which is why - // the migration's precondition is that these ledgers are quiesced for the run. - it("cannot see a row inserted behind the point it has already passed", async () => { + // 🔴 The boundary of what the rescan can promise. Each scan restarts from the + // beginning, so a row inserted between two scans is found wherever it sorts. + // A row inserted behind the point a running scan has already passed is not: + // ids are random, so it sorts before the cursor and is never visited. No + // cursor closes that - a row lock does not prevent an insert - which is why + // the migration's precondition is that these ledgers are quiesced for the run. + it("restarts each scan, so a row inserted before the lowest id is still found", async () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nextly/src/domains/field-groups/migration/__tests__/rewrite-rows.test.ts` around lines 230 - 251, Rename the test case currently named “cannot see a row inserted behind the point it has already passed” to describe the behavior it verifies: a row inserted after the scan completes is found because the scan restarts from the beginning. Keep the existing test logic and comments, including the distinction that inserts during an active scan remain untested.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/nextly/src/domains/field-groups/migration/__tests__/helpers/table-world.ts`:
- Around line 77-92: Update the tableOf, requireColumn, and the additional
helper error path at the referenced throw sites to use the appropriate
NextlyError static factories instead of bare Error, importing NextlyError from
the repository’s errors module. Preserve the existing messages and
reclassification behavior; only retain bare errors if a header comment
explicitly documents that exact adapter-mirroring requirement.
In `@packages/nextly/src/domains/field-groups/migration/data-steps.ts`:
- Around line 186-198: Update the transaction in the migration flow around
readRegistryRows and ctx.update to lock the selected registry rows, matching the
{ lock: true } protection used by the ledger path in rewrite-rows.ts. Ensure the
read→patch→update sequence is serialized within the existing transaction so
concurrent schema saves cannot overwrite changes; if ctx.select cannot provide
row locks, use the available serialization or atomic compare-and-swap mechanism
instead.
In
`@packages/nextly/src/domains/field-groups/migration/rewrite-field-definitions.ts`:
- Around line 96-116: Update the field-group reference handling in the rewrite
routine around the single and many refKeys branches to preserve an existing
target-key value when both source and target keys are present, matching the
collision rule in rewrite-content-key.ts. Guard each setOwnProperty call so it
only writes when the canonical target key is not already present, ensuring
stored key order cannot determine the surviving reference.
---
Nitpick comments:
In
`@packages/nextly/src/domains/field-groups/migration/__tests__/data-steps.test.ts`:
- Around line 232-234: Replace the partial toContain assertion in the data-steps
migration test with a full-document comparison against the expected rewritten
value. Assert the complete target.rows(table)[0]?.[property] structure so the
test verifies the old key is renamed rather than merely retaining it alongside
_fieldGroupType, while leaving the existing verify flow unchanged.
- Around line 174-203: Add a test covering the refusal path from registryPatch
by creating a world whose registry row omits the fields property. Invoke
registryDefinitionsStep through the existing stepNamed/run flow and assert that
run rejects with an error naming both the registry table and the missing
property; preserve the existing successful and unknown-table cases.
In
`@packages/nextly/src/domains/field-groups/migration/__tests__/helpers/table-world.ts`:
- Around line 94-116: Update matches to reject WhereClause instances containing
unsupported or/not keys instead of defaulting to an empty and-match list;
preserve the existing where.and matching behavior and throw a clear error
consistent with the unsupported-operator branch.
In
`@packages/nextly/src/domains/field-groups/migration/__tests__/rewrite-field-definitions.test.ts`:
- Around line 54-59: Rewrite the comment to describe only the behavior and
rationale: reference keys are property names, so matching on a property name
alone could rewrite a different field type’s configuration, such as a plugin
option named component or components. Remove references to test coverage,
preceding tests, or the broader design context.
In
`@packages/nextly/src/domains/field-groups/migration/__tests__/rewrite-rows.test.ts`:
- Around line 230-251: Rename the test case currently named “cannot see a row
inserted behind the point it has already passed” to describe the behavior it
verifies: a row inserted after the scan completes is found because the scan
restarts from the beginning. Keep the existing test logic and comments,
including the distinction that inserts during an active scan remain untested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c9c06bc-f01a-409e-ac44-daeb7f8a59b5
⛔ Files ignored due to path filters (1)
.changeset/field-group-migration-data.mdis excluded by!.changeset/**
📒 Files selected for processing (15)
packages/nextly/src/domains/field-groups/migration/__tests__/batch-cursor.test.tspackages/nextly/src/domains/field-groups/migration/__tests__/data-steps.test.tspackages/nextly/src/domains/field-groups/migration/__tests__/helpers/table-world.tspackages/nextly/src/domains/field-groups/migration/__tests__/rewrite-config-path.test.tspackages/nextly/src/domains/field-groups/migration/__tests__/rewrite-content-key.test.tspackages/nextly/src/domains/field-groups/migration/__tests__/rewrite-field-definitions.test.tspackages/nextly/src/domains/field-groups/migration/__tests__/rewrite-rows.test.tspackages/nextly/src/domains/field-groups/migration/batch-cursor.tspackages/nextly/src/domains/field-groups/migration/data-steps.tspackages/nextly/src/domains/field-groups/migration/manifest.tspackages/nextly/src/domains/field-groups/migration/rewrite-config-path.tspackages/nextly/src/domains/field-groups/migration/rewrite-content-key.tspackages/nextly/src/domains/field-groups/migration/rewrite-field-definitions.tspackages/nextly/src/domains/field-groups/migration/rewrite-rows.tspackages/nextly/src/domains/field-groups/migration/set-own-property.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be88c3cb1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 152aa95a48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex please review this PR |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
|
@codex please review this PR |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
…write definitions MIGRATION_TARGET carried only the three names B1-3 needed, so the remaining storage spellings had no counterpart anywhere. Each one now sits beside the value it replaces, which is what lets a plan and the rewrite that applies it be checked against each other. The definition rewrite is anchored to nodes it has already identified as field groups, because the same word is both the value of a type and the name of a property on that node: a rule keyed on property names alone would rewrite another field type's own options, and one keyed on strings would rewrite author content. Only the two container types are descended into, since a plugin type may carry its own fields option as private configuration. The legacy reference key is normalised onto the canonical one rather than renamed, so the compatibility spelling is retired instead of gaining a new name that nothing writes.
…th records The value is composed as dir/slug.ts, so the segment being renamed is always the leading one. Anchoring to the start and to a following separator is what keeps a project directory called my-components, or the same word deeper in a path, from being rewritten into somewhere that does not exist. A value that does not start with the segment is returned unchanged. This runs over rows written by older versions, and a path it does not recognise is one it has no business reshaping.
Deliberately not the same function as the definition rewrite. A definition is a structure this codebase authored, so it can be walked by node kind with every rewrite anchored to a node already identified as a field group. A snapshot or a webhook envelope is the author's data, and its shape is whatever their fields produced. The entry's current definitions cannot steer it either: a snapshot was written under the schema of its day, so a field since deleted or retyped has none, and those are exactly the oldest documents a rename most needs to reach. A guided walk would skip them while reporting success. So this targets the key itself, at any depth, keeping key order and refusing to let a stale key overwrite a target one that is already present.
A rewrite over a table that grows without bound runs in batches, so an interrupted one needs a durable position or every resume re-reads the whole table from the start. The cursor is deliberately weaker than the migration marker beside it. Starting a batch walk over is always correct, so an absent, undecodable or foreign cursor resolves to "start over" rather than to a refusal that would strand a run over a value that cannot make it wrong. It carries the run that wrote it, so a position recorded by another run - a run travelling the other way, above all - is ignored instead of adopted.
Two of the columns this migration rewrites belong to ledgers whose row counts grow with a site's activity rather than with its schema. Reading one into memory and updating it in a single transaction would make upgrade time and upgrade memory a function of how long a site has been running. Rows are walked by primary key, taking the next batch after the last id seen, and the position is recorded only once its batch has committed - so it can lag and can never lead. A cursor that could survive a batch that rolled back would step the next run past rows nothing rewrote. Completeness is not claimed from having walked the table. The adapter drops an order it cannot resolve rather than refusing, and a primary key's collation is the database's business, so the postcondition rescans instead: the worst a wrong cursor can do is fail the step, never pass one that skipped rows. A projection naming a property the table does not carry refuses too, since rewriting nothing and verifying the same absent property would agree there was nothing to do.
Wires the vocabulary rewrites into the step engine: stored field definitions across all three registries, the directory a field group's config path records, the scope a schema event carries, and the wire type key inside version snapshots and event payloads. These steps run before any rename, in both directions, and that is functional rather than stylistic. The adapter's typed CRUD refuses a table the ORM does not declare, and the field group registry is declared under its legacy name - so it is reachable that way only before its own rename. Going through the ORM is what keeps the driver's JSON encoding out of this module, where the same column is jsonb, json and text-with-a-json-mode across the three dialects. Inverting the plan puts these last on the way down, by which point the renames have restored the names they address, so the rule holds both ways. The registries move together in one transaction: the runtime builds its schema from those rows, so a half-rewritten set is a database whose entities disagree about what a field group is. The two ledgers are batched instead, and their postcondition rescans rather than trusting where the batches got to.
Lock the rows a batch is about to rewrite. A plain SELECT takes no lock on Postgres or MySQL, so a writer could commit between the read and the write, and because the whole document is written back that edit would be overwritten by the stale copy rather than merged. `nextly_versions` rewrites its coalesced autosave row in place, so the writer is real. The postcondition scan stays unlocked: it establishes a fact rather than preparing a write, and locking a ledger for the length of a scan would block every writer to no purpose. Keep an own `__proto__` key instead of losing it to the prototype setter. `JSON.parse` creates that key as an ordinary own property and every column these walks touch is parsed JSON, so plain assignment during the rebuild would drop author data the migration then persists. Carry a refusal out of the transaction as a value and raise it at the boundary. An error escaping a transaction callback is reclassified by the adapter into an unknown database error, discarding the context that names which table and property could not be read. The test double now models that reclassification, so a refusal raised in the wrong place fails a test rather than reaching an operator stripped. Also states what the postcondition scan cannot promise: a row inserted behind the point it has already passed sorts before the cursor and is never visited. No cursor closes that, so it is the entry point's job to quiesce these ledgers for the run.
The wire-key walk's docblock called the key "reserved by the storage format". A json field accepts anything JSON can represent and nothing more, so an author's own object carrying that key is valid content and the key is reserved only by convention. Records what the exposure actually is, and why a schema-guided walk is still the worse option.
…isions Stage every registry patch before issuing any of them. A refusal has to leave the transaction as a value, because an exception is reclassified at the boundary and loses its context - but a value returned from the callback COMMITS. So a row that could not be read late in the set was committing the rows rewritten before it, leaving exactly the mixed-vocabulary registries this step exists to prevent. Lock the registry rows too. A plain SELECT takes no lock on Postgres or MySQL, and the update writes the whole `fields` document back, so a schema save committing between the read and the write would be overwritten rather than merged. Decide the reference-key collision instead of letting stored key order decide it. A node carrying both the source key and a property already named the target one dropped one reference silently, and which one survived depended on the order the keys happened to be stored in. The content rewriter already decided this; the two now agree. The test double raises what production raises: a DatabaseError for a table the registry does not declare, a bare Error for a column the where builder cannot resolve. Testing refusal handling against error types the code will never meet proves nothing.
…ng too Normalising the legacy key onto the canonical one collides with a property already named the target spelling exactly as the source key does, but the legacy branch checked only for the source key. A node carrying the legacy key and the target key but not the source key therefore let whichever came first in the stored JSON decide which reference survived - the order dependence the sibling branches had just been fixed to remove.
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2e4a8f7 to
4f38a12
Compare
|
@codex please review this PR |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Slice B1-4 of the Components -> Field Groups storage migration (task 011). It rewrites what is inside rows; #423 moved the tables and columns those rows live in. Nothing calls the migration yet - the entry point is B1-5, so this ships dark.
What moves
dynamic_{collections,singles,components}.fieldstype: "component",component,components,componentSlugtype: "fieldGroup",fieldGroup,fieldGroups(legacy key retired, normalised onto the canonical one)dynamic_components.config_pathcomponents/field-groups/nextly_schema_events.scope_kindcomponentfieldGroupnextly_versions.snapshot,nextly_events.payload_componentType_fieldGroupTypeThree decisions worth reviewing
1. The data steps run before every rename, in both directions - and that is functional. The adapter's typed CRUD refuses a table the ORM does not declare (
adapter-drizzle/src/adapter.ts:776-780), and the field-group registry is declared under its legacy name. So it is reachable that way only before its own rename. Going through the ORM is what keeps the driver's JSON encoding out of this module: the same column isjsonbon Postgres,jsonon MySQL andtext({mode:"json"})on SQLite, and two of the three hand back an object where the third hands back a string. Inverting the plan puts these steps last on the way down, by which point the renames have restored the names they address - so the rule holds both ways and no step has to work out which name a table is currently under.2. The checkpoint may lag and may never lead.
nextly_versionsandnextly_eventsare ledgers whose size follows a site's history, so they are walked by primary key in bounded batches that each commit on their own. The position is written after its batch commits, carries themigrationIdthat wrote it, and is ignored if it came from any other run. Crucially,verifydoes not trust it: it rescans the whole table, so the worst a wrong cursor can do is fail the step, never pass one that skipped rows. That also covers a case the ordering cannot - MySQL's default collation is case-insensitive, so two ids differing only in case compare equal andid > cursorwould step past one.3. The three registries move in one transaction, deliberately unbatched. The runtime builds its schema from those rows, so a half-rewritten set is not partial progress - it is a database whose entities disagree about what a field group is.
nextly_eventsis batched too, though the plan called it small and bounded. It is a retention-governed ledger exactly likenextly_versions, and sizing one for growth but not the other would be a guess about which fills up first.A trap this had to design around
A projection naming a property the table does not carry comes back without that key.
rewrite(undefined)returnsundefined, so the walk writes nothing - and the postcondition, reading the same absent property, agrees there is nothing left to do. A silent no-op reporting success is the one outcome a data migration must not have, so an absent property refuses instead.Verification
pnpm build,pnpm check-types19/19,pnpm lintby exit code: all cleanpackages/nextlyrun diffed at test-name level against a freshorigin/mainrun in its own built worktree: byte-identical failing-test lists (400 failures / 405 FAIL lines both sides). The branch is purely additive - 0 deletions.One break initially did not fail - it had been placed after the loop, so the injected failure skipped it. Re-placed correctly, it failed the intended test.
Not in this PR
The down path, structural verification, the 3-dialect CI matrix on real databases, and the entry point are B1-5.
verifyhere checks each step's own postcondition; whole-migration verification comes with the runner that invokes it.Summary by CodeRabbit
New Features
Tests