Skip to content

⚡ Bolt: PDF 바이트 배열 검증의 콜백 오버헤드 제거 - #729

Closed
seonghobae wants to merge 15 commits into
developfrom
bolt/optimize-score-pdf-read-14940922715415907974
Closed

⚡ Bolt: PDF 바이트 배열 검증의 콜백 오버헤드 제거#729
seonghobae wants to merge 15 commits into
developfrom
bolt/optimize-score-pdf-read-14940922715415907974

Conversation

@seonghobae

@seonghobae seonghobae commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

What

readScorePdf가 일반 배열 응답을 검증할 때 Array.prototype.every() 대신 조기 종료 가능한 인덱스 루프를 사용합니다. 동시에 각 값이 0..255 범위의 정수인지 확인해 Uint8Array.from()의 암묵적 잘림·랩어라운드 변환을 차단합니다.

Why

대용량 PDF 바이트 응답에서 반복 콜백 호출 비용을 줄이고, 문자열·음수·범위 초과·소수·NaN·무한대가 유효한 바이트처럼 강제 변환되는 것을 방지하기 위함입니다.

Verification

  • 유효한 경계값 0255 변환 검증
  • 비정수 및 범위 밖 값 거부 검증
  • 첫 번째 오류에서 후속 배열 요소를 읽지 않는 조기 종료 회귀 검증
  • 기존 브리지 부재 fail-closed 검증 유지

@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6af52201-5df5-437b-97ad-1cb79fae3a03

📥 Commits

Reviewing files that changed from the base of the PR and between 2bfae9c and 74bb1fe.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • apps/desktop/src/features/score/scoreStorage.test.ts
  • apps/desktop/src/features/score/scoreStorage.ts
📝 Walkthrough

Walkthrough

readScorePdf의 배열 검증을 조기 종료하는 for 루프로 변경했습니다. 숫자 배열의 Uint8Array 변환과 비숫자 배열 요소의 오류 처리를 테스트로 검증했습니다.

Changes

점수 브리지 응답 검증

Layer / File(s) Summary
바이트 배열 검증 및 테스트
apps/desktop/src/features/score/scoreStorage.ts, apps/desktop/src/features/score/scoreStorage.test.ts, .jules/bolt.md
readScorePdffor 루프로 배열 요소를 검증하고, 숫자 배열만 Uint8Array로 변환합니다. 비숫자 요소가 있으면 "Invalid score bridge response" 오류를 발생시킵니다. 관련 테스트와 문서를 추가했습니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 바이트 배열 검증을 조기 종료 방식으로 최적화한 이번 변경의 핵심 내용을 명확하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bolt/optimize-score-pdf-read-14940922715415907974

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.jules/bolt.md:
- Around line 65-67: Update the “O(1) early exit for byte array validation”
entry in bolt.md to remove the claim that every() creates intermediate arrays or
causes O(N) validation memory overhead. State that every() short-circuits on
false with O(1) auxiliary memory, while the for loop primarily reduces predicate
callback overhead, and note that Uint8Array.from(response) allocates a separate
O(N) result buffer after validation.

In `@apps/desktop/src/features/score/scoreStorage.test.ts`:
- Around line 19-40: Extend the readScorePdf tests around the existing
byte-array validation cases to cover numeric but invalid byte values: -1, 256,
1.5, NaN, and Infinity. Verify each response is rejected with “Invalid score
bridge response,” preventing Uint8Array.from coercion from accepting
out-of-range, fractional, or non-finite numbers.

In `@apps/desktop/src/features/score/scoreStorage.ts`:
- Around line 94-105: Enforce the byte-array contract in the response handling
loop of scoreStorage.ts: accept elements only when they are integers in the
inclusive 0–255 range, rejecting negative, oversized, fractional, NaN, and
infinite values before Uint8Array.from. Add regression coverage in
scoreStorage.test.ts for each invalid input and verify those responses are
rejected rather than converted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 72299e60-21de-4540-bc9b-326b52d9bd16

📥 Commits

Reviewing files that changed from the base of the PR and between acdbea6 and 2bfae9c.

📒 Files selected for processing (3)
  • .jules/bolt.md
  • apps/desktop/src/features/score/scoreStorage.test.ts
  • apps/desktop/src/features/score/scoreStorage.ts

Comment thread .jules/bolt.md Outdated
Comment thread apps/desktop/src/features/score/scoreStorage.test.ts Outdated
Comment thread apps/desktop/src/features/score/scoreStorage.ts Outdated

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

@jules Please fix the current-head central coverage-evidence blocker on this PR branch. The organization workflow discovers apps/desktop as the changed JavaScript package, then runs npm ci from that directory; it currently fails because the repository only has the workspace-root package-lock.json, so vitest is never installed. Implement the smallest repository-owned fix that makes a clean, lifecycle-disabled npm ci from apps/desktop succeed offline from the exact lock graph while preserving the root npm-workspace install. Do not weaken or skip coverage. Keep local @bandscope/shared-types resolution repository-bounded (the central lock validator rejects absolute paths, .., node_modules links, unpinned registry URLs, and non-SHA512 integrity). Add a deterministic synchronization/validation check so any package-level lock or coverage manifest cannot silently drift from the workspace root. Verify root npm ci, npm test --workspace=@bandscope/desktop, coverage thresholds, and the repository checks, then push the focused fix to this branch.

