fix(execution): a cache record predating quote_increment is a MISS, not an "unknown"

keel/execution/executor.py

@@ -379,7 +379,7 @@ def _withdrawals_enabled(repo: Any, now_ts: int) -> bool | None:
 
 
 def _base_increment_for(
-    broker: Any, repo: Repository, product_id: str, now_ts: int
+    broker: Any, repo: Repository, product_id: str, now_ts: int, *, force_refresh: bool = False
 ) -> Decimal | None:
     """The venue's finest acceptable `base_size` for `product_id`, cached, or `None` if unknown.
 
@@ -397,12 +397,18 @@ def _base_increment_for(
     """
     key = f"{BASE_INCREMENT_PREFIX}{product_id}"
     cached = repo.get_state(key)
-    if isinstance(cached, dict):
+    if not force_refresh and isinstance(cached, dict):
         fetched_at = cached.get("fetched_at")
         raw = cached.get("increment")
         if isinstance(fetched_at, int) and now_ts - fetched_at < BASE_INCREMENT_TTL_SEC:
             return _coerce_increment(raw)
 
+    # `force_refresh` skips the freshness check, never the record: a caller uses it when the
+    # record is fresh but INCOMPLETE (`_price_increment_for`, for a record predating
+    # `quote_increment`). Deleting the row first would be the shorter way to force a miss and the
+    # wrong one -- a venue call that then fails would have thrown away a perfectly good
+    # `base_increment` and put SELL sizes back on the wire unquantized, which is #513.
+
     if broker is None:
         # Paper mode passes no broker; expected, not an error (same reasoning as
         # `_fetch_available_quote`).
