docs(hermes): explain wrapper Python compatibility - #632
Conversation
📝 WalkthroughWalkthroughThe Hermes documentation adds launcher-based Python discovery, interpreter validation, persistent side-venv setup, wrapper refresh, compatibility recovery, deployment-specific restart guidance, and profile-scoped validation. ChangesHermes wrapper setup and recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@UPDATING.md`:
- Around line 53-67: Update both launcher snippets so each required command
fails closed: add explicit exit-on-failure handling after the readlink-based
HERMES_BIN resolution and after the HERMES_PYTHON --version probe, using the
existing HERMES_BIN/HERMES_PYTHON flow as the anchor. In UPDATING.md#L53-L67 and
docs/hermes-integration.md#L53-L67, stop immediately on either failure so
dirname never runs on an unresolved path and later venv creation never uses an
unverified interpreter.
- Around line 96-102: Update the recovery block in the updating workflow so it
explicitly assigns HERMES_HOME and MNEMOSYNE_DATA_DIR before creating the new
venv, instead of only mentioning them in a comment. Keep the existing venv
creation and mnemosyne-hermes install/status steps unchanged, but make sure the
symbols HERMES_HOME, MNEMOSYNE_DATA_DIR, VENV, and the mnemosyne-hermes install
path all operate against the intended deployment/profile in a fresh shell.
- Around line 90-100: Update the installation examples so every pip command uses
the selected memory profile instead of hardcoding mnemosyne-memory[embeddings];
in UPDATING.md#L90-L100 adjust the recovery venv install to match the profile
chosen in the surrounding prose, and in docs/hermes-integration.md#L70-L76 and
docs/hermes-integration.md#L93-L100 make the initial Docker install and
compatibility-repair install commands use the same parameterized profile value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1a6f6505-dc5f-400e-a33c-6b0fefd1d924
📒 Files selected for processing (2)
UPDATING.mddocs/hermes-integration.md
d7b10c6 to
1bde83d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/hermes-integration.md`:
- Around line 104-117: Add explicit exports for HERMES_HOME, MNEMOSYNE_DATA_DIR,
and the selected profile within the recovery command block before creating the
venv or installing packages, using the documented /opt/data deployment paths and
preserving any intended profile override. Ensure the commands using VENV and
mnemosyne-hermes operate against those explicitly configured paths in a fresh
shell.
- Around line 80-87: Update the documented package-selection logic in
docs/hermes-integration.md lines 80-87 and 113-115, and UPDATING.md lines
103-121, so every wrapper installation path maps core to mnemosyne-memory,
embeddings to mnemosyne-memory[embeddings], and all to mnemosyne-memory[all].
Apply the mapping consistently to the initial Docker install, wrapper
compatibility recovery, and updating-guide recovery command without adding an
extra or invalid profile suffix.
- Around line 112-116: The recovery snippets must reject an existing side-venv
directory before creating it. In docs/hermes-integration.md lines 112-116 and
UPDATING.md lines 118-122, add a fail-closed existence check immediately before
the "$HERMES_PYTHON" -m venv "$VENV" command, emit an error describing the
existing path, and exit without overwriting it.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3dca609e-85d1-4bea-bd88-816a7c67e3e0
📒 Files selected for processing (2)
UPDATING.mddocs/hermes-integration.md
1bde83d to
a13e186
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/hermes-integration.md`:
- Around line 82-105: Make every documented setup sequence fail closed: in
docs/hermes-integration.md lines 82-105, stop on failures from venv creation,
package installation, or mnemosyne-hermes install before running status or
hermes config set; in docs/hermes-integration.md lines 143-146, stop before
force-replacing the plugin if recovery installation fails; in UPDATING.md lines
84-88, only interpret status as a successful refresh after the preceding command
succeeds; and in UPDATING.md lines 136-139, do not force-register the recovery
environment unless its creation completed successfully.
In `@UPDATING.md`:
- Around line 123-126: Preserve the existing MNEMOSYNE_PROFILE during recovery
before deriving MNEMOSYNE_REQUIREMENT: in UPDATING.md lines 123-126, require or
restore the deployed profile instead of allowing the fresh-shell embeddings
default; apply the same profile restoration before creating the replacement
virtual environment in docs/hermes-integration.md lines 130-133. Ensure
deployments using all retain their local-LLM dependencies.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fedc59ce-7917-43a4-a22a-7b3298ef1374
📒 Files selected for processing (2)
UPDATING.mddocs/hermes-integration.md
a13e186 to
13f483e
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/hermes-integration.md`:
- Around line 44-46: Update the Docker and launcher installation guidance near
the interpreter-selection instructions to state that Mnemosyne storage,
indexing, and retrieval use in-process SQLite, and that network access occurs
only through explicitly configured services. Keep the statement aligned with the
local-first architecture, or link directly to the authoritative architecture
documentation.
- Around line 175-178: Scope MNEMOSYNE_DATA_DIR to the selected profile’s data
root alongside HERMES_HOME before the profile-local wrapper install and status
commands in docs/hermes-integration.md (lines 175-178). Apply the same
data-directory scoping requirement to the profile-local refresh and status
commands in UPDATING.md (lines 165-169), keeping wrapper setup and memory data
isolated per profile.
- Around line 175-178: Update the profile-local shell example around the
mnemosyne-hermes install and status commands to define VENV as the tested
compatible side virtual environment before use, or add validation that exits
when VENV is unset; ensure both commands cannot silently expand to system paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7795083c-07ce-4f48-bdfe-379e9ce9f332
📒 Files selected for processing (2)
UPDATING.mddocs/hermes-integration.md
13f483e to
f45a5f4
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 similar comment
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…python-compatibility docs(hermes): explain wrapper Python compatibility
Summary
Documents the runtime-Python compatibility contract introduced by #630 for persistent Hermes wrappers.
python3Docker example with a bounded launcher-sibling discovery path that fails rather than guessing.--forceuse.Scope
Documentation only:
docs/hermes-integration.mdandUPDATING.md. No runtime, installer, dependency, schema, or release-version changes.Verification
python3 scripts/generate-docs.py --checkpython3 scripts/verify-docs.pygit diff --checkSummary
docs/hermes-integration.mdandUPDATING.md.python3executable.