refactor: extract TODO formatting functions and simplify data structure

[mrT23]

PR Type

Enhancement

---

Description

• Extract TODO formatting functions from main conversion logic

• Simplify TODO data structure from line ranges to single line numbers

• Remove TODO summary field and details wrapper from output

• Add item truncation limit for TODO display

---

Changes diagram

flowchart LR
  A["TODO formatting logic"] --> B["Extract functions"]
  B --> C["format_todo_item()"]
  B --> D["format_todo_items()"]
  E["line_range tuple"] --> F["line_number integer"]
  G["TODO summary field"] --> H["Remove field"]

Loading

---

Changes walkthrough 📝

	Relevant files
Enhancement	utils.py Extract TODO formatting functions and simplify structure  pr_agent/algo/utils.py Extract format_todo_item() and format_todo_items() functions from inline code Simplify TODO item structure from line ranges to single line numbers Add MAX_ITEMS limit (5) for TODO display truncation Remove details wrapper and summary from TODO output formatting +49/-64
Configuration changes	pr_reviewer.py Update TODO scan configuration access method                          pr_agent/tools/pr_reviewer.py Change require_todo_scan access from direct attribute to get() method with default False +1/-1      pr_reviewer_prompts.toml Simplify TODO data model structure                                              pr_agent/settings/pr_reviewer_prompts.toml Change TodoSection from line_range tuple to line_number integer Remove todo_summary field from Review model Update field descriptions for simplified TODO structure +4/-9

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-for-open-source]

PR Reviewer Guide 🔍

(Review updated until commit ee36c02)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Exception Handling format_todo_item now calls git_provider.get_line_link without any try/except. If link generation fails, it could raise and break the markdown conversion flow. reference_link = git_provider.get_line_link(relevant_file, line_number, line_number) Type Validation The new code retrieves line_number without verifying its type or converting it to int. Non-integer or missing values could lead to invalid links or runtime errors. line_number = todo_item.get('line_number', '') Configuration Fetch Using get_settings().pr_reviewer.get("require_todo_scan", False) assumes pr_reviewer supports dict-style .get; if it’s a Pydantic model, this may always yield the default and disable todo scanning. 'require_todo_scan': get_settings().pr_reviewer.get("require_todo_scan", False),

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Latest suggestions up to ee36c02

Category	Suggestion	Impact
Possible issue	Add error handling for git provider The function calls git_provider.get_line_link() without error handling, which could cause crashes if the git provider is None or the method fails. Add proper error handling to prevent exceptions. pr_agent/algo/utils.py [1402-1406] def format_todo_item(todo_item: TodoItem, git_provider, gfm_supported) -> str: relevant_file = todo_item.get('relevant_file', '').strip() line_number = todo_item.get('line_number', '') content = todo_item.get('content', '') - reference_link = git_provider.get_line_link(relevant_file, line_number, line_number) + reference_link = None + try: + if git_provider and relevant_file and line_number: + reference_link = git_provider.get_line_link(relevant_file, line_number, line_number) + except Exception as e: + get_logger().exception(f"Error generating link: {e}") Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the refactored format_todo_item function lacks error handling for the git_provider.get_line_link() call. The original code had this protection, so its removal is a regression. Adding the try-except block prevents potential runtime errors and makes the code more robust.	Medium
More

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

---

Previous suggestions

✅ Suggestions up to commit 7c02678

Category	Suggestion	Impact
Possible issue	Add error handling for link generation The function calls git_provider.get_line_link() without error handling, which could raise exceptions if the git provider is None or the method fails. Add a try-except block to handle potential errors gracefully. pr_agent/algo/utils.py [1402-1406] def format_todo_item(todo_item: TodoItem, git_provider, gfm_supported) -> str: relevant_file = todo_item.get('relevant_file', '').strip() line_number = todo_item.get('line_number', '') content = todo_item.get('content', '') - reference_link = git_provider.get_line_link(relevant_file, line_number, line_number) + reference_link = None + try: + if git_provider and relevant_file and line_number: + reference_link = git_provider.get_line_link(relevant_file, line_number, line_number) + except Exception as e: + get_logger().exception(f"Error generating link: {e}") Suggestion importance[1-10]: 8 __ Why: The PR refactored the format_todo_item function but removed the existing error handling for git_provider.get_line_link(). This suggestion correctly identifies this regression and proposes re-adding the try-except block, which is crucial for preventing potential crashes.	Medium
	✅ Fix incomplete Union type annotation Suggestion Impact: The commit directly implemented the suggested fix by changing the type annotation from Union[List[TodoSection]] to Union[List[TodoSection], str] to match the field description code diff: - todo_sections: Union[List[TodoSection]] = Field(description="A list of TODO comments found in the PR code. Return 'No' (as a string) if there are no TODO comments in the PR") + todo_sections: Union[List[TodoSection], str] = Field(description="A list of TODO comments found in the PR code. Return 'No' (as a string) if there are no TODO comments in the PR") The Union type annotation is incomplete - it should include str type since the description mentions returning 'No' as a string when no TODO comments are found. This creates a type mismatch. pr_agent/settings/pr_reviewer_prompts.toml [113] -todo_sections: Union[List[TodoSection]] = Field(description="A list of TODO comments found in the PR code. Return 'No' (as a string) if there are no TODO comments in the PR") +todo_sections: Union[List[TodoSection], str] = Field(description="A list of TODO comments found in the PR code. Return 'No' (as a string) if there are no TODO comments in the PR") [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the type hint for todo_sections was changed to Union[List[TodoSection]], which is inconsistent with the description stating it can also be a string 'No'. The old code correctly had Union[List[TodoSection], str]. This is a valid correction for type consistency.	Medium

[mrT23]

Persistent review updated to latest commit ee36c02

[mrT23]

Preparing review...

[mrT23]

Preparing review...

[mrT23]

Persistent review updated to latest commit ee36c02

[mrT23]

Persistent review updated to latest commit ee36c02