fix: avoid AttributeError when LLM returns null content as final response

[yakir-shriker]

Problem

holmes investigate (CLI and source plugins) crashes with:

AttributeError: 'NoneType' object has no attribute 'replace'
  File "holmes/main.py", line 769, in investigate
    console.print(result.result.replace("\n", "\n\n"), style="bold green")

It happens when the LLM finishes a tool-calling conversation by emitting a final assistant message whose content is null instead of an empty string — stop_reason=end_turn, no further tool calls, no text. The streamed ANSWER_END payload then carries content=None, which is forwarded verbatim into LLMResult.result.

Anything downstream that assumes result.result is a str then breaks:

• CLI: result.result.replace(...) in holmes/main.py — the original AttributeError.
• Interactive loop: holmes/interactive.py builds the same LLMResult(result=terminal_data["content"], ...), then forwards response.result to feedback.metadata.add_llm_response(...) and trace_span.log(output=...), so None propagates to those consumers too.
• Source plugins (PagerDuty, Jira, etc.): markdown_to_plain_text(result_data.result) in write_back_result — same AttributeError, different caller.
• HTTP / SDK consumers that put LLMResult.result into a Pydantic str field (e.g. ChatResponse.analysis) — validation error: Input should be a valid string.

The same root cause was already addressed for the /api/chat endpoint in #1789 (issue #1676), where final_response.content or "" was added in server.py. That fix only covers the streaming server path; every other place that builds an LLMResult from the streamed terminal data still propagates None. This PR moves the fix one layer down so all consumers benefit and the guard doesn't have to be repeated at every call site.

Root cause

call_stream(...) in holmes/core/tool_calling_llm.py yields StreamEvents.ANSWER_END with data["content"] = response_message.content, where response_message.content is Optional[str]. Two call sites then construct LLMResult directly from that payload:

• ToolCallingLLM.call(...) in holmes/core/tool_calling_llm.py — used by the CLI, source plugins, and SDK callers.
• run_interactive_loop(...) in holmes/interactive.py — used by holmes ask / interactive mode.

Both pass terminal_data["content"] straight through:

return LLMResult(
    result=terminal_data["content"],   # can be None
    tool_calls=list(deduped.values()),
    ...
)

LLMResult.result is typed Optional[str], so Pydantic accepts None and the failure surfaces only at the consumer.

Fix

Extract a single helper next to class LLMResult that does the None → "" coalesce and emits a WARNING log when this happens (so the empty-content behavior is observable rather than silent), then call it from every site that builds an LLMResult from the streamed terminal data:

# holmes/core/tool_calling_llm.py
def coalesce_llm_content(content: Optional[str]) -> str:
    """Normalize a possibly-None final LLM response content to a string.

    Some providers occasionally return content=None as the final
    response after tool execution (see #1676). ...
    """
    if content is None:
        logging.warning(
            "LLM returned content=None as the final response after tool "
            "execution; treating as an empty string (see #1676)."
        )
        return ""
    return content

# holmes/core/tool_calling_llm.py — ToolCallingLLM.call(...)
return LLMResult(
    result=coalesce_llm_content(terminal_data["content"]),
    ...
)

# holmes/interactive.py — run_interactive_loop(...)
response = LLMResult(
    result=coalesce_llm_content(terminal_data["content"]),
    ...
)

This keeps LLMResult.result effectively a non-null string for every consumer (CLI, interactive, source plugins, SDK, server) without changing the public type. Same pattern as #1789, applied at the LLMResult boundary instead of inside the streaming server.

Other LLMResult(...) constructions

Audited and confirmed not affected:

• holmes/core/tool_calling_llm.py (frontend-tool-paused branch) — hardcoded result="Investigation paused: ..." literal.
• holmes/interactive.py::save_conversation_to_file — deliberately sets result=None ("No single result in interactive mode").
• holmes/checks/checks.py, holmes/checks/checks_api.py — built from check results (result.message, result.rationale or result.message), not from streamed terminal_data.
• holmes/plugins/sources/pagerduty/__init__.py — test/debug literal "This is a test".

Tests

tests/test_tool_calling_llm.py — two new test classes that fail on master with the original AttributeError and pass with this change:

• TestCoalesceLLMContent — direct unit tests for the helper:
  • test_none_becomes_empty_string_and_warns — None is coalesced to "" and the warning is logged.
  • test_string_passes_through_without_warning — non-None strings pass through and no warning is emitted.
• TestEmptyLLMContent — integration tests through ToolCallingLLM.call(...):
  • test_call_no_crash_on_none_content_no_tools — LLM returns content=None immediately. Includes a smoke-test of the exact downstream call site (result.result.replace("\n", "\n\n")) that crashes in holmes/main.py.
  • test_call_no_crash_after_tool_execution — covers the post-tool-execution path: a tool runs successfully, then the model's final summary turn returns content=None. Asserts result.result == "", the tool call is preserved, and the warning is emitted.

$ poetry run pytest tests/test_tool_calling_llm.py -k "EmptyLLMContent or CoalesceLLMContent" -v
...
tests/test_tool_calling_llm.py::TestCoalesceLLMContent::test_none_becomes_empty_string_and_warns PASSED
tests/test_tool_calling_llm.py::TestCoalesceLLMContent::test_string_passes_through_without_warning PASSED
tests/test_tool_calling_llm.py::TestEmptyLLMContent::test_call_no_crash_on_none_content_no_tools PASSED
tests/test_tool_calling_llm.py::TestEmptyLLMContent::test_call_no_crash_after_tool_execution PASSED

The full tests/test_tool_calling_llm.py suite (45 tests) passes locally.

Scope

• One small, targeted change in holmes/core/tool_calling_llm.py (helper + one call site).
• One-line refactor in holmes/interactive.py to use the helper.
• New tests in tests/test_tool_calling_llm.py.
• No public API change. LLMResult.result stays Optional[str] so callers that explicitly accept None (e.g. save_conversation_to_file) still work; in practice it is now always a str after call(...) / the interactive loop.
• No new dependencies, no behavior change when the LLM returns a non-null final message.

Related

• Same root cause as /api/chat returns 500 when model's final response has empty content (ChatResponse.analysis is None) #1676; mirrors the or "" fix from Handle null analysis result in chat endpoint response #1789 but applied at the LLMResult boundary so CLI / interactive / source plugins / SDK callers all benefit, not just /api/chat.

Summary by CodeRabbit

• Bug Fixes

  • Normalize final LLM responses that are None to empty strings and emit a warning, preventing downstream errors and crashes and making streamed answers consistent for rendering and tracing.

• Tests

  • Added unit and regression tests covering normalization, warning emission, and preservation of already-executed tool call data when final LLM content is empty.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[netlify]

✅ Deploy Preview for holmes-docs ready!

Name	Link
🔨 Latest commit	d857206
🔍 Latest deploy log	https://app.netlify.com/projects/holmes-docs/deploys/69f86551552358000811f0c4
😎 Deploy Preview	https://deploy-preview-1972--holmes-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5ce626a6-daf4-48d4-b211-54a81dc26ffb

📥 Commits

Reviewing files that changed from the base of the PR and between 12e1e41 and d857206.

📒 Files selected for processing (1)

• holmes/core/tool_calling_llm.py

🚧 Files skipped from review as they are similar to previous changes (1)

• holmes/core/tool_calling_llm.py

---

Walkthrough

Normalize provider-returned content=None in tool-calling and interactive flows: emit a warning and convert None to "" so LLMResult.result is never None; add coalesce_llm_content and tests verifying behavior and warning emission, preserving tool call records.

Changes

Normalize LLM content across tool-calling + interactive flows

Layer / File(s)	Summary
Data Shape / Helper holmes/core/tool_calling_llm.py	Adds coalesce_llm_content(content: Optional[str]) -> str which converts None to "" and logs a WARNING when content is None.
Core Implementation holmes/core/tool_calling_llm.py	In ToolCallingLLM.call, set LLMResult.result = coalesce_llm_content(terminal_data["content"]) instead of using raw terminal_data["content"].
Wiring / Interactive holmes/interactive.py	Use coalesce_llm_content(...) when constructing stream/answer-end LLMResult.result so interactive stream responses never contain None.
Tests / Regression tests/test_tool_calling_llm.py	Add tests for coalesce_llm_content (None → "" and warning) and regression tests for ToolCallingLLM.call() when final assistant content is None (with and without tool calls): assert result == "", warning logged, and executed tool calls preserved.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• arikalon1
• RoiGlinik

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: handling a null/None LLM content response to prevent AttributeError crashes, which is the core issue addressed by introducing coalesce_llm_content() and updating call sites.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@holmes/core/tool_calling_llm.py`:
- Around line 596-614: The interactive flow constructs LLMResult using
terminal_data["content"] without normalizing None, causing crashes; update the
code path that creates LLMResult(result=terminal_data["content"], ...) in
interactive.py (the block around the call_stream/response assembly where
LLMResult is constructed) to coalesce content to an empty string when
terminal_data["content"] is None (i.e., if content is None: content = ""), or
factor this normalization into a shared helper used by both
ToolCallingLLM.call() and the interactive response assembly and call that helper
before constructing LLMResult.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a2835885-51dd-4530-b456-4ece85da2ad1

📥 Commits

Reviewing files that changed from the base of the PR and between 44fc7c0 and 0773ba7.

📒 Files selected for processing (2)

• holmes/core/tool_calling_llm.py
• tests/test_tool_calling_llm.py