perf(test): replace per-instance _database_initialized flag with process-global cache (#1264)

Add a process-global `_INITIALIZED_DBS: set[str]` to `features/environment.py` that caches database URLs after `_fast_init_or_upgrade` has processed them. On subsequent calls with the same URL, the function returns immediately — skipping all URL parsing, path extraction, prefix matching, and `stat()` syscalls that previously executed on every new `UnitOfWork` instance.

The cache is cleared at the start of each scenario in `before_scenario` to prevent cross-scenario state leaks, since each scenario receives fresh temp-DB paths via `tempfile.mktemp()`.

Four new Behave scenarios validate cache hits on repeated calls, cache clearing between scenarios, and non-caching of non-matching-prefix URLs.

Estimated savings: ~65,000 unnecessary function-body executions eliminated per full test run (~7 UnitOfWork instantiations × ~10,700 scenarios).

ISSUES CLOSED: #735

Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>

features/environment.py

@@ -54,6 +54,21 @@ LANGSMITH_ENV_VARS = [
 
 _TDD_ISSUE_N_RE = re.compile(r"tdd_issue_\d+")
 
+# ---------------------------------------------------------------------------
+# Process-global set of already-initialized DB paths
+# ---------------------------------------------------------------------------
+# After ``_fast_init_or_upgrade`` has processed a given database URL (either
+# by copying the template or confirming the file is non-empty), the URL is
+# added to this set.  Subsequent calls with the same URL short-circuit at
+# the very top of the function, avoiding all URL parsing, path extraction,
+# prefix matching, and ``stat()`` syscalls.  The set is cleared in
+# ``before_scenario`` to prevent cross-scenario state leaks.
+#
+# See issue #735 — eliminates ~65,000 unnecessary function executions per
+# full test run.
+# ---------------------------------------------------------------------------
+_INITIALIZED_DBS: set[str] = set()
+
 _tdd_logger = logging.getLogger("cleveragents.testing.tdd_tags")
 
 
@@ -443,6 +458,7 @@ def _install_template_db_patch() -> None:
     def _fast_init_or_upgrade(self: Any, **kwargs: Any) -> None:
         """Replace Alembic migrations with fast alternatives.
 
+        - Process-global cache hit → immediate return (no work at all)
         - Non-SQLite databases → fall through to original
         - In-memory SQLite → ``Base.metadata.create_all()`` + alembic stamp
         - File-based SQLite with matching prefix → copy template
@@ -450,6 +466,12 @@ def _install_template_db_patch() -> None:
         """
         db_url: str = getattr(self, "database_url", "")
 
+        # Process-global fast-path: if we have already initialised this
+        # exact URL in the current scenario, skip all work.  Eliminates
+        # ~65k redundant function-body executions per full test run.
+        if db_url in _INITIALIZED_DBS:
+            return
+
         # Non-SQLite: always fall through
         if not db_url.startswith("sqlite"):
             return _original_init_or_upgrade(self, **kwargs)
@@ -479,11 +501,13 @@ def _install_template_db_patch() -> None:
         # creation, SQLite lock contention, and the cumulative overhead that
         # causes intermittent hangs in parallel test runs.
         if db_path.exists() and db_path.stat().st_size > 0:
+            _INITIALIZED_DBS.add(db_url)
             return
 
         # Copy the template — creates a fully-migrated DB in ~1ms
         db_path.parent.mkdir(parents=True, exist_ok=True)
         shutil.copy2(template_path, db_file_path)
+        _INITIALIZED_DBS.add(db_url)
 
     MigrationRunner.init_or_upgrade = _fast_init_or_upgrade  # type: ignore[assignment]
 
@@ -502,6 +526,12 @@ def before_scenario(context, scenario):
         _tdd_logger.error("TDD TAG ERROR in %r: %s", scenario.name, exc)
         return
 
+    # Clear the process-global set of already-initialised DB paths so
+    # that stale entries from the previous scenario cannot leak into this
+    # one.  Each scenario receives fresh temp-DB paths, so old entries are
+    # irrelevant and would only waste memory.
+    _INITIALIZED_DBS.clear()
+
     # Store original working directory
     context.original_cwd = os.getcwd()
 

features/fast_init_upgrade.feature

@@ -57,3 +57,41 @@ Feature: Fast init_or_upgrade early-return behavior
     When I call init_or_upgrade with keyword arguments on the fast-init database
     Then the original init_or_upgrade should have been invoked exactly once
     And the original should have received the runner instance and keyword arguments
+
+  # -----------------------------------------------------------------------
+  # Process-global _INITIALIZED_DBS cache (issue #735)
+  # -----------------------------------------------------------------------
+
+  @mock_only
+  Scenario: Second call for the same non-empty DB skips all logic via global cache
+    Given a non-empty SQLite database file with a matching scenario prefix
+    And the initialized-DB cache is cleared
+    When I call init_or_upgrade on the fast-init database
+    And I call init_or_upgrade on the fast-init database a second time
+    Then the second init_or_upgrade call should have been a cache hit
+    And the database file content should be unchanged
+
+  @mock_only
+  Scenario: Second call for a newly-copied template DB skips all logic via global cache
+    Given a non-existent SQLite database path with a matching scenario prefix
+    And the initialized-DB cache is cleared
+    When I call init_or_upgrade on the fast-init database
+    And I call init_or_upgrade on the fast-init database a second time
+    Then the second init_or_upgrade call should have been a cache hit
+    And the database file should be a copy of the template
+
+  @mock_only
+  Scenario: Global cache is cleared between scenarios
+    Given a non-empty SQLite database file with a matching scenario prefix
+    And the initialized-DB cache is cleared
+    When I call init_or_upgrade on the fast-init database
+    Then the URL should be present in the initialized-DB cache
+    When the initialized-DB cache is cleared for a new scenario
+    Then the URL should not be present in the initialized-DB cache
+
+  @mock_only
+  Scenario: Non-matching prefix URLs are not added to the global cache
+    Given a SQLite database URL whose filename has a non-matching prefix
+    And the initialized-DB cache is cleared
+    When I call init_or_upgrade on the fast-init database
+    Then the URL should not be present in the initialized-DB cache

features/steps/fast_init_upgrade_steps.py

@@ -282,3 +282,103 @@ def step_assert_file_is_template_copy(context: Context) -> None:
         context.fast_init_template_path,
         shallow=False,
     ), "Database file does not match the template"
+
+
+# ---------------------------------------------------------------------------
+# Steps — process-global _INITIALIZED_DBS cache (issue #735)
+# ---------------------------------------------------------------------------
+
+
+def _get_initialized_dbs() -> set[str]:
+    """Return the ``_INITIALIZED_DBS`` set used by the ``_fast_init_or_upgrade`` closure.
+
+    Behave's ``Runner`` loads environment.py via ``exec_file()`` into a
+    private hooks dict — **not** via Python's import system.  This means
+    module-level variables defined in environment.py exist in the exec'd
+    globals, which is a *separate* namespace from
+    ``features.environment.__dict__``.  To access the same set that the
+    closure actually mutates at runtime, we read it from the closure
+    function's ``__globals__`` dict.
+    """
+    from cleveragents.infrastructure.database.migration_runner import (
+        MigrationRunner,
+    )
+
+    fast_fn: Any = MigrationRunner.init_or_upgrade
+    if not isinstance(fast_fn, types.FunctionType):
+        raise RuntimeError(
+            "MigrationRunner.init_or_upgrade is not a plain function — "
+            "the _fast_init_or_upgrade patch does not appear to be installed."
+        )
+    cache: set[str] = fast_fn.__globals__.get("_INITIALIZED_DBS", set())
+    return cache
+
+
+@given("the initialized-DB cache is cleared")
+def step_clear_initialized_db_cache(context: Context) -> None:
+    """Explicitly clear the global cache before exercising the fast-path."""
+    _get_initialized_dbs().clear()
+
+
+@when("I call init_or_upgrade on the fast-init database a second time")
+def step_call_init_or_upgrade_second_time(context: Context) -> None:
+    """Invoke init_or_upgrade a second time for the same URL.
+
+    The second call should be a cache hit — ``_fast_init_or_upgrade``
+    returns immediately without executing any of its body.  We record
+    whether the mock original was invoked during this second call.
+    """
+    from cleveragents.infrastructure.database.migration_runner import (
+        MigrationRunner,
+    )
+    from features.mocks.fast_init_test_helpers import (
+        patch_original_init_or_upgrade,
+    )
+
+    runner = MigrationRunner(database_url=context.fast_init_db_url)
+
+    with patch_original_init_or_upgrade() as mock_original:
+        runner.init_or_upgrade()
+        context.fast_init_second_call_mock_called = mock_original.called
+        context.fast_init_second_call_mock_count = mock_original.call_count
+
+
+@then("the second init_or_upgrade call should have been a cache hit")
+def step_assert_second_call_cache_hit(context: Context) -> None:
+    """Assert the second call was short-circuited by the global cache.
+
+    A cache hit means the function returned immediately — neither the
+    original ``init_or_upgrade`` nor any of the URL-parsing / stat logic
+    was executed.
+    """
+    assert not context.fast_init_second_call_mock_called, (
+        "Expected second init_or_upgrade call to be a cache hit "
+        "(no delegation to original), but the original was invoked "
+        f"{context.fast_init_second_call_mock_count} time(s)."
+    )
+
+
+@then("the URL should be present in the initialized-DB cache")
+def step_assert_url_in_cache(context: Context) -> None:
+    """Assert the fast-init database URL was added to _INITIALIZED_DBS."""
+    cache = _get_initialized_dbs()
+    assert context.fast_init_db_url in cache, (
+        f"Expected URL {context.fast_init_db_url!r} to be present in "
+        f"_INITIALIZED_DBS, but the cache contains: {cache}"
+    )
+
+
+@when("the initialized-DB cache is cleared for a new scenario")
+def step_simulate_scenario_boundary_cache_clear(context: Context) -> None:
+    """Simulate the cache clearing that happens in before_scenario."""
+    _get_initialized_dbs().clear()
+
+
+@then("the URL should not be present in the initialized-DB cache")
+def step_assert_url_not_in_cache(context: Context) -> None:
+    """Assert the fast-init database URL is not in _INITIALIZED_DBS."""
+    cache = _get_initialized_dbs()
+    assert context.fast_init_db_url not in cache, (
+        f"Expected URL {context.fast_init_db_url!r} NOT to be in "
+        f"_INITIALIZED_DBS, but it was found. Cache: {cache}"
+    )