⚡ Bolt: JSON 추출 최적화 및 버그 수정 (raw_decode 적용) - #1237
Conversation
- `scripts/ci/noema_review_gate.py` 내 `extract_json_object` 함수에서 기존 `rfind`와 슬라이싱 방식을 `json.JSONDecoder().raw_decode`로 변경 - 문자열 복사를 줄여 성능 최적화(O(N) 복사 방지) - 응답 후행에 포함된 괄호나 가비지 텍스트로 인한 파싱 오류 수정 - 100% 테스트 커버리지 및 한국어 주석 반영
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reached
Next review available in: 36 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesJSON 추출 흐름
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The parser now stops at the end of the first JSON object and ignores trailing text, reducing false CI failures and unnecessary string copying. Merge risk is low, with explicit owner follow-up to cover non-object JSON rejection and optionally remove the redundant parse on trailing-text inputs. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- 중복된 `json.loads` 빠른 경로(fast path)와 접근 불가능한 타입 검사(isinstance) 제거 - 단일 객체 외의 유효한 JSON(예: 배열) 입력 시 에러 처리 테스트 보강 - 100% 테스트 커버리지 유지 및 코드 복잡도 감소
This reverts commit cdb617a.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- 중복된 `json.loads` 빠른 경로(fast path)와 접근 불가능한 타입 검사(isinstance) 제거 - `JSONDecoder().raw_decode` 단일 실행 경로로 일원화 - 단일 객체 외 유효한 JSON(예: 배열) 입력 시 에러 처리 테스트 보강 - 100% 테스트 커버리지 유지 및 코드 복잡도 감소
This reverts commit 58f3d70.
- 이전 제출 사항을 그대로 유지 (fail-closed 원칙 보존) - 불필요한 수정 없음
This reverts commit 1771507.
- `scripts/ci/strix_quick_gate.sh` 내 `child_model_for_api_base` 함수가 `openai-direct/*` 모델 형식을 파싱하도록 수정 - JSONDecoder 파싱 최적화 시도는 보안/신뢰 경계 확장 문제로 인해 완전 롤백 및 교훈 기록 (`.jules/bolt.md`)
|
Closing this contaminated predecessor without merge. Exact head |
| for issue in list_recent_pull_requests( | ||
| target_client, | ||
| organization=organization, | ||
| repository_source=repository_source, | ||
| since=since, | ||
| on_error=record_failure, | ||
| rotation_offset=rotation_offset, | ||
| ): |
There was a problem hiding this comment.
🟡 Sweep cycle crashes when the repository listing fails
The for issue in list_recent_pull_requests(...) loop in sweep no longer sits inside a try/except. The organization repository listing runs on the generator's first advance with no isolation of its own, so when it exhausts its rate-limit retries the exception escapes sweep and main, aborting the whole cycle instead of recording one isolated failure and exiting cleanly.
Was this helpful? React with 👍 or 👎 to provide feedback.
| # Neutral skip only when ALL hold: a backend-unavailability signal is | ||
| # present and no vulnerability was reported in the relevant scope. | ||
| # This preserves real security gating while keeping uncontrollable | ||
| # provider outages from blocking current-head merge progress. | ||
| if grep -Eiq "$backend_unavailable_signal" "$strix_neutralization_scope_log" \ | ||
| && ! grep -Eiq "$reported_vulnerability_signal" "$strix_neutralization_scope_log"; then | ||
| echo "::error title=STRIX_PROVIDER_UNAVAILABLE::Strix could not complete authoritative vulnerability analysis because its provider/backend was unavailable (rate limit, token cap, connection, warm-up, or model-behavior failure). See the strix-reports artifact and run log." | ||
| exit "$strix_rc" | ||
| echo "::warning title=Strix backend unavailable::Strix could not complete because its LLM backend was unavailable (rate limit / token cap / connection or warm-up failure) before producing a vulnerability report. Treating as a neutral skip so an infrastructure outage does not block merges; genuine findings still fail the check. See the strix-reports artifact and the run log." | ||
| exit 0 |
There was a problem hiding this comment.
🔍 Strix provider outage now passes the required check
The neutral-skip branch changes from ::error title=STRIX_PROVIDER_UNAVAILABLE:: with exit "$strix_rc" to ::warning:: with exit 0, and drops the model_behavior_error_signal term. A backend outage with no vulnerability signal now passes the required check rather than failing closed on an incomplete scan. Tests were updated to match. Confirm this relaxation of the fail-closed gate is intended.
Was this helpful? React with 👍 or 👎 to provide feedback.
| raise GitHubError( | ||
| f"GitHub API {safe_method} {safe_path} failed: {bounded}" | ||
| f"GitHub API {normalized_method} {safe_path} failed: {bounded}" | ||
| ) |
There was a problem hiding this comment.
📝 Info: Method redaction removed from API diagnostics
SAFE_DIAGNOSTIC_METHODS and the safe_method wrapper are removed, so request now interpolates the caller-supplied normalized_method verbatim into the GitHubError message. Current callers pass fixed methods, but the contract that arbitrary method text is redacted from diagnostics is gone and its regression test was deleted.
Was this helpful? React with 👍 or 👎 to provide feedback.
| command = [resolved_strix_bin, "-n", "-t", ".", "--scan-mode", scan_mode] | ||
|
|
||
| try: | ||
| process = subprocess.Popen( | ||
| command, | ||
| cwd=str(scan_working_dir), | ||
| cwd=str(target_cwd), |
There was a problem hiding this comment.
🔍 Strix child reverts to running inside the scan target
run_strix_once drops the separate scan working directory: the child now runs with cwd=target_cwd and -t ., so scanner-created strix_runs/state files land inside the tree being scanned again. The deleted strix-scan-working-boundary.md had specifically fixed this to avoid self-scanned findings. The revert is internally consistent, but confirm the self-scan regression is acceptable.
Was this helpful? React with 👍 or 👎 to provide feedback.
Understood. Acknowledging that this work is now obsolete and the PR is closed due to being a stale-tree replay. Stopping work on this task. |
1 participant
What
{/ last-}slice with one standard-libraryJSONDecoder.raw_decodecall.RuntimeErrorcontract.}, malformed objects, text without an object, and valid non-object JSON ([]).Why
The former last-
}boundary could absorb a brace from trailing prose and turn an otherwise complete first object into invalid JSON. Parsing exactly one object fixes that recovery bug without adding a dependency, a second parsing pass, or a normalized input copy.This PR intentionally makes no unmeasured performance claim. It also removes the generated
.jules/bolt.mdguidance because that guidance generalized beyond the evidence in this change.Verification at exact head
466641151ae3dc32b91d2677d644893d23d687051393 passed, 1 skipped, 16 subtests passed8346 statements,3212 branches,100%100%compileallandgit diff --check: passedPR created from Jules task 573387602062571879, then reconciled with exact-head review findings and current
main.