fix(cli): enforce /v1/completions stop sequences host side for qairt - #1369
Open
Shishir Shrivastava (Shishir2405) wants to merge 1 commit into
Open
Conversation
The qairt plugin rejects any generate call that carries stop sequences (sdk/plugins/qairt/src/llm.cpp:219), so completion.go forwarding Stop unchanged to every runtime made /v1/completions unusable with QAIRT models for any client that sends a stop list, which real FIM autocompletion clients always do. Withhold Stop from the plugin config when the runtime is qairt and match the requested stop strings against the generated text instead, via a new stopMatcher that buffers just enough of the tail to catch a match split across token boundaries. A match cancels generation early through the existing OnToken return-false path and truncates the returned text at the match, for both the streaming and blocking response paths. Closes qualcomm#1341 Signed-off-by: Shishir2405 <shishirshrivastava30@gmail.com>
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
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.
Summary
The qairt plugin rejects any
generatecall that carries stop sequences (sdk/plugins/qairt/src/llm.cpp:219), butcompletion.goforwardedStopunchanged to every runtime. Any FIM autocompletion client sending astoplist (which real editor clients always do) gotErrorCode[-100016]and an empty completion on QAIRT models, while the identical request worked on llama_cpp GGUF models.This implements the host-side enforcement the issue proposed:
Stopis withheld fromGenerationConfigwhen the resolved runtime isqairt.stopMatcher(cli/server/handler/stop_match.go) matches the requested stop strings against generated text as it comes in, buffering only the minimal tail needed to catch a stop sequence split across token boundaries (e.g.<fim+_suffix+>).OnTokenreturn-falsepath (already supported by the qairt pipeline, reported asstop_reason: "user"→finish_reason: "stop"via the existingmapFinishReason).llama_cpp models are unaffected —
Stopis still forwarded to the plugin exactly as before when the runtime isn't qairt.Test plan
stop_match_test.go: table-driven tests forstopMatcher, including the exact FIM stop-sequence-split-across-tokens scenario from the issue, multiple stop sequences, earliest-match-wins, empty stop strings, and the case where a partial match never completes (must still flush the held-back text).gofmt -lclean on all three changed/added files.go vet/go test ./cli/server/handler/...orbazelisk coveragein this environment — thehandlerpackage importsbindings/go, which requires the built SDK bridge (sdk/pkg-geniex/), and per_build-sdk.ymlthat only builds on windows-arm64 / linux-arm64 (Docker) / android-arm64, none of which were available here. I'd appreciate a CI run / local verification on a supported host before merge.Notes
/v1/completions, matching the issue.chat.go's/v1/chat/completionspath doesn't forwardStoptoGenerationConfigat all today (a separate gap, if it is one) — left untouched here to keep this PR focused.stop_match.goonly usesstringsfrom the standard library, sogo.mod/go.sumare untouched.Closes #1341