-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Description
Code Inconsistencies and Proposed Changes
After reviewing the codebase, I've identified several inconsistencies and areas for improvement:
1. File Path Resolution Inconsistency
Issue: File path resolution happens in multiple places with slight variations:
chat.pylines 41-46: Resolves paths when initializing LLMChatfile_processor.py_resolve_file_path(): Another resolution method- Menu uses
os.path.relpath()while other code usesPath.resolve()
Proposed Changes:
# Create centralized path resolution utility in file_processor.py or new utils.py:
def resolve_path(path: str, root_dir: str, relative: bool = False) -> str:
"""
Centralized path resolution.
Args:
path: Input path (absolute or relative)
root_dir: Root directory for relative path resolution
relative: If True, return path relative to root_dir
Returns:
Resolved absolute or relative path
"""
path_obj = Path(path).expanduser()
if not path_obj.is_absolute():
path_obj = (Path(root_dir) / path_obj).resolve()
else:
path_obj = path_obj.resolve()
if relative:
try:
return str(path_obj.relative_to(Path(root_dir).resolve()))
except ValueError:
return str(path_obj)
return str(path_obj)
# Then use this function everywhere paths are resolved2. Error Handling Inconsistencies
Issue: Some functions have comprehensive error handling, others are minimal:
file_processor.py: Excellent error handling with specific exceptionsinput_handler.py: Minimal error handling in_get_terminal_width()menu.py: No error handling for file operations
Proposed Changes:
# Add try-except blocks to menu.py methods:
def action_delete_selected(self) -> None:
try:
# ... existing code ...
except Exception as e:
self.notify(f"Error deleting file: {e}", severity="error")
# Add error handling to input_handler.py:
def get_input_with_editing(self, default: str = "") -> str:
try:
return session.prompt(default=default)
except (KeyboardInterrupt, EOFError):
return None
except Exception as e:
logger.error(f"Input error: {e}")
return None3. Token Budget Tag Not Used
Issue: The XML response includes <budget:token_budget>1000000</budget:token_budget> but this is never parsed or enforced.
Proposed Changes:
# Either remove the tag from the XML generation, or implement token budget checking:
# In file_processor.py, parse_response():
def extract_token_budget(response: str) -> int:
"""Extract token budget from response if present."""
pattern = r'<budget:token_budget>(\d+)</budget:token_budget>'
match = re.search(pattern, response)
return int(match.group(1)) if match else None
# Then warn if response exceeds budget4. Command Handler Returns
Issue: Command handler methods return False or True to indicate "should quit", but this is not documented and could be confusing.
Proposed Changes:
# Add type hints and docstring clarity:
def handle_command(self, user_input: str) -> bool:
"""
Handle special commands.
Args:
user_input: The command string starting with '/'
Returns:
True if application should quit, False otherwise
"""
# ... existing code ...Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels