diff --git a/openspace/skill_engine/analysis_store.py b/openspace/skill_engine/analysis_store.py index 9426b30f..306dd979 100644 --- a/openspace/skill_engine/analysis_store.py +++ b/openspace/skill_engine/analysis_store.py @@ -22,7 +22,7 @@ from datetime import datetime from functools import wraps from pathlib import Path -from typing import Any, Dict, Generator, List, Optional +from typing import Any, Callable, Dict, Generator, List, Optional from openspace.utils.logging import Logger @@ -43,12 +43,12 @@ def _db_retry( max_retries: int = 5, initial_delay: float = 0.1, backoff: float = 2.0, -): +) -> Callable: """Retry on transient SQLite errors with exponential backoff.""" - def decorator(func): + def decorator(func: Callable) -> Callable: @wraps(func) - def wrapper(*args, **kwargs): + def wrapper(*args: Any, **kwargs: Any) -> Any: delay = initial_delay for attempt in range(max_retries): try: @@ -78,7 +78,7 @@ class AnalysisStore: Usage (standalone):: store = AnalysisStore(db_path=Path("analyses.db")) - store.record_execution_analysis(analysis) + store.record_execution_analysis_sync(analysis) recent = store.load_analyses(skill_id="some_skill") store.close() @@ -314,9 +314,70 @@ def hydrate_recent_analyses(self, record: SkillRecord) -> SkillRecord: last_updated=record.last_updated, recent_analyses=recent_analyses, # This is what we're hydrating tool_dependencies=record.tool_dependencies, + critical_tools=record.critical_tools, # Fix: preserve critical_tools tags=record.tags, ) + def batch_load_recent_analyses( + self, skill_ids: List[str], limit: int = 5 + ) -> Dict[str, List[ExecutionAnalysis]]: + """Batch-load recent analyses for multiple skills. + + Uses window functions to get top-N per skill in 2 queries total + (analyses + judgments), avoiding O(N*M) fan-out. + """ + if not skill_ids: + return {} + placeholders = ",".join("?" * len(skill_ids)) + with self._reader() as conn: + # 1 query: ranked analyses per skill (DISTINCT via window over unique ea.id per skill) + rows = conn.execute( + f"SELECT sub.* FROM (" + f" SELECT ea.*, sj.skill_id AS source_skill_id, " + f" ROW_NUMBER() OVER (" + f" PARTITION BY sj.skill_id ORDER BY ea.timestamp DESC" + f" ) AS rn " + f" FROM execution_analyses ea " + f" JOIN skill_judgments sj ON ea.id = sj.analysis_id " + f" WHERE sj.skill_id IN ({placeholders})" + f") sub WHERE sub.rn <= ?", + [*skill_ids, limit], + ).fetchall() + + # Collect analysis IDs and group by skill + filtered: Dict[str, list] = {} + analysis_ids: set = set() + for row in rows: + sid = row["source_skill_id"] + filtered.setdefault(sid, []).append(row) + analysis_ids.add(row["id"]) + + if not analysis_ids: + return {} + + # 1 query: all judgments for matched analyses + aid_placeholders = ",".join("?" * len(analysis_ids)) + judgment_rows = conn.execute( + f"SELECT analysis_id, skill_id, skill_applied, note " + f"FROM skill_judgments WHERE analysis_id IN ({aid_placeholders})", + list(analysis_ids), + ).fetchall() + + judgments_by_analysis: Dict[str, list] = {} + for jr in judgment_rows: + judgments_by_analysis.setdefault(jr["analysis_id"], []).append(jr) + + # Deserialize with preloaded judgments + result: Dict[str, List[ExecutionAnalysis]] = {} + for sid, ea_rows in filtered.items(): + result[sid] = [ + self._to_analysis( + conn, row, judgments_by_analysis.get(row["id"], []) + ) + for row in ea_rows + ] + return result + # ── Bulk Operations ──────────────────────────────────────────────── @_db_retry() @@ -495,6 +556,7 @@ def insert_analysis(self, a: ExecutionAnalysis) -> int: ), ) analysis_id = cur.lastrowid + assert analysis_id is not None, "INSERT must set lastrowid" for j in a.skill_judgments: self._conn.execute( @@ -505,14 +567,21 @@ def insert_analysis(self, a: ExecutionAnalysis) -> int: return analysis_id @staticmethod - def _to_analysis(conn: sqlite3.Connection, row: sqlite3.Row) -> ExecutionAnalysis: + def _to_analysis( + conn: sqlite3.Connection, + row: sqlite3.Row, + preloaded_judgments: Optional[List[sqlite3.Row]] = None, + ) -> ExecutionAnalysis: """Deserialize an execution_analyses row + judgments → ExecutionAnalysis.""" analysis_id = row["id"] - judgment_rows = conn.execute( - "SELECT skill_id, skill_applied, note FROM skill_judgments WHERE analysis_id=?", - (analysis_id,), - ).fetchall() + if preloaded_judgments is not None: + judgment_rows = preloaded_judgments + else: + judgment_rows = conn.execute( + "SELECT skill_id, skill_applied, note FROM skill_judgments WHERE analysis_id=?", + (analysis_id,), + ).fetchall() suggestions: list[EvolutionSuggestion] = [] raw_suggestions = row["evolution_suggestions"] diff --git a/openspace/skill_engine/lineage_tracker.py b/openspace/skill_engine/lineage_tracker.py index 409aabdd..c36cede5 100644 --- a/openspace/skill_engine/lineage_tracker.py +++ b/openspace/skill_engine/lineage_tracker.py @@ -116,10 +116,7 @@ def __init__( if db_path is None: raise ValueError("Either db_path or conn must be provided") self._db_path = Path(db_path) - self._conn = sqlite3.connect(str(db_path), timeout=30) - self._conn.row_factory = sqlite3.Row - self._conn.execute("PRAGMA journal_mode=WAL") - self._conn.execute("PRAGMA busy_timeout=30000") + self._conn = self._make_connection(read_only=False) # Standalone mode: ensure schema exists mm = MigrationManager(conn=self._conn, lock=self._mu) mm.initialize_schema() @@ -480,6 +477,7 @@ def get_ancestors( List of ancestor :class:`SkillRecord` objects, sorted by generation (nearest first). """ + max_depth = min(max_depth, 50) # Safety clamp to prevent DoS with self._reader() as conn: visited: set[str] = {skill_id} # Fix 1: Seed with starting skill_id to prevent cycles ancestors: List[SkillRecord] = [] @@ -551,6 +549,7 @@ def get_lineage_tree( Returns: Nested dict representing the lineage tree. """ + max_depth = min(max_depth, 50) # Safety clamp to prevent DoS with self._reader() as conn: return self._subtree(conn, skill_id, max_depth, set()) diff --git a/openspace/skill_engine/migration_manager.py b/openspace/skill_engine/migration_manager.py index 4776c2f0..fd9ab78b 100644 --- a/openspace/skill_engine/migration_manager.py +++ b/openspace/skill_engine/migration_manager.py @@ -171,7 +171,7 @@ class MigrationManager: # SkillStore creates us internally: # store._migrations = MigrationManager(conn=store._conn, lock=store._mu) - # SkillStore calls manager.initialize_schema() during __init__ + # SkillStore calls manager.ensure_current_schema() during __init__ """ def __init__( diff --git a/openspace/skill_engine/store.py b/openspace/skill_engine/store.py index f0d21fff..16f59ba9 100644 --- a/openspace/skill_engine/store.py +++ b/openspace/skill_engine/store.py @@ -29,6 +29,7 @@ from __future__ import annotations import asyncio +import dataclasses import json import sqlite3 import threading @@ -681,14 +682,15 @@ def get_versions(self, name: str) -> List[SkillRecord]: Delegates to :class:`LineageTracker` (Epic 3.3). """ - # Fix 5: Hydrate recent_analyses after delegation + # Fix: Fully hydrate records after delegation (tags, tool_deps, critical_tools, recent_analyses) records = self._lineage.get_evolution_chain(name) - return [self._hydrate_recent_analyses(record) for record in records] + return self._hydrate_records(records) @_db_retry() def load_by_category(self, category: SkillCategory, *, active_only: bool = True) -> List[SkillRecord]: """Delegate to SkillRepository for category queries.""" - return self._repo.load_by_category(category, active_only=active_only) + records = self._repo.load_by_category(category, active_only=active_only) + return self._hydrate_records(records) @_db_retry() def load_analyses( @@ -826,9 +828,9 @@ def get_ancestry(self, skill_id: str, max_depth: int = 10) -> List[SkillRecord]: Delegates to :class:`LineageTracker` (Epic 3.3). """ - # Fix 5: Hydrate recent_analyses after delegation + # Fix: Fully hydrate records after delegation (tags, tool_deps, critical_tools, recent_analyses) records = self._lineage.get_ancestors(skill_id, max_depth=max_depth) - return [self._hydrate_recent_analyses(record) for record in records] + return self._hydrate_records(records) @_db_retry() def get_lineage_tree(self, skill_id: str, max_depth: int = 5) -> Dict[str, Any]: @@ -992,6 +994,82 @@ def _hydrate_recent_analyses(self, record: SkillRecord) -> SkillRecord: """Hydrate recent_analyses for a SkillRecord from AnalysisStore delegation.""" return self._analyses.hydrate_recent_analyses(record) + def _hydrate_record(self, record: SkillRecord) -> SkillRecord: + """Fully hydrate a SkillRecord with tags, tool_deps, critical_tools, and recent_analyses. + + Used by facade methods that delegate to modules returning partially hydrated records. + """ + with self._reader() as conn: + # Hydrate tool dependencies and critical tools + dep_rows = conn.execute( + "SELECT tool_key, critical FROM skill_tool_deps WHERE skill_id=?", + (record.skill_id,), + ).fetchall() + + tool_dependencies = [r["tool_key"] for r in dep_rows] + critical_tools = [r["tool_key"] for r in dep_rows if r["critical"]] + + # Hydrate tags using TagSearch + tags = self._tag_search.get_tags(record.skill_id) + + # Hydrate recent_analyses using AnalysisStore + hydrated_record = self._analyses.hydrate_recent_analyses(record) + + # Return a new record with all fields hydrated + return dataclasses.replace( + hydrated_record, + tags=tags, + tool_dependencies=tool_dependencies, + critical_tools=critical_tools, + ) + + def _hydrate_records(self, records: List[SkillRecord]) -> List[SkillRecord]: + """Batch hydrate multiple SkillRecords — O(1) queries instead of O(N). + + Batches tag, tool_dep, and analysis queries across all records. + """ + if not records: + return records + + skill_ids = [r.skill_id for r in records] + placeholders = ",".join("?" * len(skill_ids)) + + with self._reader() as conn: + # Batch 1: tool_deps — 1 query + dep_rows = conn.execute( + f"SELECT skill_id, tool_key, critical FROM skill_tool_deps " + f"WHERE skill_id IN ({placeholders})", + skill_ids, + ).fetchall() + + deps_by_skill: Dict[str, List[str]] = {} + critical_by_skill: Dict[str, List[str]] = {} + for row in dep_rows: + sid = row["skill_id"] + deps_by_skill.setdefault(sid, []).append(row["tool_key"]) + if row["critical"]: + critical_by_skill.setdefault(sid, []).append(row["tool_key"]) + + # Batch 2: tags — 1 query (via TagSearch) + tags_by_skill = self._tag_search.get_tags_batch(skill_ids) + + # Batch 3: analyses — 2 queries (via AnalysisStore) + analyses_by_skill = self._analyses.batch_load_recent_analyses( + skill_ids, SkillRecord.MAX_RECENT + ) + + # Assemble + return [ + dataclasses.replace( + record, + tags=tags_by_skill.get(record.skill_id, []), + tool_dependencies=deps_by_skill.get(record.skill_id, []), + critical_tools=critical_by_skill.get(record.skill_id, []), + recent_analyses=analyses_by_skill.get(record.skill_id, []), + ) + for record in records + ] + # Deserialization def _to_record(self, conn: sqlite3.Connection, row: sqlite3.Row) -> SkillRecord: """Deserialize a skill_records row + related rows → SkillRecord.""" @@ -1101,6 +1179,10 @@ def get_tags(self, skill_id: str) -> List[str]: """Get all tags for a skill (facade to TagSearch).""" return self._tag_search.get_tags(skill_id) + def get_tags_batch(self, skill_ids: List[str]) -> Dict[str, List[str]]: + """Batch-get tags for multiple skills (facade to TagSearch).""" + return self._tag_search.get_tags_batch(skill_ids) + def get_all_tags(self) -> List[Dict[str, Any]]: """Get all tags with usage counts (facade to TagSearch).""" return self._tag_search.get_all_tags() diff --git a/openspace/skill_engine/tag_search.py b/openspace/skill_engine/tag_search.py index 982e9bb2..f7db6675 100644 --- a/openspace/skill_engine/tag_search.py +++ b/openspace/skill_engine/tag_search.py @@ -18,7 +18,7 @@ from contextlib import contextmanager from functools import wraps from pathlib import Path -from typing import Any, Dict, Generator, List, Optional +from typing import Any, Callable, Dict, Generator, List, Optional from openspace.utils.logging import Logger @@ -32,12 +32,12 @@ def _db_retry( max_retries: int = 5, initial_delay: float = 0.1, backoff: float = 2.0, -): +) -> Callable: """Retry on transient SQLite errors with exponential backoff.""" - def decorator(func): + def decorator(func: Callable) -> Callable: @wraps(func) - def wrapper(*args, **kwargs): + def wrapper(*args: Any, **kwargs: Any) -> Any: delay = initial_delay for attempt in range(max_retries): try: @@ -221,6 +221,23 @@ def get_tags(self, skill_id: str) -> List[str]: ).fetchall() return [r["tag"] for r in rows] + @_db_retry() + def get_tags_batch(self, skill_ids: List[str]) -> Dict[str, List[str]]: + """Batch-get tags for multiple skills. Returns {skill_id: [sorted tags]}.""" + if not skill_ids: + return {} + placeholders = ",".join("?" * len(skill_ids)) + with self._reader() as conn: + rows = conn.execute( + f"SELECT skill_id, tag FROM skill_tags " + f"WHERE skill_id IN ({placeholders}) ORDER BY skill_id, tag", + skill_ids, + ).fetchall() + result: Dict[str, List[str]] = {} + for row in rows: + result.setdefault(row["skill_id"], []).append(row["tag"]) + return result + @_db_retry() def find_skills_by_tags( self, @@ -448,8 +465,8 @@ def search_skills( List of skill records matching the criteria. """ with self._reader() as conn: - conditions = [] - params = [] + conditions: List[str] = [] + params: List[Any] = [] # Active filter if active_only: diff --git a/tests/test_facade_coverage.py b/tests/test_facade_coverage.py index cc5422fe..ebf5b008 100644 --- a/tests/test_facade_coverage.py +++ b/tests/test_facade_coverage.py @@ -548,4 +548,390 @@ def test_close_releases_resources(self, temp_db): try: assert store2.count() == 0 finally: - store2.close() \ No newline at end of file + store2.close() + + +class TestFacadeHydrationFix: + """Regression tests for facade methods returning fully hydrated SkillRecords. + + Prior to the fix, get_versions(), get_ancestry(), and load_by_category() + returned partially hydrated records missing tags, tool_deps, critical_tools, + and/or recent_analyses. + """ + + @pytest.mark.asyncio + async def test_get_versions_returns_fully_hydrated_records(self, store): + """get_versions() should return records with tags, tool_deps, critical_tools, and recent_analyses.""" + # Create a skill record with all fields populated + record = SkillRecord( + skill_id="hydration_test_v1", + name="hydration_test", + description="Test hydration fix", + category=SkillCategory.TOOL_GUIDE, + visibility=SkillVisibility.PUBLIC, + path="/test/hydration.py", + lineage=SkillLineage( + origin=SkillOrigin.IMPORTED, + generation=0, + content_snapshot={"hydration.py": "def test_skill():\n pass"}, + ), + tags=["test-tag", "hydration-tag"], + tool_dependencies=["tool1", "tool2"], + critical_tools=["tool1"], + total_selections=5, + total_applied=3, + total_completions=2, + total_fallbacks=1, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Save the record + await store.save_record(record) + + # Add an analysis to test recent_analyses hydration + analysis = ExecutionAnalysis( + task_id="hydration_task_123", + timestamp=datetime.now(), + task_completed=True, + execution_note="Test hydration analysis", + skill_judgments=[ + SkillJudgment( + skill_id=record.skill_id, + skill_applied=True, + note="Skill applied successfully in hydration test", + ) + ], + analyzed_at=datetime.now(), + ) + await store.record_analysis(analysis) + + # Test get_versions() - this delegates to LineageTracker + versions = store.get_versions("hydration_test") + assert len(versions) == 1 + + hydrated_record = versions[0] + + # Verify ALL fields are hydrated + assert set(hydrated_record.tags) == {"test-tag", "hydration-tag"} + assert set(hydrated_record.tool_dependencies) == {"tool1", "tool2"} + assert set(hydrated_record.critical_tools) == {"tool1"} + assert len(hydrated_record.recent_analyses) == 1 + assert hydrated_record.recent_analyses[0].task_id == "hydration_task_123" + + @pytest.mark.asyncio + async def test_get_ancestry_returns_fully_hydrated_records(self, store): + """get_ancestry() should return records with tags, tool_deps, critical_tools, and recent_analyses.""" + # Create a parent skill + parent_record = SkillRecord( + skill_id="parent_skill_v1", + name="parent_skill", + description="Parent skill for ancestry test", + category=SkillCategory.WORKFLOW, + visibility=SkillVisibility.PUBLIC, + path="/test/parent.py", + lineage=SkillLineage( + origin=SkillOrigin.IMPORTED, + generation=0, + content_snapshot={"parent.py": "def parent_skill():\n pass"}, + ), + tags=["parent-tag", "ancestry-tag"], + tool_dependencies=["parent-tool"], + critical_tools=["parent-tool"], + total_selections=2, + total_applied=1, + total_completions=1, + total_fallbacks=0, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Create a child skill that derives from parent + child_record = SkillRecord( + skill_id="child_skill_v1", + name="child_skill", + description="Child skill derived from parent", + category=SkillCategory.WORKFLOW, + visibility=SkillVisibility.PUBLIC, + path="/test/child.py", + lineage=SkillLineage( + origin=SkillOrigin.DERIVED, + generation=1, + parent_skill_ids=[parent_record.skill_id], + source_task_id="ancestry_derivation_task", + change_summary="Derived from parent skill", + content_diff="@@ -1,1 +1,1 @@\n-def parent_skill():\n+def child_skill():", + content_snapshot={"child.py": "def child_skill():\n pass"}, + created_by="hydration_test", + ), + tags=["child-tag", "derived-tag"], + tool_dependencies=["child-tool"], + critical_tools=["child-tool"], + total_selections=0, + total_applied=0, + total_completions=0, + total_fallbacks=0, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Save both records + await store.save_record(parent_record) + await store.save_record(child_record) + + # Add analyses for both to test recent_analyses hydration + parent_analysis = ExecutionAnalysis( + task_id="parent_task_456", + timestamp=datetime.now(), + task_completed=True, + execution_note="Parent skill analysis", + skill_judgments=[ + SkillJudgment( + skill_id=parent_record.skill_id, + skill_applied=True, + note="Parent skill executed", + ) + ], + analyzed_at=datetime.now(), + ) + await store.record_analysis(parent_analysis) + + # Test get_ancestry() - this delegates to LineageTracker + ancestry = store.get_ancestry(child_record.skill_id) + assert len(ancestry) == 1 + + hydrated_parent = ancestry[0] + + # Verify ALL fields are hydrated for the parent returned by get_ancestry + assert hydrated_parent.skill_id == parent_record.skill_id + assert set(hydrated_parent.tags) == {"parent-tag", "ancestry-tag"} + assert set(hydrated_parent.tool_dependencies) == {"parent-tool"} + assert set(hydrated_parent.critical_tools) == {"parent-tool"} + assert len(hydrated_parent.recent_analyses) == 1 + assert hydrated_parent.recent_analyses[0].task_id == "parent_task_456" + + @pytest.mark.asyncio + async def test_load_by_category_returns_fully_hydrated_records(self, store): + """load_by_category() should return records with tags, tool_deps, critical_tools, and recent_analyses.""" + # Create a skill record with all fields populated + record = SkillRecord( + skill_id="category_test_v1", + name="category_test", + description="Test category hydration", + category=SkillCategory.REFERENCE, + visibility=SkillVisibility.PUBLIC, + path="/test/category.py", + lineage=SkillLineage( + origin=SkillOrigin.IMPORTED, + generation=0, + content_snapshot={"category.py": "def category_skill():\n pass"}, + ), + tags=["category-tag", "reference-tag"], + tool_dependencies=["cat-tool1", "cat-tool2"], + critical_tools=["cat-tool1"], + total_selections=3, + total_applied=2, + total_completions=2, + total_fallbacks=0, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Save the record + await store.save_record(record) + + # Add an analysis to test recent_analyses hydration + analysis = ExecutionAnalysis( + task_id="category_task_789", + timestamp=datetime.now(), + task_completed=True, + execution_note="Category test analysis", + skill_judgments=[ + SkillJudgment( + skill_id=record.skill_id, + skill_applied=True, + note="Category skill applied successfully", + ) + ], + analyzed_at=datetime.now(), + ) + await store.record_analysis(analysis) + + # DEBUG: Let's verify the record was saved correctly + saved_record = store.load_record("category_test_v1") + assert saved_record is not None + + # Test load_by_category() - this delegates to SkillRepository + category_records = store.load_by_category(SkillCategory.REFERENCE) + assert len(category_records) == 1 + + hydrated_record = category_records[0] + + # Verify ALL fields are hydrated + assert set(hydrated_record.tags) == {"category-tag", "reference-tag"} + assert set(hydrated_record.tool_dependencies) == {"cat-tool1", "cat-tool2"} + assert set(hydrated_record.critical_tools) == {"cat-tool1"} + assert len(hydrated_record.recent_analyses) == 1 + assert hydrated_record.recent_analyses[0].task_id == "category_task_789" + + @pytest.mark.asyncio + async def test_hydration_consistency_across_facade_methods(self, store): + """All facade methods should return consistently hydrated records for the same skill.""" + # Create a skill that we'll access through multiple facade methods + record = SkillRecord( + skill_id="consistency_test_v1", + name="consistency_test", + description="Test hydration consistency", + category=SkillCategory.TOOL_GUIDE, + visibility=SkillVisibility.PUBLIC, + path="/test/consistency.py", + lineage=SkillLineage( + origin=SkillOrigin.IMPORTED, + generation=0, + content_snapshot={"consistency.py": "def consistent_skill():\n pass"}, + ), + tags=["consistency-tag", "facade-tag"], + tool_dependencies=["cons-tool1", "cons-tool2"], + critical_tools=["cons-tool1"], + total_selections=1, + total_applied=1, + total_completions=1, + total_fallbacks=0, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Save the record + await store.save_record(record) + + # Add an analysis + analysis = ExecutionAnalysis( + task_id="consistency_task_999", + timestamp=datetime.now(), + task_completed=True, + execution_note="Consistency test analysis", + skill_judgments=[ + SkillJudgment( + skill_id=record.skill_id, + skill_applied=True, + note="Consistency skill applied", + ) + ], + analyzed_at=datetime.now(), + ) + await store.record_analysis(analysis) + + # Get the same skill through different facade methods + + # Method 1: load_record (direct facade method, should be fully hydrated) + direct_record = store.load_record(record.skill_id) + + # Method 2: get_versions (delegates to LineageTracker) + versions = store.get_versions("consistency_test") + versions_record = versions[0] + + # Method 3: load_by_category (delegates to SkillRepository) + category_records = store.load_by_category(SkillCategory.TOOL_GUIDE) + category_record = next(r for r in category_records if r.skill_id == record.skill_id) + + # All three methods should return identically hydrated records + expected_tags = {"consistency-tag", "facade-tag"} + expected_tool_deps = {"cons-tool1", "cons-tool2"} + expected_critical_tools = {"cons-tool1"} + + for method_name, retrieved_record in [ + ("load_record", direct_record), + ("get_versions", versions_record), + ("load_by_category", category_record), + ]: + assert set(retrieved_record.tags) == expected_tags, f"{method_name} failed tag hydration" + assert set(retrieved_record.tool_dependencies) == expected_tool_deps, f"{method_name} failed tool_deps hydration" + assert set(retrieved_record.critical_tools) == expected_critical_tools, f"{method_name} failed critical_tools hydration" + assert len(retrieved_record.recent_analyses) == 1, f"{method_name} failed recent_analyses hydration" + assert retrieved_record.recent_analyses[0].task_id == "consistency_task_999", f"{method_name} failed recent_analyses content" + + @pytest.mark.asyncio + async def test_f5_evolved_child_inherits_tags(self, store): + """F5: When skill A (with tags) is evolved to B, B should inherit A's tags (if expected).""" + # Create a parent skill with tags + parent_record = SkillRecord( + skill_id="parent_f5_v1", + name="parent_f5", + description="Parent skill with tags", + category=SkillCategory.WORKFLOW, + visibility=SkillVisibility.PUBLIC, + path="/test/parent_f5.py", + lineage=SkillLineage( + origin=SkillOrigin.IMPORTED, + generation=0, + content_snapshot={"parent_f5.py": "def parent_f5():\n pass"}, + ), + tags=["parent-tag", "workflow-tag"], + tool_dependencies=["parent-tool"], + critical_tools=["parent-tool"], + total_selections=1, + total_applied=1, + total_completions=1, + total_fallbacks=0, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Create a child skill that derives from parent (without explicitly copying tags) + child_record = SkillRecord( + skill_id="child_f5_v1", + name="child_f5", + description="Child skill derived from parent", + category=SkillCategory.WORKFLOW, + visibility=SkillVisibility.PUBLIC, + path="/test/child_f5.py", + lineage=SkillLineage( + origin=SkillOrigin.DERIVED, + generation=1, + parent_skill_ids=[parent_record.skill_id], + source_task_id="f5_derivation_task", + change_summary="Derived from parent skill", + content_diff="@@ -1,1 +1,1 @@\n-def parent_f5():\n+def child_f5():", + content_snapshot={"child_f5.py": "def child_f5():\n pass"}, + created_by="f5_test", + ), + tags=[], # No tags initially - should inherit from parent? + tool_dependencies=["child-tool"], + critical_tools=["child-tool"], + total_selections=0, + total_applied=0, + total_completions=0, + total_fallbacks=0, + recent_analyses=[], + first_seen=datetime.now(), + last_updated=datetime.now(), + ) + + # Save parent, then evolve to child + await store.save_record(parent_record) + await store.evolve_skill(child_record, [parent_record.skill_id]) + + # Check if child inherited parent's tags + child_loaded = store.load_record(child_record.skill_id) + print(f"DEBUG F5: Parent tags: {parent_record.tags}") + print(f"DEBUG F5: Child record tags: {child_record.tags}") + print(f"DEBUG F5: Child loaded tags: {child_loaded.tags}") + + # This test documents current behavior - whether tags are inherited or not + # Based on findings, we may need to implement tag propagation in evolution + # For now, just check what actually happens + assert child_loaded is not None + + # If tags should be inherited, this would be the assertion: + # assert set(child_loaded.tags) >= {"parent-tag", "workflow-tag"} + # But let's see what actually happens first + if not child_loaded.tags: + print(f"INFO F5: Child does NOT inherit parent tags (current behavior)") + else: + print(f"INFO F5: Child has tags: {child_loaded.tags}") \ No newline at end of file