Smart importer give duplicate asset postings

[jbrok]

Hi, first of all, thanks for providing this great package. I've been moving all the families' small accounts to the Nordigen API in combination with smart importers.

There's a problem that I'm struggling to debug. Specifically, about 30% of the transactions from a specific API Importer (GoCardless/Nordigen) end up with three postings when using smart_importer. This issue doesn't occur with csvs, xls, etc.) Initially, I thought this was because of a bug in the Nordigen beancounttools importer but that doesn't seem the case. Here's an example:

2023-08-19 * "amazon.co.uk"
  creditorName: "Amazon.co.uk*1f37b5qz4"
  nordref: "64e135f0-75fa-XXXX-XXXXXX-XXXXXX"
  Expenses:Shopping
  Assets:Person1:Bank:Revolut:GBP <--- Randomly incorrectly added by smart_importer
  Assets:Person2:Bank:Revolut:GBP   -5.99 GBP
  
  2023-11-24 * "Cloudflare"
  nordref: "6560fd83-XXX-XXXXX-XXXX-XXXXX"
  creditorName: "Cloudflare"
  original: "EUR 4.32"
  Assets:Person1:Bank:Monzo:Checking <--- Randomly incorrectly added by smart_importer
  Expenses:Shopping
  Assets:Person2:Bank:Revolut:EUR    -4.32 EUR

It always seems to add an extra random Asset: posting. After researching a while ago I stumbled upon an smart_import caching issue but that issue was fixed.

My importer looks like this:

# Nordigen API accounts example
apply_hooks(nordigen.Importer(), [categories, PredictPostings(), DuplicateDetector(comparator=ReferenceDuplicatesComparator('nordref'), window_days=10)])

Removing PredictPostings() from here gives me the right results so I nailed it down to smart_importer adding the incorrect postings.

I call bean-extract like this:

