From 17fd291adfdb465f4da93101b7768ca9b2de0078 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 31 Aug 2026 15:30:23 +0900 Subject: [PATCH 1/3] fix(noema): paginate large pull request diffs Use GitHub's paginated pull-request files API so Noema can review ContextualWisdomLab/LineageWeave#640 beyond the 300-file diff endpoint limit. Reconstruct bounded diff context and fail closed when the returned file count does not match the exact PR metadata. Signed-off-by: Seongho Bae --- scripts/ci/noema_review_gate.py | 55 ++++++++++++-- tests/test_noema_review_gate.py | 72 +++++++++++++++++-- ...ry_branch_coverage_javascript_and_noema.py | 5 +- ...ository_branch_coverage_reporting_edges.py | 8 ++- 4 files changed, 128 insertions(+), 12 deletions(-) diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 54b536d583..76fe97c439 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -101,6 +101,7 @@ def graphql(query: str, **fields: str | int) -> dict[str, Any]: body isDraft headRefOid + changedFiles reviewDecision reviewThreads(first: 100) { nodes { @@ -214,10 +215,54 @@ def current_actor() -> str: return "" -def fetch_diff(repo: str, number: int) -> tuple[str, bool]: - """Fetch the PR diff and truncate it to the bounded LLM prompt size.""" - diff = run(["gh", "api", f"repos/{repo}/pulls/{number}", "-H", "Accept: application/vnd.github.v3.diff"]) - truncated = len(diff) > MAX_DIFF_CHARS +def fetch_diff( + repo: str, number: int, expected_files: int | None = None +) -> tuple[str, bool]: + """Fetch paginated PR file patches and bound them for the LLM prompt.""" + try: + pages = json.loads( + run( + [ + "gh", + "api", + f"repos/{repo}/pulls/{number}/files?per_page=100", + "--paginate", + "--slurp", + ] + ) + ) + except json.JSONDecodeError as exc: + raise RuntimeError("GitHub PR files response was not valid JSON") from exc + if not isinstance(pages, list) or any(not isinstance(page, list) for page in pages): + raise RuntimeError("GitHub PR files response had an unexpected shape") + files = [file for page in pages for file in page] + if any(not isinstance(file, dict) for file in files): + raise RuntimeError("GitHub PR files response had an unexpected file record") + if expected_files is not None and len(files) != expected_files: + raise RuntimeError( + f"GitHub returned {len(files)} of {expected_files} changed PR files" + ) + + sections: list[str] = [] + incomplete_patch = False + for file in files: + filename = str(file.get("filename") or "") + if not filename: + raise RuntimeError("GitHub PR file record omitted its filename") + old_filename = str(file.get("previous_filename") or filename) + status = str(file.get("status") or "modified") + old_label = "/dev/null" if status == "added" else f"a/{old_filename}" + new_label = "/dev/null" if status == "removed" else f"b/{filename}" + patch = file.get("patch") + if not isinstance(patch, str): + patch = "[patch unavailable from GitHub PR files API]" + incomplete_patch = True + sections.append( + f"diff --git a/{old_filename} b/{filename}\n" + f"--- {old_label}\n+++ {new_label}\n{patch}" + ) + diff = "\n".join(sections) + truncated = incomplete_patch or len(diff) > MAX_DIFF_CHARS if truncated: diff = diff[:MAX_DIFF_CHARS] return diff, truncated @@ -610,7 +655,7 @@ def inspect_and_review(repo: str, number: int) -> int: if existing_noema_review(pr, actor): print("Current head already has a Noema review; nothing to do.") return 0 - diff, truncated = fetch_diff(repo, number) + diff, truncated = fetch_diff(repo, number, pr.get("changedFiles")) review_context = build_review_context(repo, number, pr) verdict = call_llm(repo, number, pr, diff, truncated, review_context) submit_review(repo, number, pr, actor, verdict) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 3263b7021c..fe10a19f44 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -136,10 +136,39 @@ def app_identity(args, **kwargs): monkeypatch.setattr(noema, "run", app_identity) assert noema.current_actor() == "cwl-noema-review[bot]" - monkeypatch.setattr(noema, "run", lambda *args, **kwargs: "x" * (noema.MAX_DIFF_CHARS + 5)) - diff, truncated = noema.fetch_diff("owner/repo", 1) + pages = [ + [ + { + "filename": "src/added.py", + "status": "added", + "patch": "@@ -0,0 +1 @@\n+added", + } + ], + [ + { + "filename": "src/current.py", + "previous_filename": "src/old.py", + "status": "renamed", + "patch": "x" * noema.MAX_DIFF_CHARS, + } + ], + ] + + def paginated_files(args, **kwargs): + assert "repos/owner/repo/pulls/1/files?per_page=100" in args + assert "--paginate" in args + assert "--slurp" in args + return json.dumps(pages) + + monkeypatch.setattr(noema, "run", paginated_files) + diff, truncated = noema.fetch_diff("owner/repo", 1, expected_files=2) assert truncated assert len(diff) == noema.MAX_DIFF_CHARS + assert "--- /dev/null\n+++ b/src/added.py" in diff + assert "diff --git a/src/old.py b/src/current.py" in diff + + with pytest.raises(RuntimeError, match="returned 2 of 3"): + noema.fetch_diff("owner/repo", 1, expected_files=3) assert noema.extract_json_object('{"decision":"approve"}') == {"decision": "approve"} assert noema.extract_json_object('prefix {"decision":"comment"} suffix') == {"decision": "comment"} @@ -147,6 +176,37 @@ def app_identity(args, **kwargs): noema.extract_json_object("not-json") +@pytest.mark.parametrize( + ("response", "message"), + [ + ("not-json", "not valid JSON"), + ("{}", "unexpected shape"), + ("[{}]", "unexpected shape"), + ("[[null]]", "unexpected file record"), + ("[[{}]]", "omitted its filename"), + ], +) +def test_fetch_diff_rejects_incomplete_paginated_responses( + monkeypatch, response, message +): + """Malformed or incomplete GitHub file pages fail closed.""" + monkeypatch.setattr(noema, "run", lambda _args: response) + with pytest.raises(RuntimeError, match=message): + noema.fetch_diff("owner/repo", 1) + + +def test_fetch_diff_marks_unavailable_patch_as_truncated(monkeypatch): + """A file without GitHub patch text remains visible but incomplete.""" + response = json.dumps([[{"filename": "removed.bin", "status": "removed"}]]) + monkeypatch.setattr(noema, "run", lambda _args: response) + + diff, truncated = noema.fetch_diff("owner/repo", 1, expected_files=1) + + assert truncated + assert "+++ /dev/null" in diff + assert "[patch unavailable from GitHub PR files API]" in diff + + @pytest.mark.parametrize( ("actor", "installation_id", "source"), [ @@ -451,7 +511,9 @@ def test_inspect_and_review_skip_paths(monkeypatch): calls = [] monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: clean_pr) monkeypatch.setattr(noema, "current_actor", lambda: "noema") - monkeypatch.setattr(noema, "fetch_diff", lambda repo, number: ("diff", False)) + monkeypatch.setattr( + noema, "fetch_diff", lambda repo, number, expected_files=None: ("diff", False) + ) monkeypatch.setattr(noema, "build_review_context", lambda repo, number, pr: "context") monkeypatch.setattr(noema, "call_llm", lambda *args, **kwargs: {"decision": "approve", "summary": "ok", "findings": []}) monkeypatch.setattr(noema, "submit_review", lambda *args, **kwargs: calls.append(args)) @@ -489,7 +551,9 @@ def test_inspect_and_review_does_not_wait_for_other_reviews_or_checks(monkeypatc calls = [] monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: pr) monkeypatch.setattr(noema, "current_actor", lambda: "noema") - monkeypatch.setattr(noema, "fetch_diff", lambda repo, number: ("diff", False)) + monkeypatch.setattr( + noema, "fetch_diff", lambda repo, number, expected_files=None: ("diff", False) + ) monkeypatch.setattr(noema, "build_review_context", lambda repo, number, value: "context") monkeypatch.setattr(noema, "call_llm", lambda *args, **kwargs: {"decision": "approve", "summary": "ok"}) monkeypatch.setattr(noema, "submit_review", lambda *args, **kwargs: calls.append(args)) diff --git a/tests/test_repository_branch_coverage_javascript_and_noema.py b/tests/test_repository_branch_coverage_javascript_and_noema.py index 6eb5ab9baf..e9187b0c63 100644 --- a/tests/test_repository_branch_coverage_javascript_and_noema.py +++ b/tests/test_repository_branch_coverage_javascript_and_noema.py @@ -144,7 +144,10 @@ def test_noema_fetch_diff_truncates_to_prompt_budget( ) -> None: """Oversized diffs are bounded and explicitly marked truncated.""" - monkeypatch.setattr(noema, "run", lambda _args: "x" * (noema.MAX_DIFF_CHARS + 1)) + response = json.dumps( + [[{"filename": "large.py", "patch": "x" * (noema.MAX_DIFF_CHARS + 1)}]] + ) + monkeypatch.setattr(noema, "run", lambda _args: response) diff, truncated = noema.fetch_diff("owner/repo", 1) assert truncated is True assert len(diff) == noema.MAX_DIFF_CHARS diff --git a/tests/test_repository_branch_coverage_reporting_edges.py b/tests/test_repository_branch_coverage_reporting_edges.py index b4527147c8..2a52ad792c 100644 --- a/tests/test_repository_branch_coverage_reporting_edges.py +++ b/tests/test_repository_branch_coverage_reporting_edges.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from pathlib import Path import pytest @@ -107,8 +108,11 @@ def test_noema_small_diff_and_empty_context_branches( ) -> None: """Small diffs, invalid thread lines, and empty sections stay clean.""" - monkeypatch.setattr(noema, "run", lambda _args: "small diff") - assert noema.fetch_diff("owner/repo", 1) == ("small diff", False) + response = json.dumps([[{"filename": "small.py", "patch": "small diff"}]]) + monkeypatch.setattr(noema, "run", lambda _args: response) + diff, truncated = noema.fetch_diff("owner/repo", 1) + assert diff.endswith("small diff") + assert truncated is False pr = { "headRefOid": "a" * 40, From 7ff35fa3c19bf53fdd41416d86d716fb0e02b5d2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 31 Aug 2026 17:55:45 +0900 Subject: [PATCH 2/3] fix(noema): quote synthesized diff paths Signed-off-by: Seongho Bae --- scripts/ci/noema_review_gate.py | 26 ++++++++++++++++++++------ tests/test_noema_review_gate.py | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index db3d829357..f1f463cd41 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -255,14 +255,15 @@ def fetch_diff( raise RuntimeError("GitHub PR file record omitted its filename") old_filename = str(file.get("previous_filename") or filename) status = str(file.get("status") or "modified") - old_label = "/dev/null" if status == "added" else f"a/{old_filename}" - new_label = "/dev/null" if status == "removed" else f"b/{filename}" + old_label = "/dev/null" if status == "added" else _format_diff_path(old_filename, "a/") + new_label = "/dev/null" if status == "removed" else _format_diff_path(filename, "b/") patch = file.get("patch") if not isinstance(patch, str): patch = "[patch unavailable from GitHub PR files API]" incomplete_patch = True sections.append( - f"diff --git a/{old_filename} b/{filename}\n" + f"diff --git {_format_diff_path(old_filename, 'a/')} " + f"{_format_diff_path(filename, 'b/')}\n" f"--- {old_label}\n+++ {new_label}\n{patch}" ) diff = "\n".join(sections) @@ -272,6 +273,14 @@ def fetch_diff( return diff, truncated +def _format_diff_path(path: str, prefix: str) -> str: + """Return a plain or JSON-quoted diff path that round-trips safely.""" + value = f"{prefix}{path}" + if any(character in '\t\n\r"\\' or not 0x20 <= ord(character) < 0x7F for character in value): + return json.dumps(value, ensure_ascii=True) + return value + + def changed_diff_locations(diff: str) -> set[tuple[str, int, str]]: """Return exact LEFT/RIGHT changed-line locations from a unified diff.""" locations: set[tuple[str, int, str]] = set() @@ -321,9 +330,14 @@ def parse_diff_path(raw: str, prefix: str) -> str: return "" if value.startswith('"'): try: - decoded = ast.literal_eval(value) - value = decoded.encode("latin-1").decode("utf-8") - except (SyntaxError, ValueError, UnicodeError): + value = json.loads(value) + except json.JSONDecodeError: + try: + decoded = ast.literal_eval(value) + value = decoded.encode("latin-1").decode("utf-8") + except (SyntaxError, ValueError, UnicodeError): + return "" + if not isinstance(value, str): return "" return value.removeprefix(prefix) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 85abc7cea2..7f9dbaa02f 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -195,6 +195,27 @@ def test_fetch_diff_rejects_incomplete_paginated_responses( noema.fetch_diff("owner/repo", 1) +def test_fetch_diff_round_trips_special_current_and_previous_filenames(monkeypatch): + """Synthesized diff headers preserve every Git-special filename byte.""" + previous = 'src/old\t"quoted"\\name\n.py' + current = "src/새 이름\tcurrent.py" + pages = [[{ + "filename": current, + "previous_filename": previous, + "status": "renamed", + "patch": "@@ -1 +1 @@\n-old\n+new", + }]] + monkeypatch.setattr(noema, "run", lambda *args, **kwargs: json.dumps(pages)) + + diff, truncated = noema.fetch_diff("owner/repo", 1, expected_files=1) + + assert not truncated + assert noema.changed_diff_locations(diff) == { + (previous, 1, "LEFT"), + (current, 1, "RIGHT"), + } + + def test_fetch_diff_marks_unavailable_patch_as_truncated(monkeypatch): """A file without GitHub patch text remains visible but incomplete.""" response = json.dumps([[{"filename": "removed.bin", "status": "removed"}]]) From 100a46d58d9aa2a07c2240c2f42ca1391da8a31e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 31 Aug 2026 18:18:16 +0900 Subject: [PATCH 3/3] test(noema): cover malformed decoded diff path Signed-off-by: Seongho Bae --- tests/test_noema_review_gate.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 7f9dbaa02f..d950e945c3 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -216,6 +216,13 @@ def test_fetch_diff_round_trips_special_current_and_previous_filenames(monkeypat } +def test_parse_diff_path_rejects_non_string_json(monkeypatch): + """A malformed decoder result cannot become a trusted changed-file path.""" + monkeypatch.setattr(noema.json, "loads", lambda _value: None) + + assert noema.parse_diff_path('"a/file.py"', "a/") == "" + + def test_fetch_diff_marks_unavailable_patch_as_truncated(monkeypatch): """A file without GitHub patch text remains visible but incomplete.""" response = json.dumps([[{"filename": "removed.bin", "status": "removed"}]])