fix: startup-blocking SyntaxError and NameError prevent server from booting#659
fix: startup-blocking SyntaxError and NameError prevent server from booting#659arnavp27 wants to merge 2 commits intopotpie-ai:mainfrom
Conversation
…o blocks of code were de-indented outside their try blocks, causing SyntaxError on startup. Fixed by re-indenting the if/else block in update_file_lines_tool and the git_diffs block in show_diff_tool.
WalkthroughThe changes include a minor import addition in the history processor and refactoring of code change management tools to persist diff artifacts to JSON files and incorporate reasoning hash tracking from the reasoning manager. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/modules/intelligence/agents/chat_agents/history_processor.py (1)
18-18:⚠️ Potential issue | 🟡 MinorMissing
Dictimport will causeNameErrorat runtime.The import statement on line 18 does not include
Dict, yet it is used in multiple type annotations throughout the file (lines 119, 855, 1101, 1235). This will cause aNameErrorat runtime when these code paths are executed.Add
Dictto the typing imports:Proposed fix
-from typing import TYPE_CHECKING, Callable, List, Optional, Set, Tuple +from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Set, TupleAlternatively, convert to lowercase
dict(valid in Python 3.9+) for consistency with existing lowercase usage in the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/modules/intelligence/agents/chat_agents/history_processor.py` at line 18, The import line "from typing import TYPE_CHECKING, Callable, List, Optional, Set, Tuple" is missing Dict which causes NameError where annotations use Dict; update that import to include Dict (or alternatively replace those annotations with the built-in lowercase dict type for Python 3.9+), ensuring all usages of Dict in functions/classes in this module (the annotations referenced in the review) are valid.
🧹 Nitpick comments (1)
app/modules/intelligence/tools/code_changes_manager.py (1)
5462-5496: Add retention/rotation for.datadiff artifacts.A new UUID-named JSON file is created on every
show_diffcall, with no cleanup. On long-lived workers this can grow unbounded and pressure disk.Example safeguard
with open(filepath, "w", encoding="utf-8") as f: json.dump(diff_data, f, indent=2, ensure_ascii=False) + + # Best-effort retention to avoid unbounded artifact growth + max_artifacts = int(os.getenv("DIFF_ARTIFACT_MAX_FILES", "500")) + artifacts = sorted( + [ + os.path.join(data_dir, p) + for p in os.listdir(data_dir) + if p.startswith("diff_") and p.endswith(".json") + ], + key=os.path.getmtime, + ) + for stale in artifacts[:-max_artifacts]: + try: + os.remove(stale) + except OSError: + pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/modules/intelligence/tools/code_changes_manager.py` around lines 5462 - 5496, The .data folder currently accumulates one UUID-named JSON per show_diff invocation (see the show_diff_tool write block that creates data_dir, filename, filepath and writes diff_data with combined_diff and reasoning_hash), so add retention/rotation after the file is written: implement a small cleanup routine that lists files in data_dir (filtering the diff_*.json pattern), sorts by modification time, and either (a) removes files older than a configurable TTL (e.g. DIFF_RETENTION_DAYS) or (b) keeps only the newest N files (e.g. MAX_DIFF_FILES) and deletes the rest; wrap deletion in try/except and log failures via logger.warning so cleanup failures don’t break show_diff_tool.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/modules/intelligence/agents/chat_agents/history_processor.py`:
- Line 18: The import line "from typing import TYPE_CHECKING, Callable, List,
Optional, Set, Tuple" is missing Dict which causes NameError where annotations
use Dict; update that import to include Dict (or alternatively replace those
annotations with the built-in lowercase dict type for Python 3.9+), ensuring all
usages of Dict in functions/classes in this module (the annotations referenced
in the review) are valid.
---
Nitpick comments:
In `@app/modules/intelligence/tools/code_changes_manager.py`:
- Around line 5462-5496: The .data folder currently accumulates one UUID-named
JSON per show_diff invocation (see the show_diff_tool write block that creates
data_dir, filename, filepath and writes diff_data with combined_diff and
reasoning_hash), so add retention/rotation after the file is written: implement
a small cleanup routine that lists files in data_dir (filtering the diff_*.json
pattern), sorts by modification time, and either (a) removes files older than a
configurable TTL (e.g. DIFF_RETENTION_DAYS) or (b) keeps only the newest N files
(e.g. MAX_DIFF_FILES) and deletes the rest; wrap deletion in try/except and log
failures via logger.warning so cleanup failures don’t break show_diff_tool.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/modules/intelligence/agents/chat_agents/history_processor.pyapp/modules/intelligence/tools/code_changes_manager.py
1 participant
Problem
When trying to run the server locally following the README setup, the FastAPI
worker crashes immediately on boot before serving any requests. The server
cannot start at all due to 3 bugs hit in sequence during import:
Bug 1 & 2 —
code_changes_manager.pyTwo separate
try:blocks had their body code accidentally de-indented to theouter scope, making them syntactically invalid. Python requires every
try:tobe followed by
exceptorfinally— but instead it found unindented code,causing:
SyntaxError: expected 'except' or 'finally' block
line 4878: if/else block in update_file_lines_tool
line 5362: git_diffs block in show_diff_tool
These look like accidental de-indentation from a merge or refactor.
Bug 3 —
history_processor.pyModelResponseis used as a type annotation on line 494 insideTokenAwareHistoryProcessor._strip_problematic_tool_calls, but it was neverimported. Only
ModelMessagewas imported frompydantic_ai.messages.NameError: name 'ModelResponse' is not defined
Fix
try:bodies incode_changes_manager.pyModelResponseto the existing import line inhistory_processor.py:from pydantic_ai.messages import ModelMessage, ModelResponseVerification
After the fixes, the server boots and the health check passes:
$ curl http://localhost:8001/health
{"status":"ok","version":"f1674b80"}
Notes
Anyone trying to run potpie locally from the current
mainbranch will hitthese errors and be unable to start the server. No logic was changed —
only indentation and a missing import were fixed.
Summary by CodeRabbit
Release Notes
Refactor