Improve matching of assets

apps/api/src/app/endpoints/ai/ai.service.spec.ts

@@ -788,6 +788,38 @@ describe('AiService', () => {
       });
     });
 
+    it('Gives an activity for a row whose asset matches a holding with the same ISIN with spaces and in lower case', async () => {
+      const row = createActivityRow({
+        accountName: null,
+        isin: 'ie00 bk5b qt80',
+        name: 'Vanguard FTSE All-World UCITS ETF USD Accumulating',
+        symbol: null
+      });
+
+      mockRowMatches({
+        assetProfilesOfSearch: [],
+        assets: [
+          { currency: 'USD', dataSource: 'YAHOO', id: 0, symbol: 'VWRA.SW' }
+        ]
+      });
+
+      mockHoldings();
+
+      await expect(
+        aiService.getActivitiesFromRows({
+          accounts,
+          userCurrency,
+          userId,
+          rows: [row]
+        })
+      ).resolves.toEqual({
+        activities: [
+          expect.objectContaining({ dataSource: 'YAHOO', symbol: 'VWRA.SW' })
+        ],
+        rowsWithoutMatch: []
+      });
+    });
+
     it('Gives an activity for a row whose asset matches a holding without an ISIN which no search gave', async () => {
       const row = createActivityRow({
         accountName: null,
@@ -950,7 +982,7 @@ describe('AiService', () => {
       });
     });
 
-    it('Gives a row whose asset match is a holding with a data source which requires Premium as a row without a match', async () => {
+    it('Passes no holding with a data source which requires Premium to the model and gives a row whose asset match is such a holding as a row without a match', async () => {
       jest.spyOn(Logger.prototype, 'warn').mockImplementation();
 
       jest.mocked(dataProviderService.isPremiumRequired).mockReturnValue(true);
@@ -988,9 +1020,13 @@ describe('AiService', () => {
         subscription,
         dataSource: DataSource.YAHOO
       });
+
+      const { prompt } = jest.mocked(generateText).mock.calls[0][0];
+
+      expect(JSON.parse(prompt as string)).toHaveProperty('holdings', []);
     });
 
-    it('Gives a row whose asset match is a holding with a data source which is not configured as a row without a match', async () => {
+    it('Passes no holding with a data source which is not configured to the model and gives a row whose asset match is such a holding as a row without a match', async () => {
       jest.spyOn(Logger.prototype, 'warn').mockImplementation();
 
       jest
@@ -1020,6 +1056,10 @@ describe('AiService', () => {
           rows: [row]
         })
       ).resolves.toEqual({ activities: [], rowsWithoutMatch: [row] });
+
+      const { prompt } = jest.mocked(generateText).mock.calls[0][0];
+
+      expect(JSON.parse(prompt as string)).toHaveProperty('holdings', []);
     });
 
     it('Gives a row whose asset match is a cash position of the holdings as a row without a match', async () => {
@@ -1229,7 +1269,7 @@ describe('AiService', () => {
             symbol: 'AAPL'
           }
         ],
-        holdings: '## Holdings'
+        holdings: []
       });
     });
 
@@ -1467,6 +1507,76 @@ describe('AiService', () => {
       });
     });
 
+    it('Passes the currency, the data source, the ISIN, the name and the symbol of each holding without the cash position to the model', async () => {
+      mockRowMatches({});
+
+      mockHoldings();
+
+      await aiService.getActivitiesFromRows({
+        accounts,
+        userCurrency,
+        userId,
+        rows: [createActivityRow()]
+      });
+
+      const { prompt } = jest.mocked(generateText).mock.calls[0][0];
+
+      expect(JSON.parse(prompt as string)).toHaveProperty('holdings', [
+        {
+          currency: 'USD',
+          dataSource: 'YAHOO',
+          isin: 'IE00BK5BQT80',
+          name: 'Vanguard FTSE All-World UCITS ETF',
+          symbol: 'VWRA.SW'
+        },
+        {
+          currency: 'USD',
+          dataSource: 'YAHOO',
+          isin: null,
+          name: 'Apple Inc.',
+          symbol: 'AAPL'
+        }
+      ]);
+    });
+
+    it('Restores an escaped pipe in a value of the holdings', async () => {
+      mockRowMatches({});
+
+      jest.mocked(mcpService.getPortfolio).mockResolvedValue({
+        content: [
+          {
+            text: [
+              '## Holdings',
+              '',
+              '| Name        | ISIN | Symbol | Data Source | Currency |',
+              '| :---------- | :--- | :----- | :---------- | :------- |',
+              '| Fund A \\| B |      | FAB    | YAHOO       | USD      |'
+            ].join('\n'),
+            type: 'text'
+          }
+        ]
+      });
+
+      await aiService.getActivitiesFromRows({
+        accounts,
+        userCurrency,
+        userId,
+        rows: [createActivityRow()]
+      });
+
+      const { prompt } = jest.mocked(generateText).mock.calls[0][0];
+
+      expect(JSON.parse(prompt as string)).toHaveProperty('holdings', [
+        {
+          currency: 'USD',
+          dataSource: 'YAHOO',
+          isin: null,
+          name: 'Fund A | B',
+          symbol: 'FAB'
+        }
+      ]);
+    });
+
     it('Does not get the holdings without an asset', async () => {
       mockRowMatches({});
 
@@ -1486,6 +1596,7 @@ describe('AiService', () => {
       });
 
       expect(mcpService.getPortfolio).not.toHaveBeenCalled();
+      expect(dataProviderService.getDataSources).not.toHaveBeenCalled();
 
       const { prompt } = jest.mocked(generateText).mock.calls[0][0];
 
@@ -1523,6 +1634,39 @@ describe('AiService', () => {
       );
     });
 
+    it('Passes no holdings to the model and continues if the data sources cannot be got', async () => {
+      jest.spyOn(Logger.prototype, 'warn').mockImplementation();
+
+      mockRowMatches({
+        assets: [
+          { currency: 'USD', dataSource: 'YAHOO', id: 0, symbol: 'AAPL' }
+        ]
+      });
+
+      mockHoldings();
+
+      jest
+        .mocked(dataProviderService.getDataSources)
+        .mockRejectedValueOnce(new Error('Database failed'));
+
+      const { activities } = await aiService.getActivitiesFromRows({
+        accounts,
+        userCurrency,
+        userId,
+        rows: [createActivityRow()]
+      });
+
+      expect(activities).toEqual([expect.objectContaining({ symbol: 'AAPL' })]);
+
+      const { prompt } = jest.mocked(generateText).mock.calls[0][0];
+
+      expect(JSON.parse(prompt as string)).not.toHaveProperty('holdings');
+
+      expect(Logger.prototype.warn).toHaveBeenCalledWith(
+        'Could not get the holdings: Database failed'
+      );
+    });
+
     it('Restricts the routing to endpoints with zero data retention, disables the reasoning and gives the model no tools', async () => {
       mockRowMatches({});
 

apps/api/src/app/endpoints/ai/ai.service.ts

@@ -476,12 +476,13 @@ export class AiService {
   }
 
   /**
-   * Gives the data source, the ISIN and the symbol of each holding of the
+   * Gives the currency, the data source, the ISIN, the name and the symbol of
-   * table of the holdings which has a data source and a symbol. A cash
+   * each holding of the table of the holdings which has a data source and a
-   * position has no data source, hence it is left out. The ISIN is empty if
+   * symbol. A cash position has no data source, hence it is left out. The ISIN
-   * the asset profile has none, which is the case for most stocks of Yahoo
+   * is null if the asset profile has none, which is the case for most stocks
-   * Finance. The columns are found by the header line. A line is split at
+   * of Yahoo Finance. The columns are found by the header line. A line is
-   * each pipe which is not escaped, as the renderer escapes a pipe in a value.
+   * split at each pipe which is not escaped, as the renderer escapes a pipe in
+   * a value, and the escaped pipes are restored.
    */
   private getHoldingsFromTable(holdings = '') {
     const [headerLine, , ...lines] = holdings.split('\n').filter((line) => {
@@ -497,7 +498,7 @@ export class AiService {
         .split(/(?<!\\)\|/)
         .slice(1, -1)
         .map((cell) => {
-          return cell.trim();
+          return cell.trim().replaceAll('\\|', '|');
         });
     };
 
@@ -508,8 +509,10 @@ export class AiService {
         const cells = getCells(line);
 
         return {
+          currency: cells[columnNames.indexOf('Currency')],
           dataSource: cells[columnNames.indexOf('Data Source')],
-          isin: cells[columnNames.indexOf('ISIN')],
+          isin: cells[columnNames.indexOf('ISIN')] || null,
+          name: cells[columnNames.indexOf('Name')],
           symbol: cells[columnNames.indexOf('Symbol')]
         };
       })
@@ -518,6 +521,42 @@ export class AiService {
       });
   }
 
+  /**
+   * Gives the holdings of the user with the tool get-portfolio of the server
+   * of the Model Context Protocol, without the holdings whose data source the
+   * import refuses for the user
+   */
+  private async getHoldingsToMatch({
+    subscription,
+    userId
+  }: {
+    subscription?: UserWithSettings['subscription'];
+    userId: string;
+  }) {
+    const [{ content }, dataSources] = await Promise.all([
+      this.mcpService.getPortfolio({ userId, withValues: false }),
+      this.dataProviderService.getDataSources()
+    ]);
+
+    return this.getHoldingsFromTable(content[0].text).filter(
+      ({ dataSource }) => {
+        return this.isValidDataSourceForImport({
+          dataSource,
+          dataSources,
+          subscription
+        });
+      }
+    );
+  }
+
+  /**
+   * Gives the ISIN without spaces and in upper case, as a file can show an
+   * ISIN in groups or in lower case
+   */
+  private getNormalizedIsin(isin?: string | null) {
+    return isin?.replaceAll(/\s/g, '').toUpperCase();
+  }
+
   /**
    * Gives the chat model of the property, with the general model as fallback
    */
@@ -564,20 +603,23 @@ export class AiService {
    * and not the model calls the tools search-asset-profiles, get-accounts and
    * get-portfolio of the server of the Model Context Protocol and passes their
    * results to the model, as some providers ignore a required tool call, for
-   * example if the output has a schema. The accounts and the holdings are
+   * example if the output has a schema. The accounts are given without
-   * given without values. The holdings let the model select an asset profile
+   * values, the holdings with their currency, data source, ISIN, name and
-   * which the user already has, also if no search gives it, for example as no
+   * symbol only. The holdings let the model select an asset profile which the
-   * data provider finds its ISIN. If get-accounts fails, the model gets no
+   * user already has, also if no search gives it, for example as no data
-   * accounts, so that only the account hints have no match. If get-portfolio
+   * provider finds its ISIN. A holding whose data source the import refuses
-   * fails, the model gets no holdings. The reasoning is disabled, as it can
+   * for the user is not given to the model, so that the model selects an
-   * add thousands of output tokens. An asset match is left out if neither the
+   * asset profile of the search instead and the whole import does not fail.
+   * If get-accounts fails, the model gets no accounts, so that only the
+   * account hints have no match. If get-portfolio or the data sources fail,
+   * the model gets no holdings. The reasoning is disabled, as it can add
+   * thousands of output tokens. An asset match is left out if neither the
    * search of its own asset nor the holdings gave its asset profile, or if its
    * holding has another ISIN than its own asset, as the import refuses a
    * symbol which the data source cannot resolve and as the model can mix up
-   * the ids of the assets. A holding without an ISIN is matched by the model
+   * the ids of the assets. The ISINs are compared without spaces and in upper
-   * by its name only, hence the code cannot check it against the asset. A
+   * case. A holding without an ISIN is matched by the model by its name only,
-   * holding whose data source the import refuses for the user is left out as
+   * hence the code cannot check it against the asset.
-   * well, so that only its rows have no match and not the whole import fails.
    */
   private async getRowMatches({
     abortSignal,
@@ -608,16 +650,13 @@ export class AiService {
             })
         : undefined,
       assets.length > 0
-        ? this.mcpService
+        ? this.getHoldingsToMatch({ subscription, userId }).catch(
-            .getPortfolio({ userId, withValues: false })
+            (error: Error): undefined => {
-            .then(({ content }) => {
-              return content[0].text;
-            })
-            .catch((error: Error): undefined => {
               this.logger.warn(`Could not get the holdings: ${error.message}`);
 
               return undefined;
-            })
+            }
+          )
         : undefined,
       Promise.all(
         assets.map((asset) => {
@@ -660,18 +699,6 @@ export class AiService {
       })
     });
 
-    const dataSources = await this.dataProviderService.getDataSources();
-
-    const holdingsFromTable = this.getHoldingsFromTable(holdings).filter(
-      ({ dataSource }) => {
-        return this.isValidDataSourceForImport({
-          dataSource,
-          dataSources,
-          subscription
-        });
-      }
-    );
-
     const assetMatches = output.assets.filter(({ dataSource, id, symbol }) => {
       const assetProfileIdentifier = getAssetProfileIdentifier({
         symbol,
@@ -679,11 +706,13 @@ export class AiService {
       });
 
       return (
-        holdingsFromTable.some((holding) => {
+        (holdings ?? []).some((holding) => {
           return (
             holding.dataSource === dataSource &&
             holding.symbol === symbol &&
-            (!holding.isin || holding.isin === assets[id]?.isin)
+            (!holding.isin ||
+              this.getNormalizedIsin(holding.isin) ===
+                this.getNormalizedIsin(assets[id]?.isin))
           );
         }) ||
         (searches[id]?.assetProfiles ?? []).some((assetProfile) => {