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 docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ Python and the JS library use the same rules:
- The decision is the leading token after whitespace, one `<think>` block, one code fence, markdown `**` / `__` / backticks, straight or curly quotes, and an optional `Answer:` or `Decision:` prefix. That token must be exactly `yes` or `no`, then the end of the string, whitespace, or one of `. , ! : ; )`. A quoted or bold decision followed by a reason (`**no** unrelated`, `"no" unrelated`, `“no” — unrelated`) is that decision. Anything else is malformed: `failed_open=true`, the file is included, and the decision is not cached.
- A missing `choices` list, an empty list, a missing message or content field, non-string message content (such as `["no"]`), and a non-JSON body are failed open inside that same handling and are not cached.
- A cache read error is a miss: the model is called normally and the run does not abort. A cache write error is logged and does not abort the run. When the OpenAI client rejects `reasoning_effort`, the retry omits it and the decision is cached under the empty effort.
- Valid UTF-8 bytes are decoded to text, including a leading U+FEFF BOM, and are not replaced with an empty string. The 8192-byte probe tolerates only an incomplete trailing sequence of at most 3 bytes. Invalid UTF-8 anywhere else in the payload, including after that window, takes the binary path. A NUL is binary. A non-empty buffer that decodes to empty text is binary. `on_binary=include` returns the original bytes unchanged.
- Valid UTF-8 bytes are decoded to text, including a leading U+FEFF BOM, and are not replaced with an empty string. An incomplete trailing sequence of at most 3 bytes keeps the valid prefix at any length (`b"a\xc3"` decodes to `"a"`). Any other invalid UTF-8, including a bad byte after the first 8192 bytes, takes the binary path. A NUL is binary. A non-empty buffer that decodes to empty text is binary. `on_binary=include` returns the original bytes unchanged. Ignored paths are excluded before those bytes are decoded.
- Token truncation keeps the first `maxTokens * 4` Unicode code points and never splits a code point or a surrogate pair. The byte cap cuts on a code-point boundary in UTF-8 and never emits U+FFFD. The shared fixture is `tests/fixtures/parity/relevance.json`.
- The token estimate is `ceil(code points / 4)`. Empty text is 0. tiktoken is not used for this check.
- Skip globs are case-insensitive. `**` crosses directories and `*` does not. `skipped_reason` for a skip is `ignored`. The list includes `**/*.min.js` and `**/*.min.mjs`.
Expand Down
2 changes: 1 addition & 1 deletion docs/development/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ assert "Updated" in updated["file.py"]

## Relevance filter

`tests/test_relevance.py` mocks the LLM. It covers the shared 1024-token / `reasoning_effort=low` request, the length-prefixed cache key (mode, note, and the effort actually sent, including a legacy SDK that rejects the parameter), the leading-token parser (a quoted decision followed by a reason), non-string message content failed open, malformed answers counted as failed open, a cache read treated as a miss, cache write errors, transient retries, invalid UTF-8 after the 8192-byte probe, a preserved leading BOM, and the canonical skip list (`ignored`, including `*.min.mjs`). `tests/fixtures/parity/relevance.json` is a byte-identical copy of the JS fixture and is checked against the library functions, including its sha256. `tests/test_relevance_live.py` is skipped unless `LIVE_LLM=1`, in which case it reads `GPTDIFF_LLM_API_KEY` and calls the configured endpoint. CI does not set `LIVE_LLM`, so it never makes that call. The benchmark in `bench/relevance/` uses the same gate. `tests/test_relevance_bench.py` checks that a single `generate_diff` failure is stored in the JSON report with the other pipeline's diff and applied files.
`tests/test_relevance.py` mocks the LLM. It covers the shared 1024-token / `reasoning_effort=low` request, the length-prefixed cache key (mode, note, and the effort actually sent, including a legacy SDK that rejects the parameter), the leading-token parser (a quoted decision followed by a reason), non-string message content failed open, malformed answers counted as failed open, a cache read treated as a miss, cache write errors, transient retries, invalid UTF-8 after the 8192-byte probe, a truncated UTF-8 tail that keeps its valid prefix, ignored binaries excluded before decode, a preserved leading BOM, and the canonical skip list (`ignored`, including `*.min.mjs`). `tests/fixtures/parity/relevance.json` is a byte-identical copy of the JS fixture and is checked against the library functions, including its sha256. `tests/test_relevance_live.py` is skipped unless `LIVE_LLM=1`, in which case it reads `GPTDIFF_LLM_API_KEY` and calls the configured endpoint. CI does not set `LIVE_LLM`, so it never makes that call. The benchmark in `bench/relevance/` uses the same gate. `tests/test_relevance_bench.py` checks that a single `generate_diff` failure is stored in the JSON report with the other pipeline's diff and applied files.

## Running the Test Suite

