Skip to content

Commit 5a9427f

Browse files
costajohntclaude
andauthored
chore: post-audit tidies (CB flag path, correlation logging, no-touch list, dynamic-stop tiers) (#181)
Five small fixes from the comprehensive repo audit that don't justify their own PRs: H1: resilience._send_circuit_breaker_alert wrote the fallback flag file via a re-derived Path(__file__) instead of the module-level _CB_FLAG_PATH constant. Test fixtures that monkeypatch the constant (tests/conftest.py::_clear_circuit_breaker_flag) didn't redirect this fallback, so a failing alert in tests would touch the production data/ directory. Now uses the constant — fixture takes effect. L5: correlation._fetch_close_prices silently dropped symbols missing from the batch response (delisted, no data in window). Now logs a single WARNING listing the dropped symbols and the count, so operators can see when the guard is degraded. Add strategies/state_db.py to AGENT_NO_TOUCH_FILES. state_db owns the SQLite schema and atomic-transaction patterns for ALL trading state; an agent-authored migration could corrupt the live DB across the no-rollback boundary. New test_contains_state_db assertion locks it. M6: pipeline_backtest._dynamic_trailing_stop_pct hardcoded the 100/75/50/37.5 multipliers instead of reading TRAILING_STOP_TIERS from constants. The live exit manager (position_manager.py) reads the constant, so changing tier values in constants would silently desync the backtest from production. Now both consume the same source of truth. H4: scan_with_sentiment.py doesn't apply the pair-wise correlation guard. Documented as intentional — the watchlist is small (~4 curated tickers) and operators control composition directly. The orchestrator (screener/momentum entry paths) keeps the guard because its candidate universe is wide and dynamic. Test plan: - 2825/2825 pass (added 1 test_contains_state_db) - Per-file coverage floors OK (69 files; default 85%) - Total coverage 95.48% Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b4a9744 commit 5a9427f

6 files changed

Lines changed: 53 additions & 19 deletions

File tree

scripts/pipeline_backtest.py

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@
4848
compute_max_drawdown,
4949
compute_sharpe_ratio,
5050
)
51-
from strategies.constants import DEFAULT_TAKE_PROFIT_PCT, DEFAULT_TRAILING_STOP_PCT
51+
from strategies.constants import (
52+
DEFAULT_TAKE_PROFIT_PCT,
53+
DEFAULT_TRAILING_STOP_PCT,
54+
TRAILING_STOP_TIERS,
55+
TRAILING_STOP_UNDERWATER_MULTIPLIER,
56+
)
5257
from strategies.enums import Action
5358
from strategies.log import get_logger
5459
from strategies.signal_scorer import (
@@ -214,23 +219,19 @@ def is_on_cooldown(self, symbol: str, date: datetime) -> bool:
214219
def _dynamic_trailing_stop_pct(self, pnl_pct: float) -> float:
215220
"""Return effective trailing stop % based on unrealized P&L.
216221
217-
Mirrors position_manager.dynamic_trailing_stop_pct:
218-
P&L < 0%: 100% of base (full room)
219-
P&L 0-3%: 75% of base
220-
P&L 3-5%: 50% of base
221-
P&L >= 5%: 37.5% of base
222+
Mirrors position_manager.dynamic_trailing_stop_pct — both read the
223+
same tiers from strategies.constants so the backtest's behavior
224+
cannot drift from the live exit manager (a previous bug had
225+
hard-coded 100/75/50/37.5 multipliers here that silently fell out
226+
of sync when constants changed).
222227
"""
223228
if not self.use_dynamic_stop:
224229
return self.trailing_stop_pct
225230

226-
if pnl_pct < 0:
227-
return self.trailing_stop_pct
228-
elif pnl_pct < 3:
229-
return self.trailing_stop_pct * 0.75
230-
elif pnl_pct < 5:
231-
return self.trailing_stop_pct * 0.50
232-
else:
233-
return self.trailing_stop_pct * 0.375
231+
for threshold, multiplier in TRAILING_STOP_TIERS:
232+
if pnl_pct >= threshold:
233+
return self.trailing_stop_pct * multiplier
234+
return self.trailing_stop_pct * TRAILING_STOP_UNDERWATER_MULTIPLIER
234235

235236
# --- Entry ---
236237

scripts/scan_with_sentiment.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ def main():
183183
results = []
184184
tech_errors = 0
185185

186+
# Note: this watchlist scan does NOT apply the pair-wise correlation
187+
# guard (see strategies/correlation.py). The watchlist is small (~4
188+
# curated tickers) and operators control its composition directly,
189+
# so over-correlation is managed at the watchlist-design level rather
190+
# than enforced at runtime. The orchestrator's screener/momentum paths
191+
# use the correlation guard because their candidate universe is wide
192+
# and dynamic.
186193
for entry in WATCHLIST:
187194
# 1. Technical analysis
188195
strategy = MeanReversionStrategy(

strategies/correlation.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,21 +80,34 @@ def _fetch_close_prices(
8080
return {}
8181

8282
closes: dict[str, pd.Series] = {}
83+
dropped: list[str] = []
8384
if "symbol" in df.index.names:
8485
for sym in symbols:
8586
try:
8687
sym_df = df.xs(sym, level="symbol")
8788
if sym_df.empty:
89+
dropped.append(sym)
8890
continue
8991
closes[sym] = sym_df["close"].astype(float)
9092
except KeyError:
9193
# Symbol absent from the batch response (delisted, no
92-
# data in window, etc.). Drop silently.
94+
# data in window, etc.). Track and log so the missing
95+
# symbol isn't invisible to the caller.
96+
dropped.append(sym)
9397
continue
9498
else:
9599
# Single-symbol response shape (rare; normalise just in case).
96100
if len(symbols) == 1 and not df.empty:
97101
closes[symbols[0]] = df["close"].astype(float)
102+
else:
103+
dropped.extend(symbols)
104+
105+
if dropped:
106+
logger.warning(
107+
"Correlation guard: %d/%d symbols had no bars in the window "
108+
"(%s) — excluded from correlation matrix.",
109+
len(dropped), len(symbols), ", ".join(sorted(dropped)),
110+
)
98111

99112
return closes
100113

strategies/resilience.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,14 @@ def _send_circuit_breaker_alert():
156156
)
157157
except Exception as alert_err:
158158
logger.critical("CIRCUIT BREAKER: Could not send alert (%s) — check notify.py", alert_err)
159-
# Write flag file so pipeline can detect the trip even if alerts fail
159+
# Write flag file so pipeline can detect the trip even if alerts fail.
160+
# Use the module-level constant so test fixtures that monkeypatch
161+
# _CB_FLAG_PATH (e.g. tests/conftest.py::_clear_circuit_breaker_flag)
162+
# also redirect this fallback write — otherwise a failing alert in a
163+
# test would touch the production flag path.
160164
try:
161-
cb_path = Path(__file__).resolve().parent.parent / "data" / "circuit_breaker.flag"
162-
cb_path.parent.mkdir(parents=True, exist_ok=True)
163-
cb_path.write_text(str(time.time()))
165+
_CB_FLAG_PATH.parent.mkdir(parents=True, exist_ok=True)
166+
_CB_FLAG_PATH.write_text(str(time.time()))
164167
except OSError as file_err:
165168
logger.critical("CIRCUIT BREAKER: Cannot write flag file (%s) — trip has NO persistent record", file_err)
166169

strategies/safety.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@
4848
"strategies/risk_guard.py",
4949
"strategies/resilience.py",
5050
"strategies/safety.py",
51+
# state_db owns SQLite schema + atomic transaction patterns for ALL
52+
# trading state (HWMs, cooldowns, partial profits, equity snapshots,
53+
# PDT, EDGAR cache). A schema migration written by the agent could
54+
# corrupt the live DB across the no-rollback boundary.
55+
"strategies/state_db.py",
5156
"scripts/strategy_agent.py",
5257
"scripts/agent_tools.py",
5358
"scripts/agent_reads.py",

tests/test_safety.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ def test_contains_risk_guard(self):
8282
def test_contains_resilience(self):
8383
assert "strategies/resilience.py" in AGENT_NO_TOUCH_FILES
8484

85+
def test_contains_state_db(self):
86+
"""state_db.py owns SQLite schema + atomic transactions for ALL
87+
trading state — must be agent-protected."""
88+
assert "strategies/state_db.py" in AGENT_NO_TOUCH_FILES
89+
8590
def test_contains_agent_tools(self):
8691
assert "scripts/agent_tools.py" in AGENT_NO_TOUCH_FILES
8792

0 commit comments

Comments
 (0)