Skip to content

refactor(conversation_loop): extract streaming spinner cleanup slice CL-R2-1 into agent/streaming_control.py - #84310

Open
andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:gfg/conversation-loop-r21-streaming
Open

andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:gfg/conversation-loop-r21-streaming

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

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

  • Moved: the NESTED run_conversation._stop_spinner (lines 2636–2642, 322 bytes, spinner/callback cleanup) → agent/streaming_control.py as an explicit-input helper _stop_spinner(thinking_spinner, thinking_callback) returning the updated spinner
  • Golden sha (window at pin): 23c462927eaa954adf9dedffb25bf4831843054899450a5baf10c2bff328ab18
  • Seam (sanctioned non-byte-verbatim — closure adaptation): the nested closure used nonlocal thinking_spinner and read agent.thinking_callback; the destination helper makes the closure inputs explicit. The call site now uses a small _on_first_delta adapter that rebinds the enclosing thinking_spinner through the module helper; on_first_delta=_on_first_delta preserves streaming timing and callback identity. Behavior exists in exactly ONE place (the module helper); the agent object is not passed (only agent.thinking_callback). Operation order, truthiness, callback ordering, and exception propagation preserved exactly.
  • Seam tests: tests/agent/test_streaming_control.py — runtime behavioral probes (spinner-present/callback-present, spinner-absent/callback-present, both absent, callback ordering, exception propagation, call-site seam via runtime probe)
  • Zero behavior change. Diff: agent/conversation_loop.py 13 changed (nested def removed + import + adapter); new module 10 lines; seam test 112 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: a committed test used inspect.getsource/ast.parse on run_conversation — the repo's banned source-reading-test antipattern); reviewer 2 APPROVED. Fix lane replaced it with a runtime behavioral probe (commit 46a41cdcd4e, test file only). Round 2: both re-reviewers APPROVED:

  • Review 1 (r2): C:/tmp/tg-Feature Package/conversation-loop/review/CLR21-review-1-r2.md (13,317 B) — all gates PASS
  • Review 2 (r2): C:/tmp/tg-Feature Package/conversation-loop/review/CLR21-review-2-r2.md (10,234 B) — APPROVED, all gates

Suite evidence: pristine-pin vs post-extraction failure sets identical (seam 6/6, canonical subset green). No new failures.

Coordination table

Item Value
Pin ee4bb75b532e932a1055d9a710802a7435163b6a (origin/main)
Slice CL-R2-1 (conversation_loop region 2, first slice)
Window 2636–2642 (nested def, 322 bytes)
Module agent/streaming_control.py
Golden sha 23c462927eaa954adf9dedffb25bf4831843054899450a5baf10c2bff328ab18
Colliders #83437 (langfuse tracing) — live file-list check at extraction: no hunks in 2636–2642; semantic overlap LOW (spinner/callback cleanup, no tracing/telemetry). Sibling #84275 (CL-R1-1) — 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 87d564757c6 + 46a41cdcd4e)
  • 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

Signed-off-by: Axl Ibiza <andrexibiza@gmail.com>
Signed-off-by: Axl Ibiza <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>
Signed-off-by: Axl Ibiza <andrexibiza@gmail.com>
@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 streaming spinner cleanup slice CL-R2-1 into agent/streaming_control.py

  1. Name undersells the helperagent/streaming_control.py:49: _stop_spinner(thinking_spinner, thinking_callback) also invokes the thinking callback (and returns the cleared spinner so the caller can rebind). The name reads as "just stop the spinner"; _stop_spinner_and_notify (or similar) would match the actual contract. Minor — byte-fidelity extraction is a fair justification for keeping the name.

  2. Exception propagation is pinned, which is the right calltest_stop_spinner_propagates_spinner_exception_and_skips_callback asserts that a spinner.stop() raising propagates and the callback is skipped. This matches the pre-extraction inline semantics exactly; good that it is locked in rather than silently "improved" during extraction.

  3. The runtime-composition test is a nice touchtest_streaming_adapter_stops_spinner_on_first_delta_at_runtime replicates the _on_first_delta closure pattern with the rebinding (thinking_spinner = _stop_spinner(...)), proving the extracted helper composes correctly with the caller's rebind contract. No blocking issues.

@andrexibiza

Copy link
Copy Markdown
Contributor Author

This streaming-cleanup extraction is tied to current head 42922b50fd5a1d4168f53ce5f14dfd1e6bd616fc. The nested run_conversation._stop_spinner behavior now has one implementation in streaming_control.py; _on_first_delta supplies the former closure state explicitly while retaining spinner timing, callback identity, operation order, truthiness, and exception propagation. The test update replaces a source-reading seam assertion with runtime probes for those behaviors, and the attribution mapping keeps both commit identities recognized by the repository's attribution check. The covered commits are 87d564757c64ad6ec01d8a08542f3e27e708e873, 46a41cdcd4e579dd0d904ff368ebe5e25cd49176, and 42922b50fd5a1d4168f53ce5f14dfd1e6bd616fc. Exact-head check rollup: SUCCESS.

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 type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants