Skip to content

refactor(sqlite-storage): rename SqliteOrigin variants for clarity#4814

Closed
NathanFlurry wants to merge 1 commit intoengine-stabilize/sqlite-invalidate-v1-passthroughfrom
engine-stabilize/sqlite-origin-rename
Closed

refactor(sqlite-storage): rename SqliteOrigin variants for clarity#4814
NathanFlurry wants to merge 1 commit intoengine-stabilize/sqlite-invalidate-v1-passthroughfrom
engine-stabilize/sqlite-origin-rename

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Copy Markdown
Member Author

NathanFlurry commented Apr 27, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown

claude Bot commented Apr 27, 2026

Code Review: refactor(sqlite-storage): rename SqliteOrigin variants for clarity

Overview

This PR renames two SqliteOrigin enum variants for clarity:

  • NativeCreatedOnV2: removes the ambiguous "native" label, making it explicit that this actor was created directly in v2 storage format
  • MigratingFromV1MigrationFromV1InProgress: better communicates the transient/lease-held state during migration

MigratedFromV1 is unchanged (already clearly named). Changes are applied consistently across all production code, tests, and comments.


Correctness: Backward Compatibility

The most important question for a rename on a #[derive(Serialize, Deserialize)] enum is whether existing serialized data will still decode correctly. Since serde_bare encodes enum variants by their integer index (not by name), renaming is safe as long as variant order is preserved.

Original and new orderings both maintain:

0 → CreatedOnV2    (was Native)
1 → MigratedFromV1
2 → MigrationFromV1InProgress  (was MigratingFromV1)

Backward compatibility is preserved. ✓


Code Quality

  • Naming improvement is genuine. CreatedOnV2 is much clearer than Native (native to what?), and MigrationFromV1InProgress clearly signals the transient lease state vs. a terminal state.
  • All call sites updated. Logic, tests, and inline comments all consistently use the new names.
  • Test name updated correctly. decode_db_head_defaults_legacy_rows_to_created_on_v2_origin is the right rename for decode_db_head_defaults_legacy_rows_to_native_origin.

Minor Suggestions

  1. Doc comment on the enum itself — The diff shows only the tail of the SqliteOrigin doc comment (lifecycle guarantee instead of file locking or generation ownership fencing). If the full comment names the Native variant explicitly anywhere, it should be updated to CreatedOnV2 for consistency.

  2. PR description — The checklist and description fields are still blank. For a refactor like this, a one-liner on the motivation (e.g., "Native was confusing — it didn't convey that the actor was created on v2; MigratingFromV1 was ambiguous about whether it was past or present tense") would help future readers of git blame understand the intent.


Summary

Clean, mechanical rename with no logic changes. Serialization compatibility is preserved by maintaining variant order. The new names are meaningfully clearer. No blocking issues.

@NathanFlurry NathanFlurry marked this pull request as ready for review April 27, 2026 10:02
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch from 2d28410 to d55b8cf Compare April 27, 2026 17:35
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-invalidate-v1-passthrough branch from 02fd1d0 to 50a6834 Compare April 27, 2026 19:30
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch 2 times, most recently from 40f9e0b to 76265f2 Compare April 27, 2026 19:40
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-invalidate-v1-passthrough branch from 097bd69 to e04dbe3 Compare April 27, 2026 20:07
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch from 76265f2 to 9293029 Compare April 27, 2026 20:07
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-invalidate-v1-passthrough branch from e04dbe3 to a8b3d58 Compare April 27, 2026 20:30
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch 2 times, most recently from 2c9727e to cd5775d Compare April 27, 2026 20:48
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-invalidate-v1-passthrough branch from 5ae9672 to 22a83c9 Compare April 27, 2026 21:29
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch from cd5775d to 20a8181 Compare April 27, 2026 21:29
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch from 20a8181 to cfbcbfd Compare April 27, 2026 21:38
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-origin-rename branch from cfbcbfd to a36c978 Compare April 27, 2026 22:03
@NathanFlurry NathanFlurry force-pushed the engine-stabilize/sqlite-invalidate-v1-passthrough branch from 43eb873 to 8652d5e Compare April 27, 2026 22:03
@NathanFlurry
Copy link
Copy Markdown
Member Author

Landed in main via stack-merge fast-forward push. Commits are in main; closing to match.

@github-actions
Copy link
Copy Markdown
Contributor

Preview packages published to npm

Install with:

npm install rivetkit@pr-4814

All packages published as 0.0.0-pr.4814.107a2fc with tag pr-4814.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-107a2fc
docker pull rivetdev/engine:full-107a2fc
Individual packages
npm install rivetkit@pr-4814
npm install @rivetkit/react@pr-4814
npm install @rivetkit/rivetkit-napi@pr-4814
npm install @rivetkit/workflow-engine@pr-4814

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant