Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AUTO1_PATCHES.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Keep this list minimal to ease upstream rebases.
| review-severity-emoji | OPS-24090 | Enhancement | pr_agent/algo/utils.py | Add a header emoji for findings severity summary in PR review output. | Not upstreamed yet. | Remove once upstream adds a default emoji for findings severity summary. |
| improve-reflect-fallback-score | OPS-24090 | Fix | pr_agent/tools/pr_code_suggestions.py | Prevent inline suggestion spam when self-reflection fails by defaulting all suggestion scores to 0. | Not upstreamed yet. | Remove once upstream handles reflect failure without inflating scores. |
| describe-mermaid-sanitize | OPS-24090 | Fix | pr_agent/tools/pr_description.py | Strip backticks from Mermaid diagrams to avoid GitHub render failures. | Not upstreamed yet. | Remove once upstream sanitizes Mermaid diagrams or fixes prompt output. |
| jira-cloud-ticket-context | OPS-24091 | Enhancement | pr_agent/algo/utils.py, pr_agent/tickets/jira_cloud.py, pr_agent/tools/ticket_pr_compliance_check.py, pr_agent/tools/pr_reviewer.py, pr_agent/tools/pr_description.py, pr_agent/tools/pr_config.py, tests/unittest/test_convert_to_markdown.py, tests/unittest/test_jira_cloud_ticket_context.py, tests/unittest/test_pr_reviewer_ticket_note.py, tests/unittest/test_ticket_pr_compliance_check.py | Fetch Jira Cloud ticket context using PR title first and branch name only as fallback, support both basic auth and scoped-token Jira Cloud API access, ignore placeholder compliance bullets, and show review notes when the Jira ticket cannot be fetched. | Not upstreamed yet. | Remove once upstream supports Jira Cloud ticket context from PR metadata with title-first precedence, scoped-token auth, placeholder-safe compliance rendering, and comment-level fetch-failure notes. |
| jira-cloud-ticket-context | OPS-24091 | Enhancement | pr_agent/algo/utils.py, pr_agent/tickets/jira_cloud.py, pr_agent/tools/ticket_pr_compliance_check.py, pr_agent/tools/pr_reviewer.py, pr_agent/tools/pr_description.py, pr_agent/tools/pr_config.py, tests/unittest/test_convert_to_markdown.py, tests/unittest/test_jira_cloud_ticket_context.py, tests/unittest/test_pr_reviewer_ticket_note.py, tests/unittest/test_ticket_pr_compliance_check.py | Fetch Jira Cloud ticket context using PR title first and branch name only as fallback, support both basic auth and scoped-token Jira Cloud API access, ignore placeholder compliance bullets and placeholder text like `None.`, show review notes when the Jira ticket cannot be fetched, and normalize empty discovered tickets before prompt rendering. | Not upstreamed yet. | Remove once upstream supports Jira Cloud ticket context from PR metadata with title-first precedence, scoped-token auth, placeholder-safe compliance rendering, comment-level fetch-failure notes, and prompt-safe normalization for empty ticket payloads. |

## Rebase checklist

