fix: altimate-dbt compile, execute, and children commands fail with runtime errors#255
Conversation
…lbacks for dbt 1.11+ (#252) The `@altimateai/dbt-integration` library's JSON output parsing breaks with newer dbt versions (1.11.x) where the log format changed. Three commands were affected: - `execute`: `dbt show` output no longer contains `data.preview` in the expected format — `d[0].data` throws when the filter returns empty. - `compile`: `dbt compile` output no longer contains `data.compiled` — same `o[0].data` pattern failure. - `children`: `nodeMetaMap.lookupByBaseName()` fails when the manifest file-path resolution doesn't populate the model-name lookup map. Additionally, `tryExecuteViaDbt` in opencode incorrectly expected `raw.table` on `QueryExecutionResult`, which actually has `{ columnNames, columnTypes, data }` — causing the dbt-first execution path to always fall through to native drivers silently. Fixes: - Add try-catch in execute/compile/graph commands with fallback to direct `dbt` CLI subprocess calls (`dbt show`, `dbt compile`, `dbt ls`) - New `dbt-cli.ts` module with resilient multi-format JSON output parsing (handles `data.preview`, `data.rows`, `data.compiled`, `data.compiled_code`, `result.node.compiled_code`) - Fix `tryExecuteViaDbt` to recognize `QueryExecutionResult` shape first, then fall back to legacy `raw.table` format Closes #252 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Code ReviewThis repository is configured for manual code reviews. Comment |
Instead of only matching known JSON field names (which break when dbt changes its output format), each fallback function now uses: 1. Known fields — every field path seen across dbt 1.5-1.11 2. Heuristic deep scan — walks JSON tree looking for SQL-shaped strings or arrays of row objects, regardless of field names 3. Plain text fallback — re-runs without `--output json` and parses ASCII table output (for `dbt show`) or raw text (for `dbt compile`) Also adds `dbt ls` plain text fallback for older dbt versions that don't support `--output json` on the `ls` subcommand. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… 1.10, 1.11)
Adds a DuckDB-based fixture dbt project (`test/fixture/`) and an e2e test
suite that exercises `execDbtShow`, `execDbtCompile`, `execDbtCompileInline`,
and `execDbtLs` against every supported dbt version.
Fixture project:
- 4 models: `stg_customers`, `stg_orders`, `customers` (mart), `orders` (mart)
- 2 seeds: `raw_customers.csv`, `raw_orders.csv`
- DuckDB adapter (no server needed, pure file-based)
- Parent-child graph: raw → staging → marts
Multi-version setup:
- `e2e-setup.sh` creates isolated venvs in `test/.dbt-venvs/<version>/`
- Pins both `dbt-core` and `dbt-duckdb` to target minor version
- dbt 1.11: installs `dbt-core==1.11.0b3` (latest on PyPI) + `dbt-duckdb 1.10`
- Falls back to system `dbt` if no venvs exist
60 e2e tests (12 per version × 5 versions):
- `dbt show`: inline SQL, ref queries against seeded data, row count verification
- `dbt compile`: model compilation, inline Jinja, verifies no `{{ ref` in output
- `dbt ls`: children/parents graph traversal, leaf model empty result
- JSON format diagnostic: logs which field paths each version uses
Version-specific handling:
- dbt 1.7: no `--log-format json` support → tests use `--output json` only
- dbt 1.7: resolves DuckDB path relative to CWD → profiles use absolute paths
- All versions: our 3-tier fallback handles format differences transparently
Closes #252
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…llback Sentry review correctly identified that `execDbtLs` returns a Promise but the calling functions were synchronous. The try/catch could not catch async rejections, and the fallback returned a dangling Promise. Both functions are now `async` with `await` on the adapter calls and the `execDbtLs` fallback. Callers in `index.ts` already await the result. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes from multi-model code review (Claude, GPT 5.2 Codex, Gemini 3.1 Pro, Kimi K2.5, MiniMax M2.5, GLM-5): Critical: - Bun/JSC error messages differ from V8 — `"Cannot read properties of undefined"` is V8-only; JSC throws `"undefined is not an object"`. Changed all catch blocks to use `e instanceof TypeError` instead of brittle message string matching. (Gemini 3.1 Pro) Major: - `run()` now uses project's Python venv — added `configure()` to set `pythonPath`/`projectRoot` from config; `index.ts` calls it before commands run. Bare `dbt` no longer resolves from system PATH. (Gemini) - Guarded `JSON.parse(preview)` in Tier 1 with `safeJsonParse()` — malformed strings now fall through to Tier 2/3 instead of crashing. (GPT 5.2 Codex, GLM-5) - `compile` Tier 3 now reads `target/manifest.json` for `compiled_code` instead of returning raw stdout which contains log lines. (GPT 5.2) - `parseAsciiTable` separator detection uses index-based skip (`lines.slice(2)`) instead of string filter that dropped data rows containing `---`. (Gemini) Minor: - `looksLikeSql` strips leading `--` and `/* */` comments before keyword check — dbt compiled SQL often starts with comments. (Claude, Gemini, Kimi) - `parseAsciiTable` strips ANSI escape codes before parsing. (Claude) - `dbt ls` plain text fallback adds `--quiet` flag and filters log-like lines. (Gemini) - Improved type safety: `parseJsonLines` returns `Record<string, unknown>[]`, `deepFind` uses `unknown` instead of `any`. (MiniMax, GLM-5, Kimi) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| const truncated = limit ? allRows.length > limit : false | ||
| const rows = truncated ? allRows.slice(0, limit) : allRows |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
packages/dbt-tools/src/dbt-cli.ts
Outdated
| } | ||
| // Empty or unparseable — fall through to Tier 2 | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The naive `dirname(pythonPath)` approach only works for venvs where
`python` and `dbt` are siblings. It fails for pyenv shims, conda envs,
pipx installs, poetry cache-dir venvs, asdf/mise, nix, and more.
New `dbt-resolve.ts` module with priority-ordered resolution:
1. `ALTIMATE_DBT_PATH` env var (explicit user override)
2. Sibling of configured `pythonPath` (same venv bin/)
3. Real path of `pythonPath` (follow pyenv/asdf symlinks)
4. Project-local `.venv/bin/dbt`, `venv/bin/dbt`, `env/bin/dbt`
(uv, pdm, poetry in-project, standard venv)
5. `CONDA_PREFIX/bin/dbt` (conda/mamba/micromamba)
6. `VIRTUAL_ENV/bin/dbt` (activated venv)
7. `pyenv which dbt` (resolve through pyenv shim to real binary)
8. `asdf which dbt` (resolve through asdf/mise shim)
9. `which dbt` on current PATH (pipx, system pip, homebrew)
10. Known paths: `~/.local/bin/dbt`, `/usr/local/bin/dbt`,
`/opt/homebrew/bin/dbt`
Also includes:
- `validateDbt()` — checks binary works, detects dbt Fusion vs dbt-core
- `buildDbtEnv()` — constructs env with correct PATH injection
- Updated `init.ts` Python discovery: checks project-local venvs,
`VIRTUAL_ENV`, `CONDA_PREFIX` before falling back to `which`
19 scenario tests covering: venv, uv, pyenv (symlink resolution), conda,
VIRTUAL_ENV, pipx, poetry in-project, pdm, ALTIMATE_DBT_PATH override,
venv/ (no dot), env/, priority ordering, fallback behavior, buildDbtEnv,
validateDbt.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
packages/dbt-tools/src/dbt-cli.ts
Outdated
| // --- Tier 3: plain text fallback (ASCII table) --- | ||
| const plainArgs = ["show", "--inline", sql] | ||
| if (limit !== undefined) plainArgs.push("--limit", String(limit)) | ||
| const { stdout: plainOut } = await run(plainArgs) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Creates actual Python environments (not mocks) with real dbt installed, then verifies `resolveDbt()`, `validateDbt()`, and `buildDbtEnv()` work end-to-end for each environment manager. Setup script (`e2e-resolve-setup.sh`) creates 7 environments: - venv: `python -m venv` + pip install dbt-duckdb - uv: `uv venv` + uv pip install dbt-duckdb - pipx: `pipx install dbt-core --include-deps` + inject dbt-duckdb - conda: `conda create` + python -m pip install dbt-duckdb - poetry: `poetry` with in-project virtualenv + pip install - pyenv-venv: pyenv-managed python + venv + pip install - system: whatever dbt is on PATH 10 test scenarios (43 tests total): 1. venv — resolves via sibling of pythonPath 2. uv — resolves via .venv/bin/dbt in project 3. pipx — resolves via PATH (~/.local/bin) 4. conda — resolves via sibling of pythonPath (CONDA_PREFIX) 5. poetry (in-project) — resolves via .venv/bin/dbt 6. pyenv + venv — resolves via sibling 7. system — resolves via pyenv which dbt 8. VIRTUAL_ENV activated — resolves via env var 9. ALTIMATE_DBT_PATH override — takes highest priority 10. project-root-only — resolves via .venv discovery Each scenario validates: - resolveDbt() finds a real, existing binary - validateDbt() confirms working dbt-core (not Fusion) - dbt --version succeeds with buildDbtEnv() environment Priority ordering tests verify: - pythonPath sibling > project .venv - ALTIMATE_DBT_PATH > everything - VIRTUAL_ENV works without pythonPath Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
packages/dbt-tools/src/dbt-cli.ts
Outdated
|
|
||
| // --- Tier 3: read compiled SQL from manifest.json (more reliable than stdout) --- | ||
| // dbt compile writes compiled_code to target/manifest.json even when stdout is logs | ||
| await run(["compile", "--select", model]).catch(() => {}) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
E2e tests (138s for 5 dbt versions, 30s for 10 Python envs) were running on every `bun test` invocation. Moved to `test/e2e/` subdirectory so the default test command only runs the fast unit tests. - `bun run test` → 49 unit tests in 2.8s (cli, config, dbt-cli, dbt-resolve) - `bun run test:e2e` → 103 e2e tests in ~170s (opt-in, requires setup) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. register.ts truncation logic: adapter already applies limit, so
`allRows.length > limit` was always false. Changed to `>= limit`
(if exactly limit rows returned, likely more exist).
2. Empty result falls through to heuristic scan: when Tier 1 finds a
preview line with `[]` (zero rows), this is a valid empty result —
do NOT fall through to Tier 2 which could match log metadata as rows.
3. Tier 3 plain text fallbacks lack error handling: wrapped `run()` calls
in `execDbtShow`, `execDbtCompile`, and `execDbtCompileInline` with
try/catch so dbt CLI failures produce descriptive errors instead of
unhandled rejections.
4. Silent `.catch(() => {})` on compile Tier 3: replaced with try/catch
that logs intent (prior compile may have left usable manifest) and
falls through. Last-resort `run()` now also has error handling with
a descriptive error message.
Also adds `test/e2e/README.md` documenting e2e test setup and CI
integration (separate job on merge to main, not every PR).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two new CI jobs: - `dbt-tools` — runs unit tests (~3s) on PRs when dbt-tools/ changes. Same fast tests as `bun run test`. - `dbt-tools E2E` — runs on push to main only (~3 min). Sets up real dbt installations (1.8, 1.10, 1.11) and Python environments (venv, uv, system), then runs the full e2e suite. Venvs are cached across runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| if (e instanceof TypeError) { | ||
| return execDbtCompile(model) | ||
| } |
There was a problem hiding this comment.
Bug: The error handling in compile.ts only catches TypeError, unlike execute.ts and graph.ts, potentially missing other error types and preventing the intended fallback from executing.
Severity: MEDIUM
Suggested Fix
Align the error handling in compile.ts with the pattern used in execute.ts and graph.ts. Broaden the catch condition to include e instanceof Error and check for specific, relevant error messages if known, or handle all Error instances to ensure the fallback is always triggered on parsing failures.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/dbt-tools/src/commands/compile.ts#L12-L14
Potential issue: The error handling in `compile.ts` is less defensive than in
`execute.ts` and `graph.ts`. It only catches `TypeError` to trigger a fallback
mechanism. However, the underlying parsing library might throw other `Error` types with
specific messages, as is handled in the other command files. If a non-`TypeError` is
thrown during compilation, the `catch` block will be missed, the fallback will not
execute, and the user will see an unhelpful error message instead of the expected
compiled output.
What does this PR do?
Fixes three
altimate-dbtcommands that break with dbt 1.11+ due to JSON output format changes in the@altimateai/dbt-integrationlibrary, plus a separate bug in the opencodetryExecuteViaDbtresult parser.execute: Library'sdbt showparser expectsdata.previewwhich newer dbt versions don't emit → falls back to directdbt showCLI callcompile: Library'sdbt compileparser expectsdata.compiled→ falls back to directdbt compileCLI callchildren:nodeMetaMap.lookupByBaseName()fails when manifest path resolution is incomplete → falls back todbt lstryExecuteViaDbt: Was checkingraw.tablebutQueryExecutionResulthas{ columnNames, data }→ now handles both formatsType of change
Issue for this PR
Closes #252
How did you verify your code works?
dbt-cli.test.ts: 12 tests covering all fallback parsing formats (data.preview,data.rows,data.compiled,data.compiled_code,result.node.compiled_code,dbt lsoutput, error cases)tryExecuteViaDbt.test.ts: 4 tests forQueryExecutionResult→SqlExecuteResultconversion, legacy format, truncationdbt-toolsandopencodepackagesChecklist