fix(gemini): split mixed user turns and render multiline initial text

[WSxDemise]

Important

Related PR — please merge #910 as well: #910

This PR does not fix streamed Markdown cleanup at EOF. That fix belongs to
#910 by @wkramme, which touches the same region of event_stream_handler.py.
Without it, a provider that closes its stream without a final PartEndEvent
still drops the last buffered line and any unclosed code fence.

This PR is not blocked by #910 and does not need to wait for it — the two
are complementary and independently mergeable, in either order. Verified by
merging #910 into this branch locally: clean auto-merge, no conflicts, and
119 tests pass on the combined tree with _finish_text_part and the
end-of-stream flush each appearing exactly once (no double-flush).

Merging only this PR leaves the EOF truncation bug in place, so please land
both.

What

Two Gemini fixes that share a file, kept together so the internal Walmart fork and this repo stay byte-identical here.

1. /steer and interrupts no longer trigger HTTP 400s.
Gemini rejects a single user turn containing both a tool result and text. _map_messages merged consecutive user turns unconditionally, so whenever a tool finished and the user typed something before the model replied, the two got fused into one turn Gemini refused. The request failed outright. They are now emitted as two turns, tool result first.

This is not steer-specific and not Walmart-specific — a ToolReturnPart followed by a UserPromptPart is enough to reproduce it on main.

2. Multiline Markdown renders as Markdown.
A provider that does not stream can deliver an entire response in one PartStartEvent. That content was handed to Termflow as a single line, so headings and horizontal rules stayed on screen as literal ### and ---. Initial text now goes through the same per-line path that streamed deltas already used.

Why

Both are provider-shape mismatches: the request shape Gemini accepts, and the assumption that initial text is one line. Symptoms were user-visible — a hard 400 when steering or interrupting, and raw ### in the terminal.

How

• gemini_model.py: new _split_mixed_user_contents() helper, called at the end of _map_messages, splits any user turn holding both a function_response and other parts.
• event_stream_handler.py: new _render_text_content() shared by the initial-text and text-delta paths.

Testing

• tests/test_gemini_model_full_coverage.py and tests/agents/test_event_stream_handler.py: 117 passed
• Full suite on this branch: 7415 passed, 72 failed
• The same 72 fail identically on unmodified main in this environment (missing optional acp/playwright extras); a sorted diff of the two failure lists is empty, so this branch introduces none of them
• ruff check and ruff format --check clean on all four files

Scope

Two production files and two test files. No dependency or lockfile changes.

Deliberately excludes the end-of-stream flush for providers that never emit
PartEndEvent; that is #910's fix, and duplicating it here would mean two
copies of the same helper. See the note at the top — #910 and #914 are both
still needed for full Gemini correctness.

[WSxDemise]

