AltimateAI / AltimateAI/altimate-code
BigQuery finops: UX + robustness follow-ups from v0.6.1 review
- Linguagem predominante
- TypeScript
- Estrelas
- 811
- Forks
- 134
- Merge médio
- 3d 2h
- PRs com merge (30d)
- 50
Descrição
Deferred items surfaced during the v0.6.1 release review. All relate to the BigQuery finops multi-region work shipped in #739 — the fix is correct and shipped, these are UX and robustness follow-ups that were out of scope for a patch release.
## UX / visibility
**1. Surface queried region in BQ finops tool responses.** `sanitizeBqRegion` silently falls back to `us` when `location` is missing on the warehouse config. The agent has no way to see which region was actually queried, so an EU user with an unset `location` sees empty results and can't tell whether that's "no activity" or "wrong region." Proposal: add a `bq_region` / `region_used` field to the top-level response of `finops_query_history`, `finops_analyze_credits`, `finops_warehouse_advisor`, `finops_role_access`, and `finops_unused_resources` on the BigQuery branch.
**2. Warn at `warehouse_add` time when a BigQuery connection is registered without `location`.** Non-fatal warning, not an error. Single log line pointing at where `location` is documented.
**3. Friendlier error for `finops_unused_resources` 403 on BigQuery.** `INFORMATION_SCHEMA.TABLE_STORAGE` is org-level and most project-scoped service accounts 403 on it. Currently users get a raw exec error; detect the permission class and point them at `bigquery.resourceAdmin`.
**4. Region-hint on BQ error messages.** In the three `catch (e)` blocks across `query-history.ts`, `credit-analyzer.ts`, `warehouse-advisor.ts`, if the error mentions `region-` or `Not found`, append `(queried region-${region}; set "location" on the warehouse config to change this)`.
## Robustness
**5. Distinguish "no location provided" vs "location provided but sanitized to empty".** `sanitizeBqRegion` currently returns `"us"` in both cases. Compliance concern for EU/APAC tenants where an invalid `location` silently downgrades to US. Proposal: return `"us"` when input is `undefined`/`null`/`""`, but throw with an actionable message when a non-empty input sanitizes to empty (e.g. someone sets `location: "💥"`).
## Code structure
**6. Refactor BQ finops builders to options-object.** `buildGrantsSql("bigquery", undefined, undefined, 100, "us-central1")` now has 5–6 positional args across several builders. Not blocking, but getting hard to read.
## Test quality
**7. E2E test silent-skip warning.** `finops-bigquery-e2e.test.ts` wraps assertions in `if (result.success)` — if the service account lacks perms or region, the test "passes" with zero assertions run. Emit a warning when `result.success` is false but no rows came back, so hidden no-ops are visible in test output.
## Related
- #740 — PINEAPPLE blocked-term / TEAM_MEMBERS carve-out in anti-slop (already tracked).
Flagged by the five-persona review (CTO, PM, Data Engineer, Tech Lead, Chaos Gremlin) on the v0.6.1 release.
Guia de contribuição
Avaliação
Esta issue ainda não foi avaliada.