Skip to content

OBSERVATIONS

Nalin Angrish edited this page Aug 23, 2026 · 1 revision

Observations — Virtual Trading Mechanism Audit

Date: 2026-08-23 Scope: End-to-end paper-trading pipeline: Quantitative Scout, Risk Manager, Trader Gateway (incl. exit monitor, square-off interaction), Sentiment Analyst, Market Recorder, Replay engine, Report tool, shared pkg/nats & pkg/database. The Groww live-broker client was not deeply audited (virtual-trading focus). Method: Static code audit of every component plus cross-component flow tracing; selected runtime evidence from test logs. This document records problems only — remedies are intentionally out of scope.

Status (2026-08-23): 20 of 22 findings resolved on fix/audit-findings branch. Two deferred: OBS-01 (volume gate — requires design decision), OBS-18 (archive gaps — requires backfill infrastructure). OBS-16 (sentiment.update no consumer) is intentional per HLD (broadcast for future consumers).


Summary

# Severity Component Finding Status
OBS-01 Critical Quantitative Scout Entry pipeline is inert — volume gate can never pass Deferred
OBS-02 Critical PaperTrader × exit paths Closing executions open phantom reverse positions Fixed
OBS-03 High Risk Manager square-off Square-off books zero P&L on every EOD liquidation Fixed
OBS-04 High pkg/nats consumers Downtime silently loses messages (ephemeral consumers) Fixed
OBS-05 High Trader Gateway Non-idempotent execution handling → duplicate positions on redelivery Fixed
OBS-06 Medium Risk Manager Take-profit is never propagated; every position gets an arbitrary +5 % TP Fixed
OBS-07 Medium Risk Manager Transient publish failures permanently drop accepted intents Fixed
OBS-08 Medium Exposure tracking In-memory gate state diverges from the real book after intraday exits Fixed
OBS-09 Medium Risk Manager square-off Single-shot-per-day despite partial liquidation failures Fixed
OBS-10 Low Risk Manager square-off "Waiting" log line spams every 30 s all day Fixed
OBS-11 Low Trader Gateway Shutdown races produce connection-closed errors; leaked subscription on error path Fixed
OBS-12 Low Agents (general) Unguarded close(stopChan) — second Stop() panics Fixed
OBS-13 Low Quantitative Scout Stream-ensure failure is logged then ignored Fixed
OBS-14 Low Scout price publisher Throttle bypassed when exchange timestamp is 0; starved when timestamps freeze Fixed
OBS-15 Low Time guards Exact 15:00:00 IST boundary behaves opposite to documented intent Fixed
OBS-16 Info Sentiment bus sentiment.update has no consumer Intentional
OBS-17 Info Docs REPLAY_INSTRUCTIONS.md overstates replay coverage Fixed
OBS-18 Info Market Recorder Archive gaps after restarts; no backfill Deferred
OBS-19 Info Report tool Legacy closed rows (NULL closed_at) silently excluded Fixed
OBS-20 Info Layering Risk Manager imports the Quantitative Scout package Fixed
OBS-21 Info PaperTrader Constructed positions bypass Position.Validate Fixed
OBS-22 Info Schema / audit trade_log cannot be joined to positions except by string-prefix convention Fixed

Critical

OBS-01 — Entry pipeline is inert: the volume-breakout gate can never pass

Locations: src/analysts/quantitative/feed.go:125-129, src/analysts/quantitative/breakout.go:117-123, src/analysts/quantitative/indicators/volume.go:60-68

The only writer of ticks into the TickerStore is the feed connector's snapshot poller, which appends Volume: 0 for every tick ("LTP snapshot does not carry volume"). The breakout detector derives its volume series from that same store (store.RecentVolumes). In VolumeBreakout, the volume SMA of an all-zero history is 0, and the function explicitly returns IsBreakout = false when sma == 0 (volume.go:61).

Consequence: the long condition (aboveVWAP && volResult.IsBreakout && aboveUpper) and its short mirror can never be true at runtime. No TradeIntent is ever published on signal.intent.new; the Risk Manager, Trader Gateway, exit monitor, P&L accounting, and reporting layers receive no traffic in production shape. The system as deployed cannot trade.

