Ban commits/transactions using AST analysis and linters

I’ve been working on a spec for months. Finally…every stakeholder approves. I’ve cataloged every piece of code that needs to be migrated in a Notion DB. I’m working on one of the first PRs and then I see it…

“…ffffuuuuuummmmccckkkk mmmmmmeee…”

The fucking “transactions” are being committed while wearing a coat of atomicity.

“fuck, fuck, fuck, how the fuck did I miss this.”

Looking through my notes…I see…“investigate random-ass commits” and I forgot to account for them…

• ORMs can blow your leg off, this ain’t that rant.
• Parametrized SQL statements can be your saviour, this ain’t that rant.
• Query builders can be a nice middle ground, this isn’t that rant.

This rant is about fucking up code organization and abstractions so badly that code wrapped inside transactions ain’t atomic.

Sample Exhibits

These are meant to be illustrative, so code has been cleaned up and doesn’t contain all the tiny details of any framework/library. Sessions being created, etc.

The Hidden Enemy

class DBAccess:
    @staticmethod
    def create_records(records: List[DomainModel]):
        # All records are meant to be a single transaction
        with transaction():
            for r in records:
                DBAccess.create_main_records(r)           # calls helpers that eventually call commit()
                # Another transaction is auto-started
                DBAccess.create_details_records(r)

DBAccess.create_records(recs)

The code is fucking around with manual commits 2+ levels away from the transaction decorator/context manager. So, when create_main_records is called from the manager, no one has ANY god damn idea that the method commits.

The Silent Frenemy

class DBAccess:
    @staticmethod
    def fetch_records(ids: List[int]) -> List[DBModel]:
        db_models = session.query(DBModel).filter(DBModel.id.in_(ids)).all()
        return cast(List[DBModel], db_models)


with transaction():
    db_models = DBAccess.fetch_records(ids)
    db_models[0].yo_mama_fat = True                     # This is a silent DB write
# context manager exit saves your ass

The code is passing DB models around, treating them like normal domain models, setting properties that seem to do nothing other than setting the value but underneath it all, there are DB writes being triggered.

Daddy’s Milk Run

class DBAccess:
    @staticmethod
    def fetch_records(ids: List[int]) -> List[DBModel]:
        db_models = session.query(DBModel).filter(DBModel.id.in_(ids)).all()
        return cast(List[DBModel], db_models)

# transaction has been commented out
# with transaction():
db_models = DBAccess.fetch_records(ids)
db_models[0].yo_mama_fat = True
# request ends, data poofs into the ether
# like the programmer's dad leaving to get milk and never returning

The code doesn’t auto-commit and has no transactions, it’s causing data loss.

Who is responsible?

• DO NOT blame the framework
• DO NOT blame the business requirement.

The programmer is the only one responsible. In a lot of cases, you can be absolved of the blame for the state of the code, but NOT in this case. ANYTHING is better than random ass commits.

In the end, your ass, whether you wrote the code or not (my case) is on the line for fixing this shit. Like I have been for months.

WTF are we learning?

1. DO NOT sprinkle in DB sessions or transactions like salt-bae. If you do so, expect to crash and burn like him. The DB abstraction layer owns the transactions and commits.
2. DO NOT pass DB models in and out of the DB layer.
3. Read 1+2 again, then read them again. Absorb them.
4. DO NOT fuck with transactions, commits, queries or whatever outside of the DB layer. Don’t even look at it, don’t even think about it.
5. DO NOT commit manually. Especially if you use context managers or decorators. You are not that guy/gal pal.
6. DO NOT split code across helpers. Atomic multi-writes = one function. Write the 3 line god-damn insert again. The duplication keeps atomicity visible and you won’t create hidden enemies. Otherwise, watch your god-damn back.

How do you enforce the lessons?

AST Analysis:

Either through custom tests that use AST analysis or through flake8/linters. Whatever you choose, it should:

• Ban commits from being called manually, COMPLETELY
• Ban db session from being accessed outside the DB access layer
• Ban transactions from being accessed outside the DB access layer
• Ban DB model imports from outside the DB access layer

Bans using AST

class TestDBBoundaries:
    def test_no_manual_commits(self):
        violations = []
        for path, tree in parsed_source_files():         # ast.parse over your source tree
            for node in ast.walk(tree):
                if not isinstance(node, ast.Call):
                    continue
                func = node.func
                if (
                    (isinstance(func, ast.Attribute) and func.attr == "commit")  # session.commit()
                    or (isinstance(func, ast.Name) and func.id == "commit")      # commit = session.commit; commit()
                ):
                    violations.append(f"{path}:{node.lineno}  manual commit()")
        assert not violations, "\n".join(violations)

Bans using flake8

Same walk, wearing flake8’s trench coat:

import ast


class BanManualCommits:
    def __init__(self, tree):
        self.tree = tree

    def run(self):
        for node in ast.walk(self.tree):
            if not isinstance(node, ast.Call):
                continue
            func = node.func
            if (
                (isinstance(func, ast.Attribute) and func.attr == "commit")
                or (isinstance(func, ast.Name) and func.id == "commit")
            ):
                yield node.lineno, node.col_offset, "DB001 manual commit()", type(self)

What the registration looks like:

[project.entry-points."flake8.extension"]
DB0 = "flake8_db_boundaries:BanManualCommits"

AST or flake8?

Use AST tests when:

• the rule needs to look at the whole codebase
• you can’t touch the shared linter config cause that’ll (rightfully) callout other teams’ shitty code and egos are too fragile

Otherwise stick with flake8/linters. They are the standard tools for a reason.

Combined with LLM Support

LLMs should be used to:

• Ban any DB models from being returned by the DB layer

AST can’t catch this one and neither can mypy, cause:

• return annotations lie
• cast() launders types (The Silent Frenemy)
• a codebase with random-ass commits doesn’t inspire confidence in the typing

So this check is an LLM pass in the CI/CD pipeline. A deterministic script dumps the DB access layer’s public functions/methods, then the LLM answers a single question: “does anything return a DB model instead of a domain model?” No agentic drama, create a prompt and throw the output from the script in there. Results:

• yes -> human review
• maybe -> human review
• no -> pass Go, collect your paycheque

✌️

If you find me insufferable, fine. The one take away:

The DB abstraction layer owns the commits and transactions. Everything else is a workaround for getting that wrong.

So, you don’t have to spend MONTHS refactoring the whole god-damn codebase to fix fucked up transactions after discovering a random-ass commit.

ALSO READ THIS GOD-DAMN BOOK: Domain-Driven Design: Tackling Complexity in the Heart of Software.