Plan 005: Fix the four broken async embedding tests and run the async suite in CI

[strickvl]

Plan 005: Fix the four broken async embedding tests and run the async suite in CI

Executor instructions: Follow this plan step by step. Run every
verification command and confirm the expected result before moving to the
next step. If anything in the "STOP conditions" section occurs, stop and
report — do not improvise. When done, update the status row for this plan
in plans/README.md.

Drift check (run first): git diff --stat 5b5c634..HEAD -- tests/embeddings/ .github/workflows/test.yml justfile pyproject.toml CLAUDE.md AGENTS.md
If any in-scope file changed since this plan was written, compare the
"Current state" excerpts against the live code before proceeding; on a
mismatch, treat it as a STOP condition.

Status

• Priority: P2
• Effort: S
• Risk: LOW
• Depends on: none
• Category: tests
• Planned at: commit 5b5c634, 2026-06-12

Why this matters

CI deselects every async test with -m "not asyncio" — 13 tests covering the
embedding manager's cloud/local/hybrid mode selection never run on any PR.
The documented reason ("missing pytest-asyncio configuration") is stale:
pytest-asyncio is installed and works. The real blocker, verified by running
them at the planning commit, is that 4 of the 13 fail from test/mock drift
— the EmbeddingManager was refactored (batching work landed in commits
like 33e6281 and 5eb204e) and the tests still mock methods the manager no
longer calls. Until this is fixed, regressions in embedding mode fallback —
the thing privacy mode (--local) depends on — are invisible to CI.

Current state

• Verified at the planning commit:
  • uv run pytest tests/ -m "not asyncio" -q → 289 passed, 13 deselected.
  • uv run pytest tests/ -m asyncio -q → 9 passed, 4 FAILED:
    • tests/embeddings/test_manager.py::TestEmbeddingManager::test_embed_text_local_mode
    • ...::test_embed_text_cloud_mode
    • ...::test_embed_text_hybrid_mode_success
    • ...::test_embed_text_hybrid_mode_fallback
  • Failure shapes: AssertionError: assert <AsyncMock ...> == [0.7, 0.8, 0.9]
    (an awaited value is a leaked AsyncMock — the test mocked the wrong
    method) and Expected 'embed_single' to be called once. Called 0 times
    (the manager no longer calls embed_single on that path).
• src/utils/embeddings/manager.py (453 lines) — the code under test.
  embed_text likely now routes through a batch path or an LRU cache layer;
  read the current embed_text / provider dispatch implementation to learn
  what the manager actually awaits on each mode, then fix the tests' mocks to
  match current behavior.
• tests/embeddings/test_manager.py — the failing tests; other passing async
  tests in the same file show working patterns.
• .github/workflows/test.yml (test job, last step):
  uv run pytest tests/ -v -m "not asyncio" --tb=short.
• justfile ci recipe mirrors it:
  uv run pytest tests/ -v -m "not asyncio" --tb=short.
• pyproject.toml [tool.pytest.ini_options] contains only testpaths and
  python_files — no pytest-asyncio settings, which is why every run prints
  a PytestDeprecationWarning about asyncio_default_fixture_loop_scope.
• CLAUDE.md ("Testing" section): "Async tests (@pytest.mark.asyncio) are
  excluded in CI due to a missing pytest-asyncio configuration." AGENTS.md
  contains similar text (grep for asyncio there).
• Convention (from repo memory/CLAUDE.md): when patching functions that are
  lazily imported inside a function body, patch at the source module;
  for top-level imports, patch the importing module's namespace.

Commands you will need

Purpose	Command	Expected on success
Async tests only	uv run pytest tests/ -m asyncio -v	13 passed
Full suite, no marker filter	uv run pytest tests/ -v --tb=short	302 passed
Lint+format	just format && just lint	exit 0
Full CI parity	just ci	exit 0

Scope

In scope (the only files you should modify):

