holmes/core/tool_calling_llm.py (4)

386-401: Skip-numbering logic for TodoWrite is correct; consider extracting a robust tool_name helper to avoid duplication and edge cases.

The conditional increment using non_todo_write_count achieves the intended numbering behavior. However, the tool_name extraction and TodoWrite check are duplicated here and in call_stream(), and the hasattr(...) else t.custom.name branch can still throw if a future SDK shape lacks both attributes. Extract a single helper and use safe getattr to make this future-proof and DRY.

Apply this diff in-place:

- for t in tools_to_call:
+ for t in tools_to_call:
 logging.debug(f"Tool to call: {t}")
- # Check if this is a TodoWrite tool
- tool_name = (
- t.function.name if hasattr(t, "function") else t.custom.name
- )
- is_todo_write = tool_name == "TodoWrite"
+ # Derive tool name in a robust way (handles function/custom/unknown)
+ tool_name = self._get_tool_name_from_call(t)
+ is_todo_write = tool_name == "TodoWrite"

Add this helper once in the class (outside the selected lines), then reuse it in both call() and call_stream():

# Place inside class ToolCallingLLM (e.g., near other @staticmethods)
@staticmethod
def _get_tool_name_from_call(tool_call: ChatCompletionMessageToolCall) -> str:
 # Prefer .function.name when available; fall back to .custom.name; else "unknown"
 fn = getattr(tool_call, "function", None)
 if fn and getattr(fn, "name", None):
 return fn.name
 custom = getattr(tool_call, "custom", None)
 if custom and getattr(custom, "name", None):
 return custom.name
 return "unknown"

---

709-723: Duplicate TodoWrite detection and numbering logic in call_stream — reuse the same helper.

Same suggestion as in call(): de-duplicate and harden name extraction to avoid attribute errors and keep behavior consistent.

Apply this diff:

- for t in tools_to_call: # type: ignore
- # Check if this is a TodoWrite tool
- tool_name = (
- t.function.name if hasattr(t, "function") else t.custom.name
- )
- is_todo_write = tool_name == "TodoWrite"
+ for t in tools_to_call: # type: ignore
+ tool_name = self._get_tool_name_from_call(t)
+ is_todo_write = tool_name == "TodoWrite"

---

724-736: Consider including tool_number in START_TOOL stream events for non-TodoWrite tools.

This can help stream consumers show consistent numbering during live updates (even if completion order differs from execution order). Keep it omitted for TodoWrite by adding only when not None.

Apply this diff:

- yield StreamMessage(
- event=StreamEvents.START_TOOL,
- data={"tool_name": tool_name, "id": t.id},
- )
+ event_data = {"tool_name": tool_name, "id": t.id}
+ if tool_num is not None:
+ event_data["tool_number"] = tool_num
+ yield StreamMessage(
+ event=StreamEvents.START_TOOL,
+ data=event_data,
+ )

---

434-435: Type-hint tool_number as Optional[int] for mypy and caller clarity.

This parameter is now semantically optional. Add the type hint to satisfy the project’s typing guideline.

- tool_number=None,
+ tool_number: Optional[int] = None,

holmes/interactive.py (2)

748-759: Good: TodoWrite calls are hidden from /last; centralize the hidden-set to avoid string duplication.

Filtering here achieves the PR goal. To prevent drift, define a constant set of hidden tool names once and reuse it across the file.

Apply this diff in place:

- non_todo_write_tools = [
- tool_call
- for tool_call in last_response.tool_calls
- if tool_call.tool_name != "TodoWrite"
- ]
+ non_todo_write_tools = [
+ tool_call
+ for tool_call in last_response.tool_calls
+ if tool_call.tool_name not in HIDDEN_TOOL_NAMES
+ ]

Add this once near the imports (outside the selected lines):

# Tools hidden from numbering/display flows
HIDDEN_TOOL_NAMES = {"TodoWrite"}

---

785-806: DRY the TodoWrite filtering with a small helper to keep behavior consistent across code paths.

The same filter appears here and in handle_last_command and run_interactive_loop. Consider a helper filter_displayable_tool_calls(...) to reduce duplication and future mistakes.

Apply this diff in place:

- non_todo_write_tools = [
- tool_call for tool_call in tool_calls if tool_call.tool_name != "TodoWrite"
- ]
+ non_todo_write_tools = filter_displayable_tool_calls(tool_calls)

Add this helper once (outside selected lines):

def filter_displayable_tool_calls(tool_calls: List[ToolCallResult]) -> List[ToolCallResult]:
 return [tc for tc in tool_calls if tc.tool_name not in HIDDEN_TOOL_NAMES]

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

holmes/core/tool_calling_llm.py (2)

420-422: Offset update matches non-TodoWrite count — numbering stays contiguous across batches.

No issues. This ensures the next iteration starts numbering at the correct offset, ignoring TodoWrite calls.

---

402-410: Awaiting results from the tool_num lookup to confirm it’s never zero. This will verify that tool_number=None is the only falsey case and thus downstream formatting is safe.

holmes/interactive.py (2)

761-765: Usage header correctly reflects only displayable tools.

This keeps user-facing counts aligned with /show indexing. No changes needed.

---

1054-1061: Confirming /show Indexing Consistency

Based on the ripgrep output:

• In holmes/interactive.py at line 1043, tool_number_offset=len(all_tool_calls_history) is indeed the only offset passed into the LLM invocation (corresponding to the /show command) – nothing else in this file mutates or supplies a different offset.
• Immediately after receiving response.tool_calls, the code filters out "TodoWrite" and extends all_tool_calls_history only with the non-TodoWrite calls (lines 1057–1060), ensuring the history used for /show remains aligned with the numbering basis.
• In holmes/core/tool_calling_llm.py, the default tool_number_offset = 0 at line 622 is only ever used when no external offset is supplied (i.e., in contexts outside interactive.py).

There are no other occurrences of tool_number_offset in the repository. This confirms that:

• The numbering offset for /show is always derived from the length of the filtered history.
• No hidden or alternate offset sources exist that could desynchronize the displayed indices.

No changes are required. Everything aligns with the intended non-TodoWrite numbering.