Repository navigation
warp-core: one canonical delta merge for every build - #850
Conversation
The engine shipped its own merge (sort by key, compare neighbours, dedupe, then check writes to new warps) when neither test nor delta_validate was on, while merge_deltas served test and validation builds. The merge tests ran against merge_deltas, so the shipped merge was not the tested one. merge_canonical in parallel/merge.rs now holds the only implementation, generic over the writer identity: OpOrigin where builds record origins, () where they do not. merge_deltas and the shipped engine path both call it. The release-only helpers find_write_to_new_warp and check_write_to_new_warp are removed. Unit tests run the core both ways on the same inputs and check they agree. CI now runs the six delta_validate merge test targets, which no job ran before. Fixes #813. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
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 |
![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: 961eabc919
ℹ️ 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".
The test-warp-core job already runs these six targets, with parallel_engine_worker_invariance and the library tests, under delta_validate. Found by review of #850. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
Fixes #813: the merge the engine shipped was not the merge the tests ran.
Before.
merge_deltas(parallel/merge.rs) servedtestanddelta_validatebuilds.engine_impl.rs: sort by key, compare neighbours, dedupe, then check writes to new warps.delta_validate. CI already runs them in the stepcargo test (delta validation and the canonical delta merge), which the gates work added after the audit.After.
merge_canonicalis the only implementation. It is generic over the writer identity:OpOriginin builds that record origins,()in those that do not.merge_deltasand the shipped engine path both call it, keeping their existing error mapping and messages.find_write_to_new_warpandcheck_write_to_new_warpare gone.()and with origins on the same inputs and check they agree: identical writes collapse, divergent writes are refused, a write to a warp opened in the same tick is refused, and an open alone merges.Evidence (guarded container)
cargo test -p warp-core; integration tests link the library withoutcfg(test), so they exercise the shipped pathwarp-corewithnative_rule_bootstrap,trusted_runtime,host_testdelta_validatemerge targetscargo build --release -p warp-coredelta_validate, default,--release,delta_validatecargo fmt --checkThe test suites ran before two later fixes that affect only lint: gating the re-export to the builds that use it, and allowing
panic!in the test module. Lint and the merge unit tests were re-run after them.Fixes #813.
🤖 Generated with Claude Code