Skip to content

Expose skip_special_tokens parameter#4184

Open
mzegla wants to merge 8 commits intomainfrom
skip_special_tokens_per_request
Open

Expose skip_special_tokens parameter#4184
mzegla wants to merge 8 commits intomainfrom
skip_special_tokens_per_request

Conversation

@mzegla
Copy link
Copy Markdown
Collaborator

@mzegla mzegla commented May 6, 2026

Partially addressing: #4168.

Works only in configuration without parsers (skip_special_tokens=false rejected if model configuration specifies tool_parser or reasoning_parser).

Copilot AI review requested due to automatic review settings May 6, 2026 15:46
Comment thread src/test/llm/llmnode_test.cpp
Comment thread src/llm/servable.cpp
@mzegla mzegla requested review from dkalinowski and dtrawins May 7, 2026 12:45
Comment thread docs/model_server_rest_api_chat.md Outdated
Comment thread src/test/http_openai_handler_test.cpp Outdated
}

TEST_P(HttpOpenAIHandlerCommonParsingValidationTest, SkipSpecialTokensDefaultIsTrue) {
std::string json = createRequestWithSkipSpecialTokensRawValue("true");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this variable is never used

std::optional<uint32_t> maxModelLength;
auto apiHandler = createHandler(endpoint(), "llama3");

EXPECT_EQ(apiHandler->parseRequest(maxTokensLimit, bestOfLimit, maxModelLength), absl::OkStatus());
Copy link
Copy Markdown
Collaborator

@dkalinowski dkalinowski May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXPECT_EQ(apiHandler->getOutputParser(), nullptr);
shouldnt it be included?

and
EXPECT_NE(apiHandler->getOutputParser(), nullptr);
shouldnt it be included?

in the test above?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it mean "accepted"?

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.

3 participants