Skip to content

Commit c6fada0

Browse files
Brian KrafftCopilot
andcommitted
feat(3.7): P3 integration tests + store.py cleanup — Phase 3 complete
Epic 3.7: P3 Integration Tests + store.py Cleanup ✅ EPIC COMPLETE Integration Tests (tests/test_p3_integration.py): • 8 test classes with comprehensive workflows • TestSkillLifecycleWorkflow: CRUD and batch operations • TestConcurrentAccess: threading and async concurrency • TestMigrationWorkflow: schema initialization • TestEdgeCases: error conditions and empty state • TestLineageAndAnalysisWorkflow: cross-module interaction • TestSearchAndTagWorkflow: search engine integration • TestSkillEvolutionWorkflow: evolution pipeline • TestComprehensiveWorkflow: end-to-end scenarios store.py Cleanup (1082→1102 lines): • Added Phase 3 architecture documentation (lines 8-27) • Enhanced class docstring with facade pattern explanation • Restored set_schema_version as DEPRECATED (backward compatibility) • Clear module delegation structure documented Test Results: • Core integration tests: 8/8 PASSED ✅ • Main test suite: 1309 tests PASSED ✅ • Demonstrates Phase 3 decomposition works end-to-end Phase 3 Status: 🎯 COMPLETE All 7 epics delivered. SkillStore successfully decomposed from 1345-line monolith into focused modules with comprehensive test coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 93dba38 commit c6fada0

6 files changed

Lines changed: 2327 additions & 44 deletions

File tree

openspace/skill_engine/lineage_tracker.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,11 @@ def record_derivation(
192192
"""
193193
self._ensure_open()
194194
with self._mu:
195-
self._conn.execute("BEGIN")
195+
owns_txn = not self._conn.in_transaction
196+
if owns_txn:
197+
self._conn.execute("BEGIN")
198+
else:
199+
self._conn.execute("SAVEPOINT sp_record_derivation")
196200
try:
197201
# For FIXED: deactivate same-name parents (superseded)
198202
if new_record.lineage.origin == SkillOrigin.FIXED:
@@ -207,7 +211,10 @@ def record_derivation(
207211
new_record.is_active = True
208212

209213
self._repo._upsert(new_record)
210-
self._conn.commit()
214+
if owns_txn:
215+
self._conn.commit()
216+
else:
217+
self._conn.execute("RELEASE sp_record_derivation")
211218

212219
origin = new_record.lineage.origin.value
213220
logger.info(
@@ -216,7 +223,10 @@ def record_derivation(
216223
f"[{new_record.skill_id}] ← parents={parent_skill_ids}"
217224
)
218225
except Exception:
219-
self._conn.rollback()
226+
if owns_txn:
227+
self._conn.rollback()
228+
else:
229+
self._conn.execute("ROLLBACK TO sp_record_derivation")
220230
raise
221231

222232
# ── Lineage queries ───────────────────────────────────────────────

openspace/skill_engine/skill_repository.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,9 @@ def _reader(self) -> Generator[sqlite3.Connection, None, None]:
136136
"""Open a temporary read-only connection (WAL parallel reads)."""
137137
self._ensure_open()
138138
if not self._owns_conn:
139-
# When sharing a connection, just use it directly
140-
yield self._conn
139+
# When sharing a connection, acquire lock to prevent dirty reads
140+
with self._mu:
141+
yield self._conn
141142
return
142143
conn = self._make_connection(read_only=True)
143144
try:

0 commit comments

Comments
 (0)