Skip to content

Commit 937b968

Browse files
ryan-lempkamichaelshin
ryan-lempka
authored and
michaelshin
committed
chore: deprecate duplicate params in nvext (#2754)
Signed-off-by: Ryan Lempka <[email protected]> Signed-off-by: Michael Shin <[email protected]>
1 parent e02cf6c commit 937b968

File tree

4 files changed

+136
-36
lines changed

4 files changed

+136
-36
lines changed

‎lib/llm/src/protocols/openai.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use super::{
88
ContentProvider,
99
common::{self, OutputOptionsProvider, SamplingOptionsProvider, StopConditionsProvider},
1010
};
11-
use crate::protocols::openai::common_ext::CommonExtProvider;
11+
use crate::protocols::openai::common_ext::{CommonExtProvider, choose_with_deprecation};
1212

1313
pub mod chat_completions;
1414
pub mod common_ext;
@@ -61,9 +61,11 @@ trait OpenAIStopConditionsProvider {
6161
/// Get the effective ignore_eos value, considering both CommonExt and NvExt.
6262
/// CommonExt (root-level) takes precedence over NvExt.
6363
fn get_ignore_eos(&self) -> Option<bool> {
64-
// Check common first (takes precedence), then fall back to nvext
65-
self.get_common_ignore_eos()
66-
.or_else(|| self.nvext().and_then(|nv| nv.ignore_eos))
64+
choose_with_deprecation(
65+
"ignore_eos",
66+
self.get_common_ignore_eos().as_ref(),
67+
self.nvext().and_then(|nv| nv.ignore_eos.as_ref()),
68+
)
6769
}
6870
}
6971

‎lib/llm/src/protocols/openai/chat_completions.rs‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ use crate::engines::ValidateRequest;
2121

2222
use super::{
2323
OpenAIOutputOptionsProvider, OpenAISamplingOptionsProvider, OpenAIStopConditionsProvider,
24-
common_ext::{CommonExt, CommonExtProvider},
24+
common_ext::{
25+
CommonExt, CommonExtProvider, choose_with_deprecation, emit_nvext_deprecation_warning,
26+
},
2527
nvext::NvExt,
2628
nvext::NvExtProvider,
2729
validate,
@@ -149,39 +151,52 @@ impl CommonExtProvider for NvCreateChatCompletionRequest {
149151

150152
/// Guided Decoding Options
151153
fn get_guided_json(&self) -> Option<&serde_json::Value> {
154+
// Note: This one needs special handling since it returns a reference
155+
if let Some(nvext) = &self.nvext
156+
&& nvext.guided_json.is_some()
157+
{
158+
emit_nvext_deprecation_warning("guided_json", true, self.common.guided_json.is_some());
159+
}
152160
self.common
153161
.guided_json
154162
.as_ref()
155163
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_json.as_ref()))
156164
}
157165

158166
fn get_guided_regex(&self) -> Option<String> {
159-
self.common
160-
.guided_regex
161-
.clone()
162-
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_regex.clone()))
167+
choose_with_deprecation(
168+
"guided_regex",
169+
self.common.guided_regex.as_ref(),
170+
self.nvext.as_ref().and_then(|nv| nv.guided_regex.as_ref()),
171+
)
163172
}
164173

165174
fn get_guided_grammar(&self) -> Option<String> {
166-
self.common
167-
.guided_grammar
168-
.clone()
169-
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_grammar.clone()))
175+
choose_with_deprecation(
176+
"guided_grammar",
177+
self.common.guided_grammar.as_ref(),
178+
self.nvext
179+
.as_ref()
180+
.and_then(|nv| nv.guided_grammar.as_ref()),
181+
)
170182
}
171183

172184
fn get_guided_choice(&self) -> Option<Vec<String>> {
173-
self.common
174-
.guided_choice
175-
.clone()
176-
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_choice.clone()))
185+
choose_with_deprecation(
186+
"guided_choice",
187+
self.common.guided_choice.as_ref(),
188+
self.nvext.as_ref().and_then(|nv| nv.guided_choice.as_ref()),
189+
)
177190
}
178191

179192
fn get_guided_decoding_backend(&self) -> Option<String> {
180-
self.common.guided_decoding_backend.clone().or_else(|| {
193+
choose_with_deprecation(
194+
"guided_decoding_backend",
195+
self.common.guided_decoding_backend.as_ref(),
181196
self.nvext
182197
.as_ref()
183-
.and_then(|nv| nv.guided_decoding_backend.clone())
184-
})
198+
.and_then(|nv| nv.guided_decoding_backend.as_ref()),
199+
)
185200
}
186201
}
187202

@@ -224,6 +239,16 @@ impl OpenAIStopConditionsProvider for NvCreateChatCompletionRequest {
224239
fn get_common_ignore_eos(&self) -> Option<bool> {
225240
self.common.ignore_eos
226241
}
242+
243+
/// Get the effective ignore_eos value, considering both CommonExt and NvExt.
244+
/// CommonExt (root-level) takes precedence over NvExt.
245+
fn get_ignore_eos(&self) -> Option<bool> {
246+
choose_with_deprecation(
247+
"ignore_eos",
248+
self.get_common_ignore_eos().as_ref(),
249+
NvExtProvider::nvext(self).and_then(|nv| nv.ignore_eos.as_ref()),
250+
)
251+
}
227252
}
228253

