fix(update): ignore shared deps when refreshing lazy backends - #65744
fix(update): ignore shared deps when refreshing lazy backends#65744Gigisanta wants to merge 2 commits into
Conversation
Related to #27878: both use the same primary-package activation fix. This PR also changes the trace-upload dependency pin, so it is not a mechanical duplicate; please consider splitting that independent delta. |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Fix ignores shared deps when refreshing lazy backends (+19 -9). Clean dependency management fix.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused reproduction. The false-positive premise remains present on current main: tools/lazy_deps.py:870 activates a feature when any dependency is installed, and hermes_cli/main.py:7714-7726 passes that list into the update refresh.
Problems
- The replacement at
tools/lazy_deps.py:870drops the existing partial-install recovery contract documented intests/tools/test_lazy_deps.py:319-327: a remaining non-primary package no longer causeshermes updateto repair the feature. tools/lazy_deps.py:242also changes the trace-uploadhuggingface-hubpin. This is independent of activation detection, matching the split request in the existing member comment.
Suggested changes
- Split the trace dependency change.
- Preserve both requirements with durable activation state rather than inferring activation from tuple order; add tests for aiohttp-only Matrix and a non-primary partial installation.
Automated hermes-sweeper review.
| Used by ``hermes update`` to figure out which lazy backends need a | ||
| refresh pass when pins move in :data:`LAZY_DEPS`. | ||
| """ | ||
| active = [] |
There was a problem hiding this comment.
This fixes shared-dependency false positives, but it also removes the existing partial-install recovery behavior documented by the prior test: if slack-bolt is missing while another Slack-specific dependency remains, update will no longer detect and repair Slack. Please use activation state (or preserve that case) rather than making tuple position the activation record.
| ), | ||
| # HF Agent Trace Viewer upload (hermes trace upload / /upload-trace). | ||
| "tool.trace_upload": ("huggingface-hub==1.2.3",), | ||
| "tool.trace_upload": ("huggingface-hub==1.23.0",), |
There was a problem hiding this comment.
Please split this trace-upload dependency pin into a separate change. It is independent of lazy-feature activation, as the existing member comment also notes.
Summary
hermes updatecurrently treats a lazy backend as active when any declared package is installed. Shared secondary dependencies such asaiohttptherefore markplatform.matrixactive even when Matrix was never configured or installed. Every update then attempts to buildmautrix[encryption]/python-olm, producing a recurring optional-backend warning on current macOS toolchains.Use each feature's first declared spec as its primary activation anchor. Secondary/shared packages no longer trigger unrelated backend installs.
Reproduction
On a standard Hermes environment with
aiohttpinstalled but nomautrix:platform.matrix in active_features()isTrueFalseThe existing multi-package partial-install behavior remains covered through the primary package (
slack-bolt).Verification
python -m pytest tests/tools/test_lazy_deps.py -qplatform.matrixabsent