fix(description): use regex for minute suffix replacement instead of blanket .replace

[gvago]

Summary

• .replace('m', ' minutes') in the contribution time estimate rendering replaced every occurrence of the letter m in the string, not just the time suffix. For example, "30m implementation" became "30 minutes i minutes ple minutes entation".
• Replaced with re.sub(r'(\d+)m\b', r'\1 minutes', ...) which only targets m immediately following one or more digits at a word boundary (e.g. 30m -> 30 minutes), leaving the rest of the string intact.
• The re module was already imported in this file, so no new imports are needed.

Bug reproduction

# Before (broken)
>>> "30m implementation".replace('m', ' minutes')
'30 minutes i minutes ple minutes entation'

# After (fixed)
>>> import re
>>> re.sub(r'(\d+)m\b', r'\1 minutes', "30m implementation")
'30 minutes implementation'

Test plan

• Verified regex handles: "30m", "30m implementation", "1h 30m", "medium complexity" (unchanged), "45m", "2h" (unchanged)
• Verify contribution time estimate renders correctly in PR description output (GFM and non-GFM paths)

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix minute suffix replacement using regex pattern matching

🐞 Bug fix

Walkthroughs

Description

• Replace blanket .replace('m', ' minutes') with regex pattern
• Fixes incorrect replacement of all 'm' letters in strings
• Uses re.sub(r'(\d+)m\b', r'\1 minutes', ...) for precise matching
• Applied to both GFM and non-GFM contribution time estimate paths

Diagram

flowchart LR
  A["Contribution time estimate<br/>with 'm' suffix"] -->|Old: .replace| B["All 'm' letters<br/>replaced incorrectly"]
  A -->|New: re.sub| C["Only time suffix<br/>'m' replaced correctly"]

Loading

File Changes

1. pr_agent/algo/utils.py 🐞 Bug fix +2/-2

Use regex for precise minute suffix replacement

• Replaced .replace('m', ' minutes') with re.sub(r'(\d+)m\b', r'\1 minutes', ...) in GFM path
• Replaced .replace('m', ' minutes') with re.sub(r'(\d+)m\b', r'\1 minutes', ...) in non-GFM
 path
• Applied fix to all three time estimate cases: best_case, average_case, worst_case
• Prevents unintended replacement of 'm' letters in descriptive text

pr_agent/algo/utils.py

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0)   📘 Rule violations (1)   📎 Requirement gaps (0)

📘\ ☼ Reliability (1) ⭐ New (1)

1. Minute-regex change lacks tests ☑ 📘 ☼

Description

The PR changes minute-suffix formatting behavior (from blanket replacement to a targeted regex) but
does not add/extend unit tests to cover the previously broken scenario like 30m implementation or
strings containing other m characters. This risks regression because the existing test only
asserts the simple 30m case.

Code

pr_agent/algo/utils.py[R217-221]

+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}"
              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"
+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}\n\n"

Evidence

PR Compliance ID 13 requires behavior changes to be covered by pytest tests. The modified code now
uses re.sub(r'(\d+)m\b', ...) for each value, but the existing unit test for contribution time
estimate only covers worst_case='30m' and does not cover the bug scenario where other m
characters appear in the string (the case this PR fixes).

AGENTS.md
pr_agent/algo/utils.py[214-221]
tests/unittest/test_convert_to_markdown.py[225-257]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`convert_to_markdown_v2()` changed minute-suffix formatting logic to use a regex, but unit tests were not expanded to cover the bug scenario that motivated the change (e.g., strings like `30m implementation` should become `30 minutes implementation` without altering other `m` characters).
## Issue Context
There is an existing unit test for `contribution_time_cost_estimate`, but it only validates the simple `30m` input and would not fail under the previous buggy `.replace('m', ' minutes')` behavior for mixed-content strings.
## Fix Focus Areas
- tests/unittest/test_convert_to_markdown.py[225-257]
- pr_agent/algo/utils.py[214-221]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Overlong f-string at 217 ☑ 📘 ⚙

Description

The new markdown_text += f"..." lines with three inline re.sub(...) calls are very likely to
exceed the repository’s configured 120-character limit, which can fail Ruff/pycodestyle checks. This
introduces a lint/format compliance issue in a touched line.

Code

pr_agent/algo/utils.py[R217-221]

+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}"
              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"
+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}\n\n"

Evidence

PR Compliance IDs 10 and 21 require adherence to Ruff configuration (line-length 120) and lint
tooling compliance. The changed lines inline multiple re.sub(...) calls inside a single f-string,
creating an extremely long line, while pyproject.toml sets Ruff line-length = 120.

