The warning said "lifecycle transitions from superseded turn ARE NOT
being rolled back". When regenerating an OLDER turn, the listed
transitions can include intervening-turn ones that legitimately stand
on their own — they weren't authored by the superseded turn itself.
Reword to "lifecycle transitions at-or-after turn <id>" so operators
reading logs aren't misled into thinking every listed event id was
emitted by the target turn. Cosmetic change to a single log message.
Test: extends test_regenerate_with_prior_lifecycle_logs_warning to
assert the new phrasing is present and the old phrasing is gone.
The previous implementation pulled the last N rows in SQL across all
chats and dropped foreign-chat rows in Python. With LIMIT N this could
return far fewer than N relevant rows when other chats had recent
activity. Push the chat_id filter into SQL via json_extract so LIMIT N
always returns N rows scoped to the requested chat.
Test: seeds two chats with 60 turns each interleaved; queries chat_a
with limit=50; asserts exactly 50 chat_a rows returned (was 0 prior to
the fix because chat_b's rows dominated the global tail).
Extends record_turn_memory_for_present with a you_present: bool = True
kwarg so a single entry-point covers both you-scenes (witness_you=1)
and meanwhile scenes (witness_you=0). Validates that meanwhile callers
provide a guest_bot_id.
record_meanwhile_memory becomes a thin backward-compat wrapper that
delegates with you_present=False, preserving the call site in
chat/web/meanwhile.py without churn.
Cosmetic-only renumbering of the event-lifecycle detection block in
``regenerate_assistant_turn`` from ``# 10.`` to ``# 9a.`` — mirrors the
``# 8a.`` shape in ``chat.web.turns.post_turn``. The block was already
in the correct structural position (immediately after the interjection
branch); only the numbering and comment reflected an earlier draft
where it read as a final step rather than the post-interjection /
pre-(absent)-scene-close slot.
No behavioural change. All 9 regenerate tests + 18 turn_flow tests
pass without modification.
When a regenerate replaces an assistant_turn that already produced
lifecycle transitions (``event_started`` / ``event_completed`` /
``event_cancelled``), those transitions are NOT rolled back before
``detect_event_transitions`` re-runs against the new text. A
regenerate-after-completion can therefore double-emit promotion
artifacts.
Phase 3.5 first cut (per the task plan): documentation + WARNING log
naming the affected event_log ids. The actual undo pass is invasive
(re-projection / inverse-handler dispatch) and is deferred to Phase 4.
Implementation:
- TODO docstring block at the top of ``regenerate_assistant_turn``.
- Module-level ``_log = logging.getLogger(__name__)``.
- Scan immediately after the original assistant_turn row is located:
joins event_log lifecycle rows to the events table on event_id so we
can scope by chat (lifecycle payloads carry only ``event_id``, not
``chat_id``). Filter ``id > original_assistant_event_id`` as the
positional linkage to "transitions emitted as part of (or after)
this turn's processing."
Decision (asked in the brief): the scan uses the ``id > original``
heuristic rather than scanning for explicit references. Lifecycle
event payloads do not carry a back-pointer to the assistant_turn that
triggered them — the linkage is positional in the event log. A tighter
linkage would require either adding a payload field on lifecycle
events (cross-cutting schema change) or threading the just-appended
assistant_turn id into ``detect_event_transitions``'s emit calls
(narrow but still cross-cutting). The positional heuristic is loose
but conservative: a turn that emits no lifecycle events produces no
warning, and the warning's purpose is operator-visible breadcrumbs
not an exact rollback set.
Test: test_regenerate_with_prior_lifecycle_logs_warning seeds a turn
that produced ``event_started`` + ``event_completed`` rows and asserts
the WARNING fires with the expected ids.
The sibling assistant_turn lookup in ``regenerate_assistant_turn``
previously scanned every non-superseded ``assistant_turn`` row across
the whole database and filtered in Python. With many chats in the log
this is O(total_assistant_turns) per regenerate.
Push the chat_id filter into SQL via ``json_extract(payload_json,
'$.chat_id') = ?`` and add ``ORDER BY id DESC LIMIT 50`` so worst-case
work is bounded even within a single chat. Mirrors the SQL pattern in
``chat.web.meanwhile._last_meanwhile_speaker``.
Test added: test_regenerate_sibling_lookup_scoped_to_chat seeds two
chats — the second has an interjection whose ``interjection_of`` value
collides with the first chat's primary speaker. Regenerating chat A
must leave chat B's rows untouched and the regenerated chat A
interjection's ``regenerated_from`` must point at chat A's original
(not chat B's). Pre-T83.3 a global query could in principle latch
onto cross-chat rows.
The recent-dialogue read and the directed-pair edge gather were
duplicated between ``chat.services.regenerate`` and ``chat.web.turns``.
Extracted into ``chat.services.turn_common`` with two helpers:
- ``read_recent_dialogue(conn, chat_id, *, limit, exclude_event_id)``:
oldest-first ``[{speaker, text}]`` over user_turn / user_turn_edit /
assistant_turn rows, with the standard ``superseded_by IS NULL AND
hidden = 0`` filter. ``exclude_event_id`` covers regenerate's need to
drop the original assistant_turn before its supersede UPDATE lands.
- ``gather_prior_edges(conn, present_ids)``: ``{(src, tgt): edge}`` over
every directed pair across ``present_ids``, with the schema default
50/50 baseline for missing rows.
``chat.web.turns._read_recent_dialogue`` becomes a thin delegate so the
chat-detail template and other in-module callers keep their import
shape; ``_gather_state_update_inputs`` now calls into the shared edge
gather. ``regenerate_assistant_turn`` calls both helpers in three call
sites (primary + post-interjection edges, primary + interjection
recent reads), still post-processing speaker ids to display names for
its prompts.
Decision: ``chat.services.scene_summarize._read_recent_dialogue`` is
left in place — it has a ``since_event_id`` clamp (T80.2) and excludes
``user_turn_edit`` deliberately. Folding it into the shared helper
would either silently change its read shape or require a second flag,
both more invasive than the duplication. Documented in the new module
docstring.
Tests: tests/test_turn_common.py covers chronological ordering,
supersede / other-chat / exclude_event_id filtering, and prior-edge
default-fallback. Existing 6 regenerate + 18 turn_flow tests pass
unchanged.
Both the primary and the interjection sub-stream in
``regenerate_assistant_turn`` are now wrapped in ``asyncio.create_task``
and registered in the chat-keyed ``_in_flight_tasks`` registry that the
``/turns/cancel`` route reads. Without this, hitting Stop during a
mid-regenerate stream was a no-op.
Mirrors the meanwhile registration pattern in chat/web/meanwhile.py
(snapshot-tested by tests/test_meanwhile_turn_flow.py).
Test added: test_regenerate_registers_task_in_in_flight_tasks captures
``"chat_bot_a" in _in_flight_tasks`` at the first stream yield via a
custom MockLLMClient subclass and asserts post-flight cleanup.
The natural-language skip dispatch in chat.web.turns.post_turn
(intent="skip_elision") previously bypassed scene close detection
entirely. User prose like "fade out, skip an hour" carries both a
close signal and a skip directive — the close summary must capture
the closing scene's final beat (and promote per-POV memories) before
the time advances.
Insert detect_scene_close + apply_scene_close_summary BEFORE the skip
controller invocation in the skip_elision branch. Order: scene close
-> skip narration -> time advance. When there's no active scene or
the prose carries no close signal, detect_scene_close returns the
safe should_close=False default and the flow drops straight to the
skip controller — same behavior as today.
Wire chat.services.prompt.consume_pending_meanwhile_digests into
chat.web.turns.post_turn at the END of the handler, after scene-close
detection and before the response broadcast. Without this call digests
created by a meanwhile close stay pending forever — they surface in the
next you-turn's prompt (via T65) but are never marked consumed, so they
re-render on every subsequent turn.
Idempotent: re-calling the helper produces zero events when nothing's
pending. The T66 cross-feature note is updated to reflect the new
wiring; the existing direct-helper test in test_phase3_integration.py
is preserved as defensive coverage of the helper contract in isolation.
T58 stamped emitted ``thread_closed`` events with
``datetime.now(timezone.utc).isoformat()``. The rest of the close
pipeline (memories.chat_clock_at, scene_closed.ended_at, edge writes)
uses the chat's in-world clock. Threads must agree so timeline
reconstruction stays consistent under time skips and replay.
Read ``chat["time"]`` (already loaded for the per-POV path) and pass
it through as ``closed_at``. Falls back to UTC now only when chat_state
has no clock yet — defensive; chat_created always seeds it.
Adds test_thread_closed_uses_chat_clock_time.
The broad ``except Exception`` around detect_threads silently dropped
programmer errors (wrong kwargs, import-time failures, etc), making
diagnostics painful. Log at DEBUG with full exc_info so the failure
surfaces in local logs without breaking the close pipeline's
failure-tolerant contract.
Adds test_detect_threads_failure_is_logged using caplog.
apply_scene_close_summary fed detect_threads the chat-wide last-50
turns. When a chat has accumulated multiple scenes' worth of dialogue,
that bleeds prior-scene turns into the second close's classifier prompt
and risks mis-attributing threads (closing one that opened earlier,
re-opening one that already closed).
Add an optional ``since_event_id`` kwarg to ``_read_recent_dialogue``
that lower-bounds by event_log id, plus a ``_scene_opened_event_id``
helper that resolves the scene-open event for a given scene_id. Wire
both into the thread-detection call site so its scene_transcript
holds only the closing scene's turns. The per-POV summarizer keeps the
chat-wide approximation it had before — that's intentional.
Adds test_thread_detection_uses_scene_scoped_transcript.
Re-running apply_scene_close_summary on the same scene previously caused
recursive bloat: _build_key_quotes_suffix sourced quote text from
memories.pov_summary, which after the first close already carried a
"Key quotes:" suffix. The next close would then quote the quotes,
nesting deeper each time.
Strip any existing suffix from candidate text before truncating to
200 chars in the suffix builder, and from the fresh classifier output
before composing the new value in _summarize_and_apply_for_witness so
the rewrite replaces rather than stacks.
Adds test_scene_close_re_run_does_not_double_suffix.
Extend ParsedTurn with intent/landing_state_hint so the classifier can
flag skip-elision and skip-jump prose. The post_turn handler short-
circuits the regular narrative path when intent != "narrative":
elision runs through the shared controller in chat/web/skip.py;
jump returns 422 directing the user to the drawer's structured form
(simpler Phase 3 path — natural-language fiction-time delta parsing
is too fragile for v1 without a structured surface).
Extract the elision/jump logic that previously lived in drawer.py
into chat/web/skip.py so both the drawer T59 routes and the new
natural-language path share one canonical implementation. The drawer
routes become thin HTTP wrappers that translate ValueError to 400
and refresh the drawer partial; the existing drawer skip tests pass
unchanged.
The new natural-language elision derives ``new_time`` by bumping the
chat clock by 1 hour (Phase 3 stub) — the drawer's structured form
remains the path for picking a specific landing time.
Joseph Doherty