Skip to content

refactor(conversation_loop): extract turn persistence slice CL-R5-1 into agent/turn_finalizer.py - #84583

Open
andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:gfg/conversation-loop-r51-turn-finalizer
Open

refactor(conversation_loop): extract turn persistence slice CL-R5-1 into agent/turn_finalizer.py#84583
andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:gfg/conversation-loop-r51-turn-finalizer

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Blind extraction of slice CL-R5-1 from agent/conversation_loop.py (7,757 lines at pin ee4bb75b532e932a1055d9a710802a7435163b6a) into the existing agent/turn_finalizer.py module, per the repo-wide god-file sharding policy.

  • Moved: the final-message append + best-effort SessionDB flush/warning block (lines 7615–7632, 18 lines) → NEW helper persist_completed_text_turn in agent/turn_finalizer.py (keyword-only agent, messages, conversation_history, final_msg → None)
  • Golden sha (window at pin): 7fd77e562363aaee1d11a39b71f248218f5dc26a8f9b78cab3e1d3b527429b33
  • Seam (sanctioned non-byte-verbatim function-tail seam): preserves the exact append → flush → catch → warning order with best-effort failure semantics; the moved body is byte-identical to the pinned window modulo indentation — INCLUDING the 8-line [Bug]: Completed-turn assistant replies are never persisted to state.db when the session disconnects/reaps — lost after Desktop restart (v0.20.0, remote serve over Tailscale / direct IP) #81641 comment block (7616–7623) restored by the fix commit. Caller-owned control flow intact: _turn_exit_reason (7634), quiet-mode completion print (7635–7636), break (7637), finalize_turn, and __all__ all remain in run_conversation. Logger resolves through the module's established lazy-import seam (same agent.conversation_loop logger identity, same warning text, exc_info=True). No import cycle (the pin already imports turn_finalizer at line 91 for finalize_turn); startup-latency contract held.
  • Seam tests: tests/run_agent/test_turn_finalizer_seam.py — runtime behavioral probes (append-before-flush ordering, flush-failure warning-only, message/history identity, caller-owned control flow, import/patch transparency, logger/traceback semantics); no source-reading tests.
  • Zero behavior change. Diff: agent/conversation_loop.py 25 changed (block removed + import + call); helper added to the existing module; seam test 304 lines.

Method

5×2×3 double-blind decomposition (per the All Gods Must Die mandate): 5 blind region analysts → 5 blind adversarial witnesses → 5 consensus adjudicators → blind implementer → 2 blind re-reviewers. Round 1: reviewer 1 REQUEST CHANGES (sole blocker: the golden-byte guard — the pinned window's 8-line #81641 comment block was dropped from the moved helper body; executable statements were byte-identical but the guard is strict); reviewer 2 APPROVED. Fix lane restored the 8 comment lines (commit e277fcbc274, module only). Round 2: both re-reviewers APPROVED:

  • Review 1 (r2): C:/tmp/tg-Feature Package/conversation-loop/review/CLR51-review-1-r2.md (13,140 B) — all 9 gates PASS
  • Review 2 (r2): C:/tmp/tg-Feature Package/conversation-loop/review/CLR51-review-2-r2.md (13,090 B) — APPROVED, all gates (sole suite delta: a re-verified flaky race test in an untouched file)

Suite evidence: pristine-pin vs post-extraction failure sets identical (full tests/run_agent both sides; seam 14/14; #81641 persistence tests 22/22). No new failures.

Coordination table

Item Value
Pin ee4bb75b532e932a1055d9a710802a7435163b6a (origin/main)
Slice CL-R5-1 (conversation_loop region 5, first slice)
Window 7615–7632 (18 lines)
Module agent/turn_finalizer.py (existing — helper added)
Golden sha 7fd77e562363aaee1d11a39b71f248218f5dc26a8f9b78cab3e1d3b527429b33
Colliders #83437 (langfuse tracing) — live file-list check at extraction: no hunks in 7615–7632; LOW-to-MEDIUM relative semantic risk (SessionDB persistence) noted per contract. Siblings #84275, #84310, #84330, #84473 — no hunks in window
Dependencies none
Conflicts none
Merge position standalone; no stacking

Dedup statement

No prior extraction of this window exists. No duplicate work.

Credit

  • Author: Axl Ibiza, MBA (DCO-signed commits 9b9f91b0a83 + e277fcbc274)
  • Method: All Gods Must Die 5×2×3 (blind lanes, consensus contracts, blind re-review, fix cycle)

This slice is governed by the conversation_loop (posted on #78641). Former whole: 7,757 lines. Fixer roster: #83437.

Part of #78641
Part of #78647

…e CL-R5-1 into agent/turn_finalizer

Extract the final-message append + best-effort SessionDB flush/warning
block (window 7615-7632) from run_conversation into the lightweight
helper persist_completed_text_turn in agent/turn_finalizer.py.

Behavioral neutrality: exact append -> flush -> catch -> warning order,
best-effort failure semantics (flush failure must not abort the turn),
logger identity (agent.conversation_loop) + exc_info=True preserved.
Caller retains _turn_exit_reason assignment, quiet-mode completion print,
loop break, finalize_turn, and __all__.

The helper is dependency-injected (agent, messages, conversation_history,
final_msg) and resolves logger through the module's established lazy-import
seam, so no import cycle and no startup-latency cost. AST-normalized
comparison of the moved body against the pinned slice is identical.

Adds focused seam tests (append-before-flush ordering, flush-failure
warning-only, message/history identity, caller-owned control flow,
import/patch transparency, logger/traceback semantics) with no
source-reading.

Signed-off-by: Hermes Agent <hermes@nousresearch.com>
Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
…alizer helper

Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
Contributor email mapping required by check-attribution CI for the
god-file kill campaign PRs (Axl Ibiza, MBA <andrexibiza@gmail.com>).

Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 12, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

refactor(conversation_loop): extract turn persistence slice CL-R5-1 into agent/turn_finalizer.py

Faithful extraction; ordering and failure semantics are preserved and pinned by tests. Observations:

  1. "Never raises" contract is broader than the implementationagent/turn_finalizer.py (persist_completed_text_turn) docstring states the helper "never raises," but only the _flush_messages_to_session_db call is guarded. messages.append(final_msg) (raises if the caller passes a tuple/frozen list) and the lazy from agent.conversation_loop import logger (raises if import fails) are unguarded. Suggest narrowing the documented contract to "never raises on flush failure" to match reality.

  2. No type hints on a new public seam — the extracted function takes agent, messages, conversation_history, final_msg untyped. The surrounding codebase uses hints; adding them (or at least list[dict] for the message containers) would make the append-in-place contract explicit and would have caught the tuple case above.

  3. test_warning_text_is_byte_identical_to_original pins a literal log string — acceptable since ops may grep this message, but note it is a change-detector in the AGENTS.md sense; if the wording ever changes deliberately the test must be updated in lockstep.

  4. Lazy logger import inside the function — correct for cycle avoidance, but it means the module cannot log anything if agent.conversation_loop was never imported; harmless here since the caller always imports it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants