Validate message content prevents LLM parsing failures

[ssevera1]

What

Add content validation to all message-adding methods to reject None or empty strings before they enter conversation history.

Why

Empty or None content can cause downstream LLM API parsing failures. Early validation at the conversation layer prevents invalid data from propagating.

[claude]

Thanks — early validation at the conversation layer is a reasonable goal, and the scope is tight (one file, one concern). But as written, _validate_content rejects a message shape that is both valid and routine, which will crash the agent on its most common path.

Blocking: agent/core/conversation.py:55 breaks native tool calls.

add_assistant requires non-empty content, but an assistant message that carries tool_calls legitimately has empty content — that is exactly what Ollama returns when a model emits native tool calls (message.content == "", message.tool_calls populated). agent/core/engine.py:105 passes that straight through:

content = response.get("message", {}).get("content", "")      # engine.py:93 — defaults to ""
tool_calls_from_api = response.get("message", {}).get("tool_calls", [])
...
if tool_calls_from_api:
    self.conversation.add_assistant(content, tool_calls_from_api)   # engine.py:105 -> ValueError

So the first time the model calls a tool natively, this raises ValueError. It is not caught anywhere: process_message only handles OllamaError (engine.py:88), and the REPL loop in main.py:259 only handles KeyboardInterrupt/EOFError — so the CLI dies with a traceback instead of running the tool. The same applies at engine.py:130 if a model returns an empty final response.

Suggested fix — make the rule role-aware rather than uniform:

def add_assistant(self, content: str, tool_calls: Optional[list] = None):
    if not tool_calls:
        self._validate_content(content)
    self.messages.append(
        Message(role="assistant", content=content or "", tool_calls=tool_calls)
    )

Non-blocking, worth considering:

1. agent/core/conversation.py:61 — add_tool_result is called with str(result) (engine.py:182). Tools currently guard against empty output (output or "(no output)" in bash.py:135, git.py:101), so this does not fire today, but it is one tool returning ToolResult(True, "") away from the same uncaught crash. Consider normalizing to a placeholder here instead of raising.
2. load() (conversation.py:~100) rebuilds self.messages directly and bypasses validation entirely, so a saved conversation can reintroduce exactly the content this PR is meant to keep out. Worth either validating on load or documenting that this is an entry-point-only guard.
3. A test covering add_assistant("", tool_calls=[...]) would have caught the above and would pin the intended contract.

Happy to re-review once the assistant/tool-call case is allowed through.

[ssevera1]

Closing in favour of #28.

All four of these PRs (#23-#26) propose variants of the same content validation against a pre-#27 base. #27 already merged that validation into main — and it carries the exact defect the review on this PR flagged: add_assistant/add_tool_result reject empty content, which crashes the agent on native tool calls (engine.py:105) and on successful zero-output tools (engine.py:182).

#28 fixes the live regression on main and adds the regression tests that were missing. Reproduced both crash paths before fixing; full suite green after.