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

docs: update documentation to match refactored codebase

Browse source 
- Delete obsolete plans/code-review.md (all suggestions implemented)
- Rewrite docs/API.md to reflect modular architecture, DashMap, corrected
  AppError variants, accurate function signatures and module locations
- Rewrite docs/ARCHITECTURE.md to reflect DashMap concurrency, remove
  completed future enhancements, fix AnalysisContext description
- Update docs/USER_GUIDE.md with global CLI path options (--my-cars,
  --cars-db, --times-dir, --tracks-db) and updated troubleshooting
This commit is contained in:
Jan-Peter von Hunnius 2026-05-08 16:36:14 +02:00
parent a9a46ee551
commit d2c1418e5e
4 changed files with 284 additions and 539 deletions
Download patch file Download diff file Expand all files Collapse all files

263
docs/API.md
View file

@ -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. 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. Common types and utilities shared across the project.
#### Public Types ### Public Types
##### `Car` #### `Car`
**Core car information structure** **Core car information structure**
```rust ```rust
@ -20,9 +42,11 @@ Common types and utilities shared across the project.
pub struct Car { pub struct Car {
pub rid: String, // Repository identifier pub rid: String, // Repository identifier
pub brand: String, // Manufacturer/brand name pub brand: String, // Manufacturer/brand name
#[serde(rename = "onlyName")]
pub only_name: String, // Model name (without brand) pub only_name: String, // Model name (without brand)
pub year: u16, // Model year pub year: u16, // Model year
pub rq: u8, // Rarity level (10-113) pub rq: u8, // Rarity level (10-113)
#[serde(default)]
pub prize: bool, // Whether car is prize-only pub prize: bool, // Whether car is prize-only
pub guid: String, // Game-specific card identifier 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`. **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** **Best performing car record for a specific track/RQ/condition**
```rust ```rust
@ -51,7 +75,7 @@ pub struct BestCarRecord {
} }
``` ```
##### `TripletKey` #### `TripletKey`
**Composite key for unique track conditions** **Composite key for unique track conditions**
```rust ```rust
@ -59,20 +83,21 @@ pub type TripletKey = (String, u8, String);
// Track ID RQ Level Surface Condition // Track ID RQ Level Surface Condition
``` ```
##### `AnalysisContext` #### `AnalysisContext`
**Mutable analysis state** **Mutable analysis state using concurrent DashMap**
```rust ```rust
pub struct AnalysisContext { pub struct AnalysisContext {
pub best_cars: Arc<Mutex<HashMap<TripletKey, Vec<BestCarRecord>>>>, pub best_cars: Arc<DashMap<TripletKey, Vec<BestCarRecord>>>,
} }
``` ```
**Methods**: **Methods**:
- `new()` — Create a new, empty analysis context - `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** **Aggregated per-car win counts**
```rust ```rust
@ -84,18 +109,18 @@ pub struct CarCounts {
} }
``` ```
##### `MyCarsData` #### `MyCarsData`
**Parsed user data from `my_cars.json`** **Parsed user data from `my_cars.json`**
```rust ```rust
#[derive(Debug)] #[derive(Debug, Default)]
pub struct MyCarsData { pub struct MyCarsData {
pub owned: HashSet<String>, // Owned car GUIDs pub owned: HashSet<String>, // Owned car GUIDs
pub wishlist: HashSet<String>, // Wishlist car GUIDs pub wishlist: HashSet<String>, // Wishlist car GUIDs
} }
``` ```
##### `AppError` #### `AppError`
**Unified error type** **Unified error type**
```rust ```rust
@ -109,14 +134,14 @@ pub enum AppError {
Parse { file: String, reason: String }, Parse { file: String, reason: String },
#[error("Missing data file: {0}")] #[error("Missing data file: {0}")]
MissingFile(String), MissingFile(String),
#[error("Internal error: mutex poisoned — {0}")] #[error("Internal error: {0}")]
Poisoned(String), 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** **Translates internal track identifiers to human-readable names**
- **Input**: Track code (e.g., `"laguna"`) - **Input**: Track code (e.g., `"laguna"`)
@ -124,38 +149,40 @@ pub enum AppError {
- **Fallback**: Returns original code if no mapping found - **Fallback**: Returns original code if no mapping found
- **Data source**: `track_names.json` (compiled into binary via `include_str!`) - **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** **Translates condition codes to surface names**
- **Input**: Condition code (e.g., `"00"`) - **Input**: Condition code (e.g., `"00"`)
- **Output**: Surface name (e.g., `"Dry"`) - **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** **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::Network` on HTTP failures
- Returns `AppError::Io` on file write failures - Returns `AppError::Io` on file write failures
##### `load_my_cars_data(path: &str) -> Result<MyCarsData, AppError>` #### `load_my_cars_data(path: &str) -> Result<MyCarsData, AppError>`
**Loads and parses user car collection data** **Loads and parses user car collection data**
- **Input**: Path to `my_cars.json` - **Input**: Path to `my_cars.json`
- **Returns**: `MyCarsData` with owned and wishlist GUIDs - **Returns**: `MyCarsData` with owned and wishlist GUIDs
- **Errors**: `AppError::MissingFile` if file not found, `AppError::Parse` if JSON invalid - **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** **Sorting comparator for cars**
- Sorts by: RQ → Brand → Model name - Sorts by: RQ → Brand → Model name
##### `compute_car_counts(ctx: &AnalysisContext, car_list: Vec<Car>) -> Result<CarCounts, AppError>` #### `compute_car_counts(ctx: &AnalysisContext, car_list: Vec<Car>) -> Result<CarCounts, AppError>`
**Aggregates per-car win counts from analysis context** **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 - 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** **Escapes a field for safe CSV inclusion**
- Wraps in quotes if field contains comma, quote, newline, or carriage return - 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 | `<track> <condition> <rq>` |
---
## Parsing Module (`src/parsing/`)
### Constants (`src/parsing/mod.rs`)
| Constant | Value | Description | | 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) | | `TIME_TOLERANCE` | 0.005 | Tolerance for tie detection (seconds) |
| `TEST_BOWL_FACTOR` | 2000.0 | Multiplier for Test Bowl time inversion | | `TEST_BOWL_FACTOR` | 2000.0 | Multiplier for Test Bowl time inversion |
| `CONCURRENCY_LIMIT` | 20 | Maximum concurrent tasks | | `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 | | `CAR_TIMING_API_BASE` | API URL | Base URL for per-car timing data |
#### CLI Commands ### Key Functions
| Command | Description | Flags | #### `parse_car_times(car, my_cars, best_cars, times_dir) -> Result<(), AppError>`
|---------|-------------|-------| **Processes timing data for a single car** (`src/parsing/mod.rs`)
| `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 | `<track> <condition> <rq>` |
#### Key Internal Functions - **Synchronous** — designed to be called from `tokio::task::spawn_blocking`
- Reads car JSON file from the times directory
##### `run_analysis(owned: &HashSet<String>) -> 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<String>, 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
- Processes each configuration and track - Processes each configuration and track
- Applies quality filters (downvotes, minimum time) - Applies quality filters (downvotes, minimum time)
- Handles Test Bowl time inversion - 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)` #### `update_best_car(best_cars, track_id, car_rq, surface, new_record)`
**Updates best car records with a new performance entry** **Updates best car records with a new performance entry** (`src/parsing/mod.rs`)
- Iterates RQ range from car's RQ to `MAX_RQ` - Iterates RQ range from car's RQ to `MAX_RQ`
- Replaces existing record if new time is faster - Replaces existing record if new time is faster
- Adds to tie list if within `TIME_TOLERANCE` - Adds to tie list if within `TIME_TOLERANCE`
- Prevents duplicate `car_rid` entries in tie lists - 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<Car>` #### `parse_all_times(my_cars, ctx, cars_db_path, times_dir) -> Result<Vec<Car>, AppError>`
**Orchestrates concurrent parsing of all car timing files** **Orchestrates concurrent parsing of all car timing files** (`src/parsing/mod.rs`)
- Loads car list from database
- Uses `Semaphore(CONCURRENCY_LIMIT)` for bounded concurrency - 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 - 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<Vec<Car>, 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<MyCarsData, AppError>`
**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** **Computes the ideal wishlist**
- Filters to cars with count >= `min_count` - 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 - Sorts by RQ
- Caps at `max_wishlist` - Caps at `max_wishlist`
#### Output Functions ### Sorting Helpers
##### `write_sorted_csv(path, header, entries, cmp, format_row)` #### `compare_records(a: &BestCarRecord, b: &BestCarRecord) -> std::cmp::Ordering`
**Generic sorted CSV writer** **Comparison for BestCarRecord sorting**
- Sorts by index (avoids cloning entries) - Sorts by: RQ → Brand → Model name
- 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**

141
docs/ARCHITECTURE.md
View file

@ -2,7 +2,7 @@
## System Overview ## 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 ## 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` - **Data Types**: Defines `Car`, `BestCarRecord`, `AnalysisContext`, `CarCounts`, `MyCarsData`, `AppError`
- **Track Name Translation**: Converts track codes to human-readable names via `translate_track_name()` - **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()` - **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()` - **User Data Loading**: Parses `my_cars.json` via `load_my_cars_data()`
- **Car Comparison**: Sorting comparator via `compare_cars()` - **Car Comparison**: Sorting comparator via `compare_cars()`
- **Count Aggregation**: Per-car win count computation via `compute_car_counts()` - **Count Aggregation**: Per-car win count computation via `compute_car_counts()`
@ -26,18 +26,44 @@ All public functions return `Result<T, AppError>` where `AppError` is a unified
- `AppError::Network` — HTTP/download errors - `AppError::Network` — HTTP/download errors
- `AppError::Parse` — JSON parsing errors with file context - `AppError::Parse` — JSON parsing errors with file context
- `AppError::MissingFile` — Required data file not found - `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`) ### 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 #### Key Responsibilities
- **Data Fetching**: Downloads car and track databases from GitHub repository - **CLI Definition**: `clap` derive-based argument parsing with global path options
- **File Processing**: Parses JSON timing files for each car configuration - **Command Dispatch**: Routes to the appropriate command handler in `commands/`
- **Performance Analysis**: Compares timing data across RQ ranges and track conditions - **Error Reporting**: Catches all errors and prints to stderr with exit code 1
- **Collection Management**: Identifies owned cars and optimization opportunities
- **CSV Output**: Writes analysis results to CSV files with proper escaping #### 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 #### Key Constants
- `MAX_RQ: u8 = 113` — Maximum RQ level used for range analysis - `MAX_RQ: u8 = 113` — Maximum RQ level used for range analysis
@ -46,19 +72,43 @@ All public functions return `Result<T, AppError>` where `AppError` is a unified
- `TEST_BOWL_FACTOR: f64 = 2000.0` — Multiplier for Test Bowl time inversion - `TEST_BOWL_FACTOR: f64 = 2000.0` — Multiplier for Test Bowl time inversion
- `CONCURRENCY_LIMIT: usize = 20` — Maximum concurrent tasks - `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 #### Data Flow
``` ```
GitHub Repository GitHub Repository / API
↓ ↓
cars_repo.json + tracks_repo.json cars_repo.json + tracks_repo.json ← fetch command
↓ ↓
times/*.json (1000+ car files) times/*.json (1000+ car files) ← fetch command
↓ ↓
JSON Parsing → BestCarRecord Analysis parse_all_times() → parse_car_times() × N cars (concurrent via spawn_blocking)
↓ ↓
User Collection (my_cars.json) AnalysisContext (DashMap<TripletKey, Vec<BestCarRecord>>)
↓ ↓
compute_car_counts() → CarCounts
↓
User Collection (my_cars.json) → owned/wishlist sets
↓
Output: CSV files / Terminal Results Output: CSV files / Terminal Results
``` ```
@ -74,7 +124,7 @@ type TripletKey = (String, u8, String);
``` ```
**Purpose**: Composite key for uniquely identifying track conditions **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 #### Car
```rust ```rust
@ -82,10 +132,10 @@ type TripletKey = (String, u8, String);
struct Car { struct Car {
rid: String, // Repository ID rid: String, // Repository ID
brand: String, // Manufacturer brand: String, // Manufacturer
only_name: String, // Model name only_name: String, // Model name (serde rename: "onlyName")
year: u16, // Model year year: u16, // Model year
rq: u8, // Rarity level rq: u8, // Rarity level
prize: bool, // Prize car status prize: bool, // Prize car status (serde default)
guid: String, // Game GUID guid: String, // Game GUID
} }
``` ```
@ -111,13 +161,13 @@ struct BestCarRecord {
### AnalysisContext ### AnalysisContext
```rust ```rust
struct AnalysisContext { struct AnalysisContext {
best_cars: Arc<Mutex<HashMap<TripletKey, Vec<BestCarRecord>>>>, best_cars: Arc<DashMap<TripletKey, Vec<BestCarRecord>>>,
} }
``` ```
**Thread Safety**: Protected by `Mutex` for concurrent access during parsing **Thread Safety**: Uses `DashMap` for sharded internal locking, providing high-concurrency access without a global mutex.
**Lifetime**: Created in `main()`, passed through the call chain **Lifetime**: Created in command handlers, passed through the call chain.
**Access**: `lock_best_cars()` method returns `Result` with proper error handling **Access**: Direct `DashMap` methods (`insert`, `get`, `iter`, `entry`).
### CarCounts ### CarCounts
```rust ```rust
@ -152,17 +202,19 @@ Multiple validation layers ensure data quality:
2. **Downvote Filter**: Community-downvoted times are excluded 2. **Downvote Filter**: Community-downvoted times are excluded
3. **Tie Detection**: Times within `TIME_TOLERANCE` (5ms) are considered equal 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`) 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 ### Concurrent Processing
#### Bounded Concurrency Pattern #### Bounded Concurrency Pattern
- **Task Spawning**: One blocking task per car file via `tokio::task::spawn_blocking` - **Semaphore First**: Semaphore permit acquired in async context (avoids deadlocking the blocking thread pool)
- **Semaphore**: Limits concurrent operations to `CONCURRENCY_LIMIT` (20) - **Blocking Work**: Synchronous parsing runs inside `tokio::task::spawn_blocking` while holding the permit
- **Error Isolation**: Individual car failures don't stop batch processing - **Concurrency Limit**: Maximum `CONCURRENCY_LIMIT` (20) simultaneous operations
#### Synchronization #### Synchronization
- **DashMap**: Sharded concurrent HashMap — no global lock needed for parallel parsing
- **Join Handles**: Waits for all parsing tasks to complete - **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 ## File Organization
@ -170,7 +222,7 @@ Multiple validation layers ensure data quality:
``` ```
├── cars_repo.json # Car database from repository ├── cars_repo.json # Car database from repository
├── tracks_repo.json # Track definitions ├── 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) ├── my_cars.json # User's car collection (game export)
└── times/ # Individual car timing files └── times/ # Individual car timing files
├── Ferrari_F40_1987.json ├── Ferrari_F40_1987.json
@ -191,12 +243,13 @@ Multiple validation layers ensure data quality:
## Performance Characteristics ## Performance Characteristics
### Records Module
- **Startup Time**: Fast (uses cached JSON files) - **Startup Time**: Fast (uses cached JSON files)
- **Memory Usage**: Moderate (loads all timing data into memory) - **Memory Usage**: Moderate (loads all timing data into memory)
- **Accuracy**: High (comprehensive dataset) - **Accuracy**: High (comprehensive dataset)
- **Update Frequency**: Manual refresh required - **Update Frequency**: Manual refresh required
- **Concurrency**: Bounded at 20 simultaneous operations via semaphore - **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 ## Error Handling Strategy
@ -206,12 +259,13 @@ All errors are represented as `AppError`:
- **Network Errors**: Download failures with descriptive messages - **Network Errors**: Download failures with descriptive messages
- **Parse Errors**: JSON parsing failures with file name context - **Parse Errors**: JSON parsing failures with file name context
- **Missing Files**: Specific variant for absent data files - **Missing Files**: Specific variant for absent data files
- **Poisoned Mutex**: Thread panic detection - **Internal Errors**: Runtime failures (semaphore closed, task join errors)
### Graceful Degradation ### Graceful Degradation
- **Missing User Data**: `load_my_cars_or_warn()` returns empty data with a warning - **Missing User Data**: `load_my_cars_or_warn()` returns empty data with a warning
- **Network Errors**: Fetch errors logged but don't crash - **Missing Timing Files**: Individual car file failures emit warnings and skip
- **Parse Failures**: 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 ### User Feedback
- **Progress Indication**: Shows fetching/download progress - **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` 2. Add track name mapping in `track_names.json`
3. Add condition mappings in `translate_track_type()` if needed 3. Add condition mappings in `translate_track_type()` if needed
### Adding New Analysis Types ### Adding New Commands
1. Create new output function(s) using `write_sorted_csv` 1. Create new command handler in `src/commands/`
2. Add command variant to the `Commands` enum 2. Add command variant to the `Commands` enum in `src/records.rs`
3. Add command handler in `main()` 3. Add dispatch case in the `run()` function
4. Update help text and documentation 4. Update documentation
### Adding New Output Formats ### Adding New Output Formats
1. Create new output function(s) alongside existing CSV writers 1. Create new output function(s) in `src/output/mod.rs`
2. Reuse `CarCounts` for data access 2. Reuse `CarCounts` and `AnalysisContext` for data access
3. Add CLI flag for format selection if needed 3. Add CLI flag for format selection if needed
## Security Considerations ## Security Considerations
@ -241,6 +295,7 @@ All errors are represented as `AppError`:
### Network Access ### Network Access
- **HTTPS Only**: All external requests use HTTPS - **HTTPS Only**: All external requests use HTTPS
- **Rate Limiting**: Respects server limits with controlled concurrency - **Rate Limiting**: Respects server limits with controlled concurrency
- **Atomic Writes**: Downloads write to `.tmp` files first, then rename on success
### Local File Access ### Local File Access
- **Read Only**: No modification of game files - **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 3. **Configuration Files**: External configuration for analysis parameters
4. **GUI Interface**: Web-based interface for non-technical users 4. **GUI Interface**: Web-based interface for non-technical users
5. **Export Formats**: Multiple output formats (JSON, XML, etc.) 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 ### Scalability Considerations
1. **Pagination**: Handle large datasets with pagination 1. **Pagination**: Handle large datasets with pagination

23
docs/USER_GUIDE.md
View file

@ -47,6 +47,22 @@ Fetching https://api.topdrivesrecords.com/car/1002.json.
## Command Reference ## Command Reference
### Global Options
All commands accept these optional flags to customize file paths:
| Flag | Default | Description |
|------|---------|-------------|
| `--my-cars <PATH>` | `my_cars.json` | Path to user's car collection export |
| `--cars-db <PATH>` | `cars_repo.json` | Path to car database file |
| `--times-dir <DIR>` | `times` | Directory containing per-car timing JSON files |
| `--tracks-db <PATH>` | `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 ### Basic Analysis Commands
#### Best Cars Analysis #### 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 **Problem:** No arguments provided
**Solution:** Use `best`, `trash`, `wishlist`, `all`, `details`, `track`, or `fetch` **Solution:** Use `best`, `trash`, `wishlist`, `all`, `details`, `track`, or `fetch`
#### "Could not read my cars file" #### "Could not read my cars file" / "Missing data file"
**Problem:** `my_cars.json` not found or not readable **Problem:** `my_cars.json` (or another data file) not found or not readable
**Solution:** **Solution:**
1. Export fresh data from Top Drives game 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 <PATH>` to specify a custom location)
3. Check file permissions 3. Check file permissions
4. If `cars_repo.json` is missing, run `fetch` first (or use `--cars-db <PATH>`)
#### "Failed to fetch the file" #### "Failed to fetch the file"
**Problem:** Network issues or API unavailable **Problem:** Network issues or API unavailable

396
plans/code-review.md
View file

@ -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<Mutex>`. Consider having `parse_car_times` accept `&Arc<Mutex<...>>` 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<T, E>` 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<HashMap>` 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<Mutex<HashMap<TripletKey, Vec<BestCarRecord>>>>
```
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