diff --git a/.github/workflows/repair-noema-parallel-context.yml b/.github/workflows/repair-noema-parallel-context.yml new file mode 100644 index 000000000..6f7589654 --- /dev/null +++ b/.github/workflows/repair-noema-parallel-context.yml @@ -0,0 +1,100 @@ +name: Repair Noema parallel changed-file context + +on: + push: + paths: + - .github/workflows/repair-noema-parallel-context.yml + +permissions: + contents: write + +concurrency: + group: repair-noema-parallel-context-${{ github.ref }} + cancel-in-progress: false + +jobs: + repair: + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + disable-file-monitoring: true + + - name: Check out repair branch + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + ref: bolt-noema-review-api-batching-3009201169551405394 + persist-credentials: true + + - name: Apply bounded production repair + run: | + set -euo pipefail + python3 <<'PY' + from pathlib import Path + + path = Path("scripts/ci/noema_review_gate.py") + source = path.read_text(encoding="utf-8") + + old_constant = "MAX_CONTEXT_FILES = 12\nMAX_FILE_CONTEXT_CHARS = 4000" + new_constant = ( + "MAX_CONTEXT_FILES = 12\n" + "MAX_CONTEXT_FETCH_WORKERS = 10\n" + "MAX_FILE_CONTEXT_CHARS = 4000" + ) + if source.count(old_constant) != 1: + raise SystemExit("Noema context constant anchor did not match exactly once") + source = source.replace(old_constant, new_constant) + + old_target_block = ( + " target_paths = paths[:MAX_CONTEXT_FILES]\n" + " if not target_paths:\n" + " return \"Changed file context unavailable: no paths to check.\"\n\n" + " def _fetch_file_content(path: str) -> tuple[str, str | None, str | None]:\n" + " try:\n" + ) + new_target_block = ( + " target_paths = paths[:MAX_CONTEXT_FILES]\n\n" + " def _fetch_file_content(path: str) -> tuple[str, str | None, str | None]:\n" + " \"\"\"Return head text or a scrubbed RuntimeError for one changed path.\"\"\"\n\n" + " try:\n" + ) + if source.count(old_target_block) != 1: + raise SystemExit("Noema target-path helper anchor did not match exactly once") + source = source.replace(old_target_block, new_target_block) + + old_workers = " max_workers = min(10, len(target_paths))" + new_workers = ( + " max_workers = min(MAX_CONTEXT_FETCH_WORKERS, len(target_paths))" + ) + if source.count(old_workers) != 1: + raise SystemExit("Noema worker-limit anchor did not match exactly once") + source = source.replace(old_workers, new_workers) + + path.write_text(source, encoding="utf-8") + PY + + - name: Run focused regression evidence + run: | + set -euo pipefail + python3 -m compileall -q scripts/ci/noema_review_gate.py tests/test_noema_parallel_changed_file_context.py + python3 -m pytest -q \ + tests/test_noema_review_gate.py \ + tests/test_noema_parallel_changed_file_context.py + + - name: Commit verified repair and remove one-shot workflow + run: | + set -euo pipefail + rm .github/workflows/repair-noema-parallel-context.yml + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add \ + scripts/ci/noema_review_gate.py \ + tests/test_noema_parallel_changed_file_context.py \ + .github/workflows/repair-noema-parallel-context.yml + git diff --cached --check + git commit -m "perf(noema): bound parallel changed-file context" + git push origin HEAD:bolt-noema-review-api-batching-3009201169551405394 diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 9317860e4..ff01bf373 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -5,6 +5,7 @@ import argparse import base64 +import concurrent.futures import ipaddress import json import os @@ -342,17 +343,31 @@ def changed_file_context(repo: str, number: int, head_sha: str) -> str: if not paths: return "Changed file context unavailable: PR reported no changed files." sections: list[str] = [] - for path in paths[:MAX_CONTEXT_FILES]: + + target_paths = paths[:MAX_CONTEXT_FILES] + if not target_paths: + return "Changed file context unavailable: no paths to check." + + def _fetch_file_content(path: str) -> tuple[str, str | None, str | None]: try: content = fetch_head_file_content(repo, path, head_sha) + return path, content, None except RuntimeError as exc: - reason = scrub_sensitive_data(str(exc)) or "unknown error" - sections.append(f"### {path}\nUnavailable from head content API: {reason}") + return path, None, scrub_sensitive_data(str(exc)) or "unknown error" + + max_workers = min(10, len(target_paths)) + with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: + results = list(executor.map(_fetch_file_content, target_paths)) + + for path, content, error in results: + if error: + sections.append(f"### {path}\nUnavailable from head content API: {error}") continue if not content: sections.append(f"### {path}\nNo UTF-8 text content available from head content API.") continue sections.append(f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}") + if len(paths) > MAX_CONTEXT_FILES: sections.append(f"[{len(paths) - MAX_CONTEXT_FILES} changed files omitted from context budget]") return "\n\n".join(sections) diff --git a/tests/test_noema_parallel_changed_file_context.py b/tests/test_noema_parallel_changed_file_context.py new file mode 100644 index 000000000..ae839bfda --- /dev/null +++ b/tests/test_noema_parallel_changed_file_context.py @@ -0,0 +1,67 @@ +"""Concurrency regressions for Noema changed-file evidence collection.""" + +from __future__ import annotations + +import threading +import time + +from scripts.ci import noema_review_gate as noema + + +def test_changed_file_context_fetches_concurrently_and_preserves_order(monkeypatch) -> None: + """Independent reads run concurrently and keep success/error/empty order.""" + + paths = ["src/slow.py", "src/fast.py", "src/empty.py", "src/error.py"] + rendezvous = threading.Barrier(len(paths), timeout=2) + + monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths) + + def fetch_content(repo: str, path: str, head_sha: str) -> str: + rendezvous.wait() + if path == "src/slow.py": + time.sleep(0.05) + return "slow content" + if path == "src/fast.py": + return "fast content" + if path == "src/empty.py": + return "" + raise RuntimeError("Authorization: Bearer should-not-leak") + + monkeypatch.setattr(noema, "fetch_head_file_content", fetch_content) + + context = noema.changed_file_context("owner/repo", 7, "a" * 40) + + headings = [f"### {path}" for path in paths] + assert [context.index(heading) for heading in headings] == sorted( + context.index(heading) for heading in headings + ) + assert "slow content" in context + assert "fast content" in context + assert "No UTF-8 text content available from head content API." in context + assert "Authorization: Bearer ***" in context + assert "should-not-leak" not in context + + +def test_changed_file_context_caps_parallel_workers(monkeypatch) -> None: + """The API fan-out remains bounded even when the context budget is full.""" + + paths = [f"src/file_{index}.py" for index in range(noema.MAX_CONTEXT_FILES)] + observed_workers: list[int] = [] + real_executor = noema.concurrent.futures.ThreadPoolExecutor + + def bounded_executor(*, max_workers: int): + observed_workers.append(max_workers) + return real_executor(max_workers=max_workers) + + monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths) + monkeypatch.setattr( + noema, + "fetch_head_file_content", + lambda repo, path, head_sha: f"content for {path}", + ) + monkeypatch.setattr(noema.concurrent.futures, "ThreadPoolExecutor", bounded_executor) + + context = noema.changed_file_context("owner/repo", 7, "b" * 40) + + assert observed_workers == [noema.MAX_CONTEXT_FETCH_WORKERS] + assert context.count("### src/file_") == noema.MAX_CONTEXT_FILES