feat: depth-limited selection, run reports and config reuse - #80
Merged
Conversation
init overwrote the cached reference manifest with the target manifest whenever
--reference-target was unset or matched --target, on the reasoning that
"reference and target are the same". They are not the same thing: the reference
*target* selects which warehouse profile to compile against, while the reference
*state* is the baseline to diff against. Conflating them threw away the baseline.
Because deleted nodes exist only in that baseline, `delete` could no longer look
up what it was meant to drop. Reproduced against a real project: cache.json
correctly recorded model.demo.stg_orders as deleted, and generate_delete_map
then exited having found nothing to delete. So `dbt-ci delete` was a silent
no-op for anyone not passing --reference-target, which is also the default in
every README example that uses a local --state directory.
A local --state directory is now cached as the baseline just as a downloaded one
already was, and the target manifest is only used as the reference when there is
genuinely no baseline. Caching it is best-effort: a missing file leaves the
clearer error to the comparison that follows rather than aborting init.
With the fix the same scenario resolves the deletion, table id included:
{'model.demo.stg_orders': {'type': 'model', 'name': 'stg_orders',
'table_id': 'demo.main.stg_orders'}}
Three capabilities that were half-built in the codebase but never connected. **--downstream-depth** implements the `levels` parameter that was stubbed with "To be implemented in the future" in three places. Until now a change always selected its entire downstream closure, so touching a core staging model rebuilt nearly the whole project - the opposite of what change-based CI is for. Selection now walks a bounded breadth-first traversal instead. Verified against a real project with a customers -> l1 -> l2 -> l3 chain: depth 0 runs `customers`, depth 1 adds `l1`, depth 2 adds `l2`, omitting the flag runs all four. New and deleted nodes are always included, since a new model has to run whether or not anything depends on it yet. The old code path would have raised KeyError if anyone had passed `levels`: it looked up a `downstream_dependencies_level_N` key that the graph never contains. **dbt-ci report** renders the run report that every command already writes and nothing ever read - init, run, delete, ephemeral and migration record status, timings and variables into report.json, which finalize then deleted. The report covers change counts and node names, command status with durations, and the exposures downstream of the change set. It appends to $GITHUB_STEP_SUMMARY when set, so in GitHub Actions it needs no workflow wiring. Exposures are resolved against the target graph for modified and new nodes, and against the reference graph for deleted ones - an exposure downstream of a model that was just deleted is the case most worth catching. **Cached run configuration** wires up dbt/flags.py, a module written for exactly this and never imported. init records the target and vars it ran with, and the README promises "specify state once in init, reuse everywhere", but that was only true of state paths: every later command made you repeat --target and --vars. They are now inherited from the cache, with an explicitly passed flag still winning.
The ignore rule was an unanchored `dbt`, which matches a directory of that name at any depth rather than the dbt project at the repository root it was meant to exclude. Two `!dbt_ci/**/dbt` negations (the same line, twice) rescued the package directory, but nothing else: tests/unit/dbt/ was silently dropped by `git add -A`, so a new test module was excluded from a commit without warning and would never have run in CI. Anchoring the pattern to `/dbt/` restricts it to the repository root and makes the negations unnecessary. Verified both directions: a root dbt/ project is still ignored, while tests/unit/dbt/ and new files under dbt_ci/dbt/ are not. Adds the previously-swallowed tests for the cached run configuration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three capabilities that were already half-built in the codebase and never connected, plus two bugs found while verifying them.
Everything here was checked against a real dbt project (duckdb, with a local package and a
customers → l1 → l2 → l3chain), not only unit tests.--downstream-depth— blast-radius controlThe
levelsparameter was stubbed with# To be implemented in the futurein three places, so a change always selected its entire downstream closure. On a real project that means touching a core staging model rebuilds nearly everything — the opposite of what change-based CI is for.Selection now walks a bounded breadth-first traversal. Measured on the chain above, with only
customersmodified:--downstream-depth 0customers--downstream-depth 1customers,l1--downstream-depth 2customers,l1,l2customers,l1,l2,l3New and deleted nodes are always included regardless of depth — a new model has to run whether or not anything depends on it yet.
Worth noting the old code would have raised
KeyErrorhad anyone passedlevels: it looked up adownstream_dependencies_level_Nkey the graph never contains. The stub was never reachable.dbt-ci report— surfacing what was already recordedinit,run,delete,ephemeralandmigrationall write status, timings and resolved variables intoreport.json, and the change set intocache.json. Nothing ever read either back —finalizedeleted them. This renders them:<details>)It appends to
$GITHUB_STEP_SUMMARYwhen set, so in GitHub Actions it needs no workflow wiring;--outputwrites a file (e.g. to post as a PR comment) and--format jsonis machine-readable.Exposures resolve against the target graph for modified and new nodes, and against the reference graph for deleted ones — an exposure downstream of a model that was just deleted is the case most worth catching.
Settings inherited from
initdbt/flags.pywas written for exactly this and never imported.initrecords the target and vars it ran with, and the README promises "specify state once in init, reuse everywhere" — but that was only true of state paths; every later command made you repeat--targetand--vars. They are now inherited from the cache, with an explicitly passed flag still winning.Bugs found while verifying
dbt-ci deletewas a silent no-op without--reference-target.initoverwrote the cached reference manifest with the target manifest whenever--reference-targetwas unset or matched--target, reasoning that "reference and target are the same". They aren't: the reference target selects which warehouse profile to compile against, the reference state is the baseline to diff against. Deleted nodes exist only in that baseline, sodeletecould not look up what it was meant to drop.Reproduced:
cache.jsoncorrectly recordedmodel.demo.stg_ordersas deleted, andgenerate_delete_mapthen exited having found nothing. Every README example using a local--statedirectory hits this. With the fix it resolves the deletion, table id included:.gitignorewas silently swallowing directories nameddbt. The rule was an unanchoreddbt, which matches that name at any depth rather than the root-level dbt project it was for. Two!dbt_ci/**/dbtnegations (the same line, twice) rescued the package directory and nothing else —tests/unit/dbt/was dropped bygit add -A, so a new test module was excluded from a commit without warning and would never have run in CI. I hit this in this PR. Anchored to/dbt/; verified a rootdbt/project is still ignored whiletests/unit/dbt/and new files underdbt_ci/dbt/are not.Testing
220 tests, up from 179. New coverage for depth traversal (including cycles, unknown nodes and depth beyond the graph), report rendering, duration formatting, output destinations, cached-config precedence and the reference-manifest caching rules.
One bug was caught by its own test rather than by review:
format_durationreported0.0sfor a still-running command, because the end-timestamp key is derived from the status andstarted_atis its own end key. It now reports-.Both feature and fix commits pass the suite independently, so the history stays bisectable.
Notes for review
--downstream-depthis opt-in; omitting it preserves today's behaviour exactly.feat:commit means semantic-release will cut a minor (1.5.0) rather than a patch this time.delete/migrationviadbt run-operation(currently BigQuery-only), dbt 1.8+ unit tests as a tracked node type, and the Docker image / GitHub Action work.Generated by Claude Code