[BUG] SimulatedInfraScaler uses timedelta.seconds instead of total_seconds(), so a gap of a whole day reads as 0

Bug

SimulatedInfraScaler.scale (src/vasim/simulator/SimulatedInfraScaler.py:121) gates on the recovery window with .seconds:

if self.last_scaling_time is None or (time_now - self.last_scaling_time).seconds > self.recovery_time * 60:

timedelta.seconds is the sub-day remainder, not the total elapsed time: timedelta(days=1).seconds == 0. So a gap of a whole day (or any gap whose remainder-after-whole-days is under recovery_time) is misread as "still recovering", and the scale is silently suppressed.

Driving the real SimulatedInfraScaler with recovery_time=15, requesting 4 → 8 cores:

10 min (within recovery)   elapsed=   10min  scaled=False  expected=False  cpu=4
20 min (past recovery)     elapsed=   20min  scaled=True   expected=True   cpu=8
23h59m                     elapsed= 1439min  scaled=True   expected=True   cpu=8
exactly 24h                elapsed= 1440min  scaled=False  expected=True   cpu=4   <-- WRONG
2 days + 5min              elapsed= 2885min  scaled=False  expected=True   cpu=4   <-- WRONG
7 days                     elapsed=10080min  scaled=False  expected=True   cpu=4   <-- WRONG

Honest scope

I want to be straight about reachability rather than oversell this. The trigger is gap >= 24h AND (gap mod 24h) <= recovery_time — the decision has to land within ~15 minutes of the same clock time a whole number of days later, roughly 1% of possible gap phases.

I instrumented a full end-to-end run on this repo's own 8-day alibaba_control_c_29247_denom_1 data with the additive algorithm and got zero divergences: that workload rescales far too often to ever accumulate a 24h gap. So this is not reproduced by the bundled test data.

It needs a workload that holds one CPU limit across a day boundary — flat traffic, a wide min/max band, or a large lag. scale() is only called when new_limit != current_cpu_limit, and last_scaling_time only advances on a successful scale, so a stable period is exactly the case that accumulates the gap. The bundled trace does span 3+ days (2023.04.02 → 2023.04.05), so the time axis is there; the workload just isn't stable enough.

So: unambiguously wrong code, bad failure mode (the autoscaler is frozen precisely when a spike finally arrives after a quiet period), but latent rather than routinely hit.

Second defect on the same path

The "waiting to scale" log at :149 has the same .seconds flaw plus a unit error — it subtracts minutes from a seconds value:

self.recovery_time * 60 - (time_now - self.last_scaling_time).seconds // 60

With recovery_time=15 and 5 minutes elapsed it reports 895 minutes remaining instead of 10.

Suggested fix

-            if self.last_scaling_time is None or (time_now - self.last_scaling_time).seconds > self.recovery_time * 60:
+            if (
+                self.last_scaling_time is None
+                or (time_now - self.last_scaling_time).total_seconds() > self.recovery_time * 60
+            ):

-                    self.recovery_time * 60 - (time_now - self.last_scaling_time).seconds // 60,
+                    self.recovery_time - int((time_now - self.last_scaling_time).total_seconds() // 60),

Test coverage

tests/test_SimulatedInfraScaler.py only exercises recovery_time ± 1 minute (:112, :128), so neither existing test reaches a day-scale gap. The file's own TODO asks for more recovery-time scenarios; a timedelta(days=1) case fits there and fails before this change.

Happy to open a PR with the fix and that test if you'd like it.

https://claude.ai/code/session_01CFRv5kvesfnKYgHmNw9Vpf

Start in src/vasim/simulator/SimulatedInfraScaler.py at scale(), especially the recovery-window check around line 121 and the waiting log around line 149. Run tests/test_SimulatedInfraScaler.py first, then cover a timedelta(days=1) gap alongside the existing recovery-time cases. Done means day-scale gaps can scale and the remaining-time log reports minutes correctly.