Repository navigation
[Fix] Enforce exact multimodal embedding rows at final placement - #43
Open
rchalamala wants to merge 1 commit into
Open
rchalamala wants to merge 1 commit into
rchalamala wants to merge 1 commit into
Conversation
rchalamala
marked this pull request as ready for review
September 25, 2026 07:59
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
rchalamala
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The final multimodal placement check silently keeps a suffix when an embedding has too many rows. That can shift image rows onto different placeholder tokens. It also counts only the first dimension for higher-rank encoder outputs.
Modifications
Require an exact flattened token-row count at final placement, preserving the feature dimension and returning matching tensors unchanged. Report mismatches with both counts and chunked-prefill context. Model-owned padding/trimming and EVS placeholder redistribution retain their existing behavior.
This check runs after chunk extraction; it does not validate producer rows that earlier slicing already removed or provide request-level failure isolation.
Accuracy Tests
CPU tests exercise 2D–4D row counts, exact/short/excess outputs, chunked-prefill boundaries, cache reuse, list/tensor encode results, precomputed embeddings, actual input embedding placement, and EVS redistribution. Registered direct runners and neighboring chunked-embedding tests pass. Source-only regression controls fail with the original helper and pass with the change.
Registered-test validation, Ruff 0.15.1, isort 7.0.0, codespell 2.4.1, and formatting pass. No model accuracy or GPU execution was measured.
Speed Tests and Profiling
No performance benchmark. The check reuses the existing flattened-row counting helper and preserves the offset-count and EVS readback behavior.
Checklist
CI States
Latest PR Test (Base): ❌ Run #36108884616
Latest PR Test (Extra): ❌ Run #36108884198
Latest PR Test (AMD ROCm 10): ❌ Run #36108884604