xiaxinhang/pr-agent
1
0
Fork 0
mirror of https://github.com/qodo-ai/pr-agent.git synced 2025-07-02 03:40:38 +08:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1888 from qodo-ai/tr/simplify_toDo

Browse Source 
refactor: extract TODO formatting functions and simplify data structure
This commit is contained in:
Tal
2025-06-21 09:30:18 +03:00
committed by GitHub
parent 37ef4bad8f ee36c0208c
commit 75bde39b03
3 changed files with 54 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

113
pr_agent/algo/utils.py
View File

@ -224,81 +224,22 @@ def convert_to_markdown_v2(output_data: dict,
value = emphasize_header(value.strip(), only_markdown=True)
markdown_text += f"{value}\n\n"
elif 'todo sections' in key_nice.lower():
def format_todo_item(todo_item: TodoItem) -> str:
relevant_file = todo_item.get('relevant_file', '').strip()
line_range = todo_item.get('line_range', [])
content = todo_item.get('content', '')
reference_link = None
if isinstance(line_range, str):
line_range = ast.literal_eval(line_range.strip())
try:
if git_provider and relevant_file and line_range:
reference_link = git_provider.get_line_link(relevant_file, line_range[0], line_range[1])
except Exception as e:
get_logger().exception(f"Error generating link: {e}")
line_str = f"[{line_range[0]}]" if line_range[0] == line_range[1] else f"[{line_range[0]}-{line_range[1]}]"
return f"{relevant_file} {line_str}: {content}"
line_str = f"[{line_range[0]}]" if line_range[0] == line_range[1] else f"[{line_range[0]}-{line_range[1]}]"
file_ref = f"{relevant_file} {line_str}"
if reference_link:
if gfm_supported:
file_ref = f"<a href='{reference_link}'>{file_ref}</a>"
else:
file_ref = f"[{file_ref}]({reference_link})"
content_lines = content.strip().split("\n")
# if TODO content is single-line :
if len(content_lines) == 1:
return f"{file_ref}: {content_lines[0]}"
# else if TODO content is multi-line:
elif len(content_lines) > 1:
content_lines = "<br>".join(content_lines)
return f"{file_ref}: <blockquote>\n{content_lines}\n</blockquote>"
# else if TODO content is empty:
else:
return file_ref
def format_todo_items(value: list[TodoItem] | TodoItem) -> str:
markdown_text = ""
if gfm_supported:
if isinstance(value, list):
markdown_text += "<ul>\n"
for todo_item in value:
markdown_text += f"<li>{format_todo_item(todo_item)}</li>\n"
markdown_text += "</ul>\n"
else:
markdown_text += f"<p>{format_todo_item(value)}</p>\n"
else:
if isinstance(value, list):
for todo_item in value:
markdown_text += f"- {format_todo_item(todo_item)}\n"
else:
markdown_text += f"- {format_todo_item(value)}\n"
return markdown_text
if gfm_supported:
markdown_text += "<tr><td>"
if is_value_no(value):
markdown_text += f"{emoji}&nbsp;<strong>No TODO sections</strong>"
markdown_text += f"✅&nbsp;<strong>No TODO sections</strong>"
else:
markdown_todo_items = format_todo_items(value)
markdown_todo_items = format_todo_items(value, git_provider, gfm_supported)
markdown_text += f"{emoji}&nbsp;<strong>TODO sections</strong>\n<br><br>\n"
markdown_text += f"<details><summary>{todo_summary}</summary>\n\n"
markdown_text += markdown_todo_items
markdown_text += "\n</details>\n"
markdown_text += "</td></tr>\n"
else:
if is_value_no(value):
markdown_text += f"### {emoji} No TODO sections\n\n"
markdown_text += f"### ✅ No TODO sections\n\n"
else:
markdown_todo_items = format_todo_items(value)
markdown_text += f"### {emoji} TODO sections\n<details><summary>{todo_summary}</summary>\n\n"
markdown_todo_items = format_todo_items(value, git_provider, gfm_supported)
markdown_text += f"### {emoji} TODO sections\n\n"
markdown_text += markdown_todo_items
markdown_text += "\n</details>\n\n"
elif 'can be split' in key_nice.lower():
if gfm_supported:
markdown_text += f"<tr><td>"
@ -1457,3 +1398,47 @@ def set_file_languages(diff_files) -> List[FilePatchInfo]:
get_logger().exception(f"Failed to set file languages: {e}")
return diff_files
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)
file_ref = f"{relevant_file} [{line_number}]"
if reference_link:
if gfm_supported:
file_ref = f"<a href='{reference_link}'>{file_ref}</a>"
else:
file_ref = f"[{file_ref}]({reference_link})"
if content:
return f"{file_ref}: {content.strip()}"
else:
# if content is empty, return only the file reference
return file_ref
def format_todo_items(value: list[TodoItem] | TodoItem, git_provider, gfm_supported) -> str:
markdown_text = ""
MAX_ITEMS = 5 # limit the number of items to display
if gfm_supported:
if isinstance(value, list):
markdown_text += "<ul>\n"
if len(value) > MAX_ITEMS:
get_logger().debug(f"Truncating todo items to {MAX_ITEMS} items")
value = value[:MAX_ITEMS]
for todo_item in value:
markdown_text += f"<li>{format_todo_item(todo_item, git_provider, gfm_supported)}</li>\n"
markdown_text += "</ul>\n"
else:
markdown_text += f"<p>{format_todo_item(value, git_provider, gfm_supported)}</p>\n"
else:
if isinstance(value, list):
if len(value) > MAX_ITEMS:
get_logger().debug(f"Truncating todo items to {MAX_ITEMS} items")
value = value[:MAX_ITEMS]
for todo_item in value:
markdown_text += f"- {format_todo_item(todo_item, git_provider, gfm_supported)}\n"
else:
markdown_text += f"- {format_todo_item(value, git_provider, gfm_supported)}\n"
return markdown_text

