test: finops recommendations + dbt manifest edge cases

[anandgupta42]

What does this PR do?

1. generateSizingRecommendations — src/altimate/native/finops/warehouse-advisor.ts (6 new tests)

This function decides what warehouse sizing advice users see (SCALE_UP, SCALE_DOWN, BURST_SCALING, or HEALTHY). Only the HEALTHY path had a single test in schema-finops-dbt.test.ts. The actionable recommendation paths — where users make real cost decisions — were completely untested. New coverage includes:

• SCALE_UP when avg queue load exceeds 1.0 (high impact recommendation)
• BURST_SCALING when peak queue load exceeds 5.0 but average is normal
• SCALE_DOWN with correct SIZE_ORDER step-down logic (Large → Medium)
• X-Small floor guard — no SCALE_DOWN suggested when already at minimum size
• Multi-warehouse — independent recommendations for multiple warehouses in one analysis
• Missing size fallback — graceful "unknown" when SHOW WAREHOUSES fails (Snowflake privilege edge case)

2. parseManifest edge cases — src/altimate/native/dbt/manifest.ts (6 new tests)

The dbt manifest parser had only two tests: non-existent file and happy path. Real-world dbt projects frequently produce incomplete or partial manifests (older dbt versions, dbt parse before compilation, corrupted files). New coverage includes:

• Invalid JSON — corrupted manifest files return empty result instead of crashing
• Missing nodes/sources keys — partial manifests from dbt parse before full compilation
• Empty columns dict — undocumented models with no column definitions
• Missing depends_on — older dbt manifest versions that omit the field entirely
• JSON null — guards against the JS quirk where typeof null === "object"
• Column type fallback — tests the col.type fallback path in extractColumns when data_type is absent (real difference between dbt v1 and v2+ manifests)

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage from test-discovery

How did you verify your code works?

bun test test/altimate/finops-recommendations.test.ts  # 6 pass, 17 expect() calls
bun test test/altimate/dbt-manifest-edge.test.ts       # 6 pass, 18 expect() calls

Marker check: bun run script/upstream/analyze.ts --markers --base main --strict — no issues.

All tests were reviewed by a critic agent before committing. 2 tests were rejected as duplicates of existing coverage and excluded.

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

https://claude.ai/code/session_01XhZy7vaqdasKH5hQ6H9ee3

Summary by CodeRabbit

• Tests
  • Added test suite validating manifest parser gracefully handles malformed and incomplete inputs, including invalid JSON and missing required fields.
  • Added test suite for warehouse sizing recommendations across various load and concurrency scenarios, including fallback behavior for unknown warehouse sizes.

[claude]

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.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Two new test suites added for the OpenCode package: one testing dbt manifest parser edge cases with malformed inputs and missing keys, another verifying warehouse sizing recommendations across various load scenarios.

Changes

Cohort / File(s)	Summary
Test Suites for Data Processing packages/opencode/test/altimate/dbt-manifest-edge.test.ts, packages/opencode/test/altimate/finops-recommendations.test.ts	Added comprehensive test coverage for manifest parsing edge cases (invalid JSON, missing keys, empty columns, type fallback) and warehouse sizing recommendations (scale up/down/burst scaling logic based on queue load and concurrency metrics).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

contributor

Suggested reviewers

• suryaiyer95

Poem

🐰 Hopping through test cases with glee,
Edge cases caught, just wait and see!
Manifest parsed, recommendations true,
Quality assured in all that we do! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'test: finops recommendations + dbt manifest edge cases' accurately summarizes the main change—adding test coverage for two specific modules.
Description check	✅ Passed	The description is comprehensive and follows the template structure with clear sections covering what changed, how it was tested, and verification steps, addressing all required template sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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 test/hourly-20260328-0813

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

packages/opencode/test/altimate/dbt-manifest-edge.test.ts (1)

7-22: Consider using the project's tmpdir utility for temporary file management.

The coding guidelines specify using the tmpdir function from fixture/fixture.ts with await using syntax for automatic cleanup. The current manual approach with os.tmpdir() and afterEach works but deviates from the established pattern.

That said, since the file-based cleanup here is simple and self-contained, this is a minor stylistic inconsistency rather than a functional issue.

As per coding guidelines: "Use the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup" and "Always use await using syntax with tmpdir() for automatic cleanup when the variable goes out of scope".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/altimate/dbt-manifest-edge.test.ts` around lines 7 -
22, Replace the manual temp-file handling in this test (the tmpFiles array,
writeTmpManifest function and afterEach cleanup) with the project's tmpdir
helper from fixture/fixture.ts using the "await using" pattern: create a
temporary directory via tmpdir() at test scope, write manifests into that
directory (update references in writeTmpManifest or inline its logic to use the
tmpdir path), and remove the afterEach block so cleanup is automatic when the
await using scope ends; ensure any references to tmpFiles/tmpFile are updated to
use the tmpdir-created paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/test/altimate/dbt-manifest-edge.test.ts`:
- Around line 7-22: Replace the manual temp-file handling in this test (the
tmpFiles array, writeTmpManifest function and afterEach cleanup) with the
project's tmpdir helper from fixture/fixture.ts using the "await using" pattern:
create a temporary directory via tmpdir() at test scope, write manifests into
that directory (update references in writeTmpManifest or inline its logic to use
the tmpdir path), and remove the afterEach block so cleanup is automatic when
the await using scope ends; ensure any references to tmpFiles/tmpFile are
updated to use the tmpdir-created paths.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 036c4923-c05f-4abe-89d9-247ad2ae056d

📥 Commits

Reviewing files that changed from the base of the PR and between 9ba2114 and c53bf3a.

📒 Files selected for processing (2)

• packages/opencode/test/altimate/dbt-manifest-edge.test.ts
• packages/opencode/test/altimate/finops-recommendations.test.ts

[anandgupta42]

Consolidated into #545