Browse Source

Move the negative-duration clamp out of humanize_duration()

humanize_duration() is a general-purpose helper, newly exposed as a template
filter, so clamping negatives inside it made every present and future caller
suppress the exact symptom of clock skew. It now renders a negative duration
with a leading minus sign, which also fixes the nonsensical output the divmod
decomposition previously produced for one (e.g. "-1d 23h 59m 55s").

The floor moves to Job.elapsed_time, which is the value NetBox displays and
covers the list, the detail panel, the script result view and runscript in one
place. The stored execution_time is untouched, so the API and exports still
surface the anomaly.

Also renames the sub-second branch's variable, which held a value in seconds
rather than milliseconds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Jeremy Stretch 2 weeks ago
parent
commit
feaa8698a0

+ 17 - 9
netbox/utilities/string.py

@@ -13,23 +13,26 @@ def humanize_duration(value):
     """
     Express a timedelta in a human-friendly format. Example: 1h 5m 23s. Durations of a second or
     more are rounded to whole seconds; shorter durations are rounded to the millisecond (e.g.
-    0.43s). Returns an empty string for None; zero and negative durations render as "0s".
+    0.43s). A negative duration is rendered with a leading minus sign, so that an anomalous value
+    remains recognizable as one. Returns an empty string for None; zero renders as "0s".
     """
     if value is None:
         return ''
 
-    # Negative durations (which can result from clock skew) are clamped to zero
-    total_seconds = max(value.total_seconds(), 0)
+    total_seconds = value.total_seconds()
+    magnitude = abs(total_seconds)
 
     # Render sub-second durations to the millisecond, as rounding them to whole seconds would
     # report every short-lived duration as zero. Trailing zeros are stripped.
-    if 0 < total_seconds < 1:
-        milliseconds = f'{total_seconds:.3f}'.rstrip('0').rstrip('.')
-        if milliseconds != '0':
-            return f'{milliseconds}s'
+    if 0 < magnitude < 1:
+        rendered = f'{magnitude:.3f}'.rstrip('0').rstrip('.')
+        # A magnitude below a millisecond has no representation here, so fall through to "0s".
+        # Rounding up to a whole second (e.g. 0.9996) likewise falls through, to "1s".
+        if rendered not in ('0', '1'):
+            return f'-{rendered}s' if total_seconds < 0 else f'{rendered}s'
 
     # Round to whole seconds and decompose
-    days, remainder = divmod(round(total_seconds), 86400)
+    days, remainder = divmod(round(magnitude), 86400)
     hours, remainder = divmod(remainder, 3600)
     minutes, seconds = divmod(remainder, 60)
 
@@ -42,7 +45,12 @@ def humanize_duration(value):
         ret += f'{minutes}m '
     if seconds or not ret:
         ret += f'{seconds}s'
-    return ret.strip()
+    ret = ret.strip()
+
+    # Zero carries no sign, however the original value was signed
+    if total_seconds < 0 and ret != '0s':
+        ret = f'-{ret}'
+    return ret
 
 
 def enum_key(value):

+ 3 - 1
netbox/utilities/templatetags/helpers.py

@@ -217,13 +217,15 @@ def _format_speed(speed, divisor, unit):
 def humanize_duration_filter(value):
     """
     Express a timedelta in a human-friendly format, always using the largest appropriate units.
-    Sub-second durations are rendered with millisecond precision.
+    Sub-second durations are rendered with millisecond precision. A negative duration is rendered
+    with a leading minus sign rather than being suppressed.
 
     Examples:
 
         timedelta(seconds=90) => "1m 30s"
         timedelta(hours=1, minutes=5, seconds=23) => "1h 5m 23s"
         timedelta(milliseconds=430) => "0.43s"
+        timedelta(seconds=-5) => "-5s"
     """
     return humanize_duration(value)
 

+ 14 - 4
netbox/utilities/tests/test_string.py

@@ -43,7 +43,17 @@ class HumanizeDurationTest(TestCase):
         self.assertEqual(humanize_duration(timedelta(seconds=1, milliseconds=100)), '1s')
         self.assertEqual(humanize_duration(timedelta(seconds=59, milliseconds=600)), '1m')
 
-    def test_negative_duration_clamped_to_zero(self):
-        # A negative duration (e.g. resulting from clock skew) never renders as negative.
-        self.assertEqual(humanize_duration(timedelta(seconds=-1.5)), '0s')
-        self.assertEqual(humanize_duration(timedelta(days=-2)), '0s')
+    def test_negative_duration_retains_sign(self):
+        # A negative duration is anomalous, so it is rendered as such rather than suppressed here.
+        # Callers which need a floor of zero (e.g. Job.elapsed_time) apply one themselves.
+        self.assertEqual(humanize_duration(timedelta(seconds=-5)), '-5s')
+        self.assertEqual(humanize_duration(timedelta(seconds=-1.5)), '-2s')
+        self.assertEqual(humanize_duration(timedelta(days=-2)), '-2d')
+        self.assertEqual(humanize_duration(timedelta(milliseconds=-430)), '-0.43s')
+
+    def test_negative_duration_rounding_to_zero_carries_no_sign(self):
+        self.assertEqual(humanize_duration(timedelta(microseconds=-400)), '0s')
+
+    def test_sub_second_rounding_up_to_one_second(self):
+        # A magnitude which rounds up to a whole second reads as "1s", not "1.0s"
+        self.assertEqual(humanize_duration(timedelta(seconds=0.9996)), '1s')