AGENTS.md
pyproject.toml[47-49]
pr_agent/algo/utils.py[216-221]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modified `markdown_text += f"..."` lines are too long and likely violate the repo’s Ruff 120-character limit.
## Issue Context
Ruff is configured with `line-length = 120` in `pyproject.toml`. The current change inlines three `re.sub(...)` calls in a single f-string, which should be refactored to keep lines within the limit.
## Fix Focus Areas
- pr_agent/algo/utils.py[216-221]
- pyproject.toml[47-49]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. No type guard in _expand_minute_suffix() 📘 ☼ ⭐

Description

_expand_minute_suffix() and its new call sites assume best_case/average_case/worst_case are
always strings; if any are None or non-string, re.sub(...) can raise and break markdown
rendering. This violates the requirement to defensively handle nulls/types for external/untrusted
inputs before calling methods.

Code

pr_agent/algo/utils.py[R227-229]

+                best = _expand_minute_suffix(value['best_case'])
+                avg = _expand_minute_suffix(value['average_case'])
+                worst = _expand_minute_suffix(value['worst_case'])

Evidence

PR Compliance ID 15 requires null/type checks on external inputs before calling methods. The PR adds
calls that pass value['best_case']/value['average_case']/value['worst_case'] directly into
_expand_minute_suffix(), which then calls re.sub(..., text) without validating text is a
string.

pr_agent/algo/utils.py[224-237]
pr_agent/algo/utils.py[128-135]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `_expand_minute_suffix()` path assumes contribution-time values are always strings and may raise if they are `None`/non-string.

## Issue Context
`convert_to_markdown_v2()` processes data from `output_data` (external/untrusted shape). The PR introduced new calls into `_expand_minute_suffix()` without validating input type/shape, and `_expand_minute_suffix()` calls `re.sub()` directly.

## Fix Focus Areas
- pr_agent/algo/utils.py[128-135]
- pr_agent/algo/utils.py[224-237]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

Previous review results

Review updated until commit 69d8ff6

Results up to commit N/A

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Minute-regex change lacks tests ☑ 📘 ☼

Description

The PR changes minute-suffix formatting behavior (from blanket replacement to a targeted regex) but
does not add/extend unit tests to cover the previously broken scenario like 30m implementation or
strings containing other m characters. This risks regression because the existing test only
asserts the simple 30m case.

Code

pr_agent/algo/utils.py[R217-221]

+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}"
               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"
+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}\n\n"

Evidence

PR Compliance ID 13 requires behavior changes to be covered by pytest tests. The modified code now
uses re.sub(r'(\d+)m\b', ...) for each value, but the existing unit test for contribution time
estimate only covers worst_case='30m' and does not cover the bug scenario where other m
characters appear in the string (the case this PR fixes).

AGENTS.md
pr_agent/algo/utils.py[214-221]
tests/unittest/test_convert_to_markdown.py[225-257]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`convert_to_markdown_v2()` changed minute-suffix formatting logic to use a regex, but unit tests were not expanded to cover the bug scenario that motivated the change (e.g., strings like `30m implementation` should become `30 minutes implementation` without altering other `m` characters).
## Issue Context
There is an existing unit test for `contribution_time_cost_estimate`, but it only validates the simple `30m` input and would not fail under the previous buggy `.replace('m', ' minutes')` behavior for mixed-content strings.
## Fix Focus Areas
- tests/unittest/test_convert_to_markdown.py[225-257]
- pr_agent/algo/utils.py[214-221]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Overlong f-string at 217 ☑ 📘 ⚙

Description

The new markdown_text += f"..." lines with three inline re.sub(...) calls are very likely to
exceed the repository’s configured 120-character limit, which can fail Ruff/pycodestyle checks. This
introduces a lint/format compliance issue in a touched line.

Code

pr_agent/algo/utils.py[R217-221]

+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}"
               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"
+                markdown_text += f"{re.sub(r'(\d+)m\b', r'\1 minutes', value['best_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['average_case'])} | {re.sub(r'(\d+)m\b', r'\1 minutes', value['worst_case'])}\n\n"

Evidence

PR Compliance IDs 10 and 21 require adherence to Ruff configuration (line-length 120) and lint
tooling compliance. The changed lines inline multiple re.sub(...) calls inside a single f-string,
creating an extremely long line, while pyproject.toml sets Ruff line-length = 120.

AGENTS.md
pyproject.toml[47-49]
pr_agent/algo/utils.py[216-221]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modified `markdown_text += f"..."` lines are too long and likely violate the repo’s Ruff 120-character limit.
## Issue Context
Ruff is configured with `line-length = 120` in `pyproject.toml`. The current change inlines three `re.sub(...)` calls in a single f-string, which should be refactored to keep lines within the limit.
## Fix Focus Areas
- pr_agent/algo/utils.py[216-221]
- pyproject.toml[47-49]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

Persistent review updated to latest commit 69d8ff6