Skip to content

Commit f99b82e

Browse files
gvagoclaude
andauthored
fix(description): use regex for minute suffix replacement instead of blanket .replace (#2318)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 073d998 commit f99b82e

2 files changed

Lines changed: 39 additions & 3 deletions

File tree

‎pr_agent/algo/utils.py‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ def unique_strings(input_list: List[str]) -> List[str]:
126126
return unique_list
127127

128128

129+
def _expand_minute_suffix(text: str) -> str:
130+
"""Replace minute abbreviations like '30m' with '30 minutes'.
131+
132+
Only replaces when 'm' appears at a word boundary after digits
133+
(e.g. "30m" -> "30 minutes"), leaving partial-unit strings like
134+
"30ms" or "30min" unchanged.
135+
"""
136+
return re.sub(r'(\d+)m\b', r'\1 minutes', text)
137+
138+
129139
def convert_to_markdown_v2(output_data: dict,
130140
gfm_supported: bool = True,
131141
incremental_review=None,
@@ -215,11 +225,17 @@ def convert_to_markdown_v2(output_data: dict,
215225
elif 'contribution time cost estimate' in key_nice.lower():
216226
if gfm_supported:
217227
markdown_text += f"<tr><td>{emoji}&nbsp;<strong>Contribution time estimate</strong> (best, average, worst case): "
218-
markdown_text += f"{value['best_case'].replace('m', ' minutes')} | {value['average_case'].replace('m', ' minutes')} | {value['worst_case'].replace('m', ' minutes')}"
228+
best = _expand_minute_suffix(value['best_case'])
229+
avg = _expand_minute_suffix(value['average_case'])
230+
worst = _expand_minute_suffix(value['worst_case'])
231+
markdown_text += f"{best} | {avg} | {worst}"
219232
markdown_text += f"</td></tr>\n"
220233
else:
221234
markdown_text += f"### {emoji} Contribution time estimate (best, average, worst case): "
222-
markdown_text += f"{value['best_case'].replace('m', ' minutes')} | {value['average_case'].replace('m', ' minutes')} | {value['worst_case'].replace('m', ' minutes')}\n\n"
235+
best = _expand_minute_suffix(value['best_case'])
236+
avg = _expand_minute_suffix(value['average_case'])
237+
worst = _expand_minute_suffix(value['worst_case'])
238+
markdown_text += f"{best} | {avg} | {worst}\n\n"
223239
elif 'security concerns' in key_nice.lower():
224240
if gfm_supported:
225241
markdown_text += f"<tr><td>"

‎tests/unittest/test_convert_to_markdown.py‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import textwrap
33
from unittest.mock import Mock
44

5-
from pr_agent.algo.utils import PRReviewHeader, convert_to_markdown_v2
5+
from pr_agent.algo.utils import PRReviewHeader, _expand_minute_suffix, convert_to_markdown_v2
66
from pr_agent.tools.pr_description import insert_br_after_x_chars
77

88
"""
@@ -303,3 +303,23 @@ def test_br3(self):
303303
'</code> and implements <br>aaa')
304304
# print("-----")
305305
# print(file_change_description_br)
306+
307+
308+
class TestExpandMinuteSuffix:
309+
"""Tests for _expand_minute_suffix regex replacement."""
310+
311+
def test_standalone_minute_suffix(self):
312+
"""'30m' at end of string becomes '30 minutes'."""
313+
assert _expand_minute_suffix("30m") == "30 minutes"
314+
315+
def test_minute_suffix_not_replaced_when_part_of_longer_unit(self):
316+
"""'30ms' stays unchanged because 'm' is not at a word boundary."""
317+
assert _expand_minute_suffix("30ms") == "30ms"
318+
319+
def test_minute_suffix_replaced_before_space(self):
320+
"""'30m implementation' replaces '30m' because 'm' is at a word boundary."""
321+
assert _expand_minute_suffix("30m implementation") == "30 minutes implementation"
322+
323+
def test_minute_suffix_in_compound_estimate(self):
324+
"""'2h 30m' becomes '2h 30 minutes' (only the minute part is replaced)."""
325+
assert _expand_minute_suffix("2h 30m") == "2h 30 minutes"

0 commit comments

Comments
 (0)