Expand Down
69 changes: 46 additions & 23 deletions pr_agent/algo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,18 +379,38 @@ def extract_relevant_lines_str(end_line, files, relevant_file, start_line, deden
return ""


def normalize_ticket_requirement_text(text: str) -> str:
text = (text or '').strip()
_PLACEHOLDER_REQUIREMENT_ITEMS = frozenset({
"", "-", "*", "none", "(none)", "n/a", "(n/a)", "na", "(na)",
"no information", "no information provided", "none provided", "not applicable",
})
_REQUIREMENT_ITEM_PREFIX_RE = re.compile(r"^(?:[-*•]+|\d+[.)]|\[[ xX]\])\s*")


def _canonicalize_requirement_item(line: str) -> str:
canonical_line = (line or "").strip()
while canonical_line:
updated_line = _REQUIREMENT_ITEM_PREFIX_RE.sub("", canonical_line, count=1).strip()
if updated_line == canonical_line:
break
canonical_line = updated_line
return canonical_line.lower().rstrip(".:;!,")


def is_placeholder_requirement_item(line: str) -> bool:
return _canonicalize_requirement_item(line) in _PLACEHOLDER_REQUIREMENT_ITEMS


def parse_requirement_items(text: str) -> list[str]:
text = (text or "").strip()
if not text:
return ''
return []

normalized_lines = [line.strip() for line in text.splitlines() if line.strip()]
placeholder_lines = {'-', '*', 'none', '(none)', 'n/a', '(n/a)'}
normalized_lines = [line for line in normalized_lines if line.lower() not in placeholder_lines]
if not normalized_lines:
return ''
requirement_items = [line.strip() for line in text.splitlines() if line.strip()]
return [item for item in requirement_items if not is_placeholder_requirement_item(item)]

return '\n'.join(normalized_lines)

def normalize_ticket_requirement_text(text: str) -> str:
return '\n'.join(parse_requirement_items(text))


def ticket_markdown_logic(emoji, markdown_text, value, gfm_supported, ticket_note: str = '') -> str:
Expand All @@ -408,48 +428,51 @@ def ticket_markdown_logic(emoji, markdown_text, value, gfm_supported, ticket_not
ticket_url = ticket_analysis.get('ticket_url', '').strip()
explanation = ''
ticket_compliance_level = '' # Individual ticket compliance
fully_compliant_str = normalize_ticket_requirement_text(ticket_analysis.get('fully_compliant_requirements'))
not_compliant_str = normalize_ticket_requirement_text(ticket_analysis.get('not_compliant_requirements'))
requires_further_human_verification = normalize_ticket_requirement_text(
fully_compliant_items = parse_requirement_items(ticket_analysis.get('fully_compliant_requirements'))
not_compliant_items = parse_requirement_items(ticket_analysis.get('not_compliant_requirements'))
requires_further_human_verification_items = parse_requirement_items(
ticket_analysis.get('requires_further_human_verification')
)
fully_compliant_str = '\n'.join(fully_compliant_items)
not_compliant_str = '\n'.join(not_compliant_items)
requires_further_human_verification = '\n'.join(requires_further_human_verification_items)

if not fully_compliant_str and not not_compliant_str:
if not fully_compliant_items and not not_compliant_items:
get_logger().debug(f"Ticket compliance has no requirements",
artifact={'ticket_url': ticket_url})
continue

# Calculate individual ticket compliance level
if fully_compliant_str:
if not_compliant_str:
if fully_compliant_items:
if not_compliant_items:
ticket_compliance_level = 'Partially compliant'
else:
if not requires_further_human_verification:
if not requires_further_human_verification_items:
ticket_compliance_level = 'Fully compliant'
else:
ticket_compliance_level = 'PR Code Verified'
elif not_compliant_str:
elif not_compliant_items:
ticket_compliance_level = 'Not compliant'

# Store the compliance level for aggregation
if ticket_compliance_level:
all_compliance_levels.append(ticket_compliance_level)

# build compliance string
if fully_compliant_str:
if fully_compliant_items:
explanation += f"Compliant requirements:\n\n{fully_compliant_str}\n\n"
if not_compliant_str:
if not_compliant_items:
explanation += f"Non-compliant requirements:\n\n{not_compliant_str}\n\n"
if requires_further_human_verification:
if requires_further_human_verification_items:
explanation += f"Requires further human verification:\n\n{requires_further_human_verification}\n\n"
ticket_compliance_str += f"\n\n**[{ticket_url.split('/')[-1]}]({ticket_url}) - {ticket_compliance_level}**\n\n{explanation}\n\n"

# for debugging
if requires_further_human_verification:
if requires_further_human_verification_items:
get_logger().debug(f"Ticket compliance requires further human verification",
artifact={'ticket_url': ticket_url,
'requires_further_human_verification': requires_further_human_verification,
'compliance_level': ticket_compliance_level})
'requires_further_human_verification': requires_further_human_verification,
'compliance_level': ticket_compliance_level})

except Exception as e:
get_logger().exception(f"Failed to process ticket compliance: {e}")
Expand Down
1 change: 1 addition & 0 deletions pr_agent/tickets/jira_cloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ def _extract_subtasks_as_sub_issues(issue: Dict[str, Any], base_url: str, limit:
"ticket_url": _jira_issue_url(base_url, key),
"title": summary or key,
"body": "",
"labels": "",
}
)

