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 @@ -47,3 +47,6 @@
## 2026-08-09 - [대용량 로그 스캔 시 정규표현식 실행 전 O(N) 서브스트링 검증 선행]
**Learning:** `classify_testthat_failure`에서 테스트 실패 내역이 없는 2MB 로그 파일을 대상으로 정규표현식을 실행하면 약 20ms가 소요되지만, 단순 문자열 검색은 약 1ms만 소요됩니다. 문자열 존재 여부가 정규표현식 매칭의 전제 조건일 때, 콜드 패스(Cold Path)에서 순서 최적화는 매우 큰 성능 차이를 만듭니다.
**Action:** 대용량 텍스트 입력(CI 로그 등)에서 복잡한 정규표현식을 파싱하기 전에 항상 빠른 O(N) 문자열 존재 여부 확인을 먼저 수행하십시오.
## 2026-08-23 - Concurrent Github API calls in CI Scripts
**Learning:** Performing multiple independent `gh_api_json` REST API calls (like fetching reviews, check runs, and files for a pull request) sequentially inside loop iterations (e.g., `rest_pr_node` inside `fetch_open_prs`) causes severe I/O wait latency that stacks linearly.
**Action:** Always fetch independent network payloads concurrently using `concurrent.futures.ThreadPoolExecutor` when performing API queries, avoiding sequential network operations in synchronous contexts.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,7 @@ Semantic Versioning where the repository publishes a release.
- Added an organization-owned reusable exact-artifact SBOM attestation boundary that validates inert six-file wheel/sdist evidence, binds CycloneDX 1.7 predicates to exact SHA-256 subjects, signs through least-privilege GitHub artifact attestations, and exports online and offline verification bundles.
- Hardened exact-artifact SBOM verification with strict finite RFC 8259 JSON, integer CycloneDX document versions, deterministic UUIDv5 subject identities, exact filename properties and single SHA-256 root bindings, environment-only shell input transfer, pinned Ubuntu 24.04 quality runners, and checksum-sealed beginner-readable offline evidence. The decision record now cites Bray (2017) so NaN and Infinity cannot be treated as sealed SBOM numbers.
- Recorded the org control-plane architecture, including exact-artifact SBOM attestation, so agents reconstruct the signing trust boundary from the repo instead of private memory.

## 2026-08-23
### 변경 사항
- **성능 개선**: `scripts/ci/pr_review_merge_scheduler.py`의 `rest_pr_node` 함수에서 `reviews`, `checks`, `files`를 순차적으로 조회하던 것을 `concurrent.futures.ThreadPoolExecutor`를 사용하여 병렬로 가져오도록 수정했습니다. 이로 인해 I/O 대기 시간이 크게 단축됩니다.
15 changes: 12 additions & 3 deletions scripts/ci/pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,9 +797,18 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]:
head = pr.get("head") or {}
base = pr.get("base") or {}
head_repo = head.get("repo") or {}
reviews = gh_api_json(f"repos/{repo}/pulls/{number}/reviews?per_page=100")
checks = gh_api_json(f"repos/{repo}/commits/{head.get('sha')}/check-runs?per_page=100")
files = gh_api_json(f"repos/{repo}/pulls/{number}/files?per_page=20")

# ⚡ Bolt: Fetch independent PR details concurrently to prevent N+1 API bottlenecks.
# This reduces total I/O wait latency from ~3 network hops to 1 hop.
with concurrent.futures.ThreadPoolExecutor(max_workers=3) as executor:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep REST requests within the existing concurrency bound

When the GraphQL fallback processes multiple PRs—as allowed by the 100-PR default in .github/workflows/pr-review-merge-scheduler.yml—this executor runs inside the 10-worker executor in fetch_open_prs_rest, allowing 30 simultaneous gh api subprocesses rather than the intended bound of 10. GitHub's REST API best practices say to make requests serially to avoid secondary rate limits, and gh_api_json has no retry, so one throttled request aborts the whole queue scan. Use one globally bounded executor or serialize these per-PR detail requests instead of nesting pools.

Useful? React with 👍 / 👎.

future_reviews = executor.submit(gh_api_json, f"repos/{repo}/pulls/{number}/reviews?per_page=100")
future_checks = executor.submit(gh_api_json, f"repos/{repo}/commits/{head.get('sha')}/check-runs?per_page=100")
future_files = executor.submit(gh_api_json, f"repos/{repo}/pulls/{number}/files?per_page=20")
Comment on lines +804 to +806

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make the REST fallback test insensitive to completion order

These tasks may invoke gh_api_json in any order, but test_rest_pr_fallback_shapes_reviews_and_checks still asserts that the shared calls list exactly equals submission order. The isolated test is now flaky—I reproduced a failure where the files request was recorded before the check-runs request—so an otherwise correct required test run can intermittently block changes. Update the contract to compare the three calls without ordering, or add synchronization if invocation order is intended to remain part of the behavior.

Useful? React with 👍 / 👎.


reviews = future_reviews.result()
checks = future_checks.result()
files = future_files.result()
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines +803 to +810

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Nested pools multiply concurrent gh subprocesses

rest_pr_node runs concurrently from fetch_open_prs_rest (up to 10 workers at scripts/ci/pr_review_merge_scheduler.py:857-859), and each call now opens its own 3-worker pool. Up to 30 concurrent gh` subprocesses can run at once, versus 10 before. Bounded and likely acceptable, but the extra process/memory pressure on constrained runners is worth confirming.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +803 to +810

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Exception and cleanup behavior preserved

If any of the three futures raises, .result() re-raises and the executor's shutdown(wait=True) waits for siblings before propagating, matching the old sequential error contract with no thread leak. The test's switch to sorted(calls) correctly reflects the now non-deterministic call order.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


rest_merge_state = REST_MERGEABLE_STATE_MAP.get(
str(pr.get("mergeable_state") or "").lower(),
str(pr.get("mergeable_state") or "").upper(),
Expand Down
4 changes: 2 additions & 2 deletions tests/test_pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,11 @@ def fake_api(path):
},
)

assert calls == [
assert sorted(calls) == sorted([
"repos/owner/repo/pulls/42/reviews?per_page=100",
"repos/owner/repo/commits/abc123/check-runs?per_page=100",
"repos/owner/repo/pulls/42/files?per_page=20",
]
])
assert node["number"] == 42
assert node["mergeStateStatus"] == "CLEAN"
assert node["restMergeableState"] == "CLEAN"
Expand Down
Loading