Expand Down
46 changes: 24 additions & 22 deletions patchling/relevance.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
Anything else is malformed: the file is included, ``failed_open`` is set, and
the decision is not cached. Non-string message content is that same failure.
A cache read error is a miss and the model is called; a cache write error is
logged and does not abort the run. Invalid UTF-8 anywhere in a byte payload
logged and does not abort the run. A truncated UTF-8 tail keeps the valid
prefix (``b"a\\xc3"`` is ``"a"``). Any other invalid UTF-8 in a byte payload
is binary. ``on_binary="include"`` returns those original bytes unchanged.
A leading UTF-8 BOM (U+FEFF) stays in decoded text.
A leading UTF-8 BOM (U+FEFF) stays in decoded text. Ignored paths are
excluded before their bytes are decoded.
"""

from __future__ import annotations
Expand Down Expand Up @@ -631,11 +633,12 @@ def _utf8_text_or_none(sample: bytes) -> Optional[str]:
def is_binary_content(content) -> bool:
"""True when the payload is not text.

The first 8192 bytes are decoded as UTF-8. An incomplete sequence of at
most 3 bytes at that probe boundary is still text. A NUL or any other
invalid byte inside the window is binary, and so is a non-empty window
that decodes to empty text. Bytes past the window are checked too: any
invalid UTF-8 there is binary. A leading BOM is text.
The whole payload is validated as UTF-8. An incomplete trailing sequence
of at most 3 bytes keeps the valid prefix and is still text, at any
length (``b"a\\xc3"`` is ``"a"``). A NUL or any other invalid byte is
binary, and so is a non-empty buffer that decodes to empty text. The
first 8192 bytes are also checked for control-character density. A
leading BOM is text.
"""
if isinstance(content, str):
if "\x00" in content:
Expand All @@ -661,11 +664,10 @@ def _bytes_are_binary(raw: bytes) -> bool:
return True
if len(raw) <= 8192:
return False
# The probe may end inside a character. The rest of the file has to be
# well-formed UTF-8; an invalid byte after the window is not dropped.
try:
raw.decode("utf-8")
except UnicodeDecodeError:
# An incomplete trailing sequence of at most 3 bytes keeps the prefix.
# Any other invalid byte, including one past the probe window, is binary.
rest = _utf8_text_or_none(raw)
if rest is None or rest == "":
return True
return False

Expand Down Expand Up @@ -694,25 +696,24 @@ def _coerce_text(content) -> str:
def _decode_utf8_text(raw: bytes) -> Optional[str]:
"""Decode UTF-8, keeping a leading BOM (U+FEFF).

``utf-8-sig`` is not used. A buffer of at most 8192 bytes may end on an
incomplete sequence; those trailing bytes are dropped. A longer buffer
must be entirely well-formed, so a bad byte after the probe is not cut off.
``utf-8-sig`` is not used. An incomplete trailing sequence of at most
3 bytes is dropped and the valid prefix is kept, at any length. Any
other invalid byte returns ``None`` so the caller takes the binary path.
"""
try:
return raw.decode("utf-8")
except UnicodeDecodeError:
if len(raw) > 8192:
return None
return _utf8_text_or_none(raw)


def _prepare_content(content) -> tuple[bool, str]:
"""Return ``(is_binary, text)``.

