perf(pacquet): no-op short-circuit when node_modules is up to date by zkochan · Pull Request #11904 · pnpm/pnpm

zkochan · 2026-05-24T15:44:49Z

Summary

Adds a fast-path gate in Install::run that mirrors upstream pnpm's validateModules + allProjectsAreUpToDate shortcut: when the frozen-lockfile dispatch is eligible, .modules.yaml agrees with the current config, and <virtual_store_dir>/lock.yaml is byte-equal to the wanted lockfile, materialization is skipped entirely. The install emits the name: "pnpm" / level: "info" "Lockfile is up to date, resolution step is skipped" log, refreshes the workspace-state timestamp so pnpm run's verifyDepsBeforeRun doesn't fire spuriously, and returns.

The gate closes the lockfile+node_modules benchmark variation gap (currently 2-24× slower than pnpm on every fixture). It mirrors upstream's behavior at installing/deps-installer/src/install/index.ts#L913-L985 — fires under both --frozen-lockfile and preferFrozenLockfile.

Closes #11899.

Changes

pacquet/crates/reporter: new LogEvent::Pnpm variant for name: "pnpm" generic-channel logs (consumed by @pnpm/cli.default-reporter's "other" stream).
pacquet/crates/package-manager/src/install.rs: gate the dispatch on the new is_modules_yaml_consistent helper (port of validateModules's settings checks) plus current == wanted lockfile equality. On hit, emit the up-to-date log + ImportingDone + Summary, refresh workspace state, return Ok.
Unit tests cover the helper's four states (missing yaml, all-match, nodeLinker drift, included drift) plus an end-to-end gate test that asserts the log fires, materialization is skipped, and workspace state is refreshed.
CLI integration test runs install twice and asserts the NDJSON "Lockfile is up to date" record appears on the second run.

Test plan

cargo nextest run -p pacquet-package-manager — 304/304 pass
cargo nextest run -p pacquet-reporter — 28/28 pass (incl. new pnpm_event_matches_pnpm_wire_shape)
cargo nextest run -p pacquet-cli --test install — 11/11 pass (incl. new frozen_install_short_circuits_when_node_modules_is_up_to_date)
just lint, just check, just fmt — clean
Benchmark validation against vlt.sh lockfile+node_modules variation

Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

New Features
- Installer fast-path: frozen-lockfile installs now short-circuit when on-disk state is already up to date, skipping resolution/materialization for faster installs.
- Improved pnpm-compatible log output so external tools can detect "lockfile is up to date" events.
Tests
- Added end-to-end and unit tests covering frozen-lockfile short-circuit behavior and module-state consistency checks.

Adds a fast-path gate in `Install::run` that mirrors upstream pnpm's `validateModules` + `allProjectsAreUpToDate` shortcut: when the frozen-lockfile dispatch is eligible, `.modules.yaml` agrees with the current config, and `<virtual_store_dir>/lock.yaml` is byte-equal to the wanted lockfile, skip materialization entirely. The install emits the `name: "pnpm" / level: "info"` "Lockfile is up to date, resolution step is skipped" log, refreshes the workspace-state timestamp so `pnpm run`'s `verifyDepsBeforeRun` doesn't fire spuriously, and returns. Closes #11899. --- Written by an agent (Claude Code, claude-opus-4-7).

qodo-code-review · 2026-05-24T15:44:54Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

coderabbitai · 2026-05-24T15:45:05Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1c402973-c134-4400-ad2a-7042e76842fb

📥 Commits

Reviewing files that changed from the base of the PR and between d4803a0 and 3a0d661.

📒 Files selected for processing (1)

pacquet/crates/package-manager/src/install/tests.rs

📜 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). (9)

GitHub Check: Run benchmark on ubuntu-latest
GitHub Check: Lint and Test (windows-latest)
GitHub Check: Lint and Test (macos-latest)
GitHub Check: Lint and Test (ubuntu-latest)
GitHub Check: Dylint
GitHub Check: Run benchmark on ubuntu-latest
GitHub Check: Code Coverage
GitHub Check: Analyze (javascript)
GitHub Check: Compile & Lint

🧰 Additional context used

📓 Path-based instructions (1)

pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

pacquet/crates/package-manager/src/install/tests.rs

🧠 Learnings (3)

📚 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/package-manager/src/install/tests.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/package-manager/src/install/tests.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/package-manager/src/install/tests.rs

🔇 Additional comments (2)

pacquet/crates/package-manager/src/install/tests.rs (2)

4642-4649: LGTM!

4660-4670: LGTM!

📝 Walkthrough

Walkthrough

This PR implements a frozen-lockfile no-op short-circuit: when the desired pnpm-lock.yaml matches the on-disk lockfile and node_modules/.modules.yaml is consistent with the current install configuration, pacquet emits an "up to date" pnpm-format log, refreshes workspace state, and returns early without resolution or materialization.

Changes

Frozen-lockfile no-op short-circuit with modules consistency validation

Layer / File(s)	Summary
Pnpm reporter wire format `pacquet/crates/reporter/src/lib.rs`, `pacquet/crates/reporter/src/tests.rs`	`LogEvent::Pnpm(PnpmLog)` variant and `PnpmLog` struct enable NDJSON logging of pnpm-style records (`name: "pnpm"`) with level, message, and prefix; tests verify exact envelope serialization.
Modules manifest consistency validation `pacquet/crates/package-manager/src/install.rs`, `pacquet/crates/package-manager/src/install/tests.rs`	`is_modules_yaml_consistent()` reads `.modules.yaml` and verifies layout version, mapped `nodeLinker`, included dependency flags, hoist pattern/store/virtual-store settings against the current config; unit tests cover missing file, matching config, and drift cases.
Install frozen-lockfile short-circuit integration `pacquet/crates/package-manager/src/install.rs`	When `take_frozen_path` is selected and both lockfile freshness and `.modules.yaml` consistency checks pass, `Install::run` emits the pnpm-format "Lockfile is up to date, resolution step is skipped" info record, emits ImportingDone, updates workspace state, emits a summary, and returns early without running resolution/materialization.
Integration and end-to-end test coverage `pacquet/crates/package-manager/src/install/tests.rs`, `pacquet/crates/cli/tests/install.rs`	In-process integration test seeds manifests/lockfiles and asserts the fast-path behavior and workspace-state refresh; CLI test primes node_modules then reruns with `--frozen-lockfile --reporter=ndjson` and parses stderr NDJSON for the expected pnpm record.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI (pacquet)
  participant Installer as Install::run
  participant ModulesCheck as is_modules_yaml_consistent
  participant Reporter as Reporter::LogEvent::Pnpm
  participant Workspace as update_workspace_state

  CLI->>Installer: run --frozen-lockfile
  Installer->>Installer: compare desired vs current pnpm-lock.yaml
  Installer->>ModulesCheck: read and validate node_modules/.modules.yaml
  ModulesCheck-->>Installer: consistent / inconsistent
  Installer->>Reporter: emit PnpmLog("Lockfile is up to date...")
  Installer->>Workspace: update_workspace_state
  Installer-->>CLI: return early (skip resolution/materialization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

pnpm/pnpm#11899: Implements the proposed frozen-lockfile no-op short-circuit by validating .modules.yaml and skipping work when up-to-date.

Possibly related PRs

pnpm/pnpm#11824: Related edits to frozen-lockfile dispatch logic in install.rs; both PRs refine frozen-path behavior.
pnpm/pnpm#11877: Changes lockfile importer behavior that may affect whether the frozen-path equality check triggers.
pnpm/pnpm#11734: Related to NDJSON pnpm-format log routing and how pacquet logs are forwarded to @pnpm/cli.default-reporter.

Poem

🐰 I checked the modules, checked the lock,

Found everything steady—no build to knock.
The log gently chirps, "Up to date" with a wink,
I hop past work faster than you'd think.
Carrot in pocket, installation blink.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main optimization: a no-op short-circuit when node_modules is up to date, which is the core performance improvement in the changeset.
Linked Issues check	✅ Passed	The PR successfully implements both gates from issue `#11899`: lockfile freshness check reuse and validateModules port to verify .modules.yaml consistency, enabling early exit with the expected log when both pass.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the no-op short-circuit: reporter variant for pnpm logs, install.rs helper and gate logic, and comprehensive unit/integration tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch perf/pacquet-up-to-date-short-circuit

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

🤖 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/crates/package-manager/src/install.rs`:
- Around line 556-560: The fast-path no-op guard around take_frozen_path
currently omits checking the enableGlobalVirtualStore setting, so update the
consistency gate used in install.rs (the if that calls
is_modules_yaml_consistent(&config.modules_dir, config, node_linker, included))
to include enableGlobalVirtualStore drift: ensure is_modules_yaml_consistent (or
the pre-check before returning early) also compares
config.enable_global_virtual_store against the persisted value in .modules.yaml
and that the code that writes/persists modules YAML (used elsewhere in this
file) persists enableGlobal_virtual_store if missing; apply the same change to
the analogous guard later in the file (the block around the other fast-path at
the 1058–1067 location) so both early-return paths only short-circuit when
enableGlobalVirtualStore matches the persisted setting.

In `@pacquet/crates/package-manager/src/install/tests.rs`:
- Around line 4642-4649: The test currently picks the first LogEvent::Pnpm via
find_map (variable up_to_date) which is brittle; change the assertion to check
that any pnpm event contains the expected message instead. Locate the captured
iterator and replace the find_map-based selection of LogEvent::Pnpm with a check
like captured.iter().any(|event| matches!(event, LogEvent::Pnpm(log) if
log.message == "Lockfile is up to date, resolution step is skipped")) and assert
that this predicate is true, or alternatively use find_map but only return Some
when log.message matches the expected string and then expect() that result.
- Around line 4660-4667: The test currently uses sibling_link.exists() which
returns false for dangling symlinks and can miss a created broken link; replace
that check with std::fs::symlink_metadata on the sibling_link (the variable
named sibling_link created from modules_dir.join("sibling")) and assert that
symlink_metadata returns an Err (meaning no entry) or, if it returns Ok(meta),
assert that meta.file_type().is_symlink() is false (or fail) so the test fails
if a symlink (even dangling) was created; update the assert message accordingly
to reference sibling_link for clarity.

🪄 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: fd7d4230-8eec-4948-8614-35a08f3dc5b3

📥 Commits

Reviewing files that changed from the base of the PR and between a456dc7 and d4803a0.

📒 Files selected for processing (5)

pacquet/crates/cli/tests/install.rs
pacquet/crates/package-manager/src/install.rs
pacquet/crates/package-manager/src/install/tests.rs
pacquet/crates/reporter/src/lib.rs
pacquet/crates/reporter/src/tests.rs

📜 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). (8)

GitHub Check: Run benchmark on ubuntu-latest
GitHub Check: Run benchmark on ubuntu-latest
GitHub Check: Lint and Test (ubuntu-latest)
GitHub Check: Lint and Test (macos-latest)
GitHub Check: Lint and Test (windows-latest)
GitHub Check: Dylint
GitHub Check: Code Coverage
GitHub Check: Compile & Lint

🧰 Additional context used

📓 Path-based instructions (2)

pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

pacquet/crates/cli/tests/install.rs
pacquet/crates/reporter/src/lib.rs
pacquet/crates/package-manager/src/install.rs
pacquet/crates/reporter/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs

pacquet/**/{tests,test}/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested with tempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on the Host provider, threaded as Sys: <Bounds> — only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq! assertions, dbg! complex structures, skip logging for simple scalar assert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer #[cfg_attr(target_os = "windows", ignore = "...")] (or #[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Use allow_known_failure! at the not-yet-implemented boundary and update checkboxes as items land.

Files:

pacquet/crates/cli/tests/install.rs

🧠 Learnings (3)

📚 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/cli/tests/install.rs
pacquet/crates/reporter/src/lib.rs
pacquet/crates/package-manager/src/install.rs
pacquet/crates/reporter/src/tests.rs
pacquet/crates/package-manager/src/install/tests.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/cli/tests/install.rs
pacquet/crates/reporter/src/lib.rs
pacquet/crates/package-manager/src/install.rs
pacquet/crates/reporter/src/tests.rs
pacquet/crates/package-manager/src/install/tests.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/cli/tests/install.rs
pacquet/crates/reporter/src/lib.rs
pacquet/crates/package-manager/src/install.rs
pacquet/crates/reporter/src/tests.rs
pacquet/crates/package-manager/src/install/tests.rs

🔇 Additional comments (4)

pacquet/crates/reporter/src/lib.rs (1)

190-201: LGTM!

Also applies to: 720-728

pacquet/crates/reporter/src/tests.rs (1)

12-15: LGTM!

Also applies to: 98-122

pacquet/crates/package-manager/src/install.rs (1)

23-24: LGTM!

Also applies to: 28-30

pacquet/crates/cli/tests/install.rs (1)

508-563: LGTM!

github-actions · 2026-05-24T15:49:50Z

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.07      5.7±0.37ms   754.8 KB/sec    1.00      5.4±0.29ms   807.7 KB/sec

codecov-commenter · 2026-05-24T15:53:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.86%. Comparing base (74a219e) to head (3a0d661).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11904      +/-   ##
==========================================
+ Coverage   87.84%   87.86%   +0.01%     
==========================================
  Files         206      206              
  Lines       24525    24565      +40     
==========================================
+ Hits        21545    21584      +39     
- Misses       2980     2981       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Address review feedback: - Use `r#"..."#` raw-string for the up-to-date log assertion message so dylint's `perfectionist::prefer-raw-string` lint stops flagging the escaped quotes inside it. - Loosen the "up-to-date log fires" check to `any(|e| matches!(...))` so unrelated future `LogEvent::Pnpm` emits don't make the test brittle. - Swap `Path::exists()` for `std::fs::symlink_metadata().is_err()` on the "link: dep not materialized" assertion so a dangling symlink (which `exists()` reports as `false`) wouldn't sneak past. --- Written by an agent (Claude Code, claude-opus-4-7).

github-actions · 2026-05-24T16:30:27Z

Integrated-Benchmark Report (Linux)

Scenario: Isolated linker: fresh restore, cold cache + cold store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	2.346 ± 0.095	2.247	2.573	1.01 ± 0.06
`pacquet@main`	2.324 ± 0.090	2.242	2.551	1.00
`pnpm`	4.713 ± 0.063	4.614	4.811	2.03 ± 0.08

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.34595043282,
      "stddev": 0.09539003960181833,
      "median": 2.31215010672,
      "user": 2.7935817399999996,
      "system": 3.4818714999999996,
      "min": 2.24658907222,
      "max": 2.57252308122,
      "times": [
        2.37479140922,
        2.31514219722,
        2.30915801622,
        2.57252308122,
        2.3982150472200003,
        2.29356908222,
        2.24658907222,
        2.39438256822,
        2.29129983422,
        2.26383402022
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.3240613740200002,
      "stddev": 0.08991344438645027,
      "median": 2.3064679217200004,
      "user": 2.8020216399999995,
      "system": 3.5246655,
      "min": 2.24221564722,
      "max": 2.55082786722,
      "times": [
        2.32354501322,
        2.31282881622,
        2.24221564722,
        2.25723932722,
        2.28002269122,
        2.25486847922,
        2.55082786722,
        2.30010702722,
        2.34860037822,
        2.37035849322
      ]
    },
    {
      "command": "pnpm",
      "mean": 4.712859366319999,
      "stddev": 0.06276764629229503,
      "median": 4.71762552272,
      "user": 8.03499104,
      "system": 4.090710200000001,
      "min": 4.614174879219999,
      "max": 4.81132163622,
      "times": [
        4.75431329322,
        4.614174879219999,
        4.68971602322,
        4.70950698622,
        4.67135347122,
        4.81132163622,
        4.725744059219999,
        4.774458023219999,
        4.747900158219999,
        4.63010513322
      ]
    }
  ]
}

