Reloaded transcript inverts prose/tool-output order on tool-using turns #13295

Activity

1. mrveiss

   added

   bugSomething isn't workingSomething isn't working

   on Aug 2, 2026
2. github-actions

   added

   testing

   on Aug 2, 2026
3. mrveiss

   added 5 commits that reference this issue on Aug 2, 2026

   fix(chat): address review blockers on #13303 - revert bad ordering fi…

   ...

   1610f26

   fix(chat): reload fidelity for streamed replies — model/sources, turn…

   ...

   36c96b9

   fix(chat): tag every workflow message on the graph path, not only ter…

   ...

   63184de

   fix(chat): carry model and sources when no prose entry exists, drop b…

   ...

   796e95b

   fix(chat): interleave per-iteration prose with tool output on reload (#…

   ...

   53d455e
4. github-actions commented on Aug 3, 2026

   

   github-actionsbot

   on Aug 3, 2026 – with GitHub Actions

   Contributor

   More actions

   PR #13334 (merged to Dev_new_gui) references this issue with a close keyword.

   fix(chat): interleave per-iteration prose with tool output on reload (#13295)

   If this issue is fully resolved, close it manually. If work remains, no action is needed.
5. mrveiss commented on Aug 3, 2026

   

   mrveiss

   on Aug 3, 2026

   OwnerAuthor

   More actions

   Fixed by PR #13334, squash-merged to Dev_new_gui as 53d455ea6.

   This took three attempts, each blocked for a different false premise. Recording all three so a fourth does not repeat them.

   1. PR fix(chat): reload fidelity for streamed replies — model/sources, turn order, backfill (#13292, #13293, #13295, #13296) #13303 inserted the reply at the front of the batch, assuming it is chronologically first. False for the common case: a tool-using turn normally runs two iterations (manager.py:2841; _check_continuation_decision at :2515-2555 returns True after a normal tool execution), each appending its own prose, all collapsed by "\n\n".join(all_llm_responses). Front-insertion put the final answer before the tool output it was derived from.
   2. The first version of fix(chat): interleave per-iteration prose with tool output on reload (#13295) #13334 stamped metadata["iteration"] only for _TERMINAL_MESSAGE_TYPES. But the production path is LangGraph (manager.py:3637-3645 — legacy is only an except fallback), and the graph accumulates every non-streaming message. Nine other types went untagged, fell through to leftover, and landed after the answer — worse than base — while the untagged respond message became invisible to the dedup scan and produced a duplicated assistant turn, regressing bug(chat): streamed AutoBot replies are never persisted — session history stores user turns only, so every reply vanishes on reload #13214's single-reply invariant.
   3. The second version attached the model badge and KB sources only to the final iteration's prose entry. On a respond-tool turn every iteration's prose is deduped away, so no entry carried them — silently regressing bug(chat): reloaded streamed replies lose their KB citations — persisted assistant turn has empty sources #13292 on exactly the path the PR's own new test exercises.

   What landed

   Per-iteration interleaving: _build_persist_batch walks iterations 1..N emitting prose first, then that iteration's own tool entries, matching the live order (_run_continuation_iteration fully drains the prose generator before dispatching tool calls).

   Tagging is now unconditional at each yield in _process_tool_results and _collect_llm_iteration_response, plus graph.execute_tools — which calls manager._process_tool_calls directly and bypasses _handle_tool_message_types entirely — with metadata keys added to deny_msg and abort_msg, which are plain dicts.

   Two knock-on fixes the review surfaced:

   • Iteration↔index desync. all_responses.append(llm_response) was guarded by if llm_response: while iteration incremented unconditionally, so one falsy prose shifted every later iteration and filed iteration i+1's tool output under iteration i's prose.
   • Blank segments leaking into the joined prose. Fixing the desync makes all_llm_responses carry "", which would otherwise put a leading or interior "\n\n" into conversation_history — which feeds the next turn's LLM context — plus the transcript file and two hooks.

   Verification

   The decisive change was the end-to-end test: it monkeypatches only the innermost LLM-call and tool-dispatch primitives and lets _process_tool_results, _handle_tool_message_types, _run_llm_iteration, _build_workflow_messages_from_state and _persist_workflow_messages all run for real.

   That matters because the earlier versions' tests hand-wrote metadata={"iteration": 1} on a response message — a tag production never applies — so they asserted the implementation back to itself, and both blockers survived review.

   Independent probe pushing all 15 message types through the real accumulator: TOTAL accumulated: 16 UNTAGGED: []. respond-tool turn end to end: exactly 2 assistant entries, no duplicate. Falsy iteration-1 prose: tool output correctly filed under iteration 1.

   443 passed across chat_workflow/ and chat_history/. Exactly one add_messages_batch call site remains, preserving #13214's single-writer property.

   Related, not fixed

   _filter_internal_prompts (#716/#11867) runs only on the legacy path; graph.py::persist_conversation never filters, so the internal-prompt strip has never applied to the production path's persisted prose. And manager.py:2878-2882 appends to all_llm_responses before _emit_after_continuation, discarding that hook's edit on the legacy path.

   Closing manually with evidence — Closes #N only fires on the default branch, and this PR targeted Dev_new_gui.
6. mrveiss

   closed this as completedon Aug 3, 2026