Skip to content
This repository was archived by the owner on Mar 2, 2026. It is now read-only.

fix: mock dedup check in existing compilation tests#584

Merged
zonk1024 merged 1 commit intomainfrom
fix/582-test-dedup-mock
Feb 28, 2026
Merged

fix: mock dedup check in existing compilation tests#584
zonk1024 merged 1 commit intomainfrom
fix/582-test-dedup-mock

Conversation

@zonk1024
Copy link
Contributor

Problem

PR #582 added _find_similar_article() inside compile_article(), which calls generate_embedding() — a real API call. The existing tests in tests/core/test_compilation.py had no mock for this path, so they would fail in CI environments without a live OpenAI key (currently passing only because the exception is silently swallowed by the try/except guard in _find_similar_article).

Fix

Added a single module-level autouse fixture in tests/core/test_compilation.py:

@pytest.fixture(autouse=True)
def _no_dedup(monkeypatch):
    """Disable dedup check in compilation tests (tested separately in test_compilation_dedup.py)."""
    async def _no_match(*args, **kwargs):
        return None
    monkeypatch.setattr("valence.core.compilation._find_similar_article", _no_match)

This patches out _find_similar_article for every test in the file, making the intent explicit and removing the silent dependency on error-swallowing behaviour. Dedup logic is already covered by test_compilation_dedup.py.

Verification

python -m pytest tests/core/test_compilation.py -q   # 55 passed
python -m pytest tests/ -q                           # 1713 passed, 10 skipped

Changes

  • tests/core/test_compilation.py — add _no_dedup autouse fixture (+21 lines, no test logic changed)

Copilot AI review requested due to automatic review settings February 28, 2026 19:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the existing compilation tests independent of the dedup-on-compile path introduced in PR #582 by globally patching _find_similar_article() in tests/core/test_compilation.py, preventing real embedding API calls during this test suite.

Changes:

  • Add a module-level autouse fixture that monkeypatches valence.core.compilation._find_similar_article to always return None.
  • Document why dedup is suppressed in these tests and point to test_compilation_dedup.py for dedup coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +29 to 40
@pytest.fixture(autouse=True)
def _no_dedup(monkeypatch):
"""Disable dedup check in compilation tests (tested separately in test_compilation_dedup.py)."""

async def _no_match(*args, **kwargs):
return None

monkeypatch.setattr("valence.core.compilation._find_similar_article", _no_match)


from valence.core.compilation import (
DEFAULT_PROMPT_LIMITS,
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from valence.core.compilation import (...) is placed after executable code (the autouse fixture), which triggers Ruff/pycodestyle E402 (module level import not at top of file) and will fail CI. Please move this import block up with the other imports (above the fixture), or add an explicit # noqa: E402 (reordering is preferable).

Copilot uses AI. Check for mistakes.
PR #582 added _find_similar_article() inside compile_article(), which
calls generate_embedding() and makes a real API call.  Add an autouse
fixture that patches _find_similar_article to return None so existing
tests keep testing compilation logic without a live embedding API.

Dedup behaviour is exercised by test_compilation_dedup.py.
@zonk1024 zonk1024 force-pushed the fix/582-test-dedup-mock branch from 1a67f3d to 486e3df Compare February 28, 2026 19:51
@zonk1024 zonk1024 merged commit e701b78 into main Feb 28, 2026
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants