diff --git a/docs/superpowers/plans/2026-07-24-discover-insights.md b/docs/superpowers/plans/2026-07-24-discover-insights.md new file mode 100644 index 0000000..5970678 --- /dev/null +++ b/docs/superpowers/plans/2026-07-24-discover-insights.md @@ -0,0 +1,1345 @@ +# Agentic Insights in inventory.json Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add an agent-authored `insights` key to `metadata/inventory.json` during the discover phase, validated and merged by a pure tool, so the later convert phase can consume pipeline intent, Databricks patterns, and cross-pipeline relationships. + +**Architecture:** The agent *authors* an `insights` JSON object (judgment: intent, patterns, relationships that annotate #9's deterministic lineage edges); a new pure tool path *enriches* the inventory — it validates the authored JSON against the inventory (foreign keys to pipeline names and lineage edges), and on success appends exactly one `insights` key while re-serializing the rest byte-identically. There is **no LLM inside the tool**. The feature is surfaced through a new `enrich` adapter subcommand and an `enrich` MCP command, and driven by a new Step 5 in the discover skill. + +**Tech Stack:** Python 3.12, `@dataclass(slots=True, kw_only=True)` models, argparse CLI subcommands, FastMCP dispatcher tool, pytest unit tests, ruff + mypy via `make fmt`. + +## Global Constraints + +- **Python version:** 3.12+ (matches repo floor). +- **Dataclasses:** every model uses `@dataclass(slots=True, kw_only=True)` (AGENTS.md Code Style Rules). +- **Byte-identical write:** the enrich write-back MUST use `json.dumps(obj, indent=2)` with **no** `sort_keys`, **no** `default=str`, and **no** trailing newline — exactly matching discover's write at `src/flowx/parser/adf_loader.py:1021` (`inventory_path.write_text(json.dumps(inventory_dict, indent=2), encoding="utf-8")`). Any deviation breaks the "deterministic keys byte-identical" invariant. +- **Validation before I/O:** `enrich_inventory` MUST return the failure result before writing anything when violations exist. On `ok:false` the inventory file is left untouched on disk. +- **No new dependencies:** validator is hand-rolled; use only stdlib (`json`, `pathlib`, `tempfile`) and existing flowx modules. +- **Tests assert structure/schema, never prose.** No live LLM in any test — all insights fixtures are stubbed JSON literals. Unit tests live in `tests/unit/`, fixtures in `tests/resources/json/`. +- **Customer confidentiality:** no real customer names or customer-derived vocabulary in code, tests, fixtures, comments, or commit messages. Use generic placeholders ("Factory A", `entityID`, "dummy dataset"). The forbidden denylist ("a customer factory", "a large factory", `engagementID`, `engagementDBVersions`, `etl-parameters`) lives ONLY in the design doc as a grep reference — never introduce those terms. +- **Test command:** `PYTHONPATH=src uv run pytest tests/unit -v` (or a single node id with `::`). Format/lint: `make fmt` (runs `ruff format`, `ruff check --fix`, `mypy src/flowx/`). +- **`edge_identity` grammar:** for `edge_type="control"` it is the `ControlEdge.activity_name`; for `edge_type="data"` it is the `DataEdge.match_key`. Validation resolves against exactly these keys. +- **Enriched marker:** presence of the top-level `insights` key IS the enriched marker. Do NOT add any `schema_version` field. + +--- + +## File Structure + +**Created:** +- `src/flowx/parser/pipeline_insights.py` — `load_insights`, `validate_insights`, `merge_into_inventory`, `enrich_inventory`. The full validate-then-merge core. +- `tests/unit/test_pipeline_insights.py` — all unit tests for the models + parser module. + +**Modified:** +- `src/flowx/models/adf_ast.py` — add 4 dataclasses (`LineageEdgeRef`, `PipelineInsight`, `PipelineRelationship`, `Insights`) after `Lineage` (currently ends at line 403). +- `src/flowx/adapter/__main__.py` — add the `enrich` subparser (modeled on `record-results`), an `_run_enrich` handler, and dispatch in `main()`. +- `src/flowx/mcp/runner.py` — add `materialize_json(obj)` helper (parallels `materialize_adf_definitions`). +- `src/flowx/mcp/server.py` — add `_cmd_enrich`, register `"enrich"` in `_COMMANDS`, extend the `flowx` tool docstring. +- `src/flowx/reporting/coverage.py` — add a `has_insights` column (optional, gated on the `insights` key existing). +- `tests/unit/test_reporting_coverage.py` — cover the new column (only if Task 7 is done). +- `skills/flowx-discover/SKILL.md` — insert the new Step 5 (author → enrich); renumber existing Steps 5–8; reword the summary step. + +--- + +## Task 1: Insights data models + +**Files:** +- Modify: `src/flowx/models/adf_ast.py` (append after line 403, the end of `class Lineage`) +- Test: `tests/unit/test_pipeline_insights.py` + +**Interfaces:** +- Consumes: nothing (leaf dataclasses). `Literal` is already imported at `adf_ast.py:7`; `dataclass`/`field` at line 5. +- Produces: `LineageEdgeRef(edge_type: Literal["control","data"], edge_identity: str)`; `PipelineInsight(pipeline: str, pattern_name: str|None=None, intent: str|None=None, databricks_pattern: str|None=None, recommended_databricks_features: list[str]=[], conversion_notes: list[str]=[])`; `PipelineRelationship(from_pipeline: str, to_pipeline: str, lineage_edge: LineageEdgeRef, relationship_summary: str|None=None, databricks_pattern: str|None=None, risk_if_ignored: str|None=None)`; `Insights(overview: str|None=None, pipeline_insights: list[PipelineInsight]=[], pipeline_relationships: list[PipelineRelationship]=[])`. + +- [ ] **Step 1: Write the failing test** + +Create `tests/unit/test_pipeline_insights.py` with this first test (imports at top of file): + +```python +"""Tests for agentic insights models, validation, and enrichment (discover phase).""" + +from __future__ import annotations + +import json +from pathlib import Path + +from flowx.models.adf_ast import ( + Insights, + LineageEdgeRef, + PipelineInsight, + PipelineRelationship, +) + + +def test_insights_dataclasses_construct_with_defaults(): + edge = LineageEdgeRef(edge_type="control", edge_identity="Run Ingestion Pipeline") + rel = PipelineRelationship( + from_pipeline="factory_a", to_pipeline="factory_b", lineage_edge=edge + ) + insight = PipelineInsight(pipeline="factory_a") + doc = Insights( + overview="whole factory", + pipeline_insights=[insight], + pipeline_relationships=[rel], + ) + assert doc.pipeline_insights[0].pipeline == "factory_a" + assert doc.pipeline_relationships[0].lineage_edge.edge_type == "control" + assert doc.pipeline_relationships[0].lineage_edge.edge_identity == "Run Ingestion Pipeline" + # optional fields default cleanly + assert insight.recommended_databricks_features == [] + assert insight.conversion_notes == [] + assert rel.relationship_summary is None +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py::test_insights_dataclasses_construct_with_defaults -v` +Expected: FAIL with `ImportError: cannot import name 'Insights' from 'flowx.models.adf_ast'` + +- [ ] **Step 3: Add the dataclasses** + +Append to `src/flowx/models/adf_ast.py` (after line 403, following the existing section-comment style): + +```python +# --------------------------------------------------------------------------- +# Agentic insights (discover phase) -- agent-authored judgment merged into +# inventory.json. References pipelines by name and annotates deterministic +# Lineage edges; carries no facts of its own. +# --------------------------------------------------------------------------- + + +@dataclass(slots=True, kw_only=True) +class LineageEdgeRef: + """A typed reference from a PipelineRelationship to one deterministic edge. + + Attributes: + edge_type: Which lineage graph the edge lives in. + edge_identity: For ``"control"`` -- the ``ControlEdge.activity_name``; + for ``"data"`` -- the ``DataEdge.match_key``. Echoed verbatim from a + real edge so enrichment can resolve it. + """ + + edge_type: Literal["control", "data"] + edge_identity: str + + +@dataclass(slots=True, kw_only=True) +class PipelineInsight: + """Per-pipeline judgment; references a pipeline by name (foreign key).""" + + pipeline: str + pattern_name: str | None = None + intent: str | None = None + databricks_pattern: str | None = None + recommended_databricks_features: list[str] = field(default_factory=list) + conversion_notes: list[str] = field(default_factory=list) + + +@dataclass(slots=True, kw_only=True) +class PipelineRelationship: + """Cross-pipeline judgment; annotates one deterministic lineage edge.""" + + from_pipeline: str + to_pipeline: str + lineage_edge: LineageEdgeRef + relationship_summary: str | None = None + databricks_pattern: str | None = None + risk_if_ignored: str | None = None + + +@dataclass(slots=True, kw_only=True) +class Insights: + """Agent-authored insights merged into inventory.json under the ``insights`` key.""" + + overview: str | None = None + pipeline_insights: list[PipelineInsight] = field(default_factory=list) + pipeline_relationships: list[PipelineRelationship] = field(default_factory=list) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py::test_insights_dataclasses_construct_with_defaults -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add src/flowx/models/adf_ast.py tests/unit/test_pipeline_insights.py +git commit -m "$(cat <<'EOF' +Add agentic insights dataclasses to adf_ast + +LineageEdgeRef, PipelineInsight, PipelineRelationship, Insights -- the +typed round-trip side of the discover-phase insights block. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 2: The `validate_insights` pure validator + +**Files:** +- Create: `src/flowx/parser/pipeline_insights.py` +- Test: `tests/unit/test_pipeline_insights.py` + +**Interfaces:** +- Consumes: an `inventory` dict shaped like discover's `_inventory_to_dict` output — `inventory["pipelines"]` is a list of `{"name": str, "activities": [...]}`; `inventory["lineage"]["control_edges"]` is a list of `{"caller_pipeline","callee_pipeline","activity_name","wait_on_completion"}`; `inventory["lineage"]["data_edges"]` is a list of `{"dataset_name","identity","producer_pipeline","producer_activity","consumer_pipeline","consumer_activity","match_kind","match_key"}`. +- Produces: `validate_insights(raw: dict, inventory: dict) -> list[str]` — returns a list of human-readable violation strings; empty list means valid. Collects ALL violations (never fail-fast). + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/unit/test_pipeline_insights.py` (extend the import from the parser module): + +```python +from flowx.parser.pipeline_insights import validate_insights + + +def _inventory() -> dict: + """A minimal inventory dict in discover's serialized shape.""" + return { + "source_dir": "/tmp/adf", + "pipelines": [ + {"name": "factory_a", "activities": []}, + {"name": "factory_b", "activities": []}, + ], + "summary": {"pipeline_count": 2}, + "lineage": { + "control_edges": [ + { + "caller_pipeline": "factory_a", + "callee_pipeline": "factory_b", + "activity_name": "Run Ingestion Pipeline", + "wait_on_completion": True, + } + ], + "data_edges": [ + { + "dataset_name": "ds_orders", + "identity": "curated.orders", + "producer_pipeline": "factory_a", + "producer_activity": "Write Orders", + "consumer_pipeline": "factory_b", + "consumer_activity": "Read Orders", + "match_kind": "identity", + "match_key": "curated.orders", + } + ], + }, + } + + +def _good_insights() -> dict: + return { + "overview": "Two-stage ingestion then transform.", + "pipeline_insights": [ + {"pipeline": "factory_a", "intent": "Ingest", "databricks_pattern": "Autoloader"}, + {"pipeline": "factory_b", "intent": "Transform"}, + ], + "pipeline_relationships": [ + { + "from_pipeline": "factory_a", + "to_pipeline": "factory_b", + "lineage_edge": { + "edge_type": "control", + "edge_identity": "Run Ingestion Pipeline", + }, + "relationship_summary": "A invokes B", + "databricks_pattern": "run_job_task", + "risk_if_ignored": "ordering lost", + } + ], + } + + +def test_validator_accepts_good_insights(): + assert validate_insights(_good_insights(), _inventory()) == [] + + +def test_rejects_pipeline_not_in_inventory(): + raw = _good_insights() + raw["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + violations = validate_insights(raw, _inventory()) + assert violations + assert any("ghost_pipeline" in v for v in violations) + + +def test_rejects_relationship_endpoint_not_in_inventory(): + raw = _good_insights() + raw["pipeline_relationships"][0]["to_pipeline"] = "ghost_pipeline" + violations = validate_insights(raw, _inventory()) + assert any("ghost_pipeline" in v for v in violations) + + +def test_rejects_unresolvable_control_edge(): + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "No Such Activity" + violations = validate_insights(raw, _inventory()) + assert any("No Such Activity" in v for v in violations) + + +def test_data_edge_binds_on_match_key(): + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"] = { + "edge_type": "data", + "edge_identity": "curated.orders", + } + assert validate_insights(raw, _inventory()) == [] + # a non-matching key is rejected + raw["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "curated.missing" + assert validate_insights(raw, _inventory()) + + +def test_rejects_missing_required_field(): + # PipelineInsight missing 'pipeline' + raw = {"pipeline_insights": [{"intent": "x"}], "pipeline_relationships": []} + assert any("pipeline" in v for v in validate_insights(raw, _inventory())) + # PipelineRelationship missing 'lineage_edge' + raw2 = { + "pipeline_insights": [], + "pipeline_relationships": [{"from_pipeline": "factory_a", "to_pipeline": "factory_b"}], + } + assert any("lineage_edge" in v for v in validate_insights(raw2, _inventory())) + + +def test_rejects_unknown_field(): + raw = _good_insights() + raw["pipeline_insights"][0]["bogus_key"] = "x" + assert any("bogus_key" in v for v in validate_insights(raw, _inventory())) + + +def test_rejects_unknown_top_level_key(): + raw = _good_insights() + raw["surprise"] = 1 + assert any("surprise" in v for v in validate_insights(raw, _inventory())) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k validate -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'flowx.parser.pipeline_insights'` (plus the `data_edge`/`missing`/`unknown` tests erroring on import) + +- [ ] **Step 3: Write the validator** + +Create `src/flowx/parser/pipeline_insights.py`: + +```python +"""Validate and merge agent-authored insights into the discover inventory. + +The discover phase writes a pure ``metadata/inventory.json`` (pipelines, summary, +lineage). The agent then *authors* an ``insights`` object -- its judgment about +pipeline intent, Databricks patterns, and cross-pipeline relationships that +annotate the deterministic lineage edges. This module *enriches* the inventory: +it validates the authored JSON against the inventory (foreign keys to pipeline +names and lineage edges) and, only when clean, appends the single ``insights`` +key while re-serialising the rest byte-identically. + +There is no LLM here -- the tool only validates and merges. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +_INSIGHTS_TOP_KEYS = {"overview", "pipeline_insights", "pipeline_relationships"} +_INSIGHT_KEYS = { + "pipeline", + "pattern_name", + "intent", + "databricks_pattern", + "recommended_databricks_features", + "conversion_notes", +} +_RELATIONSHIP_KEYS = { + "from_pipeline", + "to_pipeline", + "lineage_edge", + "relationship_summary", + "databricks_pattern", + "risk_if_ignored", +} +_EDGE_KEYS = {"edge_type", "edge_identity"} + + +def _pipeline_names(inventory: dict) -> set[str]: + return {p.get("name") for p in inventory.get("pipelines", []) if isinstance(p, dict)} + + +def _control_edge_identities(inventory: dict) -> set[str]: + lineage = inventory.get("lineage") or {} + return {e.get("activity_name") for e in lineage.get("control_edges", []) if isinstance(e, dict)} + + +def _data_edge_identities(inventory: dict) -> set[str]: + lineage = inventory.get("lineage") or {} + return {e.get("match_key") for e in lineage.get("data_edges", []) if isinstance(e, dict)} + + +def validate_insights(raw: dict, inventory: dict) -> list[str]: + """Validate an authored insights dict against the inventory. + + Returns a list of human-readable violation strings; an empty list means the + insights are valid. All violations are collected (never fail-fast) so the + agent can fix every problem in one pass. + """ + violations: list[str] = [] + if not isinstance(raw, dict): + return [f"insights must be a JSON object, got {type(raw).__name__}"] + + for key in set(raw) - _INSIGHTS_TOP_KEYS: + violations.append(f"unknown top-level key: {key!r}") + + names = _pipeline_names(inventory) + control_ids = _control_edge_identities(inventory) + data_ids = _data_edge_identities(inventory) + + insights = raw.get("pipeline_insights", []) + if not isinstance(insights, list): + violations.append("'pipeline_insights' must be a list") + insights = [] + for i, item in enumerate(insights): + loc = f"pipeline_insights[{i}]" + if not isinstance(item, dict): + violations.append(f"{loc} must be an object") + continue + for key in set(item) - _INSIGHT_KEYS: + violations.append(f"{loc}: unknown field {key!r}") + name = item.get("pipeline") + if not name: + violations.append(f"{loc}: missing required field 'pipeline'") + elif name not in names: + violations.append(f"{loc}: pipeline {name!r} not in inventory") + + relationships = raw.get("pipeline_relationships", []) + if not isinstance(relationships, list): + violations.append("'pipeline_relationships' must be a list") + relationships = [] + for i, rel in enumerate(relationships): + loc = f"pipeline_relationships[{i}]" + if not isinstance(rel, dict): + violations.append(f"{loc} must be an object") + continue + for key in set(rel) - _RELATIONSHIP_KEYS: + violations.append(f"{loc}: unknown field {key!r}") + for endpoint in ("from_pipeline", "to_pipeline"): + value = rel.get(endpoint) + if not value: + violations.append(f"{loc}: missing required field {endpoint!r}") + elif value not in names: + violations.append(f"{loc}: {endpoint} {value!r} not in inventory") + violations.extend(_validate_edge(rel.get("lineage_edge"), loc, control_ids, data_ids)) + + return violations + + +def _validate_edge(edge: Any, loc: str, control_ids: set[str], data_ids: set[str]) -> list[str]: + """Validate one lineage_edge ref: shape + resolution to a real edge.""" + if edge is None: + return [f"{loc}: missing required field 'lineage_edge'"] + if not isinstance(edge, dict): + return [f"{loc}.lineage_edge must be an object"] + problems: list[str] = [] + for key in set(edge) - _EDGE_KEYS: + problems.append(f"{loc}.lineage_edge: unknown field {key!r}") + edge_type = edge.get("edge_type") + identity = edge.get("edge_identity") + if edge_type not in ("control", "data"): + problems.append(f"{loc}.lineage_edge: edge_type must be 'control' or 'data', got {edge_type!r}") + return problems + if not isinstance(identity, str) or not identity: + problems.append(f"{loc}.lineage_edge: edge_identity must be a non-empty string") + return problems + valid = control_ids if edge_type == "control" else data_ids + if identity not in valid: + problems.append( + f"{loc}.lineage_edge: {edge_type} edge {identity!r} does not resolve to any lineage edge" + ) + return problems +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k validate -v` then `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k "data_edge or missing or unknown" -v` +Expected: PASS (all validator + edge + field tests) + +- [ ] **Step 5: Commit** + +```bash +git add src/flowx/parser/pipeline_insights.py tests/unit/test_pipeline_insights.py +git commit -m "$(cat <<'EOF' +Add validate_insights: FK + lineage-edge validator + +Pure, violation-collecting validator. Checks pipeline-name FKs, resolves +each lineage_edge ref to a real control/data edge (activity_name / +match_key), and rejects unknown or missing fields. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 3: `load_insights`, `merge_into_inventory`, `enrich_inventory` (orchestrator + byte-identical write) + +**Files:** +- Modify: `src/flowx/parser/pipeline_insights.py` +- Test: `tests/unit/test_pipeline_insights.py` + +**Interfaces:** +- Consumes: `validate_insights` (Task 2). Reads `/metadata/inventory.json`. +- Produces: + - `load_insights(*, insights: dict|None=None, insights_path: Path|None=None) -> dict` — returns the raw insights dict from exactly one source; raises `ValueError` if neither or both are given. + - `merge_into_inventory(inventory: dict, raw: dict) -> dict` — returns a new dict with one added `insights` key; does not mutate input; no I/O. + - `enrich_inventory(output_dir: Path, *, insights: dict|None=None, insights_path: Path|None=None) -> dict` — orchestrator returning `{"ok": bool, "violations": list[str], "pipeline_insights": int, "relationships": int}`. On violations, returns `ok=False` WITHOUT writing. On success, writes the merged inventory and returns `ok=True`. + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/unit/test_pipeline_insights.py`: + +```python +import pytest + +from flowx.parser.pipeline_insights import ( + enrich_inventory, + load_insights, + merge_into_inventory, +) + + +def _write_inventory(tmp_path: Path, inventory: dict) -> Path: + """Write inventory.json exactly as discover does (indent=2, no trailing newline).""" + metadata = tmp_path / "metadata" + metadata.mkdir(parents=True, exist_ok=True) + path = metadata / "inventory.json" + path.write_text(json.dumps(inventory, indent=2), encoding="utf-8") + return path + + +def test_load_insights_requires_exactly_one_source(): + with pytest.raises(ValueError): + load_insights() + with pytest.raises(ValueError): + load_insights(insights={"a": 1}, insights_path=Path("/x")) + + +def test_load_insights_from_inline_dict(): + assert load_insights(insights={"overview": "x"}) == {"overview": "x"} + + +def test_load_insights_from_path(tmp_path: Path): + p = tmp_path / "ins.json" + p.write_text(json.dumps({"overview": "y"}), encoding="utf-8") + assert load_insights(insights_path=p) == {"overview": "y"} + + +def test_merge_into_inventory_adds_one_key_without_mutating(): + inv = {"pipelines": [], "summary": {}, "lineage": {}} + raw = {"overview": "z"} + merged = merge_into_inventory(inv, raw) + assert merged["insights"] == {"overview": "z"} + assert "insights" not in inv # input not mutated + assert set(merged) == {"pipelines", "summary", "lineage", "insights"} + + +def test_enrich_success_counts_and_writes(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + result = enrich_inventory(tmp_path, insights=_good_insights()) + assert result["ok"] is True + assert result["violations"] == [] + assert result["pipeline_insights"] == 2 + assert result["relationships"] == 1 + on_disk = json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + assert on_disk["insights"]["overview"] == "Two-stage ingestion then transform." + + +def test_two_pass_deterministic_keys_byte_identical(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + enrich_inventory(tmp_path, insights=_good_insights()) + after = json.loads(path.read_text(encoding="utf-8")) + # every key except the added 'insights' is byte-identical to the pre-enrich file + after_without_insights = {k: v for k, v in after.items() if k != "insights"} + assert json.dumps(after_without_insights, indent=2) == before + + +def test_enrich_is_idempotent(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + enrich_inventory(tmp_path, insights=_good_insights()) + first = path.read_text(encoding="utf-8") + enrich_inventory(tmp_path, insights=_good_insights()) + second = path.read_text(encoding="utf-8") + assert first == second + + +def test_validation_failure_does_not_write(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + bad = _good_insights() + bad["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + result = enrich_inventory(tmp_path, insights=bad) + assert result["ok"] is False + assert result["violations"] + assert path.read_text(encoding="utf-8") == before # file untouched + + +def test_enrich_missing_inventory_raises(tmp_path: Path): + with pytest.raises(FileNotFoundError): + enrich_inventory(tmp_path, insights=_good_insights()) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k "load_insights or merge_into or enrich or two_pass or idempotent or validation_failure" -v` +Expected: FAIL with `ImportError: cannot import name 'enrich_inventory'` + +- [ ] **Step 3: Add the orchestrator functions** + +Append to `src/flowx/parser/pipeline_insights.py`: + +```python +def load_insights(*, insights: dict | None = None, insights_path: Path | None = None) -> dict: + """Return the raw insights dict from exactly one source (inline or file). + + Raises: + ValueError: if neither or both sources are provided. + """ + if (insights is None) == (insights_path is None): + raise ValueError("provide exactly one of 'insights' (inline dict) or 'insights_path'") + if insights is not None: + return insights + return json.loads(Path(insights_path).read_text(encoding="utf-8")) + + +def merge_into_inventory(inventory: dict, raw: dict) -> dict: + """Return a new dict identical to *inventory* with one added ``insights`` key. + + Does not mutate the input. No I/O. + """ + merged = dict(inventory) + merged["insights"] = raw + return merged + + +def enrich_inventory( + output_dir: Path, + *, + insights: dict | None = None, + insights_path: Path | None = None, +) -> dict: + """Validate authored insights against the inventory, then merge on success. + + Reads ``/metadata/inventory.json``, validates the authored + insights, and -- only when there are no violations -- writes the merged + inventory back byte-identically (adding just the ``insights`` key). + + Returns ``{"ok", "violations", "pipeline_insights", "relationships"}``. + On violations, ``ok`` is False and the file is left untouched. + + Raises: + FileNotFoundError: when ``inventory.json`` does not exist. + """ + inventory_path = Path(output_dir) / "metadata" / "inventory.json" + if not inventory_path.exists(): + raise FileNotFoundError(f"No inventory.json under {inventory_path.parent}; run discover first.") + inventory = json.loads(inventory_path.read_text(encoding="utf-8")) + + raw = load_insights(insights=insights, insights_path=insights_path) + violations = validate_insights(raw, inventory) + if violations: + return {"ok": False, "violations": violations, "pipeline_insights": 0, "relationships": 0} + + merged = merge_into_inventory(inventory, raw) + inventory_path.write_text(json.dumps(merged, indent=2), encoding="utf-8") + return { + "ok": True, + "violations": [], + "pipeline_insights": len(raw.get("pipeline_insights", [])), + "relationships": len(raw.get("pipeline_relationships", [])), + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -v` +Expected: PASS (all tests in the file) + +- [ ] **Step 5: Commit** + +```bash +git add src/flowx/parser/pipeline_insights.py tests/unit/test_pipeline_insights.py +git commit -m "$(cat <<'EOF' +Add enrich_inventory: validate-then-append two-pass write + +load_insights (inline|path), merge_into_inventory (pure), and +enrich_inventory (orchestrator). Byte-identical re-serialize adds only +the 'insights' key; validation runs before any write, so a rejected +payload leaves inventory.json untouched. Idempotent. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 4: Edge-binding tests on the real nested fixture + +This task hardens the validator against a *genuine* inventory built from the shipped fixture (not a hand-faked dict), proving control-edge identities resolve on real `activity_name` values. + +**Files:** +- Test: `tests/unit/test_pipeline_insights.py` + +**Interfaces:** +- Consumes: `load_adf_definitions` (`flowx.parser.adf_loader`), `build_inventory` (`flowx.parser.adf_loader`, attaches lineage at line 247), `_inventory_to_dict` (`flowx.parser.adf_loader`), and `validate_insights` (Task 2). The fixture `pipeline_execute_pipeline_nested.json` yields control edges with `activity_name` values `"Run Ingestion Pipeline"`, `"Run Transform Pipeline"`, `"Run Cleanup Pipeline"` and callees `pipeline_copy_sql_to_delta`, `pipeline_notebook_with_params`, `pipeline_delete_recursive`. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/unit/test_pipeline_insights.py`: + +```python +from flowx.parser.adf_loader import ( + _inventory_to_dict, + build_inventory, + load_adf_definitions, +) + + +def _real_inventory(fixtures_dir) -> dict: + """Build a real inventory dict (with lineage) from the shipped fixtures.""" + definitions = load_adf_definitions(fixtures_dir) + inventory = build_inventory(definitions) + return _inventory_to_dict(inventory, str(fixtures_dir)) + + +def test_control_edge_binding_matches_and_rejects(fixtures_dir): + inventory = _real_inventory(fixtures_dir) + good = { + "pipeline_insights": [], + "pipeline_relationships": [ + { + "from_pipeline": "pipeline_execute_pipeline_nested", + "to_pipeline": "pipeline_copy_sql_to_delta", + "lineage_edge": { + "edge_type": "control", + "edge_identity": "Run Ingestion Pipeline", + }, + } + ], + } + assert validate_insights(good, inventory) == [] + + bad = json.loads(json.dumps(good)) + bad["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "No Such Activity" + assert validate_insights(bad, inventory) + + +def test_real_inventory_enrich_round_trip(fixtures_dir, tmp_path: Path): + inventory = _real_inventory(fixtures_dir) + metadata = tmp_path / "metadata" + metadata.mkdir(parents=True) + (metadata / "inventory.json").write_text(json.dumps(inventory, indent=2), encoding="utf-8") + result = enrich_inventory( + tmp_path, + insights={ + "overview": "orchestrated ingest/transform/cleanup", + "pipeline_insights": [{"pipeline": "pipeline_execute_pipeline_nested", "intent": "orchestrate"}], + "pipeline_relationships": [], + }, + ) + assert result["ok"] is True + on_disk = json.loads((metadata / "inventory.json").read_text()) + assert on_disk["insights"]["pipeline_insights"][0]["pipeline"] == "pipeline_execute_pipeline_nested" +``` + +- [ ] **Step 2: Run test to verify it passes (validator already handles this)** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k "control_edge_binding or real_inventory" -v` +Expected: PASS — the validator from Task 2 already resolves control edges by `activity_name`. If any test fails, fix `validate_insights`, not the test. (This task is a regression guard against a real inventory, so no new production code is expected.) + +- [ ] **Step 3: Commit** + +```bash +git add tests/unit/test_pipeline_insights.py +git commit -m "$(cat <<'EOF' +Test insights validation against a real fixture-built inventory + +Builds a genuine inventory (with lineage) from the shipped nested +ExecutePipeline fixture and asserts control-edge identities resolve on +real activity_name values. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 5: The `enrich` adapter subcommand + +**Files:** +- Modify: `src/flowx/adapter/__main__.py` (add subparser in `_build_parser` after the `record` block ~line 362; add `_run_enrich` handler after `_run_record_results` ~line 116; add dispatch in `main` after line 88) +- Test: `tests/unit/test_pipeline_insights.py` + +**Interfaces:** +- Consumes: `enrich_inventory` (Task 3). +- Produces: CLI `python -m flowx.adapter enrich --output-dir [--insights-path ] [--insights ]`. Returns exit code 0 on success, 1 on any failure (missing inventory, bad/absent/both payload sources, validation violations). Exposed via `adapter.__main__.main(argv)`. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/unit/test_pipeline_insights.py`: + +```python +from flowx.adapter.__main__ import main as adapter_cli_main + + +def test_adapter_enrich_success(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + ins = tmp_path / "insights.json" + ins.write_text(json.dumps(_good_insights()), encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights-path", str(ins)]) + assert code == 0 + on_disk = json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + assert "insights" in on_disk + + +def test_adapter_enrich_validation_failure_returns_1(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + bad = _good_insights() + bad["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + ins = tmp_path / "insights.json" + ins.write_text(json.dumps(bad), encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights-path", str(ins)]) + assert code == 1 + assert path.read_text(encoding="utf-8") == before # untouched + + +def test_adapter_enrich_missing_inventory_returns_1(tmp_path: Path): + ins = tmp_path / "insights.json" + ins.write_text(json.dumps(_good_insights()), encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights-path", str(ins)]) + assert code == 1 + + +def test_adapter_enrich_inline_json_string(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + code = adapter_cli_main( + ["enrich", "--output-dir", str(tmp_path), "--insights", json.dumps(_good_insights())] + ) + assert code == 0 + assert "insights" in json.loads((tmp_path / "metadata" / "inventory.json").read_text()) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k adapter_enrich -v` +Expected: FAIL — argparse exits with code 2 ("invalid choice: 'enrich'") since the subcommand does not exist yet. + +- [ ] **Step 3: Add dispatch in `main()`** + +In `src/flowx/adapter/__main__.py`, add after line 88 (`return _run_record_results(args)`): + +```python + if args.command == "enrich": + return _run_enrich(args) +``` + +- [ ] **Step 4: Add the `_run_enrich` handler** + +Add after `_run_record_results` (after line 115), following its structure: + +```python +def _run_enrich(args: argparse.Namespace) -> int: + """Implements ``enrich``: validate + merge agent-authored insights into inventory.json. + + Returns 0 on success, 1 on any failure (missing inventory, unreadable/absent/ + both payload sources, or validation violations). + """ + from flowx.parser.pipeline_insights import enrich_inventory + + metadata_dir = args.output_dir / "metadata" + if not (metadata_dir / "inventory.json").exists(): + print(f"No inventory.json under {metadata_dir}; run the discover phase first.", file=sys.stderr) + return 1 + + inline: dict[str, Any] | None = None + if args.insights is not None: + try: + inline = json.loads(args.insights) + except json.JSONDecodeError as error: + print(f"Invalid --insights JSON: {error}", file=sys.stderr) + return 1 + if (inline is None) == (args.insights_path is None): + print("Provide exactly one of --insights (inline JSON) or --insights-path.", file=sys.stderr) + return 1 + + try: + result = enrich_inventory(args.output_dir, insights=inline, insights_path=args.insights_path) + except (OSError, ValueError, json.JSONDecodeError) as error: + print(f"Failed to enrich inventory: {error}", file=sys.stderr) + return 1 + + if not result["ok"]: + for violation in result["violations"]: + print(f" - {violation}", file=sys.stderr) + print( + f"Insights validation failed ({len(result['violations'])} violation(s)); " + "inventory not modified.", + file=sys.stderr, + ) + return 1 + print( + f"Enriched inventory: {result['pipeline_insights']} pipeline insight(s), " + f"{result['relationships']} relationship(s)." + ) + return 0 +``` + +- [ ] **Step 5: Add the `enrich` subparser** + +In `_build_parser`, add after the `record` subparser block (after line 362, before the `dashboard` parser): + +```python + enrich = subparsers.add_parser( + "enrich", + help="Validate and merge agent-authored insights into metadata/inventory.json.", + ) + enrich.add_argument( + "--output-dir", + type=Path, + required=True, + help="Migration output directory (reads/writes metadata/inventory.json).", + ) + enrich.add_argument( + "--insights-path", + type=Path, + default=None, + help="Path to a JSON file holding the insights object.", + ) + enrich.add_argument( + "--insights", + type=str, + default=None, + help="Insights object as an inline JSON string (convenience for direct CLI use).", + ) +``` + +- [ ] **Step 6: Run tests to verify they pass** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k adapter_enrich -v` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add src/flowx/adapter/__main__.py tests/unit/test_pipeline_insights.py +git commit -m "$(cat <<'EOF' +Add 'enrich' adapter subcommand + +python -m flowx.adapter enrich --output-dir (--insights-path +| --insights ). Validates + merges insights; returns 1 (inventory +untouched) on missing inventory, bad payload, or validation violations. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 6: MCP `enrich` command + `runner.materialize_json` + +**Files:** +- Modify: `src/flowx/mcp/runner.py` (add `materialize_json` near `materialize_adf_definitions` ~line 156) +- Modify: `src/flowx/mcp/server.py` (add `_cmd_enrich` before `_COMMANDS` ~line 363; register `"enrich"` in `_COMMANDS` ~line 377; add a docstring bullet ~line 426) +- Test: `tests/unit/test_pipeline_insights.py` + +**Interfaces:** +- Consumes: `runner.run_adapter`, `runner.summarize_inventory`, `runner.materialize_json` (new), `runner.cleanup_materialized`, `server._phase_result`. +- Produces: `runner.materialize_json(obj: Any) -> str` (writes a temp JSON file, returns its path; cleaned up by `cleanup_materialized`). `server._cmd_enrich(p)` accepting `output_dir`, `insights` (inline dict) or `insights_path`. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/unit/test_pipeline_insights.py`: + +```python +from flowx.mcp import runner as mcp_runner +from flowx.mcp.server import _cmd_enrich + + +def test_materialize_json_round_trips(tmp_path: Path): + path = mcp_runner.materialize_json({"overview": "x"}) + try: + assert json.loads(Path(path).read_text()) == {"overview": "x"} + finally: + mcp_runner.cleanup_materialized(path) + assert not Path(path).exists() + + +def test_cmd_enrich_requires_a_payload(): + result = _cmd_enrich({"output_dir": "./flowx_output"}) + assert result["ok"] is False + assert "insights" in result["error"] + + +def test_cmd_enrich_inline_dict_success(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + result = _cmd_enrich({"output_dir": str(tmp_path), "insights": _good_insights()}) + assert result["ok"] is True + assert "insights" in json.loads((tmp_path / "metadata" / "inventory.json").read_text()) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k "materialize_json or cmd_enrich" -v` +Expected: FAIL with `ImportError: cannot import name '_cmd_enrich'` / `AttributeError: module ... has no attribute 'materialize_json'` + +- [ ] **Step 3: Add `materialize_json` to the runner** + +In `src/flowx/mcp/runner.py`, add after `materialize_adf_definitions` (after line 204): + +```python +def materialize_json(obj: Any) -> str: + """Write a JSON-serialisable object to a temp file and return its path. + + Lets the MCP server pass an inline ``insights`` dict to the adapter's + ``enrich`` subcommand (which reads from ``--insights-path``). Clean up with + :func:`cleanup_materialized`. + """ + fd, path = tempfile.mkstemp(prefix="flowx-insights-", suffix=".json") + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(obj, handle) + return path +``` + +(`os`, `json`, `tempfile` are already imported at the top of runner.py — lines 12-18.) + +- [ ] **Step 4: Add `_cmd_enrich` to the server** + +In `src/flowx/mcp/server.py`, add before `_COMMANDS` (before line 365): + +```python +def _cmd_enrich(p: dict[str, Any]) -> dict[str, Any]: + output_dir = p.get("output_dir", "./flowx_output") + insights = p.get("insights") + insights_path = p.get("insights_path") + if insights is None and not insights_path: + return {"ok": False, "error": "Provide 'insights' (inline dict) or 'insights_path'."} + tmp: str | None = None + try: + if insights is not None: + tmp = runner.materialize_json(insights) + insights_path = tmp + args: list[Any] = ["enrich", "--output-dir", output_dir, "--insights-path", insights_path] + result = runner.run_adapter(args) + out = Path(output_dir) + return _phase_result(result, out, inventory=runner.summarize_inventory(out)) + finally: + if tmp: + runner.cleanup_materialized(tmp) +``` + +- [ ] **Step 5: Register the command and document it** + +In `src/flowx/mcp/server.py`, add to the `_COMMANDS` dict (after line 368, `"convert": _cmd_convert,` grouping — place it right after `"discover": _cmd_discover,`): + +```python + "enrich": _cmd_enrich, +``` + +Then add a bullet to the `flowx` tool docstring after the "discover" bullet (after line 408): + +```python + - "enrich": output_dir(req), one of insights(inline dict) | insights_path — validate + merge + agent-authored insights into metadata/inventory.json (returns {ok:false, ...} without writing + on validation failure). +``` + +- [ ] **Step 6: Run tests to verify they pass** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_pipeline_insights.py -k "materialize_json or cmd_enrich" -v` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add src/flowx/mcp/runner.py src/flowx/mcp/server.py tests/unit/test_pipeline_insights.py +git commit -m "$(cat <<'EOF' +Add 'enrich' MCP command + runner.materialize_json + +_cmd_enrich materializes an inline insights dict to a temp file and drives +the adapter enrich subcommand; runner.materialize_json parallels +materialize_adf_definitions. Registered in _COMMANDS and documented. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 7: Optional `has_insights` reporting column + +Optional per the spec — include only if it stays trivial and does not perturb existing coverage tests. + +**Files:** +- Modify: `src/flowx/reporting/coverage.py` (`COVERAGE_METRIC_COLUMNS` ~line 22; `build_coverage_rows` ~line 57) +- Test: `tests/unit/test_reporting_coverage.py` + +**Interfaces:** +- Consumes: the inventory dict already loaded in `build_coverage_rows` at line 73. +- Produces: a `has_insights` boolean field on each coverage row, gated on the top-level `insights` key. Non-enriched inventories yield `False`; the column is identical for every pipeline in a run (it is a factory-level marker). + +- [ ] **Step 1: Write the failing test** + +Add to `tests/unit/test_reporting_coverage.py` (the module already imports `json`, `Path`, and `build_coverage_rows`, and defines the `_write_metadata(tmp_path) -> Path` helper that writes `metadata/inventory.json` without an `insights` key): + +```python +def test_has_insights_column_reflects_insights_key(tmp_path: Path): + md = _write_metadata(tmp_path) # writes inventory.json with no insights key + rows = build_coverage_rows(md) + assert all(row["has_insights"] is False for row in rows) + + inv_path = md / "inventory.json" + inv = json.loads(inv_path.read_text()) + inv["insights"] = {"overview": "x", "pipeline_insights": [], "pipeline_relationships": []} + inv_path.write_text(json.dumps(inv), encoding="utf-8") + rows2 = build_coverage_rows(md) + assert all(row["has_insights"] is True for row in rows2) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_reporting_coverage.py::test_has_insights_column_reflects_insights_key -v` +Expected: FAIL with `KeyError: 'has_insights'` + +- [ ] **Step 3: Add the column** + +In `src/flowx/reporting/coverage.py`, append `"has_insights"` to `COVERAGE_METRIC_COLUMNS` (after `"complexity_size"` at line 36): + +```python + "complexity_size", + "has_insights", +``` + +Then, in `build_coverage_rows`, compute the factory-level marker once just before the `rows: list[dict[str, Any]] = []` line (line 82): + +```python + has_insights = "insights" in inventory +``` + +and add `"has_insights": has_insights,` as the last entry of the per-pipeline dict appended in the loop, immediately after `"complexity_size": csv_row.get("complexity_size", "") or "",` (line 113): + +```python + "complexity_size": csv_row.get("complexity_size", "") or "", + "has_insights": has_insights, +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `PYTHONPATH=src uv run pytest tests/unit/test_reporting_coverage.py -v` +Expected: PASS. The two existing coverage tests (`test_build_coverage_rows_joins_inventory_and_csv`, `test_build_coverage_rows_full_coverage_and_missing_csv`) assert individual named columns, not an exact column count or the full `COVERAGE_METRIC_COLUMNS` tuple, so adding a column does not break them. + +- [ ] **Step 5: Commit** + +```bash +git add src/flowx/reporting/coverage.py tests/unit/test_reporting_coverage.py +git commit -m "$(cat <<'EOF' +Add has_insights coverage column (gated on insights key) + +Factory-level marker in the coverage rows; False for non-enriched +inventories so existing runs are unaffected. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 8: Discover skill — Step 5 (author → enrich) + summary rewording + +**Files:** +- Modify: `skills/flowx-discover/SKILL.md` + +**Interfaces:** +- Consumes: the `enrich` MCP command (Task 6) and the `enrich` adapter subcommand (Task 5). Documentation only — no test cycle; verified manually in Task 9. + +- [ ] **Step 1: Insert the new Step 5 (author → enrich)** + +In `skills/flowx-discover/SKILL.md`, insert this new section between the current Step 4b (ends at line 212) and the current `### Step 5 — Present the summary` (line 214): + +````markdown +### Step 5 — Author and merge agentic insights + +The deterministic inventory records *what* each pipeline contains; it cannot +record *what the factory is trying to do* or *how the pipelines relate as a +system*. Author that judgment now and merge it into `inventory.json` under an +`insights` key. This always runs. + +1. **Read** the just-written `inventory.json` (`pipelines`, `lineage`, `summary`) + and `profile_report.csv`. +2. **Author** an `insights` object: + - `overview` — the whole factory as one system, plus the single biggest + migration steer. + - `pipeline_insights[]` — **sparse**; per pipeline an `intent` and a + `databricks_pattern` (optionally `pattern_name`, + `recommended_databricks_features`, `conversion_notes`). Omit pipelines with + nothing worth saying. + - `pipeline_relationships[]` — annotate the lineage edges the discover phase + already found. Each relationship carries a `lineage_edge` + (`{edge_type, edge_identity}`) plus `relationship_summary`, + `databricks_pattern`, and `risk_if_ignored`. For a **control** edge, + `edge_identity` is the edge's `activity_name`; for a **data** edge, it is + the edge's `match_key` — copied **verbatim** from a real edge in + `lineage`. + - **Authoring rules:** reference only pipeline names that exist in the + inventory; every `lineage_edge` must echo a real edge; do not invent + relationships the lineage did not find (annotate, don't rediscover). +3. **Enrich** — merge the object in: + + - **MCP tool path** (inline dict; the only path in Genie Code): + + ``` + flowx(command="enrich", parameters={ + "output_dir": "", + "insights": { ...authored object... }}) + ``` + + - **venv CLI fallback** (write the object to a JSON file first): + + ```bash + "$PY" -m flowx.adapter enrich --output-dir --insights-path + ``` + +4. **On `ok:false`** the tool did **not** write the file: read `violations`, fix + the offending pipeline name / lineage edge / field, and call `enrich` again. + On `ok:true` the `insights` key is now merged into `inventory.json`. +```` + +- [ ] **Step 2: Renumber the existing reporting steps** + +Renumber the four existing headings that follow (they currently read Steps 5–8): +- `### Step 5 — Present the summary` → `### Step 6 — Present the summary` +- `### Step 6 — Detail agentic activities` → `### Step 7 — Detail agentic activities` +- `### Step 7 — Warn about unsupported activities` → `### Step 8 — Warn about unsupported activities` +- `### Step 8 — Confirm output location` → `### Step 9 — Confirm output location` + +- [ ] **Step 3: Reword the summary step to surface insights** + +In the renumbered `### Step 6 — Present the summary`, after the existing summary code block (the `Coverage: 95.7%` block ending at line 230), append: + +````markdown +Then surface the authored judgment so the user sees *what the factory does*, not +just coverage numbers: print the factory `overview`, and for each +`pipeline_insights` entry its `pattern_name` / `intent` and recommended +Databricks pattern. Read these back from the enriched `inventory.json`. +```` + +- [ ] **Step 4: Add a "Future considerations" note** + +At the end of the file (after the `## Output Artifacts` table, line 273), append: + +````markdown +## Future considerations + +Insights are currently authored in a single pass over the whole factory. For very +large factories, revisit partitioning the authoring across subagents keyed on +lineage clusters (the connected components of the combined control/data-edge +graph), so each subagent reasons about one coherent subsystem. Out of scope for +now — always enrich in one pass. +```` + +- [ ] **Step 5: Verify the skill reads coherently** + +Run: `PYTHONPATH=src uv run pytest tests/unit -q` (sanity — no test touches the skill, but confirm nothing regressed) +Read the edited `skills/flowx-discover/SKILL.md` end-to-end and confirm: steps are numbered 1→9 with no duplicates or gaps, the new Step 5 sits between 4b and the summary, and both tool paths are shown. + +- [ ] **Step 6: Commit** + +```bash +git add skills/flowx-discover/SKILL.md +git commit -m "$(cat <<'EOF' +Add discover Step 5: author + enrich agentic insights + +New always-on author->enrich loop (both MCP and venv-CLI paths), summary +step reworded to surface factory overview + per-pipeline intent/pattern, +and a future-considerations note on partitioning authoring by lineage +cluster. Renumbers the reporting steps to 6-9. + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Task 9: End-to-end verification + format/lint gate + +**Files:** none (verification only) + +**Interfaces:** exercises the shipped `discover` → `enrich` path end-to-end over the repo fixtures. + +- [ ] **Step 1: Run the full unit suite** + +Run: `PYTHONPATH=src uv run pytest tests/unit -v` +Expected: PASS (all tests, including the new `test_pipeline_insights.py` and the coverage test). + +- [ ] **Step 2: Format + lint (ruff + mypy)** + +Run: `make fmt` +Expected: ruff formats/fixes cleanly and `mypy src/flowx/` reports no errors. Fix any type errors (e.g. add annotations) and re-run until clean. + +- [ ] **Step 3: End-to-end discover → enrich over fixtures** + +Run (real CLI, real fixture inventory, temp output dir): + +```bash +cd /Users/matthew.moorcroft/Code/work/flowx-worktrees/feat-discover-insights +OUT="$(mktemp -d)" +PYTHONPATH=src uv run python -m flowx.adapter discover \ + --adf-source-path tests/resources/json --output-dir "$OUT" +# capture the deterministic portion before enrich +PYTHONPATH=src uv run python -c "import json,sys; d=json.load(open(sys.argv[1])); print(json.dumps({k:v for k,v in d.items() if k!='insights'}, indent=2))" "$OUT/metadata/inventory.json" > /tmp/before.json +# author a tiny valid insights object referencing a real pipeline + control edge, then enrich +PYTHONPATH=src uv run python -m flowx.adapter enrich --output-dir "$OUT" --insights '{"overview":"fixtures","pipeline_insights":[{"pipeline":"pipeline_execute_pipeline_nested","intent":"orchestrate"}],"pipeline_relationships":[{"from_pipeline":"pipeline_execute_pipeline_nested","to_pipeline":"pipeline_copy_sql_to_delta","lineage_edge":{"edge_type":"control","edge_identity":"Run Ingestion Pipeline"}}]}' +# confirm insights present AND deterministic portion byte-identical +PYTHONPATH=src uv run python -c "import json,sys; d=json.load(open(sys.argv[1])); assert 'insights' in d; print('insights present:', bool(d['insights']))" "$OUT/metadata/inventory.json" +PYTHONPATH=src uv run python -c "import json,sys; d=json.load(open(sys.argv[1])); print(json.dumps({k:v for k,v in d.items() if k!='insights'}, indent=2))" "$OUT/metadata/inventory.json" > /tmp/after.json +diff /tmp/before.json /tmp/after.json && echo "DETERMINISTIC PORTION BYTE-IDENTICAL" +rm -rf "$OUT" /tmp/before.json /tmp/after.json +``` + +Expected: the enrich prints `Enriched inventory: 1 pipeline insight(s), 1 relationship(s).`, `insights present: True`, and `diff` reports no differences (`DETERMINISTIC PORTION BYTE-IDENTICAL`). + +- [ ] **Step 4: Confidentiality grep** + +Run: `git grep -nE "a customer factory|a large factory|engagementID|engagementDBVersions|etl-parameters" -- ':!docs/superpowers/specs/'` +Expected: no output (zero hits outside the spec). + +- [ ] **Step 5: Final commit (only if Steps 2/3 required fixups)** + +```bash +git add -A +git commit -m "$(cat <<'EOF' +Format/lint fixups for discover insights + +Co-authored-by: Isaac +EOF +)" +``` + +--- + +## Self-Review + +**1. Spec coverage** (checked against `docs/superpowers/specs/2026-07-23-discover-insights-design.md`): +- §4 data models → Task 1. §5 parser (`load_insights`/`validate_insights`/`merge_into_inventory`/`enrich_inventory`) → Tasks 2–3. §6a adapter subcommand → Task 5. §6b MCP command + `materialize_json` → Task 6. §7 skill Step 5 + summary + future note → Task 8. §8 test matrix → Tasks 2–6 (each issue invariant maps to a named test); edge-binding on the real fixture → Task 4; optional reporting column → Task 7; end-to-end → Task 9. §3 resolved decisions (data-edge on `match_key`, no `schema_version`, always-enrich, validate-before-write, byte-identical) are enforced in Global Constraints + Tasks 3/8. §10 confidentiality → Global Constraints + Task 9 Step 4. +- Every §8 test row has a home: `test_validator_accepts_good_insights` (T2), `test_rejects_pipeline_not_in_inventory` (T2), `test_rejects_unresolvable_*_edge` (T2), `test_rejects_missing_required_field` (T2), `test_rejects_unknown_field` (T2), `test_control_edge_binding_matches_and_rejects` (T4), `test_data_edge_binds_on_match_key` (T2), `test_two_pass_deterministic_keys_byte_identical` (T3), `test_enrich_is_idempotent` (T3), `test_validation_failure_does_not_write` (T3). + +**2. Placeholder scan:** No "TBD"/"handle edge cases"/"similar to Task N" — every code step shows complete code. Task 7 anchors its edits to exact line numbers and the real `_write_metadata` helper (verified present in `test_reporting_coverage.py`), so no "inspect at implementation time" hand-waving remains. + +**3. Type consistency:** `enrich_inventory(output_dir, *, insights=None, insights_path=None) -> dict` used identically in Tasks 3, 5, 6. Return keys `ok`/`violations`/`pipeline_insights`/`relationships` consistent across Task 3 (definition), Task 5 (`_run_enrich` reads `result["ok"]`, `result["violations"]`, `result["pipeline_insights"]`, `result["relationships"]`). `validate_insights(raw, inventory) -> list[str]` consistent across Tasks 2, 3, 4. `materialize_json(obj) -> str` / `cleanup_materialized(path)` consistent in Task 6. `LineageEdgeRef`/`edge_type`/`edge_identity` naming consistent between Task 1 models and the validator's `_EDGE_KEYS` in Task 2. diff --git a/docs/superpowers/specs/2026-07-23-discover-insights-design.md b/docs/superpowers/specs/2026-07-23-discover-insights-design.md new file mode 100644 index 0000000..e3a23e0 --- /dev/null +++ b/docs/superpowers/specs/2026-07-23-discover-insights-design.md @@ -0,0 +1,449 @@ +# Design — Agentic `insights` in `inventory.json` (discover phase) + +**Issue:** databricks-field-eng/flowx #11 — *[FEATURE]: Agentic insights in inventory.json (discover phase)* +**Depends on:** #9 (deterministic lineage), implemented in PR #18 on branch `feat/discover-lineage`. +**Branch:** `feat/discover-insights`, cut from `feat/discover-lineage`. PR bases on `feat/discover-lineage`; retarget to `main` after #18 merges. +**Status:** Design approved; ready for implementation planning. + +--- + +## 1. Summary + +Add an agent-authored **`insights`** key to `metadata/inventory.json` during the **discover** phase. It gives +the later **convert** phase two things it cannot derive deterministically: + +1. **What each pipeline is trying to achieve** and the Databricks pattern that maps to it. +2. **How pipelines relate across the whole factory**, by *annotating* #9's deterministic `lineage` edges. + +This issue only **produces** the block. A separate follow-up wires `convert` to consume it (mirrors the +#9 produce → consume split). + +**Design principle — every edge is accountable.** The inventory + `lineage` are the source of truth for +deterministic *facts*. The agent adds *judgment*, referencing pipelines and edges by their existing identifiers. +Most insight is therefore cheap to validate: named pipelines must exist; annotated (`control`/`data`) edges must +resolve to a real lineage edge — *annotate, don't rediscover*. The one exception is genuine coupling the +deterministic layer structurally cannot see (data flow inside notebook code, external triggers): the agent may +assert an `inferred` edge, but it is held to the same accountability by a different key — it must cite non-empty +`evidence` and a `confidence` level, and is clearly distinguished from proven lineage. Scope is deliberately +**factory / pipeline / relationship** level — **no per-activity fields** (convert already handles that level +well). + +--- + +## 2. The two steps: "author" → "enrich" + +Step 5 of the discover skill is a small loop with a clear division of labour. There is **no LLM inside the +tool** — the tool only validates and merges. + +| Step | Who | What | +|---|---|---| +| discover (pass 1) | deterministic | Writes pure `inventory.json` (`pipelines`, `summary`, `lineage`). No `insights`. | +| **author** | the agent | Reads inventory + lineage + profile; writes the `insights` JSON (intent, patterns, relationships). The LLM-judgment part. | +| **enrich** | the tool | `flowx(command="enrich")`: validates the authored JSON against the inventory, then merges. Pure code. | +| enrich (pass 2) | deterministic | Appends **only** the `insights` key; re-serializes the deterministic portion **byte-identical**. Idempotent. | + +**Author happens first; enrich happens second.** The tool always validates whatever it is handed. The decision +of *whether* Step 5 runs lives in the **skill**, never in the tool — and per the decisions below, it **always +runs**. + +### Call shape + +```python +flowx(command="enrich", parameters={ + "output_dir": "./flowx_output", # locates metadata/inventory.json + "insights": { ... }, # inline dict (hosted / Genie path) — OR — + "insights_path": "/path/insights.json", # a readable path (local / CLI path) +}) +# Returns {ok, process, ...} + a merge summary. +# On ANY validation failure → {ok: false, violations: [...]} and DOES NOT WRITE the file. +``` + +--- + +## 3. Resolved decisions + +These supersede the issue's open questions and loose wording. + +- **`edge_identity` grammar (resolves the issue's open question).** + - `edge_type: "control"` → `edge_identity` = the `ControlEdge.activity_name` (the ExecutePipeline activity name). + - `edge_type: "data"` → `edge_identity` = the `DataEdge.match_key` — **not** "shared table/path". #9's data + edges carry a two-tier join (`match_kind` = `identity` | `expression`); `match_key` is the canonical join + value and `identity` is `null` for expression edges, so `match_key` is the only stable key. **Validation + matches on `match_key`.** + - `edge_type: "inferred"` → an agent-asserted coupling the deterministic layer never found, so there is **no** + lineage edge to resolve against. `edge_identity` is an agent-authored descriptor of the coupling (e.g. the + shared table/asset). Because it cannot be checked against a fact, the edge **must** instead carry a non-empty + `evidence` string and a `confidence` ∈ {`high`, `medium`, `low`}; validation enforces those and skips lineage + resolution. `evidence` / `confidence` are inferred-only — supplying them on a `control`/`data` edge is a + violation. +- **Why the `inferred` tier (generic data-flow capture).** The annotate-only edges (`control`/`data`) can only + describe couplings the deterministic layer surfaced. But a notebook-centric factory expresses its real data + flow *inside* notebook code (one notebook writes a table another reads), which ADF never names, so #9 finds + **zero** data edges there. The `inferred` tier gives the agent a structured, accountable place to record that + coupling instead of burying it in prose. It is deliberately **pattern-agnostic**: it does not encode *why* the + deterministic layer missed the edge (notebook I/O, external trigger, message queue, an ADF pattern we have not + seen), so it generalises. The evidence+confidence requirement preserves the invariant's spirit — every edge is + accountable to something (a proven fact for annotations, stated evidence for inferences) and an inference can + never masquerade as proven lineage. +- **No new inventory fields; ARM is the deep-dive source.** The agent must characterize data flow, which for + notebook-centric factories means reading what activities actually do. Rather than lift selected `typeProperties` + (e.g. `notebookPath`) into `inventory.json` — a treadmill that would repeat for every known and unknown activity + type and defeat the "generic" goal — Step 5 sends the agent to the verbatim `metadata/*.arm.json`, which already + contains everything for every pattern. The inventory stays the lean deterministic skeleton. +- **ARM files are addressed by glob-and-match, never a constructed name.** `write_pipeline_arm` emits one + `.arm.json` per pipeline via `_sanitize_filename` (a lossy slug: `[^0-9A-Za-z._-]+ → _`). A + filename therefore cannot be reliably reconstructed from a pipeline name (spaces/parens/unicode are mangled; + distinct names can collide to one stem). Step 5 instructs the agent to **glob `metadata/*.arm.json` and match on + the top-level `"name"` field inside each file**, not to build `.arm.json`. There is no single fixed + filename. Each file is a **flat single-pipeline object** (`{"name", "properties": {"activities": [...]}}`) — not + a multi-resource ARM envelope, so there is no `resources[]` array and no top-level `type`; activities live under + `properties.activities` (recurse nested `ForEach`/`If`/`Switch`). Step 5's wording states this shape explicitly + (a clean-room run misparsed an assumed `resources[]` envelope, so the shape is called out to prevent it). +- **Step 5 authoring guidance is criteria-based, not count-based (validated by a clean-room run).** A no-context + subagent run on a 327-pipeline factory surfaced two instruction gaps, fixed per prompt-engineering best practice + (Anthropic prompting docs: criteria over fixed numbers, explain the *why*, diverse non-skewed examples; few-shot + surface-feature bias, Zhao et al. 2021): + - *Sparse selection* is expressed as **ANY-of inclusion tests + coverage-by-role + a decision-relevance bar + + "omit is the default"**, with an explicit "guide, not a quota" escape hatch — so it scales from tiny to huge + factories without a hardcoded count (the run guessed with no sense of scale). + - *Inferred vs. annotation* is defined **by one axis — did the deterministic phase already record this as a + lineage edge — explicitly NOT by mechanism**, with an ordered decision rule, an "if in doubt → inferred" + tie-breaker, and diverse sub-case examples (data-in-code, `dependsOn` ordering, shared control asset) plus a + near-miss. The prior single-flavour examples had biased the tier toward the data-in-code sub-case. +- **Two whole-factory recommendation patterns in Step 5 (isolated, revertible).** A second-opinion review of a + real enriched run found the agent tends to *transliterate* (re-implement an ADF tier as a called job) when the + better migration is to *replace* it with a native capability, and does not actively surface *clone families*. + Step 5 now teaches two generic patterns — **"replace, don't transliterate"** (a tier whose sole purpose is a + capability Databricks offers natively → eliminate it: observability→system tables, control tables→task values, + config engines→Python-on-Jobs-API; guarded so it never recommends deleting pipelines that do real work) and + **"collapse clone families"** (cluster by activity signature, emit one insight per family recommending a single + parameterized job with the count). Both reuse the existing insight fields (no schema change) and are kept as a + single self-contained commit so they can be reverted wholesale if the added opinionation proves low-value. +- **No `schema_version` field.** A draft one was removed on #9's branch; it had no consumer. The presence of the + top-level **`insights` key IS the "enriched" marker**. +- **Skip gate: none — always enrich.** The value of `insights` (recovering intent + cross-pipeline + relationships that deterministic analysis structurally cannot recover) holds at every factory size; if + anything it grows with scale, because a per-pipeline converter is most blind to the whole-system picture on a + large factory. Step 5 therefore always authors + enriches. See §9 for the future-review note on partitioning + at scale. +- **Validator is hand-rolled, violation-collecting.** `validate_insights(raw, inventory) -> list[str]` walks the + dict, checks required/unknown fields explicitly, and resolves FKs against sets built from the inventory. No new + dependency; collects **all** violations (not fail-fast) so the agent fixes everything in one pass; matches the + plain-dict style of `merge_agentic_results`. +- **Inline payload reaches the CLI via a temp file.** `_cmd_enrich` materializes an inline `insights` dict to a + temp JSON file and passes `--insights-path` (mirrors `materialize_adf_definitions`), cleaning up after. The CLI + therefore needs only one input mode; `--insights` (raw JSON string) is a thin convenience for direct CLI users. +- **Byte-identical write.** Read `inventory.json` text → `json.loads` → set the single `insights` key → + `json.dumps(obj, indent=2)` write-back, using discover's exact dump options. Existing keys keep order and + formatting; re-running is idempotent. Validation runs **before** any write. + +--- + +## 4. Data models (`models/adf_ast.py`) + +Four new `@dataclass(slots=True, kw_only=True)` types, placed right after `Lineage`. FKs are required (no +default); everything else is optional so authoring stays sparse. + +```python +@dataclass(slots=True, kw_only=True) +class LineageEdgeRef: + """A typed reference from a PipelineRelationship to one cross-pipeline edge. + control/data annotate a deterministic #9 edge; inferred is an agent-asserted coupling.""" + edge_type: Literal["control", "data", "inferred"] + edge_identity: str # control → ControlEdge.activity_name; data → DataEdge.match_key; + # inferred → agent-authored descriptor of the coupling + evidence: str | None = None # required for inferred; must be absent otherwise + confidence: Literal["high", "medium", "low"] | None = None # required for inferred; absent otherwise + +@dataclass(slots=True, kw_only=True) +class PipelineInsight: + """Per-pipeline judgment; references a pipeline by name (FK).""" + pipeline: str # FK → pipelines[].name (validated) + pattern_name: str | None = None + intent: str | None = None + databricks_pattern: str | None = None + recommended_databricks_features: list[str] = field(default_factory=list) + conversion_notes: list[str] = field(default_factory=list) + +@dataclass(slots=True, kw_only=True) +class PipelineRelationship: + """Cross-pipeline judgment; annotates one #9 lineage edge.""" + from_pipeline: str # FK (validated) + to_pipeline: str # FK (validated) + lineage_edge: LineageEdgeRef # must resolve to a real #9 edge (validated) + relationship_summary: str | None = None + databricks_pattern: str | None = None + risk_if_ignored: str | None = None + +@dataclass(slots=True, kw_only=True) +class Insights: + overview: str | None = None + pipeline_insights: list[PipelineInsight] = field(default_factory=list) + pipeline_relationships: list[PipelineRelationship] = field(default_factory=list) +``` + +These dataclasses are the **typed round-trip / serialization** side. Validation of the raw agent JSON is done by +the pure validator (§5) *before* any dataclass is constructed, so unknown/missing fields produce collected +violation strings rather than raw `TypeError`s. + +### Why `LineageEdgeRef` (and not just `from`/`to` + prose) + +`LineageEdgeRef` carries **no facts of its own** — it is a typed foreign key into #9's lineage. It exists for +three reasons: + +1. **Binds judgment to a validatable fact.** A relationship's prose is unanchored on its own; the ref forces it + to point at one real edge (echoing `activity_name` / `match_key`), so `enrich` can resolve it and reject a + relationship that references an edge #9 never found. This is the mechanism that enforces "annotate, don't + rediscover." +2. **Disambiguates multiple facets of one pair.** The same `(from, to)` can be connected by both a control + invocation *and* a data hand-off, each needing a different Databricks pattern and carrying a different risk. + Relationships key on `(from, to, edge_type, edge_identity)`; the ref makes the pair-plus-facet addressable. +3. **Stable across re-runs.** The ref points at #9's canonical keys, not prose or array indices, so a + regenerated lineage still resolves (or fails cleanly if the edge genuinely disappeared). + +--- + +## 5. Parser module (`parser/pipeline_insights.py`) + +New file, sibling to #9's `parser/lineage.py`, shaped like `merge_agentic_results`. + +```python +def load_insights(*, insights: dict | None = None, insights_path: Path | None = None) -> dict: + """Return the RAW insights dict from an inline dict OR a JSON file. + Exactly one source must be provided (not both, not neither). Not yet validated.""" + +def validate_insights(raw: dict, inventory: dict) -> list[str]: + """Pure validator. Returns a list of human-readable violation strings (empty == valid). + Collects ALL violations, never fail-fast. Checks: + - top-level shape: only {overview, pipeline_insights, pipeline_relationships} + - each PipelineInsight: 'pipeline' present & ∈ inventory pipeline names; + no unknown fields; field types (lists are lists, strings are strings) + - each PipelineRelationship: from_pipeline / to_pipeline present & ∈ names; + lineage_edge present, well-formed (edge_type ∈ {control, data, inferred}, edge_identity str), + no unknown fields, and per tier: + control → edge_identity RESOLVES to some ControlEdge.activity_name; no evidence/confidence + data → edge_identity RESOLVES to some DataEdge.match_key; no evidence/confidence + inferred → NOT resolved against lineage; requires non-empty evidence str + and confidence ∈ {high, medium, low} + """ + +def merge_into_inventory(inventory: dict, raw: dict) -> dict: + """Pure. Return a NEW dict identical to `inventory` with exactly one added key, + 'insights', set to `raw`. Does not mutate the input. No I/O.""" + +def enrich_inventory(output_dir: Path, *, insights=None, insights_path=None) -> dict: + """Orchestrator (the only function with I/O): + 1. read /metadata/inventory.json (error if missing) + 2. raw = load_insights(...) + 3. violations = validate_insights(raw, inventory) + 4. if violations: return {ok: False, violations, ...} # NO WRITE + 5. merged = merge_into_inventory(inventory, raw) + 6. write back with json.dumps(merged, indent=2) # byte-identical prior keys + 7. return {ok: True, violations: [], pipeline_insights: N, relationships: M} + """ +``` + +Invariant-locking details: + +- **Validation before I/O** — step 4 returns before any write, satisfying "on failure, do not write the file." +- **FK sets built once** from `inventory["pipelines"][*]["name"]`, `lineage.control_edges[*].activity_name`, and + `lineage.data_edges[*].match_key` — plain set membership, no guessing. +- **Byte-identical** falls out of re-dumping the parsed dict with `indent=2` (discover's exact options) and only + *adding* a key — existing keys keep order and formatting. Idempotent because re-running overwrites `insights` + with an equal value. +- **No `default=str`** (unlike `merge_agentic_results`) — insights are plain JSON scalars/lists; matching + discover's `json.dumps(..., indent=2)` call exactly is what guarantees the byte-for-byte round-trip. + +--- + +## 6. Adapter subcommand & MCP command + +### 6a. Adapter CLI subcommand `enrich` (`adapter/__main__.py`, modeled on `record-results`) + +Standalone subcommand — deliberately **not** a `discover` flag — so it re-runs without re-parsing ADF. + +```python +enrich = subparsers.add_parser( + "enrich", + help="Merge agent-authored insights into metadata/inventory.json (validate + append).", +) +enrich.add_argument("--output-dir", type=Path, required=True, + help="Migration output directory (reads/writes metadata/inventory.json).") +enrich.add_argument("--insights-path", type=Path, default=None, + help="Path to a JSON file holding the insights object.") +enrich.add_argument("--insights", type=str, default=None, + help="Insights object as an inline JSON string (convenience for direct CLI use).") + +# in main(): +if args.command == "enrich": + return _run_enrich(args) +``` + +```python +def _run_enrich(args) -> int: + """Validate + merge insights into inventory.json. Returns 0 on success, 1 on any failure + (missing inventory, unreadable/absent/both payload sources, validation violations).""" + from flowx.parser.pipeline_insights import enrich_inventory + metadata_dir = args.output_dir / "metadata" + if not (metadata_dir / "inventory.json").exists(): + print(f"No inventory.json under {metadata_dir}; run discover first.", file=sys.stderr) + return 1 + # resolve exactly one payload source (parse --insights JSON string if given) + result = enrich_inventory(args.output_dir, insights=, insights_path=args.insights_path) + if not result["ok"]: + for v in result["violations"]: + print(f" - {v}", file=sys.stderr) + print(f"Insights validation failed ({len(result['violations'])} violation(s)); " + f"inventory not modified.", file=sys.stderr) + return 1 + print(f"Enriched inventory: {result['pipeline_insights']} pipeline insight(s), " + f"{result['relationships']} relationship(s).") + return 0 +``` + +### 6b. MCP command `_cmd_enrich` (`mcp/server.py`, added to `_COMMANDS` as `"enrich"`) + +Mirrors `_cmd_discover`'s inline-payload handling: materialize the inline dict to a temp file, pass +`--insights-path`, clean up. + +```python +def _cmd_enrich(p: dict[str, Any]) -> dict[str, Any]: + output_dir = p.get("output_dir", "./flowx_output") + insights = p.get("insights") # inline dict (hosted / Genie path) + insights_path = p.get("insights_path") # readable path (local path) + if insights is None and not insights_path: + return {"ok": False, "error": "Provide 'insights' (inline dict) or 'insights_path'."} + tmp = None + try: + if insights is not None: + tmp = runner.materialize_json(insights) # temp file, mirrors materialize_adf_definitions + insights_path = tmp + args = ["enrich", "--output-dir", output_dir, "--insights-path", insights_path] + result = runner.run_adapter(args) + out = Path(output_dir) + return _phase_result(result, out, inventory=runner.summarize_inventory(out)) + finally: + if tmp: + runner.cleanup_materialized(tmp) +``` + +Additions: +- `runner.materialize_json(obj) -> str` — writes `json.dumps(obj)` to a temp file, parallel to + `materialize_adf_definitions`. +- Register `"enrich": _cmd_enrich` in `_COMMANDS`; update the `flowx` tool docstring / `_COMMANDS` param docs to + list `enrich`. +- Confirm the runner captures subprocess stderr into `result` so validation violation lines surface to the + agent; if not, `_cmd_enrich` parses and echoes them explicitly. + +--- + +## 7. Skill Step 5 (`skills/flowx-discover/SKILL.md`) + +Insert the **author → enrich** loop as the new **Step 5**; renumber the existing reporting steps (5–8) down. + +**New Step 5 — Author and merge agentic insights** (always runs): + +1. **Read** the just-written `inventory.json` (`pipelines`, `lineage`, `summary`) plus `profile_report.csv`. +2. **Author** an `insights` object: + - `overview` — the whole factory as one system + the single biggest migration steer. + - `pipeline_insights[]` — **sparse**; per-pipeline `intent` + `databricks_pattern` (+ optional + features/notes). Omit pipelines with nothing worth saying. + - `pipeline_relationships[]` — annotate #9 lineage edges: each carries a `lineage_edge` + (`edge_type` + `edge_identity` echoed verbatim from a real edge — `activity_name` for control, `match_key` + for data), plus `relationship_summary`, `databricks_pattern`, `risk_if_ignored`. + - Authoring rules in the prose: reference only pipeline names that exist; every `lineage_edge` must echo a + real edge; don't invent relationships #9 didn't find (annotate, don't rediscover). +3. **Enrich** — call `flowx(command="enrich", parameters={"output_dir": ..., "insights": {...}})` (inline dict on + the hosted/Genie path; `insights_path` on the local CLI path). Local CLI fallback: + `"$PY" -m flowx.adapter enrich --output-dir --insights-path `. +4. **On `ok:false`** — the tool did **not** write; read `violations`, fix the offending FK/edge/field, and + re-call. On `ok:true`, the `insights` key is merged into `inventory.json`. + +**Reworded summary step** (the old "Present the summary"): after the counts table, surface the enriched +judgment — the factory `overview`, and per-pipeline `pattern_name` / `intent` — so the user sees *what the +factory does* and the recommended Databricks patterns, not just coverage numbers. + +--- + +## 8. Testing & optional reporting + +**Test file:** `tests/unit/test_pipeline_insights.py`, following `test_merge_agentic.py` precedent (helper +writers, `tmp_path`, assert **structure/schema — never prose**). **No live LLM** — all fixtures are stubbed JSON. + +**Fixtures** under `tests/resources/json/`: +- A small **good** `insights` object referencing a known inventory. +- Edge-binding tests reuse the existing `pipeline_execute_pipeline_nested.json`, whose known + `ControlEdge.activity_name` values are `"Run Ingestion Pipeline"`, `"Run Transform Pipeline"`, and + `"Run Cleanup Pipeline"`. The test builds a real inventory dict from it (via `load_adf_definitions` + + `build_lineage` + `_inventory_to_dict`, mirroring `test_lineage.py`) so FK/edge sets are genuine, not + hand-faked. + +**Test cases (1:1 with the issue's invariants):** + +| Test | Asserts | +|---|---| +| `test_validator_accepts_good_insights` | `validate_insights` returns `[]`; `enrich_inventory` returns `ok:True` with correct counts | +| `test_rejects_pipeline_not_in_inventory` | FK `pipeline`/`from_pipeline`/`to_pipeline` not in names → non-empty violations | +| `test_rejects_unresolvable_lineage_edge` | `lineage_edge` with no matching edge → violation | +| `test_rejects_missing_required_field` | missing `pipeline` / `from_pipeline` / `lineage_edge` → violation | +| `test_rejects_unknown_field` | extra key in any insight/relationship → violation | +| `test_control_edge_binding_matches_and_rejects` | on the nested fixture: `edge_identity="Run Ingestion Pipeline"` validates; a bogus name rejects | +| `test_data_edge_binds_on_match_key` | data edge resolves on `match_key`; a non-matching key rejects | +| `test_inferred_edge_with_evidence_and_confidence_validates` | `inferred` edge with real endpoints + non-empty `evidence` + valid `confidence` → `[]` | +| `test_inferred_edge_requires_evidence` | `inferred` edge missing `evidence` → violation | +| `test_inferred_edge_requires_valid_confidence` | `inferred` edge with bad/missing `confidence` → violation | +| `test_inferred_edge_does_not_resolve_against_lineage` | `inferred` `edge_identity` is *not* checked against lineage sets (arbitrary descriptor validates) | +| `test_annotation_edge_rejects_evidence_confidence` | `control`/`data` edge carrying `evidence`/`confidence` → violation (inferred-only fields) | +| `test_two_pass_deterministic_keys_byte_identical` | pre-enrich vs post-enrich: all keys except `insights` byte-identical | +| `test_enrich_is_idempotent` | running `enrich_inventory` twice → identical file bytes | +| `test_validation_failure_does_not_write` | on violations: `ok:False`, `violations` populated, **file unchanged on disk** | + +**Optional reporting** (`reporting/coverage.py`): a `has_insights` (bool) or `pattern_name` column per pipeline, +**gated on the `insights` key existing** so non-enriched inventories are unaffected. Added only if it stays +trivial and doesn't perturb existing coverage tests; the issue marks it optional, so it will not hold up the +core. + +**End-to-end verification** (before declaring done): a real `discover` then `enrich` over +`tests/resources/json/`, confirming the inventory gains a valid `insights` block while the deterministic keys are +byte-identical. Plus full unit suite + `make fmt` (ruff + mypy) clean. + +--- + +## 9. Scope + +**In scope:** +- The `insights` schema + 4 dataclasses in `models/adf_ast.py`. +- `parser/pipeline_insights.py` (`load_insights` → `validate_insights` → `merge_into_inventory` → + `enrich_inventory`). +- The `enrich` adapter subcommand + `_cmd_enrich` MCP command (+ `runner.materialize_json`). +- The two-pass byte-identical write. +- Step 5 in `flowx-discover` (always runs) + reworded summary step. +- Optional reporting column. + +**Out of scope:** +- Consuming insights in `convert` (separate follow-up). +- The #9 extraction itself; a new skill/phase; an in-code LLM client. +- Per-activity insights; domains grouping; per-edge narrative; #10 deploy-ordering. +- Any `schema_version` field. + +**Future considerations (review later — not #11):** +- Insights are currently authored in a single pass over the whole factory. For very large factories, revisit + **partitioning the authoring across subagents keyed on lineage clusters** — the connected components of the + combined control/data-edge graph — so each subagent reasons about one coherent subsystem rather than the whole + corpus at once. The open question that motivates this is "are all these pipelines even related?"; the lineage + graph already holds the answer. For #11 we always enrich in one pass. + +--- + +## 10. Customer confidentiality + +All **code, tests, fixtures, comments, commit messages, and the PR body** use generic placeholders only — they +must **never** contain real customer names or customer-derived vocabulary. + +- **Placeholders to use everywhere:** "Factory A"/"Factory B" (factories), `entityID` (loop key), + `config-params/entity-versions` (watermark path), "dummy dataset" (any dataset name). +- **Denylist — this design doc only, as a validation reference:** the terms below are recorded here **solely so + we can grep the diff, commits, and PR against them to confirm none leaked**. They must not appear in any + shipped artifact (code/tests/fixtures/comments/commits/PR): "a customer factory", "a large factory", `engagementID`, + `engagementDBVersions`, `etl-parameters`. A pre-flight check before opening the PR greps the branch for each of + these and must return zero hits outside this spec file. diff --git a/skills/flowx-discover/SKILL.md b/skills/flowx-discover/SKILL.md index b27e6f0..53a30ea 100644 --- a/skills/flowx-discover/SKILL.md +++ b/skills/flowx-discover/SKILL.md @@ -2,8 +2,9 @@ name: flowx-discover description: > Parse a source orchestrator's pipeline definitions (Azure Data Factory, Apache Airflow) into a - typed inventory that classifies every task as deterministic, agentic, or unsupported. Phase 1 of - the flowx migration workflow; routes to a source-specific guide. + typed inventory that classifies every task as deterministic, agentic, or unsupported, then author + agentic insights (intent, ranked target patterns, cross-pipeline relationships) over it. Phase 1 + of the flowx migration workflow; routes to a source-specific guide. triggers: - "discover pipelines" - "discover ADF" @@ -64,7 +65,7 @@ All under the shared `/metadata/` folder: | File | Description | |---|---| -| `metadata/inventory.json` | Classified activity inventory for the convert phase | +| `metadata/inventory.json` | Classified activity inventory (later enriched with agentic `insights`) for the convert phase | | `metadata/profile_report.csv` | Per-pipeline complexity report (counts + T-shirt size) | | `metadata/.arm.json` | (ADF) Verbatim original source for each pipeline (provenance) | @@ -74,7 +75,234 @@ The inventory classifies every task into one of three strategies: - **Agentic** — requires LLM-assisted translation from the source definition. - **Unsupported** — no known translation path; needs manual intervention. +After classification, discovery also **authors agentic insights** over the inventory and merges +them under an `insights` key — see *Author and merge agentic insights* below. This runs for every +source; the source-specific inputs come from each `sources/.md`. + +## Author and merge agentic insights (all sources) + +The deterministic inventory records *what* each pipeline contains; it cannot record *what the +factory is trying to do* or *how the pipelines relate as a system*. Author that judgment now and +merge it into `inventory.json` under an `insights` key. This always runs. + +**This step is source-neutral — it runs for every source.** The insight *schema*, the *analysis +method*, and the *pattern framework* below are shared; the source-specific inputs (how to deep-dive +the source, and its construct→Databricks pattern vocabulary) come from the "Insights — deep-dive & +pattern vocabulary" section of your `sources/.md`. + +1. **Read** the just-written `inventory.json` (`pipelines`, `lineage`, `summary`) + and `profile_report.csv`. **Then, before authoring, deep-dive the source.** The + inventory is a deterministic skeleton (types, strategy, control edges); the *why* + and *how* — queries, branch conditions, notebook paths, parameters — live only in + the verbatim source artifacts. **Which artifact to read, and how, is + source-specific: follow the "Insights — deep-dive & pattern vocabulary" section of + the `sources/.md` guide you used in Step 2.** Read the source for any + pipeline you write an insight or relationship about. +2. **Author** an `insights` object: + - `overview` — the whole factory as one system, plus the single biggest + migration steer. + - `system_recommendation` *(optional; preferred on any multi-pipeline factory)* — + the **one top-level architectural decision** a migrator must make **before** any + per-pipeline work, because it *cascades* across pipelines. A per-pipeline card + alone can't show it: e.g. "adopt a managed connector for the whole extraction + family" turns the child extractors into connector pipelines, deletes the + watermark store, **and** empties the fan-out orchestrator all at once. Author + this **first**, then keep each `pipeline_insights[].recommended_patterns` + consistent with the branch it recommends. Fields: + - `headline` — one line naming the decision (e.g. "Managed ingestion collapses + the extraction factory"). + - `recommended_patterns` — **1–4 whole-system branches**, ranked best-first and + shaped exactly like a pipeline's (each with `pattern`, `fit`, + `simplification_pattern`). `[0]` is the recommended branch; + later entries are the ranked fallbacks. Example: `[0]` = "Adopt **Lakeflow + Connect** for the whole SQL Server extraction family" (`simplification_pattern: + true`); `[1]` = "For-each orchestrator + 2 collapsed parameterized jobs" + (`simplification_pattern: false`). + - `cascade` — what choosing `[0]` **collapses or eliminates across the system** + (e.g. "5 child extractors → managed connector pipelines"; "version-watermark + CSV → gone"; "fan-out orchestrator → near-empty"). This is the payoff the + reader cannot see from any one pipeline. Omit (or `[]`) when the decision does + not cascade. + - `decision_driver` *(optional)* — the gating question that picks the branch + (e.g. "Is the Lakeflow Connect SQL Server connector GA/approved for this + source?"). + Use it whenever a **system-wide** capability (a managed connector for a whole + source, one observability tier, one control layer) would reshape many pipelines + at once; skip it for a single isolated pipeline. + - `pipeline_insights[]` — a **sparse, selective** list (per entry an `intent` + and `recommended_patterns`; optionally `pattern_name`, `databricks_pattern`, + `conversion_notes`, `risk_if_ignored`). + Omitting a pipeline is the default and needs no justification — a short, + high-signal list the reader can trust beats a note on every pipeline. + - **`risk_if_ignored`** (optional) — a one-line consequence a migrator faces + if they port this pipeline naively (e.g. "Switch-nested calls are invisible + in `lineage.control_edges`, so this reads as a leaf"). Use it only when the + insight carries a genuine migration hazard; otherwise omit. + - **`recommended_patterns` — the grounded, ranked recommendation.** A list of + **1–4** Databricks target patterns for this pipeline, ordered **best-first**. + Author it from a **holistic read of the whole pipeline** — its activities, + dependencies, datasets, linked-service source types, parameters, and intent — + **not** from a single pattern label. Each entry is an object: + - `pattern` — the **named, publicly-documented** Databricks capability (e.g. + `Lakeflow Connect SQL Server connector`, `Auto Loader`, + `Lakeflow Declarative Pipelines AUTO CDC`). Name **only** capabilities that + actually exist; **docs.databricks.com is the reference**. Never invent a name. + - `fit` — one line: why it fits *this* pipeline / what bespoke logic it replaces. + - `simplification_pattern` — `true` **only** when the pattern uses a *distinctive* + Databricks capability that collapses or eliminates a whole legacy pattern: + a managed connector (**Lakeflow Connect**), declarative CDC (**`AUTO CDC`**), + **Auto Loader**, or **system tables** replacing a home-grown logging tier. + Set it `false` for a like-for-like port **and** for plain native building + blocks that merely re-home the same work — a bare parameterized **Lakeflow + Job**, a for-each/run-job orchestrator, a plain Delta control table, + `MERGE INTO`. "Runs on Databricks" is **not** a simplification: almost + everything you migrate is native, so reserve this flag for the capability + that makes the old pattern *disappear*. Rank the `true` patterns **first**. + + **Rank simplification-first:** prefer managed ingestion over a hand-rolled + extract, declarative CDC over custom watermark logic, and collapsing clones + over N ports — but flag `simplification_pattern: true` only on the entries that + truly use a distinctive capability, not on the plain-orchestration fallback. + Keep it to 1–4 (don't pad); **omit the field** when you have no grounded + recommendation. Note GA/Preview status in `fit` when it affects the decision — + **verify** a connector's status in the docs/release notes (e.g. the Lakeflow + Connect SQL Server connector) rather than assuming GA. + + **Recognized-pattern vocabulary — a reference menu, NOT an allowlist.** Each + source guide carries a **source-construct → Databricks** mapping table (with + **current** product names) in its "Insights — deep-dive & pattern vocabulary" + section — use the one in the `sources/.md` you followed. The Databricks + (target) side is source-neutral; use it to stay grounded and consistent, but reach + past it whenever the holistic view calls for a better or newer fit. + + **Emit current names, not legacy ones:** Lakeflow Jobs (was Databricks + Workflows), Lakeflow Declarative Pipelines (was Delta Live Tables/DLT), `AUTO CDC` + (was `APPLY CHANGES INTO`), Declarative Automation Bundles (was Databricks Asset + Bundles), AI/BI dashboards (was Lakeview), `system.lakeflow` (was + `system.workflow`). + - **`databricks_pattern`** (optional) — a one-line **headline** naming the primary + target architecture. `recommended_patterns[0]` is the structured form of it, so + omit `databricks_pattern` unless a short prose headline genuinely adds signal. + - **Include a pipeline only if it meets ANY of these tests:** it anchors a + reusable *framework* or *pattern* many others depend on (an orchestrator, + a shared engine/wrapper, a logging/control-table hub); its classification + or role is *surprising* given its name; or it carries a *risk or caveat* a + migrator must know before porting it. + - **Cover distinct roles, not a fixed count.** One representative note per + notable role/archetype is usually enough — if forty pipelines are near- + identical wrappers around one engine, note the engine and one representative + wrapper, not all forty. Scale is set by how many *distinct* roles exist, not + by pipeline count: on a large factory you will typically flag only a small + minority. This is a guide, not a quota — include fewer if fewer qualify. + - Rule of thumb: include a note only if it would **change a reader's decision + or surprise a domain expert**. When in doubt, omit. + - **Two whole-factory recommendation patterns** (apply when the evidence is + there; use `pattern_name` to tag them, record the target as a + `recommended_patterns` entry with `simplification_pattern: true`, and + quantify the payoff in `intent` / `conversion_notes`. When either reshapes + the *whole* system, also surface it as the `system_recommendation`): + - **Replace, don't transliterate.** When a *whole tier or sub-factory* + exists only to provide a capability Databricks offers **natively**, + recommend eliminating it, not re-implementing it as a called job. Common + generic mappings: a logging/observability tier → system tables + (`system.lakeflow.*`) + native Lakeflow job notifications + an AI/BI + dashboard; run-state / control tables → Lakeflow job & task run state and + `dbutils.jobs.taskValues`; a config-driven Switch/template "engine" with + no native equivalent → a Python orchestrator driving the Jobs API. **Guard + against over-firing:** only recommend REPLACE when the tier's *sole* + purpose is the native capability (e.g. it only logs / only records run + state). If a pipeline does real domain work alongside the boilerplate, + migrate it normally — do not tell the reader to delete real logic. + - **Collapse clone families.** Cluster pipelines by their activity + *signature* (ordered activity/task types) and shared child-edge set across the + whole inventory. Where a family of near-identical pipelines exists, emit + **one** insight (anchored on a representative pipeline that exists in the + inventory) that names the family and its count, recommends collapsing the + N clones into a **single parameterized job invoked N times**, and + quantifies the win (e.g. "14 near-identical ingest pipelines, identical + activity signature → 1 parameterized job"). List the members in + `conversion_notes`. This supersedes writing N near-duplicate per-pipeline notes. + - `pipeline_relationships[]` — characterize **how data and control flow + between the pipelines**, whatever the mechanism. Each relationship carries + `from_pipeline` and `to_pipeline` (both must be pipeline names that exist in + the inventory) plus a `lineage_edge`, `relationship_summary`, + `databricks_pattern`, and `risk_if_ignored`. + + **A `lineage_edge` is one of two tiers. The tier is decided by ONE thing: + whether the deterministic phase already recorded this coupling as an edge in + `lineage` — NOT by the coupling's mechanism** (control call, dataset, table + written in notebook code, ordering dependency, external trigger, …). Do not + classify by mechanism. + + - **Annotation** (`edge_type` = `control` or `data`) — the coupling is + *already* an edge in `lineage`; you are adding interpretation to it. + `edge_identity` is copied **verbatim** from that edge: the `activity_name` + for a `control_edges` entry, the `match_key` for a `data_edges` entry. Do + not add `evidence` / `confidence`. + - **Inferred** (`edge_type` = `inferred`) — a *real* coupling the + deterministic phase did **not** record as an edge, by any mechanism. Set + `edge_identity` to a short descriptor of what couples the two pipelines + (e.g. the shared table/asset, or the nature of the dependency), and **you + must** supply `evidence` (the concrete source observation behind it) and + `confidence` (`high` / `medium` / `low`). Report only couplings you can + actually evidence; do not invent them. + + **Decide in order:** + 1. Is this coupling already an edge in `lineage` (a `control_edges` / + `data_edges` entry)? → **annotation** (`control` / `data`). + 2. Otherwise, is it a real coupling not present in `lineage`? → **inferred**. + 3. If in doubt — the coupling is real but you cannot point to the `lineage` + edge that names it — classify it **inferred** (never annotate an edge that + is not there). + + **Inferred covers several sub-cases — do not restrict it to any one:** + - *Data-in-code:* one pipeline's notebook writes a table another's notebook + reads (no declared dataset, so `data_edges` never saw it). + - *Ordering dependency:* a producer→consumer hand-off expressed only as + sibling ordering inside a parent orchestrator, which the deterministic + phase did not emit as a cross-pipeline edge. + - *Shared control/config asset, external trigger, message queue,* or any + other real coupling flowx could not represent. + - *Near-miss (this is an annotation, not inferred):* pipeline A invokes B and + that call is already a `control_edges` entry — even though B then does its + real work in a notebook, the coupling itself was recorded, so annotate it. + - **Authoring rules:** reference only pipeline names that exist in the + inventory; an annotation edge (`control`/`data`) must echo a real lineage + edge (annotate, don't rediscover); an `inferred` edge must carry non-empty + `evidence` and a `confidence` level and must not be dressed up as proven + lineage. +3. **Enrich** — merge the object in: + + - **MCP tool path** (inline dict; the only path in Genie Code): + + ``` + flowx(command="enrich", parameters={ + "output_dir": "", + "insights": { ...authored object... }}) + ``` + + - **venv CLI fallback** (write the object to a JSON file first): + + ```bash + "$PY" -m flowx.adapter enrich --output-dir --insights-path + ``` + +4. **On `ok:false`** the tool did **not** write the file: read `violations`, fix + the offending pipeline name / lineage edge / field, and call `enrich` again. + On `ok:true` the `insights` key is now merged into `inventory.json`. Present the + authored judgment back to the user (the factory `overview`, and each + `pipeline_insights` entry's `pattern_name` / `intent` and top ranked + `recommended_patterns`) as part of the source guide's summary step. + +## Future considerations + +Insights are currently authored in a single pass over the whole factory. For very +large factories, revisit partitioning the authoring across subagents keyed on +lineage clusters (the connected components of the combined control/data-edge +graph), so each subagent reasons about one coherent subsystem. Out of scope for +now — always enrich in one pass. + ## Reference -- `sources/adf.md` — Azure Data Factory discovery (ARM JSON, UC-volume download, complexity report) -- `sources/airflow.md` — Apache Airflow discovery (DAG `.py` parsing, operator classification) +- `sources/adf.md` — Azure Data Factory discovery (ARM JSON, UC-volume download, complexity report) + ADF insight deep-dive & pattern vocabulary +- `sources/airflow.md` — Apache Airflow discovery (DAG `.py` parsing, operator classification) + Airflow insight deep-dive & pattern vocabulary diff --git a/skills/flowx-discover/sources/adf.md b/skills/flowx-discover/sources/adf.md index 9997e27..d089130 100644 --- a/skills/flowx-discover/sources/adf.md +++ b/skills/flowx-discover/sources/adf.md @@ -83,6 +83,11 @@ Strategy Breakdown: Coverage: 95.7% ``` +Then, after the shared insights step has enriched `inventory.json`, surface the authored judgment so +the user sees *what the factory does*, not just coverage numbers: print the factory `overview`, and +for each `pipeline_insights` entry its `pattern_name` / `intent` and its top `recommended_patterns` +(ranked simplification-first). + ## Step 6 — Detail agentic activities For `agentic` activities, explain that each is translated by the agent using LLM-assisted reasoning @@ -98,3 +103,47 @@ to a PySpark notebook. Tell the user where the metadata files were written (`/metadata/`), summarise the complexity sizes, and confirm they can proceed to `flowx-convert` with the same ``. + +## Insights — deep-dive & pattern vocabulary + +Reference for the shared agentic-insights step (parent `SKILL.md` Step 5, "Author and merge agentic +insights"). Do this deep-dive before authoring insights for any ADF pipeline. + +**Deep-dive the ARM.** The inventory is a deterministic skeleton (types, strategy, control edges); +the *why* and *how* — queries, Switch conditions, notebook paths, dataset parameters — live only in +the verbatim ARM. The `metadata/` folder holds one `*.arm.json` file per pipeline; each is a **flat +single-pipeline object** shaped `{"name": "", "properties": {"activities": [...], ...}}` +(no `resources[]` array, no top-level `type`). To inspect a pipeline, **glob `metadata/*.arm.json` +and match on each file's top-level `"name"` field** — do **not** construct a filename from the +pipeline name (names are slugified and lossy, so a built path can miss or collide). Activities are +under `properties.activities` (recurse into nested `ForEach`/`If`/`Switch` bodies). Read the ARM for +any pipeline you write an insight or relationship about. + +**ADF constructs → Databricks** — a reference menu, NOT an allowlist; the target side uses current +product names, so reach past it whenever a better or newer fit exists. Flag `simplification_pattern: +true` only on entries that use a distinctive capability, never on the plain-orchestration fallback. + +| Pipeline does… | Simplifying target — `simplification_pattern: true` (rank first) | Fallback — `false` | +|---|---|---| +| Extract/Copy from a database (SQL Server, …) | **Lakeflow Connect** managed connector (change-tracking/CDC → Delta) | Auto Loader / JDBC read + `MERGE INTO` | +| Incremental load via watermark | **Lakeflow Declarative Pipelines `AUTO CDC`** | Delta `MERGE INTO` + control table / `dbutils.jobs.taskValues` | +| CDC / SQL Server change tracking | **Lakeflow Connect** or **`AUTO CDC`** | Structured Streaming over the change feed | +| Land + process files | **Auto Loader** (`cloudFiles`, file-notification mode) | — | +| Metadata-driven bulk copy (Lookup→ForEach→Copy) | **Lakeflow Connect** (multi-table) or a parameterized **Lakeflow Jobs** for-each task | — | +| Parent/child `ExecutePipeline` fan-out | **Lakeflow Jobs** for-each task + run-job task + job parameters | — | +| SCD Type 2 (data flow) | **Lakeflow Declarative Pipelines `AUTO CDC`** (SCD Type 2) | — | +| Staged load + stored-proc transform | Spark write to **Delta** + post-load step | — | +| REST API pagination | Python ingestion notebook (requests-based) | Lakeflow Connect SaaS connector if one fits | +| Custom logging / observability tier | **system tables (`system.lakeflow.*`) + native job notifications + AI/BI dashboard** | — | +| Run-state / control tables | Lakeflow job & task run state + `dbutils.jobs.taskValues` | — | +| Clone family (many near-identical pipelines) | one **parameterized Lakeflow Job** invoked N times | — | + +**Emit current names, not legacy ones:** Lakeflow Jobs (was Databricks Workflows), Lakeflow +Declarative Pipelines (was Delta Live Tables/DLT), `AUTO CDC` (was `APPLY CHANGES INTO`), Declarative +Automation Bundles (was Databricks Asset Bundles), AI/BI dashboards (was Lakeview), `system.lakeflow` +(was `system.workflow`). + +**Then author the insights (shared method).** With this deep-dive and pattern vocabulary in hand, +author and merge the `insights` object by following the source-neutral "Author and merge agentic +insights" step in the parent `SKILL.md`. The insight schema and the authoring method are shared +across sources; only the deep-dive and the construct mappings above are ADF-specific. diff --git a/skills/flowx-discover/sources/airflow.md b/skills/flowx-discover/sources/airflow.md index 190cb44..963d395 100644 --- a/skills/flowx-discover/sources/airflow.md +++ b/skills/flowx-discover/sources/airflow.md @@ -50,6 +50,11 @@ Total tasks: 8 Coverage: 87.5% ``` +Then, after the shared insights step has enriched `inventory.json`, surface the authored judgment so +the user sees *what the DAGs do*, not just coverage numbers: print the factory `overview`, and for +each `pipeline_insights` entry its `pattern_name` / `intent` and its top `recommended_patterns` +(ranked simplification-first). + ## Step 5 — Detail agentic tasks For `agentic` tasks, name the operator that has no deterministic mapping yet (e.g. a custom or @@ -69,3 +74,44 @@ that are **not** handled (dynamic TaskGroup mapping, shared multi-DAG bundle), s [`../../flowx-convert/sources/airflow-coverage.md`](../../flowx-convert/sources/airflow-coverage.md). Callables reading Airflow task context (`**context` / `ti`) or XCom, and runtime-branching decorators, are routed to placeholders for manual/agentic translation rather than converted. + +## Insights — deep-dive & pattern vocabulary + +Reference for the shared agentic-insights step (parent `SKILL.md` Step 5, "Author and merge agentic +insights"). Do this deep-dive before authoring insights for any DAG. + +**Deep-dive the DAG source.** The inventory is a deterministic skeleton (task types, strategy, +dependencies); the *why* and *how* live in the **DAG source** — the `.py` files under the +`--source-path` you discovered from. Read the DAG module for any pipeline you write about: task +callables (`PythonOperator` bodies), operator arguments, templated params, hooks / connections, and +`set_upstream` / `>>` dependencies. Recurse into `TaskGroup`s and dynamically mapped (`.expand`) +tasks. The parser already extracts operators, `>>` / `<<` edges, `schedule_interval`, and inline +callables (see "How it works" above), so read the source for the intent the static parse can't +capture — what a callable actually *does*, what a hook connects to, and why the tasks are ordered as +they are. + +**Airflow operators → Databricks** — a reference menu, NOT an allowlist; the target side uses current +product names, so reach past it whenever a better or newer fit exists. Flag `simplification_pattern: +true` only on entries that use a distinctive capability, never on the plain-orchestration fallback. + +| DAG uses… | Simplifying target — `simplification_pattern: true` (rank first) | Fallback — `false` | +|---|---|---| +| DB extract via `MsSqlOperator` / `JdbcOperator` / custom hook | **Lakeflow Connect** managed connector (change-tracking/CDC → Delta) | JDBC read + `MERGE INTO` | +| Incremental load w/ XCom or Variable watermark | **Lakeflow Declarative Pipelines `AUTO CDC`** | Delta `MERGE INTO` + `dbutils.jobs.taskValues` | +| File sensor + load (`*FileSensor` → transform) | **Auto Loader** (`cloudFiles`, file-notification mode) | — | +| `SparkSubmitOperator` / `DatabricksSubmitRunOperator` | native **Lakeflow Job** task (notebook / JAR / Python) | — | +| `PythonOperator` glue / bespoke script | notebook or Python task in a **Lakeflow Job** | — | +| `TriggerDagRunOperator` / `ExternalTaskSensor` fan-out | **Lakeflow Jobs** run-job task + job parameters | — | +| Dynamic task mapping (`.expand`) over a list | **Lakeflow Jobs** for-each task | — | +| `BashOperator` shelling out to a script | native task (notebook / Python) driven by job parameters | — | +| Custom logging / observability via XComs or a side table | **system tables (`system.lakeflow.*`) + native job notifications + AI/BI dashboard** | — | + +**Emit current names, not legacy ones:** Lakeflow Jobs (was Databricks Workflows), Lakeflow +Declarative Pipelines (was Delta Live Tables/DLT), `AUTO CDC` (was `APPLY CHANGES INTO`), Declarative +Automation Bundles (was Databricks Asset Bundles), AI/BI dashboards (was Lakeview), `system.lakeflow` +(was `system.workflow`). + +**Then author the insights (shared method).** With this deep-dive and pattern vocabulary in hand, +author and merge the `insights` object by following the source-neutral "Author and merge agentic +insights" step in the parent `SKILL.md`. The insight schema and the authoring method are shared +across sources; only the deep-dive and the construct mappings above are Airflow-specific. diff --git a/src/flowx/adapter/__main__.py b/src/flowx/adapter/__main__.py index 38545be..6691990 100644 --- a/src/flowx/adapter/__main__.py +++ b/src/flowx/adapter/__main__.py @@ -87,6 +87,8 @@ def main(argv: list[str] | None = None) -> int: return _run_resolve_agentic(args) if args.command == "record-results": return _run_record_results(args) + if args.command == "enrich": + return _run_enrich(args) if args.command == "install-dashboard": return _run_install_dashboard(args) parser.print_help(sys.stderr) @@ -163,6 +165,51 @@ def _run_record_results(args: argparse.Namespace) -> int: return 0 +def _run_enrich(args: argparse.Namespace) -> int: + """Implements ``enrich``: validate + merge agent-authored insights into inventory.json. + + Returns 0 on success, 1 on any failure (missing inventory, unreadable/absent/ + both payload sources, or validation violations). + """ + from flowx.parser.pipeline_insights import enrich_inventory + + metadata_dir = args.output_dir / "metadata" + if not (metadata_dir / "inventory.json").exists(): + print(f"No inventory.json under {metadata_dir}; run the discover phase first.", file=sys.stderr) + return 1 + + inline: dict[str, Any] | None = None + if args.insights is not None: + try: + inline = json.loads(args.insights) + except json.JSONDecodeError as error: + print(f"Invalid --insights JSON: {error}", file=sys.stderr) + return 1 + if (inline is None) == (args.insights_path is None): + print("Provide exactly one of --insights (inline JSON) or --insights-path.", file=sys.stderr) + return 1 + + try: + result = enrich_inventory(args.output_dir, insights=inline, insights_path=args.insights_path) + except (OSError, ValueError, json.JSONDecodeError) as error: + print(f"Failed to enrich inventory: {error}", file=sys.stderr) + return 1 + + if not result["ok"]: + for violation in result["violations"]: + print(f" - {violation}", file=sys.stderr) + print( + f"Insights validation failed ({len(result['violations'])} violation(s)); inventory not modified.", + file=sys.stderr, + ) + return 1 + print( + f"Enriched inventory: {result['pipeline_insights']} pipeline insight(s), " + f"{result['relationships']} relationship(s)." + ) + return 0 + + def _run_install_dashboard(args: argparse.Namespace) -> int: """Implements ``install-dashboard``: create + publish the coverage dashboard. @@ -498,6 +545,29 @@ def _build_parser() -> argparse.ArgumentParser: help="SQL warehouse id for the write. Auto-detected (prefers running serverless) when omitted.", ) + enrich = subparsers.add_parser( + "enrich", + help="Validate and merge agent-authored insights into metadata/inventory.json.", + ) + enrich.add_argument( + "--output-dir", + type=Path, + required=True, + help="Migration output directory (reads/writes metadata/inventory.json).", + ) + enrich.add_argument( + "--insights-path", + type=Path, + default=None, + help="Path to a JSON file holding the insights object.", + ) + enrich.add_argument( + "--insights", + type=str, + default=None, + help="Insights object as an inline JSON string (convenience for direct CLI use).", + ) + dashboard = subparsers.add_parser( "install-dashboard", help="Create and publish an AI/BI dashboard visualizing coverage from the results table.", diff --git a/src/flowx/mcp/runner.py b/src/flowx/mcp/runner.py index bc6986e..e173186 100644 --- a/src/flowx/mcp/runner.py +++ b/src/flowx/mcp/runner.py @@ -204,15 +204,45 @@ def materialize_adf_definitions(definitions: dict[str, Any]) -> str: return str(base) -def cleanup_materialized(source: str) -> None: - """Remove a temp tree created by :func:`materialize_adf_definitions`. +def materialize_json(obj: Any) -> str: + """Write a JSON-serialisable object to a temp file and return its path. + + Lets the MCP server pass an inline ``insights`` dict to the adapter's + ``enrich`` subcommand (which reads from ``--insights-path``). Clean up with + :func:`cleanup_materialized`. + """ + fd, path = tempfile.mkstemp(prefix="flowx-insights-", suffix=".json") + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(obj, handle) + return path + - Accepts either the returned directory or the single-file path (whose parent temp dir is - removed). Only paths under the system temp dir are deleted, as a safety guard. +_TEMP_DIR_PREFIXES = ("flowx-adf-", "flowx-vol-", "flowx-ws-") + + +def cleanup_materialized(source: str) -> None: + """Remove a temp tree/file created by :func:`materialize_adf_definitions`, + :func:`download_volume_dir`, :func:`download_workspace_dir`, or :func:`materialize_json`. + + Accepts a temp directory path (from the ``mkdtemp`` helpers), a single-file path + inside such a directory (the single ARM-template case, whose parent temp dir is + removed), or a standalone temp file created directly in the system temp root + (from :func:`materialize_json`, whose file alone is removed -- never its parent). + Only paths under the system temp dir and carrying one of our prefixes are deleted, + as a safety guard. """ + tmp_root = str(Path(tempfile.gettempdir()).resolve()) path = Path(source) + # A standalone temp file we created directly in the temp root (materialize_json): + # remove just the file -- never its parent, which is the shared system temp root. + if path.is_file() and path.name.startswith("flowx-insights-"): + if str(path.resolve()).startswith(tmp_root): + path.unlink(missing_ok=True) + return + # Otherwise the temp dir to remove is the path itself (a mkdtemp dir) or, for the + # single ARM-template case, the file's parent temp dir. target = path if path.is_dir() else path.parent - if str(target.resolve()).startswith(str(Path(tempfile.gettempdir()).resolve())): + if target.name.startswith(_TEMP_DIR_PREFIXES) and str(target.resolve()).startswith(tmp_root): shutil.rmtree(target, ignore_errors=True) diff --git a/src/flowx/mcp/server.py b/src/flowx/mcp/server.py index 36c67fc..ad7230e 100644 --- a/src/flowx/mcp/server.py +++ b/src/flowx/mcp/server.py @@ -492,9 +492,36 @@ def _cmd_install_dashboard(p: dict[str, Any]) -> dict[str, Any]: return {"ok": result.ok, "result": runner.parse_stdout_json(result), "process": result.as_dict()} +def _parse_enrich_violations(stderr: str) -> list[str]: + """Extract the ' - ' lines the adapter's enrich prints on failure.""" + return [line[4:] for line in stderr.splitlines() if line.startswith(" - ")] + + +def _cmd_enrich(p: dict[str, Any]) -> dict[str, Any]: + output_dir = p.get("output_dir", "./flowx_output") + insights = p.get("insights") + insights_path = p.get("insights_path") + if insights is None and not insights_path: + return {"ok": False, "error": "Provide 'insights' (inline dict) or 'insights_path'."} + tmp: str | None = None + try: + if insights is not None: + tmp = runner.materialize_json(insights) + insights_path = tmp + args: list[Any] = ["enrich", "--output-dir", output_dir, "--insights-path", insights_path] + result = runner.run_adapter(args) + out = Path(output_dir) + violations = _parse_enrich_violations(result.stderr) if not result.ok else None + return _phase_result(result, out, inventory=runner.summarize_inventory(out), violations=violations) + finally: + if tmp: + runner.cleanup_materialized(tmp) + + _COMMANDS: dict[str, Callable[[dict[str, Any]], dict[str, Any]]] = { "inputs": _cmd_inputs, "discover": _cmd_discover, + "enrich": _cmd_enrich, "convert": _cmd_convert, "merge_agentic": _cmd_merge_agentic, "resolve_agentic": _cmd_resolve_agentic, @@ -539,18 +566,15 @@ def flowx(command: str, parameters: dict[str, Any] | None = None) -> dict[str, A Airflow reads ``airflow_source_path`` (a DAG .py file or directory). ``package`` is source-independent (it consumes the translation report). - - "inputs": phase(req: "discover"|"convert"|"package"), source(req for discover/convert) — - list a phase's input prompts. - - "discover": source(req), one ADF source key | airflow_source_path (req), output_dir, - pipeline, exclude_dag | exclude_dags (Airflow, repeatable list) — parse and audit definitions. - - "convert": source(req), (one ADF source key | airflow_source_path), output_dir, pipeline, - exclude_dag | exclude_dags (Airflow, repeatable list). - - "merge_agentic": source(req: "adf"), report_path(req), agentic_results_dir(req), output_path — - merge ADF agent results. Airflow's legacy name-based merge is disabled; use resolve_agentic. - - "resolve_agentic": source(req: "airflow"), action(req: prepare | stage | apply), output_dir, - airflow_source_path, report_path, gap_id, candidates, replace, accept_gap | accept_gaps, accept_all, - review_complete, review_manifest, reset — - prepare, stage, and explicitly apply fingerprint-bound Airflow leaf-gap resolutions. + - "inputs": phase(req: "discover"|"convert"|"package") — list a phase's input prompts. + - "discover": one of adf_volume_path | adf_workspace_path | adf_definitions | adf_source_path + (req), output_dir, pipeline — parse ADF JSON, classify activities. + - "enrich": output_dir(req), one of insights(inline dict) | insights_path — validate + merge + agent-authored insights into metadata/inventory.json (returns {ok:false, ...} without writing + on validation failure). + - "convert": output_dir, (adf_volume_path | adf_workspace_path | adf_definitions | + adf_source_path), pipeline. + - "merge_agentic": report_path(req), agentic_results_dir(req), output_path — merge agent results. - "inspect": report_path(req) — return the full translation-option schema (every option with a `show_when` condition) for the agent to walk locally. See "Collecting options" below. - "apply_answers": report_path(req), answers(req, list of "ID=VALUE"), output_dir, lookup_csv. diff --git a/src/flowx/models/adf_ast.py b/src/flowx/models/adf_ast.py index 6047935..86b4feb 100644 --- a/src/flowx/models/adf_ast.py +++ b/src/flowx/models/adf_ast.py @@ -297,13 +297,13 @@ def get_pipeline(self, name: str | None) -> AdfPipeline | None: if not name: return None lowered = name.lower() - ci_fallback = None + exact = None for pipeline in self.pipelines: if pipeline.name == name: return pipeline - if ci_fallback is None and pipeline.name.lower() == lowered: - ci_fallback = pipeline - return ci_fallback + if exact is None and pipeline.name.lower() == lowered: + exact = pipeline + return exact # --------------------------------------------------------------------------- diff --git a/src/flowx/models/insights.py b/src/flowx/models/insights.py new file mode 100644 index 0000000..d7ce953 --- /dev/null +++ b/src/flowx/models/insights.py @@ -0,0 +1,162 @@ +"""Agentic insights (discover phase) -- agent-authored judgment merged into inventory.json. + +References pipelines by name. Its cross-pipeline edges are either ANNOTATIONS of a +deterministic ``Lineage`` edge (control/data -- carry no facts of their own) or an +agent-INFERRED coupling the deterministic layer could not see (e.g. data flow that +happens inside notebook code, an external trigger, a message queue). Inferred edges +must cite their evidence and a confidence level so they are never mistaken for proven +lineage. + +These models are **source-neutral**: they describe the shape of the ``insights`` object +the agent authors, independent of whether the pipelines were discovered from ADF or +Airflow. The validate/merge engine in ``flowx.parser.pipeline_insights`` works on the +raw dict form; these dataclasses document the contract and back the unit tests. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Literal + + +@dataclass(slots=True, kw_only=True) +class LineageEdgeRef: + """A typed reference from a PipelineRelationship to one cross-pipeline edge. + + Two tiers: + + * ``"control"`` / ``"data"`` -- an **annotation** of a deterministic edge. + ``edge_identity`` echoes that edge verbatim (``ControlEdge.activity_name`` + for control, ``DataEdge.match_key`` for data) so enrichment can resolve it + against the inventory's ``lineage``. ``evidence`` / ``confidence`` are not + used (the deterministic edge *is* the evidence, confidence is implicitly + high) and must be omitted. + * ``"inferred"`` -- an agent-asserted coupling the deterministic layer did + not find. There is no lineage edge to resolve against, so ``edge_identity`` + is an agent-authored descriptor of what couples the pipelines (e.g. a + shared table or asset name), and ``evidence`` (why the agent believes the + coupling exists) plus ``confidence`` are **required**. This tier stays + pattern-agnostic: it does not encode *why* the deterministic layer missed + the edge, so it generalises to couplings flowx cannot yet see. + + Attributes: + edge_type: The tier -- ``"control"``, ``"data"``, or ``"inferred"``. + edge_identity: For ``"control"`` the ``ControlEdge.activity_name``; for + ``"data"`` the ``DataEdge.match_key`` (both echoed verbatim from a + real edge); for ``"inferred"`` an agent-authored descriptor of the + coupling. + evidence: Inferred edges only -- the observable basis for the asserted + coupling. Required for ``"inferred"``; must be omitted otherwise. + confidence: Inferred edges only -- ``"high"`` / ``"medium"`` / ``"low"``. + Required for ``"inferred"``; must be omitted otherwise. + """ + + edge_type: Literal["control", "data", "inferred"] + edge_identity: str + evidence: str | None = None + confidence: Literal["high", "medium", "low"] | None = None + + +@dataclass(slots=True, kw_only=True) +class RecommendedPattern: + """One ranked Databricks target pattern recommended for a pipeline. + + A pipeline insight carries 1-4 of these, ordered best-first, drawn from the + agent's *holistic* read of the pipeline and grounded in publicly-documented + Databricks capabilities. ``simplification_pattern`` ranks the distinctive + capabilities that collapse a legacy pattern ahead of like-for-like ports and + plain building blocks. + + Attributes: + pattern: The named, publicly-documented Databricks capability (e.g. + ``"Lakeflow Connect SQL Server connector"``). Never an invented name. + fit: One line on why it fits this pipeline / what custom logic it replaces. + simplification_pattern: ``True`` *only* when the pattern uses a **distinctive** + Databricks capability that collapses or eliminates a whole legacy + pattern -- a managed connector (Lakeflow Connect), declarative CDC + (``AUTO CDC``), Auto Loader, or system tables replacing a home-grown + logging tier. ``False`` for a like-for-like port AND for plain native + building blocks that merely re-home the same work (a bare parameterized + Lakeflow Job, a for-each/run-job orchestrator, a plain Delta control + table, ``MERGE INTO``) -- "runs on Databricks" is not a simplification, + so reserve this flag for the capability that makes the old pattern + *disappear*. Rank the ``True`` patterns first. + """ + + pattern: str + fit: str + simplification_pattern: bool + + +@dataclass(slots=True, kw_only=True) +class SystemRecommendation: + """The single top-level architectural decision spanning the whole factory. + + Per-pipeline ``recommended_patterns`` are chosen *under* this decision: the + system-level branch you pick (e.g. adopt a managed connector for an entire + extraction family) cascades into what each pipeline becomes, so it is authored + first and the per-pipeline patterns are kept consistent with it. It captures + the payoff a reader cannot see from any single pipeline card. + + Attributes: + headline: One line naming the decision a migrator must make before any + per-pipeline work (e.g. "Managed ingestion collapses the extraction + factory"). + recommended_patterns: 1-4 whole-system target architectures, ordered + best-first (the simplifying/native branch first), each a + :class:`RecommendedPattern`. ``recommended_patterns[0]`` is the + recommended branch; later entries are the ranked fallbacks. + cascade: What choosing ``recommended_patterns[0]`` collapses or eliminates + across the whole system (e.g. "5 child extractors -> managed connector + pipelines", "version-watermark CSV -> gone"). Empty when the decision + does not cascade. + decision_driver: The gating question that selects the branch (e.g. "Is the + Lakeflow Connect SQL Server connector GA/approved for this source?"); + omit when there is no single deciding factor. + """ + + headline: str + recommended_patterns: list[RecommendedPattern] = field(default_factory=list) + cascade: list[str] = field(default_factory=list) + decision_driver: str | None = None + + +@dataclass(slots=True, kw_only=True) +class PipelineInsight: + """Per-pipeline judgment; references a pipeline by name (foreign key).""" + + pipeline: str + pattern_name: str | None = None + intent: str | None = None + databricks_pattern: str | None = None + recommended_patterns: list[RecommendedPattern] = field(default_factory=list) + conversion_notes: list[str] = field(default_factory=list) + risk_if_ignored: str | None = None + + +@dataclass(slots=True, kw_only=True) +class PipelineRelationship: + """Cross-pipeline judgment. + + Either annotates one deterministic lineage edge (``lineage_edge.edge_type`` + is ``"control"`` / ``"data"``) or records an agent-inferred coupling the + deterministic layer could not see (``"inferred"``). Both endpoints are always + real pipeline names validated against the inventory. + """ + + from_pipeline: str + to_pipeline: str + lineage_edge: LineageEdgeRef + relationship_summary: str | None = None + databricks_pattern: str | None = None + risk_if_ignored: str | None = None + + +@dataclass(slots=True, kw_only=True) +class Insights: + """Agent-authored insights merged into inventory.json under the ``insights`` key.""" + + overview: str | None = None + system_recommendation: SystemRecommendation | None = None + pipeline_insights: list[PipelineInsight] = field(default_factory=list) + pipeline_relationships: list[PipelineRelationship] = field(default_factory=list) diff --git a/src/flowx/parser/pipeline_insights.py b/src/flowx/parser/pipeline_insights.py new file mode 100644 index 0000000..ab20180 --- /dev/null +++ b/src/flowx/parser/pipeline_insights.py @@ -0,0 +1,371 @@ +"""Validate and merge agent-authored insights into the discover inventory. + +The discover phase writes a pure ``metadata/inventory.json`` (pipelines, summary, +lineage). The agent then *authors* an ``insights`` object -- its judgment about +pipeline intent, Databricks patterns, and cross-pipeline relationships. This +module *enriches* the inventory: it validates the authored JSON against the +inventory and, only when clean, appends the single ``insights`` key while +re-serialising the rest byte-identically. + +A relationship's ``lineage_edge`` comes in two tiers, validated differently: + +* ``control`` / ``data`` -- an **annotation** of a deterministic edge. Its + ``edge_identity`` must resolve to a real edge in the inventory's ``lineage`` + (a ``ControlEdge.activity_name`` or ``DataEdge.match_key``); ``evidence`` / + ``confidence`` must be absent. +* ``inferred`` -- an agent-asserted coupling the deterministic layer never + found (e.g. data flow inside notebook code). There is nothing to resolve + against, so instead the edge must carry a non-empty ``evidence`` string and a + ``confidence`` of ``high`` / ``medium`` / ``low``. Endpoints are still real + pipeline names. This keeps every edge accountable -- annotations to a proven + fact, inferences to stated evidence -- without letting an inference + masquerade as proven lineage. + +There is no LLM here -- the tool only validates and merges. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +_INSIGHTS_TOP_KEYS = {"overview", "system_recommendation", "pipeline_insights", "pipeline_relationships"} +_INSIGHT_KEYS = { + "pipeline", + "pattern_name", + "intent", + "databricks_pattern", + "recommended_patterns", + "conversion_notes", + "risk_if_ignored", +} +_RECOMMENDED_PATTERN_KEYS = {"pattern", "fit", "simplification_pattern"} +_MAX_RECOMMENDED_PATTERNS = 4 +_SYSTEM_RECOMMENDATION_KEYS = {"headline", "recommended_patterns", "cascade", "decision_driver"} +_RELATIONSHIP_KEYS = { + "from_pipeline", + "to_pipeline", + "lineage_edge", + "relationship_summary", + "databricks_pattern", + "risk_if_ignored", +} +_EDGE_KEYS = {"edge_type", "edge_identity", "evidence", "confidence"} +_CONFIDENCE_LEVELS = {"high", "medium", "low"} + + +def _pipeline_names(inventory: dict) -> set[str]: + return {str(p["name"]) for p in inventory.get("pipelines", []) if isinstance(p, dict) and p.get("name") is not None} + + +def _control_edge_triples(inventory: dict) -> set[tuple[str, str, str]]: + """Real control edges as ``(caller_pipeline, callee_pipeline, activity_name)``. + + Resolving on the full triple (not the bare ``activity_name``) is what pins a + relationship to a *specific* edge: ADF names the ExecutePipeline activity + after the callee, so a single ``activity_name`` is shared by every caller of + that callee -- a global-set check would accept a relationship whose + ``from``/``to`` point at the wrong pair. + """ + lineage = inventory.get("lineage") or {} + return { + (str(e["caller_pipeline"]), str(e["callee_pipeline"]), str(e["activity_name"])) + for e in lineage.get("control_edges", []) + if isinstance(e, dict) + and e.get("caller_pipeline") is not None + and e.get("callee_pipeline") is not None + and e.get("activity_name") is not None + } + + +def _data_edge_triples(inventory: dict) -> set[tuple[str, str, str]]: + """Real data edges as ``(producer_pipeline, consumer_pipeline, match_key)``. + + Same rationale as control edges: a ``match_key`` (a shared table/path) can be + produced and consumed across many pipeline pairs, so the producer/consumer + endpoints must match too. A relationship's ``from``/``to`` map to + producer/consumer respectively. + """ + lineage = inventory.get("lineage") or {} + return { + (str(e["producer_pipeline"]), str(e["consumer_pipeline"]), str(e["match_key"])) + for e in lineage.get("data_edges", []) + if isinstance(e, dict) + and e.get("producer_pipeline") is not None + and e.get("consumer_pipeline") is not None + and e.get("match_key") is not None + } + + +def validate_insights(raw: dict, inventory: dict) -> list[str]: + """Validate an authored insights dict against the inventory. + + Returns a list of human-readable violation strings; an empty list means the + insights are valid. All violations are collected (never fail-fast) so the + agent can fix every problem in one pass. + """ + violations: list[str] = [] + if not isinstance(raw, dict): + return [f"insights must be a JSON object, got {type(raw).__name__}"] + + for key in set(raw) - _INSIGHTS_TOP_KEYS: + violations.append(f"unknown top-level key: {key!r}") + + if "system_recommendation" in raw: + violations.extend(_validate_system_recommendation(raw["system_recommendation"])) + + names = _pipeline_names(inventory) + control_triples = _control_edge_triples(inventory) + data_triples = _data_edge_triples(inventory) + + insights = raw.get("pipeline_insights", []) + if not isinstance(insights, list): + violations.append("'pipeline_insights' must be a list") + insights = [] + for i, item in enumerate(insights): + loc = f"pipeline_insights[{i}]" + if not isinstance(item, dict): + violations.append(f"{loc} must be an object") + continue + for key in set(item) - _INSIGHT_KEYS: + violations.append(f"{loc}: unknown field {key!r}") + name = item.get("pipeline") + if not name: + violations.append(f"{loc}: missing required field 'pipeline'") + elif name not in names: + violations.append(f"{loc}: pipeline {name!r} not in inventory") + if "recommended_patterns" in item: + violations.extend(_validate_recommended_patterns(item["recommended_patterns"], loc)) + + relationships = raw.get("pipeline_relationships", []) + if not isinstance(relationships, list): + violations.append("'pipeline_relationships' must be a list") + relationships = [] + for i, rel in enumerate(relationships): + loc = f"pipeline_relationships[{i}]" + if not isinstance(rel, dict): + violations.append(f"{loc} must be an object") + continue + for key in set(rel) - _RELATIONSHIP_KEYS: + violations.append(f"{loc}: unknown field {key!r}") + from_pipeline = rel.get("from_pipeline") + to_pipeline = rel.get("to_pipeline") + for endpoint, value in (("from_pipeline", from_pipeline), ("to_pipeline", to_pipeline)): + if not value: + violations.append(f"{loc}: missing required field {endpoint!r}") + elif value not in names: + violations.append(f"{loc}: {endpoint} {value!r} not in inventory") + violations.extend( + _validate_edge(rel.get("lineage_edge"), loc, from_pipeline, to_pipeline, control_triples, data_triples) + ) + + return violations + + +def _validate_edge( + edge: Any, + loc: str, + from_pipeline: Any, + to_pipeline: Any, + control_triples: set[tuple[str, str, str]], + data_triples: set[tuple[str, str, str]], +) -> list[str]: + """Validate one lineage_edge ref. + + ``control`` / ``data`` edges annotate a deterministic edge: the full + ``(from, to, edge_identity)`` triple must resolve against the inventory's + lineage -- so the annotation connects exactly the pipelines it claims, not + merely some edge that happens to share the ``activity_name`` / ``match_key`` + -- and ``evidence`` / ``confidence`` must be absent. ``inferred`` edges assert + a coupling the deterministic layer never found: nothing to resolve, but a + non-empty ``evidence`` string and a ``confidence`` level are required instead. + """ + if edge is None: + return [f"{loc}: missing required field 'lineage_edge'"] + if not isinstance(edge, dict): + return [f"{loc}.lineage_edge must be an object"] + problems: list[str] = [] + for key in set(edge) - _EDGE_KEYS: + problems.append(f"{loc}.lineage_edge: unknown field {key!r}") + edge_type = edge.get("edge_type") + identity = edge.get("edge_identity") + if edge_type not in ("control", "data", "inferred"): + problems.append(f"{loc}.lineage_edge: edge_type must be 'control', 'data', or 'inferred', got {edge_type!r}") + return problems + if not isinstance(identity, str) or not identity: + problems.append(f"{loc}.lineage_edge: edge_identity must be a non-empty string") + return problems + + if edge_type == "inferred": + problems.extend(_validate_inferred_edge(edge, loc)) + return problems + + # Annotation tier: must resolve to a real edge, and must NOT carry the + # inferred-only evidence/confidence fields. + for field_name in ("evidence", "confidence"): + if edge.get(field_name) is not None: + problems.append(f"{loc}.lineage_edge: {field_name!r} is only valid on an 'inferred' edge") + # Resolve on the full triple. Endpoint problems are already reported above; only + # attempt the lookup when both endpoints are strings, else it is meaningless. + if not isinstance(from_pipeline, str) or not isinstance(to_pipeline, str): + return problems + valid = control_triples if edge_type == "control" else data_triples + if (from_pipeline, to_pipeline, identity) not in valid: + problems.append( + f"{loc}.lineage_edge: {edge_type} edge {identity!r} does not resolve to a lineage edge " + f"from {from_pipeline!r} to {to_pipeline!r}" + ) + return problems + + +def _validate_recommended_patterns(value: Any, loc: str) -> list[str]: + """Validate a pipeline_insight's ``recommended_patterns`` ranked list. + + When present it must hold 1-``_MAX_RECOMMENDED_PATTERNS`` objects, ordered + best-first. Each object requires a non-empty ``pattern`` and ``fit`` string + and a boolean ``simplification_pattern``. All problems are collected. + + Shared by both a pipeline's ``recommended_patterns`` and the top-level + ``system_recommendation.recommended_patterns`` (``loc`` distinguishes them). + """ + field_loc = f"{loc}.recommended_patterns" + if not isinstance(value, list): + return [f"{field_loc} must be a list"] + if not value: + return [ + f"{field_loc} must contain 1-{_MAX_RECOMMENDED_PATTERNS} patterns when present " + f"(omit the field instead of sending an empty list)" + ] + problems: list[str] = [] + if len(value) > _MAX_RECOMMENDED_PATTERNS: + problems.append(f"{field_loc} has {len(value)} patterns; at most {_MAX_RECOMMENDED_PATTERNS} are allowed") + for j, pattern in enumerate(value): + ploc = f"{field_loc}[{j}]" + if not isinstance(pattern, dict): + problems.append(f"{ploc} must be an object") + continue + for key in set(pattern) - _RECOMMENDED_PATTERN_KEYS: + problems.append(f"{ploc}: unknown field {key!r}") + for required in ("pattern", "fit"): + text = pattern.get(required) + if not isinstance(text, str) or not text.strip(): + problems.append(f"{ploc}: {required!r} must be a non-empty string") + # A JSON bool parses to Python bool; reject ints/strings so 1/"yes" don't slip through. + if not isinstance(pattern.get("simplification_pattern"), bool): + problems.append( + f"{ploc}: 'simplification_pattern' must be a boolean (true/false), " + f"got {type(pattern.get('simplification_pattern')).__name__}" + ) + return problems + + +def _validate_system_recommendation(value: Any) -> list[str]: + """Validate the optional top-level ``system_recommendation`` object. + + The one whole-factory architectural decision, authored before per-pipeline + insights. When present it must be an object with a non-empty ``headline`` and + a ``recommended_patterns`` ranked list (validated exactly like a pipeline's -- + the whole-system branches, best-first). ``cascade`` (a list of non-empty + strings naming what the top branch collapses) and ``decision_driver`` (the + gating question) are optional. All problems are collected. + """ + loc = "system_recommendation" + if not isinstance(value, dict): + return [f"{loc} must be an object"] + problems: list[str] = [] + for key in set(value) - _SYSTEM_RECOMMENDATION_KEYS: + problems.append(f"{loc}: unknown field {key!r}") + headline = value.get("headline") + if not isinstance(headline, str) or not headline.strip(): + problems.append(f"{loc}: 'headline' must be a non-empty string") + if "recommended_patterns" not in value: + problems.append(f"{loc}: missing required field 'recommended_patterns'") + else: + problems.extend(_validate_recommended_patterns(value["recommended_patterns"], loc)) + cascade = value.get("cascade") + if cascade is not None and ( + not isinstance(cascade, list) or not all(isinstance(c, str) and c.strip() for c in cascade) + ): + problems.append(f"{loc}: 'cascade' must be a list of non-empty strings when present") + driver = value.get("decision_driver") + if driver is not None and (not isinstance(driver, str) or not driver.strip()): + problems.append(f"{loc}: 'decision_driver' must be a non-empty string when present") + return problems + + +def _validate_inferred_edge(edge: dict, loc: str) -> list[str]: + """Validate the inferred-only fields: non-empty evidence + a confidence level.""" + problems: list[str] = [] + evidence = edge.get("evidence") + if not isinstance(evidence, str) or not evidence.strip(): + problems.append(f"{loc}.lineage_edge: an 'inferred' edge requires a non-empty 'evidence' string") + confidence = edge.get("confidence") + if confidence not in _CONFIDENCE_LEVELS: + problems.append( + f"{loc}.lineage_edge: an 'inferred' edge requires 'confidence' in " + f"{{'high', 'medium', 'low'}}, got {confidence!r}" + ) + return problems + + +def load_insights(*, insights: dict | None = None, insights_path: Path | None = None) -> dict: + """Return the raw insights dict from exactly one source (inline or file). + + Raises: + ValueError: if neither or both sources are provided. + """ + if (insights is None) == (insights_path is None): + raise ValueError("provide exactly one of 'insights' (inline dict) or 'insights_path'") + if insights is not None: + return insights + assert insights_path is not None # guaranteed by the guard above + return json.loads(insights_path.read_text(encoding="utf-8")) + + +def merge_into_inventory(inventory: dict, raw: dict) -> dict: + """Return a new dict identical to *inventory* with one added ``insights`` key. + + Does not mutate the input. No I/O. + """ + merged = dict(inventory) + merged["insights"] = raw + return merged + + +def enrich_inventory( + output_dir: Path, + *, + insights: dict | None = None, + insights_path: Path | None = None, +) -> dict: + """Validate authored insights against the inventory, then merge on success. + + Reads ``/metadata/inventory.json``, validates the authored + insights, and -- only when there are no violations -- writes the merged + inventory back byte-identically (adding just the ``insights`` key). + + Returns ``{"ok", "violations", "pipeline_insights", "relationships"}``. + On violations, ``ok`` is False and the file is left untouched. + + Raises: + FileNotFoundError: when ``inventory.json`` does not exist. + """ + inventory_path = Path(output_dir) / "metadata" / "inventory.json" + if not inventory_path.exists(): + raise FileNotFoundError(f"No inventory.json under {inventory_path.parent}; run discover first.") + inventory = json.loads(inventory_path.read_text(encoding="utf-8")) + + raw = load_insights(insights=insights, insights_path=insights_path) + violations = validate_insights(raw, inventory) + if violations: + return {"ok": False, "violations": violations, "pipeline_insights": 0, "relationships": 0} + + merged = merge_into_inventory(inventory, raw) + inventory_path.write_text(json.dumps(merged, indent=2), encoding="utf-8") + return { + "ok": True, + "violations": [], + "pipeline_insights": len(raw.get("pipeline_insights", [])), + "relationships": len(raw.get("pipeline_relationships", [])), + } diff --git a/src/flowx/reporting/coverage.py b/src/flowx/reporting/coverage.py index 8f15238..298b075 100644 --- a/src/flowx/reporting/coverage.py +++ b/src/flowx/reporting/coverage.py @@ -49,6 +49,7 @@ "finding_fingerprints", "complexity_score", "complexity_size", + "has_insights", ) _CSV_INT_COLUMNS: tuple[str, ...] = ( @@ -122,6 +123,7 @@ def build_coverage_rows(metadata_dir: Path) -> list[dict[str, Any]]: for row in csv.DictReader(handle): csv_by_pipeline[row["pipeline"]] = row + has_insights = "insights" in inventory rows: list[dict[str, Any]] = [] for pipeline in inventory.get("pipelines", []): name = pipeline.get("name", "") @@ -204,6 +206,7 @@ def _csv_int(col: str, _csv_row: dict[str, str] = csv_row) -> int: "finding_fingerprints": json.dumps(fingerprints, separators=(",", ":")), "complexity_score": _csv_int("complexity_score"), "complexity_size": csv_row.get("complexity_size", "") or "", + "has_insights": has_insights, } ) rows.sort(key=lambda row: row["pipeline"]) diff --git a/src/flowx/reporting/results.py b/src/flowx/reporting/results.py index 7ec5f50..d77b14c 100644 --- a/src/flowx/reporting/results.py +++ b/src/flowx/reporting/results.py @@ -48,6 +48,7 @@ "finding_fingerprints": "STRING", "complexity_score": "INT", "complexity_size": "STRING", + "has_insights": "BOOLEAN", } RESULTS_COLUMNS: tuple[tuple[str, str], ...] = ( @@ -69,6 +70,7 @@ } ) _FLOAT_METRICS: frozenset[str] = frozenset({"coverage_pct", "deterministic_coverage_pct", "code_attached_coverage_pct"}) +_BOOL_METRICS: frozenset[str] = frozenset({"has_insights"}) def _sql_str(value: Any) -> str: @@ -80,6 +82,8 @@ def _metric_value_sql(column: str, value: Any) -> str: """Renders one metric column value as a SQL literal.""" if column in _STRING_METRICS: return _sql_str(value) + if column in _BOOL_METRICS: + return "TRUE" if value else "FALSE" if column in _FLOAT_METRICS: return repr(float(value or 0)) return str(int(value or 0)) diff --git a/src/flowx/sources/adf/loader.py b/src/flowx/sources/adf/loader.py index b14e365..22470e3 100644 --- a/src/flowx/sources/adf/loader.py +++ b/src/flowx/sources/adf/loader.py @@ -1122,8 +1122,8 @@ def main(argv: list[str] | None = None) -> int: logger.info("Wrote %d pipeline ARM JSON file(s) to %s", len(arm_paths), metadata_dir) summary = inventory_dict["summary"] - print("\nADF Profile Summary") - print("===================") + print("\nADF Discovery Summary") + print("=====================") print(f"Pipelines parsed: {summary['pipeline_count']}") print(f"Total activities: {summary['activity_count']}") print("\nStrategy Breakdown:") diff --git a/tests/unit/test_pipeline_insights.py b/tests/unit/test_pipeline_insights.py new file mode 100644 index 0000000..c9c9ba8 --- /dev/null +++ b/tests/unit/test_pipeline_insights.py @@ -0,0 +1,845 @@ +"""Tests for agentic insights models, validation, and enrichment (discover phase).""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +pytest.importorskip("mcp") + +from flowx.adapter.__main__ import main as adapter_cli_main # noqa: E402 +from flowx.mcp import runner as mcp_runner # noqa: E402 +from flowx.mcp.server import _cmd_enrich # noqa: E402 +from flowx.models.insights import ( # noqa: E402 + Insights, + LineageEdgeRef, + PipelineInsight, + PipelineRelationship, + RecommendedPattern, + SystemRecommendation, +) +from flowx.parser.pipeline_insights import ( # noqa: E402 + enrich_inventory, + load_insights, + merge_into_inventory, + validate_insights, +) +from flowx.sources.adf.loader import ( # noqa: E402 + _inventory_to_dict, + build_inventory, + load_adf_definitions, +) + + +def test_insights_dataclasses_construct_with_defaults(): + edge = LineageEdgeRef(edge_type="control", edge_identity="Run Ingestion Pipeline") + rel = PipelineRelationship(from_pipeline="factory_a", to_pipeline="factory_b", lineage_edge=edge) + insight = PipelineInsight(pipeline="factory_a") + doc = Insights( + overview="whole factory", + pipeline_insights=[insight], + pipeline_relationships=[rel], + ) + assert doc.pipeline_insights[0].pipeline == "factory_a" + assert doc.pipeline_relationships[0].lineage_edge.edge_type == "control" + assert doc.pipeline_relationships[0].lineage_edge.edge_identity == "Run Ingestion Pipeline" + # optional fields default cleanly + assert insight.recommended_patterns == [] + assert insight.conversion_notes == [] + assert insight.risk_if_ignored is None + assert rel.relationship_summary is None + + +def test_recommended_pattern_dataclass_defaults(): + pat = RecommendedPattern( + pattern="Lakeflow Connect SQL Server connector", + fit="Managed CDC ingestion replaces the bespoke watermark Copy", + simplification_pattern=True, + ) + assert pat.pattern == "Lakeflow Connect SQL Server connector" + assert pat.simplification_pattern is True + + +def test_system_recommendation_dataclass_defaults(): + sr = SystemRecommendation(headline="Managed ingestion collapses the extraction factory") + assert sr.headline.startswith("Managed ingestion") + assert sr.recommended_patterns == [] + assert sr.cascade == [] + assert sr.decision_driver is None + + +def _inventory() -> dict: + """A minimal inventory dict in discover's serialized shape.""" + return { + "source_dir": "/tmp/adf", + "pipelines": [ + {"name": "factory_a", "activities": []}, + {"name": "factory_b", "activities": []}, + ], + "summary": {"pipeline_count": 2}, + "lineage": { + "control_edges": [ + { + "caller_pipeline": "factory_a", + "callee_pipeline": "factory_b", + "activity_name": "Run Ingestion Pipeline", + "wait_on_completion": True, + } + ], + "data_edges": [ + { + "dataset_name": "ds_orders", + "identity": "curated.orders", + "producer_pipeline": "factory_a", + "producer_activity": "Write Orders", + "consumer_pipeline": "factory_b", + "consumer_activity": "Read Orders", + "match_kind": "identity", + "match_key": "curated.orders", + } + ], + }, + } + + +def _good_insights() -> dict: + return { + "overview": "Two-stage ingestion then transform.", + "pipeline_insights": [ + { + "pipeline": "factory_a", + "intent": "Ingest", + "databricks_pattern": "Autoloader", + "recommended_patterns": [ + { + "pattern": "Lakeflow Connect SQL Server connector", + "fit": "Managed CDC ingestion replaces the bespoke watermark Copy", + "simplification_pattern": True, + }, + { + "pattern": "Auto Loader", + "fit": "Incremental file ingestion when a managed connector is unavailable", + "simplification_pattern": False, + }, + ], + "risk_if_ignored": "Switch-nested calls read as a leaf in lineage", + }, + {"pipeline": "factory_b", "intent": "Transform"}, + ], + "pipeline_relationships": [ + { + "from_pipeline": "factory_a", + "to_pipeline": "factory_b", + "lineage_edge": { + "edge_type": "control", + "edge_identity": "Run Ingestion Pipeline", + }, + "relationship_summary": "A invokes B", + "databricks_pattern": "run_job_task", + "risk_if_ignored": "ordering lost", + } + ], + } + + +def test_validator_accepts_good_insights(): + assert validate_insights(_good_insights(), _inventory()) == [] + + +def test_rejects_pipeline_not_in_inventory(): + raw = _good_insights() + raw["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + violations = validate_insights(raw, _inventory()) + assert violations + assert any("ghost_pipeline" in v for v in violations) + + +def test_rejects_relationship_endpoint_not_in_inventory(): + raw = _good_insights() + raw["pipeline_relationships"][0]["to_pipeline"] = "ghost_pipeline" + violations = validate_insights(raw, _inventory()) + assert any("ghost_pipeline" in v for v in violations) + + +def test_rejects_unresolvable_control_edge(): + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "No Such Activity" + violations = validate_insights(raw, _inventory()) + assert any("No Such Activity" in v for v in violations) + + +def test_data_edge_binds_on_match_key(): + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"] = { + "edge_type": "data", + "edge_identity": "curated.orders", + } + assert validate_insights(raw, _inventory()) == [] + # a non-matching key is rejected + raw["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "curated.missing" + assert validate_insights(raw, _inventory()) + + +def test_inferred_edge_with_evidence_and_confidence_validates(): + """An inferred edge needs no deterministic edge to resolve against -- just + a real endpoint pair, a non-empty evidence string, and a confidence level.""" + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"] = { + "edge_type": "inferred", + "edge_identity": "curated.orders_enriched", + "evidence": "factory_a's notebook writes curated.orders_enriched; factory_b's notebook reads it.", + "confidence": "medium", + } + assert validate_insights(raw, _inventory()) == [] + + +def test_inferred_edge_requires_evidence(): + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"] = { + "edge_type": "inferred", + "edge_identity": "curated.orders_enriched", + "confidence": "low", + } + violations = validate_insights(raw, _inventory()) + assert any("evidence" in v for v in violations) + + +def test_inferred_edge_requires_valid_confidence(): + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"] = { + "edge_type": "inferred", + "edge_identity": "curated.orders_enriched", + "evidence": "shared table observed in both notebooks", + "confidence": "pretty-sure", + } + violations = validate_insights(raw, _inventory()) + assert any("confidence" in v for v in violations) + # missing confidence entirely is also rejected + del raw["pipeline_relationships"][0]["lineage_edge"]["confidence"] + assert any("confidence" in v for v in validate_insights(raw, _inventory())) + + +def test_inferred_edge_does_not_resolve_against_lineage(): + """An inferred edge_identity is agent-authored, not a real edge key, so it must + NOT be validated against the deterministic lineage sets.""" + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"] = { + "edge_type": "inferred", + # deliberately not a real control activity_name or data match_key + "edge_identity": "not-a-real-lineage-key", + "evidence": "coupling inferred from shared notebook output path", + "confidence": "high", + } + assert validate_insights(raw, _inventory()) == [] + + +def test_annotation_edge_rejects_evidence_confidence(): + """evidence/confidence are inferred-only; a control/data edge carrying them is rejected.""" + raw = _good_insights() + raw["pipeline_relationships"][0]["lineage_edge"]["evidence"] = "should not be here" + raw["pipeline_relationships"][0]["lineage_edge"]["confidence"] = "high" + violations = validate_insights(raw, _inventory()) + assert any("evidence" in v for v in violations) + assert any("confidence" in v for v in violations) + + +def test_control_edge_resolves_on_full_triple_not_just_activity_name(): + """A control edge_identity that is a real activity_name but belongs to a + DIFFERENT (caller, callee) pair must be rejected. + + ADF names the ExecutePipeline activity after the callee, so one activity_name + is shared by every caller of that callee; resolving on the bare name (a global + set) would wrongly accept a relationship whose from/to point at another pair. + """ + inventory = { + "pipelines": [ + {"name": "orchestrator_a", "activities": []}, + {"name": "orchestrator_b", "activities": []}, + {"name": "shared_callee", "activities": []}, + ], + "summary": {"pipeline_count": 3}, + "lineage": { + "control_edges": [ + { + "caller_pipeline": "orchestrator_a", + "callee_pipeline": "shared_callee", + "activity_name": "Run Shared", + "wait_on_completion": True, + }, + { + "caller_pipeline": "orchestrator_b", + "callee_pipeline": "shared_callee", + "activity_name": "Run Shared", # same name, different caller + "wait_on_completion": True, + }, + ], + "data_edges": [], + }, + } + # Real edge: orchestrator_a -> shared_callee with "Run Shared" resolves. + good = { + "pipeline_insights": [], + "pipeline_relationships": [ + { + "from_pipeline": "orchestrator_a", + "to_pipeline": "shared_callee", + "lineage_edge": {"edge_type": "control", "edge_identity": "Run Shared"}, + } + ], + } + assert validate_insights(good, inventory) == [] + + # Wrong pair: no edge orchestrator_a -> orchestrator_b exists, even though the + # activity_name "Run Shared" is a real name elsewhere. Must be rejected. + bad = json.loads(json.dumps(good)) + bad["pipeline_relationships"][0]["to_pipeline"] = "orchestrator_b" + violations = validate_insights(bad, inventory) + assert violations, "a valid activity_name on the wrong (from,to) pair must not resolve" + assert any("orchestrator_b" in v for v in violations) + + # Right pair, wrong identity: the edge orchestrator_a -> shared_callee is real, + # but "Nope" is not its activity_name. Must be rejected. + wrong_id = json.loads(json.dumps(good)) + wrong_id["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "Nope" + assert validate_insights(wrong_id, inventory) + + # Reversed direction: the real edge is caller=orchestrator_a -> callee=shared_callee. + # Swapping from/to is NOT a real edge and must be rejected -- pins the non-reversed + # from->caller / to->callee mapping so a future refactor cannot silently flip it. + reversed_rel = json.loads(json.dumps(good)) + reversed_rel["pipeline_relationships"][0]["from_pipeline"] = "shared_callee" + reversed_rel["pipeline_relationships"][0]["to_pipeline"] = "orchestrator_a" + assert validate_insights(reversed_rel, inventory), "reversed-direction edge must not resolve" + + +def test_data_edge_resolves_on_full_triple_not_just_match_key(): + """Two producers write the same match_key. A relationship must resolve only to + the producer/consumer pair that actually exists, not to any edge with that key.""" + inventory = { + "pipelines": [ + {"name": "producer_p", "activities": []}, + {"name": "producer_q", "activities": []}, + {"name": "consumer_c", "activities": []}, + ], + "summary": {"pipeline_count": 3}, + "lineage": { + "control_edges": [], + "data_edges": [ + { + "dataset_name": "ds", + "identity": "curated.shared", + "producer_pipeline": "producer_p", + "producer_activity": "Write", + "consumer_pipeline": "consumer_c", + "consumer_activity": "Read", + "match_kind": "identity", + "match_key": "curated.shared", + }, + { + "dataset_name": "ds", + "identity": "curated.shared", + "producer_pipeline": "producer_q", # same key, different producer + "producer_activity": "Write", + "consumer_pipeline": "consumer_c", + "consumer_activity": "Read", + "match_kind": "identity", + "match_key": "curated.shared", + }, + ], + }, + } + good = { + "pipeline_insights": [], + "pipeline_relationships": [ + { + "from_pipeline": "producer_q", + "to_pipeline": "consumer_c", + "lineage_edge": {"edge_type": "data", "edge_identity": "curated.shared"}, + } + ], + } + assert validate_insights(good, inventory) == [] + + # producer_p -> producer_q is NOT a real edge, though both know curated.shared. + bad = json.loads(json.dumps(good)) + bad["pipeline_relationships"][0]["from_pipeline"] = "producer_p" + bad["pipeline_relationships"][0]["to_pipeline"] = "producer_q" + assert validate_insights(bad, inventory) + + +def test_rejects_missing_required_field(): + # PipelineInsight missing 'pipeline' + raw = {"pipeline_insights": [{"intent": "x"}], "pipeline_relationships": []} + assert any("pipeline" in v for v in validate_insights(raw, _inventory())) + # PipelineRelationship missing 'lineage_edge' + raw2 = { + "pipeline_insights": [], + "pipeline_relationships": [{"from_pipeline": "factory_a", "to_pipeline": "factory_b"}], + } + assert any("lineage_edge" in v for v in validate_insights(raw2, _inventory())) + + +def test_rejects_unknown_field(): + raw = _good_insights() + raw["pipeline_insights"][0]["bogus_key"] = "x" + assert any("bogus_key" in v for v in validate_insights(raw, _inventory())) + + +def test_rejects_unknown_top_level_key(): + raw = _good_insights() + raw["surprise"] = 1 + assert any("surprise" in v for v in validate_insights(raw, _inventory())) + + +# --- recommended_patterns ------------------------------------------------- + + +def _set_patterns(raw: dict, patterns: object) -> dict: + """Set factory_a's recommended_patterns to *patterns* and return the dict.""" + raw["pipeline_insights"][0]["recommended_patterns"] = patterns + return raw + + +def test_recommended_patterns_optional_when_omitted(): + raw = _good_insights() + del raw["pipeline_insights"][0]["recommended_patterns"] + assert validate_insights(raw, _inventory()) == [] + + +def test_recommended_patterns_accepts_one_to_four(): + one = [{"pattern": "Lakeflow Jobs", "fit": "orchestration", "simplification_pattern": True}] + assert validate_insights(_set_patterns(_good_insights(), one), _inventory()) == [] + four = [{"pattern": f"Pattern {n}", "fit": f"reason {n}", "simplification_pattern": n % 2 == 0} for n in range(4)] + assert validate_insights(_set_patterns(_good_insights(), four), _inventory()) == [] + + +def test_recommended_patterns_rejects_more_than_four(): + five = [{"pattern": f"Pattern {n}", "fit": f"reason {n}", "simplification_pattern": True} for n in range(5)] + violations = validate_insights(_set_patterns(_good_insights(), five), _inventory()) + assert any("recommended_patterns" in v for v in violations) + + +def test_recommended_patterns_rejects_empty_list(): + violations = validate_insights(_set_patterns(_good_insights(), []), _inventory()) + assert any("recommended_patterns" in v for v in violations) + + +def test_recommended_patterns_rejects_non_list(): + violations = validate_insights(_set_patterns(_good_insights(), "Lakeflow Jobs"), _inventory()) + assert any("recommended_patterns" in v and "list" in v for v in violations) + + +def test_recommended_patterns_rejects_non_dict_item(): + violations = validate_insights(_set_patterns(_good_insights(), ["Lakeflow Jobs"]), _inventory()) + assert any("recommended_patterns[0]" in v for v in violations) + + +def test_recommended_patterns_requires_pattern_and_fit(): + missing_pattern = [{"fit": "x", "simplification_pattern": True}] + assert any( + "pattern" in v for v in validate_insights(_set_patterns(_good_insights(), missing_pattern), _inventory()) + ) + missing_fit = [{"pattern": "Lakeflow Jobs", "simplification_pattern": True}] + assert any("fit" in v for v in validate_insights(_set_patterns(_good_insights(), missing_fit), _inventory())) + blank_pattern = [{"pattern": " ", "fit": "x", "simplification_pattern": True}] + assert any("pattern" in v for v in validate_insights(_set_patterns(_good_insights(), blank_pattern), _inventory())) + + +def test_recommended_patterns_simplification_pattern_must_be_bool(): + bad = [{"pattern": "Lakeflow Jobs", "fit": "x", "simplification_pattern": "yes"}] + violations = validate_insights(_set_patterns(_good_insights(), bad), _inventory()) + assert any("simplification_pattern" in v for v in violations) + # missing entirely is also rejected (the field is required) + missing = [{"pattern": "Lakeflow Jobs", "fit": "x"}] + assert any( + "simplification_pattern" in v for v in validate_insights(_set_patterns(_good_insights(), missing), _inventory()) + ) + + +def test_recommended_patterns_effort_field_removed(): + """`effort` was dropped from the schema; it must now be rejected as an unknown field.""" + bad = [{"pattern": "Lakeflow Jobs", "fit": "x", "simplification_pattern": True, "effort": "moderate"}] + violations = validate_insights(_set_patterns(_good_insights(), bad), _inventory()) + assert any("effort" in v for v in violations) + + +def test_recommended_patterns_rejects_unknown_item_field(): + bad = [{"pattern": "Lakeflow Jobs", "fit": "x", "simplification_pattern": True, "bogus": 1}] + violations = validate_insights(_set_patterns(_good_insights(), bad), _inventory()) + assert any("bogus" in v for v in violations) + + +# --- system_recommendation ------------------------------------------------ + + +def _good_system_recommendation() -> dict: + return { + "headline": "Managed ingestion collapses the extraction factory", + "recommended_patterns": [ + { + "pattern": "Lakeflow Connect for the whole SQL Server extraction family", + "fit": "One managed connector replaces the fan-out orchestrator, the clones, and the watermark CSV", + "simplification_pattern": True, + }, + { + "pattern": "For-each orchestrator + collapsed parameterized jobs", + "fit": "Fallback when the connector is not approved for this source", + "simplification_pattern": False, + }, + ], + "cascade": [ + "clone extractors -> managed connector pipelines", + "version-watermark CSV -> gone", + ], + "decision_driver": "Is the Lakeflow Connect SQL Server connector GA/approved for this source?", + } + + +def test_system_recommendation_optional_when_omitted(): + raw = _good_insights() + assert "system_recommendation" not in raw + assert validate_insights(raw, _inventory()) == [] + + +def test_system_recommendation_accepts_good(): + raw = _good_insights() + raw["system_recommendation"] = _good_system_recommendation() + assert validate_insights(raw, _inventory()) == [] + + +def test_system_recommendation_must_be_object(): + raw = _good_insights() + raw["system_recommendation"] = "nope" + assert any("system_recommendation" in v for v in validate_insights(raw, _inventory())) + + +def test_system_recommendation_requires_headline(): + raw = _good_insights() + sr = _good_system_recommendation() + del sr["headline"] + raw["system_recommendation"] = sr + assert any("headline" in v for v in validate_insights(raw, _inventory())) + + +def test_system_recommendation_requires_recommended_patterns(): + raw = _good_insights() + sr = _good_system_recommendation() + del sr["recommended_patterns"] + raw["system_recommendation"] = sr + assert any("recommended_patterns" in v for v in validate_insights(raw, _inventory())) + + +def test_system_recommendation_reuses_pattern_validation(): + """The branch patterns go through the same validator, scoped under system_recommendation.""" + raw = _good_insights() + sr = _good_system_recommendation() + sr["recommended_patterns"][0]["simplification_pattern"] = "yes" # must be a bool + raw["system_recommendation"] = sr + violations = validate_insights(raw, _inventory()) + assert any("simplification_pattern" in v and "system_recommendation" in v for v in violations) + + +def test_system_recommendation_rejects_unknown_field(): + raw = _good_insights() + sr = _good_system_recommendation() + sr["bogus"] = 1 + raw["system_recommendation"] = sr + assert any("bogus" in v for v in validate_insights(raw, _inventory())) + + +def test_system_recommendation_cascade_must_be_list_of_strings(): + raw = _good_insights() + sr = _good_system_recommendation() + sr["cascade"] = "not a list" + raw["system_recommendation"] = sr + assert any("cascade" in v for v in validate_insights(raw, _inventory())) + + +def test_system_recommendation_survives_enrich_round_trip(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + raw = _good_insights() + raw["system_recommendation"] = _good_system_recommendation() + result = enrich_inventory(tmp_path, insights=raw) + assert result["ok"] is True + on_disk = json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + assert on_disk["insights"]["system_recommendation"]["headline"].startswith("Managed ingestion") + + +def _write_inventory(tmp_path: Path, inventory: dict) -> Path: + """Write inventory.json exactly as discover does (indent=2, no trailing newline).""" + metadata = tmp_path / "metadata" + metadata.mkdir(parents=True, exist_ok=True) + path = metadata / "inventory.json" + path.write_text(json.dumps(inventory, indent=2), encoding="utf-8") + return path + + +def test_load_insights_requires_exactly_one_source(): + with pytest.raises(ValueError): + load_insights() + with pytest.raises(ValueError): + load_insights(insights={"a": 1}, insights_path=Path("/x")) + + +def test_load_insights_from_inline_dict(): + assert load_insights(insights={"overview": "x"}) == {"overview": "x"} + + +def test_load_insights_from_path(tmp_path: Path): + p = tmp_path / "ins.json" + p.write_text(json.dumps({"overview": "y"}), encoding="utf-8") + assert load_insights(insights_path=p) == {"overview": "y"} + + +def test_merge_into_inventory_adds_one_key_without_mutating(): + inv = {"pipelines": [], "summary": {}, "lineage": {}} + raw = {"overview": "z"} + merged = merge_into_inventory(inv, raw) + assert merged["insights"] == {"overview": "z"} + assert "insights" not in inv # input not mutated + assert set(merged) == {"pipelines", "summary", "lineage", "insights"} + + +def test_enrich_success_counts_and_writes(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + result = enrich_inventory(tmp_path, insights=_good_insights()) + assert result["ok"] is True + assert result["violations"] == [] + assert result["pipeline_insights"] == 2 + assert result["relationships"] == 1 + on_disk = json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + assert on_disk["insights"]["overview"] == "Two-stage ingestion then transform." + + +def test_two_pass_deterministic_keys_byte_identical(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + enrich_inventory(tmp_path, insights=_good_insights()) + after = json.loads(path.read_text(encoding="utf-8")) + # every key except the added 'insights' is byte-identical to the pre-enrich file + after_without_insights = {k: v for k, v in after.items() if k != "insights"} + assert json.dumps(after_without_insights, indent=2) == before + + +def test_enrich_is_idempotent(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + enrich_inventory(tmp_path, insights=_good_insights()) + first = path.read_text(encoding="utf-8") + enrich_inventory(tmp_path, insights=_good_insights()) + second = path.read_text(encoding="utf-8") + assert first == second + + +def test_validation_failure_does_not_write(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + bad = _good_insights() + bad["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + result = enrich_inventory(tmp_path, insights=bad) + assert result["ok"] is False + assert result["violations"] + assert path.read_text(encoding="utf-8") == before # file untouched + + +def test_enrich_missing_inventory_raises(tmp_path: Path): + with pytest.raises(FileNotFoundError): + enrich_inventory(tmp_path, insights=_good_insights()) + + +def _real_inventory(fixtures_dir) -> dict: + """Build a real inventory dict (with lineage) from the shipped fixtures.""" + definitions = load_adf_definitions(fixtures_dir) + inventory = build_inventory(definitions) + return _inventory_to_dict(inventory, str(fixtures_dir)) + + +def test_control_edge_binding_matches_and_rejects(fixtures_dir): + inventory = _real_inventory(fixtures_dir) + good = { + "pipeline_insights": [], + "pipeline_relationships": [ + { + "from_pipeline": "pipeline_execute_pipeline_nested", + "to_pipeline": "pipeline_copy_sql_to_delta", + "lineage_edge": { + "edge_type": "control", + "edge_identity": "Run Ingestion Pipeline", + }, + } + ], + } + assert validate_insights(good, inventory) == [] + + bad = json.loads(json.dumps(good)) + bad["pipeline_relationships"][0]["lineage_edge"]["edge_identity"] = "No Such Activity" + assert validate_insights(bad, inventory) + + +def test_real_inventory_enrich_round_trip(fixtures_dir, tmp_path: Path): + inventory = _real_inventory(fixtures_dir) + metadata = tmp_path / "metadata" + metadata.mkdir(parents=True) + (metadata / "inventory.json").write_text(json.dumps(inventory, indent=2), encoding="utf-8") + result = enrich_inventory( + tmp_path, + insights={ + "overview": "orchestrated ingest/transform/cleanup", + "pipeline_insights": [{"pipeline": "pipeline_execute_pipeline_nested", "intent": "orchestrate"}], + "pipeline_relationships": [], + }, + ) + assert result["ok"] is True + on_disk = json.loads((metadata / "inventory.json").read_text()) + assert on_disk["insights"]["pipeline_insights"][0]["pipeline"] == "pipeline_execute_pipeline_nested" + + +def test_adapter_enrich_success(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + ins = tmp_path / "insights.json" + ins.write_text(json.dumps(_good_insights()), encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights-path", str(ins)]) + assert code == 0 + on_disk = json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + assert "insights" in on_disk + + +def test_adapter_enrich_validation_failure_returns_1(tmp_path: Path): + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + bad = _good_insights() + bad["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + ins = tmp_path / "insights.json" + ins.write_text(json.dumps(bad), encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights-path", str(ins)]) + assert code == 1 + assert path.read_text(encoding="utf-8") == before # untouched + + +def test_adapter_enrich_missing_inventory_returns_1(tmp_path: Path): + ins = tmp_path / "insights.json" + ins.write_text(json.dumps(_good_insights()), encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights-path", str(ins)]) + assert code == 1 + + +def test_adapter_enrich_inline_json_string(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights", json.dumps(_good_insights())]) + assert code == 0 + assert "insights" in json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + + +def test_materialize_json_round_trips(tmp_path: Path): + path = mcp_runner.materialize_json({"overview": "x"}) + try: + assert json.loads(Path(path).read_text()) == {"overview": "x"} + finally: + mcp_runner.cleanup_materialized(path) + assert not Path(path).exists() + + +def test_cmd_enrich_requires_a_payload(): + result = _cmd_enrich({"output_dir": "./flowx_output"}) + assert result["ok"] is False + assert "insights" in result["error"] + + +def test_cmd_enrich_inline_dict_success(tmp_path: Path): + _write_inventory(tmp_path, _inventory()) + result = _cmd_enrich({"output_dir": str(tmp_path), "insights": _good_insights()}) + assert result["ok"] is True + assert "insights" in json.loads((tmp_path / "metadata" / "inventory.json").read_text()) + + +def test_validate_insights_rejects_non_dict(): + """validate_insights must reject non-dict inputs with an actionable violation.""" + violations_none = validate_insights(None, _inventory()) # type: ignore[arg-type] + assert violations_none, "expected violations for None input" + assert any("JSON object" in v or "NoneType" in v for v in violations_none) + + violations_list = validate_insights([], _inventory()) # type: ignore[arg-type] + assert violations_list, "expected violations for list input" + assert any("JSON object" in v or "list" in v for v in violations_list) + + +def test_adapter_enrich_rejects_neither_source(tmp_path: Path): + """enrich must return 1 when neither --insights nor --insights-path is given.""" + _write_inventory(tmp_path, _inventory()) + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path)]) + assert code == 1 + + +def test_adapter_enrich_rejects_both_sources(tmp_path: Path): + """enrich must return 1 when both --insights and --insights-path are given.""" + _write_inventory(tmp_path, _inventory()) + ins_file = tmp_path / "insights.json" + ins_file.write_text(json.dumps(_good_insights()), encoding="utf-8") + code = adapter_cli_main( + [ + "enrich", + "--output-dir", + str(tmp_path), + "--insights", + json.dumps(_good_insights()), + "--insights-path", + str(ins_file), + ] + ) + assert code == 1 + + +def test_adapter_enrich_rejects_malformed_inline_json(tmp_path: Path): + """enrich must return 1 and not write when --insights is not valid JSON.""" + path = _write_inventory(tmp_path, _inventory()) + before = path.read_text(encoding="utf-8") + code = adapter_cli_main(["enrich", "--output-dir", str(tmp_path), "--insights", "{not valid json"]) + assert code == 1 + assert path.read_text(encoding="utf-8") == before # inventory untouched + + +def test_cmd_enrich_failure_surfaces_structured_violations(tmp_path: Path): + """On a validation failure, _cmd_enrich must return ok:false with a non-empty violations list.""" + _write_inventory(tmp_path, _inventory()) + bad = _good_insights() + bad["pipeline_insights"][0]["pipeline"] = "ghost_pipeline" + result = _cmd_enrich({"output_dir": str(tmp_path), "insights": bad}) + assert result["ok"] is False + violations = result.get("violations") + assert violations, "expected a non-empty violations list on failure" + assert any("ghost_pipeline" in v for v in violations) + + +def test_cmd_enrich_success_has_no_violations(tmp_path: Path): + """On a successful enrich, _cmd_enrich must not carry stray violations.""" + _write_inventory(tmp_path, _inventory()) + result = _cmd_enrich({"output_dir": str(tmp_path), "insights": _good_insights()}) + assert result["ok"] is True + assert not result.get("violations") + + +def test_cleanup_materialized_handles_all_prefixes(tmp_path: Path): + import tempfile as _tempfile + from pathlib import Path as _Path + + # mkdtemp dirs for all three prefixes are removed + for prefix in ("flowx-adf-", "flowx-vol-", "flowx-ws-"): + d = _tempfile.mkdtemp(prefix=prefix) + assert _Path(d).is_dir() + mcp_runner.cleanup_materialized(d) + assert not _Path(d).exists() + + # single ARM-template file inside a flowx-adf- dir removes the parent dir + base = _tempfile.mkdtemp(prefix="flowx-adf-") + arm = _Path(base) / "arm_template.json" + arm.write_text("{}", encoding="utf-8") + mcp_runner.cleanup_materialized(str(arm)) + assert not _Path(base).exists() + + # materialize_json file is unlinked WITHOUT removing the system temp root + f = mcp_runner.materialize_json({"a": 1}) + temp_root = _Path(_tempfile.gettempdir()) + mcp_runner.cleanup_materialized(f) + assert not _Path(f).exists() + assert temp_root.is_dir() # temp root itself untouched diff --git a/tests/unit/test_reporting_coverage.py b/tests/unit/test_reporting_coverage.py index 17c43d7..4a2d6f8 100644 --- a/tests/unit/test_reporting_coverage.py +++ b/tests/unit/test_reporting_coverage.py @@ -94,96 +94,14 @@ def test_build_coverage_rows_full_coverage_and_missing_csv(tmp_path: Path): assert beta["datasets"] == 0 and beta["complexity_size"] == "" # defaulted, no CSV -def test_audited_counts_drive_translation_and_deterministic_coverage(tmp_path: Path) -> None: - metadata = tmp_path / "metadata" - metadata.mkdir() - inventory = { - "source": "airflow", - "pipelines": [ - { - "name": "verified_with_gap", - "activities": [], - "audited_activity_count": 8, - "deterministic_count": 7, - "agentic_count": 1, - "failed_count": 0, - "excluded_count": 0, - "reconciliation_status": "verified_with_gaps", - "migration_status": "included", - "findings": [{"fingerprint": "abc123", "severity": "gap"}], - }, - { - "name": "failed", - "activities": [], - "audited_activity_count": 9, - "deterministic_count": 7, - "agentic_count": 1, - "failed_count": 1, - "excluded_count": 0, - "reconciliation_status": "failed", - "migration_status": "included", - "findings": [{"fingerprint": "def456", "severity": "failed"}], - }, - ], - } - (metadata / "inventory.json").write_text(json.dumps(inventory), encoding="utf-8") - - rows = {row["pipeline"]: row for row in build_coverage_rows(metadata)} - - verified = rows["verified_with_gap"] - assert verified["activities"] == 8 - assert verified["audited_activities"] == 8 - assert verified["coverage_pct"] == 100.0 - assert verified["deterministic_coverage_pct"] == 87.5 - assert verified["code_attached_coverage_pct"] == 87.5 - assert verified["resolved_agentic_count"] == 0 - assert verified["unresolved_agentic_count"] == 1 - assert json.loads(verified["agentic_resolution_outcomes"]) == { - "resolved": 0, - "needs_input": 0, - "deferred": 0, - "declined": 0, - "unreviewed": 1, - } - assert verified["finding_count"] == 1 - assert json.loads(verified["finding_fingerprints"]) == ["abc123"] - - failed = rows["failed"] - assert failed["activities"] == 9 - assert failed["failed_activities"] == 1 - assert failed["coverage_pct"] == 88.9 - assert failed["deterministic_coverage_pct"] == 77.8 - assert failed["code_attached_coverage_pct"] == 77.8 - assert failed["reconciliation_status"] == "failed" - - -def test_excluded_activities_remain_in_coverage_denominator(tmp_path: Path) -> None: - metadata = tmp_path / "metadata" - metadata.mkdir() - inventory = { - "source": "airflow", - "pipelines": [ - { - "name": "excluded", - "activities": [], - "audited_activity_count": 3, - "deterministic_count": 0, - "agentic_count": 0, - "failed_count": 0, - "excluded_count": 3, - "reconciliation_status": "verified", - "migration_status": "excluded", - "findings": [], - } - ], - } - (metadata / "inventory.json").write_text(json.dumps(inventory), encoding="utf-8") - - row = build_coverage_rows(metadata)[0] - - assert row["activities"] == 3 - assert row["excluded_activities"] == 3 - assert row["coverage_pct"] == 0.0 - assert row["deterministic_coverage_pct"] == 0.0 - assert row["code_attached_coverage_pct"] == 0.0 - assert row["migration_status"] == "excluded" +def test_has_insights_column_reflects_insights_key(tmp_path: Path): + md = _write_metadata(tmp_path) # writes inventory.json with no insights key + rows = build_coverage_rows(md) + assert all(row["has_insights"] is False for row in rows) + + inv_path = md / "inventory.json" + inv = json.loads(inv_path.read_text()) + inv["insights"] = {"overview": "x", "pipeline_insights": [], "pipeline_relationships": []} + inv_path.write_text(json.dumps(inv), encoding="utf-8") + rows2 = build_coverage_rows(md) + assert all(row["has_insights"] is True for row in rows2) diff --git a/tests/unit/test_reporting_results.py b/tests/unit/test_reporting_results.py index f7232cc..734b32a 100644 --- a/tests/unit/test_reporting_results.py +++ b/tests/unit/test_reporting_results.py @@ -243,35 +243,59 @@ def test_write_results_executes_create_schema_check_then_insert(tmp_path: Path): assert run_id in stmts[2][1] -def test_write_results_evolves_an_existing_legacy_schema_before_insert(tmp_path: Path) -> None: - legacy_columns = { - "run_id", - "run_date", - "run_by", - "pipeline", - "activities", - "datasets", - "linked_services", - "collapsible_patterns", - "databricks_native_activities", - "control_flow_activities", - "other_activities", - "deterministic_activities", - "agentic_activities", - "unsupported_activities", - "coverage_pct", - "complexity_score", - "complexity_size", +def _base_row(pipeline: str = "p1", *, has_insights: bool = False) -> dict: + """Minimal coverage row with all required columns, mirroring COVERAGE_METRIC_COLUMNS.""" + return { + "pipeline": pipeline, + "activities": 1, + "datasets": 0, + "linked_services": 0, + "collapsible_patterns": 0, + "databricks_native_activities": 1, + "control_flow_activities": 0, + "other_activities": 0, + "deterministic_activities": 1, + "agentic_activities": 0, + "unsupported_activities": 0, + "coverage_pct": 100.0, + "complexity_score": 2, + "complexity_size": "S", + "has_insights": has_insights, } - client = _FakeClient([_FakeWarehouse("wh1", "RUNNING", serverless=True)], columns=legacy_columns) - - R.write_results(_metadata(tmp_path), "cat.sch.tbl", client=client) - - statements = [statement for _warehouse, statement in client.statement_execution.statements] - assert len(statements) == 4 - assert statements[2].startswith("ALTER TABLE cat.sch.tbl ADD COLUMNS") - assert "audited_activities INT" in statements[2] - assert "deterministic_coverage_pct DOUBLE" in statements[2] - assert "code_attached_coverage_pct DOUBLE" in statements[2] - assert "agentic_resolution_outcomes STRING" in statements[2] - assert statements[3].startswith("INSERT INTO cat.sch.tbl") + + +def test_insert_sql_renders_has_insights_as_boolean_literal(): + """has_insights must render as TRUE/FALSE, not as integer 1/0. + + Databricks ANSI storeAssignmentPolicy rejects int literals into BOOLEAN DDL columns. + """ + rows = [_base_row("pipe_true", has_insights=True), _base_row("pipe_false", has_insights=False)] + sql = R.build_insert_sql("cat.sch.tbl", rows, "run-x") + + # TRUE/FALSE must appear; integer literals 1 or 0 must NOT stand in for has_insights. + assert "TRUE" in sql, "expected SQL boolean TRUE for has_insights=True" + assert "FALSE" in sql, "expected SQL boolean FALSE for has_insights=False" + + # Confirm neither row falls back to an integer representation: split off the VALUES + # portion and verify the has_insights position for each tuple. + from flowx.reporting.coverage import COVERAGE_METRIC_COLUMNS + + hi_index = list(COVERAGE_METRIC_COLUMNS).index("has_insights") + # Each VALUES tuple follows CURRENT_USER(), so the metric columns start at position 3 + # (run_id, CURRENT_TIMESTAMP(), CURRENT_USER() are the first three). + for line in sql.split("\n"): + line = line.strip().rstrip(",") + if not line.startswith("("): + continue + # Strip outer parens and split on ", " is unreliable for nested strings; + # use a simple positional approach: split by ", " after removing the outer parens. + inner = line[1:-1] if line.endswith(")") else line[1:] + # Find the metric section after the third comma-separated token + # (run_id literal, CURRENT_TIMESTAMP(), CURRENT_USER()) + parts = inner.split(", ", 3) # at most 4 chunks; last chunk is the metrics + if len(parts) < 4: + continue + metric_parts = parts[3].split(", ") + if hi_index < len(metric_parts): + hi_val = metric_parts[hi_index] + assert hi_val in ("TRUE", "FALSE"), f"has_insights rendered as {hi_val!r} instead of TRUE/FALSE"