feat: immutable editions - #9357
Open
joseph-isaacs wants to merge 11 commits into
Open
Conversation
A frozen edition carries a read-forever guarantee, so its encoding set must never change again. Enforcement so far was a single inline assertion pinning core2026.07.0 -- not the edition the default writer targets -- leaving three of five frozen core editions unpinned, and living in the same mutable tree as the declarations it guarded. Generate one TOML record per frozen edition, holding the identifier, the recorded min_vortex_version, and the full computed encoding set. A test keeps the records in step with EDITION_DECLARATIONS and rejects a record with no frozen edition behind it, so deleting or unfreezing a declaration fails too. Update mode never removes a file, so unfreezing cannot be laundered through the generator. Two CI checks close the loop. The generated-files job regenerates the records and fails if git is dirty, alongside the existing flatbuffers and proto generation. A new job rejects any diff that modifies, deletes, or renames a record, and any newly added edition that is not newer than its family's newest recorded edition. required_vortex_release is deliberately left out of the records: it is backfilled from compat-fixture evidence after an edition freezes, so pinning it would put that backfill in conflict with the append-only rule. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Record it in a table of its own rather than beside each encoding. It is the one fact in a record that is not fixed at freeze time -- it is backfilled from compat-fixture evidence as that evidence appears -- so giving it its own table makes a backfill purely an added line. The append-only check parses both revisions of a modified record and permits exactly that transition: the table may gain entries, but everything else must be byte-identical and a release that was already recorded may never change or be dropped. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Editions are drafts until a min_vortex_version is recorded, and a draft is
free to change, so recording only frozen editions left the drafts invisible.
Generate a record for every declared edition instead. A draft's record may
change, move, or go away with the draft; freezing turns the record into a
read-forever contract that never changes again.
The CI check reads frozen-ness from the record at the base revision, so a diff
cannot unfreeze an edition and edit it in the same change, and the generator
refuses to unfreeze a record it finds on disk.
Encoding ids in a declaration may now be paired with the release that first
read them -- `&("vortex.alp", "0.36.0")` -- which flows into
EditionInclusion::required_vortex_release and into the records. Each core
edition's members are recorded as requiring the release current when that
edition froze, checked against the published release timeline: 0.36.0
(2025-05-28), 0.40.0 (2025-06-26), 0.54.0 (2025-10-20), 0.84.0 (2026-08-07),
and 0.65.0 (2026-03-25) for core2026.07.0, whose only member vortex.variant
first shipped there. These are upper bounds under each edition's own immutable
min_vortex_version, so the check permits refining them as compat-fixture
evidence narrows them, but never dropping one.
Renamed the module and script from `frozen` to `records`, since they now cover
drafts too.
Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
The exporter belongs with the repo's other generated files, so move it to `cargo run -p xtask -- generate-editions`, alongside generate-fbs and generate-proto, and run it from the same generated-files CI job that already checks git is clean afterwards. The `#[cfg(test)]` module and its UPDATE_EDITION_RECORDS environment variable are gone; the two rules git history cannot see -- a record may not be deleted, and a frozen edition may not return to draft -- now fail the exporter itself. Drop the per-encoding release from the records. An edition declares the release it froze in, so repeating it on all 23 of core2025.05.0's members said nothing that the edition did not already say. Declarations return to plain encoding lists, EditionInclusion::required_vortex_release goes back to being unset until there is per-encoding evidence to record, and the append-only check simplifies to rejecting any change at all to a frozen record. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Contributor
Author
|
Thoughts on using CODEOWNERS for these files? |
Merging this PR will degrade performance by 2.52%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | compress_fsst[(1000, 64, 8)] |
1 ms | 1.1 ms | -11.31% |
| ❌ | Simulation | take[small_m/shuffled/primitive/nonnull/chunks=16384/indices=16] |
1.1 ms | 1.2 ms | -11.19% |
| ⚡ | Simulation | cold_misaligned[(64, 256)] |
5.1 ms | 4.4 ms | +17.6% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/immutable-core-editions-e2zybh (c5967c5) with develop (204d1d4)
Footnotes
-
89 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Families version independently, so a flat directory mixed two unrelated chronologies and left the reader to spot the family from a filename prefix. Group the records as `vortex/editions/<family>/<edition>.toml`, mirroring the declarations in `vortex/src/editions`. The exporter creates a directory per family and treats a record under the wrong one as a stray, since that is what it is. The append-only check requires a newly added record's directory to match the family its name declares, so a record cannot be filed under a family whose chronology it does not extend. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Contributor
|
what do you want to enforce with codeowners? We can maybe require +2 for changes to these? |
Contributor
|
also before we do this we should make sure editions have all the objects we care about |
AdamGS
reviewed
Aug 12, 2026
| @@ -0,0 +1,213 @@ | |||
| #!/usr/bin/env python3 | |||
AdamGS
reviewed
Aug 12, 2026
| ) | ||
|
|
||
|
|
||
| def git(*args: str) -> str: |
Contributor
There was a problem hiding this comment.
there must be a better way to do this
AdamGS
reviewed
Aug 12, 2026
Comment on lines
+53
to
+63
| def merge_base(base: str) -> str: | ||
| result = subprocess.run( | ||
| ["git", "merge-base", base, "HEAD"], capture_output=True, text=True, check=False | ||
| ) | ||
| if result.returncode != 0: | ||
| sys.exit( | ||
| f"cannot find a merge base between {base} and HEAD:\n" | ||
| f"{result.stderr.strip()}\n" | ||
| "The checkout is probably too shallow; this check needs `fetch-depth: 0`." | ||
| ) | ||
| return result.stdout.strip() |
Contributor
Author
There was a problem hiding this comment.
usually they are just wrapper but happy to do it
AdamGS
reviewed
Aug 12, 2026
| anyhow = { workspace = true } | ||
| clap = { workspace = true, features = ["derive"] } | ||
| prost-build = { workspace = true } | ||
| vortex = { workspace = true } |
Contributor
There was a problem hiding this comment.
can we just pull vortex_editions here? This will really bloat the build time of cargo xtask
AdamGS
reviewed
Aug 12, 2026
Comment on lines
+4
to
+5
| A record's mutability follows its edition. A draft is still being assembled, so its record may | ||
| change, be renamed, or be dropped. Freezing -- recording a `min_vortex_version` -- turns the |
Contributor
There was a problem hiding this comment.
What's our process to freezing an edition?
AdamGS
reviewed
Aug 12, 2026
| @@ -0,0 +1,19 @@ | |||
| # Generated by `cargo run -p xtask -- generate-editions`. | |||
Contributor
There was a problem hiding this comment.
what are unstable editions?
…with pygit2 The declarations name encodings by id string and import nothing but the types in vortex-edition, so nothing kept them in the vortex facade. Moving them makes the exporter cheap: xtask depended on vortex to reach EDITION_DECLARATIONS, which dragged 61 vortex crates and 338 packages into a build that also serves generate-fbs and generate-proto. It now pulls 4 and 81. `vortex::editions` re-exports every moved item, so its public API is unchanged, and it keeps the session wiring that does need the facade. The record check drove git through subprocess, parsing --name-status -z by hand. Read the object database with pygit2 instead: revisions resolve through revparse, renames come from the diff's own similarity detection rather than status-letter parsing, and record contents are read from the commit trees, so the check sees committed state only and never the working tree. Run it with `uv run --script`, which resolves the dependency from the script's inline metadata. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
The spec described editions as frozen sets carrying a read-forever guarantee, then named unstable2026.06.0 once in passing without saying what the family is. It is the exception to the whole document: every unstable edition is a permanent draft, it never freezes, and it is only written when the unstable_encodings feature is selected. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Contributor
Author
|
I want to make changing one of these files either +2 or specific people can approve that |
A family had no definition anywhere in the code: it was a bare string repeated on each EditionId, a directory name, and prose in two module doc comments and the spec, none of it reachable from an edition. Nothing validated the name either, so a typo minted a family of one whose editions were unordered against every real edition. Declare families explicitly. EditionFamily carries the name and what the family is for, EditionSession registers them beside editions, and validate() now rejects an edition whose family was never declared, or a family that documents nothing. The exporter writes a family.toml beside each family's editions. That record is documentation rather than a contract, so unlike an edition record it stays editable and the append-only check exempts it. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Contributor
|
you can use codeowners or you can use policy bot, the policy lives in the root of the repo |
The check lived in a Python script run by a job of its own, which meant a second language, a second dependency manager, and a second CI job for a rule that belongs beside the exporter that writes the records. Move it into `cargo run -p xtask -- check-editions` and run it in the generated-files job, next to generate-fbs, generate-proto and generate-editions. The two halves now share their layout constants rather than restating the directory name and the family file in two languages. git2 reads the object database, mirroring what pygit2 did: revisions resolve through revparse, renames come from the diff's own similarity detection, and both revisions are read from commit trees, so the check still sees committed state only. The job's checkout gains fetch-depth: 0 for the merge base. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
Two, both mine. The workspace dependency list is sorted -- taplo.toml sets reorder_keys for it -- and I appended git2 and toml wherever the edit landed, which failed lint-toml. And git2 0.20 carries RUSTSEC-2026-0183 and RUSTSEC-2026-0184, two unsoundness advisories fixed in 0.21, which failed the advisories check. Neither reproduces on a `cargo deny check licenses` alone, which is all I had run. While here: run check-editions even when regenerating the records has already failed. The two look at different things -- one at the working tree, one at history -- and a job stops at its first failing step, so a stale record was hiding whether a frozen one had also been edited. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
yamllint's quoted-strings rule requires them, matching the `if` on duckdb-ready. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
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
4 participants
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.
Currently there is no way to ensure editions persist apart from code review.
This pr add a CI check and also creates useful output that can be used to code-gen other docs