@seonghobae
seonghobae marked this pull request as draft August 3, 2026 01:04
auto-merge was automatically disabled August 3, 2026 01:04

Pull request was converted to draft

@seonghobae
seonghobae marked this pull request as ready for review August 3, 2026 01:05
@seonghobae seonghobae closed this Aug 3, 2026
@seonghobae
seonghobae force-pushed the bolt/optimize-score-pdf-read-14940922715415907974 branch from f2905c9 to acdbea6 Compare August 3, 2026 01:26
@seonghobae seonghobae changed the title ⚡ Bolt: [성능 개선] O(1) 메모리 조기 종료를 통한 바이트 배열 검증 최적화 ⚡ Bolt: PDF 바이트 배열 검증의 콜백 오버헤드 제거 Aug 3, 2026
@seonghobae seonghobae reopened this Aug 3, 2026
@seonghobae
seonghobae enabled auto-merge (squash) August 3, 2026 02:06

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@opencode-agent opencode-agent Bot left a comment

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.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head d91f8f012e45d194637f7781396893bf37076797.

  • Head SHA: d91f8f012e45d194637f7781396893bf37076797

  • Workflow run: 30789846426

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 3

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: c26770486ea87fb670fa84dec85e28f3e289f832
  • Workflow run: 30811824097
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head c26770486ea87fb670fa84dec85e28f3e289f832.

  • Head SHA: c26770486ea87fb670fa84dec85e28f3e289f832

  • Workflow run: 30811824097

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (6 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (6 files)"]
  R1 --> V1["required checks"]
Loading

@seonghobae
seonghobae dismissed opencode-agent[bot]’s stale review August 3, 2026 08:15

Dismissed as non-actionable automation noise: all changed-code findings are resolved and the repository's current-head CI, security, and measured test workflows passed. The referenced external coverage-evidence run is unavailable from this repository and provides no file-level remediation.

@opencode-agent opencode-agent Bot left a comment

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.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head d91f8f012e45d194637f7781396893bf37076797.

  • Head SHA: d91f8f012e45d194637f7781396893bf37076797

  • Workflow run: 30798240295

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 3

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
Loading

@opencode-agent opencode-agent Bot left a comment

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.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head c26770486ea87fb670fa84dec85e28f3e289f832.

  • Head SHA: c26770486ea87fb670fa84dec85e28f3e289f832

  • Workflow run: 30811824097

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (6 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (6 files)"]
  R1 --> V1["required checks"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 3, 2026 14:21
@seonghobae
seonghobae enabled auto-merge (squash) August 4, 2026 03:43
@seonghobae
seonghobae force-pushed the bolt/optimize-score-pdf-read-14940922715415907974 branch from a44652d to 7cd019f Compare August 4, 2026 04:57

Copy link
Copy Markdown
Collaborator Author

The current head now includes the same bounded root-lock Undici 7.29.0 refresh already validated in #744. This addresses the exact-head security-audit blocker without adding a nested workspace lock or changing the PDF bridge runtime implementation. All required checks and independent review must rerun on the new head before merge.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Please review exact head f5d7be12529b7e109ceef866c05566310d97a002. The repository checks expose a current-head security regression: the branch's root lock resolves vulnerable undici 7.28.0 and the PR also carries an unrelated generated nested workspace lock/proxy workaround. Treat the failed npm audit and scope drift as blockers; a repair commit will restore the single root-workspace lock and retain only the byte-validation change plus the bounded undici 7.29.0 remediation.

@seonghobae
seonghobae marked this pull request as draft August 4, 2026 08:00
auto-merge was automatically disabled August 4, 2026 08:00

Pull request was converted to draft

Comment thread .github/workflows/repair-score-pdf-pr.yml Outdated
@seonghobae
seonghobae marked this pull request as ready for review August 4, 2026 09:13
@seonghobae
seonghobae enabled auto-merge (squash) August 4, 2026 09:13
@seonghobae
seonghobae dismissed stale reviews from opencode-agent[bot] and opencode-agent[bot] August 4, 2026 11:02

Dismissed because it evaluated an obsolete head and only reflected the superseded central coverage sandbox. A new independent review is still required on the current head.

Copy link
Copy Markdown
Collaborator Author

@cwl-noema-review @opencode-agent Please independently review exact head 74bb1fe30dc9c12e41e8ec015a64d084213d7b64. Verify the PDF bridge accepts only integer bytes in 0..255, fails early without unintended reads, preserves the desktop-only fail-closed boundary, and contains only the bounded root-lock security refresh beyond the score change. Approve only after current-head checks are permitted and succeed.

Copy link
Copy Markdown
Collaborator Author

Superseded by #750, which preserves this exact clean three-file head and retriggers the normal current-head review/check path. The original PR's latest workflows remained in action_required before any job started; #750 carries no workflow or permission changes and retains auto-merge under the same repository protections.

@seonghobae seonghobae closed this Aug 4, 2026
auto-merge was automatically disabled August 4, 2026 11:33

Pull request was closed

@google-labs-jules

Copy link
Copy Markdown

Superseded by #750, which preserves this exact clean three-file head and retriggers the normal current-head review/check path. The original PR's latest workflows remained in action_required before any job started; #750 carries no workflow or permission changes and retains auto-merge under the same repository protections.

이해했습니다. 이 작업이 다른 PR(#750)로 대체(Superseded)되었음을 확인하였으며, 이 작업에 대한 추가 개발을 중단하겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants