Skip to content

fix(langchain): recognise is_healthy in health tool status normalization - #5885

Open
varun-projects wants to merge 1 commit into
volcengine:mainfrom
varun-projects:fix/5876-langchain-health-is-healthy
Open

varun-projects wants to merge 1 commit into
volcengine:mainfrom
varun-projects:fix/5876-langchain-health-is-healthy

Conversation

@varun-projects

Copy link
Copy Markdown
Contributor

Description

The LangChain viking_health tool reported state: "unknown" and healthy: false for a
healthy backend. The observer system-status result serializes its top-level health flag as
is_healthy, and SyncHTTPClient.get_status() returns that result dict unchanged. The tool
prefers get_status() over is_healthy(), but _infer_health_state only inspected the
boolean keys healthy and ok before falling back to string status/state fields, so the
SDK's shape matched nothing and fell through to "unknown". The safe summary also omitted the
canonical flag.

This teaches the existing normalizer about is_healthy rather than adding a parallel code
path. The key is appended after healthy and ok, so responses carrying the already
recognized healthy, ok, status, or state fields keep their current precedence. The
summary keeps its restricted field allowlist; raw components and errors values are still
never exposed, only the existing component_count.

This is an adapter compatibility fix for the existing response field, not a change to server
health computation or observer authorization.

The issue was reproduced before fixing, not inferred from code alone: a scripted repro using
the issue's own InMemoryOpenVikingClient subclass returned state: "unknown",
healthy: false, and summary: {"type": "dict"} for
{"is_healthy": true, "errors": [], "components": {}}. Reproduction was at the
adapter/SDK-response level; it does not exercise a deployed OpenViking server, model service,
or authentication configuration.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #5876

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • _infer_health_state now recognizes is_healthy as a boolean health key, checked after the
    existing healthy and ok keys so current precedence is preserved.
  • _safe_status_summary adds is_healthy to its safe-field allowlist so the canonical flag
    reaches the model-facing summary; the allowlist remains otherwise unchanged.
  • Added parametrized coverage to the existing health-tool test in
    tests/unit/test_langchain_integration.py for is_healthy: true, is_healthy: false, and
    the healthy: false + is_healthy: true precedence case, asserting the summary still
    excludes components and errors.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Validated with the same commands as the repository's own langchain-tests CI job, after
uv pip install -e sdk/python and uv pip install -e "examples/langchain[langgraph,test,dev]":

ruff format --check examples/langchain/src examples/langchain/tests \
    openviking/integrations/langchain tests/integration/langchain_langgraph \
    tests/unit/test_langchain*.py
# 42 files already formatted

ruff check <same paths>
# All checks passed!

mypy --config-file examples/langchain/pyproject.toml examples/langchain/src/langchain_openviking
# Success: no issues found in 15 source files

python -m pytest -q --noconftest -o addopts='' \
    tests/unit/test_langchain*.py tests/integration/langchain_langgraph
# 209 passed

The same suite on this branch's unmodified base gives 206 passed; the difference is the three
parametrized cases added here.

Before and after the change, verified with a temporary reproduction script: before,
is_healthy: true yielded unknown/false with summary: {"type": "dict"}; after, it
yields healthy/true with summary: {"is_healthy": true}, is_healthy: false yields
unhealthy/false, and {"healthy": false, "is_healthy": true} still resolves via healthy
to unhealthy/false.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

Scope is deliberately minimal: two tuple entries in the existing health normalizer plus test
coverage. No new helper, no new configuration surface. No user-facing documentation describes
the health tool's status-key handling, so no docs change was needed.

The observer system-status result serializes its top-level health flag as
is_healthy, and SyncHTTPClient.get_status() returns that result unchanged.
The viking_health tool prefers get_status() over is_healthy(), but its
normalizer only inspected the boolean keys healthy and ok before falling
back to string status/state fields, so the SDK shape matched nothing.

Before this change, {"is_healthy": true, "errors": [], "components": {}}
produced state "unknown" and healthy false, and the safe summary omitted
the flag entirely. Now the same response produces state "healthy" and
healthy true, is_healthy false produces state "unhealthy" and healthy
false, and the summary reports the flag. The key is checked after healthy
and ok, so responses carrying the already recognised healthy, ok, status
or state fields keep their existing precedence, and the summary stays
restricted to the safe field allowlist.

This branch has not been deployed

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

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: LangChain health tool ignores the SDK is_healthy status flag

1 participant