Skip to content

fix(indexer): remove duplicate RoPE quant helper - #108

Merged
lukealonso merged 2 commits into
dev/fathomless-firmamentfrom
codex/ff-sparse-indexer-dedup-20260717
Jul 17, 2026
Merged

fix(indexer): remove duplicate RoPE quant helper#108
lukealonso merged 2 commits into
dev/fathomless-firmamentfrom
codex/ff-sparse-indexer-dedup-20260717

Conversation

@voipmonitor

@voipmonitor voipmonitor commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • remove the first of two byte-identical _fused_indexer_q_rope_quant_kernel / fused_indexer_q_rope_quant definitions
  • remove two imports that are unused in the canonical FF tree
  • leave the surviving runtime implementation unchanged
  • refresh stale B12X sparse-indexer fixtures to the current FF DCP and KV-cache contracts

Why

The duplicate entered the FF branch when the B12X sparse-indexer integration and upstream sparse-indexer work converged. Python silently replaced the first wrapper with the second, so runtime behavior was unaffected, but Ruff reports F811 plus the two stale F401 imports. This can fail pre-commit for later PRs that touch the file.

Validation

  • the removed kernel and wrapper were byte-identical to the surviving definitions
  • ruff check vllm/model_executor/layers/sparse_attn_indexer.py
  • ruff format --check vllm/model_executor/layers/sparse_attn_indexer.py
  • git diff --check
  • 15 passed in tests/model_executor/layers/test_sparse_attn_indexer_b12x.py

Summary by CodeRabbit

  • Refactor

    • Streamlined sparse-attention indexing and decoding internals for more consistent execution across supported configurations.
    • Improved handling of specialized runtime paths without changing the public API.
  • Tests

    • Expanded coverage for distributed sparse-attention merging, top-k results, score output, and schedule metadata.
    • Standardized test setup across multiple parallelism configurations.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR centralizes fake B12X DCP merge setup across tests, updates the merge argument and schedule metadata configuration, and removes module-level sparse indexer imports while relocating a fused quantization definition.

Changes

B12X sparse indexer maintenance

Layer / File(s) Summary
Sparse indexer import and kernel organization
vllm/model_executor/layers/sparse_attn_indexer.py
Removes module-level imports for capture control and FP8/FP4 logits, and deletes the earlier fused indexer quantization definition block.
B12X DCP merge test harness
tests/model_executor/layers/test_sparse_attn_indexer_b12x.py
Adds shared fake DCP merge setup, updates the interleave argument expectation, reuses the helper across merge tests, and configures storage through kv_cache_spec.

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

Possibly related PRs

Suggested reviewers: lukealonso

🚥 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 The title accurately describes the main code change: removing the duplicate RoPE quant helper from the indexer.
✨ 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 codex/ff-sparse-indexer-dedup-20260717

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.

❤️ Share

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

@voipmonitor

Copy link
Copy Markdown
Author

Follow-up validation: the three stale B12X fixtures were aligned with the current FF contracts without changing runtime behavior. The complete tests/model_executor/layers/test_sparse_attn_indexer_b12x.py suite now passes: 15 passed. Ruff lint, Ruff format, and git diff --check also pass.

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