From 3b0bceecf4281eb34112de8dd546d4a58beb3fcc Mon Sep 17 00:00:00 2001 From: Focused Instability <70747559+MrAshRhodes@users.noreply.github.com> Date: Mon, 15 Jun 2026 23:29:52 +0200 Subject: [PATCH] fix(cache): name the missing piece in semantic detector guard (#1018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The `test` and `test-extras` CI jobs are currently red on `main`: `tests/test_cache/test_dynamic_detector.py::TestSemanticDetectorGuards::test_none_exemplars_early_return` fails. This PR fixes the underlying regression. It is independent of any feature branch (it only touches `headroom/cache/dynamic_detector.py` and its test). #950 folded the exemplar-embeddings None-check into the model None-guard: ```python if self._model is None or self._exemplar_embeddings is None: return [], self._load_error or "semantic detector is not initialized" ``` So a `SemanticDetector` with a loaded model but unset exemplar embeddings now returns the generic *"semantic detector is not initialized"* message, shadowing the specific *"exemplar embeddings not initialized"* message and leaving the later guard as unreachable dead code. That directly contradicts #950's own `test_none_exemplars_early_return`, which asserts the specific message — hence the red main. ## Type of Change - [ ] New feature - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Breaking change - [ ] Documentation ## Changes Made - `SemanticDetector.detect`: split the combined guard into two checks — `_model` then `_exemplar_embeddings` — both *before* `encode()`. The model-missing case keeps the generic message; the exemplar-missing case reports the specific *"exemplar embeddings not initialized"*. Checking before `encode()` avoids a wasted encode in the error path and preserves the mypy narrowing for `np.dot(..., self._exemplar_embeddings.T)`. The previously-shadowed duplicate guard is removed. - `tests/test_cache/test_dynamic_detector.py`: `test_missing_exemplar_embeddings_returns_warning` sets the same model-present / exemplar-None state, so its assertion is aligned to the specific message to match `test_none_exemplars_early_return`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] New and existing unit tests pass locally with my changes ### Test Output ```text $ pytest tests/test_cache/test_dynamic_detector.py -q 38 passed, 2 skipped in 9.94s $ pytest tests/test_cache/ -q 198 passed, 2 skipped in 11.58s $ ruff check headroom/cache/dynamic_detector.py tests/test_cache/test_dynamic_detector.py All checks passed! $ ruff format --check headroom/cache/dynamic_detector.py tests/test_cache/test_dynamic_detector.py 2 files already formatted $ mypy headroom/cache/dynamic_detector.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - Environment: local macOS, repo .venv, Python 3.11.9, numpy installed - Exact command / steps: construct a `SemanticDetector` via `object.__new__` with `_model` set (mock) and `_exemplar_embeddings = None`, then call `.detect(...)`. Before fix: on `upstream/main` (7c8c909c) `test_none_exemplars_early_return` fails with `assert 'semantic detector is not initialized' == 'exemplar embeddings not initialized'`. After fix: full file 38 passed, full `tests/test_cache/` 198 passed. - Observed result: model-present + exemplar-None now returns `(spans=[], "exemplar embeddings not initialized")`; model-None still returns the generic message; `np.dot` is never reached with a None matrix. - Not tested: live model load / real embeddings — the guards are the unavailable-state paths, exercised via the existing mock-based unit tests. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Additional Notes This takes the **specific-message** direction because it matches #950's newest test, the original (now-dead) specific guard string, and gives a more actionable warning. The conservative **alternative** — keep the generic unified message, delete the dead specific guard, and update `test_none_exemplars_early_return` to assert the generic string — also turns CI green with no production behavior change. Happy to switch to that if you prefer; it's your call on the intended contract. --- headroom/cache/dynamic_detector.py | 17 ++++++++++------- tests/test_cache/test_dynamic_detector.py | 4 +++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/headroom/cache/dynamic_detector.py b/headroom/cache/dynamic_detector.py index 5030862e9..3c5117f83 100644 --- a/headroom/cache/dynamic_detector.py +++ b/headroom/cache/dynamic_detector.py @@ -777,20 +777,23 @@ class SemanticDetector: return [], "numpy not installed. Install with: pip install numpy" sentence_texts = [s[0] for s in sentences] - if self._model is None or self._exemplar_embeddings is None: + # `is_available` only guarantees `_model` is set. Guard each piece + # separately and *before* encoding so a None never reaches `.T` (a + # real crash), mypy can narrow the `Any | None` attributes, and the + # caller gets a warning that names the actual missing piece — the + # model vs. the exemplar matrix. (Folding both into one guard, as a + # prior change did, returned the generic "semantic detector" message + # even when only the exemplars were missing.) + if self._model is None: return [], self._load_error or "semantic detector is not initialized" + if self._exemplar_embeddings is None: + return [], "exemplar embeddings not initialized" sentence_embeddings = self._model.encode( sentence_texts, convert_to_numpy=True, ) - # Compute similarities. `is_available` only guarantees `_model` is - # set; guard the exemplar matrix explicitly so a None never reaches - # `.T` (real crash) and mypy can narrow the `Any | None` attribute. - if self._exemplar_embeddings is None: - return [], "exemplar embeddings not initialized" - similarities = np.dot(sentence_embeddings, self._exemplar_embeddings.T) for i, (text, start, end) in enumerate(sentences): diff --git a/tests/test_cache/test_dynamic_detector.py b/tests/test_cache/test_dynamic_detector.py index 154a309d9..309efa66f 100644 --- a/tests/test_cache/test_dynamic_detector.py +++ b/tests/test_cache/test_dynamic_detector.py @@ -486,7 +486,9 @@ class TestSemanticDetector: spans, warning = detector.detect("The current stock price changes every minute.") assert spans == [] - assert warning == "semantic detector is not initialized" + # Model present but exemplar matrix missing → the warning names the + # actual missing piece (matches TestSemanticDetectorGuards below). + assert warning == "exemplar embeddings not initialized" class TestIntegrationWithAllTiers: