From 89493714d2cffdc1f81a8f417ea09891453d7009 Mon Sep 17 00:00:00 2001 From: Radhakrishnan Pachyappan Date: Wed, 12 Aug 2026 10:05:28 +0530 Subject: [PATCH] fix(health): label kompress as degraded/optional when not yet loaded (#2865) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 Co-authored-by: JD Davis --- headroom/proxy/server.py | 21 +++++++++++-- tests/test_proxy_health.py | 61 ++++++++++++++++++++++++++++++++------ 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/headroom/proxy/server.py b/headroom/proxy/server.py index ea9615fcc..10e5260c7 100644 --- a/headroom/proxy/server.py +++ b/headroom/proxy/server.py @@ -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), ), } diff --git a/tests/test_proxy_health.py b/tests/test_proxy_health.py index efdad4cc1..b45cb42ad 100644 --- a/tests/test_proxy_health.py +++ b/tests/test_proxy_health.py @@ -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, + }, ), ], )