Skip to content

feat: ship dbt-tools with altimate-code npm package#312

Open
suryaiyer95 wants to merge 2 commits intomainfrom
fix/ship-dbt-tools-with-altimate-code
Open

feat: ship dbt-tools with altimate-code npm package#312
suryaiyer95 wants to merge 2 commits intomainfrom
fix/ship-dbt-tools-with-altimate-code

Conversation

@suryaiyer95
Copy link
Contributor

@suryaiyer95 suryaiyer95 commented Mar 20, 2026

Summary

  • Resolver (bash.ts): Adds dbtToolsBin lazy resolver that finds altimate-dbt and injects it into PATH for spawned bash commands. Handles env var override, dev source tree, scoped npm install (@altimateai/altimate-code), unscoped npm install (altimate-code), and bounded walk-up fallback.
  • Bundling (publish.ts): Builds dbt-tools during publish and bundles it into both scoped and unscoped wrapper packages. Adds altimate-dbt to the bin field so npm creates global symlinks. Bundle is ~3 MB (JS + Python packages); native .node files excluded since @altimateai/altimate-core runtime dep provides them.

Why this matters

Without this, altimate-dbt was never shipped to customers — the resolver from PR #308 was a no-op in production because dbt-tools wasn't compiled, bundled, or published alongside altimate-code.

Resolver strategy

  1. ALTIMATE_DBT_TOOLS_BIN env var (explicit override)
  2. Dev mode: import.meta.dirname → source tree sibling
  3. npm installed: checks both scoped (node_modules/@altimateai/altimate-code/dbt-tools/bin) and unscoped (node_modules/altimate-code/dbt-tools/bin) wrapper packages
  4. Walk-up fallback: bounded at 8 levels, stops at filesystem root

Supersedes

Replaces #308 with a clean implementation that fixes the unscoped npm layout bug (the original resolver couldn't find dbt-tools when installed via npm i -g altimate-code).

Test plan

  • Verified resolver finds altimate-dbt in scoped npm layout
  • Verified resolver finds altimate-dbt in unscoped npm layout
  • Verified resolver returns undefined for isolated binary (graceful degradation)
  • Verified dev mode resolution from source tree
  • Removed system altimate-dbt from PATH and re-tested to confirm no false positives
  • Tested altimate-dbt build, execute, columns, graph children/parents against jaffle-shop with DuckDB

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added the altimate-dbt command-line tool to published packages for dbt integration.
    • Bundled dbt-tools with releases so the dbt binary and runtime files are included automatically.
    • Implemented automatic discovery and PATH resolution for dbt binaries across dev, installed, and system environments.

- Add `dbtToolsBin` resolver in `bash.ts` that finds `altimate-dbt`
  and injects it into PATH for spawned bash commands
- Resolver checks: env var → dev source tree → scoped npm wrapper →
  unscoped npm wrapper → walk-up fallback
- Build and bundle dbt-tools in `publish.ts` for both scoped
  (`@altimateai/altimate-code`) and unscoped (`altimate-code`) packages
- Register `altimate-dbt` in published `bin` field so npm creates
  global symlinks on install
- Bundle is ~3 MB (JS + Python packages), not 200 MB (native .node
  files excluded — provided by `@altimateai/altimate-core` runtime dep)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 20, 2026 00:07
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@coderabbitai
Copy link

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

Integrates dbt-tools into the publish build (builds and copies dbt-tools into package dist outputs and adds a bin entry) and adds runtime dbt-tools detection to BashTool, which resolves a dbt-tools/bin path and prepends it to spawned process PATH when found.

Changes

Cohort / File(s) Summary
Publish script / bundling
packages/opencode/script/publish.ts
Adds building of ../dbt-tools, copies dbt-tools/bin/altimate-dbt and dbt-tools/dist/index.js (and optional dist/altimate_python_packages) into both ./dist/${pkg.name} and ./dist/${unscopedName}, and adds "altimate-dbt": "./dbt-tools/bin/altimate-dbt" to generated package.json bin maps.
Runtime detection / Bash tool
packages/opencode/src/tool/bash.ts
Introduces a lazy dbtToolsBin() resolver that checks ALTIMATE_DBT_TOOLS_BIN, dev-tree path, node_modules wrapper locations, and a walk-up from process.execPath; uses existsSync and debug logging. When resolved, the path is prepended to the spawned process PATH via the spawn env merge.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev
  participant PublishScript as Publish Script
  participant DbtTools as ../dbt-tools
  participant Dist as package ./dist outputs
  Dev->>PublishScript: run publish
  PublishScript->>DbtTools: run bun run build
  DbtTools-->>PublishScript: built bin + dist
  PublishScript->>Dist: create dbt-tools/bin & dbt-tools/dist
  PublishScript->>Dist: copy altimate-dbt, dist/index.js, optional packages
  PublishScript->>Dist: update package.json bin -> "altimate-dbt"
Loading
sequenceDiagram
  autonumber
  participant App as Runtime App
  participant BashTool as BashTool
  participant FS as FileSystem / Env
  participant Spawned as Child Process
  App->>BashTool: request to spawn shell command
  BashTool->>BashTool: dbtToolsBin() resolution
  BashTool->>FS: check ALTIMATE_DBT_TOOLS_BIN, dev paths, node_modules, execPath walk-up
  FS-->>BashTool: resolved path or undefined
  BashTool->>Spawned: spawn with PATH (prepended with dbt-tools/bin if resolved)
  Spawned-->>App: command output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped to build and bundle bright,

I stitched the bins so they work just right.
From dev tree paths to npm's lane,
I nudged PATH so dbt's found again.
Hop, code, repeat — a rabbit's tiny gain 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: ship dbt-tools with altimate-code npm package' clearly and concisely describes the main change: bundling dbt-tools with the npm package.
Description check ✅ Passed The description covers all required template sections (Summary with comprehensive context, Test Plan with detailed verification steps, and Checklist acknowledgment), provides clear rationale, resolver strategy, and test evidence.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ship-dbt-tools-with-altimate-code
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ensures the altimate-dbt CLI (from dbt-tools) is actually shipped with the altimate-code npm packages and made discoverable at runtime by the bash tool, so dbt operations can run without relying on a system-installed altimate-dbt.

Changes:

  • Add a lazy dbtToolsBin resolver in the bash tool and prepend the resolved directory to PATH for spawned commands.
  • Build and bundle dbt-tools during the publish pipeline into both the scoped and unscoped wrapper packages.
  • Register altimate-dbt in the generated wrapper packages’ bin field so npm installs shims for it.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
