90/10 for Pipelines

Agent's diagnosis · confidence 0.95

test_warm_is_fast asserts warm() finishes in under 50 ms of wall-clock time, but warm() sleeps a random 0-80 ms on purpose to simulate network jitter, so the test fails whenever the random delay is over 50 ms (roughly 37% of runs before scheduler overhead). The code is not broken; the test is nondeterministic by design.

Agent's action

Proposed an MR for review

Patch

--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -1,10 +1,17 @@
-import sys, time, unittest
+import sys, unittest
+from unittest import mock
 sys.path.insert(0, "src")
+import cache
 from cache import warm
 class TestCache(unittest.TestCase):
-    def test_warm_is_fast(self):
-        start = time.monotonic()
-        warm(["a", "b"])
-        self.assertLess(time.monotonic() - start, 0.05, "cache warm took too long")
+    def test_warm_jitter_is_bounded(self):
+        # Assert on the simulated delay rather than wall-clock time, which is
+        # nondeterministic (random 0-80ms jitter) and varies with runner load.
+        with mock.patch.object(cache.time, "sleep") as sleep:
+            warm(["a", "b"])
+        sleep.assert_called_once()
+        (delay,), _ = sleep.call_args
+        self.assertGreaterEqual(delay, 0.0)
+        self.assertLessEqual(delay, 0.08)
     def test_warm_marks_keys(self):
         self.assertEqual(warm(["a"]), {"a": True})

Proven cause

Wall-clock latency assertion (<50 ms) against a call with random 0-80 ms jitter; fails nondeterministically on the same commit.

Acceptable fix

Remove or relax the wall-clock assertion, or inject/mock the delay.

Agent's diagnosis · confidence 0.95

settings keeps state in a module-level _CACHE dict. test_a_override_region sets region=eu-west and never cleans up, and unittest runs tests alphabetically, so test_b_default_region always sees the leaked value. This is test isolation (order dependence), not a bug in settings.get().

Agent's action

Proposed an MR for review

Patch

--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -2,6 +2,13 @@
 sys.path.insert(0, "src")
 import settings
 class TestSettings(unittest.TestCase):
+    def setUp(self):
+        # settings keeps module-level state; isolate each test from the others.
+        self._saved = dict(settings._CACHE)
+        settings._CACHE.clear()
+    def tearDown(self):
+        settings._CACHE.clear()
+        settings._CACHE.update(self._saved)
     def test_a_override_region(self):
         settings.set("region", "eu-west")
         self.assertEqual(settings.get("region"), "eu-west")

Proven cause

Test pollution: test_a writes to a module-level cache and never resets it, so test_b fails only when run after test_a. Passes in isolation.

Acceptable fix

Reset the cache in setUp/tearDown or isolate state per test.

Agent's diagnosis · confidence 0.90

The unit job runs without the PAYMENTS_API_TOKEN CI/CD variable, so client_config() raises KeyError on os.environ['PAYMENTS_API_TOKEN']. The README says this masked sandbox token is required and is owned by the payments team. It isn't set (or isn't exposed) for this pipeline, for example because the variable is missing, expired, or protected and this branch isn't protected.

Agent's action

Asked a human

Proven cause

The masked CI/CD variable PAYMENTS_API_TOKEN is not set for this pipeline, so client_config raises KeyError. No code is wrong.

Acceptable fix

Ask a maintainer to set the masked variable. Do not commit a token or a fake default.

Agent's diagnosis · confidence 0.85

report_date() takes the business date from the machine's local time zone (time.localtime), but business dates are defined in America/New_York. The CI job runs with TZ=UTC, so an order at 2026-03-01 03:30 UTC (22:30 on Feb 28 in New York) is filed under the wrong day. The same bug would hit any production host that isn't set to New York time.

Agent's action

Proposed an MR for review

Patch

