doctor: tighten generic memory-provider wording (#36)#37
Merged
Conversation
After auditing the non-Honcho / non-Mem0 memory providers
(openviking, supermemory, hindsight, byterover, retaindb,
holographic), every implementation's is_available() only checks
env-vars / CLI presence — none probe backend reachability.
The doctor message "X provider active" overstates that as a
connection-validated state. Wrong key, expired token, unreachable
host all silently produce a green ✓.
Tighten the wording to "X provider configured" + add a check_info
row that:
- states is_available() only verified env-vars / CLI presence
- calls out the asymmetry (Honcho + Mem0 do real probes via
#15/#34/#35; others wait on a health_check() ABC method)
- links #36 for the design discussion
No code-level probe added here — adding per-provider probes
requires deep SDK knowledge for 6 providers AND the right shape
is probably a new ABC method, not 6 elif blocks. Both belong in
the follow-up. This PR's job is just to stop the false-positive
green check from misleading operators.
Closes #36 (message-tightening scope).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #36 (message-tightening scope). Continuing the diagnostic-loudness sweep after #34/#35. Audited every non-Honcho / non-Mem0 memory provider:
is_available()checksOPENVIKING_ENDPOINTenv varSUPERMEMORY_API_KEY+import supermemorybrvCLI on PATHRETAINDB_API_KEYenv varNone probe the backend. So
"X provider active"overstates reality — wrong key, expired token, or unreachable host all silently produce ✓.Fix
Two minimal changes to the generic else-branch in
hermes_cli/doctor.py:"X provider active"→"X provider configured".check_inforow: statesis_available()only verified env-vars / CLI presence, notes the asymmetry (Honcho + Mem0 do real probes), links hermes doctor: generic memory provider check overstates is_available() — "X provider active" implies reachability that wasn't probed #36 for the design discussion.No per-provider probe added. That requires deep SDK knowledge across 6 providers, AND the right shape is probably a new ABC
health_check() -> tuple[bool, str]method, not 6elifblocks. Both belong in the follow-up; this PR's job is just to stop the false-positive green check from misleading operators today.Test plan
test_generic_provider_wording_no_longer_implies_reachabilityintests/hermes_cli/test_doctor.py::TestDoctorMemoryProviderSection: stubsload_memory_providerto return a SimpleNamespace withis_available() → True, asserts the new output contains "configured" and not "active", references#36, and includes the "backend reachability not probed" phrase.pytest tests/hermes_cli/test_doctor.py→ 65 passed (64 existing + 1 new). No regression.Filed by hermes-maintainer (PowerCreek).