Skip to content
Engineering Notes

A variable I refactored into one function — and kept referencing from another. Python lazy evaluation hid it, and an AST test finally caught it

One day the browser automation flow started failing right after plugin updates with NameError: name 'plugin_form_selectors' is not defined in the post-update “residual check” step.

The refactor that introduced this had landed back in v1.6.1. The error didn’t surface until many rounds later. Reading the code, the cause is obvious in seconds — but nobody hit it for ages, because Python’s lazy evaluation kept the leftover reference hidden until exactly the right execution path ran. This post walks through what the bug was and how we structurally prevented its kind via an AST static-analysis test.

What happened — a reference that crossed a scope boundary

browser_utils.py has two functions involved: run_browser_update_flow(), which orchestrates the whole update flow, and browser_update_remaining_plugins(), which handles only the plugin-update logic. The list of plugin-form selector candidates, plugin_form_selectors, used to be a local variable inside run_browser_update_flow().

In the v1.6.1 refactor — “let’s split plugin update into its own function” — we created browser_update_remaining_plugins() and moved the plugin_form_selectors definition into it.

# After v1.6.1 refactor
def browser_update_remaining_plugins(page, site, update_url):
    plugin_form_selectors = [        # ← defined here
        '#update-plugins-table-wrap form',
        'form[name="upgrade-plugins"]',
        'form[action*="do-plugin-upgrade"]',
        '.plugins-php form',
    ]
    # ... update logic ...


def run_browser_update_flow(site, page):
    # ... call to plugin updater ...
    browser_update_remaining_plugins(page, site, update_url)

    # ★ post-update "residual check" still uses the old local name
    for selector in plugin_form_selectors:   # NameError
        if page.locator(selector).count() > 0:
            pending_browser.append(...)

The “after updating, make sure no plugin update forms are still visible” residual check stayed in run_browser_update_flow(). During the refactor, the call to extract this loop alongside the update logic — or alternatively to wire it to a different source — got missed. The result: a piece of code that tried to read a function-local variable from outside that function. A simple mistake in shape.

Why it took several rounds to surface — Python’s lazy evaluation

A Python NameError happens when that line of code actually executes, not when the module is imported. The name plugin_form_selectors in for selector in plugin_form_selectors: doesn’t need to be bound at function-definition time — Python only tries to resolve it when execution reaches that loop.

In practical terms:

  • App startup: fine (nobody calls the function)
  • Sites where SSH-based plugin updates succeed: fine (the browser-update path isn’t taken)
  • Sites where browser-based updates run: NameError, the moment that loop is reached

The bug only fires for sites that need browser automation. If you only run SSH-eligible sites, you never see it. Tests didn’t cover that path either. That’s why it took several rounds before someone hit the trigger.

Promote to a module-level constant; reference from both places

The direction is straightforward — put it somewhere both functions can see. Promote to a module-level constant.

# Top of module
PLUGIN_FORM_SELECTORS = [
    '#update-plugins-table-wrap form',
    'form[name="upgrade-plugins"]',
    'form[action*="do-plugin-upgrade"]',
    '.plugins-php form',
]


def browser_update_remaining_plugins(page, site, update_url):
    plugin_form_selectors = PLUGIN_FORM_SELECTORS    # bind to existing local name
    # ... update logic ...


def run_browser_update_flow(site, page):
    # ... call to plugin updater ...

    # ★ residual check now reads the same constant
    for selector in PLUGIN_FORM_SELECTORS:
        if page.locator(selector).count() > 0:
            pending_browser.append(...)

Keeping the local name plugin_form_selectors inside the function is intentional — we want minimal diff inside the existing function body. The internal code reads the same; only the single source-of-truth has moved to module scope.

Why didn’t pyflakes / ruff catch this?

“Surely a linter would have flagged this?” is a fair question. pyflakes and ruff do statically detect undefined names via F821. The trap, though, is finer-grained.

The name plugin_form_selectors was defined as a local variable inside browser_update_remaining_plugins(), so to Python’s static-analysis view, it’s a name that exists somewhere in the project. When linters see the same name referenced inside run_browser_update_flow(), they can’t distinguish that it’s another function’s local variable. That’s the limit of pyflakes.

To be precise, pyflakes detects “not defined anywhere” rather than “not reachable from the current scope.” If the same name happens to be a local in some other function, it slips through.

The supplement: a scope-aware AST static-analysis test that we write ourselves.

The AST undefined-name detector — test_browser_undefined_names.py

We added a test that walks every function in browser_utils.py, and for each name reference, actually checks whether that name is reachable from that function’s scope:

import ast

def test_no_undefined_names_in_browser_utils():
    """Every name referenced inside a function in browser_utils.py must be
    resolvable to either: a module-level definition, a function argument,
    or a function-local definition. Otherwise: fail."""

    module = ast.parse(BROWSER_UTILS_PY.read_text())
    module_names = collect_module_level_definitions(module)
    builtin_names = set(dir(builtins))

    failures = []
    for func in walk_functions(module):
        # function args + names assigned inside the function
        local_names = collect_local_definitions(func)
        scope = module_names | local_names | builtin_names

        for name_node in walk_names(func, ctx=ast.Load):
            if name_node.id not in scope:
                failures.append(
                    f"{func.name}:{name_node.lineno} undefined: {name_node.id}"
                )

    assert not failures, "\n".join(failures)

This test applies the actual scoping rule that “function-local variables aren’t visible from other functions”, which fills the gap pyflakes leaves. A regression like our plugin_form_selectors case — another function’s local accidentally referenced — would now fail in CI.

The real implementation handles import statements, from ... import *, conditional definitions (if hasattr(...) else ...), etc. more rigorously than the sketch above, but the core idea is the same: re-solve name resolution at every function boundary, independently.

Lessons — close refactor leftovers with machine discipline

Three principles to take away:

  1. Refactors that promote a function-local out of its function need a grep companion. Whenever you extract or move a function-local variable, check whether other functions reference that name before you move it. Simple but easy to miss in the heat of a refactor. A single grep covers it
  2. Python’s lazy evaluation hides refactor leftovers for a long time. NameError only fires when execution reaches the line. Rarely-executed paths can hide the bug for months. Especially in code paths with low test coverage. This is a language characteristic — you can’t paper over it
  3. Scope-crossing references that F821 won’t catch — write an AST test to catch them. Standard linters detect “not defined anywhere” but not “not reachable from the current scope.” A scope-aware AST static-analysis test catches the “refactored-into-some-function” reference at CI time

This fix lives in the same family as the .first habit and AST test for Playwright strict-mode violation: both replace human attention with structural discipline enforced by the machine. In long-lived code like a WordPress maintenance tool, accumulating small AST tests like these one at a time visibly lowers the psychological cost of refactoring.