13
pr_agent/settings/pr_reviewer_prompts.toml
View File

@ -74,9 +74,9 @@ class KeyIssuesComponentLink(BaseModel):
{%- if require_todo_scan %}
class TodoSection(BaseModel):
relevant_file: str = Field(description="The file containing the TODO comment")
line_range: Tuple[int, int] = Field(description="Start and end line numbers of the TODO comment (inclusive). Must be a tuple of two integers, e.g., (7, 7) for a single line or (7, 10) for a range. Do not use list format [7, 7].")
content: str = Field(description="The content of the TODO comment. Only include actual TODO comments within code comments (e.g., lines starting with '#', '//', '/*', '<!--'). Remove leading 'TODO' prefixes. Include lines like '# TODO', even if empty after prefix removal. Exclude TODOs outside comments or without appropriate prefixes. Use | block style only when the line range covers multiple lines (e.g., (7, 10)).")
relevant_file: str = Field(description="The full path of the file containing the TODO comment")
line_number: int = Field(description="The line number where the TODO comment starts")
content: str = Field(description="The content of the TODO comment. Only include actual TODO comments within code comments (e.g., comments starting with '#', '//', '/*', '<!--', ...). Remove leading 'TODO' prefixes. If more than 10 words, summarize the TODO comment to a single short sentence up to 10 words.")
{%- endif %}
{%- if related_tickets %}
@ -110,8 +110,7 @@ class Review(BaseModel):
security_concerns: str = Field(description="Does this PR code introduce possible vulnerabilities such as exposure of sensitive information (e.g., API keys, secrets, passwords), or security concerns like SQL injection, XSS, CSRF, and others ? Answer 'No' (without explaining why) if there are no possible issues. If there are security concerns or issues, start your answer with a short header, such as: 'Sensitive information exposure: ...', 'SQL injection: ...', etc. Explain your answer. Be specific and give examples if possible")
{%- endif %}
{%- if require_todo_scan %}
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"
todo_summary: str = Field(description="Up to 6 words summarizing the functional areas of the TODO comments found in the code.")
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")
{%- endif %}
{%- if require_can_be_split_review %}
can_be_split: List[SubPR] = Field(min_items=0, max_items=3, description="Can this PR, which contains {{ num_pr_files }} changed files in total, be divided into smaller sub-PRs with distinct tasks that can be reviewed and merged independently, regardless of the order ? Make sure that the sub-PRs are indeed independent, with no code dependencies between them, and that each sub-PR represent a meaningful independent task. Output an empty list if the PR code does not need to be split.")
@ -162,8 +161,6 @@ review:
{%- if require_todo_scan %}
todo_sections: |
No
todo_summary: |
...
{%- endif %}
{%- if require_can_be_split_review %}
can_be_split:
@ -286,8 +283,6 @@ review:
{%- if require_todo_scan %}
todo_sections: |
No
todo_summary: |
...
{%- endif %}
{%- if require_can_be_split_review %}
can_be_split:

2
pr_agent/tools/pr_reviewer.py
View File

@ -87,7 +87,7 @@ class PRReviewer:
"require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review,
'require_can_be_split_review': get_settings().pr_reviewer.require_can_be_split_review,
'require_security_review': get_settings().pr_reviewer.require_security_review,
'require_todo_scan': get_settings().pr_reviewer.require_todo_scan,
'require_todo_scan': get_settings().pr_reviewer.get("require_todo_scan", False),
'question_str': question_str,
'answer_str': answer_str,
"extra_instructions": get_settings().pr_reviewer.extra_instructions,