Notes:

  • Unit tests pass because they inject synthetic volumes directly; nothing tests the feed→store→detector composition.
  • A historical-candles REST client exists (pkg/groww/historical.go) but is referenced nowhere in the scout; candle-based volume is therefore available in principle yet unused.
  • This also means the phantom-position defect (OBS-02) is currently masked: with no entries there is nothing to exit. Both defects surface together the moment OBS-01 is repaired.

OBS-02 — Closing executions open phantom reverse positions

Locations: src/analysts/trader/papertrader.go:31-47, src/analysts/trader/exitmonitor.go:163-201, src/analysts/riskmanager/squareoff.go:82-122

Both exit paths perform a dual write: they persist the closure themselves (MarkClosed on the original row) and publish an opposite-side execution onto signal.execute.<TICKER>:

  • Exit monitor: publishes exit-<posID>-<ts> SHORT-to-close-LONG (exitmonitor.go:184), then marks the original LONG closed (:190).
  • Square-off: publishes sqoff-… (squareoff.go:120), then marks closed (:82).

The Trader Gateway consumes everything on signal.execute.* and PaperTrader.Execute unconditionally inserts a new OPEN position (papertrader.go:33) — there is no netting logic, no recognition of SignalReason ∈ {stop_loss, take_profit, auto_square_off}, and no filtering of exit-* / sqoff-* correlation-ref prefixes anywhere in handleExecution (agent.go:158-190).

