fix: remove accidental 'from json import tool' import; align abstract completion() default

[harshadkhetpal]

Summary

1. holmes/core/tool_calling_llm.py — line 3 has from json import tool, which imports Python's json.tool CLI module (the thing behind python -m json.tool). It looks like an IDE auto-import accident: the name is never used as that module — every use of tool in this file is a local variable (tool = self.tool_executor.get_tool_by_name(...)) that shadows it, which is why ruff reports F811 twice. Removed the import.

2. holmes/core/llm.py — the abstract completion() declares tools: Optional[List[Dict[str, Any]]] = [] (mutable default, ruff B006), while the concrete implementation of the same method in this same file already uses = None. Aligned the abstract signature to = None.

Testing

python -m py_compile passes on both files; ruff check --select F811,B006 goes from 3 errors to clean. No behavior change: the removed import was shadowed everywhere, and the abstract method's default is never executed (implementations define their own).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved the reliability of language model completion requests by preventing unintended reuse of tool configuration between calls.

• Chores

  • Removed an unused internal import without changing user-visible functionality.

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

[linux-foundation-easycla]

• ❌ - login: @harshadkhetpal / name: Harshad Khetpal. The commit (bb1bc9e) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please visit our EasyCLA portal and chat with our support bot.

[netlify]

✅ Deploy Preview for holmes-docs ready!

Name	Link
🔨 Latest commit	bb1bc9e
🔍 Latest deploy log	https://app.netlify.com/projects/holmes-docs/deploys/6a8da49b4ff44f000822b2f2
😎 Deploy Preview	https://deploy-preview-2406--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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dc408fe7-38c0-46ff-b067-b8c2750d53e7

📥 Commits

Reviewing files that changed from the base of the PR and between 0089f60 and f5c610f.

📒 Files selected for processing (2)

• holmes/core/llm.py
• holmes/core/tool_calling_llm.py

💤 Files with no reviewable changes (1)

• holmes/core/tool_calling_llm.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

Walkthrough

The abstract LLM.completion method now defaults tools to None. The tool-calling module removes an unused import from json.

Changes

LLM interface cleanup

Layer / File(s)	Summary
Completion default and import cleanup holmes/core/llm.py, holmes/core/tool_calling_llm.py	The completion method uses None instead of a shared mutable list for tools. The unused json.tool import is removed.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to f5c61

The changes remove an unused import and replace a mutable abstract-method default without changing runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: moshemorad

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both primary changes: removing the unused import and aligning the abstract completion() default.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

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

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