test: dbt helpers — loadRawManifest, getUniqueId, extractColumns, listModelNames#690
test: dbt helpers — loadRawManifest, getUniqueId, extractColumns, listModelNames#690anandgupta42 wants to merge 1 commit intomainfrom
Conversation
…mns, listModelNames These four exported helper functions in src/altimate/native/dbt/helpers.ts had zero test coverage. They are shared foundations used by dbtLineage(), generateDbtUnitTests(), and parseManifest(). A cache bug in loadRawManifest would silently serve stale manifest data to every dbt tool; a fallback-chain bug in extractColumns would produce wrong column types in generated YAML. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> https://claude.ai/code/session_01627CWGKN6jHiJvoTRQVj1e
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughA new Bun test suite added to validate core dbt helper functions including manifest loading, unique ID resolution, column extraction, and model name listing. The tests verify filesystem interactions, caching behavior, symlink handling, and edge cases like invalid JSON and missing resources. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
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)
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: 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/test/altimate/dbt-helpers.test.ts`:
- Around line 14-42: Replace the manual tempdir lifecycle (mkdtempSync in
makeTmpDir, tmpDirs array and afterEach cleanup with rmSync) with the shared
tmpdir fixture: import tmpdir from "fixture/fixture" and in each test use "await
using const tmp = await tmpdir()" (or the project's async fixture pattern) and
reference tmp.path instead of makeTmpDir; remove makeTmpDir, tmpDirs, and the
afterEach block, and update tests that call makeTmpDir to use tmp.path so
cleanup is handled by the fixture.
🪄 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: dca83229-522a-4cdc-86d6-04d5a0ad7c10
📒 Files selected for processing (1)
packages/opencode/test/altimate/dbt-helpers.test.ts
| import { describe, test, expect, afterEach } from "bun:test" | ||
| import { writeFileSync, mkdtempSync, rmSync, symlinkSync, utimesSync } from "fs" | ||
| import { tmpdir } from "os" | ||
| import { join } from "path" | ||
| import { | ||
| loadRawManifest, | ||
| getUniqueId, | ||
| extractColumns, | ||
| listModelNames, | ||
| } from "../../src/altimate/native/dbt/helpers" | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Fixtures | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| let tmpDirs: string[] = [] | ||
|
|
||
| function makeTmpDir(): string { | ||
| const dir = mkdtempSync(join(tmpdir(), "dbt-helpers-test-")) | ||
| tmpDirs.push(dir) | ||
| return dir | ||
| } | ||
|
|
||
| afterEach(() => { | ||
| for (const dir of tmpDirs) { | ||
| rmSync(dir, { recursive: true, force: true }) | ||
| } | ||
| tmpDirs = [] | ||
| }) |
There was a problem hiding this comment.
Use the repository tmpdir fixture pattern instead of manual mkdtemp/rm cleanup.
This test file currently bypasses the required test tempdir lifecycle (mkdtempSync + shared array + afterEach). Please switch to tmpdir() from fixture/fixture.ts with await using and tmp.path in each test.
Suggested refactor pattern
-import { describe, test, expect, afterEach } from "bun:test"
-import { writeFileSync, mkdtempSync, rmSync, symlinkSync, utimesSync } from "fs"
-import { tmpdir } from "os"
+import { describe, test, expect } from "bun:test"
+import { writeFileSync, symlinkSync, utimesSync } from "fs"
+import { tmpdir } from "../fixture/fixture"
-let tmpDirs: string[] = []
-
-function makeTmpDir(): string {
- const dir = mkdtempSync(join(tmpdir(), "dbt-helpers-test-"))
- tmpDirs.push(dir)
- return dir
-}
-
-afterEach(() => {
- for (const dir of tmpDirs) {
- rmSync(dir, { recursive: true, force: true })
- }
- tmpDirs = []
-})
+// Example inside each test:
+test("parses valid JSON manifest and returns object", async () => {
+ await using tmp = tmpdir()
+ const manifestPath = join(tmp.path, "valid-manifest.json")
+ // ...
+})As per coding guidelines: packages/opencode/test/**/*.test.ts must use tmpdir from fixture/fixture.ts, await using for cleanup, and temporary path access via tmp.path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/test/altimate/dbt-helpers.test.ts` around lines 14 - 42,
Replace the manual tempdir lifecycle (mkdtempSync in makeTmpDir, tmpDirs array
and afterEach cleanup with rmSync) with the shared tmpdir fixture: import tmpdir
from "fixture/fixture" and in each test use "await using const tmp = await
tmpdir()" (or the project's async fixture pattern) and reference tmp.path
instead of makeTmpDir; remove makeTmpDir, tmpDirs, and the afterEach block, and
update tests that call makeTmpDir to use tmp.path so cleanup is handled by the
fixture.
Summary
1.
loadRawManifest—src/altimate/native/dbt/helpers.ts(7 new tests)This function loads and caches dbt manifest.json files by resolved path + mtime. It is the shared entry point used by
dbtLineage(),generateDbtUnitTests(), andparseManifest(). Zero tests existed. A cache bug would silently serve stale manifest data to every dbt tool. New coverage includes:typeofguard (not caught by thetypeof !== "object"check — known edge case)2.
getUniqueId—src/altimate/native/dbt/helpers.ts(6 new tests)Looks up a model's unique_id by either direct key match or name scan, filtering by
resource_type === "model". Used bydbtLineage()to populatemodel_unique_idin results. A bug here means lineage silently returns the wrong model ID. New coverage includes:3.
extractColumns—src/altimate/native/dbt/helpers.ts(6 new tests)Extracts typed
ModelColumn[]from a raw manifest columns dict, with fallback chains (col.name → dict key,col.data_type → col.type → ""). Used by unit test generation and lineage. Wrong column types propagate into generated YAML. New coverage includes:4.
listModelNames—src/altimate/native/dbt/helpers.ts(3 new tests)Filters manifest nodes by
resource_type === "model"and returns names. Used for error messages. New coverage includes:Type of change
Issue for this PR
N/A — proactive test coverage for shared dbt helper functions that had zero tests
How did you verify your code works?
Marker guard check passes:
bun run script/upstream/analyze.ts --markers --base main --strictChecklist
https://claude.ai/code/session_01627CWGKN6jHiJvoTRQVj1e
Summary by cubic
Add unit tests for shared dbt helper functions to prevent stale manifests and wrong column types from slipping through. Tests cover
loadRawManifest(cache by resolved path + mtime, invalid JSON, symlinks),getUniqueId(model-only lookup by key or name),extractColumns(name/type fallbacks, descriptions), andlistModelNames(model filtering).Written for commit 0166434. Summary will update on new commits.
Summary by CodeRabbit