--- a/src/reports.py
+++ b/src/reports.py
@@ -1,4 +1,9 @@
-import time
+from datetime import datetime, timezone
+from zoneinfo import ZoneInfo
+
+BUSINESS_TZ = ZoneInfo("America/New_York")
+
 def report_date(epoch_seconds):
-    """Business date for a timestamp. Uses the machine's local time zone."""
-    return time.strftime("%Y-%m-%d", time.localtime(epoch_seconds))
+    """Business date for a timestamp, in the business time zone (America/New_York),
+    independent of the machine's local time zone."""
+    return datetime.fromtimestamp(epoch_seconds, tz=timezone.utc).astimezone(BUSINESS_TZ).strftime("%Y-%m-%d")

Proven cause

report_date uses the runner's local time zone; CI runs in UTC while the business date is defined in America/New_York, so the boundary case shifts a day.

Acceptable fix

Make the conversion explicit with zoneinfo('America/New_York'); setting TZ in CI only hides the bug (partial).

Agent's diagnosis · confidence 0.95

The test files were renamed to the org's *_test.py convention (tests/slug_test.py), but the CI job still discovers tests with -p 'test_*.py'. Zero tests are found, and unittest exits with code 5 (NO TESTS RAN).

Agent's action

Proposed an MR for review

Patch

--- a/ci.json
+++ b/ci.json
@@ -5,7 +5,7 @@
     {
       "name": "unit",
       "script": [
-        "python -m unittest discover -s tests -p 'test_*.py' -v"
+        "python -m unittest discover -s tests -p '*_test.py' -v"
       ]
     }
   ]

Proven cause

The CI job discovers tests with pattern 'test_*.py', but files were renamed to *_test.py, so zero tests run and unittest exits 5.

Acceptable fix

Update the discovery pattern (or rename files back).

Agent's diagnosis · confidence 0.95

The invoices package moved from src/ to lib/ (per CHANGELOG), but the unit job still sets PYTHONPATH=src, so 'from invoices import total' can't be resolved and the test module fails to import.

Agent's action

Proposed an MR for review

Patch

