perf(pacquet): reuse lockfile resolutions on re-resolution#12113
Conversation
📝 WalkthroughWalkthroughThis PR implements lockfile-resolution reuse in pacquet: a feature that avoids re-resolving unchanged dependency trees during non-frozen installs by reusing the prior lockfile's recorded resolution and snapshot graph when the locked version satisfies the current manifest specifier and update targets are not involved. ChangesLockfile-Resolution Reuse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 #12113 +/- ##
==========================================
+ Coverage 87.03% 87.38% +0.34%
==========================================
Files 252 256 +4
Lines 28205 29070 +865
==========================================
+ Hits 24548 25402 +854
- Misses 3657 3668 +11 ☔ 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.10408862798,
"stddev": 0.09597730381986544,
"median": 2.0974232854799997,
"user": 2.62746916,
"system": 3.429632699999999,
"min": 2.01652245998,
"max": 2.31769452498,
"times": [
2.21136569898,
2.1078725879799998,
2.03685020398,
2.02818033498,
2.09797070998,
2.1110220219799998,
2.31769452498,
2.01652245998,
2.09687586098,
2.0165318759799997
]
},
{
"command": "pacquet@main",
"mean": 2.08644860228,
"stddev": 0.06106755388635291,
"median": 2.08779209748,
"user": 2.6866411599999998,
"system": 3.3964288,
"min": 2.0105681879799997,
"max": 2.18789522998,
"times": [
2.18789522998,
2.10090311898,
2.15193930498,
2.0346494859799997,
2.0105681879799997,
2.10583060998,
2.0386637899799998,
2.01867093698,
2.14068428198,
2.07468107598
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6794575914800001,
"stddev": 0.03734588623386306,
"median": 0.66499914878,
"user": 0.38689031999999995,
"system": 1.3396657600000001,
"min": 0.64117306978,
"max": 0.76898152678,
"times": [
0.76898152678,
0.71380873878,
0.69249491978,
0.66642026278,
0.66357803478,
0.65544894478,
0.66837979578,
0.66301968578,
0.66127093578,
0.64117306978
]
},
{
"command": "pacquet@main",
"mean": 0.66188030938,
"stddev": 0.016451937881962867,
"median": 0.6636792202799999,
"user": 0.36421761999999996,
"system": 1.34286766,
"min": 0.64344324978,
"max": 0.69924991578,
"times": [
0.69924991578,
0.66456294178,
0.66388468778,
0.6472398257799999,
0.64344324978,
0.65323830478,
0.66347375278,
0.66945715678,
0.66925508778,
0.64499817078
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.3971526469800004,
"stddev": 0.022314134397374404,
"median": 2.3950974668800002,
"user": 3.8775483399999997,
"system": 3.23987888,
"min": 2.3621028288800003,
"max": 2.42571541088,
"times": [
2.40610651788,
2.3621028288800003,
2.39090791488,
2.42391013888,
2.42571541088,
2.42146132488,
2.39412635888,
2.39606857488,
2.36971470288,
2.38141269688
]
},
{
"command": "pacquet@main",
"mean": 2.40204718908,
"stddev": 0.04730807498999655,
"median": 2.3778718218800003,
"user": 3.8893482399999995,
"system": 3.2180751800000005,
"min": 2.3542191258800003,
"max": 2.48740760188,
"times": [
2.45561053688,
2.45968054788,
2.3542191258800003,
2.39905878288,
2.3802801598800003,
2.36790413288,
2.48740760188,
2.36705721488,
2.3737903038800003,
2.3754634838800004
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.56820833156,
"stddev": 0.025032985063063933,
"median": 1.56989598456,
"user": 1.8113040999999999,
"system": 1.8682688200000002,
"min": 1.52862562506,
"max": 1.60430447106,
"times": [
1.5677750480600001,
1.55616967206,
1.52862562506,
1.58221955506,
1.5395936480599999,
1.5505785840600002,
1.60264030406,
1.5781594870600002,
1.57201692106,
1.60430447106
]
},
{
"command": "pacquet@main",
"mean": 1.56164503406,
"stddev": 0.02495774852359846,
"median": 1.5521237970600001,
"user": 1.7679203000000001,
"system": 1.8762905200000002,
"min": 1.53281589806,
"max": 1.6021413770600001,
"times": [
1.5516739620600002,
1.6021413770600001,
1.53281589806,
1.58936091406,
1.54105224006,
1.58977664906,
1.5353721930600002,
1.54905098406,
1.55257363206,
1.57263249106
]
}
]
} |
|
Bencher Report| Branch | pr/12113 |
| 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,397.15 ms(+1.79%)Baseline: 2,354.97 ms | 2,825.97 ms (84.83%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,568.21 ms(+3.14%)Baseline: 1,520.53 ms | 1,824.64 ms (85.95%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,104.09 ms(+0.87%)Baseline: 2,085.95 ms | 2,503.14 ms (84.06%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 679.46 ms(+1.72%)Baseline: 667.98 ms | 801.57 ms (84.77%) |
Port pnpm's resolvedDependencies / parentPkg.updated reuse mechanism so a non-frozen install reuses the prior lockfile's resolution and transitive subtree for dependencies that are still satisfied and not being updated, instead of re-resolving everything from the registry. resolve_node now decides reuse before the resolver chain: direct deps match the importer's recorded resolution via semver-satisfies; transitive deps take their resolved key directly from the parent's snapshot child refs. A reused node synthesizes its ResolveResult from the lockfile (cloned resolution + integrity, manifest reconstructed from the recorded peer/platform metadata) and walks its children from the snapshot graph; fresh-resolved nodes force their whole subtree to re-resolve. Reuse is conservative — registry (plain-semver) packages only, and only when the entire transitive subtree is reusable; any link:/file:/git/ tarball/non-semver shape anywhere makes the subtree fall through to a fresh resolve. pacquet update suppresses reuse for its targets (and their subtrees) via UpdateReuseScope so compatible bumps still re-resolve. See pacquet/plans/LOCKFILE_RESOLUTION_REUSE.md (Stage 3).
Review Summary by QodoPort pnpm's lockfile-resolution reuse to pacquet for performance
WalkthroughsDescription• Port pnpm's lockfile-resolution reuse to pacquet for non-frozen installs - Reuse prior lockfile's resolution + transitive subtree for unchanged dependencies - Avoid re-resolving from registry when locked version still satisfies manifest range • Implement hybrid resolve: snapshot-walk unchanged subtrees, fresh-resolve new/changed deps - Thread prior lockfile through resolver tree context - Semver-satisfies gate for direct deps; snapshot child-refs for transitive deps • Add update-suppression scope so pacquet update targets re-resolve to highest-in-range - Propagate updated flag down subtree to force re-resolution of bumped packages • Comprehensive test coverage: discriminating no-re-resolve test + structural equivalence test - Verify reused lockfile equals fresh resolve (content-identical, modulo byte-ordering) Diagramflowchart LR
A["Prior pnpm-lock.yaml"] -->|"thread into resolver"| B["WorkspaceTreeCtx"]
B -->|"semver-satisfies gate"| C["reusable_importer_dep"]
C -->|"direct deps match"| D["try_reuse_node"]
D -->|"subtree fully reusable?"| E["subtree_fully_reusable"]
E -->|"yes: snapshot walk"| F["resolve_reused_node"]
E -->|"no: fresh resolve"| G["resolve_node"]
F -->|"synthesize from lockfile"| H["ResolveResult"]
G -->|"registry resolve"| H
H -->|"merge into graph"| I["DependenciesGraph"]
J["UpdateReuseScope"] -->|"suppress reuse for targets"| D
File Changes1. pacquet/crates/cli/tests/lockfile_resolution_reuse.rs
|
There was a problem hiding this comment.
Pull request overview
This PR ports pnpm’s “lockfile-resolution reuse” behavior into pacquet so non-frozen installs can reuse already-resolved subtrees from the prior pnpm-lock.yaml (when still satisfied and not update-targeted), reducing re-resolution work and registry traffic.
Changes:
- Thread the prior lockfile (
wanted_lockfile) and an update reuse suppression scope through the workspace resolver context. - Add a lockfile reuse gate + subtree-walk path that synthesizes
ResolveResultfrom lockfile metadata and walks children from the snapshot graph. - Add unit and integration tests covering semver-satisfies reuse gating and reuse-vs-fresh structural equivalence.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pacquet/plans/LOCKFILE_RESOLUTION_REUSE.md | Design/staging plan and known follow-ups for lockfile subtree reuse. |
| pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs | Updates test workspace options to include new reuse fields. |
| pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs | Threads wanted_lockfile + update_reuse_scope into WorkspaceTreeCtx; passes importer id into importer resolution. |
| pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs | Plumbs importer id down to direct-dependency resolution so importer-level reuse can consult importer snapshots. |
| pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs | Implements reuse source threading, subtree reusability memoization, and reused-node resolution/walk. |
| pacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rs | Adds semver-satisfies gate and ResolveResult synthesis from lockfile metadata. |
| pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs | Unit tests for importer-gate semantics and synthesized result behavior. |
| pacquet/crates/resolving-deps-resolver/src/lib.rs | Wires in new module + exports UpdateReuseScope. |
| pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs | Exposes satisfies_including_prerelease for reuse gate semver checks. |
| pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs | Passes prior lockfile + update reuse scope into workspace resolve options (with packageExtensions drift guard). |
| pacquet/crates/cli/tests/lockfile_resolution_reuse.rs | New integration test proving reuse can succeed with registry unreachable; adds reuse-vs-fresh structural equivalence test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pacquet/crates/cli/tests/lockfile_resolution_reuse.rs (2)
55-58: 💤 Low valueAppending to
pnpm-workspace.yamlwithout validation.The test appends
minimumReleaseAge: 0\nto the workspace config by concatenating strings. If the existing file doesn't end with a newline, this could produce invalid YAML (though in practice the test fixture likely has a trailing newline).🛡️ Optional: ensure newline before append
let existing = fs::read_to_string(&workspace_yaml).expect("read pnpm-workspace.yaml"); - fs::write(&workspace_yaml, format!("{existing}minimumReleaseAge: 0\n")) + let separator = if existing.ends_with('\n') { "" } else { "\n" }; + fs::write(&workspace_yaml, format!("{existing}{separator}minimumReleaseAge: 0\n")) .expect("append minimumReleaseAge to pnpm-workspace.yaml");🤖 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 `@pacquet/crates/cli/tests/lockfile_resolution_reuse.rs` around lines 55 - 58, The test currently concatenates to the existing workspace file (variables workspace_yaml and existing) which can produce invalid YAML if the file doesn't end with a newline; update the write logic used with fs::write so you first ensure existing ends with a newline (e.g., check existing.ends_with('\n') and if not, add one) before appending "minimumReleaseAge: 0\n" and then write the combined string back, or alternatively parse/modify the YAML and reserialize it instead of raw string concatenation.
31-42: 💤 Low valuePotential race condition in dead port selection.
The
dead_registry_urlhelper binds an ephemeral port, reads it, then drops the listener. Between the drop and the registry connection attempt, another process could bind the same port. While unlikely in a test environment, this could cause flakiness.Consider documenting this race or accepting it as an acceptable test-environment tradeoff.
📝 Optional: document the inherent race
fn dead_registry_url() -> String { // Bind to an ephemeral port, read it, then drop the listener so the // port is (almost certainly) free again — anything that connects to - // it gets refused. + // it gets refused. Note: a race exists where another process could + // bind this port before the test connects, but in practice this is + // extremely unlikely in an isolated test environment. let listener =🤖 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 `@pacquet/crates/cli/tests/lockfile_resolution_reuse.rs` around lines 31 - 42, The helper dead_registry_url() has a potential race where the ephemeral port dropped by TcpListener could be re-bound by another process before the test connects; update the function's surrounding comment (or add a doc comment above dead_registry_url) to explicitly acknowledge this race condition and justify it as an acceptable test-environment tradeoff (or note acceptable flakiness), or alternatively implement a retry/verify strategy before returning the URL; reference dead_registry_url when making the comment or change so reviewers can find the fix quickly.
🤖 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 `@pacquet/plans/LOCKFILE_RESOLUTION_REUSE.md`:
- Around line 38-40: Update the stale caveat sentence about
readPackageHook/packageExtensions invalidation: remove or replace the “Out of
scope for the first cut” phrasing and mark that
packageExtensions/readPackageHook invalidation is implemented in this PR series
(or reference the corresponding change), so the LOCKFILE_RESOLUTION_REUSE.md
note no longer suggests it’s pending; ensure the text mentions readPackageHook
and packageExtensions explicitly and that reuse is invalidated when they change.
---
Nitpick comments:
In `@pacquet/crates/cli/tests/lockfile_resolution_reuse.rs`:
- Around line 55-58: The test currently concatenates to the existing workspace
file (variables workspace_yaml and existing) which can produce invalid YAML if
the file doesn't end with a newline; update the write logic used with fs::write
so you first ensure existing ends with a newline (e.g., check
existing.ends_with('\n') and if not, add one) before appending
"minimumReleaseAge: 0\n" and then write the combined string back, or
alternatively parse/modify the YAML and reserialize it instead of raw string
concatenation.
- Around line 31-42: The helper dead_registry_url() has a potential race where
the ephemeral port dropped by TcpListener could be re-bound by another process
before the test connects; update the function's surrounding comment (or add a
doc comment above dead_registry_url) to explicitly acknowledge this race
condition and justify it as an acceptable test-environment tradeoff (or note
acceptable flakiness), or alternatively implement a retry/verify strategy before
returning the URL; reference dead_registry_url when making the comment or change
so reviewers can find the fix quickly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a4c6d04-6105-4656-855c-44b5b79007cf
📒 Files selected for processing (11)
pacquet/crates/cli/tests/lockfile_resolution_reuse.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/plans/LOCKFILE_RESOLUTION_REUSE.md
📜 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). (9)
- GitHub Check: Agent
- GitHub Check: Code Coverage
- GitHub Check: Dylint
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/cli/tests/lockfile_resolution_reuse.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
pacquet/**/tests/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — usetempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or matching#[cfg(unix)]gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests withinstaand carefully review diffs when intentional changes alter snapshots; accept withcargo insta reviewonly after careful review
Tests that need the mocked registry should startpnprthroughpacquet-testing-utils;cargo test/cargo nextest runshould not require a separatejust registry-mock launchstep
Files:
pacquet/crates/cli/tests/lockfile_resolution_reuse.rs
🧠 Learnings (5)
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/cli/tests/lockfile_resolution_reuse.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/cli/tests/lockfile_resolution_reuse.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/cli/tests/lockfile_resolution_reuse.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
📚 Learning: 2026-05-24T21:10:50.292Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: When implementing dependency “revisit”/optional-folding in pacquet, mirror pnpm’s `resolveDependencies.ts` behavior: on revisit, update the `optional` flag only for the directly visited package, and do not automatically change `optional` for transitive descendants. pnpm corrects stale optional flags later via `copyDependencySubGraph` (a BFS in `lockfile/pruner/src/index.ts`/`copyDependencySubGraph`). Until pacquet has an equivalent pruner/copy-subgraph step, be aware that the raw `node.optional` can flow through to the lockfile/virtual store via `dependencies_graph_to_lockfile.rs` → `create_virtual_store.rs` → `installability.rs`, so review changes that affect optional flag propagation for this mismatch.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
📚 Learning: 2026-05-24T21:10:50.292Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: In pacquet’s Rust resolver layer, the behavior of `ResolvedPackage.optional` “AND-folding”/revisit updates is intentionally aligned with pnpm: when a package is directly revisited, only that package’s `optional` flag is updated; transitive descendants are not re-walked/rescored in this stage. pnpm later corrects any stale optional flags downstream via its pruner/BFS logic (`copyDependencySubGraph` + `nonOptional` reachability stamping in the lockfile pruner). Since pacquet does not yet have that pruner equivalent, review should NOT flag the downstream/stale optional propagation as a bug in pacquet PRs; treat the resolver-layer optional propagation gap as expected parity until the pruning/stamping step is ported.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/hoist_peers.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rspacquet/crates/resolving-deps-resolver/src/lib.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rspacquet/crates/resolving-deps-resolver/src/resolve_workspace.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
🪛 LanguageTool
pacquet/plans/LOCKFILE_RESOLUTION_REUSE.md
[grammar] ~28-~28: Ensure spelling is correct
Context: ...skips fetch. ## Key simplification for pacquet A given package **version's dependency s...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~32-~32: Consider an alternative for the overused word “exactly”.
Context: ...rent version, its transitive subtree is exactly the snapshot's recorded child-refs — we...
(EXACTLY_PRECISELY)
🔇 Additional comments (25)
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
822-849: LGTM!pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs (1)
214-214: LGTM!pacquet/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs (1)
134-135: LGTM!pacquet/crates/resolving-deps-resolver/src/lockfile_reuse.rs (1)
1-192: LGTM!pacquet/crates/resolving-deps-resolver/src/lockfile_reuse/tests.rs (1)
1-159: LGTM!pacquet/crates/resolving-deps-resolver/src/lib.rs (1)
70-70: LGTM!Also applies to: 90-90
pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs (13)
39-54: LGTM!
56-77: LGTM!
251-252: LGTM!
418-434: LGTM!Also applies to: 449-451, 482-503
714-759: LGTM!
783-810: LGTM!Also applies to: 1032-1043
1103-1141: LGTM!
1143-1152: LGTM!
1154-1182: LGTM!
1211-1378: LGTM!
1400-1407: LGTM!
1184-1209: Resolvedep_ref.resolve()API on lockfile dependency refs
dep_ref.resolve(child_name)is backed bypacquet-crates/lockfile’sSnapshotDepRef-style API:pacquet/crates/lockfile/src/snapshot_dep_ref.rsdefinespub fn resolve(&self, alias_name: &PkgName) -> Option<PkgNameVerPeer>, matching the call’s expected input/output types.
1380-1398:dep_ref.resolve()call is valid (method exists with the right signature).SnapshotDepRefdefinespub fn resolve(&self, alias_name: &PkgName) -> Option<PkgNameVerPeer>, and insnapshot_child_refsthe map iteration yieldsalias: &PkgNameanddep_ref: &SnapshotDepRef, sodep_ref.resolve(alias)matches.pacquet/crates/cli/tests/lockfile_resolution_reuse.rs (2)
44-110: LGTM!
112-169: LGTM!pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs (2)
236-236: LGTM!Also applies to: 281-281, 347-347, 370-370
217-226: ⚡ Quick win
ROOT_IMPORTER_KEYis compatible withimporter_id: &str
pacquet_lockfile::Lockfile::ROOT_IMPORTER_KEYis used throughout as a borrowed string (e.g., calling.to_string()and passing it directly toget(...)/comparisons), so passing it into the&str-typedimporter_idparameter is type-correct across the crate boundary.pacquet/crates/resolving-deps-resolver/src/resolve_workspace.rs (2)
21-23: LGTM!Also applies to: 82-91
133-141: LGTM!Also applies to: 175-175
What
Port pnpm's lockfile-resolution reuse to pacquet: during a non-frozen install, reuse the prior
pnpm-lock.yaml's resolution + transitive subtree for dependencies that are still satisfied and not being updated, instead of re-resolving everything from manifests (pacquet previously only fed the lockfile into preferred-versions seeding). Follow-up to #12096, which closed the same gap only for remote tarballs.Design notes:
pacquet/plans/LOCKFILE_RESOLUTION_REUSE.md.Approach
Faithful port of pnpm's
getInfoFromLockfile/resolvedDependencies/parentPkg.updatedmachinery (installing/deps-resolver/src/resolveDependencies.ts), adapted to pacquet's resolver as a hybrid resolve: snapshot-walk unchanged subtrees (reusinginstall_frozen_lockfile's logic — a package version's dependency set is immutable, so the snapshot child-refs are authoritative) and fresh-resolve new/changed/update-targeted deps, with anupdatedflag propagated down to break the reuse chain. The reuse gate uses semver-satisfies (pnpm parity), not string-equality.Conservative by construction — anything not provably safe falls through to a fresh resolve:
Tarballwith integrity); remote-tarball / git / directory / binary / non-semver /link:, and any subtree containing one, re-resolve.subtree_fully_reusablerequires the entire transitive subtree to be synthesizable before reusing.pacquet updatetargets (and their subtrees, at any depth) re-resolve viaUpdateReuseScope.packageExtensionsconfig invalidates reuse (the prior lockfile is withheld whenpackageExtensionsChecksumno longer matches).Tests
ResolveResultsynthesis (registry reuse, peer-metadata carry, non-registry / absent / non-semver rejection).lockfile_resolution_reuse.rs): a dead-registry test that succeeds only via reuse (verified to fail without the change), and a reuse-vs-fresh structural-equivalence test asserting a reused tree matches a fresh resolve (parsed-lockfile compare, so it's robust to pacquet: pnpm-lock.yaml map ordering is non-canonical (HashMap iteration order) #12117).pacquet-resolving-deps-resolver/pacquet-package-manager/pacquet-clisuites green.Follow-ups (tracked, non-blocking)
overridesdrift isn't yet guarded for transitive reuse.Written by an agent (Claude Code, claude-opus-4-8).