packages/opencode/src/tool/bash.ts Adds runtime resolver for dbt-tools/bin and injects it into PATH for bash tool spawns.
packages/opencode/script/publish.ts Builds dbt-tools, copies its artifacts into wrapper packages, and adds altimate-dbt to the generated bin map.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/opencode/script/publish.ts`:
- Around line 56-64: The generated dbt-tools bundle currently lacks a package
boundary declaring ESM, so add creating a dbt-tools/package.json with { "type":
"module" } when copying files in copyDbtTools so Node treats dist/index.js as
ESM; update the copyDbtTools function to write a small package.json into
${destRoot}/dbt-tools (or ${destRoot}/dbt-tools/dist) after the mkdir steps,
ensuring the altimate-dbt import("../dist/index.js") resolves as ESM even when
the wrapper package.json (the generated wrapper block) defaults to CommonJS.

In `@packages/opencode/src/tool/bash.ts`:
- Around line 222-232: The current logic rebuilds PATH from process.env.PATH
when dbtToolsBin() returns a value, which overwrites any PATH modifications
provided by shellEnv.env from Plugin.trigger("shell.env"); update the envPATH
computation so it prefixes extraPath to shellEnv.env.PATH if present (falling
back to process.env.PATH only if shellEnv.env.PATH is undefined), and ensure the
spawn call (where env is composed and PATH is conditionally set) uses that
envPATH so plugin-provided PATH entries are preserved.
- Around line 39-43: The dev resolver only checks one relative path and misses
the alternative source layout; update the block that uses import.meta.dirname to
attempt additional candidate paths (e.g. also resolve "../../../dbt-tools/bin"
in addition to "../../../../dbt-tools/bin"), calling hasDbtBinary against each
candidate and returning the first match; modify the logic around
import.meta.dirname, devPath and the hasDbtBinary check to iterate over these
alternate resolved paths so both built and source sibling layouts
(packages/dbt-tools/bin) are supported.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e6240496-625e-4197-8037-8e05ff909179

📥 Commits

Reviewing files that changed from the base of the PR and between 4cee25f and af3a4b7.

📒 Files selected for processing (2)
  • packages/opencode/script/publish.ts
  • packages/opencode/src/tool/bash.ts

- Fix dev-mode path: `../../../../dbt-tools/bin` → `../../../dbt-tools/bin`
  (was resolving to repo root instead of `packages/dbt-tools/bin`,
  meaning `altimate-dbt` was never found in development)
- Fix PATH merge order: compute `envPATH` from merged `baseEnv`
  (process.env + shellEnv.env) so plugin PATH entries are preserved
- Move `log` declaration above `dbtToolsBin` resolver to eliminate
  temporal dependency on lazy execution order
- Add blank line between resolver closure and `DEFAULT_TIMEOUT`

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/opencode/src/tool/bash.ts`:
- Around line 225-227: The code that builds envPATH uses baseEnv.PATH only,
which on Windows can miss an inherited "Path" entry and result in
duplicate/missing PATH in the child process; update the construction to check
both casings (use baseEnv.PATH ?? baseEnv.Path ?? "") when prefixing extraPath
(the envPATH variable) and apply the same change to the other similar spot
around where PATH is set (the second instance mentioned in the review); this
follows the existing pattern used in which.ts and ensures inherited "Path"
entries are preserved when adding extraPath.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1860cf46-55ab-4eba-b0f8-aa0e82be33ad

📥 Commits

Reviewing files that changed from the base of the PR and between af3a4b7 and ab44f67.

📒 Files selected for processing (1)
  • packages/opencode/src/tool/bash.ts

@kulvirgit
Copy link
Collaborator

Multi-Model Code Review — fix/ship-dbt-tools-with-altimate-code

Verdict: APPROVE
Critical Issues: 0 | Major Issues: 0 | Minor Issues: 7 | Nits: 3

Reviewed by 7 models (Claude, Codex, Gemini, Kimi, Grok, MiniMax, Qwen). Convergence: 1 round, 5/6 APPROVE (Gemini pending). Codex contributed via diff-only review after sandbox issues.


Minor Issues

1. No unit tests for dbtToolsBin() resolver (Testing) — bash.ts:29-78
The resolver has 4 code paths (env var, dev, npm, walk-up) with platform branching. None are tested. A failure silently falls through to undefined.
Suggestion: Extract resolver logic into a pure function accepting execPath, dirname, and env as parameters for testability.
Flagged by: Claude, Kimi, MiniMax, Codex — Consensus

2. No validation of dbt-tools build output (Bug) — publish.ts:51
bun run build is called but there's no check that dist/index.js exists before copyDbtTools() runs. Bun's $ throws on non-zero exit, but subtler build failures (empty output) would pass through.
Suggestion: Add existsSync check on dbt-tools/dist/index.js after the build step.
Flagged by: Claude, MiniMax — Consensus

3. Windows support gap (Bug) — bash.ts:24-27, publish.ts
DBT_BINARY_NAMES checks for .exe/.cmd on Windows, but no Windows launcher (.cmd file) is actually shipped — only the Unix shim #!/usr/bin/env node. Windows users won't be able to invoke altimate-dbt via the injected PATH.
Suggestion: Ship a .cmd launcher alongside the Unix shim, e.g.:

