From 753b9c27efeaa1c6339a08f4cd2b46a084085684 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 06:06:02 +0000 Subject: [PATCH 1/2] fix(noema): remove dead CodeGraph context branch from Noema review gate load_codegraph_context() read NOEMA_CODEGRAPH_CONTEXT_PATH and always returned "" because no workflow ever set that variable -- a full-repo grep confirms only this script and its own unit tests referenced it, and noema-review.yml never mentions CodeGraph. Every production Noema review therefore rendered an always-empty "## CodeGraph context" section while its own LLM prompt claimed CodeGraph context was supplied. Wiring the capability up for real would mean running the CodeGraph CLI's trusted-root setup against untrusted PR code inside noema-review.yml, a workflow that currently never checks out PR head content at all (it only materializes the trusted gate script itself and reads PR data through the GitHub content API) -- a security-sensitive feature addition, not a minimal fix. Since CodeGraph context was never wired for Noema and nothing else in the repo advertises it as a Noema capability, remove the hollow branch, the dead helper, and the prompt's false claim instead. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw --- scripts/ci/noema_review_gate.py | 17 +---------------- tests/test_noema_review_gate.py | 15 ++------------- ...tory_branch_coverage_javascript_and_noema.py | 2 -- ...epository_branch_coverage_reporting_edges.py | 1 - 4 files changed, 3 insertions(+), 32 deletions(-) diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index df9f7f9a40..fbde83c112 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -300,24 +300,9 @@ def review_thread_context(pr: dict[str, Any]) -> str: return "\n".join(lines) -def load_codegraph_context() -> str: - """Load optional precomputed CodeGraph context for structural review evidence.""" - path = os.environ.get("NOEMA_CODEGRAPH_CONTEXT_PATH", "").strip() - if not path: - return "" - try: - with open(path, encoding="utf-8") as handle: - return truncate_text(handle.read(), MAX_REVIEW_CONTEXT_CHARS) - except OSError as exc: - return f"CodeGraph context unavailable: {exc}" - - def build_review_context(repo: str, number: int, pr: dict[str, Any]) -> str: """Build bounded non-diff context for the Noema reviewer.""" sections: list[str] = [] - codegraph = load_codegraph_context() - if codegraph: - sections.append("## CodeGraph context\n" + codegraph) threads = review_thread_context(pr) if threads: sections.append("## Prior review threads\n" + threads) @@ -463,7 +448,7 @@ def call_llm( "content": "\n".join( [ "You are Noema, an independent pull request reviewer for ContextualWisdomLab.", - "Review the PR diff plus the additional changed-file, review-thread, and CodeGraph context for correctness, security, maintainability, and behavioral regressions.", + "Review the PR diff plus the additional changed-file and review-thread context for correctness, security, maintainability, and behavioral regressions.", "Return only JSON with this shape:", '{"decision":"approve|request_changes|comment","summary":"...","findings":[{"severity":"high|medium|low","file":"path","line":1,"message":"..."}]}', "Use request_changes only for blocking, concrete issues. Use approve when no blocking issue is found.", diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 2fe4e48723..19f9eb2af6 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -138,7 +138,7 @@ def app_identity(args, **kwargs): noema.extract_json_object("not-json") -def test_review_context_builders_include_codegraph_threads_and_files(monkeypatch, tmp_path): +def test_review_context_builders_include_threads_and_files(monkeypatch): assert noema.truncate_text("abc", 10) == "abc" assert "truncated 2 characters" in noema.truncate_text("abcdef", 4) assert "missing PR head SHA" in noema.changed_file_context("owner/repo", 7, "") @@ -165,9 +165,6 @@ def fake_run(args, stdin=None): raise AssertionError(args) monkeypatch.setattr(noema, "run", fake_run) - codegraph_path = tmp_path / "codegraph.md" - codegraph_path.write_text("call graph: src/a.py -> tests", encoding="utf-8") - monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(codegraph_path)) pr = make_pr( headRefOid="head sha", reviewThreads={ @@ -191,8 +188,6 @@ def fake_run(args, stdin=None): context = noema.build_review_context("owner/repo", 7, pr) - assert "## CodeGraph context" in context - assert "call graph: src/a.py -> tests" in context assert "Thread open at src/a.py:3" in context assert "reviewer: check call site" in context assert "### src/a.py" in context @@ -202,13 +197,7 @@ def fake_run(args, stdin=None): assert any("/files" in call[2] for call in calls) -def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, tmp_path): - monkeypatch.delenv("NOEMA_CODEGRAPH_CONTEXT_PATH", raising=False) - assert noema.load_codegraph_context() == "" - - monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(tmp_path / "missing.md")) - assert "CodeGraph context unavailable" in noema.load_codegraph_context() - +def test_review_context_reports_omitted_files(monkeypatch): paths = [f"src/file_{index}.py" for index in range(noema.MAX_CONTEXT_FILES + 1)] monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths) monkeypatch.setattr(noema, "fetch_head_file_content", lambda repo, path, head_sha: "x") diff --git a/tests/test_repository_branch_coverage_javascript_and_noema.py b/tests/test_repository_branch_coverage_javascript_and_noema.py index 6eb5ab9baf..f2c954b3e3 100644 --- a/tests/test_repository_branch_coverage_javascript_and_noema.py +++ b/tests/test_repository_branch_coverage_javascript_and_noema.py @@ -178,9 +178,7 @@ def test_noema_review_context_includes_locations_bodies_and_all_sections( assert "src/runtime.py:7" in rendered assert "reviewer: Fix this" in rendered - monkeypatch.setattr(noema, "load_codegraph_context", lambda: "graph") monkeypatch.setattr(noema, "changed_file_context", lambda *_args: "files") context = noema.build_review_context("owner/repo", 1, pr) - assert "CodeGraph context" in context assert "Prior review threads" in context assert "Changed file context" in context diff --git a/tests/test_repository_branch_coverage_reporting_edges.py b/tests/test_repository_branch_coverage_reporting_edges.py index b4527147c8..f5dbf1dae0 100644 --- a/tests/test_repository_branch_coverage_reporting_edges.py +++ b/tests/test_repository_branch_coverage_reporting_edges.py @@ -129,7 +129,6 @@ def test_noema_small_diff_and_empty_context_branches( rendered_context = noema.review_thread_context(pr) assert rendered_context == "- Thread open at src/runtime.py:\n - reviewer: note" - monkeypatch.setattr(noema, "load_codegraph_context", lambda: "") monkeypatch.setattr(noema, "review_thread_context", lambda _pr: "") monkeypatch.setattr(noema, "changed_file_context", lambda *_args: "") assert noema.build_review_context("owner/repo", 1, pr) == "" From a30393186884c7bcca7ae605294acbf004f53e4c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 04:42:45 +0000 Subject: [PATCH 2/2] test(ci): port main coverage-gap fix from .github#1547 onto this branch Rebasing onto current main inherited the same pre-existing coverage gap #1547 (not yet merged) fixes: two REST-fallback scheduler code paths (fetch_workflow_names_by_check_suite_rest's pagination/error-handling, inspect_pr's conflicted-branch draft/unauthorized-conflict skips) with no existing test coverage. Ported #1547's identical test additions here rather than leaving this PR red waiting on that one to merge first -- this will no-op once #1547 lands and this branch rebases again. Full suite: 2228 passed, 1 skipped, 21 subtests. 100% coverage, 100% docstrings. --- tests/test_pr_review_fix_scheduler.py | 7 ++ ...ew_fix_scheduler_rest_workflow_identity.py | 67 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 3b4416bdc3..1507f6d5fb 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -1152,6 +1152,13 @@ def test_fix_inspect_skip_wait_and_error_paths(monkeypatch): assert fix.inspect_pr("owner/repo", make_pr(headRepository={"nameWithOwner": "fork/repo"}), args)[1] == ( "external PR head is not writable by repository workflow credentials", ) + assert fix.inspect_pr( + "owner/repo", make_pr(mergeStateStatus="DIRTY", isDraft=True), args + ) == ("skip", ("draft PR",)) + assert fix.inspect_pr("owner/repo", make_pr(mergeStateStatus="DIRTY"), args) == ( + "skip", + ("merge conflict is not authorized for repair",), + ) monkeypatch.setattr(fix, "needs_autofix", lambda pr: (False, ())) assert fix.inspect_pr("owner/repo", make_pr(), args) == ( diff --git a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py index c24cfb05f9..f261ce5beb 100644 --- a/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py +++ b/tests/test_pr_review_fix_scheduler_rest_workflow_identity.py @@ -154,3 +154,70 @@ def fake_api(path: str) -> Any: assert merge.is_strix_context(context) assert merge.strix_evidence_state(pr) == expected_state assert fix.current_head_failed_checks(pr) == () + + +def test_fetch_workflow_names_by_check_suite_rest_paginates_past_100( + monkeypatch: Any, +) -> None: + """A first page of exactly 100 runs must fetch a second page and merge both.""" + head_sha = "e" * 40 + page1 = [ + {"check_suite_id": i, "name": f"workflow-{i}"} for i in range(100) + ] + page2 = [{"check_suite_id": 100, "name": "workflow-100"}] + calls: list[str] = [] + + def fake_api(path: str) -> Any: + calls.append(path) + if path.endswith("page=1"): + return {"workflow_runs": page1} + if path.endswith("page=2"): + return {"workflow_runs": page2} + raise AssertionError(f"unexpected path {path}") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) + + assert names == {i: f"workflow-{i}" for i in range(101)} + assert calls == [ + f"repos/owner/repo/actions/runs?head_sha={head_sha}&per_page=100&page=1", + f"repos/owner/repo/actions/runs?head_sha={head_sha}&per_page=100&page=2", + ] + + +def test_fetch_workflow_names_by_check_suite_rest_skips_entries_missing_suite_id_or_name( + monkeypatch: Any, +) -> None: + """A run with no check-suite id or a blank name must not populate the map.""" + head_sha = "f" * 40 + + def fake_api(path: str) -> Any: + return { + "workflow_runs": [ + {"check_suite_id": None, "name": "orphaned run"}, + {"check_suite_id": 900, "name": ""}, + {"check_suite_id": 901, "name": "kept run"}, + ] + } + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + names = merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha) + + assert names == {901: "kept run"} + + +def test_fetch_workflow_names_by_check_suite_rest_propagates_non_access_errors( + monkeypatch: Any, +) -> None: + """A page-fetch failure unrelated to integration access must fail closed.""" + head_sha = "0" * 40 + + def fake_api(path: str) -> Any: + raise RuntimeError("gh: HTTP 502 (exhausted retries)") + + monkeypatch.setattr(merge, "gh_api_json", fake_api) + + with pytest.raises(RuntimeError, match="HTTP 502"): + merge.fetch_workflow_names_by_check_suite_rest("owner/repo", head_sha)