fix: make altimate-dbt available in bash tool PATH#308
fix: make altimate-dbt available in bash tool PATH#308anandgupta42 wants to merge 1 commit intomainfrom
altimate-dbt available in bash tool PATH#308Conversation
The builder agent prompt instructs using `altimate-dbt` for dbt operations, but the binary was never in PATH — causing 100% of benchmark tasks to fall back to raw `dbt build` and `python3 -c duckdb.connect()`. Changes: - Resolve `dbt-tools/bin` directory at startup via lazy resolver that checks: `ALTIMATE_DBT_TOOLS_BIN` env var, dev source tree, compiled binary location, and `node_modules/.bin` fallback - Prepend resolved path to `PATH` when spawning bash commands - Enhance builder prompt with JOIN/aggregation correctness, output validation, column naming, and floating point precision guidance Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR enhances the dbt verification workflow with stricter validation requirements, expanded correctness guardrails, and introduces a new data visualization skill. It also implements a dynamic resolver in the bash tool to locate the altimate-dbt executable with fallback path strategies and automatic PATH environment configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/src/tool/bash.ts (1)
44-53: Consider logging suppressed errors for debuggability.The empty
catch {}block silently discards errors. While this is acceptable for a fallback mechanism, logging at debug level would help diagnose path resolution issues.♻️ Optional: Add debug logging
try { const binDir = path.dirname(process.execPath) // Walk up to find a directory containing dbt-tools/bin let dir = binDir for (let i = 0; i < 8; i++) { candidates.push(path.join(dir, "dbt-tools", "bin")) candidates.push(path.join(dir, "packages", "dbt-tools", "bin")) dir = path.dirname(dir) } - } catch {} + } catch (e) { + log.debug("dbtToolsBin: failed to resolve from execPath", { error: e }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/tool/bash.ts` around lines 44 - 53, The empty catch swallowing errors around the path resolution loop (the try block using process.execPath, path.dirname and pushing into candidates) hides useful debugging info; update the catch in packages/opencode/src/tool/bash.ts to log the caught error at debug level (e.g., use the module's logger.debug or console.debug as a fallback) and include context mentioning the candidates/path resolution and process.execPath so failures to find dbt-tools/bin are visible while preserving the fallback behavior.
🤖 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/altimate/prompts/builder.txt`:
- Line 108: The heading "**Floating point precision**" should use the hyphenated
compound adjective; update the prompt text (the line starting with "**Floating
point precision**") to "**Floating-point precision**" so the compound modifier
is grammatically correct while keeping the rest of the sentence unchanged.
In `@packages/opencode/src/tool/bash.ts`:
- Around line 58-62: The loop that searches candidates only checks for
"altimate-dbt" and misses Windows wrapper names; update the check inside the
candidates loop (the existsSync(path.join(candidate, "altimate-dbt")) call) to
test for common executable extensions (e.g., "", ".cmd", ".exe", ".bat") or use
PATHEXT on Windows: for each candidate, try path.join(candidate,
`altimate-dbt${ext}`) for each ext and return the candidate if any exists; keep
the rest of the loop intact so both POSIX and Windows binaries are discovered.
---
Nitpick comments:
In `@packages/opencode/src/tool/bash.ts`:
- Around line 44-53: The empty catch swallowing errors around the path
resolution loop (the try block using process.execPath, path.dirname and pushing
into candidates) hides useful debugging info; update the catch in
packages/opencode/src/tool/bash.ts to log the caught error at debug level (e.g.,
use the module's logger.debug or console.debug as a fallback) and include
context mentioning the candidates/path resolution and process.execPath so
failures to find dbt-tools/bin are visible while preserving the fallback
behavior.
🪄 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: def3b99c-c3f5-415e-b63d-668b2f75afeb
📒 Files selected for processing (2)
packages/opencode/src/altimate/prompts/builder.txtpackages/opencode/src/tool/bash.ts
| - **Stopping at compile**: Compile only checks Jinja syntax. Always follow up with `altimate-dbt build` to catch runtime SQL errors. | ||
| - **Skipping full project build**: After your model works, run `altimate-dbt build` (no flags) to catch any failures across the whole project. | ||
| - **Ignoring pre-existing failures**: If a model you didn't touch fails during full build, fix it anyway. The project must be fully green. | ||
| - **Floating point precision**: Use CAST(x AS DECIMAL) or ROUND() for monetary/percentage values. FLOAT arithmetic drifts — 0.158 instead of 0.16 will fail validation. |
There was a problem hiding this comment.
Minor grammar: "Floating-point" should be hyphenated.
"Floating-point" is a compound adjective modifying "precision" and should be hyphenated.
-- **Floating point precision**: Use CAST(x AS DECIMAL) or ROUND() for monetary/percentage values.
+- **Floating-point precision**: Use CAST(x AS DECIMAL) or ROUND() for monetary/percentage values.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Floating point precision**: Use CAST(x AS DECIMAL) or ROUND() for monetary/percentage values. FLOAT arithmetic drifts — 0.158 instead of 0.16 will fail validation. | |
| - **Floating-point precision**: Use CAST(x AS DECIMAL) or ROUND() for monetary/percentage values. FLOAT arithmetic drifts — 0.158 instead of 0.16 will fail validation. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~108-~108: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...y. The project must be fully green. - Floating point precision: Use CAST(x AS DECIMAL) or ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/altimate/prompts/builder.txt` at line 108, The heading
"**Floating point precision**" should use the hyphenated compound adjective;
update the prompt text (the line starting with "**Floating point precision**")
to "**Floating-point precision**" so the compound modifier is grammatically
correct while keeping the rest of the sentence unchanged.
| for (const candidate of candidates) { | ||
| if (existsSync(path.join(candidate, "altimate-dbt"))) { | ||
| return candidate | ||
| } | ||
| } |
There was a problem hiding this comment.
Windows compatibility: binary may have .cmd extension.
On Windows, npm creates .cmd wrapper files (e.g., altimate-dbt.cmd), not bare executables. The current check only looks for altimate-dbt, which will fail to locate the binary on Windows.
🛠️ Proposed fix to handle Windows extensions
+const DBT_BINARY = process.platform === "win32" ? "altimate-dbt.cmd" : "altimate-dbt"
+
for (const candidate of candidates) {
- if (existsSync(path.join(candidate, "altimate-dbt"))) {
+ if (existsSync(path.join(candidate, DBT_BINARY))) {
return candidate
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const candidate of candidates) { | |
| if (existsSync(path.join(candidate, "altimate-dbt"))) { | |
| return candidate | |
| } | |
| } | |
| const DBT_BINARY = process.platform === "win32" ? "altimate-dbt.cmd" : "altimate-dbt" | |
| for (const candidate of candidates) { | |
| if (existsSync(path.join(candidate, DBT_BINARY))) { | |
| return candidate | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/tool/bash.ts` around lines 58 - 62, The loop that
searches candidates only checks for "altimate-dbt" and misses Windows wrapper
names; update the check inside the candidates loop (the
existsSync(path.join(candidate, "altimate-dbt")) call) to test for common
executable extensions (e.g., "", ".cmd", ".exe", ".bat") or use PATHEXT on
Windows: for each candidate, try path.join(candidate, `altimate-dbt${ext}`) for
each ext and return the candidate if any exists; keep the rest of the loop
intact so both POSIX and Windows binaries are discovered.
🚨 dbt-tools is not shipped with the package — this fix only works in dev modeI traced the full build + distribution pipeline and
What the 4 resolver paths actually find in production
Result: In production, all 4 paths return What needs to happenFor this to work for real users, we need to ship
Without one of these, this PR fixes benchmarks running from the monorepo but is a no-op for every installed user. |
✅ Tests — All PassedTypeScript — passedcc @anandgupta42 |
The builder agent prompt instructs using
altimate-dbtfor dbt operations, but the binary was never in PATH — causing 100% of benchmark tasks to fall back to rawdbt buildandpython3 -c duckdb.connect().Changes:
dbt-tools/bindirectory at startup via lazy resolver that checks:ALTIMATE_DBT_TOOLS_BINenv var, dev source tree, compiled binary location, andnode_modules/.binfallbackPATHwhen spawning bash commandsSummary
What changed and why?
Test Plan
How was this tested?
Checklist
Summary by CodeRabbit
New Features
Enhancements