Curious#3
Merged
Merged
Conversation
- Added `github.com/cinar/indicator/v2` as a new indirect dependency in `go.mod`. - Updated `Makefile` to include new debug targets for improved debugging workflows. - Refactored WebSocket handling in `cmd/root.go` to ensure proper execution of concurrent tasks. - Enhanced signal gauge drawing logic in `frontend/src/components/charts/confidence/draw-signal-gauge.ts` to account for calibration state. - Expanded test coverage for websocket handlers in `frontend/src/providers/websocket-handlers.test.ts` to validate new functionality. - Improved websocket handling in `frontend/src/providers/websocket.ts` for better error management and message processing. - Refactored various components to streamline code and enhance maintainability.
- Updated the Makefile to enable debugging with the `DATURA_INSPECT` environment variable during the run command. - Improved the signal gauge drawing logic to account for calibration state in `frontend/src/components/charts/confidence/draw-signal-gauge.ts`. - Added a new test case in `frontend/src/providers/websocket-handlers.test.ts` to validate gauge hydration from state readings. - Enhanced WebSocket handling in `frontend/src/providers/websocket.ts` for better error management. - Refactored the `Crypto` struct in `trader/crypto.go` to streamline broadcast management and improve subscription handling. - Removed obsolete test files to clean up the codebase and improve maintainability.
…handling - Deleted the `websocket_test.go` file to clean up the codebase and remove unused tests. - Improved the `onMessage` method in `websocket.go` to ensure the WebSocket connection is established before processing messages, enhancing error handling and reliability.
- Refactored the `onMessage` method in `websocket.go` to utilize a context-aware approach for connection checks, improving error handling. - Introduced new functions in `signal_test.go` to insert trader tickers with timestamps and validate the order of measurements. - Enhanced the `Measure` method in `signal.go` to sort artifacts by timestamp and update cursors accordingly, ensuring accurate signal processing. - Added tests in `hub_test.go` to verify WebSocket message writing and artifact handling, improving overall test coverage and reliability.
- Replaced the `DashboardLayout` component with `SymmTerminal` in the main routing file, streamlining the application structure. - Improved the `Measure` method across various signal implementations to enhance artifact handling and ensure robust processing of measurement data. - Updated signal measurement tests to validate the new logic and ensure comprehensive coverage of signal processing scenarios.
- Added multiple images of the S.Y.M.M. Terminal to the README for better visualization. - Updated CSS styles to improve the overall appearance, including scrollbar customization and selection colors. - Refactored terminal component styles for consistency and improved layout structure. - Enhanced signal processing tests to validate new health status logic and ensure comprehensive coverage.
- Added a new test case for rendering the decision tree candidate surface in the dashboard, ensuring proper display of backend data. - Refactored dashboard styles for consistency, including updates to color variables and layout adjustments. - Improved health panel metrics display and updated kernel list styling for better user experience. - Enhanced WebSocket handling to support new decision trace events, improving real-time data processing capabilities.
- Introduced a new trading model check to differentiate between live trading and paper trading, enhancing WebSocket connection handling for private and public data streams. - Refactored cognitive frame parsing to support nested cognition attributes, improving data normalization and retrieval for cognitive readings. - Added new test cases to validate the parsing of nested cognition attributes and ensure comprehensive coverage of cognitive frame scenarios. - Updated frontend components to reflect changes in cognitive data handling and improve overall user experience.
- Added a new guideline for error handling, specifying that instance variables for errors should always be named `err` and logged using `errnie`. - Revised the review guidelines to emphasize correctness, performance, and complexity, providing clearer instructions for code reviews. - Removed outdated sections to streamline the review process and enhance clarity in expectations for contributors.
- Added `github.com/cloudwego/base64x v0.1.7` as a new indirect dependency in `go.mod`. - Refactored error handling in `confidenceBaseline`, `ringCapacity`, and `spreadWideRatio` functions to include error checks and logging using `errnie`. - Updated test cases in `operand_test.go` to utilize the shorthand for convey assertions, enhancing readability and consistency.
- Updated the review guidelines in `review.md` to clarify correctness checks and performance considerations, including new rules for dynamic processing and artifact handling. - Added a new `Logo` component to standardize branding across the application. - Refactored CSS styles in `styles.css` and `layout.css` to improve layout consistency and responsiveness. - Introduced a new `Flex` component for better layout management in the UI. - Added new tab components to enhance user navigation and organization within the application. - Improved overall styling and structure of the frontend components for a more cohesive user experience.
- Updated the `Desk` struct to handle trader actions more effectively, including the introduction of a new `execute` method for processing buy and sell orders. - Enhanced stoploss management by implementing `armStop`, `retireStop`, and `ratchet` methods to ensure proper trailing stop functionality. - Improved error handling in the `execute` method to validate actions and ensure robust order processing. - Refactored the `Stoploss` struct to encapsulate stop management logic and facilitate artifact serialization for shared tree storage. - Updated the `Update` method in `Desk` to process multiple actions in a single call, enhancing performance and usability. - Removed obsolete code and improved overall structure for better maintainability and clarity.
- Added a new section in `AGENTS.md` outlining the project objective and emphasizing the importance of principled decision-making in trading. - Updated the `Measure` method in various signal implementations to return an iterable sequence of artifacts, improving flexibility in handling measurements. - Introduced a `Held` method in the `Balances` struct to check for positive balances, enhancing ledger management. - Refactored the `Update` method in `Story` to incorporate live holdings, ensuring accurate evaluation of playbook conditions. - Improved the `SymbolFromTicker` function to accept a row index, allowing for better handling of multiple ticker updates. - Enhanced test cases across signal implementations to validate the new measurement logic and ensure comprehensive coverage.
…ions - Updated `AGENTS.md` to improve formatting and clarity in the decision-making process for trading stages. - Enhanced the `Desk` struct to include a new `quote` field for better handling of market quotes. - Refactored the `execute` method to incorporate risk-based sizing for buy orders, improving order management. - Improved error handling in the `send` method to ensure robust communication with the broker. - Updated the `markFor` method to utilize a timestamp-based prefix for ticker retrieval, enhancing data accuracy. - Enhanced test coverage for the `Desk` struct to validate new functionalities and ensure reliability.
- Updated the `Desk` struct to include new methods for aligning entry quantities and rounding to exchange increments, ensuring compliance with trading constraints. - Introduced an `instrument` struct to encapsulate exchange constraints for trading pairs, improving order management. - Enhanced the `sizeBuy` method to align order sizes with instrument constraints, preventing sub-minimum entries. - Added tests to validate the new alignment logic and ensure robust handling of trading operations. - Updated the configuration for UI address to improve application connectivity.
…cessing - Updated the `AllocationSidePanel` to enhance allocation logic, integrating real-time data handling and improved UI rendering. - Refactored the `DecisionTreeView` to streamline decision evaluation, incorporating causal ladder visualization and better state management. - Enhanced the `CausalLadder` component to display causal measurement insights effectively. - Updated the `Makefile` to streamline frontend testing commands. - Added configuration options in `config.yml` to cap subscribed symbols for websocket connections, improving performance and reliability.
… and cognitive visualization - Updated the `markFor` method in the `Desk` struct to enhance ticker retrieval logic, improving data accuracy. - Refactored the `allocationRows` function to streamline allocation logic, integrating new metrics for candidate evaluation. - Enhanced cognitive visualization components to utilize backend DMT branches, improving the representation of cognitive readings. - Added new properties to `CognitiveReading` and related types to support expanded functionality in cognitive processing. - Improved error handling and state management in various frontend components for better user experience and reliability.
…proved data processing - Updated the measurements store to support batching of readings and maintain history for each measurement, improving data accuracy and performance. - Refactored tests for the measurements store to validate new functionality and ensure robust handling of updates. - Enhanced terminal components to utilize the new measurement history features, improving visualization of confidence metrics. - Introduced new utility functions for managing measurement history in terminal components, streamlining data access and rendering. - Improved websocket handling for measurement updates, ensuring efficient data processing and reduced latency.
…omponents - Added a pprof server to the application for performance profiling, configurable via environment variables and application settings. - Refactored decision handling in the frontend to improve state management and ensure accurate updates based on incoming frames. - Enhanced the decision store to aggregate decisions based on their sequence, improving the handling of decision frames. - Updated tests to validate the new decision processing logic and ensure robust functionality across components. - Removed deprecated configuration options related to symbol capping, streamlining the configuration file.
- Added .codebase-memory/graph.db.zst to .gitignore to prevent unnecessary file tracking. - Introduced a new test case in cognitive-viz.test.ts to verify that the current sensory sequence is highlighted correctly, ensuring accurate representation of cognitive beams. - Refactored cortex-tree.ts to improve the handling of beam prefixes, enhancing the logic for active prefixes based on reading and beam data. - Updated decisions-from-walk.ts to allow for dynamic kernel scoring based on symbol-specific conditions, improving decision accuracy. - Enhanced terminal dashboard rows to include edgePositive state in tests, ensuring comprehensive coverage of decision metrics.
…omponents - Updated the artifact.json to reflect new commit and indexing details. - Enhanced the allocation model to utilize backend decision frames, improving decision accuracy. - Added tests to validate the integration of backend decision batches in allocation logic. - Refactored decision handling in the frontend to ensure accurate updates based on incoming frames. - Improved snapshot handling in trader components to maintain the latest measurements per origin/symbol.
- Added .codebase-memory/artifact.json to .gitignore to prevent unnecessary file tracking. - Updated the Desk's sizing logic to account for configured adverse slippage, improving order quantity calculations. - Introduced a new test for Desk sizing against configured slippage, ensuring accurate handling of market entries. - Refactored decision processing in frontend components to maintain accurate updates based on incoming frames.
- Updated the Desk's `Update`, `sizeBuy`, and `roundQuantity` methods to return errors for invalid inputs, improving robustness. - Enhanced the `markFor` method to return errors when no valid mark is found, ensuring better error reporting. - Refactored the `alignEntry` method to return errors for entries below minimum requirements, preventing invalid trades. - Improved error handling in WebSocket connections and message processing across various components, enhancing reliability. - Updated tests to validate new error handling logic and ensure accurate behavior in trading scenarios.
- Removed the `components.json` file to streamline the configuration. - Updated `package.json` to simplify build and development scripts, removing unnecessary commands. - Cleaned up `pnpm-lock.yaml` by removing unused dependencies and updating existing ones. - Deleted unused styles in `styles.css` to reduce bundle size and improve performance. - Refactored tests in `measurements.test.ts` for clarity and accuracy in reading updates. - Removed several unused components, including `audit.tsx`, `header.tsx`, and others, to enhance code maintainability.
- Updated Desk's state management to utilize atomic pointers for better concurrency handling. - Refactored stoploss logic to improve state updates and ensure accurate trailing stop behavior. - Enhanced error handling in Desk's message processing, ensuring robust responses to invalid inputs. - Introduced new tests to validate the updated Desk functionality and ensure accurate trading operations. - Improved configuration settings in `config.yml` for optimized market depth and subscription pacing.
- Updated the Desk's state structure to include orders and positions, improving trading logic and state management. - Enhanced error handling in Desk's message processing to prevent panics on invalid artifacts. - Introduced new tests for Desk functionality, including handling of invalid artifacts and ensuring accurate state restoration on startup. - Updated artifact serialization methods to handle nil cases gracefully, improving robustness. - Enhanced configuration settings in `config.yml` for optimized stop management and trading parameters.
- Added new dependencies for the Kraken futures WebSocket client. - Removed outdated and unused code related to paper trading and execution handling. - Refactored WebSocket connection logic to improve error handling and connection stability. - Updated artifact handling in the WebSocket to ensure accurate message routing and processing. - Cleaned up test cases related to paper trading, ensuring they align with the current architecture. - Enhanced configuration settings for better management of WebSocket connections and message processing.
…nsity - Introduced the `HawkesIntensityPanel` component to visualize Hawkes self-exciting intensity with a canvas drawing. - Implemented logic to handle sample data and display intensity metrics dynamically. - Enhanced the layout of the route component to integrate the new panel, improving user interface for metrics visualization.
- Introduced setup key handling in Desk's order processing to improve decision tracking and risk management. - Updated Desk's state management to include new maps for pending orders and acknowledgments, enhancing order lifecycle management. - Added tests to validate setup key integration in order payloads and ensure accurate processing of market and limit orders. - Refactored order handling logic to incorporate setup key checks, improving the robustness of order execution and risk assessment. - Enhanced stoploss and edge estimation logic to utilize setup keys for better decision-making in trading scenarios.
…ality - Updated Makefile to introduce new optimization parameters for audit and replay files, improving the optimization process. - Refactored the optimize-playbook command to support additional flags for replay, symbols, and iterations, enhancing flexibility in optimization. - Modified desk component to streamline order processing by removing unnecessary order roles, improving clarity in execution handling. - Enhanced test cases to validate changes in decision processing and ensure accurate symbol handling in terminal components.
- Removed the OPTIMIZE_ITERATIONS variable from the Makefile to streamline the optimization command. - Updated the optimize command to simplify its parameters, enhancing usability and flexibility. - Added validation checks in the Desk component to handle nil artifacts and improve error handling. - Updated artifact.json to reflect new commit details and metrics, ensuring accurate tracking of changes. - Enhanced various components to ensure robust handling of invalid artifacts and improve overall system stability.
…ocessing - Updated artifact.json to reflect new commit details and revised metrics for nodes, edges, original size, and compressed size. - Introduced new tests in the Desk component to validate unfilled limit order cancellation after transit TTL, ensuring accurate order lifecycle management. - Enhanced Desk's order processing logic to include decision and action IDs, improving tracking and management of orders. - Refactored various components to streamline error handling and improve overall system stability.
…tionality - Updated artifact.json to reflect new commit details and revised metrics for nodes, edges, original size, and compressed size. - Removed the deprecated DispatchWebSocketFrame function and refactored WebSocket handling for clearer message processing. - Standardized channel naming from "TradesChannel" to "TradeChannel" across multiple files for consistency. - Enhanced the CrossSection and Signal components to improve handling of ticker artifacts and measurement logic. - Removed unused Symbol struct and related functions to streamline the codebase. - Updated tests to validate changes in artifact handling and ensure robust measurement across signals.
- Updated AGENTS.md to clarify implementation principles for signal processing, emphasizing rigorous adherence to specifications and avoidance of shortcuts. - Removed deprecated causal history and node management files to streamline the codebase. - Enhanced the Signal struct to incorporate new components for ticker, book, and trade, improving measurement capabilities. - Updated artifact.json to reflect new commit details and revised metrics for nodes and edges. - Improved test coverage for signal measurement and artifact handling, ensuring robust validation of changes.
- Updated artifact.json to reflect new commit details and revised metrics for nodes, edges, original size, and compressed size. - Enhanced signal measurement logic to include origin tracking and structured payload validation. - Improved test coverage for signal measurement, ensuring robust validation of changes and accurate artifact handling. - Refactored signal components to streamline data processing and improve overall functionality.
…easurements - Simplified app state management by removing unused properties and consolidating state updates. - Introduced a new Circular buffer for managing measurements, enhancing data handling efficiency. - Refactored decision handling logic to utilize the new decisionStore, improving clarity and functionality. - Added new components for dashboard visualization, including Canvas and FluidLegend, to enhance user interface. - Removed deprecated decision funnel logic to streamline the codebase and improve maintainability.
- Removed the `smallnest/ringbuffer` dependency from `go.mod` and `go.sum` as it was no longer needed. - Refactored the `measurements` store to improve state management and streamline data handling. - Updated tests to reflect changes in the measurements logic, ensuring robust validation of the new structure. - Enhanced the `decisions` store to utilize the updated measurements handling, improving overall functionality.
- Added tests for handling pending orders and stoplosses, ensuring robust validation of order lifecycle management. - Introduced logic to refresh entry block status based on stoploss conditions, improving decision-making in order processing. - Refactored existing methods to streamline order quantity calculations and enhance overall functionality. - Updated test cases to reflect changes in order handling and ensure accurate processing of market and limit orders.
- Removed the `pending` field from the Desk struct, simplifying order management. - Refactored stoploss state handling to utilize a structured snapshot, enhancing clarity and maintainability. - Updated methods to directly manipulate stoploss state attributes, improving performance and readability. - Enhanced tests to validate changes in stoploss behavior and ensure robust handling of order states. - Removed unused functions related to stoploss state management, streamlining the codebase.
- Updated WebSocket routing to handle unrouted artifact roles, improving error management. - Refactored measurements handling to ensure stability during updates, maintaining the integrity of the measurements map. - Introduced new tests to validate the stability of measurements while updating the touched origin, ensuring robust order management. - Enhanced the Circular buffer implementation for measurements, improving data handling efficiency. - Removed deprecated test files to streamline the codebase and improve maintainability.
- Introduced DEFAULT_KERNELS for consistent kernel management in app state. - Updated measurements handling to include regime data, improving measurement accuracy. - Enhanced Pulse component to display rejection messages based on decision store updates. - Refactored health components to replace "Measured" with "Healthy" for clarity. - Improved WebSocket routing to accommodate new regime artifacts, ensuring robust data flow. - Added tests to validate regime handling and ensure accurate measurement updates.
…umentation and improve clarity on signal processing principles.
- Introduced synchronization for concurrent access to WebSocket connections, ensuring thread safety. - Updated live invariant tests to utilize configurable timeout values for improved maintainability. - Added a new test for median absolute value calculation, enhancing statistical validation. - Refactored Crypto component to streamline balance handling and improve error management. - Enhanced signal measurement tests to validate quiet regime behavior, ensuring accurate signal processing.
- Refactored the Desk struct to simplify order management by removing unnecessary fields and introducing structured components for balances, ticker, and orders. - Updated go.mod to include new dependencies for Gorilla WebSocket and Kraken API, enhancing WebSocket functionality and API integration. - Removed outdated files and tests related to previous implementations, streamlining the codebase for better maintainability. - Enhanced documentation in AGENTS.md to clarify principles and best practices for signal processing and order management.
- Refactored the Broker component to improve order management and balance handling by introducing structured types and simplifying the update logic. - Added new tests for balance book, capital reserve, and pending order updates to ensure robust validation of order lifecycle and state management. - Updated dependencies in go.mod to include necessary packages for improved functionality and maintainability. - Removed outdated files and streamlined the codebase for better clarity and performance.
- Added new category types: Laminar Resonance, Turbulent Resonance, and Equilibrium to the category system for improved classification. - Implemented a new function, KnownCategory, to validate category types and ensure robustness in measurement handling. - Enhanced ApplyClassifier method to include additional validation checks for measurement parameters, ensuring data integrity. - Updated tests to validate the new measurement readiness logic and category handling, improving overall test coverage and reliability.
- Replaced references to the outdated `datura` package with the new `symm/cognitive/dmt` package across multiple files, ensuring consistency in the codebase. - Updated the `go.mod` and `go.sum` files to remove unused dependencies and reflect the changes in package imports. - Refactored test cases to align with the new frame structure, enhancing clarity and maintainability in the testing framework. - Improved the fixture generation logic to utilize frames instead of artifacts, streamlining the data handling process.
- Added new indirect dependencies including `capnproto.org/go/capnp/v3`, `github.com/colega/zeropool`, and others to `go.mod` and `go.sum` for enhanced functionality. - Replaced references to the outdated `dmt` package with the new `theapemachine/datura/dmt` package in multiple files, ensuring consistency and improved integration. - Removed the obsolete `cognitive` package files, streamlining the codebase and enhancing maintainability. - Updated import paths in `optimize.go` and `root.go` to reflect the new package structure, improving clarity in the codebase.
- Updated references from the `logic` package to the new `types` package across multiple files, ensuring consistency in type usage. - Refactored the `Holdings` method in the `BalanceBook` and `Desk` components to utilize the new `types.Holdings` structure. - Adjusted action handling in the `Desk` and `OrderFactory` components to align with the new `types.Action` structure, improving clarity and maintainability. - Removed obsolete test files and streamlined the codebase by deleting unused components, enhancing overall performance and readability.
- Deleted obsolete files related to the broker's balance, capital, order, and pending order functionalities, streamlining the codebase. - Removed associated test files to enhance maintainability and reduce clutter in the repository. - This cleanup improves overall performance and clarity in the broker component structure.
- Refactored the Desk component to enhance order management by introducing a structured approach for handling positions and executions. - Updated the `go.mod` file to include the `github.com/google/uuid` package as an indirect dependency, improving UUID handling. - Added new tests for the Desk component to validate the handling of open positions and UI forwarding, ensuring robust functionality. - Streamlined the codebase by removing obsolete imports and enhancing overall maintainability.
- Integrated appStore into the instruments and terminal components to manage focus symbols more effectively. - Refactored SignalDetail and CausalLadder components to utilize appStore for focus symbol retrieval, improving state management. - Updated health metrics and decision-making components to reflect changes in focus symbol handling, enhancing overall functionality. - Added new metrics and visualizations in the DecisionsSurface and HealthPanel components for better user insights. - Improved websocket handling to ensure timely updates of instrument data across connected clients, enhancing real-time performance.
- Introduced order management capabilities in the Desk component by adding a new channel for orders and updating the Run method to handle incoming order messages. - Updated tests to validate the handling of balance, execution, and order payloads, ensuring robust functionality and accurate UI forwarding. - Refactored data structures to utilize json.RawMessage for improved flexibility in handling different payload types. - Enhanced the PaperOrder type and related methods to support the new order management features, improving overall integration with the Kraken API.
- Introduced cognitive priors in the decision-making process, allowing for dynamic adjustments based on market conditions. - Enhanced the decision runtime to incorporate prior settings, improving the accuracy of solver controls. - Updated tests to validate the integration of cognitive priors and ensure correct behavior of the decision clock with sparse events. - Refactored category handling in physical evidence to utilize the new canonical category index, improving consistency across the codebase.
|
Important Review skippedToo many files! This PR contains 567 files, which is 417 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (20)
📒 Files selected for processing (567)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request overhauls the symm trading system by removing the qpool dependency, refactoring the broker Desk to manage websocket streams directly, and introducing a dedicated Position management structure. It also replaces the frontend dashboard with a lightweight terminal layout and adds a critical codebase review document. The review feedback highlights several critical concurrency and reliability issues in the new broker code, including data races on Desk fields, a blocking channel send on the UI update path, and non-atomic map updates in Buy(). Additionally, the feedback addresses a double-selling risk in Sell(), duplicate fields in the Position struct causing stale data bugs, flaky test sleeps, and performance inefficiencies from unthrottled UI updates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
22
to
33
| type Desk struct { | ||
| ctx context.Context | ||
| cancel context.CancelFunc | ||
| pool *qpool.Q[any] | ||
| tree *dmt.Tree | ||
| ctx context.Context | ||
| cancel context.CancelFunc | ||
| channels map[string]chan []byte | ||
| public websocket.Socket | ||
| private websocket.Private | ||
| balance *kraken.BalanceDataSlice | ||
| executions []*kraken.ExecutionDataSlice | ||
| orders *kraken.PaperOrderSlice | ||
| positions *sync.Map | ||
| UIForward chan []byte | ||
| } |
There was a problem hiding this comment.
Critical Data Race Hazard
There is a critical data race on the desk.balance field (and potentially desk.executions / desk.orders).
- Write Path: The
Run()goroutine updatesdesk.balanceconcurrently when a new balance message is received:desk.balance = kraken.NewBalanceDataSlice(msg)
- Read Path: Other goroutines (such as the trader's tick loop) concurrently read
desk.balanceviaReady()andBuy():"balance": desk.balance
In Go, concurrent read and write access to a non-atomic pointer/slice variable without synchronization is a data race, which can lead to undefined behavior, memory corruption, or panics.
Recommendation
Add a sync.RWMutex to the Desk struct and use it to synchronize access to balance, executions, and orders across all goroutines.
Suggested change
| type Desk struct { | |
| ctx context.Context | |
| cancel context.CancelFunc | |
| pool *qpool.Q[any] | |
| tree *dmt.Tree | |
| ctx context.Context | |
| cancel context.CancelFunc | |
| channels map[string]chan []byte | |
| public websocket.Socket | |
| private websocket.Private | |
| balance *kraken.BalanceDataSlice | |
| executions []*kraken.ExecutionDataSlice | |
| orders *kraken.PaperOrderSlice | |
| positions *sync.Map | |
| UIForward chan []byte | |
| } | |
| type Desk struct { | |
| ctx context.Context | |
| cancel context.CancelFunc | |
| mu sync.RWMutex | |
| channels map[string]chan []byte | |
| public websocket.Socket | |
| private websocket.Private | |
| balance *kraken.BalanceDataSlice | |
| executions []*kraken.ExecutionDataSlice | |
| orders *kraken.PaperOrderSlice | |
| positions *sync.Map | |
| UIForward chan []byte | |
| } |
| out["positions"] = positions | ||
| } | ||
|
|
||
| desk.UIForward <- out.Marshal() |
There was a problem hiding this comment.
Blocking Send on UIForward Channel
The channel send desk.UIForward <- out.Marshal() is a blocking operation. If the UI client or receiver is slow, disconnected, or fails to drain the channel, the entire Run() loop will block.
Because Run() is responsible for draining the websocket message channels (desk.channels), blocking here will stop processing incoming market data, balances, and executions. This can lead to Kraken disconnecting the client due to ping timeouts or buffer overflow.
Recommendation
Use a non-blocking select send or dispatch the UI update to a separate goroutine to ensure the hot path of draining websocket channels is never blocked:
select {
case desk.UIForward <- out.Marshal():
default:
// Drop update or log warning if the UI consumer is falling behind
} select {
case desk.UIForward <- out.Marshal():
default:
}
Comment on lines
+177
to
+184
| positions, ok := desk.positions.LoadOrStore(positionData.Symbol, []*Position{position}) | ||
|
|
||
| if ok { | ||
| desk.positions.Store( | ||
| positionData.Symbol, | ||
| append(positions.([]*Position), position), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Race Condition / Lost Update Hazard in Buy()
The sequence of LoadOrStore followed by Store is not atomic. If two concurrent goroutines call Buy() for the same symbol, both might load the same existing positions slice, append their respective new position, and then call Store. The one that stores last will overwrite the other's write, resulting in a lost position update.
Recommendation
Use a mutex to synchronize updates to the positions map, or use a lock-free concurrent list/slice update pattern. Since Buy is not a high-frequency operation, protecting the map updates with a simple mutex on Desk is the safest and most robust solution.
Comment on lines
+201
to
214
| for _, position := range positions.([]*Position) { | ||
| err = errors.Join(err, position.Exit()) | ||
| } | ||
|
|
||
| if err != nil { | ||
| return errnie.Error(errnie.Err( | ||
| errnie.UnprocessableContent, | ||
| err.Error(), | ||
| err, | ||
| )) | ||
| } | ||
|
|
||
| desk.positions.Delete(symbol) | ||
| return nil |
There was a problem hiding this comment.
Partial Failure and Double-Selling Risk
In Sell(), if there are multiple positions for a symbol and some fail to exit while others succeed, the function returns early due to the non-nil error, and desk.positions.Delete(symbol) is never called. This leaves the successfully exited positions in the active list. If the trader retries the Sell() operation, it will call Exit() again on all positions, including the ones that already successfully exited, leading to double-selling/unintentional shorting.
Recommendation
Only retain positions in the map that actually failed to exit, or remove each position from the active list immediately upon a successful Exit() call.
Comment on lines
+22
to
+28
| type Position struct { | ||
| private websocket.Private | ||
| mu sync.RWMutex | ||
| data PositionData | ||
| Symbol string | ||
| Qty float64 | ||
| } |
There was a problem hiding this comment.
Duplicate Fields and Stale Data Bug
The Position struct contains duplicate fields Symbol and Qty both at the root level and inside the nested PositionData struct. In NewPosition, only position.data is initialized; the root Symbol and Qty are left empty. However, in broker/desk_test.go, positions are constructed directly using the root fields.
This causes a major bug in Data(): if a position is constructed directly (with position.data.Symbol == ""), Data() will always return a PositionData with only Symbol and Qty set. Even if Update() is called and updates position.data.Mark and position.data.PnL, position.data.Symbol remains "", so Data() will continue to return 0 for Mark and PnL, completely ignoring all updates!
Recommendation
Remove the duplicate Symbol and Qty fields from the Position struct, and simplify Data() to always return position.data. Update the tests/benchmarks to initialize data: PositionData{Symbol: "...", Qty: ...} instead.
type Position struct {
private websocket.Private
mu sync.RWMutex
data PositionData
}
Comment on lines
+99
to
+111
| func (position *Position) Data() PositionData { | ||
| position.mu.RLock() | ||
| defer position.mu.RUnlock() | ||
|
|
||
| if position.data.Symbol == "" { | ||
| return PositionData{ | ||
| Symbol: position.Symbol, | ||
| Qty: position.Qty, | ||
| } | ||
| } | ||
|
|
||
| return position.data | ||
| } |
There was a problem hiding this comment.
Simplify Data() to Avoid Stale Data Bug
Following the removal of duplicate Symbol and Qty fields from the Position struct, Data() can be simplified to always return position.data directly. This avoids the stale data bug where updates to Mark and PnL are ignored.
Recommendation
Simplify Data() to return position.data directly under a read lock.
func (position *Position) Data() PositionData {
position.mu.RLock()
defer position.mu.RUnlock()
return position.data
}
Comment on lines
+222
to
+223
| desk.positions.Store("BTC/USD", []*Position{{Symbol: "BTC/USD", Qty: 0.01}}) | ||
| desk.positions.Store("SOL/USD", []*Position{{Symbol: "SOL/USD", Qty: 3.5}}) |
There was a problem hiding this comment.
Update Benchmark to Match Refactored Position Struct
Since the duplicate Symbol and Qty fields are removed from the Position struct, the benchmark should be updated to initialize the nested data field directly.
Recommendation
Update the benchmark to initialize data: PositionData{Symbol: "...", Qty: ...}.
Suggested change
| desk.positions.Store("BTC/USD", []*Position{{Symbol: "BTC/USD", Qty: 0.01}}) | |
| desk.positions.Store("SOL/USD", []*Position{{Symbol: "SOL/USD", Qty: 3.5}}) | |
| desk.positions.Store("BTC/USD", []*Position{{data: PositionData{Symbol: "BTC/USD", Qty: 0.01}}}) | |
| desk.positions.Store("SOL/USD", []*Position{{data: PositionData{Symbol: "SOL/USD", Qty: 3.5}}}) |
Comment on lines
+89
to
+90
| time.Sleep(10 * time.Millisecond) | ||
| cancel() |
There was a problem hiding this comment.
Flaky Test Pattern with time.Sleep
Using time.Sleep(10 * time.Millisecond) to wait for asynchronous goroutines to process messages is a flaky test anti-pattern. If the test environment is under heavy load, 10ms might not be enough for desk.Run() to process all three messages, leading to intermittent test failures.
Recommendation
Instead of sleeping, wait for the expected number of frames (3) to be received on the desk.UIForward channel before calling cancel().
Comment on lines
+119
to
+148
| positions := make([]PositionData, 0) | ||
|
|
||
| desk.positions.Range(func(_ any, value any) bool { | ||
| for _, position := range value.([]*Position) { | ||
| positions = append(positions, position.Data()) | ||
| } | ||
|
|
||
| return true | ||
| }) | ||
|
|
||
| out := datura.Map[any]{} | ||
|
|
||
| if desk.balance != nil { | ||
| out["balance"] = desk.balance | ||
| } | ||
|
|
||
| if len(desk.executions) > 0 { | ||
| out["executions"] = desk.executions | ||
| } | ||
|
|
||
| if desk.orders != nil { | ||
| out["orders"] = desk.orders | ||
| } | ||
|
|
||
| if len(positions) > 0 { | ||
| out["positions"] = positions | ||
| } | ||
|
|
||
| desk.UIForward <- out.Marshal() | ||
| } |
There was a problem hiding this comment.
Inefficient UI Updates on Every Message
Currently, on every single message received on any channel (including high-frequency ticker updates), the entire state is marshaled and sent to the UI. This will cause massive CPU usage and allocation churn under live market conditions.
Recommendation
Throttle or debounce UI updates (e.g., using a time.Ticker to send updates at a fixed interval like 100ms or 1s) instead of sending on every single message.
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.
No description provided.