Full audit: fix every confirmed issue

mt940/models.py

@@ -214,15 +214,20 @@
         """Coerce ``amount`` to a signed :class:`decimal.Decimal`.
 
         ``status`` is ``'C'`` for credit (positive) or ``'D'`` for debit, in
-        which case the amount is negated. Extra keyword arguments are ignored
-        so a parsed tag dictionary can be splatted in directly.
+        which case the amount is negated. The comparison is case-insensitive:
+        the tag patterns are compiled with :data:`re.IGNORECASE`, so a bank
+        that sends a lowercase ``'d'`` mark must still have its debit negated
+        rather than silently stored as a positive amount. Reversal marks
+        (``'RD'``/``'RC'``) are intentionally left unchanged here. Extra
+        keyword arguments are ignored so a parsed tag dictionary can be
+        splatted in directly.
         """
         self.amount = decimal.Decimal(amount.replace(',', '.'))
         self.currency = currency
 
         # C = credit, D = debit
 
-        if status == 'D':
+        if status.upper() == 'D':
             self.amount = -self.amount
 
     def __eq__(self, other: object) -> bool:
@@ -619,12 +624,19 @@
     def _update_transaction(self, result: dict[str, Any]) -> None:
         """Merge a transaction-scoped result into the current transaction.
 
-        New keys are set directly; string values for keys that already exist
-        are appended on a new line (e.g. multi-line ``:86:`` details).
+        New keys are set directly; when both the existing and incoming
+        values are strings, the incoming one is appended on a new line
+        (e.g. multi-line ``:86:`` details). Any other combination assigns
+        the incoming value: a structured ``:86:`` emits ``None`` for absent
+        sub-fields, so a later string must replace an existing ``None``
+        rather than crash on ``None += str`` (and an incoming ``None``
+        still overwrites, preserving the merge semantics the fixture
+        goldens encode).
         """
         transaction = self.transactions[-1]
         for k, v in result.items():
-            if k in transaction.data and hasattr(v, 'strip'):
+            existing = transaction.data.get(k)
+            if hasattr(existing, 'strip') and hasattr(v, 'strip'):
                 transaction.data[k] += f'\n{v.strip()}'
             else:
                 transaction.data[k] = v
@@ -724,7 +736,9 @@
         return valid_matches
 
 
-class TransactionsAndTransaction(Transactions, Transaction):  # type: ignore[misc]
+class TransactionsAndTransaction(  # type: ignore[misc]  # pyright: ignore[reportUnsafeMultipleInheritance]
+    Transactions, Transaction
+):
     """
     Subclass of both Transactions and Transaction for scope definitions.
 

mt940/parser.py

@@ -75,7 +75,12 @@ def safe_is_file(filename: Any) -> bool:
             raise exception  # pragma: no cover
 
     assert isinstance(data, str)
-    return data
+    # Strip a leading byte-order mark (U+FEFF). A BOM survives decoding as
+    # U+FEFF for UTF-8 input (and for UTF-16 only when an explicit
+    # utf-16-le/-be encoding is passed). It is not whitespace, so it would
+    # otherwise displace the first `:20:` off the start-of-line tag anchor and
+    # silently drop that tag's data.
+    return data.removeprefix('\ufeff')
 
 
 def parse(

mt940/processors.py

@@ -357,8 +357,17 @@ def _parse_mt940_gvcodes(purpose: str) -> dict[str, str | None]:
 
     for index, char in enumerate(purpose):
         # Detect the beginning of a GVC segment: if a '+' is encountered
-        # and the four characters preceding it form a valid GVC key.
-        if char == '+' and purpose[index - 4 : index] in GVC_KEYS:
+        # and the four characters preceding it form a valid GVC key. GVC
+        # keywords are four characters wide, so a '+' before index 4 cannot
+        # terminate one; guarding on ``index >= 4`` also avoids a negative
+        # ``purpose[index - 4:index]`` slice wrapping to the empty string
+        # (which spuriously matched the empty-string GVC key and truncated a
+        # literal '+' in the leading free text).
+        if (
+            char == '+'
+            and index >= 4
+            and purpose[index - 4 : index] in GVC_KEYS
+        ):
             if segment_type:
                 # If already processing a segment, finalize it by removing
                 # the trailing GVC key and reset the text accumulator.
@@ -372,10 +381,10 @@ def _parse_mt940_gvcodes(purpose: str) -> dict[str, str | None]:
         else:
             text += char
 
-    if segment_type:  # pragma: no branch
+    if segment_type:
         tmp[segment_type] = text
     else:
-        tmp[''] = text  # pragma: no cover
+        tmp[''] = text
 
     for key, value in tmp.items():
         result[GVC_KEYS[key]] = value

mt940/tags.py

@@ -78,6 +78,7 @@
 import logging
 import re
 import typing
+from typing import ClassVar
 
 from . import models
 
@@ -90,12 +91,14 @@
     """
 
     id: str | int = 0
-    RE_FLAGS = re.IGNORECASE | re.VERBOSE | re.UNICODE
-    scope: type[models.Transactions | models.Transaction] = models.Transactions
-    pattern: str
-    name: str
-    slug: str
-    logger: logging.Logger
+    RE_FLAGS: ClassVar[re.RegexFlag] = re.IGNORECASE | re.VERBOSE | re.UNICODE
+    scope: ClassVar[type[models.Transactions | models.Transaction]] = (
+        models.Transactions
+    )
+    pattern: ClassVar[str]
+    name: ClassVar[str]
+    slug: ClassVar[str]
+    logger: ClassVar[logging.Logger]
 
     def __init__(self) -> None:
         self.re = re.compile(self.pattern, self.RE_FLAGS)
@@ -117,7 +120,7 @@
             RuntimeError: If the value does not match the tag's pattern.
         """
         match = self.re.match(value)
-        if match:  # pragma: no branch
+        if match:
             self.logger.debug(
                 'matched (%d) %r against "%s", got: %s',
                 len(value),
@@ -126,7 +129,7 @@
                 match.groupdict(),
             )
             return match.groupdict()
-        else:  # pragma: no cover
+        else:
             self.logger.error(
                 'matching id=%s (len=%d) "%s" against\n    %s',
                 self.id,
@@ -139,13 +142,20 @@
                 f'Unable to parse {self!r} from {value!r}', self, value
             )
 
-    def _debug_partial_match(self, value: str) -> None:  # pragma: no cover
+    def _debug_partial_match(self, value: str) -> None:
         """
         Helper function to debug partial matches against the pattern.
         """
         part_value = value
         for pattern in self.pattern.split('\n'):
-            match = re.match(pattern, part_value, self.RE_FLAGS)
+            try:
+                match = re.match(pattern, part_value, self.RE_FLAGS)
+            except re.error:
+                # Single lines of a pattern with multi-line groups are not
+                # valid patterns on their own; skip them instead of masking
+                # the RuntimeError raised by `parse`.
+                self.logger.info('cannot compile fragment %r', pattern)
+                continue
             if match:
                 self.logger.info(
                     'matched %r against %r, got: %s',
@@ -211,13 +221,24 @@
     (?P<day>\d{2})
     (?P<hour>\d{2})
     (?P<minute>\d{2})
-    (\+(?P<offset>\d{4})|)
+    ((?P<offset_sign>[+-])(?P<offset>\d{4}))?
     """
 
     def __call__(
         self, transactions: models.Transactions, value: dict[str, typing.Any]
     ) -> dict[str, object]:
         data = super().__call__(transactions, value)
+        # The offset subfield is a signed HHMM value (e.g. +0130 is 1 hour
+        # and 30 minutes east of UTC), while `models.DateTime` expects the
+        # offset as a number of minutes. Convert it here and drop the raw
+        # groups so they are not passed on when the offset is absent.
+        sign: str | None = data.pop('offset_sign', None)
+        offset: str | None = data.pop('offset', None)
+        if offset:
+            minutes: int = int(offset[:2]) * 60 + int(offset[2:])
+            if sign == '-':
+                minutes = -minutes
+            data['offset'] = minutes
         return {'date': models.DateTime(**data)}
 
 
@@ -285,8 +306,14 @@
             dict[str, str],
             super().__call__(transactions, value),
         )
-        if data['status']:
-            key = data['status'].lower() + '_floor_limit'
+        # Normalize the D/C mark: a space (sent by e.g. Fiducia/Volksbank,
+        # d36c51b) means "both", like an absent mark, and a lowercase mark
+        # (the patterns match case-insensitively) must behave like its
+        # uppercase form so the debit amount is negated consistently.
+        status: str = data['status'].strip().upper()
+        if status:
+            data['status'] = status
+            key: str = status.lower() + '_floor_limit'
             return {key: models.Amount(**data)}
         data_d = data.copy()
         data_c = data.copy()
@@ -309,18 +336,15 @@
     Pattern: `2!n35x | *x`
     """
 
     scope = models.TransactionsAndTransaction
     id = 'NS'
 
+    # NS content is bank specific and free-form, so accept anything
+    # (including multi-line values whose lines do not all start with a
+    # two-digit sub-tag); `__call__` extracts the `2!n35x` structure per
+    # line where present.
     pattern = r"""
-    (?P<non_swift>
-        (
-            (\d{2}.{0,})
-            (\n\d{2}.{0,})*
-        )|(
-            [^\n]*
-        )
-    )
+    (?P<non_swift>[\s\S]*)
     $"""
     sub_pattern = r"""
     (?P<ns_id>\d{2})(?P<ns_data>.{0,})
@@ -340,10 +364,13 @@
                 ns = frag.groupdict()
                 value['non_swift_' + ns['ns_id']] = ns['ns_data']
                 text.append(ns['ns_data'])
-            elif len(text) and text[-1]:
-                text.append('')
             elif line.strip():
+                # Free-form line without a two-digit sub-tag: keep the
+                # content instead of dropping it.
                 text.append(line.strip())
+            elif len(text) and text[-1]:
+                # Blank line: collapse runs into one paragraph separator.
+                text.append('')
         value['non_swift_text'] = '\n'.join(text)
         value['non_swift'] = data
         return value
@@ -596,8 +623,13 @@
 
     id = 86
     scope = models.Transaction
+    # The SWIFT spec caps this field at 6 lines of 65 characters, but many
+    # banks send more. A previous cap of nine 65-char chunks silently
+    # truncated anything longer, so the capture is unbounded: the parser in
+    # `models.Transactions.parse` already limits the value to this tag's own
+    # slice of the statement.
     pattern = r"""
-    (?P<transaction_details>(([\s\S]{0,65}\r?\n?){0,8}[\s\S]{0,65}))
+    (?P<transaction_details>[\s\S]*)
     """
 
 
@@ -610,7 +642,7 @@
     (?P<currency>.{3})  # 3!a Currency
     (?P<amount>[\d,]{1,15})  # 15d Amount
     """
-    status: str
+    status: ClassVar[str]
 
     def __call__(
         self, transactions: models.Transactions, value: dict[str, typing.Any]