AltimateAI / AltimateAI/altimate-code

BigQuery finops: UX + robustness follow-ups from v0.6.1 review

Aberta
#754 0 comentários 0 reações 0 responsáveis Ver no GitHub
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

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.