Fix duplicate holdings (#187)

- Fix duplicate holdings, adding unit test

Author: Martin-Molinero

QuantConnect.InteractiveBrokersBrokerage.Tests/InteractiveBrokersBrokerageAdditionalTests.cs

@@ -44,6 +44,7 @@
 using Order = QuantConnect.Orders.Order;
 using IB = QuantConnect.Brokerages.InteractiveBrokers.Client;
 using QuantConnect.Securities.IndexOption;
+using static QuantConnect.Brokerages.InteractiveBrokers.InteractiveBrokersAccountData;
 
 namespace QuantConnect.Tests.Brokerages.InteractiveBrokers
 {
@@ -57,19 +58,53 @@ public class InteractiveBrokersBrokerageAdditionalTests
 
         private InteractiveBrokersSymbolMapper _symbolMapper = new InteractiveBrokersSymbolMapper(Composer.Instance.GetPart<IMapFileProvider>());
 
-        [SetUp]
+        [OneTimeSetUp]
         public void Setup()
         {
             Log.LogHandler = new NUnitLogHandler();
             PythonInitializer.Initialize();
         }
 
-        [TearDown]
+        [OneTimeTearDown]
         public void TearDown()
         {
             PythonInitializer.Shutdown();
         }
 
+        [TestCase(-500, 623.794, 100, 622.181, -400, 624.19725, "B")]
+        [TestCase(100, 210.101, -200, 210.044, -100, 209.987, "B")]
+        [TestCase(-500, 623.794, -500, 623.794, -500, 623.794, "A")] // double A is ignored
+        public void MergeHoldingMergesOppositeSignedAAPLPositions(decimal holdingPositionQuantity, decimal holdingAvgPrice, decimal incomePositionQuantity,
+            decimal incomeAvgPrice, decimal expectedNewPositionQuantity, decimal expectedAvgPrice, string incomingAccount)
+        {
+            var aapl = Symbols.AAPL;
+
+            var holdings = new Dictionary<Symbol, MergedHoldings>();
+            holdings[aapl] = new();
+
+            holdings[aapl].Merge(new Holding
+            {
+                Symbol = aapl,
+                Quantity = holdingPositionQuantity,
+                AveragePrice = holdingAvgPrice
+            }, "A");
+
+            var incoming = new Holding
+            {
+                Symbol = aapl,
+                Quantity = incomePositionQuantity,
+                AveragePrice = incomeAvgPrice
+            };
+
+            InteractiveBrokersBrokerage.MergeHolding(holdings, incoming, incomingAccount);
+
+            var merged = holdings[aapl];
+
+            Assert.AreEqual(expectedNewPositionQuantity, merged.Holding.Quantity);
+            Assert.AreEqual(expectedAvgPrice, merged.Holding.AveragePrice);
+            Assert.AreEqual(aapl, merged.Holding.Symbol);
+        }
+
         [Test]
         public void LoginFailOnInvalidUserName()
         {

QuantConnect.InteractiveBrokersBrokerage.Tests/InteractiveBrokersBrokerageTests.cs

@@ -19,7 +19,6 @@
 using QuantConnect.Configuration;
 using QuantConnect.Data;
 using QuantConnect.Data.Market;
-using QuantConnect.Lean.Engine.DataFeeds;
 using QuantConnect.Logging;
 using QuantConnect.Orders;
 using QuantConnect.Securities;
@@ -733,37 +732,6 @@ public void PlaceMarketOrderWithDifferentFAGroupFilter()
             Assert.IsFalse(holdingsGroup2.Any(h => h.Symbol == Symbols.AAPL), "TestGroup2 should not hold AAPL.");
         }
 
-        [TestCase(-500, 623.794, 100, 622.181, -400, 624.19725)]
-        [TestCase(100, 210.101, -200, 210.044, -100, 209.987)]
-        public void MergeHoldingMergesOppositeSignedAAPLPositions(decimal holdingPositionQuantity, decimal holdingAvgPrice, decimal incomePositionQuantity, decimal incomeAvgPrice, decimal expectedNewPositionQuantity, decimal expectedAvgPrice)
-        {
-            var aapl = Symbols.AAPL;
-
-            var holdings = new Dictionary<string, Holding>
-            {
-                ["AAPL"] = new Holding
-                {
-                    Symbol = aapl,
-                    Quantity = holdingPositionQuantity,
-                    AveragePrice = holdingAvgPrice
-                }
-            };
-
-            var incoming = new Holding
-            {
-                Symbol = aapl,
-                Quantity = incomePositionQuantity,
-                AveragePrice = incomeAvgPrice
-            };
-
-            InteractiveBrokersBrokerage.MergeHolding(holdings, incoming);
-
-            var merged = holdings["AAPL"];
-
-            Assert.AreEqual(expectedNewPositionQuantity, merged.Quantity);
-            Assert.AreEqual(expectedAvgPrice, merged.AveragePrice);
-        }
-
         private List<Holding> ExecuteMarketOrderForGroup(string faGroup, Symbol symbol, int quantity)
         {
             Config.Set("ib-financial-advisors-group-filter", faGroup);

QuantConnect.InteractiveBrokersBrokerage/InteractiveBrokersAccountData.cs

@@ -13,6 +13,7 @@
  * limitations under the License.
 */
 
+using System.Collections.Generic;
 using System.Collections.Concurrent;
 
 namespace QuantConnect.Brokerages.InteractiveBrokers
@@ -35,7 +36,7 @@ public class InteractiveBrokersAccountData
         /// <summary>
         /// The account holdings indexed by symbol
         /// </summary>
-        public ConcurrentDictionary<string, Holding> AccountHoldings { get; } = new ConcurrentDictionary<string, Holding>();
+        public ConcurrentDictionary<Symbol, MergedHoldings> AccountHoldings { get; } = new ();
 
         /// <summary>
         /// Clears this instance of <see cref="InteractiveBrokersAccountData"/>
@@ -46,5 +47,36 @@ public void Clear()
             CashBalances.Clear();
             AccountHoldings.Clear();
         }
+
+        public class MergedHoldings
+        {
+            private readonly HashSet<string> _accounts = new();
+            public Holding Holding { get; set; } = new();
+            public void Merge(Holding incoming, string account)
+            {
+                // only merge in holdings of multiple accounts, IB can send the same holdings for the same account multiple times
+                if (_accounts.Add(account))
+                {
+                    if (_accounts.Count == 1)
+                    {
+                        // shortcut
+                        Holding = incoming;
+                        return;
+                    }
+
+                    // Merge with existing holding: add quantities and recalculate average price
+                    // This happens when holdings are reported separately (e.g., across FA group accounts)
+                    var totalCost = (Holding.AveragePrice * Holding.Quantity) + (incoming.AveragePrice * incoming.Quantity);
+
+                    Holding.Quantity += incoming.Quantity;
+
+                    // Avoid division by zero when position is fully closed
+                    if (Holding.Quantity != 0)
+                    {
+                        Holding.AveragePrice = totalCost / Holding.Quantity;
+                    }
+                }
+            }
+        }
     }
 }