Valid UTF-8 bytes are decoded to text, including a leading U+FEFF, and
are never replaced with an empty string. Invalid UTF-8 anywhere, or a
NUL byte, takes the binary path. Callers that include a binary file pass
the original bytes through instead of this text.
are never replaced with an empty string. A truncated trailing sequence
keeps the valid prefix. Any other invalid UTF-8, or a NUL byte, takes
the binary path. Callers that include a binary file pass the original
bytes through instead of this text.
"""
if isinstance(content, str):
return is_binary_content(content), content
Expand Down Expand Up @@ -977,8 +978,9 @@ def filter_relevant(
the empty effort.

Binary files and files over ``max_bytes`` / ``max_tokens`` are not sent in
full. Invalid UTF-8 after the first 8192 bytes is still binary.
``on_binary="include"`` keeps the original bytes. ``on_binary`` and
full. A truncated UTF-8 tail keeps the valid prefix. Any other invalid
UTF-8 is binary. Ignored paths are excluded before their bytes are
decoded. ``on_binary="include"`` keeps the original bytes. ``on_binary`` and
``on_oversize`` are ``include`` (keep, no model
call), ``exclude`` (drop, no model call), or ``truncate`` (ask the model
about a truncated or redacted preview). Pass ``None`` for ``max_bytes`` or
Expand Down
24 changes: 21 additions & 3 deletions tests/fixtures/parity/relevance.json
Original file line number Diff line number Diff line change
Expand Up @@ -163,21 +163,39 @@
{ "text": "a€b", "maxBytes": 4, "expected": "a€" }
],
"bytes": {
"rule": "Valid UTF-8 bytes decode to text and a non-empty buffer is never decoded as empty. Invalid UTF-8 takes the binary path. Different bytes at the same path do not share a key.",
"rule": "Valid UTF-8 bytes decode to text and a non-empty buffer is never decoded as empty. A truncated trailing sequence keeps the valid prefix only when those bytes are a legal partial character (0x61 0xC3 and 0x61 0xE0 0xA0 decode to \"a\"). Any other invalid UTF-8 takes the binary path. Different bytes at the same path do not share a key.",
"validUtf8": "café",
"changed": ["alpha", "beta"]
},
"binary": {
"sampleBytes": 8192,
"rule": "Validate the whole payload as UTF-8. An incomplete trailing sequence is tolerated only when it ends on the 8192-byte probe boundary. A NUL or any other invalid byte anywhere is binary. A non-empty buffer that decodes to empty text is binary. Included binary files keep their original bytes.",
"rule": "Validate the whole payload as UTF-8. Tolerate a tail only when it is a truncated but valid partial sequence at the exact end of the sample: the lead must be able to start a character (not C0, C1, or F5-FF), every continuation must be 80-BF, and the second byte must be in range (E0 is A0-BF, ED is 80-9F, F0 is 90-BF, F4 is 80-8F). That partial keeps the valid prefix at any length, including 0x61 0xC3 and 0x61 0xE0 0xA0 decoding to \"a\". Bytes 0x61 0xE0 0x80, 0x61 0xED 0xA0, 0x61 0xF0 0x80, and 0x61 0xF4 0x90 are binary. A NUL or any other invalid byte anywhere is binary. A non-empty buffer that decodes to empty text is binary. Included binary files keep their original bytes.",
"cases": [
{ "name": "2-byte cut", "pad": 8191, "tail": [195], "binary": false },
{ "name": "3-byte cut", "pad": 8190, "tail": [226, 130], "binary": false },
{ "name": "4-byte cut", "pad": 8189, "tail": [240, 159, 152], "binary": false },
{ "name": "invalid lead", "pad": 8191, "tail": [255], "binary": true },
{ "name": "nul", "pad": 0, "tail": [104, 105, 0], "binary": true },
{ "name": "incomplete only", "pad": 0, "tail": [195], "binary": true },
{ "name": "8192 ASCII + 0xff + TAIL", "pad": 8192, "tail": [255, 84, 65, 73, 76], "binary": true }
{ "name": "truncated tail keeps prefix", "pad": 0, "tail": [97, 195], "binary": false, "text": "a" },
{ "name": "valid e0 partial keeps prefix", "pad": 0, "tail": [97, 224, 160], "binary": false, "text": "a" },
{ "name": "8192 ASCII + 0xff + TAIL", "pad": 8192, "tail": [255, 84, 65, 73, 76], "binary": true },
{ "name": "e0 second byte 80", "pad": 1, "tail": [224, 128], "binary": true },
{ "name": "ed second byte a0", "pad": 1, "tail": [237, 160], "binary": true },
{ "name": "f0 second byte 80", "pad": 1, "tail": [240, 128], "binary": true },
{ "name": "f4 second byte 90", "pad": 1, "tail": [244, 144], "binary": true },
{ "name": "e0 second byte 80 at 8192", "pad": 8192, "tail": [224, 128], "binary": true },
{ "name": "ed second byte a0 at 8192", "pad": 8192, "tail": [237, 160], "binary": true },
{ "name": "f0 second byte 80 at 8192", "pad": 8192, "tail": [240, 128], "binary": true },
{ "name": "f4 second byte 90 at 8192", "pad": 8192, "tail": [244, 144], "binary": true },
{ "name": "e0 second byte 80 at 9000", "pad": 9000, "tail": [224, 128], "binary": true },
{ "name": "ed second byte a0 at 9000", "pad": 9000, "tail": [237, 160], "binary": true },
{ "name": "f0 second byte 80 at 9000", "pad": 9000, "tail": [240, 128], "binary": true },
{ "name": "f4 second byte 90 at 9000", "pad": 9000, "tail": [244, 144], "binary": true },
{ "name": "lead c0", "pad": 0, "tail": [97, 192], "binary": true },
{ "name": "lead c1", "pad": 0, "tail": [97, 193], "binary": true },
{ "name": "lead f5", "pad": 0, "tail": [97, 245], "binary": true },
{ "name": "continuation below 80", "pad": 0, "tail": [97, 195, 127], "binary": true }
]
},
"bom": {
Expand Down
104 changes: 101 additions & 3 deletions tests/test_relevance.py
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,97 @@ def fake_call_llm(api_key, base_url, model, messages, max_tokens, temperature, b
assert included.files["a.bin"].endswith(b"\xffTAIL")


def test_truncated_utf8_tail_keeps_the_valid_prefix():
from patchling.relevance import _decode_utf8_text, _prepare_content, is_binary_content

short = b"a\xc3"
assert is_binary_content(short) is False
assert _decode_utf8_text(short) == "a"
assert _prepare_content(short) == (False, "a")
assert _prepare_content(b"hello\xe2\x82") == (False, "hello")
assert _prepare_content(b"a\xf0\x9f\x98") == (False, "a")
# A lone incomplete sequence decodes to empty text and stays binary.
assert is_binary_content(b"\xc3") is True
assert _prepare_content(b"\xc3") == (True, "")
# A bad continuation is not a truncated tail.
assert is_binary_content(b"a\xc3X") is True
assert _decode_utf8_text(b"a\xc3X") is None

long = b"a" * 8192 + b"\xc3"
assert len(long) > 8192
assert is_binary_content(long) is False
binary, text = _prepare_content(long)
assert binary is False
assert text == "a" * 8192
assert "\ufffd" not in text

bodies = []

def fake_call_llm(api_key, base_url, model, messages, max_tokens, temperature, budget_tokens=None):
bodies.append(messages[-1]["content"])
return _Response("yes")

result = filter_relevant(
{"a.py": short},
"task",
model="m",
cache=False,
api_key="k",
call_llm=fake_call_llm,
)
assert result.decisions[0].skipped_reason is None
assert result.files["a.py"] == "a"
assert "File contents:\na\n\nIs this file relevant" in bodies[0]
assert "\ufffd" not in bodies[0]


def test_ignored_binary_is_not_decoded(monkeypatch):
import patchling.relevance as relevance

payload = bytes([0x61, 0xC3, 0xFF, 0x00])
decoded = []

def _spy(name, fn):
def wrapped(content, *args, **kwargs):
raw = content
if isinstance(raw, (bytes, bytearray)) and bytes(raw) == payload:
decoded.append(name)
return fn(content, *args, **kwargs)
return wrapped

for name in (
"_prepare_content",
"is_binary_content",
"_decode_utf8_text",
"_utf8_text_or_none",
"_coerce_text",
):
monkeypatch.setattr(relevance, name, _spy(name, getattr(relevance, name)))

def fake_call_llm(api_key, base_url, model, messages, max_tokens, temperature, budget_tokens=None):
user = messages[-1]["content"]
assert "dist/app.min.js" not in user
return _Response("yes")

result = filter_relevant(
{
"dist/app.min.js": payload,
"src/keep.js": "export const n = 1;\n",
},
"task",
model="m",
cache=False,
api_key="k",
call_llm=fake_call_llm,
)
ignored = next(item for item in result.decisions if item.path == "dist/app.min.js")
assert ignored.decision == "no"
assert ignored.skipped_reason == "ignored"
assert "dist/app.min.js" not in result.files
assert decoded == []
assert result.files["src/keep.js"] == "export const n = 1;\n"


def test_leading_bom_is_preserved_in_decoded_text():
from patchling.relevance import PROMPT_VERSION, _prepare_content, _truncate_text

Expand Down Expand Up @@ -1710,9 +1801,10 @@ def fake_apply(file_path, original_content, file_diff, model, api_key=None, base


# sha256 of tests/fixtures/parity/relevance.json. That file is a byte-identical
# copy of test/fixtures/parity/relevance.json on 255BITS/patchling branch
# cursor/relevance-filter-followup-915b. A mismatch fails CI.
RELEVANCE_FIXTURE_SHA256 = "6976ad71714be0138a6043d9bdeb724abe34a65515f8389913dca10a0c48c4f2"
# copy of test/fixtures/parity/relevance.json on 255BITS/patchling commit
# f26c770a026f2554da3972c10b7f7918d075fd56 (branch cursor/main-green-utf8-915b).
# A mismatch fails CI.
RELEVANCE_FIXTURE_SHA256 = "dbe5c7033b279be2a0b923d82dffb1349039b051e18a3650b224570374b8914b"


def test_vendored_relevance_fixture_matches_library():
Expand All @@ -1722,6 +1814,7 @@ def test_vendored_relevance_fixture_matches_library():
PROMPT_VERSION,
_SYSTEM_PROMPT,
_build_user_prompt,
_prepare_content,
_truncate_text,
count_tokens,
is_binary_content,
Expand Down Expand Up @@ -1843,6 +1936,11 @@ def test_vendored_relevance_fixture_matches_library():
sample = b"a" * case["pad"] + tail
expect = case.get("binary", case.get("expect_binary"))
assert is_binary_content(sample) is expect, case
if "text" in case:
binary_flag, text = _prepare_content(sample)
assert binary_flag is False, case
assert text == case["text"], case
assert "\ufffd" not in text

bom = data.get("bom")
if bom:
Expand Down
Loading