Skip to content

refactor(conversation_loop): extract content normalization slice CL-R4-1 into agent/response_normalization.py - #84473

Open
andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:gfg/conversation-loop-r41-response-norm
Open

refactor(conversation_loop): extract content normalization slice CL-R4-1 into agent/response_normalization.py#84473
andrexibiza wants to merge 3 commits into
NousResearch:mainfrom
andrexibiza:gfg/conversation-loop-r41-response-norm

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Blind extraction of slice CL-R4-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 inline content-shape normalization band (lines 6082–6101, 1,236 bytes) → NEW helper normalize_assistant_content in agent/response_normalization.py
  • Golden sha (window at pin): af3d33d2a9e213f9f41b1ebec05785359a5455e523ae38578f0b4c0cb3ef6b68
  • Seam (sanctioned non-byte-verbatim — inline band becomes a callable): the helper mutates the supplied assistant_message IN PLACE (same object reaches the lifecycle hook — no copy/replacement); preserves exactly the dict precedence (raw.get("text","") or raw.get("content","") or json.dumps(raw)), list part handling (strings appended directly, type=="text" dicts append part.get("text",""), other dicts containing text append str(part["text"]), others ignored, "\n" join), scalar str(raw), exception/mutation timing, and no side effects. Call site at the identical response-processing location (after normalize_response, before post_api_request); the post_api_request hook band (6103–6143) is NOT absorbed. No false re-export; run_conversation identity, monkeypatch surfaces, and source scanners preserved; json binding owned by the new module; startup-latency contract held (one stdlib import + function def).
  • Seam tests: tests/agent/test_response_normalization_seam.py — runtime behavioral probes (normalization cases, in-place mutation/object identity, lifecycle-hook ordering via runtime spy, .strip() safety, import identity, monkeypatch transparency)
  • Zero behavior change. Diff: agent/conversation_loop.py 22 changed (inline band removed + call + deferred import); new module 21 lines; seam test 115 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: BOTH reviewers REQUEST CHANGES (committed seam test violated the test-integrity gate — inspect.getsource/ast.parse/Path.read_text/subprocess, the repo's banned source-reading-test antipattern). Fix lane replaced them with runtime behavioral probes (commit 1a2e6330a8e, test file only). Round 2: both re-reviewers APPROVED:

  • Review 1 (r2): C:/tmp/tg-Feature Package/conversation-loop/review/CLR41-review-1-r2.md (17,942 B) — all gates PASS
  • Review 2 (r2): C:/tmp/tg-Feature Package/conversation-loop/review/CLR41-review-2-r2.md (15,232 B) — APPROVED, all gates

Suite evidence: pristine-pin vs post-extraction failure sets identical (canonical subset 272 passed both sides; seam 6/6). No new failures.

Coordination table

Item Value
Pin ee4bb75b532e932a1055d9a710802a7435163b6a (origin/main)
Slice CL-R4-1 (conversation_loop region 4, first slice)
Window 6082–6101 (20 lines, 1,236 bytes)
Module agent/response_normalization.py
Golden sha af3d33d2a9e213f9f41b1ebec05785359a5455e523ae38578f0b4c0cb3ef6b68
Colliders #83437 (langfuse tracing) — live file-list check at extraction: no hunks in 6082–6101; the 6103–6143 hook band (primary adjacency) untouched. Siblings #84275, #84310, #84330 — 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 614f7b27a36 + 1a2e6330a8e)
  • 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, MBA <andrexibiza@gmail.com>
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>
@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 content normalization slice CL-R4-1 into agent/response_normalization.py

Clean, behavior-preserving extraction with unusually thorough seam tests. A few observations:

  1. Function-local import on the hot pathagent/conversation_loop.py (in _perform_api_call): from agent.response_normalization import normalize_assistant_content runs on every API call. The import machinery caches it, but the attribute lookup still costs per call and the local import makes the module-level import graph harder to reason about. Since agent.response_normalization has no import-cycle risk, a module-level import would be simpler.

  2. Test fixture purges sys.modulestests/agent/test_response_normalization_seam.py (agent_env fixture) deletes every module whose name is run_agent or starts with agent. / tools. / hermes_ and re-imports run_agent. This works under the repo's subprocess-per-file test runner, but it is aggressive: any other in-flight reference to those module objects (e.g., a monkeypatched callback captured from an earlier import) silently diverges. Consider importing the module under test once and relying on importlib.reload only for the module actually being exercised.

  3. Mutable class-level state on the mock server_MockHandler.captured_requests / response_queue are class attributes shared across tests in the file. The fixture resets them, but a future test that forgets will leak state; instance attributes created in setup would be safer.

  4. test_run_conversation_import_identity_and_monkeypatch_transparency asserts not hasattr(conversation_loop, "normalize_assistant_content") — this is an extraction-boundary check that will fail the moment the (suggested) module-level import is hoisted. It documents intent, but it also bakes in the current "function-local import" implementation detail; consider loosening it if the import is moved.

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