diff --git a/CHANGELOG.md b/CHANGELOG.md index 03aff7d..2cfcfe3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - `plans/code-review-v2.md` (completed action items) +### Fixed +- `trash.csv` Config column now lists only the copies to delete: for cars where just one tune is useless (e.g. owned 332 + best-everywhere 323) only the dead tune is shown instead of all owned configs +- Cars the records site knows but has no times for (typically new prize cars, returned as `200` with an empty body) no longer abort `fetch` after 3 retries — they are skipped with an info log +- Empty timing files are now treated like missing files instead of producing `EOF while parsing a value` parse errors + ## [0.3.0] - 2026-05-07 ### Added diff --git a/src/http.rs b/src/http.rs index c76a593..aca3a3b 100644 --- a/src/http.rs +++ b/src/http.rs @@ -21,10 +21,13 @@ fn retry_delay(attempt: usize) -> std::time::Duration { /// Writes to a temporary file first, then renames on success, so a failed /// download never deletes the existing file. /// -/// Retries up to `MAX_FETCH_RETRIES` times on transient failures (server errors, -/// connection errors, or empty responses). Permanent HTTP errors (4xx) fail immediately. +/// Retries up to `MAX_FETCH_RETRIES` times on transient failures (server errors +/// or connection errors). Permanent conditions fail immediately: client HTTP +/// errors (4xx) and empty bodies ([`AppError::EmptyResponse`] — the records API +/// reports "no data submitted yet" as a 200 with an empty body, which no amount +/// of retrying can change). pub async fn get_file(file_url: &str, local_file_path: &str) -> Result<(), AppError> { - /// Maximum number of retries when the server returns an empty response. + /// Maximum number of retries on transient failures (5xx, connection errors). const MAX_FETCH_RETRIES: usize = 3; // Parse the URL once so malformed URLs are caught before any network I/O. @@ -58,18 +61,9 @@ pub async fn get_file(file_url: &str, local_file_path: &str) -> Result<(), AppEr match response.text().await { Ok(content) => { if content.is_empty() { - if attempt < MAX_FETCH_RETRIES { - log::warn!( - "empty response from {file_url} (attempt {}/{MAX_FETCH_RETRIES}), retrying...", - attempt + 1, - ); - tokio::time::sleep(retry_delay(attempt)).await; - attempt += 1; - continue; - } - return Err(AppError::Network(format!( - "Empty response from {file_url} after {MAX_FETCH_RETRIES} retries" - ))); + // "Known resource, no data yet" — deterministic, so + // fail immediately instead of burning retries. + return Err(AppError::EmptyResponse { url: file_url.to_string() }); } fs::write(&tmp_path, &content).map_err(AppError::Io)?; return fs::rename(&tmp_path, local_file_path).map_err(AppError::Io); diff --git a/src/output/mod.rs b/src/output/mod.rs index 7b0de7c..de045bb 100644 --- a/src/output/mod.rs +++ b/src/output/mod.rs @@ -359,6 +359,11 @@ pub fn plan_trash_rules( /// categories 1 and 2 because they belong in `improve.csv` instead (unless /// caught by category 3). Prize cars and RQ ≥ 80 are always excluded. /// +/// The Config column lists only the copies to delete: for category 2 just the +/// final builds that are not best on any track (a best-on-213-tracks 323 next +/// to a useless 332 shows only `332`); for categories 1 and 3 all owned +/// configs (the whole car goes). +/// /// `improve` holds the precomputed improve entries (see /// [`collect_improve_entries`]) so callers can share one computation with /// [`plan_trash_rules`]. @@ -393,31 +398,41 @@ pub fn write_trash_csv( let trash: Vec = owned .iter() - .filter(|guid| { - if low_value_guids.contains(guid.as_str()) { - return true; - } - let Some(best_configs) = counts.guid_to_rid.get(*guid) else { - return true; - }; - if let Some(configs) = counts.owned_configs.get(*guid) { - // Any copy with a final build not matching any best config → trash - if configs.iter().any(|actual_config| { - is_final_build(actual_config) - && !best_configs.iter().any(|(_, nc)| nc == actual_config) - }) { - return true; - } - } - false - }) .filter_map(|guid| { let car = counts.car_map.get(guid)?; if car.prize || car.rq >= TRASH_MIN_RQ { return None; } - let config = - counts.owned_configs.get(guid).map(|configs| configs.join("/")).unwrap_or_default(); + // Categories 1 (not best anywhere) and 3 (low-value fodder) trash + // the whole car — every owned copy goes. Category 2 trashes only + // the copies whose final build is not best on any track; valuable + // copies of the same car (e.g. a best 323 next to a useless 332) + // stay and must not appear in the Config column. + let whole_car = + low_value_guids.contains(guid.as_str()) || !counts.guid_to_rid.contains_key(guid); + let config = if whole_car { + counts.owned_configs.get(guid).map(|configs| configs.join("/")).unwrap_or_default() + } else { + let best_configs = &counts.guid_to_rid[guid]; + counts + .owned_configs + .get(guid) + .map(|configs| { + configs + .iter() + .filter(|actual_config| { + is_final_build(actual_config) + && !best_configs.iter().any(|(_, nc)| nc == *actual_config) + }) + .cloned() + .collect::>() + }) + .unwrap_or_default() + .join("/") + }; + if !whole_car && config.is_empty() { + return None; + } Some(TrashEntry { car, config }) }) .collect(); @@ -1395,6 +1410,43 @@ mod tests { ); } + #[test] + fn test_write_trash_csv_partial_copies_shows_only_deletable_config() { + let ctx = AnalysisContext::new(); + + // Car_A is best on 3 tracks with 323; player owns a 332 copy too. + let mut rec_323 = make_record("Car_A", 10, 90.0, "guid_a"); + rec_323.config = "323".to_string(); + rec_323.normalized_config = "323".to_string(); + + for i in 0..3 { + ctx.best_cars + .insert((format!("Track{i}"), 10, "Dry".to_string()), vec![rec_323.clone()]); + } + + let car_list = vec![make_car("Car_A", 10, "guid_a")]; + + let mut owned_configs = HashMap::new(); + owned_configs.insert("guid_a".to_string(), vec!["332".to_string(), "323".to_string()]); + + let counts = compute_car_counts(&ctx, car_list, owned_configs, HashMap::new()); + + let owned: HashSet = vec!["guid_a".to_string()].into_iter().collect(); + + let dir = tempfile::tempdir().unwrap(); + let output_dir = dir.path().to_str().unwrap(); + write_trash_csv(output_dir, &counts, &owned, None, &[]).unwrap(); + + let content = fs::read_to_string(dir.path().join("trash.csv")).unwrap(); + let lines: Vec<&str> = content.lines().collect(); + + assert_eq!(lines.len(), 2, "Car_A should be trash via its 332 copy, got: {lines:?}"); + assert_eq!( + lines[1], "10,TestBrand,TestModel,2020,332", + "Config column must list only the deletable 332 copy, not 332/323" + ); + } + // ======================================================================= // compare_records tests // ======================================================================= diff --git a/src/parsing/mod.rs b/src/parsing/mod.rs index 14756f8..e92dcb5 100644 --- a/src/parsing/mod.rs +++ b/src/parsing/mod.rs @@ -97,6 +97,12 @@ pub fn parse_car_times( } }; + // An empty file means "no timing data" (the API reports this as an empty + // body) — same as a missing file, not a parse error. + if file_content.trim().is_empty() { + return Ok(()); + } + // Parse JSON data let json_data: Value = match serde_json::from_str(&file_content) { Ok(v) => v, @@ -399,12 +405,23 @@ pub async fn fetch_times(cars_db_path: &str, times_dir: &str) -> Result<(), AppE } /// Downloads timing data for a single car from the remote API. +/// +/// An empty response means the records site knows the car but no times have +/// been submitted yet (typical for freshly released prize cars) — treated as +/// a skip, not a failure, so the overall fetch is not aborted. async fn fetch_car(rid: &str, times_dir: &str) -> Result<(), AppError> { let file_name = rid.to_owned() + ".json"; let url = format!("{CAR_TIMING_API_BASE}{rid}"); let local_file_path = format!("{times_dir}/{file_name}"); log::info!("Fetching {url}."); - top_drives::get_file(&url, &local_file_path).await + match top_drives::get_file(&url, &local_file_path).await { + Ok(()) => Ok(()), + Err(AppError::EmptyResponse { .. }) => { + log::info!("no records on the site yet for {rid} — skipped"); + Ok(()) + } + Err(e) => Err(e), + } } // --------------------------------------------------------------------------- @@ -734,6 +751,30 @@ mod tests { assert_eq!(ctx.best_cars.len(), 0, "No entries for missing file"); } + #[test] + fn test_parse_car_times_empty_file_treated_as_missing() { + let ctx = AnalysisContext::new(); + let car = Car { + rid: "EmptyCar_2020".to_string(), + brand: "TestBrand".to_string(), + only_name: "TestModel".to_string(), + year: 2020, + rq: 85, + prize: false, + guid: "guid_test".to_string(), + }; + + let dir = tempfile::tempdir().unwrap(); + let times_dir = dir.path().to_str().unwrap(); + fs::write(dir.path().join("EmptyCar_2020.json"), "").unwrap(); + + let my_cars: HashSet = HashSet::new(); + // Empty file must be Ok(()) like a missing file, not a parse error. + parse_car_times(car, &my_cars, &ctx.best_cars, times_dir).unwrap(); + + assert_eq!(ctx.best_cars.len(), 0, "No entries for empty file"); + } + // ======================================================================= // Track name / type tests (from lib) // ======================================================================= diff --git a/src/types.rs b/src/types.rs index 62f6b54..ebad412 100644 --- a/src/types.rs +++ b/src/types.rs @@ -29,6 +29,16 @@ pub enum AppError { reason: String, }, + /// The server responded with 200 but an empty body. + /// + /// The records API uses this to signal "known resource, no data submitted yet" + /// (typically newly released prize cars). Deterministic — retrying cannot help. + #[error("Empty response from {url} (no data available on the server)")] + EmptyResponse { + /// The URL that returned the empty body + url: String, + }, + /// A required data file is missing #[error("Missing data file: {0}")] MissingFile(String), diff --git a/tests/http_tests.rs b/tests/http_tests.rs index 2ac3eef..34c6b9f 100644 --- a/tests/http_tests.rs +++ b/tests/http_tests.rs @@ -31,22 +31,23 @@ async fn test_get_file_success() { } #[tokio::test] -async fn test_get_file_empty_response_retries_and_fails() { +async fn test_get_file_empty_response_fails_immediately_without_retry() { let mut server = Server::new_async().await; - // Return 200 with empty body — triggers retry, then fails - let m = server - .mock("GET", "/data.json") - .with_status(200) - .with_body("") - .expect(4) // 1 initial + 3 retries - .create(); + // 200 with empty body means "no data available yet" — permanent, so exactly + // one request must be made (no retries) and a distinguishable error returned. + let m = server.mock("GET", "/data.json").with_status(200).with_body("").expect(1).create(); let dir = tempfile::tempdir().unwrap(); let out_path = dir.path().join("output.txt"); let url = format!("{}/data.json", server.url()); let result = get_file(&url, out_path.to_str().unwrap()).await; - assert!(result.is_err(), "Empty response should fail after retries"); + let err = result.expect_err("Empty response should fail immediately"); + assert!( + matches!(err, top_drives::AppError::EmptyResponse { .. }), + "Empty response should yield AppError::EmptyResponse, got: {err}" + ); + assert!(!out_path.exists(), "No file should be written for an empty response"); m.assert(); }