diff --git a/backend/tests/unit/services/test_demo_reseed_partial_completion_fast.py b/backend/tests/unit/services/test_demo_reseed_partial_completion_fast.py new file mode 100644 index 00000000..05689b3d --- /dev/null +++ b/backend/tests/unit/services/test_demo_reseed_partial_completion_fast.py @@ -0,0 +1,238 @@ +# SPDX-FileCopyrightText: 2026 soundminds.ai +# +# SPDX-License-Identifier: Apache-2.0 + +"""Fast guard test for the demo-reseed PARTIAL-completion path. + +`infra_solr_ci_readiness` made `reseed_demo_state` engine-tolerant: when an +engine is unreachable its scenario is skipped, the reseed still finishes +``status="complete"`` with a non-empty ``scenarios_skipped``, and exactly one +``demo_reseed_partial_completion_engines_unreachable`` WARN fires (AC-7). The +all-engines-unreachable verdict + the worker's failed-status mapping already +have fast unit coverage in ``test_demo_seeding_partial_completion.py``; the +END-TO-END partial path (ES + OpenSearch + rich seed, Solr skips) was only +asserted by the 13-19 min heavy-lane ``test_demo_seeding_ubi_full.py`` (needs +the full stack + a live OpenAI key). + +This is the fast guard for that headline behavior. It drives the real +``reseed_demo_state`` orchestrator with every I/O helper monkeypatched to +canned success (chore_demo_reseed_partial_completion_fast_test, locked +approach b' — patch the module-level helpers, NOT an httpx-URL mock and NOT a +seam extraction, so the orchestrator structure is untouched and the test stays +a pure unit). ``is_engine_reachable`` reports only Solr down, so the loop must +skip exactly ``acme-kb-docs-solr`` and complete everything else. +""" + +from __future__ import annotations + +import logging +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from backend.app.services import demo_seeding +from backend.app.services.demo_seeding import ( + SCENARIOS, + AllEnginesUnreachableError, + DemoSeedingError, + ReseedStatusResponse, + reseed_demo_state, +) + +# Union of every scenario's query texts -> a fake id. The reseed body fetches +# the query rows for each scenario and raises DemoSeedingError if any of its +# query texts is absent, so the fake GET returns the full union (each scenario +# filters down to its own texts; extras are harmless). +_ALL_QUERY_TEXTS: list[str] = [ + q["query_text"] for scenario in SCENARIOS for q in scenario["queries"] +] + + +def _install_canned_seed_path(monkeypatch: pytest.MonkeyPatch) -> None: + """Monkeypatch every demo_seeding I/O helper to canned success. + + Leaves the orchestrator's control flow (reachability gating, skip + accounting, the partial-completion WARN, status assignment) entirely real — + that flow is exactly what this test guards. + """ + counter = {"n": 0} + + async def _fake_post( + client: Any, + url: str, + *, + json: Any = None, + auth: Any = None, + client_label: str = "", + step: str = "", + ) -> dict[str, Any]: + counter["n"] += 1 + n = counter["n"] + # Order matters: the queries sub-resource ends with "/queries" and must + # be matched BEFORE the bare "/query-sets" create. + if url.endswith("/queries"): + return {} + if url.endswith("/clusters"): + return {"id": f"cluster-{n}"} + if url.endswith("/query-templates"): + return {"id": f"template-{n}"} + if url.endswith("/query-sets"): + return {"id": f"qset-{n}"} + if url.endswith("/judgment-lists/import"): + return {"id": f"jlist-{n}"} + if url.endswith("/judgments/generate-from-ubi"): + return {"judgment_list_id": f"ubi-jlist-{n}"} + # Engine _refresh and anything else: shape not consumed. + return {} + + async def _fake_get( + client: Any, + url: str, + *, + params: Any = None, + auth: Any = None, + client_label: str = "", + step: str = "", + ) -> dict[str, Any]: + return { + "data": [ + {"query_text": text, "id": f"q-{i}"} for i, text in enumerate(_ALL_QUERY_TEXTS) + ] + } + + async def _fake_put( + client: Any, + url: str, + *, + json: Any = None, + auth: Any = None, + client_label: str = "", + step: str = "", + ) -> dict[str, Any]: + return {} + + monkeypatch.setattr(demo_seeding, "_post", _fake_post) + monkeypatch.setattr(demo_seeding, "_get", _fake_get) + monkeypatch.setattr(demo_seeding, "_put", _fake_put) + # High-level seed helpers — return the scalars the body consumes. + monkeypatch.setattr( + demo_seeding, + "_seed_real_study_for_scenario", + AsyncMock(side_effect=lambda *a, **k: f"study-{counter['n']}"), + ) + monkeypatch.setattr( + demo_seeding, "_seed_rich_scenario", AsyncMock(return_value="rich-study-id") + ) + monkeypatch.setattr(demo_seeding, "ensure_ubi_indices", AsyncMock(return_value=None)) + monkeypatch.setattr(demo_seeding, "seed_synthetic_ubi", AsyncMock(return_value=100)) + monkeypatch.setattr( + demo_seeding, "_poll_judgment_list_until_terminal", AsyncMock(return_value=None) + ) + # Pure-domain generator: return empty (events not consumed beyond a count + # that seed_synthetic_ubi mocks). + monkeypatch.setattr(demo_seeding, "fabricate_ubi_for_scenario", lambda **k: ([], [])) + + +def _only_solr_unreachable() -> Any: + """An ``is_engine_reachable`` replacement: every engine up except Solr.""" + + async def _probe(url: str, engine_type: str, **kwargs: Any) -> bool: + # **kwargs absorbs the real is_engine_reachable's keyword-only + # `timeout_s` (and any future kwargs) so the mock can't TypeError. + return engine_type != "solr" + + return _probe + + +def _mock_db() -> AsyncMock: + db = AsyncMock() + db.execute = AsyncMock(return_value=None) + db.commit = AsyncMock(return_value=None) + return db + + +def _mock_engine_client() -> AsyncMock: + """Engine client whose only DIRECT call (Step 1b index DELETE) returns a + tolerated 204. Every other engine call routes through the monkeypatched + ``_put`` / ``_post`` helpers.""" + client = AsyncMock() + client.delete = AsyncMock(return_value=MagicMock(status_code=204)) + return client + + +async def test_partial_completion_skips_only_solr_and_warns_once( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """ES/OS/rich seed, Solr skips: status=complete, one skip, one WARN.""" + _install_canned_seed_path(monkeypatch) + monkeypatch.setattr(demo_seeding, "is_engine_reachable", _only_solr_unreachable()) + + captured: dict[str, ReseedStatusResponse] = {} + + async def _capture(progress: ReseedStatusResponse) -> None: + captured["progress"] = progress + + with caplog.at_level(logging.WARNING, logger=demo_seeding.logger.name): + summary = await reseed_demo_state( + _mock_db(), + MagicMock(), # api_client — unused (every helper is mocked) + _mock_engine_client(), + status_callback=_capture, + ) + + progress = captured["progress"] + # Exactly the Solr scenario skipped; nothing else. + assert progress.scenarios_skipped == ["acme-kb-docs-solr"] + # Partial != failure: the reseed completes. + assert progress.status == "complete" + # The 4 reachable SCENARIOS + the rich scenario all completed. + assert progress.scenarios_completed == len(SCENARIOS) - 1 + 1 + assert summary.studies_completed >= len(SCENARIOS) - 1 + 1 + # AC-7: exactly one partial-completion WARN. + partial_warns = [ + r + for r in caplog.records + if r.getMessage() == "demo_reseed_partial_completion_engines_unreachable" + ] + assert len(partial_warns) == 1 + # The structured `extra={"scenarios_skipped": ...}` rides on the LogRecord + # as a runtime attribute (getattr keeps mypy happy — LogRecord has no + # static field for it). + assert getattr(partial_warns[0], "scenarios_skipped", None) == ["acme-kb-docs-solr"] + + +async def test_reachable_scenario_failure_is_hard_error_not_a_skip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """AC-3: a reachable scenario that fails mid-seed raises a generic + ``DemoSeedingError`` (NOT ``AllEnginesUnreachableError``) and is never + silently moved into ``scenarios_skipped``.""" + _install_canned_seed_path(monkeypatch) + monkeypatch.setattr(demo_seeding, "is_engine_reachable", _only_solr_unreachable()) + # Make a REACHABLE scenario fail mid-seed (the study-create step). + monkeypatch.setattr( + demo_seeding, + "_seed_real_study_for_scenario", + AsyncMock(side_effect=DemoSeedingError("acme-products-prod/create_study: HTTP 503")), + ) + + captured: dict[str, ReseedStatusResponse] = {} + + async def _capture(progress: ReseedStatusResponse) -> None: + captured["progress"] = progress + + with pytest.raises(DemoSeedingError) as exc_info: + await reseed_demo_state( + _mock_db(), MagicMock(), _mock_engine_client(), status_callback=_capture + ) + + # A mid-seed failure on a REACHABLE engine is a hard error, never the + # tolerated all-unreachable verdict. + assert not isinstance(exc_info.value, AllEnginesUnreachableError) + # And the failing scenario is NOT in scenarios_skipped — skip accounting + # only happens at the reachability gate, never in the seed body. progress is + # always captured (several _emit_progress calls precede the step-2h study + # failure), so access it directly — a missing capture should fail the test. + progress = captured["progress"] + assert "acme-products-prod" not in progress.scenarios_skipped diff --git a/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/bug_fix.md b/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/bug_fix.md new file mode 100644 index 00000000..84d02cac --- /dev/null +++ b/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/bug_fix.md @@ -0,0 +1,48 @@ +# Bug fix — chore_demo_reseed_partial_completion_fast_test + +**Source idea:** [idea.md](./idea.md) +**Branch:** `chore/demo-reseed-partial-completion-fast-test` +**Type:** chore — test-robustness (medium; routed through /bug-fix per the preflight decision tree). Not a defect fix: a *guard* test for already-correct behavior. +**Date:** 2026-06-05 + +## Problem + +`infra_solr_ci_readiness` made `reseed_demo_state` engine-tolerant — an unreachable engine's scenario is skipped, the reseed still finishes `status="complete"` with a non-empty `scenarios_skipped`, and exactly one `demo_reseed_partial_completion_engines_unreachable` WARN fires (AC-7). The building blocks have fast unit coverage, but the **end-to-end partial path** (ES + OS + rich seed actually complete, Solr skips) was asserted only by the heavy-lane `test_demo_seeding_ubi_full.py` (full stack + live OpenAI key, 13–19 min). A regression in the skip accounting or the WARN would only surface in the slow lane. + +## Reproduction + +This is a guard test for correct behavior, so the "fails on main" inverts: the new test PASSES against current correct code, and a mutation that breaks the partial-completion logic makes it FAIL. Verified load-bearing: + +```bash +# Passes on current code: +.venv/bin/python -m pytest backend/tests/unit/services/test_demo_reseed_partial_completion_fast.py -q --no-cov +# Mutation (suppress the partial WARN: `if progress.scenarios_skipped:` -> `if False:`) +# -> test_partial_completion_skips_only_solr_and_warns_once FAILS (len(partial_warns) 0 != 1). +``` + +## Root cause + +N/A — no defect. The gap is a *missing fast guard* for the partial-completion control flow in `reseed_demo_state`. + +- Owning layer: service — [`backend/app/services/demo_seeding.py:1442`](../../../../backend/app/services/demo_seeding.py) (`reseed_demo_state`); the guarded logic is the per-scenario reachability gate (:1578-1584), the rich-scenario gate (:1962-1972), the all-unreachable verdict (:1997), and the partial WARN (:1999-2006). + +## Fix design (locked decisions) + +1. **Approach (b′): monkeypatch demo_seeding's module-level I/O helpers, NOT an httpx-URL mock and NOT a seam extraction.** The per-scenario seed body is a chain of module-level `async def`s (`_post`/`_get`/`_put`/`_seed_real_study_for_scenario`/`_seed_rich_scenario`/`ensure_ubi_indices`/`seed_synthetic_ubi`/`fabricate_ubi_for_scenario`/`_poll_judgment_list_until_terminal`). The test `monkeypatch.setattr`s these to canned success, leaving the orchestrator's control flow real. Cites: keeps the orchestrator structure untouched → no conflict with the deferred `chore_demo_seeding_integration_tests_rewrite`; pure unit (no DB/engine/OpenAI) → locally verifiable. Rejected: (a) `httpx.MockTransport` URL routing (fragile, must enumerate every URL); (c) Postgres-only integration (needs a real DB in CI for no extra signal — persistence is delegated through the mocked `api_client`). +2. **`is_engine_reachable` → `engine_type != "solr"`** so only the Solr scenario skips. The Solr seed path (`_seed_solr_scenario`) is never reached, so it needs no mock. +3. **`db` is an `AsyncMock`** (the body only does TRUNCATE + study-rename `execute` + `commit`); the engine client's one direct call (Step 1b index DELETE) returns a tolerated 204. +4. **`_get` returns the union of all SCENARIOS' query texts** so each scenario's text-lookup resolves (the body raises `DemoSeedingError` on a missing text); extras are harmless (each scenario filters to its own). + +## Regression test plan + +| Layer | Path | What it asserts | +|---|---|---| +| unit | `backend/tests/unit/services/test_demo_reseed_partial_completion_fast.py` | (1) only-Solr-down → `scenarios_skipped == ["acme-kb-docs-solr"]`, `status="complete"`, `scenarios_completed == 5`, exactly one partial WARN carrying the skip list; (2) AC-3 — a reachable scenario failing mid-seed raises a generic `DemoSeedingError` (not `AllEnginesUnreachableError`) and the slug is never added to `scenarios_skipped`. | + +## Rollout + +None — test-only change, no migration, no production diff. + +## Tangential observations + +None. diff --git a/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/idea.md b/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/idea.md index d75eb0eb..dc326a43 100644 --- a/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/idea.md +++ b/docs/00_overview/planned_features/02_mvp2/chore_demo_reseed_partial_completion_fast_test/idea.md @@ -6,6 +6,13 @@ **Origin:** `infra_solr_ci_readiness` Epic 1 implementation (PR for `feature/infra-solr-ci-readiness`). Surfaced in the GPT-5.5 phase-gate review (Finding 5) and the post-impl tangential sweep. **Depends on:** `infra_solr_ci_readiness` Phase 1 merged. +> **PREFLIGHT (2026-06-05).** Live-codebase audit: all concrete claims verified. +> - `reseed_demo_state` at [`demo_seeding.py:1442`](../../../../backend/app/services/demo_seeding.py); `is_engine_reachable` :446; `snapshot_engine_reachability` :488; `AllEnginesUnreachableError` :210 / `_is_all_engines_unreachable` :232; worker `_build_failed_status` at [`demo_reseed.py:64`](../../../../backend/workers/demo_reseed.py); the `demo_reseed_partial_completion_engines_unreachable` WARN at `demo_seeding.py:2001`; `scenarios_skipped.append(slug)` at :1583; the Solr slug `acme-kb-docs-solr` confirmed in `SCENARIOS`. Existing fast coverage: [`test_demo_seeding_partial_completion.py`](../../../../backend/tests/unit/services/test_demo_seeding_partial_completion.py) (building blocks); the end-to-end partial path only in heavy-lane [`test_demo_seeding_ubi_full.py:144`](../../../../backend/tests/integration/test_demo_seeding_ubi_full.py). +> - **Local verifiability:** `.venv/bin/pytest` is present and the chosen approach is a pure unit test (no DB, no engines, no OpenAI), so it runs offline before push — no CI-blind risk. +> - **DESIGN FORK LOCKED → (b′) monkeypatch the module-level I/O helpers, NOT a seam extraction and NOT httpx-URL routing.** The per-scenario seed body is a chained sequence of module-level `async def`s in `demo_seeding.py` (`_post` :?, `_get`, `_put` :580, `_seed_real_study_for_scenario` :965, `_seed_rich_scenario` :1123, `_seed_solr_scenario` :657, `ensure_ubi_indices` / `fabricate_ubi_for_scenario` / `seed_synthetic_ubi`). The test `monkeypatch.setattr`s these to canned-success (returning the exact shapes the body consumes — e.g. `_post` cluster/template/qset/jlist → dicts with the `id` keys the body reads), plus `is_engine_reachable` (Solr→False, ES/OS→True), plus an `AsyncMock` `db` (the body only does one TRUNCATE + commit). This avoids touching the orchestrator's structure — so it does NOT conflict with the deferred `chore_demo_seeding_integration_tests_rewrite`, and keeps the test a pure unit. Rejected: (a) full `httpx.MockTransport` URL-routing over both clients (fragile, must enumerate every URL); (c) Postgres-only integration (needs a real DB in CI for no extra signal — the body delegates all persistence through `api_client`, which is mocked anyway). +> - **Coordination:** overlaps with the deferred `chore_demo_seeding_integration_tests_rewrite` — proceed **standalone now** (the rewrite is deferred pending a local stack; this focused unit test is small and the rewrite can absorb/supersede it later). +> - **Route:** `chore_` + just-locked design fork + bounded test-only backend scope → `/bug-fix --ship` (focused `bug_fix.md` locking the helper-monkeypatch approach, then `/impl-execute --ad-hoc`). + ## Problem `infra_solr_ci_readiness` made the demo reseed engine-tolerant: when an engine is @@ -46,9 +53,15 @@ a live engine/API/OpenAI, asserting: The hard part is the seed path: `reseed_demo_state` inlines a lot (engine PUT/collection-create, `api_client` cluster/template/query-set/queries/judgments/seed-completed-study, UBI synth). -Options: (a) a Postgres-only integration test with `api_client`/`engine_client` mocked to +~~Options: (a) a Postgres-only integration test with `api_client`/`engine_client` mocked to return canned success shapes; (b) extract the per-scenario seed body into a seam that can -be stubbed; (c) a `respx`/`httpx-mock` layer over both clients. +be stubbed; (c) a `respx`/`httpx-mock` layer over both clients.~~ + +**LOCKED at preflight → (b′): monkeypatch the module-level I/O helpers** (`_post`/`_get`/`_put`/ +`_seed_real_study_for_scenario`/`_seed_rich_scenario`/`_seed_solr_scenario`/`ensure_ubi_indices`/ +`fabricate_ubi_for_scenario`/`seed_synthetic_ubi`) to canned-success + `is_engine_reachable` +(Solr→False) + `AsyncMock` db. Pure unit test, no seam extraction, no orchestrator-structure +change. See the PREFLIGHT block above for the full rationale + rejected alternatives. ## Scope signals