Scenario: Isolated linker: fresh restore, hot cache + hot store

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`pacquet@HEAD`	686.9 ± 49.8	664.1	827.4	1.00
`pacquet@main`	692.6 ± 34.1	666.1	771.6	1.01 ± 0.09
`pnpm`	2525.6 ± 108.8	2404.2	2739.4	3.68 ± 0.31

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.68688705868,
      "stddev": 0.04975248989943747,
      "median": 0.6696078238800001,
      "user": 0.37994809999999996,
      "system": 1.4733848999999999,
      "min": 0.66408810888,
      "max": 0.8273528908800001,
      "times": [
        0.8273528908800001,
        0.67507477088,
        0.67159216688,
        0.67913667688,
        0.66662470588,
        0.68363331188,
        0.66762348088,
        0.66731745288,
        0.66408810888,
        0.66642702088
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.69263060768,
      "stddev": 0.03413749576315834,
      "median": 0.67799916188,
      "user": 0.3766541,
      "system": 1.4788223999999999,
      "min": 0.66610076488,
      "max": 0.77160987888,
      "times": [
        0.77160987888,
        0.70046045788,
        0.66610076488,
        0.6832001358800001,
        0.67078619688,
        0.66926991088,
        0.67719788188,
        0.67558501988,
        0.67880044188,
        0.73329538788
      ]
    },
    {
      "command": "pnpm",
      "mean": 2.52557056478,
      "stddev": 0.10880175974549618,
      "median": 2.50456647788,
      "user": 3.0132434999999997,
      "system": 2.2079075,
      "min": 2.40418515188,
      "max": 2.73939814988,
      "times": [
        2.43147452088,
        2.41150919188,
        2.73939814988,
        2.5802128088800003,
        2.49424102988,
        2.58354854988,
        2.45935351088,
        2.51489192588,
        2.40418515188,
        2.63689080788
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + cold store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	5.091 ± 0.239	4.836	5.639	1.01 ± 0.05
`pacquet@main`	5.039 ± 0.135	4.872	5.250	1.00
`pnpm`	6.245 ± 0.144	6.096	6.627	1.24 ± 0.04

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 5.09113208734,
      "stddev": 0.2391169765331954,
      "median": 5.04412268674,
      "user": 6.852651519999999,
      "system": 3.53467812,
      "min": 4.83550421324,
      "max": 5.63864763424,
      "times": [
        4.9389305412399995,
        4.9016280092399995,
        5.63864763424,
        4.83550421324,
        5.268722697239999,
        5.22638993924,
        5.11117198624,
        5.043292312239999,
        5.044953061239999,
        4.9020804792399995
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 5.03929707834,
      "stddev": 0.13548629296749404,
      "median": 5.00556481774,
      "user": 6.79298612,
      "system": 3.5683439199999993,
      "min": 4.87194802424,
      "max": 5.250394142239999,
      "times": [
        4.94924222224,
        4.87194802424,
        5.250394142239999,
        5.05206726424,
        4.9392856062399995,
        4.95906237124,
        5.157036082239999,
        5.18576271024,
        4.88890141024,
        5.139270950239999
      ]
    },
    {
      "command": "pnpm",
      "mean": 6.24465565974,
      "stddev": 0.14372855777200047,
      "median": 6.21022858724,
      "user": 10.361055619999998,
      "system": 4.40125532,
      "min": 6.09625250324,
      "max": 6.62655603924,
      "times": [
        6.24854073724,
        6.17927987724,
        6.176386454239999,
        6.281889189239999,
        6.62655603924,
        6.1914544532399995,
        6.09625250324,
        6.24051744124,
        6.17667718124,
        6.22900272124
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, hot cache + hot store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	3.994 ± 0.065	3.893	4.106	1.00
`pacquet@main`	4.066 ± 0.167	3.865	4.445	1.02 ± 0.05
`pnpm`	4.190 ± 0.077	4.104	4.344	1.05 ± 0.03

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 3.99350224394,
      "stddev": 0.0654148237893708,
      "median": 3.99028839374,
      "user": 4.30830246,
      "system": 2.25212124,
      "min": 3.89285888624,
      "max": 4.10580735424,
      "times": [
        4.10580735424,
        3.89285888624,
        3.9798352882400003,
        4.01011045224,
        4.07208889024,
        3.98610982624,
        3.99446696124,
        3.93064815624,
        4.03074120924,
        3.93235541524
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 4.06646772704,
      "stddev": 0.16693219382374405,
      "median": 4.076880131739999,
      "user": 4.39053206,
      "system": 2.2262599399999994,
      "min": 3.8651483782400002,
      "max": 4.44498948124,
      "times": [
        4.01567723124,
        4.06978010124,
        4.08398016224,
        4.17953958824,
        4.44498948124,
        4.09471442424,
        4.09124849124,
        3.88974054024,
        3.8651483782400002,
        3.92985887224
      ]
    },
    {
      "command": "pnpm",
      "mean": 4.1900371105400005,
      "stddev": 0.07707880019886432,
      "median": 4.17325440674,
      "user": 5.17263166,
      "system": 2.6366386399999997,
      "min": 4.10402964924,
      "max": 4.34431115024,
      "times": [
        4.17999614024,
        4.15067847124,
        4.34431115024,
        4.16651267324,
        4.11558798224,
        4.28373723424,
        4.10402964924,
        4.19659520124,
        4.23180677724,
        4.12711582624
      ]
    }
  ]
}

github-actions · 2026-05-24T16:30:30Z

Bencher Report

Branch	pr/11904
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	5,091.13 ms (+2.06%) Baseline: 4,988.42 ms	5,986.10 ms (85.05%)
isolated-linker.fresh-install.hot-cache.hot-store	📈 view plot 🚷 view threshold	3,993.50 ms (+1.96%) Baseline: 3,916.76 ms	4,700.12 ms (84.97%)
isolated-linker.fresh-restore.cold-cache.cold-store	📈 view plot 🚷 view threshold	2,345.95 ms (-1.61%) Baseline: 2,384.43 ms	2,861.32 ms (81.99%)
isolated-linker.fresh-restore.hot-cache.hot-store	📈 view plot 🚷 view threshold	686.89 ms (+4.58%) Baseline: 656.79 ms	788.14 ms (87.15%)

🐰 View full continuous benchmarking report in Bencher

…-op short-circuit fires `read_modules_manifest` joins a stored relative `virtualStoreDir` with `modules_dir` to recover an absolute path, mirroring upstream's `path.join(modulesDir, modules.virtualStoreDir)` at <https://github.com/pnpm/pnpm/blob/1819226b51/installing/modules-yaml/src/index.ts#L66-L70>. Node's `path.join` collapses interior `..` segments; Rust's `PathBuf::join` does not. Stored values like `../../../Users/zoltan/Library/pnpm/store/v11/links` therefore came back as `<modules_dir>/../../../Users/.../links` instead of the collapsed absolute form. The fallout: `Install::run`'s no-op short-circuit (added in #11904) compares this recovered path byte-for-byte against `Config::effective_virtual_store_dir()`, and the unnormalised form never matched the normalised config side. Every install whose store sits outside the project (the default `~/.local/share/pnpm/store` setup on macOS/Linux) silently skipped the fast path and re-ran the full materialisation pipeline on a clean install. Route the joined path through `pacquet_fs::lexical_normalize`. The new `round_trip_recovers_normalized_absolute_for_non_descendant_store` test pins the round-trip; verified to fail without this change. The no-op short-circuit now fires for the `lockfile+node_modules`-style cells in <https://benchmarks.vlt.sh>; the residual gap to pnpm on those cells is the resolution-verifier cold path (the abbreviated-modified shortcut + on-disk mirror layers upstream uses but pacquet has not yet ported — see the `Phase 4 stubs the abbreviated-shortcut and on-disk-mirror layers` comment in `create_npm_resolution_verifier`). --- Written by an agent (Claude Code, claude-opus-4-7).

… verifier shortcut (#11931) Two fixes that together unlock pnpm-parity on the `benchmarks.vlt.sh` `lockfile+node_modules` shape — the row where pacquet was 2-12× slower than pnpm on every fixture. ### 1. `fix(modules-yaml)`: normalise joined `virtualStoreDir` `read_modules_manifest` joins a stored relative `virtualStoreDir` with `modules_dir` to recover an absolute path, mirroring upstream's `path.join(modulesDir, modules.virtualStoreDir)`. Node's `path.join` normalises interior `..` segments; Rust's `PathBuf::join` does not. Stored values like `../../../Users/.../store/v11/links` came back as `<modules_dir>/../../../Users/.../links` — never byte-matched `Config::effective_virtual_store_dir()`, so the no-op short-circuit added in #11904 silently missed every install whose store sits outside the project (the default macOS / Linux setup). The accompanying refactor lifts `lexical_normalize` (already duplicated in `cmd-shim` and `store-dir`) into `pacquet-fs` so `modules-yaml` doesn't make it a third copy. ### 2. `perf(resolving-npm-resolver)`: port the missing verifier layers The npm resolution verifier walked a 4-layer fallback chain in upstream pnpm (abbreviated-modified shortcut → on-disk full-meta mirror → npm attestation endpoint → full packument fetch); pacquet only had the last two. The module's doc-comment explicitly noted "Phase 4 stubs the abbreviated-shortcut and on-disk-mirror layers (no cached fetcher / no mirror yet); Phase 5 ports `fetchFullMetadataCached.ts`…" — this is Phase 5. Result: a cold lockfile-verification pass now pays at most one *abbreviated* GET per name (small payload, decided by package-level `modified`) instead of a full-meta GET per name (hundreds of KB each). ## Bench 5-iteration cold-cache measured pass on `vltpkg/benchmarks/fixtures/svelte` (`pnpm-lock.yaml` + `node_modules` present, `~/.cache/pnpm` and store wiped before each run), 10-core M-series Mac: | | pnpm | pacquet@main | this PR | |-------------|------:|-------------:|--------:| | wall time | 0.54 s | 2.16 s | 0.71 s | 3.0× faster on the `lockfile+node_modules` row.

coderabbitai Bot requested changes May 24, 2026

View reviewed changes

Comment thread pacquet/crates/package-manager/src/install.rs

Comment thread pacquet/crates/package-manager/src/install/tests.rs Outdated

Comment thread pacquet/crates/package-manager/src/install/tests.rs Outdated

coderabbitai Bot approved these changes May 24, 2026

View reviewed changes

zkochan merged commit e549cd1 into main May 24, 2026
28 checks passed

zkochan deleted the perf/pacquet-up-to-date-short-circuit branch May 24, 2026 16:36

zkochan mentioned this pull request May 25, 2026

perf(pacquet): unlock no-op short-circuit + port abbreviated-modified verifier shortcut #11931

Merged

6 tasks

zkochan mentioned this pull request May 27, 2026

chore(release): 11.4.0 #11989

Merged

coderabbitai Bot mentioned this pull request Jun 20, 2026

feat(pacquet): purge node_modules on layout changes matching validateModules #12536

Merged

4 tasks

coderabbitai Bot mentioned this pull request Jun 28, 2026

fix(package-manager): don't wipe node_modules on an included-deps change #12676

Open

2 tasks

Uh oh!

Uh oh!

Conversation

zkochan commented May 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Summary by CodeRabbit

Uh oh!

qodo-code-review Bot commented May 24, 2026

Qodo reviews are paused for this user.

Uh oh!

coderabbitai Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Micro-Benchmark Results

Linux

Uh oh!

codecov-commenter commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented May 24, 2026

Integrated-Benchmark Report (Linux)

Scenario: Isolated linker: fresh restore, cold cache + cold store

Scenario: Isolated linker: fresh restore, hot cache + hot store

Scenario: Isolated linker: fresh install, cold cache + cold store

Scenario: Isolated linker: fresh install, hot cache + hot store

Uh oh!

github-actions Bot commented May 24, 2026

Bencher Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented May 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 24, 2026 •

edited

Loading

github-actions Bot commented May 24, 2026 •

edited

Loading

codecov-commenter commented May 24, 2026 •

edited

Loading