A subclass override skipped the validation its base class depended on

A partner's accounting team emailed us a CSV line that should not exist: an invoice with a total of -1240.00. Their importer had accepted it, booked it as a credit, and someone noticed only at month end. Our system does not have negative invoices. Credits are a separate document type with their own approval flow.

The CSV came from our export service. It has a small class hierarchy: a base InvoiceExporter and one subclass per partner format. The partner in question had moved to a new format two weeks earlier, served by a new subclass, LedgerExporterV2.

We pulled the exact invoice and re-ran it through both exporters in a shell:

>>> InvoiceExporter().export([inv])
ValueError: invoice INV-88213 has negative total -1240.00
>>> LedgerExporterV2().export([inv])
'INV-88213;2026-10-03;-1240.00;EUR\n'

The old path rejected it. The new one wrote it straight out. The data problem (a bad discount rule that produced a negative total) was real and we fixed it separately, but the bigger question was why the guard did not fire.

Reading the hierarchy

The base class looked like this:

class InvoiceExporter:
    def export(self, invoices):
        rows = []
        for inv in invoices:
            self._validate(inv)
            rows.append(self.format_row(inv))
        return "".join(rows)

    def _validate(self, inv):
        if inv.total < 0:
            raise ValueError(f"invoice {inv.number} has negative total {inv.total:.2f}")
        if not inv.currency:
            raise ValueError(f"invoice {inv.number} missing currency")

    def format_row(self, inv):
        return f"{inv.number},{inv.issued_on:%Y-%m-%d},{inv.total:.2f},{inv.currency}\n"

And the new subclass:

class LedgerExporterV2(InvoiceExporter):
    def export(self, invoices):
        # V2 wants invoices grouped by currency, so we override export
        out = []
        for currency, group in groupby(sorted(invoices, key=attrgetter("currency")),
                                       key=attrgetter("currency")):
            for inv in group:
                out.append(self.format_row(inv))
        return "".join(out)

    def format_row(self, inv):
        return f"{inv.number};{inv.issued_on:%Y-%m-%d};{inv.total:.2f};{inv.currency}\n"

The developer needed grouping, so they overrode export. The validation call lived inside export, so it went with it. Nothing in the code, the type hints or the tests said "anything that overrides export must call _validate". The rule existed only in the base class body.

The fragile base class problem

This is a classic failure of implementation inheritance, usually called the fragile base class problem. A subclass inherits not just methods but also the unwritten rules about how those methods work together. In our case:

• The base class had an invariant: every exported row has passed _validate.
• That invariant was enforced by the order of calls inside one overridable method.
• Overriding that method replaced the whole sequence, and the invariant disappeared without any error.

The Liskov Substitution Principle says a subclass should be usable anywhere the base class is, without breaking the guarantees callers rely on. LedgerExporterV2 was substitutable as far as the type checker knew: same method names, same signatures. It was not substitutable in behaviour, because it weakened a guarantee. Type systems check shapes; they do not check that you kept the base class's promises.

Why the review missed it

The pull request diff showed only the new subclass. A reviewer looking at it saw clean grouping logic and a format change. To spot the bug they would have needed to open the base class, notice that export did two jobs (validate and format), and realise only one was being kept. That is a lot to ask of a review, and it is exactly the kind of coupling inheritance hides.

Environment

We checked the other five subclasses. Two of them also overrode export. One called super().export() and was fine. The other had copied the validation in by hand, but an older version of it that did not check currency.

We made the invariant impossible to skip, then reduced how much subclasses could change.

Step 1: template method with a non-overridable entry point

First, the quick fix. The public method owns the sequence and is marked final; subclasses can only change the parts that are meant to vary:

from typing import final

class InvoiceExporter:
    @final
    def export(self, invoices):
        checked = [self._validate(inv) for inv in invoices]
        ordered = self.order(checked)
        return "".join(self.format_row(inv) for inv in ordered)

    def _validate(self, inv):
        if inv.total < 0:
            raise ValueError(f"invoice {inv.number} has negative total {inv.total:.2f}")
        if not inv.currency:
            raise ValueError(f"invoice {inv.number} missing currency")
        return inv

    def order(self, invoices):          # hook: subclasses may override
        return invoices

    def format_row(self, inv):          # hook: subclasses may override
        raise NotImplementedError

