From ba5871ac5940f72bec82bf71b848548b3afa07b0 Mon Sep 17 00:00:00 2001 From: Glenn Jocher Date: Sun, 23 Aug 2026 15:45:30 +0200 Subject: [PATCH] Sanitize PR review text (#890) --- actions/__init__.py | 2 +- actions/review_pr.py | 7 ++++--- actions/utils/openai_utils.py | 32 +++++++++++++++++++++++++++++--- tests/test_first_interaction.py | 17 ++++++++++++++++- tests/test_openai_utils.py | 23 +++++++++++++++++++++++ 5 files changed, 73 insertions(+), 8 deletions(-) diff --git a/actions/__init__.py b/actions/__init__.py index 39178dd..0c6e154 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -28,4 +28,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.3.16" +__version__ = "0.3.17" diff --git a/actions/review_pr.py b/actions/review_pr.py index 5246664..03816b0 100644 --- a/actions/review_pr.py +++ b/actions/review_pr.py @@ -699,11 +699,12 @@ def generate_pr_review( # Do not pass background=True; queued background reviews can consume the full 900s poll timeout. ) - # Sanitize leaked tool-citation tokens from model output + # Sanitize model-authored text before posting it to GitHub response["summary"] = sanitize_ai_text(response.get("summary", "")) for c in response.get("comments", []): - if "message" in c: - c["message"] = sanitize_ai_text(c["message"]) + for field in ("message", "suggestion"): + if c.get(field): + c[field] = sanitize_ai_text(c[field]) print(json.dumps(response, indent=2)) diff --git a/actions/utils/openai_utils.py b/actions/utils/openai_utils.py index a1ecb18..aec8de7 100644 --- a/actions/utils/openai_utils.py +++ b/actions/utils/openai_utils.py @@ -6,6 +6,7 @@ import json import os import re import time +import unicodedata from collections.abc import Callable from concurrent.futures import ThreadPoolExecutor from uuid import uuid4 @@ -69,14 +70,39 @@ SYSTEM_PROMPT_ADDITION = """Guidance: - Use the "@" symbol to refer to GitHub users, e.g. @glenn-jocher. - Tone: Adopt a professional, friendly, and concise tone. """ +_PRIVATE_USE_RANGE = r"\uE000-\uF8FF\U000F0000-\U000FFFFD\U00100000-\U0010FFFD" _CITATION_PATTERN = re.compile( - r"[\uE000-\uF8FF]*\bcite[\uE000-\uF8FF]*(turn\d+(?:search|view)\d+|[\w\d]+)[\uE000-\uF8FF]*" + rf"[{_PRIVATE_USE_RANGE}]*\bcite[{_PRIVATE_USE_RANGE}]*(turn\d+(?:search|view)\d+|[\w\d]+)" + rf"[{_PRIVATE_USE_RANGE}]*|\bturn\d+[a-z_]+\d+[{_PRIVATE_USE_RANGE}]+" +) +_UNSAFE_UNICODE_CATEGORIES = {"Cc", "Cf", "Co", "Cs"} +_SAFE_FORMAT_CHARACTERS = {"\u200c", "\u200d"} +_EMOJI_TAG_SEQUENCE_PATTERN = re.compile( + "\U0001f3f4\U000e0067\U000e0062" + "(?:\U000e0065\U000e006e\U000e0067|\U000e0073\U000e0063\U000e0074|\U000e0077\U000e006c\U000e0073)" + "\U000e007f" ) def sanitize_ai_text(s: str) -> str: - """Strip private-use citation tokens (for example, ``cite...`` markers).""" - return _CITATION_PATTERN.sub("", s) if s else "" + """Strip citation tokens and unsafe Unicode characters from AI output.""" + if not s: + return "" + s = _CITATION_PATTERN.sub("", s) + safe_tag_positions = { + i for match in _EMOJI_TAG_SEQUENCE_PATTERN.finditer(s) for i in range(match.start(), match.end()) + } + return "".join( + c + for i, c in enumerate(s) + if i in safe_tag_positions + or c in "\n\t" + or c in _SAFE_FORMAT_CHARACTERS + or ( + unicodedata.category(c) not in _UNSAFE_UNICODE_CATEGORIES + and not (0xFDD0 <= ord(c) <= 0xFDEF or (ord(c) & 0xFFFF) in (0xFFFE, 0xFFFF)) + ) + ) def remove_outer_codeblocks(string): diff --git a/tests/test_first_interaction.py b/tests/test_first_interaction.py index 20c2507..38fd973 100644 --- a/tests/test_first_interaction.py +++ b/tests/test_first_interaction.py @@ -145,7 +145,20 @@ def test_get_first_interaction_response(mock_get_response, mock_check_links): def test_generate_pr_review_uses_synchronous_response(mock_get_agent_response, mock_checkout, tmp_path, monkeypatch): """Test PR reviews avoid background polling for code diffs.""" monkeypatch.chdir(tmp_path) - mock_get_agent_response.return_value = {"comments": [], "summary": "LGTM"} + mock_get_agent_response.return_value = { + "comments": [ + { + "file": "test.py", + "line": 1, + "side": "RIGHT", + "severity": "MEDIUM", + "message": "Finding.\ue201", + "start_line": None, + "suggestion": "old = False\ue201", + } + ], + "summary": "LGTM\ue201", + } mock_event = MagicMock() mock_event.repository = "ultralytics/actions" mock_event.pr = {"number": 1, "head": {"sha": "headsha"}} @@ -163,6 +176,8 @@ def test_generate_pr_review_uses_synchronous_response(mock_get_agent_response, m review = review_pr.generate_pr_review("ultralytics/actions", (diff, []), "Test PR", "", event=mock_event) assert review["summary"] == "LGTM" + assert review["comments"][0]["message"] == "Finding." + assert review["comments"][0]["suggestion"] == "old = False" mock_get_agent_response.assert_called_once() kwargs = mock_get_agent_response.call_args.kwargs assert "background" not in kwargs diff --git a/tests/test_openai_utils.py b/tests/test_openai_utils.py index 6a52065..0fa2bf9 100644 --- a/tests/test_openai_utils.py +++ b/tests/test_openai_utils.py @@ -19,6 +19,7 @@ from actions.utils.openai_utils import ( get_response, get_review_model, remove_outer_codeblocks, + sanitize_ai_text, ) @@ -96,6 +97,28 @@ def test_remove_outer_codeblocks(): assert remove_outer_codeblocks(input_str) == input_str +def test_sanitize_ai_text(): + """Strip citation markers and non-printing characters while preserving normal Unicode and Markdown whitespace.""" + assert sanitize_ai_text("Finding. \ue200cite\ue202turn10search1\ue201") == "Finding. " + assert sanitize_ai_text("Finding. turn10search1\ue201") == "Finding. " + assert sanitize_ai_text("Finding. turn7fetch3\ue201") == "Finding. " + assert sanitize_ai_text("Finding. turn10search1\U000f0000") == "Finding. " + assert ( + sanitize_ai_text( + "Finding.\x00\u200b\u202e\u2066\ufeff\ufdd0\ufff9\uffff\U000e0001\U000e0020\U0010ffff\ue201\U000f0000" + ) + == "Finding." + ) + assert sanitize_ai_text("Finding. turn10search1") == "Finding. turn10search1" + assert sanitize_ai_text("Café 中文 🩷 👩‍💻\n\tMarkdown") == "Café 中文 🩷 👩‍💻\n\tMarkdown" + for flag in ( + "\U0001f3f4\U000e0067\U000e0062\U000e0065\U000e006e\U000e0067\U000e007f", + "\U0001f3f4\U000e0067\U000e0062\U000e0073\U000e0063\U000e0074\U000e007f", + "\U0001f3f4\U000e0067\U000e0062\U000e0077\U000e006c\U000e0073\U000e007f", + ): + assert sanitize_ai_text(flag) == flag + + def test_get_review_model_override(): """Test review model override logic.""" with patch("actions.utils.openai_utils.REVIEW_MODEL", "claude-opus-4-7"), patch(