fix: [#827] finops_* 100% failure — add warehouse-resolver fallback#828
Merged
Conversation
…lback Telemetry (memory/telemetry-analysis-2026-05-21.md, 2,503 machines, 14 days) flagged `finops_analyze_credits` (8/8) and `finops_query_history` (1/1) as 100% broken. Every user who tried them hit a dead-end "Credit analysis is not available for unknown warehouses." error with no path forward. Unlike `sql.explain` (which auto-falls-back to `warehouses[0]`), the finops handlers had no resolution logic and returned an unhelpful error whenever the caller omitted the warehouse name, supplied an unknown one, or supplied one whose type wasn't supported. Changes - New shared helper `finops/warehouse-resolver.ts` with a 4-branch resolution contract (ok / not configured / unknown name / unsupported type) and actionable errors that name the available warehouses and suggest `warehouse_add` when nothing is configured. - `credit-analyzer.ts` (analyzeCredits, getExpensiveQueries) and `query-history.ts` (getQueryHistory) wired to the resolver. The dead `getWhType` helpers removed. - `warehouse` parameter made optional on the `finops_analyze_credits` and `finops_query_history` tool schemas; descriptions updated to document the auto-pick behaviour and the full supported-warehouse list. - `CreditAnalysisParams`, `ExpensiveQueriesParams`, `QueryHistoryParams` marked `warehouse?` to match the new optionality contract. - 12 new unit tests in `test/altimate/finops-warehouse-resolver.test.ts` pinning every resolution branch including empty-string and whitespace-only requested values. Expected impact - Error rate on `finops_analyze_credits` and `finops_query_history` drops from 100% to whatever the real operational failure rate is (warehouse unreachable, ACCOUNT_USAGE permission missing, etc.). The resolver no longer masks those as "unknown warehouse" — driver errors pass through with their original message. - Cost-audit wedge demo now works on any tenant with a Snowflake / BigQuery / Databricks warehouse configured, without the LLM needing to guess the exact warehouse name. Validation - `tsgo --noEmit`: pass, zero errors. - `bun test test/altimate/finops-*`: 37 pass / 0 fail (12 new + 25 existing). - `bun test test/altimate/finops-*-e2e.test.ts test/altimate/schema-finops-dbt.test.ts test/altimate/finops-role-access.test.ts`: 85 pass / 67 skip (credentials-gated) / 0 fail. - Marker check: pass — no upstream-shared files touched. Closes #827 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a centralized resolveFinopsWarehouse resolver; makes FinOps ChangesFinOps warehouse resolution and optional parameters
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
Next StepPlease address the failing cases above and re-run verification. |
… handlers + formatter fix + case-insensitivity Multi-model consensus review of the original commit identified four in-scope issues this commit addresses: 1. finops_expensive_queries tool schema not updated (5/6 reviewer consensus) The handler in credit-analyzer.ts was already wired to the resolver, but the tool schema still required `warehouse` and the description claimed "Snowflake only" — leaving the auto-pick fallback unreachable for that tool. Schema and description updated to match the other two tools. 2. Formatter field-name mismatch in finops-query-history.ts and finops-expensive-queries.ts (2/6 independent catches, both MAJOR) The formatters read `summary.avg_execution_time` / `q.execution_time`, but the native handler returns `_sec`-suffixed keys (`avg_execution_time_sec` and per-row `execution_time_sec`). Pre-existing silent data loss: average time line never rendered, per-query exec time always "-". Auto-pick from the prior commit routes more users through these formatters, expanding the impact. Both formatters updated to read the correct keys. 3. Other finops handlers had the same dead-end pattern (Gemini CRITICAL, Kimi MAJOR). warehouse-advisor.ts (adviseWarehouse), unused-resources.ts (findUnusedResources), role-access.ts (queryGrants, queryRoleHierarchy, queryUserRoles) all rewired to use resolveFinopsWarehouse. Their tool schemas (finops-warehouse-advice, finops-unused-resources, three tools in finops-role-access) made warehouse optional. Type signatures in native/types.ts updated to match. role-access uses a SNOWFLAKE_ONLY_TYPES const for queryRoleHierarchy and queryUserRoles. 4. Case-sensitivity bug (DeepSeek unique catch). The driver lookup in registry.ts uses toLowerCase() via DRIVER_MAP, so configs with `type: "Snowflake"` (capitalized) get a working driver — but the resolver's case-sensitive supportedTypes.includes(match.type) failed them with a spurious "not available for Snowflake" error. Resolver now normalizes type via toLowerCase() at every comparison, and the returned `type` field is the lowercased form so downstream SQL-template selectors continue to compare against "snowflake" / "bigquery" / "databricks". Plus two cleanup items from the review: - Removed unused `sanitizeBqRegion` import from query-history.ts (Claude NIT) - Added a code comment in the resolver documenting the intentional warehouse-name disclosure in error messages (resolves the disputed security concern with the synthesis's rationale) Test changes - Added 3 new test cases pinning case-insensitive type matching, including mixed-case (`type: "Snowflake"`), uppercase (`type: "SNOWFLAKE"`), and auto-pick filtering with mixed-case types. - Updated two assertions in schema-finops-dbt.test.ts to match the new "not configured" error message (replacing the less-accurate old "not available for X" — the warehouse never existed, regardless of type). Validation - `tsgo --noEmit`: pass, zero errors. - `bun test test/altimate/finops-*`: 124 pass / 0 fail / 67 skip (credential-gated). - `bun test test/altimate/tool-error-propagation.test.ts test/altimate/simulation-suite.test.ts`: 854 pass / 0 fail. - Marker check: pass — no upstream-shared files touched. Expected telemetry impact unchanged from prior commit: error rate on finops_* tools should drop to operational baseline. With this commit, all five finops handlers benefit, not just the two from the original PR. The formatter fix recovers two display values that have been silently broken for all users since these tools shipped. Closes review feedback from PR #828 multi-model consensus. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Contributor
Author
Review fixes applied (commit ed6f247)Multi-model consensus review surfaced four in-scope issues with the original commit. All are now addressed: Required-before-merge fixes
Strongly-recommended fixes
Other cleanups
Test changes
Validation
Out of scope for this PR (follow-up acceptable)
|
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/src/altimate/native/finops/warehouse-resolver.ts (1)
59-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrim
requestedbefore warehouse lookup.Line 60 uses the raw
requestedvalue, so inputs like" prod_wh "fail lookup even when the warehouse exists. Normalize once and use the trimmed value for lookup and errors.Suggested patch
export function resolveFinopsWarehouse(opts: ResolveOptions): FinopsWarehouseResolution { const { requested, supportedTypes, operationName } = opts + const requestedName = requested?.trim() const all = Registry.list().warehouses const supportedList = supportedTypes.join(", ") @@ - if (requested && requested.trim() !== "") { - const match = all.find((w) => w.name === requested) + if (requestedName) { + const match = all.find((w) => w.name === requestedName) if (!match) { @@ - `Warehouse ${JSON.stringify(requested)} is not configured. ` + + `Warehouse ${JSON.stringify(requestedName)} is not configured. ` +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/native/finops/warehouse-resolver.ts` around lines 59 - 73, The code uses the raw requested string for lookup and error text; trim requested once into a normalized variable (e.g., const requestedTrimmed = requested.trim()) and use that when calling all.find((w) => w.name === requested) (update to requestedTrimmed) and when composing the error message/JSON.stringify so whitespace-surrounded inputs like " prod_wh " correctly match and produce the trimmed value in the error text; update references to requested in this block to use the trimmed variable (including availableNames generation if needed).
🧹 Nitpick comments (1)
packages/opencode/src/altimate/native/finops/role-access.ts (1)
213-221: 💤 Low valueInconsistent error stringification pattern.
The catch blocks here use
String(e), whilequery-history.tsusese instanceof Error ? e.message : String(e). The latter produces cleaner messages forErrorinstances (just the message, not the fullError: ...prefix). Consider aligning for consistency across handlers.Suggested change for consistency
} catch (e) { return { success: false, grants: [], grant_count: 0, privilege_summary: {}, - error: String(e), + error: e instanceof Error ? e.message : String(e), } }Apply similarly to
queryRoleHierarchyandqueryUserRolescatch blocks.Also applies to: 255-262, 296-303
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/altimate/native/finops/role-access.ts` around lines 213 - 221, Replace the inconsistent String(e) error stringification in the catch blocks with the pattern used in query-history.ts so Error instances yield just the message; specifically update the catch return objects in the functions queryRoleHierarchy, queryUserRoles and the other catch blocks in this file (the block that currently returns success:false, grants:[], grant_count:0, privilege_summary:{}, error: String(e)) to set error to e instanceof Error ? e.message : String(e).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/opencode/src/altimate/native/finops/warehouse-resolver.ts`:
- Around line 59-73: The code uses the raw requested string for lookup and error
text; trim requested once into a normalized variable (e.g., const
requestedTrimmed = requested.trim()) and use that when calling all.find((w) =>
w.name === requested) (update to requestedTrimmed) and when composing the error
message/JSON.stringify so whitespace-surrounded inputs like " prod_wh "
correctly match and produce the trimmed value in the error text; update
references to requested in this block to use the trimmed variable (including
availableNames generation if needed).
---
Nitpick comments:
In `@packages/opencode/src/altimate/native/finops/role-access.ts`:
- Around line 213-221: Replace the inconsistent String(e) error stringification
in the catch blocks with the pattern used in query-history.ts so Error instances
yield just the message; specifically update the catch return objects in the
functions queryRoleHierarchy, queryUserRoles and the other catch blocks in this
file (the block that currently returns success:false, grants:[], grant_count:0,
privilege_summary:{}, error: String(e)) to set error to e instanceof Error ?
e.message : String(e).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 67476bbc-f465-4eba-91b8-0ddf36d113d5
📒 Files selected for processing (13)
packages/opencode/src/altimate/native/finops/query-history.tspackages/opencode/src/altimate/native/finops/role-access.tspackages/opencode/src/altimate/native/finops/unused-resources.tspackages/opencode/src/altimate/native/finops/warehouse-advisor.tspackages/opencode/src/altimate/native/finops/warehouse-resolver.tspackages/opencode/src/altimate/native/types.tspackages/opencode/src/altimate/tools/finops-expensive-queries.tspackages/opencode/src/altimate/tools/finops-query-history.tspackages/opencode/src/altimate/tools/finops-role-access.tspackages/opencode/src/altimate/tools/finops-unused-resources.tspackages/opencode/src/altimate/tools/finops-warehouse-advice.tspackages/opencode/test/altimate/finops-warehouse-resolver.test.tspackages/opencode/test/altimate/schema-finops-dbt.test.ts
This was referenced May 23, 2026
…stency + formatter completeness Third-pass on PR #828 after follow-up code review. Three follow-ups in scope. 1. Test determinism (MINOR per reviewer) `test/altimate/schema-finops-dbt.test.ts` was reading the warehouse resolver's error message verbatim ("not configured") to assert the unknown-warehouse path. That message only appears when the registry has at least one warehouse configured — if the test ran in a fresh sandbox or hermetic CI, the resolver would return "...none are set up..." and the assertion would silently fail. Added a beforeAll / afterAll block that captures the prior registry state, pins a known two-warehouse config for the test, and restores afterwards. 2. Error stringification consistency (MINOR) The first PR commit upgraded most `catch (e) { error: String(e) }` to `e instanceof Error ? e.message : String(e)`, but role-access.ts was missed (three handlers). Normalized all three to match the credit-analyzer / query-history pattern — driver errors now surface as `"...message..."` rather than `"Error: ...message..."`. 3. Formatter completeness (NIT) `finops-query-history.ts` formatter surfaced only avg_execution_time even though the underlying handler emits total_execution_time_sec and error_count too. Added both: total exec time always shown; error count shown only when > 0 to avoid cluttering the typical 0-error case. Validation - `tsgo --noEmit`: pass. - `bun test test/altimate/schema-finops-dbt.test.ts test/altimate/finops-warehouse-resolver.test.ts test/altimate/finops-recommendations.test.ts`: 95 pass / 0 fail. - Marker check: pass (no upstream-shared files in these paths). Reviewer note - GPT-5.4 second-opinion was unavailable (OpenRouter credit exhaustion). Single-reviewer Claude analysis. Deferred to follow-up - Surface auto-picked warehouse name in handler success metadata (reviewer NIT — separate ROADMAP item already tracked). - Restore role-access BigQuery-IAM / Databricks-Unity hints lost in the resolver rewrite (reviewer NIT — would require per-operation `hint` field on `ResolveOptions`, separate enhancement). - Tool-level integration test for the auto-pick path (reviewer suggestion — requires Dispatcher mock scaffolding). Closes review feedback for PR #828 (third pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Contributor
Author
Third-pass review fixes applied (commit 76ce370)Follow-up consensus review on the combined diff (original c4ab025 + ed6f247 follow-up). 2 MINOR + 1 NIT. All addressed: Polish (MINOR)
NIT
Validation
Reviewer noteGPT-5.4 second-opinion was unavailable (OpenRouter credit exhaustion). Single-reviewer Claude analysis. Deferred to follow-up
|
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…ast 2 String(e) Fourth pass on PR #828. Re-review found one MAJOR formatter bug missed by the previous formatter audit, plus the last two String(e) instances that the third pass left behind. 1. MAJOR — formatter field-name mismatch in finops-warehouse-advice Same class of bug as the one commit ed6f247 fixed for finops-query-history.ts and finops-expensive-queries.ts — the formatter read keys the handler never emits: - Performance summary: read `avg_execution_time`, `avg_queue_time`, `status`, `health`; handler emits `query_count`, `avg_time_sec`, `p95_time_sec`, `avg_bytes_scanned`, `total_credits`. - Load metrics: read `warehouse_size`, `avg_load`, `peak_load`, `utilization`; handler emits `avg_concurrency`, `avg_queue_load`, `peak_queue_load`, `sample_count`. Every value rendered as "-" regardless of input. Auto-pick from the prior commits expanded the user population hitting this formatter. Fix: rewrote both table headers and row loops to read the actual handler keys. Added inline comments pointing back at the previous instance of this bug pattern so the next reviewer doesn't have to discover it independently. 2. MINOR — null-guard on result.recommendations `result.recommendations.length` was read directly. If a future handler variance or partial success ever dropped the field, the tool throws a TypeError instead of returning a structured error. Coalesced to `[]`. Also defensive-defaulted warehouse_load and warehouse_performance for the same reason. 3. MINOR — last two String(e) call sites The third pass normalized role-access.ts but missed: - unused-resources.ts:234 - warehouse-advisor.ts:301 Both fixed in this commit. All five finops handlers now use the same `e instanceof Error ? e.message : String(e)` pattern, so driver errors no longer carry "Error:" prefixes inconsistently. Validation - `tsgo --noEmit`: pass. - `bun test test/altimate/finops-*.test.ts test/altimate/schema-finops-dbt.test.ts`: 95 pass / 0 fail / 67 skip (credential-gated). - Marker check: pass. Reviewer note - GPT-5.4 and Kimi K2.5 second-opinions did not produce output this round either — kilo CLI was preempted by other concurrent jobs on the same OpenRouter quota. Single-reviewer Claude analysis. Deferred to follow-up - Tool-level integration test for each formatter (would have caught all three formatter bugs at once — tracked as a separate quality bar enhancement). - warehouse_filter param silently ignored for BigQuery/Databricks (description already documents Snowflake-only; tracking as NIT). Closes review feedback for PR #828 (fourth pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Contributor
Author
Fourth-pass review fix (commit b915703)Re-review (after credit restoration) caught one MAJOR bug the previous formatter audit missed, plus the last 2 Required fix (MAJOR)
Polish
Validation
Reviewer noteExternal models did not produce output this round either — kilo CLI was preempted by other concurrent jobs on the same OpenRouter quota despite the credit restoration. Single-reviewer Claude analysis. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Contributor
Author
Re-review verdict already addressedBackground reviewer finally returned (kimi/external models timed out and was killed; this is a single-Claude review). The three issues flagged in the synthesis were already addressed in the existing commit chain — reviewer was looking at pre-
No code change in this comment — verified all three are already on the branch. The synthesis was correct in spirit but operating on stale diff. |
…atching Cubic flagged that valid warehouse names with leading/trailing whitespace were incorrectly reported as unknown. LLMs occasionally surface names with stray whitespace from prompt copy-paste or YAML/JSON serialization edge cases — without the trim, " prod_wh" reports as unknown even though "prod_wh" is configured. The trim is on the lookup key only; configured names are stored as-is. Added a 4-variant test that pins the contract (leading-only, trailing-only, both-sides, tabs+newlines) so a refactor can't silently drop the trim. 15/15 resolver tests pass; marker check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
anandgupta42
added a commit
that referenced
this pull request
May 24, 2026
Telemetry-driven hardening release. Five P0 fixes from the telemetry-analysis-2026-05-21 pass (#828 finops auto-pick, #831 project_scan defensive spawn, #833 build-agent name normalization, #837 tokens_input_total semantics, #839 webfetch 404 cache) plus a multi-persona pre-release review that surfaced privacy/security hardening: credential strip on git.remoteUrl, masked git stderr, agent-name input hardening (control-char strip + NFKC + length cap), auth-bearing query-param strip from the webfetch cache key, cache-hit error prefix, warehouse_filter param description disambiguation, DEFAULT_FINOPS_TYPES single source of truth, and docs alignment for the auto-pick behavior. 34 adversarial tests pin every invariant. 4 deferred items filed as #840 #841 #842 #843. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the
finops_*tools that telemetry showed at 100% error rate across the last 14 days (memory/telemetry-analysis-2026-05-21.md, 2,503 machines). Specifically:finops_analyze_credits: 8/8 calls failfinops_query_history: 1/1 call failsThe dominant failure mode was the LLM either omitting
warehouseor passing an unknown / unsupported name, and getting back a dead-end"Credit analysis is not available for unknown warehouses."error with no path forward.This PR adds a shared
resolveFinopsWarehousehelper that:warehouse(matches the pattern already used bysql.explain)warehouse_add, listing the supported typeswarehouseis now an optional parameter on both tool schemas, so the LLM can call them without guessing a name.This is on the critical path for the cost-audit wedge use-case — the workflow cannot complete without these tools working.
Type of change
Issue for this PR
Closes #827
How did you verify your code works?
tsgo --noEmit: pass, zero errorstest/altimate/finops-warehouse-resolver.test.ts: 12 tests covering every resolution branch (ok / unknown name / unsupported type / no config / empty string / whitespace-only / auto-pick / per-operation type lists / operation-name surfacing)bun test test/altimate/finops-*: 37 pass / 0 fail (12 new + 25 existing)bun teston the credentials-gated e2e suites (finops-snowflake-e2e,finops-bigquery-e2e,finops-databricks-e2e,schema-finops-dbt,finops-role-access): 85 pass / 67 skip (credential-gated) / 0 fail — no regressionsbun run script/upstream/analyze.ts --markers --base main --strict): pass — no upstream-shared files touchedChecklist
/consensus:code-review— deferred until reviewer requests; this is a small, well-tested fixFiles changed (7):
packages/opencode/src/altimate/native/finops/warehouse-resolver.ts(new)packages/opencode/src/altimate/native/finops/credit-analyzer.tspackages/opencode/src/altimate/native/finops/query-history.tspackages/opencode/src/altimate/native/types.ts(warehousemade optional in 3 param interfaces)packages/opencode/src/altimate/tools/finops-analyze-credits.tspackages/opencode/src/altimate/tools/finops-query-history.tspackages/opencode/test/altimate/finops-warehouse-resolver.test.ts(new)Expected telemetry impact: error rate on
finops_analyze_creditsandfinops_query_historyshould drop from 100% to whatever the real operational failure rate is (warehouse unreachable, ACCOUNT_USAGE permissions missing, etc.). The resolver no longer masks driver errors as "unknown warehouse" — they pass through with their original message.Summary by cubic
Adds a shared warehouse resolver across all
finops_*tools to auto-pick a compatible warehouse, trim whitespace in requested names, and return clear, actionable errors; makeswarehouseoptional and fixes formatter/output issues so timings and driver messages are correct. Fixes #827.resolveFinopsWarehouseto credits, expensive queries, query history, role/access, warehouse advice, and unused resources; auto-picks when omitted, trims requested names, matches types case-insensitively, and returns clear errors with available names and supported types (credits/expensive/unused/advice: Snowflake/BigQuery/Databricks; history: +Postgres/+ClickHouse; role hierarchy/user roles: Snowflake only).warehouseoptional in native params and tool schemas; descriptions updated to document fallback.query_previewand useexecution_time_sec; query history readsavg_execution_time_sec,total_execution_time_sec, anderror_count; warehouse advice uses real fields (avg_time_sec,p95_time_sec,avg_concurrency,avg_queue_load,peak_queue_load,sample_count) and guards nulls.e.messageacross all handlers; internal template-missing paths report clear “internal error” messages instead of unsupported-type text.schema-finops-dbtwith a pinned registry. Expected impact: finops error rates drop to real operational failures (permissions/connectivity) with original driver messages preserved.Written for commit 05739c5. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Improvements
Tests