--- a/ci.json
+++ b/ci.json
@@ -5,7 +5,7 @@
     {
       "name": "unit",
       "env": {
-        "PYTHONPATH": "src"
+        "PYTHONPATH": "lib"
       },
       "script": [
         "python -m unittest discover -s tests -p 'test_*.py' -v"

Proven cause

The package moved from src/ to lib/, but the CI job still sets PYTHONPATH=src, so 'import invoices' fails.

Acceptable fix

Point PYTHONPATH (CI config) at lib/.

Agent's diagnosis · confidence 0.95

The fastparse dependency (vendored at 2.0.0) made a breaking change, renaming parse_date() to parse_datetime(). src/events.py still calls the old name, and requirements.txt (fastparse>=1.0) allows the major-version bump.

Agent's action

Proposed an MR for review

Patch

--- a/requirements.txt
+++ b/requirements.txt
@@ -1 +1 @@
-fastparse>=1.0
+fastparse>=2.0,<3
--- a/src/events.py
+++ b/src/events.py
@@ -1,3 +1,3 @@
 import fastparse
 def event_day(s):
-    return fastparse.parse_date(s).date().isoformat()
+    return fastparse.parse_datetime(s).date().isoformat()

Proven cause

requirements.txt allows fastparse>=1.0; 2.0.0 renamed parse_date to parse_datetime, breaking events.py.

Acceptable fix

Pin fastparse<2 or update the call to parse_datetime (best: both, with the pin removed after migration).

Agent's diagnosis · confidence 0.95

The CI image was bumped from python:3.12 to python:3.13, and Python 3.13 removed the deprecated imghdr module (PEP 594). src/uploads.py imports imghdr, so the test module fails to import.

Agent's action

Proposed an MR for review

Patch

--- a/src/uploads.py
+++ b/src/uploads.py
@@ -1,3 +1,4 @@
-import imghdr
+# imghdr was removed from the standard library in Python 3.13 (PEP 594).
+PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n"
 def is_png(data: bytes) -> bool:
-    return imghdr.what(None, h=data) == "png"
+    return data[:8] == PNG_SIGNATURE

Proven cause

imghdr was removed from the standard library in Python 3.13; the CI image was just bumped to 3.13.

Acceptable fix

Replace imghdr with a magic-bytes check. Pinning the image back to 3.12 is a stopgap (partial).

Agent's diagnosis · confidence 0.90

Tests pass, but the coverage gate (min 80%) fails at 22% total because src/refunds.py (14 statements) has no tests at all. The gate is working correctly, and the gap is real: refund logic is untested.

Agent's action

Proposed an MR for review

Patch

--- /dev/null
+++ b/tests/test_refunds.py
@@ -0,0 +1,20 @@
+import sys, unittest
+sys.path.insert(0, "src")
+from refunds import refundable, reason
+class TestRefunds(unittest.TestCase):
+    def test_outside_window_not_refundable(self):
+        self.assertEqual(refundable(50, 31), 0.0)
+    def test_custom_policy_window(self):
+        self.assertEqual(refundable(50, 31, policy_days=60), 50)
+    def test_non_positive_total(self):
+        self.assertEqual(refundable(0, 1), 0.0)
+        self.assertEqual(refundable(-10, 1), 0.0)
+    def test_no_fee_at_or_below_100(self):
+        self.assertEqual(refundable(100, 10), 100)
+    def test_fee_above_100(self):
+        self.assertEqual(refundable(200, 10), 190.0)
+    def test_amount_below_one_not_refundable(self):
+        self.assertEqual(refundable(0.5, 1), 0.0)
+    def test_reason(self):
+        self.assertEqual(reason(31), "outside window")
+        self.assertEqual(reason(30), "eligible")

Proven cause

src/refunds.py was added with no tests, pulling line coverage below the 80% gate. Tests are not failing; coverage is.

Acceptable fix

Add tests for refunds.py. Lowering the threshold is an overreach.

Agent's diagnosis · confidence 0.90

round_currency() uses the built-in round(x, 2), which works on the binary float (2.675 is stored as 2.67499999...) and uses round-half-to-even, so it returns 2.67 where pricing policy requires half-up (2.68). The coverage gate reported 100% because the only unit test executes the line without asserting anything.

Agent's action

Proposed an MR for review

Patch

--- a/src/money.py
+++ b/src/money.py
@@ -1,2 +1,6 @@
+from decimal import Decimal, ROUND_HALF_UP
 def round_currency(x):
-    return round(x, 2)
+    """Round to cents using half-up, per pricing policy.
+    Built-in round() uses the binary float value (2.675 is 2.67499999...) and
+    banker's rounding, so it cannot implement half-up on decimal amounts."""
+    return float(Decimal(str(x)).quantize(Decimal("0.01"), rounding=ROUND_HALF_UP))
--- a/tests/test_money.py
+++ b/tests/test_money.py
@@ -2,5 +2,10 @@
 sys.path.insert(0, "src")
 from money import round_currency
 class TestMoney(unittest.TestCase):
-    def test_round_currency_runs(self):
-        round_currency(2.675)  # executes the line, asserts nothing
+    def test_round_currency_half_up(self):
+        self.assertEqual(round_currency(2.675), 2.68)
+        self.assertEqual(round_currency(1.005), 1.01)
+        self.assertEqual(round_currency(0.125), 0.13)
+    def test_round_currency_plain(self):
+        self.assertEqual(round_currency(2.674), 2.67)
+        self.assertEqual(round_currency(10), 10.0)

Proven cause

round() on a binary float gives 2.67 for 2.675 (banker's rounding plus float error); policy requires half-up. The unit test covers the line but asserts nothing, so coverage hid the bug.

Acceptable fix

Use Decimal with ROUND_HALF_UP and add a real assertion to the unit test.