# filter only .yaml files to debug the Nordigen issue and running it on one account
❯ bean-extract config.py ./import-files/*.yaml -e main.beancount > tmp.beancount && code tmp.beancount
DEBUG:smart_importer.predictor:Loaded training data with 22022 transactions for account , filtered from 22022 total transactions
DEBUG:smart_importer.predictor:Trained the machine learning model.
DEBUG:smart_importer.predictor:Apply predictions with pipeline
DEBUG:smart_importer.predictor:Added predictions to 82 transactions

For the last months, I've been removing the extra postings with a regex find&replace but recently I found out it also impacts deduplication so it doesn't duplicate those transactions. Not sure if it's because of how the API calls are made or if it's a smart_imported issue (seems the latter). I also tried to fork the code and limit the prediction on 1 posting, but that didn't help, it seems the wrong posting had still the highest prediction score.

My beancount file with training data doesn't contain errors, nor transactions with three postings (checked with bean-check and custom scripts).

Any ideas that can point me in the right direction to a solution? Much appreciated!

[dev590t]

Similar issue for me, smart_importer works when I import from local ofx file, but give me three postings for some transaction when using smart_importer with tarioch/beancounttools Nordigen importer.

And it isn't easy to debug because I have only 4 API call per day in my Nordigen account. tarioch/beancounttools Nordigen importer download and transform the data with the same function : extract(). Maybe I should make a PR to rewrite this function into 2 functions separately for easier debugging.

[jodoox]

Having the same issue with nordigen + smart_importer.

I've just uploaded a library (pip install beancount-gocardless) that leverages requests-cache to overcome GoCardLess' quotas.
It adds a few cache options (eg cache_options.expire_after: 86400), hopefully this will help debug the duplicate asset posting issue.

Library docs on https://beancount-gocardless.readthedocs.io/en/latest/

[jstammers]

I'm also having this same issue. I think this could be related to the fact that the EntryPredictor does not filter by import account when using NordigenImporter because the filter always returns True based on EntryPredictor.account.

I suspect that when training a single model across all transactions, it's overfitting to postings related the most frequent filing account which is what seems to happen for my data.

I think this would need some way of training separate classifiers per filing account, perhaps by subclassing EntryPredictor and modifying NordigenImport.account(file)

[dev590t]

It's a shame there's no a ofxstatement-nordigen tool in ofxstatement project that converts GoCardless data into Ofx.
The Ofx import process works well for all accounting software, including Beancount + smart_import.
If ofxstatement-nordigen exists, this solves the problem of integrating GoCardLess data for all accounting programs at the one time. There is no need to maintain GocardLess plug-ins for each accounting program.

I think ofxstatement-nordigen is a better approach, which is missing from the open banking ecosystem.

I think also than maintain a tool like ofxstatement-nordigen is much easy than debug the issue why nordigen + smart_importer no work.

[tarioch]

@jodoox great what you did, in trying to focus on not duplicating efforts, should I deprecate/remove the importer on my side and rather redirect to your library or should I try to take the caching over?
Still trying to figure out what exactly is going on to cause the issues. My setup is very simple, where I'm only pulling information for one single account from nordigen, so I'm guessing the issues only start once you pull multiple accounts.

[jstammers]

@jodoox I've had some success by using the following subclass which stores transactions into separate json files and infers the corresponding filing account using these. I think this is closer to the intended use for Importer.extract, where filepath refers to a file that contains transaction data.

class MyNordigenImporter(NordigenImporter):
    """A subclass of NordigenImporter that correctly infers the filename based on the config and account id"""

    def __init__(self, config_path: str, import_path: str,):
        self.config_path = config_path
        self.import_path = import_path
        super().__init__()
        self.config = self.load_config(config_path)
        self.ingest_transactions()

    def identify(self, filepath: str) -> bool:
        if not filepath.endswith(".json"):
            return False
        account_ids = list(x["id"] for x in self.config["accounts"])
        account_id = filepath.split("/")[-1].split(".")[0]
        return account_id in account_ids

    def account(self, filepath: str) -> str:
        account_id = filepath.split("/")[-1].split(".")[0]
        for account in self.config["accounts"]:
            if str(account["id"]) == account_id:
                return account["asset_account"]
        raise ValueError(f"No account mapping found for account number {account_id}")

    def extract(self, filepath: str, existing: data.Entries) -> data.Entries:
        accounts = self.config["accounts"]
        asset_account = None
        account = {}
        for account in accounts:
            if account["id"] in filepath:
                asset_account = account["asset_account"]
                break
        if asset_account is None:
            raise ValueError(f"No account mapping found for file: {filepath}")
        custom_metadata = account.get("metadata", {})
        transactions_data = json.load(open(filepath))
        all_transactions = self.get_all_transactions(transactions_data)
        entries = []
        for transaction, status in all_transactions:
            entry = self.create_transaction_entry(
                transaction, status, asset_account, custom_metadata
            )
            entries.append(entry)
        return entries

    def ingest_transactions(self, days_back=180):
        for account in self.config["accounts"]:
            account_id = account["id"]
            transactions = self.client.get_transactions(account_id, days_back)
            filepath = f"{self.import_path}/{account_id}.json"
            if not os.path.exists(self.import_path):
                os.makedirs(self.import_path)
            with open(filepath, "w") as f:
                json.dump(transactions, f)

I like @dev590t's suggestion for an ofxstatement-nordigen plugin, as ultimately it feels easier to build upon what already exists to parse OFX files.

[dev590t]

I like @dev590t's suggestion for an ofxstatement-nordigen plugin, as ultimately it feels easier to build upon what already exists to parse OFX files.

@jstammers here a reddit discussion about this idea:
https://www.reddit.com/r/plaintextaccounting/comments/1jrpf1v/ofxstatementgocardless_a_unified_solution_for/

[tarioch]

In regards to the caching question, I'm wondering if it would make more sense to have this on the level of the results of an importer, that way it could be reused for any importer. I'm currently playing around with moving smart importer from the legacy hooks to the new beangulp hooks, once I get this working, I might create a simple hook which supports caching.
I'm not opposed to reusing something from ofxstatement or similar if available, but as usual it depends who will create and maintain it.

[jstammers]

@dev590t I've made a start on an ofxstatement-nordigen plugin.

Any feedback on this would be greatly appreciated if you think it's worth pursing further

[markferry]

The Ofx import process works well for all accounting software, including Beancount + smart_import

@dev590t as I discovered recently the beangulp ofx importer has been demoted to an example (beancount/beangulp@4f25b09) and comes with no guarantees.

I'm in the process of switching all my importers from TrueLayer and Quickfile to GoCardless only.
But as it stands right now I would rather not go via OFX.

[dev590t]

@markferry , I use redstreet/beancount_reds_importers 's ofxreader. It support beancount v2 and v3.