Skip to content

feat: depth-limited selection, run reports and config reuse - #80

Merged
pannoury merged 3 commits into
mainfrom
claude/feature-ideas-code-improvements-e313bt
Aug 9, 2026
Merged

feat: depth-limited selection, run reports and config reuse#80
pannoury merged 3 commits into
mainfrom
claude/feature-ideas-code-improvements-e313bt

Conversation

@pannoury

@pannoury pannoury commented Aug 9, 2026

Copy link
Copy Markdown
Member

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 → l3 chain), not only unit tests.


--downstream-depth — blast-radius control

The levels parameter was stubbed with # To be implemented in the future in 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 customers modified:

Flag Models run
--downstream-depth 0 customers
--downstream-depth 1 customers, l1
--downstream-depth 2 customers, l1, l2
(omitted, the default) customers, l1, l2, l3

New 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 KeyError had anyone passed levels: it looked up a downstream_dependencies_level_N key the graph never contains. The stub was never reachable.

dbt-ci report — surfacing what was already recorded

init, run, delete, ephemeral and migration all write status, timings and resolved variables into report.json, and the change set into cache.json. Nothing ever read either back — finalize deleted them. This renders them:

  • change counts and node names, grouped modified / new / deleted (long lists fold into <details>)
  • exposure impact — exposures downstream of the change set
  • per-command status and duration

It appends to $GITHUB_STEP_SUMMARY when set, so in GitHub Actions it needs no workflow wiring; --output writes a file (e.g. to post as a PR comment) and --format json is 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 init

dbt/flags.py was 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.


Bugs found while verifying

dbt-ci delete was a silent no-op without --reference-target. init overwrote the cached reference manifest with the target manifest whenever --reference-target was 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, so delete could not look up what it was meant to drop.

Reproduced: cache.json correctly recorded model.demo.stg_orders as deleted, and generate_delete_map then exited having found nothing. Every README example using a local --state directory hits this. With the fix it resolves the deletion, table id included:

{'model.demo.stg_orders': {'type': 'model', 'name': 'stg_orders',
                           'table_id': 'demo.main.stg_orders'}}

.gitignore was silently swallowing directories named dbt. The rule was an unanchored dbt, which matches that name at any depth rather than the root-level dbt project it was for. Two !dbt_ci/**/dbt negations (the same line, twice) rescued the package directory and nothing else — tests/unit/dbt/ was dropped by git 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 root dbt/ project is still ignored while tests/unit/dbt/ and new files under dbt_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_duration reported 0.0s for a still-running command, because the end-timestamp key is derived from the status and started_at is 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-depth is opt-in; omitting it preserves today's behaviour exactly.
  • The feat: commit means semantic-release will cut a minor (1.5.0) rather than a patch this time.
  • Not included, and still open from the earlier discussion: warehouse-agnostic delete/migration via dbt 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

claude added 3 commits August 9, 2026 10:39
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.
@pannoury
pannoury merged commit b1beb05 into main Aug 9, 2026
4 checks passed
@pannoury
pannoury deleted the claude/feature-ideas-code-improvements-e313bt branch August 9, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants