Refactor: Library #545
Refactor: Library #545
Conversation
WalkthroughIntroduces the Potpie library—an AI-powered code analysis package featuring a modular architecture for code parsing, indexing, querying, and conversation. Comprises configuration management, type definitions, analysis components (Parser, Indexer, QueryEngine), conversation handling (ChatEngine, ConversationMemory), runtime infrastructure (Storage, GraphStore, TaskRunner, AsyncQueue), LLM integration, and a CLI with index, ask, chat, and serve commands. All operational methods are currently scaffold stubs raising NotImplementedError. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
potpie/types.py (2)
18-29: Critical: Duplicate CodeNode definition.This
CodeNodedefinition conflicts withpotpie/types/graph.py. See the comment onpotpie/types/graph.pyfor details on the critical inconsistencies that must be resolved.
32-39: Critical: Duplicate CodeEdge definition.This
CodeEdgedefinition conflicts withpotpie/types/graph.py. See the comment onpotpie/types/graph.pyfor details on the critical inconsistencies that must be resolved.
🧹 Nitpick comments (12)
potpie/config.py (1)
27-35:project_pathis not updated to the resolved absolute path.The resolved
projectPath is used to computedb_pathandgraph_path, butself.project_pathremains as the original value (e.g.,"."). This creates an inconsistency—db_pathcontains an absolute path whileproject_pathstays relative.🔎 Proposed fix
def __post_init__(self): project = Path(self.project_path).resolve() + self.project_path = str(project) potpie_dir = project / ".potpie" if self.db_path is None: self.db_path = str(potpie_dir / "potpie.db") if self.graph_path is None: self.graph_path = str(potpie_dir / "graph.json")potpie/runtime/tasks.py (1)
94-96: Consider makingget_statussynchronous.This method is a simple dict lookup with no I/O. Making it sync would be more consistent with
cleanup_completedand avoid unnecessary async overhead.- async def get_status(self, task_id: str) -> Optional[TaskInfo]: + def get_status(self, task_id: str) -> Optional[TaskInfo]: """Get status of a task.""" return self._tasks.get(task_id)potpie/types/embeddings.py (1)
4-10: Consider more specific type hints formetadata.For consistency with other types in this PR (e.g.,
ChatMessage.metadata: dict | None), consider specifying the dict type parameters and whether it can beNone.+from typing import Any + + @dataclass class VectorSearchResult: """Result from vector similarity search.""" node_id: str score: float - metadata: dict + metadata: dict[str, Any]potpie/analysis/__init__.py (1)
7-7: Consider sorting__all__alphabetically.Static analysis (Ruff RUF022) suggests sorting
__all__for consistency. However, the current order (Parser → Indexer → QueryEngine) follows a logical workflow, so this is purely a style preference.-__all__ = ["Parser", "Indexer", "QueryEngine"] +__all__ = ["Indexer", "Parser", "QueryEngine"]potpie/runtime/__init__.py (1)
9-17: Consider sorting all for consistency.The static analysis tool suggests applying an isort-style sort to
__all__. While not critical, alphabetically sorting the exports improves maintainability and reduces merge conflicts.Proposed refactor
__all__ = [ + "AsyncQueue", + "GraphStore", + "NetworkXGraphStore", "Potpie", "Storage", "SQLiteStorage", - "GraphStore", - "NetworkXGraphStore", "TaskRunner", - "AsyncQueue", ]potpie/analysis/parsing.py (1)
53-71: Consider documenting language detection limitations.The
_detect_languagemethod handles common file extensions well. Consider adding a docstring noting limitations such as:
.hfiles are always treated as C (could be C++ headers)- Some languages like C# (
.cs), Swift (.swift), Kotlin (.kt), or Scala (.scala) are not supported- Case-sensitivity relies on
.lower()transformationThis helps future maintainers understand the scope.
Suggested documentation enhancement
def _detect_language(self, file_path: Path) -> Optional[str]: - """Detect programming language from file extension.""" + """Detect programming language from file extension. + + Note: This method has limitations: + - .h files are treated as C (may be C++ headers) + - Does not support all languages (e.g., C#, Swift, Kotlin, Scala) + - Returns None for unsupported extensions + """ extension_map = {potpie/conversations/memory.py (2)
39-58: Consider using MessageRole enum for type safety.The
roleparameter (line 42) is defined asstr. For consistency withpotpie/types/base.py, consider using theMessageRoleenum to enforce valid roles and provide better IDE support.Proposed refactor
+from potpie.types.base import MessageRole + async def add_message( self, conversation_id: str, - role: str, + role: MessageRole, content: str, ) -> str: """Add a message to a conversation. Args: conversation_id: Conversation to add to. - role: Message role ("user", "assistant", "system"). + role: Message role. content: Message content.
79-96: Consider stronger typing for conversation metadata.The
list_conversationsmethod returnslist[dict]. Consider defining a typed dataclass or TypedDict for conversation metadata to improve type safety and make the expected structure explicit.For example, you could define:
@dataclass class ConversationInfo: id: str title: Optional[str] created_at: datetime message_count: int last_activity: datetimeThen change the return type to
list[ConversationInfo].potpie/analysis/indexing.py (1)
76-83: Consider stronger typing for index status.The
get_index_statusmethod returnsdict. Consider defining a typed structure (dataclass or TypedDict) to make the expected status fields explicit and improve type safety.For example:
@dataclass class IndexStatus: total_files: int indexed_files: int total_nodes: int total_edges: int last_indexed: Optional[datetime] is_indexing: boolThen change the return type to
IndexStatus.potpie/runtime/queue.py (1)
24-31: Consider renamingmaxlentomaxsizefor consistency.The parameter is named
maxlenbut is passed asmaxsizetoasyncio.Queue. Usingmaxsizethroughout would align with asyncio's terminology and reduce potential confusion.potpie/runtime/api.py (1)
82-97: Consider alternative pattern for async generator stub.The unreachable
yieldstatement is necessary to make this an async generator, but it creates a code smell. Consider using a cleaner pattern for async generator stubs:🔎 Alternative implementation
async def chat_stream( self, message: str, conversation_id: Optional[str] = None ) -> AsyncIterator[str]: """Stream a chat response. Args: message: User's message. conversation_id: Optional ID to continue existing conversation. Yields: Chunks of the assistant's response. """ await self._ensure_initialized() - # TODO: Implement streaming - raise NotImplementedError("chat_stream() not yet implemented") - yield # Make this a generator + # TODO: Implement streaming + if False: # Pragma: no cover + yield # Make this a generator + raise NotImplementedError("chat_stream() not yet implemented")This makes the yield conditionally unreachable while preserving the generator signature.
potpie/conversations/engine.py (1)
69-85: Consider alternative pattern for async generator stub.The unreachable
yieldstatement at line 85 is necessary to make this an async generator, but creates a code smell. Consider the same pattern suggested forpotpie/runtime/api.py:chat_stream():🔎 Alternative implementation
async def chat_stream( self, message: str, conversation_id: Optional[str] = None, ) -> AsyncIterator[str]: """Stream a chat response. Args: message: User's message. conversation_id: Optional conversation ID. Yields: Chunks of the response. """ - # TODO: Implement streaming - raise NotImplementedError("ChatEngine.chat_stream() not yet implemented") - yield # Make this a generator + # TODO: Implement streaming + if False: # Pragma: no cover + yield # Make this a generator + raise NotImplementedError("ChatEngine.chat_stream() not yet implemented")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
potpie/__init__.py(1 hunks)potpie/analysis/__init__.py(1 hunks)potpie/analysis/indexing.py(1 hunks)potpie/analysis/parsing.py(1 hunks)potpie/analysis/queries.py(1 hunks)potpie/cli.py(1 hunks)potpie/config.py(1 hunks)potpie/conversations/__init__.py(1 hunks)potpie/conversations/engine.py(1 hunks)potpie/conversations/memory.py(1 hunks)potpie/llm/__init__.py(1 hunks)potpie/llm/provider.py(1 hunks)potpie/runtime/__init__.py(1 hunks)potpie/runtime/api.py(1 hunks)potpie/runtime/graph_store.py(1 hunks)potpie/runtime/queue.py(1 hunks)potpie/runtime/storage.py(1 hunks)potpie/runtime/tasks.py(1 hunks)potpie/types.py(1 hunks)potpie/types/__init__.py(1 hunks)potpie/types/base.py(1 hunks)potpie/types/chat.py(1 hunks)potpie/types/embeddings.py(1 hunks)potpie/types/graph.py(1 hunks)pyproject.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
potpie/types/chat.py (1)
potpie/types.py (2)
ChatMessage(53-58)ChatResponse(62-67)
potpie/llm/__init__.py (1)
potpie/llm/provider.py (1)
LLMProvider(6-76)
potpie/types.py (2)
potpie/types/graph.py (2)
CodeNode(8-21)CodeEdge(25-32)potpie/types/chat.py (2)
ChatMessage(6-12)ChatResponse(16-21)
potpie/analysis/parsing.py (1)
potpie/types.py (3)
CodeEdge(33-39)CodeNode(19-29)ParseResult(43-49)
potpie/analysis/__init__.py (3)
potpie/analysis/parsing.py (1)
Parser(9-71)potpie/analysis/indexing.py (1)
Indexer(11-83)potpie/analysis/queries.py (1)
QueryEngine(10-98)
potpie/analysis/queries.py (4)
potpie/runtime/graph_store.py (1)
GraphStore(12-63)potpie/runtime/storage.py (3)
Storage(7-43)query(39-43)query(86-91)potpie/types.py (1)
CodeNode(19-29)potpie/types/graph.py (1)
CodeNode(8-21)
potpie/runtime/tasks.py (3)
app/modules/tasks/task_service.py (1)
create_task(11-18)potpie/runtime/queue.py (1)
get(89-109)potpie/runtime/storage.py (2)
get(24-26)get(71-74)
potpie/analysis/indexing.py (4)
potpie/analysis/parsing.py (1)
Parser(9-71)potpie/runtime/graph_store.py (1)
GraphStore(12-63)potpie/runtime/storage.py (1)
Storage(7-43)potpie/types.py (1)
ParseResult(43-49)
potpie/conversations/__init__.py (2)
potpie/conversations/engine.py (1)
ChatEngine(11-113)potpie/conversations/memory.py (1)
ConversationMemory(9-110)
potpie/conversations/memory.py (2)
potpie/runtime/storage.py (1)
Storage(7-43)potpie/types.py (1)
ChatMessage(53-58)
potpie/runtime/graph_store.py (1)
potpie/types.py (2)
CodeEdge(33-39)CodeNode(19-29)
potpie/runtime/storage.py (3)
potpie/runtime/graph_store.py (4)
initialize(18-20)initialize(82-85)close(22-24)close(87-90)potpie/runtime/api.py (1)
close(99-102)potpie/runtime/queue.py (1)
get(89-109)
potpie/types/graph.py (2)
potpie/types/base.py (1)
EdgeType(4-12)potpie/types.py (2)
CodeNode(19-29)CodeEdge(33-39)
potpie/runtime/__init__.py (5)
potpie/runtime/api.py (1)
Potpie(9-102)potpie/runtime/storage.py (2)
Storage(7-43)SQLiteStorage(46-91)potpie/runtime/graph_store.py (2)
GraphStore(12-63)NetworkXGraphStore(66-133)potpie/runtime/tasks.py (1)
TaskRunner(42-161)potpie/runtime/queue.py (1)
AsyncQueue(18-138)
potpie/__init__.py (2)
potpie/config.py (1)
PotpieConfig(10-42)potpie/runtime/api.py (1)
Potpie(9-102)
potpie/types/__init__.py (4)
potpie/types/base.py (2)
EdgeType(4-12)MessageRole(15-20)potpie/types/chat.py (2)
ChatMessage(6-12)ChatResponse(16-21)potpie/types/graph.py (2)
CodeNode(8-21)CodeEdge(25-32)potpie/types/embeddings.py (1)
VectorSearchResult(5-10)
🪛 Ruff (0.14.8)
potpie/runtime/api.py
52-52: Unused method argument: question
(ARG002)
52-52: Unused method argument: repo_path
(ARG002)
67-67: Unused method argument: message
(ARG002)
67-67: Unused method argument: conversation_id
(ARG002)
83-83: Unused method argument: message
(ARG002)
83-83: Unused method argument: conversation_id
(ARG002)
potpie/conversations/engine.py
71-71: Unused method argument: message
(ARG002)
72-72: Unused method argument: conversation_id
(ARG002)
potpie/analysis/__init__.py
7-7: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
potpie/runtime/tasks.py
75-75: Consider moving this statement to an else block
(TRY300)
98-98: Unused method argument: timeout
(ARG002)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
potpie/runtime/__init__.py
9-17: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
potpie/llm/provider.py
46-46: Unused method argument: messages
(ARG002)
47-47: Unused method argument: temperature
(ARG002)
48-48: Unused method argument: max_tokens
(ARG002)
49-49: Unused method argument: kwargs
(ARG002)
potpie/types/__init__.py
7-20: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🔇 Additional comments (15)
pyproject.toml (1)
1-3: LGTM! Build system and CLI entry point configured correctly.The hatchling build system is a modern choice, and the console script entry point is properly defined to wire up the CLI.
Also applies to: 111-112
potpie/config.py (1)
37-42: LGTM!The
ensure_directoriesmethod correctly creates parent directories withparents=Trueandexist_ok=True.potpie/runtime/tasks.py (1)
131-161: LGTM!The
cancelandcleanup_completedmethods are correctly implemented with proper state checks.potpie/conversations/__init__.py (1)
1-6: LGTM!Clean package initialization with appropriate re-exports and
__all__definition.potpie/types/__init__.py (1)
1-20: LGTM!The categorical organization of
__all__with comments is more readable than strict alphabetical sorting. The static analysis hint (RUF022) can be safely ignored here since the grouping is intentional and aids maintainability.potpie/llm/__init__.py (1)
1-5: LGTM!Clean package initialization with proper re-export of
LLMProviderand__all__declaration.potpie/__init__.py (1)
1-7: LGTM!Clean package initialization establishing the public API surface with
Potpie,PotpieConfig, and version metadata.potpie/types/base.py (1)
1-20: LGTM! Clean enum definitions.The EdgeType and MessageRole enums are well-structured with appropriate string values and clear docstrings. The enum pattern follows Python best practices.
potpie/llm/provider.py (1)
6-76: LGTM! Well-structured LLM provider interface.The
LLMProviderclass defines a clean interface for LLM operations (completion, streaming, and embeddings). The stub implementations with descriptive TODO comments and NotImplementedError exceptions are appropriate for this scaffolding phase.Note: The static analysis warnings about unused arguments in
stream()are false positives since these are intentional stubs.potpie/analysis/parsing.py (1)
9-51: LGTM! Well-structured parser interface.The
Parserclass provides a clean interface for tree-sitter based parsing with appropriate async methods. The stub implementations are appropriate for this scaffolding phase.potpie/conversations/memory.py (1)
9-37: LGTM! Clean conversation management interface.The
ConversationMemoryclass provides well-documented methods for managing conversation history. The stub implementations are appropriate for this scaffolding phase.potpie/analysis/indexing.py (1)
11-74: LGTM! Well-designed orchestration layer.The
Indexerclass provides a clean separation of concerns, coordinating between the Parser, GraphStore, and Storage components. The method signatures are well-documented with clear parameter and return type descriptions. The stub implementations are appropriate for this scaffolding phase.potpie/runtime/queue.py (2)
79-87: Clarify timeout behavior in subscribe.The
subscribemethod breaks and stops yielding messages after the first timeout. This means if no message arrives within the timeout period, the subscription ends permanently. Consider whether the intended behavior is:
- Stop subscription on first timeout (current behavior)
- Continue waiting and only timeout individual message waits
- Allow the caller to control this behavior
If option 2 is preferred, consider removing the
breakstatement.
53-61: Add retry loop to handle potential race condition.The second
put_nowaitat line 61 can also raiseQueueFullif another coroutine fills the queue between lines 58 and 61. While unlikely, this could cause message loss in high-concurrency scenarios.🔎 Proposed fix with retry loop
- try: - queue.put_nowait(message) - except asyncio.QueueFull: - # Remove oldest message if full - try: - queue.get_nowait() - except asyncio.QueueEmpty: - pass - queue.put_nowait(message) + while True: + try: + queue.put_nowait(message) + break + except asyncio.QueueFull: + # Remove oldest message if full + try: + queue.get_nowait() + except asyncio.QueueEmpty: + # Queue was emptied by another coroutine, retry + continueLikely an incorrect or invalid review comment.
potpie/cli.py (1)
46-53: No issues found. TheParseResulttype inpotpie/typesincludes all accessed attributes:file_count,nodes,edges, anderror_count.
| response = await pp.ask(question) | ||
| click.echo(response.content) | ||
| if response.sources: | ||
| click.echo("\nSources:") | ||
| for source in response.sources: | ||
| click.echo(f" - {source}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find ChatResponse class definition in the repository
find . -type f -name "*.py" | head -20Repository: potpie-ai/potpie
Length of output: 1587
🏁 Script executed:
#!/bin/bash
# Search for ChatResponse class definition in Python files
rg "class ChatResponse" --type py -A 10Repository: potpie-ai/potpie
Length of output: 555
🏁 Script executed:
#!/bin/bash
# Also search for imports of ChatResponse
rg "import.*ChatResponse|from.*ChatResponse" --type pyRepository: potpie-ai/potpie
Length of output: 283
🏁 Script executed:
#!/bin/bash
# Search for response.content and response.sources patterns
rg "response\.content|response\.sources" --type py -B 2 -A 2Repository: potpie-ai/potpie
Length of output: 1250
🏁 Script executed:
#!/bin/bash
# Check the actual ChatResponse definitions in both files
echo "=== Content of potpie/types.py ==="
cat -n potpie/types.py | head -30
echo ""
echo "=== Content of potpie/types/chat.py ==="
cat -n potpie/types/chat.py | head -40
echo ""
echo "=== Content of potpie/types/__init__.py ==="
cat -n potpie/types/__init__.pyRepository: potpie-ai/potpie
Length of output: 2067
🏁 Script executed:
#!/bin/bash
# Check what cli.py imports from types
grep -n "from potpie.types import\|from .types import\|import.*ChatResponse" potpie/cli.py | head -20Repository: potpie-ai/potpie
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Double check what cli.py actually imports and uses
head -50 potpie/cli.py | grep -E "import|from"Repository: potpie-ai/potpie
Length of output: 198
🏁 Script executed:
#!/bin/bash
# Check if ChatResponse is defined later in potpie/types.py (not just first 30 lines)
cat potpie/types.pyRepository: potpie-ai/potpie
Length of output: 1353
🏁 Script executed:
#!/bin/bash
# Also check the Potpie class to see what ask() method returns
rg "def ask|async def ask" --type py -A 10Repository: potpie-ai/potpie
Length of output: 3222
🏁 Script executed:
#!/bin/bash
# Verify the exact import chain
echo "=== What potpie/runtime/api.py imports ==="
grep -n "from potpie.types import\|from.*ChatResponse" potpie/runtime/api.py | head -5
echo ""
echo "=== ChatResponse definition locations ==="
echo "1. In potpie/types.py (line 59-63):"
rg "class ChatResponse:" --type py -A 4 -B 1 potpie/types.py
echo ""
echo "2. In potpie/types/chat.py (line 15-20):"
rg "class ChatResponse:" --type py -A 4 -B 1 potpie/types/chat.pyRepository: potpie-ai/potpie
Length of output: 499
ChatResponse structure mismatch will cause AttributeError: The exported ChatResponse from potpie.types (imported from potpie.types.chat) has attributes message, usage, and timestamp, but the code accesses response.content and response.sources which do not exist. Either export the ChatResponse definition from potpie/types.py (which has the correct content and sources attributes) or refactor to access response.message.content and handle sources differently.
🤖 Prompt for AI Agents
In potpie/cli.py around lines 88-93, the code assumes ChatResponse has content
and sources but the imported type has attributes message, usage, and timestamp;
update the call site to use the actual shape: replace response.content with
response.message.content (or access the content field on response.message) and
replace response.sources with the correct location for sources (e.g.,
response.message.sources if sources are nested on message) — alternatively, if
you prefer minimal change, re-export the ChatResponse definition from
potpie/types.py so the imported type matches the code; implement one of these
fixes and ensure you null-check message before accessing its fields to avoid
AttributeError.
| conversation_id = None | ||
|
|
||
| click.echo(f"Potpie v{__version__} - Interactive Code Chat") | ||
| click.echo(f"Repository: {Path(path).resolve()}") | ||
| click.echo("Type 'quit' or 'exit' to end the session.\n") | ||
|
|
||
| try: | ||
| while True: | ||
| try: | ||
| user_input = click.prompt("You", prompt_suffix="> ") | ||
| except click.Abort: | ||
| break | ||
|
|
||
| if user_input.lower() in ("quit", "exit", "q"): | ||
| break | ||
|
|
||
| if not user_input.strip(): | ||
| continue | ||
|
|
||
| try: | ||
| response = await pp.chat(user_input, conversation_id) | ||
| click.echo(f"\nAssistant: {response.content}\n") | ||
| except NotImplementedError as e: | ||
| click.echo(f"\nNot yet implemented: {e}\n", err=True) | ||
| break |
There was a problem hiding this comment.
Update conversation_id to maintain chat context.
The conversation_id is initialized to None (line 126) but never updated after receiving responses. This means each message starts a new conversation instead of continuing the existing one. The chat response should contain a conversation ID that must be captured and reused for subsequent messages.
🔎 Proposed fix
try:
response = await pp.chat(user_input, conversation_id)
+ conversation_id = conversation_id or response.conversation_id
click.echo(f"\nAssistant: {response.content}\n")
except NotImplementedError as e:Note: This assumes ChatResponse includes a conversation_id attribute, which should be verified in the type definition.
🤖 Prompt for AI Agents
In potpie/cli.py around lines 126 to 150, conversation_id is initialized to None
but never updated after each chat response, so subsequent messages don't reuse
the conversation context; update the loop to capture the conversation_id
returned on successful responses (e.g., set conversation_id =
response.conversation_id) immediately after receiving response.content so future
calls pass the existing ID, and ensure the ChatResponse type actually exposes
conversation_id before relying on it.
|
|
||
| id: str | ||
| data: Any | ||
| timestamp: float = field(default_factory=lambda: asyncio.get_event_loop().time()) |
There was a problem hiding this comment.
Replace deprecated asyncio.get_event_loop() with safer alternative.
The asyncio.get_event_loop() call is deprecated and can raise exceptions if no event loop is running. For timestamp generation, use time.time() which is simpler and doesn't depend on asyncio context.
🔎 Proposed fix
+import time
from dataclasses import dataclass, field
- timestamp: float = field(default_factory=lambda: asyncio.get_event_loop().time())
+ timestamp: float = field(default_factory=time.time)🤖 Prompt for AI Agents
In potpie/runtime/queue.py around line 15 the dataclass default_factory uses
asyncio.get_event_loop().time() which is deprecated and can fail when no event
loop exists; replace it with time.time() for a simple, synchronous timestamp
source, and add an import for the time module at the top of the file (or update
existing imports) so the default_factory becomes a lambda calling time.time().
| asyncio_task = asyncio.create_task(wrapped()) | ||
| self._tasks[task_id] = TaskInfo( | ||
| task_id=task_id, | ||
| name=task_name, | ||
| status=TaskStatus.PENDING, | ||
| asyncio_task=asyncio_task, | ||
| ) | ||
|
|
||
| return task_id |
There was a problem hiding this comment.
Race condition: TaskInfo may not exist when wrapped() accesses it.
asyncio.create_task(wrapped()) can schedule execution before the TaskInfo is added to _tasks. If the coroutine runs immediately (e.g., hits an await point or fast path), self._tasks[task_id] on line 71 will raise KeyError.
🔎 Proposed fix: Create TaskInfo first, then assign the asyncio task
- asyncio_task = asyncio.create_task(wrapped())
- self._tasks[task_id] = TaskInfo(
+ self._tasks[task_id] = TaskInfo(
task_id=task_id,
name=task_name,
status=TaskStatus.PENDING,
- asyncio_task=asyncio_task,
+ asyncio_task=None,
)
+ asyncio_task = asyncio.create_task(wrapped())
+ self._tasks[task_id].asyncio_task = asyncio_task
return task_id📝 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.
| asyncio_task = asyncio.create_task(wrapped()) | |
| self._tasks[task_id] = TaskInfo( | |
| task_id=task_id, | |
| name=task_name, | |
| status=TaskStatus.PENDING, | |
| asyncio_task=asyncio_task, | |
| ) | |
| return task_id | |
| self._tasks[task_id] = TaskInfo( | |
| task_id=task_id, | |
| name=task_name, | |
| status=TaskStatus.PENDING, | |
| asyncio_task=None, | |
| ) | |
| asyncio_task = asyncio.create_task(wrapped()) | |
| self._tasks[task_id].asyncio_task = asyncio_task | |
| return task_id |
🤖 Prompt for AI Agents
In potpie/runtime/tasks.py around lines 84 to 92, there is a race where
asyncio.create_task(wrapped()) may start running before the TaskInfo is stored
in self._tasks causing a KeyError; fix by constructing and inserting the
TaskInfo into self._tasks first (with a placeholder/None for asyncio_task or set
asyncio_task after creation), then call asyncio.create_task(wrapped()) and
assign the returned asyncio.Task back onto the existing TaskInfo.asyncio_task;
ensure the TaskStatus is set consistently (e.g., PENDING before creation, update
to RUNNING inside wrapped) and that no path reads self._tasks[task_id] before
it's inserted.
| async def wait(self, task_id: str, timeout: Optional[float] = None) -> TaskResult: | ||
| """Wait for a task to complete. | ||
|
|
||
| Args: | ||
| task_id: Task to wait for. | ||
| timeout: Optional timeout in seconds. | ||
|
|
||
| Returns: | ||
| TaskResult with outcome. | ||
|
|
||
| Raises: | ||
| KeyError: If task_id not found. | ||
| asyncio.TimeoutError: If timeout exceeded. | ||
| """ | ||
| info = self._tasks.get(task_id) | ||
| if not info: | ||
| raise KeyError(f"Task {task_id} not found") | ||
|
|
||
| # if info.asyncio_task: | ||
| # try: | ||
| # await asyncio.wait_for(info.asyncio_task, timeout=timeout) | ||
| # except asyncio.TimeoutError: | ||
| # raise | ||
| # except Exception: | ||
| # pass # Error captured in task wrapper | ||
|
|
||
| return TaskResult( | ||
| task_id=task_id, | ||
| status=info.status, | ||
| result=info.result, | ||
| error=info.error, | ||
| ) |
There was a problem hiding this comment.
timeout parameter is documented but unused.
The method signature and docstring claim timeout support with asyncio.TimeoutError, but the implementation ignores it entirely. The commented-out code (lines 116-122) suggests this was intentional deferral.
Either implement the timeout or remove it from the signature to avoid a misleading API.
🔎 Option 1: Implement timeout support
if not info:
raise KeyError(f"Task {task_id} not found")
- # if info.asyncio_task:
- # try:
- # await asyncio.wait_for(info.asyncio_task, timeout=timeout)
- # except asyncio.TimeoutError:
- # raise
- # except Exception:
- # pass # Error captured in task wrapper
+ if info.asyncio_task and not info.asyncio_task.done():
+ try:
+ await asyncio.wait_for(
+ asyncio.shield(info.asyncio_task), timeout=timeout
+ )
+ except asyncio.TimeoutError:
+ raise
+ except Exception:
+ pass # Error captured in task wrapper
return TaskResult(🔎 Option 2: Remove timeout from signature
- async def wait(self, task_id: str, timeout: Optional[float] = None) -> TaskResult:
+ async def wait(self, task_id: str) -> TaskResult:
"""Wait for a task to complete.
Args:
task_id: Task to wait for.
- timeout: Optional timeout in seconds.
Returns:
TaskResult with outcome.
Raises:
KeyError: If task_id not found.
- asyncio.TimeoutError: If timeout exceeded.
"""📝 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.
| async def wait(self, task_id: str, timeout: Optional[float] = None) -> TaskResult: | |
| """Wait for a task to complete. | |
| Args: | |
| task_id: Task to wait for. | |
| timeout: Optional timeout in seconds. | |
| Returns: | |
| TaskResult with outcome. | |
| Raises: | |
| KeyError: If task_id not found. | |
| asyncio.TimeoutError: If timeout exceeded. | |
| """ | |
| info = self._tasks.get(task_id) | |
| if not info: | |
| raise KeyError(f"Task {task_id} not found") | |
| # if info.asyncio_task: | |
| # try: | |
| # await asyncio.wait_for(info.asyncio_task, timeout=timeout) | |
| # except asyncio.TimeoutError: | |
| # raise | |
| # except Exception: | |
| # pass # Error captured in task wrapper | |
| return TaskResult( | |
| task_id=task_id, | |
| status=info.status, | |
| result=info.result, | |
| error=info.error, | |
| ) | |
| async def wait(self, task_id: str, timeout: Optional[float] = None) -> TaskResult: | |
| """Wait for a task to complete. | |
| Args: | |
| task_id: Task to wait for. | |
| timeout: Optional timeout in seconds. | |
| Returns: | |
| TaskResult with outcome. | |
| Raises: | |
| KeyError: If task_id not found. | |
| asyncio.TimeoutError: If timeout exceeded. | |
| """ | |
| info = self._tasks.get(task_id) | |
| if not info: | |
| raise KeyError(f"Task {task_id} not found") | |
| if info.asyncio_task and not info.asyncio_task.done(): | |
| try: | |
| await asyncio.wait_for( | |
| asyncio.shield(info.asyncio_task), timeout=timeout | |
| ) | |
| except asyncio.TimeoutError: | |
| raise | |
| except Exception: | |
| pass # Error captured in task wrapper | |
| return TaskResult( | |
| task_id=task_id, | |
| status=info.status, | |
| result=info.result, | |
| error=info.error, | |
| ) |
| async def wait(self, task_id: str, timeout: Optional[float] = None) -> TaskResult: | |
| """Wait for a task to complete. | |
| Args: | |
| task_id: Task to wait for. | |
| timeout: Optional timeout in seconds. | |
| Returns: | |
| TaskResult with outcome. | |
| Raises: | |
| KeyError: If task_id not found. | |
| asyncio.TimeoutError: If timeout exceeded. | |
| """ | |
| info = self._tasks.get(task_id) | |
| if not info: | |
| raise KeyError(f"Task {task_id} not found") | |
| # if info.asyncio_task: | |
| # try: | |
| # await asyncio.wait_for(info.asyncio_task, timeout=timeout) | |
| # except asyncio.TimeoutError: | |
| # raise | |
| # except Exception: | |
| # pass # Error captured in task wrapper | |
| return TaskResult( | |
| task_id=task_id, | |
| status=info.status, | |
| result=info.result, | |
| error=info.error, | |
| ) | |
| async def wait(self, task_id: str) -> TaskResult: | |
| """Wait for a task to complete. | |
| Args: | |
| task_id: Task to wait for. | |
| Returns: | |
| TaskResult with outcome. | |
| Raises: | |
| KeyError: If task_id not found. | |
| """ | |
| info = self._tasks.get(task_id) | |
| if not info: | |
| raise KeyError(f"Task {task_id} not found") | |
| # if info.asyncio_task: | |
| # try: | |
| # await asyncio.wait_for(info.asyncio_task, timeout=timeout) | |
| # except asyncio.TimeoutError: | |
| # raise | |
| # except Exception: | |
| # pass # Error captured in task wrapper | |
| return TaskResult( | |
| task_id=task_id, | |
| status=info.status, | |
| result=info.result, | |
| error=info.error, | |
| ) |
🧰 Tools
🪛 Ruff (0.14.8)
98-98: Unused method argument: timeout
(ARG002)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In potpie/runtime/tasks.py around lines 98 to 129 the timeout parameter is
documented but unused; implement timeout support by, when info.asyncio_task
exists, awaiting it wrapped in asyncio.wait_for(timeout) so the caller gets
asyncio.TimeoutError on timeout, catching asyncio.TimeoutError to re-raise and
catching/ignoring other exceptions (they are captured in the task wrapper)
before returning TaskResult; also ensure asyncio is imported and update the
docstring/signature if you choose to remove timeout instead.
| @dataclass | ||
| class ChatMessage: | ||
| """A message in a conversation.""" | ||
|
|
||
| role: str # "user", "assistant", "system" | ||
| content: str | ||
| metadata: Optional[dict[str, Any]] = None |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Suggest using MessageRole enum for type safety.
The role field is defined as str with a comment indicating valid values. Consider using the MessageRole enum from potpie/types/base.py for stronger type safety and IDE autocomplete support.
Proposed refactor
+from potpie.types.base import MessageRole
+
@dataclass
class ChatMessage:
"""A message in a conversation."""
- role: str # "user", "assistant", "system"
+ role: MessageRole
content: str
metadata: Optional[dict[str, Any]] = None📝 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.
| @dataclass | |
| class ChatMessage: | |
| """A message in a conversation.""" | |
| role: str # "user", "assistant", "system" | |
| content: str | |
| metadata: Optional[dict[str, Any]] = None | |
| from potpie.types.base import MessageRole | |
| @dataclass | |
| class ChatMessage: | |
| """A message in a conversation.""" | |
| role: MessageRole | |
| content: str | |
| metadata: Optional[dict[str, Any]] = None |
🤖 Prompt for AI Agents
In potpie/types.py around lines 52 to 58, the ChatMessage.role is currently
typed as a plain string with a comment indicating valid values; replace that
with the MessageRole enum from potpie/types/base.py for stronger typing and IDE
support: import MessageRole at the top of the file and change the role
annotation to MessageRole (adjust any call sites where ChatMessage instances are
created to pass MessageRole.<VALUE> or cast/convert incoming strings to
MessageRole), keeping metadata as Optional[dict[str, Any]].
| @dataclass | ||
| class ChatMessage: | ||
| """A message in a conversation.""" | ||
|
|
||
| role: str # "user", "assistant", "system" | ||
| content: str | ||
| metadata: Optional[dict[str, Any]] = None | ||
|
|
||
|
|
||
| @dataclass | ||
| class ChatResponse: | ||
| """Response from the chat engine.""" | ||
|
|
||
| content: str | ||
| sources: list[str] | ||
| metadata: Optional[dict[str, Any]] = None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for ChatMessage and ChatResponse definitions across the codebase
echo "=== ChatMessage definitions ==="
rg -n -A 5 "class ChatMessage" --type py
echo -e "\n=== ChatResponse definitions ==="
rg -n -A 5 "class ChatResponse" --type py
echo -e "\n=== Imports of these types ==="
rg -n "from.*import.*(ChatMessage|ChatResponse)" --type py
echo -e "\n=== Check if potpie/types/chat.py exists ==="
find . -path "*/types/chat.py" -o -path "*/types.py" | head -20Repository: potpie-ai/potpie
Length of output: 2117
🏁 Script executed:
#!/bin/bash
# Check if types.py definitions are imported directly anywhere
echo "=== Direct imports from types.py (not through __init__) ==="
rg -n "from potpie\.types\.py import|from \.\.types import" --type py
echo -e "\n=== Check potpie/types/__init__.py content ==="
head -10 potpie/types/__init__.py
echo -e "\n=== All imports referencing types module ==="
rg -n "import.*ChatMessage|import.*ChatResponse" --type py | head -20Repository: potpie-ai/potpie
Length of output: 790
Remove duplicate ChatMessage and ChatResponse definitions from types.py.
The definitions at lines 52-67 in potpie/types.py are dead code—they're never imported or used anywhere in the codebase. All imports correctly route through potpie/types/__init__.py, which exports the canonical definitions from potpie/types/chat.py. The duplicate definitions in types.py differ significantly from the active ones (missing/extra fields like timestamp and metadata), creating confusion and maintenance burden. Delete the unused class definitions from types.py to keep a single source of truth.
🤖 Prompt for AI Agents
In potpie/types.py around lines 52 to 67, there are duplicate/dead dataclass
definitions for ChatMessage and ChatResponse that conflict with the canonical
types exported from potpie/types/chat.py; remove these class definitions (the
entire ChatMessage and ChatResponse blocks in that range) so the module relies
on the single source of truth in potpie/types/__init__.py and update any local
imports in this file to import the canonical types if needed.
| @dataclass | ||
| class ChatMessage: | ||
| """A message in a conversation.""" | ||
|
|
||
| role: str | ||
| content: str | ||
| timestamp: datetime | ||
| metadata: dict | None = None | ||
|
|
||
|
|
||
| @dataclass | ||
| class ChatResponse: | ||
| """Response from the LLM API.""" | ||
|
|
||
| message: ChatMessage | ||
| usage: dict | ||
| timestamp: datetime |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify if both files exist and examine their actual content
echo "=== Checking if both files exist ==="
ls -la potpie/types.py potpie/types/chat.py 2>&1 || true
echo -e "\n=== Contents of potpie/types.py ==="
if [ -f potpie/types.py ]; then
cat -n potpie/types.py
fi
echo -e "\n=== Contents of potpie/types/chat.py (lines 1-30) ==="
if [ -f potpie/types/chat.py ]; then
head -30 -n potpie/types/chat.py
fiRepository: potpie-ai/potpie
Length of output: 2157
🏁 Script executed:
#!/bin/bash
# Read the full contents of chat.py file
echo "=== Full contents of potpie/types/chat.py ==="
cat -n potpie/types/chat.py
echo -e "\n=== Check if __init__.py exists and re-exports these types ==="
if [ -f potpie/types/__init__.py ]; then
cat -n potpie/types/__init__.py
else
echo "No __init__.py found in potpie/types/"
fi
echo -e "\n=== Search for imports of ChatMessage and ChatResponse ==="
rg "from potpie\.types import|from potpie\.types\.chat import" --type=py -nRepository: potpie-ai/potpie
Length of output: 1849
Remove duplicate ChatMessage and ChatResponse definitions in potpie/types.py.
The ChatMessage and ChatResponse classes are defined both here in potpie/types/chat.py and again in potpie/types.py with different structures. However, potpie/types/__init__.py re-exports only the versions from chat.py, making the types.py definitions dead code.
| Field | potpie/types/chat.py |
potpie/types.py |
|---|---|---|
ChatMessage |
role, content, timestamp, metadata | role, content, metadata (no timestamp) |
ChatResponse |
message, usage, timestamp | content, sources, metadata |
Delete the duplicate definitions from potpie/types.py (lines 52–67) to avoid confusion and maintain a single source of truth.
🤖 Prompt for AI Agents
In potpie/types.py around lines 52 to 67, remove the duplicate ChatMessage and
ChatResponse dataclass definitions (they conflict with the canonical definitions
in potpie/types/chat.py which are re-exported); delete those lines so only the
chat.py versions remain as the single source of truth and update any imports in
the file if necessary to import the classes from potpie.types.chat instead of
using the removed local definitions.
| @dataclass | ||
| class CodeNode: | ||
| """Represents a node in the code knowledge graph.""" | ||
|
|
||
| id: str | ||
| type: str # class, function, variable, etc. | ||
| name: str | ||
| content: str | None = None | ||
| file_path: str | None = None | ||
| start_line: int | None = None | ||
| end_line: int | None = None | ||
| language: str | None = None | ||
| project_id: str | None = None | ||
| metadata: dict | None = None | ||
| embedding: List[float] | None = None | ||
|
|
||
|
|
||
| @dataclass | ||
| class CodeEdge: | ||
| """Represents an edge in the code knowledge graph.""" | ||
|
|
||
| id: str | ||
| source_id: str | ||
| target_id: str | ||
| edge_type: EdgeType | ||
| metadata: dict | None = None |
There was a problem hiding this comment.
Critical: Duplicate and inconsistent type definitions with potpie/types.py.
Both potpie/types.py and potpie/types/graph.py define CodeNode and CodeEdge with incompatible structures:
CodeNode differences:
- potpie/types.py:
node_type: NodeType(enum) vs potpie/types/graph.py:type: str(string) - potpie/types.py: lacks
language,project_id,embeddingfields - potpie/types.py: non-optional
file_path,start_line,end_linevs potpie/types/graph.py: all optional
CodeEdge differences:
- potpie/types.py:
edge_type: strvs potpie/types/graph.py:edge_type: EdgeType(strongly typed) - potpie/types/graph.py: includes
idfield, potpie/types.py does not
This duplication will cause import conflicts and type errors across the codebase. You must consolidate to a single canonical definition.
Recommended resolution
Choose one location for these types (recommend potpie/types/graph.py for the more complete definition) and remove from the other. Then update all imports across the codebase to use the canonical location.
If both versions are needed for different purposes, rename one set of types (e.g., LegacyCodeNode) and document the distinction clearly.
🤖 Prompt for AI Agents
In potpie/types/graph.py around lines 7-32 there are duplicate, inconsistent
definitions of CodeNode and CodeEdge that conflict with potpie/types.py;
consolidate by making potpie/types/graph.py the single canonical definition
(keep the more complete fields: type/name/content/file_path optionality,
language, project_id, embedding, id on edges, and use the preferred enums/types
for node/edge types), remove the duplicated definitions from potpie/types.py,
update all imports across the repo to import CodeNode and CodeEdge from
potpie.types.graph, and if backward compatibility is required either export
renamed legacy types (e.g., LegacyCodeNode) or add thin adapters; after changes
run project-wide type-check and search/replace to ensure no stale imports
remain.
1 participant
Refactor Potpie for library.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.