All verification is complete and green. Phase 139's three tasks (01–03) were already implemented in prior passes; this was the final verification pass, and every gate holds. ## Phase 139 — final verification report **Verified in working tree (no code changes needed):** - `readRangeSuffix` single-source-of-truth helper in `app.js` + byte-identical parity copy in `shared.js` (EN-dash ` (lines N–M)`, `(from line N)`, `(first N lines)`, `Number.isInteger(x) && x >= 1` guard mirroring the server's `_whole_line_number`) - `appendToolLine(wrap, name, argument, offset = null, limit = null)` — `<span class="read-range">` sibling after `<code>`, only when suffix non-empty; `shared.js` `addToolLines` parity - Live `tool` branch: guarded `ev.offset`/`ev.limit`, additive `toolAcc` records (non-range keeps pre-phase shape), status label carries the suffix (A2); restore path passes stored `t.offset`/`t.limit` through, phase-95 truncation note untouched - E2E: `SAME_FILE_TRIGGER` mock flow (same file read twice, offsets 10/20 then 40/15) + 3-test Playwright suite **Test / lint / coverage results:** - `uv run pytest` (unit + integration): **3283 passed, 9 skipped, 0 failed** - `uv run pytest --cov=app --cov-report=term-missing`: **TOTAL 99%** (>90% gate) - `uv run pytest tests/e2e/test_read_range_tool_lines.py -v --no-cov`: **3 passed** in isolation - `uv run ruff check .`: clean; `uv run pyright`: **0 errors, 0 warnings** **Completion criteria:** 1. Range read renders `(lines N–M)` live; two same-file reads show DIFFERENT ranges — ✅ (E2E test 1) 2. Plain read byte-identical; phase-87/117 pins + emoji guard pass unedited — ✅ (E2E test 2 + full suite) 3. Suffix survives save → restore on chat page + shared page — ✅ (E2E test 3) 4. Full suite green, coverage >90%, E2E isolated green, ruff + pyright clean — ✅ 5. Commit + phase move — left to the harness per executor rules (no `git add`/`commit` run) **Notable:** the missing pytest summary line on one run was a transient output-buffering artifact, confirmed by re-run; DB was already up and healthy. **Next pending phase:** `140_pictorial_pdf_pages`
201 lines
8.2 KiB
Python
201 lines
8.2 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 — tool entries in the CURRENT full
|
|
shape, the phase-95/139 additive fields as nulls for a plain read)
|
|
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,
|
|
"offset": None,
|
|
"limit": 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,
|
|
}
|