10 create end to end trade management logic by mccaffers · Pull Request #14 · mccaffers/backtesting-engine-cpp

mccaffers · 2026-04-12T07:57:13Z

No description provided.

Copilot

Pull request overview

Adds an initial end-to-end “run” path for pulling tick data from QuestDB, parsing a Base64 strategy config, and iterating through streamed ticks (with supporting model/API updates).

Changes:

Introduces Base64→JSON configuration parsing and a new Operations::run() tick-processing entry point.
Reworks DB access to build symbol-union queries and parse results into an updated PriceData model that includes ask/bid/timestamp/symbol.
Updates CMake/OpenMP linkage and scripts/docs to support running the new flow.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 20 comments.

Show a summary per file

File	Description
source/utilities/jsonParser.cpp	New Base64 JSON configuration parser used by CLI.
source/sqlManager.cpp	Builds multi-symbol query and calls DB streaming API.
source/operations.cpp	Adds tick loop/printing “operations” runner.
source/main.cpp	Wires config parsing + tick streaming + operations run into CLI.
source/databaseConnection.cpp	Adds faster timestamp parsing and new `streamQuery()` result parsing.
source/CMakeLists.txt	Adds a duplicate CMake build file for the `source/` subtree.
scripts/run.sh	Adds build-failure guard and execution timing.
scripts/build_dep.sh	Adds a helper script to build libpqxx dependency.
include/sqlManager.hpp	Updates SqlManager API to `streamPriceData`/symbol list.
include/operations.hpp	Declares new `Operations` runner.
include/models/priceData.hpp	Updates tick model fields and constructors.
include/databaseConnection.hpp	Adds `streamQuery()` declaration (keeps `executeQuery()` declaration).
documents/questdb.md	Adds a short note on starting QuestDB (macOS).
CMakeLists.txt	Adds OpenMP linking for the main build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-12T08:03:51Z

+    std::string query;
+    for (size_t i = 0; i < symbols.size(); ++i) {
+        if (i > 0) {
+            query += " UNION ALL ";
+        }
+        query += "SELECT '" + symbols[i] + "' as symbol, * FROM '" + symbols[i] + "' WHERE timestamp >= dateadd('M', -" + std::to_string(LAST_MONTHS) + ", now())";
+    }


The generated SQL uses single quotes around the table name (FROM 'EURUSD'), which in SQL denotes a string literal, not an identifier. This will produce invalid SQL (or query the wrong thing). Use an unquoted identifier or proper identifier quoting (typically double quotes), and ensure symbols are validated/escaped to avoid SQL injection via symbols[i].

Copilot · 2026-04-12T08:03:51Z