• tests/embeddings/test_manager.py
• pyproject.toml (pytest config block only)
• .github/workflows/test.yml
• justfile (ci recipe only)
• CLAUDE.md, AGENTS.md (the stale async-exclusion note)

Out of scope (do NOT touch):

• src/utils/embeddings/manager.py and the provider modules — this plan
  fixes tests, not the manager. If the manager itself is buggy, that is a
  STOP condition, not a fix-it-here.
• Other test files.

Git workflow

• Branch: advisor/005-async-tests-in-ci
• Commit style: imperative subject, backticks around code identifiers.
• Do NOT push or open a PR unless the operator instructed it.

Steps

Step 1: Configure pytest-asyncio explicitly

In pyproject.toml [tool.pytest.ini_options], add:

asyncio_default_fixture_loop_scope = "function"

(The tests use explicit @pytest.mark.asyncio markers, so the default
strict mode is fine — do not set asyncio_mode = "auto".)

Verify: uv run pytest tests/ -m asyncio -q 2>&1 | grep -c PytestDeprecationWarning → 0

Step 2: Fix the four drifted tests

For each failing test, read the current code path in
src/utils/embeddings/manager.py (start from embed_text and follow the
mode dispatch), then update the test's mocks and assertions to target the
methods the manager calls today. Typical fixes: mock the provider's
batch method instead of embed_single; make mocked async methods return
real values (AsyncMock(return_value=[0.7, 0.8, 0.9])) instead of leaving
bare AsyncMocks to leak into assertions. Preserve each test's intent
(local mode uses the local provider; cloud mode the cloud provider; hybrid
prefers local and falls back to cloud on failure) — only the mechanics
change.

Verify: uv run pytest tests/ -m asyncio -v → 13 passed, 0 failed

Step 3: Remove the CI exclusion

• .github/workflows/test.yml: change the test step to
  uv run pytest tests/ -v --tb=short.
• justfile ci recipe: same change (keep the two ruff lines untouched).

Verify: just ci → exit 0, output shows 302 passed (289 + 13), 0 deselected

Step 4: Update the docs that codified the workaround

• CLAUDE.md: replace the "Async tests ... are excluded in CI" sentence
  with: "Async tests (@pytest.mark.asyncio) run in CI like everything else
  (pytest-asyncio is configured in pyproject.toml)."
• AGENTS.md: find the equivalent sentence (grep -n "asyncio" AGENTS.md)
  and make the same correction.

Verify: grep -rn "not asyncio" CLAUDE.md AGENTS.md .github/ justfile → no matches

Test plan

No new tests — this plan repairs 4 existing tests and promotes 13 into CI.
The full-suite count moving from 289 to 302 is the verification.

Done criteria

• uv run pytest tests/ -v --tb=short → 302 passed, 0 failed, 0 deselected
• just ci exits 0
• grep -rn '"not asyncio"' .github/ justfile → no matches
• No PytestDeprecationWarning about asyncio fixture loop scope in test output
• No files outside the in-scope list modified (git status)
• plans/README.md status row updated

STOP conditions

Stop and report back (do not improvise) if:

• While fixing a test you conclude the manager behavior is wrong (e.g.
  hybrid mode does not actually fall back to cloud on local failure). That is
  a product bug — report it with the evidence; do not change
  src/utils/embeddings/manager.py under this plan.
• The 4 tests fail for a reason other than mock drift (e.g. they need a
  network call or the optional sentence-transformers extra installed) —
  report; the fix may be a skip-if-missing-extra marker, which needs a
  decision.
• After Step 3, CI-parity run flakes intermittently (async timing) — report
  the flaky test rather than adding retries.

Maintenance notes

• Future embedding-manager refactors must update these tests in the same PR —
  they are now load-bearing in CI instead of silently skipped.
• Reviewer should scrutinize: that the fixed tests still assert provider
  selection (the mode logic), not just "some method was called".
• The asyncio marker filter is gone; if a future test genuinely cannot run
  in CI, use an explicit @pytest.mark.skipif with a reason string, not a
  marker exclusion in the CI command.