diff --git a/docs/API.md b/docs/API.md index 92635e6..70428fe 100644 --- a/docs/API.md +++ b/docs/API.md @@ -4,15 +4,37 @@ This document provides comprehensive API documentation for the Top Drives Records Analyzer, including all public functions, data structures, and their usage patterns. -## Core Modules +## Module Structure -### Shared Library (`src/lib.rs`) +``` +src/ +├── lib.rs # Shared library: types, utilities, error handling +├── records.rs # Binary entry point: CLI definition and dispatch +├── commands/ +│ ├── mod.rs # Command dispatch, run_analysis(), load_my_cars_or_warn() +│ ├── fetch.rs # Fetch command: download car/track/timing data +│ ├── best.rs # Best command: best.csv + very_best.csv +│ ├── trash.rs # Trash command: trash.csv +│ ├── wishlist.rs # Wishlist command: wishlist.csv, add.csv, remove.csv +│ ├── details.rs # Details command: details.csv +│ ├── track.rs # Track command: stdout query +│ └── all.rs # All command: generates all output files +├── parsing/ +│ ├── mod.rs # Core parsing: parse_car_times, update_best_car, parse_all_times +│ └── car_list.rs # Car database loading: get_car_list +└── output/ + └── mod.rs # CSV output: write_sorted_csv, write_best_csv, wishlist logic, etc. +``` + +--- + +## Shared Library (`src/lib.rs`) Common types and utilities shared across the project. -#### Public Types +### Public Types -##### `Car` +#### `Car` **Core car information structure** ```rust @@ -20,9 +42,11 @@ Common types and utilities shared across the project. pub struct Car { pub rid: String, // Repository identifier pub brand: String, // Manufacturer/brand name + #[serde(rename = "onlyName")] pub only_name: String, // Model name (without brand) pub year: u16, // Model year pub rq: u8, // Rarity level (10-113) + #[serde(default)] pub prize: bool, // Whether car is prize-only pub guid: String, // Game-specific card identifier } @@ -32,7 +56,7 @@ pub struct Car { **Deserialization**: Uses `serde::Deserialize` with `#[serde(rename = "onlyName")]` for the `only_name` field and `#[serde(default)]` for `prize`. -##### `BestCarRecord` +#### `BestCarRecord` **Best performing car record for a specific track/RQ/condition** ```rust @@ -51,7 +75,7 @@ pub struct BestCarRecord { } ``` -##### `TripletKey` +#### `TripletKey` **Composite key for unique track conditions** ```rust @@ -59,20 +83,21 @@ pub type TripletKey = (String, u8, String); // Track ID RQ Level Surface Condition ``` -##### `AnalysisContext` -**Mutable analysis state** +#### `AnalysisContext` +**Mutable analysis state using concurrent DashMap** ```rust pub struct AnalysisContext { - pub best_cars: Arc>>>, + pub best_cars: Arc>>, } ``` **Methods**: - `new()` — Create a new, empty analysis context -- `lock_best_cars()` — Acquire a lock on the best_cars map (returns `Result`, converts poison errors to `AppError`) -##### `CarCounts` +**Thread Safety**: Uses `DashMap` for sharded internal locking, allowing concurrent access from multiple parsing tasks without a global lock. + +#### `CarCounts` **Aggregated per-car win counts** ```rust @@ -84,18 +109,18 @@ pub struct CarCounts { } ``` -##### `MyCarsData` +#### `MyCarsData` **Parsed user data from `my_cars.json`** ```rust -#[derive(Debug)] +#[derive(Debug, Default)] pub struct MyCarsData { pub owned: HashSet, // Owned car GUIDs pub wishlist: HashSet, // Wishlist car GUIDs } ``` -##### `AppError` +#### `AppError` **Unified error type** ```rust @@ -109,14 +134,14 @@ pub enum AppError { Parse { file: String, reason: String }, #[error("Missing data file: {0}")] MissingFile(String), - #[error("Internal error: mutex poisoned — {0}")] - Poisoned(String), + #[error("Internal error: {0}")] + Internal(String), } ``` -#### Public Functions +### Public Functions -##### `translate_track_name(track_code: &str) -> String` +#### `translate_track_name(track_code: &str) -> String` **Translates internal track identifiers to human-readable names** - **Input**: Track code (e.g., `"laguna"`) @@ -124,38 +149,40 @@ pub enum AppError { - **Fallback**: Returns original code if no mapping found - **Data source**: `track_names.json` (compiled into binary via `include_str!`) -##### `translate_track_type(track_code: &str) -> &'static str` +#### `translate_track_type(track_code: &str) -> &'static str` **Translates condition codes to surface names** - **Input**: Condition code (e.g., `"00"`) - **Output**: Surface name (e.g., `"Dry"`) -- **Fallback**: Returns `"Unknown"` for unrecognized codes +- **Fallback**: Returns the raw code string for unrecognized codes -##### `get_file(file_url: &str, local_file_path: &str) -> Result<(), AppError>` +#### `get_file(file_url: &str, local_file_path: &str) -> Result<(), AppError>` **Downloads a file from a URL and saves it locally** -- Removes existing file before downloading +- Writes to a `.tmp` file first, then renames on success (atomic write) +- Retries up to 3 times on server errors (5xx) and empty responses - Returns `AppError::Network` on HTTP failures - Returns `AppError::Io` on file write failures -##### `load_my_cars_data(path: &str) -> Result` +#### `load_my_cars_data(path: &str) -> Result` **Loads and parses user car collection data** - **Input**: Path to `my_cars.json` - **Returns**: `MyCarsData` with owned and wishlist GUIDs - **Errors**: `AppError::MissingFile` if file not found, `AppError::Parse` if JSON invalid -##### `compare_cars(a: &Car, b: &Car) -> std::cmp::Ordering` +#### `compare_cars(a: &Car, b: &Car) -> std::cmp::Ordering` **Sorting comparator for cars** - Sorts by: RQ → Brand → Model name -##### `compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> Result` +#### `compute_car_counts(ctx: &AnalysisContext, car_list: Vec) -> Result` **Aggregates per-car win counts from analysis context** +- Iterates all entries in the `DashMap` - Returns `CarCounts` with by_rid, guid_to_rid, car_map, and car_list -##### `escape_csv_field(field: &str) -> String` +#### `escape_csv_field(field: &str) -> String` **Escapes a field for safe CSV inclusion** - Wraps in quotes if field contains comma, quote, newline, or carriage return @@ -163,11 +190,36 @@ pub enum AppError { --- -### Records Binary (`src/records.rs`) +## Records Binary (`src/records.rs`) -Primary analysis engine for processing JSON-based timing data. +CLI entry point. Defines command-line arguments and dispatches to command handlers. -#### Constants +### CLI Global Options + +| Option | Default | Description | +|--------|---------|-------------| +| `--my-cars` | `my_cars.json` | Path to user's car collection export | +| `--cars-db` | `cars_repo.json` | Path to car database file | +| `--times-dir` | `times` | Directory containing per-car timing JSON files | +| `--tracks-db` | `tracks_repo.json` | Path to tracks database file | + +### CLI Commands + +| Command | Description | Flags | +|---------|-------------|-------| +| `fetch` | Download latest car, track, and timing data | None | +| `best` | Generate `best.csv` and `very_best.csv` | `--min-count N` (default: 10) | +| `trash` | Generate `trash.csv` | None | +| `wishlist` | Generate `wishlist.csv`, `add.csv`, `remove.csv` | `--min-count N`, `--max-wishlist M` | +| `all` | Generate all output files in a single pass | `--min-count N`, `--max-wishlist M` | +| `details` | Generate `details.csv` with all timing data | None | +| `track` | Show best car for specific track/condition/RQ | ` ` | + +--- + +## Parsing Module (`src/parsing/`) + +### Constants (`src/parsing/mod.rs`) | Constant | Value | Description | |----------|-------|-------------| @@ -176,57 +228,106 @@ Primary analysis engine for processing JSON-based timing data. | `TIME_TOLERANCE` | 0.005 | Tolerance for tie detection (seconds) | | `TEST_BOWL_FACTOR` | 2000.0 | Multiplier for Test Bowl time inversion | | `CONCURRENCY_LIMIT` | 20 | Maximum concurrent tasks | -| `DEFAULT_MY_CARS_PATH` | `"my_cars.json"` | Default user data file path | -| `CARS_DB_PATH` | `"cars_repo.json"` | Car database file path | -| `TIMES_DIR` | `"times"` | Timing data directory | -| `CARS_DB_URL` | GitHub URL | Remote car database URL | -| `TRACKS_DB_URL` | GitHub URL | Remote tracks database URL | | `CAR_TIMING_API_BASE` | API URL | Base URL for per-car timing data | -#### CLI Commands +### Key Functions -| Command | Description | Flags | -|---------|-------------|-------| -| `fetch` | Download latest car and track data | None | -| `best` | Generate `best.csv` and `very_best.csv` | `--min-count N` (default: 10) | -| `trash` | Generate `trash.csv` | None | -| `wishlist` | Generate `wishlist.csv`, `add.csv`, `remove.csv` | `--min-count N`, `--max-wishlist M` | -| `all` | Generate all output files in a single pass | `--min-count N`, `--max-wishlist M` | -| `details` | Generate `details.csv` with all timing data | None | -| `track` | Show best car for specific track/condition/RQ | ` ` | +#### `parse_car_times(car, my_cars, best_cars, times_dir) -> Result<(), AppError>` +**Processes timing data for a single car** (`src/parsing/mod.rs`) -#### Key Internal Functions - -##### `run_analysis(owned: &HashSet) -> Result<(AnalysisContext, CarCounts), AppError>` -**Common analysis setup** - -Loads user data, parses all timing files, and computes car counts. - -##### `parse_car_times(car: Car, my_cars: &HashSet, ctx: &AnalysisContext)` -**Processes timing data for a single car** - -- Runs inside `tokio::task::spawn_blocking` to avoid blocking async runtime -- Reads car JSON file from `times/` directory +- **Synchronous** — designed to be called from `tokio::task::spawn_blocking` +- Reads car JSON file from the times directory - Processes each configuration and track - Applies quality filters (downvotes, minimum time) - Handles Test Bowl time inversion +- Uses `rsplit_once("_a")` for correct track name splitting -##### `update_best_car(best_cars, track_id, car_rq, surface, new_record)` -**Updates best car records with a new performance entry** +#### `update_best_car(best_cars, track_id, car_rq, surface, new_record)` +**Updates best car records with a new performance entry** (`src/parsing/mod.rs`) - Iterates RQ range from car's RQ to `MAX_RQ` - Replaces existing record if new time is faster - Adds to tie list if within `TIME_TOLERANCE` - Prevents duplicate `car_rid` entries in tie lists +- Optimized: avoids cloning on the last iteration via `is_last` guard -##### `parse_all_times(my_cars, ctx) -> Vec` -**Orchestrates concurrent parsing of all car timing files** +#### `parse_all_times(my_cars, ctx, cars_db_path, times_dir) -> Result, AppError>` +**Orchestrates concurrent parsing of all car timing files** (`src/parsing/mod.rs`) +- Loads car list from database - Uses `Semaphore(CONCURRENCY_LIMIT)` for bounded concurrency -- Spawns blocking tasks via `tokio::task::spawn_blocking` +- Acquires semaphore in async context, then runs sync parsing via `spawn_blocking` - Returns the full car list for downstream use -##### `compute_ideal_wishlist(counts, owned, min_count, max_wishlist) -> Vec<(BestCarRecord, usize)>` +#### `fetch_times(cars_db_path, times_dir) -> Result<(), AppError>` +**Fetches timing data files for all cars** (`src/parsing/mod.rs`) + +- Uses bounded concurrency via semaphore +- Returns first error encountered, or `Ok(())` if all succeed + +#### `get_car_list(cars_db_path: &str) -> Result, AppError>` +**Loads the car database** (`src/parsing/car_list.rs`) + +- Uses serde deserialization for type-safe parsing +- Returns `AppError::MissingFile` or `AppError::Parse` on failure + +--- + +## Commands Module (`src/commands/`) + +### `run_analysis(owned, cars_db_path, times_dir) -> Result<(AnalysisContext, CarCounts), AppError>` +**Common analysis setup** (`src/commands/mod.rs`) + +Loads car database, parses all timing files concurrently, and computes car counts. + +### `load_my_cars_or_warn(path: &str) -> Result` +**Loads user data with graceful degradation** (`src/commands/mod.rs`) + +- Returns empty `MyCarsData::default()` with a warning if file is missing +- Propagates parse errors and other failures + +--- + +## Output Module (`src/output/`) + +### CSV Output Functions + +#### `write_sorted_csv(path, header, entries, cmp, format_row) -> Result<(), AppError>` +**Generic sorted CSV writer** + +- Sorts by index (avoids cloning entries) +- Writes header row +- Formats each row via closure + +#### `write_best_csv(counts)`, `write_very_best_csv(counts, min_count)` +**Generate best car CSV files** + +- Format: `RQ,Brand,Model,Year,Count` +- Fields are CSV-escaped + +#### `write_trash_csv(counts, owned)` +**Generate trash car CSV file** + +- Filters: not best on any track, RQ < 80, not prize car +- Format: `RQ,Brand,Model,Year` + +#### `write_wishlist_files(counts, owned, wishlist, min_count, max_wishlist)` +**Generate wishlist, add, and remove CSV files** + +- Computes ideal wishlist once +- Derives all three files from it +- Format: `RQ,Brand,Model,Year,Count` + +#### `write_details_csv(ctx)` +**Generate comprehensive timing data CSV** + +- Format: `RQ,TrackID,Condition,CarRQ,Brand,Model,Year,Config,Time` +- Sorted by RQ then track ID + +#### `print_best_car(ctx, track, condition, rq) -> Result<(), AppError>` +**Print best car for a specific track/condition/RQ to stdout** + +#### `compute_ideal_wishlist(counts, owned, min_count, max_wishlist) -> Vec<(BestCarRecord, usize)>` **Computes the ideal wishlist** - Filters to cars with count >= `min_count` @@ -234,39 +335,9 @@ Loads user data, parses all timing files, and computes car counts. - Sorts by RQ - Caps at `max_wishlist` -#### Output Functions +### Sorting Helpers -##### `write_sorted_csv(path, header, entries, cmp, format_row)` -**Generic sorted CSV writer** +#### `compare_records(a: &BestCarRecord, b: &BestCarRecord) -> std::cmp::Ordering` +**Comparison for BestCarRecord sorting** -- Sorts by index (avoids cloning entries) -- Writes header row -- Formats each row via closure - -##### `write_best_csv(counts)`, `write_very_best_csv(counts, min_count)` -**Generate best car CSV files** - -- Format: `RQ,Brand,Model,Year,Count` -- Fields are CSV-escaped - -##### `write_trash_csv(counts, owned)` -**Generate trash car CSV file** - -- Filters: not best on any track, RQ < 80, not prize car -- Format: `RQ,Brand,Model,Year` - -##### `write_wishlist_files(counts, owned, wishlist, min_count, max_wishlist)` -**Generate wishlist, add, and remove CSV files** - -- Computes ideal wishlist once -- Derives all three files from it -- Format: `RQ,Brand,Model,Year,Count` - -##### `write_details_csv(ctx)` -**Generate comprehensive timing data CSV** - -- Format: `RQ,TrackID,Condition,CarRQ,Brand,Model,Year,Config,Time` -- Sorted by RQ then track ID - -##### `print_best_car(ctx, track, condition, rq)` -**Print best car for a specific track/condition/RQ to stdout** +- Sorts by: RQ → Brand → Model name diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7cf5567..fd834b8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -2,7 +2,7 @@ ## System Overview -The Top Drives Records Analyzer consists of a main analysis binary and a shared library working together to provide comprehensive car performance analysis for the Top Drives racing game. +The Top Drives Records Analyzer is a Rust CLI tool for analyzing car performance data in the Top Drives racing game. It consists of a shared library (`src/lib.rs`), a binary entry point (`src/records.rs`), and three internal module trees: `commands/`, `parsing/`, and `output/`. ## Component Architecture @@ -14,7 +14,7 @@ The Top Drives Records Analyzer consists of a main analysis binary and a shared - **Data Types**: Defines `Car`, `BestCarRecord`, `AnalysisContext`, `CarCounts`, `MyCarsData`, `AppError` - **Track Name Translation**: Converts track codes to human-readable names via `translate_track_name()` - **Track Type Translation**: Converts condition codes to surface descriptions via `translate_track_type()` -- **File Download**: Async file download utility via `get_file()` +- **File Download**: Async file download utility with retry logic via `get_file()` - **User Data Loading**: Parses `my_cars.json` via `load_my_cars_data()` - **Car Comparison**: Sorting comparator via `compare_cars()` - **Count Aggregation**: Per-car win count computation via `compute_car_counts()` @@ -26,18 +26,44 @@ All public functions return `Result` where `AppError` is a unified - `AppError::Network` — HTTP/download errors - `AppError::Parse` — JSON parsing errors with file context - `AppError::MissingFile` — Required data file not found -- `AppError::Poisoned` — Mutex poisoning from panicked threads +- `AppError::Internal` — Runtime errors (semaphore closed, task join failures) ### 2. Records Binary (`src/records.rs`) -**Primary analysis engine** that processes pre-downloaded JSON timing data. +**CLI entry point** that defines command-line arguments and dispatches to command handlers. #### Key Responsibilities -- **Data Fetching**: Downloads car and track databases from GitHub repository -- **File Processing**: Parses JSON timing files for each car configuration -- **Performance Analysis**: Compares timing data across RQ ranges and track conditions -- **Collection Management**: Identifies owned cars and optimization opportunities -- **CSV Output**: Writes analysis results to CSV files with proper escaping +- **CLI Definition**: `clap` derive-based argument parsing with global path options +- **Command Dispatch**: Routes to the appropriate command handler in `commands/` +- **Error Reporting**: Catches all errors and prints to stderr with exit code 1 + +#### Global CLI Options +- `--my-cars` — Path to user's car collection export (default: `my_cars.json`) +- `--cars-db` — Path to car database file (default: `cars_repo.json`) +- `--times-dir` — Directory for per-car timing JSON files (default: `times`) +- `--tracks-db` — Path to tracks database file (default: `tracks_repo.json`) + +### 3. Commands Module (`src/commands/`) + +**Command handlers** that orchestrate analysis and output generation. + +| Module | Command | Description | +|--------|---------|-------------| +| `fetch.rs` | `fetch` | Downloads car DB, tracks DB, and per-car timing files | +| `best.rs` | `best` | Generates `best.csv` and `very_best.csv` | +| `trash.rs` | `trash` | Generates `trash.csv` (owned cars not best on any track) | +| `wishlist.rs` | `wishlist` | Generates `wishlist.csv`, `add.csv`, `remove.csv` | +| `details.rs` | `details` | Generates `details.csv` with all timing data | +| `track.rs` | `track` | Prints best car for a specific track/condition/RQ | +| `all.rs` | `all` | Generates all output files in a single analysis pass | + +#### Shared Helpers (`src/commands/mod.rs`) +- `run_analysis(owned, cars_db_path, times_dir)` — Common analysis setup: loads car DB, parses timing files, computes counts +- `load_my_cars_or_warn(path)` — Loads user data with graceful degradation (returns empty data if file missing) + +### 4. Parsing Module (`src/parsing/`) + +**Core parsing logic** for car timing data and car database loading. #### Key Constants - `MAX_RQ: u8 = 113` — Maximum RQ level used for range analysis @@ -46,19 +72,43 @@ All public functions return `Result` where `AppError` is a unified - `TEST_BOWL_FACTOR: f64 = 2000.0` — Multiplier for Test Bowl time inversion - `CONCURRENCY_LIMIT: usize = 20` — Maximum concurrent tasks +#### Key Functions +- `parse_car_times()` — Synchronous per-car timing file parser (runs inside `spawn_blocking`) +- `update_best_car()` — Updates `DashMap` with new performance records across RQ range +- `parse_all_times()` — Orchestrates concurrent parsing with semaphore + `spawn_blocking` +- `fetch_times()` — Downloads per-car timing files with bounded concurrency +- `get_car_list()` (`car_list.rs`) — Loads car database via serde deserialization + +### 5. Output Module (`src/output/`) + +**CSV output utilities** and sorting helpers. + +- `write_sorted_csv()` — Generic sorted CSV writer (index-based sorting to avoid cloning) +- `write_best_csv()`, `write_very_best_csv()` — Best car CSV output +- `write_trash_csv()` — Trash car CSV output +- `write_details_csv()` — Comprehensive timing data CSV output +- `write_wishlist_files()` — Wishlist, add, and remove CSV output +- `print_best_car()` — Stdout output for track-specific query +- `compute_ideal_wishlist()` — Ideal wishlist computation +- `compare_records()` — Sorting comparator for `BestCarRecord` + #### Data Flow ``` -GitHub Repository - ↓ -cars_repo.json + tracks_repo.json - ↓ -times/*.json (1000+ car files) - ↓ -JSON Parsing → BestCarRecord Analysis - ↓ -User Collection (my_cars.json) - ↓ +GitHub Repository / API + ↓ +cars_repo.json + tracks_repo.json ← fetch command + ↓ +times/*.json (1000+ car files) ← fetch command + ↓ +parse_all_times() → parse_car_times() × N cars (concurrent via spawn_blocking) + ↓ +AnalysisContext (DashMap>) + ↓ +compute_car_counts() → CarCounts + ↓ +User Collection (my_cars.json) → owned/wishlist sets + ↓ Output: CSV files / Terminal Results ``` @@ -74,7 +124,7 @@ type TripletKey = (String, u8, String); ``` **Purpose**: Composite key for uniquely identifying track conditions -**Usage**: HashMap key for storing best car records +**Usage**: `DashMap` key for storing best car records #### Car ```rust @@ -82,10 +132,10 @@ type TripletKey = (String, u8, String); struct Car { rid: String, // Repository ID brand: String, // Manufacturer - only_name: String, // Model name + only_name: String, // Model name (serde rename: "onlyName") year: u16, // Model year rq: u8, // Rarity level - prize: bool, // Prize car status + prize: bool, // Prize car status (serde default) guid: String, // Game GUID } ``` @@ -111,13 +161,13 @@ struct BestCarRecord { ### AnalysisContext ```rust struct AnalysisContext { - best_cars: Arc>>>, + best_cars: Arc>>, } ``` -**Thread Safety**: Protected by `Mutex` for concurrent access during parsing -**Lifetime**: Created in `main()`, passed through the call chain -**Access**: `lock_best_cars()` method returns `Result` with proper error handling +**Thread Safety**: Uses `DashMap` for sharded internal locking, providing high-concurrency access without a global mutex. +**Lifetime**: Created in command handlers, passed through the call chain. +**Access**: Direct `DashMap` methods (`insert`, `get`, `iter`, `entry`). ### CarCounts ```rust @@ -152,17 +202,19 @@ Multiple validation layers ensure data quality: 2. **Downvote Filter**: Community-downvoted times are excluded 3. **Tie Detection**: Times within `TIME_TOLERANCE` (5ms) are considered equal 4. **Special Cases**: Test Bowl track uses inverted time calculation (`TEST_BOWL_FACTOR / time`) +5. **Track Name Splitting**: Uses `rsplit_once("_a")` to correctly handle track names containing `_a` (e.g., `airplaneHangars`) ### Concurrent Processing #### Bounded Concurrency Pattern -- **Task Spawning**: One blocking task per car file via `tokio::task::spawn_blocking` -- **Semaphore**: Limits concurrent operations to `CONCURRENCY_LIMIT` (20) -- **Error Isolation**: Individual car failures don't stop batch processing +- **Semaphore First**: Semaphore permit acquired in async context (avoids deadlocking the blocking thread pool) +- **Blocking Work**: Synchronous parsing runs inside `tokio::task::spawn_blocking` while holding the permit +- **Concurrency Limit**: Maximum `CONCURRENCY_LIMIT` (20) simultaneous operations #### Synchronization +- **DashMap**: Sharded concurrent HashMap — no global lock needed for parallel parsing - **Join Handles**: Waits for all parsing tasks to complete -- **Mutex Protection**: Safe concurrent access to shared `AnalysisContext` +- **Error Collection**: Parse errors are counted and reported; all-cars-failure propagates as an error ## File Organization @@ -170,7 +222,7 @@ Multiple validation layers ensure data quality: ``` ├── cars_repo.json # Car database from repository ├── tracks_repo.json # Track definitions -├── track_names.json # Track code → name mappings (compiled in) +├── track_names.json # Track code → name mappings (compiled into binary) ├── my_cars.json # User's car collection (game export) └── times/ # Individual car timing files ├── Ferrari_F40_1987.json @@ -191,12 +243,13 @@ Multiple validation layers ensure data quality: ## Performance Characteristics -### Records Module - **Startup Time**: Fast (uses cached JSON files) - **Memory Usage**: Moderate (loads all timing data into memory) - **Accuracy**: High (comprehensive dataset) - **Update Frequency**: Manual refresh required - **Concurrency**: Bounded at 20 simultaneous operations via semaphore +- **Lock Contention**: Minimal (DashMap sharded locking replaces global Mutex) +- **Allocation Optimization**: `update_best_car` avoids cloning on the last RQ iteration ## Error Handling Strategy @@ -206,12 +259,13 @@ All errors are represented as `AppError`: - **Network Errors**: Download failures with descriptive messages - **Parse Errors**: JSON parsing failures with file name context - **Missing Files**: Specific variant for absent data files -- **Poisoned Mutex**: Thread panic detection +- **Internal Errors**: Runtime failures (semaphore closed, task join errors) ### Graceful Degradation - **Missing User Data**: `load_my_cars_or_warn()` returns empty data with a warning -- **Network Errors**: Fetch errors logged but don't crash -- **Parse Failures**: Individual car file failures emit warnings and skip +- **Missing Timing Files**: Individual car file failures emit warnings and skip +- **Network Errors**: Fetch errors logged; first error propagated +- **All Parse Failure**: If every car fails to parse, the error is propagated ### User Feedback - **Progress Indication**: Shows fetching/download progress @@ -225,15 +279,15 @@ All errors are represented as `AppError`: 2. Add track name mapping in `track_names.json` 3. Add condition mappings in `translate_track_type()` if needed -### Adding New Analysis Types -1. Create new output function(s) using `write_sorted_csv` -2. Add command variant to the `Commands` enum -3. Add command handler in `main()` -4. Update help text and documentation +### Adding New Commands +1. Create new command handler in `src/commands/` +2. Add command variant to the `Commands` enum in `src/records.rs` +3. Add dispatch case in the `run()` function +4. Update documentation ### Adding New Output Formats -1. Create new output function(s) alongside existing CSV writers -2. Reuse `CarCounts` for data access +1. Create new output function(s) in `src/output/mod.rs` +2. Reuse `CarCounts` and `AnalysisContext` for data access 3. Add CLI flag for format selection if needed ## Security Considerations @@ -241,6 +295,7 @@ All errors are represented as `AppError`: ### Network Access - **HTTPS Only**: All external requests use HTTPS - **Rate Limiting**: Respects server limits with controlled concurrency +- **Atomic Writes**: Downloads write to `.tmp` files first, then rename on success ### Local File Access - **Read Only**: No modification of game files @@ -255,8 +310,6 @@ All errors are represented as `AppError`: 3. **Configuration Files**: External configuration for analysis parameters 4. **GUI Interface**: Web-based interface for non-technical users 5. **Export Formats**: Multiple output formats (JSON, XML, etc.) -6. **Module Split**: Break `records.rs` into a module tree for better organization -7. **Concurrent HashMap**: Use `DashMap` for finer-grained locking ### Scalability Considerations 1. **Pagination**: Handle large datasets with pagination diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index fcd26b8..085b776 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -47,6 +47,22 @@ Fetching https://api.topdrivesrecords.com/car/1002.json. ## Command Reference +### Global Options + +All commands accept these optional flags to customize file paths: + +| Flag | Default | Description | +|------|---------|-------------| +| `--my-cars ` | `my_cars.json` | Path to user's car collection export | +| `--cars-db ` | `cars_repo.json` | Path to car database file | +| `--times-dir ` | `times` | Directory containing per-car timing JSON files | +| `--tracks-db ` | `tracks_repo.json` | Path to tracks database file | + +**Example with custom paths:** +```bash +cargo run --bin records -- --cars-db /data/cars.json --times-dir /data/times best +``` + ### Basic Analysis Commands #### Best Cars Analysis @@ -407,12 +423,13 @@ The tool generates CSV files for external analysis. All files are sorted by car **Problem:** No arguments provided **Solution:** Use `best`, `trash`, `wishlist`, `all`, `details`, `track`, or `fetch` -#### "Could not read my cars file" -**Problem:** `my_cars.json` not found or not readable +#### "Could not read my cars file" / "Missing data file" +**Problem:** `my_cars.json` (or another data file) not found or not readable **Solution:** 1. Export fresh data from Top Drives game -2. Ensure file is named exactly `my_cars.json` +2. Ensure file is named exactly `my_cars.json` (or use `--my-cars ` to specify a custom location) 3. Check file permissions +4. If `cars_repo.json` is missing, run `fetch` first (or use `--cars-db `) #### "Failed to fetch the file" **Problem:** Network issues or API unavailable diff --git a/plans/code-review.md b/plans/code-review.md deleted file mode 100644 index 56c8da0..0000000 --- a/plans/code-review.md +++ /dev/null @@ -1,396 +0,0 @@ -# Top Drives Records Analyzer — Code Review - -## Executive Summary - -The project is a well-structured Rust CLI tool for analyzing car performance data from the Top Drives racing game. The code is generally clean, well-documented, and follows good Rust practices. The recent v0.3.0 refactor (moving from global mutable state to `AnalysisContext`, adding `CarCounts`, and introducing the `all` command) shows a clear improvement trajectory. - -However, there are several areas where the codebase could be significantly improved: **error handling robustness**, **test coverage depth**, **documentation accuracy**, **performance optimizations**, and **code organization**. - ---- - -## 1. Code Structure & Design - -### 1.1 ✅ What's Done Well - -- **Clean separation** between library ([`lib.rs`](src/lib.rs)) and binary ([`records.rs`](src/records.rs)) -- **Good use of `AnalysisContext`** to eliminate global mutable state — this is a solid architectural improvement -- **`CarCounts` struct** centralizes derived data computation, avoiding redundant calculations -- **`write_sorted_csv` generic helper** eliminates CSV writing boilerplate across output functions -- **Bounded concurrency** via `Semaphore(20)` prevents resource exhaustion -- **`clap` derive macros** for CLI parsing — idiomatic and maintainable - -### 1.2 🔧 Suggested Improvements - -#### A. Monolithic `records.rs` — Break Into Modules - -[`records.rs`](src/records.rs) is 923 lines with parsing logic, output logic, fetch logic, CLI definition, and tests all in one file. This should be split into a module tree: - -``` -src/ -├── lib.rs -├── records.rs # main() + CLI only -├── commands/ -│ ├── mod.rs # Command dispatch -│ ├── fetch.rs # Fetch logic -│ ├── best.rs # Best/very_best output -│ ├── trash.rs # Trash output -│ ├── wishlist.rs # Wishlist/add/remove output -│ ├── details.rs # Details output -│ └── track.rs # Track-specific output -├── parsing/ -│ ├── mod.rs # parse_all_times, parse_car_times -│ └── car_list.rs # get_car_list -└── output/ - ├── mod.rs - └── csv_writer.rs # write_sorted_csv, format_counted_row -``` - -#### B. `parse_car_times` Is Async But Does No Async Work - -[`parse_car_times()`](src/records.rs:491) is declared `async` but only performs synchronous file I/O and JSON parsing. It's called inside `tokio::task::spawn`, which means it's running on the Tokio runtime but blocking it with `fs::read_to_string` and `serde_json::from_str`. - -**Recommendation**: Either use `tokio::task::spawn_blocking` for the synchronous work, or use `tokio::fs::read_to_string` for truly async file I/O. The current approach blocks the Tokio runtime threads. - -#### C. `update_best_car` Allocates Excessively - -[`update_best_car()`](src/records.rs:571) clones `new_record` for every RQ level from `car_rq` to `MAX_RQ` (potentially 28 clones for an RQ 85 car). It also allocates a new `String` for each key on every iteration. - -**Recommendation**: Consider building the key once and reusing it, or restructuring to reduce allocations in this hot path. - -#### D. `AnalysisContext` Re-construction in `parse_all_times` - -In [`parse_all_times()`](src/records.rs:625), a new `AnalysisContext` is constructed from the cloned `Arc`: -```rust -parse_car_times(car, &my_cars, &AnalysisContext { best_cars }).await; -``` -This works but is semantically odd — it creates a new struct just to pass the same `Arc`. Consider having `parse_car_times` accept `&Arc>` directly, or pass `&AnalysisContext` by cloning the Arc. - ---- - -## 2. Error Handling - -### 2.1 🔴 Critical: Silent Failures - -Several functions silently swallow errors and continue with empty data: - -- [`get_car_list()`](src/records.rs:688) returns `Vec::new()` on file-not-found or parse errors — if `cars_repo.json` is missing, the entire analysis runs on zero cars with no indication -- [`parse_car_times()`](src/records.rs:497) returns `()` and silently skips cars with unreadable files -- [`load_my_cars_data()`](src/lib.rs:172) returns empty sets on failure - -**Recommendation**: Use `Result` return types consistently. For the binary, consider using `anyhow` or `thiserror` for ergonomic error handling. At minimum, distinguish between "file missing" (warn) and "file corrupt" (error). - -### 2.2 🔴 Mixed Error Handling Strategies - -The codebase mixes: -- `Result<(), String>` for async functions ([`get_file()`](src/lib.rs:242)) -- `std::io::Result<()>` for output functions -- `eprintln!` + early return for input parsing -- `expect()` for mutex poisoning - -**Recommendation**: Define a custom error type using `thiserror`: -```rust -#[derive(Debug, thiserror::Error)] -enum AppError { - #[error("IO error: {0}")] - Io(#[from] std::io::Error), - #[error("Network error: {0}")] - Network(String), - #[error("Parse error in {file}: {reason}")] - Parse { file: String, reason: String }, -} -``` - -### 2.3 🟡 `unwrap()` vs `expect()` Inconsistency - -- [`update_best_car()`](src/records.rs:581) uses `best_cars.lock().unwrap()` -- [`compute_car_counts()`](src/lib.rs:117) uses `.expect("best_cars mutex poisoned...")` - -**Recommendation**: Use `.expect()` consistently with descriptive messages, or better yet, use a helper method on `AnalysisContext` that centralizes lock acquisition. - ---- - -## 3. Test Coverage - -### 3.1 🔴 Critical: Minimal Test Coverage - -The test suite in [`records.rs:736-923`](src/records.rs:736) has only **8 tests**, and several are superficial: - -| Test | What It Actually Tests | Quality | -|------|----------------------|---------| -| `test_split_once` | Standard library `split_once` behavior | ❌ Trivial — tests stdlib | -| `test_translate_track_name` | Track name lookup | ✅ Good | -| `test_translate_track_type` | Condition code lookup | ✅ Good | -| `test_update_best_car_logic` | Only checks struct field values | ❌ Doesn't test `update_best_car` | -| `test_rq_range_analysis` | RQ range insertion | ✅ Good | -| `test_time_validation` | Two float comparisons | ❌ Trivial — tests nothing | -| `test_track_name_completeness` | Sample of track name mappings | ✅ Good | -| `test_condition_completeness` | Sample of condition mappings | ✅ Good | -| `test_time_tolerance_constant` | Tie-breaking logic | ✅ Best test in the suite | - -### 3.2 🔧 Missing Test Categories - -**No tests for**: -- [`compute_car_counts()`](src/lib.rs:115) — critical aggregation logic -- [`write_sorted_csv()`](src/records.rs:202) — output formatting -- [`write_best_csv()`](src/records.rs:232), [`write_trash_csv()`](src/records.rs:260), [`write_wishlist_files()`](src/records.rs:296) — all output functions -- [`compute_ideal_wishlist()`](src/records.rs:277) — wishlist algorithm -- [`get_car_list()`](src/records.rs:688) — car database parsing -- [`load_my_cars_data()`](src/lib.rs:172) — user data parsing -- [`compare_cars()`](src/lib.rs:232), [`compare_records()`](src/records.rs:191) — sort comparators -- [`parse_car_times()`](src/records.rs:491) — core parsing logic -- CSV output correctness (field ordering, escaping, headers) -- Edge cases: empty data, single car, all cars owned, no cars owned - -### 3.3 🔧 Recommended Test Improvements - -1. **Add integration tests** in a `tests/` directory that exercise the full pipeline with fixture data -2. **Add property-based tests** using `proptest` for the sorting and comparison functions -3. **Test `update_best_car`** more thoroughly: test replacement, test tie-breaking, test multiple cars at different RQ levels -4. **Test CSV output** by writing to a buffer and comparing against expected output -5. **Remove trivial tests** that only test standard library behavior (`test_split_once`, `test_time_validation`) -6. **Add test utilities** for creating fixture `Car`, `BestCarRecord`, and `AnalysisContext` instances - ---- - -## 4. Performance - -### 4.1 🟡 `update_best_car` Hot Path - -[`update_best_car()`](src/records.rs:571) is called for every track × condition × car combination (potentially millions of calls). Current issues: - -- **Holds mutex lock while iterating RQ range** — lock is held for `MAX_RQ - car_rq + 1` iterations -- **Clones `new_record` for each RQ level** — up to 28 clones per call -- **Allocates `String` keys in a loop** — heap allocation per iteration - -**Recommendation**: -- Pre-compute a batch of updates and apply them under a single lock -- Use `String` interning or `&str` references for track IDs and conditions to reduce allocations -- Consider `DashMap` instead of `Mutex` for finer-grained locking - -### 4.2 🟡 `parse_car_times` Does Synchronous I/O on Async Runtime - -As noted in §1.2.B, `fs::read_to_string` blocks the Tokio runtime. With 1000+ cars and a semaphore of 20, this means 20 Tokio threads can be blocked simultaneously on file I/O. - -### 4.3 🟡 `compute_car_counts` Clones All Records - -[`compute_car_counts()`](src/lib.rs:115) clones every `BestCarRecord` when building `by_rid`. For large datasets, this is significant memory allocation. - -### 4.4 ✅ Good: Semaphore-Based Concurrency - -The `Semaphore(20)` in both [`parse_all_times()`](src/records.rs:616) and [`fetch_times()`](src/records.rs:644) is a good pattern for controlling concurrency. - ---- - -## 5. Documentation Issues - -### 5.1 🔴 Critical: `docs/API.md` Is Severely Outdated - -[`docs/API.md`](docs/API.md) references: -- `print_best_cars(filter_out_owned: bool)` — **removed in v0.3.0** -- `print_best_times()` — **removed/renamed** -- `print_trash()` — **removed in v0.3.0** -- `missing` command — **removed in v0.3.0** -- `full_script` command — **never existed in current form** -- Old function signatures that no longer match the code - -**This is the most critical documentation issue.** The API docs are actively misleading. - -### 5.2 🔴 `docs/ARCHITECTURE.md` References Removed Code - -[`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md:96) still references: -``` -static BEST_CARS: Lazy>>> -``` -This global static was replaced by `AnalysisContext` in v0.3.0. The doc also lists `sorted.csv` as an output file, which doesn't appear in the current code. - -### 5.3 🟡 `README.md` Dependency List Is Wrong - -[`README.md`](README.md:289) lists `once_cell` as a dependency, but it's not in [`Cargo.toml`](Cargo.toml). The code uses `std::sync::OnceLock` instead (which is correct — the README is outdated). - -### 5.4 🟡 `BestCarRecord` Missing `prize` Field in README - -The [`README.md` data structure listing](README.md:319) for `BestCarRecord` doesn't include the `prize` field that exists in the [actual struct](src/lib.rs:69). - -### 5.5 🟡 `docs/DATA_STRUCTURES.md` Code Examples Are Outdated - -[`docs/DATA_STRUCTURES.md`](docs/DATA_STRUCTURES.md:237) shows code examples using `unwrap_or("")` patterns, while the actual code in [`get_car_list()`](src/records.rs:706) uses `filter_map` with `?` operator. - -### 5.6 ✅ Good: Inline Documentation - -The inline rustdoc comments are generally excellent — well-structured with purpose, arguments, examples, and special cases. The module-level doc comments are informative. - ---- - -## 6. Specific Code Issues - -### 6.1 🟡 Hardcoded File Paths - -Multiple functions use hardcoded relative file paths: -- `"my_cars.json"` in [`load_my_cars_data()`](src/lib.rs:173) -- `"cars_repo.json"` in [`get_car_list()`](src/records.rs:689) -- `"times/"` prefix in [`parse_car_times()`](src/records.rs:494) and [`fetch_car()`](src/records.rs:679) -- Output file names: `"best.csv"`, `"trash.csv"`, etc. - -**Recommendation**: Define constants or accept configurable paths via CLI flags. - -### 6.2 🟡 Hardcoded URLs - -[`get_tracks_file()`](src/records.rs:727) and [`get_cars_file()`](src/records.rs:732) have hardcoded GitHub URLs. These should be constants at minimum. - -### 6.3 🟡 CSV Escaping - -[`format_counted_row()`](src/records.rs:225) and other output functions don't handle CSV escaping. If car brand or model names contain commas, quotes, or newlines, the CSV output will be malformed. - -**Recommendation**: Use the `csv` crate, or at minimum implement proper CSV escaping. - -### 6.4 🟡 `Test Bowl` Magic Number - -The Test Bowl time inversion `2000.0 / time` at [line 533](src/records.rs:533) uses a magic number with no explanation of why 2000.0 is the correct constant. - -### 6.5 🟢 Minor: `year as u16` and `rq as u8` Casting - -In [`get_car_list()`](src/records.rs:715), `year` and `rq` are cast from `u64` to `u16`/`u8` without bounds checking. If the JSON contains values outside the valid range, this will silently truncate. - -### 6.6 🟢 Minor: `test_update_best_car_logic` Test Name Is Misleading - -The test at [line 777](src/records.rs:777) is named `test_update_best_car_logic` but never actually calls `update_best_car()`. It only checks struct field values. - ---- - -## 7. Dependency Concerns - -### 7.1 🟡 `tokio` with `features = ["full"]` - -[`Cargo.toml`](Cargo.toml:16) enables all Tokio features. For a CLI tool, only `rt-multi-thread`, `macros`, and `fs` are likely needed. Using `full` increases compile time and binary size. - -### 7.2 🟡 No `serde` Derive for Data Structures - -`Car` and `BestCarRecord` use manual JSON field extraction via `serde_json::Value`. If the JSON schemas are stable, deriving `serde::Deserialize` would be safer and more performant. - -### 7.3 🟢 Missing `csv` Crate - -Output is hand-formatted CSV. The `csv` crate would handle escaping, quoting, and provide a type-safe writing interface. - ---- - -## 8. `.gitignore` and Project Configuration - -### 8.1 🟡 Output CSV Files Not in `.gitignore` - -[`.gitignore`](.gitignore) ignores `times/`, `cars_repo.json`, `tracks_repo.json`, and `my_cars.json`, but doesn't ignore generated output files: `best.csv`, `very_best.csv`, `trash.csv`, `wishlist.csv`, `add.csv`, `remove.csv`, `details.csv`. - -### 8.2 🟡 No `rustfmt.toml` or `clippy.toml` - -No configuration for code formatting or linting standards. - ---- - -## 9. Proposed Improvement Plan - -### Priority 1 — High Impact - -| # | Improvement | Rationale | -|---|-------------|-----------| -| 1 | Fix outdated `docs/API.md` | Currently misleading — references removed functions | -| 2 | Fix outdated `docs/ARCHITECTURE.md` | References removed global static | -| 3 | Fix `README.md` dependency list and struct definitions | Lists wrong deps and missing fields | -| 4 | Add output CSV files to `.gitignore` | Prevent accidental commits | -| 5 | Add tests for `compute_car_counts` | Critical untested aggregation logic | -| 6 | Add tests for `compute_ideal_wishlist` | Wishlist algorithm has no coverage | -| 7 | Add tests for CSV output functions | Output correctness is unverified | - -### Priority 2 — Medium Impact - -| # | Improvement | Rationale | -|---|-------------|-----------| -| 8 | Introduce custom error type with `thiserror` | Consistent error handling | -| 9 | Use `Result` returns instead of silent failures | Prevent hidden data loss | -| 10 | Add integration tests with fixture data | End-to-end correctness | -| 11 | Fix `parse_car_times` async/blocking mismatch | Prevents runtime thread starvation | -| 12 | Add CSV escaping or use `csv` crate | Data correctness for edge cases | -| 13 | Extract hardcoded URLs as constants | Maintainability | -| 14 | Remove trivial tests, rename misleading ones | Test suite quality | - -### Priority 3 — Lower Impact / Refactoring - -| # | Improvement | Rationale | -|---|-------------|-----------| -| 15 | Split `records.rs` into module tree | Maintainability as project grows | -| 16 | Consider `DashMap` for concurrent HashMap | Performance under high contention | -| 17 | Reduce allocations in `update_best_car` | Hot path optimization | -| 18 | Narrow `tokio` features from `full` | Compile time and binary size | -| 19 | Add `serde` derive for data structures | Type-safe parsing | -| 20 | Add `rustfmt.toml` and `clippy.toml` | Code consistency | -| 21 | Add configurable file paths via CLI | Flexibility | - ---- - -## 10. Architecture Diagram — Current vs Proposed - -### Current Architecture - -```mermaid -graph TD - A[main - records.rs] --> B[CLI parsing - clap] - B --> C[Fetch] - B --> D[Best] - B --> E[Trash] - B --> F[Wishlist] - B --> G[All] - B --> H[Details] - B --> I[Track] - - C --> J[get_file - lib.rs] - D --> K[run_analysis] - E --> K - F --> K - G --> K - H --> K - I --> K - - K --> L[parse_all_times] - L --> M[parse_car_times x N] - M --> N[update_best_car] - N --> O[AnalysisContext.best_cars] - - K --> P[compute_car_counts] - P --> Q[CarCounts] - - Q --> R[write_sorted_csv] - R --> S[CSV files] - - style A fill:#f9f,stroke:#333 - style O fill:#ff9,stroke:#333 - style Q fill:#9f9,stroke:#333 -``` - -### Proposed Module Structure - -```mermaid -graph TD - A[main.rs - CLI only] --> B[commands/mod.rs] - B --> C[fetch.rs] - B --> D[best.rs] - B --> E[trash.rs] - B --> F[wishlist.rs] - B --> G[details.rs] - B --> H[track.rs] - - I[lib.rs] --> J[types.rs - Car, BestCarRecord, etc.] - I --> K[analysis.rs - AnalysisContext, CarCounts] - I --> L[translate.rs - track name/type] - I --> M[io.rs - get_file, load_my_cars_data] - - D --> N[parsing/mod.rs] - E --> N - F --> N - G --> N - H --> N - N --> O[parse_all_times] - O --> P[parse_car_times] - - Q[output/mod.rs] --> R[csv_writer.rs] - D --> Q - E --> Q - F --> Q - G --> Q