Expand Down
10 changes: 5 additions & 5 deletions pr_agent/tools/pr_reviewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from pr_agent.algo.token_handler import TokenHandler
from pr_agent.algo.utils import (ModelType, PRReviewHeader,
convert_to_markdown_v2, github_action_output,
load_yaml, normalize_ticket_requirement_text,
load_yaml, parse_requirement_items,
show_relevant_configurations)
from pr_agent.config_loader import get_settings
from pr_agent.git_providers import (get_git_provider,
Expand Down Expand Up @@ -46,11 +46,11 @@ def build_suspected_ticket_mismatch_note(ticket_compliance_check) -> str:
if not isinstance(ticket_analysis, dict):
continue

fully_compliant_str = normalize_ticket_requirement_text(ticket_analysis.get("fully_compliant_requirements"))
not_compliant_str = normalize_ticket_requirement_text(ticket_analysis.get("not_compliant_requirements"))
if fully_compliant_str:
fully_compliant_items = parse_requirement_items(ticket_analysis.get("fully_compliant_requirements"))
not_compliant_items = parse_requirement_items(ticket_analysis.get("not_compliant_requirements"))
if fully_compliant_items:
return ""
if not_compliant_str:
if not_compliant_items:
has_not_compliant_requirements = True

if not has_not_compliant_requirements:
Expand Down
66 changes: 58 additions & 8 deletions pr_agent/tools/ticket_pr_compliance_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,58 @@
r'(https://github[^/]+/[^/]+/[^/]+/issues/\d+)|(\b(\w+)/(\w+)#(\d+)\b)|(#\d+)'
)

TICKET_PROMPT_FIELDS = ("ticket_id", "ticket_url", "title", "body", "labels", "requirements")


def _normalize_ticket_value(value):
if value is None:
return ""
if isinstance(value, str):
return value
if isinstance(value, (list, tuple, set)):
return ", ".join(str(item) for item in value if item is not None)
return str(value)


def _normalize_ticket_for_prompt(ticket):
ticket = ticket if isinstance(ticket, dict) else {}
normalized_ticket = {
field: _normalize_ticket_value(ticket.get(field, "")) for field in TICKET_PROMPT_FIELDS
}

sub_issues = ticket.get("sub_issues")
if isinstance(sub_issues, list):
normalized_sub_issues = []
for sub_issue in sub_issues:
normalized_sub_issue = _normalize_ticket_for_prompt(sub_issue)
if _ticket_has_prompt_content(normalized_sub_issue):
normalized_sub_issues.append(normalized_sub_issue)
if normalized_sub_issues:
normalized_ticket["sub_issues"] = normalized_sub_issues

return normalized_ticket


def _ticket_has_prompt_content(ticket):
return any(ticket.get(field) for field in TICKET_PROMPT_FIELDS)


def _extract_related_tickets_for_prompt(tickets):
related_tickets = []
for ticket in tickets or []:
normalized_ticket = _normalize_ticket_for_prompt(ticket)
for sub_issue in normalized_ticket.get("sub_issues", []):
prompt_sub_issue = {field: sub_issue.get(field, "") for field in TICKET_PROMPT_FIELDS}
if _ticket_has_prompt_content(prompt_sub_issue):
related_tickets.append(prompt_sub_issue)

prompt_ticket = {field: normalized_ticket.get(field, "") for field in TICKET_PROMPT_FIELDS}
if _ticket_has_prompt_content(prompt_ticket):
related_tickets.append(prompt_ticket)

return related_tickets


