mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(health): label kompress as degraded/optional when not yet loaded (#2865)
## Description `/readyz` reports kompress as `"status": "unhealthy"` while the top-level payload simultaneously reports `"status": "healthy"` and `"ready": true`. This is a visible contradiction — kompress is intentionally excluded from the aggregate readiness gate, but it still receives the harshest label when it hasn't finished loading. This PR is a superset of #2829: it makes the same `degraded` status change **and** adds an `"optional": true` field to the component dict so API consumers can distinguish optional components from gating ones without parsing the `status` string. Fixes #2813. ## Type of Change - [x] 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 change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/proxy/server.py` — `_component_health()` accepts `optional: bool = False`; when `optional=True` and not-ready, status is `"degraded"` instead of `"unhealthy"`; `"optional": True` is added to the returned dict so callers can identify optional components without parsing the status string. Kompress call passes `optional=True`. - `tests/test_proxy_health.py` — All 11 kompress assertion dicts updated: `"status": "degraded"` for not-ready cases and `"optional": True` for all kompress cases (covering disabled/healthy/degraded states in the full parametrized matrix). ## Schema diff **Before** (kompress not yet loaded): ```json { "enabled": true, "ready": false, "status": "unhealthy", "backend": null } ``` **After**: ```json { "enabled": true, "ready": false, "status": "degraded", "optional": true, "backend": null } ``` The `"optional": true` field is additive — existing consumers that only check `status` are unaffected. The field gives consumers a stable machine-readable signal without requiring them to enumerate which component names are optional. ## Testing - [x] Unit tests pass (`pytest`) — CI only; `headroom._core` (compiled Rust extension) is not available locally, blocking direct `pytest tests/test_proxy_health.py` locally. All tests that don't import through `headroom.proxy.server → headroom.transforms → headroom._core` run locally. - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [ ] New tests added for new functionality (existing tests updated to cover the new status value and the new `"optional"` field) - [ ] Manual testing performed ### Test Output ``` $ uv run ruff check headroom/proxy/server.py tests/test_proxy_health.py All checks passed! $ uv run mypy headroom/proxy/server.py Success: no issues found in 1 source file ``` Full test suite (`tests/test_proxy_health.py`) is verified by CI; local run blocked by missing `headroom._core` native extension. ## Real Behavior Proof - Environment: local dev checkout, Windows 11, Python 3.14.3 - Ruff + mypy pass locally on both changed files (see Test Output above) - `tests/test_proxy_health.py` test suite requires `headroom._core` (compiled Rust extension not available locally) — CI run covers this - Diff is a mechanical expansion of the same `optional` flag already approved in #2829's head, plus the additive `"optional": true` response field ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` — it is generated by release-please from my Conventional Commit PR title --------- Signed-off-by: Radhakrishnan Pachyappan <radhakrishnan.p@op.tech> Co-authored-by: JD Davis <mxjerrett@gmail.com>
This commit is contained in:
Radhakrishnan Pachyappan
GitHub
parent
685ebe457d
commit
89493714d2
2 changed files with 70 additions and 12 deletions
|
|
@ -2685,15 +2685,29 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI:
|
|||
*,
|
||||
enabled: bool,
|
||||
ready: bool,
|
||||
optional: bool = False,
|
||||
**details: Any,
|
||||
) -> dict[str, Any]:
|
||||
status = "disabled" if not enabled else ("healthy" if ready else "unhealthy")
|
||||
return {
|
||||
if not enabled:
|
||||
status = "disabled"
|
||||
elif ready:
|
||||
status = "healthy"
|
||||
elif optional:
|
||||
# Optional/non-gating components report "degraded" rather than
|
||||
# "unhealthy" so the top-level status: "healthy" / ready: true
|
||||
# payload is not contradicted by a component-level failure label.
|
||||
status = "degraded"
|
||||
else:
|
||||
status = "unhealthy"
|
||||
result: dict[str, Any] = {
|
||||
"enabled": enabled,
|
||||
"ready": (ready if enabled else True),
|
||||
"status": status,
|
||||
**details,
|
||||
}
|
||||
if optional:
|
||||
result["optional"] = True
|
||||
result.update(details)
|
||||
return result
|
||||
|
||||
def _kompress_health_routers() -> list[ContentRouter]:
|
||||
routers: list[ContentRouter] = []
|
||||
|
|
@ -2803,6 +2817,7 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI:
|
|||
"kompress": _component_health(
|
||||
enabled=kompress_enabled,
|
||||
ready=proxy.warmup.kompress.status == "loaded",
|
||||
optional=True,
|
||||
backend=proxy.warmup.kompress.info.get("backend", None),
|
||||
),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -69,7 +69,8 @@ def test_readyz_excludes_kompress_from_aggregate_readiness(monkeypatch):
|
|||
assert payload["checks"]["kompress"] == {
|
||||
"enabled": True,
|
||||
"ready": False,
|
||||
"status": "unhealthy",
|
||||
"status": "degraded",
|
||||
"optional": True,
|
||||
"backend": None,
|
||||
}
|
||||
|
||||
|
|
@ -85,6 +86,7 @@ def test_readyz_promotes_deferred_kompress_after_runtime_load(monkeypatch):
|
|||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "onnx",
|
||||
}
|
||||
# The promotion must also clear the startup marker, otherwise the slot
|
||||
|
|
@ -110,6 +112,7 @@ def test_readyz_promotes_kompress_from_module_cache(monkeypatch, attached):
|
|||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "onnx",
|
||||
}
|
||||
assert proxy.warmup.kompress.handle is model
|
||||
|
|
@ -141,7 +144,8 @@ def test_readyz_keeps_pending_kompress_unloaded(monkeypatch):
|
|||
assert payload["checks"]["kompress"] == {
|
||||
"enabled": True,
|
||||
"ready": False,
|
||||
"status": "unhealthy",
|
||||
"status": "degraded",
|
||||
"optional": True,
|
||||
"backend": None,
|
||||
}
|
||||
assert compressor.calls == ["is_ready"]
|
||||
|
|
@ -181,6 +185,7 @@ def test_readyz_disabled_kompress_skips_inspection(monkeypatch):
|
|||
"enabled": False,
|
||||
"ready": True,
|
||||
"status": "disabled",
|
||||
"optional": True,
|
||||
"backend": None,
|
||||
}
|
||||
assert compressor.calls == []
|
||||
|
|
@ -202,6 +207,7 @@ def test_readyz_per_provider_kompress_override_reenables_health(monkeypatch):
|
|||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "onnx",
|
||||
}
|
||||
assert compressor.calls == ["is_ready", "ready_backend"]
|
||||
|
|
@ -222,7 +228,8 @@ def test_readyz_never_calls_lazy_kompress_getters(monkeypatch):
|
|||
assert payload["checks"]["kompress"] == {
|
||||
"enabled": True,
|
||||
"ready": False,
|
||||
"status": "unhealthy",
|
||||
"status": "degraded",
|
||||
"optional": True,
|
||||
"backend": None,
|
||||
}
|
||||
|
||||
|
|
@ -234,37 +241,73 @@ def test_readyz_never_calls_lazy_kompress_getters(monkeypatch):
|
|||
"null",
|
||||
None,
|
||||
False,
|
||||
{"enabled": True, "ready": False, "status": "unhealthy", "backend": None},
|
||||
{
|
||||
"enabled": True,
|
||||
"ready": False,
|
||||
"status": "degraded",
|
||||
"optional": True,
|
||||
"backend": None,
|
||||
},
|
||||
),
|
||||
(
|
||||
"null",
|
||||
_ReadyCompressor(),
|
||||
False,
|
||||
{"enabled": True, "ready": True, "status": "healthy", "backend": "onnx"},
|
||||
{
|
||||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "onnx",
|
||||
},
|
||||
),
|
||||
(
|
||||
"null",
|
||||
_ReadyCompressor(backend="remote"),
|
||||
False,
|
||||
{"enabled": True, "ready": True, "status": "healthy", "backend": "remote"},
|
||||
{
|
||||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "remote",
|
||||
},
|
||||
),
|
||||
(
|
||||
"error",
|
||||
_ReadyCompressor(),
|
||||
False,
|
||||
{"enabled": True, "ready": True, "status": "healthy", "backend": "onnx"},
|
||||
{
|
||||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "onnx",
|
||||
},
|
||||
),
|
||||
(
|
||||
"loaded",
|
||||
_ReadyCompressor(),
|
||||
False,
|
||||
{"enabled": True, "ready": True, "status": "healthy", "backend": "existing"},
|
||||
{
|
||||
"enabled": True,
|
||||
"ready": True,
|
||||
"status": "healthy",
|
||||
"optional": True,
|
||||
"backend": "existing",
|
||||
},
|
||||
),
|
||||
(
|
||||
"null",
|
||||
_ReadyCompressor(),
|
||||
True,
|
||||
{"enabled": False, "ready": True, "status": "disabled", "backend": None},
|
||||
{
|
||||
"enabled": False,
|
||||
"ready": True,
|
||||
"status": "disabled",
|
||||
"optional": True,
|
||||
"backend": None,
|
||||
},
|
||||
),
|
||||
],
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue