Skip to content

perf(pnpr): stream /v1/resolve, and fix the integrated benchmark to actually exercise pnpr#12237

Merged
zkochan merged 6 commits into
mainfrom
perf/12234
Jun 6, 2026
Merged

perf(pnpr): stream /v1/resolve, and fix the integrated benchmark to actually exercise pnpr#12237
zkochan merged 6 commits into
mainfrom
perf/12234

Conversation

@zkochan

@zkochan zkochan commented Jun 6, 2026

Copy link
Copy Markdown
Member

Closes #12234.

This PR has two parts. The headline turned out to be the benchmark, not the feature.

Part 1 — /v1/resolve streaming (the #12234 feature)

POST /v1/resolve now streams NDJSON instead of buffering the whole lockfile: one package frame per resolved tarball as the server's tree walk yields it, then a terminal done (full lockfile + stats), error, or violations frame. The client fetches each tarball as its frame arrives, overlapping the server's resolution — matching the native PrefetchingResolver shape.

  • Server: new ResolutionObserver/ObservingResolver in package-manager, threaded through Install; handle_resolve runs the resolve in a detached task whose observer pushes frames into the response channel; application/x-ndjson is excluded from the gzip layer so frames flush incrementally.
  • Client: resolve_streaming(opts, on_package); install_via_pnpr drives a new TarballPrefetcher that warms the shared mem cache as frames arrive.
  • Breaking change within protocol v1 (no version bump — experimental pnpr allows it).

Honest caveat: streaming only helps when the server's resolve is slow (cold/distant server). Against a warm server it's inert — see the results below (pnpr@HEADpnpr@main everywhere). Whether to keep this commit or defer it is an open question; the benchmark fixes below stand on their own.

Part 2 — make the integrated benchmark actually measure pnpr

While validating Part 1, the benchmark turned out not to be exercising pnpr at all, plus it was serving the registry far faster than reality. Fixes:

  • pnpr@<rev> targets never routed through pnpr. .pnpr-env exported a bare PNPR_SERVER, but pacquet reads config env vars only under the PNPM_CONFIG_* prefix, so config.pnpr_server was always None and every pnpr row was a silent duplicate of its direct row. Fixed the env var name; added a post-run guard that fails the benchmark if a pnpr target's pnpr-storage is empty (proof it never served a resolve).
  • Emulate a real registry link for every client. The latency proxy modeled RTT but not throughput, and fronted only direct targets. Generalized it to a LinkProfile (one-way delay + per-direction bandwidth cap), added --registry-bandwidth-mbps, and routed all registry traffic through it (direct installs, the pnpr server's resolve, the pnpr client's fetches) so the registry-mock is uniformly as remote as real npm. CI runs it at 50 ms + 200 Mbit/s (≈ the measured public-npm peak).
  • Make "cold cache" cold for resolution. Forced cacheDir bench-local and wipe it in cold-cache scenarios, so a direct install actually pays the packument-fetch waterfall (previously the global metadata mirror survived every wipe).
  • New scenario fresh-install.cold-cache.hot-store that isolates resolution (cold metadata, hot store → no download to mask it).

Results (Linux CI, after the fixes)

Scenario pacquet@HEAD pnpr@HEAD pnpr speedup
fresh-install · cold cache · hot store 5.06 s 0.69 s 7.4×
fresh-install · cold cache · cold store 5.36 s 2.03 s 2.65×
fresh-install · hot cache · hot store 1.46 s 0.69 s 2.1×
fresh-restore (frozen) · cold cache · cold store 10.08 s 5.13 s 2.0×
fresh-restore (frozen) · hot cache · hot store 0.71 s 0.80 s 0.89× (slower)

pnpr offloads the client's resolution (and, on the frozen path, lockfile verification) to its warm server: 2–7× faster wherever the client would otherwise pay that cost. The lone regression is the fully-warm frozen install, where there's nothing to offload and pnpr's one round trip is pure overhead. pnpr@HEAD vs pnpr@main is flat throughout — i.e. the streaming commit (Part 1) adds ~nothing against a warm server, while the base pnpr win (offload to a warm server) is large.

Pacquet/pnpr only — no TS mirror and no changeset.


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

Summary by CodeRabbit

  • New Features

    • Streaming NDJSON resolution protocol for real-time dependency feedback during installation
    • Background tarball prefetching to improve installation performance
    • Registry bandwidth throttling support in integrated benchmarks
    • New benchmark scenario for fresh installs with cold cache and hot store
  • Improvements

    • Enhanced network link emulation combining bandwidth and latency effects

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR implements streaming resolution with incremental tarball prefetching and extends integrated benchmarks with bandwidth-aware network emulation. The pnpr server now emits NDJSON frames per-package during resolution, the client parses these streams with callbacks, and the install pipeline uses observers to trigger background tarball downloads. Benchmarks gain a LinkProfile-based proxy for latency and bandwidth limits, a new cold-cache/hot-store scenario, and updated CI workflow coverage.

Changes

Streaming Resolution and Tarball Prefetching

Layer / File(s) Summary
Resolution observer contracts and crate exports
pacquet/crates/package-manager/src/resolution_observer.rs, lib.rs
New ResolutionObserver trait and ResolvedPackageHint struct; ObservingResolver wrapper deduplicates and notifies on resolved tarballs; crate-level re-exports enable public API.
Install pipeline observer field and wiring
pacquet/crates/package-manager/src/install.rs, install_with_fresh_lockfile.rs, add.rs, remove.rs, update.rs, install/tests.rs
Install and InstallWithFreshLockfile add optional resolution_observer fields; fresh-lockfile path conditionally wraps resolver with ObservingResolver; all call sites updated to pass explicit observer values (usually None).
Tarball prefetch orchestration and spawned downloads
pacquet/crates/package-manager/src/tarball_prefetch.rs
New TarballPrefetcher coordinator spawns background downloads triggered by streamed package hints; deduplicates by URL; manages shared MemCache and store-index writer lifecycle.
CLI pnpr install streaming and prefetch integration
pacquet/crates/cli/src/cli_args/install.rs
Pnpr install path refactored to build ResolveOptions/PnprClient explicitly; conditionally creates TarballPrefetcher and uses resolve_streaming with per-package prefetch callbacks; awaits prefetcher shutdown after materialization.
pnpr server NDJSON streaming protocol
pnpr/crates/pnpr/src/resolver.rs, server.rs
/v1/resolve handler emits NDJSON package frames per-resolved tarball plus terminal done/error/violations frame; refactored shared cache to Arc<Mutex<>> for detached streaming task; NDJSON protocol helpers and compression exclusion.
pnpr client streaming API and frame parsing
pacquet/crates/pnpr-client/src/lib.rs, src/tests.rs, tests/integration.rs, Cargo.toml
resolve_streaming method with per-package callback; ResolvedPackage struct for streamed frames; NDJSON frame parsing and deserialization; updated tests for frame parsing and streamed-package validation.
pnpr resolve function observer parameter
pnpr/crates/pnpr/src/resolver/resolve.rs
Public resolve API accepts optional ResolutionObserver parameter forwarded into Install configuration for streaming observer notifications.

Benchmark Network Emulation and Scenario Expansion

Layer / File(s) Summary
LinkProfile configuration and proxy throughput pacing
pacquet/tasks/integrated-benchmark/src/latency_proxy.rs, latency_proxy/tests.rs
New LinkProfile struct with one-way latency and optional rate-limit; LatencyProxy::spawn updated to accept profile; pump scheduling enforces per-chunk latency and sustained serialization time limits; new bandwidth throttling test.
Benchmark CLI scenarios, inputs, and runner wiring
pacquet/tasks/integrated-benchmark/src/cli_args.rs, main.rs
Adds --registry-bandwidth-mbps CLI option; introduces IsolatedFreshInstallColdCacheHotStore scenario variant; wires registry bandwidth through main and WorkEnv initialization.
WorkEnv pnpr verification, routing, and workspace cache defaults
pacquet/tasks/integrated-benchmark/src/work_env.rs
Pnpr-target routing verification via dir_contains_file helper; corrects pnpr env var exports; expands cleanup scope; conditionally spawns bandwidth-aware registry proxy with LinkProfile routing; guarantees cacheDir workspace defaults.
GitHub Actions scenario execution and result reporting
.github/workflows/pacquet-integrated-benchmark.yml
Adds REGISTRY_BANDWIDTH_MBPS environment variable; passes bandwidth to benchmark invocations; executes new cold-cache/hot-store scenario; extends summary output; updates bencher result tag mappings.

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • pnpm/pnpm#12234: Both implement streaming NDJSON /v1/resolve frames so clients can start fetching tarballs during resolution.
  • pnpm/pnpm#11842: Changes introduce observer-based resolution streaming and tarball prefetching that directly impact the resolve performance gap addressed by the issue.

Possibly related PRs

  • pnpm/pnpm#12166: Introduces the latency proxy and LatencyProxy::spawn(upstream, one_way_delay); this PR extends it with LinkProfile and bandwidth rate-limiting.
  • pnpm/pnpm#11837: Implements pipelined tarball prefetching into shared MemCache/store index during resolution, similar to this PR's TarballPrefetcher approach.
  • pnpm/pnpm#12154: Both modify integrated-benchmark harness and pnpr@ routing via work_env.rs; this PR adds registry bandwidth and new scenario/report enhancements.

Suggested reviewers

  • KSXGitHub

Poem

🐰 A rabbit hops through streams so bright,
Where packages flow in NDJSON light,
Prefetching tarballs as resolution flows,
While bandwidth-aware proxies set the tempo,
Fresh scenarios blooming in benchmarks cold—
Streaming futures, swiftly told! 🌟

🚥 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 PR title clearly summarizes the main changes: streaming implementation for /v1/resolve and benchmark fixes for pnpr exercise.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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/12234

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.

@codecov-commenter

codecov-commenter commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.45299% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.38%. Comparing base (da4c80c) to head (6ee4886).

Files with missing lines Patch % Lines
.../crates/package-manager/src/resolution_observer.rs 76.74% 10 Missing ⚠️
...uet/crates/package-manager/src/tarball_prefetch.rs 92.10% 9 Missing ⚠️
pnpr/crates/pnpr/src/resolver.rs 93.54% 6 Missing ⚠️
pacquet/crates/pnpr-client/src/lib.rs 87.09% 4 Missing ⚠️
pacquet/crates/cli/src/cli_args/install.rs 98.14% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #12237    +/-   ##
========================================
  Coverage   87.37%   87.38%            
========================================
  Files         276      278     +2     
  Lines       32254    32506   +252     
========================================
+ Hits        28182    28404   +222     
- Misses       4072     4102    +30     

☔ View full report in Codecov by Harness.
📢 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.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.01      8.2±0.47ms   530.0 KB/sec    1.00      8.1±0.29ms   536.8 KB/sec

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Each scenario has pacquet rows (direct install) and pnpr rows (the same client through the pnpr install accelerator), so pnpr@HEAD vs pacquet@HEAD is the pnpr-vs-direct ratio. Cold-store scenarios wipe the client store between runs (warm server); hot-store scenarios keep it warm. The pacquet@HEAD rows feed the pacquet Bencher testbed; the pnpr@HEAD rows feed the pnpr testbed.

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 10.075 ± 0.089 9.967 10.266 2.00 ± 0.03
pacquet@main 10.095 ± 0.138 9.925 10.383 2.00 ± 0.04
pnpr@HEAD 5.129 ± 0.185 4.982 5.470 1.02 ± 0.04
pnpr@main 5.042 ± 0.063 4.984 5.203 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 10.07532117404,
      "stddev": 0.08939446537594474,
      "median": 10.074451896740001,
      "user": 3.4460005599999994,
      "system": 4.23050542,
      "min": 9.967256842740001,
      "max": 10.265628859740001,
      "times": [
        9.967256842740001,
        10.048601600740001,
        10.12057526974,
        10.265628859740001,
        10.01426623274,
        10.10818349874,
        10.13452491474,
        10.001428283740001,
        9.992444044740001,
        10.100302192740001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 10.09490842194,
      "stddev": 0.13843789891969868,
      "median": 10.07133997124,
      "user": 3.410986859999999,
      "system": 4.21926562,
      "min": 9.92457501374,
      "max": 10.382537472740001,
      "times": [
        10.06742552174,
        10.05486968774,
        9.92457501374,
        9.95058570974,
        10.19621991474,
        10.209148303740001,
        10.07525442074,
        9.97821422174,
        10.11025395274,
        10.382537472740001
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 5.128999056939999,
      "stddev": 0.18455548381488882,
      "median": 5.04425115974,
      "user": 2.56443366,
      "system": 3.82491822,
      "min": 4.9817530117399995,
      "max": 5.47033889074,
      "times": [
        5.04669971174,
        4.99061863374,
        5.02051641774,
        5.09254896374,
        5.03137535274,
        5.46323024274,
        5.15110673674,
        5.47033889074,
        5.041802607739999,
        4.9817530117399995
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 5.04236293534,
      "stddev": 0.06328214164351706,
      "median": 5.02811514274,
      "user": 2.5433338599999997,
      "system": 3.84683432,
      "min": 4.984445549739999,
      "max": 5.20335360974,
      "times": [
        5.03472533274,
        5.014025944739999,
        5.04379334074,
        4.995206221739999,
        5.000079664739999,
        5.0803773867399995,
        4.984445549739999,
        5.20335360974,
        5.02150495274,
        5.046117349739999
      ]
    }
  ]
}

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

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 708.1 ± 57.2 661.2 864.6 1.01 ± 0.11
pacquet@main 699.6 ± 52.6 668.4 836.3 1.00
pnpr@HEAD 799.1 ± 36.9 762.4 875.6 1.14 ± 0.10
pnpr@main 797.7 ± 67.5 757.4 985.2 1.14 ± 0.13
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.7080711184800002,
      "stddev": 0.05717949020848144,
      "median": 0.6994817120800001,
      "user": 0.36744057999999996,
      "system": 1.3583330200000001,
      "min": 0.6612320365800001,
      "max": 0.8646439215800001,
      "times": [
        0.8646439215800001,
        0.6995765715800001,
        0.6917943655800001,
        0.70193609158,
        0.6805140805800001,
        0.70601935258,
        0.6612320365800001,
        0.7062599005800001,
        0.6693480115800001,
        0.6993868525800001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6995844106800001,
      "stddev": 0.052613206036693366,
      "median": 0.6761151355800001,
      "user": 0.36658618,
      "system": 1.34455882,
      "min": 0.6683706465800001,
      "max": 0.8362502355800001,
      "times": [
        0.7195476195800001,
        0.7282961545800001,
        0.6735148595800001,
        0.67871541158,
        0.6789420835800001,
        0.8362502355800001,
        0.6701459215800001,
        0.67081181558,
        0.6683706465800001,
        0.6712493585800001
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.7991041602800001,
      "stddev": 0.03693289133360197,
      "median": 0.7822022020800001,
      "user": 0.38619908,
      "system": 1.3418946199999997,
      "min": 0.7624217575800001,
      "max": 0.8755681285800001,
      "times": [
        0.8515985595800001,
        0.7624217575800001,
        0.8755681285800001,
        0.7983235225800001,
        0.7787985505800001,
        0.7730738285800001,
        0.8107224495800001,
        0.7808400125800001,
        0.7761304015800001,
        0.7835643915800001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.79768920158,
      "stddev": 0.06752903938606072,
      "median": 0.7772592860800001,
      "user": 0.38598718,
      "system": 1.33740712,
      "min": 0.7574492465800001,
      "max": 0.9852131455800001,
      "times": [
        0.79642211358,
        0.80126918558,
        0.7759392445800001,
        0.9852131455800001,
        0.78519762258,
        0.7591033755800001,
        0.77770654258,
        0.7617795095800001,
        0.7768120295800001,
        0.7574492465800001
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 5.363 ± 0.059 5.245 5.445 2.65 ± 0.11
pacquet@main 5.331 ± 0.045 5.276 5.395 2.64 ± 0.11
pnpr@HEAD 2.025 ± 0.067 1.910 2.106 1.00 ± 0.05
pnpr@main 2.021 ± 0.080 1.919 2.176 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 5.36345689088,
      "stddev": 0.05946511280367522,
      "median": 5.3785344333800005,
      "user": 3.7329080599999998,
      "system": 3.27738514,
      "min": 5.2451970898799996,
      "max": 5.44539983688,
      "times": [
        5.39442047688,
        5.309303659879999,
        5.44539983688,
        5.4079764488799995,
        5.36783184188,
        5.40853097588,
        5.38923702488,
        5.35233013188,
        5.2451970898799996,
        5.31434142188
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 5.33067737668,
      "stddev": 0.04518272273158579,
      "median": 5.33067281788,
      "user": 3.69320576,
      "system": 3.2381426400000004,
      "min": 5.27637337688,
      "max": 5.39458175488,
      "times": [
        5.35658680588,
        5.28731316988,
        5.29052464888,
        5.36560132288,
        5.39458175488,
        5.36213758988,
        5.27637337688,
        5.28825840488,
        5.30475882988,
        5.38063786288
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 2.02513465948,
      "stddev": 0.06682355394386985,
      "median": 2.02693919688,
      "user": 2.4980736599999998,
      "system": 3.14023574,
      "min": 1.90987026788,
      "max": 2.10555170688,
      "times": [
        2.00387834088,
        1.99380982088,
        2.05339058188,
        1.9399724608800002,
        2.0044021288800002,
        1.90987026788,
        2.0992000548800003,
        2.09179496688,
        2.10555170688,
        2.04947626488
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 2.0214284503799997,
      "stddev": 0.07991453195002338,
      "median": 1.9965522093799999,
      "user": 2.5200280599999996,
      "system": 3.10950874,
      "min": 1.9190898418800002,
      "max": 2.17602400688,
      "times": [
        2.0882532828800002,
        1.99539506888,
        1.9190898418800002,
        2.17602400688,
        1.99770934988,
        1.96606260288,
        1.96549326988,
        2.01717510888,
        1.9734159798800002,
        2.1156659918800003
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 1.455 ± 0.024 1.428 1.501 2.13 ± 0.14
pacquet@main 1.481 ± 0.086 1.438 1.724 2.16 ± 0.19
pnpr@HEAD 0.685 ± 0.044 0.655 0.795 1.00
pnpr@main 0.702 ± 0.031 0.675 0.780 1.03 ± 0.08
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.4551937138000002,
      "stddev": 0.024414025066492817,
      "median": 1.4503993297,
      "user": 1.5637392,
      "system": 1.8041155199999999,
      "min": 1.4280100761999999,
      "max": 1.5011201851999998,
      "times": [
        1.4900975462,
        1.5011201851999998,
        1.4341586932,
        1.4539246912,
        1.4602120342,
        1.4622074672,
        1.4307441222,
        1.4445883542,
        1.4280100761999999,
        1.4468739682
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.4808369812,
      "stddev": 0.0861328554938481,
      "median": 1.4575679882,
      "user": 1.5775221999999998,
      "system": 1.81245312,
      "min": 1.4381692181999999,
      "max": 1.7240476322,
      "times": [
        1.7240476322,
        1.4579328802,
        1.4429740921999998,
        1.4714282172,
        1.4572030962,
        1.4427980282,
        1.4381692181999999,
        1.4613115522,
        1.4650725752,
        1.4474325202
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.6846402768,
      "stddev": 0.04407212721423959,
      "median": 0.6670618887,
      "user": 0.33456289999999994,
      "system": 1.27115082,
      "min": 0.6545028172,
      "max": 0.7950246402000001,
      "times": [
        0.7950246402000001,
        0.7280231772000001,
        0.6575393812000001,
        0.6656161352000001,
        0.6692834332,
        0.6638865062000001,
        0.6545028172,
        0.6810610142000001,
        0.6629580212,
        0.6685076422
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.7022912449000001,
      "stddev": 0.030609591595600717,
      "median": 0.6936953402000001,
      "user": 0.3350733,
      "system": 1.2883602199999997,
      "min": 0.6750772962000001,
      "max": 0.7801386542000001,
      "times": [
        0.7801386542000001,
        0.6792082422000001,
        0.7083762472,
        0.6944050522,
        0.6790412622,
        0.7177075122000001,
        0.6909941942000001,
        0.7049783602,
        0.6929856282000001,
        0.6750772962000001
      ]
    }
  ]
}

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

Resolution-only: cold packument cache (full re-resolve over the registry link) with a hot store (no tarball download), so this isolates pnpr offloading the client resolution to its warm server.

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 5.059 ± 0.026 5.024 5.094 7.36 ± 0.29
pacquet@main 5.076 ± 0.027 5.041 5.118 7.39 ± 0.29
pnpr@HEAD 0.687 ± 0.027 0.669 0.761 1.00
pnpr@main 0.718 ± 0.031 0.689 0.800 1.04 ± 0.06
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 5.05919852782,
      "stddev": 0.0264139834921337,
      "median": 5.05257658102,
      "user": 1.7578535800000001,
      "system": 1.96855546,
      "min": 5.024411193520001,
      "max": 5.09431015852,
      "times": [
        5.05237398352,
        5.044688180520001,
        5.07314055752,
        5.024411193520001,
        5.087366057520001,
        5.09431015852,
        5.0278185445200005,
        5.040846423520001,
        5.09425100052,
        5.052779178520001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 5.07644643602,
      "stddev": 0.027027233555889197,
      "median": 5.071767897020001,
      "user": 1.77369608,
      "system": 1.9654242600000003,
      "min": 5.04098981952,
      "max": 5.11827130352,
      "times": [
        5.0975992555200005,
        5.053252644520001,
        5.11827130352,
        5.07773141152,
        5.09311977052,
        5.04416384052,
        5.10837963452,
        5.04098981952,
        5.0658043825200005,
        5.06515229752
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.68738627742,
      "stddev": 0.026922626002667083,
      "median": 0.67977438152,
      "user": 0.34814007999999996,
      "system": 1.2921982600000002,
      "min": 0.66877744652,
      "max": 0.76082411452,
      "times": [
        0.68301994952,
        0.69001378252,
        0.66877744652,
        0.76082411452,
        0.6749744935199999,
        0.69189288252,
        0.68049759852,
        0.66940984052,
        0.67905116452,
        0.67540150152
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.71797772672,
      "stddev": 0.031186508978150076,
      "median": 0.7117728185200001,
      "user": 0.34977398,
      "system": 1.29487076,
      "min": 0.68940756052,
      "max": 0.80013296752,
      "times": [
        0.70206027952,
        0.68940756052,
        0.69815420652,
        0.72130554752,
        0.72929062552,
        0.71300204552,
        0.71054359152,
        0.80013296752,
        0.71570962152,
        0.70017082152
      ]
    }
  ]
}

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12237
Testbedpacquet

🚨 2 Alerts

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
isolated-linker.fresh-install.cold-cache.cold-storeLatency
seconds (s)
📈 plot
🚷 threshold
🚨 alert (🔔)
5.36 s
(+143.27%)Baseline: 2.20 s
2.65 s
(202.72%)

isolated-linker.fresh-restore.cold-cache.cold-storeLatency
seconds (s)
📈 plot
🚷 threshold
🚨 alert (🔔)
10.08 s
(+144.42%)Baseline: 4.12 s
4.95 s
(203.68%)

Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
isolated-linker.fresh-install.cold-cache.cold-store📈 view plot
🚷 view threshold
🚨 view alert (🔔)
5,363.46 ms
(+143.27%)Baseline: 2,204.78 ms
2,645.73 ms
(202.72%)

isolated-linker.fresh-install.cold-cache.hot-store📈 view plot
🚷 view threshold
5,059.20 ms
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
🚷 view threshold
1,455.19 ms
(+8.70%)Baseline: 1,338.74 ms
1,606.48 ms
(90.58%)
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
🚷 view threshold
🚨 view alert (🔔)
10,075.32 ms
(+144.42%)Baseline: 4,122.16 ms
4,946.59 ms
(203.68%)

isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
🚷 view threshold
708.07 ms
(+4.33%)Baseline: 678.66 ms
814.39 ms
(86.94%)
🐰 View full continuous benchmarking report in Bencher

@zkochan zkochan force-pushed the perf/12234 branch 2 times, most recently from d5bb0fe to df6f3c2 Compare June 6, 2026 09:54
@zkochan zkochan changed the title perf(pnpr): stream resolved packages from /v1/resolve so the client overlaps tarball fetch with resolution perf(pnpr): stream /v1/resolve, and fix the integrated benchmark to actually exercise pnpr Jun 6, 2026
@zkochan zkochan marked this pull request as ready for review June 6, 2026 12:35
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Stream /v1/resolve as NDJSON and fix integrated benchmark to measure pnpr performance accurately

✨ Enhancement 🧪 Tests 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• **Stream /v1/resolve as NDJSON**: POST /v1/resolve now streams resolved packages incrementally
  instead of buffering the entire lockfile, with per-package frames followed by a terminal
  done/error/violations frame, enabling client-side tarball prefetching to overlap with server
  resolution
• **Implement client-side tarball prefetching**: New TarballPrefetcher spawns background downloads
  as package frames arrive from the server, warming the shared memory cache before the install phase
• **Fix integrated benchmark to actually exercise pnpr**: Corrected env var naming
  (PNPM_CONFIG_PNPR_SERVER), added verification that pnpr targets were routed through the server,
  and added post-run guards to detect silent failures
• **Emulate realistic registry link**: Generalized latency proxy to LinkProfile supporting both
  latency and bandwidth caps, routed all registry traffic (direct installs, pnpr server resolves, pnpr
  client fetches) through it uniformly, and added --registry-bandwidth-mbps CLI argument
• **Isolate resolution performance**: Forced cacheDir bench-local, wiped it in cold-cache
  scenarios, and added new fresh-install.cold-cache.hot-store scenario to measure resolution cost
  without download masking
• **Benchmark results**: pnpr achieves 7.4× speedup on cold-cache hot-store (resolution-only), 2.65×
  on cold-cache cold-store, 2.1× on hot-cache hot-store, and 2.0× on frozen restore
• **Wire resolution observer through install pipeline**: Added ResolutionObserver trait and
  ObservingResolver wrapper to instrument resolver, threaded through Install and
  InstallWithFreshLockfile structs
• **Update pnpr client protocol**: Changed from buffered gzip JSON to streaming NDJSON with
  frame-based parsing, added resolve_streaming() method with per-package callbacks, and updated
  tests for frame variants
• **Exclude NDJSON from gzip layer**: Modified compression predicate to bypass buffering for
  application/x-ndjson content type, enabling incremental frame flushing
• **Refactor tarball download logic**: Extracted spawn_tarball_download() function and
  TarballDownload struct into reusable tarball_prefetch module
• **Update all install operations**: Added resolution_observer: None field to Install
  configurations in add, remove, update, and test operations for consistency
• **Update CI workflow**: Added bandwidth-limited registry link emulation (200 Mbit/s) and new
  resolution-only benchmark scenario to CI pipeline
• **Dependency update**: Replaced flate2 with futures-util to support streaming response
  handling
Diagram
flowchart LR
  A["pnpr Server<br/>ResolutionObserver"] -- "streams package frames<br/>as NDJSON" --> B["pnpr Client<br/>TarballPrefetcher"]
  B -- "prefetches tarballs<br/>to MemCache" --> C["Install Phase<br/>cache hits"]
  D["Benchmark<br/>LinkProfile"] -- "caps bandwidth +<br/>latency uniformly" --> E["Registry Mock"]
  E -- "serves all clients<br/>direct + pnpr" --> F["Fair Comparison"]
  G["Cold Cache<br/>Isolation"] -- "wipes cache-dir<br/>per iteration" --> H["Accurate Resolution<br/>Measurement"]

Loading

Grey Divider

File Changes

1. pacquet/crates/package-manager/src/install/tests.rs 🧪 Tests +62/-0

Add resolution_observer field to install tests

• Added resolution_observer: None field to all test Install builder invocations
• Maintains consistency with new resolution_observer field added to Install struct
• No functional changes to test logic, purely structural updates

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


2. pacquet/tasks/integrated-benchmark/src/work_env.rs ✨ Enhancement +158/-68

Enhance benchmark with bandwidth caps and routing verification

• Added registry_bandwidth_mbps field to emulate throughput caps on registry link
• Generalized latency proxy to use LinkProfile struct supporting both latency and bandwidth
• Fixed env var name from PNPR_SERVER to PNPM_CONFIG_PNPR_SERVER for proper config reading
• Added cache-dir to pre-benchmark wipe and verification that pnpr targets were actually routed
• Extended registry proxy to front all clients (direct + pnpr) uniformly

pacquet/tasks/integrated-benchmark/src/work_env.rs


3. pacquet/crates/pnpr/src/resolver.rs ✨ Enhancement +0/-0

Stream /v1/resolve as NDJSON with per-package frames

• Changed /v1/resolve response from gzipped JSON to NDJSON streaming format
• Implemented StreamObserver to emit package frames as tarballs resolve
• Added terminal frame types: done (success), error (failure), violations (policy rejection)
• Detached resolve task pushes frames into channel while response body drains them
• Excluded NDJSON from gzip layer to enable incremental frame flushing

pacquet/crates/pnpr/src/resolver.rs


View more (23)
4. pacquet/tasks/integrated-benchmark/src/latency_proxy.rs ✨ Enhancement +61/-31

Add bandwidth rate-limiting to latency proxy

• Introduced LinkProfile struct to model both latency and bandwidth constraints
• Enhanced proxy to apply per-direction throughput caps alongside latency injection
• Updated pump logic to track link availability and pace chunks at rate limit
• Expanded documentation to explain bandwidth emulation rationale

pacquet/tasks/integrated-benchmark/src/latency_proxy.rs


5. pacquet/crates/package-manager/src/tarball_prefetch.rs ✨ Enhancement +250/-0

Add tarball prefetcher for pnpr streaming resolution

• New module providing TarballPrefetcher for pnpr client tarball prefetching
• Spawns background downloads as package frames stream from server
• Shares MemCache with install pass for immediate cache hits
• Manages store-index writer to persist downloaded tarball metadata

pacquet/crates/package-manager/src/tarball_prefetch.rs


6. pacquet/crates/pnpr-client/src/lib.rs ✨ Enhancement +101/-51

Implement streaming NDJSON resolution with per-package callbacks

• Added resolve_streaming() method that invokes callback for each package frame
• Changed response parsing from gzipped JSON to line-by-line NDJSON frame parsing
• Introduced ResolvedPackage struct to carry per-frame fetch hints
• Replaced Frame enum with tagged variants for package, done, error, violations
• Removed gzip decompression logic (NDJSON streams plain JSON lines)

pacquet/crates/pnpr-client/src/lib.rs


7. pacquet/tasks/integrated-benchmark/src/cli_args.rs ✨ Enhancement +55/-15

Add bandwidth CLI arg and cold-cache-hot-store scenario

• Added --registry-bandwidth-mbps CLI argument for throughput cap emulation
• Added IsolatedFreshInstallColdCacheHotStore scenario to isolate resolution cost
• Updated scenario cleanup to wipe cache-dir in cold-cache scenarios
• Enhanced documentation explaining bandwidth cap rationale and registry uniformity

pacquet/tasks/integrated-benchmark/src/cli_args.rs


8. pacquet/crates/package-manager/src/prefetching_resolver.rs Refactoring +22/-60

Extract tarball download logic into reusable module

• Refactored tarball download spawning into shared spawn_tarball_download() function
• Extracted TarballDownload struct to encapsulate download parameters
• Simplified resolver code by delegating to new tarball_prefetch module
• Maintains same prefetch semantics with cleaner separation of concerns

pacquet/crates/package-manager/src/prefetching_resolver.rs


9. pacquet/crates/cli/src/cli_args/install.rs ✨ Enhancement +80/-37

Wire tarball prefetcher into pnpr client install flow

• Integrated TarballPrefetcher into pnpr install path
• Spawns prefetcher that fires downloads as package frames stream in
• Calls resolve_streaming() with prefetch callback instead of resolve()
• Drains prefetcher after materialization install to persist store-index rows
• Added resolution_observer: None to frozen materialization install

pacquet/crates/cli/src/cli_args/install.rs


10. pacquet/crates/package-manager/src/resolution_observer.rs ✨ Enhancement +113/-0

Add resolution observer trait and wrapper resolver

• New module providing ResolutionObserver trait for resolver instrumentation
• Implemented ObservingResolver wrapper that reports each tarball resolution
• Deduplicates reports by tarball URL to collapse multi-edge packages
• Enables pnpr server to stream fetch hints to client without I/O overhead

pacquet/crates/package-manager/src/resolution_observer.rs


11. pacquet/crates/pnpr-client/src/tests.rs 🧪 Tests +32/-15

Update pnpr client tests for NDJSON frame protocol

• Updated tests to parse NDJSON Frame variants instead of monolithic response
• Added test for package frame parsing with fetch hint fields
• Renamed tests to reflect frame-based protocol (e.g., violations_frame)
• Simplified test assertions to work with tagged frame enum

pacquet/crates/pnpr-client/src/tests.rs


12. pacquet/tasks/integrated-benchmark/src/latency_proxy/tests.rs 🧪 Tests +50/-3

Add bandwidth rate-limit test to latency proxy

• Updated existing latency test to use LinkProfile struct
• Added new test caps_throughput_to_the_rate_limit() validating bandwidth cap
• Verifies 256 KiB transfer at 1 MB/s takes ~0.25s (not sub-millisecond loopback)

pacquet/tasks/integrated-benchmark/src/latency_proxy/tests.rs


13. pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs ✨ Enhancement +17/-0

Wire resolution observer into fresh lockfile install

• Added resolution_observer field to InstallWithFreshLockfile struct
• Wrapped resolver chain with ObservingResolver when observer is provided
• Observer reports each resolved tarball as tree walk yields it
• Enables pnpr server to stream fetch frames to client during resolution

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs


14. pacquet/crates/pnpr-client/tests/integration.rs 🧪 Tests +34/-0

Add integration test for streaming resolution

• Added integration test streams_resolved_packages_before_the_lockfile()
• Validates that package frames arrive before terminal done frame
• Confirms all streamed packages appear in final lockfile
• Verifies frame fields (integrity, tarball URL, id format)

pacquet/crates/pnpr-client/tests/integration.rs


15. pacquet/crates/package-manager/src/install.rs ✨ Enhancement +9/-0

Add resolution observer field to Install struct

• Added resolution_observer field to Install struct
• Documented observer purpose: stream fetch frames during resolution
• Passed observer through to InstallWithFreshLockfile
• Ignored on frozen path (no tree walk to observe)

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


16. pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs ✨ Enhancement +9/-0

Add cache_dir to workspace manifest for cold-cache isolation

• Added cache_dir field to MinimalWorkspaceManifest struct
• Forces packument-metadata cache bench-local (./cache-dir)
• Ensures cold-cache scenarios actually wipe resolution metadata
• Prevents global cache from surviving per-iteration cleanup

pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs


17. pnpr/crates/pnpr/src/resolver/resolve.rs ✨ Enhancement +7/-1

Thread resolution observer through resolve function

• Added ResolutionObserver import to support streaming resolution updates
• Added optional observer parameter to resolve() function signature
• Threaded resolution_observer through Install configuration to enable streaming of resolved
 packages to clients
• Added documentation explaining how the observer streams each resolved tarball as NDJSON frames

pnpr/crates/pnpr/src/resolver/resolve.rs


18. pnpr/crates/pnpr/src/server.rs ✨ Enhancement +11/-7

Exclude NDJSON streams from gzip compression layer

• Extended gzip compression layer to exclude application/x-ndjson content type alongside existing
 application/octet-stream exclusion
• Added detailed comment explaining why NDJSON streaming responses must bypass gzip buffering to
 allow incremental frame flushing
• Restructured compression predicate logic for clarity with explicit .and() chaining

pnpr/crates/pnpr/src/server.rs


19. pacquet/tasks/integrated-benchmark/src/main.rs ✨ Enhancement +2/-0

Add registry bandwidth parameter to benchmark

• Added registry_bandwidth_mbps parameter to benchmark CLI arguments
• Threaded registry_bandwidth_mbps through benchmark configuration to enable bandwidth-limited
 registry link emulation

pacquet/tasks/integrated-benchmark/src/main.rs


20. pacquet/crates/package-manager/src/lib.rs ✨ Enhancement +4/-0

Export new resolution observer and tarball prefetch modules

• Added new module resolution_observer and exported its public interface
• Added new module tarball_prefetch and exported its public interface
• These modules support streaming resolution and prefetching tarballs during pnpr client operations

pacquet/crates/package-manager/src/lib.rs


21. pacquet/crates/package-manager/src/remove.rs 🐞 Bug fix +1/-0

Initialize resolution observer field in remove operation

• Added resolution_observer: None to Install configuration in remove operation
• Ensures consistency with updated Install struct that now requires the observer field

pacquet/crates/package-manager/src/remove.rs


22. pacquet/crates/package-manager/src/add.rs 🐞 Bug fix +1/-0

Initialize resolution observer field in add operation

• Added resolution_observer: None to Install configuration in add operation
• Ensures consistency with updated Install struct that now requires the observer field

pacquet/crates/package-manager/src/add.rs


23. pacquet/crates/package-manager/src/update.rs 🐞 Bug fix +1/-0

Initialize resolution observer field in update operation

• Added resolution_observer: None to Install configuration in update operation
• Ensures consistency with updated Install struct that now requires the observer field

pacquet/crates/package-manager/src/update.rs


24. .github/workflows/pacquet-integrated-benchmark.yml ✨ Enhancement +56/-18

Add bandwidth-limited registry link and resolution-only benchmark scenario

• Added REGISTRY_BANDWIDTH_MBPS: 200 environment variable to emulate realistic registry throughput
 (~190 Mbit/s peak)
• Updated all benchmark step commands to pass --registry-bandwidth-mbps parameter for consistent
 network emulation
• Added new benchmark scenario isolated-linker.fresh-install.cold-cache.hot-store to isolate
 resolution performance (cold metadata, hot store)
• Expanded benchmark report generation to include the new resolution-only scenario with detailed
 documentation
• Updated Bencher result mapping to include the new cold-cache.hot-store scenario for pnpr and
 pacquet targets
• Enhanced workflow comments explaining network emulation strategy and fair pnpr-vs-direct
 comparison methodology

.github/workflows/pacquet-integrated-benchmark.yml


25. pacquet/crates/pnpr-client/Cargo.toml Dependencies +1/-1

Replace flate2 with futures-util for streaming support

• Replaced flate2 dependency with futures-util to support streaming response handling
• Aligns with shift from buffered gzip decompression to streaming NDJSON frame processing

pacquet/crates/pnpr-client/Cargo.toml


26. pnpr/crates/pnpr/src/resolver.rs Additional files +165/-80

...

pnpr/crates/pnpr/src/resolver.rs


Grey Divider

Qodo Logo

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/tasks/integrated-benchmark/src/work_env.rs`:
- Around line 659-661: The function mbps_to_bytes_per_sec currently truncates
tiny positive Mbps to 0 which yields an invalid throughput cap; update
mbps_to_bytes_per_sec to compute bytes_per_sec = (mbps * 125_000.0).ceil() cast
to u64 (or otherwise ensure rounding up) and then clamp to at least 1 before
returning Some(value) for mbps > 0.0, preserving None for non-positive inputs;
refer to the mbps_to_bytes_per_sec function name to locate and modify the
implementation.
🪄 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: 150ecdcc-9dbf-499c-b0f4-e3ddd9702d08

📥 Commits

Reviewing files that changed from the base of the PR and between 1f549eb and 8580abb.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (25)
  • .github/workflows/pacquet-integrated-benchmark.yml
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/package-manager/src/lib.rs
  • pacquet/crates/package-manager/src/prefetching_resolver.rs
  • pacquet/crates/package-manager/src/remove.rs
  • pacquet/crates/package-manager/src/resolution_observer.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
  • pacquet/crates/package-manager/src/update.rs
  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/pnpr-client/src/lib.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/pnpr-client/tests/integration.rs
  • pacquet/tasks/integrated-benchmark/src/cli_args.rs
  • pacquet/tasks/integrated-benchmark/src/latency_proxy.rs
  • pacquet/tasks/integrated-benchmark/src/latency_proxy/tests.rs
  • pacquet/tasks/integrated-benchmark/src/main.rs
  • pacquet/tasks/integrated-benchmark/src/work_env.rs
  • pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs
  • pnpr/crates/pnpr/src/resolver.rs
  • pnpr/crates/pnpr/src/resolver/resolve.rs
  • pnpr/crates/pnpr/src/server.rs

Comment thread pacquet/tasks/integrated-benchmark/src/work_env.rs
zkochan added 6 commits June 6, 2026 15:01
`POST /v1/resolve` now streams NDJSON instead of buffering the whole
lockfile: one `package` frame per resolved tarball as the server's tree
walk yields it, then a terminal `done` (full lockfile + stats), `error`,
or `violations` frame. The client fetches each tarball the moment its
frame arrives, so download overlaps the server's resolution — giving the
pnpr path the same fetch-overlaps-resolution shape the native
`PrefetchingResolver` already has (#12234).

Server: new `ResolutionObserver`/`ObservingResolver` in package-manager,
threaded through `Install`; `handle_resolve` runs the resolve in a
detached task whose observer pushes frames into the response channel.
`application/x-ndjson` is excluded from the gzip layer so frames flush
incrementally.

Client: `resolve_streaming(opts, on_package)` consumes the stream;
`install_via_pnpr` drives a new `TarballPrefetcher` that warms the shared
mem cache as frames arrive, so the frozen materialization install finds
every tarball already fetched.

Breaking change within protocol v1 (no version bump — experimental pnpr
allows breaking changes).
…ents

The latency proxy modeled RTT but not throughput, and fronted the
registry only for direct targets, so tarball downloads ran at loopback
speed (~GB/s) — orders of magnitude faster than the public npm registry
(measured ~190 Mbit/s / ~24 MB/s peak on a fast link, far less on a
typical one). With downloads effectively free, any optimization that
overlaps or shortens tarball fetch — including the /v1/resolve streaming
in this branch — has nothing to show in the benchmark.

Generalize the proxy from a latency-only delay to a LinkProfile (one-way
delay + optional per-direction bytes/sec cap) and add
--registry-bandwidth-mbps. Route every client through it — direct
pacquet/pnpm installs, the pnpr server's resolution, and the pnpr
client's tarball fetches alike — because a request to the registry-mock
should cost the same regardless of who makes it; only the untimed
proxy-cache populator keeps the raw link. pnpr's advantage then shows up
as fewer client round trips, not a faster backend.

The flags default to 0 (loopback), so an ad-hoc `just integrated-benchmark`
is unchanged. The CI benchmark — the canonical run — opts in: it already
set --registry-latency-ms=50, and now also sets
--registry-bandwidth-mbps=200 (≈ the measured npm peak) so its numbers
reflect a real registry link.
… just the store

The cold-cache scenarios only wiped the bench-local CAS (`store-dir`), but
the packument-metadata mirror that resolution actually reads
(`<cacheDir>/v11/metadata-full`) was left at its global default
(`~/.cache/pnpm`), outside the work-env, so it survived every wipe. After
the warmup run primed it, every direct install resolved all 1356 packages
from a warm mirror with zero registry round trips — exactly the
packument-fetch waterfall pnpr offloads to its warm server. So a direct
install never paid the cost pnpr removes, and pnpr@HEAD ≈ pacquet@HEAD in
the fresh-install scenario where pnpr should win big. "cold cache" was
cold in name only.

Force `cacheDir: ./cache-dir` bench-local (mirroring the existing
`storeDir: ./store-dir`; both pnpm and pacquet read it from
pnpm-workspace.yaml) and wipe it in the cold-cache scenarios' per-iteration
`--prepare` and the upfront pre-benchmark wipe. Now a cold-cache fresh
install fetches every packument over the emulated registry link, while the
long-running pnpr server keeps its own warm mirror under `pnpr-storage` —
so the benchmark finally exercises pnpr's core win: offloading the
client's cold resolution to a hot server.
…cenario

None of the existing scenarios isolate resolution, so pnpr's core win —
offloading the client's resolution to a warm server — never shows:

- cold-cache.cold-store: the throttled tarball download dominates (~5s)
  and is identical for both, and pacquet-direct already overlaps its
  resolution with downloads via PrefetchingResolver, so offloading
  resolution saves ~nothing.
- hot-cache.hot-store: warm client metadata → direct resolves fast too,
  and there's no download. Nothing to win.

Add a scenario that decouples the two: no lockfile (resolution happens),
COLD packument cache (the direct client pays a full re-resolve — a fetch
waterfall over the registry link), but a HOT store (every tarball already
present, so no download dominates or hides it). Resolution becomes the
only variable cost: a direct install pays it in full while pnpr offloads
it to its warm server and materializes from the hot store. This is where
pnpr-vs-direct should finally show a clear gap.
…ame)

The benchmark's `.pnpr-env` exported a bare `PNPR_SERVER`, but pacquet
reads config env vars only under the `PNPM_CONFIG_*` / `pnpm_config_*`
prefix (`config/src/env_overlay.rs` → `PNPM_CONFIG_PNPR_SERVER`), and
nothing reads the bare name. So `config.pnpr_server` was always `None`
and every `pnpr@<rev>` target silently ran a *direct* install — making
each pnpr row an exact duplicate of its `pacquet@<rev>` row. That's why
pnpr never showed a difference in any scenario: it was never exercised.

Export `PNPM_CONFIG_PNPR_SERVER` so the client actually routes through
the server. Add a post-run guard (`verify_pnpr_targets_were_routed`)
that fails the benchmark if a pnpr target's `pnpr-storage` is empty — a
resolve always populates the server's store/cache, so an empty one means
the request never reached pnpr. This turns the previously-silent failure
into a hard error.
`mbps_to_bytes_per_sec` truncated tiny positive `--registry-bandwidth-mbps`
values to `Some(0)`. A 0 byte/s cap would stall the proxy — the pacing
math divides the chunk length by the rate, so a zero rate yields an
infinite per-chunk delay. Guard non-finite/non-positive inputs to `None`
(no cap) and floor any positive rate at 1 byte/s. Realistic values
(50-200) are unaffected; this just removes the degenerate edge.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pacquet/crates/cli/src/cli_args/install.rs (1)

541-610: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Shutdown the prefetcher on error paths too.

prefetcher.shutdown().await only runs after a successful frozen install. If resolve_streaming(...) fails at Lines 541-552 or the local Install fails at Lines 601-603, the function returns before draining the writer task, so completed prefetches can miss their final index.db flush and detached background work can outlive the failed command.

💡 Suggested fix
-    let prefetcher = if link.lockfile_only {
+    let mut prefetcher = if link.lockfile_only {
         None
     } else {
         Some(
             TarballPrefetcher::new(
                 state.config,
@@
-    let outcome = match result {
+    let outcome = match result {
         Ok(outcome) => outcome,
         // The server rejected the input lockfile under our policy.
         // Surface the reconstructed `VerifyError` so the abort + the
         // `ERR_PNPM_*` diagnostic code match the local gate exactly.
         Err(PnprClientError::Verification(verify_err)) => {
+            if let Some(prefetcher) = prefetcher.take() {
+                prefetcher.shutdown().await;
+            }
             return Err(miette::Report::new(verify_err));
         }
         Err(err) => {
+            if let Some(prefetcher) = prefetcher.take() {
+                prefetcher.shutdown().await;
+            }
             return Err(miette::miette!("{err}"))
                 .wrap_err("resolving dependencies via the pnpr server");
         }
     };
@@
-    Install {
+    let install_result = Install {
         tarball_mem_cache: std::sync::Arc::clone(&state.tarball_mem_cache),
         http_client: &state.http_client,
         http_client_arc: std::sync::Arc::clone(&state.http_client),
         config: state.config,
@@
         auth_override: None,
         resolution_observer: None,
     }
     .run::<Reporter>()
     .await
-    .wrap_err("linking dependencies resolved via the pnpr server")?;
+    .wrap_err("linking dependencies resolved via the pnpr server");
 
-    // The materialization install has awaited every tarball's mem-cache
-    // slot, so all prefetch downloads have finished and queued their
-    // store-index rows. Drain the writer so those rows are persisted for
-    // the next install before returning.
-    if let Some(prefetcher) = prefetcher {
+    if let Some(prefetcher) = prefetcher {
         prefetcher.shutdown().await;
     }
 
+    install_result?;
+
     Ok(())
 }
🤖 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/src/cli_args/install.rs` around lines 541 - 610, The code
only calls prefetcher.shutdown().await on the happy path, so when
resolve_streaming(...) (the match that yields outcome) or
Install::run::<Reporter>().await returns Err the function returns early and
never calls prefetcher.shutdown().await; ensure the prefetcher is always shut
down by moving the main logic into a scoped async block or capturing the results
into a local Result (e.g. let result = (async { ...resolve_streaming +
materialization... }).await;) and then, after that block, unconditionally call
if let Some(prefetcher) = prefetcher { prefetcher.shutdown().await; } before
returning the outer result; reference the resolve_streaming call that produces
outcome, the Install::run::<Reporter>().await invocation, and
prefetcher.shutdown().await so you update all early-return paths to await
shutdown.
🤖 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.

Outside diff comments:
In `@pacquet/crates/cli/src/cli_args/install.rs`:
- Around line 541-610: The code only calls prefetcher.shutdown().await on the
happy path, so when resolve_streaming(...) (the match that yields outcome) or
Install::run::<Reporter>().await returns Err the function returns early and
never calls prefetcher.shutdown().await; ensure the prefetcher is always shut
down by moving the main logic into a scoped async block or capturing the results
into a local Result (e.g. let result = (async { ...resolve_streaming +
materialization... }).await;) and then, after that block, unconditionally call
if let Some(prefetcher) = prefetcher { prefetcher.shutdown().await; } before
returning the outer result; reference the resolve_streaming call that produces
outcome, the Install::run::<Reporter>().await invocation, and
prefetcher.shutdown().await so you update all early-return paths to await
shutdown.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a6a8684d-aabe-4498-a7e3-7eb1818fcab3

📥 Commits

Reviewing files that changed from the base of the PR and between dfc3b77 and 6ee4886.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (24)
  • .github/workflows/pacquet-integrated-benchmark.yml
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/package-manager/src/lib.rs
  • pacquet/crates/package-manager/src/remove.rs
  • pacquet/crates/package-manager/src/resolution_observer.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
  • pacquet/crates/package-manager/src/update.rs
  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/pnpr-client/src/lib.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/pnpr-client/tests/integration.rs
  • pacquet/tasks/integrated-benchmark/src/cli_args.rs
  • pacquet/tasks/integrated-benchmark/src/latency_proxy.rs
  • pacquet/tasks/integrated-benchmark/src/latency_proxy/tests.rs
  • pacquet/tasks/integrated-benchmark/src/main.rs
  • pacquet/tasks/integrated-benchmark/src/work_env.rs
  • pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs
  • pnpr/crates/pnpr/src/resolver.rs
  • pnpr/crates/pnpr/src/resolver/resolve.rs
  • pnpr/crates/pnpr/src/server.rs
🚧 Files skipped from review as they are similar to previous changes (17)
  • pnpr/crates/pnpr/src/server.rs
  • pacquet/crates/pnpr-client/tests/integration.rs
  • pacquet/crates/package-manager/src/update.rs
  • pacquet/tasks/integrated-benchmark/src/main.rs
  • pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/package-manager/src/lib.rs
  • pacquet/tasks/integrated-benchmark/src/latency_proxy/tests.rs
  • pacquet/crates/package-manager/src/resolution_observer.rs
  • pacquet/tasks/integrated-benchmark/src/workspace_manifest.rs
  • pnpr/crates/pnpr/src/resolver/resolve.rs
  • pacquet/tasks/integrated-benchmark/src/latency_proxy.rs
  • pacquet/crates/package-manager/src/install/tests.rs
  • pacquet/tasks/integrated-benchmark/src/cli_args.rs
  • pacquet/crates/pnpr-client/src/lib.rs
  • pacquet/tasks/integrated-benchmark/src/work_env.rs
  • pnpr/crates/pnpr/src/resolver.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). (7)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Code Coverage
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (macos-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 fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and 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 infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in 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 handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums 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 in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

  • pacquet/crates/package-manager/src/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
pacquet/**/Cargo.toml

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/Cargo.toml: Check whether the workspace already depends on something suitable in [workspace.dependencies] in the root Cargo.toml before adding a new dependency
Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it

Files:

  • pacquet/crates/pnpr-client/Cargo.toml
🧠 Learnings (41)
📓 Common learnings
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12181
File: worker/src/start.ts:504-520
Timestamp: 2026-06-04T06:04:05.107Z
Learning: In pnpm/pnpm's pnpr install accelerator, the `/v1/install` response has a two-level framing structure:
1. **Outer layer** (full HTTP body): `[u32 outer header length][outer header JSON][files payload]` — `fetchFromPnpmRegistry` (pnpr/client/src/fetchFromPnpmRegistry.ts) strips the outer layer with `body.subarray(4 + headerLength)` and passes the remaining bytes to `writeCafsFiles`.
2. **Inner layer** (files payload): the files payload itself starts with its own `[u32 inner json length][inner header JSON]` prefix (built by the server's `build_files_payload` / `empty_files_payload_prefix`), followed by `[64-byte digest][u32 size][1-byte exec][content]` frames and a 64-zero-byte end marker.

`writeCafsFiles` in `worker/src/start.ts` is correct to read `jsonLen = payload.readUInt32BE(0)` and start frames at `offset = 4 + jsonLen` — this skips the inner header. The same two-level structure is mirrored in the Rust reference client (`parse_inline_response` + `write_files_payload`). Do not fla...
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12189
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:435-439
Timestamp: 2026-06-04T14:40:29.444Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs` (pnpm/pnpm repo), the pnpr install accelerator always invokes `Install` with `lockfile_only: true` (hard-coded in `pnpr/crates/pnpr/src/install_accelerator/resolve.rs`). Under `lockfile_only: true`:
1. The `PrefetchingResolver` wrapper is skipped — the bare `inner_resolver` is used instead, so `PrefetchContext { config }` is never constructed.
2. The function returns before `CreateVirtualStore` is reached, so `install_package_by_snapshot` and its `config.auth_headers` fetch path are never hit.
pnpr's tarball fetch is handled separately in `resolve::fetch_uncached`, which independently receives the request-scoped `auth_headers`. Therefore, `auth_override` only needs to be threaded into the resolver-side components (NpmResolver, TarballResolver, NamedRegistryResolver) — not into PrefetchingResolver or CreateVirtualStore — on the pnpr path. For local installs (`lockfile_only: false`), `auth_override` is always `None` a...
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:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:08:06.093Z
Learning: Pacquet (pnpm's Rust port) has a cardinal rule: "match pnpm exactly — do not fix pnpm quirks unless the same fix has landed in pnpm first." Review comments should not suggest behavioral deviations from upstream pnpm, even when the upstream behavior appears buggy. If a real bug is identified, it must be fixed upstream first.
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:11:04.272Z
Learning: In pacquet (pnpm/pnpm repo), `ResolvedPackage.optional` AND-folding intentionally mirrors pnpm's resolveDependencies.ts:1627-1648 revisit behavior: only the directly-visited package's `optional` flag is updated on revisit, not transitive descendants. pnpm CLI corrects stale optional flags via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`. Pacquet does not yet have this pruner equivalent, so raw `node.optional` flows directly into snapshot/virtual-store via `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up issue to port `copyDependencySubGraph` is planned.
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Reference the upstream pnpm commit/PR when porting code from pnpm in commit messages
📚 Learning: 2026-05-23T09:14:43.635Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11867
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:726-730
Timestamp: 2026-05-23T09:14:43.635Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs`, the fresh-lockfile path intentionally does not invoke `BuildModules` and discards `side_effects_maps_by_snapshot` from `CreateVirtualStoreOutput`. This is pre-existing, documented behavior (mirroring upstream `link.ts:167-170`): `importing_done` fires once extraction and symlink linking are complete, and the fresh-lockfile path does not run lifecycle scripts. The frozen-lockfile path wires `BuildModules` end-to-end as normal. Do not flag this omission as a bug; wiring lifecycle scripts into the fresh-lockfile path is tracked as future work separate from perf refactors.

Applied to files:

  • pacquet/crates/package-manager/src/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-23T09:14:43.635Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11867
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:726-730
Timestamp: 2026-05-23T09:14:43.635Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs`, the fresh-lockfile path intentionally does not run lifecycle scripts (`BuildModules` is not invoked, and `side_effects_maps_by_snapshot` from `CreateVirtualStoreOutput` is discarded). This is pre-existing behavior documented by an in-code comment mirroring upstream `link.ts:167-170`. The frozen-lockfile path wires `BuildModules` end-to-end as normal. Wiring lifecycle scripts into the fresh-lockfile path is tracked as future work, separate from this file's concern.

Applied to files:

  • pacquet/crates/package-manager/src/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-06-04T14:40:29.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12189
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:435-439
Timestamp: 2026-06-04T14:40:29.444Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs` (pnpm/pnpm repo), the pnpr install accelerator always invokes `Install` with `lockfile_only: true` (hard-coded in `pnpr/crates/pnpr/src/install_accelerator/resolve.rs`). Under `lockfile_only: true`:
1. The `PrefetchingResolver` wrapper is skipped — the bare `inner_resolver` is used instead, so `PrefetchContext { config }` is never constructed.
2. The function returns before `CreateVirtualStore` is reached, so `install_package_by_snapshot` and its `config.auth_headers` fetch path are never hit.
pnpr's tarball fetch is handled separately in `resolve::fetch_uncached`, which independently receives the request-scoped `auth_headers`. Therefore, `auth_override` only needs to be threaded into the resolver-side components (NpmResolver, TarballResolver, NamedRegistryResolver) — not into PrefetchingResolver or CreateVirtualStore — on the pnpr path. For local installs (`lockfile_only: false`), `auth_override` is always `None` a...

Applied to files:

  • pacquet/crates/package-manager/src/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • .github/workflows/pacquet-integrated-benchmark.yml
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
📚 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/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.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/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.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/remove.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/Cargo.toml : Declare new shared dependencies in the root [workspace.dependencies] and use { workspace = true } in pnpr crate's Cargo.toml

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/Cargo.toml : Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/Cargo.toml : Check whether the workspace already depends on something suitable in `[workspace.dependencies]` in the root `Cargo.toml` before adding a new dependency

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/*.rs : Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, and error handling

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Do not add a dependency not already declared in `[workspace.dependencies]` without explicit human request; ask for approval and request addition to the workspace root `Cargo.toml`

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/crates/**/Cargo.toml : Cargo.toml files for new registry-only crates must use the pnpr- prefix for the package name

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Prefer existing pacquet-* crates over writing new code; check pacquet-tarball, pacquet-crypto-hash, pacquet-crypto-shasums-file, pacquet-package-manifest, pacquet-network, pacquet-registry, pacquet-fs, and pacquet-diagnostics before implementing non-trivial functionality

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/crates/**/Cargo.toml : New registry-only crates must be placed under pnpr/crates/<short-name>/ and named pnpr-<short-name> in Cargo.toml, never using the pacquet- prefix

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/*.rs : Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/cli/src/cli_args/install.rs
  • .github/workflows/pacquet-integrated-benchmark.yml
📚 Learning: 2026-05-20T19:41:03.063Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:41:03.063Z
Learning: In the pacquet project (Rust), the semver crate used is `node-semver` (version ~2.2.0), NOT `nodejs-semver`. These are two distinct crates. `node-semver` only exposes `Range::satisfies` as its public satisfaction API — there is no separate `satisfies_with_prerelease` method. Prerelease-tolerant matching is handled inline by retrying against the stripped `MAJOR.MINOR.PATCH` base when `Range::satisfies` rejects a prerelease version.

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : Follow Rust API Guidelines for naming

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-24T21:11:04.272Z
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:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Prefer `#[cfg_attr(target_os = "windows", ignore = "...")]` (or matching `#[cfg(unix)]` gates) over runtime probe-and-skip helpers for platform-locked tools

Applied to files:

  • pacquet/crates/pnpr-client/Cargo.toml
📚 Learning: 2026-06-04T14:55:52.163Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12189
File: pacquet/crates/cli/src/cli_args/install.rs:0-0
Timestamp: 2026-06-04T14:55:52.163Z
Learning: In `pacquet/crates/cli/src/cli_args/install.rs` (pnpm/pnpm repo), the `install_via_pnpr` function intentionally forwards the **full** `state.config.auth_headers` map to the pnpr server (not filtered to only the declared default/named registries). This is required for correctness: transitive dependencies can be scope-routed to registries not in the explicit registry list, or pinned to tarball URLs on hosts present in `.npmrc` but not a declared registry. Filtering to the declared registries silently drops tokens those sub-dependencies need, causing 401s on the server. pnpr uses the forwarded map to attach the right token per fetched URL exactly as a local install does (`AuthHeaders::for_url`). The pnpr-server's own credential is sent separately in the `Authorization` header and is excluded from the body map. Do NOT flag this as a credential-leakage issue — the rationale is documented in a comment at both call sites.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-24T21:11:04.272Z
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:11:04.272Z
Learning: In pacquet (pnpm/pnpm repo), `ResolvedPackage.optional` AND-folding intentionally mirrors pnpm's resolveDependencies.ts:1627-1648 revisit behavior: only the directly-visited package's `optional` flag is updated on revisit, not transitive descendants. pnpm CLI corrects stale optional flags via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`. Pacquet does not yet have this pruner equivalent, so raw `node.optional` flows directly into snapshot/virtual-store via `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up issue to port `copyDependencySubGraph` is planned.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-20T20:41:50.322Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11773
File: pacquet/crates/package-manager/src/install_package_from_registry.rs:111-114
Timestamp: 2026-05-20T20:41:50.322Z
Learning: In pacquet (pnpm/pnpm repo, Rust codebase), `install_package_from_registry` is the npm-only install path. The npm resolver always stamps `ResolveResult.id` (a `PkgResolutionId`) as `nameversion`. Parsing it back through `PkgNameVer` with `.expect()` is intentional — a parse failure means a mis-dispatch bug, not malformed external input. Per pacquet's CLAUDE.md: "Don't add error handling, fallbacks, or validation for scenarios that can't happen. Trust internal code and framework guarantees." Do not suggest replacing such `.expect()` calls with graceful error handling.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-06-04T06:04:05.107Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12181
File: worker/src/start.ts:504-520
Timestamp: 2026-06-04T06:04:05.107Z
Learning: In pnpm/pnpm's pnpr install accelerator, the `/v1/install` response has a two-level framing structure:
1. **Outer layer** (full HTTP body): `[u32 outer header length][outer header JSON][files payload]` — `fetchFromPnpmRegistry` (pnpr/client/src/fetchFromPnpmRegistry.ts) strips the outer layer with `body.subarray(4 + headerLength)` and passes the remaining bytes to `writeCafsFiles`.
2. **Inner layer** (files payload): the files payload itself starts with its own `[u32 inner json length][inner header JSON]` prefix (built by the server's `build_files_payload` / `empty_files_payload_prefix`), followed by `[64-byte digest][u32 size][1-byte exec][content]` frames and a 64-zero-byte end marker.

`writeCafsFiles` in `worker/src/start.ts` is correct to read `jsonLen = payload.readUInt32BE(0)` and start frames at `offset = 4 + jsonLen` — this skips the inner header. The same two-level structure is mirrored in the Rust reference client (`parse_inline_response` + `write_files_payload`). Do not fla...

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : User-facing errors go through `miette` via the `pacquet-diagnostics` crate; match pnpm's error codes and messages where pnpm defines them since error codes are part of the public contract

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Tests that need the mocked registry should start `pnpr` through `pacquet-testing-utils`; `cargo test` / `cargo nextest run` should not require a separate `just registry-mock launch` step

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
  • .github/workflows/pacquet-integrated-benchmark.yml
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-25T14:58:11.105Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11931
File: pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs:560-589
Timestamp: 2026-05-25T14:58:11.105Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`, all per-`(registry, name[, version])` caches in `NpmResolutionVerifier` (`published_at`, `full_meta`, `full_meta_for_trust`, `abbreviated_meta`, `local_meta`) intentionally use the same pattern: lock → miss-check → release lock → await fetch/load → re-acquire lock → insert. This uniform pattern is deliberate; do not flag individual caches for using it. The known follow-up improvement (replacing the pattern with `tokio::sync::OnceCell` per key inside a `Mutex<HashMap<…>>`) is tracked as a future structural change to cover all five caches simultaneously.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
  • pacquet/crates/package-manager/src/tarball_prefetch.rs
📚 Learning: 2026-05-20T23:07:43.668Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11783
File: pacquet/crates/engine-runtime-bun-resolver/src/bun_resolver.rs:70-88
Timestamp: 2026-05-20T23:07:43.668Z
Learning: In `pacquet/crates/engine-runtime-bun-resolver/src/bun_resolver.rs`, `resolve_impl` intentionally passes `ResolveOptions::default()` (not the caller-provided `opts`) when delegating to the npm resolver. This mirrors the upstream TypeScript code at `engine/runtime/bun-resolver/src/index.ts#L46` which passes an empty `{}` literal; forwarding outer opts would diverge from pnpm's behavior.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-20T01:42:44.958Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11755
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:370-380
Timestamp: 2026-05-20T01:42:44.958Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/pick_package.rs`, the fetch-error fallback that returns `Ok(PickPackageResult { meta: disk, picked_package: picked })` — even when `picked` is `None` — is intentional upstream parity with pnpm's `pickPackage.ts` catch block (https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackage.ts#L420-L431). When a fetch fails and a disk mirror exists, the stale-mirror pick (including null/None) is returned verbatim; the transport error is logged via `tracing::debug!`. Do not flag this as a bug.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
📚 Learning: 2026-05-23T16:55:36.507Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: pacquet/crates/cli/tests/lockfile_verification.rs:158-162
Timestamp: 2026-05-23T16:55:36.507Z
Learning: In `pacquet/crates/cli/tests/lockfile_verification.rs`, the `trust_lockfile_skips_verification` and `trust_lockfile_cli_flag_skips_verification` tests intentionally do NOT assert `output.status.success()`. The hand-rolled fixture lockfile uses a placeholder integrity hash (`sha512-AAA…`), so the install always fails the downstream tarball integrity check regardless of the supply-chain gate. The contract being tested is "gate-skipped, not install-succeeded"; asserting success would require generating a real lockfile via the `generate_lockfile` pattern (see `hoist.rs`) which is considered not worth the extra wiring for an opt-out smoke test.

Applied to files:

  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-24T08:18:06.019Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11895
File: pnpm/test/deploy.ts:91-95
Timestamp: 2026-05-24T08:18:06.019Z
Learning: In the pnpm/pnpm repository, integration tests that hit the real `registry.npmjs.org` (e.g., for `pacquet` or `pnpm/pacquet`) do NOT use a runtime env-var gate (such as `PNPM_RUN_PUBLIC_REGISTRY_TESTS`). They simply pass `--config.registry=https://registry.npmjs.org/` directly to `execPnpm` and set a higher timeout. This is the established pattern, as seen in `pnpm/test/install/pacquet.ts` and `pnpm/test/deploy.ts`. Do not suggest adding env-var guards for these tests.

Applied to files:

  • .github/workflows/pacquet-integrated-benchmark.yml
📚 Learning: 2026-06-03T09:27:50.473Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12154
File: pacquet/tasks/integrated-benchmark/src/work_env.rs:401-423
Timestamp: 2026-06-03T09:27:50.473Z
Learning: In `pnpm/pnpm` (pacquet Rust port), `pick_unused_port()` from `pacquet-registry-mock` is the established repo-wide pattern for choosing a port when spawning child processes (pnpr server, registry mock) in tests and benchmarks. The alternative of `--listen 127.0.0.1:0` (ephemeral port) is not currently viable because pnpr's `serve()` logs the *requested* `config.listen` address rather than `listener.local_addr()`, so the actual bound port cannot be discovered without a pnpr-side change. Do not flag `pick_unused_port()` usage in benchmark orchestration code as a TOCTOU bug without acknowledging this constraint.

Applied to files:

  • .github/workflows/pacquet-integrated-benchmark.yml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Match how the same feature is implemented in the TypeScript pnpm CLI — any change in pacquet must match pnpm's behavior, logic, edge cases, config resolution, error messages, file/lockfile formats, and existing tests

Applied to files:

  • .github/workflows/pacquet-integrated-benchmark.yml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Run `just ready` before pushing to verify all automated checks pass

Applied to files:

  • .github/workflows/pacquet-integrated-benchmark.yml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : Tests are documentation — do not duplicate test scenarios, edge cases, failure modes, or worked examples in prose when they are already captured by tests

Applied to files:

  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-06-02T13:18:30.659Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12134
File: pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs:311-325
Timestamp: 2026-06-02T13:18:30.659Z
Learning: In pacquet's lockfile resolution verifier (`pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`), URL-keyed tarball dependencies do NOT need a separate `non_semver_version` field in `VerifyCtx`. Unlike the TypeScript side (which derives `version` from `snapshot.version` and threads `nonSemverVersion` separately), pacquet's `collect_candidates` takes `version` from the lockfile key suffix. For a URL-keyed dep the key is `name@<url>`, so `ctx.version` is the URL string, which fails `node_semver::Version::parse(ctx.version)` and the existing guard `if node_semver::Version::parse(ctx.version).is_err() { return ResolutionVerification::Ok; }` already skips the registry lookup correctly. Adding a `non_semver_version` field to `VerifyCtx` for this purpose would be inert.

Applied to files:

  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Use snapshot tests with `insta` and carefully review diffs when intentional changes alter snapshots; accept with `cargo insta review` only after careful review

Applied to files:

  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-20T21:18:56.391Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11778
File: pacquet/crates/resolving-local-resolver/tests/resolve.rs:365-372
Timestamp: 2026-05-20T21:18:56.391Z
Learning: In `pacquet/crates/resolving-local-resolver/tests/resolve.rs`, the test `fail_when_resolving_from_not_existing_directory_an_injected_dependency` intentionally uses `injected: false`. The test is a verbatim port of the upstream pnpm TypeScript test (resolving/local-resolver/test/index.ts at ef87f3ccff). The `injected` flag only affects the file/link protocol choice for plain directory paths; when the `file:` scheme is explicit in the bare specifier, the flag has no effect on the resolution code path. The misleading test name is inherited from upstream.

Applied to files:

  • pacquet/crates/pnpr-client/src/tests.rs
📚 Learning: 2026-05-20T20:41:30.632Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11773
File: pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs:115-117
Timestamp: 2026-05-20T20:41:30.632Z
Learning: In the pacquet Rust port of pnpm, the `is_http_url` helper in `pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs` intentionally uses `bare.starts_with("http:") || bare.starts_with("https:")` (not `"http://"` / `"https://"`) to match upstream pnpm's `startsWith('http:')` / `startsWith('https:')` check byte-for-byte. Pacquet's cardinal rule (pacquet/AGENTS.md) requires matching pnpm even on quirks; malformed non-URL inputs are rejected downstream by `reqwest::Url::parse` as a `ResolveError`.

Applied to files:

  • pacquet/crates/pnpr-client/src/tests.rs
🔇 Additional comments (1)
.github/workflows/pacquet-integrated-benchmark.yml (1)

83-104: LGTM!

Also applies to: 270-270, 283-283, 297-297, 308-308, 312-325, 381-394, 432-433, 439-440

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(pnpr): stream resolved packages from /v1/resolve so the client overlaps tarball fetch with resolution

2 participants