From c9b8f23663d69f7109cf9cd4d613e52ab8a2f7e3 Mon Sep 17 00:00:00 2001 From: dev-belly Date: Sun, 11 Oct 2026 12:11:31 +0800 Subject: [PATCH] Close SQLite handles before atomic warehouse replacement --- .github/workflows/ci.yml | 8 +++++-- README.md | 21 ++++++++++-------- src/database.py | 12 +++++++---- tests/test_warehouse_contract.py | 37 ++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4353139..fb85479 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,12 +21,16 @@ permissions: jobs: test: - name: Python ${{ matrix.python-version }} - runs-on: ubuntu-latest + name: Python ${{ matrix.python-version }} (${{ matrix.os }}) + runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: + os: [ubuntu-latest] python-version: ["3.11", "3.12"] + include: + - os: windows-latest + python-version: "3.12" steps: - uses: actions/checkout@v7 diff --git a/README.md b/README.md index 81fbf27..04edbda 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![CI](https://github.com/dev-belly/AuditLens/actions/workflows/ci.yml/badge.svg)](https://github.com/dev-belly/AuditLens/actions/workflows/ci.yml) [![Python 3.11+](https://img.shields.io/badge/python-3.11%2B-blue.svg)](https://www.python.org/downloads/) [![License: MIT](https://img.shields.io/badge/license-MIT-green.svg)](LICENSE) -[![Tests: 464](https://img.shields.io/badge/tests-464%20passing-brightgreen.svg)](#testing) +[![Tests: 465](https://img.shields.io/badge/tests-465%20passing-brightgreen.svg)](#testing) **Financial anomaly detection and audit analytics over a 30,000-voucher general ledger.** @@ -58,7 +58,7 @@ Three design commitments drive everything else: | Anomalies caught by neither | **15** | | Isolation Forest | ROC-AUC **0.816**, precision 0.291, recall 0.286 | | Benford first-digit MAD | **0.00218** — close conformity | -| Test suite | **464 tests**, including dashboard render, warehouse and review-plan checks | +| Test suite | **465 tests**, including dashboard render, warehouse and review-plan checks | ### The most important number here is 98.2%, and it is a warning @@ -565,7 +565,7 @@ AuditLens/ │ ├── components.py # KPI cards, risk badges, charts, tables │ └── views/ # the six pages ├── sql/audit_queries.sql # 15 named business queries -├── tests/ # 464 tests, incl. cross-process reproducibility +├── tests/ # 465 tests, incl. cross-process reproducibility ├── docs/ │ ├── architecture.md # design decisions, data contracts, what is deliberately excluded │ ├── methodology.md # every threshold, every weight, every mistake @@ -579,7 +579,7 @@ AuditLens/ │ ├── notebook_content.py # what the three notebooks contain, kept apart from the builder │ ├── capture_screenshots.py # headless captures of all six dashboard pages, via DevTools Protocol │ └── ci_summary.py # headline figures for the CI job summary -├── .github/workflows/ci.yml # pipeline + 464 tests + a reproducibility check, on 3.11 and 3.12 +├── .github/workflows/ci.yml # pipeline + 465 tests; Ubuntu 3.11/3.12 and Windows 3.12 └── Makefile ``` @@ -750,7 +750,7 @@ builder, so two consecutive builds are byte-identical. ```bash pip install -r requirements.txt python src/run_pipeline.py # ~30 seconds, deterministic -python -m pytest tests/ -q # 464 tests +python -m pytest tests/ -q # 465 tests ``` Two consecutive runs produce byte-identical reports under `outputs/reports/`. This is @@ -758,7 +758,7 @@ enforced by `tests/test_reproducibility.py`, not assumed. ## Testing -464 tests. `make test` runs the lot; `make test-fast` skips the dashboard render suite. +465 tests. `make test` runs the lot; `make test-fast` skips the dashboard render suite. | File | Tests | What it pins down | |---|---|---| @@ -777,7 +777,7 @@ enforced by `tests/test_reproducibility.py`, not assumed. | `test_documentation.py` | 19 | The testing table lists every test file, refers to no deleted ones, sums to the badge, and — the check that was missing — matches what pytest actually collects, per file and in total. Also pins the project-structure tree against the modules on disk, every relative link and embedded image against the filesystem, and the Makefile's `.PHONY` declaration against the targets it actually defines, against `make help`'s output, and against every `make` command the docs name | | `test_screenshots.py` | 11 | The six dashboard pages are named in four places — the `st.Page` titles, the `page_header` strings the views render, the capture tool's click labels and expected headings, and the committed PNGs — and all four must agree. Checked in both directions, so a rename cannot leave an orphan capture that the README still displays | | `test_readme_claims.py` | 27 | Every headline figure, the review-plan figures, all nine rules' flagged/precision/recall, the Benford table and every MAD quoted in the prose, the risk-band counts and values, the component weights, and the structural counts the prose quotes — checked against `outputs/reports/` and against the code | -| `test_warehouse_contract.py` | 10 | Rejects partial SQLite builds, duplicate keys, orphaned or missing alerts and stale summary totals; preserves the prior warehouse when reconciliation fails | +| `test_warehouse_contract.py` | 11 | Rejects partial SQLite builds, duplicate keys, orphaned or missing alerts and stale summary totals; preserves the prior warehouse when reconciliation fails and closes check/read connections before replacement | | `test_review_plan.py` | 18 | Fixed-budget coverage and risk ranking, random inclusion probabilities, label blindness, input-order invariance, complete rule identity, formula-safe CSV export, workpaper checksum and separate synthetic benchmark | | `test_review_outcomes.py` | 9 | A completed workpaper can edit review fields only; every finding needs evidence, counts and digests use the same CSV snapshot, and results remain route-specific counts without synthetic labels or population inference | @@ -790,8 +790,11 @@ seven methodology entries while the document held eight, and nothing compared th ### What CI verifies -[`.github/workflows/ci.yml`](.github/workflows/ci.yml) runs on every push, on Python -3.11 and 3.12, and checks the following claims: +[`.github/workflows/ci.yml`](.github/workflows/ci.yml) runs on every push, on Ubuntu +with Python 3.11 and 3.12 and on Windows with Python 3.12. SQLite check and read +connections close before the next warehouse replacement; a regression keeps +connection references alive to catch file locks without relying on garbage +collection. The workflow checks the following claims: 1. **The pipeline runs** from a clean checkout, on a different operating system from the one it was developed on. diff --git a/src/database.py b/src/database.py index 7cfc97a..e5986c2 100644 --- a/src/database.py +++ b/src/database.py @@ -35,7 +35,7 @@ import sys import tempfile from math import isclose, isfinite -from contextlib import contextmanager +from contextlib import closing, contextmanager from pathlib import Path from typing import Any, Iterator @@ -46,6 +46,7 @@ import pandas as pd from sqlalchemy import create_engine, text from sqlalchemy.engine import Engine +from sqlalchemy.pool import NullPool from src.utils import ( AUDIT_SUMMARY_REPORT, @@ -91,7 +92,9 @@ def get_engine(db_path: Path = DB_PATH) -> Engine: """Create a SQLAlchemy engine for the AuditLens warehouse.""" db_path.parent.mkdir(parents=True, exist_ok=True) - return create_engine(f"sqlite:///{db_path}", future=True) + # Connections must close when a reader finishes so Windows can replace the + # live warehouse on the next successful build. + return create_engine(f"sqlite:///{db_path}", future=True, poolclass=NullPool) @contextmanager @@ -113,7 +116,8 @@ def _staging_engine() -> Iterator[Engine]: engine = get_engine(staged) yield engine engine.dispose() - with sqlite3.connect(staged) as connection: + # sqlite3's transaction context commits/rolls back; it does not close. + with closing(sqlite3.connect(staged)) as connection: check = connection.execute("PRAGMA quick_check").fetchone()[0] if check != "ok": raise RuntimeError(f"Staged SQLite warehouse failed quick_check: {check}") @@ -503,7 +507,7 @@ def list_tables(db_path: Path = DB_PATH) -> list[str]: """Return the tables present in the warehouse.""" if not db_path.exists(): return [] - with sqlite3.connect(db_path) as connection: + with closing(sqlite3.connect(db_path)) as connection: rows = connection.execute( "SELECT name FROM sqlite_master WHERE type='table' ORDER BY name" ).fetchall() diff --git a/tests/test_warehouse_contract.py b/tests/test_warehouse_contract.py index e92738d..cc35242 100644 --- a/tests/test_warehouse_contract.py +++ b/tests/test_warehouse_contract.py @@ -3,6 +3,7 @@ from __future__ import annotations import sqlite3 +from contextlib import closing import pandas as pd import pytest @@ -148,6 +149,42 @@ def test_reconciled_warehouse_publishes_the_detail_and_summary(tmp_path, monkeyp assert connection.execute("SELECT total_amount FROM risk_summary").fetchone() == (300.0,) +def test_warehouse_closes_check_and_reader_connections_before_replacement( + tmp_path, monkeypatch +) -> None: + """Keep connection references alive so garbage collection cannot hide locks.""" + live_path = tmp_path / "auditlens.db" + monkeypatch.setattr(database, "DB_PATH", live_path) + original_connect = sqlite3.connect + original_replace = database.os.replace + connections = [] + + def track_connection(*args, **kwargs): + connection = original_connect(*args, **kwargs) + connections.append(connection) + return connection + + def replace_only_closed(source, destination): + for connection in connections: + with pytest.raises(sqlite3.ProgrammingError, match="closed database"): + connection.execute("SELECT 1") + original_replace(source, destination) + + monkeypatch.setattr(sqlite3, "connect", track_connection) + monkeypatch.setattr(database.os, "replace", replace_only_closed) + database.load_warehouse(*_reconciliation_inputs()) + assert "transactions" in database.list_tables(live_path) + engine = database.get_engine(live_path) + with engine.connect() as connection: + assert connection.exec_driver_sql("SELECT COUNT(*) FROM transactions").scalar() == 2 + # No dispose/GC between reading and rebuilding: pooled readers must not + # keep a Windows file handle alive after their context has ended. + database.load_warehouse(*_reconciliation_inputs()) + with closing(original_connect(live_path)) as connection: + assert connection.execute("PRAGMA quick_check").fetchone() == ("ok",) + assert not list(tmp_path.glob(".auditlens-build-*.db")) + + @pytest.mark.parametrize("corruption", ["missing_alert", "stale_total", "stale_flagged"]) def test_reconciliation_failure_preserves_previous_warehouse( tmp_path, monkeypatch, corruption: str