Skip to content

fix(kda): preserve gate alias contract during GLM decode - #824

Closed
voipmonitor wants to merge 1 commit into
dev/karmic-krakenfrom
fix/karmic-glm-kda-disjoint-output
Closed

voipmonitor wants to merge 1 commit into
dev/karmic-krakenfrom
fix/karmic-glm-kda-disjoint-output

Conversation

@voipmonitor

@voipmonitor voipmonitor commented Sep 21, 2026

Copy link
Copy Markdown

Behavior

When a B12X KDA decode output overlaps the live gate tensor, vLLM binds a disjoint caller-owned output, runs recurrence, then copies the result into the requested output. Disjoint outputs retain the existing allocation-free path. Other B12X binding failures still propagate.

Reason and compatibility

B12X correctly rejects a mutable output that overlaps its read-only raw_g input. A reported GLM-5.3-Flash TP2/DCP2/MTP3 service crashed under concurrent long-context traffic with exactly that error. This change preserves the B12X alias contract and existing output semantics; the exceptional alias case adds one temporary output and copy.

Validation

  • A forced-alias unit reproducer checks that B12X sees a disjoint output, reads the unchanged gate, and returns the requested result. Four targeted model tests passed in a GPU-capable environment; Ruff passed.
  • A patched-source GLM-5.3-Flash-NVFP4-Spark TP2/DCP2/MTP3 server on two RTX PRO 6000 GPUs completed full and piecewise CUDA graph capture, a health check, and overlapping decode with a roughly 30k-token cold prefill.
  • The reported 850k-token, three-concurrent-request workload was not reproduced. This is bounded correctness validation, not a claim that every long-context failure is fixed.

Report: https://discord.com/channels/1466898002793857221/1471527895439638528/1551693471558017085

Summary by CodeRabbit

  • Bug Fixes
    • Improved gated attention decoding when output buffers overlap with read-only gate data.
    • Preserved gate data during decoding while continuing to propagate unrelated binding errors.
    • Added coverage to verify correct results and buffer handling in this scenario.

Signed-off-by: Martin Vit <martin@voipmonitor.org>
@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9c2265f5-49d9-4d62-8585-ad8905f9f737

📥 Commits

Reviewing files that changed from the base of the PR and between 9e5d179 and 0aa1e08.

📒 Files selected for processing (2)
  • tests/models/test_glm5next_model.py
  • vllm/model_executor/layers/mamba/gdn/kimi_gdn_linear_attn.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The B12X KDA decode path now handles output buffers that overlap the read-only raw_g tensor. A regression test verifies temporary output binding, gate preservation, and final result copying.

Changes

B12X KDA alias-safe decode

Layer / File(s) Summary
Alias-safe decode and regression coverage
vllm/model_executor/layers/mamba/gdn/kimi_gdn_linear_attn.py, tests/models/test_glm5next_model.py
The decode binding uses a temporary destination when the output overlaps raw_g. Other ValueError exceptions remain unchanged. The test verifies that raw_g stays unchanged and that the result is copied back.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: lukealonso

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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 clearly and concisely describes the main change: preserving the gate alias contract during GLM decode in KDA.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

Superseded by #821, which now contains this PR’s history, gate/output alias correction, tests and immutable release fragment. The consolidation preserves the exact alias-safe implementation already qualified in beta and resolves its overlap with recovery-record binding. Merge #798, then #821; do not reapply this separate branch. Composed QSA/GDN/warmup/alias checks: 112 passed, nine declared skips.

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.

1 participant