@final is enforced by type checkers such as mypy and pyright, not at runtime. With it in place, our CI type check rejects any subclass that overrides export:

exporters/ledger_v2.py:14: error: Cannot override final attribute "export"
(previously declared in base class "InvoiceExporter")  [misc]

The V2 subclass now overrides only order to group by currency. Validation runs no matter what.

Step 2: composition for the parts that vary

The template method closed the hole, but the hierarchy was still the wrong shape. The six "exporters" differed in two independent ways: how rows are ordered and how a row is formatted. Inheritance forced each combination into one class. We moved to composition: one exporter class that takes the varying parts as objects.

from dataclasses import dataclass
from typing import Callable, Iterable, Protocol

class RowFormat(Protocol):
    def __call__(self, inv: "Invoice") -> str: ...

def semicolon_row(inv) -> str:
    return f"{inv.number};{inv.issued_on:%Y-%m-%d};{inv.total:.2f};{inv.currency}\n"

def by_currency(invoices: Iterable["Invoice"]) -> list["Invoice"]:
    return sorted(invoices, key=lambda i: (i.currency, i.number))

@dataclass(frozen=True)
class Exporter:
    format_row: RowFormat
    order: Callable[[list], list] = lambda xs: xs

    def export(self, invoices) -> str:
        checked = [validate(inv) for inv in invoices]
        return "".join(self.format_row(inv) for inv in self.order(checked))

EXPORTERS = {
    "ledger_v2": Exporter(format_row=semicolon_row, order=by_currency),
    "classic":   Exporter(format_row=comma_row),
}

validate is a plain module function. There is no method to override and no base class whose internals a new partner format needs to understand. Adding a partner means writing one formatting function and one line in the registry.

When inheritance is still fine

We did not remove inheritance from the codebase. It works well when the base class is designed for it: small, documented hooks, a fixed public entry point, and invariants that live in code the subclass cannot replace. What we stopped doing is overriding a method that contains a sequence of steps and hoping the override keeps every step.

1. Shared behaviour tests across every exporter. One parametrised test runs the invariants against every entry in the registry, so a new partner format is covered the day it is added:

import pytest
from exporters import EXPORTERS

@pytest.mark.parametrize("name", sorted(EXPORTERS))
def test_rejects_negative_total(name, make_invoice):
    with pytest.raises(ValueError, match="negative total"):
        EXPORTERS[name].export([make_invoice(total=-1)])

@pytest.mark.parametrize("name", sorted(EXPORTERS))
def test_rejects_missing_currency(name, make_invoice):
    with pytest.raises(ValueError, match="missing currency"):
        EXPORTERS[name].export([make_invoice(currency="")])

Run against the old hierarchy, the negative-total test failed for ledger_v2 and the currency test failed for the subclass with the copied, outdated validation. Both pass on the new code.

2. Output equivalence. Before switching, we exported the last 30 days of invoices with the old and new code for every partner and diffed the files. They were byte-identical except for the rows the old V2 exporter should have rejected.

3. Type check gate. mypy --strict exporters/ runs in CI. We confirmed it fails on a test branch that overrides export.

4. Security and data implications. A negative invoice is effectively a credit that skipped the credit approval flow, so this was a financial control bypass, not just a formatting bug. We worked with the partner to reverse the one booked credit, searched the export history for any other negative totals (none), and added an alert when any exporter raises a validation error so the next data problem is visible to us before a partner finds it.

Takeaways

• An override replaces everything the method did, including checks you did not know were there.
• Keep invariants in code subclasses cannot replace: a final entry point or a plain function.
• When classes vary along independent axes, compose small objects instead of building a class per combination.
• Test invariants against every implementation, not just the base class.

References: typing.final, mypy final names and methods, Python dataclasses.