Skip to content

Conversation

dstoc
Copy link

@dstoc dstoc commented Sep 6, 2025

Previously only size zeroATTR_CONTROL tokens were treated as special.

Previously only size zero tokens were treated as special.
@dstoc dstoc force-pushed the llguidance-special-tokens branch from 3afd455 to f24989b Compare September 6, 2025 13:45
@aldehir
Copy link
Collaborator

aldehir commented Sep 7, 2025

Try commenting out these lines and see if it works with llguidance and gpt-oss, without your fix here. Might need to address this instead.

llama.cpp/src/llama-vocab.cpp

Lines 2343 to 2347 in 79bc429

for (const auto & t : token_to_id) {
if (t.first == "<|channel|>" || t.first == "<|message|>" || t.first == "<|start|>" || t.first == "<|constrain|>") {
id_to_token[t.second].attr = LLAMA_TOKEN_ATTR_USER_DEFINED;
}
}

id_to_token[end_id].attr = LLAMA_TOKEN_ATTR_USER_DEFINED;

@dstoc
Copy link
Author

dstoc commented Sep 7, 2025

Try commenting out these lines...

Ah! That explains why they were CONTROL in the GGUF but not at runtime!

Yes, disabling those changes allows the llguidance grammar to parse as I expected.

llama.cpp/src/llama-vocab.cpp

Lines 2959 to 2965 in 79bc429

int32_t llama_vocab::impl::token_to_piece(llama_token token, char * buf, int32_t length, int32_t lstrip, bool special) const {
// ref: https://github.com/ggerganov/llama.cpp/pull/7587#discussion_r1620983843
static const int attr_special = LLAMA_TOKEN_ATTR_UNKNOWN | LLAMA_TOKEN_ATTR_CONTROL;
const llama_token_attr attr = token_get_attr(token);
if (!special && (attr & attr_special)) {
return 0;
}

Given that, my patch isn't doing much other than also checking for USER_DEFINED. If USER_DEFINED is considered special, then I think it still makes sense.

@dstoc
Copy link
Author

dstoc commented Sep 10, 2025

@ngxson @mmoskal

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