@ECHO OFF
node "%~dp0..\dist\index.js" %*

Flagged by: Codex — Unique

4. lazy() permanently caches undefined on miss (Logic Error) — bash.ts:35, lazy.ts:8-10
If the first resolution returns undefined (binary not found), lazy() sets loaded = true and caches undefined permanently. In practice this is low-risk for a CLI tool (binary location won't change mid-process), but it's a subtle gotcha.
Suggestion: Only cache non-undefined results, or document the behavior.
Flagged by: Codex — Unique

5. Dev path may match incorrectly in bundled-but-not-compiled mode (Logic Error) — bash.ts:43-46
The $bunfs check guards against Bun's compiled VFS, but a bun build bundle (not compiled to single binary) would have import.meta.dirname pointing to dist/, causing the dev path to resolve incorrectly. The npm fallback (step 3) would still catch it.
Suggestion: Consider an additional guard, e.g., checking that package.json exists at the resolved dev root.
Flagged by: MiniMax, Claude — Consensus

6. ALTIMATE_DBT_TOOLS_BIN naming suggests a binary, not a directory (Design) — bash.ts:37
The env var name ends in _BIN but the code expects a directory containing the binary.
Suggestion: Consider renaming to ALTIMATE_DBT_TOOLS_DIR or ALTIMATE_DBT_TOOLS_PATH.
Flagged by: Kimi — Unique

7. fs import inconsistency (Code Quality) — bash.ts:10,20
fs/promises imported as fs on line 10, then existsSync imported separately from fs on line 20. Not a bug but confusing.
Flagged by: Kimi — Unique

Nits

  1. Walk-up depth magic numberbash.ts:67: for (let i = 0; i < 8; i++) has no comment explaining why 8 levels. (Kimi)
  2. "opencode" in wrapper names may be dead codebash.ts:59: for (const wrapper of ["altimate-code", "opencode"]) — is "opencode" still a valid package name that ships dbt-tools? (Claude)
  3. Relative path "../dbt-tools" depends on CWDpublish.ts:50: Works because process.chdir(dir) at line 10, but fragile if invocation context changes. (Claude)

Positive Observations

  • Clean 4-level fallback strategy with clear comments at each level
  • Platform-aware binary detection (DBT_BINARY_NAMES handles Windows extensions)
  • lazy() memoization avoids repeated filesystem checks
  • PATH prepend order correctly prioritizes bundled binary
  • Defensive existsSync checks prevent crashes
  • Smart exclusion of .node files saves ~200MB
  • Consistent packaging across scoped and unscoped wrapper packages
  • Graceful degradation — missing dbt-tools doesn't crash the bash tool
  • copyDbtTools() is a good refactor point keeping scoped/unscoped publish flows aligned

Missing Tests

  • dbtToolsBin() resolver — 4 code paths untested
  • hasDbtBinary() — not tested
  • PATH construction in bash tool spawn — no integration test
  • copyDbtTools() — no test verifying correct file layout
  • Windows-specific test for runnable launcher
  • Smoke test: install built npm package → run altimate-dbt --help

Finding Attribution

Issue Origin Type
No unit tests for dbtToolsBin() resolver Claude, Kimi, MiniMax, Codex Consensus
No validation of dbt-tools build output Claude, MiniMax Consensus
Windows support gap — no .cmd launcher shipped Codex Unique
lazy() permanently caches undefined on miss Codex Unique
Dev path incorrect in bundled-but-not-compiled mode MiniMax, Claude Consensus
ALTIMATE_DBT_TOOLS_BIN naming misleading Kimi Unique
fs import inconsistency Kimi Unique
Walk-up depth magic number 8 Kimi Unique
"opencode" wrapper name may be dead code Claude Unique
Relative path depends on CWD Claude Unique

🤖 Generated with Claude Code — multi-model review panel (7 models, 1 convergence round)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants