Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added error handling #76

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 63 additions & 43 deletions src/ai/anthropic.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import anthropic
from anthropic.types import APIError, APITimeoutError, APIConnectionError
from src.ai.tool import ToolError
import logger

class AnthropicError(Exception):
"""Custom exception for anthropic-related errors."""
pass
Comment on lines +6 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom exception, created you have. But inherit from ToolError, it should. Consistency across the project, maintain we must.

Comment on lines +6 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom exception, created you have. Wise decision, it is. But consider, you must, adding more specific error messages or attributes. Debugging, easier it will make.


async def chat_completion(
system_prompt: str,
Expand All @@ -8,24 +14,31 @@ async def chat_completion(
):
client = anthropic.AsyncClient()

message = await client.messages.create(
model=model,
max_tokens=2000,
temperature=0,
system=system_prompt,
messages=[
{
"role": "user",
"content": [
{
"type": "text",
"text": prompt,
}
],
}
],
)
return message.content
try:
message = await client.messages.create(
model=model,
max_tokens=2000,
temperature=0,
system=system_prompt,
messages=[
{
"role": "user",
"content": [
{
"type": "text",
"text": prompt,
}
],
}
],
)
return message.content
except (APIError, APITimeoutError, APIConnectionError) as e:
logger.log.error(f"Anthropic API error in chat_completion: {str(e)}")
raise AnthropicError(f"Anthropic API error: {str(e)}")
except Exception as e:
logger.log.error(f"Unexpected error in chat_completion: {str(e)}")
Comment on lines +36 to +40
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling, improved it is. But log the full exception traceback, you should. Valuable information for debugging, it provides.

raise AnthropicError(f"Unexpected error: {str(e)}")


async def tool_completion(
Expand All @@ -44,28 +57,35 @@ async def tool_completion(
else:
tool_choice = {"type": "auto"}

message = await client.messages.create(
model=model,
max_tokens=2000,
temperature=0,
system=system_prompt,
messages=[
{
"role": "user",
"content": [
{
"type": "text",
"text": prompt,
}
],
}
],
tools=tools,
tool_choice=tool_choice,
)
if message.stop_reason == "tool_use":
for response in message.content:
if response.type == "tool_use":
return response.input
else:
return message.content[0].text
try:
message = await client.messages.create(
model=model,
max_tokens=2000,
temperature=0,
system=system_prompt,
messages=[
{
"role": "user",
"content": [
{
"type": "text",
"text": prompt,
}
],
}
],
Comment on lines +67 to +76
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate code from chat_completion function, I see. Common function for message creation, extract you should. DRY principle, follow we must.

tools=tools,
tool_choice=tool_choice,
)
if message.stop_reason == "tool_use":
for response in message.content:
if response.type == "tool_use":
return response.input
else:
return message.content[0].text
except (APIError, APITimeoutError, APIConnectionError) as e:
logger.log.error(f"Anthropic API error in tool_completion: {str(e)}")
raise AnthropicError(f"Anthropic API error: {str(e)}")
except Exception as e:
logger.log.error(f"Unexpected error in tool_completion: {str(e)}")
raise AnthropicError(f"Unexpected error: {str(e)}")
147 changes: 84 additions & 63 deletions src/ai/assistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import logger
from . import prompt, schema
from .anthropic import tool_completion
from .tool import TOOLS
from .tool import TOOLS, ToolError


class Assistant:
Expand All @@ -21,26 +21,33 @@ async def overview(
self,
context: code.model.PullRequestContextModel,
) -> code.review.model.OverviewModel:
completion = await tool_completion(
system_prompt=self._builder.render_template(
name="overview",
prefix="system",
persona=self._persona,
),
prompt=self._builder.render_template(
name="overview",
prefix="user",
),
model=self._model_name,
tools=[TOOLS["review_files"]],
tool_override="review_files",
)
response = schema.ReviewRequestsResponseModel(**completion)
try:
completion = await tool_completion(
system_prompt=self._builder.render_template(
name="overview",
prefix="system",
persona=self._persona,
),
prompt=self._builder.render_template(
name="overview",
prefix="user",
),
model=self._model_name,
tools=[TOOLS["review_files"]],
tool_override="review_files",
)
response = schema.ReviewRequestsResponseModel(**completion)

return code.review.comment.parse_overview(
response=response,
context=context,
)
return code.review.comment.parse_overview(
response=response,
context=context,
)
except ToolError as e:
logger.log.error(f"Error in overview: {str(e)}")
raise
except Exception as e:
logger.log.error(f"Unexpected error in overview: {str(e)}")
raise
Comment on lines +45 to +50
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic Exception catching, broad it is. Specific exceptions, catch you should. More precise error handling, leads to better debugging.


async def review_file(
self,
Expand All @@ -53,54 +60,68 @@ async def review_file(
logger.log.debug(f"Reviewing file: {context.path}")
logger.log.debug(f"Hunks: {context.patch.hunks}")

completion = await tool_completion(
system_prompt=self._builder.render_template(
name="file-review",
prefix="system",
persona=self._persona,
),
prompt=self._builder.render_template(
name="file-review",
prefix="user",
observations=observations,
file_context=context,
),
model=self._model_name,
tools=[TOOLS["post_feedback"]],
tool_override="post_feedback",
)
response = schema.FileReviewResponseModel(**completion)
try:
completion = await tool_completion(
system_prompt=self._builder.render_template(
name="file-review",
prefix="system",
persona=self._persona,
),
prompt=self._builder.render_template(
name="file-review",
prefix="user",
observations=observations,
file_context=context,
),
model=self._model_name,
tools=[TOOLS["post_feedback"]],
tool_override="post_feedback",
)
response = schema.FileReviewResponseModel(**completion)

return code.review.comment.extract_comments(
response=response,
file_context=context,
severity_limit=severity_limit,
)
return code.review.comment.extract_comments(
response=response,
file_context=context,
severity_limit=severity_limit,
Comment on lines +63 to +85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar structure to 'overview' method, I see. Refactor common error handling logic, you could. DRY principle, follow we must.

)
except ToolError as e:
logger.log.error(f"Error in review_file for {context.path}: {str(e)}")
raise
except Exception as e:
logger.log.error(f"Unexpected error in review_file for {context.path}: {str(e)}")
raise

async def get_feedback(
self,
prioritized_comments: list[code.model.GitHubCommentModel],
remaining_comments: list[code.model.GitHubCommentModel],
) -> code.review.model.Feedback:
completion = await tool_completion(
system_prompt=self._builder.render_template(
name="review-summary",
prefix="system",
persona=self._persona,
),
prompt=self._builder.render_template(
name="review-summary",
prefix="user",
prioritized_comments=prioritized_comments,
comments=remaining_comments,
),
model=self._model_name,
tools=[TOOLS["submit_review"]],
tool_override="submit_review",
)
response = schema.ReviewResponseModel(**completion)
try:
completion = await tool_completion(
system_prompt=self._builder.render_template(
name="review-summary",
prefix="system",
persona=self._persona,
),
prompt=self._builder.render_template(
name="review-summary",
prefix="user",
prioritized_comments=prioritized_comments,
comments=remaining_comments,
),
model=self._model_name,
tools=[TOOLS["submit_review"]],
tool_override="submit_review",
)
response = schema.ReviewResponseModel(**completion)

return code.review.comment.parse_feedback(
response=response,
comments=prioritized_comments,
)
return code.review.comment.parse_feedback(
response=response,
comments=prioritized_comments,
)
except ToolError as e:
logger.log.error(f"Error in get_feedback: {str(e)}")
raise
except Exception as e:
logger.log.error(f"Unexpected error in get_feedback: {str(e)}")
raise
24 changes: 20 additions & 4 deletions src/ai/prompt.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
import code.model
import code.review.model

class PromptError(Exception):
"""Custom exception for prompt-related errors."""
pass

PROMPT_DIR = pathlib.Path(__file__).parent / "prompts"


Expand All @@ -29,14 +33,26 @@ def _load_template(
name: str,
prefix: typing.Literal["system", "user", "persona"],
) -> jinja2.Template:
return self._templates[prefix].get_template(name)
try:
return self._templates[prefix].get_template(name)
except jinja2.TemplateNotFound:
raise PromptError(f"Template not found: {prefix}/{name}")
except jinja2.TemplateError as e:
raise PromptError(f"Error loading template {prefix}/{name}: {str(e)}")

def render_template(
self,
name: str,
prefix: typing.Literal["system", "user", "persona"],
**kwargs,
) -> str:
template = self._load_template(f"{name}.md", prefix=prefix)
overview = template.render(context=self._context, prefix=prefix, **kwargs)
return overview
try:
template = self._load_template(f"{name}.md", prefix=prefix)
overview = template.render(context=self._context, prefix=prefix, **kwargs)
return overview
except PromptError as e:
raise e
except jinja2.TemplateError as e:
raise PromptError(f"Error rendering template {prefix}/{name}.md: {str(e)}")
except Exception as e:
raise PromptError(f"Unexpected error rendering template {prefix}/{name}.md: {str(e)}")
35 changes: 25 additions & 10 deletions src/ai/tool.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,37 @@
import json
import pathlib
from typing import Any
from typing import Any, Dict

TOOL_DIR = pathlib.Path(__file__).parent / "tools"
import logger

TOOL_DIR = pathlib.Path(__file__).parent / "tools"

def load_tool(name: str) -> dict[str, Any]:
path = TOOL_DIR / f"{name}.json"
with open(path, "r") as file:
return json.load(file)
class ToolError(Exception):
"""Custom exception for tool-related errors."""
pass

def load_tool(name: str) -> Dict[str, Any]:
try:
path = TOOL_DIR / f"{name}.json"
with open(path, "r") as file:
return json.load(file)
except FileNotFoundError:
raise ToolError(f"Tool file not found: {name}.json")
except json.JSONDecodeError:
raise ToolError(f"Invalid JSON in tool file: {name}.json")

def get_all_tools() -> dict[str, dict[str, Any]]:
def get_all_tools() -> Dict[str, Dict[str, Any]]:
tools = {}
for file in TOOL_DIR.glob("*.json"):
tool_name = file.stem
tools[tool_name] = load_tool(tool_name)
try:
tools[tool_name] = load_tool(tool_name)
except ToolError as e:
logger.log.error(f"Error loading tool {tool_name}: {str(e)}")
return tools


TOOLS = get_all_tools()
try:
TOOLS = get_all_tools()
except Exception as e:
logger.log.critical(f"Failed to load tools: {str(e)}")
TOOLS = {}
Loading