Retrieving specific field for TextBlock and ThinkingBlock

[tolstovdmit]

Description

Instead of attr check, checked type. Also changed getattr to field call Issue

Related Issue

Fixes #161

Type of Change

• Bug fix

Testing

• Manual testing completed

Checklist

• Code follows project style guidelines
• Self-review completed
• Documentation updated
• No breaking changes (or clearly documented)

[RichardAtCT]

Good fix overall — the isinstance approach is strictly better than hasattr for typed SDK objects, and the direct field access on ToolUseBlock is safe since these are typed dataclasses/Pydantic models where the fields are guaranteed present.

A few things worth considering:

ThinkingBlock → text_parts: debatable

Appending block.thinking to text_parts means thinking content gets surfaced to users alongside regular text. Depending on how text_parts is consumed downstream (e.g. sent directly to Telegram), this could expose raw chain-of-thought in user-facing messages. Is that intentional? A common pattern is to either discard thinking blocks or accumulate them separately (e.g. thinking_parts) and only log/debug them. Worth a comment in the code at minimum confirming this is deliberate.

ToolUseBlock direct access

The switch from getattr(block, "name", "unknown") to block.name is correct if the SDK guarantees these fields are always populated. If the SDK can return a partial/streaming ToolUseBlock with name=None mid-stream, you've traded a safe default for a potential None propagation. Quick sanity check: does the SDK type stub for ToolUseBlock.name show str or Optional[str]? If Optional, the old defensive defaults were load-bearing.

Missing types

If the SDK has other block types (e.g. ImageBlock, future variants), they'll silently be dropped here. That's probably fine for now, but a final else: logger.debug("unhandled block type", block_type=type(block).__name__) would make future debugging much easier.

Tests

PR body says "manual testing completed" but no automated test changes. Given this fixes a crash/regression (#161), a unit test with a mock ThinkingBlock and TextBlock would prevent it regressing. Worth a nudge.

Bottom line: Merge-worthy with the ThinkingBlock behavior either confirmed intentional or adjusted. The isinstance refactor is the right call.

— Friday, AI assistant to @RichardAtCT (posted as @RichardAtCT — FridayOpenClawBot access pending)