def find_jira_tickets(text):
# Regular expression patterns for JIRA tickets
patterns = [
Expand Down Expand Up @@ -124,7 +176,8 @@ async def extract_tickets(git_provider) -> tuple[list | None, str]:
sub_issues_content.append({
'ticket_url': sub_issue_url,
'title': sub_issue.title,
'body': sub_body
'body': sub_body,
'labels': "",
})
except Exception as e:
get_logger().warning(f"Failed to fetch sub-issue content for {sub_issue_url}: {e}")
Expand Down Expand Up @@ -191,6 +244,7 @@ async def extract_and_cache_pr_tickets(git_provider, vars):
if not get_settings().get('pr_reviewer.require_ticket_analysis_review', False):
return

vars['related_tickets'] = []
vars['ticket_compliance_note'] = ""

related_tickets = get_settings().get('related_tickets', [])
Expand All @@ -203,22 +257,18 @@ async def extract_and_cache_pr_tickets(git_provider, vars):
get_settings().set('ticket_compliance_note', vars['ticket_compliance_note'])

if tickets_content:
# Store sub-issues along with main issues
for ticket in tickets_content:
if "sub_issues" in ticket and ticket["sub_issues"]:
for sub_issue in ticket["sub_issues"]:
related_tickets.append(sub_issue) # Add sub-issues content

related_tickets.append(ticket)
related_tickets = _extract_related_tickets_for_prompt(tickets_content)

get_logger().info("Extracted tickets and sub-issues from PR description",
artifact={"tickets": related_tickets})

vars['related_tickets'] = related_tickets
get_settings().set('related_tickets', related_tickets)
else:
related_tickets = _extract_related_tickets_for_prompt(related_tickets)
get_logger().info("Using cached tickets", artifact={"tickets": related_tickets})
vars['related_tickets'] = related_tickets
get_settings().set('related_tickets', related_tickets)
vars['ticket_compliance_note'] = cached_ticket_compliance_note or ''


Expand Down
48 changes: 47 additions & 1 deletion tests/unittest/test_convert_to_markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import textwrap
from unittest.mock import Mock

from pr_agent.algo.utils import PRReviewHeader, convert_to_markdown_v2
from pr_agent.algo.utils import (PRReviewHeader, convert_to_markdown_v2,
normalize_ticket_requirement_text,
parse_requirement_items)
from pr_agent.tools.pr_description import insert_br_after_x_chars

"""
Expand Down Expand Up @@ -247,6 +249,50 @@ def test_ticket_compliance_ignores_none_placeholders(self):
assert 'Compliant requirements:' not in result
assert '(none)' not in result

def test_ticket_compliance_ignores_none_with_punctuation(self):
input_data = {'review': {
'ticket_compliance_check': [
{
'ticket_url': 'https://example.com/ticket/LOG-11264',
'ticket_requirements': '- Requirement 1\n',
'fully_compliant_requirements': '- Requirement 1\n',
'not_compliant_requirements': 'None.\n',
'requires_further_human_verification': '- No information provided.\n',
}
]
}}

result = convert_to_markdown_v2(input_data)

assert '**🎫 Ticket compliance analysis ✅**' in result
assert '**[LOG-11264](https://example.com/ticket/LOG-11264) - Fully compliant**' in result
assert 'Compliant requirements:' in result
assert 'Non-compliant requirements:' not in result
assert 'Requires further human verification:' not in result
assert 'None.' not in result
assert 'No information provided.' not in result

def test_parse_requirement_items_filters_placeholder_variants(self):
items = parse_requirement_items(
'- Requirement 1\n'
'• None.\n'
'1. N/A:\n'
'[ ] No information provided.\n'
'- Requirement 2\n'
)

assert items == ['- Requirement 1', '- Requirement 2']

def test_normalize_ticket_requirement_text_joins_filtered_items(self):
normalized = normalize_ticket_requirement_text(
'- Requirement 1\n'
'-\n'
'* None.\n'
'2) Requirement 2\n'
)

assert normalized == '- Requirement 1\n2) Requirement 2'

def test_convert_to_markdown_does_not_mutate_review_input(self):
input_data = {'review': {
'score': '78/100',
Expand Down
1 change: 1 addition & 0 deletions tests/unittest/test_jira_cloud_ticket_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ async def fake_fetch_issue_json(session, auth, key):
assert ticket["sub_issues"][0]["ticket_url"] == "https://example.atlassian.net/browse/SUB-2"
assert ticket["sub_issues"][0]["title"] == "Subtask 1"
assert ticket["sub_issues"][0]["body"] == ""
assert ticket["sub_issues"][0]["labels"] == ""

def test_fetch_jira_ticket_context_uses_scoped_token_with_cloud_id(self, monkeypatch):
monkeypatch.setenv("JIRA_BASE_URL", "https://example.atlassian.net")
Expand Down
13 changes: 13 additions & 0 deletions tests/unittest/test_pr_reviewer_ticket_note.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ def test_build_suspected_ticket_mismatch_note_ignores_none_placeholders():
assert "wrong Jira ticket" in result


def test_build_suspected_ticket_mismatch_note_ignores_none_with_punctuation():
result = build_suspected_ticket_mismatch_note(
[
{
"fully_compliant_requirements": "",
"not_compliant_requirements": "None.\n",
}
]
)

assert result == ""


def test_build_suspected_ticket_mismatch_note_skips_non_dict_entries():
result = build_suspected_ticket_mismatch_note(
[
Expand Down
Loading