Conversation
pacquet serialized the `importers`, `packages`, `snapshots`, and per-importer/per-snapshot dependency maps in `std::HashMap` iteration order — a per-instance random seed — so two installs of the same resolution could emit byte-different `pnpm-lock.yaml` files. This diverges from pnpm, whose lockfile is canonically ordered, and produces spurious git diffs on no-op re-installs (#12117). Sort every lockfile map by its rendered key string at emit time via two `serialize_with` helpers in `serialize_yaml`, matching pnpm's `sortLockfileKeys`/`lexCompare`. Sorting by the rendered key (not a field-wise `Ord`) is load-bearing: the `@` separating `name@version` and the leading `@` of a scoped name both order differently as struct fields than as the concatenated string pnpm compares. `overrides` is the one map pnpm leaves unsorted (declaration order), so it moves from `HashMap` to `IndexMap` to preserve insertion order rather than being sorted — deterministic and faithful to pnpm. The reuse-vs-fresh equivalence test now asserts byte-for-byte parity (was a parsed-struct comparison working around the old non-determinism), and a new test guards that a no-op re-install leaves the lockfile bytes unchanged.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (3)pacquet/**/Cargo.toml📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
pacquet/**/tests/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (4)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-22T00:08:44.646ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
📚 Learning: 2026-05-20T23:23:40.606ZApplied to files:
🔇 Additional comments (13)
📝 WalkthroughWalkthroughThe PR makes ChangesLockfile determinism and byte-stability
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12120 +/- ##
==========================================
+ Coverage 87.34% 87.41% +0.06%
==========================================
Files 255 260 +5
Lines 28703 29375 +672
==========================================
+ Hits 25072 25679 +607
- Misses 3631 3696 +65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.17753773488,
"stddev": 0.186686222498313,
"median": 2.10834939198,
"user": 2.6813086800000003,
"system": 3.4833246599999996,
"min": 2.0510873584800002,
"max": 2.6471934964800004,
"times": [
2.14776205148,
2.11288342948,
2.05391287748,
2.15829035048,
2.0704396694800002,
2.0510873584800002,
2.0795935444800002,
2.35039921648,
2.6471934964800004,
2.10381535448
]
},
{
"command": "pacquet@main",
"mean": 2.0829256513800005,
"stddev": 0.03938835033056582,
"median": 2.0868610494800004,
"user": 2.6635872800000002,
"system": 3.43882536,
"min": 2.02629232948,
"max": 2.14486361248,
"times": [
2.03524125248,
2.0476763134800002,
2.14486361248,
2.06027947948,
2.1110459714800003,
2.1154741714800003,
2.11466128448,
2.08783176848,
2.02629232948,
2.0858903304800003
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6597741216000002,
"stddev": 0.009058340956507958,
"median": 0.6591102968,
"user": 0.36403121999999993,
"system": 1.33915784,
"min": 0.6498004098000001,
"max": 0.6746553648000001,
"times": [
0.6746553648000001,
0.6595735468,
0.6498004098000001,
0.6648118938,
0.6525169448000001,
0.6612072788000001,
0.6586470468000001,
0.6737532008000001,
0.6512024348000001,
0.6515730948
]
},
{
"command": "pacquet@main",
"mean": 0.7001027842000002,
"stddev": 0.03888837578654229,
"median": 0.6934296468000001,
"user": 0.37240522,
"system": 1.34042054,
"min": 0.6613669348000001,
"max": 0.7908469478000001,
"times": [
0.7132695748000001,
0.6913656068,
0.6954936868000001,
0.6963746418000001,
0.6884278658,
0.7326363188000001,
0.6625809428000001,
0.6686653218,
0.6613669348000001,
0.7908469478000001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.38840678034,
"stddev": 0.03441428883066735,
"median": 2.39803868364,
"user": 3.8208116199999993,
"system": 3.20560878,
"min": 2.32747500214,
"max": 2.44411684614,
"times": [
2.35183657314,
2.40143017514,
2.40894784614,
2.3547576011399998,
2.39464719214,
2.44411684614,
2.38684788214,
2.4119284361399997,
2.40208024914,
2.32747500214
]
},
{
"command": "pacquet@main",
"mean": 2.4353036767400003,
"stddev": 0.06378521847733426,
"median": 2.42172582414,
"user": 3.85223052,
"system": 3.2611651800000003,
"min": 2.33954486614,
"max": 2.56807620514,
"times": [
2.33954486614,
2.38687153414,
2.42957002114,
2.56807620514,
2.40028313614,
2.46952958314,
2.41388162714,
2.40122197714,
2.49105971014,
2.45299810714
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.55998744084,
"stddev": 0.017785511642964737,
"median": 1.5614981667399999,
"user": 1.7879530199999998,
"system": 1.8861212599999995,
"min": 1.53138384374,
"max": 1.5836639477399999,
"times": [
1.5655828247399999,
1.54047933374,
1.54718438374,
1.5574135087399998,
1.56881792774,
1.53138384374,
1.5493504427399998,
1.57321389574,
1.58278429974,
1.5836639477399999
]
},
{
"command": "pacquet@main",
"mean": 1.58597640384,
"stddev": 0.03764286224097454,
"median": 1.5688805287399998,
"user": 1.8234330200000002,
"system": 1.88600426,
"min": 1.55795996274,
"max": 1.67830968274,
"times": [
1.59009094374,
1.57502222874,
1.67830968274,
1.55795996274,
1.6097926567399998,
1.56225156174,
1.6033128857399999,
1.56045893974,
1.55982634774,
1.56273882874
]
}
]
} |
|
Bencher Report| Branch | pr/12120 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,388.41 ms(+1.75%)Baseline: 2,347.32 ms | 2,816.78 ms (84.79%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,559.99 ms(+2.85%)Baseline: 1,516.76 ms | 1,820.11 ms (85.71%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,177.54 ms(+4.88%)Baseline: 2,076.26 ms | 2,491.51 ms (87.40%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 659.77 ms(-0.47%)Baseline: 662.89 ms | 795.46 ms (82.94%) |
Review Summary by QodoMake pnpm-lock.yaml serialization byte-stable with canonical key ordering
WalkthroughsDescription• Sort all lockfile maps by rendered key at emit time for byte-stable output • Matches pnpm's canonical ordering via sortLockfileKeys algorithm • Preserves overrides map insertion order using IndexMap instead of HashMap • Adds tests verifying byte-identical lockfiles across reuse and re-install scenarios Diagramflowchart LR
A["HashMap maps<br/>in random order"] -->|"sorted_map helpers<br/>at emit time"| B["Sorted by rendered<br/>key string"]
B -->|"matches pnpm<br/>lexCompare"| C["Byte-stable<br/>pnpm-lock.yaml"]
D["overrides map<br/>HashMap"] -->|"changed to<br/>IndexMap"| E["Preserves user<br/>declaration order"]
E -->|"pnpm behavior"| C
C -->|"no spurious<br/>git diffs"| F["Reuse & fresh<br/>resolve identical"]
File Changes1. pacquet/crates/lockfile/src/serialize_yaml.rs
|
There was a problem hiding this comment.
Pull request overview
Makes pacquet's pnpm-lock.yaml byte-stable by emitting every lockfile map in canonical key order, matching pnpm's sortLockfileKeys. Previously, maps were serialized in HashMap iteration (random-seed) order, so re-installs of the same resolution could produce differently ordered (but content-identical) lockfiles.
Changes:
- Added
sorted_map/sorted_map_opthelpers inserialize_yamlthat sort entries by theirDisplayrendering (matching pnpm'slexCompare), and wire them onto every map field inLockfile,ProjectSnapshot,SnapshotEntry, andPackageMetadata. - Switched
overrides(inLockfileandGraphToLockfileOptions) fromHashMaptoIndexMapto preserve user declaration order — pnpm intentionally leavesoverridesout ofsortLockfileKeys. - Tightened tests: reuse-vs-fresh equivalence now asserts byte parity, and a new
reinstalling_an_unchanged_manifest_keeps_the_lockfile_byte_identicalguards no-op re-install stability.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pacquet/crates/lockfile/src/serialize_yaml.rs | Adds sorted_map/sorted_map_opt helpers that sort by rendered key string. |
| pacquet/crates/lockfile/src/lib.rs | Sorts importers/packages/snapshots on emit; switches overrides to IndexMap. |
| pacquet/crates/lockfile/src/project_snapshot.rs | Sorts specifiers/dependencies/optionalDependencies/devDependencies. |
| pacquet/crates/lockfile/src/snapshot_entry.rs | Sorts snapshot dependencies/optionalDependencies. |
| pacquet/crates/lockfile/src/package_metadata.rs | Sorts engines/peerDependencies/peerDependenciesMeta. |
| pacquet/crates/lockfile/Cargo.toml | Adds indexmap dependency. |
| pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs | Changes overrides parameter type to IndexMap. |
| pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs | Updates test helper signature for the IndexMap change. |
| pacquet/crates/cli/tests/lockfile_resolution_reuse.rs | Switches reuse-vs-fresh comparison to byte equality; adds re-install byte-stability test. |
| pacquet/crates/cli/tests/inject_workspace_packages.rs | Comment cleanup; removes stale ordering caveat. |
| pacquet/plans/LOCKFILE_RESOLUTION_REUSE.md | Marks the byte-ordering follow-up resolved. |
| Cargo.lock | Lockfile update for indexmap dep on pacquet-lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
Makes pacquet's
pnpm-lock.yamlserialization byte-stable by emitting every lockfile map in canonical key order, matching pnpm. Fixes #12117.Why
pacquet serialized the
importers,packages,snapshots, and per-importer/per-snapshot dependency maps instd::HashMapiteration order — a per-instance random seed — so two installs of the same resolution could emit byte-different lockfiles, producing spurious git diffs on no-op re-installs. pnpm's lockfile is canonically ordered (sortLockfileKeys).How
serialize_withhelpers inserialize_yaml(sorted_map/sorted_map_opt) sort each map by its rendered key string at emit time, wired onto every map field acrosslib.rs,project_snapshot.rs,snapshot_entry.rs, andpackage_metadata.rs. The in-memory maps stayHashMap(no iteration-order ripple elsewhere).Ord) is load-bearing and matches pnpm'slexCompare: the@separatingname@versionand the leading@of a scoped name both order differently as struct fields than as the concatenated string pnpm compares (react-dom@1.0.0beforereact@17.0.2;@types/nodebeforenode).overridesis the one map pnpm deliberately leaves out ofsortLockfileKeys(it keeps declaration order). Rather than sort it, the lockfile'soverridesfield moves fromHashMaptoIndexMapso the user's declaration order is preserved — deterministic and faithful to pnpm.Tests
reinstalling_an_unchanged_manifest_keeps_the_lockfile_byte_identicalguards that a no-op re-install leaves the lockfile bytes unchanged.Verification
cargo check --workspace --all-targets, clippy (-D warnings),cargo dylint,typos, andcargo fmt/taploall clean.pacquet-lockfile(183),pacquet-package-manager(381), and the reuse + inject CLI tests pass.pacquet-only (pnpm already orders canonically), so no TypeScript change and no changeset.
Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
Bug Fixes
Tests