Validate message content to prevent silent LLM failures

[ssevera1]

What

Add validation to all message-adding methods to reject empty or None content.

Why

Empty/None messages in conversation history cause silent parsing failures downstream in LLM API calls, making bugs hard to diagnose.

[claude]

This breaks the main agent loop for native tool calls. Requesting changes.

1. add_assistant rejects the most common assistant message in this codebase (agent/core/conversation.py:50-55)

agent/core/engine.py:105 does self.conversation.add_assistant(content, tool_calls_from_api), where content comes from response["message"]["content"] (engine.py:94). When Ollama returns native tool calls, message.content is normally an empty string — the model's output is the tool call. With this change every native tool call raises ValueError.

That exception is not caught anywhere: process_message only catches OllamaError (engine.py:89), and the REPL in agent/main.py:246-267 only catches KeyboardInterrupt/EOFError. So the result is a full traceback and process exit — strictly worse than the "silent parsing failure" the PR is trying to prevent. An assistant message carrying tool_calls should be exempt from the empty-content check.

Same issue in the terminal branch at engine.py:130: a model returning an empty final message now crashes instead of returning "".

2. add_tool_result can crash on legitimate empty tool output (conversation.py:57-62)

engine.py:182 records str(result), and ToolResult.__str__ (agent/tools/base.py:15-18) returns self.output verbatim on success. bash.py:135 and git.py:101 happen to guard with or "(no output)", but that convention is not enforced across tools, and a tool legitimately producing no output should not abort the turn. Normalize to a placeholder here rather than raising.

3. The invariant is not actually enforced

Conversation.load (conversation.py:~103-119) constructs Message objects directly and bypasses all four checks, so a saved conversation with empty content still flows straight into the API call.

4. Smaller items

• The same two lines are copy-pasted four times; extract a _validate_content(content) helper.
• The isinstance(content, str) guard means non-str, non-None values (e.g. 0, []) pass validation and later blow up in _trim_context's len(m.content).
• add_system lost its # Replace any existing system message comment (conversation.py:39-42) — unrelated to the stated scope, please restore it.

Note that CI passing here is not meaningful: no test exercises the tool_calls_from_api path (tests/test_engine.py only covers _extract_tool_calls). Please add a test that calls add_assistant("", tool_calls=[...]) and one for an empty tool result.

[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.