Add prospective maker observatory - #411
Draft
DarriEy wants to merge 3 commits into
Draft
Conversation
Owner
Author
|
Heads-up before this leaves draft (Claude, 2026-08-06): schema version collision. main is at SCHEMA_VERSION 48. This PR implements Separately, from a first pass over the diff, one item to fix while you are in there: the nine Full review to follow once it is out of draft. |
The instrument was measurable but its own contract was not. Nine of twenty mutations to the production behaviour this PR claims to guard survived its test suite, including the headline one: adding "synthetic" to CREDIBLE_FILL_EVIDENCE — making paper fills count toward promotion, the exact thing the evidence contract forbids — left all six tests green. The old assertion could not catch it, because its fixture's zero came from is_holdout=0 (t0 predates the holdout start), not from the evidence filter. Every claim now has a test that fails when the claim stops holding; all twenty mutations are killed. Migration renumbered v49 -> v50. #417 took 49 on main, so both migrations were named _migrate_v48_to_v49 in one class: Python keeps the last definition and the arb post-check would have silently never run. Verified from a real v48 database that the chain now lands on 50 with BOTH the postcheck columns and the observatory tables. Observatory config moved out of market_maker: into its own top-level maker_observatory: section. ExecutionGateway._capture_decision hashes settings.market_maker into market_maker's strategy_version and _prospective_stats joins on the LATEST version, so nine knobs parked in that section re-versioned the strategy this PR only claims to watch. It cost nothing today (market_maker records zero decision_snapshots, so no clock exists to reset) but it made "no feature below changes quotes" false as written. The hash is back to its pre-PR value, d4f1270c, unchanged from main. Retention keyed to wall-clock instead of a cycle counter. The counter started at 0 every process start and fired at cycle 2880, so any stack restarting more often than daily would never prune — and _mark_due's per-refresh scan grows with the table it never trimmed. record_fill renamed to record_observed_fill. That name is in exposure_registry.SENSITIVE_METHODS, which is scanned by attribute name, so the old name forced a pure measurement write to be registered as a prediction_quoting exposure-mutation callsite. It books nothing. Registration cached per process: it was re-issuing four INSERT OR IGNOREs on every refresh to re-learn a constant, on a path where every statement takes the process-wide Database serializer. Ten round-trips per observation, now six. What is NOT fixed, and needs an operator decision (documented in docs/maker-observatory.md): observe() runs inline in the quoting loop, and its cost grows with retained fills — 7 ms at day 0, 236 ms at day 7, 917 ms at day 45, i.e. 4.6 s per five-market cycle, 93% of it _mark_due rescanning every retained fill. The growth is driven by synthetic paper fills that the evidence contract can never promote (~1.3M rows, ~590 MB on a live DB already at 1382 MB). Also flagged, not touched: signed_flow reads a tracker fed only for the first 20 discovered markets, so it can be identically 0.0 for reasons unrelated to flow, and nothing distinguishes that from balanced flow. Assisted-by: Claude (Anthropic)
DarriEy
force-pushed
the
feat/maker-observatory
branch
from
August 6, 2026 13:09
fda0a75 to
d995c48
Compare
Owner
Author
|
Review + fixes applied (d995c48), rebased onto current main, still draft (Claude, 2026-08-06). 2291 passed, coverage 67.16% >= 65.0, ruff clean. The metrics are correct and the restart-safety claim holds in code — but two findings change what this PR is, and one of them is the headline. CRITICAL, fixed: the migration collision was real. HIGH: "no feature below changes quotes" is false as a COST claim.
93% of that is HIGH, fixed: MEDIUM, fixed: the nine MEDIUM, fixed: Metrics verified correct — microprice is the standard size-weighted form, depth-imbalance sign agrees with it, decay units are right (exactly 0.5 at one half-life), and the markout signs are correct on both legs including the ask leg's Mutation testing — 9 of 20 mutations survived the PR's own tests. The worst is M4: adding For operator judgment — deliberately not touched:
|
Two ways this instrument could have corrupted what it measures. Markout resolution ran inline in observe(), inside _quote_market's op_timeout window, holding the process-wide Database serializer that ~30 pillar tasks share. It was 93% of observe()'s cost and it grew with retained history: a 5-market quoting cycle measured 21-35 ms at day 0 but 4.5-6.1 s at 45-day retention, because the predicate `unixepoch(?) - unixepoch(filled_at) >= ?` wrapped the column in a function and so re-walked every retained fill for the market on every refresh. Slow quotes get picked off. An observatory that adds seconds to the quote path would therefore CAUSE the adverse selection it exists to detect, which is worse than not measuring at all. Resolution now runs on its own bot task on its own timer. What is left on the quoting path is flat in retained history — 13-24 ms per 5-market cycle at every volume from 0 to 259k fills per market. The offline pass is bounded by work outstanding rather than by history: a marks_pending column drives a partial index holding only fills that still owe a mark, and the range predicate compares filled_at against a precomputed bound, so EXPLAIN QUERY PLAN seeks that index instead of walking 259k rows. Moving the scan does not move its conclusions. A mark is taken from the first observation at or after filled_at + horizon, which is precisely the book the inline scan used — it ran on the observation that first found the fill due. Horizons, lateness, bounded validity and late-but-recorded is_valid=0 are untouched, and resolving every cycle versus once, 400 seconds later, is asserted to write identical rows. An interrupted pass still leaves due marks unresolved for the next pass and never fabricates one: marks_pending is cleared only in the transaction that writes a fill's last mark, and every mark is INSERT OR IGNORE against its primary key. signed_flow could not distinguish "no feed" from "balanced". OrderFlowTracker is fed only by the websocket price monitor's on_trade for the first 20 discovered markets; the maker picks its five by spread and usually is not in that set, so the column sat at exactly 0.0 for reasons that had nothing to do with flow. That is the 2026-07-29 qwen3 null result again — a metric incapable of moving, looking healthy for a week. The column is now nullable and records NULL when no feed reached the market, 0.0 only when a market the feed did reach genuinely netted to zero. Every reader keeps the distinction: the feature report excludes NULLs from the warmup median and from both holdout buckets and reports covered_n/marks_n per feature, quote coverage reports COUNT(signed_flow), and the CLI prints trade-feed coverage and warns when it is partial, so an absent result cannot be read as a negative one. The v50 migration this PR already owns is extended rather than duplicated; it is unreleased, so no deployed database is affected. Assisted-by: Claude (Anthropic)
Owner
Author
|
Amendments applied (d43c6db), still draft. 2305 passed, coverage 67.32%, ruff clean. Markout resolution is off the quote path, and the numbers hold flat. Same harness as the earlier measurement:
Flat at 13-14 ms at every volume, and Semantics preserved by construction, not by cadence. A mark is taken from the first observation at or after Three corrections to my brief, all of which improve on it:
A hazard it introduced and then caught itself: the batch ceiling drains oldest-first, so fills stranded by a market leaving the maker's five would have held the head of the queue until retention pruned them 45 days later, starving every newer mark. The scan now skips fills whose market has not been observed as far as their earliest horizon; they stay pending and still take their late/invalid mark if the market returns.
27 mutations, 27 killed, including inline resolution restored, marks fabricated for unmarkable fills, Remaining for operator judgment (unchanged, deliberately untouched): market_maker still absent from |
DarriEy
added a commit
that referenced
this pull request
Aug 6, 2026
Until #420 the portfolio monitor's exit loop raised on every tick, so no live exit ever executed and this policy was tuned against a dead path. That path now runs, so every threshold here moves money on the next tick. Six amendments, in descending order of what they cost. 1. THE ENTRY-TIME FALLBACK, restored. The lifecycle target reads a position's age, and the new code sourced it only from a token-scoped reverse-inventory walk over `fills`. On a miss it set `entry_dt = now`, which pins `fraction_remaining` at exactly 1.0 on EVERY tick — not once, permanently — so the target freezes at the widest 75% band and can never tighten to 50% or reach the 25% near-expiry band at all. That miss is not rare. Measured against the live DB, 37% of open positions have no token-scoped fill ancestry: they predate the fills ledger, or they are live rows a venue mirror created without ever writing a fill (bot.py and kalshi.py both upsert cost_basis/portfolio directly). 40% of those DO have a `trades` row that the old lifecycle path used, and 65% of those in turn have a known end_date — so the near-expiry band was reachable for them before and is not now. Resolution order is now fills, then trades, then "just entered", and the last branch documents what it costs. The trades leg is book-scoped but NOT token-scoped, because `trades` has no token column (broker/ledger.py says so explicitly and reaches token scope only by joining back through fills.order_id — which is exactly what these positions lack). Coarser than the fills path, strictly more than nothing. The fills comparison also gains a size tolerance. `qty >= pos.size` is an exact float compare against a summed quantity: 0.7 + 0.1 is 0.7999999999999999, so a position whose fills plainly cover it reports no ancestry at all. `cost_basis` was considered as the source and rejected. #412 did make it the authoritative per-(market, is_paper, token) holdings table, and it agrees with portfolio.size for 97% of rows — but its only timestamp is `updated_at`, stamped on every write by PnLTracker, both venue mirrors and both settlement paths. It records when inventory last MOVED, not when it was entered, so it cannot answer this question at all. 2. THE ENTRY FEE, dropped from the gate. `binary_exit_economics` charged entry + exit fees. broker/pnl.py realizes `(fill.price - old_avg_cost) * size - fill.fee` — the exit leg alone — and builds the pnl_ledger event only on the SELL branch; the BUY branch stores `total_cost = price * size`, so avg_cost is fee-EXCLUSIVE and the entry fee never reaches the ledger anywhere. The gate was demanding a cost the books never record, holding every winner past its target for it. Measured drag removed across open binary positions: 1.47 points at the median, 4.00 at p90, 6.86 at the maximum. The entry leg stays on ExitEconomics as a diagnostic so calibration keeps the number without the decision paying for it. 3. THE HOLD TELEMETRY, off the money path. `_record_exit_decision` was awaited inline before `exits.append(...)`, taking the shared serialized write lock once per evaluated position — and the HOLD branch fires for every non-exiting position on every 60s tick. Observations now accumulate in a list and land in one `executemany` after the loop, by which time every exit is already in the returned list: one lock acquisition per cycle instead of one per position, a >99% reduction on a full book. Rows are pruned to `execution.exit_decision_retention_days` (default 14) by a bounded per-cycle delete on the indexed time column, following candidate_dispositions. HOLD rows are kept rather than dropped: they are the counterfactual, and a calibration that sees only exits cannot say what a different threshold would have done. The existing exception guard stays and is now tested — telemetry must never be able to trap a position. PR #411 moved a scan off the quoting loop for the same reason. 4. THE MIGRATION COLLISION CLASS, closed. Schema v50 is kept here (money path, small migration); #411 is a draft observatory and renumbers to v51. A duplicate `async def _migrate_vN_to_vM` in the class body is silently shadowed — Python keeps the last definition, ruff does not flag it, and one migration's DDL simply never runs while the version counter still advances. That has now happened twice, surviving review and CI both times. A new test AST-parses the class and asserts every step is declared once, dispatched exactly once, resolves to as many distinct function objects as there are steps, and forms an unbroken single-version chain up to SCHEMA_VERSION. 5. AN UNSCOPED READ NO LONGER DEFAULTS A LIVE POSITION TO PAPER. `mode = 1 if mode_flag is None else mode_flag` answered a live position's lifecycle question off the paper ledger. mode_flag is None whenever `settings.is_live` is not a bool — which includes every tick with the kill switch armed, since is_live is False there — and check_exits passes that straight through, leaving the position list holding both books. #420 gave Position an `is_paper` field populated from the row, so the position is now asked directly; only one that cannot answer falls back to an unscoped read, which is imprecise for everyone rather than wrong for live. The telemetry row resolves its book the same way. 6. ZERO NOW DISABLES THE TRAILING STOP. `Field(gt=0)` on trailing_stop_activation_pct and trailing_stop_giveback_fraction crashed startup on 0, stricter than the neighbouring stop_loss_pct / profit_target_pct, which carry no validators. Relaxing to ge=0 alone would have been worse than the crash: unguarded, activation 0 arms the tier against every non-negative peak and giveback 0 reduces the test to `peak > current`, so "disabled" would have sold every position on its first adverse tick. `trailing_stop_triggered` now returns False when either knob is 0, and both readings are tested. Stop-loss and trailing-stop band VALUES are unchanged and verified byte-identical to the originals. bot.py's exit loop and #421's exit-liveness readiness criterion are untouched. TESTS. Six new behaviours, each mutation-verified — the behaviour was broken, the test confirmed failing, then restored. 17 mutations run, 17 killed: a TRAILING_STOP emitted through the real check_exits (4 mutations: activation compare, giveback compare, peak ignored, wrong ExitReason); a profit-target case that clears GROSS but not NET at 0.61 (0.62 clears both once the entry fee is gone, and the suite's 0.63 could never discriminate); the telemetry batch raising while the exit still returns; an entry-time fixture whose NO and live fills are NEWER than the paper YES entry, so a lost token or book scope changes the answer (the old fixture's newest fill alone satisfied qty >= size and could not detect either); an IBKR pair whose round-trip commissions bracket take_profit_pct; and the migration-distinctness test above. Verification: 2304 passed, 5 skipped, 1 deselected; coverage 67.44% against the 65.0 floor; `ruff check .` clean. Assisted-by: Claude (Anthropic)
DarriEy
added a commit
that referenced
this pull request
Aug 6, 2026
Until #420 the portfolio monitor's exit loop raised on every tick, so no live exit ever executed and this policy was tuned against a dead path. That path now runs, so every threshold here moves money on the next tick. Six amendments, in descending order of what they cost. 1. THE ENTRY-TIME FALLBACK, restored. The lifecycle target reads a position's age, and the new code sourced it only from a token-scoped reverse-inventory walk over `fills`. On a miss it set `entry_dt = now`, which pins `fraction_remaining` at exactly 1.0 on EVERY tick — not once, permanently — so the target freezes at the widest 75% band and can never tighten to 50% or reach the 25% near-expiry band at all. That miss is not rare. Measured against the live DB, 37% of open positions have no token-scoped fill ancestry: they predate the fills ledger, or they are live rows a venue mirror created without ever writing a fill (bot.py and kalshi.py both upsert cost_basis/portfolio directly). 40% of those DO have a `trades` row that the old lifecycle path used, and 65% of those in turn have a known end_date — so the near-expiry band was reachable for them before and is not now. Resolution order is now fills, then trades, then "just entered", and the last branch documents what it costs. The trades leg is book-scoped but NOT token-scoped, because `trades` has no token column (broker/ledger.py says so explicitly and reaches token scope only by joining back through fills.order_id — which is exactly what these positions lack). Coarser than the fills path, strictly more than nothing. The fills comparison also gains a size tolerance. `qty >= pos.size` is an exact float compare against a summed quantity: 0.7 + 0.1 is 0.7999999999999999, so a position whose fills plainly cover it reports no ancestry at all. `cost_basis` was considered as the source and rejected. #412 did make it the authoritative per-(market, is_paper, token) holdings table, and it agrees with portfolio.size for 97% of rows — but its only timestamp is `updated_at`, stamped on every write by PnLTracker, both venue mirrors and both settlement paths. It records when inventory last MOVED, not when it was entered, so it cannot answer this question at all. 2. THE ENTRY FEE, dropped from the gate. `binary_exit_economics` charged entry + exit fees. broker/pnl.py realizes `(fill.price - old_avg_cost) * size - fill.fee` — the exit leg alone — and builds the pnl_ledger event only on the SELL branch; the BUY branch stores `total_cost = price * size`, so avg_cost is fee-EXCLUSIVE and the entry fee never reaches the ledger anywhere. The gate was demanding a cost the books never record, holding every winner past its target for it. Measured drag removed across open binary positions: 1.47 points at the median, 4.00 at p90, 6.86 at the maximum. The entry leg stays on ExitEconomics as a diagnostic so calibration keeps the number without the decision paying for it. 3. THE HOLD TELEMETRY, off the money path. `_record_exit_decision` was awaited inline before `exits.append(...)`, taking the shared serialized write lock once per evaluated position — and the HOLD branch fires for every non-exiting position on every 60s tick. Observations now accumulate in a list and land in one `executemany` after the loop, by which time every exit is already in the returned list: one lock acquisition per cycle instead of one per position, a >99% reduction on a full book. Rows are pruned to `execution.exit_decision_retention_days` (default 14) by a bounded per-cycle delete on the indexed time column, following candidate_dispositions. HOLD rows are kept rather than dropped: they are the counterfactual, and a calibration that sees only exits cannot say what a different threshold would have done. The existing exception guard stays and is now tested — telemetry must never be able to trap a position. PR #411 moved a scan off the quoting loop for the same reason. 4. THE MIGRATION COLLISION CLASS, closed. Schema v50 is kept here (money path, small migration); #411 is a draft observatory and renumbers to v51. A duplicate `async def _migrate_vN_to_vM` in the class body is silently shadowed — Python keeps the last definition, ruff does not flag it, and one migration's DDL simply never runs while the version counter still advances. That has now happened twice, surviving review and CI both times. A new test AST-parses the class and asserts every step is declared once, dispatched exactly once, resolves to as many distinct function objects as there are steps, and forms an unbroken single-version chain up to SCHEMA_VERSION. 5. AN UNSCOPED READ NO LONGER DEFAULTS A LIVE POSITION TO PAPER. `mode = 1 if mode_flag is None else mode_flag` answered a live position's lifecycle question off the paper ledger. mode_flag is None when `settings.is_live` is not a bool and the tracker holds no settings of its own, leaving get_positions unscoped over BOTH books. A real `Settings.is_live` ANDs three bools and so always is one — with the kill switch armed it is False, which scopes the read to paper rather than unscoping it — so this is the duck-typed/no-settings path, not a live-trading one. It still must not hardcode a book: the wrong answer was silent and the caller had no way to notice. #420 gave Position an `is_paper` field populated from the row, so the position is now asked directly; only one that cannot answer falls back to an unscoped read, which is imprecise for everyone rather than wrong for live. The telemetry row resolves its book the same way. 6. ZERO NOW DISABLES THE TRAILING STOP. `Field(gt=0)` on trailing_stop_activation_pct and trailing_stop_giveback_fraction crashed startup on 0, stricter than the neighbouring stop_loss_pct / profit_target_pct, which carry no validators. Relaxing to ge=0 alone would have been worse than the crash: unguarded, activation 0 arms the tier against every non-negative peak and giveback 0 reduces the test to `peak > current`, so "disabled" would have sold every position on its first adverse tick. `trailing_stop_triggered` now returns False when either knob is 0, and both readings are tested. Stop-loss and trailing-stop band VALUES are unchanged and verified byte-identical to the originals. bot.py's exit loop and #421's exit-liveness readiness criterion are untouched. TESTS. Six new behaviours, each mutation-verified — the behaviour was broken, the test confirmed failing, then restored. 17 mutations run, 17 killed: a TRAILING_STOP emitted through the real check_exits (4 mutations: activation compare, giveback compare, peak ignored, wrong ExitReason); a profit-target case that clears GROSS but not NET at 0.61 (0.62 clears both once the entry fee is gone, and the suite's 0.63 could never discriminate); the telemetry batch raising while the exit still returns; an entry-time fixture whose NO and live fills are NEWER than the paper YES entry, so a lost token or book scope changes the answer (the old fixture's newest fill alone satisfied qty >= size and could not detect either); an IBKR pair whose round-trip commissions bracket take_profit_pct; and the migration-distinctness test above. Verification: 2304 passed, 5 skipped, 1 deselected; coverage 67.44% against the 65.0 floor; `ruff check .` clean. Assisted-by: Claude (Anthropic)
DarriEy
added a commit
that referenced
this pull request
Aug 6, 2026
* Harden take-profit discipline * Make the take-profit gate safe to point at real money Until #420 the portfolio monitor's exit loop raised on every tick, so no live exit ever executed and this policy was tuned against a dead path. That path now runs, so every threshold here moves money on the next tick. Six amendments, in descending order of what they cost. 1. THE ENTRY-TIME FALLBACK, restored. The lifecycle target reads a position's age, and the new code sourced it only from a token-scoped reverse-inventory walk over `fills`. On a miss it set `entry_dt = now`, which pins `fraction_remaining` at exactly 1.0 on EVERY tick — not once, permanently — so the target freezes at the widest 75% band and can never tighten to 50% or reach the 25% near-expiry band at all. That miss is not rare. Measured against the live DB, 37% of open positions have no token-scoped fill ancestry: they predate the fills ledger, or they are live rows a venue mirror created without ever writing a fill (bot.py and kalshi.py both upsert cost_basis/portfolio directly). 40% of those DO have a `trades` row that the old lifecycle path used, and 65% of those in turn have a known end_date — so the near-expiry band was reachable for them before and is not now. Resolution order is now fills, then trades, then "just entered", and the last branch documents what it costs. The trades leg is book-scoped but NOT token-scoped, because `trades` has no token column (broker/ledger.py says so explicitly and reaches token scope only by joining back through fills.order_id — which is exactly what these positions lack). Coarser than the fills path, strictly more than nothing. The fills comparison also gains a size tolerance. `qty >= pos.size` is an exact float compare against a summed quantity: 0.7 + 0.1 is 0.7999999999999999, so a position whose fills plainly cover it reports no ancestry at all. `cost_basis` was considered as the source and rejected. #412 did make it the authoritative per-(market, is_paper, token) holdings table, and it agrees with portfolio.size for 97% of rows — but its only timestamp is `updated_at`, stamped on every write by PnLTracker, both venue mirrors and both settlement paths. It records when inventory last MOVED, not when it was entered, so it cannot answer this question at all. 2. THE ENTRY FEE, dropped from the gate. `binary_exit_economics` charged entry + exit fees. broker/pnl.py realizes `(fill.price - old_avg_cost) * size - fill.fee` — the exit leg alone — and builds the pnl_ledger event only on the SELL branch; the BUY branch stores `total_cost = price * size`, so avg_cost is fee-EXCLUSIVE and the entry fee never reaches the ledger anywhere. The gate was demanding a cost the books never record, holding every winner past its target for it. Measured drag removed across open binary positions: 1.47 points at the median, 4.00 at p90, 6.86 at the maximum. The entry leg stays on ExitEconomics as a diagnostic so calibration keeps the number without the decision paying for it. 3. THE HOLD TELEMETRY, off the money path. `_record_exit_decision` was awaited inline before `exits.append(...)`, taking the shared serialized write lock once per evaluated position — and the HOLD branch fires for every non-exiting position on every 60s tick. Observations now accumulate in a list and land in one `executemany` after the loop, by which time every exit is already in the returned list: one lock acquisition per cycle instead of one per position, a >99% reduction on a full book. Rows are pruned to `execution.exit_decision_retention_days` (default 14) by a bounded per-cycle delete on the indexed time column, following candidate_dispositions. HOLD rows are kept rather than dropped: they are the counterfactual, and a calibration that sees only exits cannot say what a different threshold would have done. The existing exception guard stays and is now tested — telemetry must never be able to trap a position. PR #411 moved a scan off the quoting loop for the same reason. 4. THE MIGRATION COLLISION CLASS, closed. Schema v50 is kept here (money path, small migration); #411 is a draft observatory and renumbers to v51. A duplicate `async def _migrate_vN_to_vM` in the class body is silently shadowed — Python keeps the last definition, ruff does not flag it, and one migration's DDL simply never runs while the version counter still advances. That has now happened twice, surviving review and CI both times. A new test AST-parses the class and asserts every step is declared once, dispatched exactly once, resolves to as many distinct function objects as there are steps, and forms an unbroken single-version chain up to SCHEMA_VERSION. 5. AN UNSCOPED READ NO LONGER DEFAULTS A LIVE POSITION TO PAPER. `mode = 1 if mode_flag is None else mode_flag` answered a live position's lifecycle question off the paper ledger. mode_flag is None when `settings.is_live` is not a bool and the tracker holds no settings of its own, leaving get_positions unscoped over BOTH books. A real `Settings.is_live` ANDs three bools and so always is one — with the kill switch armed it is False, which scopes the read to paper rather than unscoping it — so this is the duck-typed/no-settings path, not a live-trading one. It still must not hardcode a book: the wrong answer was silent and the caller had no way to notice. #420 gave Position an `is_paper` field populated from the row, so the position is now asked directly; only one that cannot answer falls back to an unscoped read, which is imprecise for everyone rather than wrong for live. The telemetry row resolves its book the same way. 6. ZERO NOW DISABLES THE TRAILING STOP. `Field(gt=0)` on trailing_stop_activation_pct and trailing_stop_giveback_fraction crashed startup on 0, stricter than the neighbouring stop_loss_pct / profit_target_pct, which carry no validators. Relaxing to ge=0 alone would have been worse than the crash: unguarded, activation 0 arms the tier against every non-negative peak and giveback 0 reduces the test to `peak > current`, so "disabled" would have sold every position on its first adverse tick. `trailing_stop_triggered` now returns False when either knob is 0, and both readings are tested. Stop-loss and trailing-stop band VALUES are unchanged and verified byte-identical to the originals. bot.py's exit loop and #421's exit-liveness readiness criterion are untouched. TESTS. Six new behaviours, each mutation-verified — the behaviour was broken, the test confirmed failing, then restored. 17 mutations run, 17 killed: a TRAILING_STOP emitted through the real check_exits (4 mutations: activation compare, giveback compare, peak ignored, wrong ExitReason); a profit-target case that clears GROSS but not NET at 0.61 (0.62 clears both once the entry fee is gone, and the suite's 0.63 could never discriminate); the telemetry batch raising while the exit still returns; an entry-time fixture whose NO and live fills are NEWER than the paper YES entry, so a lost token or book scope changes the answer (the old fixture's newest fill alone satisfied qty >= size and could not detect either); an IBKR pair whose round-trip commissions bracket take_profit_pct; and the migration-distinctness test above. Verification: 2304 passed, 5 skipped, 1 deselected; coverage 67.44% against the 65.0 floor; `ruff check .` clean. Assisted-by: Claude (Anthropic) * Index exit_decisions by time so the retention prune can seek The per-cycle prune is `DELETE FROM exit_decisions WHERE observed_at < datetime('now', ?)`, and the only index shipped for the table was `(market_id, token, is_paper, observed_at)`. `observed_at` is its FOURTH column and the predicate constrains none of the three before it, so SQLite has no seekable prefix and cannot use that index at all. EXPLAIN QUERY PLAN on the shipped schema: SCAN exit_decisions So every exit cycle full-scanned the table. That is the worst place to put a scan: the delete runs inside the `portfolio.exit_decisions` transaction span, holding the process-wide write lock, on the path that closes positions — and the table it scans is one the same path appends to on every tick, including a HOLD row per non-exiting position, so the scan gets more expensive the longer the bot runs. Add a dedicated single-column index on `observed_at`. The plan becomes: SEARCH exit_decisions USING INDEX idx_exit_decisions_observed_at (observed_at<?) The composite index stays; it serves the calibration script's per-position reads, which do constrain the leading columns. The index goes in BOTH places the table is defined — `db/models.py`'s TABLES script and the v49->v50 migration in `db/database.py` — so a fresh database and a migrated one agree. `candidate_dispositions`, the table this prune was modelled on, already carries exactly this index; only the index was missed when the pattern was copied. The regression test plans the SHIPPED statement rather than a copy of it, which is why the SQL is now a named constant beside the insert. It checks both schema paths, and it exercises the migration by calling the step directly: `_init_schema` runs TABLES *before* dispatching migrations, so a reconnect-based test would create the indexes from TABLES and then run the migration against a schema that already satisfies it — an index missing from the migration would pass. Verified by mutation: removing the index from either file fails the corresponding half. Assisted-by: Claude (Anthropic)
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
Evidence contract
Verification