gpt-5.3 codex review: Phase 7 and the addition of the new test-effectiveness-auditor skill.

tests/services/test_service_base.py

@@ -1,36 +1,137 @@
+from __future__ import annotations
+
+from contextlib import asynccontextmanager
+
 import pytest
 
+from transcription.config import Settings
+from transcription.services.base import ServiceBase
 
-class TestServiceBase:
-    class TestInitialization:
-        def test_initializes_with_defaults(self):
-            """Test initialization with default session factory and queue."""
 
-        def test_initializes_with_custom_session_factory(self):
-            """Test initialization with a provided session factory."""
+class _TrackingSession:
+    def __init__(self) -> None:
+        self.commits = 0
+        self.flushes = 0
+        self.refreshed: list[object] = []
 
-        def test_initializes_with_custom_queue(self):
-            """Test initialization with a provided queue."""
+    async def commit(self) -> None:
+        self.commits += 1
 
-    class TestSessionScope:
-        @pytest.mark.asyncio
-        async def test_uses_provided_session(self):
-            """Test that session scope reuses a provided session."""
+    async def flush(self) -> None:
+        self.flushes += 1
 
-        @pytest.mark.asyncio
-        async def test_creates_new_session_when_none_provided(self):
-            """Test that session scope creates a new session when none is provided."""
+    async def refresh(self, obj: object) -> None:
+        self.refreshed.append(obj)
 
-    class TestContextManagerBehavior:
-        @pytest.mark.asyncio
-        async def test_yields_session(self):
-            """Test that session scope yields a usable session object."""
 
-        @pytest.mark.asyncio
-        async def test_multiple_operations(self):
-            """Test multiple operations within a single session scope."""
+def _settings() -> Settings:
+    return Settings(_env_file=None, openrouter_api_key="test-key")
 
-    class TestEdgeCases:
-        @pytest.mark.asyncio
-        async def test_handles_exception_propagation(self):
-            """Test exception propagation behavior inside session scope."""
+
+def test_initializes_with_defaults(monkeypatch):
+    settings = _settings()
+    session_factory = object()
+
+    monkeypatch.setattr("transcription.services.base.get_settings", lambda: settings)
+    monkeypatch.setattr(
+        "transcription.services.base.resolve_session_factory",
+        lambda **kwargs: session_factory,
+    )
+
+    service = ServiceBase()
+
+    assert service.settings is settings
+    assert service.session_factory is session_factory
+
+
+def test_initializes_with_custom_session_factory():
+    settings = _settings()
+    session_factory = object()
+
+    service = ServiceBase(settings=settings, session_factory=session_factory)
+
+    assert service.settings is settings
+    assert service.session_factory is session_factory
+
+
+@pytest.mark.asyncio
+async def test_session_scope_reuses_provided_session(monkeypatch):
+    captured: dict[str, object | None] = {}
+    provided_session = object()
+    yielded = object()
+    service = ServiceBase(settings=_settings(), session_factory=object())
+
+    @asynccontextmanager
+    async def _fake_session_scope(*, session_factory=None, session=None):
+        captured["session_factory"] = session_factory
+        captured["session"] = session
+        yield yielded if session is None else session
+
+    monkeypatch.setattr("transcription.services.base.session_scope", _fake_session_scope)
+
+    async with service._session_scope(session=provided_session) as active:
+        assert active is provided_session
+
+    assert captured == {"session_factory": service.session_factory, "session": provided_session}
+
+
+@pytest.mark.asyncio
+async def test_session_scope_creates_owned_session_when_none_provided(monkeypatch):
+    captured: dict[str, object | None] = {}
+    owned_session = object()
+    service = ServiceBase(settings=_settings(), session_factory=object())
+
+    @asynccontextmanager
+    async def _fake_session_scope(*, session_factory=None, session=None):
+        captured["session_factory"] = session_factory
+        captured["session"] = session
+        yield owned_session
+
+    monkeypatch.setattr("transcription.services.base.session_scope", _fake_session_scope)
+
+    async with service._session_scope() as active:
+        assert active is owned_session
+
+    assert captured == {"session_factory": service.session_factory, "session": None}
+
+
+@pytest.mark.asyncio
+async def test_session_scope_propagates_exceptions(monkeypatch):
+    service = ServiceBase(settings=_settings(), session_factory=object())
+
+    @asynccontextmanager
+    async def _fake_session_scope(*, session_factory=None, session=None):
+        _ = (session_factory, session)
+        yield object()
+
+    monkeypatch.setattr("transcription.services.base.session_scope", _fake_session_scope)
+
+    with pytest.raises(RuntimeError, match="boom"):
+        async with service._session_scope():
+            raise RuntimeError("boom")
+
+
+@pytest.mark.asyncio
+async def test_finalize_commits_for_service_owned_session():
+    service = ServiceBase(settings=_settings(), session_factory=object())
+    session = _TrackingSession()
+    refreshed = object()
+
+    await service._finalize(session=session, caller_session=None, refresh=(refreshed,))
+
+    assert session.commits == 1
+    assert session.flushes == 0
+    assert session.refreshed == [refreshed]
+
+
+@pytest.mark.asyncio
+async def test_finalize_flushes_for_caller_owned_session():
+    service = ServiceBase(settings=_settings(), session_factory=object())
+    session = _TrackingSession()
+    refreshed = object()
+
+    await service._finalize(session=session, caller_session=object(), refresh=(refreshed,))
+
+    assert session.commits == 0
+    assert session.flushes == 1
+    assert session.refreshed == [refreshed]

tests/services/test_v45_candidates.py

@@ -87,11 +87,15 @@ async def test_promotion_rejects_unrelated_attempt(default_session_factory):
     services = _services(default_session_factory, settings)
     source = await _seed_source(services)
 
-    with pytest.raises(CandidatePromotionError):
+    with pytest.raises(CandidatePromotionError) as exc_info:
         await services.evidence.promote_machine_attempt(
             source_id=source.id,
             execution_attempt_id=uuid4(),
         )
+    error = exc_info.value
+    assert error.category.value == "validation_error"
+    assert "successful transcription attempt" in error.message
+    assert "Select an available successful candidate" in error.suggestion
 
 
 @pytest.mark.integration