Fix 23 adversarial-review findings, Hyperliquid socket churn, and health honesty - #10
Merged
Merged
Conversation
…store failures loud M1: `snapshots` and `paper_trades` had no writer anywhere in src/ (the real paper-trade log lives in data/paper_trades.db with a different schema). Stop creating them, drop the v2 migration that only touched them, and add a v3 step that drops the legacy tables when empty (a populated one is left alone and reported). `_run_migrations` now runs only the steps above the DB's recorded version instead of every step on every startup, so future migrations no longer have to be idempotent-by-accident. M10: `discovered_addresses` is declared in DataStore's versioned schema. address_store write failures log at WARNING (were DEBUG); read failures RAISE instead of returning an empty set, so an unreadable store can no longer be mistaken for "no addresses yet" and silently re-discovered. The retention COUNT/DELETE moves from every write into prune(), called hourly by the hub, and the scanner persists only newly discovered addresses instead of rewriting the whole set. DataStore is constructed before PositionScanner so a corrupted DB reaches the quarantine path before address_store opens it. Also fixes L2 (negative dropped count on generator input). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rywhere a tier is shown rank_all() used fixed "top 100 / bottom 100" boundaries, so with fewer than 200 qualified wallets — the normal state of a fresh install — the "average" branch was unreachable and every ranked wallet was labelled smart or dumb (50 qualified -> all 50 "smart", including the worst). Tiers are now the top/bottom 10% of the qualified population (capped at 100 each); below 10 qualified wallets everything is ranked but "average". WalletProfile.confidence was computed and consumed by nothing. It now travels with the label: SmartMoneySignal.wallet_confidence (set in check_signals and the demo generator), wallets.confidence + smart_money_signals.wallet_confidence columns (v3 migration, save_wallet/load_wallets/get_signals), and a CONF column beside every smart/dumb row plus each signal line in the smart-money panel. Two tests in test_hardening.py asserted the buggy behaviour (a single qualified wallet labelled "smart") and are corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…igin allowlist for REST and WS On a loopback bind with no HYPERDATA_CORS_ORIGINS, _resolve_security returned None and _make_cors_middleware(None) emitted `Access-Control-Allow-Origin: *` on every response with no auth installed — so any web page the user had open could fetch() /v1/whales and /v1/positions/danger-zone and read the tracked wallet addresses back. Loopback is not a boundary against the user's own browser. - CORS headers are now emitted only for an Origin in the explicit allowlist, on every bind. No allowlist -> no CORS headers (curl/SDK clients send no Origin and are unaffected). - New first-position Host-guard middleware: on a loopback bind the Host header must itself be loopback (localhost / 127.x / [::1], with or without port), closing DNS rebinding for non-CORS clients too. - M7: _resolve_security returns the same set on every bind and both the REST CORS middleware and the WebSocket Origin gate read it, so the two surfaces can no longer disagree (REST was wildcard-open while WS rejected every browser origin). - README, .env.example, docs/DATA_INTEGRITY.md and the module docstring no longer describe wildcard CORS as the loopback default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nsumer
C3: the only warning for a half-dead order-flow feed was gated on
`venue_data_age(venue) != float("inf")`, which excluded exactly the venue
that had never delivered a byte — the confirmed-live Binance regional
block (handshake succeeds, zero frames, forever). Every consumer then
read the blended path: health_monitor called is_stale() (max of both
venues), cvd_dashboard/hub_panels rendered engine.cumulative_cvd as
"CVD", and /v1/orderflow returned the same sum. get_cumulative_cvd() had
no production callers.
OrderFlowEngine now tracks per-venue connection, frame, trade and
parse-error counters (VenueState) and classifies each venue as
ok / connecting / silent / frozen / stale / disconnected with a reason.
Consumers rewired:
- hub._update_feed_staleness: guard deleted; warns (debounced) for ANY
non-ok venue incl. never-connected; orderflow_engine status gains
'partial' when one venue is out; startup grace via status.started_at.
- health_monitor._check_freshness: order_flow_hyperliquid /
order_flow_binance checks (warn when that venue is out, fail when all
are); blended check names the contributing venues.
- api_server.handle_orderflow: cumulative_cvd_by_venue, venue_coverage,
venues_contributing next to the combined figure. handle_health: the
bare `except: pass` around venue_freshness() is gone (L7).
- cvd_dashboard.venue_cvd_text renders `CVD: +X [HL +a | BN silent]`;
both build_price_bar and HubCVD use it; demo engines are labelled
[DEMO] instead of being attributed to a venue.
- combined_dashboard badge: 'warn' -> ⚠ PARTIAL (was ✓ LIVE).
- README (CVD row, Binance data-source row) and docs/DATA_INTEGRITY.md
document the statuses and the regional caveat.
H2: _handle_binance_trade stamped liveness BEFORE parsing and swallowed
parse errors with a bare `pass` ("fresh but frozen", zero log output),
and frames without `data` (acks, error envelopes) returned before any
bookkeeping. Frames are now counted first; liveness is stamped only by a
successfully parsed trade; parse failures increment a per-venue counter
and log a rate-limited WARNING with the exception; all of it is exposed
via venue_freshness() -> /v1/health. Same treatment for the HL handler.
The Binance loop now logs the exception type/message on error and
distinguishes a clean server close (L5).
M11: _start_component's on_ok set 'connected' on a start() that only
creates tasks. WS-driven feeds now start as 'connecting' and the
watchdog promotes them on first real data (trade / book / liquidation /
HLP snapshot). Status panel shows connecting/partial in yellow with the
state name. test_hardening asserted the old 'connected' and is updated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r on warn; fix docs URL `status = "ok" if overall in (None, "ok", "warn") else "degraded"` mapped both "checks have not run yet" (the first ~45s of every live session) and "warn" (BTC price unavailable, zero funding symbols, one order-flow venue silent, ...) to top-level "ok", so a monitor keyed on status=="ok" never fired for a genuinely degraded terminal. Now: `initializing` (live mode, no verification result yet), `ok`, `warn` (a health check warned or a feed is 'partial'), `degraded` (a feed is stale/erroring, a cross-reference drifted, or a component failed to start). Documented in the README endpoint table. M12: the advertised docs URL pointed at a non-existent repository; it now matches `git remote` (Co-Messi/HyperData-Terminal). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`conn.execute("PRAGMA integrity_check")` never fetched its result. The
pragma does not raise — it returns `('ok',)` or a list of corruption
descriptions — so only damage severe enough to fail the open itself
("file is not a database") ever reached the quarantine path; a
page-corrupted file opened "successfully" and the app ran on it.
DataStore now runs `PRAGMA quick_check`, fetches the row, and raises
sqlite3.DatabaseError on anything but 'ok' into the existing
quarantine-and-recreate path. quick_check skips the index-consistency
scan so startup on a large DB stays fast. Tested by corrupting page 2 of
a real database file while leaving the header intact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…otice when it doesn't requirements.lock pinned rich==14.3.3 and pandas==3.0.1 while pyproject.toml declared rich<14.0 and pandas<3.0. CI installed the lock and then `pip install -e . --no-deps` — precisely the flag that stopped pip from raising the conflict — so every green run validated a dependency set that `pip install -e .` (the README's install path) could not produce. - pyproject.toml widens to rich<15 and pandas<4: the versions CI has actually been testing, and what a fresh `pip install -e .` resolves to today (rich 14.3.4 / pandas 3.0.5, `pip check` clean). requirements.txt mirrors the ranges. - tests/test_dependency_lock.py asserts, via tomllib + packaging, that every lock pin satisfies its pyproject specifier, that the lock has no undeclared direct deps, that requirements.txt mirrors pyproject, and that CI installs with dependency resolution and runs `pip check`. - CI: the lock leg installs `-e .` WITH dependencies and runs `pip check`; a new matrix leg installs via plain `pip install -e .` on 3.13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…everse-on-opposite flag M2: `if self._db:` skipped the entire persist-first block when the trade log was not open (never started, or already stopped) and apply_mutation() ran anyway — the one path the docstring's "a trade that cannot be logged is not executed" did not cover. `_db is None` is now a hard refusal logged at ERROR. The existing accounting tests built traders with no DB and relied on that hole; they now attach an in-memory log. M3: an opposite-side signal silently closed the position and dropped the strategy's directional intent until the next check_interval. The behaviour is now an explicit constructor flag, reverse_on_opposite_signal (default False, unchanged semantics — test_close_realizes_pnl still holds). The default is logged at start() and once at the first close-only close; with the flag, the close is followed by a second, fully checked and logged trade opening the reverse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An editor/tool hook in the development environment writes <file>.bak next to edited files; 'git add -A' swept them into the preceding commits. They are exact copies of the pre-edit sources (no secrets). Removed from the index and ignored going forward. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…IP WS cap; escape exchange/LLM strings in Rich output M4: hub.stop() wrapped nine component shutdowns in bare `except: pass`. They now run in a loop that logs each failure with logger.exception and keeps stopping the remaining components (and the API server). M5: _RateLimiter swept its whole dict on every request once it held >10k keys and removed nothing while those keys were active — O(n) per request exactly when the limiter mattered. Keys now live in an OrderedDict LRU with O(1) eviction of the least recently seen key above MAX_TRACKED_KEYS. M6: MAX_WS_CONNECTIONS was global only, so one client opening 10 sockets locked everyone else out. A per-source-address cap (MAX_WS_CONNECTIONS_PER_IP=3) is checked first; _WSClient records its remote. M9: exchange-supplied symbols reached Rich as bare `str` cells (hub_panels x4, liquidation_stream, market_overview x3, whale_tracker x3) and the LLM's `reason` reached console.print unescaped in paper_trader — after apply_mutation() had already changed the books. A symbol like "[/bold]" raised MarkupError inside the Live loop; "[bold red]X[/]" silently restyled the table. All sites now wrap in Text() or rich.markup.escape(). Regression test renders every affected panel and table with both probes (the reviewer's "[bold" example is literal text for Rich; an unmatched closing tag is what raises). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ancellable LLM transport, no refund on timeout M8: `_compute_sharpe` was mean/std of DOLLAR closed PnL with no annualisation, no time basis and no small-sample handling, so ten similar $50 scalps produced a ratio that clamped to the +1.0 maximum and earned the full GAMMA weight; `_compute_pnl_score` (log10/10) compressed $1k..$1M into 0.30..0.60 so BETA's 0.40 nominal weight had ~0.12 of real range; and ALPHA/BETA/GAMMA had no stated rationale. - analyze_wallet collects per-trade RETURNS (closedPnl / fill notional); `_compute_risk_adjusted` is mean/std of those, multiplied by n/(n+20) so a handful of trades cannot max the component, and treats float-noise dispersion as zero. The persisted field keeps its `sharpe_ratio` name (DB column) and is documented as not a Sharpe. - `_compute_pnl_score` saturates at $1M (log10/6): $1k->0.5, $1M->1.0. - Weights documented as an ordinal preference (PnL > win rate > risk tie-breaker), composite bounded in [-0.65, 1]. Monotonicity, bounds and shrinkage are tested. H3: evaluate() ran urllib in a single-worker ThreadPoolExecutor under wait_for — a timeout abandoned the await but not the thread, so one trickling response wedged every later evaluation forever — and the timeout branch REFUNDED the hourly budget slot, letting a slow-but-billing provider run at ~2x the nominal cap. The already-written aiohttp _async_evaluate was dead code. - _async_evaluate (cancellable) is now the only path under wait_for; the executor, _sync_evaluate and _sync_call are removed. - Refund only on aiohttp.ClientConnectorError (never reached the provider). Timeouts, HTTP errors and parse failures keep their slot. - In-flight guard: a tick arriving mid-evaluation is skipped, not queued. - `reason` is bounded to MAX_REASON_CHARS. - TestLLMRound2.test_transport_failure_refunds_budget_slot rewritten for the async transport. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sitions are shown
scan() fetched EVERY tracked address at 10 req/s, so cycle time grew
linearly with the address store — 50,000 addresses (the retention cap)
was an ~83-minute cycle behind a 15s scan_interval — and nothing
reported it: _position_scan_loop set position_scanner="connected" after
each success and the staleness watchdog never looked at the scanner, so
/v1/health, the whale tracker and the danger zone served hour-old
liquidation distances as live.
- Each cycle scans at most SCAN_ADDRESS_BUDGET (150 ≈ 15s) addresses,
round-robin across cycles, and keeps a per-address position cache; a
failed request keeps the previous entry instead of reading as "no
positions". self.positions is rebuilt from the cache every cycle with
current_price/distance_pct recomputed from fresh mids.
- TrackedPosition.scanned_at records when a position was actually
fetched; PositionScanner gains last_scan_at, last_full_pass_at,
scan_age_seconds(), oldest_position_age_seconds(), is_stale(),
as_of() and freshness().
- Hub watchdog flips position_scanner to 'stale' (POSITION_STALE_AFTER
= 600s); health monitor emits a position_scanner freshness check
(warn before the first cycle, fail when stale); /v1/health carries
position_scan {scan_age_seconds, oldest_position_age_seconds, ...};
/v1/whales and /v1/positions/danger-zone carry as_of and
scan_age_seconds; the whales panel subtitle shows scan age (red when
stale). Demo positions are stamped so demo mode is not "stale".
- address_store.MAX_TRACKED_ADDRESSES is 3,000, derived from the
budget: 20 cycles ≈ 10 min to revisit every address (tested).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r thread _save_trade / _save_liquidation (hub callbacks invoked from inside the WebSocket read loops) ran a blocking INSERT under a threading.Lock on the asyncio event loop — at full Binance+HL trade rates, thousands of synchronous disk writes per second on the thread running every feed, the API server and the heartbeat. get_db_stats (two COUNT(*) scans) and prune() (full-table DELETEs) ran on the same loop from the status tick. - DataStore owns a daemon writer thread fed by a bounded deque. Every INSERT path (liquidations, sampled trades, smart-money signals, HLP trades/snapshots, funding, LSR, options) enqueues and returns; the writer applies batches under the connection lock with the existing 50-event / COMMIT_INTERVAL commit cadence and a periodic wake so a trickle is never left uncommitted. - Queue capped at WRITE_QUEUE_MAX (50k); overflow is dropped and counted (dropped_writes / write_queue_pending in get_db_stats, rate-limited WARNING). - flush()/close()/prune()/every get_* drain the queue first (sequence numbers, bounded wait with a logged backlog), so a read right after a write still sees it and shutdown persists everything. - Trade sampling counter moves under its own tiny lock; no I/O on the read path. - Hub calls get_db_stats / store.prune / address_store.prune via asyncio.to_thread. (address_store's per-write COUNT/expiry already moved to prune() and the scanner already persists only new addresses in the M1/M10 commit.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ocessing from api_server.py Mechanical extraction, no behaviour change: - src/data_layer/hub_demo.py holds the nine demo-mode generators (466 lines moved verbatim, `self.` -> `hub.`). HyperDataHub keeps one-line `_demo_*` delegators so task names, call sites and tests are unchanged. hub.py: 1,287 -> 903 lines. - src/data_layer/liquidation_processing.py holds LiquidationProcessor: per-exchange dedup, cascade detection/bypass, symbol cleanup, leverage estimation and the 60s stats log — the data-layer logic that lived in the REST server. HyperDataAPI constructs one in __init__ (replacing the lazy `__init_dedup` hasattr pattern), delegates _is_duplicate_liq/_check_cascade/_clean_symbol/_log_liq_stats, and exposes the historical attribute names (_liq_seen, _cascade_bypass, _cascade_bypass_started, _CASCADE_BYPASS_DURATION, ...) as views onto the processor so every existing caller and TestLiquidationDedup pass unmodified. Equivalence evidence: the full pre-existing suite passes unchanged; the new TestM13Extraction behavioural tests (demo generators populate the same component state; the demo hub starts, produces data and stops) pass against the pre-extraction commit and after it, while only the structural assertions fail before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…thread and PARTIAL badge Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lean WS close Both found by the live end-to-end run of this branch, and both confirmed pre-existing on 2f7338a (107 HL reconnects and one "database is locked" in a 75s base run) — the C3 venue counters and the M10 WARNING-level logging made them visible. H6 follow-up: the writer held its SQLite transaction open between commits (every 50 events / 5s), so under trade load the WAL write lock was held for seconds at a time and address_store's second connection to the same file starved on the millisecond gaps: "database is locked" despite a 10s busy timeout, 57 discovered addresses not persisted. Every applied batch is now its own committed transaction (WAL + synchronous=NORMAL: not an fsync). _maybe_commit is gone. L5: OrderFlowEngine._run_forever reset the backoff and looped with NO sleep after a clean server-side close, so a venue that closes right after the 50-symbol subscribe burst was reconnected ~1.5x/second (138 connects in 92s), tripping the venue's message rate limit and sustaining the storm. A clean close now backs off (1s doubling to 60s) unless the connection lived past STALE_AFTER_SECONDS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…of <=8 Root cause of the HL reconnect churn surfaced by the C3 venue counters (107 connects in a 75s base run; "disconnected 35s ago, connects=3" in the coordinator's verification). Measured live against wss://api.hyperliquid.xyz/ws: a socket carrying 8 `trades` subscriptions stays up indefinitely; the 10th subscription kills it with close code 1006 within ~0.2s, whether the subscriptions are sent in a burst or 1s apart, and concurrent sockets from one IP with 8 each all stay up. The engine subscribed all 50 DEFAULT_SYMBOLS on one socket, so every connection died 0.6s after connecting — since forever. The old loop slept 0s after a clean close and re-harvested ~0.6s of trades per connect ~1.5x/second, which read as "connected"; the L5 backoff then turned that into long gaps. - HL symbols are partitioned into shards of HL_SUBSCRIPTIONS_PER_SOCKET (8), one reconnect loop + socket per shard (7 for the defaults). - VenueState.connects counts every socket; the venue flips connected on the first shard up and disconnected only when the last one is gone. - Short-lived clean closes back off to at most 15s (errors still 60s). - OrderFlowEngine.force_reconnect() / hl_sockets_open replace the hub watchdog's reach into a single private `_ws`. - docs/DATA_INTEGRITY.md documents the cap and the sharding. - .gitignore: .venv-verify/ (coordinator's verification venv). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e; let stop() finish closing sessions Corrects the previous commit's diagnosis. Per-symbol live probe: PEPE, BONK and FLOKI in DEFAULT_SYMBOLS are not in Hyperliquid's `meta` universe (HL lists them as kPEPE/kBONK/kFLOKI), and a `trades` subscription for an unlisted coin closes the socket with 1006 and no error message. Every other symbol keeps a socket up indefinitely. That — not a ~10-subscription cap — is why the 50-symbol socket died 0.6s after every connect (the 10th symbol in the earlier sweep was PEPE), and why shards 1 (PEPE) and 4 (BONK, FLOKI) kept dying at 0.6s after sharding while the other five never did (18 short-lived closes / 94s). - Subscriptions are filtered against the universe (fetched once per hour through the shard's own session, serialised across shards). An unlisted symbol is skipped with ONE WARNING naming it and the alias HL uses. If the fetch fails, subscriptions go out unfiltered and the last good universe is kept — a REST hiccup must not darken the venue. - Sharding stays, re-justified honestly: a coin delisted between universe refreshes takes down one shard, not the venue. - stop(): closing a shard's socket makes its loop close its ClientSession in `finally`; stop() cancelled the loops while they were inside that close, leaving one "Unclosed client session" per healthy shard at exit (5, seen live). stop() now waits up to STOP_GRACE_SECONDS for the loops to unwind before cancelling, and the close is shielded. - Constant comments, test docstrings and docs/DATA_INTEGRITY.md no longer state the wrong cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-sync tracked set after prune POSITION_STALE_AFTER_SECONDS was a hardcoded 600s while a full round-robin pass over the 3,000-address cap took >=580s at zero latency (150/cycle, 14s of batch sleeps + 15s scan_interval), so every real install read the scanner as 'stale' and /v1/health as 'degraded' forever. The threshold is now derived: worst-case healthy cycle (per-batch latency allowance, sleeps, price/meta refresh, hub interval) x cycles over the in-memory bound, x1.5 margin (~25 min). The hub's default scan_interval comes from the same constant and a longer one warns at construction. The in-memory set was also never bounded: PositionScanner loaded it once and only ever .update()d it; address_store.prune() trimmed the table but never the set, so full-pass time was unbounded. PositionScanner. resync_addresses() re-reads the store off the loop (keeping anything discovered meanwhile) and the hub calls it right after each hourly prune; the derivation bounds the set at the cap plus what discovery can add between prunes. Fixes the wrong 'bounds the tracked set to ~3,000 addresses' comment, the address_store cap comment and the docs' 10-minute claim; staleness tests use the constant instead of a hardcoded 700s. TestB1StalenessBudget pins the arithmetic (fails on the old numbers) and the prune->re-sync path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…atus; fix the shard plan at start() S1: last_hl_message_at was stamped by ANY shard's trades and the venue only went 'disconnected' when no socket was left, so with one shard alive Hyperliquid read `ok` however many were dark; hl_sockets_open reached nothing but a log line. Each shard now carries a ShardState (connect time, down-since, consecutive short-lived closes, idle). A shard is dark when its socket has been gone past CONNECT_GRACE_SECONDS or when it is flapping — its last HL_SHARD_FLAP_CLOSES sockets each died within STALE_AFTER_SECONDS of connecting, the confirmed-live failure mode where a rejected subscription closes the socket ~0.6s after every connect and the backoff keeps each gap under the grace. Any dark shard turns the venue 'partial' (trades flowing, symbols missing); venue_freshness() — hence /v1/health → orderflow_venues.hyperliquid — carries sockets_open, sockets_expected, shards_dark, shards_idle and dark_symbols. The hub flips orderflow_engine to 'partial' (health 'warn'), the health monitor's per-venue check warns, the CVD renderers keep the number and flag it, and contributing venues include 'partial'. S7: the shard->symbols plan is snapshotted at start(); _connect_and_listen reads the snapshot instead of re-partitioning self.symbols on reconnect, so add_symbol() (latent: its one call site cannot fire) can no longer hand existing shards new symbols with no loop for the overflow; it warns when called while running. hub.py's 'dynamically expanded after market data loads' comment was false and is gone. Nit: the idle branch (no listed symbols in a shard) slept HL_UNIVERSE_TTL without checking _running, so stop() burned the full STOP_GRACE_SECONDS; it now waits on a stop event. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… nothing wrote wallets.confidence was added by the v3 migration and written by DataStore.save_wallet, but save_wallet/load_wallets had zero production callers (only tests) and attach() never registered a wallet callback: the column could only ever be 0.0. The `wallets` table itself never had a writer in src/ — the same class of dead table M1 already removed. Chosen fix: remove, not wire. SmartMoneyEngine keeps profiles in memory and recomputes them from fills every session; loading persisted scores (including sharpe_ratio, whose formula changed on this branch) into a ranking that is rebuilt anyway would have been the dishonest option. v4 drops `wallets` when empty and, like M1, keeps and reports a populated one; v3 no longer touches it; save_wallet/load_wallets are gone. The WalletProfile.sharpe_ratio comment no longer claims to be a persisted DB column, which also closes the "stale rows carry incomparable values" nit: there are no rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…est failed pre-fix" docstring The module docstring claimed every test failed against the pre-fix tree. Three did not — test_non_empty_legacy_table_is_preserved, test_with_db_the_same_trade_executes_and_is_logged and test_composite_is_bounded — they are legitimate control cases pinning the behaviour a fix must not break. They now say "Control:"/"Guard:" up front, and the docstring explains the two kinds; nothing was deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t drop count is deterministic test_queue_is_bounded_and_drops_are_counted waited for the writer to take the first batch with a 5ms sleep loop against a 2s deadline and then asserted dropped_writes == 4 exactly; under CI load the loop could time out with the first item still queued, making the count 5. The writer is now parked by a gate inside _apply, and "the first item is in flight" is an Event the test waits on. 20/20 runs green locally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…()/close() flush() called _drain() and threw away its bool; close() then closed the connection regardless, so a drain that timed out abandoned everything in the write queue with a single WARNING on a logger run_api.py gives no stdout handler. The drain timeout is now an ERROR naming the backlog; flush() returns the drain result; close() returns False and logs at ERROR — with the count — when the drain timed out or the writer did not stop before the join timeout; hub.stop() repeats the failure in its own shutdown log. New pending_writes() exposes the backlog. Also fixes _drain's timeout being a default argument bound at definition time, which made DRAIN_TIMEOUT_SECONDS impossible to tune at runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ver tested H1 widened the ranges to rich>=13.7,<15 and pandas>=2.2,<4, but CI runs the lock (rich 14.3.3 / pandas 3.0.1) on both Pythons and the pyproject leg resolves to the newest in range on 3.13 only; nothing exercises the older floors. The lower bounds are now the pins, requirements.txt mirrors them, and tests/test_dependency_lock.py asserts the floor IS the pin for both packages so the ranges cannot be widened downward silently again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get_db_stats() returned write_queue_pending and dropped_writes but no /v1/health field carried them. HubStatus now holds both (refreshed with the other DB stats every 30s) and /v1/health exposes them under a `persistence` block alongside db_size_mb / events_persisted. Informational only: a drop is a persistence loss, not a live-data fault, and the counter never resets, so it does not gate the top-level status. Docs updated. (The other nits landed with their finding: the idle-shard sleep with S1, the last_full_pass_at docstring with B1, sharpe_ratio with S2.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f12a4d541
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+219
to
+223
| for addr, result in zip(batch, results): | ||
| if isinstance(result, list): | ||
| for p in result: | ||
| p.scanned_at = fetched_at | ||
| self._position_cache[addr] = result |
There was a problem hiding this comment.
When an address successfully returns an empty position list, the cache stores [] without any freshness timestamp. If later requests for that address fail, the empty entry is retained, self.positions contains nothing for it, and last_scan_at still advances after every cycle; consequently oldest_position_age_seconds() returns zero and is_stale() remains false indefinitely. During a clearinghouse endpoint outage, wallets that were previously flat can therefore open positions that are never displayed while the scanner continues reporting healthy. Track the last successful fetch per address, including empty results, or avoid marking an all-failed cycle as fresh.
Useful? React with 👍 / 👎.
Comment on lines
+192
to
+195
| finally: | ||
| with self._q_cond: | ||
| self._applied_seq += len(batch) | ||
| self._q_cond.notify_all() |
There was a problem hiding this comment.
_apply fails
If _apply() raises, such as from a full disk or SQLite commit/I/O failure, the batch has already been removed from _write_q but this finally block unconditionally marks every item as applied. The batch is neither retried nor added to dropped_writes, so _drain() can return true, the health endpoint can report zero pending/dropped writes, and shutdown can appear successful even though the batch was lost. Advance _applied_seq only after a successful durable commit, or requeue/count the failed batch.
Useful? React with 👍 / 👎.
pip-audit in CI flagged three advisories against aiohttp 3.14.1 that were published after the lock was written. 3.14.3 fixes all three. It satisfies the existing pyproject range (>=3.9.0,<4.0) and needs no transitive changes; pip check, pip-audit, ruff and the full suite pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fixture's setup check assumed PRAGMA quick_check returns a non-ok row for a smashed table page. That holds on macOS, but the SQLite on GitHub's Ubuntu runners raises DatabaseError for the same file, so the test failed during setup on every CI job. DataStore already handles both: _check_integrity runs inside the try/except sqlite3.DatabaseError that quarantines the file. Only the test was too strict. It now accepts either a non-ok verdict or a raised error, and the quarantine assertion is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all 23 findings from an adversarial code review (3 Critical, 7 High, 13 Medium), plus 8 more from a pre-merge review of that work, plus a Hyperliquid socket bug found along the way. Each commit names the finding ID it fixes, so you can review one at a time.
The review's main point: the previous hardening pass (#2) added safe APIs and tests but never changed the code that calls them.
venue_freshness(),get_cumulative_cvd()andWalletProfile.confidencewere built, tested and documented, and nothing used them. The dashboards, API and health monitor all kept reading the old unsafe path. For every fix here, the callers were changed too, and the pre-merge review checked each one.Critical
rank_all()could never assign theaveragetier. Tiers are now proportional, and wallet confidence is shown next to every tier.Access-Control-Allow-Origin: *with no auth, so any page you visited could fetch whale positions and wallet addresses. It never sends a wildcard now, a new Host-header check blocks DNS-rebinding requests, and REST and WebSocket use one origin allowlist.ok,connecting,silent,frozen,stale,disconnectedorpartial, each with a reason. It shows in/v1/health, both CVD dashboards, the health monitor and the status badge.Found along the way: Hyperliquid order flow never worked
DEFAULT_SYMBOLSincludesPEPE,BONKandFLOKI. Hyperliquid lists them askPEPE,kBONKandkFLOKI(checked against the livemetaAPI). One trades subscription for an unlisted coin closes the socket with code 1006 and no error message. So the single 50-symbol socket closed 0.6s after every connect: 107 reconnects in 75s onmain, while it still reported "connected". Subscriptions are now checked against the live symbol list and split across sockets of at most 8 symbols.Note:
802e441's commit message has the wrong cause (it guesses a subscription cap).9f2b3b1corrects it in the code, comments and its own commit message.Binance
From some regions,
wss://fstream.binance.comaccepts the connection and then sends nothing, while Binance REST still answers. That's a regional block, not a bug in this code. The bug was that we logged "Connected, streaming 15 symbols" and said nothing after that. It now shows assilentwith the likely cause, and the README says so.Pre-merge review fixes
/v1/healthwould have stayeddegradedon any install left running overnight. A full position-scan pass took at least 580s against a 600s staleness limit, even with no network delay. The in-memory address set was also never trimmed, so it grew forever. The limit is now worked out from the slowest possible pass (998s pass, 1497s limit), and the set is trimmed after each prune./v1/healthnow reports per-socket status, and the venue showspartialwhen some sockets are down.walletstable, made a failed shutdown flush log an error, fixed a timing-dependent test, raised dependency minimums to the versions actually tested, and guardedadd_symbol()against changing the socket split mid-session.Other notable changes
requirements.lockpinned versions thatpyproject.tomldidn't allow, and CI's--no-depsinstall hid that. Fixed, and CI now runspip check, installs frompyproject.tomlin one job, and tests that the two files agree.hub.py(1287 → 903 lines) and liquidation processing out ofapi_server.py, with no behaviour change. Existing attribute access still works, and a test checks it.Test plan
pytest tests/ -q: 296 passed, 2 skipped, up from 166 onmainruff check src/ tests/: cleanpip install -r requirements.lock && pip install -e ., thenpip check: cleanrun_api.pyagainst live feeds for 115s: Hyperliquidok, 7/7 sockets, 4117 trades, 141 liquidations; Binancesilentwith its reason; position scan not stale; 0 pending and 0 dropped writesKnown and left for later
/v1/healthstays atwarnwherever Binance is blocked. That's accurate, but don't point a monitor at it expectingok.DEFAULT_SYMBOLSstill listsPEPE/BONK/FLOKI. They're now skipped with a warning, so the socket stays up. Renaming them to theknames would add three symbols; that's a separate change.mainas well. They're labelledControl:/Guard:because they check that nothing broke, not that a bug was fixed.🤖 Generated with Claude Code