Skip to content
Closed
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
3 changes: 3 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,6 @@
## 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.
## 2026-07-25 - Avoid N+1 API blocking in Noema review context fetching
**Learning:** In `scripts/ci/noema_review_gate.py`, the `changed_file_context` function was fetching file contents from the GitHub API sequentially, causing an N+1 API bottleneck.
**Action:** Use `concurrent.futures.ThreadPoolExecutor` to fetch bounded file contents concurrently, bounded by an explicit conservative `max_workers` limit (no more than 6), while preserving the fast serial path for single-item inputs and ensuring deterministic output order.
23 changes: 17 additions & 6 deletions scripts/ci/noema_review_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import argparse
import base64
import concurrent.futures
import ipaddress
import json
import os
Expand Down Expand Up @@ -41,6 +42,7 @@
MAX_FILE_CONTEXT_CHARS = 4000
MAX_REVIEW_CONTEXT_CHARS = 24000
MAX_THREAD_BODY_CHARS = 1200
MAX_CONTEXT_WORKERS = 6

# ⚡ Bolt: Pre-compiled regex patterns to avoid recompilation on every scrub_sensitive_data call.
# Impact: Improves string processing performance in error reporting.
Expand Down Expand Up @@ -342,17 +344,26 @@ 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]

def process_path(path: str) -> str:
"""Fetch and truncate one changed file for the bounded review context."""
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)}"

if len(target_paths) <= 1:
sections.extend(process_path(path) for path in target_paths)
else:
max_workers = min(MAX_CONTEXT_WORKERS, len(target_paths))
with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor:
sections.extend(executor.map(process_path, target_paths))

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)
Expand Down
46 changes: 46 additions & 0 deletions tests/test_noema_review_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,52 @@ def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch,
assert "1 changed files omitted from context budget" in context


paths = ["src/file_only.py"]
monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths)
context = noema.changed_file_context("owner/repo", 7, "head")
assert "src/file_only.py" in context


def test_changed_file_context_concurrency_and_ordering(monkeypatch):
import time

paths = ["src/a.py", "src/b.py", "src/c.py", "src/empty.txt"]
monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths)

completion_order = []

def fake_fetch_head_file_content(repo, path, head_sha):
if path == "src/a.py":
completion_order.append(path)
raise RuntimeError("API error")
elif path == "src/b.py":
time.sleep(0.1)
completion_order.append(path)
return "b content"
elif path == "src/c.py":
completion_order.append(path)
return "c content"
elif path == "src/empty.txt":
completion_order.append(path)
return ""
return "content"

monkeypatch.setattr(noema, "fetch_head_file_content", fake_fetch_head_file_content)

context = noema.changed_file_context("owner/repo", 7, "head")

assert "### src/a.py\nUnavailable from head content API: API error" in context
assert "### src/b.py\nb content" in context
assert "### src/c.py\nc content" in context
assert "### src/empty.txt\nNo UTF-8 text content available" in context
Comment thread
seonghobae marked this conversation as resolved.

pos_a = context.find("### src/a.py")
pos_b = context.find("### src/b.py")
pos_c = context.find("### src/c.py")

assert pos_a < pos_b < pos_c, "Output order does not match input order"
Comment thread
seonghobae marked this conversation as resolved.


class FakeResponse:
"""Small context-manager response for urllib monkeypatches."""

Expand Down
101 changes: 101 additions & 0 deletions tests/test_noema_review_gate_concurrency_contract.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
"""Focused contracts for changed-file review context concurrency."""

from collections.abc import Callable, Iterable, Iterator
from typing import Any

from scripts.ci import noema_review_gate as noema


class RecordingExecutor:
"""Synchronous executor double that records worker and map contracts."""

instances: list["RecordingExecutor"] = []

def __init__(self, *, max_workers: int) -> None:
"""Record the configured worker limit."""
self.max_workers = max_workers
self.map_inputs: list[tuple[str, ...]] = []
self.instances.append(self)

def __enter__(self) -> "RecordingExecutor":
"""Return the executor double for context-manager use."""
return self

def __exit__(self, *args: object) -> bool:
"""Propagate exceptions raised by the code under test."""
return False

def map(
self,
function: Callable[[str], str],
values: Iterable[str],
) -> Iterator[str]:
"""Record ordered inputs and evaluate them synchronously."""
ordered_values = tuple(values)
self.map_inputs.append(ordered_values)
return map(function, ordered_values)


def install_recording_executor(monkeypatch: Any) -> None:
"""Install a fresh recording executor double."""
RecordingExecutor.instances.clear()
monkeypatch.setattr(noema.concurrent.futures, "ThreadPoolExecutor", RecordingExecutor)


def test_single_file_context_does_not_create_executor(monkeypatch: Any) -> None:
"""Keep the one-file fast path strictly serial."""
install_recording_executor(monkeypatch)
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: "only content")

context = noema.changed_file_context("owner/repo", 7, "head")

assert RecordingExecutor.instances == []
assert context == "### src/only.py\nonly content"


def test_parallel_file_context_uses_bounded_map_and_scrubs_errors(monkeypatch: Any) -> None:
"""Verify bounded parallel mapping, stable order, and error redaction."""
install_recording_executor(monkeypatch)
paths = [f"src/file_{index}.py" for index in range(noema.MAX_CONTEXT_WORKERS + 2)]
sensitive_value = "".join(("github_", "pat_", "123456789"))
monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths)

def fake_fetch_head_file_content(repo: str, path: str, head_sha: str) -> str:
"""Return deterministic content while exercising error and empty paths."""
if path == paths[0]:
raise RuntimeError(f"API error token {sensitive_value}")
if path == paths[-1]:
return ""
return f"content for {path}"

monkeypatch.setattr(noema, "fetch_head_file_content", fake_fetch_head_file_content)

context = noema.changed_file_context("owner/repo", 7, "head")

assert len(RecordingExecutor.instances) == 1
executor = RecordingExecutor.instances[0]
assert executor.max_workers == min(noema.MAX_CONTEXT_WORKERS, len(paths))
assert executor.max_workers <= noema.MAX_CONTEXT_WORKERS
assert executor.max_workers <= len(paths)
assert executor.map_inputs == [tuple(paths)]
assert "Unavailable from head content API: API error token ***" in context
assert sensitive_value not in context
assert "No UTF-8 text content available from head content API." in context

section_positions = [context.index(f"### {path}") for path in paths]
assert section_positions == sorted(section_positions)


def test_parallel_file_context_worker_count_tracks_small_batches(monkeypatch: Any) -> None:
"""Limit worker creation to the number of files in a small batch."""
install_recording_executor(monkeypatch)
paths = ["src/first.py", "src/second.py"]
monkeypatch.setattr(noema, "fetch_changed_file_paths", lambda repo, number: paths)
monkeypatch.setattr(noema, "fetch_head_file_content", lambda repo, path, head_sha: path)

noema.changed_file_context("owner/repo", 7, "head")

executor = RecordingExecutor.instances[0]
assert executor.max_workers == len(paths)
assert executor.map_inputs == [tuple(paths)]
Loading