Stop hiding the thinking process

[rumpl]

Seeing the thinking process of an LLM is actually quite useful, I want to be able to steer it while it's thinking but I can't if I don't see what the agent is doing while thinking.

[krissetto]

the UX gods disapprove

[docker-agent]

Assessment: 🟢 APPROVE

The changes cleanly remove the collapse/fade machinery from reasoning blocks and make the thinking process always visible. The logic is sound:

• renderExpanded correctly handles blank-line spacing for all first-item combinations (reasoning or tool)
• SetSize, Update, and Init patterns correctly propagate to tool sub-views via interface pointers
• Removed code (cache invalidation, fade animation, collapsed rendering) is cleanly excised with no dangling references
• Test coverage updated appropriately to reflect the new always-expanded behavior

No bugs were found in the changed code.

[docker-agent]

Assessment: 🟢 APPROVE

This PR correctly simplifies the reasoning block by removing the collapse/expand toggle and all associated animation, fade, and cache logic. The always-expanded behaviour is coherent and the dead-code removal is clean.

Three low-severity observations are left as inline comments. None are blockers.

[docker-agent]

[LOW] Dead field: animationRegistered is retained but never set to true

animationRegistered is kept in the struct (with its comment "whether we're registered with animation coordinator"), but all code that ever sets it to true was deleted in this PR. StopAnimation() still guards animation.Unregister() behind a check on this field, so unregistration will never fire from the reasoningBlock path. This is currently harmless (the block now has no animation to unregister), but the retained field and its comment are misleading for future contributors and form a latent trap: if anyone re-adds animation registration without also setting the flag, the unregister will silently be skipped and leak a subscription.

Consider either removing the field entirely (since it can never be true in the new design) or adding a //nolint / explanatory comment clarifying it is intentionally always false.

[docker-agent]

[LOW] Misleading variable names: beforeIdx/toolIdx/afterIdx hold bool, not integer indices

beforeIdx := assert.Contains(t, stripped, "Before tool")
toolIdx   := assert.Contains(t, stripped, "read_file")
afterIdx  := assert.Contains(t, stripped, "After tool")
assert.True(t, beforeIdx && toolIdx && afterIdx)

assert.Contains returns bool (pass/fail), not an index. The *Idx suffix implies an integer position, which can mislead a reader into thinking the test is also verifying ordering (e.g., beforeIdx < toolIdx < afterIdx). The test only checks presence, not order.

If ordering matters for the "interleaves" contract, consider asserting positions via strings.Index. If ordering doesn't matter, rename to beforeFound/toolFound/afterFound to match what the variables actually contain.

[docker-agent]

[LOW] SetSize result discarded when iterating toolEntries by value

view.SetSize(m.contentWidth(), 0)
m.toolEntries[i] = entry
m.toolEntries[i].view = view

view.SetSize() returns a tea.Cmd that may carry side-effecting messages (e.g., spinner ticks, resize events). The return value is silently dropped here. Current tool implementations happen to return nil or mutate in place, so no commands are lost today — but this is a subtle contract violation that could cause silent breakage if any future tool view returns a non-nil Cmd from SetSize.

Consider collecting and returning (or batching) the Cmd from SetSize, consistent with how Update chains commands elsewhere in this file.