Skip to content

perf(cli): skip npm install during hermes update when Node manifests are unchanged (#39397 salvage) - #61580

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/39397-39399-npm-update-perf
Jul 14, 2026
Merged

perf(cli): skip npm install during hermes update when Node manifests are unchanged (#39397 salvage)#61580
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/39397-39399-npm-update-perf

Conversation

@kshitijk4poor

@kshitijk4poor kshitijk4poor commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

hermes update no longer pays 30-60s+ of npm dead time on Python-only/docs-only updates: both npm install steps are skipped when none of the Node manifests changed since the last successful install.

Salvages #39397 by @rodboev (cherry-picked, authorship preserved). Scope change after review: #39399's --prefer-offline was dropped entirely — local 3-run benchmarks on the repo's real manifests showed the flag is noise on warm-cache npm ci (root: 0.90s vs 0.84s avg; workspaces: 4.02s vs 4.00s avg), and it carried a stale-resolution risk on the npm install fallback (@ethernet8023's review). All the real win is the skip itself.

Changes

  • hermes_cli/main.py (perf(cli): skip npm install during update when lockfile is unchanged (#17268) #39397, rodboev): skip both _update_node_dependencies() install steps when the manifest digest matches the one recorded after the last successful install. Cache file keyed by PROJECT_ROOT (parallel worktrees don't collide), guarded on node_modules existing, hash written only on success.
  • hermes_cli/main.py (review follow-up): the skip key is a combined digest over package-lock.json + root package.json + every workspace package.json expanded from the root workspaces globs (npm's own source of truth — on the real repo: 8 manifests incl. apps/desktop, apps/bootstrap-installer, apps/shared, nested ui-tui/packages/hermes-ink). A package.json-only edit (no npm run) defeats the skip, so the npm install out-of-sync fallback still fires exactly when it's needed.
  • tests: skip/record/invalidate semantics + reviewer scenarios (package.json-only edit, glob-matched workspace edit incl. desktop, new workspace appearing under a glob) — all mutation-verified against the skip-only prefix commit.

Validation

Before After
hermes update, no Node changes npm ci x2 (~5s+ warm, 30-60s+ cold) skipped
any manifest changed / install failed / node_modules missing installs installs

E2E (real imports, temp HERMES_HOME): full lifecycle incl. per-root isolation; real-repo check confirms all 8 workspace manifests enter the key. 58 targeted tests green. ruff clean.

Supersedes #39397. #39399 should be closed separately as no-measured-benefit (benchmark data in review thread).

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants