feat/posthog-org-360

cred_consumidos shown without status guard — stale value for churned orgs

infra/posthog/insights.tf

mrr_creditos_mxn is correctly zeroed with `if(l.ledger_status = 'active', ..., 0)`, but `cred_consumidos` is `round(l.credits_consumed, 0)` with no guard. A churned org will show mrr=0 alongside a non-zero credit consumption from the last active period — stale and misleading for ops reading the table. Fix: apply `if(l.ledger_status = 'active', round(l.credits_consumed, 0), NULL)` or `0` to stay consistent with the MRR column.

argMax(x, billing_period_start) non-deterministic when two ledger rows share the same period

infra/posthog/insights.tf

ClickHouse `argMax` picks an arbitrary row on ties. A late correction row with the same `billing_period_start` as the original (e.g. a mid-period credit adjustment) ties and resolves unpredictably — which is the exact scenario the change was designed to fix. Use a tuple to break ties: `argMax(x, (billing_period_start, created_at))`. This preserves the intent (prefer the most recent period) while deterministically preferring the latest-inserted row on period ties.

pct_entregados denominator excludes non-data-error RPU failures — overstates delivery health

infra/posthog/insights.tf

The denominator is `rpus_delivered_30d + rpus_failing_30d` where `rpus_failing_30d` counts only RPUs with RPU_INVALID/SERVICE_NAME_MISMATCH that never delivered. RPUs failing for other reasons (auth errors, timeouts, CFE outages) appear in neither bucket. An org with 5 delivered + 5 auth-failing RPUs shows pct_entregados = 1.0 (falsely claiming 100% delivery health). The metric name implies a comprehensive health rate, but the denominator is scoped to data-quality errors only. Consider documenting this explicitly or adding an `rpus_uncategorized_30d` column for the hidden third state.

ratio_activos cap (least(..., 1.0)) silently hides data anomalies

infra/posthog/insights.tf

A ratio > 1.0 (active users > member count) is a data signal that the membership snapshot may be stale relative to the 30d engagement window. The cap hides it. Not a bug in the displayed value — capping is the right UX choice — but without a flag column, ops cannot detect orgs where the underlying data diverge significantly (e.g. ratio was 3.0 before capping).

No security issues found

No hardcoded credentials, SQL injection vectors, or PII exposure in the diff. HogQL accesses org-level aggregates only.

LIMIT 500 undocumented — no comment explaining the bound

infra/posthog/insights.tf

The query uses `LIMIT 500` with no inline comment. Reviewers cannot tell whether this is a PostHog DataVisualizationNode cap, a performance guard, or an expected org count ceiling. Other scoped insights (e.g. `rpus_invalidos_por_org` with `LIMIT 25`) have implicit rationale from context. Add a comment: `-- PostHog table UI cap; expected <200 active orgs` or similar.

Mixed Spanish/English in inline SQL comments within org_360

infra/posthog/insights.tf

The HogQL block has Spanish (`-- miembros vigentes (excluye removidos)`) and English (`-- engagement: usuarios con eventos de PRODUCTO Batu`) comments mixed together. Surrounding Terraform block-level comments and other HogQL resources use Spanish throughout. Harmonise to Spanish for consistency.

ratio_activos uses least() guard; pct_entregados uses nullIf() — inconsistent strategy for ratio columns

infra/posthog/insights.tf

`ratio_activos` is capped with `least(..., 1.0)` to guarantee the value never exceeds 1. `pct_entregados` relies on structural mutual-exclusivity (delivered+failing ≤ total) rather than a least() guard, but this is not documented. Either add a comment explaining why `pct_entregados` does not need the cap, or apply `least(..., 1.0)` consistently across both ratio columns for defensive consistency.

Staging and prod plan steps use continue-on-error: true — plan failures are non-blocking

.github/workflows/pr-checks.yml

The `posthog-checks` CI job applies Terraform to dev (blocking) but uses `continue-on-error: true` for stg and prod plan steps. A plan error (API key issue, provider regression) passes silently and is surfaced only as a PR comment. Consider removing `continue-on-error` from at least the stg plan step so structural breakage is caught before merge.

CI dev-apply validates HogQL syntax at upsert time — adequate for analytics IaC

The `posthog-checks` job runs `terraform apply` against the dev PostHog project on every PR touching `infra/posthog/`. PostHog's API rejects invalid HogQL at upsert, so the dev apply acts as a live integration gate. No separate unit tests exist for the embedded queries — accepted for analytics IaC.

pct_entregados returns NULL (not 0) when org has no delivery data

infra/posthog/insights.tf

When the LEFT JOIN on `d` yields no row, the numerator `coalesce(d.rpus_delivered_30d, 0)` evaluates to 0, but the denominator `nullIf(d.rpus_delivered_30d + d.rpus_failing_30d, 0)` evaluates to `nullIf(NULL, 0)` = NULL — so the result is NULL/NULL division = NULL, not 0. This is semantically ambiguous: a reader sees NULL and doesn't know whether the org has zero subscriptions or whether the data is missing. Fix: `round(coalesce(d.rpus_delivered_30d, 0) / nullIf(coalesce(d.rpus_delivered_30d, 0) + coalesce(d.rpus_failing_30d, 0), 0), 2)` or document NULL = no subscriptions.

Redundant if(isNotNull(e.last_active), toDate(...), NULL) — simplify to toDate(e.last_active)

infra/posthog/insights.tf

`toDate(NULL)` returns NULL in HogQL/ClickHouse, so the explicit `if(isNotNull(...), ..., NULL)` guard is a no-op. Simplify to `toDate(e.last_active) AS ultima_actividad`. This was changed from `e.last_active > 0` (a type-coercion timestamp guard) to `isNotNull` — the `isNotNull` is correct in spirit but the wrapper is redundant.

collect_jobs_30d counts raw job executions including retries — semantics should be documented

infra/posthog/insights.tf

`sum(jobs)` in the outer delivery query sums raw row counts from the inner `count()` per (org_id, rpu). This includes retries (e.g. 5 retries for one RPU = 5 jobs). Given that other insights document retry rates of 5–13x, this metric substantially over-counts unique collection attempts. The column name `jobs_recoleccion_30d` does not signal retry inclusion. Add a comment or rename to `job_executions_30d` to avoid confusion.

Missing rpus_uncategorized_30d column — RPUs failing for non-data reasons are invisible

infra/posthog/insights.tf

RPUs that had jobs but neither succeeded nor had RPU_INVALID/SERVICE_NAME_MISMATCH errors (e.g. auth failures, network timeouts) are not counted in `rpus_entregados_30d` or `rpus_fallando_30d`. They appear in `rpus_suscritos` but disappear from the delivery picture. Adding `countIf(delivered = 0 AND rpu_failed = 0) AS rpus_uncategorized_30d` would expose this third state and make `rpus_suscritos = rpus_entregados + rpus_fallando + rpus_uncategorized` the complete partition.