fix: converge incremental install by refreshing stale transitive pins (alternative to #11502)#12472
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughFixes a lockfile non-convergence bug in incremental installs: when a reused lockfile entry contains stale transitive pins, the resolver now detects a higher direct-dependency version and overrides the pin. The fix is implemented in both the pnpm TypeScript resolver ( Changespnpm TypeScript Resolver – Stale Pin Override
pacquet Rust Resolver – Stale Pin Refresh Gate
Sequence Diagram(s)sequenceDiagram
participant Installer
participant resolveDependencies
participant getDepsToResolve
participant findHigherDirectDepVersion
rect rgba(100, 149, 237, 0.5)
Note over Installer,findHigherDirectDepVersion: Incremental install with bumped direct dep
end
Installer->>resolveDependencies: resolve deps with preferredVersions
resolveDependencies->>getDepsToResolve: { wantedDeps, preferredVersions }
getDepsToResolve->>getDepsToResolve: alias found with lockfile pin
getDepsToResolve->>findHigherDirectDepVersion: (pinnedRange, preferredVersions, DIRECT_DEP_SELECTOR_WEIGHT)
alt higher direct version satisfies range
findHigherDirectDepVersion-->>getDepsToResolve: higherVersion (e.g. 100.1.0)
getDepsToResolve->>getDepsToResolve: proceed=true, preferredVersion=higherVersion, skip pin reuse
getDepsToResolve-->>resolveDependencies: extendedWantedDeps with preferredVersion
resolveDependencies-->>Installer: lockfile updated to 100.1.0, stale 100.0.0 removed
else no higher version
findHigherDirectDepVersion-->>getDepsToResolve: undefined
getDepsToResolve->>getDepsToResolve: reuse pinned lockfile reference
getDepsToResolve-->>resolveDependencies: extendedWantedDeps (pin reused)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
zkochan
added a commit
that referenced
this pull request
Jun 17, 2026
…igher direct deps Port of the pnpm fix in #12472. When a package is reused from the prior lockfile, its child edges are taken verbatim and bypass the preferred-versions walk, so a transitive dependency can stay pinned to an older version even after a direct dependency resolved to a higher version that satisfies the same range — leaving the lockfile non-convergent (an incremental install keeps a duplicate a fresh install would not). pacquet's full-subtree reuse (resolve_reused_node) is coarser than pnpm's per-edge reuse and carries neither the direct-dep versions nor the parent manifest's ranges that the refresh needs. Three coordinated changes bridge that, mirroring pnpm's phase structure: - Record, per importer, which direct deps changed vs. the prior lockfile (race-free, from the manifest + lockfile) and the resolved direct-dep versions (after the direct-dep level barrier). - Decline full-subtree reuse for a parent that depends on a changed direct dep, so it resolves fresh and walks its children against their real manifest ranges (extract_children), where the range is available. - In walk_node_children, when a child's recorded pin is below a higher direct-dep version that still satisfies the edge's range, force the edge onto that version. The lower version is never resolved or fetched — no wasted download, no post-resolution pruning. Both maps are keyed by importer to match pnpm's per-importer preferredVersions, so one importer's direct dep never refreshes another's transitive pins.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12472 +/- ##
==========================================
+ Coverage 88.12% 88.15% +0.02%
==========================================
Files 311 312 +1
Lines 41924 42280 +356
==========================================
+ Hits 36947 37270 +323
- Misses 4977 5010 +33 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Micro-Benchmark ResultsLinux |
Contributor
Integrated-Benchmark Report (Linux)Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.1637677125799994,
"stddev": 0.20566732189226825,
"median": 4.194706975879999,
"user": 3.8255669999999995,
"system": 3.4893428999999996,
"min": 3.8930023738800004,
"max": 4.60024773688,
"times": [
4.60024773688,
3.91268854788,
4.220648080879999,
3.99671901688,
4.25305756788,
4.113151011879999,
3.8930023738800004,
4.16876587088,
4.256884042879999,
4.22251287588
]
},
{
"command": "pacquet@main",
"mean": 4.01253042788,
"stddev": 0.10350203632747954,
"median": 4.00858593538,
"user": 3.7918341999999994,
"system": 3.4883604999999998,
"min": 3.86390471588,
"max": 4.20738191388,
"times": [
3.98155194288,
4.05196772588,
4.114781853879999,
4.20738191388,
4.04962075988,
3.9988949118800003,
3.86390471588,
3.96673128188,
3.87219221388,
4.01827695888
]
},
{
"command": "pnpr@HEAD",
"mean": 2.11974514218,
"stddev": 0.16113225691220906,
"median": 2.05185921688,
"user": 2.6486172999999993,
"system": 3.0094448999999996,
"min": 1.9731397428800002,
"max": 2.39458935688,
"times": [
1.98098356788,
1.9731397428800002,
2.08224701988,
2.28883260088,
2.34973468988,
2.05120471688,
2.04452054788,
1.97968546188,
2.39458935688,
2.05251371688
]
},
{
"command": "pnpr@main",
"mean": 2.13018747338,
"stddev": 0.10420941402413554,
"median": 2.08596321488,
"user": 2.6216266,
"system": 3.0038533,
"min": 2.02868996088,
"max": 2.3231801288800003,
"times": [
2.3231801288800003,
2.05926046688,
2.02868996088,
2.04638866688,
2.04904072488,
2.07511371888,
2.14412859688,
2.27996723188,
2.09681271088,
2.19929252688
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6443794112000001,
"stddev": 0.018358153490603567,
"median": 0.6428904009,
"user": 0.3783556199999999,
"system": 1.3514309199999999,
"min": 0.6154683984,
"max": 0.6819879624,
"times": [
0.6501417494,
0.6819879624,
0.6342251284,
0.6411791304000001,
0.6572276124,
0.6553359234,
0.6360206344,
0.6154683984,
0.6276059014,
0.6446016714
]
},
{
"command": "pacquet@main",
"mean": 0.643269458,
"stddev": 0.040267204907542535,
"median": 0.6327400219,
"user": 0.37266282,
"system": 1.32589732,
"min": 0.6126741124,
"max": 0.7543832574,
"times": [
0.6288184714,
0.6347975914,
0.6353155794,
0.6182926554,
0.6126741124,
0.6306824524,
0.6468400004,
0.6304777574,
0.6404127024,
0.7543832574
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6825074429000001,
"stddev": 0.020606763117955788,
"median": 0.6798505094,
"user": 0.38365141999999997,
"system": 1.3545862199999998,
"min": 0.6576791644000001,
"max": 0.7182370444,
"times": [
0.6871006364000001,
0.7043928414,
0.7031708544,
0.6800364494000001,
0.6796645694000001,
0.6651392924,
0.6703315224,
0.6576791644000001,
0.6593220544,
0.7182370444
]
},
{
"command": "pnpr@main",
"mean": 0.6793169695000001,
"stddev": 0.018738623726595183,
"median": 0.6734562949,
"user": 0.38851362,
"system": 1.34009322,
"min": 0.6602529654,
"max": 0.7265908194,
"times": [
0.7265908194,
0.6602529654,
0.6749830324,
0.6824034804,
0.6852275304000001,
0.6635514174,
0.6714053004,
0.6865174454,
0.6719295574,
0.6703081464
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.189390962459999,
"stddev": 0.04037970474761205,
"median": 4.18451054146,
"user": 3.70666784,
"system": 3.39222052,
"min": 4.129217188959999,
"max": 4.25471624596,
"times": [
4.15266768396,
4.218089883959999,
4.25471624596,
4.129217188959999,
4.16444662096,
4.2331193229599995,
4.19728141596,
4.2153593129599995,
4.17173966696,
4.15727228196
]
},
{
"command": "pacquet@main",
"mean": 4.20498347596,
"stddev": 0.03220728111195031,
"median": 4.20671100096,
"user": 3.7400558399999992,
"system": 3.3630308199999996,
"min": 4.14555442296,
"max": 4.25316612196,
"times": [
4.20681098596,
4.16881239396,
4.25316612196,
4.2127882759599995,
4.18857621896,
4.24677636596,
4.20628975196,
4.214449205959999,
4.14555442296,
4.20661101596
]
},
{
"command": "pnpr@HEAD",
"mean": 2.2048705389600003,
"stddev": 0.11752930883307397,
"median": 2.17824382096,
"user": 2.4690428399999997,
"system": 2.9189795199999997,
"min": 2.0903707539600003,
"max": 2.3857174439600004,
"times": [
2.1003565279600003,
2.29694528596,
2.09696468896,
2.3857174439600004,
2.35286513496,
2.24745126496,
2.26734859096,
2.0903707539600003,
2.1090363769600002,
2.10164932096
]
},
{
"command": "pnpr@main",
"mean": 2.07945257286,
"stddev": 0.05789267550814077,
"median": 2.08105860446,
"user": 2.49665504,
"system": 2.9123775199999993,
"min": 1.9914712649600002,
"max": 2.20519596496,
"times": [
2.1014151779600003,
2.20519596496,
2.0427176299600003,
2.10684137396,
2.1047532489600003,
2.03575525196,
2.04425860696,
1.9914712649600002,
2.0919359489600002,
2.07018125996
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.3338492079999997,
"stddev": 0.00900207041590481,
"median": 1.3314057251,
"user": 1.33496096,
"system": 1.7179035200000001,
"min": 1.3252460271,
"max": 1.3522091041,
"times": [
1.3371094541,
1.3522091041,
1.3252460271,
1.3264440420999999,
1.3413259091,
1.3263370240999999,
1.3307876581,
1.3414886581,
1.3255204110999999,
1.3320237921
]
},
{
"command": "pacquet@main",
"mean": 1.3658591376,
"stddev": 0.0670171336056993,
"median": 1.3438307081,
"user": 1.3456552600000002,
"system": 1.74732442,
"min": 1.3365080971,
"max": 1.5555113190999998,
"times": [
1.3589251961,
1.3423660681,
1.5555113190999998,
1.3445611341,
1.3365080971,
1.3382074631,
1.3431002821,
1.3518221731,
1.3372626561,
1.3503269870999999
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6750680781,
"stddev": 0.037900312446568475,
"median": 0.6602906331,
"user": 0.33506746,
"system": 1.28937882,
"min": 0.6500334491,
"max": 0.7698644271,
"times": [
0.6644498731,
0.6535946781,
0.7132019461,
0.6549708651,
0.6682558741,
0.6500334491,
0.7698644271,
0.6567056141000001,
0.6638756521,
0.6557284021
]
},
{
"command": "pnpr@main",
"mean": 0.6614773942000001,
"stddev": 0.07122117747513218,
"median": 0.6408672260999999,
"user": 0.32198086,
"system": 1.2855231200000001,
"min": 0.6243799381,
"max": 0.8628904991,
"times": [
0.6272390511,
0.6464703301,
0.6395923841,
0.6375931541000001,
0.6243799381,
0.6471772321,
0.6485126391,
0.6387766461,
0.6421420681,
0.8628904991
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.03409544062,
"stddev": 0.03704096670187535,
"median": 3.02878118032,
"user": 1.75992428,
"system": 2.04440856,
"min": 2.9891762233200003,
"max": 3.10940944732,
"times": [
3.02962814232,
2.9891762233200003,
3.0041256453200003,
3.03400750632,
3.07018898832,
3.02793421832,
3.02579639232,
3.10940944732,
3.05833977632,
2.99234806632
]
},
{
"command": "pacquet@main",
"mean": 2.98077254822,
"stddev": 0.03289622010110773,
"median": 2.9754310633200003,
"user": 1.7319284799999999,
"system": 1.99604976,
"min": 2.9442145013200003,
"max": 3.03785210232,
"times": [
3.02873308232,
2.9557491053200002,
2.99707776732,
2.96859630932,
2.9442145013200003,
2.94859935632,
2.9899836623200002,
3.03785210232,
2.95465377832,
2.98226581732
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6654972853200001,
"stddev": 0.010215356385779075,
"median": 0.66480716482,
"user": 0.32273708,
"system": 1.3009758599999999,
"min": 0.6531256953200001,
"max": 0.6852055313200001,
"times": [
0.6531256953200001,
0.6573161353200001,
0.6696318573200001,
0.6572921453200001,
0.6567259313200001,
0.6604478623200001,
0.6852055313200001,
0.6701184123200001,
0.6691664673200001,
0.6759428153200001
]
},
{
"command": "pnpr@main",
"mean": 0.6684518783200002,
"stddev": 0.09184982961472532,
"median": 0.6407623373200001,
"user": 0.32827158,
"system": 1.2792172599999998,
"min": 0.6276712823200001,
"max": 0.9291175633200001,
"times": [
0.6480940673200001,
0.6463573703200001,
0.6373534003200001,
0.6397666213200001,
0.64789507432,
0.6417580533200001,
0.6307841603200001,
0.6276712823200001,
0.6357211903200001,
0.9291175633200001
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12472 |
| 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 | 4,189.39 ms(-0.72%)Baseline: 4,219.83 ms | 5,063.80 ms (82.73%) |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot 🚷 view threshold | 3,034.10 ms(+0.92%)Baseline: 3,006.30 ms | 3,607.56 ms (84.10%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,333.85 ms(+0.13%)Baseline: 1,332.16 ms | 1,598.59 ms (83.44%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,163.77 ms(-0.28%)Baseline: 4,175.48 ms | 5,010.57 ms (83.10%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 644.38 ms(+4.36%)Baseline: 617.43 ms | 740.92 ms (86.97%) |
Contributor
|
| Branch | pr/12472 |
| Testbed | pnpr |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot | 2,204.87 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 665.50 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 675.07 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,119.75 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 682.51 ms |
zkochan
added a commit
that referenced
this pull request
Jun 17, 2026
…igher direct deps Port of #12472. pacquet's full-subtree lockfile reuse (resolve_reused_node) is coarser than pnpm's per-edge reuse and carries neither the resolved direct-dep versions nor the parent manifest's ranges the refresh needs. Three coordinated changes bridge that, mirroring pnpm's phase structure: - Record, per importer, which direct deps changed vs. the prior lockfile (race-free) and the resolved direct-dep versions (after the direct-dep level barrier). - Decline full-subtree reuse for a parent that depends on a changed direct dep, so it resolves fresh and walks its children against their real manifest ranges. - In walk_node_children, force a child edge onto a higher direct-dep version when the recorded pin is below it and it still satisfies the edge's range. The lower version is never resolved or fetched. Both maps are keyed by importer to match pnpm's per-importer preferredVersions.
751bf30 to
2aa4fa2
Compare
PR Summary by QodoFix incremental install by refreshing stale transitive lockfile pins Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1. Refresh loses lockfile context
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
installing/deps-resolver/src/resolveDependencies.ts (1)
1450-1453: ⚡ Quick winUse an options object for
findHigherDirectDepVersionparameters.Line 1549 introduces 4 positional parameters in a new helper; this is easy to misorder in resolver hot-path call sites. Please convert it to a single options object.
Proposed refactor
- const higherDirectVersion = pinned == null - ? undefined - : findHigherDirectDepVersion(options.preferredVersions, pinned.name, pinned.version, wantedDependency.bareSpecifier) + const higherDirectVersion = pinned == null + ? undefined + : findHigherDirectDepVersion({ + preferredVersions: options.preferredVersions, + pinnedName: pinned.name, + pinnedVersion: pinned.version, + bareSpecifier: wantedDependency.bareSpecifier, + }) -function findHigherDirectDepVersion ( - preferredVersions: PreferredVersions | undefined, - pinnedName: string, - pinnedVersion: string, - bareSpecifier: string -): string | undefined { +function findHigherDirectDepVersion ( + opts: { + preferredVersions: PreferredVersions | undefined + pinnedName: string + pinnedVersion: string + bareSpecifier: string + } +): string | undefined { - if (preferredVersions == null) return undefined - if (!semver.valid(pinnedVersion)) return undefined - if (semver.validRange(bareSpecifier) === null) return undefined - const selectors = preferredVersions[pinnedName] + if (opts.preferredVersions == null) return undefined + if (!semver.valid(opts.pinnedVersion)) return undefined + if (semver.validRange(opts.bareSpecifier) === null) return undefined + const selectors = opts.preferredVersions[opts.pinnedName] if (selectors == null) return undefined let best: string | undefined for (const [candidate, selector] of Object.entries(selectors)) { if ( typeof selector === 'object' && selector.selectorType === 'version' && selector.weight === DIRECT_DEP_SELECTOR_WEIGHT && semver.valid(candidate) && - semver.gt(candidate, pinnedVersion) && - semver.satisfies(candidate, bareSpecifier, true) && + semver.gt(candidate, opts.pinnedVersion) && + semver.satisfies(candidate, opts.bareSpecifier, true) && (best == null || semver.gt(candidate, best)) ) { best = candidate } } return best }As per coding guidelines,
**/*.{ts,tsx}functions should have ≤2-3 arguments and use an options object for more.Also applies to: 1549-1575
🤖 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 `@installing/deps-resolver/src/resolveDependencies.ts` around lines 1450 - 1453, The function findHigherDirectDepVersion currently accepts 4 positional parameters which violates the coding guideline of keeping functions to 2-3 arguments. Refactor this function (defined around line 1549-1575) to accept a single options object parameter containing preferredVersions, name, version, and bareSpecifier properties instead of four separate arguments. Then update all call sites of findHigherDirectDepVersion, including the one at line 1450-1453, to pass an options object with these properties rather than individual positional arguments.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@installing/deps-resolver/src/resolveDependencies.ts`:
- Around line 1450-1453: The function findHigherDirectDepVersion currently
accepts 4 positional parameters which violates the coding guideline of keeping
functions to 2-3 arguments. Refactor this function (defined around line
1549-1575) to accept a single options object parameter containing
preferredVersions, name, version, and bareSpecifier properties instead of four
separate arguments. Then update all call sites of findHigherDirectDepVersion,
including the one at line 1450-1453, to pass an options object with these
properties rather than individual positional arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d7e8d8f-c55d-42b4-9847-c7a255179575
📒 Files selected for processing (5)
.changeset/refresh-stale-lockfile-pins.mdinstalling/deps-installer/test/install/dedupe.tsinstalling/deps-resolver/src/resolveDependencies.tspacquet/crates/cli/tests/stale_pin_dedupe.rspacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs
|
Code review by qodo was updated up to the latest commit 2aa4fa2 |
zkochan
added a commit
that referenced
this pull request
Jun 17, 2026
…igher direct deps Port of #12472. pacquet's full-subtree lockfile reuse (resolve_reused_node) is coarser than pnpm's per-edge reuse and carries neither the resolved direct-dep versions nor the parent manifest's ranges the refresh needs. Three coordinated changes bridge that, mirroring pnpm's phase structure: - Record, per importer, which direct deps changed vs. the prior lockfile (race-free) and the resolved direct-dep versions (after the direct-dep level barrier). - Decline full-subtree reuse for a parent that depends on a changed direct dep, so it resolves fresh and walks its children against their real manifest ranges. - In walk_node_children, force a child edge onto a higher direct-dep version when the recorded pin is below it and it still satisfies the edge's range. The lower version is never resolved or fetched. Both maps are keyed by importer to match pnpm's per-importer preferredVersions. The direct-dep version map stores parsed versions behind an Arc so the hot child walk snapshots it once per node instead of locking and parsing per edge.
2aa4fa2 to
72cc9f1
Compare
|
Code review by qodo was updated up to the latest commit 72cc9f1 |
zkochan
added a commit
that referenced
this pull request
Jun 17, 2026
…igher direct deps Port of #12472. pacquet's full-subtree lockfile reuse (resolve_reused_node) is coarser than pnpm's per-edge reuse and carries neither the resolved direct-dep versions nor the parent manifest's ranges the refresh needs. Three coordinated changes bridge that, mirroring pnpm's phase structure: - Record, per importer, which direct deps changed vs. the prior lockfile (race-free) and the resolved direct-dep versions (after the direct-dep level barrier). - Decline full-subtree reuse for a parent that depends on a changed direct dep, so it resolves fresh and walks its children against their real manifest ranges. - In walk_node_children, force a child edge onto a higher direct-dep version when the recorded pin is below it and it still satisfies the edge's range. The lower version is never resolved or fetched. Both maps are keyed by importer to match pnpm's per-importer preferredVersions. The direct-dep version map stores parsed versions behind an Arc so the hot child walk snapshots it once per node instead of locking and parsing per edge. Range satisfaction uses plain semver (not prerelease-inclusive), matching pnpm's semver.satisfies(.., true).
72cc9f1 to
4a27779
Compare
|
Code review by qodo was updated up to the latest commit 4a27779 |
When a package is reused from the lockfile, its child edges are taken verbatim and bypass the preferred-versions walk, so a transitive dependency can stay pinned to an older version even after a direct dependency resolved to a higher version that satisfies the same range — leaving the lockfile non-convergent (an incremental install keeps a duplicate a fresh install would not). getDepsToResolve now refreshes such a stale pin to the higher direct-dependency version during resolution, via `preferredVersion` (singular), which overrides the EXISTING_VERSION_SELECTOR_WEIGHT stability bias. The older version is never resolved or fetched, and the incremental result converges to the fresh one. Tests cover the core scenario, caret-range and aliased (npm:) specifiers, idempotency, and a multi-importer workspace (where all importers' direct deps resolve before any transitive, so the refresh converges across importers to match a fresh install).
…igher direct deps Port of #12472. pacquet's full-subtree lockfile reuse (resolve_reused_node) is coarser than pnpm's per-edge reuse and carries neither the resolved direct-dep versions nor the parent manifest's ranges the refresh needs. Three coordinated changes bridge that, mirroring pnpm's phase structure: - Record, per importer, which direct deps changed vs. the prior lockfile (race-free) and the resolved direct-dep versions (after the direct-dep level barrier). - Decline full-subtree reuse for a parent that depends on a changed direct dep, so it resolves fresh and walks its children against their real manifest ranges. - In walk_node_children, force a child edge onto a higher direct-dep version when the recorded pin is below it and it still satisfies the edge's range. The lower version is never resolved or fetched. Both maps are keyed by importer. Unlike pnpm — which resolves every importer's direct deps before any transitive and so converges cross-importer — this crate resolves importers sequentially, so a shared map would refresh one importer's edges from another's direct deps order-dependently; per-importer keying keeps it deterministic (the multi-importer convergence pnpm does is a known parity gap). The direct-dep version map stores parsed versions behind an Arc so the hot child walk snapshots it once per node instead of locking and parsing per edge. Range satisfaction uses plain semver (not prerelease-inclusive), matching pnpm's semver.satisfies(.., true).
4a27779 to
a738dc9
Compare
|
Code review by qodo was updated up to the latest commit a738dc9 |
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
2 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.
Summary
A prototype alternative to the post-resolution dedupe pass in #11502. Instead of resolving a lower version, fetching it (and its subtree), then rewriting the edge and pruning the orphan, this dedupes during resolution so the lower version is never resolved or fetched.
Background
On a fresh resolve pnpm already dedupes: every level of the tree walk seeds
preferredVersionswith the versions resolved in ancestor/sibling packages, and direct deps additionally getDIRECT_DEP_SELECTOR_WEIGHT. So a transitivefoo@^1.0.0already prefers afoo@1.1.0that a direct dependency resolved to.The duplicate that #11502 targets is purely a lockfile-stickiness artifact: a package that isn't being re-resolved (its manifest didn't change) has its child edges taken verbatim from the lockfile, bypassing the preferred-versions walk. That keeps a stale lower version pinned even though a direct dependency now resolves to a higher version in the same range — and because it's a normally-resolved package, its tarball gets fetched.
Approach
In
getDepsToResolve, when a child edge is about to be reused from its lockfile pin, check whether a direct dependency (the deterministic, resolved-first anchor, marked withDIRECT_DEP_SELECTOR_WEIGHT) already resolved to a higher version that satisfies the edge's range. If so, force the edge onto that version viapreferredVersion(singular), which overrides theEXISTING_VERSION_SELECTOR_WEIGHTstability bias that would otherwise re-pick the lower version. The lower version is never resolved and never fetched — no wasted download, no post-resolution pruning.Comparison with #11502
preferredVersionleveroptional/prod/dev) stalenessScope / limitations
By design this only dedupes a transitive against a direct-dependency version — the deterministic anchor, and the shape of every motivating case in #11502. It deliberately does not touch the transitive-vs-transitive case, which is the order-dependent one that motivated the original removal of automatic deduping (#11110).
This is a behavior change from the post-#11110 "keep already-installed versions" default, so it is opened as a draft for discussion: whether it should be the default or gated behind a setting, plus a wider installer sweep before shipping.
Pacquet port (included)
The matching pacquet (Rust) port is in this PR. pacquet's full-subtree lockfile reuse (
resolve_reused_node) is coarser than pnpm's per-edge reuse — it reuses a parent's children straight from the snapshot's exact pins and carries neither the resolved direct-dep versions nor the parent manifest's ranges the refresh needs. Three coordinated changes bridge that, mirroring pnpm's phase structure:walk_node_children, force a child edge onto a higher direct-dep version when the recorded pin is below it and it still satisfies the edge's range. The lower version is never resolved or fetched.Both maps are keyed by importer to match pnpm's per-importer
preferredVersions. New pacquet tests (stale_pin_dedupe.rs) cover the exact-version and caret-range (pnpm add) cases; the resolver (143), package-manager (537), and reuse/dedupe/add/update/parity CLI suites pass, as dofmt/clippy/doc/dylint/taplo.Tests
refreshes a stale transitive pin to a higher direct-dependency version at resolution time(the exact scenario feat: smarter install dedupe behavior #11502 targeted;100.0.0is absent from the lockfile because it is never resolved).dedupe.ts,dedupeDirectDeps/dedupeInWorkspace/autoInstallPeers,update.ts,peerDependencies.ts. The onelockfile.tsfailure here is a pre-existing offline failure (it fetches a realregistry.npmjs.orgtarball).🤖 This PR was prepared by Claude Code (model: Opus 4.8) on behalf of @zkochan.
Summary by CodeRabbit
Squash-merge commit message