[Wes's CodePuppy Agent Review]

Summary - Multiline Markdown Rendering: Refactors event_stream_handler.py to route initial TextPart.content through _render_text_content(), parsing lines individually instead of passing multiline blocks to Termflow as a single line. - Gemini Mixed User Turns: Adds _split_mixed_user_contents() in gemini_model.py to separate function_response parts from prompt text into distinct user turns, avoiding Gemini 400 errors when steering or interrupting runs. - Tests: Comprehensive unit tests added for line-by-line parsing in test_event_stream_handler.py and mixed-content turn separation in test_gemini_model_full_coverage.py. CI checks are passing. #### Really Should Fix These None #### Nits None

[AndrewTilson]

Review: fix(gemini): split mixed user turns and render multiline initial text

Reviewed the full diff, read the surrounding code in both production files, and independently reproduced the test claims locally (Python 3.12, clean venv).

Verdict: LGTM — good to merge. Two genuinely user-visible bugs, small blast radius, real regression tests. A few non-blocking notes below.

---

Verification I ran myself

Check	Result
tests/test_gemini_model_full_coverage.py + tests/agents/test_event_stream_handler.py on this branch	117 passed (matches the PR description exactly)
The 3 new tests against main's production files	3 failed — these are real regression tests, not tests written to fit the fix
Full suite (minus tests/integration + test_bottom_bar_screen.py, which fail to collect on main too — missing acp / pyte)	7813 passed, 2 failed
Those 2 failures (tests/mcp/test_tool_failure_not_fatal.py)	Confirmed identical failures on main (mcp 2.x renamed FastMCP) — not introduced here
ruff check diff vs main on the four touched files	Zero new findings
ruff format --check	Clean

Sanity note on the description: ruff check isn't literally clean on these files (24 pre-existing SIM117/UP045/BLE001/RUF059 findings live there already), but the diff introduces none of them. Not this PR's job to fix.

---

What I like

_render_text_content is a proper DRY extraction, not a cosmetic one. The initial-text path was doing termflow_line_buffers[event.index] = part.content — assigning raw content straight into a buffer whose entire contract is "incomplete trailing line only". The buffer only ever gets drained by the while "\n" in buffer loop, which the assignment path skipped, so any non-streaming provider's whole response got shoved through parse_line() as one line at PartEndEvent. Two callers, one invariant, one function now owns it. Textbook.

The docstring earns its keep too — "one complete line at a time" is the invariant, stated once, in the one place that enforces it.

_split_mixed_user_contents is correctly placed. After the trailing-model-turn pop from 10dfea9 and before the empty-contents fallback — so it can't resurrect an empty contents or get undone by the pop. Ordering matters here and it's right.

The parametrized test is the right shape. same_request vs separate_requests covers both the "one ModelRequest with mixed parts" case and the "two requests merged by _map_messages" case, which are genuinely different code paths into the same bug. And asserting response_index < text_index guards the ordering contract rather than just the "no mixing" contract — that's the assertion most people would forget to write.

---

Non-blocking notes

1. Merge-then-unmerge is a slightly odd shape (design nit, not a defect).

_map_messages merges consecutive user turns unconditionally, then this helper walks the whole list and un-merges the ones that shouldn't have merged. The alternative — making the merge conditional at the source, i.e. don't extend into contents[-1] if it would mix a function_response with anything else — is arguably more direct.

That said, I think the chosen approach is defensible and probably the better call: the split is a single, testable, self-contained pass with a clear name, whereas conditional-merge logic would smear the rule across an already-busy loop and be much easier to break later. "Flat is better than nested." Just calling it out so it's a conscious choice.

2. Reordering when text precedes the tool result. The helper hoists all function_response parts ahead of all other parts. If a merged user turn ever ends up ordered text-first, the split silently reverses it:

>>> c = [{'role':'user','parts':[{'text':'first thing user said'},
...                              {'function_response':{'name':'shell','id':'c1'}}]}]
>>> _split_mixed_user_contents(c)
[{'role':'user','parts':[{'function_response': ...}]},
 {'role':'user','parts':[{'text':'first thing user said'}]}]   # order flipped

The comment says "Tool results first: they answer the model's preceding call" — which is exactly right for the /steer and interrupt cases this PR targets, and the parametrized test pins both. My question is whether text-before-response is reachable at all (a UserPromptPart request followed by a ToolReturnPart request with no model turn between). If it isn't, this is a non-issue and the current behaviour is correct. If it is, the flip could reorder a user's actual words relative to a tool result.

Cheap insurance either way: a one-line assertion or a third parametrize case pinning the text-first input, so the intent is documented rather than inferred.

3. Interleaved parts get regrouped, not just split. Related to the above — [resp_a, text_1, resp_b, text_2] becomes [resp_a, resp_b] + [text_1, text_2]. Relative order within each group is preserved, which is the part that matters, so I think this is fine. Worth a sentence in the docstring that the helper regroups rather than strictly partitions at a boundary.

4. In-place mutation via contents[:] = split, returning None. Works, and keeps the call site a clean one-liner. But -> None + slice-assignment is the kind of thing a future reader has to stop and squint at, and it makes the helper harder to test in isolation than contents = _split_mixed_user_contents(contents) would be. "Explicit is better than implicit." Genuinely minor — the function is eleven lines and the name says what it does.

5. Micro-nit: parts = content.get("parts", []) is bound before the role != "user" early-continue, so it's computed and discarded on every model turn. Moving it below the guard is a one-line tidy. Also, the local named split shadows the concept of the function itself; split_contents would read better. Both are truly cosmetic.

---

On the #910 dependency note

The header block is unusually good PR hygiene — stating that the two PRs are complementary, independently mergeable in either order, and that you verified the combined tree locally (clean auto-merge, 119 tests, _finish_text_part and the EOF flush each appearing exactly once) pre-empts the exact "will these conflict?" question a reviewer would otherwise have to go answer themselves.

I'd reinforce the point though: merging this one alone still leaves the EOF truncation bug live. Worth confirming #910 actually lands, because the failure mode after this PR (last line silently dropped, unclosed code fence) is subtle enough that it could sit unnoticed for a while.

---

Risk assessment: Low

• 143 additions / 18 deletions across 4 files, 2 of them tests.
• No dependency or lockfile changes.
• event_stream_handler.py change is a pure refactor on the delta path (byte-identical logic, verified line by line) plus a genuine behaviour fix on the initial-text path.
• gemini_model.py change is additive and no-ops (continues early) on every content that isn't a mixed user turn.
• Commits are cleanly split one-fix-per-commit with conventional-commit subjects. git bisect will thank you.

Nothing here blocks the merge. Nice, tightly-scoped bugfix.

_{Reviewed by Luna — Code Puppy}