Unverified Commit e3619ce0 authored by ryan-lempka's avatar ryan-lempka Committed by GitHub
Browse files

chore: deprecate duplicate params in nvext (#2754)


Signed-off-by: default avatarRyan Lempka <rlempka@nvidia.com>
parent 78a11074
......@@ -8,7 +8,7 @@ use super::{
ContentProvider,
common::{self, OutputOptionsProvider, SamplingOptionsProvider, StopConditionsProvider},
};
use crate::protocols::openai::common_ext::CommonExtProvider;
use crate::protocols::openai::common_ext::{CommonExtProvider, choose_with_deprecation};
pub mod chat_completions;
pub mod common_ext;
......@@ -61,9 +61,11 @@ trait OpenAIStopConditionsProvider {
/// Get the effective ignore_eos value, considering both CommonExt and NvExt.
/// CommonExt (root-level) takes precedence over NvExt.
fn get_ignore_eos(&self) -> Option<bool> {
// Check common first (takes precedence), then fall back to nvext
self.get_common_ignore_eos()
.or_else(|| self.nvext().and_then(|nv| nv.ignore_eos))
choose_with_deprecation(
"ignore_eos",
self.get_common_ignore_eos().as_ref(),
self.nvext().and_then(|nv| nv.ignore_eos.as_ref()),
)
}
}
......
......@@ -21,7 +21,9 @@ use crate::engines::ValidateRequest;
use super::{
OpenAIOutputOptionsProvider, OpenAISamplingOptionsProvider, OpenAIStopConditionsProvider,
common_ext::{CommonExt, CommonExtProvider},
common_ext::{
CommonExt, CommonExtProvider, choose_with_deprecation, emit_nvext_deprecation_warning,
},
nvext::NvExt,
nvext::NvExtProvider,
validate,
......@@ -149,6 +151,12 @@ impl CommonExtProvider for NvCreateChatCompletionRequest {
/// Guided Decoding Options
fn get_guided_json(&self) -> Option<&serde_json::Value> {
// Note: This one needs special handling since it returns a reference
if let Some(nvext) = &self.nvext
&& nvext.guided_json.is_some()
{
emit_nvext_deprecation_warning("guided_json", true, self.common.guided_json.is_some());
}
self.common
.guided_json
.as_ref()
......@@ -156,32 +164,39 @@ impl CommonExtProvider for NvCreateChatCompletionRequest {
}
fn get_guided_regex(&self) -> Option<String> {
self.common
.guided_regex
.clone()
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_regex.clone()))
choose_with_deprecation(
"guided_regex",
self.common.guided_regex.as_ref(),
self.nvext.as_ref().and_then(|nv| nv.guided_regex.as_ref()),
)
}
fn get_guided_grammar(&self) -> Option<String> {
self.common
.guided_grammar
.clone()
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_grammar.clone()))
choose_with_deprecation(
"guided_grammar",
self.common.guided_grammar.as_ref(),
self.nvext
.as_ref()
.and_then(|nv| nv.guided_grammar.as_ref()),
)
}
fn get_guided_choice(&self) -> Option<Vec<String>> {
self.common
.guided_choice
.clone()
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_choice.clone()))
choose_with_deprecation(
"guided_choice",
self.common.guided_choice.as_ref(),
self.nvext.as_ref().and_then(|nv| nv.guided_choice.as_ref()),
)
}
fn get_guided_decoding_backend(&self) -> Option<String> {
self.common.guided_decoding_backend.clone().or_else(|| {
choose_with_deprecation(
"guided_decoding_backend",
self.common.guided_decoding_backend.as_ref(),
self.nvext
.as_ref()
.and_then(|nv| nv.guided_decoding_backend.clone())
})
.and_then(|nv| nv.guided_decoding_backend.as_ref()),
)
}
}
......@@ -224,6 +239,16 @@ impl OpenAIStopConditionsProvider for NvCreateChatCompletionRequest {
fn get_common_ignore_eos(&self) -> Option<bool> {
self.common.ignore_eos
}
/// Get the effective ignore_eos value, considering both CommonExt and NvExt.
/// CommonExt (root-level) takes precedence over NvExt.
fn get_ignore_eos(&self) -> Option<bool> {
choose_with_deprecation(
"ignore_eos",
self.get_common_ignore_eos().as_ref(),
NvExtProvider::nvext(self).and_then(|nv| nv.ignore_eos.as_ref()),
)
}
}
impl OpenAIOutputOptionsProvider for NvCreateChatCompletionRequest {
......
......@@ -67,6 +67,35 @@ pub trait CommonExtProvider {
fn get_guided_decoding_backend(&self) -> Option<String>;
}
/// Helper function to emit deprecation warnings for nvext parameters
pub fn emit_nvext_deprecation_warning(
field_name: &str,
nvext_has_value: bool,
common_has_value: bool,
) {
if nvext_has_value && !common_has_value {
tracing::warn!(
"DEPRECATION WARNING: 'nvext.{field_name}' is deprecated and will be removed in a future release. Use '{field_name}' at the top level or in 'extra_body' instead."
);
} else if nvext_has_value && common_has_value {
tracing::warn!(
"DEPRECATION WARNING: 'nvext.{field_name}' is deprecated and will be removed in a future release. Top-level '{field_name}' takes precedence. Use '{field_name}' at the top level or in 'extra_body' instead."
);
}
}
/// Helper function to choose between common and nvext values with deprecation warnings
pub fn choose_with_deprecation<T: Clone>(
field: &'static str,
common: Option<&T>,
nv: Option<&T>,
) -> Option<T> {
if nv.is_some() {
emit_nvext_deprecation_warning(field, true, common.is_some());
}
common.cloned().or_else(|| nv.cloned())
}
#[cfg(test)]
mod tests {
use super::*;
......@@ -163,4 +192,23 @@ mod tests {
assert_eq!(common_ext.min_tokens, None);
assert!(common_ext.validate().is_ok());
}
#[test]
fn test_choose_with_deprecation() {
// Common takes precedence
let result = choose_with_deprecation(
"test_field",
Some(&"common_value".to_string()),
Some(&"nvext_value".to_string()),
);
assert_eq!(result, Some("common_value".to_string()));
// Fallback to nvext
let result = choose_with_deprecation("test_field", None, Some(&"nvext_value".to_string()));
assert_eq!(result, Some("nvext_value".to_string()));
// Both None
let result: Option<String> = choose_with_deprecation("test_field", None, None);
assert_eq!(result, None);
}
}
......@@ -24,7 +24,9 @@ use super::{
ContentProvider, OpenAIOutputOptionsProvider, OpenAISamplingOptionsProvider,
OpenAIStopConditionsProvider,
common::{self, OutputOptionsProvider, SamplingOptionsProvider, StopConditionsProvider},
common_ext::{CommonExt, CommonExtProvider},
common_ext::{
CommonExt, CommonExtProvider, choose_with_deprecation, emit_nvext_deprecation_warning,
},
nvext::{NvExt, NvExtProvider},
validate,
};
......@@ -143,6 +145,12 @@ impl CommonExtProvider for NvCreateCompletionRequest {
/// Guided Decoding Options
fn get_guided_json(&self) -> Option<&serde_json::Value> {
// Note: This one needs special handling since it returns a reference
if let Some(nvext) = &self.nvext
&& nvext.guided_json.is_some()
{
emit_nvext_deprecation_warning("guided_json", true, self.common.guided_json.is_some());
}
self.common
.guided_json
.as_ref()
......@@ -150,32 +158,39 @@ impl CommonExtProvider for NvCreateCompletionRequest {
}
fn get_guided_regex(&self) -> Option<String> {
self.common
.guided_regex
.clone()
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_regex.clone()))
choose_with_deprecation(
"guided_regex",
self.common.guided_regex.as_ref(),
self.nvext.as_ref().and_then(|nv| nv.guided_regex.as_ref()),
)
}
fn get_guided_grammar(&self) -> Option<String> {
self.common
.guided_grammar
.clone()
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_grammar.clone()))
choose_with_deprecation(
"guided_grammar",
self.common.guided_grammar.as_ref(),
self.nvext
.as_ref()
.and_then(|nv| nv.guided_grammar.as_ref()),
)
}
fn get_guided_choice(&self) -> Option<Vec<String>> {
self.common
.guided_choice
.clone()
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_choice.clone()))
choose_with_deprecation(
"guided_choice",
self.common.guided_choice.as_ref(),
self.nvext.as_ref().and_then(|nv| nv.guided_choice.as_ref()),
)
}
fn get_guided_decoding_backend(&self) -> Option<String> {
self.common.guided_decoding_backend.clone().or_else(|| {
choose_with_deprecation(
"guided_decoding_backend",
self.common.guided_decoding_backend.as_ref(),
self.nvext
.as_ref()
.and_then(|nv| nv.guided_decoding_backend.clone())
})
.and_then(|nv| nv.guided_decoding_backend.as_ref()),
)
}
}
......@@ -199,6 +214,16 @@ impl OpenAIStopConditionsProvider for NvCreateCompletionRequest {
fn get_common_ignore_eos(&self) -> Option<bool> {
self.common.ignore_eos
}
/// Get the effective ignore_eos value, considering both CommonExt and NvExt.
/// CommonExt (root-level) takes precedence over NvExt.
fn get_ignore_eos(&self) -> Option<bool> {
choose_with_deprecation(
"ignore_eos",
self.get_common_ignore_eos().as_ref(),
NvExtProvider::nvext(self).and_then(|nv| nv.ignore_eos.as_ref()),
)
}
}
#[derive(Builder)]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment