fix(adr-186): make exported .rvf filenames process-unique (fixes CI flake)

Root cause: the trained-model filename was `trained-{type}-{%Y%m%d_%H%M%S}` — a
**second-resolution** timestamp. Two training runs of the same type that finish
in the same wall-clock second compute the SAME path and silently overwrite each
other's artifact. This is a real robustness gap (a second rapid run clobbers the
first model), and it surfaced as a flaky test on the Linux CI runner:
`http_train_start_produces_model_and_streams` asserts (via a before/after
directory diff) that a new `.rvf` appeared, but the concurrent
`training_job_streams_real_progress_and_writes_model` test — same `supervised`
type, same second — wrote to the identical path and then deleted it in its own
cleanup, so the diff saw no new file. Windows scheduling happened to avoid the
same-second overlap; Linux CI hit it (the sibling `TRAIN_ENV_LOCK` holders then
failed via std::sync::Mutex poison-cascade after the first panic).

Reproduced locally by isolating the three model-writing tests (which forces the
overlap): ~10/20 runs flaked before; 0/30 after.

Fix: `next_model_id()` appends microseconds (`%6f`) plus a monotonic per-process
`AtomicU64` counter, so every run gets a distinct path even for same-microsecond
concurrent completions. Pinned by `model_ids_are_unique_per_call` (1000 ids, all
distinct) so the scheme can't regress to non-unique. No test-level band-aid
(no added sleeps / serialization) — the collision is removed at the source.

Verified: cargo test -p wifi-densepose-sensing-server -p wifi-densepose-train
--no-default-features — bin 218 passed / 0 failed, all suites 0 failed; the
isolated-tests stress loop is 0/15 flakes post-fix.
This commit is contained in:
ruv
2026-07-21 18:10:17 -07:00
parent d99ea153ff
commit 8409f434c9
@@ -26,7 +26,7 @@
use std::collections::VecDeque; use std::collections::VecDeque;
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use axum::{ use axum::{
@@ -54,6 +54,24 @@ pub const MODELS_DIR: &str = "data/models";
/// `dataset_id` maps to `{RECORDINGS_DIR}/{dataset_id}.csi.jsonl`. /// `dataset_id` maps to `{RECORDINGS_DIR}/{dataset_id}.csi.jsonl`.
pub const RECORDINGS_DIR: &str = "data/recordings"; pub const RECORDINGS_DIR: &str = "data/recordings";
/// Monotonic per-process counter appended to exported model filenames so two
/// runs that complete in the same wall-clock microsecond still get distinct
/// paths (prevents silent overwrite; keeps concurrent runs from colliding).
static MODEL_ID_SEQ: AtomicU64 = AtomicU64::new(0);
/// Build a process-unique model id `trained-{type}-{ts_micros}-{seq}`. A
/// second-resolution timestamp alone collided for runs finishing in the same
/// second (silent overwrite); microseconds + the monotonic counter guarantee
/// uniqueness even for same-microsecond concurrent completions.
fn next_model_id(training_type: &str) -> String {
format!(
"trained-{}-{}-{}",
training_type,
chrono::Utc::now().format("%Y%m%d_%H%M%S_%6f"),
MODEL_ID_SEQ.fetch_add(1, Ordering::Relaxed)
)
}
/// Number of COCO keypoints. /// Number of COCO keypoints.
const N_KEYPOINTS: usize = 17; const N_KEYPOINTS: usize = 17;
/// Dimensions per keypoint in the target vector (x, y, z). /// Dimensions per keypoint in the target vector (x, y, z).
@@ -1368,11 +1386,7 @@ async fn run_training_job(
if let Err(e) = tokio::fs::create_dir_all(MODELS_DIR).await { if let Err(e) = tokio::fs::create_dir_all(MODELS_DIR).await {
error!("Failed to create models directory: {e}"); error!("Failed to create models directory: {e}");
} else { } else {
let model_id = format!( let model_id = next_model_id(training_type);
"trained-{}-{}",
training_type,
chrono::Utc::now().format("%Y%m%d_%H%M%S")
);
let rvf_path = PathBuf::from(MODELS_DIR).join(format!("{model_id}.rvf")); let rvf_path = PathBuf::from(MODELS_DIR).join(format!("{model_id}.rvf"));
let mut builder = RvfBuilder::new(); let mut builder = RvfBuilder::new();
@@ -2303,6 +2317,19 @@ mod tests {
); );
} }
/// Exported model ids must be unique per call — a second-resolution
/// timestamp alone collided for runs finishing in the same wall-clock second
/// (silently overwriting each other's `.rvf`, which also flaked the
/// concurrent model-writing tests on CI). Guards against regressing the
/// filename scheme back to non-unique.
#[test]
fn model_ids_are_unique_per_call() {
let ids: Vec<String> = (0..1000).map(|_| next_model_id("supervised")).collect();
let unique: std::collections::HashSet<&String> = ids.iter().collect();
assert_eq!(unique.len(), ids.len(), "every model id must be distinct");
assert!(ids[0].starts_with("trained-supervised-"));
}
/// ADR-186 P5: the enablement gate is enabled by default and only disabled /// ADR-186 P5: the enablement gate is enabled by default and only disabled
/// by an explicit truthy opt-out, so a `--no-default-features` / default /// by an explicit truthy opt-out, so a `--no-default-features` / default
/// build always has server training ON (no silent regression to disabled). /// build always has server training ON (no silent regression to disabled).