jan/top_drives
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix review findings: wishlist qualifying count, improve docs, and lint cleanup

Browse source 
- Count only configs meeting min_count as qualifying in the wishlist
  copy-coverage check (previously ALL guid_to_rid entries inflated the
  denominator, producing phantom wishlist entries), pre-computed once
  per guid instead of per record
- Update stale write_improve_csv doc to match single-best-target
  behavior including cross-copy exclusion
- Borrow owned configs for the trash CSV join instead of cloning the Vec
- Build owned_final_configs via filter_map + flat_map without the
  throwaway intermediate Vec
- Remove runtime assertions on literal constants (clippy
  assertions_on_constants) and replace a redundant closure with the
  function reference (clippy redundant_closure)
- Apply rustfmt across the crate and drop a duplicated test-section header
This commit is contained in:
Jan-Peter von Hunnius 2026-08-15 18:58:12 +02:00
parent 62688926c3
commit 2ab66a249b
8 changed files with 83 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.gitignore vendored
View file

@ -5,3 +5,5 @@ cars_repo.json
tracks_repo.json
times/
*.csv
AGENTS.md

7
src/commands/mod.rs
View file

@ -57,6 +57,11 @@ pub async fn run_analysis(
let ctx = AnalysisContext::new();
let owned = Arc::new(my_data.owned.clone());
let car_list = parse_all_times(owned, &ctx, cars_db_path, times_dir).await?;
let counts = compute_car_counts(&ctx, car_list, my_data.owned_configs.clone(), my_data.copy_count.clone());
let counts = compute_car_counts(
&ctx,
car_list,
my_data.owned_configs.clone(),
my_data.copy_count.clone(),
);
Ok((ctx, counts))
}

4
src/lib.rs
View file

@ -45,7 +45,9 @@ pub use context::{compute_car_counts, AnalysisContext, CarCounts};
pub use csv_util::{compare_cars, escape_csv_field};
pub use http::get_file;
pub use translate::{translate_track_name, translate_track_type};
pub use types::{is_final_build, normalize_to_final_build, AppError, BestCarRecord, Car, TripletKey};
pub use types::{
is_final_build, normalize_to_final_build, AppError, BestCarRecord, Car, TripletKey,
};
pub use user_data::{load_my_cars_data, MyCarsData};
#[cfg(feature = "test-utils")]

119
src/output/mod.rs
View file

