Fix per-patch Poetry/PDM lock re-parse (#760, #762) - #877
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Hosted scans rewrite poetry.lock and pdm.lock once per patched package, rendering and re-parsing the whole lock each time. Count the engine's whole-lock renders and require one per lock for a dozen patched packages. Both tests fail today with 12 renders. Refs #760, #762 Assisted-by: Claude Code:claude-opus-5-5
A hosted scan rewrote poetry.lock and pdm.lock once per patched package, and every rewrite rendered and re-parsed the whole lock. A project with a dozen patches paid for a dozen full parses, which made Poetry and PDM scans 3.5-4.5x slower per package than other managers. The shared lock-splice engine now plans every package against one parsed lock, applies all the changes, renders and re-parses once, and splices each package's changed fragments into the original text. The result is checked against the rendering byte for byte. When a lock mixes line endings, a package is rewritten twice, or any check fails, the rewrite falls back to the old package-by-package path, so output and recorded edits never change. Differential tests run both paths over every Poetry and PDM lock generation, LF, CRLF and mixed, with refusals, missing packages and re-runs mixed in, and require identical text and per-package results. Fixes #760, #762 Assisted-by: Claude Code:claude-opus-5-5
The rewrite never changes [metadata] lock_version, so read it from the parse the presence probe already took instead of parsing the output. Assisted-by: Claude Code:claude-opus-5-5
main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5
|
[agent] Generated by Claude Code |
|
BugBot review Generated by Claude Code |
|
[agent] Generated by Claude Code |
Running cargo fmt over the whole workspace reformatted 117 files this PR does not otherwise touch, because main is not rustfmt-clean and CI does not check formatting. Restore those files to main and keep the diff to the Poetry/PDM rewrite and the ported digest fix. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6e45592. Configure here.
|
[agent] Generated by Claude Code |
|
[agent] CI on 6e45592: nine workflows show red (CI, Gradle, Go, Bun, Composer, npm, pnpm, Benchmarks, Audit GHA), but none of them has a failed job. In each one, jobs were Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
|
[agent] Ready for review at Generated by Claude Code |
This branch has not been deployed
3 participants
LLM Description written by Claude Code:claude-opus-5-5
Fixes #760
Fixes #762
Summary
Hosted Poetry and PDM scans now rewrite
poetry.lock/pdm.lockwith one parse and one render per lock, not one of each per patched package. On the bench fixture (400 packages, 12 patched), the hosted scan's median wall time drops by about half:poetry/hostedpdm/hosted(
socket-patch-bench run -f '^<pm>/hosted', perf profile, both binaries alternated on the same sandbox runner.)Root cause
Both rewriters go through the shared fragment-splice engine (
utils/lock_fragments.rs::finish). For each patched dep, it mutated the parsed document, rendered the whole lock (DocumentMut::to_string), and parsed the rendering again to take the package's new fragments.LockParseonly saved the input parse, so every dep still paid one full render and one full parse: O(patches × lock size). Callgrind put 77% of the hosted scan in this loop for both formats.Fix
lock_fragments::rewrite_batch(shared by both formats) handles every dep in one pass. It plans each dep against the document as the earlier deps left it, so refusal and not-found verdicts match the step-by-step ones. It then applies all mutations, renders and re-parses once, and splices each dep's changed bytes into the original text. The splice must reproduce the rendering byte for byte.poetry_lock/pdm_locksplit their rewrite into checks, plan and mutate steps, and addrewrite_*_lock_all(batch or steps). They return aLockStepper dep:Rewritten(edits),Unchanged,NotFoundorRefused.patch/redirect/poetry.rs,patch/redirect/pdm.rs) consume those steps. Warnings, uuid sets andFileEdits come out in the same order as before. Vendored callers (one dep at a time) are unchanged.npm/,pypi/andgem/only dispatch to the binary.Out of scope: both issues also note a secondary cost,
vex::discover::pypi_locks::extractre-parsing the lock for VEX (~10%). It's untouched here and can be a follow-up if it still shows up in the bench.Tests
patch::redirect::poetry::equivalence_tests::many_patches_render_the_lock_once(every lock generation 1.0+, LF and CRLF, 12 patched packages)left: 12, right: 1→ passespatch::redirect::pdm::parse_reuse_equivalence_tests::many_patches_render_the_lock_once(every supported PDM generation, LF and CRLF)left: 12, right: 1→ passesEquivalence:
utils::poetry_lock::batch_equivalence_tests::batch_matches_the_step_by_step_rewriteandutils::pdm_lock::batch_equivalence_tests::…: across every lock generation, LF / CRLF / mixed, with adjacent and sparse packages, reversed order, interleaved refusals and not-found deps, a package rewritten twice, and a re-run over the output, the batch's text and every dep's step equal the step-by-step result. The tests also assert the batch actually answers in every clean case and hands back in the mixed / rewritten-twice cases.poetry_rewrite,pdm_plan,pdm_rewrite_shared_parse,pdm_lock_reused_parse, …) pass unchanged, so there are no output changes.Local runs (on 6e45592):
cargo clippy --workspace --all-features -- -D warnings: clean. Formatting follows the surrounding code.mainitself isn't rustfmt-clean and CI doesn't check formatting, so I didn't runcargo fmt --all. 29735ea had run it and swept 117 unrelated files; 6e45592 restores them tomain.cargo test -p socket-patch-core --all-features --lib: 5250 passed. The 4 failures (copy_tree,vlt_heal,pypi_poetry,pypi_requirementswrite-failure tests) are chmod-based. They can't fail when the sandbox runs as root, and they fail the same way onmain. All 35 core integration targets pass.--lib(847),in_process_redirect_poetry,in_process_redirect_pdm,hosted_memory_engine,hosted_memory_parity,mode_migration_pypi,covgap_commands_vex,e2e_vex_vendor,in_process_vendor,in_process_python_envs: all pass.CI on 6e45592: all green. All 12 workflows pass (CI, Poetry, PDM, vlt, Gradle, Go, Bun, Composer, npm, pnpm, Benchmarks, Audit GHA), including every
e2e_vex_buildPoetry leg (1.0.10–2.4.3) and PDM leg (1.4.5–2.29.2). Jobs that the runner queue had cancelled before any step ran were re-run once. Bugbot found no issues; no open review threads.Also in this PR
c25dd8e cherry-picks #878 (
Route Gradle digests through utils::digest).main@ 9c43dfc failsutils::digest::tests::production_digests_go_through_the_helperson every PR. The cherry-pick becomes a no-op once #878 merges.🤖 Generated with Claude Code
https://claude.ai/code/session_01HBf8J4E4vApc1bRJfAUDu8
Generated by Claude Code