Regenerate actually regenerates
`ensure` is keyed on message_id and idempotent on purpose -- a page load
finding an unfinished reply must attach to it rather than start a second
one, and `_follow` calls it too. But finished generations linger in the
registry for KEEP_FINISHED so a follower arriving at the last moment still
gets the final frames, and regenerate is the only caller that reuses an
existing Message row instead of creating a new one. So `ensure` handed back
the finished generation: no request was made, `_follow` replayed the old
answer, and the `done` frame re-rendered a streaming shell because the row
said incomplete. That is the reconnect loop, and the Send button stuck on
Stop. It appeared to work after five minutes only by accident, and only
sometimes: `_prune` sat below the early return, so it was unreachable for
exactly the message that needed it.
`restart()` is the explicit opposite of `ensure`, and regenerate calls it.
`_prune` moves above the lookup.
Cancelling a live predecessor makes its `finally:` run `_persist` on the
same row, which would overwrite the reply that replaced it. `_persist` now
refuses when another generation owns the message -- "someone else owns this
row now", not "this one is registered", so a direct call still writes.
Three things found next door, all in the same area and all bugs:
- `done` was set before `_persist` committed, while `_follow`'s docstring
claimed the opposite. `_follow` breaks out the instant it sees the flag
and re-renders the bubble from the row, so the row has to be right
first. Harmless today, a guaranteed loss once metrics land there.
- Live reasoning duplicated quadratically. The frame carries the whole
block each time, exactly as `render` and `tools` do, but the target
swapped it `beforeend`.
- `sse.KEEPALIVE` was defined and never yielded. A model thinking for
ninety seconds emits nothing, and an idle connection is what a proxy
closes.
There was no test for regenerate at all. There is now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+19
-2
@@ -5,6 +5,7 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
import time
|
||||
from collections.abc import AsyncIterator
|
||||
|
||||
from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
|
||||
@@ -28,6 +29,10 @@ log = logging.getLogger(__name__)
|
||||
|
||||
router = APIRouter(prefix="/api/chats", tags=["chats"])
|
||||
|
||||
# Seconds of silence before a comment frame is sent to hold the connection open.
|
||||
# Well under nginx's 60s default; see services/sse.py:KEEPALIVE.
|
||||
KEEPALIVE_AFTER = 15.0
|
||||
|
||||
def _owned_chat(db: DBSession, chat_id: str, user_id: str) -> Chat:
|
||||
chat = db.get(Chat, chat_id)
|
||||
# 404 rather than 403 for someone else's chat: whether a given id exists is
|
||||
@@ -240,6 +245,7 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]:
|
||||
generation = generation_service.ensure(chat_id, message_id)
|
||||
generation.followers += 1
|
||||
seen = -1
|
||||
last_frame = time.monotonic()
|
||||
|
||||
try:
|
||||
while True:
|
||||
@@ -251,9 +257,18 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]:
|
||||
yield sse.event("tools", _tool_activity(generation.tool_events))
|
||||
if generation.content:
|
||||
yield sse.event("render", render_markdown(generation.text))
|
||||
last_frame = time.monotonic()
|
||||
|
||||
if generation.done:
|
||||
break
|
||||
|
||||
# A reasoning model can think for a minute or more without emitting
|
||||
# anything, and an idle connection is what a proxy closes. The
|
||||
# comment frame keeps it open and is ignored by the browser.
|
||||
if time.monotonic() - last_frame > KEEPALIVE_AFTER:
|
||||
yield sse.KEEPALIVE
|
||||
last_frame = time.monotonic()
|
||||
|
||||
# Polling rather than per-follower wakeups: the producer already
|
||||
# works in RENDER_INTERVAL steps, so a short sleep is simpler and
|
||||
# cannot drop a notification.
|
||||
@@ -261,7 +276,7 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]:
|
||||
finally:
|
||||
generation.followers = max(0, generation.followers - 1)
|
||||
|
||||
# The producer writes the message before marking itself done, so by here
|
||||
# The producer commits the message before marking itself done, so by here
|
||||
# the row is authoritative and the final bubble can be rendered from it.
|
||||
with session_scope() as db:
|
||||
message = db.get(Message, message_id)
|
||||
@@ -592,7 +607,9 @@ async def regenerate(
|
||||
message.complete = False
|
||||
message.model_id = chat.model_id
|
||||
db.commit()
|
||||
generation_service.ensure(chat.id, message.id)
|
||||
# restart, not ensure: this is the one caller that reuses a Message row, and
|
||||
# the finished generation for it is still registered.
|
||||
generation_service.restart(chat.id, message.id)
|
||||
|
||||
return templates.TemplateResponse(
|
||||
request,
|
||||
|
||||
Reference in New Issue
Block a user