mirror of
https://github.com/qodo-ai/pr-agent.git
synced 2025-12-12 02:45:18 +00:00
PR reviewer tool: add an opt-in work time estimation (#2006)
* feat: add `ContributionTimeCostEstimate` * docs: mentiond `require_estimate_contribution_time_cost` for `reviewer` * feat: implement time cost estimate for `reviewer` * test: non-GFM output To ensure parity and prevent regressions in plain Markdown rendering.
This commit is contained in:
parent
d2c304eede
commit
5fc466bfbc
6 changed files with 79 additions and 1 deletions
|
|
@ -91,6 +91,10 @@ extra_instructions = "..."
|
||||||
<td><b>require_estimate_effort_to_review</b></td>
|
<td><b>require_estimate_effort_to_review</b></td>
|
||||||
<td>If set to true, the tool will add a section that estimates the effort needed to review the PR. Default is true.</td>
|
<td>If set to true, the tool will add a section that estimates the effort needed to review the PR. Default is true.</td>
|
||||||
</tr>
|
</tr>
|
||||||
|
<tr>
|
||||||
|
<td><b>require_estimate_contribution_time_cost</b></td>
|
||||||
|
<td>If set to true, the tool will add a section that estimates the time required for a senior developer to create and submit such changes. Default is false.</td>
|
||||||
|
</tr>
|
||||||
<tr>
|
<tr>
|
||||||
<td><b>require_can_be_split_review</b></td>
|
<td><b>require_can_be_split_review</b></td>
|
||||||
<td>If set to true, the tool will add a section that checks if the PR contains several themes, and can be split into smaller PRs. Default is false.</td>
|
<td>If set to true, the tool will add a section that checks if the PR contains several themes, and can be split into smaller PRs. Default is false.</td>
|
||||||
|
|
|
||||||
|
|
@ -148,6 +148,7 @@ def convert_to_markdown_v2(output_data: dict,
|
||||||
"Insights from user's answers": "📝",
|
"Insights from user's answers": "📝",
|
||||||
"Code feedback": "🤖",
|
"Code feedback": "🤖",
|
||||||
"Estimated effort to review [1-5]": "⏱️",
|
"Estimated effort to review [1-5]": "⏱️",
|
||||||
|
"Contribution time cost estimate": "⏳",
|
||||||
"Ticket compliance check": "🎫",
|
"Ticket compliance check": "🎫",
|
||||||
}
|
}
|
||||||
markdown_text = ""
|
markdown_text = ""
|
||||||
|
|
@ -207,6 +208,14 @@ def convert_to_markdown_v2(output_data: dict,
|
||||||
markdown_text += f"### {emoji} PR contains tests\n\n"
|
markdown_text += f"### {emoji} PR contains tests\n\n"
|
||||||
elif 'ticket compliance check' in key_nice.lower():
|
elif 'ticket compliance check' in key_nice.lower():
|
||||||
markdown_text = ticket_markdown_logic(emoji, markdown_text, value, gfm_supported)
|
markdown_text = ticket_markdown_logic(emoji, markdown_text, value, gfm_supported)
|
||||||
|
elif 'contribution time cost estimate' in key_nice.lower():
|
||||||
|
if gfm_supported:
|
||||||
|
markdown_text += f"<tr><td>{emoji} <strong>Contribution time estimate</strong> (best, average, worst case): "
|
||||||
|
markdown_text += f"{value['best_case'].replace('m', ' minutes')} | {value['average_case'].replace('m', ' minutes')} | {value['worst_case'].replace('m', ' minutes')}"
|
||||||
|
markdown_text += f"</td></tr>\n"
|
||||||
|
else:
|
||||||
|
markdown_text += f"### {emoji} Contribution time estimate (best, average, worst case): "
|
||||||
|
markdown_text += f"{value['best_case'].replace('m', ' minutes')} | {value['average_case'].replace('m', ' minutes')} | {value['worst_case'].replace('m', ' minutes')}\n\n"
|
||||||
elif 'security concerns' in key_nice.lower():
|
elif 'security concerns' in key_nice.lower():
|
||||||
if gfm_supported:
|
if gfm_supported:
|
||||||
markdown_text += f"<tr><td>"
|
markdown_text += f"<tr><td>"
|
||||||
|
|
@ -1465,4 +1474,4 @@ def format_todo_items(value: list[TodoItem] | TodoItem, git_provider, gfm_suppor
|
||||||
markdown_text += f"- {format_todo_item(todo_item, git_provider, gfm_supported)}\n"
|
markdown_text += f"- {format_todo_item(todo_item, git_provider, gfm_supported)}\n"
|
||||||
else:
|
else:
|
||||||
markdown_text += f"- {format_todo_item(value, git_provider, gfm_supported)}\n"
|
markdown_text += f"- {format_todo_item(value, git_provider, gfm_supported)}\n"
|
||||||
return markdown_text
|
return markdown_text
|
||||||
|
|
|
||||||
|
|
@ -79,6 +79,7 @@ require_tests_review=true
|
||||||
require_estimate_effort_to_review=true
|
require_estimate_effort_to_review=true
|
||||||
require_can_be_split_review=false
|
require_can_be_split_review=false
|
||||||
require_security_review=true
|
require_security_review=true
|
||||||
|
require_estimate_contribution_time_cost=false
|
||||||
require_todo_scan=false
|
require_todo_scan=false
|
||||||
require_ticket_analysis_review=true
|
require_ticket_analysis_review=true
|
||||||
# general options
|
# general options
|
||||||
|
|
|
||||||
|
|
@ -89,6 +89,14 @@ class TicketCompliance(BaseModel):
|
||||||
requires_further_human_verification: str = Field(description="Bullet-point list of items from the 'ticket_requirements' section above that cannot be assessed through code review alone, are unclear, or need further human review (e.g., browser testing, UI checks). Leave empty if all 'ticket_requirements' were marked as fully compliant or not compliant")
|
requires_further_human_verification: str = Field(description="Bullet-point list of items from the 'ticket_requirements' section above that cannot be assessed through code review alone, are unclear, or need further human review (e.g., browser testing, UI checks). Leave empty if all 'ticket_requirements' were marked as fully compliant or not compliant")
|
||||||
{%- endif %}
|
{%- endif %}
|
||||||
|
|
||||||
|
{%- if require_estimate_contribution_time_cost %}
|
||||||
|
|
||||||
|
class ContributionTimeCostEstimate(BaseModel):
|
||||||
|
best_case: str = Field(description="An expert in the relevant technology stack, with no unforeseen issues or bugs during the work.", examples=["45m", "5h", "30h"])
|
||||||
|
average_case: str = Field(description="A senior developer with only brief familiarity with this specific technology stack, and no major unforeseen issues.", examples=["45m", "5h", "30h"])
|
||||||
|
worst_case: str = Field(description="A senior developer with no prior experience in this specific technology stack, requiring significant time for research, debugging, or resolving unexpected errors.", examples=["45m", "5h", "30h"])
|
||||||
|
{%- endif %}
|
||||||
|
|
||||||
class Review(BaseModel):
|
class Review(BaseModel):
|
||||||
{%- if related_tickets %}
|
{%- if related_tickets %}
|
||||||
ticket_compliance_check: List[TicketCompliance] = Field(description="A list of compliance checks for the related tickets")
|
ticket_compliance_check: List[TicketCompliance] = Field(description="A list of compliance checks for the related tickets")
|
||||||
|
|
@ -96,6 +104,9 @@ class Review(BaseModel):
|
||||||
{%- if require_estimate_effort_to_review %}
|
{%- if require_estimate_effort_to_review %}
|
||||||
estimated_effort_to_review_[1-5]: int = Field(description="Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. Take into account the size, complexity, quality, and the needed changes of the PR code diff.")
|
estimated_effort_to_review_[1-5]: int = Field(description="Estimate, on a scale of 1-5 (inclusive), the time and effort required to review this PR by an experienced and knowledgeable developer. 1 means short and easy review , 5 means long and hard review. Take into account the size, complexity, quality, and the needed changes of the PR code diff.")
|
||||||
{%- endif %}
|
{%- endif %}
|
||||||
|
{%- if require_estimate_contribution_time_cost %}
|
||||||
|
contribution_time_cost_estimate: ContributionTimeCostEstimate = Field(description="An estimate of the time required to implement the changes, based on the quantity, quality, and complexity of the contribution, as well as the context from the PR description and commit messages.")
|
||||||
|
{%- endif %}
|
||||||
{%- if require_score %}
|
{%- if require_score %}
|
||||||
score: str = Field(description="Rate this PR on a scale of 0-100 (inclusive), where 0 means the worst possible PR code, and 100 means PR code of the highest quality, without any bugs or performance issues, that is ready to be merged immediately and run in production at scale.")
|
score: str = Field(description="Rate this PR on a scale of 0-100 (inclusive), where 0 means the worst possible PR code, and 100 means PR code of the highest quality, without any bugs or performance issues, that is ready to be merged immediately and run in production at scale.")
|
||||||
{%- endif %}
|
{%- endif %}
|
||||||
|
|
@ -170,6 +181,15 @@ review:
|
||||||
title: ...
|
title: ...
|
||||||
- ...
|
- ...
|
||||||
{%- endif %}
|
{%- endif %}
|
||||||
|
{%- if require_estimate_contribution_time_cost %}
|
||||||
|
contribution_time_cost_estimate:
|
||||||
|
best_case: |
|
||||||
|
...
|
||||||
|
average_case: |
|
||||||
|
...
|
||||||
|
worst_case: |
|
||||||
|
...
|
||||||
|
{%- endif %}
|
||||||
```
|
```
|
||||||
|
|
||||||
Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|')
|
Answer should be a valid YAML, and nothing else. Each YAML output MUST be after a newline, with proper indent, and block scalar indicator ('|')
|
||||||
|
|
@ -299,6 +319,15 @@ review:
|
||||||
title: ...
|
title: ...
|
||||||
- ...
|
- ...
|
||||||
{%- endif %}
|
{%- endif %}
|
||||||
|
{%- if require_estimate_contribution_time_cost %}
|
||||||
|
contribution_time_cost_estimate:
|
||||||
|
best_case: |
|
||||||
|
...
|
||||||
|
average_case: |
|
||||||
|
...
|
||||||
|
worst_case: |
|
||||||
|
...
|
||||||
|
{%- endif %}
|
||||||
```
|
```
|
||||||
(replace '...' with the actual values)
|
(replace '...' with the actual values)
|
||||||
{%- endif %}
|
{%- endif %}
|
||||||
|
|
|
||||||
|
|
@ -85,6 +85,7 @@ class PRReviewer:
|
||||||
"require_score": get_settings().pr_reviewer.require_score_review,
|
"require_score": get_settings().pr_reviewer.require_score_review,
|
||||||
"require_tests": get_settings().pr_reviewer.require_tests_review,
|
"require_tests": get_settings().pr_reviewer.require_tests_review,
|
||||||
"require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review,
|
"require_estimate_effort_to_review": get_settings().pr_reviewer.require_estimate_effort_to_review,
|
||||||
|
"require_estimate_contribution_time_cost": get_settings().pr_reviewer.require_estimate_contribution_time_cost,
|
||||||
'require_can_be_split_review': get_settings().pr_reviewer.require_can_be_split_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_security_review': get_settings().pr_reviewer.require_security_review,
|
||||||
'require_todo_scan': get_settings().pr_reviewer.get("require_todo_scan", False),
|
'require_todo_scan': get_settings().pr_reviewer.get("require_todo_scan", False),
|
||||||
|
|
|
||||||
|
|
@ -222,6 +222,40 @@ class TestConvertToMarkdown:
|
||||||
|
|
||||||
assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()
|
assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()
|
||||||
|
|
||||||
|
def test_contribution_time_cost_estimate(self):
|
||||||
|
input_data = {
|
||||||
|
'review': {
|
||||||
|
'contribution_time_cost_estimate': {
|
||||||
|
'best_case': '1h',
|
||||||
|
'average_case': '2h',
|
||||||
|
'worst_case': '30m',
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
expected_output = textwrap.dedent(f"""
|
||||||
|
{PRReviewHeader.REGULAR.value} 🔍
|
||||||
|
|
||||||
|
Here are some key observations to aid the review process:
|
||||||
|
|
||||||
|
<table>
|
||||||
|
<tr><td>⏳ <strong>Contribution time estimate</strong> (best, average, worst case): 1h | 2h | 30 minutes</td></tr>
|
||||||
|
</table>
|
||||||
|
""")
|
||||||
|
assert convert_to_markdown_v2(input_data).strip() == expected_output.strip()
|
||||||
|
|
||||||
|
# Non-GFM branch
|
||||||
|
expected_output_no_gfm = textwrap.dedent(f"""
|
||||||
|
{PRReviewHeader.REGULAR.value} 🔍
|
||||||
|
|
||||||
|
Here are some key observations to aid the review process:
|
||||||
|
|
||||||
|
### ⏳ Contribution time estimate (best, average, worst case): 1h | 2h | 30 minutes
|
||||||
|
|
||||||
|
""")
|
||||||
|
assert convert_to_markdown_v2(input_data, gfm_supported=False).strip() == expected_output_no_gfm.strip()
|
||||||
|
|
||||||
|
|
||||||
# Tests that the function works correctly with an empty dictionary input
|
# Tests that the function works correctly with an empty dictionary input
|
||||||
def test_empty_dictionary_input(self):
|
def test_empty_dictionary_input(self):
|
||||||
input_data = {}
|
input_data = {}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue