phase: 189_chat_save_long_turns
Build and Push Containers / build-and-push-app (push) Successful in 3m4s
Build and Push Containers / build-and-push-db (push) Successful in 12s

**Phase 189 final verification — all green**

- **Verified (all 4 tasks already implemented):** A1 caps in `app/schemas.py` — `sources`/`related`/`tools` 200, `suggestions` 100, `segments` 450; `text`/`thinking` 600 000, `error`/`image` 500, `answer_text` 50, `messages` 500, 8 M budget all UNCHANGED; rationale docstrings rewritten to `merge_cited_docs`/`agent_max_rounds`. A2 `shareCurrentChat` linked branch upserts (PUT, 404→create+re-link) before `POST /{id}/share`, aborts on failure (banner, no token); unlinked branch byte-identical. A3 `savePending` + `finally` re-run. A4 `saveFailedSticky` + one 5 s `setTimeout` retry on network/5xx only, 4xx never retries.
- `uv run pytest --cov=app --cov-report=term-missing` → **4609 passed, 1 warning, EXIT=0**, `TOTAL 99%`, `app/schemas.py 100%` (>90%).
- `uv run pytest tests/e2e/test_chat_save_long_turns.py -v --no-cov` → **4 passed, EXIT=0** (isolation).
- Regression green & **unedited** (`git status` clean): `test_chat_save_payload_limits.py` 2, `test_chat_history.py` 5, `test_share_chat.py` 4. Extra blast-radius sweep (share/save/persist/segments surface, all EXIT=0): `test_save_share_ux`, `test_stale_saved_chats`, `test_history_pagination`, `test_history_copy`, `test_bottom_chat_actions`, `test_reasoning_bubble`, `test_tool_budget_telemetry`, `test_verbatim_show_text`, `test_smoke`. No leaked `saved_chats` rows (`save-long-%` = 0).
- `uv run ruff check .` → All checks passed; `uv run pyright` → **0 errors**.
- **Completion criteria:** caps ✓ · incident payload 22/33/51 → 201/200 anonymously + 201 tools/451 segments/101 suggestions → 422 naming the offender, row unchanged ✓ · share-saves-first + abort + unlinked byte-identical ✓ · overlap re-run + sticky note + one-shot retry ✓ · phase E2E in isolation + 3 regression suites unedited ✓ · full suite / coverage / lint / types ✓ · `git diff --stat` = `app/schemas.py`, `frontend/assets/app.js`, 3 test files, `README.md` + new E2E & phase files (no `app/api/chats.py`, no migration, no new dependency) ✓ · commit + `todo/`→`complete/` move left to the harness (rules forbid git index writes).
- **Docs:** full README review (all 2 258 lines) + machine cross-check — all 85 settings documented in the Configuration reference, all 34 `.env.example` vars documented, documented defaults match `app/config.py`. Fixed 2 inconsistencies: (1) the 117-char prose line task 04 left when reflowing the Saved-chats pager paragraph; (2) the phase-189 sentence implying the 8 M total bounds the widened arrays — now states precisely that the total counts `text`/`thinking`/verbatim-block text only, and that the arrays are bounded by fixed item COUNT (matches `_require_within_total_budget`). AGENTS.md verified consistent — it enumerates no save contract or list cap (rule 5's phase-166 in-flight-turn clause untouched), so **no AGENTS.md change** per the phase mandate.
- **Notable/deviation:** the wider caps raise the *theoretical* anonymous-POST worst case (arrays are outside the 8 M char budget); left as designed (A1 fixes every other bound), documented honestly in README — realistic payloads stay ~10 MB, so the existing `client_max_body_size ≥ 16 MB` guidance still holds. No live-infra changes were made (data-repair of the two stale shared rows stays owner action).
- **Next pending phase:** none — `189_chat_save_long_turns` is the last entry in `.agents/phases/todo/`.
This commit is contained in:
2026-10-17 07:36:28 -04:00
parent f20d40e4fe
commit 140a343b92
36 changed files with 2642 additions and 65 deletions
+318 -7
View File
@@ -41,6 +41,25 @@ so a silent regression is caught without a browser:
slide-down + fade entry via ``.toast.is-visible``, and the
reduced-motion override (transform dropped for BOTH states, the
opacity fade kept);
* phase 189 task 02 (A2, the owner incident 2026-10-16): the LINKED
``shareCurrentChat`` branch UPSERTS the current conversation (the
``persistConversation`` PUT + the 404→recreate fallback + the 201
re-link) BEFORE minting the share token, and an upsert failure ABORTS
the share (the neutral banner, no token, no toast) — a Share can never
publish a row the conversation has outgrown. The UNLINKED branch
(``POST /api/chats`` with ``share: true``) is byte-identical;
* phase 189 task 03 (A3/A4, the same incident): the ``persisting``
double-fire guard no longer DROPS an overlapping save point — it sets
``savePending`` and the in-flight upsert's ``finally`` re-runs
``persistConversation()`` ONCE after it settles (``persisting = false``
first, the flag cleared before the re-run — never concurrent, never
stacking). A failed save's one-line note is STICKY (``saveFailedSticky``,
cleared by the next SUCCESS along with ``#send-status``), and a
TRANSIENT failure (the network ``catch`` arm or a 5xx) schedules ONE
retry after 5 s behind the one-shot ``retryScheduled`` guard — a 4xx
NEVER retries (a 422 is deterministic; task 01's cap realign is the
remedy). The A2 contract's SHAPE is unchanged: the same note text,
still no error banner, still silent on success.
* phase 55 task 05 (the action row, owner-locked A5): a single
``<div class="chat-actions">`` in index.html wraps BOTH pills as its
element children (DOM order New chat → Share), replacing the two
@@ -203,7 +222,12 @@ def test_persist_conversation_is_headless_and_quiet() -> None:
one-line #send-status note — on BOTH failure paths (non-ok HTTP and
network) — with the "next save point retries" promise. The
module-level `persisting` flag is the double-fire guard (released
in the finally — never stuck)."""
in the finally — never stuck).
Phase 189 (A3) turns the DROPPED call into a PENDING one: the guard
arm still returns without issuing a second request (the upsert stays
one-at-a-time), it only remembers the save point for the ``finally``
re-run pinned by test_persisting_overlap_sets_pending_and_reruns."""
js = _js()
body = _fn(js, "persistConversation")
assert "showErrorBanner" not in body, "A2: a failed auto-save never raises a banner"
@@ -223,14 +247,177 @@ def test_persist_conversation_is_headless_and_quiet() -> None:
assert re.search(r"^let persisting = false", js, re.M), (
"the persisting flag is module scope (save points can overlap)"
)
assert "if (persisting) return;" in body, "an in-flight upsert skips the second call"
assert "persisting = true;" in body
# Phase 189 (A3): the guard arm still SKIPS the call (no second
# upsert, no second request) — it only records the save point for the
# finally re-run pinned by test_persisting_overlap_sets_pending_and_reruns.
guard = body.find("if (persisting) {")
guard_return = body.find("return;", guard)
assert -1 < guard < guard_return < body.find("persisting = true;"), (
"an in-flight upsert skips the second call"
)
assert body.count("persisting = true;") == 1, "one upsert in flight, ever"
finally_idx = body.rfind("finally")
assert finally_idx != -1 and "persisting = false" in body[finally_idx:], (
"the flag is released in the finally — never stuck"
)
def test_persisting_overlap_sets_pending_and_reruns() -> None:
"""Phase 189 (A3, the owner incident 2026-10-16): a save point that
arrives while an upsert is in flight is PENDING, not dropped. The
guard arm sets ``savePending`` and returns; a fresh run CONSUMES the
flag at the top of its body (so a re-run never schedules a second
one); the ``finally`` re-runs ``persistConversation()`` once, AFTER
``persisting = false`` (the re-run is a fresh serialized call —
never concurrent with the upsert it trails) and clears the flag
before re-entering. Because the payload is built at CALL time, the
re-run carries the LATEST ``conversation`` — the done-time save a
still-running send-time save used to swallow lands with the brain
record."""
js = _js()
body = _fn(js, "persistConversation")
# All three phase-189 flags live at module scope (state across the
# overlapping save points), beside the pre-existing `persisting`.
for flag in ("savePending", "saveFailedSticky", "retryScheduled"):
assert re.search(rf"^let {flag} = false;", js, re.M), (
f"{flag} is module scope like `persisting`"
)
noop = body.find("if (!conversation.length) return;")
first_fetch = body.find("await fetch(")
guard = body.find("if (persisting) {")
guard_return = body.find("return;", guard)
assert -1 < noop < guard < first_fetch, (
"the overlap guard still sits before any request"
)
set_pending = body.find("savePending = true;", guard)
assert guard < set_pending < guard_return, (
"the in-flight arm marks the save point PENDING instead of dropping it"
)
# A fresh run consumes the flag — before the payload is built, so the
# re-run is THIS run and cannot schedule a second re-run.
start = body.find("persisting = true;")
consume = body.find("savePending = false;", start)
assert -1 < start < consume < first_fetch, (
"the run consumes the pending flag at the top of its body"
)
payload = body.find("JSON.stringify({ messages: conversation })")
assert consume < payload, (
"the payload is built at call time — the re-run reads the LATEST conversation"
)
# The finally: release FIRST, then re-run once (never concurrent).
finally_idx = body.rfind("finally")
assert finally_idx > first_fetch, "the release lives in the finally"
tail = body[finally_idx:]
release = tail.find("persisting = false")
recheck = tail.find("if (savePending)", release)
rerun = tail.find("void persistConversation();", recheck)
assert -1 < release < recheck < rerun, (
"the re-run starts only after the in-flight upsert fully settled"
)
clear = tail.find("savePending = false;", recheck)
assert recheck < clear < rerun, "the flag is cleared before re-entering — one re-run, not two"
assert tail.count("void persistConversation();") == 1, "exactly one re-run site"
def test_failed_save_note_is_sticky_until_success() -> None:
"""Phase 189 (A4): a failed auto-save's note does not vanish with the
next paint — both failure arms set ``saveFailedSticky`` after the
EXACT phase-55 note text (the A2 shape is untouched: the same
one-line #send-status note, still NO error banner, still no success
text), and the SUCCESS path clears the flag and resets
``#send-status``. The clear sits on the success path only — after the
201 link and the stale-banner clear, never in a failure arm."""
js = _js()
body = _fn(js, "persistConversation")
note = "Couldn't save automatically — will try on the next message."
assert body.count(note) == 2, "the A2 note text is unchanged, on both failure paths"
assert "showErrorBanner" not in body, "A2 unchanged: a failed auto-save never raises a banner"
assert "Conversation saved." not in body, "A2 unchanged: success stays silent"
# Non-ok arm: note, then the sticky flag.
notok = body.find("if (!res.ok)")
first_note = body.find(note)
first_sticky = body.find("saveFailedSticky = true;", first_note)
assert -1 < notok < first_note < first_sticky, (
"the non-ok arm notes AND marks the note sticky"
)
# Network arm (the catch): the same note, the same flag.
catch_idx = body.find("} catch {")
second_note = body.find(note, catch_idx)
second_sticky = body.find("saveFailedSticky = true;", second_note)
assert catch_idx < second_note < second_sticky, (
"the catch arm notes AND marks the note sticky"
)
assert body.count("saveFailedSticky = true;") == 2, "both — and only both — failure arms"
# Success: the flag clears and the status line resets, on the success
# path only (after the 201 link + the stale-banner clear).
link = body.find("res.status === 201")
hide = body.find("staleBanner.hidden = true")
clear_guard = body.find("if (saveFailedSticky)")
assert -1 < link < hide < clear_guard < catch_idx, (
"the stuck note clears on the success path, never on failure"
)
block = body[clear_guard : body.find("}", clear_guard)]
assert "saveFailedSticky = false;" in block, "the flag resets with the note"
assert 'sendStatus.textContent = "";' in block, "the status line returns to empty"
def test_transient_failure_retries_once_after_5s() -> None:
"""Phase 189 (A4): a TRANSIENT failure — the network ``catch`` arm or
a 5xx status — schedules ONE retry after 5 000 ms. The one-shot
``retryScheduled`` guard is set BEFORE the ``setTimeout`` and reset
INSIDE the callback (overlapping failures never stack retries), and
the retry is a plain ``persistConversation()`` — the LATEST
conversation through the same guards (the pending re-run included)."""
js = _js()
body = _fn(js, "persistConversation")
assert body.count("setTimeout(") == 2, "one retry site per transient arm — no more"
for arm_start in (body.find("if (!res.ok)"), body.find("} catch {")):
arm_end = body.find("} finally {", arm_start)
assert -1 < arm_start < arm_end
timeout = body.find("setTimeout(", arm_start)
assert arm_start < timeout < arm_end, "the arm schedules the retry"
guard = body.find("retryScheduled = true;", arm_start)
assert arm_start < guard < timeout, (
"the one-shot guard is set BEFORE the setTimeout"
)
delay = body.find(", 5000);", timeout)
assert timeout < delay < arm_end, "the retry waits 5 000 ms"
callback = body[timeout:delay]
assert "retryScheduled = false;" in callback, "the guard resets INSIDE the callback"
assert "void persistConversation();" in callback, "the retry is a plain auto-save"
# The 5xx condition is the ONLY status gate on the non-ok arm.
notok_arm = body[body.find("if (!res.ok)") : body.find("} catch {")]
assert "res.status >= 500" in notok_arm
def test_4xx_never_retries() -> None:
"""Phase 189 (A4): a 4xx is DETERMINISTIC — a 422 (the incident: the
22-source payload over the old 20-cap) would 422 again, and task 01's
cap realign is the remedy. The non-ok arm's only scheduling site sits
behind ``res.status >= 500``, so every other non-ok status (404 aside
— the recreate fallback — 409, 422, 400) notes and returns with no
timer; the arm still marks the note sticky (it stays visible)."""
js = _js()
body = _fn(js, "persistConversation")
notok = body.find("if (!res.ok)")
catch_idx = body.find("} catch {")
arm = body[notok:catch_idx]
assert arm.count("setTimeout(") == 1, "the non-ok arm has exactly ONE scheduling site"
gate = arm.find("if (res.status >= 500 && !retryScheduled) {")
timeout = arm.find("setTimeout(")
assert -1 < gate < timeout, "the only retry is gated on a 5xx status"
# Nothing else in the arm keys off a 4xx, and nothing after the gate's
# block runs before the return.
for forbidden in ("status === 4", "status >= 4", "status < 500", "retryScheduled"):
assert forbidden not in arm[timeout + len("setTimeout(") :].replace(
"retryScheduled = false;", ""
), f"no extra scheduling branch keyed on {forbidden!r}"
assert arm.find("saveFailedSticky = true;") < arm.find("return;"), (
"a 4xx still leaves the note visible — it just never retries"
)
assert body.count("setTimeout(") == 2, "the catch arm is the only other scheduling site"
def test_auto_save_wired_to_the_save_points() -> None:
"""The headless helper is referenced from the save points: the user
send (runTurn's ``!reask`` block — after the localStorage
@@ -489,7 +676,14 @@ def test_share_current_chat_save_then_share_branch() -> None:
again" — the write surface is public, phase 55 task 01, so a 403
is no longer a sign-in problem for a guest); network → the
reachable? banner. The double-click guard releases in the finally —
never stale."""
never stale.
Phase 189 (task 02) moves the ORDERING pins to the create-with-share
payload: the linked branch now ALSO holds a POST (its 404→recreate
fallback), so "the first /api/chats POST is the unlinked arm" no
longer reads — ``share: true`` is what identifies that arm (the
upsert-before-mint ordering itself is pinned by
test_share_linked_branch_upserts_before_the_share_mint)."""
js = _js()
body = _fn(js, "shareCurrentChat")
# No-op first: nothing to share → live-region line, no fetch.
@@ -506,7 +700,7 @@ def test_share_current_chat_save_then_share_branch() -> None:
assert 'JSON.stringify({ messages: conversation, share: true })' in body, (
"the create-with-share payload — the server sets the token in the same commit"
)
post_idx = body.find('fetch("/api/chats", {')
post_idx = body.find('JSON.stringify({ messages: conversation, share: true })')
assert -1 < share_fetch < post_idx, "the linked branch precedes the unlinked fallback"
created_idx = body.find("currentChatId = String(created.id)", post_idx)
assert created_idx != -1, "one action saved AND shared: the conversation links to the row"
@@ -529,8 +723,10 @@ def test_share_current_chat_save_then_share_branch() -> None:
# is public); the network banner keeps its own line.
assert 'showErrorBanner("Couldn\'t share the conversation — is the app reachable?")' in body
neutral = "Couldn't share the conversation — try again."
assert body.count(neutral) == 2, (
"both the linked and the unlinked branch carry the neutral non-ok banner"
assert body.count(neutral) == 3, (
"three non-ok arms carry the SAME neutral banner: the phase-189 "
"upsert abort, the linked share mint, and the unlinked create "
"(no sign-in wording on any of them)"
)
assert "signed in" not in body, "no sign-in wording left in the share handler (task 03)"
# The double-click guard releases on EVERY outcome.
@@ -571,6 +767,121 @@ def test_share_current_chat_save_then_share_branch() -> None:
assert "document.createRange()" in sel and "selectNodeContents(el)" in sel
def test_share_linked_branch_upserts_before_the_share_mint() -> None:
"""Phase 189 (task 02, A2): the LINKED branch of ``shareCurrentChat``
upserts the CURRENT conversation BEFORE minting the token — the owner
incident (2026-10-16: a row holding only the question because the
done-time auto-save 422'd into the phase-55 quiet failure; the Share
minted on that stale row and published a truncated snapshot) is the
why. The upsert carries ``persistConversation``'s semantics: the
``{ messages: conversation }`` payload + JSON headers, the PUT to
/api/chats/<id>, the 404→recreate fallback (unlink, then POST
/api/chats with the SAME body/headers) and the 201 re-link. The
share mint is the LAST request of the branch (string-ordering pin —
no browser)."""
js = _js()
body = _fn(js, "shareCurrentChat")
branch = body.find("if (currentChatId) {")
else_idx = body.find("} else {")
assert -1 < branch < else_idx
arm = body[branch:else_idx]
put = arm.find('`/api/chats/${currentChatId}`')
mint = arm.find('`/api/chats/${currentChatId}/share`')
assert -1 < put < mint, "the upsert PUT precedes the share mint (save-first)"
# The persistConversation-shaped payload + headers, byte-identical to
# the auto-save's (ONE upsert shape, not two).
persist = _fn(js, "persistConversation")
assert arm.find('const body = JSON.stringify({ messages: conversation });') < put
assert 'const body = JSON.stringify({ messages: conversation });' in persist, (
"the share upsert reuses the auto-save's payload shape"
)
assert 'const headers = { "Content-Type": "application/json" };' in arm
assert 'method: "PUT"' in arm, "the linked branch PUTs the linked row"
# The 404→recreate fallback (the same stale-link rule as the auto-save).
notfound = arm.find("res.status === 404")
assert put < notfound < mint, "a PUT 404 (row deleted on History) is handled"
assert "currentChatId = null" in arm[notfound:mint], "the stale link is dropped"
fallback = arm.find('fetch("/api/chats", { method: "POST", headers, body })')
assert notfound < fallback < mint, (
"the 404 retries as a create with the SAME body/headers"
)
relink = arm.find("currentChatId = String((await res.json()).id)")
assert fallback < relink < mint, (
"the 201 re-links the conversation to the recreated row"
)
# The mint rides AFTER the upsert settled — and it is the branch's
# LAST request (nothing publishes before the row catches up).
assert arm.count("await fetch(") == 3, "PUT, the recreate POST, then the mint"
last_fetch = arm[arm.rfind("await fetch(") :].split("\n")[0]
assert "/share`" in last_fetch, "the share mint is the linked branch's LAST fetch"
# The docstring says WHY (the incident), not just what.
doc = js[: js.find("async function shareCurrentChat()")]
doc = doc[doc.rfind("/*"):]
assert "UPSERTS the current conversation BEFORE minting the token" in doc
assert "truncated snapshot" in doc, "the incident rationale is in the docstring"
def test_share_aborts_on_upsert_failure() -> None:
"""Phase 189 (task 02, A2): an upsert FAILURE aborts the share. The
failure arm that sits BETWEEN the upsert and the mint raises the
EXACT neutral banner text and ``return``s — no token minted, no
toast, no link handed to the owner (the mint line and every success
path come after it). The double-click guard still releases in the
finally, and a network throw still lands on the reachable? banner
(the outer catch, unchanged)."""
js = _js()
body = _fn(js, "shareCurrentChat")
branch = body.find("if (currentChatId) {")
arm = body[branch : body.find("} else {")]
mint = arm.find('`/api/chats/${currentChatId}/share`')
notok = arm.find("if (!res.ok)")
banner = arm.find(
'showErrorBanner("Couldn\'t share the conversation — try again.")', notok
)
assert -1 < notok < banner < mint, (
"the non-ok upsert raises the neutral banner BEFORE the mint line"
)
ret = arm.find("return;", banner)
assert notok < ret < mint, "the abort returns — the mint is never reached"
abort = arm[notok:ret]
assert "showToast" not in abort, "the abort toasts nothing (the toast stays success-only)"
assert "share_url" not in abort, "no link is read or handed out on the abort"
assert "copyShareLinkWithFallback" not in abort, "no copy on the abort"
assert "showToast" not in arm[:mint], "no toast anywhere before the mint"
# Both non-ok arms of the linked branch carry the SAME neutral copy:
# the upsert abort and the mint's own failure.
assert arm.count('showErrorBanner("Couldn\'t share the conversation — try again.")') == 2, (
"the upsert abort + the mint's non-ok arm — the neutral copy, twice in the branch"
)
finally_idx = body.rfind("finally")
assert finally_idx != -1 and "shareBtn.disabled = false" in body[finally_idx:], (
"the button releases even on the abort — never stale"
)
def test_share_unlinked_branch_unchanged() -> None:
"""Phase 189 (task 02): the UNLINKED branch is byte-identical — one
``POST /api/chats`` with ``{ messages: conversation, share: true }``
(the server sets the 128-bit token in the same commit — one action
saves AND shares, owner-locked) and the 201 re-link. It carries no
PUT and no separate share mint: the save-first upsert belongs to the
linked arm only (its POST already saves the CURRENT conversation)."""
js = _js()
body = _fn(js, "shareCurrentChat")
else_idx = body.find("} else {")
copy_idx = body.find("copyShareLinkWithFallback(absoluteShareUrl(shareUrl))")
assert -1 < else_idx < copy_idx
arm = body[else_idx:copy_idx]
assert 'fetch("/api/chats", {' in arm
assert 'method: "POST"' in arm
assert 'JSON.stringify({ messages: conversation, share: true })' in arm
assert "currentChatId = String(created.id)" in arm, "the 201 re-links (saved AND shared)"
assert 'shareUrl = created.share_url' in arm, "the token comes from the same commit"
assert 'method: "PUT"' not in arm, "the unlinked arm never PUTs — unchanged"
assert "/share" not in arm, "the unlinked arm never mints a separate token"
assert "share: true" not in body[:else_idx], "share: true stays out of the linked arm"
def test_share_button_has_no_reveal_gate() -> None:
"""Phase 55 task 03: the pill is VISIBLE TO EVERY VISITOR — app.js
queries #share-chat-btn and binds the click to shareCurrentChat, but