phase: 164_chat_error_honesty
All gates green. Report: - **Verified (phase 164, all 4 tasks already in `complete/`)**: `HistoryTurn`/`ChatMessage`/`ChatThinkingSegment` caps → 600 000, `history`/saved-messages lists → 500, new `SavedChatCreate/Update` total-8 000 000-char validator; exported `errorDetailText` at the two previously unguarded chat paths (pre-stream `!res.ok` + SSE `error` frame); `FAILED_REQUEST_TEXT` + `bubbleText` threading + phase-114 banner hint; dedicated E2E `tests/e2e/test_chat_error_honesty.py`. - **Final-pass additions**: `tests/integration/test_chat_api.py::test_history_rejects_one_char_over_the_text_cap` (endpoint-level >600k → 422 with the pydantic list-of-objects `msg` — the criterion's upper half was only unit-pinned); README env rows for `BOR_APP_VERSION` + `BOR_SESSION_COOKIE`; restored `test_document_viewer.py`'s tracked screenshot artifact dirtied by the E2E run (via read-only `git show`, no index/history touched). - **Tests/lint (exact commands)**: `uv run pytest --cov=app --cov-report=term-missing` → **3837 passed**, `app/` coverage **99%** (>90%); `uv run pytest tests/e2e/test_chat_error_honesty.py -v --no-cov` → **4 passed in isolation**; per-file loop over **all 148** E2E files (`uv run pytest tests/e2e/<f> -q --no-cov`) → **148 × exit 0**, incl. `test_failed_turn_retry`, `test_chat_save_payload_limits`, `test_edit_and_resend`, `test_llm_history`, `test_smoke`; `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors. - **Completion criteria**: pre-stream 422 shows the distinct line + verbatim `msg`, `[object Object]` unrepresentable + no `input` in DOM — **met** (unit totality/source pins + E2E asserts 1–2); 525k history accepted (200 settles) / >600k → 422 / saved chat 201 + 8M bound — **met**; transport + SSE-frame settles keep `FAILED_TURN_TEXT`, stop/abort untouched, §7.4 re-enables — **met**; budget trim, `message` 4000, the 7 guarded detail-readers and all SSE shapes byte-identical (diff touches only caps + the two chat paths) — **met**; README/AGENTS full review + proxy deploy note — **met**; commit/`complete/` move = pipeline gate. - **Docs**: README reviewed in full (env tables cross-checked programmatically: 74 `BOR_*` rows vs `Settings` defaults — no mismatches; every `.env.example` var documented; referenced scripts/pages/endpoints/`query_log` columns exist) — found & fixed the 2 undocumented real knobs above; the phase's payload-limits section, failed-turn copy and `client_max_body_size ≥ 16 MB` deploy note verified accurate. `AGENTS.md` verified consistent (rules, gitignore list, quick-reference commands, referenced paths all correct) — no changes needed. - **Deviations**: `ChatThinkingSegment.text` raised 32 000 → 600 000 too (not itemized in A1, but its own docstring mirrors `HistoryTurn.thinking`; without it a long-thinking turn 422s its own auto-save — same mirror invariant). No live-infra changes made (the proxy limit lives in the separate deployment repo; README note only). - **Next pending phase**: none — `164_chat_error_honesty` was the only entry in `todo/`.
This commit is contained in:
@@ -2204,18 +2204,101 @@ def test_history_rejects_unknown_who(client, db) -> None:
|
||||
assert r.status_code == 422
|
||||
|
||||
|
||||
def test_history_rejects_more_than_100_entries(client, db) -> None:
|
||||
"""Schema pin: the DoS sanity ceiling is 100 turns — 101 is a 422
|
||||
def test_history_rejects_more_than_500_entries(client, db) -> None:
|
||||
"""Schema pin: the DoS sanity ceiling is 500 turns (phase 164, locked
|
||||
A1 — raised from 100 because the client sends the WHOLE
|
||||
``bor.chat.v1`` record minus the current question) — 501 is a 422
|
||||
(the config budgets do the real trimming; this only keeps a
|
||||
pathological body from wasting the mapper's work)."""
|
||||
r = client.post(
|
||||
"/api/chat",
|
||||
json={
|
||||
"message": "hi",
|
||||
"history": [{"who": "user", "text": f"q{i}"} for i in range(101)],
|
||||
"history": [{"who": "user", "text": f"q{i}"} for i in range(501)],
|
||||
},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
assert r.json()["detail"][0]["loc"] == ["body", "history"]
|
||||
|
||||
|
||||
def test_history_rejects_one_char_over_the_text_cap(client, db) -> None:
|
||||
"""The upper half of the phase-164 boundary at the ENDPOINT (task 01,
|
||||
locked A1): one character past the 600 000 per-message cap is a 422 —
|
||||
and the ``detail`` FastAPI hands the browser is the pydantic LIST OF
|
||||
OBJECTS (``loc``/``msg``), the exact shape the client's serializer
|
||||
must read ``msg`` from (the live "[object Object]"). The 4000-char
|
||||
``message`` cap is unchanged (the composer mirrors it client-side)."""
|
||||
r = client.post(
|
||||
"/api/chat",
|
||||
json={"message": "hi", "history": [{"who": "brain", "text": "a" * 600_001}]},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
(entry,) = r.json()["detail"]
|
||||
assert entry["loc"] == ["body", "history", 0, "text"]
|
||||
assert entry["type"] == "string_too_long"
|
||||
assert entry["msg"] == "String should have at most 600000 characters"
|
||||
|
||||
|
||||
def test_history_accepts_a_full_length_prior_answer(
|
||||
client,
|
||||
db,
|
||||
seeded_kb: FakeRagLLM,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Phase 164 task 01 — the live 422. The turn AFTER a full-length
|
||||
answer (525k chars ≈ a 131 072-token generation) sends that answer
|
||||
back as ``history``; the 32 000 ceiling rejected it, so the
|
||||
follow-up died with "[object Object]". It is now ACCEPTED: the
|
||||
stream opens (200) and settles with ``done``, no 422 anywhere.
|
||||
|
||||
What the model SEES is the locked budget trim's business (A4, not
|
||||
touched here): a turn over ``history_max_chars`` is dropped WHOLE,
|
||||
so this giant turn trims itself out of the prompt and
|
||||
``history_msgs=0`` — the follow-up works with a trimmed window.
|
||||
The acceptance is what this pin owns."""
|
||||
long_answer = "a" * 525_000
|
||||
history = [
|
||||
{"who": "user", "text": "Write me the whole k3s runbook."},
|
||||
{"who": "brain", "text": long_answer},
|
||||
]
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: seeded_kb
|
||||
try:
|
||||
caplog.set_level(logging.INFO, logger="app.chat")
|
||||
frames = _stream_chat_with_history(client, "And the rollback part?", history)
|
||||
finally:
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
|
||||
assert frames[-1]["type"] == "done" # accepted: the turn settled normally
|
||||
lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()]
|
||||
assert lines and "history_msgs=0" in lines[-1]
|
||||
(messages,) = seeded_kb.seen_messages
|
||||
assert messages[1] == {"role": "user", "content": "And the rollback part?"}
|
||||
|
||||
|
||||
def test_history_accepts_a_full_length_answer_bounded_by_the_char_budget(
|
||||
client,
|
||||
db,
|
||||
seeded_kb: FakeRagLLM,
|
||||
) -> None:
|
||||
"""The same request with the giant answer NOT the newest turn: the
|
||||
newer small turns fit ``history_max_chars`` and reach the model,
|
||||
while the 525k turn is dropped WHOLE (locked A3/A4 — never cut,
|
||||
never a 422). Proves the raise changed what ARRIVES, not what the
|
||||
trim keeps."""
|
||||
history = [
|
||||
{"who": "brain", "text": "a" * 525_000},
|
||||
{"who": "user", "text": "Short follow-up question."},
|
||||
]
|
||||
fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: seeded_kb
|
||||
try:
|
||||
_stream_chat_with_history(client, "And the rollback part?", history)
|
||||
finally:
|
||||
fastapi_app.dependency_overrides.clear()
|
||||
|
||||
(messages,) = seeded_kb.seen_messages
|
||||
# The giant turn is gone whole; the fitting newer turn is kept.
|
||||
assert {"role": "user", "content": "Short follow-up question."} in messages
|
||||
assert all(len(m["content"]) < 525_000 for m in messages)
|
||||
|
||||
|
||||
def test_done_event_serializes_column_maximum_source_refs() -> None:
|
||||
|
||||
@@ -1642,18 +1642,20 @@ def test_page_route_serves_shared_html_when_present(
|
||||
# handler runs — no route change, nothing ever lands in the JSONB) ----------
|
||||
|
||||
|
||||
def test_post_rejects_text_over_32000_and_stores_nothing(
|
||||
def test_post_rejects_text_over_600000_and_stores_nothing(
|
||||
client: TestClient, admin_client: TestClient
|
||||
) -> None:
|
||||
"""The audit vector (SEC-05): one 32_001-char message 422s at the
|
||||
boundary (``ChatMessage.text`` mirrors ``HistoryTurn``'s 32 000
|
||||
cap) and NOTHING is stored — the admin list is unchanged (the happy
|
||||
path for in-cap bodies stays the existing guest/admin pins)."""
|
||||
"""The audit vector (SEC-05): one 600_001-char message 422s at the
|
||||
boundary (``ChatMessage.text`` mirrors ``HistoryTurn``'s 600 000 cap
|
||||
— raised TOGETHER in phase 164, locked A1) and NOTHING is stored —
|
||||
the admin list is unchanged (the happy path for in-cap bodies stays
|
||||
the existing guest/admin pins, incl. the full-cap-answer save
|
||||
below)."""
|
||||
baseline = admin_client.get("/api/chats").json()["chats"]
|
||||
|
||||
r = client.post(
|
||||
"/api/chats",
|
||||
json={"messages": [_user("a" * 32_001), _user("follow-up")]},
|
||||
json={"messages": [_user("a" * 600_001), _user("follow-up")]},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
|
||||
@@ -1661,16 +1663,87 @@ cap) and NOTHING is stored — the admin list is unchanged (the happy
|
||||
assert listing == baseline, "an oversized POST must not store a row"
|
||||
|
||||
|
||||
def test_post_rejects_more_than_200_messages_and_stores_nothing(
|
||||
def test_post_accepts_a_full_cap_answer_and_round_trips(
|
||||
client: TestClient, admin_client: TestClient
|
||||
) -> None:
|
||||
"""The unbounded message list is the second DoS lever (SEC-05):
|
||||
201 minimally-valid messages 422 (``SavedChatCreate.messages``
|
||||
``max_items=200``) and nothing lands."""
|
||||
"""Phase 164 task 01 (the live report's save side): a conversation
|
||||
carrying ONE full-length brain answer (525k chars — a
|
||||
``BOR_MAX_OUTPUT_TOKENS=131_072`` answer at ~4 chars/token)
|
||||
saves 201 and round-trips byte-for-byte through the JSONB — the
|
||||
whole ``done``-time auto-save of a long answer must not 422 into
|
||||
the phase-113 quiet-failure. No column change: ``messages`` is raw
|
||||
JSONB, so the only gate is the schema boundary."""
|
||||
answer = "a" * 525_000
|
||||
r = client.post(
|
||||
"/api/chats",
|
||||
json={
|
||||
"messages": [
|
||||
_user("Write the whole runbook"),
|
||||
{"who": "brain", "text": answer},
|
||||
_user("And the rollback?"),
|
||||
{"who": "brain", "text": "Short rollback."},
|
||||
]
|
||||
},
|
||||
)
|
||||
assert r.status_code == 201, r.text
|
||||
chat_id = r.json()["id"]
|
||||
assert r.json()["messages"][1]["text"] == answer
|
||||
|
||||
stored = admin_client.get(f"/api/chats/{chat_id}").json()
|
||||
assert stored["messages"][1]["text"] == answer
|
||||
assert len(stored["messages"]) == 4
|
||||
|
||||
|
||||
def test_post_rejects_over_total_char_budget_and_stores_nothing(
|
||||
client: TestClient, admin_client: TestClient
|
||||
) -> None:
|
||||
"""The ONE new anonymous bound (phase 164, locked A1): messages that
|
||||
are individually legal but total over 8 000 000 characters 422 with
|
||||
the budget named in the ``detail`` (the model-level validator, so
|
||||
``loc`` is empty) — and nothing lands. 16 full-cap answers
|
||||
= 8 388 800 chars."""
|
||||
baseline = len(admin_client.get("/api/chats").json()["chats"])
|
||||
|
||||
r = client.post(
|
||||
"/api/chats", json={"messages": [_user(f"question {i}") for i in range(201)]}
|
||||
"/api/chats",
|
||||
json={"messages": [{"who": "brain", "text": "a" * 524_300} for _ in range(16)]},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
assert "8,000,000-character budget" in json.dumps(r.json())
|
||||
|
||||
assert len(admin_client.get("/api/chats").json()["chats"]) == baseline
|
||||
|
||||
|
||||
def test_put_rejects_over_total_char_budget_and_leaves_row_unchanged(
|
||||
client: TestClient, admin_client: TestClient
|
||||
) -> None:
|
||||
"""The re-Save path carries the SAME validator (phase 164): an
|
||||
over-budget replacement 422s and the row keeps its original
|
||||
payload."""
|
||||
created = client.post("/api/chats", json={"messages": _simple_conversation()}).json()
|
||||
|
||||
r = client.put(
|
||||
f"/api/chats/{created['id']}",
|
||||
json={"messages": [{"who": "brain", "text": "a" * 524_300} for _ in range(16)]},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
assert "8,000,000-character budget" in json.dumps(r.json())
|
||||
|
||||
kept = admin_client.get(f"/api/chats/{created['id']}").json()
|
||||
assert kept["messages"] == _expect(_simple_conversation())
|
||||
|
||||
|
||||
def test_post_rejects_more_than_500_messages_and_stores_nothing(
|
||||
client: TestClient, admin_client: TestClient
|
||||
) -> None:
|
||||
"""The unbounded message list is the second DoS lever (SEC-05):
|
||||
501 minimally-valid messages 422 (``SavedChatCreate.messages``
|
||||
``max_items=500`` — phase 164 raised it WITH
|
||||
:attr:`ChatRequest.history`) and nothing lands."""
|
||||
baseline = len(admin_client.get("/api/chats").json()["chats"])
|
||||
|
||||
r = client.post(
|
||||
"/api/chats", json={"messages": [_user(f"question {i}") for i in range(501)]}
|
||||
)
|
||||
assert r.status_code == 422
|
||||
|
||||
@@ -1707,7 +1780,7 @@ def test_put_rejects_oversized_message_and_leaves_row_unchanged(
|
||||
client: TestClient, admin_client: TestClient
|
||||
) -> None:
|
||||
"""The re-Save path is gated by the SAME caps (A1: create AND update
|
||||
carry the bounds): a 32_001-char message 422s and the row keeps its
|
||||
carry the bounds): a 600_001-char message 422s and the row keeps its
|
||||
original payload byte-for-byte (the stored shape is untouched — the
|
||||
rejected body never reaches the JSONB)."""
|
||||
created = client.post(
|
||||
@@ -1716,7 +1789,7 @@ def test_put_rejects_oversized_message_and_leaves_row_unchanged(
|
||||
|
||||
r = client.put(
|
||||
f"/api/chats/{created['id']}",
|
||||
json={"messages": [_user("b" * 32_001)]},
|
||||
json={"messages": [_user("b" * 600_001)]},
|
||||
)
|
||||
assert r.status_code == 422
|
||||
|
||||
@@ -1976,9 +2049,10 @@ def test_segments_over_50_entries_rejected(admin_client: TestClient) -> None:
|
||||
|
||||
|
||||
def test_thinking_segment_text_caps_rejected(admin_client: TestClient) -> None:
|
||||
"""The thinking entry's ``text`` cap (HistoryTurn's 32 000, phase
|
||||
83): 32 001 chars 422; the ``min_length=1`` bound — an EMPTY text
|
||||
(the zero-char run the client never stores) 422s too."""
|
||||
"""The thinking entry's ``text`` cap (HistoryTurn's 600 000 — the
|
||||
phase-164 raise, mirrored so a long-thinking turn never 422s its own
|
||||
auto-save): 600 001 chars 422; the ``min_length=1`` bound — an EMPTY
|
||||
text (the zero-char run the client never stores) 422s too."""
|
||||
over = admin_client.post(
|
||||
"/api/chats",
|
||||
json={
|
||||
@@ -1986,7 +2060,7 @@ def test_thinking_segment_text_caps_rejected(admin_client: TestClient) -> None:
|
||||
{
|
||||
"who": "brain",
|
||||
"text": "ok",
|
||||
"segments": [{"kind": "thinking", "text": "x" * 32_001}],
|
||||
"segments": [{"kind": "thinking", "text": "x" * 600_001}],
|
||||
}
|
||||
]
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user