perf(segmenter): vectorize checkerboard novelty reference - #746
perf(segmenter): vectorize checkerboard novelty reference#746seonghobae wants to merge 33 commits into
Conversation
|
👋 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: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesCheckerboard 벡터화
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
seonghobae
changed the title
…d-vectorization.yml)에 필수 GIT_CONFIG 환경변수 누락 추가
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/finalize-checkerboard-vectorization.yml:
- Around line 6-8: Remove scripts/ci/finalize_checkerboard_vectorization.py from
the paths filter of the finalize-checkerboard-vectorization workflow, leaving
the workflow file path so its initial addition still triggers the one-time
finalization. Keep the existing script compilation and deletion steps unchanged.
In `@services/analysis-engine/tests/test_segmenter.py`:
- Around line 128-143: Extend the parameter set for
test_checkerboard_novelty_reference_matches_independent_loop with a case where
matrix_size is smaller than kernel_size, such as (2, 4), so the test directly
exercises _checkerboard_novelty_reference’s early-return path. Keep the existing
seeded input generation and oracle comparison unchanged.
🪄 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: 4d80b920-2735-423d-9e17-4998e1962423
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
.github/workflows/finalize-checkerboard-vectorization.yml.jules/bolt.mdservices/analysis-engine/src/bandscope_analysis/sections/segmenter.pyservices/analysis-engine/tests/test_segmenter.py
|
The exact head now carries the bounded root-lock Undici 7.29.0 refresh already isolated and validated in #744. The previous |
Acknowledged. I have restored the accidentally deleted |
|
Removed the reintroduced self-modifying finalizer again. That restoration was not a CI repair: the reviewed final state intentionally contains no branch-pushing one-shot workflow or helper. The branch is now restored byte-for-byte to the previously reviewed clean tree ( |
Understood. Acknowledging that the deletion of the workflow and helper script is intentional and that the branch is manually restored to the verified clean tree ( |
|
@cwl-noema-review @opencode-agent Please independently review exact head |
Acknowledged. Standing by for independent review on the exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== HEAD ==='
git rev-parse HEAD
git status --short
git show --no-ext-diff --format=fuller --stat HEAD
printf '%s\n' '=== CHANGED PATHS AT HEAD ==='
git diff-tree --no-commit-id --name-status -r HEAD
printf '%s\n' '=== SEGMENTER OUTLINE ==='
ast-grep outline services/analysis-engine/src/bandscope_analysis/sections/segmenter.py --items all --match 'checkerboard|novelty|kernel' || true
printf '%s\n' '=== REFERENCE IMPLEMENTATION CONTEXT ==='
rg -n -A80 -B20 'def _checkerboard_novelty_reference\b' services/analysis-engine/src/bandscope_analysis/sections/segmenter.py
printf '%s\n' '=== TEST ORACLE CONTEXT ==='
rg -n -A130 -B20 'checkerboard|novelty|loop' services/analysis-engine/tests/test_segmenter.py
printf '%s\n' '=== RUST KERNEL CANDIDATES ==='
rg -n -i -A30 -B15 'checkerboard|novelty' --glob '*.rs' .
printf '%s\n' '=== WORKFLOW / HELPER PRESENCE ==='
fd -H -t f .github .jules 2>/dev/null | sortLength of output: 34613 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== PR METADATA ==='
gh pr view 746 --repo ContextualWisdomLab/bandscope \
--json number,baseRefName,baseRefOid,headRefName,headRefOid,changedFiles,commits,files \
--jq '{number, baseRefName, baseRefOid, headRefName, headRefOid, changedFiles, commits: [.commits[].oid], files: [.files[] | {path, additions, deletions}]}'
printf '%s\n' '=== PR FILE DIFF ==='
gh pr diff 746 --repo ContextualWisdomLab/bandscope --patch \
-- services/analysis-engine/src/bandscope_analysis/sections/segmenter.py \
services/analysis-engine/tests/test_segmenter.py \
services/analysis-engine/tests/test_numeric_parity.py \
|| true
printf '%s\n' '=== PARITY AND MUTATION TESTS ==='
fd -H -t f 'test*parity*.py' services/analysis-engine/tests -x sh -c \
'echo "=== $1 ==="; rg -n -A100 -B15 "checkerboard|mutat|copy|shares_memory" "$1"' sh {} \; || true
printf '%s\n' '=== NUMPY IMPORT AND STATIC CALL SITES ==='
rg -n -A8 -B8 'sliding_window_view|np\.diagonal\(|np\.einsum\(' \
services/analysis-engine/src/bandscope_analysis/sections/segmenter.py
printf '%s\n' '=== PR-RELATED WORKFLOW / HELPER DIFF STATUS ==='
gh pr diff 746 --repo ContextualWisdomLab/bandscope --name-only | \
rg -n '(^|/)(\.github/|\.jules/|.*finalize.*|.*checkerboard.*)' || trueLength of output: 4904 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='acdbea6344fe1231c39535b575f4de35e4c607c9'
head='434516b8919c335621901bf3a04c53aa44969b66'
printf '%s\n' '=== EXACT BASE-TO-HEAD DIFF: NUMPY AND TESTS ==='
git diff --no-ext-diff --unified=80 "$base" "$head" -- \
services/analysis-engine/src/bandscope_analysis/sections/segmenter.py \
services/analysis-engine/tests/test_segmenter.py \
services/analysis-engine/tests/test_segmenter_short_reference.py
printf '%s\n' '=== SHORT-REFERENCE TEST OUTLINE AND CONTENT ==='
ast-grep outline services/analysis-engine/tests/test_segmenter_short_reference.py --items all
cat -n services/analysis-engine/tests/test_segmenter_short_reference.py
printf '%s\n' '=== ALL CHECKERBOARD TEST AND PARITY REFERENCES ==='
rg -n -A50 -B15 \
'(_checkerboard_novelty_reference|_checkerboard_novelty_rust|checkerboard_novelty|HAVE_RUST|assert_allclose|array_equal|copy\(\)|shares_memory)' \
services/analysis-engine/tests
printf '%s\n' '=== LOCKFILE UNDICI DIFF ==='
git diff --no-ext-diff --unified=12 "$base" "$head" -- package-lock.json
printf '%s\n' '=== DIFF PATHS WITH STATUS ==='
git diff --name-status "$base" "$head"Length of output: 75708
No blocking issue found.
I did not execute repository code in this review sandbox. The supplied successful CI results provide the runtime verification. ✅ Action performedReview finished.
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore 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 headecfcb3c5db49e53c541104820596340c80114cda. -
Head SHA:
ecfcb3c5db49e53c541104820596340c80114cda -
Workflow run: 31028936586
-
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 (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
|
@opencode-agent @cwl-noema-review Please independently re-review exact current head |
Acknowledged. Standing by for independent exact-head review on |
|
@opencode-agent @cwl-noema-review Please independently review exact current head |
Acknowledged. Standing by for independent exact-head review on |
There was a problem hiding this comment.
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
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore 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 head22cc3a43d5c3b5cc9ce6160ccedbcd5967695680. -
Head SHA:
22cc3a43d5c3b5cc9ce6160ccedbcd5967695680 -
Workflow run: 31111576552
-
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 (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
|
@opencode-agent address Keep the vectorization as a bounded fallback/parity-oracle optimization on exact head Add evidence that the new stride/diagonal/einsum path is actually better and does not violate the stated memory contract: a deterministic benchmark over representative |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
The exact-head NumPy reference implementation is now a focused, mathematically reviewable vectorization: centered-window selection matches the prior even/odd boundary, the independent explicit-patch oracle covers unit, short, odd, even, and near-boundary kernels, and the production Rust path remains unchanged. One scope repair remains before the central rerun: restore the unrelated Undici lockfile change to protected develop and add an Unreleased CHANGELOG entry for the reference-path performance improvement. Then rerun the focused segmenter suite, Rust↔NumPy parity/native tests, full coverage/docstrings, Ruff/mypy, and repository gates on the new exact head.
|
@opencode-agent address Clean exact current head Restore The final base-to-head paths must be |
|
@opencode-agent Please review exact current head |
Acknowledged. Standing by for independent exact-head review on |
There was a problem hiding this comment.
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
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore 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 head5d83153744e8692ea9416d95307a470a34f732c0. -
Head SHA:
5d83153744e8692ea9416d95307a470a34f732c0 -
Workflow run: 31188962909
-
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 (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (2 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (2 files)"]
R2 --> V2["targeted test run"]
1 participant
What
Vectorize the retained NumPy checkerboard-novelty reference with sliding-window and diagonal views, then contract each valid centered patch directly into the preallocated result. The production path remains the Rust numeric kernel.
Exact current scope
Exact head:
dc19a40e26b77cbd012bba5da563d32eecaeba75.Exactly four files differ from protected
develop: the NumPy reference implementation, two direct regression files, andCHANGELOG.md. The unrelated npm lock update was removed.Correctness boundary
Merge gate
Require exact-head focused/full Python tests, production statement and branch coverage 100%, public docstrings 100%, Ruff, mypy, native Rust tests, Rust-to-NumPy parity, repository CI/security, central review, zero unresolved actionable threads, qualifying independent approval, and branch protection without bypass.