diff --git a/CHANGELOG.md b/CHANGELOG.md index b889583..1620b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,13 @@ public API may still change between minor versions. ### Fixed +- **A failed store open no longer leaks its SQLite connection.** `SQLiteConnection` opened the + underlying `sqlite3` connection before applying its setup PRAGMAs, and `SQLiteTraceStore` + opened the connection before creating its schema — so pointing either at a file that is not a + usable database raised the expected `DatabaseError` but left the already-open connection (and + its file descriptor) to the garbage collector. Both constructors now close the connection + before re-raising, so error paths (a typo'd `--db`, a corrupted golden baseline) clean up + after themselves. - **`RegressionRisk` no longer misses materially-changed or skipped critical steps.** The risk verdict was derived only from `removed`/`reordered` alignment states, but a critical step that was changed or skipped binds to a same-type event (type match alone clears the matcher diff --git a/conformance/conformance_event.py b/conformance/conformance_event.py index 02c3844..2d83444 100644 --- a/conformance/conformance_event.py +++ b/conformance/conformance_event.py @@ -12,25 +12,18 @@ from __future__ import annotations -import os -import sys import uuid -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import Dict, List, Tuple -# Make the package importable whether this module is loaded by the generator (run from -# anywhere) or by pytest (which adds ``src`` via conftest, but we must not depend on that). -_SRC = os.path.join(os.path.dirname(__file__), "..", "src") - -from dprovenancekit import ( # noqa: E402 - AlignmentConfiguration, +from dprovenancekit import ( AnyEquivalenceEvaluator, TraceableEvent, TraceEvent, TracePriority, TraceRun, ) -from dprovenancekit.query import ( # noqa: E402 +from dprovenancekit.query import ( AfterNode, AndNode, BeforeNode, diff --git a/demo/demo_gif.py b/demo/demo_gif.py index bfce6b9..db83b97 100644 --- a/demo/demo_gif.py +++ b/demo/demo_gif.py @@ -87,7 +87,7 @@ def main() -> None: candidate = record(store, "research-agent · PR-42", CANDIDATE) def line(label, run, color): - parts, prev = [], None + parts = [] for e in run.events: a = e.payload.type_identifier if parts and parts[-1][0] == a: @@ -105,7 +105,7 @@ def line(label, run, color): out(f"{DIM}$ dprovenancekit gate --golden main --candidate PR-42{R}", 0.7) report = RegressionGate().check(golden, candidate) rules = [ToolDropRule("verify"), LoopingRule("search", max_repeats=3)] - anomalies = AnomalyDetector(store).detect_anomalies(rules) + AnomalyDetector(store).detect_anomalies(rules) verdict = "PASS" if report.passed else "REGRESSION" out(f" {RED}{B}✗ {verdict}{R} {GREY}severity {R}{RED}{report.regression_level.value.upper()}{R}" diff --git a/dprovenancekit/sqlite_store.py b/dprovenancekit/sqlite_store.py index d5ceafd..469880d 100644 --- a/dprovenancekit/sqlite_store.py +++ b/dprovenancekit/sqlite_store.py @@ -46,9 +46,15 @@ class SQLiteConnection: def __init__(self, path: str): self._lock = threading.RLock() self._db = sqlite3.connect(path, isolation_level=None, check_same_thread=False) - self.execute("PRAGMA journal_mode=WAL;") - self.execute("PRAGMA synchronous=NORMAL;") - self.execute("PRAGMA temp_store=MEMORY;") + try: + self.execute("PRAGMA journal_mode=WAL;") + self.execute("PRAGMA synchronous=NORMAL;") + self.execute("PRAGMA temp_store=MEMORY;") + except BaseException: + # The first PRAGMA is where a non-database file surfaces as DatabaseError; + # the connection is already open by then and would leak without this. + self._db.close() + raise def execute(self, sql: str, params=()) -> None: with self._lock: @@ -340,16 +346,22 @@ def __init__( ): self._event_type = event_type self._db = SQLiteConnection(path) - self._buffer = TraceWriteBuffer( - max_global_buffer=max_global_buffer, max_per_run_buffer=max_per_run_buffer - ) - self._drop_tally = TraceDropTally() - self._writer = SQLiteWriter(self._db, self._buffer, self._drop_tally) + try: + self._buffer = TraceWriteBuffer( + max_global_buffer=max_global_buffer, max_per_run_buffer=max_per_run_buffer + ) + self._drop_tally = TraceDropTally() + self._writer = SQLiteWriter(self._db, self._buffer, self._drop_tally) - self._create_schema() + self._create_schema() - if start_writer: - self._writer.start() + if start_writer: + self._writer.start() + except BaseException: + # A failed __init__ never hands the store to the caller, so nobody else can + # close the connection we just opened. + self._db.close() + raise def _create_schema(self) -> None: db = self._db diff --git a/examples/basic_agent_trace.py b/examples/basic_agent_trace.py index 0ebe482..42bb015 100644 --- a/examples/basic_agent_trace.py +++ b/examples/basic_agent_trace.py @@ -1,4 +1,3 @@ -import uuid from dprovenancekit import ( DProvenanceKit, SQLiteTraceStore, diff --git a/examples/pr_gate_demo/agent.py b/examples/pr_gate_demo/agent.py index 92e3af5..f36c78f 100644 --- a/examples/pr_gate_demo/agent.py +++ b/examples/pr_gate_demo/agent.py @@ -1,6 +1,4 @@ import argparse -import sys -from pathlib import Path from langchain_core.runnables import RunnableLambda from dprovenancekit import SQLiteTraceStore from dprovenancekit.integrations.langchain import DProvenanceTracer, LangChainTraceEvent diff --git a/examples/regression_testing.py b/examples/regression_testing.py index 5139ec7..34e3b0e 100644 --- a/examples/regression_testing.py +++ b/examples/regression_testing.py @@ -32,7 +32,6 @@ import sys import uuid from dataclasses import dataclass -from typing import Optional # Make the package importable when run straight from a checkout (no install needed). sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) diff --git a/record_example.py b/record_example.py deleted file mode 100644 index f3d6f48..0000000 --- a/record_example.py +++ /dev/null @@ -1,45 +0,0 @@ - -import json -from dprovenancekit import DProvenanceKit, SQLiteTraceStore, AnyTraceableEvent, TracePriority - -DB = "my-traces.sqlite" -store = SQLiteTraceStore(AnyTraceableEvent, DB, start_writer=False) - -def record(context_id, steps): - """steps = list of (engine, action, detail). 'verify'/'decide' are marked CRITICAL.""" - kit = DProvenanceKit(AnyTraceableEvent) - with kit.run(context_id=context_id, store=store) as run: - rid = run.run_id - for engine, action, detail in steps: - prio = TracePriority.CRITICAL if action in ("verify", "decide") else TracePriority.STRUCTURAL - with kit.with_engine(engine): - kit.record(AnyTraceableEvent( - type_identifier_value=action, - priority_value=int(prio), - raw_json=json.dumps({"detail": detail}), - )) - return rid - -# The known-good baseline: -golden = record("my-agent · main", [ - ("planner", "plan", "break the task down"), - ("retriever", "search", "look up the docs"), - ("verifier", "verify", "cross-check two sources"), - ("planner", "decide", "final answer"), -]) - -# A regressed change (looped search, skipped verify): -candidate = record("my-agent · PR-1", [ - ("planner", "plan", "break the task down"), - ("retriever", "search", "look up the docs"), - ("retriever", "search", "retry"), - ("retriever", "search", "retry"), - ("retriever", "search", "retry"), - ("planner", "decide", "final answer"), # <-- no verify! -]) - -store.close() -print("db :", DB) -print("golden :", golden) -print("candidate:", candidate) - diff --git a/tests/conftest.py b/tests/conftest.py index 76c65c9..df52aaf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,8 +6,6 @@ from __future__ import annotations -import os -import sys import uuid from dataclasses import dataclass diff --git a/tests/integrations/test_fastapi.py b/tests/integrations/test_fastapi.py index 34f515d..842fa23 100644 --- a/tests/integrations/test_fastapi.py +++ b/tests/integrations/test_fastapi.py @@ -4,7 +4,6 @@ from dprovenancekit.kit import DProvenanceKit from dprovenancekit.store import InMemoryTraceStore from dprovenancekit.event import TraceableEvent -from dataclasses import dataclass pytest.importorskip("fastapi") pytest.importorskip("httpx") diff --git a/tests/integrations/test_jupyter.py b/tests/integrations/test_jupyter.py index f1559e2..9fdebaf 100644 --- a/tests/integrations/test_jupyter.py +++ b/tests/integrations/test_jupyter.py @@ -2,7 +2,6 @@ from dprovenancekit.kit import DProvenanceKit from dprovenancekit.store import InMemoryTraceStore from dprovenancekit.event import TraceableEvent -from dataclasses import dataclass pytest.importorskip("IPython") diff --git a/tests/integrations/test_mcp.py b/tests/integrations/test_mcp.py index 1028be3..3ad9289 100644 --- a/tests/integrations/test_mcp.py +++ b/tests/integrations/test_mcp.py @@ -2,7 +2,6 @@ from dprovenancekit.kit import DProvenanceKit from dprovenancekit.store import InMemoryTraceStore from dprovenancekit.event import TraceableEvent -from dataclasses import dataclass import asyncio from dprovenancekit.integrations.mcp import traced_mcp_tool diff --git a/tests/test_cli_export.py b/tests/test_cli_export.py index 26e9400..e5c1278 100644 --- a/tests/test_cli_export.py +++ b/tests/test_cli_export.py @@ -35,8 +35,8 @@ def trace_db(tmp_path): def test_export_emits_valid_ordered_jsonl(trace_db, capsys): db, run_id = trace_db assert main(["export", "--db", db, "--run", str(run_id)]) == 0 - lines = [l for l in capsys.readouterr().out.splitlines() if l.strip()] - rows = [json.loads(l) for l in lines] # every line must be valid JSON + lines = [ln for ln in capsys.readouterr().out.splitlines() if ln.strip()] + rows = [json.loads(ln) for ln in lines] # every line must be valid JSON assert [r["type"] for r in rows] == ["retrieved", "decided"] assert [r["sequence"] for r in rows] == [0, 1] assert all(r["run_id"] == str(run_id) for r in rows) diff --git a/tests/test_conformance.py b/tests/test_conformance.py index 5cc083b..50deb3f 100644 --- a/tests/test_conformance.py +++ b/tests/test_conformance.py @@ -90,6 +90,7 @@ def test_run_fingerprint(desc, case): rows = store._db.query( "SELECT fingerprint FROM runs WHERE run_id = ?", (str(run.run_id),) ) + store.close() assert rows[0][0] == case["fingerprint"] @@ -131,6 +132,7 @@ def test_query_semantics(backend, desc, case): dsl = dsl_from_wire_dsl(case["dsl"]) matched = sorted(r.context_id for r in store.query_runs(dsl)) + store.close() assert matched == case["expected_context_ids"] diff --git a/tests/test_facade.py b/tests/test_facade.py index 575636d..a58b98c 100644 --- a/tests/test_facade.py +++ b/tests/test_facade.py @@ -78,10 +78,10 @@ def test_facade_jsonl_save_is_one_event_per_line(tmp_path): path = tmp_path / "run.jsonl" trace.save(path) - lines = [json.loads(l) for l in path.read_text().splitlines() if l.strip()] + lines = [json.loads(ln) for ln in path.read_text().splitlines() if ln.strip()] # Three spans, a .start/.end pair each. assert len(lines) == 6 - types = [l["type"] for l in lines] + types = [ln["type"] for ln in lines] assert "Agent Workflow.start" in types assert "Verify Claims.end" in types # Every event carries the envelope fields the loader needs. diff --git a/tests/test_instrument.py b/tests/test_instrument.py index d120ea2..eb3f129 100644 --- a/tests/test_instrument.py +++ b/tests/test_instrument.py @@ -419,6 +419,7 @@ def b(): fp1 = _fingerprint_after(store, r1.run_id) fp2 = _fingerprint_after(store, r2.run_id) fp3 = _fingerprint_after(store, r3.run_id) + store.close() assert fp1 == fp2 assert fp1 != fp3 diff --git a/tests/test_integration_langchain.py b/tests/test_integration_langchain.py index 94ffc64..2e4c151 100644 --- a/tests/test_integration_langchain.py +++ b/tests/test_integration_langchain.py @@ -389,6 +389,7 @@ def test_identical_paths_share_a_fingerprint_divergent_paths_do_not(): fp_a = _fingerprint_after(store, fa) fp_b = _fingerprint_after(store, fb) fp_c = _fingerprint_after(store, fc) + store.close() assert fp_a == fp_b # same path → same fingerprint assert fp_a != fp_c # reordered path → different fingerprint diff --git a/tests/test_integration_openai_agents.py b/tests/test_integration_openai_agents.py index fe57d6f..aafb42f 100644 --- a/tests/test_integration_openai_agents.py +++ b/tests/test_integration_openai_agents.py @@ -289,6 +289,7 @@ def test_force_flush_writes_open_trace(): rows = store._db.query( "SELECT event_count FROM runs WHERE run_id = ?", (str(run_id),) ) + store.close() assert rows and rows[0][0] == 1 @@ -356,6 +357,7 @@ def test_same_path_shares_fingerprint_divergent_path_differs(): _fingerprint_after(store, b), _fingerprint_after(store, c), ) + store.close() assert fp_a == fp_b assert fp_a != fp_c diff --git a/tests/test_otel_ingest.py b/tests/test_otel_ingest.py index f2a2e6e..cf2c930 100644 --- a/tests/test_otel_ingest.py +++ b/tests/test_otel_ingest.py @@ -9,7 +9,6 @@ from dprovenancekit.diff import ChangeKind, TraceDiffEngine from dprovenancekit.otel_ingest import ( - IngestedRun, OTelSpanEvent, ingest_otlp, run_id_for_trace, diff --git a/tests/test_public_api.py b/tests/test_public_api.py index b33431a..2809102 100644 --- a/tests/test_public_api.py +++ b/tests/test_public_api.py @@ -9,7 +9,6 @@ from __future__ import annotations -import importlib import dprovenancekit diff --git a/tests/test_raw_store_roundtrip.py b/tests/test_raw_store_roundtrip.py index 4e0e93b..56868c2 100644 --- a/tests/test_raw_store_roundtrip.py +++ b/tests/test_raw_store_roundtrip.py @@ -17,9 +17,11 @@ def test_written_run_survives_raw_trace_store_reopen(temp_db_path): kit.record(TestEvent.error_detected()) kit.record(TestEvent.process_finished()) store.flush() + store.close() reader = RawTraceStore(temp_db_path) runs = reader.fetch_all_runs() + reader.close() assert len(runs) == 1 run = runs[0] diff --git a/tests/test_sqlite_get_run.py b/tests/test_sqlite_get_run.py index 7f2b96c..20925d4 100644 --- a/tests/test_sqlite_get_run.py +++ b/tests/test_sqlite_get_run.py @@ -41,6 +41,7 @@ def test_get_run_returns_run_by_id(tmp_path): kit.record(_E("b")) got = store.get_run(run.run_id) # flushes, then indexed fetch + store.close() assert got is not None assert got.context_id == "c" assert [e.payload.type_identifier for e in got.events] == ["a", "b"] @@ -48,4 +49,6 @@ def test_get_run_returns_run_by_id(tmp_path): def test_get_run_missing_is_none(tmp_path): store = SQLiteTraceStore(_E, str(tmp_path / "t.sqlite"), start_writer=False) - assert store.get_run(uuid.uuid4()) is None + missing = store.get_run(uuid.uuid4()) + store.close() + assert missing is None diff --git a/tests/test_sqlite_insert_failure_drop.py b/tests/test_sqlite_insert_failure_drop.py index c84d65f..c257e8c 100644 --- a/tests/test_sqlite_insert_failure_drop.py +++ b/tests/test_sqlite_insert_failure_drop.py @@ -72,6 +72,7 @@ def test_failed_batch_insert_is_tallied_and_breaks_integrity(temp_db_path): assert not stats.preserved_integrity assert _count(db, "SELECT COUNT(*) FROM runs;") == 0 + db.close() def test_successful_insert_tallies_nothing_and_records_accurate_metadata(temp_db_path): @@ -94,3 +95,4 @@ def test_successful_insert_tallies_nothing_and_records_accurate_metadata(temp_db assert _count(db, "SELECT COUNT(*) FROM trace_events;") == 3 assert _count(db, "SELECT event_count FROM runs WHERE run_id = 'run-1';") == 3 + db.close() diff --git a/tests/test_visualizer.py b/tests/test_visualizer.py index 5bb1458..af4e277 100644 --- a/tests/test_visualizer.py +++ b/tests/test_visualizer.py @@ -1,6 +1,5 @@ import uuid -import pytest -from dprovenancekit import TraceGraph, TraceEdge, TraceEdgeType, InMemoryTraceStore +from dprovenancekit import TraceGraph, TraceEdge, TraceEdgeType from dprovenancekit.visualizer import render_trace_html from conftest import TestEvent