Skip to content

fix(lazy_deps): stop an exact pin on a shared transitive dep from downgrading the core - #68008

Closed
spiky02plateau wants to merge 1 commit into
NousResearch:mainfrom
spiky02plateau:fix/lazy-deps-shared-dep-no-downgrade
Closed

fix(lazy_deps): stop an exact pin on a shared transitive dep from downgrading the core#68008
spiky02plateau wants to merge 1 commit into
NousResearch:mainfrom
spiky02plateau:fix/lazy-deps-shared-dep-no-downgrade

Conversation

@spiky02plateau

Copy link
Copy Markdown
Contributor

What does this PR do?

LAZY_DEPS["tool.trace_upload"] pinned huggingface-hub==1.2.3. huggingface-hub
is not private to the trace-upload feature — it is a shared transitive
dependency of the embedding stack
(transformers / sentence_transformers)
that the local / local_embedded Hindsight memory backend needs. transformers
requires huggingface-hub>=1.5.0,<2.0.

Because the pin is an exact ==, it satisfies nothing but itself: _is_satisfied
returns False for any other installed version. So the moment the feature is
"active" (its package present) and hermes update runs its refresh_active_features
lazy-refresh pass, pip reinstalls — i.e. DOWNGRADES — huggingface-hub back to
1.2.3
, even though the core just resolved a newer one it depends on. That
silently breaks import sentence_transformers, and the local Hindsight embedded
daemon aborts at startup on every retain/recall. The existing _is_satisfied
version guard cannot prevent this: an == pin is a downgrade instruction
whenever the installed version differs.

This PR fixes it in two layers:

  1. Range, not exact pin for tool.trace_upload: huggingface-hub>=1.5,<2.0,
    which brackets what the trace-upload client needs and what the embedding
    stack requires, so an already-healthy shared version is left untouched.

  2. A general no-downgrade guard in _is_satisfied. When the installed
    version is outside a spec but every upper bound in that spec sits below the
    installed version — i.e. satisfying the spec would require going backwards —
    the guard treats the already-installed higher version as satisfied, leaves it
    in place, and logs a warning telling the maintainer to widen the pin. This
    backstops the whole failure class: no lazy, opt-in backend can ever silently
    downgrade a package the core (or another backend) already installed at a
    higher version. Legitimate upgrades (installed below the floor) are
    unaffected — those still install. It is pure local version arithmetic; no
    network, no behavior change for specs that are already satisfied.

Reproduction

With transformers/sentence_transformers installed (pulling huggingface-hub
≥1.5), activate the trace-upload feature and run the update refresh pass (or
ensure("tool.trace_upload")): huggingface-hub is downgraded to 1.2.3 and
python -c "import sentence_transformers" then raises
ImportError: huggingface-hub>=1.5.0,<2.0 is required ... but found huggingface-hub==1.2.3.

Related Issue

No existing issue — found during an operational memory-outage investigation and
reproduced directly against main.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Notes for maintainers

  • The tools/lazy_deps.py module docstring / TTS comment references a
    "no-ranges policy to match pyproject extras." That policy is right for
    feature-private packages; the point of this change is that huggingface-hub
    is not feature-private. If you prefer to keep the literal pin exact, the
    no-downgrade guard alone still prevents the outage — but the range is the more
    honest description of the real constraint.
  • Consider auditing the rest of LAZY_DEPS for other exact pins on packages
    that also appear in the core dependency tree (aiohttp, starlette,
    numpy, Pillow); those already carry security rationale, but the same
    downgrade risk applies if the core ever floats above the pinned version.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MN8RMDLwxCfFxwtADoEJJf

…r huggingface-hub

An exact ==1.2.3 pin on huggingface-hub (feature tool.trace_upload)
force-downgraded the shared venv on every lazy refresh whenever the core
embedding stack (transformers/sentence-transformers, used by
local/local_embedded Hindsight) had installed a newer version — transformers
5.x requires huggingface-hub>=1.5,<2.0, so the downgrade made
sentence_transformers unimportable and the embedded Hindsight daemon abort
at startup (silent memory loss until noticed).

Two layers:
- Track the compatibility range the trace-upload client actually needs
  (>=1.5,<2.0) instead of an exact pin, so an already-healthy shared
  version satisfies the spec and is left alone.
- Add a general no-downgrade guard in _is_satisfied: a lazy, opt-in
  backend must never move an already-installed package backwards; treat
  'installed newer than the pin allows' as satisfied and warn to widen
  the pin. Legitimate upgrades (installed below the spec) are unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MN8RMDLwxCfFxwtADoEJJf
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/memory Memory tool and memory providers area/config Config system, migrations, profiles dependencies Pull requests that update a dependency file sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 20, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @spiky02plateau — your PR had the sharpest analysis of the class-level problem (an exact pin on a SHARED transitive dep letting a lazy refresh downgrade the core), and the no-downgrade guard is a thoughtful backstop. The merged resolution in #72320 went a different route per maintainer policy: exact pins stay (no ranges — every version change must be code-reviewed), and the tree converges on ONE hub version (==1.24.0) with LAZY_DEPS and uv.lock held in lockstep by invariant tests. That makes the downgrade structurally impossible for this pin without adding runtime guard logic that treats out-of-spec versions as satisfied (which would also mask legitimate pin bumps). Closing in favor of that — appreciated the depth here.

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

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard dependencies Pull requests that update a dependency file P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants