From 6cc2a87b44fba7f49e1f3519c3df5d4920119bdb Mon Sep 17 00:00:00 2001 From: sloemodzn Date: Sat, 22 Aug 2026 20:18:33 +0530 Subject: [PATCH] fix(cli): warn on silent embedder degradation in update paths The embedder-degradation fallback was reported once per call site, not once per invocation. A single 'repowise update' builds the embedder for its decision semantic-dedup store and again for the deterministic page path, so a repo with an unavailable embedder printed the same full warning twice in one run (and a workspace update once per repo). build_embedder's degradation warning now fires at most once per process, matching the init header probe, with a reset hook so tests and embedded runners can re-arm it for the next logical invocation. --- .../src/repowise/cli/providers/embedders.py | 44 ++++++++++--- tests/unit/cli/conftest.py | 17 ++++++ tests/unit/cli/test_embedder_resolution.py | 61 ++++++++++++++++++- 3 files changed, 114 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/repowise/cli/providers/embedders.py b/packages/cli/src/repowise/cli/providers/embedders.py index 12f70b0fe..0df8f8279 100644 --- a/packages/cli/src/repowise/cli/providers/embedders.py +++ b/packages/cli/src/repowise/cli/providers/embedders.py @@ -8,6 +8,27 @@ from repowise.cli.providers.keys import embedder_key_env_vars, resolve_embedder_api_key +# Once-per-invocation gate for the degradation warning below. A single command +# trips the fallback in several sites: ``repowise update`` builds the embedder +# for the decision semantic-dedup store and again for the deterministic page +# path, and a workspace run builds one per repo. Printing the full warning each +# time stacks the same sentence over and over, which buries the signal. The +# first fallback in a process reports it; later ones are just as degraded but +# stay quiet, exactly as the init header probe does. +_degradation_warned = False + + +def reset_degradation_warning() -> None: + """Clear the once-per-invocation gate. + + The gate is process-wide so the warning survives every build in one CLI + invocation, but a process that runs several logical invocations (tests, + an embedded runner) needs a way to arm it again. Callers that expect to + see the warning on a fresh unit of work call this first. + """ + global _degradation_warned + _degradation_warned = False + def _embedder_kwargs(embedder_name: str, repo_path: Any = None) -> dict[str, Any]: kwargs: dict[str, Any] = {} @@ -212,11 +233,20 @@ def build_embedder(embedder_name_resolved: str, repo_path: Any = None) -> Any: places = "\n".join(f" · {place}" for place in lookup.searched) searched = f"\nNo API key was found in any of:\n{places}" - err_console.print( - f"[yellow]Embedder '{embedder_name_resolved}' could not be built:[/yellow] " - f"{type(exc).__name__}: {exc}{searched}\n" - "Falling back to keyless embeddings, which means [bold]no semantic search[/bold] " - "for this run.\nFull-text and symbol search are unaffected. Fix the key or " - "endpoint and run [cyan]repowise reindex[/cyan]\nto restore semantic search." - ) + # Once per invocation, not per site. One ``repowise update`` builds the + # embedder for its decision store and again for the deterministic page + # path; warning on each would print this same sentence twice (and a + # workspace update once per repo). The first build of a run reports the + # degradation, the rest ride along — the init header probe gates the + # same way. + global _degradation_warned + if not _degradation_warned: + _degradation_warned = True + err_console.print( + f"[yellow]Embedder '{embedder_name_resolved}' could not be built:[/yellow] " + f"{type(exc).__name__}: {exc}{searched}\n" + "Falling back to keyless embeddings, which means [bold]no semantic search[/bold] " + "for this run.\nFull-text and symbol search are unaffected. Fix the key or " + "endpoint and run [cyan]repowise reindex[/cyan]\nto restore semantic search." + ) return KeylessEmbedder() diff --git a/tests/unit/cli/conftest.py b/tests/unit/cli/conftest.py index 14e212254..b4baeeb48 100644 --- a/tests/unit/cli/conftest.py +++ b/tests/unit/cli/conftest.py @@ -39,6 +39,23 @@ import pytest +@pytest.fixture(autouse=True) +def _reset_degradation_warning(): + """Re-arm the once-per-invocation embedder-degradation warning. + + ``build_embedder``'s degradation warning fires at most once per process so + a single ``update`` that builds the embedder for several stores prints the + sentence once, not once per site. That gate is module-global and would + otherwise stay armed across tests, silencing the very warning a later test + asserts on. + """ + from repowise.cli.providers.embedders import reset_degradation_warning + + reset_degradation_warning() + yield + reset_degradation_warning() + + @pytest.fixture(autouse=True) def _isolated_home(tmp_path_factory, monkeypatch): fake_home = tmp_path_factory.mktemp("home") diff --git a/tests/unit/cli/test_embedder_resolution.py b/tests/unit/cli/test_embedder_resolution.py index b3e7a0329..c613f662a 100644 --- a/tests/unit/cli/test_embedder_resolution.py +++ b/tests/unit/cli/test_embedder_resolution.py @@ -451,7 +451,8 @@ async def embed(self, texts: list[str]) -> list[list[float]]: return [[0.0] * 1024 for _ in texts] monkeypatch.setattr( - "repowise.cli.providers.embedders.build_embedder", lambda _n, _p=None: _MisreportingEmbedder() + "repowise.cli.providers.embedders.build_embedder", + lambda _n, _p=None: _MisreportingEmbedder(), ) assert _vector_dims(tmp_path) == (None, None) @@ -654,3 +655,61 @@ def test_build_embedder_is_silent_for_the_keyless_default(monkeypatch): assert isinstance(embedder, KeylessEmbedder) assert printed == [] + + +def test_build_embedder_warns_once_per_invocation(monkeypatch): + """A run that degrades in several sites reports the fallback once. + + ``repowise update`` builds the embedder for its decision semantic-dedup + store and again for the deterministic page path; a workspace run builds one + per repo. If each build printed the full warning, one command stacks the + same sentence N times, burying the signal. The first fallback in a process + reports it, the rest stay quiet — the same once-per-invocation gate the + init header probe uses. + """ + from repowise.core.providers.embedding.base import KeylessEmbedder + + def _boom(name: str, **kwargs: object): + raise RuntimeError("connection refused") + + monkeypatch.setattr("repowise.core.providers.embedding.registry.get_embedder", _boom) + printed = _capture_console(monkeypatch) + + first = providers.build_embedder("openai") + second = providers.build_embedder("gemini") + third = providers.build_embedder("ollama") + + assert isinstance(first, KeylessEmbedder) + assert isinstance(second, KeylessEmbedder) + assert isinstance(third, KeylessEmbedder) + said = " ".join(printed) + assert "semantic search" in said.lower() + # Exactly one warning across the three degraded builds. + assert said.count("could not be built") == 1 + + +def test_build_embedder_warns_again_after_a_reset(monkeypatch): + """The once-per-invocation gate is per invocation, not permanent. + + A process that runs several logical invocations (tests, an embedded + runner) re-arms the gate and sees the warning again on the next degraded + build, so the sentence is not lost to the first run. + """ + from repowise.core.providers.embedding.base import KeylessEmbedder + + def _boom(name: str, **kwargs: object): + raise RuntimeError("connection refused") + + monkeypatch.setattr("repowise.core.providers.embedding.registry.get_embedder", _boom) + printed = _capture_console(monkeypatch) + + from repowise.cli.providers.embedders import reset_degradation_warning + + assert isinstance(providers.build_embedder("openai"), KeylessEmbedder) + assert isinstance(providers.build_embedder("openai"), KeylessEmbedder) + assert " ".join(printed).count("could not be built") == 1 + + reset_degradation_warning() + + assert isinstance(providers.build_embedder("openai"), KeylessEmbedder) + assert " ".join(printed).count("could not be built") == 2