Skip to content

**Breaking**: from data["estimated_bytes"] to data["offer"]["estimated_bytes"] in maintain check and maintain semantic commands; semantic plan reports what changed, not what was re-typed; - #250

Open
marcociav-exmergo wants to merge 8 commits into
mainfrom
issue-109-136

Conversation

@marcociav-exmergo

Copy link
Copy Markdown
Member

Two envelopes that told the truth about everything except themselves

Closes #109. Closes #136.

Both issues are the same defect wearing different clothes: the envelope reported
something that was not what happened, in the one field a reader uses to decide
whether to trust the result. A third instance turned up in the branch #136
touches, and is fixed here too.

#136: a free answer stopped arriving shaped like a bill

maintain check and maintain semantic finish their free axes on every call.
Schema, volume, and the reference and definition half of semantic are metadata
reads: they complete, they settle, and nothing about them is pending. Both
commands 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, 373
findings in one field report, in a response shaped like a pending charge for work
the caller had not asked for and might never want. Two costs followed. Confirming
things that cost nothing is a habit, and the handshake only earns its keep on the
commands where confirming does cost something. And the shape taught the wrong
read: reaching for data.findings inside a refusal is the natural move, and doing
it is how the stale-baseline line sitting in warnings got missed.

The split now turns on whether the caller asked for the work, which is mechanical
rather than a judgement call: confirmation_request only returns a request when
--confirm was absent, and every other refusal (over-ceiling, no-ceiling) raises.

  • needs_confirmation means dex is waiting on you for work you requested, and
    nothing you asked for has run.
  • An unrequested priced phase rides on a completed answer instead: status: ok,
    findings final, 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
finished. That pair is load-bearing now that the status no longer implies it:
without it, "grain ran and found nothing" and "grain did not run" look identical.
cost.estimate stays unset, so an ok never carries a number that reads as
spend. The spend gate itself did not move: the confirmed re-issue is identical and
no scan runs without it.

explore relationships --verify and explore map --verify keep
needs_confirmation, deliberately. Their checkpoint fires mid-command on an
already-confirmed run whose budget ran out, so the caller did ask for the probes
and dex genuinely is blocked. The safety spine's
test_api_verify_checkpoint_keeps_what_it_already_paid_for passes untouched,
which is the evidence the line is drawn in the right place.

Breaking: a host reading data["estimated_bytes"] on these two commands reads
data["offer"]["estimated_bytes"].

The adjacent defect, found while reading that branch

The early-return path built its result without _baseline_warnings, which the
settled path includes. Every reason the baseline may no longer describe the
warehouse (a cache newer than the snapshot, a snapshot pinned from an
already-stale cache) was therefore dropped from precisely the response most
sessions read, and never from the one they read second.

Confirmed in the field, not just in the code. Against the dogfood project, same
session and same baseline: maintain semantic reported a 327-hour-old baseline,
and maintain check reported nothing at all. Both paths now build identical
warnings, because what bounds the settled answer bounds the free one.

#109: semantic plan reports what changed, not what was re-typed

check_mode 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 restating every definition in it.

Measured on the dogfood project before the change: a payload adding two metrics to
a 27-metric file produced a +16/-0 diff and reported 27 objects as updated.
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 restated identically in the file that already
holds it is unchanged. Two deliberate edges: key order and formatting are not
changes, but list order is (a reordered dimensions: block is a real diff), and
identical content written to a different file is a move whose diff is real, so it
stays updated. A plan whose every definition is unchanged warns that it changes
nothing.

Same payload after: defined: 2, updated: 0, unchanged: 25.

#109's stronger fix: a per-definition edit unit

The whole-file unit is what generated the noise, and retyping twenty-seven
untouched definitions to add two is also how a stray key gets injected into a
metric by hand, which the field report caught by eye and by the parse gate rather
than by design. So --definitions-file sits beside --edits-file on
semantic define|update|plan:

{"definitions": [
  {"kind": "metric", "content": "name: files_per_session\ntype: ratio\n..."}
]}

The name is read from the content, so the two cannot disagree. path may be
omitted for a definition the project already declares and defaults 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 and only
dbt parse would notice.

Text splice, not a YAML round trip. safe_dump would reformat the file and
strip the comments a hand-written semantic layer accumulates, producing a larger
diff than the payload it replaces and destroying the adjudication notes in the
dogfood project's own YAML. So each definition is spliced in place and every other
byte survives. A comment at the item's indent is a section banner and stays put; a
comment indented inside the body belongs to the definition and goes with it.

It fails closed. A layout the scanner cannot span safely (flow-style sequence,
anchors or aliases, multiple documents, tab indentation) is refused with
--edits-file named as the way in, and the spliced result is re-parsed and
compared against what was sent, so a splice that reads as valid YAML but says
something else cannot reach the plan store.

Classification is scoped to the definitions named. This one is worth flagging: the
unit lowers to whole-file PlanEdits, so without the scope a spliced file's other
twenty-five definitions would come back as unchanged, a quieter version of the
noise the whole change exists to remove. Caught by a test, not by review.

It lowers to the edit unit the engine already stores, so the plan format, the
diffs, the conflict hashing, and transform apply are untouched. Deleting a
definition remains a whole-file edit; the semantic verbs author and do not delete,
and keeping that rule intact beats the symmetry.

Same change as a definitions payload: defined: 2, unchanged: 0, one edit, a
+16/-0 diff, comments untouched.

Verification

1965 tests pass. New coverage: the three-way classification including the two
edges, the patch unit and its refusals, the flipped billed-handshake tests, the
ok-with-offer and both-set-raises cases at the envelope layer, maintain semantic's offer branch, and a safety-spine assertion that an unconfirmed
two-phase command returns ok, issues only dry runs, and keeps the estimate out
of cost.estimate.

Dogfooded against internal-product-analytics (BigQuery, exmergo-viz, 10
semantic models and 27 metrics), before and after on the same baseline:

before after
maintain check needs_confirmation, 42 findings, no stale-baseline warning ok, 42 findings, offer.axes: ["grain"], 891 MB estimate, stale-baseline warning present
semantic plan (whole file, +2 metrics) updated: 27 defined: 2, unchanged: 25
semantic plan (no-op restate) updated: 27 all unchanged, warns it changes nothing
semantic plan (definitions) not available defined: 2, unchanged: 0, +16/-0

The paid path was then exercised for real, once, at exactly the offered estimate:
maintain check --confirm --budget 891289600 returned ok with all four axes
(grain: 5 new findings, 47 total), no offer key, and
spend.bytes_billed: 891289600, matching the estimate to the byte. The
stale-baseline warning is present there too, which is the parity the third fix is
about.

Also verified: DuckDB is unaffected (plain ok, all four axes, no offer, and
paradigm: free_local), the unconfirmed BigQuery path issues dry runs only, and
explore semantic against the live dbt Cloud Semantic Layer still returns ok
(13 rows for active_users by metric_time__month) with its cost-guard warning
intact. That last one is a neighbouring-surface regression check, not evidence for
either fix: neither code path touches the hosted backend.

Plan runs left four unapplied plan files in that repo's .dex/plans/ and touched
.dex/drift.json; nothing was applied and no project file was written.

Not in this PR

  • transform plan (the model and schema path) has no classification at all.
    Whether it should is a separate question, not a regression.
  • Deleting a single definition through the patch unit.
  • A reauth-expired ADC surfaces as reason: internal rather than connection or
    prerequisite. Noticed when credentials lapsed mid-session; worth its own
    issue, since the reason code is what a host branches on to decide whether a
    retry could ever help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant