fix: heid-code-review fixups — donut voiced-interview slices 2+3
Triaged the heid-code-review panel (3 arms; reconciled against 56dce00 — three
findings already closed by the bug-hunt, and the two firewalled lenses converged
independently on the same three defects). Fixed the real one + contract precision.
Code:
- kb_bridge: no-hit sentinel (F7, the sharpest solo). The consult prompt asks Mimir
to emit NO_CORPUS_MATCH when nothing is relevant; pin_kb_context drops any extract
containing it -> [], so a non-empty "no results found" answer is never pinned as the
character's own memory. Live-proven: a grounding query pins (5.6s), a nonsense query
-> [] (0.7s); the sentinel is artificial so `in` can't false-positive on a passage.
Contract (the CODE is correct; the spec had drifted/undercounted — kept canonical):
- DEC-8: the custom "donut" voice was registered EARLY (verified live), so mapping
ratatoskr:donut -> "donut" is right; "preset now" was stale. A live gateway read
INVERTED the 3-arm remedy (reverting to a preset would have been the regression).
- FN tts_synthesize: declared the `url` swap-seam param (F3); voice membership is
gateway-enforced not client-asserted (F2); the postcondition is a container-level
RIFF/WAVE check, not a 16-bit-PCM fmt parse.
- FN tts_endpoint: pad is BROWSER-SENT per DEC-7, not a server PAD lookup (F5);
documented the 413 text cap.
- FN pin_kb_context: documented the sentinel + the session-delete hygiene.
Tests (real coverage gaps):
- the read_note prompt test asserts the distinguishing "do NOT call read_note" phrase,
not the bare token an inverted prompt would also carry (#8 mutation-blind).
- extract-bound asserts the literal 2000, not the impl constant it slices by (#9).
- route roster asserts /api/tts + /api/sessions/{id}/messages (#10 undercount).
- new server test: a degraded KB consult ([]) still streams the turn to done (F9).
Accepted (not fixed): caller-supplied agent_id (LAN/no-auth debug-tool trust model);
no DEC-5 concurrency test (asyncio.Lock is trivially correct — a test would test
asyncio, not our code). 545 green.
This commit is contained in:
+15
-3
@@ -112,22 +112,34 @@ class TestConsult:
|
||||
create_body = fake.calls[0][1]
|
||||
assert create_body["agent_id"] == "mimir"
|
||||
stream_prompt = fake.calls[1][1][1] # ("stream_turn", (sid, prompt), {})
|
||||
# The foot-gun mitigation: EXACTLY one search_library call, no read_note.
|
||||
# The foot-gun mitigation: EXACTLY one search_library call, and read_note FORBIDDEN.
|
||||
# Assert the distinguishing phrase, not the bare token "read_note" — a mitigation-
|
||||
# inverted prompt ("DO call read_note") would still contain the token (heid-code-review).
|
||||
assert "EXACTLY ONCE" in stream_prompt
|
||||
assert "read_note" in stream_prompt # forbids the loop tool
|
||||
assert "do NOT call read_note" in stream_prompt # forbids the loop tool (negation)
|
||||
assert "Dungeon Crawler Carl" in stream_prompt # corpus scoping
|
||||
assert "who is Princess Donut?" in stream_prompt # the question
|
||||
assert "NO_CORPUS_MATCH" in stream_prompt # the no-hit sentinel instruction
|
||||
|
||||
async def test_empty_consult_returns_empty(self) -> None:
|
||||
fake = _FakeSessions(events=[_text(" "), _done(" ")])
|
||||
out = await pin_kb_context("q", _DONUT, client=_FakeClient(fake))
|
||||
assert out == []
|
||||
|
||||
async def test_no_match_sentinel_pins_nothing(self) -> None:
|
||||
# A non-empty "no results" answer carrying the sentinel must NOT be pinned as
|
||||
# corpus memory (heid-code-review F7) — even wrapped in other prose.
|
||||
fake = _FakeSessions(events=[_done("I searched but found NO_CORPUS_MATCH here.")])
|
||||
out = await pin_kb_context("q", _DONUT, client=_FakeClient(fake))
|
||||
assert out == []
|
||||
|
||||
async def test_extract_bounded_to_max_chars(self) -> None:
|
||||
long = "x" * 5000
|
||||
fake = _FakeSessions(events=[_done(long)])
|
||||
out = await pin_kb_context("q", _DONUT, client=_FakeClient(fake))
|
||||
assert len(out[0]["text"]) == kb_bridge._MAX_EXTRACT_CHARS
|
||||
# Assert the concrete bound (2000), not the impl constant it's sliced by — a
|
||||
# mutated constant must fail this (heid-code-review: non-circular assertion).
|
||||
assert len(out[0]["text"]) == 2000
|
||||
|
||||
|
||||
class TestDegradation:
|
||||
|
||||
@@ -764,8 +764,10 @@ class TestCreateAppShape:
|
||||
"/api/memory/chunks",
|
||||
# v0.19.2 debug-surface parity (create_app POST-002)
|
||||
"/api/sessions/{session_id}/tools",
|
||||
"/api/sessions/{session_id}/messages",
|
||||
"/api/sessions/{session_id}/bifrost",
|
||||
"/api/admin/events",
|
||||
"/api/tts",
|
||||
"/api/turns/{session_id}", "/api/turns/{session_id}/stream",
|
||||
"/api/turns/{session_id}/cancel",
|
||||
):
|
||||
@@ -1550,8 +1552,7 @@ class TestTtsEndpoint:
|
||||
assert json.loads(route.calls.last.request.content)["voice"] == "Cora"
|
||||
|
||||
def test_text_too_large_returns_413(self) -> None:
|
||||
from ratatoskr.web.server import create_app
|
||||
from ratatoskr.web.server import _TTS_MAX_TEXT_CHARS
|
||||
from ratatoskr.web.server import _TTS_MAX_TEXT_CHARS, create_app
|
||||
|
||||
app = create_app(_mock_client_factory(), tts_url=self._TTS)
|
||||
resp = TestClient(app).post(
|
||||
@@ -1628,3 +1629,31 @@ class TestKbBridgeWiring:
|
||||
b"".join(resp.iter_bytes())
|
||||
body = json.loads(route.calls.last.request.content)
|
||||
assert "memory_context" not in body or body["memory_context"] is None
|
||||
|
||||
@respx.mock
|
||||
def test_failed_consult_still_streams_the_turn(self, monkeypatch) -> None:
|
||||
# A degraded/failed KB consult (returns []) must NOT block or fail the character
|
||||
# turn — it streams to `done` with no pinned context (INV-KB-3, server-level;
|
||||
# heid-code-review F9: the turn-path failure seam was untested end to end).
|
||||
from ratatoskr.web import server
|
||||
from ratatoskr.web.server import create_app
|
||||
|
||||
async def _degraded_pin(question, agent_id, *, client):
|
||||
return [] # consult failed/timed-out internally → degraded to []
|
||||
|
||||
monkeypatch.setattr(server, "pin_kb_context", _degraded_pin)
|
||||
respx.post("https://w.example/sessions/s-1/messages").mock(
|
||||
return_value=_sse_resp(
|
||||
_sse_chunk("42:1", {"type": "text", "content": "hi"})
|
||||
+ _sse_chunk("42:2", _DONE_BODY)
|
||||
)
|
||||
)
|
||||
c = TestClient(create_app(_mock_client_factory()))
|
||||
tid = c.post(
|
||||
"/api/turns/s-1",
|
||||
json={"content": "who is Carl?", "agent_id": "ratatoskr:donut"},
|
||||
).json()["turn_id"]
|
||||
with c.stream("GET", f"/api/turns/s-1/stream?turn_id={tid}") as resp:
|
||||
raw = b"".join(resp.iter_bytes())
|
||||
events = [e["event"] for e in _parse_browser_sse(raw)]
|
||||
assert "text" in events and "done" in events # the turn completed normally
|
||||
|
||||
Reference in New Issue
Block a user