From ce28f2eb0f97ed891206e0b433e074fad81c0246 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:01:57 +0000 Subject: [PATCH 1/5] =?UTF-8?q?=E2=9A=A1=20Bolt:=20[=EC=84=B1=EB=8A=A5=20?= =?UTF-8?q?=EA=B0=9C=EC=84=A0]=20Noema=20PR=20=EB=A6=AC=EB=B7=B0=20?= =?UTF-8?q?=EC=8B=9C=20N+1=20API=20=EB=B3=91=EB=AA=A9=20=ED=98=84=EC=83=81?= =?UTF-8?q?=20=EC=A0=9C=EA=B1=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `changed_file_context`에서 변경된 파일들의 내용을 가져올 때 `ThreadPoolExecutor`를 사용하여 병렬로 요청을 보냅니다. --- .jules/bolt.md | 6 ++++++ scripts/ci/noema_review_gate.py | 15 +++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index a86b7aaf..b7f55cc7 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -43,3 +43,9 @@ ## 2026-07-09 - Avoid N+1 API blocking in SBOM aggregator **Learning:** The `collect_inventories` function in `scripts/ci/sbom_inventory_aggregator.py` was fetching SBOMs from the GitHub dependency graph synchronously for every repository in the organization. For large organizations (up to 500 repos), this N+1 network/CLI bottleneck significantly stalled the aggregation workflow. **Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch SBOMs concurrently when multiple repositories are provided, bounded by a `max_workers` limit (e.g., 10) to avoid overwhelming the CLI/API, while preserving the fast serial path for single-item inputs. +## 2024-11-24 - Avoid N+1 API blocking in LLM review context gathering +**Learning:** The `changed_file_context` function in `scripts/ci/noema_review_gate.py` was fetching changed file contents sequentially using synchronous GitHub API calls (via `fetch_head_file_content`). This caused N+1 network request bottlenecks proportional to the number of files (up to `MAX_CONTEXT_FILES`), significantly increasing the execution time. +**Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch file contents concurrently when building bounded context from external APIs in PR gates, preserving the order using `executor.map`. +## 2024-11-24 - LLM 리뷰 컨텍스트 수집 시 N+1 API 차단 방지 +**Learning:** `scripts/ci/noema_review_gate.py`의 `changed_file_context` 함수는 `fetch_head_file_content`를 통해 동기식 GitHub API 호출을 사용하여 순차적으로 변경된 파일의 내용을 가져왔습니다. 이는 파일 수(최대 `MAX_CONTEXT_FILES`)에 비례하여 N+1 네트워크 요청 병목 현상을 일으켜 전체 실행 시간을 크게 증가시켰습니다. +**Action:** PR 게이트의 외부 API에서 바운디드 컨텍스트를 구성할 때 `concurrent.futures.ThreadPoolExecutor`를 사용하여 파일 내용을 동시에 가져오고, `executor.map`을 사용하여 순서를 유지하도록 최적화해야 합니다. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5b024c84..5fb6fbb9 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -3,6 +3,7 @@ from __future__ import annotations +import concurrent.futures import argparse import base64 import ipaddress @@ -338,17 +339,19 @@ 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]: + def _fetch(path: str) -> str: try: content = fetch_head_file_content(repo, path, head_sha) except RuntimeError as exc: reason = scrub_sensitive_data(str(exc)) or "unknown error" - sections.append(f"### {path}\nUnavailable from head content API: {reason}") - continue + return f"### {path}\nUnavailable from head content API: {reason}" 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)}") + return f"### {path}\nNo UTF-8 text content available from head content API." + return f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}" + + with concurrent.futures.ThreadPoolExecutor(max_workers=MAX_CONTEXT_FILES) as executor: + for result in executor.map(_fetch, paths[:MAX_CONTEXT_FILES]): + sections.append(result) 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) From 381f46ea19c57c42fb0ee9d8c6d5fb8a0c963f3e Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:18:13 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL/HIGH]=20Fix=20pyasn1=20denial=20of=20service=20vulnerability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 💡 What: Update pyasn1 to 0.6.4 in requirements-strix-ci-hashes.txt to patch CVE-2026-59885 and CVE-2026-59886. 🎯 Why: Vulnerable dependencies fail pip-audit checks in CI. 📊 Impact: Prevents Denial-of-Service when processing attacker-controlled payload arcs/exponents. 🔬 Measurement: pip-audit completes without errors. ⚡ Bolt: [성능 개선] Noema PR 리뷰 시 N+1 API 병목 현상 제거 - `changed_file_context`에서 변경된 파일들의 내용을 가져올 때 `ThreadPoolExecutor`를 사용하여 병렬로 요청을 보냅니다. --- .jules/sentinel.md | 4 ++++ requirements-strix-ci-hashes.txt | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index be2dfa4b..accbe6b1 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -35,3 +35,7 @@ **Vulnerability:** Command Injection **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. +## 2024-11-24 - Update pyasn1 to patch denial-of-service vulnerabilities +**Vulnerability:** The CI dependencies (specifically `requirements-strix-ci-hashes.txt`) pinned `pyasn1` to version `0.6.3`, which contained two quadratic/exponential parsing vulnerabilities (CVE-2026-59885, CVE-2026-59886) that can lead to Denial-of-Service when processing attacker-controlled payload arcs/exponents. +**Learning:** Hard-pinned dependencies in CI scripts are prone to silently rotting and accumulating severe vulnerabilities, surfacing only when an explicit dependency audit runs and blocks the pipeline. +**Prevention:** Regularly run `pip-audit` to detect CVEs proactively before CI enforces an abrupt block. When `pip-audit` catches a vulnerability, ensure both the version and the expected SHA256 hashes are simultaneously updated. diff --git a/requirements-strix-ci-hashes.txt b/requirements-strix-ci-hashes.txt index 4cf45203..1aa159c1 100644 --- a/requirements-strix-ci-hashes.txt +++ b/requirements-strix-ci-hashes.txt @@ -1501,9 +1501,9 @@ protobuf==6.33.6 \ # grpc-google-iam-v1 # grpcio-status # proto-plus -pyasn1==0.6.3 \ - --hash=sha256:697a8ecd6d98891189184ca1fa05d1bb00e2f84b5977c481452050549c8a72cf \ - --hash=sha256:a80184d120f0864a52a073acc6fc642847d0be408e7c7252f31390c0f4eadcde +pyasn1==0.6.4 \ + --hash=sha256:deda9277cfd454080ec40b207fb6df82206a3a2688735233cdcd8d3d565f088b \ + --hash=sha256:9c447d8431c947fe4c8febc4ed9e760bc29011a5b01e5c74b67025bd9fb8ce81 # via pyasn1-modules pyasn1-modules==0.4.2 \ --hash=sha256:29253a9207ce32b64c3ac6600edc75368f98473906e8fd1043bd6b5b1de2c14a \ From 27fd575621276ec84eac74f8abe0a22cb2956a69 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 23 Jul 2026 00:08:00 +0900 Subject: [PATCH 3/5] fix(noema): keep single-file context serial --- .jules/bolt.md | 4 ++-- .jules/sentinel.md | 4 ++-- scripts/ci/noema_review_gate.py | 14 ++++++++++---- tests/test_noema_review_gate.py | 27 +++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index b7f55cc7..ca26add1 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -43,9 +43,9 @@ ## 2026-07-09 - Avoid N+1 API blocking in SBOM aggregator **Learning:** The `collect_inventories` function in `scripts/ci/sbom_inventory_aggregator.py` was fetching SBOMs from the GitHub dependency graph synchronously for every repository in the organization. For large organizations (up to 500 repos), this N+1 network/CLI bottleneck significantly stalled the aggregation workflow. **Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch SBOMs concurrently when multiple repositories are provided, bounded by a `max_workers` limit (e.g., 10) to avoid overwhelming the CLI/API, while preserving the fast serial path for single-item inputs. -## 2024-11-24 - Avoid N+1 API blocking in LLM review context gathering +## 2026-07-22 - Avoid N+1 API blocking in LLM review context gathering **Learning:** The `changed_file_context` function in `scripts/ci/noema_review_gate.py` was fetching changed file contents sequentially using synchronous GitHub API calls (via `fetch_head_file_content`). This caused N+1 network request bottlenecks proportional to the number of files (up to `MAX_CONTEXT_FILES`), significantly increasing the execution time. **Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch file contents concurrently when building bounded context from external APIs in PR gates, preserving the order using `executor.map`. -## 2024-11-24 - LLM 리뷰 컨텍스트 수집 시 N+1 API 차단 방지 +## 2026-07-22 - LLM 리뷰 컨텍스트 수집 시 N+1 API 차단 방지 **Learning:** `scripts/ci/noema_review_gate.py`의 `changed_file_context` 함수는 `fetch_head_file_content`를 통해 동기식 GitHub API 호출을 사용하여 순차적으로 변경된 파일의 내용을 가져왔습니다. 이는 파일 수(최대 `MAX_CONTEXT_FILES`)에 비례하여 N+1 네트워크 요청 병목 현상을 일으켜 전체 실행 시간을 크게 증가시켰습니다. **Action:** PR 게이트의 외부 API에서 바운디드 컨텍스트를 구성할 때 `concurrent.futures.ThreadPoolExecutor`를 사용하여 파일 내용을 동시에 가져오고, `executor.map`을 사용하여 순서를 유지하도록 최적화해야 합니다. diff --git a/.jules/sentinel.md b/.jules/sentinel.md index accbe6b1..a23c55ad 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -35,7 +35,7 @@ **Vulnerability:** Command Injection **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. -## 2024-11-24 - Update pyasn1 to patch denial-of-service vulnerabilities -**Vulnerability:** The CI dependencies (specifically `requirements-strix-ci-hashes.txt`) pinned `pyasn1` to version `0.6.3`, which contained two quadratic/exponential parsing vulnerabilities (CVE-2026-59885, CVE-2026-59886) that can lead to Denial-of-Service when processing attacker-controlled payload arcs/exponents. +## 2026-07-22 - Update pyasn1 to patch denial-of-service vulnerabilities +**Vulnerability:** The CI dependencies (specifically `requirements-strix-ci-hashes.txt`) pinned `pyasn1` to version `0.6.3`, which contained three resource-exhaustion and parser vulnerabilities (CVE-2026-59884, CVE-2026-59885, CVE-2026-59886) that can lead to Denial-of-Service when processing attacker-controlled ASN.1 values. **Learning:** Hard-pinned dependencies in CI scripts are prone to silently rotting and accumulating severe vulnerabilities, surfacing only when an explicit dependency audit runs and blocks the pipeline. **Prevention:** Regularly run `pip-audit` to detect CVEs proactively before CI enforces an abrupt block. When `pip-audit` catches a vulnerability, ensure both the version and the expected SHA256 hashes are simultaneously updated. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5fb6fbb9..b36e0833 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -3,9 +3,9 @@ from __future__ import annotations -import concurrent.futures import argparse import base64 +import concurrent.futures import ipaddress import json import os @@ -349,9 +349,15 @@ def _fetch(path: str) -> str: return f"### {path}\nNo UTF-8 text content available from head content API." return f"### {path}\n{truncate_text(content, MAX_FILE_CONTEXT_CHARS)}" - with concurrent.futures.ThreadPoolExecutor(max_workers=MAX_CONTEXT_FILES) as executor: - for result in executor.map(_fetch, paths[:MAX_CONTEXT_FILES]): - sections.append(result) + bounded_paths = paths[:MAX_CONTEXT_FILES] + if len(bounded_paths) == 1: + sections.append(_fetch(bounded_paths[0])) + else: + with concurrent.futures.ThreadPoolExecutor( + max_workers=len(bounded_paths) + ) as executor: + for result in executor.map(_fetch, bounded_paths): + sections.append(result) 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_review_gate.py b/tests/test_noema_review_gate.py index 75d40dbe..ca2736a3 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -296,6 +296,33 @@ def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, assert "1 changed files omitted from context budget" in context +def test_changed_file_context_uses_serial_path_for_one_file(monkeypatch): + """One changed file must not pay the cost of creating a worker pool.""" + calls = [] + monkeypatch.setattr( + noema, + "fetch_changed_file_paths", + lambda repo, number: ["src/only.py"], + ) + monkeypatch.setattr( + noema, + "fetch_head_file_content", + lambda repo, path, head_sha: calls.append((repo, path, head_sha)) + or "print('only')\n", + ) + monkeypatch.setattr( + noema.concurrent.futures, + "ThreadPoolExecutor", + lambda *args, **kwargs: pytest.fail("single-file context created a worker pool"), + ) + + context = noema.changed_file_context("owner/repo", 7, "head") + + assert "### src/only.py" in context + assert "print('only')" in context + assert calls == [("owner/repo", "src/only.py", "head")] + + class FakeResponse: """Small context-manager response for urllib monkeypatches.""" From 68fb94e74c22af2218526a45739b7dcb2db1a41a Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:21:19 +0000 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL/HIGH]=20Fix=20pyasn1=20denial=20of=20service=20vulnerability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 💡 What: Update pyasn1 to 0.6.4 in requirements-strix-ci-hashes.txt to patch CVE-2026-59884, CVE-2026-59885 and CVE-2026-59886. 🎯 Why: Vulnerable dependencies fail pip-audit checks in CI. 📊 Impact: Prevents Denial-of-Service when processing attacker-controlled payload arcs/exponents. 🔬 Measurement: pip-audit completes without errors. ⚡ Bolt: [성능 개선] Noema PR 리뷰 시 N+1 API 병목 현상 제거 - `changed_file_context`에서 변경된 파일들의 내용을 가져올 때 파일 개수에 따라 동기/비동기 방식을 선택적으로 사용합니다. --- .jules/sentinel.md | 2 +- scripts/ci/noema_review_gate.py | 5 ++- tests/test_noema_review_gate.py | 56 +++++++++++++++++---------------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a23c55ad..09d60be2 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -36,6 +36,6 @@ **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. ## 2026-07-22 - Update pyasn1 to patch denial-of-service vulnerabilities -**Vulnerability:** The CI dependencies (specifically `requirements-strix-ci-hashes.txt`) pinned `pyasn1` to version `0.6.3`, which contained three resource-exhaustion and parser vulnerabilities (CVE-2026-59884, CVE-2026-59885, CVE-2026-59886) that can lead to Denial-of-Service when processing attacker-controlled ASN.1 values. +**Vulnerability:** The CI dependencies (specifically `requirements-strix-ci-hashes.txt`) pinned `pyasn1` to version `0.6.3`, which contained three quadratic/exponential parsing vulnerabilities (CVE-2026-59884, CVE-2026-59885, CVE-2026-59886) that can lead to Denial-of-Service when processing attacker-controlled payload arcs/exponents. **Learning:** Hard-pinned dependencies in CI scripts are prone to silently rotting and accumulating severe vulnerabilities, surfacing only when an explicit dependency audit runs and blocks the pipeline. **Prevention:** Regularly run `pip-audit` to detect CVEs proactively before CI enforces an abrupt block. When `pip-audit` catches a vulnerability, ensure both the version and the expected SHA256 hashes are simultaneously updated. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index b36e0833..90b23436 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -353,9 +353,8 @@ def _fetch(path: str) -> str: if len(bounded_paths) == 1: sections.append(_fetch(bounded_paths[0])) else: - with concurrent.futures.ThreadPoolExecutor( - max_workers=len(bounded_paths) - ) as executor: + max_workers = min(MAX_CONTEXT_FILES, len(bounded_paths)) + with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: for result in executor.map(_fetch, bounded_paths): sections.append(result) if len(paths) > MAX_CONTEXT_FILES: diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index ca2736a3..fd260025 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -296,33 +296,6 @@ def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, assert "1 changed files omitted from context budget" in context -def test_changed_file_context_uses_serial_path_for_one_file(monkeypatch): - """One changed file must not pay the cost of creating a worker pool.""" - calls = [] - monkeypatch.setattr( - noema, - "fetch_changed_file_paths", - lambda repo, number: ["src/only.py"], - ) - monkeypatch.setattr( - noema, - "fetch_head_file_content", - lambda repo, path, head_sha: calls.append((repo, path, head_sha)) - or "print('only')\n", - ) - monkeypatch.setattr( - noema.concurrent.futures, - "ThreadPoolExecutor", - lambda *args, **kwargs: pytest.fail("single-file context created a worker pool"), - ) - - context = noema.changed_file_context("owner/repo", 7, "head") - - assert "### src/only.py" in context - assert "print('only')" in context - assert calls == [("owner/repo", "src/only.py", "head")] - - class FakeResponse: """Small context-manager response for urllib monkeypatches.""" @@ -569,3 +542,32 @@ def test_parse_args_and_main(monkeypatch): with pytest.raises(SystemExit, match="--pr-number must be positive"): noema.main(["--repo", "owner/repo", "--pr-number", "0"]) +import pytest +from scripts.ci import noema_review_gate as noema +import concurrent.futures + +def test_changed_file_context_single_file_executor(monkeypatch): + """Ensure executor is not created for single file contexts to save overhead.""" + # Mock to return exactly 1 file + monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: ["single.txt"]) + + # Mock content fetch + monkeypatch.setattr(noema, "fetch_head_file_content", lambda repo, path, sha: "content") + + # Track executor creation + original_executor = concurrent.futures.ThreadPoolExecutor + created = False + + class FakeExecutor(original_executor): + def __init__(self, *args, **kwargs): + nonlocal created + created = True + super().__init__(*args, **kwargs) + + monkeypatch.setattr(concurrent.futures, "ThreadPoolExecutor", FakeExecutor) + + result = noema.changed_file_context("owner/repo", 1, "sha") + + assert "### single.txt" in result + assert "content" in result + assert not created, "ThreadPoolExecutor should not be created for a single file" From f004f522c5dc399b34ea2bcc53d2dd21ef6724a6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 23 Jul 2026 00:52:02 +0900 Subject: [PATCH 5/5] fix(noema): bound changed-file fetch workers --- scripts/ci/noema_review_gate.py | 3 +- tests/test_noema_review_gate.py | 82 ++++++++++++++++++++++++--------- 2 files changed, 62 insertions(+), 23 deletions(-) diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 90b23436..f896bf9f 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -39,6 +39,7 @@ RUNNING_STATES = {"QUEUED", "IN_PROGRESS", "PENDING", "REQUESTED", "WAITING", "EXPECTED"} MAX_DIFF_CHARS = 60000 MAX_CONTEXT_FILES = 12 +MAX_CONTEXT_WORKERS = 6 MAX_FILE_CONTEXT_CHARS = 4000 MAX_REVIEW_CONTEXT_CHARS = 24000 MAX_THREAD_BODY_CHARS = 1200 @@ -353,7 +354,7 @@ def _fetch(path: str) -> str: if len(bounded_paths) == 1: sections.append(_fetch(bounded_paths[0])) else: - max_workers = min(MAX_CONTEXT_FILES, len(bounded_paths)) + max_workers = min(MAX_CONTEXT_WORKERS, len(bounded_paths)) with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: for result in executor.map(_fetch, bounded_paths): sections.append(result) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index fd260025..08dfaa88 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -542,32 +542,70 @@ def test_parse_args_and_main(monkeypatch): with pytest.raises(SystemExit, match="--pr-number must be positive"): noema.main(["--repo", "owner/repo", "--pr-number", "0"]) -import pytest -from scripts.ci import noema_review_gate as noema -import concurrent.futures -def test_changed_file_context_single_file_executor(monkeypatch): - """Ensure executor is not created for single file contexts to save overhead.""" - # Mock to return exactly 1 file - monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: ["single.txt"]) - # Mock content fetch - monkeypatch.setattr(noema, "fetch_head_file_content", lambda repo, path, sha: "content") +def test_changed_file_context_uses_serial_path_for_one_file(monkeypatch): + """One changed file must not pay the cost of creating a worker pool.""" + calls = [] + monkeypatch.setattr( + noema, + "fetch_changed_file_paths", + lambda repo, number: ["src/only.py"], + ) + monkeypatch.setattr( + noema, + "fetch_head_file_content", + lambda repo, path, head_sha: calls.append((repo, path, head_sha)) + or "print('only')\n", + ) + monkeypatch.setattr( + noema.concurrent.futures, + "ThreadPoolExecutor", + lambda *args, **kwargs: pytest.fail("single-file context created a worker pool"), + ) + + context = noema.changed_file_context("owner/repo", 7, "head") + + assert "### src/only.py" in context + assert "print('only')" in context + assert calls == [("owner/repo", "src/only.py", "head")] - # Track executor creation - original_executor = concurrent.futures.ThreadPoolExecutor - created = False - class FakeExecutor(original_executor): - def __init__(self, *args, **kwargs): - nonlocal created - created = True - super().__init__(*args, **kwargs) +def test_changed_file_context_caps_workers_and_preserves_path_order(monkeypatch): + """Multiple files use a bounded pool whose map output keeps path order.""" + paths = [f"src/file-{index}.py" for index in range(noema.MAX_CONTEXT_WORKERS + 2)] + executor_calls = [] + original_executor = noema.concurrent.futures.ThreadPoolExecutor - monkeypatch.setattr(concurrent.futures, "ThreadPoolExecutor", FakeExecutor) + class RecordingExecutor: + def __init__(self, *, max_workers): + executor_calls.append(max_workers) + self._executor = original_executor(max_workers=max_workers) - result = noema.changed_file_context("owner/repo", 1, "sha") + def __enter__(self): + self._executor.__enter__() + return self + + def __exit__(self, *args): + return self._executor.__exit__(*args) + + def map(self, function, iterable): + return self._executor.map(function, iterable) + + 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", RecordingExecutor + ) + + context = noema.changed_file_context("owner/repo", 7, "head") + heading_positions = [context.index(f"### {path}") for path in paths] - assert "### single.txt" in result - assert "content" in result - assert not created, "ThreadPoolExecutor should not be created for a single file" + assert executor_calls == [noema.MAX_CONTEXT_WORKERS] + assert heading_positions == sorted(heading_positions)