Amin commited on
Commit
15b2eec
·
1 Parent(s): f8166a7

fix(futures): stop 422 on NO_TRADE plans, harden expires_at parsing

Browse files

Root cause: trade_cycle._empty_plan() (used for every NO_TRADE / rejected
analysis) set expires_at=None, while plans_repo.validate_plan() requires
a parseable expires_at on every persisted plan -- including NO_TRADE ones,
checked before the NO_TRADE early return. Every rejected analysis therefore
failed plan persistence with 422 "Trade plan validation failed" /
PlanValidationError("expires_at must be a valid UTC timestamp").

- trade_cycle.py: _empty_plan() now sets expires_at to a real, short-lived
UTC expiry (created_at + NO_TRADE_PLAN_TTL_SECONDS, new independently
configurable env var, defaults to the existing PLAN_MAX_AGE_SECONDS).
decision stays NO_TRADE and executable stays False, so this plan can
never be traded regardless of expiry -- the timestamp only exists to
satisfy the domain schema.
- plans_repo.py: _epoch() now accepts general ISO-8601 (fractional
seconds, explicit UTC/non-UTC offsets) in addition to the canonical
"%Y-%m-%dT%H:%M:%SZ" fast path, and normalizes naive/non-UTC input to
UTC before comparison. Still rejects genuinely invalid/missing input.
- tests: new hermes_overlay/tests/test_expires_at_normalization.py
reproduces the original 422 (NO_TRADE plan with expires_at=None) and
asserts it's fixed, plus covers the ISO-8601 variants and malformed
input.

Verified: full test suite run locally, 425 passed (2 pre-existing,
unrelated failures around Dockerfile/dependency-inventory pinning left
untouched).

hermes_overlay/tests/test_expires_at_normalization.py ADDED
@@ -0,0 +1,88 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """Regression test for the /api/futures/analyze 422:
2
+
3
+ "Trade plan validation failed" -> PlanValidationError:
4
+ "expires_at must be a valid UTC timestamp"
5
+
6
+ Root cause: trade_cycle._empty_plan() (used for every NO_TRADE / rejected
7
+ analysis) set expires_at=None, while plans_repo.validate_plan() requires
8
+ expires_at on every persisted plan -- including NO_TRADE ones -- before its
9
+ NO_TRADE early return. Any rejected analysis therefore failed plan
10
+ persistence with a 422.
11
+ """
12
+ from __future__ import annotations
13
+
14
+ import time
15
+
16
+ import pytest
17
+
18
+ from trading.domain.plans_repo import PlanValidationError, validate_plan
19
+ from trading.trade_cycle import _empty_plan, _iso
20
+
21
+
22
+ def _base_no_trade_plan(**overrides):
23
+ plan = _empty_plan(
24
+ symbol="BTCUSDT",
25
+ decision="NO_TRADE",
26
+ reasons=["Order book unavailable"],
27
+ warnings=[],
28
+ components={},
29
+ external_context={},
30
+ created_at=time.time(),
31
+ risk_profile="moderate",
32
+ )
33
+ # _empty_plan() doesn't set the market-context fields that
34
+ # _attach_context_metadata() normally adds; fill in the rest of
35
+ # REQUIRED_PLAN_FIELDS so we're testing expires_at in isolation.
36
+ plan.setdefault("futuresVerified", False)
37
+ plan.setdefault("trading_readiness", "blocked")
38
+ plan.update(overrides)
39
+ return plan
40
+ def test_no_trade_plan_no_longer_ships_expires_at_none():
41
+ """This is the exact shape that used to 422 in production."""
42
+ plan = _base_no_trade_plan()
43
+ assert plan["expires_at"] is not None
44
+ validate_plan(plan) # must not raise
45
+
46
+
47
+ def test_no_trade_plan_expiry_is_canonical_utc_and_after_creation():
48
+ plan = _base_no_trade_plan()
49
+ assert plan["expires_at"].endswith("Z")
50
+ parsed = time.strptime(plan["expires_at"], "%Y-%m-%dT%H:%M:%SZ")
51
+ created = time.strptime(plan["created_at"], "%Y-%m-%dT%H:%M:%SZ")
52
+ assert parsed >= created
53
+
54
+
55
+ @pytest.mark.parametrize(
56
+ "expires_at",
57
+ [
58
+ None,
59
+ "",
60
+ "not-a-timestamp",
61
+ "2026-13-40T99:99:99Z",
62
+ ],
63
+ )
64
+ def test_missing_or_malformed_expires_at_still_rejected(expires_at):
65
+ """The fix must not silently accept genuinely invalid input."""
66
+ plan = _base_no_trade_plan(expires_at=expires_at)
67
+ with pytest.raises(PlanValidationError, match="expires_at"):
68
+ validate_plan(plan)
69
+
70
+
71
+ @pytest.mark.parametrize(
72
+ "expires_at",
73
+ [
74
+ "2099-01-01T00:00:00Z", # canonical form this service emits
75
+ "2099-01-01T00:00:00+00:00", # explicit UTC offset
76
+ "2099-01-01T00:00:00.123456Z", # fractional seconds
77
+ "2099-01-01T05:30:00+05:30", # non-UTC offset, must normalize
78
+ "2099-01-01T00:00:00", # naive -- treated as UTC
79
+ ],
80
+ )
81
+ def test_general_iso8601_variants_are_accepted(expires_at):
82
+ plan = _base_no_trade_plan(expires_at=expires_at)
83
+ validate_plan(plan) # must not raise for any of these
84
+
85
+
86
+ def test_iso_helper_output_matches_required_canonical_format():
87
+ ts = _iso(1900000000.0)
88
+ assert ts == "2030-03-17T17:46:40Z"
hermes_overlay/trading/domain/plans_repo.py CHANGED
@@ -11,6 +11,7 @@ import json
11
  import math