QuantConnect.InteractiveBrokersBrokerage/InteractiveBrokersBrokerage.cs

@@ -694,9 +694,9 @@ public override List<Holding> GetAccountHoldings()
             var utcNow = DateTime.UtcNow;
             var holdings = new List<Holding>();
 
-            foreach (var kvp in _accountData.AccountHoldings)
+            foreach (var kvp in _accountData.AccountHoldings.Values)
             {
-                var holding = ObjectActivator.Clone(kvp.Value);
+                var holding = ObjectActivator.Clone(kvp.Holding);
 
                 if (holding.Quantity != 0)
                 {
@@ -2673,7 +2673,7 @@ private void HandlePortfolioUpdates(object sender, IB.UpdatePortfolioEventArgs e
                 if (_loadExistingHoldings)
                 {
                     var holding = CreateHolding(e);
-                    MergeHolding(_accountData.AccountHoldings, holding);
+                    MergeHolding(_accountData.AccountHoldings, holding, e.AccountName);
                 }
             }
             catch (Exception exception)
@@ -2718,30 +2718,15 @@ private void HandlePortfolioUpdates(object sender, IB.UpdatePortfolioEventArgs e
         /// This method is used when multiple updates for the same symbol can be received,
         /// such as when FA (Financial Advisor) group accounts return positions for multiple sub-accounts.
         /// </remarks>
-        internal static void MergeHolding(IDictionary<string, Holding> holdings, Holding incoming)
+        internal static void MergeHolding(IDictionary<Symbol, InteractiveBrokersAccountData.MergedHoldings> holdings, Holding incoming, string accountName)
         {
-            var key = incoming.Symbol.Value;
-
             lock (holdings)
             {
-                if (holdings.TryGetValue(key, out var existing))
-                {
-                    // Merge with existing holding: add quantities and recalculate average price
-                    // This happens when holdings are reported separately (e.g., across FA group accounts)
-                    var totalCost = (existing.AveragePrice * existing.Quantity) + (incoming.AveragePrice * incoming.Quantity);
-
-                    existing.Quantity += incoming.Quantity;
-
-                    // Avoid division by zero when position is fully closed
-                    if (existing.Quantity != 0)
-                    {
-                        existing.AveragePrice = totalCost / existing.Quantity;
-                    }
-                }
-                else
+                if (!holdings.TryGetValue(incoming.Symbol, out var mergedHoldings))
                 {
-                    holdings[key] = incoming;
+                    holdings[incoming.Symbol] = mergedHoldings = new InteractiveBrokersAccountData.MergedHoldings();
                 }
+                mergedHoldings.Merge(incoming, accountName);
             }
         }