fix(test): add Alembic revision freshness check to _ensure_template_db() #731

New issue

Open

opened 2026-03-12 13:18:47 +00:00 by CoreRasurae · 0 comments

CoreRasurae commented

2026-03-12 13:18:47 +00:00

Member

Metadata

Commit Message: fix(test): add Alembic revision freshness check to _ensure_template_db()
Branch: fix/m3-template-db-freshness

Summary

The _ensure_template_db() function in features/environment.py reuses an existing template DB if template_path.is_file() returns true, with no check that the template's Alembic revision matches the current HEAD. If schema changes are made but the template isn't regenerated (e.g., in local developer workflows or when build/ isn't cleaned), all tests silently use a stale schema.

Background and Context

Identified during the code review of PR #727 (Finding #8 in the APPROVED review, Bug Detection Review Findings #1/#2, and Performance Review P7 by @hurui200320). This is a pre-existing issue not introduced by that PR. The PR's change (bare return for existing non-empty DBs) removed one layer of defense-in-depth — previously, the original _original_init_or_upgrade could detect and apply pending migrations as a safety net. Adding a freshness check is the proper fix.

Current Behavior

# features/environment.py, _ensure_template_db()
if template_path.is_file():
    os.environ["CLEVERAGENTS_TEMPLATE_DB"] = str(template_path.resolve())
    return  # Reused without verifying Alembic revision

The template is reused unconditionally. If the Alembic migration set has changed since the template was created, tests run against a stale schema.

Expected Behavior

_ensure_template_db() should verify the cached template's Alembic revision matches the current Alembic HEAD before reusing it. If they differ, the template should be recreated. For example:

if template_path.is_file():
    # Quick freshness check: compare template's alembic_version against HEAD
    engine = create_engine(f"sqlite:///{template_path}")
    with engine.connect() as conn:
        result = conn.execute(text("SELECT version_num FROM alembic_version"))
        template_rev = result.scalar()
    engine.dispose()
    current_head = get_alembic_head_revision()  # from script directory
    if template_rev == current_head:
        os.environ["CLEVERAGENTS_TEMPLATE_DB"] = str(template_path.resolve())
        return
    # Stale — fall through to recreate

Acceptance Criteria

_ensure_template_db() verifies the template's Alembic revision before reusing
A stale template is automatically recreated when the Alembic HEAD has changed
All tests pass after adding a new (empty) Alembic migration and running without cleaning build/
No measurable performance regression in before_all() (the freshness check should be < 50ms)

Supporting Information

Identified in: PR #727 Code Review Finding #8, Bug Detection Findings #1/#2, Performance Review P7
Review URL: #727
Reviewer: @hurui200320

Subtasks

Implement Alembic HEAD revision lookup (from script directory or config)
Add revision comparison to _ensure_template_db() reuse path
Delete stale template and recreate when revision mismatch detected
Test: verify stale template is detected and recreated
Run nox -s unit_tests to verify no regressions

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

## Metadata - **Commit Message**: `fix(test): add Alembic revision freshness check to _ensure_template_db()` - **Branch**: `fix/m3-template-db-freshness` ## Summary The `_ensure_template_db()` function in `features/environment.py` reuses an existing template DB if `template_path.is_file()` returns true, with no check that the template's Alembic revision matches the current HEAD. If schema changes are made but the template isn't regenerated (e.g., in local developer workflows or when `build/` isn't cleaned), all tests silently use a stale schema. ## Background and Context Identified during the code review of PR #727 (Finding #8 in the APPROVED review, Bug Detection Review Findings #1/#2, and Performance Review P7 by @hurui200320). This is a pre-existing issue not introduced by that PR. The PR's change (bare `return` for existing non-empty DBs) removed one layer of defense-in-depth — previously, the original `_original_init_or_upgrade` could detect and apply pending migrations as a safety net. Adding a freshness check is the proper fix. ## Current Behavior ```python # features/environment.py, _ensure_template_db() if template_path.is_file(): os.environ["CLEVERAGENTS_TEMPLATE_DB"] = str(template_path.resolve()) return # Reused without verifying Alembic revision ``` The template is reused unconditionally. If the Alembic migration set has changed since the template was created, tests run against a stale schema. ## Expected Behavior `_ensure_template_db()` should verify the cached template's Alembic revision matches the current Alembic HEAD before reusing it. If they differ, the template should be recreated. For example: ```python if template_path.is_file(): # Quick freshness check: compare template's alembic_version against HEAD engine = create_engine(f"sqlite:///{template_path}") with engine.connect() as conn: result = conn.execute(text("SELECT version_num FROM alembic_version")) template_rev = result.scalar() engine.dispose() current_head = get_alembic_head_revision() # from script directory if template_rev == current_head: os.environ["CLEVERAGENTS_TEMPLATE_DB"] = str(template_path.resolve()) return # Stale — fall through to recreate ``` ## Acceptance Criteria - [ ] `_ensure_template_db()` verifies the template's Alembic revision before reusing - [ ] A stale template is automatically recreated when the Alembic HEAD has changed - [ ] All tests pass after adding a new (empty) Alembic migration and running without cleaning `build/` - [ ] No measurable performance regression in `before_all()` (the freshness check should be < 50ms) ## Supporting Information - Identified in: PR #727 Code Review Finding #8, Bug Detection Findings #1/#2, Performance Review P7 - Review URL: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/727 - Reviewer: @hurui200320 ## Subtasks - [ ] Implement Alembic HEAD revision lookup (from script directory or config) - [ ] Add revision comparison to `_ensure_template_db()` reuse path - [ ] Delete stale template and recreate when revision mismatch detected - [ ] Test: verify stale template is detected and recreated - [ ] Run `nox -s unit_tests` to verify no regressions ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.

CoreRasurae added the

labels

2026-03-12 13:20:24 +00:00