Skip to content

refactor(cli): sweep vex_consumed.rs and vex_sources.rs - #1948

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
arch-refactor/sweep-cli-vex-consumed-sources
Nov 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
arch-refactor/sweep-cli-vex-consumed-sources

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Architecture sweep of crates/socket-patch-cli/src/commands/vex_consumed.rs and vex_sources.rs. These are the next free rows in coverage/files.tsv, and their notes are tracked in the architecture discussion #1830. No open PR touches either file. vex.rs (held by #1698 and #1893) is not changed.

Changes

vex_consumed.rs

  • Deleted the not_installed() wrapper (9 call sites now use HostedCopies::default()).
  • Deleted the cfg(test) forwarder npm_identity_fallback. The _reusing variant takes its name, and the two docs are merged.
  • Removed comments: 3 section banners and 5 bare issue refs (#599, #1098, #1494 ×2, #856).
  • Tests:
    • Fixed the stale doc on npm_identity_fallback_fills_only_misses. It said the targeted lookup probes only node_modules/left-pad, which is no longer true; it now says what the asserts check.
    • Rewrote the garbled vlt_alias_is_consumed_through_its_store_copy doc ("no importer dir is an alias nothing").
    • Deleted the write_pkg wrapper and reused local() for an inline CrawlerOptions.
    • Moved npm_alias_copies_finds_only_alias_installs and npm_alias_copies_walks_every_workspace_members_tree to ecosystem_dispatch tests. They test find_manifest_package_copies_reusing, and are renamed manifest_copies_take_npm_alias_installs_only and manifest_copies_walk_every_npm_workspace_members_tree. Their new write_package_json helper is now also behind write_npm_package.

vex_sources.rs

  • Inlined recognized_dead_note into its only caller, dead_claim_notes, which is now one loop.
  • Deleted the note() forwarder. The 10 call sites now build PlanNote { code, detail } directly.
  • Made 12 NOTE_* consts private, since nothing outside the module uses them. NOTE_API_AUTH_FALLBACK stays pub(crate).
  • Moved the 3 unattested-note consts (NOTE_NPM_SHRINKWRAP_ONLY, NOTE_MAVEN_REACTOR_ROOT, NOTE_MAVEN_CLASSIFIER_UNPATCHED) up beside their siblings, above unattested_note.
  • Removed comments: 3 section banners and 3 bare refs (#745, #899, #261).

Audit notes not applied

/code-review high

There were no correctness findings and 7 cleanup or test-coverage notes. 6 are fixed:

  • restored fetch_concurrency and its test;
  • restored build_candidates;
  • inlined the re-created manifest_copies wrapper;
  • kept the "no second alias walk" reason;
  • renamed the moved tests;
  • write_npm_package now shares the package.json writer.

1 is left: a sorted helper now exists in two different test modules. A shared test-util module for one three-line helper isn't worth it.

Stats

git diff --stat: 3 files, +247 / −285.

  • Production: +104 / −141 (net −37). Most of the + lines in vex_sources.rs come from the PlanNote literals that replace note().
  • Tests: +146 / −144. Most of that is the two tests moving to ecosystem_dispatch.rs.

Verification

  • cargo clippy --workspace --all-features -- -D warnings is clean.
  • cargo test -p socket-patch-cli --lib: 940 passed.
  • The vex, e2e_vex, e2e_embedded_vex, e2e_vex_redirect and e2e_vex_vendor suites all pass (234 tests).
  • No platform-specific code changed.
  • CI: ci-ok green on 14ba988; Bugbot found no issues.

Behavior change

None. Output bytes, JSON, warning codes and exit codes are unchanged.

Risk

Low: local inlining and comment and visibility changes only.

https://claude.ai/code/session_011f9aeQ8265a5x29iqeryUU


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Nov 1, 2026
vex_consumed.rs: drop the not_installed() wrapper and the cfg(test)
npm_identity_fallback forwarder (the reusing variant takes its name),
drop banner and bare issue-ref comments, fix the stale and garbled
test docs, and drop the write_pkg test wrapper. The two
npm_alias_copies_* tests exercise ecosystem_dispatch's every-copy
lookup, so they move there.

vex_sources.rs: inline fetch_concurrency, recognized_dead_note and
build_candidates, drop the note() forwarder for PlanNote literals,
make the module-only NOTE_* consts private and group the unattested
ones together, and drop banners and bare issue refs. The deleted
fetch_concurrency test only re-checked core's api_concurrency.

No behavior change.

Assisted-by: Claude Code:claude-opus-5-5
/code-review: fetch_concurrency's test is the only check of this
window's own FETCH_CONCURRENCY ceiling, and build_candidates names a
step plan() is better off without inlined, so both stay. The moved
ecosystem_dispatch tests are renamed after the lookup they test, call
it directly, keep the "no second alias walk" reason, and share the
package.json writer with write_npm_package.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 1, 2026 19:11
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor 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.

✅ 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 14ba988. Configure here.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Nov 1, 2026
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review at 14ba988.

  • CI: 82/82 check runs on 14ba988 green (success/skipped/neutral); no main-wide failures.
  • Bugbot: reviewed 14ba988, no findings; no open review threads.
  • Mergeable, no conflicts with main; no CHANGELOG.md change.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: A behavior-preserving cleanup of vex_consumed.rs and vex_sources.rs. It inlines the not_installed(), note() and recognized_dead_note wrappers and the test-only npm_identity_fallback forwarder, and makes 12 NOTE_* consts private. It also moves two alias-install tests into ecosystem_dispatch tests, where the function they exercise lives.

Risk: low. Every production change is a mechanical inline. Note codes, message text and ordering are unchanged. Narrowing the consts to private compiles, so nothing else used them.

Look here:

  • vex_sources.rs:894-923:`` dead_claim_notes, the only control-flow rewrite (filter_map/map became a for/continue loop).
  • vex_sources.rs:179-234:``` the visibility narrowing and the moved consts.
  • vex_consumed.rs:145-171:```` npm_identity_fallback (formerly `_reusing`), with the production call at :99.
  • ecosystem_dispatch.rs:2259-2380:````` the moved tests and their new helpers.

Verified:

  • Read the full diff against main.
  • Assertions in the moved tests are byte-identical to the originals.
  • cargo test -p socket-patch-cli --lib -- vex_consumed vex_sources ecosystem_dispatch: 71 passed.
  • cargo test -p socket-patch-cli --test e2e_vex: 45 passed.
  • clippy (lib/bins, test profile): clean.
  • CHANGELOG.md is untouched.
  • CI: ci-ok and clippy are green on the head.

Changes I made: In the PR description, corrected "12 call sites" to 10 for note().

Open questions: None. A small sorted() test helper is now duplicated in two test modules; the description acknowledges this.

Auto-merge is armed: approving sends this straight to the merge queue.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[agent] Reviewed 14ba988283c739b8fc6e0823d4f77f846aaa9b94. Checked VEX consumed-copy fallback reuse, authority/liveness notes and equivalent record-fetch outcomes. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 3da3d14 Nov 2, 2026
82 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the arch-refactor/sweep-cli-vex-consumed-sources branch November 2, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-refactor PR opened by the scheduled architecture refactor routine Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants