-
Notifications
You must be signed in to change notification settings - Fork 262
Support add_generation_prompt request parameter for chat completions (#3877) #4331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ca493f1
de8b330
16da220
a587952
709fb43
c7d15d4
cb00436
6230277
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -67,7 +67,6 @@ absl::Status ChatTemplateProcessor::process(InputRequest& req) { | |||||||||||||||||||||||||||||||||||||||||||
| SPDLOG_LOGGER_TRACE(llm_calculator_logger, "chatHistory.get_extra_context(): {}", chatHistory.get_extra_context().to_json_string()); | ||||||||||||||||||||||||||||||||||||||||||||
| SPDLOG_LOGGER_TRACE(llm_calculator_logger, "tools: {}", chatHistory.get_tools().empty() ? std::string("<none>") : chatHistory.get_tools().to_json_string()); | ||||||||||||||||||||||||||||||||||||||||||||
| SPDLOG_LOGGER_TRACE(llm_calculator_logger, "chatTemplateKwargs: {}", chatHistory.get_extra_context().empty() ? std::string("<none>") : chatHistory.get_extra_context().to_json_string()); | ||||||||||||||||||||||||||||||||||||||||||||
| SPDLOG_LOGGER_TRACE(llm_calculator_logger, "addGenerationPrompt: {}", true); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| #if (PYTHON_DISABLE == 0) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -82,9 +81,21 @@ absl::Status ChatTemplateProcessor::process(InputRequest& req) { | |||||||||||||||||||||||||||||||||||||||||||
| req.promptText = std::move(promptText); | ||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||||||||||||||||||||
| constexpr bool addGenerationPrompt = true; | ||||||||||||||||||||||||||||||||||||||||||||
| const auto& tools = chatHistory.get_tools(); | ||||||||||||||||||||||||||||||||||||||||||||
| const auto& kwargs = chatHistory.get_extra_context(); | ||||||||||||||||||||||||||||||||||||||||||||
| // add_generation_prompt lives inside chat_template_kwargs; MINJA's apply_chat_template | ||||||||||||||||||||||||||||||||||||||||||||
| // takes it as a dedicated argument, so extract it here and drop it from the kwargs map | ||||||||||||||||||||||||||||||||||||||||||||
| // passed alongside so it isn't supplied twice. | ||||||||||||||||||||||||||||||||||||||||||||
| ov::genai::JsonContainer kwargs = chatHistory.get_extra_context(); | ||||||||||||||||||||||||||||||||||||||||||||
| bool addGenerationPrompt = true; | ||||||||||||||||||||||||||||||||||||||||||||
| if (kwargs.contains("add_generation_prompt")) { | ||||||||||||||||||||||||||||||||||||||||||||
| const auto asBool = kwargs["add_generation_prompt"].as_bool(); | ||||||||||||||||||||||||||||||||||||||||||||
| if (!asBool.has_value()) { | ||||||||||||||||||||||||||||||||||||||||||||
| return absl::Status(absl::StatusCode::kInvalidArgument, | ||||||||||||||||||||||||||||||||||||||||||||
| "add_generation_prompt accepts values true or false"); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| addGenerationPrompt = asBool.value(); | ||||||||||||||||||||||||||||||||||||||||||||
| kwargs.erase("add_generation_prompt"); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+85
to
+98
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please separate it to method
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| const std::optional<ov::genai::JsonContainer> optTools = | ||||||||||||||||||||||||||||||||||||||||||||
| tools.empty() ? std::nullopt : std::make_optional(tools); | ||||||||||||||||||||||||||||||||||||||||||||
| const std::optional<ov::genai::JsonContainer> optKwargs = | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,7 @@ using InputPayload = std::variant<ov::genai::ChatHistory, std::string>; | |
| struct InputRequest { | ||
| InputPayload input; // set in parseRequest() | ||
| ov::genai::GenerationConfig generationConfig; // set in parseRequest() | ||
| // add_generation_prompt is folded into ChatHistory's extra_context (chat_template_kwargs). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. drop this comment entirely - only explicit metion of add_generation_prompt could be in chat kwargs description in the docs that you already added |
||
|
|
||
| std::string promptText; // written by ChatTemplateProcessor / RawPromptExtractor | ||
| ov::Tensor inputIds; // written by TokenizationProcessor (all paths) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like that
add_generation_promptstands out here. Kwargs can carry more parameters and I would not specifically mention any of them in the comments here. I think we can remove this comment entirely.