Conversation
Decision sequences live on the main context only; propagating n_seq_max = n_parallel + n_seq_decision to the MTP draft context violates the n_outputs_max assert (n_outputs_max <= cparams.n_outputs_max). Verified: MTP + /v1/decision coexist after this fix (no assert crash).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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.
Hi Anirban —
I've been running your
parallel-decisionwork in production: thePOST /v1/decisionendpoint on Qwen3.8-27B (2x RTX 3090, fully local),plugged into a LangGraphJS workflow engine. For closed decision tasks
(routing / classification / scoring) it has been a structural fix —
~18-20x faster per decision vs json_mode, zero output tokens, and it
eliminated three silent data-loss paths we were hitting in production.
This PR: with MTP speculative decoding (
--spec-type draft-mtp),the decision path propagates
n_seq_decisioninto the speculativecontext and hits an assert. A one-line guard fixes it. Verified with a
production MTP config: MTP +
/v1/decisioncoexist, no assert crash.Separately I opened #14 with a hybrid decision extension (closed
fields + one bounded open field in a single call) — feel free to close
it if it is not wanted.
The question before I go further: are you interested in upstreaming
/v1/decisionto ggml-org/llama.cpp? Your patches are already rebasedonto current master (clean apply, compile + functional verification
pass) — happy to prepare a joint PR with proper credit, or you can drive
and I'll support. If not, no pressure: we'll keep running the fork and
contribute fixes here. I'm also writing up the production experience as
an article (links to your repo).
Either way, thanks for the work — it is doing real production duty.