Consequences per SL/TP/square-off event:

  1. The original position is closed (correct), and simultaneously a phantom opposite-side position is opened (e.g. LONG stopped out ⇒ naked SHORT appears). Each phantom carries default ±5 % boundaries (see OBS-06), is itself exit-monitored, and is squared off at EOD — generating further churn (a phantom SHORT's square-off spawns a phantom LONG, etc.).
  2. trade_log accumulates SIMULATED rows for trades that never represented independent decisions.
  3. Reported statistics are polluted: closed_trades, win/loss counts, and by_exit_reason attribution in the report tool count phantom round trips.
  4. Realized-P&L distortion is partially masked because phantoms tend to close near their entry at EOD, but intraday drawdown, exposure-sensitive metrics, and any per-trade analysis are wrong.

There is no owner of the invariant "the open-position set remains economically consistent across a close": each component behaves correctly under its own contract, and unit tests stub the counterparties (the exit-monitor tests never route the published execution back through PaperTrader).

High

OBS-03 — Square-off books zero P&L on every EOD liquidation

Location: src/analysts/riskmanager/squareoff.go:82 (and the published fill price at :113)

liquidate calls MarkClosed(pos.ID, pos.EntryPrice, ReasonSquareOff). Because realized P&L is derived from (exit_price − entry_price) × qty, every square-off close computes exactly 0.00 P&L. The same entry-price value is used as the published execution price, so the trade-log fill agrees with the position row — consistently wrong rather than inconsistently.

Impact: the daily P&L reported by the report tool omits all EOD-liquidated open profit/loss; by_exit_reason["square_off"] is structurally ~0, making exit-attribution analysis meaningless for anything held to close. A last-known market price is already streaming on signal.price.* and is unused here.

OBS-04 — Downtime silently loses messages (at-least-once does not survive restarts)

Location: src/pkg/nats/pubsub.go:45-54

Subscribe creates ephemeral push consumers with DeliverNew(): no durable name, no bound deliver policy beyond "new only". If an agent restarts (crash, deploy, connectivity blip longer than the reconnect window), a fresh consumer starts at the stream head — every intent, execution, and price tick published while the agent was away is never delivered, with no error and no trace.

Impact examples: executions published during a Trader Gateway restart vanish (the RM believes it promoted; no position ever appears); price ticks missing from the recorder archive create permanent gaps (see OBS-18). The "at-least-once delivery" guarantee documented on the Trader agent holds only within a single consumer lifetime.

OBS-05 — Non-idempotent execution handling produces duplicate positions on redelivery

Locations: src/analysts/trader/papertrader.go:31-47, src/analysts/trader/agent.go:175-181, src/migrations/001_initial_schema.up.sql (positions table)

handleExecution Naks on any Execute error, relying on JetStream redelivery. Inside Execute, the position insert and the trade-log insert are two independent writes with no transaction:

  • If the trade-log insert fails after the position insert succeeded, the handler Naks; on redelivery the whole Execute runs again and inserts a second identical OPEN position (same ticker/side/qty/ref). Nothing dedupes: execution_ref carries no UNIQUE constraint in the schema, and the handler performs no lookup-before-insert.
  • Redelivery is immediate: m.Nak() is called without a delay, so a database outage produces a tight redeliver-hot-loop hammering the DB for the life of the outage, then potentially inserting multiples once the DB recovers mid-retry.

Related: the same pattern means a crash between the two inserts leaves a position with no trade-log row.

Medium

OBS-06 — Take-profit never propagated; every position governed by an arbitrary +5 %

Locations: src/analysts/riskmanager/executor.go:63-74 (buildExecution sets StopLoss but not TakeProfit), src/analysts/trader/papertrader.go:59-66 (defaults TP = price × 1.05, SL = price × 0.95 when absent)

models.TradeExecution has a take_profit field, but the Risk Manager never populates it. Consequently every paper position is created with a mechanical +5 % take-profit regardless of volatility, strategy, or the risk distance used for sizing, and the exit monitor enforces that arbitrary boundary as if it were a strategy decision. Positions whose natural target differs are closed early (or never) based on a constant, and by_exit_reason["take_profit"] statistics measure the constant, not the strategy.

OBS-07 — Transient publish failures permanently drop accepted intents

Location: src/analysts/riskmanager/agent.go (handleIntent promote path)

When promote fails, the intent is Acked as terminal. The accompanying comment reasons about insufficient capital (correctly terminal), but promote also returns the raw error from js.Publish (executor.go:84). A momentary NATS unavailability therefore converts a fully-approved trade into a silently discarded message — the intent is acknowledged upstream and never reconsidered, with only a log line distinguishing "strategy declined" from "infrastructure hiccup".

OBS-08 — Exposure-gate state diverges from the actual book after intraday exits

Locations: src/analysts/riskmanager/exposure.go (Register/Release), src/analysts/riskmanager/squareoff.go:86-88 (Release only here), src/analysts/trader/exitmonitor.go (closes without notifying RM)

The Risk Manager tracks open-position counts in memory (Register on promote, Release only during square-off). Intraday SL/TP closures happen in the Trader process and never release RM-side capacity: after an exit, the sector cap (1) and platform ceiling (4) remain consumed by positions that no longer exist, blocking new entries for the rest of the day (systematic under-trading). In the opposite direction, phantom positions created by OBS-02 occupy DB space but consume no RM budget at all. There are two unsynchronised sources of truth for "what is open": the RM's atomics and the positions table.

OBS-09 — Square-off gives up for the day even when individual liquidations fail

Locations: src/analysts/riskmanager/squareoff.go:57-91

Per-position failures inside liquidate merely continue ("keep the position open so it can retry"), but squareOff sets sqDone = day as soon as liquidate returns nil — and it returns nil when only individual positions failed (only a total listing error aborts). A position whose mark-closed or publish step failed at 15:15 is therefore stranded OPEN overnight; the next attempt happens the following trading day, contradicting the in-code retry promise and the MIS rationale for squaring off by 15:15.

Low

OBS-10 — Square-off "waiting" log line emitted every 30 s all trading day

Location: src/analysts/riskmanager/squareoff.go:49-52

The now.Before(trigger) branch logs "waiting until 15:15" on every 30-second tick from market open onward (~330 lines/day of identical noise), before the once-per-day guard has any effect.

OBS-11 — Trader shutdown races and an error-path subscription leak

Locations: src/analysts/trader/agent.go (Run/Stop)

  • Stop() closes stopChan and immediately calls js.Close() without waiting for Run to finish tearing down. Run's deferred unsubscribes then race the closed connection, producing recurring Unsubscribe error: nats: connection closed log noise (observed in test output). The Recorder implements a stopped handshake; the Trader does not.
  • If the price-stream subscribe fails after the execute-subscribe succeeded, Run returns without unsubscribing the execute subscription, leaving a live consumer attached until Stop closes the connection.

OBS-12 — Second Stop() panics on most agents

Locations: Stop() implementations across analysts (safe exception: recorder)

close(g.stopChan) is unguarded; a second Stop() invocation (shutdown race, defensive caller, future supervisor code) panics on double-close. Only the Recorder guards with a select on the already-closed channel.

OBS-13 — Scout ignores stream-setup failure and continues running

Location: src/analysts/quantitative/agent.go:58-60

Unlike every other agent (which aborts Run on EnsureStream failure), the Scout logs the error and starts feeding anyway. All subsequent intent/price publishes then fail per-message, yielding a long-lived degraded process that looks healthy while emitting continuous publish errors.

OBS-14 — Price-publish throttle misbehaves on degenerate timestamps

Location: src/analysts/quantitative/feed.go:144-148

The guard tsMillis > 0 && … < interval means an exchange snapshot with TsInMillis == 0 bypasses the throttle entirely, publishing on every 100 ms poll per symbol (up to 10 msg/s/ticker against the intended ≤1). Conversely, a frozen-but-nonzero timestamp (stale feed) suppresses publishing indefinitely even as prices change. Throttle state keys off event time rather than wall-clock receipt time.

OBS-15 — Exactly-15:00:00 IST behaves opposite to documentation

Locations: src/pkg/utils/time.go:39-47 (now.After(cutoff) — strictly after), src/analysts/trader/exitmonitor.go:110

At precisely 15:00:00.000 IST, IsClosingCutoff returns false, so the exit monitor may still liquidate at that instant although the helper's doc comment specifies "at or past 3:00 PM". One-tick-wide boundary inconsistency between documented intent and implementation.

Informational

OBS-16 — sentiment.update is published but has no subscriber

Locations: src/analysts/sentiment/agent.go:234, src/pkg/nats/types.go:29

The Risk Manager reads scores from Postgres (ScoreStore.LatestScore), not from the bus. Every sentiment update is persisted to the sentiment stream with zero consumers — dead traffic today (harmless, but the HLD implies a broadcast contract nothing fulfils).

OBS-17 — REPLAY_INSTRUCTIONS.md overstates replay coverage

Location: docs/REPLAY_INSTRUCTIONS.md (pipeline diagram & framing)

Replaying signal.price.* exercises the exit monitor, recorder, and reporting paths, but cannot generate new entries: breakout detection needs volume-bearing candles (OBS-01: LTP ticks carry none), and the sentiment gate requires live scrape/LLM activity. The document's "re-drive the full pipeline" diagram and Step-2 guidance imply otherwise; as written, a reader should expect trades from a replay alone and will observe none.

OBS-18 — Recorder archives have unrecoverable gaps after downtime

Location: src/analysts/recorder/agent.go (ephemeral subscription)

Ticks published while the Recorder is down are gone (OBS-04 mechanism); there is no durable consumer, no sequence-numbering, and no reconciliation against the stream, so archives are lossy by construction around restarts.

OBS-19 — Report silently excludes legacy closed rows

Location: src/pkg/database/report.go (ClosedPositions range predicate)

Rows closed before migration 003 (NULL closed_at) fail the closed_at >= $1 predicate and drop out of all summaries without warning; long-lived deployments upgrading mid-history will see analytics that start at the migration date with no indication of prior data.

OBS-20 — Risk Manager imports the Quantitative Scout package

Location: src/analysts/riskmanager/exposure.go (import block)

An analyst-to-analyst compile-time dependency (for universe/sector types) couples agents that are otherwise peers communicating only via NATS; it constrains refactor freedom and contradicts the agents-as-independent-modules structure.

OBS-21 — PaperTrader skips Position.Validate

Location: src/analysts/trader/papertrader.go:54-77

Positions are assembled and inserted directly, relying on DB CHECK constraints (side/status enums) to catch malformed rows; model-level invariants (e.g. positive boundaries, required sector) are never exercised on this path, so validation drift between the model and the schema would go unnoticed.

OBS-22 — No structural link between trade_log rows and positions

Locations: src/migrations/001_initial_schema.up.sql (trade_log), correlation-ref conventions (risk-*, exit-<posID>-*, sqoff-<ref>-*)

Reconstructing "which fill closed which position" requires parsing string-prefix conventions out of trade_log; the schema offers no position_id foreign key, and square-off refs embed the entry execution ref while exit refs embed the row id — three different identifier domains with no index or constraint backing them.


Cross-cutting notes

  • Testing posture: component tests mock their neighbours' interfaces, so every defect above involving composition (OBS-01, 02, 04, 05, 08) is invisible to the suite. There is no integration test asserting end-state invariants (e.g. "net exposure returns to zero after an SL", "quantity of OPEN positions equals RM's counter").
  • CI gates are build/fmt/lint/unit-test plus a PR-title check; none exercise a broker-less end-to-end run, which is precisely the gap the recorder/replay/ report tooling (this stack) was created to close.

🌊 Navier-Stocks Documentation


Clone this wiki locally