fix(P05): verifier P0s — browser descriptor served, 409 post-finish, 422 empty audio, verdict persisted
Four gaps found by independent verifier probing of the defense endpoints (all Must-Have-relevant, all trivially fixed): 1. Browser-mode descriptor was dead code: BROWSER_FALLBACK_DESCRIPTOR existed but start always returned mode='mock' even with AI_VOICE_PROVIDER=browser (Must-Have #6 violated). start now derives the descriptor from settings.voice_provider (D-030). 2. answer after finish returned 200 and appended turns to a sealed transcript — the store explicitly assigns sequencing to the endpoints (defense_store.py: 'turns after finalize are a sequencing bug for the endpoints to prevent, task 5-3-01'); the endpoints didn't. Now 409. 3. Zero-byte audio upload crashed the mock provider (MockVoiceFailure -> 500); a real provider would 500 the same way. Empty upload is a client error: 422 before any provider call (provider contract unchanged). 4. Verdict was NOT persisted (Must-Have #1 'verdict + transcript persisted'): finish persisted only signals; GET after finish could not re-serve the verdict. The verdict now nests in integrity_signals (JSON-object dict per the DefenseStore.finalize contract). 3 regression tests added (empty-audio 422, post-finish 409, verdict retrievable from GET; browser-descriptor test). Suite 386 green; ruff clean. ---ci--- phase: 5 milestone: v0.3 status: verify requirements: covered: [REQ-3-006] partial: [] lessons: - A descriptor that exists but is never served is indistinguishable from dead code until you probe the configured mode end-to-end (factory tests proved selection, not service). - Store contracts that 'assign' sequencing to callers need an endpoint test for the forbidden transition, or the assignment is decorative. ---/ci---
This commit is contained in:
@@ -31,6 +31,7 @@ from ..agents.examiner import ExaminerAgent
|
|||||||
from ..grading.features import TraceDigest, compute_digest
|
from ..grading.features import TraceDigest, compute_digest
|
||||||
from ..llm.types import Message
|
from ..llm.types import Message
|
||||||
from ..voice.base import VoiceDescriptor
|
from ..voice.base import VoiceDescriptor
|
||||||
|
from ..voice.browser import BROWSER_FALLBACK_DESCRIPTOR
|
||||||
from ..voice.defense_store import DefenseRecord, DefenseStore, DefenseTurn
|
from ..voice.defense_store import DefenseRecord, DefenseStore, DefenseTurn
|
||||||
from .deps import (
|
from .deps import (
|
||||||
get_examiner,
|
get_examiner,
|
||||||
@@ -88,6 +89,21 @@ async def _digest_for_task(
|
|||||||
return (compute_digest(trace) if trace else None), (len(gaps) == 0)
|
return (compute_digest(trace) if trace else None), (len(gaps) == 0)
|
||||||
|
|
||||||
|
|
||||||
|
def _voice_descriptor(settings) -> VoiceDescriptor:
|
||||||
|
"""The capability descriptor for the configured voice mode (D-030).
|
||||||
|
|
||||||
|
Must-Have #6: browser mode returns BROWSER_FALLBACK_DESCRIPTOR so the
|
||||||
|
web client selects native SpeechRecognition/speechSynthesis; mock mode
|
||||||
|
returns the mock descriptor. (A v0.4 server provider would return
|
||||||
|
mode="server" — the protocol seam.)
|
||||||
|
"""
|
||||||
|
if (settings.voice_provider or "mock").strip().lower() == "browser":
|
||||||
|
return BROWSER_FALLBACK_DESCRIPTOR
|
||||||
|
return VoiceDescriptor(
|
||||||
|
mode="mock", sr_available=True, tts_available=True, hint=""
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.post("/start", response_model=StartResponse)
|
@router.post("/start", response_model=StartResponse)
|
||||||
async def start_defense(
|
async def start_defense(
|
||||||
body: StartRequest,
|
body: StartRequest,
|
||||||
@@ -128,8 +144,8 @@ async def start_defense(
|
|||||||
created_at=datetime.now(UTC),
|
created_at=datetime.now(UTC),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
descriptor = getattr(voice_provider, "descriptor", None) or VoiceDescriptor(
|
descriptor = getattr(voice_provider, "descriptor", None) or _voice_descriptor(
|
||||||
mode="mock", sr_available=True, tts_available=True
|
settings
|
||||||
)
|
)
|
||||||
return StartResponse(
|
return StartResponse(
|
||||||
defense_id=record.id,
|
defense_id=record.id,
|
||||||
@@ -153,6 +169,14 @@ async def answer_defense(
|
|||||||
record = voice_store.get(defense_id)
|
record = voice_store.get(defense_id)
|
||||||
if record is None:
|
if record is None:
|
||||||
raise HTTPException(status_code=404, detail=f"no defense {defense_id!r}")
|
raise HTTPException(status_code=404, detail=f"no defense {defense_id!r}")
|
||||||
|
if record.status == "finished":
|
||||||
|
# The store owns the finished transition but does NOT police turn
|
||||||
|
# sequencing (defense_store.py: "turns after finalize are a sequencing
|
||||||
|
# bug for the endpoints to prevent") — this is the endpoint half of
|
||||||
|
# that contract: a sealed transcript is append-only-no-more.
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=409, detail="defense is finished; start a new defense"
|
||||||
|
)
|
||||||
if text is None and audio is None:
|
if text is None and audio is None:
|
||||||
raise HTTPException(status_code=422, detail="provide {text} or audio")
|
raise HTTPException(status_code=422, detail="provide {text} or audio")
|
||||||
|
|
||||||
@@ -161,6 +185,11 @@ async def answer_defense(
|
|||||||
if audio is not None:
|
if audio is not None:
|
||||||
stt_started = time.perf_counter()
|
stt_started = time.perf_counter()
|
||||||
raw = await audio.read()
|
raw = await audio.read()
|
||||||
|
if not raw:
|
||||||
|
# Empty upload is a client error (422), not a provider crash
|
||||||
|
# (500): validate before the provider call so every provider —
|
||||||
|
# mock today, the v0.4 real one — sees the same contract.
|
||||||
|
raise HTTPException(status_code=422, detail="audio upload is empty")
|
||||||
fmt = (audio.content_type or "audio/wav").split("/")[-1]
|
fmt = (audio.content_type or "audio/wav").split("/")[-1]
|
||||||
segment = await voice_provider.transcribe(raw, fmt)
|
segment = await voice_provider.transcribe(raw, fmt)
|
||||||
stt_ms = int((time.perf_counter() - stt_started) * 1000)
|
stt_ms = int((time.perf_counter() - stt_started) * 1000)
|
||||||
@@ -268,6 +297,13 @@ async def finish_defense(
|
|||||||
if t.role == _ROLE_LEARNER and (t.latency_ms or 0) > PAUSE_THRESHOLD_MS
|
if t.role == _ROLE_LEARNER and (t.latency_ms or 0) > PAUSE_THRESHOLD_MS
|
||||||
],
|
],
|
||||||
"pause_threshold_ms": PAUSE_THRESHOLD_MS,
|
"pause_threshold_ms": PAUSE_THRESHOLD_MS,
|
||||||
|
# Must-Have #1: "verdict + transcript persisted" — the verdict is
|
||||||
|
# stored INSIDE integrity_signals so GET /{id} after finish can
|
||||||
|
# re-serve it (the finish response alone would lose it). Signals
|
||||||
|
# are a JSON object dict (DefenseStore.finalize contract), so the
|
||||||
|
# verdict nests under the "verdict" key alongside the A-109
|
||||||
|
# markers the Proctor/Mentor feeds read.
|
||||||
|
"verdict": verdict.model_dump(),
|
||||||
}
|
}
|
||||||
voice_store.finalize(defense_id, signals)
|
voice_store.finalize(defense_id, signals)
|
||||||
return FinishResponse(verdict=verdict.model_dump(), integrity_signals=signals)
|
return FinishResponse(verdict=verdict.model_dump(), integrity_signals=signals)
|
||||||
|
|||||||
@@ -95,6 +95,29 @@ class TestStart:
|
|||||||
assert body["trace_complete"] is True
|
assert body["trace_complete"] is True
|
||||||
|
|
||||||
|
|
||||||
|
class TestBrowserFallback:
|
||||||
|
def test_browser_mode_serves_browser_descriptor(self, tmp_path: Path) -> None:
|
||||||
|
"""Must-Have #6: AI_VOICE_PROVIDER=browser → start returns the
|
||||||
|
browser-native SR/TTS fallback descriptor (D-030), not 'mock'."""
|
||||||
|
application = create_app(Settings(provider="mock", voice_provider="browser"))
|
||||||
|
application.state.provider = ScriptedLLM()
|
||||||
|
application.state.trace_store = SQLiteTraceStore(db_path=tmp_path / "t.db")
|
||||||
|
application.state.variant_store = SQLiteVariantStore(db_path=tmp_path / "v.db")
|
||||||
|
application.state.grade_store = SQLiteGradeStore(db_path=tmp_path / "g.db")
|
||||||
|
application.state.trace_integrity = TraceIntegrityMap()
|
||||||
|
application.state.defense_store = SQLiteDefenseStore(db_path=tmp_path / "d.db")
|
||||||
|
application.state.voice_provider = MockVoiceProvider(["answer"])
|
||||||
|
llm = ScriptedLLM()
|
||||||
|
application.state.examiner_agent = ExaminerAgent(
|
||||||
|
llm, Settings(provider="mock", voice_provider="browser")
|
||||||
|
)
|
||||||
|
with TestClient(application) as c:
|
||||||
|
body = _start(c)
|
||||||
|
assert body["voice_descriptor"]["mode"] == "browser"
|
||||||
|
assert body["voice_descriptor"]["sr_available"]
|
||||||
|
assert "SpeechRecognition" in body["voice_descriptor"]["hint"]
|
||||||
|
|
||||||
|
|
||||||
class TestAnswer:
|
class TestAnswer:
|
||||||
def test_typed_answer_yields_followup_with_latency(self, client) -> None:
|
def test_typed_answer_yields_followup_with_latency(self, client) -> None:
|
||||||
defense_id = _start(client)["defense_id"]
|
defense_id = _start(client)["defense_id"]
|
||||||
@@ -119,6 +142,29 @@ class TestAnswer:
|
|||||||
assert learner_turns, "learner turn missing after audio answer"
|
assert learner_turns, "learner turn missing after audio answer"
|
||||||
assert learner_turns[0]["text"] == "the fix was in the retry loop"
|
assert learner_turns[0]["text"] == "the fix was in the retry loop"
|
||||||
|
|
||||||
|
def test_empty_audio_is_422_not_500(self, client) -> None:
|
||||||
|
"""Zero-byte upload must 422 before the provider call (a real
|
||||||
|
provider would raise the same way the mock does — validate first)."""
|
||||||
|
defense_id = _start(client)["defense_id"]
|
||||||
|
resp = client.post(
|
||||||
|
f"/v1/defense/{defense_id}/answer",
|
||||||
|
files={"audio": ("answer.wav", b"", "audio/wav")},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 422, resp.text
|
||||||
|
stored = client.get(f"/v1/defense/{defense_id}").json()
|
||||||
|
assert len(stored["turns"]) == 1 # nothing appended
|
||||||
|
|
||||||
|
def test_answer_after_finish_is_409(self, client) -> None:
|
||||||
|
"""A sealed transcript is append-only-no-more: the endpoints own
|
||||||
|
turn-vs-finalize sequencing (defense_store contract)."""
|
||||||
|
defense_id = _start(client)["defense_id"]
|
||||||
|
client.post(f"/v1/defense/{defense_id}/answer", data={"text": "a"})
|
||||||
|
assert client.post(f"/v1/defense/{defense_id}/finish").status_code == 200
|
||||||
|
resp = client.post(f"/v1/defense/{defense_id}/answer", data={"text": "late"})
|
||||||
|
assert resp.status_code == 409, resp.text
|
||||||
|
stored = client.get(f"/v1/defense/{defense_id}").json()
|
||||||
|
assert len(stored["turns"]) == 3 # ex, lrn, ex — no post-finish turns
|
||||||
|
|
||||||
def test_neither_text_nor_audio_422(self, client) -> None:
|
def test_neither_text_nor_audio_422(self, client) -> None:
|
||||||
defense_id = _start(client)["defense_id"]
|
defense_id = _start(client)["defense_id"]
|
||||||
resp = client.post(f"/v1/defense/{defense_id}/answer")
|
resp = client.post(f"/v1/defense/{defense_id}/answer")
|
||||||
@@ -154,6 +200,9 @@ class TestFinishAndGet:
|
|||||||
stored = client.get(f"/v1/defense/{defense_id}").json()
|
stored = client.get(f"/v1/defense/{defense_id}").json()
|
||||||
assert stored["status"] == "finished"
|
assert stored["status"] == "finished"
|
||||||
assert stored["integrity_signals"]
|
assert stored["integrity_signals"]
|
||||||
|
# Must-Have #1: "verdict + transcript persisted" — the verdict must
|
||||||
|
# be retrievable from GET after finish, not only in the finish body.
|
||||||
|
assert stored["integrity_signals"]["verdict"]["verdict"] == "developing"
|
||||||
|
|
||||||
def test_long_pause_flagged(self, client, app) -> None:
|
def test_long_pause_flagged(self, client, app) -> None:
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime
|
||||||
|
|||||||
Reference in New Issue
Block a user