All verification complete. Final report: **Phase 120 — Failed-turn retry: verification pass (all 3 tasks were done; final verification + 1 regression fix)** **Verified:** `ChatMessage.failed`/`error` (≤500, `extra="forbid"` intact); `finalizeFailedTurn` funnel on the 3 failure paths (catch-else, stream-drop guard, zero-frame fallback) with `failed: true` + capped detail + `markLastRetryable`; `appendFailedNote` restore branch (Save-as-doc/Tune excluded); `showErrorBanner`/`retryLastTurn` byte-pinned untouched; only the three paths persist `failed: true` (grep + unit pin); no test asserts the old broken behavior. **Defect found & fixed (rule 7):** a real navigate-away mid-turn let the browser's teardown fetch rejection (TypeError, not AbortError) leak into the failed funnel, persisting a phantom failed brain record — `test_sources_midstream_bug.py::test_no_orphan_brain_message_when_navigated_before_first_token` failed (2 `.msg` after reload) and violated the phase-20 navigate-away convention. Fixed: turn-scoped `leftThePage` flag (set unconditionally on `pagehide`, reset in `runTurn`) skips the funnel in the catch-else branch; pinned by new unit test `test_navigate_away_is_not_a_failed_turn`. No phase-overview/PLAN/todo/complete files touched; no commits made. **Gates (exact):** - `uv run pytest` → 2577 passed - `uv run pytest --cov=app --cov-report=term-missing` → TOTAL 4271 stmts, 99% (>90%) - `uv run pytest tests/e2e/test_failed_turn_retry.py -v --no-cov` → 4 passed (isolated) - `uv run ruff check . && uv run pyright` → clean (0 errors) - Regression E2E, isolated: `test_sources_midstream_bug.py` 6/6 (was 5/6); `test_llm_retry`/`test_tool_scaffolding_guardrails`/`test_stop_generation`/`test_navbar_refresh` 17/17 **Completion criteria:** (1) network error → banner + in-bubble Retry, re-ask without re-typing ✅ (E2E A); (2) refresh restores failed bubble + working Retry, no "new chat" ✅ (E2E C); (3) stopped/successful turns byte-identical ✅ (negative E2E, stop suite, byte-identity units); (4) pytest/coverage/lint/types ✅; (5) commit + phase move — left to the harness per pass rules. **Notable:** deviation = the regression fix above (a navigation is not a failed turn; phase-20 partial-persist convention restored). Next pending phase: `121_git_source_tokens`.
197 lines
8.0 KiB
Python
197 lines
8.0 KiB
Python
"""Unit: the failed-turn schema boundary (phase 120, task 03).
|
|
|
|
Phase 120 (task 01, locked A1) added two OPTIONAL keys to
|
|
``ChatMessage`` (``app/schemas.py``) — ``failed`` (a bool marker) and
|
|
``error`` (the persisted error detail, capped at 500) — the phase-48
|
|
``stopped`` precedent: a FAILED chat turn (network error, SSE ``error``
|
|
frame, stream drop) persists as a BRAIN record
|
|
``{who: "brain", text: <detail or fallback>, failed: true, error:
|
|
<detail>}`` — no separate error table, no new API (``retryLastTurn``'s
|
|
pop-the-last-brain-record logic works on a failed record UNCHANGED).
|
|
|
|
This module pins the boundary the phase plan names:
|
|
|
|
* a failed record (``failed: true`` + ``error``) validates and
|
|
round-trips losslessly;
|
|
* ``error`` of 501 chars 422s at the boundary (the phase-83
|
|
value-bounds style; exactly 500 passes);
|
|
* an unknown key still 422s (``extra="forbid"`` intact — the new keys
|
|
are DECLARED, they did not loosen the boundary);
|
|
* a record WITHOUT the new keys validates, serializes with the new keys
|
|
as explicit nulls, and — on every pre-phase key — is byte-identical
|
|
to the pre-phase-120 stored shape (the phase-50 contract: the server
|
|
stores ``model_dump()`` without ``exclude_none``, so a pre-phase
|
|
round-trip is untouched apart from the two added nulls).
|
|
|
|
House convention: pure schema tests (no DB, no client) — the API-level
|
|
round-trip pins live in ``tests/integration/test_chats_api.py``.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
from pydantic import ValidationError
|
|
|
|
from app.schemas import ChatMessage
|
|
|
|
#: The phase-120 failed record shape (locked A1) — the zero-frame
|
|
#: network-error case exactly as ``finalizeFailedTurn`` persists it.
|
|
FAILED_RECORD: dict = {
|
|
"who": "brain",
|
|
"text": "My answer didn't make it — the connection dropped. Use Retry to ask again.",
|
|
"failed": True,
|
|
"error": "The chat model dropped the connection — try again?",
|
|
}
|
|
|
|
|
|
# ---------- acceptance: the failed record validates + round-trips ----------
|
|
|
|
|
|
def test_failed_record_validates() -> None:
|
|
"""A failed brain record (``failed: true`` + the capped ``error``
|
|
detail) crosses the boundary and the fields survive the trip."""
|
|
msg = ChatMessage.model_validate(FAILED_RECORD)
|
|
assert msg.who == "brain"
|
|
assert msg.text == FAILED_RECORD["text"]
|
|
assert msg.failed is True
|
|
assert msg.error == FAILED_RECORD["error"]
|
|
|
|
|
|
def test_failed_record_round_trips_losslessly() -> None:
|
|
"""The stored shape (``model_dump`` — plain, no ``exclude_none``,
|
|
the phase-50 storage convention) keeps the marker + detail and
|
|
fills the remaining optional keys with explicit nulls (the
|
|
restore path is null-safe)."""
|
|
dumped = ChatMessage.model_validate(FAILED_RECORD).model_dump()
|
|
assert dumped["failed"] is True
|
|
assert dumped["error"] == FAILED_RECORD["error"]
|
|
for key in ("sources", "related", "deflected", "suggestions", "thinking", "tools", "stopped"):
|
|
assert dumped[key] is None, f"{key} must be an explicit null, got {dumped[key]!r}"
|
|
# Re-validate the stored shape — the round-trip is lossless.
|
|
assert ChatMessage.model_validate(dumped).model_dump() == dumped
|
|
|
|
|
|
def test_failed_false_is_an_explicit_marker() -> None:
|
|
"""``failed: false`` is a legal value (a full ``bor.chat.v1`` brain
|
|
record carries the key explicitly — the integration FULL_BRAIN
|
|
round-trip relies on it); it must not be coerced to absent."""
|
|
msg = ChatMessage.model_validate(
|
|
{"who": "brain", "text": "hi", "failed": False, "error": None}
|
|
)
|
|
assert msg.failed is False
|
|
assert msg.error is None
|
|
assert msg.model_dump()["failed"] is False
|
|
|
|
|
|
# ---------- the value bounds: error ≤ 500 (phase-83 style) ----------
|
|
|
|
|
|
def test_error_at_500_chars_passes() -> None:
|
|
"""The bound is inclusive: exactly 500 chars validate."""
|
|
msg = ChatMessage.model_validate(
|
|
{"who": "brain", "text": "hi", "failed": True, "error": "e" * 500}
|
|
)
|
|
assert len(msg.error or "") == 500
|
|
|
|
|
|
def test_error_over_500_chars_is_rejected() -> None:
|
|
"""501 chars 422s at the boundary (the API surfaces this as a 422 —
|
|
the hostile detail string is capped at the schema, the
|
|
``finalizeFailedTurn`` 500-char slice is the UI-side first cut)."""
|
|
with pytest.raises(ValidationError):
|
|
ChatMessage.model_validate(
|
|
{"who": "brain", "text": "hi", "failed": True, "error": "e" * 501}
|
|
)
|
|
|
|
|
|
# ---------- the boundary stays strict: extra="forbid" intact ----------
|
|
|
|
|
|
def test_unknown_key_is_still_rejected() -> None:
|
|
"""``extra="forbid"`` was NOT loosened by the new keys: a stray
|
|
key still rejects at the boundary (the corrupted / HTML-shaped
|
|
payload defense, unchanged)."""
|
|
with pytest.raises(ValidationError):
|
|
ChatMessage.model_validate({"who": "brain", "text": "hi", "foo": 1})
|
|
|
|
|
|
def test_unknown_key_is_rejected_even_with_the_new_keys_present() -> None:
|
|
"""A record carrying the new keys AND an unknown key still
|
|
rejects — the new declarations did not widen the accepted key set
|
|
beyond ``failed``/``error``."""
|
|
with pytest.raises(ValidationError):
|
|
ChatMessage.model_validate(
|
|
{"who": "brain", "text": "hi", "failed": True, "error": "x", "foo": 1}
|
|
)
|
|
|
|
|
|
# ---------- backward compatibility: the pre-phase-120 shape ----------
|
|
|
|
|
|
def test_record_without_the_new_keys_validates_with_none() -> None:
|
|
"""A pre-phase record (no ``failed``/``error`` keys) still
|
|
validates; both new fields default to ``None`` (they round-trip as
|
|
nulls — absent/None, exactly like ``stopped`` today)."""
|
|
msg = ChatMessage.model_validate({"who": "brain", "text": "hi"})
|
|
assert msg.failed is None
|
|
assert msg.error is None
|
|
dumped = msg.model_dump()
|
|
assert dumped["failed"] is None
|
|
assert dumped["error"] is None
|
|
|
|
|
|
def test_pre_phase_record_round_trips_byte_identical_on_existing_keys() -> None:
|
|
"""The phase-50 contract through the new schema: a pre-phase-120
|
|
STORED record (every phase-50 key present, explicit nulls where an
|
|
optional key does not apply) validates, and re-serializes
|
|
byte-identically on EVERY pre-phase key — the only diff is the two
|
|
added keys as explicit nulls. Old saved chats and shared links
|
|
therefore render/restore exactly as before."""
|
|
pre_phase: dict = {
|
|
"who": "brain",
|
|
"text": "Your k3s cluster runs on three nodes — you've got this.",
|
|
"sources": [{"source": "Homelab", "path": "kubernetes.md", "title": "Kubernetes"}],
|
|
"related": [{"source": "Homelab", "path": "traefik.md", "title": "Traefik"}],
|
|
"deflected": False,
|
|
"suggestions": ["What ports does Traefik expose?"],
|
|
"thinking": "scratchpad",
|
|
"tools": [
|
|
{
|
|
"name": "read",
|
|
"argument": "Homelab/kubernetes.md",
|
|
"truncated": False,
|
|
"chars_shown": None,
|
|
"chars_total": None,
|
|
}
|
|
],
|
|
"stopped": None,
|
|
}
|
|
dumped = ChatMessage.model_validate(pre_phase).model_dump()
|
|
# Every pre-phase key survives byte-identical…
|
|
for key, value in pre_phase.items():
|
|
assert dumped[key] == value, f"pre-phase key {key!r} changed: {dumped[key]!r}"
|
|
# …and the ONLY additions are the two new keys as explicit nulls.
|
|
assert set(dumped) == set(pre_phase) | {"failed", "error"}
|
|
assert dumped["failed"] is None
|
|
assert dumped["error"] is None
|
|
# The stored shape re-validates (round-trip through the DB JSONB).
|
|
assert ChatMessage.model_validate(dumped).model_dump() == dumped
|
|
|
|
|
|
def test_minimal_user_record_unchanged() -> None:
|
|
"""A user record (no brain metadata at all) is untouched by the
|
|
phase: it validates and carries the new keys as nulls only."""
|
|
dumped = ChatMessage.model_validate({"who": "user", "text": "hi"}).model_dump()
|
|
assert dumped == {
|
|
"who": "user",
|
|
"text": "hi",
|
|
"sources": None,
|
|
"related": None,
|
|
"deflected": None,
|
|
"suggestions": None,
|
|
"thinking": None,
|
|
"tools": None,
|
|
"stopped": None,
|
|
"failed": None,
|
|
"error": None,
|
|
}
|