fix(amp): stop suppressing thinking blocks in streaming mode

[jveko]

Summary

• Remove unconditional suppressAmpThinking call from rewriteStreamEvent in streaming mode
• Thinking blocks now pass through with signature injection via ensureAmpSignature, matching ampcode.com proxy behavior
• Non-streaming responses still suppress thinking when tool_use is present via rewriteModelInResponse

Problem

When using a local provider (e.g., antigravity) for claude-opus-4-6 with extended thinking, the Amp TUI crashes with undefined is not an object (evaluating 'r.type'). The streaming thinking suppression strips content_block_start and thinking_delta events but leaves signature_delta events orphaned, breaking SSE index alignment.

Test plan

• Verify local provider path with thinking-enabled models no longer crashes Amp TUI
• Verify ampcode.com proxy path still works unchanged
• Verify non-streaming responses still suppress thinking blocks when tool_use is present

Closes #2471

[gemini-code-assist]

Code Review

This pull request modifies the SSE stream processing in response_rewriter.go to stop suppressing thinking blocks, ensuring they are passed through with signature injection to maintain SSE index alignment and TUI rendering. Feedback suggests cleaning up several components of the ResponseRewriter struct and associated methods that have become dead code as a result of this change.

[gemini-code-assist]

Removing the call to suppressAmpThinking from the streaming path makes several components of the ResponseRewriter dead code, as they were exclusively used for tracking and suppressing streaming content blocks. Specifically, the suppressedContentBlock map (line 23), its initialization (line 32), the markSuppressedContentBlock/isSuppressedContentBlock methods (lines 157-167), and the streaming-specific logic in suppressAmpThinking (lines 187-205) appear to be no longer reachable. Consider cleaning these up to improve maintainability.

[xkonjin]

Review: Stop Suppressing Thinking Blocks in Streaming Mode

This fixes a real bug - suppressing thinking blocks was breaking SSE stream alignment which would cause client parsing errors. Good catch.

Strengths:

• The root cause analysis in the PR description is accurate
• Minimal, surgical change - just removes the suppression logic
• Updated comment documents the intentional passthrough behavior

Verification needed:

1. Confirm signature injection still works: The ensureAmpSignature() call happens after the suppression was removed - verify thinking blocks still get proper signatures when amp-signature-required is set

2. Edge case: What happens if a thinking block arrives as the last chunk? The stream should still terminate correctly - worth a test case.

Code review:

• The removed nil-check pattern was used in two places (event rewrite and data rewrite) - both cleaned up correctly
• Comment update is helpful context for future maintainers

Test suggestion: Add a test case with a stream containing thinking blocks interleaved with content blocks to verify index alignment is preserved.

Simple fix for a subtle bug. LGTM.

[jveko]

Thanks for the review @xkonjin! Addressed both points:

1. Signature injection: Added TestRewriteStreamChunk_PassesThinkingBlocksWithSignatureInjection — confirms ensureAmpSignature injects "signature":"" on thinking blocks after suppression removal.

2. Interleaved thinking + content: Added TestRewriteStreamChunk_ThinkingInterleavedWithContent_PreservesIndices — verifies index 0 (thinking) and index 1 (text) both pass through with all event types (thinking_delta, signature_delta, text_delta).

3. Thinking as last chunk (edge case): Added TestRewriteStreamChunk_ThinkingBlockAsLastChunk — verifies stream terminates correctly with message_delta and message_stop when thinking is the final content block.

All tests passing: dd9bc3e

[jveko]

Closing in favor of #2474 which includes this same streaming thinking passthrough fix plus additional Gemini-related fixes (tool schema sanitization, trailing unanswered function call stripping).