+std::string SqlManager::getBaseQuery(const std::vector<std::string>& symbols, int LAST_MONTHS) {
+    if (symbols.empty()) {
+        return "";
+    }


getBaseQuery returns an empty string when symbols is empty, but streamPriceData will still execute the query string it gets back. Returning an empty query can lead to runtime SQL errors; consider returning an empty result early in streamPriceData (or throwing) when symbols is empty.

Copilot · 2026-04-12T08:03:52Z

+        struct tm tm = {};
+        if (localtime_r(&time_t, &tm) == nullptr) {
+            std::cerr << "Error: failed to convert timestamp" << std::endl;
+        }
+        char buffer[20];
+        std::strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", &tm);
+        auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(tick.timestamp.time_since_epoch()) % 1000;


If localtime_r fails, the code logs an error but then continues to call std::strftime using an uninitialized tm value. This can print garbage timestamps. Consider continue; (or otherwise handling the error) after the failure so the formatting code is skipped.

Copilot · 2026-04-12T08:03:54Z

+if(APPLE)
+    set(OpenMP_C_FLAGS "-Xclang -fopenmp")
+    set(OpenMP_CXX_FLAGS "-Xclang -fopenmp")
+    set(OpenMP_C_LIB_NAMES "omp")
+    set(OpenMP_CXX_LIB_NAMES "omp")
+    set(OpenMP_omp_LIBRARY /opt/homebrew/opt/libomp/lib/libomp.dylib)
+    find_package(OpenMP REQUIRED)
+    target_include_directories(BacktestingEngineLib PRIVATE /opt/homebrew/opt/libomp/include)
+endif()
+
+target_link_libraries(BacktestingEngineLib PUBLIC pqxx OpenMP::OpenMP_CXX)
+


This CMake file links OpenMP::OpenMP_CXX unconditionally, but find_package(OpenMP REQUIRED) is only called inside if(APPLE). On non-Apple platforms this will fail because the imported target won’t exist. Consider calling find_package(OpenMP REQUIRED) for all platforms (keeping only the Apple-specific hints in the if(APPLE) block) or conditionally linking OpenMP when found.

Copilot · 2026-04-12T08:03:54Z

+cd ./external/libpqxx
+
+mkdir -p build


This script is missing a shebang (e.g., #!/bin/bash or #!/usr/bin/env bash). As-is, running it directly (instead of sh scripts/build_dep.sh) may fail depending on permissions/default shell. Add a shebang and consider set -euo pipefail so dependency builds fail fast.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…com:mccaffers/backtesting-engine-cpp into 10-create-end-to-end-trade-management-logic

… in it's own child process

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 15 comments.

Comments suppressed due to low confidence (2)

source/utilities/jsonParser.cpp:19

This function writes the decoded JSON and RUN_ID to stdout unconditionally. That can leak potentially sensitive configuration data and also makes the parser noisy for library callers/tests; consider removing these prints or gating them behind an explicit debug/logging facility.
source/utilities/jsonParser.cpp:31
This function now returns a Configuration, but j.get<trading_definitions::Configuration>() will throw if parsing failed (the earlier parse error is only logged). Consider making the error handling explicit (rethrow/return an error type) so callers don’t proceed with an invalid/partial configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T20:25:03Z

+    auto tradeManager = TradeManager::getInstance();
+


tradeManager is retrieved but never used (all example calls are commented). This will trigger unused-variable warnings and adds dead code. Either remove it for now or implement the intended trade-management flow so Operations::run has a concrete effect.

Copilot · 2026-04-27T20:25:03Z

+        // (void)tick;
+
+        // print first tick
+        auto time_t = std::chrono::system_clock::to_time_t(tick.timestamp);
+        struct tm tm = {};
+        if (localtime_r(&time_t, &tm) == nullptr) {
+            std::cerr << "Error: failed to convert timestamp" << std::endl;
+        }
+        char buffer[20];
+        std::strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", &tm);
+        auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(tick.timestamp.time_since_epoch()) % 1000;
+        // printf("symbol=%s, ask=%.4f, bid=%.4f timestamp=%s.%03lld\n", tick.symbol.c_str(), tick.ask, tick.bid, buffer,
+        //        static_cast<long long>(ms.count()));


Inside the tick loop, expensive timestamp formatting work is done for every tick even though the output is commented out. This will significantly slow large backtests; remove this work or gate it behind a debug flag / only sample a small number of ticks.

Suggested change

// (void)tick;

// print first tick

auto time_t = std::chrono::system_clock::to_time_t(tick.timestamp);

struct tm tm = {};

if (localtime_r(&time_t, &tm) == nullptr) {

std::cerr << "Error: failed to convert timestamp" << std::endl;

}

char buffer[20];

std::strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", &tm);

auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(tick.timestamp.time_since_epoch()) % 1000;

// printf("symbol=%s, ask=%.4f, bid=%.4f timestamp=%s.%03lld\n", tick.symbol.c_str(), tick.ask, tick.bid, buffer,

// static_cast<long long>(ms.count()));

(void)tick;

Copilot · 2026-04-27T20:25:03Z

+        auto time_t = std::chrono::system_clock::to_time_t(tick.timestamp);
+        struct tm tm = {};
+        if (localtime_r(&time_t, &tm) == nullptr) {
+            std::cerr << "Error: failed to convert timestamp" << std::endl;


If localtime_r fails, the code logs an error but still calls strftime on tm, which can produce misleading output; consider continue on failure. Also rename time_t to avoid shadowing the time_t type name.

Suggested change

auto time_t = std::chrono::system_clock::to_time_t(tick.timestamp);

struct tm tm = {};

if (localtime_r(&time_t, &tm) == nullptr) {

std::cerr << "Error: failed to convert timestamp" << std::endl;

auto tick_time = std::chrono::system_clock::to_time_t(tick.timestamp);

struct tm tm = {};

if (localtime_r(&tick_time, &tm) == nullptr) {

std::cerr << "Error: failed to convert timestamp" << std::endl;

continue;

Copilot · 2026-04-27T20:25:04Z

+# Disable warningsfor external libraries
+set(PREV_CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -w")
+elseif(MSVC)


This CMakeLists globally appends -w//w, which disables warnings for all targets (including your own code) and can mask real issues. Prefer limiting warning suppression to third-party targets only (e.g., via per-target compile options or SYSTEM includes).

Copilot · 2026-04-27T20:25:04Z

      - name: Build C++ Libraries
        run: >
-            bash ./.github/workflows/build.sh
+            sh ./scripts/build.sh
      - name: Install Python 3.12 for gcovr


The Ubuntu Sonar job runs sh ./scripts/build.sh, but that script relies on macOS tooling (Homebrew + xcrun) and is expected to fail on Ubuntu. Replace this with Linux-compatible build steps (apt deps + cmake) or make scripts/build.sh OS-aware.

Copilot · 2026-04-27T20:25:05Z

+# Collect all .cpp files in the src directory
+file(GLOB_RECURSE SOURCES "source/*.cpp")


file(GLOB_RECURSE SOURCES "source/*.cpp") inside source/CMakeLists.txt will look for source/source/*.cpp relative to this CMakeLists, which is likely not intended and can produce an empty library. Use paths relative to the correct root (e.g., "*.cpp" if this file is the project root) or switch to explicit target sources to avoid fragile globs.

Suggested change

# Collect all .cpp files in the src directory

file(GLOB_RECURSE SOURCES "source/*.cpp")

# Collect all .cpp files in the current source directory

file(GLOB_RECURSE SOURCES "*.cpp")

Copilot · 2026-04-27T20:25:06Z

+        query += "SELECT '" + symbols[i] + "' as symbol, * FROM '" + symbols[i] + "' WHERE timestamp >= dateadd('M', -" + std::to_string(LAST_MONTHS) + ", now())";
+    }


symbols[i] is concatenated directly into the SQL string (including the FROM clause), so a malicious or malformed symbol can inject SQL or break the query. Validate symbols against an allowlist and/or implement safe identifier escaping rather than raw string concatenation.

Copilot · 2026-04-27T20:25:06Z

+    // Cache timegm per date — tick data is time-ordered so date changes rarely
+    static char cachedDate[11] = {};
+    static time_t cachedEpoch = 0;
+    if (std::memcmp(ts, cachedDate, 10) != 0) {
+        std::memcpy(cachedDate, ts, 10);


fastParseTimestamp uses shared mutable function-local static cache state. If streamQuery is called concurrently, this is a data race (UB). Consider making the cache thread_local or moving it into a caller-owned object.

Copilot · 2026-04-27T20:25:06Z

+cmake .. \
+  -DCMAKE_CXX_STANDARD=20 \
+  -DCMAKE_BUILD_TYPE=Release \
+  -DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path) \
+  -DSKIP_BUILD_TEST=ON



This script always passes -DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path) to CMake; xcrun won’t exist on Linux, and this script is invoked from the Ubuntu Sonar job. Guard the xcrun call or only pass CMAKE_OSX_SYSROOT on Apple platforms.

Suggested change

cmake .. \

-DCMAKE_CXX_STANDARD=20 \

-DCMAKE_BUILD_TYPE=Release \

-DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path) \

-DSKIP_BUILD_TEST=ON

CMAKE_ARGS=".. \

-DCMAKE_CXX_STANDARD=20 \

-DCMAKE_BUILD_TYPE=Release \

-DSKIP_BUILD_TEST=ON"

if [ "$(uname)" = "Darwin" ] && command -v xcrun &>/dev/null; then

CMAKE_ARGS="$CMAKE_ARGS -DCMAKE_OSX_SYSROOT=$(xcrun --show-sdk-path)"

fi

cmake $CMAKE_ARGS

Copilot · 2026-04-27T20:25:06Z

+  for (std::string token; std::getline(ss, token, ',');) {
+    symbols.push_back(token);
+  }
+  std::vector<PriceData> ticks = SqlManager::streamPriceData(db, symbols, 3);


LAST_MONTHS is provided by the decoded configuration but this call hard-codes 3, so the CLI config is ignored. Use config.LAST_MONTHS (with validation) so backtests match the provided config.

Suggested change

std::vector<PriceData> ticks = SqlManager::streamPriceData(db, symbols, 3);

// Use the configured lookback window for backtesting and validate it

int lastMonths = config.LAST_MONTHS;

if (lastMonths <= 0) {

std::cerr << "Invalid LAST_MONTHS value in configuration: " << lastMonths

<< ". Expected a positive integer." << std::endl;

return 1;

}

std::vector<PriceData> ticks = SqlManager::streamPriceData(db, symbols, lastMonths);

sonarqubecloud · 2026-04-29T21:19:49Z

Quality Gate failed

Failed conditions
2 Security Hotspots
18.2% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

feat: add QuestDB tick streaming with multi-symbol support and refactored trade flow * Integrate QuestDB tick data streaming with multi-symbol loop support * Extract strategy logic for querying QuestDB into dedicated module * Refactor tick flow, operations flow, and trade management operations * Fix run script shell scoping issue (build script running in child process) * Update and stabilise GitHub Actions workflow (macOS target, build & test optimisation) * Update CMakeLists.txt for updated source structure

mccaffers added 3 commits April 11, 2026 11:28

Added QuestDB tick streaming

05435e4

Refactoring tick flow

16fb2c3

Looping around tick data from multiple symbols

3ba4cec

mccaffers self-assigned this Apr 12, 2026

Copilot AI review requested due to automatic review settings April 12, 2026 07:57

mccaffers linked an issue Apr 12, 2026 that may be closed by this pull request

Create end to end trade management logic #10

Closed

Copilot started reviewing on behalf of mccaffers April 12, 2026 07:57 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

mccaffers and others added 8 commits April 12, 2026 09:37

Update source/CMakeLists.txt

46e720d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestions from code review

61309bd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Refactoring

48a9a99

Merge branch '10-create-end-to-end-trade-management-logic' of github.…

fab3106

…com:mccaffers/backtesting-engine-cpp into 10-create-end-to-end-trade-management-logic

fix run script, shell scopping behaviour was putting the build script…

85be15b

… in it's own child process

Update build os to target macos-latest

cbc033d

Update build os to target macos-latest

bb24f06

Updating github workflow os

27f82f1

mccaffers assigned Copilot Apr 12, 2026

mccaffers added 7 commits April 19, 2026 18:01

Refactoring build pipeline

e4e8e13

fixing workflow

0b6d40e

Updating github workflow os

8a0a710

Updating github workflow os

801c846

Updating github workflow os

b57db36

Updating github workflow os

9a60a27

Updating main flow, extracting the strategy to query questdB

edb253f

mccaffers requested a review from Copilot April 27, 2026 20:14

mccaffers added the enhancement New feature or request label Apr 27, 2026

mccaffers added this to @mccaffers Backtesting Project Tracking Apr 27, 2026

mccaffers moved this to In review in @mccaffers Backtesting Project Tracking Apr 27, 2026

mccaffers moved this from In review to In progress in @mccaffers Backtesting Project Tracking Apr 27, 2026

Copilot started reviewing on behalf of mccaffers April 27, 2026 20:17 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Updating trade management operations

657b5e2

mccaffers removed this from @mccaffers Backtesting Project Tracking Apr 28, 2026

mccaffers unassigned Copilot Apr 29, 2026

Refactoring operations flow

87bc95e

mccaffers added 5 commits May 2, 2026 10:52

updated workflows and optimised build & test

83d838f

updated workflow references

e1ddeaa

updated workflow references

28ce018

updated workflow references

cb840fd

updated workflow references

3f52047

mccaffers merged commit 617c263 into main May 2, 2026
1 check passed

		# Collect all .cpp files in the src directory
		file(GLOB_RECURSE SOURCES "source/*.cpp")

		query += "SELECT '" + symbols[i] + "' as symbol, * FROM '" + symbols[i] + "' WHERE timestamp >= dateadd('M', -" + std::to_string(LAST_MONTHS) + ", now())";
		}

-  std::vector<PriceData> ticks = SqlManager::streamPriceData(db, symbols, 3);
+  // Use the configured lookback window for backtesting and validate it
+  int lastMonths = config.LAST_MONTHS;
+  if (lastMonths <= 0) {
+    std::cerr << "Invalid LAST_MONTHS value in configuration: " << lastMonths
+              << ". Expected a positive integer." << std::endl;
+    return 1;
+  }
+  std::vector<PriceData> ticks = SqlManager::streamPriceData(db, symbols, lastMonths);

Conversation

mccaffers commented Apr 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 29, 2026

Quality Gate failed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants