redstreet / beancount_reds_importers

Simple ingesting tools for Beancount (plain text, double entry accounting software). More importantly, a framework to allow you to easily write your own importers.
GNU General Public License v3.0
113 stars 38 forks source link

Handling Fidelity (and other) accounts with banking features #41

Closed thehilll closed 1 year ago

thehilll commented 1 year ago

Hello,

I ran into an issue trying to parse downloads from Fidelity accounts that have banking features enabled (in particular accounts that receive paper checks and use bill payment...I could see there being similar issues with writing checks, but I don't have any examples to try).

The OFX file looks something like this for the transactions that are failing:

<INVBANKTRAN>    <STMTTRN>
    <TRNTYPE>DEP</TRNTYPE>
<DTPOSTED>20220909000000.000[-5:EDT]</DTPOSTED>
<TRNAMT>+00000000000001.0000</TRNAMT>
<FITID>XXXXXXXXXXXXX1220220909</FITID>
<NAME>CHECK RECEIVED</NAME>
<MEMO>CHECK RECEIVED</MEMO>
<CURRENCY>
    <CURRATE>1.00</CURRATE>
<CURSYM>USD</CURSYM>
</CURRENCY>    </STMTTRN>
<SUBACCTFUND>CASH</SUBACCTFUND>    </INVBANKTRAN>
<INVBANKTRAN>    <STMTTRN>
    <TRNTYPE>PAYMENT</TRNTYPE>
<DTPOSTED>20220909000000.000[-5:EDT]</DTPOSTED>
<TRNAMT>-00000000000001.0000</TRNAMT>
<FITID>XXXXXXXXXXXXX1120220909</FITID>
<NAME>BILL PAYMENT         MYPAYEE NA</NAME>
<MEMO>BILL PAYMENT         MYPAYEE NAME            /0047/Y*******</MEMO>
<CURRENCY>
    <CURRATE>1.00</CURRATE>
<CURSYM>USD</CURSYM>
</CURRENCY>    </STMTTRN>
<SUBACCTFUND>CASH</SUBACCTFUND>    </INVBANKTRAN>

and the exceptions are e.g.

ERROR: unknown entry type: payment

However, the fix is quite simple, extending the cash transaction lists on line 257 and 318 to include:

elif ot.type in ['other', 'credit', 'debit', 'dep', 'cash', 'payment', 'check']:

elif ot.type in ['other', 'credit', 'debit', 'transfer', 'dep', 'income',
                             'dividends', 'capgainsd_st', 'capgainsd_lt', 'cash', 'payment', 'check']:

Allows these to be processed like other cash transactions.

redstreet commented 1 year ago

Thanks for the report and suggestion! I'll take your word for it, since I don't have an ofx file to test against. Fix committed.

redstreet commented 1 year ago

Actually, I added payment, but will hold off on adding check until someone can confirm that it is a type used by an institution.

thehilll commented 1 year ago

I did find that Fidelity's ofx file generated payment transactions for bills payed through the web site and check transactions for checks received. The above snippet would not parse w/o both items added to the lists, and it was exactly from fidelity minus some sanitizing.

redstreet commented 1 year ago

👍🏻 I'll add it in a minute.

chase-dwelle commented 1 year ago

@thehilll Would you be willing to provide your import configs for your Fidelity CMA/"banking"-like accounts? I'm currently using the banking transaction builder but wondering if I should be using investments as I'm running into occasional, odd errors showing a disconnect from the outputs of ofx-summarize and what's being shown in bean-extract.

I have a "checking" account which acts as the overdraft for my CMA account which has the following config:

# The `fidelity_banking` importer class
class Importer(banking.Importer, ofxreader.Importer):
    IMPORTER_NAME = "Fidelity Banking Accounts"

    def custom_init(self):
        if not self.custom_init_run:
            self.max_rounding_error = 0.04
            self.filename_pattern_def = "fidelity_checking*"
            self.custom_init_run = True
# How it's applied in import configs.
  apply_hooks(
      fidelity_banking.Importer( 
          {
              "account_number": "X90787461",
              "main_account": "Assets:Fidelity:Checking",
              "filename_pattern": "fidelity_checking.ofx",
          }
      ),
      [PredictPostings()],
  ),
redstreet commented 1 year ago

@chase-dwelle, FYI, I've only run some very basic cases myself, but the Fidelity ofx importer (which uses the investment transaction builder) works fine out of the box with the CMA account (call it exactly as you would a standard brokerage account, with your CMA account number).

At some point, beancount_reds_importers should address the issue of the right transaction builder to use for checking-like accounts, which straddle investments and banking. I don't have enough experience, or a sense of requirements for these accounts, or test cases for these, but am happy to follow along and hear from folks about what works best.

Possibilities include:

thehilll commented 1 year ago

@chase-dwelle I went down the path of trying to use the banking transaction builder, but I gave up pretty quickly. Fidelity seems to report the OFX file of CMA accounts as something very close to a standard investment account.

I'm using this configuration:

apply_hooks(fidelity.Importer({
        'main_account': 'Assets:My-Fidelity_Acct:{ticker}',
        'cash_account': 'Assets:My-Fidelity_Acct',
        'dividends': 'Income:My-Fidelity_Acct:{ticker}:Div',
        'interest': 'Income:My-Fidelity_Acct:{ticker}:IntInc',
        'cg': 'Income:Realized-Gain',
        'capgainsd_lt': 'Income:My-Fidelity_Acct:{ticker}:CGLong',
        'capgainsd_st': 'Income:My-Fidelity_Acct:{ticker}:CGShort',
        'fees': 'Expenses:Discretionary:Bnk-and-CC-Fees',
        'fund_info': fund_info,
        'account_number': 'A12345678',
        'filename_pattern': 'Fidelity-A12345678*'
}), [PredictPostings()]),

With a version of investments.py that includes the changes discussed here. I also very lightly modified the provided fidelity class to look like this:

class Importer(investments.Importer, ofxreader.Importer):
    IMPORTER_NAME = 'Fidelity Net Benefits / Fidelity Investments OFX'

    def custom_init(self):
        self.max_rounding_error = 0.14
        self.filename_pattern_def = '.*fidelity'
        self.get_ticker_info = self.get_ticker_info_from_id
        self.get_payee = lambda ot: ot.memo.split(";", 1)[0] if ';' in ot.memo else ot.memo

    def security_narration(self, ot):
        ticker, ticker_long_name = self.get_ticker_info(ot.security)
        return f"[{ticker}]"

    def file_name(self, file):
        return 'fidelity-{}-{}'.format(self.config['account_number'], ntpath.basename(file.name))

    def get_target_acct_custom(self, transaction, ticker=None):
        if transaction.memo.startswith("CONTRIBUTION"):
            return self.config['transfer']
        if transaction.memo.startswith("FEES"):
            return self.config['fees']
        elif transaction.type == 'income' and getattr(transaction, 'income_type', None) == 'DIV':
            return self.target_account_map.get('dividends', None)
        elif getattr(transaction, 'income_type', None) == "CGLONG":
            return self.target_account_map.get('capgainsd_lt', None)
        elif getattr(transaction, 'income_type', None) == "CGSHORT":
            return self.target_account_map.get('capgainsd_st', None)
        else:
            return None

The issue I ran into was that CGLONG and CGSHORT distributions were showing up in the OFX with a transaction.type of income, and then would be classified as dividends based on this code in investments.py:

def get_target_acct(self, transaction, ticker):
        target = self.get_target_acct_custom(transaction, ticker)
        if target:
            return target
        if transaction.type == 'income' and getattr(transaction, 'income_type', None) == 'DIV':
            return self.target_account_map.get('dividends', None)
        return self.target_account_map.get(transaction.type, None)

That last line was the one that returned, and it would classify even CG distributions as dividends. This seems like it is a variation on the OFX file that wasn't considered, and I thought about opening another issue here. However, self.get_target_acct_custom is specifically described as something that exists only for importers to override, so I just did that.

With these changes (those noted in the first post here, and the custom fidelity class) along with this config I was able to import all of my Fidelity accounts (a few that are largely bank accounts and several more that are almost only investment transactions).

I have not had full success getting the smart importers to work. I did succeed with all of the CMA accounts (where it really matters), but most of the investment accounts did not work. The error is always:

Cannot train the machine learning modelNone of the training data matches the accounts
Cannot train the machine learning model because there are no targets.

This isn't really true. I've triple checked the account strings match. I think it has something to do with the setting for cash_account, but given that I got it to work for CMA I didn't investigate much further. The investment accounts just don't need the smart importer that much.

For the investment accounts I use something like 'cash_account': 'Assets:My-Fidelity_Acct:Cash' in the config b/c I use security leaf accounts there. I think that's the issue. I'm not sure why though.

redstreet commented 1 year ago

Very cool, and very helpful, thank you for sharing in detail, @thehilll !

Based on your message, it seems like the investment transaction builder is the right one to use for cash management accounts, which is good to know.

Opened #42 to guess income type better in the main code where possible, despite inconsistencies.

Reg smart importer, the investment transaction builder outputs all target accounts needed. Wouldn't this leave smart importer with nothing to guess? I'm trying to figure out what you're using smart importer for in this scenario.

thehilll commented 1 year ago

On the smart importer...you are right. It really isn't needed on a true investment account. Across many accounts like this I only ran into two types of transactions that weren't classified:

Where I do need the smart importer is for the two accounts I treat as a banking account as those have a lot of bill payment transactions. In this case I did get the smart importer to work, and it worked well. I believe the issue I was having was when I included the :Cash suffix on the cash_account setting (which I use on my true investment accounts but not on my "banking" ones.

Really I was just trying to have a single importer config for simplicity (this is my first pass of imports after converting to beancount).

chase-dwelle commented 1 year ago

Thanks @thehilll , this worked and seems to have fixed some odd errors I was getting with my Fidelity "banking" accounts, like some transactions missing date attributes.

redstreet commented 1 year ago

Makes sense, thanks again for sharing @thehilll.

BTW, your setup might be helpful to others to post on the Beancount mailing list, if you feel up to it. CMA accounts seem to be getting more common.

thehilll commented 1 year ago

@redstreet I will post something, but I was trying to iron out a couple of nagging issues that made the success here seem a bit fragile.

First, a trivial item I came across was an additional cash transaction type of fee. This isn't a fee associated with a transaction but rather its own line-item. It is an account-level fee. The fix is the same as above, adding fee in two places, but at this point would it make sense to allow this argument to be provided in some way? Or allow cash transaction types in addition to your defaults to be passed in?

The second thing that was bothering me was that I have two different investment accounts that function as bank accounts, and I needed (through trial and error) to have slightly different configs to get the smart importer to work for them (and these benefit from the smart importer). This config worked for the first account:

apply_hooks(fidelity.Importer({
    'main_account': 'Assets:Banking:Fidelity:Account_1:{ticker}',
    'cash_account': 'Assets:Banking:Fidelity:Account_1',
    'dividends': 'Income:Banking:Div:Account_1:{ticker}',
    'interest': 'Income:Banking:IntInc:Account_1:{ticker}',
    'cg': 'Income:Realized-Gain',
    'capgainsd_lt': 'Income:Banking:CGLong:Account_1:{ticker}',
    'capgainsd_st': 'Income:Banking:CGShort:Account_1:{ticker}',
    'fees': 'Expenses:Discretionary:Bnk-Fees',
    'fund_info': fund_info,
    'account_number': 'ABCDEFGHI',
    'filename_pattern': 'xxxfidelity*'
}), [PredictPostings()]),

Bit for the second account I needed to make the first two settings:

apply_hooks(fidelity.Importer({
    'main_account': 'Assets:Banking:Fidelity:Account_2:Cash',
    'cash_account': 'Assets:Banking:Fidelity:Account_2:Cash',

(the other settings were similar to the first account). If I did not hard code main_account on the second account to end in :Cash the smart importer would complain about no matching accounts in the training set.

Digging into things I found that your importer is properly using cash_account for these banking transactions (as you would expect), but then when the transactions are run through the smart importer I was losing all the training data in this function in predictor.py:

def training_data_filter(self, txn):
    """Filter function for the training data."""
    found_import_account = False
    for pos in txn.postings:
        if pos.account not in self.open_accounts:
            return False
        if self.account == pos.account:
            found_import_account = True
    return found_import_account or not self.account

In particular, self.account would never match pos.account and since self.account was set the response from this function was always False. This led to an empty training set.

The reason these two would never match for this account is because predictor.py sets it to:

self.account = importer.file_account(file)

and in your reader.py that is:

def file_account(self, _):
    return self.config['main_account'].replace(':{ticker}', '').replace(':{currency}', '')

This explains the difference between my two accounts. In Account_1 I do not have a :Cash sub-account...it really has only ever been a bank account, and I just figured it would be easier to treat it that way (it does have a couple of money-markets in it which have sub-accounts...this is pretty inconsistent now that I look at it). However, Account_2 did years ago have a few securities in it, so I set it up as a more canonical investment account where everything including cash is held in a sub-account.

In this case I run into an edge case where if I use the config from the first account where main_account ends in {ticker} I filter out the training data.

I'm only looking at this as an outsider, so it's tough to say if there is a way to address this generally. My first reaction is that maybe it is just a point to make in explanation...cash_account and main_account must match after applying the function file_account to main_account.

The one issue with that is I think it might make for a bit of a mess if you have an account that gets both banking transactions and trade transactions. I don't have any of those now, but maybe you know?

Alternatively, is there a way to define file_account that would work here w/o breaking other functionality?

redstreet commented 1 year ago

First, a trivial item I came across was an additional cash transaction type of fee. This isn't a fee associated with a transaction but rather its own line-item. It is an account-level fee. The fix is the same as above, adding fee in two places, but at this point would it make sense to allow this argument to be provided in some way? Or allow cash transaction types in addition to your defaults to be passed in?

Yes, adding an overridable self.transfer_entry_types to the constructor would take care of this line. It's fine to add fees to the default list since that seems innocuous, but parameterizing is will help future types that are inconsistent among institutions.

For this line, perhaps there is a better way of deciding upon using ot.amount vs ot.total then listing types? If not, another overridable variable it is.

Response to your other part soon.

redstreet commented 1 year ago

Trying to understand this better before I respond:

thehilll commented 1 year ago

Trying to understand this better before I respond:

I believe this is very specific to my setup and history with these accounts (really my question is how would you explain this to people in the generic case, and only secondarily if there is anything to change).

  • Are account_1 and account_2 both "cash management accounts"?

Yes, they are today.

  • are you using investments in one and just cash in another?

account_1 has never had anything other than money market funds...it has only ever been effectively a cash account. account_2 used to have investments (non-money market funds) many years ago and so has that history. Because of this I set the two accounts up slightly differently. I personally consider them both banking accounts (I am closely following your account structure), but the second account does need to handle its history of holding shares. The account openings for these two look like this:

2000-01-01 open Assets:Banking:Fidelity:Account_1 USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:ABCXX USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:DEFXX USD "FIFO"

2000-01-01 open Assets:Banking:Fidelity:Account_2 USD "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:Cash USD "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:GHIXX GHIXX "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:JKLXX JKLXX "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:MNOXX MNOXX "FIFO"
  • are you wanting to use smart importer with both?

Yes. These both have frequent bill pay activity for which the smart importer really helps. With the current configs smart importer works with both.

If so, wouldn't a subset of the investment related transactions be deterministic? Are you only wanting smart importer to work on the remaining?

Yes, and this works. The deterministic transactions are correctly constructed by the importer, and the PredictPostings() call of the smart importer ignores them because they have two postings. The banking transactions have only one posting coming out of the importer, and with the config I am using smart importer properly (tries to) figure out a second posting.

  • is the only issue that having two different configs doesn't feel right?

It was really two things:

  1. It didn't feel right, and I wanted to understand what was going on
  2. I was trying to figure out how I would answer the more generic question of how to use these importers with smart import on a CMA account

I think 1 is really answered...it's explainable and a consequence of my setup. On 2 I think the short answer is that whatever someone sets as the value for main_account must match the value for cash_account after being run through this function:

def file_account(self, _):
    return self.config['main_account'].replace(':{ticker}', '').replace(':{currency}', '')

That is possible for me. I think it would be possible for most people. I don't really know if it would be possible to make the setup simpler, but my guess is maybe no?

Thanks a lot for this. It has saved me dozens of hours.

redstreet commented 1 year ago

Thanks a bunch for the response including the open statements, that makes it very clear!

You raise good points. I'd love to straighten and simplify these if needed. First: with my own accounts, I realized this structure had way too many problems to work out well:

2000-01-01 open Assets:Banking:Fidelity:Account_1 USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:ABCXX USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:DEFXX USD "FIFO"

and thus, I myself use this structure instead (leaf-accounts only):

2000-01-01 open Assets:Banking:Fidelity:Account_1:USD USD
2000-01-01 open Income:Banking:Div:Account_1:ABCXX ABCXX "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:DEFXX DEFXX "FIFO"

So beancount_reds_importers handles the latter structure well, but probably not the former. IMHO, the former structure is not worth covering as a use case. I could be convinced otherwise, but before we debate that, would this structure work for both your accounts?

'main_account': 'Assets:My-Fidelity_Acct:{ticker}',
'cash_account': 'Assets:My-Fidelity_Acct:{currency}',
thehilll commented 1 year ago

First: with my own accounts, I realized this structure had way too many problems to work out well:

2000-01-01 open Assets:Banking:Fidelity:Account_1 USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:ABCXX USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:DEFXX USD "FIFO"

That's understandable. I did it because I really view this as a banking account, but even writing this up it seemed odd. However, I don't think just eliminating this case solves the issue.

and thus, I myself use this structure instead (leaf-accounts only):

2000-01-01 open Assets:Banking:Fidelity:Account_1:USD USD
2000-01-01 open Income:Banking:Div:Account_1:ABCXX ABCXX "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:DEFXX DEFXX "FIFO"

This is very close to my setup for Account_2:

2000-01-01 open Assets:Banking:Fidelity:Account_2 USD "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:Cash USD "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:GHIXX GHIXX "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:JKLXX JKLXX "FIFO"
2000-01-01 open Assets:Banking:Fidelity:Account_2:MNOXX MNOXX "FIFO"

Note that first line never really comes into play...no postings are booked to it, and it only exists because I didn't understand that you could have a :Cash account without first opening the parent.

The only difference from your setup is that I used :Cash rather than :USD and as such I never tried to use your {currency} substitution. That turned out to be lucky in that it actually made things work quite by accident.

So beancount_reds_importers handles the latter structure well, but probably not the former. IMHO, the former structure is not worth covering as a use case. I could be convinced otherwise, but before we debate that, would this structure work for both your accounts?

'main_account': 'Assets:My-Fidelity_Acct:{ticker}',
'cash_account': 'Assets:My-Fidelity_Acct:{currency}',

I modified my accounts to match your structures:

2000-01-01 open Assets:Banking:Fidelity:Account_1:USD USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:ABCXX USD "FIFO"
2000-01-01 open Income:Banking:Div:Account_1:DEFXX USD "FIFO"

And then use this setting in my-smart.import:

apply_hooks(fidelity.Importer({
    'main_account': 'Assets:Banking:Fidelity:Account_1:{ticker}',
    'cash_account': 'Assets:Banking:Fidelity:Account_1:{currency}',
    'dividends': 'Income:Banking:Div:Account_1:{ticker}',
    'interest': 'Income:Banking:IntInc:Account_1:{ticker}',
    'cg': 'Income:Realized-Gain',
    'capgainsd_lt': 'Income:Banking:CGLong:Account_1:{ticker}',
    'capgainsd_st': 'Income:Banking:CGShort:Account_1:{ticker}',
    'fees': 'Expenses:Discretionary:Bnk-and-CC-Fees',
    'fund_info': fund_info,
    'account_number': '123456789',
    'filename_pattern': 'fidelity*'
}), [PredictPostings()]),

However, when I run bean-extract I get the complaint that no accounts matched:

Cannot train the machine learning modelNone of the training data matches the accounts
Cannot train the machine learning model because there are no targets.

The problem comes from this line in smart_importer/predictor.py where the existing transactions are filtered through training_data_filter.

def training_data_filter(self, txn):
        """Filter function for the training data."""
        found_import_account = False
        for pos in txn.postings:
            if pos.account not in self.open_accounts:
                return False
            if self.account == pos.account:
                found_import_account = True
        return found_import_account or not self.account

Each transaction goes through that filter individually, and it is kept in the training set if the function returns True. The first test if pos.account not in self.open_accounts works properly (i.e. it allows transactions in open accounts to remain), but the second test if self.account == pos.account is never met (that is the code never sets found_import_account = True for any transaction). This means this function always returns false and the training set is always empty.

That test is a comparison of pos.account, the posting account for a banking transaction where only one posting will exist. This will be e.g.

2023-01-13 * "BILL PAYMENT         ABC DEF  /0002/N*******" "payment"
  Assets:Banking:Fidelity:Account_1:USD  -250.0000 USD

So Assets:Banking:Fidelity:Account_1:USD needs to match self.account. That is set here to

self.account = importer.file_account(file)

That function in reader.py here does a simple replacement on the config:

def file_account(self, _):
    return self.config['main_account'].replace(':{ticker}', '').replace(':{currency}', '')

And so in the case where the config is as you suggest, the file_account function would pass to the smart importer a truncated account that does not match the position created by the importer, in my case:

Assets:Banking:Fidelity:Account_1

I got lucky trying different configs (and account setups) and didn't use the tokens in the config. In that case the replacement above won't match my hard coded :Cash or were I to use your setup a hard coded :USD. This allows file_account to return an account that matches the one found in the postings.

The limitation is that as of now I don't think you can do these four things together:

That isn't a very big limitation. I think people could get around it just by hard coding the currency in the config (though obviously that has some multi-currency limitations, but I haven't really thought that through).

To avoid having to do that, I think file_account would need to return something that would match the postings that the smart importer is trying to match, but I don't know what other problems that would cause.

Also, I've only done a few imports since setting up beancount. I could well be missing some complications here.

redstreet commented 1 year ago

I agree, something doesn't seem right with file_account being called at all by smart_importer. If you don't mind posting a small, one-transaction anonymized example (existing transaction file, import config, import ofx/csv), I can take a look. I'd imagine either smart_importer must be fixed, or file_account() must do something better.

redstreet commented 1 year ago

Ah, file_account() that you figured was the root of the problem is returning what it is to accommodate commodity leaf accounts. That is likely what needs to be fixed. Please do post an example if you can, and I'll fix it.

thehilll commented 1 year ago

Sorry to not reply. I should be able to get something later today, assuming I can figure out the ofx part of it.

thehilll commented 1 year ago

OK, I believe the six attached files are a useful example that corresponds to the above. They are:

2007-02-05 open Assets:Banking:Fidelity:Account1 USD "FIFO"

smart.import.works.py: smart import config for this account setup, e.g.

'main_account': 'Assets:Banking:Fidelity:Account1:{ticker}',
'cash_account': 'Assets:Banking:Fidelity:Account1',

Using these accounts gives:

bean-extract ./my-smart.import.works.py -f ./test_works.bc ./test.ofx

;; -*- mode: beancount -*-
**** /path/to/test/test.ofx

2023-02-06 * "DIRECT               DEBIT GROCERTYSTORE     PMT" "cash"
  Assets:Banking:Fidelity:Account1  -175.0000 USD
  Expenses:Discretionary:ExpType1

where that transaction date and amount match what is in test.ofx and the category choice makes sense given what is in test_works.bc.

However, when I use:

test_fails.bc: a single transaction bc file with account settings that fail currently, e.g.

2007-02-05 open Assets:Banking:Fidelity:Account1:USD USD "FIFO"

and my-smart.import.fails.py: smart import config for the failing account setup, e.g.

'main_account': 'Assets:Banking:Fidelity:Account1:{ticker}',
'cash_account': 'Assets:Banking:Fidelity:Account1:{currency}',

The smart importer is unable to match any transactions:

bean-extract ./my-smart.import.fails.py -f ./test_fails.bc ./test.ofx

Cannot train the machine learning modelNone of the training data matches the accounts
Cannot train the machine learning model because there are no targets.
;; -*- mode: beancount -*-
**** /path/to/test/test.ofx

2023-02-06 * "DIRECT               DEBIT GROCERTYSTORE     PMT" "cash"
  Assets:Banking:Fidelity:Account1:USD  -175.0000 USD

Thanks for digging into this. test.zip

redstreet commented 1 year ago

Awesome, that helps a lot, @thehilll! I took a look, and have filed the issue linked above with smart_importer. More context is in that ticket.We can take it forward based on what we learn there.

redstreet commented 1 year ago

Until it's fixed upstream, I just committed a pretty convenient hack. To use it, add this to your importer config:

  'smart_importer_hack': 'Account:USD'

That account will be sent to smart_importer while everything else will remain as such (importantly, bean-file will continue to receive what it was previously receiving).

Let me know how this works out for you.

thehilll commented 1 year ago

Yes, this seems to give all the flexibility needed. Thanks a lot.

davraamides commented 1 year ago

I also ran into this issue as I use my Fidelity brokerage account for bill payments, check writing and debit card transactions. One other detail I wanted to mention was parsing of the check numbers. Here is an example of one in the OFX file:

<INVBANKTRAN>
    <STMTTRN>
        <TRNTYPE>CHECK</TRNTYPE>
        <DTPOSTED>20230327000000.000[-4:EDT]</DTPOSTED>
        <TRNAMT>-00000000000085.0000</TRNAMT>
        <FITID>XXXXXXXXXXXX01420230327</FITID>
        <CHECKNUM>0000004419</CHECKNUM>
        <NAME>Check Paid #0000004419</NAME>
        <MEMO>Check Paid #0000004419</MEMO>
        <CURRENCY>
            <CURRATE>1.00</CURRATE>
            <CURSYM>USD</CURSYM>
        </CURRENCY>
    </STMTTRN>
    <SUBACCTFUND>CASH</SUBACCTFUND>
</INVBANKTRAN>

I'm a little confused as to what the intent is in the Importer-related classes, with regards to checks. I see that the sample in beancount_reds_importers/schwab_checking_csv/__init__.py uses checknum in its header_map but I don't see any other reference to checknum in the entire repository. In any case, here's what I added to my custom Fidelity Importer class (I save the id, too):

def build_metadata(self, file, metatype=None, data={}):
    meta = {}
    if 'transaction' in data:
        meta['id'] = data['transaction'].id
        check = getattr(data['transaction'], 'checknum', None)
        if check:
            meta['check'] = f"{int(check):04d}"
    return meta
redstreet commented 1 year ago

@davraamides, since there is so much variance between institutions on if and how they include the check number in their data (csv or ofx), there is no general approach to handling them in the importers currently. I've seen check numbers appear in memo, checknum, and refnum. The relevant ofx standard is here.

That said, your code to add check number as metadata above is general enough that I'm happy to include it in the default build_metadata() in banking.py if you want to send a PR with tests. It'd be good to move this into a new ticket if we go ahead with it.

patbakdev commented 1 year ago

I have about 10 Fidelity Investment, NetBenefits, and CMA accounts. I am currently using OFXClient to download and red's fidelity OFX importer to handle all of them. Unfortunately one of them started to give me a download error and I think we are quickly approaching the end of simple CLI download of OFX data. It seems we will all have to manually login and download unless you are Intuit. Fortunately, Fidelity will allow me to download all of the account data in one file. The bad news is that they only offer CSV.

I'd love to have a single all-in-one Fidelity CSV importer that can handle such a file. I tried using the Fidelity CSA importer against it, but it didn't recognize it (I assume it doesn't support multiple accounts). Given the comments above, it seems like basing something off of the investment builder makes the most sense. Has anyone given any thought to such an importer. I have another project I am currently working, on, but I have a feeling I am going to have to deal with this sooner than later.

EDIT: I'm looking at the schwab importer now. Maybe we need somethings similar like fidelity{csv, ofx}{balances, checking, brokerage}.py

redstreet commented 1 year ago

@patbakdev, I too have most types of of account Fidelity has (a ton of them), and regularly use bean-download (ships with this repo) to download them. No trouble there, even as of two minutes ago.

CSV is of course, the ultimate fall back solution, but for many reasons, it is a huge downgrade over .ofx, and I only use the fall back if there is no other option.

With Fidelity, a) they seem committed to solid .ofx support when I talked to them b) they seem to have the best direct-download support so far (I imagine they'll provide notification if they decide to pull support for this, an c) even if they pull the plug on (b), they do offer downloads .ofx via their web interface still. Is this right? Are you saying they do, but it's an individual download for each account?

I'm wondering if there is a solvable issue with your .ofx CLI downloads that can be resolved easily? What error are you seeing?

patbakdev commented 1 year ago

@redstreet Interesting. I took a look at bean-download and it gave me a few ideas. I was able to fix-up the NetBenefits account I was having trouble with. I think I was just irritated because that account had escaped my purview (either didn't download or appeared to download correctly, but without data) and I was making portfolio decisions with 6 months of missing data and not knowing it.

I completely agree with OFX > CSV. This process can be a bit fragile and every time it breaks my search comes across new info like these (including one from you 😄) so maybe I am just a little big jumpy:

I cannot find any QFX/OFX download option from the Fidelity website for either a single accounts or all-accounts. Statements can be downloaded as either CSV or PDF, but activity and positions appear to only be CSV. Then again, they change the interface sometimes so its possible I am on a newer/beta version. Maybe if I enroll in Full View it as an options, but that is not something I want to do. I also have Chase accounts and they stopped allowing direct download. Fidelity is the only reason I continue to maintain my automated download scripts for.

But alas, for now, I am back in a working state and can avoid the webscrape and csv hell I fear in the future.

redstreet commented 1 year ago

Going back to the original issue of this thread: the importers work fine for CMA now, with the hack. I'm not seeing enthusiasm at smart importer to fix this issue, so the hack in this thread may be the way going forward for now.

This thread has otherwise become unwieldy and a bit of a catch-all. Instead, I invite anyone with an issue to open a focused thread on that topic. As an example, I've opened #66 for the latest issue posted here.