Skip to content

Commit 4d61d66

Browse files
grahamkingkmkelle-nv
authored andcommitted
fix: Handle invalid JSON in config.json (#3043)
Signed-off-by: Graham King <[email protected]> Signed-off-by: Kristen Kelleher <[email protected]>
1 parent 4616c94 commit 4d61d66

File tree

7 files changed

+170
-18
lines changed

7 files changed

+170
-18
lines changed

Cargo.lock

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/llm/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ erased-serde = { version = "0.4" }
147147
itertools = { version = "0.14.0" }
148148
minijinja = { version = "2.10.2", features = ["loader"] }
149149
minijinja-contrib = { version = "2.10.2", features = ["pycompat"] }
150+
json-five = { version = "0.3" }
150151

151152
# GGUF
152153
ggus = "0.4.0"

lib/llm/src/lib.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,56 @@ pub fn file_json_field<T: serde::de::DeserializeOwned>(
9999
})
100100
}
101101

102+
/// Pretty-print the part of JSON that has an error.
103+
pub fn log_json_err(filename: &str, json: &str, err: &serde_json::Error) {
104+
const ERROR_PREFIX: &str = ">> ";
105+
106+
// Only log errors that relate to the content of the JSON file
107+
if !(err.is_syntax() || err.is_data()) {
108+
return;
109+
}
110+
// These are 1 based for humans so subtract
111+
let line = err.line().saturating_sub(1);
112+
let column = err.column().saturating_sub(1);
113+
114+
let json_lines: Vec<&str> = json.lines().collect();
115+
if json_lines.is_empty() {
116+
tracing::error!("JSON parsing error in {filename}: File is empty.");
117+
return;
118+
}
119+
120+
// Two lines before
121+
let start_index = (line - 2).max(0);
122+
// The problem line and two lines after
123+
let end_index = (line + 3).min(json_lines.len());
124+
125+
// Collect the context
126+
let mut context_lines: Vec<String> = (start_index..end_index)
127+
.map(|i| {
128+
if i == line {
129+
format!("{ERROR_PREFIX}{}", json_lines[i])
130+
} else {
131+
// Six places because tokenizer.json is very long
132+
format!("{:06} {}", i + 1, json_lines[i])
133+
}
134+
})
135+
.collect();
136+
137+
// Insert the column indicator
138+
let col_indicator = "_".to_string().repeat(column + ERROR_PREFIX.len()) + "^";
139+
let error_in_context_idx = line - start_index;
140+
if error_in_context_idx < context_lines.len() {
141+
context_lines.insert(error_in_context_idx + 1, col_indicator);
142+
}
143+
144+
tracing::error!(
145+
"JSON parsing error in {filename}: Line {}, column {}:\n{}",
146+
err.line(),
147+
err.column(),
148+
context_lines.join("\n")
149+
);
150+
}
151+
102152
#[cfg(test)]
103153
mod file_json_field_tests {
104154
use super::file_json_field;

lib/llm/src/model_card.rs

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,16 @@ impl ModelDeploymentCard {
174174

175175
/// Load a model deployment card from a JSON file
176176
pub fn load_from_json_file<P: AsRef<Path>>(file: P) -> std::io::Result<Self> {
177-
Ok(serde_json::from_str(&std::fs::read_to_string(file)?)?)
177+
let contents = std::fs::read_to_string(&file)?;
178+
Ok(serde_json::from_str(&contents).inspect_err(|err| {
179+
crate::log_json_err(&file.as_ref().display().to_string(), &contents, err)
180+
})?)
178181
}
179182

180183
/// Load a model deployment card from a JSON string
181-
pub fn load_from_json_str(json: &str) -> Result<Self, anyhow::Error> {
182-
Ok(serde_json::from_str(json)?)
184+
pub fn load_from_json_str(contents: &str) -> Result<Self, anyhow::Error> {
185+
Ok(serde_json::from_str(contents)
186+
.inspect_err(|err| crate::log_json_err("unknown", contents, err))?)
183187
}
184188

185189
//
@@ -227,7 +231,15 @@ impl ModelDeploymentCard {
227231
let p = checked_file.path().ok_or_else(||
228232
anyhow::anyhow!("Tokenizer is URL-backed ({:?}); call move_from_nats() before tokenizer_hf()", checked_file.url())
229233
)?;
230-
HfTokenizer::from_file(p).map_err(anyhow::Error::msg)
234+
HfTokenizer::from_file(p)
235+
.inspect_err(|err| {
236+
if let Some(serde_err) = err.downcast_ref::<serde_json::Error>()
237+
&& let Ok(contents) = std::fs::read_to_string(p)
238+
{
239+
crate::log_json_err(&p.display().to_string(), &contents, serde_err);
240+
}
241+
})
242+
.map_err(anyhow::Error::msg)
231243
}
232244
Some(TokenizerKind::GGUF(t)) => Ok(*t.clone()),
233245
None => {
@@ -627,11 +639,18 @@ impl HFConfig {
627639
fn from_json_file<P: AsRef<Path>>(file: P) -> Result<Arc<dyn ModelInfo>> {
628640
let file_path = file.as_ref();
629641
let contents = std::fs::read_to_string(file_path)?;
630-
let mut config: Self = serde_json::from_str(&contents)?;
642+
let mut config: Self = json_five::from_str(&contents)
643+
.inspect_err(|err| {
644+
tracing::error!(path=%file_path.display(), %err, "Failed to parse config.json as JSON5");
645+
})?;
631646
if config.text_config.is_none() {
632-
let text_config: HFTextConfig = serde_json::from_str(&contents)?;
647+
let text_config: HFTextConfig = json_five::from_str(&contents)
648+
.inspect_err(|err| {
649+
tracing::error!(path=%file_path.display(), %err, "Failed to parse text config from config.json as JSON5");
650+
})?;
633651
config.text_config = Some(text_config);
634652
}
653+
635654
// Sometimes bos_token_id is in generation_config.json not config.json
636655
let Some(text_config) = config.text_config.as_mut() else {
637656
anyhow::bail!(
@@ -882,4 +901,14 @@ mod tests {
882901
assert_eq!(config.bos_token_id(), 200000);
883902
Ok(())
884903
}
904+
905+
/// The Python JSON parser accepts `Infinity` as a numeric value. This is explicitly against the
906+
/// JSON spec, but inevitably people rely on it, so we have to allow it.
907+
/// We treat that file as JSON5 (a lenient superset of JSON) to be able to parse it.
908+
#[test]
909+
fn test_invalid_json_but_py_accepts_it() {
910+
dynamo_runtime::logging::init();
911+
let path = "tests/data/sample-models/NVIDIA-Nemotron-Nano-12B-v2-Base/config.json";
912+
let _ = HFConfig::from_json_file(path).unwrap();
913+
}
885914
}

lib/llm/src/preprocessor/prompt/template.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,12 @@ impl PromptFormatter {
3030
mdc.display_name
3131
);
3232
};
33-
let content = std::fs::read_to_string(file)
33+
let contents = std::fs::read_to_string(file)
3434
.with_context(|| format!("fs:read_to_string '{}'", file.display()))?;
35-
let mut config: ChatTemplate = serde_json::from_str(&content)?;
35+
let mut config: ChatTemplate =
36+
serde_json::from_str(&contents).inspect_err(|err| {
37+
crate::log_json_err(&file.display().to_string(), &contents, err)
38+
})?;
3639

3740
// Some HF model (i.e. meta-llama/Llama-4-Maverick-17B-128E-Instruct-FP8)
3841
// stores the chat template in a separate file, we check if the file exists and

lib/llm/src/request_template.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2022-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
3-
// Licensed under the Apache License, Version 2.0 (the "License");
4-
// you may not use this file except in compliance with the License.
5-
// You may obtain a copy of the License at
6-
// http://www.apache.org/licenses/LICENSE-2.0
7-
// Unless required by applicable law or agreed to in writing, software
8-
// distributed under the License is distributed on an "AS IS" BASIS,
9-
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10-
// See the License for the specific language governing permissions and
11-
// limitations under the License.
123

134
use anyhow::Result;
145
use serde::{Deserialize, Serialize};
@@ -24,7 +15,8 @@ pub struct RequestTemplate {
2415
impl RequestTemplate {
2516
pub fn load(path: &Path) -> Result<Self> {
2617
let template = std::fs::read_to_string(path)?;
27-
let template: Self = serde_json::from_str(&template)?;
18+
let template: Self = serde_json::from_str(&template)
19+
.inspect_err(|err| crate::log_json_err(&path.display().to_string(), &template, err))?;
2820
Ok(template)
2921
}
3022
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
{
2+
"architectures": [
3+
"NemotronHForCausalLM"
4+
],
5+
"attention_bias": false,
6+
"attention_dropout": 0.0,
7+
"auto_map": {
8+
"AutoConfig": "configuration_nemotron_h.NemotronHConfig",
9+
"AutoModelForCausalLM": "modeling_nemotron_h.NemotronHForCausalLM"
10+
},
11+
"bos_token_id": 1,
12+
"chunk_size": 128,
13+
"conv_kernel": 4,
14+
"eos_token_id": 2,
15+
"head_dim": 128,
16+
"hidden_dropout": 0.0,
17+
"hidden_size": 5120,
18+
"hybrid_override_pattern": "M-M-M-M*-M-M-M-M*-M-M-M-M*-M-M-M-M*-M-M-M-M*-M-M-M-M*-M-M-M-M-",
19+
"initializer_range": 0.02,
20+
"intermediate_size": 20480,
21+
"layer_norm_epsilon": 1e-05,
22+
"mamba_head_dim": 80,
23+
"mamba_hidden_act": "silu",
24+
"mamba_num_groups": 8,
25+
"mamba_num_heads": 128,
26+
"mamba_proj_bias": false,
27+
"mamba_state_dim": 128,
28+
"max_position_embeddings": 131072,
29+
"mlp_bias": false,
30+
"mlp_hidden_act": "relu2",
31+
"model_type": "nemotron_h",
32+
"n_groups": 8,
33+
"num_attention_heads": 40,
34+
"num_hidden_layers": 62,
35+
"num_key_value_heads": 8,
36+
"num_logits_to_keep": 1,
37+
"num_query_groups": 8,
38+
"pad_token_id": 0,
39+
"rescale_prenorm_residual": true,
40+
"residual_in_fp32": false,
41+
"rms_norm_eps": 1e-05,
42+
"sliding_window": null,
43+
"ssm_state_size": 128,
44+
"tie_word_embeddings": false,
45+
"time_step_floor": 0.0001,
46+
"time_step_limit": [
47+
0.0,
48+
Infinity
49+
],
50+
"time_step_max": 0.1,
51+
"time_step_min": 0.001,
52+
"time_step_rank": 256,
53+
"torch_dtype": "bfloat16",
54+
"transformers_version": "4.53.2",
55+
"use_bias": false,
56+
"use_cache": true,
57+
"use_conv_bias": true,
58+
"use_mamba_kernels": true,
59+
"vocab_size": 131072
60+
}

0 commit comments

Comments
 (0)