fix(orders): close(id) closes logical unclosed qty, not physical lots (grid bots)

include/pineforge/engine.hpp

@@ -267,6 +267,23 @@ class BacktestEngine {
     int position_entry_count_ = 0;  // number of entries in current direction (for pyramiding)
     int position_open_bar_ = -1;    // bar_index_ when position was opened (for exit delay)
     std::vector<PyramidEntry> pyramid_entries_;  // individual entries for trade reporting
+    // Per-entry-id UNCLOSED quantity ledger, used ONLY by strategy.close(id)
+    // under the default FIFO close-entries rule to decide how much to close.
+    //
+    // TradingView's strategy.close(id) closes the quantity of the entries
+    // tagged `id` that have NOT already been targeted by a prior close(id) —
+    // it does NOT re-sum the physical open lots. That distinction is
+    // invisible when each id maps to one lot (the common case), but it is
+    // load-bearing for strategies that re-use one entry id across sequential
+    // buy/sell cycles (grid bots): there, the FIFO trade-record drain removes
+    // the OLDEST physical lot — which may carry a different id — leaving the
+    // id-tagged lot physically present. Summing physical lots would then
+    // double-count it on the next close(id) and over-close. This ledger
+    // tracks "entered qty for id minus already-closed-by-close(id) qty for
+    // id", so close(id) closes exactly the right amount (the whole position
+    // for a same-id DCA pyramid; one slot for a grid cycle). It is never read
+    // under the ANY rule (which closes id-matched physical lots directly).
+    std::unordered_map<std::string, double> id_unclosed_qty_;
 
     // --- Strategy parameters (set from strategy() declaration) ---
     double initial_capital_ = 1000000.0;

src/engine_fills.cpp

@@ -811,7 +811,9 @@ void BacktestEngine::apply_raw_order_fill(PendingOrder& order, double fill_price
         position_open_bar_ = bar_index_;
         trail_best_price_ = fill_price;
         pyramid_entries_.clear();
+        id_unclosed_qty_.clear();
         pyramid_entries_.push_back({fill_price, current_bar_.timestamp, qty, order.id, bar_index_});
+        id_unclosed_qty_[order.id] += qty;
         if (!std::isnan(order.stop_price) || !std::isnan(order.limit_price)) {
             set_entry_fill_excursion_masks(pyramid_entries_.back(), current_bar_, fill_price);
         }
@@ -860,6 +862,7 @@ void BacktestEngine::apply_raw_order_fill(PendingOrder& order, double fill_price
             position_entry_count_++;
             trail_best_price_ = fill_price;
             pyramid_entries_.push_back({fill_price, current_bar_.timestamp, new_qty, order.id, bar_index_});
+            id_unclosed_qty_[order.id] += new_qty;
             if (is_priced_entry) {
                 set_entry_fill_excursion_masks(pyramid_entries_.back(), current_bar_, fill_price);
             }

src/engine_orders.cpp

@@ -450,6 +450,7 @@ void BacktestEngine::reset_position_state_to_flat() {
     position_open_bar_ = -1;
     trail_best_price_ = std::numeric_limits<double>::quiet_NaN();
     pyramid_entries_.clear();
+    id_unclosed_qty_.clear();
     consumed_partial_exit_ids_.clear();
 }
 
@@ -488,8 +489,10 @@ void BacktestEngine::open_fresh_position(PositionSide requested, double fill_pri
     position_open_bar_ = bar_index_;
     trail_best_price_ = fill_price;
     pyramid_entries_.clear();
+    id_unclosed_qty_.clear();
     consumed_partial_exit_ids_.clear();
     pyramid_entries_.push_back({fill_price, current_bar_.timestamp, qty, id, bar_index_});
+    id_unclosed_qty_[id] += qty;
 }
 
 
@@ -644,6 +647,7 @@ void BacktestEngine::add_to_pyramid_market(const std::string& id, bool is_long,
     position_entry_count_++;
     trail_best_price_ = fill_price;
     pyramid_entries_.push_back({fill_price, current_bar_.timestamp, new_qty, id, bar_index_});
+    id_unclosed_qty_[id] += new_qty;
 }
 
 

src/engine_strategy_commands.cpp

@@ -452,6 +452,44 @@ bool BacktestEngine::compute_close_target_qty(const std::string& id,
                                               double& matching_qty_out,
                                               double& qty_to_close_out,
                                               bool& all_entries_match_out) {
+    const double eps0 = kQtyEpsilon;
+    // Default FIFO close-entries rule: strategy.close(id) with no explicit
+    // qty/qty_percent closes the UNCLOSED quantity tagged `id`
+    // (id_unclosed_qty_) and FIFO-attributes the resulting trade records to
+    // the OLDEST open entries (handled downstream by the plain FIFO drain).
+    //
+    // This is what TradingView does: close(id) closes the quantity entered
+    // under `id` and not yet targeted by a prior close(id) — it does NOT
+    // re-sum the physical open lots carrying that id. The two agree when each
+    // id maps to a single open lot. They diverge for grid bots that re-use
+    // one entry id across sequential buy/sell cycles: the FIFO trade-record
+    // drain removes the oldest lot (often a DIFFERENT id), so the id-tagged
+    // lot stays physically open even though a prior close(id) already
+    // accounted for it. Summing physical lots then double-closes it (engine
+    // over-closes 2x), while a TP whose id-lot was drained away by an earlier
+    // close would find no physical match and be skipped (engine under-closes)
+    // — both fixed here by consulting the logical ledger instead.
+    //
+    // The ANY rule keeps the physical id-matched path (closes_any_qty).
+    if (!close_entries_rule_any_ && !id.empty()
+        && std::isnan(qty) && std::isnan(qty_percent)) {
+        auto it = id_unclosed_qty_.find(id);
+        double unclosed = (it != id_unclosed_qty_.end()) ? it->second : 0.0;
+        double target = std::min(unclosed, position_qty_);
+        all_entries_match_out = false;  // FIFO drain may span lots of other ids
+        if (target <= eps0) {
+            return false;
+        }
+        matching_qty_out = target;
+        qty_to_close_out = target;
+        // Consume the id's unclosed ledger now that the close commits.
+        it->second -= target;
+        if (it->second <= eps0) {
+            id_unclosed_qty_.erase(it);
+        }
+        return true;
+    }
+
     bool has_matching_entry = id.empty();
     all_entries_match_out = id.empty() ? true : !pyramid_entries_.empty();
     matching_qty_out = id.empty() ? position_qty_ : 0.0;

tests/test_integration.cpp

@@ -3771,6 +3771,60 @@ static void test_strategy_close_pooc_fifo_only_closes_requested_leg_size() {
     CHECK(strat.get_open_entry_id() == "Buy2");
 }
 
+// Grid-bot pattern: the SAME entry id is re-used across sequential
+// buy/close cycles. Under the default FIFO close-entries rule, the trade
+// record drains the OLDEST physical lot (a different id), so the id-tagged
+// lot stays physically open after its close. A later re-entry of that id then
+// leaves TWO physical lots carrying it, while only ONE is logically unclosed.
+// strategy.close(id) must close ONE slot (the logical/unclosed quantity), not
+// the physical sum of both lots — otherwise it over-closes 2x (the bug this
+// guards). Mirrors the 3Commas grid-bot corpus strategies.
+static void test_strategy_close_reused_id_closes_one_logical_slot() {
+    std::printf("test_strategy_close_reused_id_closes_one_logical_slot\n");
+
+    class Strat : public BacktestEngine {
+    public:
+        Strat() {
+            initial_capital_ = 1'000'000;
+            default_qty_type_ = QtyType::FIXED;
+            default_qty_value_ = 1.0;
+            commission_value_ = 0.0;
+            slippage_ = 0;
+            process_orders_on_close_ = true;
+            pyramiding_ = 10;
+        }
+        void on_bar(const Bar&) override {
+            switch (bar_index_) {
+                case 0: strategy_entry("A", true); break;  // older, different id
+                case 1: strategy_entry("L", true); break;  // L lot #1
+                case 2: strategy_close("L");        break;  // drains A (FIFO), L#1 stays
+                case 3: strategy_entry("L", true); break;  // L lot #2 (re-use id)
+                case 4: strategy_close("L");        break;  // must close ONE slot, not two
+                default: break;
+            }
+        }
+        double pos() const { return signed_position_size(); }
+        int open_lots() const { return static_cast<int>(pyramid_entries_.size()); }
+    };
+
+    Strat strat;
+    Bar bars[] = {
+        {100.0, 101.0,  99.0, 100.0, 50,   900'000},
+        { 90.0,  91.0,  89.0,  90.0, 50, 1'800'000},
+        {110.0, 111.0, 109.0, 110.0, 50, 2'700'000},
+        { 80.0,  81.0,  79.0,  80.0, 50, 3'600'000},
+        {120.0, 121.0, 119.0, 120.0, 50, 4'500'000},
+    };
+    strat.run(bars, 5);
+
+    // Two close("L") calls each close exactly one unit: 2 closed trades, and
+    // one L lot (qty 1) remains open. The pre-fix sum-of-id behaviour closed
+    // both physical L lots on bar 4 (3 trades, flat) — that is the regression.
+    CHECK(strat.trade_count() == 2);
+    CHECK(near(strat.pos(), 1.0, 1e-9));
+    CHECK(strat.open_lots() == 1);
+}
+
 static void test_strategy_close_pooc_sets_exit_comment() {
     std::printf("test_strategy_close_pooc_sets_exit_comment\n");
 
@@ -4081,6 +4135,7 @@ int main() {
     test_strategy_close_pooc_partial_close_keeps_other_exit_bracket();
     test_strategy_close_fifo_only_closes_requested_leg_size();
     test_strategy_close_pooc_fifo_only_closes_requested_leg_size();
+    test_strategy_close_reused_id_closes_one_logical_slot();
     test_strategy_close_pooc_sets_exit_comment();
     test_strategy_close_qty_percent_reduces_position();
     test_strategy_close_qty_reduces_position();