From c7b8592305c4f5ed8f315a2dd93eb8b283176b23 Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:26:43 +0200 Subject: [PATCH 1/8] Support pending_offer in engine results --- .../dex-core/src/exmergo_dex_core/results.py | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/dex-core/src/exmergo_dex_core/results.py b/packages/dex-core/src/exmergo_dex_core/results.py index 7a4d759..bcf21c8 100644 --- a/packages/dex-core/src/exmergo_dex_core/results.py +++ b/packages/dex-core/src/exmergo_dex_core/results.py @@ -74,12 +74,27 @@ def __init__( class Result(BaseModel): """Base for every engine result. - ``pending_confirmation`` is the two-phase case, and it is why confirmation is - not always an exception. Commands that price a second phase only after paying - for the first (``relationships`` and ``map`` pricing verify probes after - inference finds candidates; ``check`` pricing its scanning axes after the free - ones complete) must return the work already paid for *and* the ask for the - rest. Raising would throw away results the user has already been billed for. + Two fields carry a priced second phase, and which one a command uses decides + the status its envelope prints. The question they answer is not "is there + money on the table" but "did the caller get what they asked for". + + ``pending_confirmation`` is the ask dex is *waiting on*: the caller requested + the paid work and it has not been authorized, so nothing they asked for has + finished. It is still not an exception, because a command that priced its + second phase only after paying for the first must return the work already + billed alongside the ask; raising would discard results the user has paid + for. ``relationships`` and ``map`` price verify probes after inference finds + candidates, and that is this case: ``--verify`` was requested. + + ``pending_offer`` is work the caller *did not* ask for. ``maintain check`` + and ``maintain semantic`` complete every free axis on any call, and the + scanning axes are an extension offered on top. Reporting that as a pending + charge trains a caller to confirm things that cost nothing, which erodes the + handshake everywhere it does matter, and frames a complete answer as though + nothing had run. So it prints ``ok`` with the offer under ``data.offer``, + and confirming remains the only way to spend. + + Setting both is a contradiction and :func:`to_envelope` refuses it. """ # Whether the payload carries `notes` even when there is nothing to say. @@ -99,6 +114,7 @@ class Result(BaseModel): # Reviewable diffs (propose-don't-impose). Nothing is applied by being here. diffs: list[dict[str, Any]] = Field(default_factory=list) pending_confirmation: ConfirmationRequest | None = None + pending_offer: ConfirmationRequest | None = None def data(self) -> dict[str, Any]: """The command-specific payload, keyed as the command contract documents.""" @@ -136,6 +152,12 @@ def to_envelope(result: Result, *, hints: dict[str, Any] | None = None) -> env.E if result.spend is not None: data["spend"] = result.spend + if result.pending_confirmation is not None and result.pending_offer is not None: + raise ValueError( + "a result cannot both wait on a confirmation and offer optional paid " + "work: pick the one that describes whether the caller asked for it" + ) + if result.pending_confirmation is not None: pending = result.pending_confirmation # The ask first, then the results already paid for: a two-phase command @@ -147,4 +169,20 @@ def to_envelope(result: Result, *, hints: dict[str, Any] | None = None) -> env.E warnings=[*result.warnings, *pending.warnings], diffs=result.diffs, ) + + if result.pending_offer is not None: + offer = result.pending_offer + # Nested rather than merged, unlike the confirmation path above: this + # envelope's own payload is the answer, and an estimate for work that did + # not run has no business sitting beside findings that did. For the same + # reason the offer's estimate stays out of `cost`, which on an `ok` reads + # as what this run cost. It lives in one place, `data.offer`, where + # reaching for it is a deliberate act. + return env.ok( + {**data, "offer": offer.data}, + cost=result.cost, + warnings=[*result.warnings, *offer.warnings], + diffs=result.diffs, + ) + return env.ok(data, cost=result.cost, warnings=result.warnings, diffs=result.diffs) From a32a13e556a2dde1f30391cbd15c5bde68484535 Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:26:48 +0200 Subject: [PATCH 2/8] Add support for axes in `command_args` function --- .../dex-core/src/exmergo_dex_core/command_args.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/dex-core/src/exmergo_dex_core/command_args.py b/packages/dex-core/src/exmergo_dex_core/command_args.py index 908e72e..738585c 100644 --- a/packages/dex-core/src/exmergo_dex_core/command_args.py +++ b/packages/dex-core/src/exmergo_dex_core/command_args.py @@ -99,6 +99,7 @@ def billed_handshake( *, per_table: dict[str, float] | None = None, notes: list[str] | None = None, + axes: list[str] | None = None, ) -> None: """The cost-before-spend handshake on billed connectors. @@ -139,6 +140,11 @@ def billed_handshake( } if per_table: data["per_table_bytes"] = per_table + if axes: + # What the estimate would add. Load-bearing for an offer, whose + # envelope reports `ok`: without it, an axis that did not run is + # indistinguishable from one that ran and found nothing. + data["axes"] = axes if notes: data.setdefault("notes", []) data["notes"] = [*data["notes"], *notes] @@ -165,6 +171,11 @@ def confirmation_request( let this raise, because discarding them to ask about the billed half would make the caller pay attention twice for one answer. That is ``maintain check`` and ``maintain semantic``, whose free axes always complete. + + Those two carry the returned request as ``Result.pending_offer`` rather than + ``pending_confirmation``, because the caller never asked for the scanning + axes: the request is priced work on offer, not a charge dex is waiting on. + Pass ``axes`` so the offer names what the estimate would add. """ try: From c86e1d3f6ded988c60acc0bf220588bcf1413472 Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:26:59 +0200 Subject: [PATCH 3/8] Refactor maintain commands to replace `pending_confirmation` with `pending_offer` --- .../src/exmergo_dex_core/maintain/commands.py | 91 +++++++++++-------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/packages/dex-core/src/exmergo_dex_core/maintain/commands.py b/packages/dex-core/src/exmergo_dex_core/maintain/commands.py index 6ae9c58..faefbb9 100644 --- a/packages/dex-core/src/exmergo_dex_core/maintain/commands.py +++ b/packages/dex-core/src/exmergo_dex_core/maintain/commands.py @@ -466,9 +466,10 @@ def semantic_drift(engine: DexEngine, objects: list[str] | None = None) -> Drift """Definitions that no longer match: dangling references, new categoricals. Two-phase on billed connectors: definition and reference checks are free and - run immediately; the dimension-cardinality scan waits behind the handshake, - and an unconfirmed call still returns the complete free findings alongside - the estimate rather than throwing away work that cost nothing but is real. + run immediately; the dimension-cardinality scan is offered on top. An + unconfirmed call is a complete answer for the free half, not a pending + charge, so it returns ``ok`` carrying those findings and an offer for the + scan rather than throwing away work that cost nothing but is real. """ store = engine.store @@ -501,22 +502,23 @@ def semantic_drift(engine: DexEngine, objects: list[str] | None = None) -> Drift checks = drift_mod.cardinality_plan( current_semantic, snap, _semantic_names(scope_names) if scope_names else None ) - pending: ConfirmationRequest | None = None + offer: ConfirmationRequest | None = None billed_findings: list[drift_mod.DriftFinding] = [] if checks: estimate, per_table = drift_mod.cardinality_estimate(adapter, checks) - pending = command_args.confirmation_request( + offer = command_args.confirmation_request( "maintain semantic", adapter, estimate, per_table=per_table, + axes=["semantic_cardinality"], notes=[ - "the definition and reference checks are free and already " - "complete (their findings are included in this envelope); " - "the estimate covers only the dimension-cardinality scan" + "the definition and reference findings in this envelope are " + "final; the estimate buys the dimension-cardinality scan on top " + "of them" ], ) - if pending is None: + if offer is None: billed_findings = _semantic_scope( drift_mod.cardinality_drift(adapter, checks, current_semantic), scope_names, @@ -524,19 +526,20 @@ def semantic_drift(engine: DexEngine, objects: list[str] | None = None) -> Drift ranked = drift_mod.rank_findings(free_findings + billed_findings) _record_axes(store, snap, connector, {"semantic": (ranked, scope_names)}) - if pending is not None: - # The free half is complete and real, so it returns alongside the ask - # for the scanning half rather than being discarded and re-derived. + # Identical warnings either way. Every reason this baseline may not describe + # the warehouse bounds the free findings exactly as it bounds the settled + # ones, and the unconfirmed call is the one a session makes first, so it is + # the last place that caveat should go missing. + warnings = warnings + _baseline_warnings( + store, snap, engine.config.profile_freshness_hours + ) + if offer is not None: + # The free half is complete and real, so it returns as the answer it is, + # with the scanning half offered on top rather than gating it. result = _drift_result({"semantic": ranked}, snap, store, warnings=warnings) - result.pending_confirmation = pending + result.pending_offer = offer return result - result = _drift_result( - {"semantic": ranked}, - snap, - store, - warnings=warnings - + _baseline_warnings(store, snap, engine.config.profile_freshness_hours), - ) + result = _drift_result({"semantic": ranked}, snap, store, warnings=warnings) return command_args.stamp_spend(result, adapter) @@ -550,7 +553,10 @@ def check(engine: DexEngine, objects: list[str] | None = None) -> DriftResult: Two-phase by construction: the free axes (schema, volume, semantic references) always run and their findings always return; the scanning axes (grain, cardinality) run immediately on free connectors and behind one - combined estimate on billed ones. + combined estimate on billed ones. An unconfirmed call on a billed connector + is therefore a complete answer for three axes rather than a pending charge, + and says so: ``ok``, with ``axes_run`` naming what finished and ``offer`` + naming what the estimate would add. ``objects`` narrows every axis exactly like the focused detectors do: schema/volume/grain resolve it against known identifiers (raising if a @@ -609,27 +615,46 @@ def check(engine: DexEngine, objects: list[str] | None = None) -> DriftResult: scans_needed = bool( plan.key_checks or plan.fanout_pairs or plan.composite_checks or checks ) - pending: ConfirmationRequest | None = None + offer: ConfirmationRequest | None = None if scans_needed and command_args.cost_gate(adapter) is not None: grain_total, grain_per = drift_mod.grain_estimate(adapter, plan) card_total, card_per = drift_mod.cardinality_estimate(adapter, checks) per_table = dict(grain_per) for identifier, estimate in card_per.items(): per_table[identifier] = per_table.get(identifier, 0.0) + estimate - pending = command_args.confirmation_request( + # Only the axes with work planned, so the offer names what the estimate + # actually buys rather than the pair it usually covers. + grain_planned = bool( + plan.key_checks or plan.fanout_pairs or plan.composite_checks + ) + offered_axes = [ + axis + for axis, planned in ( + ("grain", grain_planned), + ("semantic_cardinality", bool(checks)), + ) + if planned + ] + offer = command_args.confirmation_request( "maintain check", adapter, grain_total + card_total, per_table=per_table, + axes=offered_axes, notes=[ - "the schema, volume, and semantic reference checks are free " - "and already complete (their findings are included in this " - "envelope); the estimate covers the grain and " - "dimension-cardinality scans" + "the schema, volume, and semantic reference findings in this " + "envelope are final; the estimate buys the grain and " + "dimension-cardinality scans on top of them" ], ) - if pending is not None: + # Identical warnings on both paths: what bounds the settled answer bounds the + # free one too, and the unconfirmed call is the one a session opens with. + warnings = warnings + _baseline_warnings( + store, snap, engine.config.profile_freshness_hours + ) + + if offer is not None: drift_mod.annotate_impacts(schema_findings + volume_findings, snap) by_axis = { "schema": drift_mod.rank_findings(schema_findings), @@ -641,7 +666,7 @@ def check(engine: DexEngine, objects: list[str] | None = None) -> DriftResult: store, snap, connector, {a: (f, scope_names) for a, f in by_axis.items()} ) result = _drift_result(by_axis, snap, store, warnings=warnings) - result.pending_confirmation = pending + result.pending_offer = offer return result grain_findings = drift_mod.grain_drift( @@ -662,13 +687,7 @@ def check(engine: DexEngine, objects: list[str] | None = None) -> DriftResult: _record_axes( store, snap, connector, {a: (f, scope_names) for a, f in by_axis.items()} ) - result = _drift_result( - by_axis, - snap, - store, - warnings=warnings - + _baseline_warnings(store, snap, engine.config.profile_freshness_hours), - ) + result = _drift_result(by_axis, snap, store, warnings=warnings) return command_args.stamp_spend(result, adapter) From 5282e086c2760bcd6e91d0fc5df818389e824608 Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:29:45 +0200 Subject: [PATCH 4/8] Add support for per-definition semantic payloads and classification of unchanged definitions in semantic plans --- packages/dex-core/src/exmergo_dex_core/cli.py | 3 + .../exmergo_dex_core/transform/commands.py | 115 ++++- .../src/exmergo_dex_core/transform/results.py | 8 +- .../exmergo_dex_core/transform/semantic.py | 431 +++++++++++++++++- 4 files changed, 528 insertions(+), 29 deletions(-) diff --git a/packages/dex-core/src/exmergo_dex_core/cli.py b/packages/dex-core/src/exmergo_dex_core/cli.py index 114846a..6493f95 100644 --- a/packages/dex-core/src/exmergo_dex_core/cli.py +++ b/packages/dex-core/src/exmergo_dex_core/cli.py @@ -261,6 +261,9 @@ def _build_parser() -> argparse.ArgumentParser: if group == "semantic": sp.add_argument("argument", nargs="?", default=None) sp.add_argument("--edits-file", default=None) + # The per-definition payload: name only what changes, and the + # engine writes it into the file that holds it. + sp.add_argument("--definitions-file", default=None) sp.add_argument("--no-parse", action="store_true", default=False) # maintain detectors take an optional object scope (default: whole # project); reconcile takes an optional drift class to fix. diff --git a/packages/dex-core/src/exmergo_dex_core/transform/commands.py b/packages/dex-core/src/exmergo_dex_core/transform/commands.py index b80cdec..c5833b1 100644 --- a/packages/dex-core/src/exmergo_dex_core/transform/commands.py +++ b/packages/dex-core/src/exmergo_dex_core/transform/commands.py @@ -45,6 +45,7 @@ PlanListResult, PlanResult, ) +from .validate import EditValidationError if TYPE_CHECKING: from ..engine import DexEngine @@ -589,7 +590,12 @@ def cmd_deps(args: argparse.Namespace, engine: DexEngine) -> env.Envelope: def semantic_define( - engine: DexEngine, intent: str, edits: list[PlanEdit], *, no_parse: bool = False + engine: DexEngine, + intent: str, + edits: list[PlanEdit], + *, + definitions: list[semantic_mod.DefinitionEdit] | None = None, + no_parse: bool = False, ) -> PlanResult: """Author new semantic definitions (entities, dimensions, measures, metrics). @@ -597,11 +603,18 @@ def semantic_define( caught rather than applied; use :func:`semantic_update` to evolve one. """ - return _semantic_plan(engine, intent, edits, mode="define", no_parse=no_parse) + return _semantic_plan( + engine, intent, edits, mode="define", definitions=definitions, no_parse=no_parse + ) def semantic_update( - engine: DexEngine, intent: str, edits: list[PlanEdit], *, no_parse: bool = False + engine: DexEngine, + intent: str, + edits: list[PlanEdit], + *, + definitions: list[semantic_mod.DefinitionEdit] | None = None, + no_parse: bool = False, ) -> PlanResult: """Evolve existing semantic definitions. @@ -609,17 +622,27 @@ def semantic_update( already have, so a typo does not silently create a second definition. """ - return _semantic_plan(engine, intent, edits, mode="update", no_parse=no_parse) + return _semantic_plan( + engine, intent, edits, mode="update", definitions=definitions, no_parse=no_parse + ) def semantic_plan( - engine: DexEngine, intent: str, edits: list[PlanEdit], *, no_parse: bool = False + engine: DexEngine, + intent: str, + edits: list[PlanEdit], + *, + definitions: list[semantic_mod.DefinitionEdit] | None = None, + no_parse: bool = False, ) -> PlanResult: """Mixed-intent semantic authoring: one payload may evolve existing definitions and add the new ones they depend on; each name is classified - as defined or updated instead of the whole payload being refused.""" + as defined, updated, or unchanged instead of the whole payload being + refused.""" - return _semantic_plan(engine, intent, edits, mode="plan", no_parse=no_parse) + return _semantic_plan( + engine, intent, edits, mode="plan", definitions=definitions, no_parse=no_parse + ) # Mode to the public function that owns it. The shims dispatch through this @@ -654,8 +677,13 @@ def _semantic_envelope( _edits_from_payload( getattr(args, "edits_file", None), default_kind=EditKind.SEMANTIC_YML ), + definitions=_definitions_from_payload( + getattr(args, "definitions_file", None) + ), no_parse=bool(getattr(args, "no_parse", False)), ) + except EditValidationError as exc: + return env.error_for(exc) except DbtParseError as exc: return env.error_for(exc, warnings=exc.warnings) except ValueError as exc: @@ -913,12 +941,38 @@ def _semantic_plan( edits: list[PlanEdit], *, mode: str, + definitions: list[semantic_mod.DefinitionEdit] | None = None, no_parse: bool = False, ) -> PlanResult: + from ..dbt_project import load as load_project + + project = engine.project_dir() + view = load_project(project) + + if definitions and edits: + raise ValueError( + f"semantic {mode} takes one payload: whole files (--edits-file) or " + "definitions (--definitions-file), not both" + ) + # What the caller named, when they named definitions rather than files. The + # classification is narrowed to it: a spliced file carries every definition + # it already held, and those were not part of this change. + scope: set[semantic_mod.DefinitionKey] | None = None + if definitions: + # Lowered to the whole-file unit before anything else looks at them, so + # the validation, classification, parse gate, and plan store below stay + # on one code path regardless of how the caller expressed the change. + edits = [ + PlanEdit(path=path, kind=EditKind.SEMANTIC_YML, new_content=content) + for path, content in semantic_mod.splice_definitions(definitions, view) + ] + scope = {(d.kind, d.name) for d in definitions} + if not edits: raise ValueError( - f"semantic {mode} needs content: pass --edits-file with the " - "authored dbt semantic YAML" + f"semantic {mode} needs content: pass --edits-file with whole " + "semantic YAML files, or --definitions-file with the " + "individual definitions to write" ) non_semantic = [e.path for e in edits if e.kind is not EditKind.SEMANTIC_YML] if non_semantic: @@ -933,12 +987,9 @@ def _semantic_plan( "semantic YAML with `transform plan` instead, for: " + ", ".join(deletions) ) - from ..dbt_project import load as load_project - - project = engine.project_dir() - view = load_project(project) - parsed_edits = [yaml.safe_load(e.new_content) for e in edits] - classification = semantic_mod.check_mode(mode, parsed_edits, view) + parsed_by_path = [(e.path, yaml.safe_load(e.new_content)) for e in edits] + parsed_edits = [parsed for _path, parsed in parsed_by_path] + classification = semantic_mod.check_mode(mode, parsed_by_path, view, scope=scope) semantic_mod.check_references(parsed_edits, view) spine_warning = semantic_mod.time_spine_warning(view, parsed_edits) @@ -973,6 +1024,16 @@ def _semantic_plan( result = _make_plan(engine, intent, edits) result.defined = classification["defined"] result.updated = classification["updated"] + result.unchanged = classification["unchanged"] + # `plans.plan` emits one diff per edit whether or not the content moved, so + # a no-op is an all-empty diff set rather than an absent one. + if result.unchanged and all( + d["additions"] == 0 and d["deletions"] == 0 for d in result.diffs + ): + result.warnings.append( + "every definition in this payload is identical to the project's " + "current content, so this plan changes nothing" + ) if spine_warning: result.warnings.append(spine_warning) if parse_warning: @@ -981,6 +1042,30 @@ def _semantic_plan( return result +def _definitions_from_payload( + definitions_file: str | None, +) -> list[semantic_mod.DefinitionEdit]: + """Read the per-definition payload (a file path, or ``-`` for stdin). + + Shape: ``{"definitions": [{"kind": ..., "path": ..., "content": ...}, ...]}``. + ``kind`` is ``semantic_model`` or ``metric``; ``content`` is that one + definition's YAML body; ``path`` may be omitted for a definition the project + already declares, which is then rewritten where it already lives. + """ + + if definitions_file is None: + return [] + raw = sys.stdin.read() if definitions_file == "-" else _read_file(definitions_file) + try: + payload = json.loads(raw) + except json.JSONDecodeError as exc: + raise ValueError(f"definitions payload is not valid JSON: {exc}") from exc + entries = payload.get("definitions") if isinstance(payload, dict) else None + if not isinstance(entries, list): + raise ValueError('definitions payload must be {"definitions": [...]}') + return semantic_mod.parse_definition_payload(entries) + + def _edits_from_payload( edits_file: str | None, default_kind: EditKind | None = None ) -> list[PlanEdit]: diff --git a/packages/dex-core/src/exmergo_dex_core/transform/results.py b/packages/dex-core/src/exmergo_dex_core/transform/results.py index 95a686b..acb13db 100644 --- a/packages/dex-core/src/exmergo_dex_core/transform/results.py +++ b/packages/dex-core/src/exmergo_dex_core/transform/results.py @@ -46,9 +46,13 @@ class PlanResult(Result): paths: list[str] = Field(default_factory=list) plan_path: str = "" # Semantic authoring classifies each name it touched, so a mixed payload - # reports which definitions it created and which it evolved. + # reports which definitions it created, which it evolved, and which it + # merely re-stated. The third class is what keeps the first two readable: + # a whole-file edit restates every definition in the file, and without it + # a two-object change reports as a thirty-object one. defined: list[str] | None = None updated: list[str] | None = None + unchanged: list[str] | None = None def data(self) -> dict[str, Any]: payload: dict[str, Any] = { @@ -62,6 +66,8 @@ def data(self) -> dict[str, Any]: payload["defined"] = self.defined if self.updated is not None: payload["updated"] = self.updated + if self.unchanged is not None: + payload["unchanged"] = self.unchanged return payload diff --git a/packages/dex-core/src/exmergo_dex_core/transform/semantic.py b/packages/dex-core/src/exmergo_dex_core/transform/semantic.py index 35e9ac8..0c4f311 100644 --- a/packages/dex-core/src/exmergo_dex_core/transform/semantic.py +++ b/packages/dex-core/src/exmergo_dex_core/transform/semantic.py @@ -5,11 +5,22 @@ the agent authored; MetricFlow's own schemas (via ``dbt-semantic-interfaces``, pulled in by dbt-duckdb) are the validator of record, with a structural fallback when that package is absent so validation degrades to a warning, never to silence. + +A payload arrives in one of two units. A **whole file** is the original: the +caller sends the complete new content for a path. A **definition** names one +semantic model or metric and nothing else; :func:`splice_definitions` resolves a +set of those into whole-file content, so everything downstream (the plan store, +the diffs, the conflict hashing, ``transform apply``) sees the unit it always +has. The definition unit exists because the whole-file one forces a caller +adding two metrics to restate the twenty-seven it is not touching, which buries +the real change in the review and puts twenty-seven hand-copied definitions at +risk of a typo that only ``dbt parse`` would catch. """ from __future__ import annotations -from collections.abc import Iterable +import re +from collections.abc import Iterable, Sequence from typing import Any, NamedTuple import yaml @@ -120,6 +131,46 @@ def existing_semantic_names(view: DbtProjectView) -> set[str]: return ns.semantic_models | ns.metrics +# A definition is addressed by role and name, never by name alone. The two +# namespaces are merged for collision checks (dbt resolves a reference against +# both), but a metric and a semantic model that happen to share a name are +# different objects, and comparing one against the other would report a change +# that is really a category error. +DefinitionKey = tuple[str, str] + + +def project_definitions( + view: DbtProjectView, +) -> dict[DefinitionKey, tuple[str, dict[str, Any]]]: + """Every top-level definition the project declares, keyed by role and name. + + The value is the file that declares it and its parsed body, which is what + makes "did this edit actually change anything" answerable. Measures are not + included: they are addressed inside their semantic model, so the model's own + comparison already covers them. + + A duplicate name across two files is the project's own bug and dbt will + refuse it; whichever file is read last wins here, because there is no right + answer and this function is not the place to raise about it. + """ + + definitions: dict[DefinitionKey, tuple[str, dict[str, Any]]] = {} + for source in view.files.values(): + if not source.path.endswith((".yml", ".yaml")): + continue + try: + parsed = yaml.safe_load(source.content) + except yaml.YAMLError: + continue # a broken hand-written file is not this command's problem + if not isinstance(parsed, dict): + continue + for kind, key in (("semantic_model", "semantic_models"), ("metric", "metrics")): + for entry in parsed.get(key) or []: + if isinstance(entry, dict) and entry.get("name"): + definitions[(kind, entry["name"])] = (source.path, entry) + return definitions + + def time_spine_warning( view: DbtProjectView, parsed_edits: list[dict[str, Any]] ) -> str | None: @@ -159,24 +210,70 @@ def declares_spine(parsed: Any) -> bool: def check_mode( - mode: str, parsed_edits: list[dict[str, Any]], view: DbtProjectView + mode: str, + parsed_edits: Sequence[tuple[str, dict[str, Any]]], + view: DbtProjectView, + *, + scope: set[DefinitionKey] | None = None, ) -> dict[str, list[str]]: - """Classify every proposed name as new or existing; enforce the strict modes. + """Classify every proposed name; enforce the strict modes. ``define`` refuses existing names and ``update`` refuses new ones (both are typo guards); ``plan`` accepts a mix, so one payload can evolve existing - definitions and add the helpers they depend on. Returns the classification: - ``{"defined": [new names], "updated": [existing names]}``. + definitions and add the helpers they depend on. + + Three classes, not two: ``defined`` is new to the project, ``updated`` + genuinely differs from what is on disk, and ``unchanged`` is re-stated + content identical to the current definition in the same file. The third + class exists because the whole-file edit unit makes re-stating the untouched + definitions the normal way to add one, so name membership alone reports a + two-object change as a thirty-object one and buries the real blast radius in + the place a reviewer looks to confirm it. + + Equality is over the parsed structure, so key order and formatting are not + changes but list order is: a reordered ``dimensions:`` block is a real diff + and reads as one. The same-file requirement is deliberate. Identical content + landing in a different file is a move, the diff for it is real, and calling + that unchanged would be the same lie pointing the other way. + + ``parsed_edits`` pairs each edit's project-relative path with its parsed + document, because a definition's identity here includes where it lives. + + ``scope`` narrows the classification to definitions the caller actually + named. A per-definition payload is lowered to whole-file edits before it + gets here, so without it the file's other definitions would be classified + too, and a two-definition payload would report the file's other twenty-five + as ``unchanged``: a quieter version of the noise this class exists to remove. """ existing = existing_semantic_names(view) - proposed = { - entry["name"] - for parsed in parsed_edits - for entry in (parsed.get("semantic_models") or []) - + (parsed.get("metrics") or []) - if isinstance(entry, dict) and entry.get("name") - } + on_disk = project_definitions(view) + + proposed: set[str] = set() + # Tracked per name rather than per role, because the envelope classifies + # names: one name can be proposed as both a semantic model and a metric, and + # a change to either half is a change to the name. + matched: set[str] = set() + differed: set[str] = set() + for path, parsed in parsed_edits: + if not isinstance(parsed, dict): + continue + for kind, key in (("semantic_model", "semantic_models"), ("metric", "metrics")): + for entry in parsed.get(key) or []: + if not isinstance(entry, dict) or not entry.get("name"): + continue + name = entry["name"] + if scope is not None and (kind, name) not in scope: + continue + proposed.add(name) + current = on_disk.get((kind, name)) + if current is None: + # No counterpart in this role. Either the name is new (it + # lands in `defined`) or it exists only in the other role, + # which makes this entry an addition, so nothing to match. + continue + (matched if current == (path, entry) else differed).add(name) + if mode == "define": clashes = sorted(proposed & existing) if clashes: @@ -193,9 +290,15 @@ def check_mode( "`semantic define` to add a new definition, or " "`semantic plan` to mix new and existing names" ) + + # Unchanged means every proposed entry for the name matched. One half of a + # dual-role name differing makes the name updated, so `differed` subtracts. + evolving = proposed & existing + unchanged = (matched - differed) & evolving return { "defined": sorted(proposed - existing), - "updated": sorted(proposed & existing), + "updated": sorted(evolving - unchanged), + "unchanged": sorted(unchanged), } @@ -314,3 +417,305 @@ def _structural_check( raise EditValidationError( f"{path}: each metric needs at least name, type, and type_params" ) + + +# --- the definition edit unit ------------------------------------------------- + +_TOP_LEVEL_KEY = {"semantic_model": "semantic_models", "metric": "metrics"} + + +class DefinitionEdit(NamedTuple): + """One semantic model or metric, authored on its own. + + ``content`` is the entry's body as a YAML mapping, not a list item: the + caller writes ``name: revenue`` at column zero and the splice indents it to + match its siblings. ``path`` may be ``None`` for a definition the project + already declares, in which case it is rewritten where it already lives. + """ + + kind: str + name: str + content: str + parsed: dict[str, Any] + path: str | None = None + + +def parse_definition_payload(entries: Iterable[Any]) -> list[DefinitionEdit]: + """Read the ``definitions`` payload into typed edits. + + The name is read from the content rather than declared beside it, so the + two cannot disagree. + """ + + parsed_entries: list[DefinitionEdit] = [] + for index, entry in enumerate(entries): + where = f"definitions[{index}]" + if not isinstance(entry, dict): + raise EditValidationError(f"{where}: each definition must be an object") + kind = entry.get("kind") + if kind not in _TOP_LEVEL_KEY: + raise EditValidationError( + f"{where}: kind must be one of {', '.join(sorted(_TOP_LEVEL_KEY))}, " + f"got {kind!r}" + ) + content = entry.get("content") + if not isinstance(content, str) or not content.strip(): + raise EditValidationError(f"{where}: needs YAML content") + try: + body = yaml.safe_load(content) + except yaml.YAMLError as exc: + raise EditValidationError(f"{where}: invalid YAML: {exc}") from exc + if not isinstance(body, dict) or not body.get("name"): + raise EditValidationError( + f"{where}: content must be a YAML mapping with a name; write the " + "definition's body alone, without the leading '- '" + ) + path = entry.get("path") + if path is not None and not isinstance(path, str): + raise EditValidationError(f"{where}: path must be a string") + parsed_entries.append(DefinitionEdit(kind, body["name"], content, body, path)) + return parsed_entries + + +def splice_definitions( + definitions: Sequence[DefinitionEdit], view: DbtProjectView +) -> list[tuple[str, str]]: + """Lower per-definition edits into whole-file content, one entry per path. + + Every byte outside the definitions being written is preserved, which is the + whole point: a YAML round trip through ``safe_dump`` would reformat the file + and strip the comments a hand-written semantic layer depends on, producing a + larger diff than the whole-file payload this unit replaces. + + Refuses rather than guesses. A file whose structure the line scanner cannot + span safely is reported with ``--edits-file`` named as the way to edit it, + and the spliced result is re-parsed and compared against the incoming body + so a splice that landed in the wrong place cannot reach the plan store. + """ + + on_disk = project_definitions(view) + targets: list[tuple[str, DefinitionEdit]] = [] + for definition in definitions: + current = on_disk.get((definition.kind, definition.name)) + if definition.path is None: + if current is None: + raise EditValidationError( + f"{definition.kind} '{definition.name}' is not in the project, " + "so there is no file to rewrite: give it a path" + ) + targets.append((current[0], definition)) + continue + if current is not None and current[0] != definition.path: + raise EditValidationError( + f"{definition.kind} '{definition.name}' is declared in " + f"'{current[0]}' but this edit targets '{definition.path}'. " + "Writing it to both would duplicate the name; move a definition " + "with whole-file edits (--edits-file) so the removal and the " + "addition land in one plan" + ) + targets.append((definition.path, definition)) + + ordered_paths: list[str] = [] + for path, _definition in targets: + if path not in ordered_paths: + ordered_paths.append(path) + + spliced: list[tuple[str, str]] = [] + for path in ordered_paths: + source = view.files.get(path) + text = source.content if source is not None else "" + applied = [d for target_path, d in targets if target_path == path] + for definition in applied: + text = _splice_entry(path, text, definition) + _verify_splice(path, text, applied) + spliced.append((path, text)) + return spliced + + +# Constructs the line scanner cannot span without risking a wrong edit. Each +# refusal costs the caller a whole-file edit, which still works, so failing +# closed here is cheap; a mis-sliced definition would not be. +def _reject_unspannable(path: str, text: str) -> None: + if "\t" in text: + raise EditValidationError( + f"{path}: the file indents with tabs, which YAML does not allow for " + "structure and this editor will not guess at; use --edits-file" + ) + if re.search(r"^---\s*$", text[1:], re.MULTILINE) or re.search( + r"^\.\.\.\s*$", text, re.MULTILINE + ): + raise EditValidationError( + f"{path}: the file holds more than one YAML document; a definition " + "edit cannot tell which one to write into, use --edits-file" + ) + if re.search(r"(^|[\s:\[{,])[&*][A-Za-z0-9_][^\s]*", text): + raise EditValidationError( + f"{path}: the file uses YAML anchors or aliases, whose expansion the " + "definition editor does not track; use --edits-file" + ) + + +class _Block(NamedTuple): + """Where a top-level sequence lives in the file, by line index.""" + + start: int # first line after the `key:` line + end: int # one past the block's last line + indent: str # the indent its `- ` items carry + + +def _find_block(path: str, lines: list[str], key: str) -> _Block | None: + key_line = None + for index, line in enumerate(lines): + if re.match(rf"^{key}:\s*$", line): + key_line = index + break + if re.match(rf"^{key}:\s*\[", line): + raise EditValidationError( + f"{path}: '{key}' is written as an inline (flow) sequence, which " + "this editor will not rewrite; use --edits-file" + ) + if key_line is None: + return None + + end = len(lines) + for index in range(key_line + 1, len(lines)): + line = lines[index] + if line.strip() and not line[:1].isspace(): + end = index + break + + indent = None + for index in range(key_line + 1, end): + match = re.match(r"^(\s+)-\s", lines[index]) + if match: + indent = match.group(1) + break + return _Block(key_line + 1, end, indent if indent is not None else " ") + + +def _entry_spans(lines: list[str], block: _Block) -> list[tuple[str | None, int, int]]: + """Each item in the block as ``(name, start, end)`` line indices. + + An entry ends at its last line of content, so the blank lines and comments + that separate it from the next item stay where the author put them: those + usually head the *following* definition, and moving them would relocate a + section banner every time a neighbour is edited. + """ + + starts = [ + index + for index in range(block.start, block.end) + if lines[index].startswith(f"{block.indent}- ") + ] + spans: list[tuple[str | None, int, int]] = [] + for position, start in enumerate(starts): + limit = starts[position + 1] if position + 1 < len(starts) else block.end + end = start + for index in range(start, limit): + line = lines[index] + if not line.strip(): + continue + if line.lstrip().startswith("#") and not _inside_entry(line, block.indent): + # A comment at the item's own indent is a banner for what comes + # next (`# ---- metrics ----`), so it stays put when the entry + # above it is rewritten. One indented deeper annotates a field of + # this entry, and leaving it behind would strand a note about a + # line that no longer exists. + continue + end = index + 1 + item = "".join(lines[start:end]) + try: + parsed = yaml.safe_load(item) + except yaml.YAMLError: + parsed = None + name = None + if isinstance(parsed, list) and parsed and isinstance(parsed[0], dict): + name = parsed[0].get("name") + spans.append((name, start, end)) + return spans + + +def _inside_entry(line: str, item_indent: str) -> bool: + """Is this comment line part of the entry above it, or a banner below it? + + Depth decides. A comment written at the item's own indent sits between two + definitions and belongs to neither; anything deeper is inside the mapping. + """ + + return len(line) - len(line.lstrip()) > len(item_indent) + + +def _as_item(content: str, indent: str) -> str: + body = content.rstrip("\n").split("\n") + rendered = [f"{indent}- {body[0].rstrip()}\n"] + rendered += [ + f"{indent} {line.rstrip()}\n" if line.strip() else "\n" for line in body[1:] + ] + return "".join(rendered) + + +def _splice_entry(path: str, text: str, definition: DefinitionEdit) -> str: + _reject_unspannable(path, text) + key = _TOP_LEVEL_KEY[definition.kind] + lines = text.splitlines(keepends=True) + block = _find_block(path, lines, key) + + if block is None: + # No block for this kind yet. Append one, keeping a blank line between + # it and whatever the file already holds. + prefix = text if text.endswith("\n") or not text else text + "\n" + if not prefix: + prefix = "version: 2\n" + separator = "" if prefix.endswith("\n\n") else "\n" + return f"{prefix}{separator}{key}:\n{_as_item(definition.content, ' ')}" + + item = _as_item(definition.content, block.indent) + for name, start, end in _entry_spans(lines, block): + if name == definition.name: + return "".join(lines[:start]) + item + "".join(lines[end:]) + + # New definition in an existing block: after the last item's content, so it + # lands inside the block rather than after any trailing comment. + spans = _entry_spans(lines, block) + insert_at = spans[-1][2] if spans else block.start + tail = "".join(lines[insert_at:]) + head = "".join(lines[:insert_at]) + # A file that does not end in a newline would otherwise fuse with the item. + if head and not head.endswith("\n"): + head += "\n" + return f"{head}\n{item}{tail}" + + +def _verify_splice(path: str, text: str, definitions: Sequence[DefinitionEdit]) -> None: + """The result must parse, and each definition must be exactly what was sent. + + Cheap insurance against the line scanner landing a definition in the wrong + item or the wrong block: the comparison is against the caller's own parsed + body, so a splice that reads as valid YAML but says something else cannot be + stored. + """ + + try: + parsed = yaml.safe_load(text) + except yaml.YAMLError as exc: + raise EditValidationError( + f"{path}: writing these definitions produced invalid YAML ({exc}); " + "use --edits-file for this file" + ) from exc + if not isinstance(parsed, dict): + raise EditValidationError( + f"{path}: writing these definitions produced a document that is not a " + "mapping; use --edits-file for this file" + ) + for definition in definitions: + key = _TOP_LEVEL_KEY[definition.kind] + landed = [ + entry + for entry in parsed.get(key) or [] + if isinstance(entry, dict) and entry.get("name") == definition.name + ] + if len(landed) != 1 or landed[0] != definition.parsed: + raise EditValidationError( + f"{path}: {definition.kind} '{definition.name}' did not write " + "cleanly into this file's layout; use --edits-file for it" + ) From 18a85c87e6db8c8a2f8259d73ea1b1b5b6173f8f Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:30:12 +0200 Subject: [PATCH 5/8] Update agent-facing documentation --- AGENTS.md | 35 ++++++++++++++++++---- references/command-contract.md | 54 ++++++++++++++++++++++++++++++++-- skills/maintain/SKILL.md | 32 +++++++++++++------- skills/transform/SKILL.md | 32 +++++++++++++++----- 4 files changed, 127 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c0edff5..09785c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,13 +62,13 @@ project's real config instead of a wrong default. | `transform macro [name]` | no name lists the shipped dbt macros; a name proposes scaffolding it into the project's macro directory as a plan (dbt-parse-checked, applied with `transform apply`); re-running diffs the project's copy against the shipped version | | `transform build --target dev` | prod-looking targets refused outright; then a free dev-target preflight (refuses when `.dex/config.yml` and the rendered `profiles.yml` disagree, or when the dev database does not exist, naming the fix); then the cost preflight, priced upfront by a free `dbt compile` dry-run of each node (a partial floor when a cold dev target has not built a node's inputs yet; degrades to no estimate when dex cannot open its own connection); runs only with `--confirm` and a budget; cwd pinned to the project dir; auto-runs `dbt deps` when packages are declared but not installed | | `transform deps` | install/refresh dbt packages (repo-confined; no warehouse spend) | -| `semantic define\|update\|plan ... --edits-file ` | dbt semantic model edits as diffs; validated up to and including dbt's own parser (a throwaway project copy) before the plan is stored; `plan` accepts a mix and classifies per name; degrades to a warning when dbt is absent, `--no-parse` skips; applied with `transform apply` like any other plan | +| `semantic define\|update\|plan ... --edits-file \|--definitions-file ` | dbt semantic model edits as diffs; validated up to and including dbt's own parser (a throwaway project copy) before the plan is stored; `plan` accepts a mix and classifies each name `defined`, `updated`, or `unchanged`; `--definitions-file` names one definition at a time instead of a whole file; degrades to a warning when dbt is absent, `--no-parse` skips; applied with `transform apply` like any other plan | | `maintain snapshot` | capture/refresh the known-good baseline in `.dex/snapshot.json` (pins the `.dex/` map + per-layer definition fingerprints) | -| `maintain check` | sweep every drift axis vs the snapshot; ranked drift report (read-only); two-phase on billed connectors (free axes now, one estimate for the scanning axes) | +| `maintain check` | sweep every drift axis vs the snapshot; ranked drift report (read-only); two-phase on billed connectors: the free axes complete and return `ok`, with one estimate for the scanning axes under `data.offer` | | `maintain schema []` | structural drift: columns/tables added, dropped, retyped, renamed; nullability; dangling sources (free) | | `maintain volume []` | freshness drift: row counts that collapsed, emptied, or spiked (free metadata) | | `maintain grain []` | cardinality/identity drift: lost key uniqueness, changed grain, join fanout (scans; gated on billed connectors) | -| `maintain semantic []` | definition drift and dangling refs (free) plus categorical dimension cardinality change (scans; gated on billed connectors) | +| `maintain semantic []` | definition drift and dangling refs (free, and returned as `ok`) plus categorical dimension cardinality change (scans; offered under `data.offer` and gated on billed connectors) | | `maintain reconcile []` | propose the dbt edits that reconcile detected drift, as a stored plan of diffs tagged mechanical or advisory (never applied; apply with `transform apply `) | | `viz preview` | emit the dbt semantic model to the Viz preview (not yet implemented) | @@ -82,6 +82,13 @@ is not free: `schema`, `volume`, and the reference half of `semantic` are metada scan and go through the `--confirm --budget` handshake on billed connectors. The engine does not care which skill fronts a subcommand. +A command whose free half completed reports `ok` and puts the price of the +scanning half in `data.offer`, rather than gating the whole answer behind a +confirmation. `needs_confirmation` means dex is waiting on you for work you asked +for; an offer is work you did not ask for, and ignoring it is a valid choice. +Read `data.axes_run` for what completed and `data.offer.axes` for what the +estimate would add, since with an `ok` status those are no longer implied. + Authored content reaches the engine through `--edits-file ` (or `-` for stdin): a JSON payload of `{"edits": [{"path", "kind", "op", "content"}, ...]}` with `kind` one of `model_sql`, `schema_yml`, `semantic_yml`, `packages_yml` (the @@ -89,7 +96,15 @@ guarded way to author the project-root `packages.yml`/`dependencies.yml`, so declaring a dbt package is a reviewable diff too), `macro_sql`, `project_yml` (the project-root `dbt_project.yml`), or `profiles_yml` (the project-root `profiles.yml`, secret-guarded so a credential never enters the diff: reference -secrets via `{{ env_var('NAME') }}`). `op` is `upsert` (create or update, the +secrets via `{{ env_var('NAME') }}`). The semantic commands take a second, +narrower payload instead: `--definitions-file ` with +`{"definitions": [{"kind", "path", "content"}, ...]}`, where `kind` is +`semantic_model` or `metric` and `content` is that one definition's YAML body. +The name is read from the content, and `path` may be omitted for a definition +the project already declares, in which case it is rewritten where it lives. Use +it whenever a change touches part of a shared file: the engine writes each +definition in place and leaves every other byte, including comments, untouched, +so the diff and the classification both describe only what changed. `op` is `upsert` (create or update, the default, carrying `content`) or `delete` (remove the file, no `content`); a delete is a reviewable diff too, guarded so the plan is refused if any surviving file still `ref()`s a deleted model, and a rename is one plan (delete old, create @@ -110,7 +125,17 @@ would spend requires an explicit `--confirm` and a session budget: on a metered connector (BigQuery, Snowflake, Databricks, Redshift, and Postgres) the first call returns `needs_confirmation` with a free estimate, and the same command is re-issued with `--confirm --budget ` once the user -has agreed to the spend. The magnitude is paradigm-relative: **bytes** on +has agreed to the spend. + +One exception to the status, not to the rule: a command that finished free work +the caller did want, and can offer paid work they did not ask for, returns `ok` +with the estimate under `data.offer` instead. That is `maintain check` and +`maintain semantic`. The re-issue is identical (`--confirm --budget`), nothing +runs until it arrives, and `cost.estimate` stays empty so an `ok` never reads as +though it spent. Reserve `needs_confirmation` for reading "dex is waiting on +me", and an offer for "there is more available if I want it". + +The magnitude is paradigm-relative: **bytes** on BigQuery (an exact free dry-run figure), **warehouse-seconds** on Snowflake (a heuristic labeled `estimate_quality: "heuristic"`, with a credit translation alongside) and on Databricks (a floor labeled diff --git a/references/command-contract.md b/references/command-contract.md index 3f98240..f588cf8 100644 --- a/references/command-contract.md +++ b/references/command-contract.md @@ -198,8 +198,32 @@ replace) inlines a literal credential, so no secret ever reaches the diff. with a pointer to the full log when anything was trimmed. - `semantic define` refuses names that already exist in the project (use `update`); `update` refuses names that do not (use `define`); `semantic plan` - accepts a mix and classifies per name, reporting `defined` and `updated` in - the envelope. Names implicitly created by `create_metric: true` measures count + accepts a mix and classifies per name, reporting `defined`, `updated`, and + `unchanged` in the envelope. `updated` means the definition's parsed content + actually differs from the project's; a definition re-stated identically in the + file that already holds it is `unchanged`. The distinction matters because a + whole-file payload restates every definition in the file, so without it a + two-metric change reports thirty objects as updated and the real blast radius + is invisible in the one place a reviewer checks it. Key order and formatting + are not changes; list order is, and identical content written to a different + file is a move, so both read as `updated`. +- **Two payload units.** `--edits-file` carries whole files. `--definitions-file` + carries individual definitions: + `{"definitions": [{"kind", "path", "content"}, ...]}`, `kind` being + `semantic_model` or `metric`, `content` that definition's YAML body (a mapping, + written without the leading `- `). The name is read from the content, so the + two cannot disagree. `path` is required for a name the project does not have + and optional for one it does, defaulting to the file that declares it; an + explicit path that would relocate an existing definition is refused, because + writing it to a second file duplicates the name. The engine writes each + definition in place and preserves every other byte, comments included, then + re-parses the result and compares it against what was sent. A file whose + layout it cannot span safely (a flow-style sequence, anchors or aliases, + multiple documents, tab indentation) is refused with `--edits-file` named as + the way to edit it. Classification is scoped to the definitions named, so a + spliced file's other definitions are reported in no class at all. Removing a + definition is still a whole-file edit; the semantic verbs author and do not + delete. Names implicitly created by `create_metric: true` measures count as existing metrics everywhere. Beyond MetricFlow's schemas, the engine resolves every metric input (ratio and derived inputs must reference metrics, not measures) and then runs the emitted YAML through dbt's own parser against @@ -221,6 +245,19 @@ the warehouse and take the `--confirm --budget` handshake on billed connectors; `check` runs the free axes first and returns one combined estimate for the scanning axes. +That estimate arrives as an **offer on a complete answer**, not as a pending +charge. `check` and `semantic` finish their free axes on every call and return +`ok`, with the price of the scanning axes under `data.offer` (`estimated_bytes` +or the connector's own estimate shape, `per_table_bytes`, `axes` naming what the +estimate would add, and the `--confirm --budget` hint). `data.axes_run` names +what completed, and reading both is how a caller tells "grain found nothing" +from "grain did not run", which the status used to imply. Nothing scans until +the confirmed re-issue arrives, and `cost.estimate` stays unset on the offer so +an `ok` never carries a number that reads as spend. The reason for the split is +that `needs_confirmation` is a request for a decision dex is blocked on, and +spending it on work the caller never asked for teaches them to confirm +reflexively, which is the one habit the handshake cannot survive. + **A baseline reports its own coverage, and the axes only compare what it covers.** `maintain snapshot` pins the exploration cache, and a cache is thin whenever `explore map` stopped at its rank cutoff: past 50 objects it profiles @@ -367,7 +404,18 @@ Rules the envelope enforces, all of them Tier-2 eval targets: spend returns `needs_confirmation` unless given `--confirm` (and a `--budget` on billed connectors; DuckDB is free, so the confirm handshake alone gates it). An estimate over the ceiling is refused outright; confirmation cannot override - it. On billed connectors the estimate comes from free dry-runs, the confirmed + it. +- **A priced phase the caller did not request is an offer, not a refusal.** When + a command's free half is a complete answer in its own right, the envelope is + `ok` and the price of the optional half sits in `data.offer`, carrying the + same estimate, breakdown, and `--confirm --budget` hint a refusal would. + `maintain check` and `maintain semantic` are the two. The spend gate is + unchanged (nothing runs without the confirmed re-issue) and `cost.estimate` + stays unset, so a reader can keep treating a populated `cost.estimate` on an + `ok` as settled preflight for work that ran. `needs_confirmation` stays + reserved for work the caller asked for and has not authorized, which is the + only case where dex is genuinely blocked. + On billed connectors the estimate comes from free dry-runs, the confirmed run re-checks every statement against the budget with a server-side cap as backstop, actual spend is reported under `data.spend`, and every billed byte is appended to the `.dex/spend.jsonl` ledger, against which the optional diff --git a/skills/maintain/SKILL.md b/skills/maintain/SKILL.md index c32cd76..ec26a67 100644 --- a/skills/maintain/SKILL.md +++ b/skills/maintain/SKILL.md @@ -78,17 +78,27 @@ connector (BigQuery, Snowflake, Databricks, Postgres, Redshift). The axes split: - **Schema, volume, and the reference/definition half of semantic are free** everywhere: they read metadata and the snapshot, and run immediately. - **Grain and the dimension-cardinality half of semantic scan the warehouse**, so - on a metered connector they run the two-step handshake. The first call returns - `needs_confirmation` with an estimate in `cost.estimate` (and a per-table - breakdown). Surface it to the user in human units, get an explicit budget, and - re-issue the same command with `--confirm --budget ` in the - paradigm's unit (bytes on BigQuery, warehouse-seconds on Snowflake and - Databricks, compute-seconds on Redshift, database-seconds on Postgres). - Never invent a budget the user did not agree to, and never retry with a - raised budget on an over-ceiling refusal without asking. -- **`check` is two-phase on a metered connector**: the free axes complete - immediately and their findings ride along in the `needs_confirmation` envelope, - with one combined estimate for the scanning axes. Confirm to complete the sweep. + on a metered connector they run the two-step handshake. Asked for directly, + `maintain grain` returns `needs_confirmation` with an estimate in + `cost.estimate` (and a per-table breakdown). Surface it to the user in human + units, get an explicit budget, and re-issue the same command with + `--confirm --budget ` in the paradigm's unit (bytes on BigQuery, + warehouse-seconds on Snowflake and Databricks, compute-seconds on Redshift, + database-seconds on Postgres). Never invent a budget the user did not agree + to, and never retry with a raised budget on an over-ceiling refusal without + asking. +- **`check` and `semantic` answer first and offer second.** Their free axes + complete on every call, so the envelope is `ok` and the findings in it are + final. The price of the scanning axes sits in `data.offer`, with `axes` naming + what it would add; `data.axes_run` names what already ran. Confirming is a + choice, not a required next step: quote the estimate, say which axes are still + dark, and let the user decide. A triage pass that stops at the free axes is a + complete piece of work, not an abandoned one. +- **Read `warnings` on these responses, always.** They carry the reasons the + baseline may no longer describe the warehouse (a cache newer than the + snapshot, a baseline pinned from a stale cache), which bound every finding + above them. A stale baseline is often the most important line in the response + and it is never in `findings`. On DuckDB everything is free and local, so nothing prompts. diff --git a/skills/transform/SKILL.md b/skills/transform/SKILL.md index f6054a0..5372b77 100644 --- a/skills/transform/SKILL.md +++ b/skills/transform/SKILL.md @@ -203,13 +203,31 @@ database, which is fine for model-only builds. ### The semantic layer -- `semantic define ... --edits-file ` and `semantic update ...` author - and evolve the dbt semantic models (entities, dimensions, measures, metrics) - as plans. `define` refuses names that already exist (use `update`); `update` - refuses names that do not (use `define`). For one logical change that mixes - both (evolve existing metrics and add the helpers they depend on), use - `semantic plan ...`: it accepts mixed intent and classifies each name, and the - envelope reports the split as `defined` and `updated`. +- `semantic define ...` and `semantic update ...` author and evolve the dbt + semantic models (entities, dimensions, measures, metrics) as plans. `define` + refuses names that already exist (use `update`); `update` refuses names that + do not (use `define`). For one logical change that mixes both (evolve existing + metrics and add the helpers they depend on), use `semantic plan ...`: it + accepts mixed intent and classifies each name, and the envelope reports the + split as `defined`, `updated`, and `unchanged`. +- **Prefer `--definitions-file` over `--edits-file` for the semantic layer.** A + real project keeps its metrics in one shared file, so a whole-file payload + means retyping every definition you are not touching: the diff and the + `updated` list then describe the whole file instead of your change, and every + restated line is a chance to corrupt a definition by hand. Send only what + changes instead: + `{"definitions": [{"kind": "metric", "content": "name: ...\n..."}]}`, where + `kind` is `semantic_model` or `metric` and `content` is that definition's YAML + body with no leading `- `. The name comes from the content, and `path` can be + omitted for anything the project already declares (the engine rewrites it + where it lives). Everything else in the file, comments included, is preserved + byte for byte. Reach for `--edits-file` when you are creating a file, removing + a definition, or moving one between files, and when the engine refuses a + layout it will not splice into. +- `unchanged` means you re-stated a definition exactly as the project already + has it. It is not an error, but if a plan is entirely `unchanged` it changes + nothing, and the envelope warns as much: check whether you meant to edit + something. - Plan-time validation is layered so a plan that validates will build: MetricFlow's schemas check the shape; the engine resolves every metric input (ratio and derived metrics reference **metrics**, not measures; a measure only From 11a7218619ad02d10549bb775cd2216d2798a99b Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:30:48 +0200 Subject: [PATCH 6/8] Add unit tests for unchanged definitions in semantic plans and maintain commands to handle pending offers --- .../tests/maintain/test_billed_handshake.py | 84 ++- .../dex-core/tests/maintain/test_semantic.py | 56 ++ packages/dex-core/tests/test_envelope.py | 56 ++ .../dex-core/tests/transform/test_semantic.py | 480 +++++++++++++++++- 4 files changed, 664 insertions(+), 12 deletions(-) diff --git a/packages/dex-core/tests/maintain/test_billed_handshake.py b/packages/dex-core/tests/maintain/test_billed_handshake.py index 56aeffa..eeb6f13 100644 --- a/packages/dex-core/tests/maintain/test_billed_handshake.py +++ b/packages/dex-core/tests/maintain/test_billed_handshake.py @@ -5,7 +5,7 @@ from __future__ import annotations import argparse -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from pathlib import Path import pytest @@ -13,7 +13,7 @@ pytest.importorskip("google.cloud.bigquery") from exmergo_dex_core.adapters.bigquery import BigQueryAdapter -from exmergo_dex_core.cache import ColumnProfile, Dataset +from exmergo_dex_core.cache import CacheProvenance, ColumnProfile, Dataset, DexCache from exmergo_dex_core.cli import dispatch from exmergo_dex_core.config import BigQueryTarget, DexConfig from exmergo_dex_core.engine import DexEngine @@ -229,18 +229,32 @@ def test_over_ceiling_grain_cannot_be_confirmed_through( assert all(c.dry_run for c in fake_bq_client.query_calls) -def test_unconfirmed_check_is_two_phase(fake_bq_client, route_adapter, tmp_path): +def test_unconfirmed_check_answers_and_offers(fake_bq_client, route_adapter, tmp_path): + """The free axes are the answer, not a pending charge. + + An unconfirmed `check` completed three of four axes and spent nothing, so it + reports `ok`. The scanning axes are work the caller never asked for, and they + come back as an offer: gating the whole response behind confirming them would + train a caller to confirm what costs nothing. + """ + _seed_snapshot(tmp_path, extra_baseline_column=True) route_adapter(fake_bq_client) envelope = _dispatch(tmp_path, "check") - assert envelope.status.value == "needs_confirmation" - # Phase one is complete and returned: the free axes' findings ride along - # with the estimate for the scanning axes. + assert envelope.status.value == "ok" codes = {f["code"] for f in envelope.data["findings"]} assert "column_dropped" in codes - assert envelope.data["estimated_bytes"] == 10 * MB - assert any("free" in note for note in envelope.data["notes"]) + + offer = envelope.data["offer"] + assert offer["estimated_bytes"] == 10 * MB + # What did not run has to be nameable, now that the status no longer says so. + assert offer["axes"] == ["grain"] + assert envelope.data["axes_run"] == ["schema", "volume"] + assert any("final" in note for note in offer["notes"]) + # An `ok` must never carry the estimate where a reader looks for what a run + # cost; the offer is the only place that number lives. + assert envelope.cost.estimate is None assert all(c.dry_run for c in fake_bq_client.query_calls) # The free axes are already persisted for reconcile; grain waits. @@ -313,8 +327,8 @@ def test_check_fanout_estimate_reflects_per_query_floor( route_adapter(fake_bq_client) unconfirmed = _dispatch(tmp_path, "check") - assert unconfirmed.status.value == "needs_confirmation" - assert unconfirmed.data["estimated_bytes"] == 2 * 10 * MB + assert unconfirmed.status.value == "ok" + assert unconfirmed.data["offer"]["estimated_bytes"] == 2 * 10 * MB fake_bq_client.row_resolver = lambda sql: [{"d_0": 100}] route_adapter(fake_bq_client) @@ -322,6 +336,54 @@ def test_check_fanout_estimate_reflects_per_query_floor( tmp_path, "check", confirm=True, - budget=float(unconfirmed.data["estimated_bytes"]), + budget=float(unconfirmed.data["offer"]["estimated_bytes"]), ) assert confirmed.status.value == "ok" + # The completed sweep has nothing left to offer, so the key is absent + # entirely rather than present and empty. + assert "offer" not in confirmed.data + + +def test_the_offer_carries_the_same_caveats_as_the_settled_answer( + fake_bq_client, route_adapter, tmp_path +): + """A baseline that may be wrong bounds the free findings too. + + The unconfirmed call is the one a maintenance session opens with, so it is + the last place a caveat about the baseline should go missing. It used to: + the confirmation branch built its result without the baseline warnings the + settled branch adds, which silently dropped the "your baseline is stale" + line from the only response most sessions ever read. + """ + + _seed_snapshot(tmp_path, extra_baseline_column=True) + store = FilesystemStore(tmp_path) + snap = store.load_snapshot() + later = datetime.fromisoformat(snap.created_at) + timedelta(hours=1) + store.save_cache( + DexCache( + datasets=[], + provenance=CacheProvenance( + connector="bigquery", updated_at=later.isoformat() + ), + ) + ) + route_adapter(fake_bq_client) + + unconfirmed = _dispatch(tmp_path, "check") + assert unconfirmed.status.value == "ok" + assert "offer" in unconfirmed.data + assert [w for w in unconfirmed.warnings if "newer than the drift baseline" in w] + + fake_bq_client.row_resolver = lambda sql: [{"d_0": 100}] + route_adapter(fake_bq_client) + confirmed = _dispatch( + tmp_path, + "check", + confirm=True, + budget=float(unconfirmed.data["offer"]["estimated_bytes"]), + ) + stale = "newer than the drift baseline" + assert [w for w in confirmed.warnings if stale in w] == [ + w for w in unconfirmed.warnings if stale in w + ] diff --git a/packages/dex-core/tests/maintain/test_semantic.py b/packages/dex-core/tests/maintain/test_semantic.py index 745f9cb..7353a29 100644 --- a/packages/dex-core/tests/maintain/test_semantic.py +++ b/packages/dex-core/tests/maintain/test_semantic.py @@ -347,3 +347,59 @@ def test_scope_by_semantic_name(maintain_repo): _rc, payload = maintain_repo.dex("maintain", "semantic", "status") codes = {f["code"] for f in payload["data"]["findings"]} assert codes == {"dimension_cardinality_changed"} + + +def test_the_free_half_is_an_answer_and_the_scan_is_an_offer( + maintain_repo, monkeypatch +): + """`maintain semantic` on a billed connector, at the branch that decides. + + The reference and definition checks are free and complete on every call, so + the response is `ok` and those findings are final. The cardinality scan is + priced work the caller did not ask for, and it comes back as an offer rather + than gating the answer behind a confirmation of it. Driven through a stubbed + handshake because the fixture warehouse is DuckDB, which has no cost gate at + all and so never reaches this branch. + """ + + from exmergo_dex_core import command_args + from exmergo_dex_core.envelope import Cost, Paradigm, Status + from exmergo_dex_core.maintain import commands as maintain_cmds + from exmergo_dex_core.results import ConfirmationRequest, to_envelope + + maintain_repo.snapshot() + maintain_repo.sql( + "INSERT INTO stg_orders VALUES (999, 1, 5.0, 'refunded', DATE '2024-03-01')" + ) + + captured: dict = {} + + def stub(command, adapter, estimate, **kwargs): + captured.update(command=command, estimate=estimate, **kwargs) + return ConfirmationRequest( + cost=Cost(paradigm=Paradigm.BYTES_SCANNED, estimate=estimate), + data={"command": command, "estimated_bytes": estimate, **kwargs}, + ) + + monkeypatch.setattr(command_args, "confirmation_request", stub) + + from exmergo_dex_core.engine import DexEngine + + with DexEngine.from_repo(str(maintain_repo.root)) as engine: + result = maintain_cmds.semantic_drift(engine) + + assert result.pending_confirmation is None + assert result.pending_offer is not None + assert captured["axes"] == ["semantic_cardinality"] + + envelope = to_envelope(result) + assert envelope.status is Status.OK + assert envelope.data["offer"]["axes"] == ["semantic_cardinality"] + assert envelope.cost.estimate is None + # The free half really is complete: the definition/reference findings are in + # the envelope, while the value behind the cardinality delta never ran. + assert not [ + f + for f in envelope.data["findings"] + if f["code"] == "dimension_cardinality_changed" + ] diff --git a/packages/dex-core/tests/test_envelope.py b/packages/dex-core/tests/test_envelope.py index dafb03a..dfe636d 100644 --- a/packages/dex-core/tests/test_envelope.py +++ b/packages/dex-core/tests/test_envelope.py @@ -8,6 +8,7 @@ import pytest from exmergo_dex_core import envelope as env +from exmergo_dex_core import results def test_envelope_round_trips_to_json(): @@ -180,3 +181,58 @@ def test_reason_is_none_outside_error_status(): assert env.ok().reason is None assert env.needs_confirmation().reason is None assert env.not_implemented("x").reason is None + + +# --- the two shapes of a priced second phase --------------------------------- + + +class _Findings(results.Result): + """A result with a payload of its own, so key collisions would show.""" + + def data(self): + return {"findings": ["a", "b"], "axes_run": ["schema"]} + + +def _request(estimate: float) -> results.ConfirmationRequest: + return results.ConfirmationRequest( + cost=env.Cost(paradigm=env.Paradigm.BYTES_SCANNED, estimate=estimate), + data={ + "command": "maintain check", + "estimated_bytes": estimate, + "axes": ["grain"], + }, + ) + + +def test_an_unrequested_paid_phase_is_an_offer_on_a_complete_answer(): + """`ok`, because the caller got what they asked for. + + Reporting a finished free sweep as `needs_confirmation` teaches a caller to + confirm work that costs nothing, which is the habit the handshake depends on + them not having. + """ + + result = _Findings(pending_offer=_request(4096.0)) + envelope = results.to_envelope(result) + + assert envelope.status is env.Status.OK + assert envelope.data["findings"] == ["a", "b"] + assert envelope.data["offer"]["estimated_bytes"] == 4096.0 + # The estimate never reaches `cost`, where on an `ok` it would read as what + # this run already spent. + assert envelope.cost.estimate is None + + +def test_a_requested_paid_phase_still_waits_on_confirmation(): + result = _Findings(pending_confirmation=_request(4096.0)) + envelope = results.to_envelope(result) + + assert envelope.status is env.Status.NEEDS_CONFIRMATION + assert envelope.cost.estimate == 4096.0 + assert "offer" not in envelope.data + + +def test_a_result_cannot_both_wait_and_offer(): + result = _Findings(pending_confirmation=_request(1.0), pending_offer=_request(1.0)) + with pytest.raises(ValueError, match="cannot both"): + results.to_envelope(result) diff --git a/packages/dex-core/tests/transform/test_semantic.py b/packages/dex-core/tests/transform/test_semantic.py index 230fa3e..bdbcf21 100644 --- a/packages/dex-core/tests/transform/test_semantic.py +++ b/packages/dex-core/tests/transform/test_semantic.py @@ -465,7 +465,11 @@ def test_semantic_plan_accepts_mixed_new_and_existing_names( assert rc == 0, envelope assert envelope["status"] == "ok" assert envelope["data"]["defined"] == ["customer_count_doubled"] - assert envelope["data"]["updated"] == ["customer_count", "customers"] + # The label edit is a real change and stays `updated`. The semantic model + # rides along in the same whole-file payload untouched, which is the noise + # `unchanged` exists to separate out. + assert envelope["data"]["updated"] == ["customer_count"] + assert envelope["data"]["unchanged"] == ["customers"] def test_define_reports_classification(dbt_project_dir: Path, tmp_path: Path, capsys): @@ -575,3 +579,477 @@ def test_no_time_spine_warning_when_the_project_has_one( ) assert envelope["status"] == "ok" assert not any("time spine" in w for w in envelope["warnings"]) + + +# --- the unchanged class (#109) ---------------------------------------------- + + +def _land_baseline(tmp_path: Path, capsys, content: str = _VALID_SEMANTIC_YAML) -> None: + payload = _payload_file(tmp_path, content) + _run( + [ + "--repo-root", + str(tmp_path), + "semantic", + "define", + "v1", + "--edits-file", + str(payload), + ], + capsys, + ) + rc, envelope = _run(["--repo-root", str(tmp_path), "transform", "apply"], capsys) + assert rc == 0, envelope + + +def _plan_with(tmp_path: Path, capsys, content: str, name: str) -> dict: + payload = _payload_file(tmp_path, content, name=name) + rc, envelope = _run( + [ + "--repo-root", + str(tmp_path), + "semantic", + "plan", + "restate", + "--edits-file", + str(payload), + ], + capsys, + ) + assert rc == 0, envelope + return envelope + + +def test_byte_identical_restate_is_unchanged_not_updated( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """The issue as filed: a whole-file payload that changes nothing. + + Restating a file verbatim is what the whole-file edit unit forces on anyone + extending a shared semantic YAML, and reporting every definition in it as + `updated` sends a reviewer diff-hunting for a change that is not there. + """ + + _land_baseline(tmp_path, capsys) + envelope = _plan_with(tmp_path, capsys, _VALID_SEMANTIC_YAML, "restate.json") + + assert envelope["data"]["defined"] == [] + assert envelope["data"]["updated"] == [] + assert envelope["data"]["unchanged"] == ["customer_count", "customers"] + assert any("changes nothing" in w for w in envelope["warnings"]) + assert all(d["additions"] == 0 and d["deletions"] == 0 for d in envelope["diffs"]) + + +def test_only_the_definition_that_moved_reads_as_updated( + dbt_project_dir: Path, tmp_path: Path, capsys +): + _land_baseline(tmp_path, capsys) + edited = _VALID_SEMANTIC_YAML.replace("Customer count", "Count of customers") + envelope = _plan_with(tmp_path, capsys, edited, "one-change.json") + + assert envelope["data"]["updated"] == ["customer_count"] + assert envelope["data"]["unchanged"] == ["customers"] + assert not any("changes nothing" in w for w in envelope["warnings"]) + + +def test_reordering_a_list_is_a_change(dbt_project_dir: Path, tmp_path: Path, capsys): + """Key order is formatting; list order is content. + + A parsed mapping compares equal whatever order its keys were written in, so + reformatting is not reported as a change. A reordered sequence is a + different document and produces a real diff, so it has to read as one. + """ + + _land_baseline(tmp_path, capsys) + swapped = _VALID_SEMANTIC_YAML.replace( + " - name: email_domain\n" + " type: categorical\n" + " - name: signup_date\n" + " type: time\n" + " expr: cast('2020-01-01' as date)\n" + " type_params:\n" + " time_granularity: day\n", + " - name: signup_date\n" + " type: time\n" + " expr: cast('2020-01-01' as date)\n" + " type_params:\n" + " time_granularity: day\n" + " - name: email_domain\n" + " type: categorical\n", + ) + envelope = _plan_with(tmp_path, capsys, swapped, "swapped.json") + assert envelope["data"]["updated"] == ["customers"] + assert envelope["data"]["unchanged"] == ["customer_count"] + + +def test_identical_content_in_another_file_is_a_move_not_a_no_op( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """Same bytes, different file. The project changes, so `updated` is honest.""" + + _land_baseline(tmp_path, capsys) + elsewhere = tmp_path / "moved.json" + elsewhere.write_text( + json.dumps( + { + "edits": [ + { + "path": "models/semantic/other.yml", + "content": _VALID_SEMANTIC_YAML, + } + ] + } + ), + encoding="utf-8", + ) + rc, envelope = _run( + [ + "--repo-root", + str(tmp_path), + "semantic", + "plan", + "relocate", + "--edits-file", + str(elsewhere), + "--no-parse", + ], + capsys, + ) + assert rc == 0, envelope + assert envelope["data"]["unchanged"] == [] + assert envelope["data"]["updated"] == ["customer_count", "customers"] + + +# --- the definition edit unit (#109) ----------------------------------------- + + +# The valid baseline, plus the two things a round-trip through safe_dump would +# destroy: a banner between sections, and a note indented inside one +# definition's body. +_COMMENTED_YAML = _VALID_SEMANTIC_YAML.replace( + "metrics:\n - name: customer_count\n label: Customer count\n" + " type: simple\n type_params:\n measure: customer_count\n", + "metrics:\n" + " # ---- volume -----------------------------------------------------\n" + " - name: customer_count\n" + " label: Customer count\n" + " type: simple\n" + " type_params:\n" + " measure: customer_count\n" + " # counts rows, not distinct ids\n" + "\n" + " # ---- ratios -----------------------------------------------------\n" + " - name: doubled\n" + " label: Doubled\n" + " type: derived\n" + " type_params:\n" + " expr: customer_count * 2\n" + " metrics:\n" + " - name: customer_count\n", +) +assert "# ---- ratios" in _COMMENTED_YAML, "fixture must patch the metrics block" + + +def _definitions_file(tmp_path: Path, definitions: list[dict], name: str) -> Path: + payload = tmp_path / name + payload.write_text(json.dumps({"definitions": definitions}), encoding="utf-8") + return payload + + +def _plan_definitions( + tmp_path: Path, capsys, definitions: list[dict], name: str, mode: str = "plan" +) -> tuple[int, dict]: + payload = _definitions_file(tmp_path, definitions, name) + return _run( + [ + "--repo-root", + str(tmp_path), + "semantic", + mode, + "one definition", + "--definitions-file", + str(payload), + "--no-parse", + ], + capsys, + ) + + +def test_definition_edit_touches_only_its_own_definition( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """The point of the unit: name one metric, diff one metric. + + The same change through the whole-file unit restates every definition in the + file, so the diff is the same size but the classification is not, and every + untouched definition is retyped by hand on the way in. + """ + + _land_baseline(tmp_path, capsys, _COMMENTED_YAML) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "content": ( + "name: customer_count\n" + "label: Count of customers\n" + "type: simple\n" + "type_params:\n" + " measure: customer_count\n" + ), + } + ], + "one.json", + ) + assert rc == 0, envelope + assert envelope["data"]["updated"] == ["customer_count"] + assert envelope["data"]["unchanged"] == [] + # `doubled` was never mentioned, so it is in no class at all. + assert envelope["data"]["defined"] == [] + + diff = envelope["diffs"][0]["unified"] + changed = [ + line + for line in diff.splitlines() + if line[:1] in {"+", "-"} and not line.startswith(("+++", "---")) + ] + assert "- label: Customer count" in changed + assert "+ label: Count of customers" in changed + # The note nested in this definition's body goes with it; the section banner + # and the neighbouring metric do not move at all. + assert changed == [ + "- label: Customer count", + "+ label: Count of customers", + "- # counts rows, not distinct ids", + ] + + +def test_definition_edit_keeps_the_rest_of_the_file_byte_for_byte( + dbt_project_dir: Path, tmp_path: Path, capsys +): + _land_baseline(tmp_path, capsys, _COMMENTED_YAML) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "content": ( + "name: tripled\n" + "label: Tripled\n" + "type: derived\n" + "type_params:\n" + " expr: customer_count * 3\n" + " metrics:\n" + " - name: customer_count\n" + ), + "path": "models/semantic/customers.yml", + } + ], + "add.json", + ) + assert rc == 0, envelope + assert envelope["data"]["defined"] == ["tripled"] + + diff = envelope["diffs"][0] + assert diff["deletions"] == 0 + assert "+ - name: tripled" in diff["unified"] + + +def test_definition_edit_finds_the_file_that_already_declares_the_name( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """Omitting the path is the ergonomic half: the caller should not have to + know, or restate, where a definition currently lives.""" + + _land_baseline(tmp_path, capsys, _COMMENTED_YAML) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "content": ( + "name: doubled\n" + "label: Twice over\n" + "type: derived\n" + "type_params:\n" + " expr: customer_count * 2\n" + " metrics:\n" + " - name: customer_count\n" + ), + } + ], + "nopath.json", + ) + assert rc == 0, envelope + assert envelope["data"]["paths"] == ["models/semantic/customers.yml"] + assert envelope["data"]["updated"] == ["doubled"] + + +def test_definition_edit_drops_a_note_about_a_line_it_removed( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """A comment indented inside the body is part of that definition. + + Leaving it behind would strand a note about a field that no longer exists, + at an indentation that no longer means anything. + """ + + _land_baseline(tmp_path, capsys, _COMMENTED_YAML) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "content": ( + "name: customer_count\n" + "label: Customer count\n" + "type: simple\n" + "type_params:\n" + " measure: customer_count\n" + ), + } + ], + "note.json", + ) + assert rc == 0, envelope + assert "- # counts rows, not distinct ids" in envelope["diffs"][0]["unified"] + + +def test_definition_edit_refuses_to_duplicate_a_name_into_a_second_file( + dbt_project_dir: Path, tmp_path: Path, capsys +): + _land_baseline(tmp_path, capsys, _COMMENTED_YAML) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "path": "models/semantic/other.yml", + "content": "name: doubled\ntype: simple\ntype_params:\n measure: x\n", + } + ], + "move.json", + ) + assert rc == 1 + assert envelope["status"] == "error" + assert "--edits-file" in envelope["errors"][0] + + +def test_definition_edit_refuses_a_file_it_cannot_span( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """Fail closed and name the way out. A wrong splice is worse than a refusal + that costs the caller a whole-file edit.""" + + _land_baseline(tmp_path, capsys) + (dbt_project_dir / "models" / "semantic" / "flow.yml").write_text( + "version: 2\n\nmetrics: [{name: flow_metric, type: simple, " + "type_params: {measure: customer_count}}]\n", + encoding="utf-8", + ) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "content": ( + "name: flow_metric\n" + "label: Renamed\n" + "type: simple\n" + "type_params:\n" + " measure: customer_count\n" + ), + } + ], + "flow.json", + ) + assert rc == 1 + assert "flow" in envelope["errors"][0] + assert "--edits-file" in envelope["errors"][0] + + +def test_definition_payload_reads_the_name_from_the_content( + dbt_project_dir: Path, tmp_path: Path, capsys +): + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "path": "models/semantic/x.yml", + "content": "type: simple\n", + } + ], + "nameless.json", + ) + assert rc == 1 + assert "name" in envelope["errors"][0] + + +def test_the_two_payload_units_are_mutually_exclusive( + dbt_project_dir: Path, tmp_path: Path, capsys +): + edits = _payload_file(tmp_path, _VALID_SEMANTIC_YAML) + definitions = _definitions_file( + tmp_path, + [ + { + "kind": "metric", + "path": "models/semantic/x.yml", + "content": "name: n\ntype: simple\ntype_params:\n measure: m\n", + } + ], + "both.json", + ) + rc, envelope = _run( + [ + "--repo-root", + str(tmp_path), + "semantic", + "plan", + "both", + "--edits-file", + str(edits), + "--definitions-file", + str(definitions), + ], + capsys, + ) + assert rc == 1 + assert "not both" in envelope["errors"][0] + + +def test_definition_mode_guards_still_apply( + dbt_project_dir: Path, tmp_path: Path, capsys +): + """The unit changes how a change is expressed, not what the modes mean.""" + + _land_baseline(tmp_path, capsys, _COMMENTED_YAML) + rc, envelope = _plan_definitions( + tmp_path, + capsys, + [ + { + "kind": "metric", + "content": ( + "name: customer_count\n" + "label: Clash\n" + "type: simple\n" + "type_params:\n" + " measure: customer_count\n" + ), + } + ], + "clash.json", + mode="define", + ) + assert rc == 1 + assert "already defined" in envelope["errors"][0] From 99a43cc9e6be8170b8794c2d17d8608d13729c38 Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:31:02 +0200 Subject: [PATCH 7/8] Add safety_spine test for maintain commands to validate pending offers and costless scans --- packages/dex-core/tests/test_safety_spine.py | 68 ++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/packages/dex-core/tests/test_safety_spine.py b/packages/dex-core/tests/test_safety_spine.py index d11ff85..4db7727 100644 --- a/packages/dex-core/tests/test_safety_spine.py +++ b/packages/dex-core/tests/test_safety_spine.py @@ -3282,6 +3282,74 @@ def test_api_verify_checkpoint_keeps_what_it_already_paid_for( assert any("saved unverified" in note for note in result.notes) +def test_api_unrequested_paid_work_is_offered_not_demanded( + api_engine, fake_bq_client, tmp_path +): + """Family 2: the handshake guards spend, not the delivery of free answers. + + `maintain check` completes its free axes on every call. Returning those + inside a `needs_confirmation` envelope asked the caller to confirm work they + had not requested in order to read work that cost nothing, which teaches the + habit of confirming reflexively. The guarantee that matters is unchanged and + asserted here: no scan runs, and the estimate is surfaced first. What + changed is that the free answer is delivered as one. + """ + + from exmergo_dex_core.cache import ColumnProfile, Dataset + from exmergo_dex_core.maintain.snapshot import Snapshot, WarehouseBaseline + from exmergo_dex_core.results import to_envelope + + now = datetime.now(UTC).isoformat() + FilesystemStore(tmp_path).save_snapshot( + Snapshot( + created_at=now, + connector="bigquery", + warehouse=WarehouseBaseline( + datasets=[ + Dataset( + identifier="test-proj.shop.customers", + row_count=100, + byte_size=5_000, + columns=[ + ColumnProfile( + name="id", + data_type="INTEGER", + nullable=False, + null_fraction=0.0, + distinct_count=100, + distinct_count_exact=True, + is_unique=True, + ) + ], + candidate_keys=[["id"]], + grain=["id"], + profiled_at=now, + ) + ] + ), + warehouse_from="cache", + ) + ) + + with api_engine() as engine: + from exmergo_dex_core.maintain import commands as maintain_cmds + + result = maintain_cmds.check(engine) + + # Nothing dex was not asked to do has run, and nothing was billed. + assert result.pending_confirmation is None + assert result.pending_offer is not None + assert all(c.dry_run for c in fake_bq_client.query_calls) + assert result.spend is None + + envelope = to_envelope(result) + assert envelope.status is env.Status.OK + # Cost before spend still holds: the price is on the response, in the one + # place that means "not yet spent" rather than "already spent". + assert envelope.data["offer"]["estimated_bytes"] > 0 + assert envelope.cost.estimate is None + + def test_api_pii_stays_flagged_and_never_surfaced(duckdb_file: Path): # Family 3, through the API: the firewall's verdict does not depend on which # door the query came in through. From 69712d6a030c48eef22b04acbfe3facc08001cf9 Mon Sep 17 00:00:00 2001 From: Marco Ciavarella Date: Sun, 9 Aug 2026 15:31:15 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6686af..53e3ffe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,94 @@ tag releases both in lockstep, so entries below are keyed by the engine version. ## [Unreleased] +### Changed + +- **A free answer stops arriving shaped like a bill** ([#136]). `maintain check` + and `maintain semantic` complete their free axes on every call: schema, volume, + and the reference and definition half of semantic are metadata reads that finish + and settle. Both returned that finished work inside a `needs_confirmation` + envelope, because the axes that scan were priced and unconfirmed. So the command + a maintenance session opens with reported its entire triage, in one field report + 373 findings, in a response shaped like a pending charge for work the caller had + not asked for and might never want. + + Two costs came out of that. Confirming things that cost nothing is a habit, and + the handshake only works on commands where it does cost something. And the + framing taught the wrong reading: reaching for `data.findings` inside a refusal + is the natural move, and doing it is how the stale-baseline line in `warnings` + got missed. + + The split is now on whether the caller asked. `needs_confirmation` means dex is + waiting on you for work you requested, and nothing you asked for has run. + Optional priced work rides on a completed answer instead: `status: ok`, findings + final, and the estimate under `data.offer` with the same breakdown and + `--confirm --budget` hint a refusal carried. `data.offer.axes` names what the + estimate would add and `data.axes_run` what already finished, which is what now + separates "grain found nothing" from "grain did not run" since the status no + longer implies it. `cost.estimate` stays unset, so an `ok` never carries a + number that reads as spend. Nothing about the spend gate moved: the confirmed + re-issue is identical and no scan runs without it. `explore relationships + --verify` and `explore map --verify` keep `needs_confirmation`, correctly, since + there the caller did ask for the probes and the budget ran out mid-command. + + A host reading `data["estimated_bytes"]` on these two commands reads + `data["offer"]["estimated_bytes"]`. + +- **The same two commands stopped dropping their baseline caveats on the + unconfirmed call.** The branch that returned early built its result without + `_baseline_warnings`, which the settled branch includes, so the warnings that a + baseline no longer describes the warehouse (a cache newer than the snapshot, a + snapshot pinned from an already-stale cache) were missing from precisely the + response most sessions read. Confirmed against the dogfood project, where + `maintain semantic` reported a 327-hour-old baseline and `maintain check`, same + session and same baseline, reported nothing. Both paths now carry identical + warnings, because what bounds the settled answer bounds the free one. + +- **`semantic plan` reports what changed, not what was re-typed** ([#109]). + Classification compared names against the project and nothing else, so any name + already present read as `updated`. The edit unit is a whole file, so extending a + shared `semantic_models.yml` means re-stating every definition in it, and a + two-metric change reported 27 objects as updated with a `+16/-0` diff. The one + place a reviewer confirms blast radius was the place it was hidden. + + There is now a third class. `updated` means the parsed definition actually + differs from the project's; a definition re-stated identically in the file that + already holds it is `unchanged`. Key order and formatting are not changes; list + order is, and identical content written to a different file is a move, so both + still read as `updated`. A plan whose every definition is unchanged warns that + it changes nothing. + +### Added + +- **A per-definition edit unit for the semantic layer** ([#109]). + `semantic define|update|plan` take `--definitions-file ` beside + `--edits-file`: `{"definitions": [{"kind", "path", "content"}, ...]}`, where + `kind` is `semantic_model` or `metric` and `content` is that one definition's + YAML body. This is the stronger half of the fix. The whole-file unit is what + generated the `updated` noise, and re-typing twenty-seven untouched definitions + to add two is also how a stray key gets injected into a metric by hand, caught + in the field only by eye and by the parse gate. + + The name is read from the content, so the two cannot disagree, and `path` may be + omitted for a definition the project already declares, defaulting to the file + that holds it. An explicit path that would relocate an existing definition is + refused, because writing it to a second file duplicates the name. Each + definition is spliced into its file as text, preserving every other byte + including the comments a semantic layer accumulates; a round trip through + `safe_dump` would reformat the file and produce a larger diff than the payload + it replaces. The result is re-parsed and compared against what was sent, and a + layout the splice cannot span safely (a flow-style sequence, anchors or aliases, + multiple documents, tab indentation) is refused with `--edits-file` named as the + way in. Classification is scoped to the definitions named, so a spliced file's + other definitions appear in no class at all. + + It lowers to the whole-file `PlanEdit` the engine already stores, so the plan + format, the diffs, the conflict hashing, and `transform apply` are unchanged. + Deleting a definition remains a whole-file edit. + + `AGENTS.md`, `references/command-contract.md`, and both the `transform` and + `maintain` skills document the new payload, the third class, and the offer. + ## [1.6.1] - 2026-08-09 ### Fixed