fix: adjust vehicle creation, validation and tests on merge actions

Author: jsrwell

crates/ev-core/src/domain/vehicle.rs

@@ -13,7 +13,10 @@ use crate::error::ValidationError;
 use crate::validation::Validate;
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
 pub struct Vehicle {
+    #[serde(rename = "$schema", skip_serializing_if = "Option::is_none")]
+    pub schema_url: Option<String>,
     pub schema_version: String,
     pub make: SlugName,
     pub model: SlugName,
@@ -198,6 +201,7 @@ mod tests {
 
     fn create_test_vehicle() -> Vehicle {
         Vehicle {
+            schema_url: None,
             schema_version: "1.0.0".to_string(),
             make: SlugName {
                 slug: "tesla".to_string(),

crates/ev-etl/src/ingest/reader.rs

@@ -107,19 +107,19 @@ fn parse_vehicle_file(path: &Path, base_dir: &Path) -> Result<Option<VehicleFile
             return Ok(None);
         }
 
-        let expected_base_name = format!("{}_{}", make_slug, model_slug);
-        if file_name == expected_base_name
-            || file_name.starts_with(&format!("{}_", expected_base_name))
-        {
-            if file_name == expected_base_name {
-                (Some(year), FileType::YearBase)
-            } else {
-                (Some(year), FileType::Variant)
-            }
-        } else if !file_name.contains('_') {
+        // Strict naming convention check
+        if file_name == model_slug {
             (Some(year), FileType::YearBase)
-        } else {
+        } else if file_name.starts_with(&format!("{}_", model_slug)) {
             (Some(year), FileType::Variant)
+        } else {
+            return Err(anyhow::anyhow!(
+                "Invalid file name '{}.json' in year {} folder. Expected exact match '{}' or starts with '{}_'",
+                file_name,
+                year,
+                model_slug,
+                model_slug
+            ));
         }
     } else {
         return Ok(None);

crates/ev-etl/src/merge/mod.rs

@@ -2,7 +2,7 @@ mod strategy;
 
 use std::collections::HashMap;
 
-use anyhow::Result;
+use anyhow::{Result, anyhow};
 use ev_core::Vehicle;
 use serde_json::Value;
 
@@ -19,11 +19,14 @@ pub fn merge_all(files: &[VehicleFile]) -> Result<Vec<Vehicle>> {
     }
 
     let mut vehicles = Vec::new();
+    let mut errors: Vec<String> = Vec::new();
 
     for ((make_slug, model_slug), model_files) in grouped {
         let base_file = model_files
             .iter()
             .find(|f| f.file_type == FileType::ModelBase);
+
+        // Base file is optional for the model, but if missing, year files must be complete
         let base_content = base_file
             .map(|f| f.content.clone())
             .unwrap_or(Value::Object(serde_json::Map::new()));
@@ -46,39 +49,69 @@ pub fn merge_all(files: &[VehicleFile]) -> Result<Vec<Vehicle>> {
                 .collect();
 
             if let Some(year_base_file) = year_base {
-                let merged_base = deep_merge(&base_content, &year_base_file.content);
+                let merged_year_base = deep_merge(&base_content, &year_base_file.content);
 
-                match serde_json::from_value::<Vehicle>(merged_base.clone()) {
+                match serde_json::from_value::<Vehicle>(merged_year_base.clone()) {
                     Ok(vehicle) => vehicles.push(vehicle),
                     Err(e) => {
-                        tracing::warn!(
-                            "Failed to parse base vehicle {}/{}/{}: {}",
+                        let msg = format!(
+                            "{}/{}/{}/{}: {}",
                             make_slug,
                             model_slug,
                             year,
+                            year_base_file
+                                .path
+                                .file_name()
+                                .unwrap_or_default()
+                                .to_string_lossy(),
                             e
                         );
+                        errors.push(msg);
                     }
                 }
 
                 for variant_file in variants {
-                    let merged_variant = deep_merge(&merged_base, &variant_file.content);
+                    let merged_variant = deep_merge(&merged_year_base, &variant_file.content);
 
                     match serde_json::from_value::<Vehicle>(merged_variant) {
                         Ok(vehicle) => vehicles.push(vehicle),
                         Err(e) => {
-                            tracing::warn!(
-                                "Failed to parse variant {:?}: {}",
-                                variant_file.path,
+                            let msg = format!(
+                                "{}/{}/{}/{}: {}",
+                                make_slug,
+                                model_slug,
+                                year,
+                                variant_file
+                                    .path
+                                    .file_name()
+                                    .unwrap_or_default()
+                                    .to_string_lossy(),
                                 e
                             );
+                            errors.push(msg);
                         }
                     }
                 }
+            } else {
+                // Critical Error: Year Base file missing
+                let msg = format!(
+                    "Missing Year Base file for {}/{} (Year {}). Expected file named '{}.json'",
+                    make_slug, model_slug, year, model_slug
+                );
+                errors.push(msg);
             }
         }
     }
 
+    if !errors.is_empty() {
+        let error_msg = format!(
+            "Failed to parse {} vehicle(s):\n{}",
+            errors.len(),
+            errors.join("\n")
+        );
+        return Err(anyhow!(error_msg));
+    }
+
     vehicles.sort_by(|a, b| {
         a.make
             .slug

…hicles/byd/dolphin/2024/byd_dolphin.json …e_vehicles/byd/dolphin/2024/dolphin.jsonfixtures/sample_vehicles/byd/dolphin/2024/byd_dolphin.json renamed to fixtures/sample_vehicles/byd/dolphin/2024/dolphin.json fixtures/sample_vehicles/byd/dolphin/2024/byd_dolphin.json renamed to fixtures/sample_vehicles/byd/dolphin/2024/dolphin.json

@@ -3,6 +3,7 @@ use ev_core::{Battery, Charging, Powertrain, Range, SlugName, Vehicle, VehicleTy
 fn create_base_vehicle() -> Vehicle {
     use ev_core::Drivetrain;
     Vehicle {
+        schema_url: None,
         schema_version: "1.0.0".to_string(),
         make: SlugName {
             slug: "tesla".into(),

…es/tesla/model_3/2024/tesla_model_3.json …vehicles/tesla/model_3/2024/model_3.jsonfixtures/sample_vehicles/tesla/model_3/2024/tesla_model_3.json renamed to fixtures/sample_vehicles/tesla/model_3/2024/model_3.json fixtures/sample_vehicles/tesla/model_3/2024/tesla_model_3.json renamed to fixtures/sample_vehicles/tesla/model_3/2024/model_3.json

@@ -15,10 +15,10 @@ fn test_scan_directory_structure() {
     // Create structure: tesla/model_y/2024/tesla_model_y.json (YearBase)
     let year_dir = model_dir.join("2024");
     fs::create_dir_all(&year_dir).unwrap();
-    fs::write(year_dir.join("tesla_model_y.json"), "{}").unwrap();
+    fs::write(year_dir.join("model_y.json"), "{}").unwrap();
 
-    // Create structure: tesla/model_y/2024/tesla_model_y_perf.json (Variant)
-    fs::write(year_dir.join("tesla_model_y_perf.json"), "{}").unwrap();
+    // Create structure: tesla/model_y/2024/model_y_perf.json (Variant)
+    fs::write(year_dir.join("model_y_perf.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
 
@@ -72,7 +72,7 @@ fn test_scan_directory_hidden_files() {
     // Hidden file should be ignored
     fs::write(year_dir.join(".hidden.json"), "{}").unwrap();
     // Valid file
-    fs::write(year_dir.join("tesla_model_3.json"), "{}").unwrap();
+    fs::write(year_dir.join("model_3.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
     assert_eq!(files.len(), 1);
@@ -103,7 +103,7 @@ fn test_scan_directory_year_base_without_underscore() {
     let year_dir = model_dir.join("2024");
     fs::create_dir_all(&year_dir).unwrap();
     // File without underscore pattern should still be YearBase
-    fs::write(year_dir.join("base.json"), "{}").unwrap();
+    fs::write(year_dir.join("dolphin.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
     assert_eq!(files.len(), 1);
@@ -119,8 +119,8 @@ fn test_scan_directory_sorting() {
     let year_dir = model_dir.join("2024");
     fs::create_dir_all(&year_dir).unwrap();
     fs::write(model_dir.join("base.json"), "{}").unwrap();
-    fs::write(year_dir.join("tesla_model_3.json"), "{}").unwrap();
-    fs::write(year_dir.join("tesla_model_3_lr.json"), "{}").unwrap();
+    fs::write(year_dir.join("model_3.json"), "{}").unwrap();
+    fs::write(year_dir.join("model_3_lr.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
     assert_eq!(files.len(), 3);
@@ -141,7 +141,7 @@ fn test_scan_directory_non_json_files() {
     fs::create_dir_all(&year_dir).unwrap();
     fs::write(year_dir.join("readme.txt"), "text").unwrap();
     fs::write(year_dir.join("config.yaml"), "yaml").unwrap();
-    fs::write(year_dir.join("tesla_model_3.json"), "{}").unwrap();
+    fs::write(year_dir.join("model_3.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
     assert_eq!(files.len(), 1);
@@ -162,12 +162,12 @@ fn test_scan_directory_multiple_makes() {
     // Tesla
     let tesla_dir = root.join("tesla").join("model_3").join("2024");
     fs::create_dir_all(&tesla_dir).unwrap();
-    fs::write(tesla_dir.join("tesla_model_3.json"), "{}").unwrap();
+    fs::write(tesla_dir.join("model_3.json"), "{}").unwrap();
 
     // BYD
     let byd_dir = root.join("byd").join("dolphin").join("2024");
     fs::create_dir_all(&byd_dir).unwrap();
-    fs::write(byd_dir.join("byd_dolphin.json"), "{}").unwrap();
+    fs::write(byd_dir.join("dolphin.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
     assert_eq!(files.len(), 2);
@@ -186,12 +186,13 @@ fn test_scan_directory_variant_with_underscore() {
     let year_dir = model_dir.join("2024");
     fs::create_dir_all(&year_dir).unwrap();
 
-    // YearBase file: tesla_model_3.json
-    fs::write(year_dir.join("tesla_model_3.json"), "{}").unwrap();
-    // Variant file with underscore suffix: tesla_model_3_long_range.json
-    fs::write(year_dir.join("tesla_model_3_long_range.json"), "{}").unwrap();
+    // YearBase file: model_3.json
+    fs::write(year_dir.join("model_3.json"), "{}").unwrap();
+    // Variant file with underscore suffix: model_3_long_range.json
+    fs::write(year_dir.join("model_3_long_range.json"), "{}").unwrap();
     // Another variant: a file with underscore but not matching expected pattern exactly
-    fs::write(year_dir.join("some_other_variant.json"), "{}").unwrap();
+    // UPDATE: to be valid, must start with model_slug
+    fs::write(year_dir.join("model_3_other.json"), "{}").unwrap();
 
     let files = scan_directory(root).unwrap();
 

tests/unit/ev-core/domain/vehicle_test.rs

@@ -26,7 +26,7 @@ fn create_valid_test_dataset() -> TempDir {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3.json"),
+        base.join("tesla/model_3/2024/model_3.json"),
         serde_json::to_string_pretty(&vehicle).unwrap(),
     )
     .expect("Failed to write");
@@ -244,8 +244,9 @@ fn create_invalid_test_dataset() -> TempDir {
 
     std::fs::create_dir_all(base.join("tesla/model_3/2024")).expect("Failed to create dirs");
 
-    // Invalid vehicle: missing battery capacity, charge ports, range, sources
+    // Invalid vehicle: missing battery capacity, charge ports, range, sources AND unknown field
     let vehicle = json!({
+        "unknown_field": "should_fail",
         "schema_version": "1.0.0",
         "make": {"slug": "tesla", "name": "Tesla"},
         "model": {"slug": "model_3", "name": "Model 3"},
@@ -261,7 +262,7 @@ fn create_invalid_test_dataset() -> TempDir {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3.json"),
+        base.join("tesla/model_3/2024/model_3.json"),
         serde_json::to_string_pretty(&vehicle).unwrap(),
     )
     .expect("Failed to write");
@@ -285,7 +286,7 @@ fn test_run_validation_with_invalid_vehicle() {
 }
 
 #[test]
-fn test_run_pipeline_filters_invalid_vehicles() {
+fn test_run_pipeline_fails_on_invalid_vehicles() {
     let temp_dir = TempDir::new().expect("Failed to create temp dir");
     let base = temp_dir.path();
 
@@ -308,8 +309,9 @@ fn test_run_pipeline_filters_invalid_vehicles() {
         "sources": [{"type": "oem", "title": "Tesla", "url": "https://tesla.com", "accessed_at": "2024-01-01"}]
     });
 
-    // Invalid vehicle: missing required fields
+    // Invalid vehicle: missing required fields AND has unknown field
     let invalid_vehicle = json!({
+        "unknown_field": "should_fail",
         "schema_version": "1.0.0",
         "make": {"slug": "byd", "name": "BYD"},
         "model": {"slug": "dolphin", "name": "Dolphin"},
@@ -325,13 +327,13 @@ fn test_run_pipeline_filters_invalid_vehicles() {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3.json"),
+        base.join("tesla/model_3/2024/model_3.json"),
         serde_json::to_string_pretty(&valid_vehicle).unwrap(),
     )
     .expect("Failed to write");
 
     std::fs::write(
-        base.join("byd/dolphin/2024/byd_dolphin.json"),
+        base.join("byd/dolphin/2024/dolphin.json"),
         serde_json::to_string_pretty(&invalid_vehicle).unwrap(),
     )
     .expect("Failed to write");
@@ -345,14 +347,7 @@ fn test_run_pipeline_filters_invalid_vehicles() {
         false,
     );
 
-    // Pipeline should succeed but skip invalid vehicle
+    // Pipeline should fail due to invalid vehicle (fail-fast)
     let result = run_pipeline(&cli);
-    assert!(result.is_ok());
-
-    // Verify output contains only valid vehicle
-    let output_json = std::fs::read_to_string(output_dir.path().join("vehicles.json")).unwrap();
-    let output: serde_json::Value = serde_json::from_str(&output_json).unwrap();
-
-    // Should have only 1 vehicle (Tesla, not BYD)
-    assert_eq!(output["vehicle_count"], 1);
+    assert!(result.is_err());
 }

tests/unit/ev-etl/ingest/reader_test.rs

@@ -28,7 +28,7 @@ fn create_test_dataset() -> TempDir {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3.json"),
+        base.join("tesla/model_3/2024/model_3.json"),
         serde_json::to_string_pretty(&year_base).unwrap(),
     )
     .expect("Failed to write year base");
@@ -51,7 +51,7 @@ fn create_test_dataset() -> TempDir {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3_long_range.json"),
+        base.join("tesla/model_3/2024/model_3_long_range.json"),
         serde_json::to_string_pretty(&variant).unwrap(),
     )
     .expect("Failed to write variant");
@@ -248,16 +248,15 @@ fn test_merge_all_invalid_base_vehicle() {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3.json"),
+        base.join("tesla/model_3/2024/model_3.json"),
         serde_json::to_string_pretty(&invalid_base).unwrap(),
     )
     .expect("Failed to write");
 
     let files = ev_etl::ingest::load_dataset(base).expect("Failed to load dataset");
 
-    let vehicles = ev_etl::merge::merge_all(&files).expect("Failed to merge");
-
-    assert!(vehicles.is_empty());
+    let vehicles = ev_etl::merge::merge_all(&files);
+    assert!(vehicles.is_err());
 }
 
 #[test]
@@ -283,7 +282,7 @@ fn test_merge_all_invalid_variant_vehicle() {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3.json"),
+        base.join("tesla/model_3/2024/model_3.json"),
         serde_json::to_string_pretty(&valid_base).unwrap(),
     )
     .expect("Failed to write base");
@@ -294,15 +293,13 @@ fn test_merge_all_invalid_variant_vehicle() {
     });
 
     std::fs::write(
-        base.join("tesla/model_3/2024/tesla_model_3_invalid.json"),
+        base.join("tesla/model_3/2024/model_3_invalid.json"),
         serde_json::to_string_pretty(&invalid_variant).unwrap(),
     )
     .expect("Failed to write variant");
 
     let files = ev_etl::ingest::load_dataset(base).expect("Failed to load dataset");
 
-    let vehicles = ev_etl::merge::merge_all(&files).expect("Failed to merge");
-
-    assert_eq!(vehicles.len(), 1);
-    assert_eq!(vehicles[0].trim.slug, "base");
+    let vehicles = ev_etl::merge::merge_all(&files);
+    assert!(vehicles.is_err());
 }