From 8409f434c9b57d85d25621303aaed8f5497dcc9e Mon Sep 17 00:00:00 2001
From: ruv <ruv@ruv.net>
Date: Tue, 21 Jul 2026 18:10:17 -0700
Subject: [PATCH] fix(adr-186): make exported .rvf filenames process-unique
 (fixes CI flake)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 .../src/training_api.rs                       | 39 ++++++++++++++++---
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/v2/crates/wifi-densepose-sensing-server/src/training_api.rs b/v2/crates/wifi-densepose-sensing-server/src/training_api.rs
index da059109..f9666914 100644
--- a/v2/crates/wifi-densepose-sensing-server/src/training_api.rs
+++ b/v2/crates/wifi-densepose-sensing-server/src/training_api.rs
@@ -26,7 +26,7 @@
 
 use std::collections::VecDeque;
 use std::path::PathBuf;
-use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
 use std::sync::{Arc, Mutex};
 
 use axum::{
@@ -54,6 +54,24 @@ pub const MODELS_DIR: &str = "data/models";
 /// `dataset_id` maps to `{RECORDINGS_DIR}/{dataset_id}.csi.jsonl`.
 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.
 const N_KEYPOINTS: usize = 17;
 /// 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 {
             error!("Failed to create models directory: {e}");
         } else {
-            let model_id = format!(
-                "trained-{}-{}",
-                training_type,
-                chrono::Utc::now().format("%Y%m%d_%H%M%S")
-            );
+            let model_id = next_model_id(training_type);
             let rvf_path = PathBuf::from(MODELS_DIR).join(format!("{model_id}.rvf"));
 
             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
     /// by an explicit truthy opt-out, so a `--no-default-features` / default
     /// build always has server training ON (no silent regression to disabled).