From 9f3f11ea7e66cc0e03a646b0e0d6a7f9e39ce2e1 Mon Sep 17 00:00:00 2001 From: Thomas Kaul <4159106+dtslvr@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:22:40 +0200 Subject: [PATCH] Improve matching of assets --- .../src/app/endpoints/ai/ai.service.spec.ts | 150 +++++++++++++++++- apps/api/src/app/endpoints/ai/ai.service.ts | 109 ++++++++----- 2 files changed, 216 insertions(+), 43 deletions(-) diff --git a/apps/api/src/app/endpoints/ai/ai.service.spec.ts b/apps/api/src/app/endpoints/ai/ai.service.spec.ts index 9e2b450d97..69636a19fb 100644 --- a/apps/api/src/app/endpoints/ai/ai.service.spec.ts +++ b/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({}); diff --git a/apps/api/src/app/endpoints/ai/ai.service.ts b/apps/api/src/app/endpoints/ai/ai.service.ts index adbb061ab4..516e93ad81 100644 --- a/apps/api/src/app/endpoints/ai/ai.service.ts +++ b/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 - * table of the holdings which has a data source and a symbol. A cash - * position has no data source, hence it is left out. The ISIN is empty if - * the asset profile has none, which is the case for most stocks of Yahoo - * Finance. The columns are found by the header line. A line is split at - * each pipe which is not escaped, as the renderer escapes a pipe in a value. + * Gives the currency, the data source, the ISIN, the name and the symbol of + * each holding of the table of the holdings which has a data source and a + * symbol. A cash position has no data source, hence it is left out. The ISIN + * is null if the asset profile has none, which is the case for most stocks + * of Yahoo Finance. The columns are found by the header line. A line is + * 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(/(? { - 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 - * given without values. The holdings let the model select an asset profile - * which the user already has, also if no search gives it, for example as no - * data provider finds its ISIN. If get-accounts fails, the model gets no - * accounts, so that only the account hints have no match. If get-portfolio - * fails, 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 + * example if the output has a schema. The accounts are given without + * values, the holdings with their currency, data source, ISIN, name and + * symbol only. The holdings let the model select an asset profile which the + * user already has, also if no search gives it, for example as no data + * provider finds its ISIN. A holding whose data source the import refuses + * for the user is not given to the model, so that the model selects an + * 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 - * by its name only, hence the code cannot check it against the asset. A - * holding whose data source the import refuses for the user is left out as - * well, so that only its rows have no match and not the whole import fails. + * the ids of the assets. The ISINs are compared without spaces and in upper + * case. A holding without an ISIN is matched by the model by its name only, + * hence the code cannot check it against the asset. */ private async getRowMatches({ abortSignal, @@ -608,16 +650,13 @@ export class AiService { }) : undefined, assets.length > 0 - ? this.mcpService - .getPortfolio({ userId, withValues: false }) - .then(({ content }) => { - return content[0].text; - }) - .catch((error: Error): undefined => { + ? this.getHoldingsToMatch({ subscription, userId }).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) => {