diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index 0fdc8019fe..802ffe68cd 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -609,10 +609,44 @@ def dedent_code(self, relevant_file, relevant_lines_start, new_code_snippet): original_initial_spaces = len(original_initial_line) - len(original_initial_line.lstrip()) # lstrip works both for spaces and tabs suggested_initial_spaces = len(suggested_initial_line) - len(suggested_initial_line.lstrip()) delta_spaces = original_initial_spaces - suggested_initial_spaces + # Detect indentation character from original line + indent_char = '\t' if original_initial_line.startswith('\t') else ' ' if delta_spaces > 0: - # Detect indentation character from original line - indent_char = '\t' if original_initial_line.startswith('\t') else ' ' new_code_snippet = textwrap.indent(new_code_snippet, delta_spaces * indent_char).rstrip('\n') + elif delta_spaces < 0: + # Remove exactly -delta_spaces leading spaces from each + # non-empty line. textwrap.dedent() strips the *common* + # indent which may remove too much when lines have + # varying indentation levels (Qodo bug #3). + remove_count = -delta_spaces + dedented_lines = [] + for dline in new_code_snippet.split('\n'): + if dline.strip(): + leading = len(dline) - len(dline.lstrip()) + dedented_lines.append(dline[min(remove_count, leading):]) + else: + dedented_lines.append(dline) + new_code_snippet = '\n'.join(dedented_lines).rstrip('\n') + # Normalize all lines in the suggestion to use the original's + # indentation character. This fixes the bug where /improve + # replaces tabs with spaces in Go, Makefile, and other + # tab-indented codebases (#1858). + if indent_char == '\t': + new_lines = [] + for line in new_code_snippet.split('\n'): + stripped = line.lstrip(' ') + leading_spaces = len(line) - len(stripped) + if leading_spaces > 0: + # Preserve remainder spaces when converting + # to tabs. e.g. 6 spaces → 1 tab + 2 spaces + # instead of silently dropping 2 spaces + # (Qodo bug #3). + tabs = leading_spaces // 4 + remainder = leading_spaces % 4 + new_lines.append('\t' * tabs + ' ' * remainder + stripped) + else: + new_lines.append(line) + new_code_snippet = '\n'.join(new_lines) except Exception as e: get_logger().error(f"Error when dedenting code snippet for file {relevant_file}, error: {e}") diff --git a/pr_agent/tools/ticket_pr_compliance_check.py b/pr_agent/tools/ticket_pr_compliance_check.py index 6d25d76b19..1df60e70e5 100644 --- a/pr_agent/tools/ticket_pr_compliance_check.py +++ b/pr_agent/tools/ticket_pr_compliance_check.py @@ -35,6 +35,38 @@ def find_jira_tickets(text): return list(tickets) +# Compiled Asana task patterns +_ASANA_TASK_URL_PATTERN = re.compile( + r'https://app\.asana\.com/0/(\d+)/(\d+)' +) +_ASANA_TASK_SHORT_PATTERN = re.compile( + r'(?:^|[^A-Za-z0-9])(?:ASANA|asana)[- ]?(\d{12,20})', + re.IGNORECASE, +) + + +def find_asana_tickets(text: str) -> list: + """Extract Asana task references from text. + + Supports both full Asana URLs and shorthand ``ASANA-123456789012`` + format. Returns a list of unique task URLs. + + Args: + text: The text to scan for Asana task references. + + Returns: + A list of Asana task URLs. + """ + tickets = set() + for match in _ASANA_TASK_URL_PATTERN.finditer(text): + tickets.add(match.group(0)) + for match in _ASANA_TASK_SHORT_PATTERN.finditer(text): + task_id = match.group(1) + if task_id: + tickets.add(f"https://app.asana.com/0/0/{task_id}") + return sorted(tickets) + + def extract_ticket_links_from_pr_description(pr_description, repo_path, base_url_html='https://github.com'): """ Extract all ticket links from PR description @@ -123,9 +155,22 @@ async def extract_tickets(git_provider): if link not in seen: seen.add(link) merged.append(link) + + # Also detect Asana ticket references in the PR description + asana_tickets = find_asana_tickets(user_description) + for link in asana_tickets: + if link not in seen: + seen.add(link) + merged.append(link) + asana_links = [t for t in merged if t.startswith("https://app.asana.com/")] + github_like = [t for t in merged if not t.startswith("https://app.asana.com/")] if len(merged) > 3: get_logger().info(f"Too many tickets (description + branch): {len(merged)}") - tickets = merged[:3] + # Reserve at least one slot for an Asana reference when + # present so it is not systematically dropped. + asana_slot = asana_links[:1] + gh_slots = 3 - len(asana_slot) + tickets = github_like[:gh_slots] + asana_slot else: tickets = merged tickets_content = [] @@ -133,6 +178,19 @@ async def extract_tickets(git_provider): if tickets: for ticket in tickets: + # Skip Asana URLs — these are external references, + # included for visibility but cannot be fetched via GitHub API. + if ticket.startswith("https://app.asana.com/"): + tickets_content.append({ + "ticket_id": ticket, + "ticket_url": ticket, + "title": f"Asana Task: {ticket}", + "body": ("Asana task referenced in PR description. " + "Fetch task details from Asana for full context."), + "labels": "", + }) + continue + repo_name, original_issue_number = git_provider._parse_issue_url(ticket) try: diff --git a/tests/unittest/test_ticket_compliance.py b/tests/unittest/test_ticket_compliance.py new file mode 100644 index 0000000000..8a85e2c4ab --- /dev/null +++ b/tests/unittest/test_ticket_compliance.py @@ -0,0 +1,96 @@ +""" +Unit tests for Asana ticket detection in ticket_pr_compliance_check.py. + +Tests cover: +- Full Asana URL detection +- Shorthand ASANA- prefix detection +- Edge cases (mixed content, no tickets, duplicates) +""" +from pr_agent.tools.ticket_pr_compliance_check import find_asana_tickets + + +class TestFindAsanaTickets: + """Tests for find_asana_tickets().""" + + def test_detects_full_asana_url(self): + """Full Asana task URLs should be detected.""" + text = "See https://app.asana.com/0/123456/789012 for details" + tickets = find_asana_tickets(text) + assert "https://app.asana.com/0/123456/789012" in tickets + + def test_detects_asana_shorthand(self): + """ASANA-123456789012 shorthand format should be detected.""" + text = "Task ASANA-123456789012 is complete" + tickets = find_asana_tickets(text) + assert any("123456789012" in t for t in tickets) + + def test_detects_multiple_tickets(self): + """Multiple Asana references should all be found.""" + text = ( + "See ASANA-111111111111 and https://app.asana.com/0/22/333333333333" + ) + tickets = find_asana_tickets(text) + assert len(tickets) == 2 + + def test_deduplicates_identical_tickets(self): + """Duplicate references to the same task should be deduplicated.""" + text = ( + "ASANA-123456789012 mentioned twice: ASANA-123456789012 again" + ) + tickets = find_asana_tickets(text) + assert len(tickets) == 1 + + def test_returns_empty_for_no_tickets(self): + """Text without Asana references returns an empty list.""" + text = "No tickets here, just regular text" + tickets = find_asana_tickets(text) + assert tickets == [] + + def test_returns_empty_for_empty_string(self): + """Empty string returns an empty list.""" + tickets = find_asana_tickets("") + assert tickets == [] + + def test_ignores_github_urls(self): + """GitHub issue URLs should not be mistaken for Asana tickets.""" + text = "Fix https://github.com/owner/repo/issues/42" + tickets = find_asana_tickets(text) + assert tickets == [] + + def test_shorthand_is_case_insensitive(self): + """Both ASANA- and asana- should be detected.""" + text = "asana-999999999999 lowercase works too" + tickets = find_asana_tickets(text) + assert len(tickets) == 1 + assert "999999999999" in tickets[0] + + def test_shorthand_mixed_case_asana(self): + """Mixed-case shorthand like Asana-123... should also be detected.""" + text = "Asana-888888888888 mixed case" + tickets = find_asana_tickets(text) + assert len(tickets) == 1 + assert "888888888888" in tickets[0] + + def test_shorthand_respects_word_boundary(self): + """Text preceding the shorthand must not be alphanumeric. + NOTASANA-123 (no delimiter before ASANA) should not match.""" + text = "Check NOTASANA-123456789012 in logs" + tickets = find_asana_tickets(text) + assert tickets == [] + + def test_tickets_are_sorted(self): + """Returned list should be sorted alphabetically.""" + text = "ASANA-222222222222 ASANA-111111111111" + tickets = find_asana_tickets(text) + assert tickets == sorted(tickets) + + def test_tickets_in_pr_description_mixed_content(self): + """Asana tickets mixed with other content in a PR description.""" + text = """## Summary + Fixes ASANA-123456789012 + Related to https://app.asana.com/0/99/888888888888 + + Also see GitHub issue #42 + """ + tickets = find_asana_tickets(text) + assert len(tickets) == 2