@ -9,8 +9,8 @@ use std::io::Write;
use std::path::PathBuf;
use top_drives::{
compare_cars, escape_csv_field, is_final_build, AnalysisContext, AppError, BestCarRecord,
Car, CarCounts, TripletKey,
compare_cars, escape_csv_field, is_final_build, AnalysisContext, AppError, BestCarRecord, Car,
CarCounts, TripletKey,
};
// ---------------------------------------------------------------------------
@ -204,8 +204,8 @@ pub fn write_trash_csv(
if car.prize || car.rq >= TRASH_MIN_RQ {
return None;
}
let configs = counts.owned_configs.get(guid).cloned().unwrap_or_default();
let config = configs.join("/");
let config =
counts.owned_configs.get(guid).map(|configs| configs.join("/")).unwrap_or_default();
Some(TrashEntry { car, config })
})
.collect();
@ -302,6 +302,20 @@ pub fn compute_ideal_wishlist(
// Track the best qualifying entry per GUID (highest per-config count).
let mut guid_best_entry: HashMap<String, (BestCarRecord, usize)> = HashMap::new();
// Number of qualifying configs (track count >= min_count) per guid,
// pre-computed once so the loop below does not revisit guid_to_rid per record.
let qualifying_per_guid: HashMap<&str, usize> = counts
.guid_to_rid
.iter()
.map(|(guid, keys)| {
let qualifying = keys
.iter()
.filter(|key| counts.by_rid.get(*key).is_some_and(|(_, count)| *count >= min_count))
.count();
(guid.as_str(), qualifying)
})
.collect();
for (record, count) in counts.by_rid.values() {
if *count < min_count {
continue;
@ -309,11 +323,8 @@ pub fn compute_ideal_wishlist(
if owned.contains(&record.guid) {
let copies = counts.copy_count.get(&record.guid).copied().unwrap_or(1);
let qualifying_configs = counts
.guid_to_rid
.get(&record.guid)
.map(|s| s.len())
.unwrap_or(0);
let qualifying_configs =
qualifying_per_guid.get(record.guid.as_str()).copied().unwrap_or(0);
if copies >= qualifying_configs && qualifying_configs > 0 {
// Player has enough copies to cover all qualifying configs → skip
@ -337,9 +348,8 @@ pub fn compute_ideal_wishlist(
}
// Keep the entry with the highest per-config count (or first on tie).
let entry = guid_best_entry
.entry(record.guid.clone())
.or_insert_with(|| (record.clone(), *count));
let entry =
guid_best_entry.entry(record.guid.clone()).or_insert_with(|| (record.clone(), *count));
if count > &entry.1 {
*entry = (record.clone(), *count);
}
@ -351,7 +361,6 @@ pub fn compute_ideal_wishlist(
candidates
}
/// Writes wishlist.csv, add.csv, and remove.csv from a single computed ideal wishlist.
///
/// Computes the ideal wishlist once and derives all three output files from it,
@ -411,7 +420,8 @@ pub fn write_wishlist_files(
// First try: find the best entry for this car (if it's best on any track)
if let Some(keys) = counts.guid_to_rid.get(guid) {
// Pick the entry with the highest count (most impactful config)
if let Some(key) = keys.iter()
if let Some(key) = keys
.iter()
.max_by_key(|key| counts.by_rid.get(key).map(|(_, c)| *c).unwrap_or(0))
{
if let Some((record, count)) = counts.by_rid.get(key) {
@ -435,7 +445,7 @@ pub fn write_wishlist_files(
year: car.year,
rq: car.rq,
normalized_config: String::new(), // No config info available
track_count: 0, // Not best on any track
track_count: 0, // Not best on any track
});
}
@ -481,22 +491,21 @@ pub fn write_wishlist_files(
// CSV output: improve
// ---------------------------------------------------------------------------
/// Writes improve.csv — owned locked cars whose actual config is not yet a final build
/// (323/233/332), sorted by number of best tracks descending (most impactful upgrades first).
/// Writes improve.csv — owned locked cars with at least one unfinished copy,
/// sorted by number of best tracks descending (most impactful upgrades first).
///
/// Uses the **actual player config** from `my_cars.json` (engineMajor/weightMajor/chassisMajor)
/// to determine whether a car is finished. Only unfinished cars (config sum < 8) are included.
/// to determine whether each copy is finished. Cars whose copies are all final builds are
/// skipped entirely.
///
/// For each unfinished car, one row per normalized config that the car is best on is emitted.
/// The `Target` column shows the ideal final build from the timing data (not derived from the
/// player's current config). The `Count` column shows how many tracks this car is best on
/// under that specific normalized config.
/// For each car with an unfinished copy, one row is emitted showing the best remaining
/// target config (highest track count), excluding configs the player already owns as final
/// builds on another copy of the same car. The `Target` column shows the ideal final build
/// from the timing data (not derived from the player's current config). The `Count` column
/// shows how many tracks this car is best on under that specific normalized config.
///
/// Format: RQ,Brand,Model,Year,Target,Count
pub fn write_improve_csv(
output_dir: &str,
counts: &CarCounts,
) -> Result<usize, AppError> {
pub fn write_improve_csv(output_dir: &str, counts: &CarCounts) -> Result<usize, AppError> {
/// A car that needs upgrading to a final build, with its target config and track count.
struct ImproveEntry {
brand: String,
@ -512,16 +521,10 @@ pub fn write_improve_csv(
let owned_final_configs: HashSet<(String, String)> = counts
.owned_configs
.iter()
.filter_map(|(guid, configs)| {
let car = counts.car_map.get(guid)?;
let finals: Vec<(String, String)> = configs
.iter()
.filter(|c| is_final_build(c))
.map(|c| (car.rid.clone(), c.clone()))
.collect();
if finals.is_empty() { None } else { Some(finals) }
.filter_map(|(guid, configs)| counts.car_map.get(guid).map(|car| (car, configs)))
.flat_map(|(car, configs)| {
configs.iter().filter(|c| is_final_build(c)).map(|c| (car.rid.clone(), c.clone()))
})
.flatten()
.collect();
// Iterate over owned cars from the player's actual config data.
@ -536,19 +539,20 @@ pub fn write_improve_csv(
}
let Some(car) = counts.car_map.get(guid) else {
eprintln!(
"Warning: owned car {guid} not found in cars database — skipped for improve"
);
eprintln!("Warning: owned car {guid} not found in cars database — skipped for improve");
continue;
};
// Collect target configs, excluding any already owned on another copy of this car.
// Keep only the best remaining target (highest track count).
if let Some(keys) = counts.guid_to_rid.get(guid) {
let best_target = keys.iter()
let best_target = keys
.iter()
.filter_map(|key| {
let (record, track_count) = counts.by_rid.get(key)?;
if owned_final_configs.contains(&(car.rid.clone(), record.normalized_config.clone())) {
if owned_final_configs
.contains(&(car.rid.clone(), record.normalized_config.clone()))
{
return None;
}
Some((record.normalized_config.clone(), *track_count))
@ -736,7 +740,10 @@ mod tests {
let wishlist = compute_ideal_wishlist(&counts, &owned, 10, 300);
assert_eq!(wishlist.len(), 1, "Owned car with different config should be wishlisted");
assert_eq!(wishlist[0].0.normalized_config, "332", "Should wishlist the config player doesn't have");
assert_eq!(
wishlist[0].0.normalized_config, "332",
"Should wishlist the config player doesn't have"
);
assert!(wishlist[0].1 >= 10, "Track count should meet minimum");
}
@ -766,7 +773,7 @@ mod tests {
assert_eq!(wishlist.len(), 0, "Car with same config should be excluded from wishlist");
}
#[test]
#[test]
fn test_compute_ideal_wishlist_respects_max() {
let ctx = AnalysisContext::new();
@ -851,7 +858,7 @@ mod tests {
"RQ,Brand,Model,Year,FinalBuild,Count",
&[entry],
|_, _| std::cmp::Ordering::Equal,
|e, file| format_counted_row(e, file),
format_counted_row,
)
.unwrap();
@ -912,10 +919,8 @@ mod tests {
rec_332.normalized_config = "332".to_string();
for i in 0..5 {
ctx.best_cars.insert(
(format!("Track{i}"), 70, "Dry".to_string()),
vec![rec_332.clone()],
);
ctx.best_cars
.insert((format!("Track{i}"), 70, "Dry".to_string()), vec![rec_332.clone()]);
}
let car_list = vec![
@ -929,9 +934,8 @@ mod tests {
let counts = compute_car_counts(&ctx, car_list, owned_configs, HashMap::new());
let owned: HashSet<String> = vec!["guid_a".to_string(), "guid_b".to_string()]
.into_iter()
.collect();
let owned: HashSet<String> =
vec!["guid_a".to_string(), "guid_b".to_string()].into_iter().collect();
let dir = tempfile::tempdir().unwrap();
let output_dir = dir.path().to_str().unwrap();
@ -1129,8 +1133,7 @@ mod tests {
rec.normalized_config = "323".to_string();
for i in 0..5 {
ctx.best_cars
.insert((format!("Track{i}"), 85, "Dry".to_string()), vec![rec.clone()]);
ctx.best_cars.insert((format!("Track{i}"), 85, "Dry".to_string()), vec![rec.clone()]);
}
let car = make_car("Car_A", 85, "guid_a");
@ -1286,7 +1289,6 @@ mod tests {
}
// =======================================================================
// remove.csv tests
// remove.csv tests for cars not in best cars data
// =======================================================================
@ -1314,13 +1316,10 @@ mod tests {
let output_dir = dir.path().to_str().unwrap();
// Wishlist contains: guid_a (best car - should stay), guid_b (not best - should be removed), guid_c (not best - should be removed)
let wishlist: HashSet<String> = vec![
"guid_a".to_string(),
"guid_b".to_string(),
"guid_c".to_string(),
]
.into_iter()
.collect();
let wishlist: HashSet<String> =
vec!["guid_a".to_string(), "guid_b".to_string(), "guid_c".to_string()]
.into_iter()
.collect();
write_wishlist_files(output_dir, &counts, &HashSet::new(), &wishlist, 10, 300).unwrap();

24
src/parsing/mod.rs
View file

@ -105,7 +105,9 @@ pub fn parse_car_times(
// unexpected formats gracefully).
let config = raw_config
.rsplit_once('_')
.filter(|(_, suffix)| suffix.len() == 3 && suffix.bytes().all(|b| b.is_ascii_digit()))
.filter(|(_, suffix)| {
suffix.len() == 3 && suffix.bytes().all(|b| b.is_ascii_digit())
})
.map(|(_, suffix)| suffix.to_string())
.unwrap_or_else(|| raw_config.clone());
// Process timing data for each track
@ -675,7 +677,8 @@ mod tests {
parse_car_times(car, &my_cars, &ctx.best_cars, times_dir).unwrap();
// Verify the config was stripped to "233" (not "v2b_233")
let records = ctx.best_cars.get(&("Laguna Seca".to_string(), 85, "Dry".to_string())).unwrap();
let records =
ctx.best_cars.get(&("Laguna Seca".to_string(), 85, "Dry".to_string())).unwrap();
assert_eq!(records[0].config, "233", "v2b_ prefix should be stripped");
assert_eq!(records[0].normalized_config, "233", "233 is already a final build");
}
@ -723,25 +726,20 @@ mod tests {
parse_car_times(car, &my_cars, &ctx.best_cars, times_dir).unwrap();
// Best on Laguna Seca should be the v3a_233 entry (95.0 beats 110.0)
let records = ctx.best_cars.get(&("Laguna Seca".to_string(), 90, "Dry".to_string())).unwrap();
let records =
ctx.best_cars.get(&("Laguna Seca".to_string(), 90, "Dry".to_string())).unwrap();
assert_eq!(records[0].config, "233", "v3a_ prefix should be stripped");
assert_eq!(records[0].time, 95.0);
// Monaco Streets from bare "323"
let records = ctx.best_cars.get(&("Monaco Streets".to_string(), 90, "Wet".to_string())).unwrap();
let records =
ctx.best_cars.get(&("Monaco Streets".to_string(), 90, "Wet".to_string())).unwrap();
assert_eq!(records[0].config, "323", "bare config should pass through");
}
// =======================================================================
// Constants tests
// =======================================================================
#[test]
fn test_constants_are_reasonable() {
assert_eq!(MAX_RQ, 113);
assert!(MIN_VALID_TIME > 0.0);
assert!(TIME_TOLERANCE > 0.0);
assert!(TEST_BOWL_FACTOR > 0.0);
assert!(CONCURRENCY_LIMIT > 0);
}
// Constants are compile-time validated - explicit tests removed as they
// check literal values that cannot fail unless the constants themselves are edited
}

1
src/types.rs
View file

@ -149,7 +149,6 @@ pub fn normalize_to_final_build(config: &str) -> String {
digits.iter().map(|&d| (b'0' + d) as char).collect()
}
/// Returns `true` if the config is a final build (digit sum == 8) or maxed (sum > 8).
///
/// Configs with sum < 8 are non-final and still have upgrade steps remaining.

4
src/user_data.rs
View file

@ -79,9 +79,7 @@ pub fn load_my_cars_data(path: &str) -> Result<MyCarsData, AppError> {
let config = format!("{e}{w}{c}");
owned_configs.entry(guid.to_string()).or_default().push(config);
} else {
eprintln!(
"Warning: locked card {guid} has incomplete tune data — skipped"
);
eprintln!("Warning: locked card {guid} has incomplete tune data — skipped");
}
}
}

2
tests/proptest_tests.rs
View file

@ -1,7 +1,7 @@
//! Property-based tests using proptest for sorting and comparison functions.
use std::collections::HashMap;
use proptest::prelude::*;
use std::collections::HashMap;
use top_drives::test_utils::{make_car, make_record};
use top_drives::{compare_cars, compute_car_counts, empty_my_data, AnalysisContext, Car};