Add function calling with slow clap #150
Conversation
WalkthroughThis update introduces a tool execution subsystem to the Glados architecture. It adds a new Changes
Sequence Diagram(s)Tool Call Processing FlowsequenceDiagram
participant User
participant SpeechListener
participant Glados
participant LanguageModelProcessor
participant ToolExecutor
participant Tool
User->>SpeechListener: Speak command
SpeechListener->>Glados: {"role": "user", "content": "..."}
Glados->>LanguageModelProcessor: {"role": "user", "content": "..."}
LanguageModelProcessor->>Glados: (LLM streaming response)
alt Tool call detected
LanguageModelProcessor->>ToolExecutor: {"name": "...", "arguments": "...", "id": "..."}
ToolExecutor->>Tool: run(tool_call_id, arguments)
Tool->>ToolExecutor: (result)
ToolExecutor->>LanguageModelProcessor: (result via llm_queue)
else Text response
LanguageModelProcessor->>Glados: (text for TTS)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jessecampos
force-pushed
the
jc-function-calling
branch
from
254b20c to
ad3e600
Compare
jessecampos
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/glados/core/tool_executor.py (1)
55-62: OpenAI vs Ollama arguments are handledString JSON vs dict is supported here, covering both APIs. This addresses the earlier question about Ollama compatibility.
🧹 Nitpick comments (6)
src/glados/tools/__init__.py (1)
10-12: Avoid duplicated tool name strings; derive from definitionUse the function name from slow_clap_def to prevent drift if the name changes.
-from .slow_clap import tool_definition as slow_clap_def, SlowClap +from .slow_clap import tool_definition as slow_clap_def, SlowClap + +# Single source of truth for the tool name +_SLOW_CLAP_NAME = slow_clap_def["function"]["name"] @@ -tool_classes = { - "slow clap": SlowClap, -} +tool_classes = { + _SLOW_CLAP_NAME: SlowClap, +} @@ -all_tools = list(tool_classes.keys()) +all_tools = list(tool_classes.keys())Also applies to: 15-15
src/glados/tools/slow_clap.py (2)
26-31: Add return type for init (ruff ANN204)Annotate as returning None.
-class SlowClap: - def __init__( +class SlowClap: + def __init__( self, llm_queue: queue.Queue[dict[str, Any]], audio_path: str = "data/slow-clap.mp3" - ): + ) -> None:
61-75: Return richer success info (optional)Consider including the number of claps performed in the content for clarity.
- self.llm_queue.put({ - "role": "tool", - "tool_call_id": tool_call_id, - "content": "success", - "type": "function_call_output" - }) + self.llm_queue.put({ + "role": "tool", + "tool_call_id": tool_call_id, + "content": f"success: played {claps} slow clap(s)", + "type": "function_call_output", + })src/glados/core/engine.py (1)
234-240: Missing back-pressure / queue-size guards
ToolExecutoris fed by an unboundedtool_calls_queue. If the LLM streams tool calls faster than they can be executed, memory will grow without limit. Consider usingQueue(maxsize=N)plus non-blockingput_nowait/ retry or explicit drop policy.tests/test_tool_executor.py (2)
1-13: Clean up unused imports to satisfy Ruff
pytest,patch,MagicMock,sys,Any,glados.tools, andloggerare unused.
Drop them to silence F401 warnings.-import pytest -from unittest.mock import Mock, patch, MagicMock -import sys -from typing import Any -import glados.tools as tools -from loguru import logger +from unittest.mock import Mock
42-43: Remove dead variabletest_tool
test_toolis assigned but never used in four tests – delete the assignment to avoid F841 lint errors.Also applies to: 91-92, 141-142, 189-190
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
data/slow-clap.mp3is excluded by!**/*.mp3
📒 Files selected for processing (8)
pyproject.toml(1 hunks)src/glados/core/engine.py(6 hunks)src/glados/core/llm_processor.py(7 hunks)src/glados/core/speech_listener.py(3 hunks)src/glados/core/tool_executor.py(1 hunks)src/glados/tools/__init__.py(1 hunks)src/glados/tools/slow_clap.py(1 hunks)tests/test_tool_executor.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dnhkng
PR: dnhkng/GLaDOS#142
File: src/glados/core/llm_processor.py:46-51
Timestamp: 2025-06-04T06:00:27.087Z
Learning: The LanguageModelProcessor in src/glados/core/llm_processor.py should support APIs that don't require API keys, such as Ollama, alongside authenticated APIs like OpenAI.
📚 Learning: 2025-06-04T06:00:27.087Z
Learnt from: dnhkng
PR: dnhkng/GLaDOS#142
File: src/glados/core/llm_processor.py:46-51
Timestamp: 2025-06-04T06:00:27.087Z
Learning: The LanguageModelProcessor in src/glados/core/llm_processor.py should support APIs that don't require API keys, such as Ollama, alongside authenticated APIs like OpenAI.
Applied to files:
src/glados/core/speech_listener.pysrc/glados/core/tool_executor.pysrc/glados/core/engine.pysrc/glados/core/llm_processor.py
📚 Learning: 2025-01-29T09:37:18.332Z
Learnt from: dnhkng
PR: dnhkng/GLaDOS#115
File: src/glados/engine.py:786-831
Timestamp: 2025-01-29T09:37:18.332Z
Learning: In GLaDOS, use threading.Event for thread synchronization flags that are accessed from multiple threads (e.g., speaking state), but keep boolean flags for configuration settings that don't change during runtime (e.g., interruptible flag).
Applied to files:
src/glados/core/engine.py
🧬 Code Graph Analysis (2)
src/glados/tools/__init__.py (1)
src/glados/tools/slow_clap.py (1)
SlowClap(25-86)
src/glados/core/tool_executor.py (2)
src/glados/tools/slow_clap.py (1)
run(42-86)src/glados/core/llm_processor.py (1)
run(193-307)
🪛 Ruff (0.12.2)
src/glados/tools/slow_clap.py
26-26: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
src/glados/core/engine.py
197-197: Line too long (128 > 120)
(E501)
198-198: Line too long (122 > 120)
(E501)
tests/test_tool_executor.py
1-1: pytest imported but unused
Remove unused import: pytest
(F401)
2-2: unittest.mock.patch imported but unused
Remove unused import
(F401)
2-2: unittest.mock.MagicMock imported but unused
Remove unused import
(F401)
5-5: sys imported but unused
Remove unused import: sys
(F401)
8-8: typing.Any imported but unused
Remove unused import: typing.Any
(F401)
11-11: glados.tools imported but unused
Remove unused import: glados.tools
(F401)
12-12: loguru.logger imported but unused
Remove unused import: loguru.logger
(F401)
14-14: Missing return type annotation for public function test_run_shutdown_event
Add return type annotation: None
(ANN201)
14-14: Missing type annotation for function argument caplog
(ANN001)
33-33: Missing return type annotation for public function test_tool_call_discarded_if_processing_inactive
Add return type annotation: None
(ANN201)
33-33: Missing type annotation for function argument mocker
(ANN001)
33-33: Missing type annotation for function argument caplog
(ANN001)
81-81: Missing return type annotation for public function test_process_valid_tool_call
Add return type annotation: None
(ANN201)
81-81: Missing type annotation for function argument mocker
(ANN001)
81-81: Missing type annotation for function argument caplog
(ANN001)
130-130: Missing return type annotation for public function test_json_decode_error
Add return type annotation: None
(ANN201)
130-130: Missing type annotation for function argument mocker
(ANN001)
130-130: Missing type annotation for function argument caplog
(ANN001)
140-140: Local variable test_tool is assigned to but never used
Remove assignment to unused variable test_tool
(F841)
179-179: Missing return type annotation for public function test_unknown_tool
Add return type annotation: None
(ANN201)
179-179: Missing type annotation for function argument mocker
(ANN001)
179-179: Missing type annotation for function argument caplog
(ANN001)
189-189: Local variable test_tool is assigned to but never used
Remove assignment to unused variable test_tool
(F841)
🔇 Additional comments (4)
pyproject.toml (1)
27-28: Dev plugins addition looks goodpytest-loguru and pytest-mock are appropriate for the new tests. No concerns.
src/glados/core/speech_listener.py (2)
11-11: Type alignment with structured LLM messagesSwitching llm_queue to Queue[dict[str, Any]] is consistent with the new structured pipeline.
Also applies to: 40-40
250-253: Structured user message enqueuedUsing {"role": "user", "content": ...} aligns with LLMProcessor’s expected message format.
src/glados/core/engine.py (1)
197-199: No heterogeneous payloads in llm_queue—only dict envelopes are used
I’ve confirmed that both SpeechListener and ToolExecutor enqueue dictionaries (e.g.{ "role": "user", "content": ... }and{ "role": "tool", "content": ..., "type": "function_call_output" }) rather than raw strings. The TTS queue still handles plain strings separately. TheQueue[dict[str, Any]]annotation correctly reflects what’s actually pushed, so there’s no risk of mixed payloads or schema drift here.Likely an incorrect or invalid review comment.
| delta = line.get("choices", [{}])[0].get("delta", {}) | ||
| tool_calls = delta.get("tool_calls") | ||
| if tool_calls: | ||
| return tool_calls | ||
|
|
||
| content = delta.get("content") | ||
| return str(content) if content else None | ||
| # Handle Ollama format | ||
| else: | ||
| content = line.get("message", {}).get("content") | ||
| message = line.get("message", {}) | ||
| tool_calls = message.get("tool_calls") | ||
| if tool_calls: | ||
| return tool_calls | ||
|
|
||
| content = message.get("content") | ||
| return content if content else None |
There was a problem hiding this comment.
Tool-call detection OK, but content may be lost on mixed chunks
When a chunk contains both content and tool_calls (can happen with OpenAI), the early return of tool_calls discards the text. Consider extracting both and returning a tuple or two separate events so narration isn’t silently dropped.
🤖 Prompt for AI Agents
In src/glados/core/llm_processor.py around lines 100 to 115, the current code
returns early when detecting tool_calls, which causes any accompanying content
in the same chunk to be lost. Modify the logic to extract both tool_calls and
content from the chunk and return them together, for example as a tuple or a
structured object, so that neither tool_calls nor content is discarded when both
are present.
| # update values for the tool call from the new chunk data | ||
| if tool_id: | ||
| tool_call["id"] += tool_id | ||
| if name: | ||
| tool_call["function"]["name"] += name | ||
| if arguments: | ||
| if isinstance(arguments, str): | ||
| # OpenAI format | ||
| tool_call["function"]["arguments"] += arguments | ||
| else: |
There was a problem hiding this comment.
ID / function name concatenated repeatedly
tool_call["id"] += tool_id and likewise for name will duplicate the same token on every partial chunk ("abc" → "abcabcabc" …). For stable fields just assign once:
-if tool_id:
- tool_call["id"] += tool_id
+if tool_id and not tool_call["id"]:
+ tool_call["id"] = tool_idApply the same logic to function["name"].
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # update values for the tool call from the new chunk data | |
| if tool_id: | |
| tool_call["id"] += tool_id | |
| if name: | |
| tool_call["function"]["name"] += name | |
| if arguments: | |
| if isinstance(arguments, str): | |
| # OpenAI format | |
| tool_call["function"]["arguments"] += arguments | |
| else: | |
| # update values for the tool call from the new chunk data | |
| if tool_id and not tool_call["id"]: | |
| tool_call["id"] = tool_id | |
| if name: | |
| tool_call["function"]["name"] += name | |
| if arguments: | |
| if isinstance(arguments, str): | |
| # OpenAI format | |
| tool_call["function"]["arguments"] += arguments | |
| else: |
🤖 Prompt for AI Agents
In src/glados/core/llm_processor.py around lines 151 to 160, the code
concatenates tool_id and function name repeatedly on each partial chunk, causing
duplication. Instead of using '+=' to append, assign tool_call["id"] = tool_id
and tool_call["function"]["name"] = name once to avoid repeated concatenation.
| logger.info("ToolExecutor: Interruption signal active, discarding tool call.") | ||
| continue | ||
|
|
||
| logger.info(f"ToolExecutor: Received tool call: '{tool_call}'") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce logging of full tool payload to avoid PII leakage
Log only tool id and name, not the full dict with arguments.
[security]
- logger.info(f"ToolExecutor: Received tool call: '{tool_call}'")
+ logger.info(
+ "ToolExecutor: Received tool call id={} name={}",
+ tool_call.get("id"),
+ tool_call.get("function", {}).get("name"),
+ )Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/glados/core/tool_executor.py at line 50, the current logging statement
outputs the full tool call payload, which may contain PII. Modify the
logger.info call to log only the tool's id and name fields instead of the entire
tool_call dictionary to prevent sensitive information exposure.
| tool = tool_call["function"]["name"] | ||
| tool_call_id = tool_call["id"] | ||
|
|
||
| try: | ||
| raw_args = tool_call["function"]["arguments"] | ||
| if isinstance(raw_args, str): |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden parsing against missing keys
Accessing tool_call["function"]["name"]/["arguments"] will KeyError if the shape is unexpected. Guard and return an error to the LLM.
- tool = tool_call["function"]["name"]
- tool_call_id = tool_call["id"]
+ function = tool_call.get("function") or {}
+ tool = function.get("name")
+ tool_call_id = tool_call.get("id")
+ if not tool or not tool_call_id:
+ logger.error("ToolExecutor: malformed tool call payload: {}", tool_call)
+ self.llm_queue.put({
+ "role": "tool",
+ "tool_call_id": tool_call_id or "<unknown>",
+ "content": "error: malformed tool call payload",
+ "type": "function_call_output",
+ })
+ continue📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tool = tool_call["function"]["name"] | |
| tool_call_id = tool_call["id"] | |
| try: | |
| raw_args = tool_call["function"]["arguments"] | |
| if isinstance(raw_args, str): | |
| function = tool_call.get("function") or {} | |
| tool = function.get("name") | |
| tool_call_id = tool_call.get("id") | |
| if not tool or not tool_call_id: | |
| logger.error("ToolExecutor: malformed tool call payload: {}", tool_call) | |
| self.llm_queue.put({ | |
| "role": "tool", | |
| "tool_call_id": tool_call_id or "<unknown>", | |
| "content": "error: malformed tool call payload", | |
| "type": "function_call_output", | |
| }) | |
| continue | |
| try: | |
| raw_args = tool_call["function"]["arguments"] | |
| if isinstance(raw_args, str): |
🤖 Prompt for AI Agents
In src/glados/core/tool_executor.py around lines 51 to 56, the code directly
accesses keys "name" and "arguments" inside tool_call["function"], which can
raise KeyError if these keys are missing. To fix this, add checks to verify the
presence of these keys before accessing them. If any key is missing, handle the
situation gracefully by returning an error response to the LLM instead of
letting the exception propagate.
| logger.trace( | ||
| f"ToolExecutor: Failed to parse non-JSON tool call args: " | ||
| f"{tool_call["function"]["arguments"]}" | ||
| ) |
There was a problem hiding this comment.
Syntax error in f-string quoting
Inner double quotes break the f-string. Use logger’s brace formatting or switch to single quotes.
- logger.trace(
- f"ToolExecutor: Failed to parse non-JSON tool call args: "
- f"{tool_call["function"]["arguments"]}"
- )
+ logger.trace(
+ "ToolExecutor: Failed to parse non-JSON tool call args: {}",
+ tool_call["function"]["arguments"],
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.trace( | |
| f"ToolExecutor: Failed to parse non-JSON tool call args: " | |
| f"{tool_call["function"]["arguments"]}" | |
| ) | |
| logger.trace( | |
| "ToolExecutor: Failed to parse non-JSON tool call args: {}", | |
| tool_call["function"]["arguments"], | |
| ) |
🤖 Prompt for AI Agents
In src/glados/core/tool_executor.py around lines 63 to 66, the f-string uses
inner double quotes that conflict with the outer double quotes, causing a syntax
error. Fix this by either changing the inner quotes to single quotes or by using
logger's brace formatting style to avoid quote conflicts within the f-string.
| if tool in all_tools: | ||
| tool_instance = tool_classes.get(tool)( | ||
| llm_queue=self.llm_queue | ||
| ) | ||
| tool_instance.run(tool_call_id, args) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Instantiate and execute with robust error reporting
Catch errors from tool construction/run and notify the LLM, instead of letting them bubble to the outer loop silently.
- if tool in all_tools:
- tool_instance = tool_classes.get(tool)(
- llm_queue=self.llm_queue
- )
- tool_instance.run(tool_call_id, args)
+ if tool in all_tools:
+ ToolClass = tool_classes[tool]
+ try:
+ tool_instance = ToolClass(llm_queue=self.llm_queue)
+ tool_instance.run(tool_call_id, args)
+ except Exception as e:
+ logger.exception("ToolExecutor: tool '{}' failed: {}", tool, e)
+ self.llm_queue.put({
+ "role": "tool",
+ "tool_call_id": tool_call_id,
+ "content": f"error: tool '{tool}' failed: {e}",
+ "type": "function_call_output",
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if tool in all_tools: | |
| tool_instance = tool_classes.get(tool)( | |
| llm_queue=self.llm_queue | |
| ) | |
| tool_instance.run(tool_call_id, args) | |
| if tool in all_tools: | |
| ToolClass = tool_classes[tool] | |
| try: | |
| tool_instance = ToolClass(llm_queue=self.llm_queue) | |
| tool_instance.run(tool_call_id, args) | |
| except Exception as e: | |
| logger.exception("ToolExecutor: tool '{}' failed: {}", tool, e) | |
| self.llm_queue.put({ | |
| "role": "tool", | |
| "tool_call_id": tool_call_id, | |
| "content": f"error: tool '{tool}' failed: {e}", | |
| "type": "function_call_output", | |
| }) |
🤖 Prompt for AI Agents
In src/glados/core/tool_executor.py around lines 69 to 73, the code instantiates
and runs a tool without error handling, which can cause silent failures. Wrap
the tool instantiation and run calls in a try-except block, catch any
exceptions, and notify the LLM about the error instead of letting the exception
propagate silently. This ensures robust error reporting and better fault
tolerance.
| tool_definition = { | ||
| "type": "function", | ||
| "function": { | ||
| "name": "slow clap", | ||
| "description": "Performs a slow clap.", | ||
| "parameters": { | ||
| "type": "object", | ||
| "properties": { | ||
| "claps": { | ||
| "type": "number", | ||
| "description": "The number of slow claps to perform." | ||
| } | ||
| }, | ||
| "required": ["claps"] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Tighten tool schema and consider a tool name without spaces
- Prefer "integer" with min/max/default to match runtime behavior.
- OpenAI function names typically disallow spaces; consider "slow_clap" or "slow-clap" for broad compatibility.
tool_definition = {
"type": "function",
"function": {
- "name": "slow clap",
+ "name": "slow_clap",
"description": "Performs a slow clap.",
"parameters": {
"type": "object",
"properties": {
"claps": {
- "type": "number",
- "description": "The number of slow claps to perform."
+ "type": "integer",
+ "minimum": 1,
+ "maximum": 5,
+ "default": 1,
+ "description": "The number of slow claps to perform (1-5)."
}
},
- "required": ["claps"]
+ "required": []
}
}
}If you adopt the new name, update the tools registry key accordingly (in src/glados/tools/init.py).
🏁 Script executed:
#!/bin/bash
# Display content of slow_clap tool definition
echo "=== slow_clap.py ==="
sed -n '1,200p' src/glados/tools/slow_clap.py || echo "File not found: src/glados/tools/slow_clap.py"
# Display the tools registry in __init__.py to check naming
echo ""
echo "=== __init__.py ==="
sed -n '1,200p' src/glados/tools/__init__.py || echo "File not found: src/glados/tools/__init__.py"Length of output: 3076
Tighten schema & rename tool for compatibility
Please update both the function definition and registry to use an underscore name and a stricter integer schema:
• In src/glados/tools/slow_clap.py
- Rename
"name": "slow clap"→"name": "slow_clap" - Change the
clapsproperty to:
•"type": "integer"
•"minimum": 1,"maximum": 5,"default": 1
• Remove the required array (we fall back to the default)
• In src/glados/tools/init.py - Update the
tool_classeskey from"slow clap": SlowClap→"slow_clap": SlowClap - Ensure
all_toolsreflects the new"slow_clap"name
src/glados/tools/slow_clap.py
tool_definition = {
"type": "function",
"function": {
- "name": "slow clap",
+ "name": "slow_clap",
"description": "Performs a slow clap.",
"parameters": {
"type": "object",
"properties": {
"claps": {
- "type": "number",
- "description": "The number of slow claps to perform."
+ "type": "integer",
+ "minimum": 1,
+ "maximum": 5,
+ "default": 1,
+ "description": "The number of slow claps to perform (1–5)."
}
},
- "required": ["claps"]
+ "required": []
}
}
}
src/glados/tools/__init__.py
tool_classes = {
- "slow clap": SlowClap,
+ "slow_clap": SlowClap,
}
all_tools = list(tool_classes.keys())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tool_definition = { | |
| "type": "function", | |
| "function": { | |
| "name": "slow clap", | |
| "description": "Performs a slow clap.", | |
| "parameters": { | |
| "type": "object", | |
| "properties": { | |
| "claps": { | |
| "type": "number", | |
| "description": "The number of slow claps to perform." | |
| } | |
| }, | |
| "required": ["claps"] | |
| } | |
| } | |
| } | |
| # src/glados/tools/slow_clap.py | |
| tool_definition = { | |
| "type": "function", | |
| "function": { | |
| "name": "slow_clap", | |
| "description": "Performs a slow clap.", | |
| "parameters": { | |
| "type": "object", | |
| "properties": { | |
| "claps": { | |
| "type": "integer", | |
| "minimum": 1, | |
| "maximum": 5, | |
| "default": 1, | |
| "description": "The number of slow claps to perform (1–5)." | |
| } | |
| }, | |
| "required": [] | |
| } | |
| } | |
| } | |
| # src/glados/tools/__init__.py | |
| tool_classes = { | |
| "slow_clap": SlowClap, | |
| } | |
| all_tools = list(tool_classes.keys()) |
🤖 Prompt for AI Agents
In src/glados/tools/slow_clap.py lines 7 to 23, rename the function "name" from
"slow clap" to "slow_clap" and update the "claps" parameter schema to use
"type": "integer" with "minimum": 1, "maximum": 5, and "default": 1, removing
the "required" field entirely. Additionally, in src/glados/tools/__init__.py,
update the tool_classes dictionary key from "slow clap" to "slow_clap" and
ensure the all_tools list or dictionary also uses the "slow_clap" name to
maintain consistency.
| except FileNotFoundError: | ||
| # Raised if the audio file is not found at the specified path | ||
| print(f"Error: Audio not found. Check the path: {self.audio_path}") | ||
|
|
||
| except ValueError as ve: | ||
| # Raised by soundfile for invalid file formats or parameters | ||
| print(f"ValueError: {ve}") | ||
|
|
||
| except sd.PortAudioError as pa_err: | ||
| # Raised by sounddevice for audio device-related issues | ||
| print(f"PortAudioError: {pa_err}") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t print; log and notify LLM on errors
Use loguru and propagate errors back to the LLM so it can react.
- except FileNotFoundError:
- # Raised if the audio file is not found at the specified path
- print(f"Error: Audio not found. Check the path: {self.audio_path}")
-
- except ValueError as ve:
- # Raised by soundfile for invalid file formats or parameters
- print(f"ValueError: {ve}")
-
- except sd.PortAudioError as pa_err:
- # Raised by sounddevice for audio device-related issues
- print(f"PortAudioError: {pa_err}")
+ except FileNotFoundError:
+ err = f"error: audio not found at path {self.audio_path}"
+ logger.error("SlowClap: {}", err)
+ self.llm_queue.put({
+ "role": "tool",
+ "tool_call_id": tool_call_id,
+ "content": err,
+ "type": "function_call_output",
+ })
+ except ValueError as ve:
+ err = f"error: invalid audio or parameters: {ve}"
+ logger.error("SlowClap: {}", err)
+ self.llm_queue.put({
+ "role": "tool",
+ "tool_call_id": tool_call_id,
+ "content": err,
+ "type": "function_call_output",
+ })
+ except sd.PortAudioError as pa_err:
+ err = f"error: audio device issue: {pa_err}"
+ logger.error("SlowClap: {}", err)
+ self.llm_queue.put({
+ "role": "tool",
+ "tool_call_id": tool_call_id,
+ "content": err,
+ "type": "function_call_output",
+ })Add the missing import at the top if you adopt this:
+from loguru import logger📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except FileNotFoundError: | |
| # Raised if the audio file is not found at the specified path | |
| print(f"Error: Audio not found. Check the path: {self.audio_path}") | |
| except ValueError as ve: | |
| # Raised by soundfile for invalid file formats or parameters | |
| print(f"ValueError: {ve}") | |
| except sd.PortAudioError as pa_err: | |
| # Raised by sounddevice for audio device-related issues | |
| print(f"PortAudioError: {pa_err}") | |
| # at the top of src/glados/tools/slow_clap.py, add: | |
| from loguru import logger | |
| … # other imports | |
| # later in your method, replace the original except blocks with: | |
| except FileNotFoundError: | |
| err = f"error: audio not found at path {self.audio_path}" | |
| logger.error("SlowClap: {}", err) | |
| self.llm_queue.put({ | |
| "role": "tool", | |
| "tool_call_id": tool_call_id, | |
| "content": err, | |
| "type": "function_call_output", | |
| }) | |
| except ValueError as ve: | |
| err = f"error: invalid audio or parameters: {ve}" | |
| logger.error("SlowClap: {}", err) | |
| self.llm_queue.put({ | |
| "role": "tool", | |
| "tool_call_id": tool_call_id, | |
| "content": err, | |
| "type": "function_call_output", | |
| }) | |
| except sd.PortAudioError as pa_err: | |
| err = f"error: audio device issue: {pa_err}" | |
| logger.error("SlowClap: {}", err) | |
| self.llm_queue.put({ | |
| "role": "tool", | |
| "tool_call_id": tool_call_id, | |
| "content": err, | |
| "type": "function_call_output", | |
| }) |
🤖 Prompt for AI Agents
In src/glados/tools/slow_clap.py around lines 76 to 86, replace the print
statements in the exception handlers with loguru logging calls to properly log
the errors. Additionally, propagate the exceptions or return error information
so the LLM can be notified and react accordingly. Also, add the missing import
statement for loguru at the top of the file.
1 participant
Add function calling starting with the slow clap tool.
These changes allow for multiple custom tools to be added for use by the LLM.
Tools live in the
src/glados/tools/directory.A new thread
ToolExecutorlistens for tool calls made by the LLM on thetool_calls_queue.Noteworthy change:
llm_queueneeded to be changed fromQueue[str]toQueue[dict[str, Any]]to support sending tool call execution results back to the LLM.Refer to inline comments below for Ollama supportedit: resolvedResolves issue #48
Summary by CodeRabbit
New Features
Improvements
Tests
Chores