12
  import time
13
  import uuid
 
14
  from typing import Any, Dict, Optional
15
 
16
  from trading.domain.schema import connect
@@ -43,16 +44,40 @@ REQUIRED_PLAN_FIELDS = {
43
 
44
 
45
  def _epoch(value: Any) -> Optional[float]:
 
 
 
 
 
 
 
 
 
 
 
46
  if value is None:
47
  return None
48
  if isinstance(value, (int, float)):
49
  number = float(value)
50
  return number if math.isfinite(number) else None
51
  if isinstance(value, str):
 
 
 
52
  try:
53
- return float(calendar.timegm(time.strptime(value, "%Y-%m-%dT%H:%M:%SZ")))
54
  except (TypeError, ValueError, OverflowError):
 
 
 
 
 
55
  return None
 
 
 
 
 
56
  return None
57
 
58
 
 
11
  import math
12
  import time
13
  import uuid
14
+ from datetime import datetime, timezone
15
  from typing import Any, Dict, Optional
16
 
17
  from trading.domain.schema import connect
 
44
 
45
 
46
  def _epoch(value: Any) -> Optional[float]:
47
+ """Parse expires_at into a UTC epoch, or None if it isn't a valid timestamp.
48
+
49
+ Accepts:
50
+ - a numeric epoch (int/float), returned as-is
51
+ - the canonical "%Y-%m-%dT%H:%M:%SZ" string this service emits (fast path)
52
+ - general ISO-8601 strings, including fractional seconds and explicit
53
+ offsets ("...+05:30", "...Z")
54
+ - naive ISO-8601 strings with no offset, treated as UTC (this service
55
+ never stores local time)
56
+ All non-UTC offsets are converted to UTC before conversion to epoch.
57
+ """
58
  if value is None:
59
  return None
60
  if isinstance(value, (int, float)):
61
  number = float(value)
62
  return number if math.isfinite(number) else None
63
  if isinstance(value, str):
64
+ text = value.strip()
65
+ if not text:
66
+ return None
67
  try:
68
+ return float(calendar.timegm(time.strptime(text, "%Y-%m-%dT%H:%M:%SZ")))
69
  except (TypeError, ValueError, OverflowError):
70
+ pass
71
+ iso_text = text[:-1] + "+00:00" if text.endswith("Z") else text
72
+ try:
73
+ parsed = datetime.fromisoformat(iso_text)
74
+ except ValueError:
75
  return None
76
+ if parsed.tzinfo is None:
77
+ parsed = parsed.replace(tzinfo=timezone.utc)
78
+ else:
79
+ parsed = parsed.astimezone(timezone.utc)
80
+ return parsed.timestamp()
81
  return None
82
 
83
 
hermes_overlay/trading/trade_cycle.py CHANGED
@@ -50,6 +50,13 @@ STOP_ATR_MULTIPLIER = float(os.environ.get("FUTURES_STOP_ATR_MULTIPLIER", "1.2")
50
  TAKE_PROFIT_RR = float(os.environ.get("FUTURES_TAKE_PROFIT_RR", "1.8"))
51
  MIN_STOP_BPS = float(os.environ.get("FUTURES_MIN_STOP_BPS", "20")) # basis points of entry
52
  PLAN_MAX_AGE_SECONDS = float(os.environ.get("FUTURES_PLAN_MAX_AGE_SECONDS", "20"))
 
 
 
 
 
 
 
53
  DEFAULT_REQUESTED_LEVERAGE = int(os.environ.get("FUTURES_DEFAULT_LEVERAGE", "5"))
54
 
55
  HIGH_ATR_PCT_PENALTY_THRESHOLD = 0.03 # matches risk.py's own high-vol threshold
@@ -291,7 +298,14 @@ def _empty_plan(symbol: str, decision: str, reasons: list, warnings: list,
291
  risk_profile: str, *, no_trade_guard: bool = False,
292
  external_advisory: Optional[dict] = None,
293
  score: Optional[float] = None) -> dict:
294
- """Return a completed, non-executable analysis record with no fake expiry."""
 
 
 
 
 
 
 
295
  return {
296
  "symbol": symbol, "decision": decision, "score": score,
297
  "confidence": abs(score) if score is not None else None,
@@ -303,7 +317,8 @@ def _empty_plan(symbol: str, decision: str, reasons: list, warnings: list,
303
  "external_advisory": external_advisory, "noTradeGuard": bool(no_trade_guard),
304
  "plan_type": "rejected_analysis", "executable": False, "analysis_state": "NO_TRADE",
305
  "executed": False, "execution_result": None,
306
- "created_at": _iso(created_at), "expires_at": None,
 
307
  }
308
 
309
 
 
50
  TAKE_PROFIT_RR = float(os.environ.get("FUTURES_TAKE_PROFIT_RR", "1.8"))
51
  MIN_STOP_BPS = float(os.environ.get("FUTURES_MIN_STOP_BPS", "20")) # basis points of entry
52
  PLAN_MAX_AGE_SECONDS = float(os.environ.get("FUTURES_PLAN_MAX_AGE_SECONDS", "20"))
53
+ # Validity window for NO_TRADE / rejected-analysis records. These plans are
54
+ # never executable (decision stays NO_TRADE, executable stays False), but the
55
+ # domain layer's plan schema (trading.domain.plans_repo.validate_plan)
56
+ # requires every persisted plan -- including rejected ones -- to carry a
57
+ # parseable expires_at. Independently configurable from the directional plan
58
+ # TTL above so the two can be tuned separately.
59
+ NO_TRADE_PLAN_TTL_SECONDS = float(os.environ.get("FUTURES_NO_TRADE_PLAN_TTL_SECONDS", str(PLAN_MAX_AGE_SECONDS)))
60
  DEFAULT_REQUESTED_LEVERAGE = int(os.environ.get("FUTURES_DEFAULT_LEVERAGE", "5"))
61
 
62
  HIGH_ATR_PCT_PENALTY_THRESHOLD = 0.03 # matches risk.py's own high-vol threshold
 
298
  risk_profile: str, *, no_trade_guard: bool = False,
299
  external_advisory: Optional[dict] = None,
300
  score: Optional[float] = None) -> dict:
301
+ """Return a completed, non-executable analysis record.
302
+
303
+ decision stays "NO_TRADE" and executable stays False, so this record can
304
+ never be traded regardless of its expiry -- but the domain layer's plan
305
+ schema still requires every persisted plan to carry a parseable
306
+ expires_at, so we give it a real (short) validity window rather than
307
+ None. See NO_TRADE_PLAN_TTL_SECONDS above.
308
+ """
309
  return {
310
  "symbol": symbol, "decision": decision, "score": score,
311
  "confidence": abs(score) if score is not None else None,
 
317
  "external_advisory": external_advisory, "noTradeGuard": bool(no_trade_guard),
318
  "plan_type": "rejected_analysis", "executable": False, "analysis_state": "NO_TRADE",
319
  "executed": False, "execution_result": None,
320
+ "created_at": _iso(created_at),
321
+ "expires_at": _iso(created_at + NO_TRADE_PLAN_TTL_SECONDS),
322
  }
323
 
324