229254
impl OpenAIOutputOptionsProvider for NvCreateChatCompletionRequest {

‎lib/llm/src/protocols/openai/common_ext.rs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,35 @@ pub trait CommonExtProvider {
6767
fn get_guided_decoding_backend(&self) -> Option<String>;
6868
}
6969

70+
/// Helper function to emit deprecation warnings for nvext parameters
71+
pub fn emit_nvext_deprecation_warning(
72+
field_name: &str,
73+
nvext_has_value: bool,
74+
common_has_value: bool,
75+
) {
76+
if nvext_has_value && !common_has_value {
77+
tracing::warn!(
78+
"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."
79+
);
80+
} else if nvext_has_value && common_has_value {
81+
tracing::warn!(
82+
"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."
83+
);
84+
}
85+
}
86+
87+
/// Helper function to choose between common and nvext values with deprecation warnings
88+
pub fn choose_with_deprecation<T: Clone>(
89+
field: &'static str,
90+
common: Option<&T>,
91+
nv: Option<&T>,
92+
) -> Option<T> {
93+
if nv.is_some() {
94+
emit_nvext_deprecation_warning(field, true, common.is_some());
95+
}
96+
common.cloned().or_else(|| nv.cloned())
97+
}
98+
7099
#[cfg(test)]
71100
mod tests {
72101
use super::*;
@@ -163,4 +192,23 @@ mod tests {
163192
assert_eq!(common_ext.min_tokens, None);
164193
assert!(common_ext.validate().is_ok());
165194
}
195+
196+
#[test]
197+
fn test_choose_with_deprecation() {
198+
// Common takes precedence
199+
let result = choose_with_deprecation(
200+
"test_field",
201+
Some(&"common_value".to_string()),
202+
Some(&"nvext_value".to_string()),
203+
);
204+
assert_eq!(result, Some("common_value".to_string()));
205+
206+
// Fallback to nvext
207+
let result = choose_with_deprecation("test_field", None, Some(&"nvext_value".to_string()));
208+
assert_eq!(result, Some("nvext_value".to_string()));
209+
210+
// Both None
211+
let result: Option<String> = choose_with_deprecation("test_field", None, None);
212+
assert_eq!(result, None);
213+
}
166214
}

‎lib/llm/src/protocols/openai/completions.rs‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ use super::{
2424
ContentProvider, OpenAIOutputOptionsProvider, OpenAISamplingOptionsProvider,
2525
OpenAIStopConditionsProvider,
2626
common::{self, OutputOptionsProvider, SamplingOptionsProvider, StopConditionsProvider},
27-
common_ext::{CommonExt, CommonExtProvider},
27+
common_ext::{
28+
CommonExt, CommonExtProvider, choose_with_deprecation, emit_nvext_deprecation_warning,
29+
},
2830
nvext::{NvExt, NvExtProvider},
2931
validate,
3032
};
@@ -143,39 +145,52 @@ impl CommonExtProvider for NvCreateCompletionRequest {
143145

144146
/// Guided Decoding Options
145147
fn get_guided_json(&self) -> Option<&serde_json::Value> {
148+
// Note: This one needs special handling since it returns a reference
149+
if let Some(nvext) = &self.nvext
150+
&& nvext.guided_json.is_some()
151+
{
152+
emit_nvext_deprecation_warning("guided_json", true, self.common.guided_json.is_some());
153+
}
146154
self.common
147155
.guided_json
148156
.as_ref()
149157
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_json.as_ref()))
150158
}
151159

152160
fn get_guided_regex(&self) -> Option<String> {
153-
self.common
154-
.guided_regex
155-
.clone()
156-
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_regex.clone()))
161+
choose_with_deprecation(
162+
"guided_regex",
163+
self.common.guided_regex.as_ref(),
164+
self.nvext.as_ref().and_then(|nv| nv.guided_regex.as_ref()),
165+
)
157166
}
158167

159168
fn get_guided_grammar(&self) -> Option<String> {
160-
self.common
161-
.guided_grammar
162-
.clone()
163-
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_grammar.clone()))
169+
choose_with_deprecation(
170+
"guided_grammar",
171+
self.common.guided_grammar.as_ref(),
172+
self.nvext
173+
.as_ref()
174+
.and_then(|nv| nv.guided_grammar.as_ref()),
175+
)
164176
}
165177

166178
fn get_guided_choice(&self) -> Option<Vec<String>> {
167-
self.common
168-
.guided_choice
169-
.clone()
170-
.or_else(|| self.nvext.as_ref().and_then(|nv| nv.guided_choice.clone()))
179+
choose_with_deprecation(
180+
"guided_choice",
181+
self.common.guided_choice.as_ref(),
182+
self.nvext.as_ref().and_then(|nv| nv.guided_choice.as_ref()),
183+
)
171184
}
172185

173186
fn get_guided_decoding_backend(&self) -> Option<String> {
174-
self.common.guided_decoding_backend.clone().or_else(|| {
187+
choose_with_deprecation(
188+
"guided_decoding_backend",
189+
self.common.guided_decoding_backend.as_ref(),
175190
self.nvext
176191
.as_ref()
177-
.and_then(|nv| nv.guided_decoding_backend.clone())
178-
})
192+
.and_then(|nv| nv.guided_decoding_backend.as_ref()),
193+
)
179194
}
180195
}
181196

@@ -199,6 +214,16 @@ impl OpenAIStopConditionsProvider for NvCreateCompletionRequest {
199214
fn get_common_ignore_eos(&self) -> Option<bool> {
200215
self.common.ignore_eos
201216
}
217+
218+
/// Get the effective ignore_eos value, considering both CommonExt and NvExt.
219+
/// CommonExt (root-level) takes precedence over NvExt.
220+
fn get_ignore_eos(&self) -> Option<bool> {
221+
choose_with_deprecation(
222+
"ignore_eos",
223+
self.get_common_ignore_eos().as_ref(),
224+
NvExtProvider::nvext(self).and_then(|nv| nv.ignore_eos.as_ref()),
225+
)
226+
}
202227
}
203228

204229
#[derive(Builder)]

0 commit comments

Comments
 (0)