@@ -422,9 +428,18 @@ def _base_increment_for(
     # Both increments ride ONE fetch and ONE cached record. Splitting them would double the
     # venue round-trips inside the order-placement path -- the latency the per-product read in
     # `get_instrument` exists to avoid -- for two fields of the same response (#802).
-    record: dict[str, object] = {"increment": str(increment), "fetched_at": now_ts}
-    if instrument.quote_increment is not None:
-        record["quote_increment"] = str(instrument.quote_increment)
+    # `quote_increment` is written ALWAYS, explicitly `None` when the venue reports none. The
+    # key's PRESENCE is what tells `_price_increment_for` this record was written by a build that
+    # knew to ask; writing it only when there is a value would make "venue has no tick"
+    # indistinguishable from "record predates the field", and the two need opposite handling --
+    # honour the first, refetch the second.
+    record: dict[str, object] = {
+        "increment": str(increment),
+        "fetched_at": now_ts,
+        "quote_increment": (
+            None if instrument.quote_increment is None else str(instrument.quote_increment)
+        ),
+    }
     repo.set_state(key, record)
     return increment
 
@@ -439,17 +454,31 @@ def _price_increment_for(
     unrounded, exactly as they did before #802. That is deliberately NOT a refusal -- refusing
     here would leave a filled position with no stop at all, which is the outcome #799 documents.
 
-    A record written before #802 carries no `quote_increment` key and reads as unknown until it
-    expires, which is correct: nothing knew the tick when it was written.
+    **A record that predates the field is a MISS, not an "unknown"** -- and the first cut of this
+    function got that wrong. It treated the absent key as unknown and let the 7-day TTL run, so a
+    deployment upgraded mid-TTL kept sending unrounded prices for up to a week, with the position
+    the fix was cut for sitting unprotected the whole time (live, 2026-09-17: a record written 24
+    hours earlier by the previous build counted as fresh, and the venue rejected the bracket
+    exactly as it had before the upgrade).
+
+    The two cases are distinguishable because the writer above always records the key, explicitly
+    `None` when the venue reports no tick. So `quote_increment` **absent** means "written before
+    anyone asked" -> refetch once; **present and null** means "the venue was asked and has none"
+    -> honour it, and do NOT re-ask on every cycle, which would put a venue round-trip back into
+    the order path this cache exists to keep out of it.
+
+    A returned `None` still means UNKNOWN: prices go on the wire unrounded, exactly as they did
+    before #802. That is deliberately NOT a refusal -- refusing here would leave a filled position
+    with no stop at all, which is the outcome #799 documents.
     """
     key = f"{BASE_INCREMENT_PREFIX}{product_id}"
     cached = repo.get_state(key)
-    if isinstance(cached, dict):
+    if isinstance(cached, dict) and "quote_increment" in cached:
         fetched_at = cached.get("fetched_at")
         if isinstance(fetched_at, int) and now_ts - fetched_at < BASE_INCREMENT_TTL_SEC:
             return _coerce_increment(cached.get("quote_increment"))
 
-    _base_increment_for(broker, repo, product_id, now_ts)
+    _base_increment_for(broker, repo, product_id, now_ts, force_refresh=True)
     refreshed = repo.get_state(key)
     if isinstance(refreshed, dict):
         return _coerce_increment(refreshed.get("quote_increment"))

tests/execution/test_price_precision.py

@@ -268,12 +268,17 @@ def test_both_increments_share_one_fetch(repo):  # noqa: F811
     assert broker.calls == 1
 
 
-def test_a_record_without_a_quote_increment_reads_unknown(repo):  # noqa: F811
-    """A cache record written before #802 carries no `quote_increment` key. Unknown is correct --
-    nothing knew the tick when it was written -- and unknown must not become a guessed 0.01."""
+def test_an_unparseable_quote_increment_reads_unknown(repo):  # noqa: F811
+    """Unknown must never become a guessed `0.01`.
+
+    This test used to assert that a record with the key ABSENT also read unknown. That was the
+    defect, not the contract -- see `test_a_record_written_before_the_field_existed_is_refetched`
+    below, and the live failure it names. What remains true is the narrower claim: a value that
+    is present and unusable is unknown, and is not repaired by guessing the common tick.
+    """
     repo.set_state(
         f"{executor.BASE_INCREMENT_PREFIX}PAXG-USD",
-        {"increment": "0.00000001", "fetched_at": NOW_TS},
+        {"increment": "0.00000001", "quote_increment": "not-a-number", "fetched_at": NOW_TS},
     )
 
     assert executor._price_increment_for(_InstrumentBroker(), repo, "PAXG-USD", NOW_TS) is None
@@ -292,3 +297,61 @@ def test_no_broker_reads_unknown_rather_than_raising(repo):  # noqa: F811
     """Paper mode passes no broker. `_base_increment_for` is documented as never raising, and
     this must not be the function that reintroduces one into the order path."""
     assert executor._price_increment_for(None, repo, "PAXG-USD", NOW_TS) is None
+
+
+# -- a cache record that predates the field must not wait out its TTL --------------------------
+
+
+def test_a_record_written_before_the_field_existed_is_refetched(repo):  # noqa: F811
+    """The live failure this covers (2026-09-17, keel-live.db).
+
+    `base_increment:PAXG-USD` was written by a pre-#802 build 24 hours before the cycle, so the
+    7-day TTL counted it FRESH and `_price_increment_for` returned the absent key as "unknown".
+    Prices went out unrounded and the venue rejected them -- for up to a week, on exactly the
+    deployment the fix was cut for, while the position sat unprotected.
+
+    "Unknown" is right when nothing knows the tick. It is wrong when the record simply predates
+    the question, and the two are distinguishable: a record that was WRITTEN with knowledge of
+    the field always carries the key, explicitly null when the venue reports none.
+    """
+    repo.set_state(
+        f"{executor.BASE_INCREMENT_PREFIX}PAXG-USD",
+        {"increment": "0.00001", "fetched_at": NOW_TS},  # no `quote_increment` key at all
+    )
+    broker = _InstrumentBroker()
+
+    assert executor._price_increment_for(broker, repo, "PAXG-USD", NOW_TS) == CENT
+    assert broker.calls == 1, "an incomplete record must be refetched, not waited out"
+
+
+def test_the_refetch_happens_once_and_then_the_record_is_complete(repo):  # noqa: F811
+    """Self-healing, not a fetch on every cycle."""
+    repo.set_state(
+        f"{executor.BASE_INCREMENT_PREFIX}PAXG-USD",
+        {"increment": "0.00001", "fetched_at": NOW_TS},
+    )
+    broker = _InstrumentBroker()
+
+    executor._price_increment_for(broker, repo, "PAXG-USD", NOW_TS)
+    executor._price_increment_for(broker, repo, "PAXG-USD", NOW_TS)
+
+    assert broker.calls == 1
+
+
+def test_a_venue_that_reports_no_tick_is_not_refetched_every_cycle(repo):  # noqa: F811
+    """The trap in the obvious fix.
+
+    "Refetch when the key is missing" would refetch FOREVER for a product whose venue genuinely
+    reports no tick, adding a venue round-trip to every order -- the latency the per-product read
+    exists to avoid. The key is therefore always written, explicitly null, so "absent" means
+    "written before the field" and nothing else.
+    """
+    broker = _InstrumentBroker(quote_increment=None)
+
+    assert executor._price_increment_for(broker, repo, "X-USD", NOW_TS) is None
+    assert executor._price_increment_for(broker, repo, "X-USD", NOW_TS) is None
+
+    assert broker.calls == 1, "a venue's honest 'no tick' must be cached, not re-asked"
+    record = repo.get_state(f"{executor.BASE_INCREMENT_PREFIX}X-USD")
+    assert "quote_increment" in record, "the key must be written even when the value is unknown"
+    assert record["quote_increment"] is None