Repository navigation
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughImage transforms now report per-image patch-grid dimensions through optional metadata. ColModernVBERT passes this metadata to image-input preprocessing, which validates each grid against the real patch count and uses it to build image prompts. New tests cover split and unsplit images, mixed batches, resizing, padding, and invalid grids. Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change makes image prompts use the actual patch grid instead of one inferred from patch count. No merge-blocking issue was identified in the supplied evidence. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The reviewed change keeps image geometry associated with each request and checks its consistency before execution. Existing default outputs are preserved, and no material security risk was found in the changed path. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
2 participants
Summary
ColModernVBERT reconstructs an image's patch grid from its patch count, which loses the original rows and columns. For example, two local patches can be a 2×1 portrait or a 1×2 landscape grid, and four can be 1×4, 4×1, or 2×2. The current near-square guess supplies incorrect row/column tokens and row separators for some of these images.
This preserves the grid produced by
ImageSplitterin per-call metadata and uses it when constructing the image prompt. The existing return types ofImageSplitterandComposeare unchanged. Unsplit images retain their global-image prompt, and geometry metadata is not passed to ONNX.Regression coverage
Generated tiny RGB images exercise the real
onnx_embed_imagepreprocessing path, including resizing, splitting, rescaling, normalization, nested-patch padding, prompt tokenization, and token padding. Tests cover portrait, landscape, wide, tall, square, unsplit, rounded/resized grids, mixed batches, both padding directions, and existing transform return shapes.The tokenizer is a small real BPE model trained locally. Model construction/downloads are bypassed and the final ONNX
runcall is mocked to record its inputs. The initial unchanged-baseline tests produced 6 failures and 5 passing controls; the failures are actual prompt-token mismatches.Validation
test_colmodernvbert_preprocessing.py,test_image_transform.py,test_common.py)git diff --check: passedAn earlier broader attempt including
test_preprocessor_utils.pyproduced 32 passes and 31 fixture setup errors: the existing tokenizer fixture reaches a model-download path whose configured SOCKS proxy lackssocksio. Those tests never reached their assertions.No pretrained model tokenizer, model weights, ONNX session, model inference, or retrieval-quality evaluation was used. The full model-download suite and multi-platform/Python-version CI matrix have not passed locally. This is an offline preprocessing fix, with no embedding-accuracy claim.
All Submissions
The repository's complete CI suite remains to be run by CI. Pre-commit hooks were not installed locally; their pinned Ruff 0.3.4 checks were run directly.