Route API catalog & endpoint-list telemetry reads to TimeFusion

[tonyalaribe]

Problem

The API Catalog and endpoint-list pages showed empty event counts, last-seen, and activity charts for projects with TimeFusion reads enabled. Their traffic-stats queries (dependenciesAndEventsCount, endpointRequestStatsByProject) read the Postgres pool unconditionally, while dashboard/AI reads were already migrated to TF.

Fix

TimeFusion is telemetry-only and can't serve apis.* tables, so the single joined query can't just be routed. Each query is split:

1. Metadata (endpoint/host identity) → Postgres, always.
2. Telemetry aggregate (counts, last/first-seen, activity buckets) → withHasqlTimefusion useTf.
3. Merge + sort + paginate in Haskell.

The telemetry SQL uses only TF/DataFusion-proven constructs (floor-epoch bucketing instead of width_bucket/generate_series, per-row services instead of ARRAY_AGG, #>> path extraction instead of chained ->/->>). hostCoalesceExpr switched to the equivalent #>> form. The two page handlers thread enableTimefusionReads from AuthContext.

Also fixes two pre-existing test compile breaks (AgenticSpec, ProcessMessageSpec) from an earlier signature change.

Testing

All 17 ApiCatalogSpec examples pass, including per-host event attribution and pagination disjointness/total. The real-TF dialect can't be exercised locally (tests point the "timefusion" pool at the same Postgres), so that rests on using only prod-TF-proven constructs.

🤖 Generated with Claude Code

[claude]

Review: Route API catalog & endpoint-list telemetry reads to TimeFusion

Overview

Solid fix for the underlying problem: metadata (apis.*) and telemetry aggregates are now fetched from separate stores and merged in Haskell, since TimeFusion can't join against Postgres-only tables. The SQL rewrites (floor-epoch bucketing, #>> path extraction, per-row service instead of ARRAY_AGG) are sensible, engine-portable choices, and the extracted pure helpers (periodWindow, denseBuckets, pickZoned, epochSecs) read cleanly and reuse ordNub/foldMap/record-dot syntax idiomatically per the project's extension set.

Correctness / bugs

• Unbounded fan-out before pagination. endpointRequestStatsByProject now fetches all matching apis.endpoints metadata rows and all matching telemetry-aggregate rows (grouped by url_path, method, service, bucket_idx, uncapped) for the project, then sorts/paginates in Haskell (Endpoints.hs:248-293). The old query pushed OFFSET/LIMIT into Postgres. For a project with a large endpoint catalog or high service/URL cardinality, this is a full-table materialization per request instead of a bounded one. Other queries in this same file guard against unbounded results with an explicit LIMIT (e.g. getUnmergedEndpoints LIMIT 5000, getCanonicalEndpoints LIMIT 10000) — worth adding a similar safety cap here (or at least confirming the "hard to avoid because pagination is by aggregated telemetry count" tradeoff is intentional and monitored).
• Same pattern in dependenciesAndEventsCount (Endpoints.hs:337-375): host count is bounded by distinct hosts so this is lower risk, but the telemetry query is still unbounded by service/bucket_idx cardinality before the final take 200.
• Sort field mismatch (pre-existing, not introduced here, but worth a second look): endpointRequestStatsByProject's "first_seen"/"last_seen" sort options order by m.createdAt (endpoint's own created_at), not by the freshly-computed lastSeen/firstSeen telemetry fields on EndpointRequestStats (Endpoints.hs:289-292). This mirrors the old enp.created_at ASC/DESC behavior, so it's not a regression, but now that real lastSeen is computed per row anyway, it's an easy, low-risk win to sort by the actual telemetry timestamp instead of endpoint-creation time — probably what users expect from a "last seen" sort.

Performance

• Floating-point epoch bucketing (epochSecs :: UTCTime -> Double) is fine for hour/day-scale bucket widths, but worth double-checking there isn't a boundary off-by-one under Double rounding for sub-second timestamps right at bucket edges — the GREATEST(...,0)/LEAST(...,numBuckets-1) clamps already guard the failure mode, so this is a minor note rather than a blocker.
• Map.fromListWith (<>) over Data.Map.Lazy accumulating list-valued entries (Endpoints.hs:271, 358) can build up thunk chains under high per-key row counts. Since the codebase already imports Data.Map.Strict elsewhere, consider switching this aggregation to strict maps to avoid the (admittedly usually small) thunk buildup.

Test coverage

• Good behavioral coverage already exists (ApiCatalogSpec: per-host attribution, pagination disjointness/totals, archiving) and the PR fixes two real compile breaks in AgenticSpec/ProcessMessageSpec.
• The new pure functions (periodWindow, denseBuckets, pickZoned) are prime candidates for small unit tests (e.g. bucket boundary behavior, tie-breaking in pickZoned, DST/leap-second edges) since this bucketing logic used to live in SQL (implicitly tested via integration) and now lives in Haskell.
• As the PR description acknowledges, the actual TimeFusion/DataFusion dialect path isn't exercised locally (test pool points "timefusion" at the same Postgres instance), so useTf = True is untested in CI. That's an infra limitation rather than a review blocker, but flagging it since it's the core of this PR's risk surface — a manual/staging smoke test against real TimeFusion before merge would be reassuring.

Style / conventions

• Nice use of the project's enabled extensions (OverloadedRecordDot, BlockArguments, foldMap-based clause building) to keep the SQL-building code concise, consistent with the rest of the file.
• EndpointTelRow/EndpointMetaRow/HostTelRow are clear, minimal, and the positional HI.DecodeRow field order matches each SELECT — verified this lines up correctly for all three.

Security

No injection concerns — all dynamic SQL fragments go through bound #{} interpolation; the only raw-text fragments (hostCoalesceExpr, archivedHostClauseSql) are static/enum-driven, not user input.

Overall: a well-reasoned split of an unsupportable single-query design, with clean pure-function extraction. Main thing I'd want addressed before merge is bounding the now-unbounded fetch-then-paginate queries for large projects, and ideally a quick staging check against real TimeFusion given the local test limitation.

[tonyalaribe]

Thanks for the review. Addressed / responded per point:

#>> path extraction is NOT engine-portable — this was the one thing the review got backwards, and I verified it live. I spun up a real TimeFusion instance (debug binary + MinIO) and ran the exact queries. attributes #>> '{http,route}' errors with Operator #>> is not yet supported — TF's VariantAwareExprPlanner only rewrites chained ->/->> on the Variant attributes column. Fixed in 02f2247 (reverted hostCoalesceExpr and the route expr to ->/->>) and confirmed both queries now return correct rows on real TF, including the deep attributes->'net'->'host'->>'name' chain, split_part, (kind='client') boolean, and floor-epoch bucketing. Same commit also restructured to a filtered CTE + outer GROUP BY on CTE column names (DataFusion won't group by same-level output aliases), dropped LEAST/GREATEST (denseBuckets ignores out-of-range indices), and switched to integer epoch params + inline ISO timestamp literals matching buildDateRange.

Lazy Map thunks — fixed in 9be6c3c, switched the whole Data.Map import to Data.Map.Strict.

Unbounded fetch-then-paginate — real tradeoff, but note the old single query already had to aggregate + sort all matching endpoints in Postgres before OFFSET/LIMIT (you can't ORDER BY events DESC without scanning them all), so DB-side work is unchanged; the new cost is transferring the metadata rows to the app + an in-memory sort, bounded by the project's endpoint catalog. I deliberately did not add an arbitrary LIMIT cap here: unlike the sibling getUnmergedEndpoints/getCanonicalEndpoints queries, an unordered cap would silently drop the highest-traffic endpoints from an events-sorted page. The correctness-preserving fix at scale is telemetry-driven ranking (rank the bounded telemetry set first, fetch metadata only for the page) — noting as a follow-up rather than shipping a cap that corrupts the default sort.

Sort by createdAt vs telemetry lastSeen — pre-existing; the handlers only ever pass events/name sorts today, so the first_seen/last_seen branches are currently unreachable. Left as-is to keep the change surgical.

useTf=True untested in CI — covered by the manual real-TF run above. The local test pool still points "timefusion" at the same Postgres, so CI exercises the merge/sort/pagination logic (17 ApiCatalogSpec examples green) but not the dialect; the live run covers the dialect surface.

[claude]

Review

Nice write-up in the PR description — the metadata/telemetry split is a reasonable way to work around TF not serving apis.* tables, and the follow-up commit fixing the #>>/CTE issues against real TF shows this was actually validated, not just written and hoped for.

Bug: empty-string service names are no longer filtered out

Both endpoint_stats/host_stats CTEs previously excluded empty-string services explicitly:

COALESCE(ARRAY_AGG(DISTINCT service) FILTER (WHERE service IS NOT NULL AND service != ''), ARRAY[]::text[])
-- and for hosts:
... WHERE service IS NOT NULL AND service != '' AND service != host ...

The new Haskell-side merge only does:

services = V.fromList $ ordNub $ mapMaybe (.service) rs                       -- endpointRequestStatsByProject
services = V.fromList $ filter (/= h) $ ordNub $ mapMaybe (.service) rs       -- dependenciesAndEventsCount

mapMaybe (.service) drops NULL/Nothing, but not Just "". resource___service___name can be an empty string in real data (there's precedent for this exact filter elsewhere, e.g. Telemetry.hs), so this can now surface a blank entry in the services facet/chips where it was previously excluded. Suggest:

services = V.fromList $ ordNub $ filter (not . T.null) $ mapMaybe (.service) rs
services = V.fromList $ filter (`notElem` [h, ""]) $ ordNub $ mapMaybe (.service) rs

Performance: endpoint metadata query lost its LIMIT/OFFSET push-down

endpointRequestStatsByProject's Postgres query previously applied ORDER BY ... OFFSET #{offset} LIMIT #{perPage} itself. Now the apis.endpoints query fetches every matching endpoint for the project (unbounded), and pagination/sorting happens in Haskell after merging with the full telemetry aggregate (one row per url_path/method/service/bucket). For projects with a large endpoint surface, that's the full row set + full bucketed telemetry shipped over the wire and held in memory on every page load, rather than Postgres handling it in one round trip. This is an inherent consequence of splitting the join (fair trade-off given TF's constraints), but worth a conscious call-out/monitoring, since it changes scaling from "bounded by page size" to "bounded by project endpoint count." (dependenciesAndEventsCount's host-list version is less concerning since host counts are typically much smaller, and the old query already had to materialize the full host×bucket join before its own LIMIT.)

Succinctness: pickZoned reimplements maximumBy

pickZoned :: (UTCTime -> UTCTime -> Ordering) -> [Maybe ZonedTime] -> Maybe ZonedTime
pickZoned cmp zs = case catMaybes zs of
  [] -> Nothing
  (x : xs) -> Just $ foldl' (\a b -> if cmp (zonedTimeToUTC a) (zonedTimeToUTC b) == LT then b else a) x xs

This is a hand-rolled maximumBy. Given OverloadedRecordDot/Relude are already in play, this could collapse to:

pickZoned cmp = fmap (maximumBy (cmp `on` zonedTimeToUTC)) . nonEmpty . catMaybes

using maximumBy/on/nonEmpty from base — fewer lines, no manual fold, and it reads as "pick the extremum" rather than a loop you have to trace through.

Maintainability: undocumented reason for raw literal timestamps

startSql/windowStartSql inline the window-start timestamp as a rawSql string literal instead of a bound #{...} parameter. Per the second commit's message this is deliberate — it mirrors "the proven buildDateRange pattern" for TF compatibility — but that reasoning isn't captured in a source comment, unlike hostCoalesceExpr, which does explain its TF-specific constraint inline. Since this is exactly the kind of thing a future cleanup pass could "simplify" back to a bound parameter and silently break the TF path, it'd be worth a one-line comment at the periodWindow/startSql call sites explaining why it must stay a literal.

Minor

• EndpointTelRow.bucketIdx :: Int vs HostTelRow/cnt :: Int64 for the same bigint SQL type — harmless but inconsistent; Int64 everywhere would read as more intentional.
• Test coverage gap is self-acknowledged in the PR description (TF dialect can't be exercised locally). Given the "timefusion" pool points at the same Postgres in tests, it'd still be worth one test that calls these functions with useTf = True just to exercise the Labeled/withHasqlTimefusion wiring end-to-end — cheap insurance against a wiring regression that useTf = False tests can't catch.

Overall solid, well-scoped change — the main actionable item is the services-array empty-string regression; the rest is lower-priority polish.

[claude]

Review

Solid piece of work — cleanly splits identity (Postgres) from telemetry (TF-routable), and the SQL rewrite to TF-safe constructs (epoch-floor bucketing, no width_bucket/generate_series/ARRAY_AGG) is well reasoned and already verified against a live TF instance per the PR description.

Bug: empty-string service no longer filtered out

Both services fields silently regress vs. the old SQL:

-- src/Models/Apis/Endpoints.hs:292
services = V.fromList $ ordNub $ mapMaybe (.service) rs

-- src/Models/Apis/Endpoints.hs:377
services = V.fromList $ filter (/= h) $ ordNub $ mapMaybe (.service) rs

The original SQL excluded empty strings explicitly: FILTER (WHERE service IS NOT NULL AND service != '') (and AND service != host for hosts). mapMaybe (.service) rs only drops Nothing, not Just "". Since resource___service___name can legitimately be an empty string (it's treated as falsy via NULLIF(...,'') in hostCoalesceExpr just above), rows with an empty service name will now leak a blank entry into the catalog/endpoint-list "services" badges. Suggest:

services = V.fromList $ ordNub $ filter (not . T.null) $ mapMaybe (.service) rs
services = V.fromList $ filter (\s -> s /= h && not (T.null s)) $ ordNub $ mapMaybe (.service) rs

Succinctness: pickZoned can drop to a one-liner

pickZoned :: (UTCTime -> UTCTime -> Ordering) -> [Maybe ZonedTime] -> Maybe ZonedTime
pickZoned cmp zs = case catMaybes zs of
  [] -> Nothing
  (x : xs) -> Just $ foldl' (\a b -> if cmp (zonedTimeToUTC a) (zonedTimeToUTC b) == LT then b else a) x xs

plus the compare / flip compare juggling at the two call sites, can be replaced with Data.List.maximumBy + comparing (both already used elsewhere in this codebase, e.g. Pkg/PatternMerge.hs, BackgroundJobs.hs):

lastSeen   = pickZoned zonedTimeToUTC (map (.lastSeen) rs)                 -- max
first_seen = pickZoned (Down . zonedTimeToUTC) (map (.firstSeen) rs)       -- min

pickZoned :: Ord b => (ZonedTime -> b) -> [Maybe ZonedTime] -> Maybe ZonedTime
pickZoned key zs = case catMaybes zs of
  [] -> Nothing
  xs -> Just $ maximumBy (comparing key) xs

Fewer moving parts, no custom fold, and it reads as "max by this key" instead of requiring the reader to work out what flip compare buys you.

Minor duplication

• The "'" <> toText (iso8601Show t) <> "'" literal-embedding is repeated for startSql and windowStartSql. A tiny local isoLit :: UTCTime -> HI.Sql (mirroring the existing tsLit in Web/FacetsFallback.hs) would remove the duplication — minor at 2 call sites, but worth it if a third shows up.
• endpointRequestStatsByProject and dependenciesAndEventsCount now share near-identical shape: build a Map.fromListWith (<>) telemetry map, then a mk that sums cnt, picks last/first seen, builds dense buckets, and dedupes services. Not blocking, but if a third telemetry-merge call site shows up, it's worth factoring into one generic aggregateTelemetry helper rather than a third copy.

Performance

Pagination moved from SQL OFFSET/LIMIT to Haskell take/drop after fetching all matching endpoint/host metadata rows per request. This is inherent to merging two stores and hard to avoid here, but it does change the scaling characteristics for projects with very large endpoint/host counts — previously only one page of metadata rows crossed the network per request; now the full filtered set does on every page load.

Test coverage

Correctly called out in the PR description already: the TF dialect can't be exercised in CI (the "timefusion" pool points at the same Postgres locally), so the #>>-vs-->/->> planner quirk and the integer-epoch bucketing are only verified by the author's manual run against live TF. Worth a follow-up ticket to wire an integration test against a real (or better-mocked) TF instance, given this is exactly the kind of query-dialect mismatch that bit this PR twice already (commits 2 and 3 in this branch).

Other

• Field ordering between each SELECT and its corresponding DecodeRow record (EndpointMetaRow, EndpointTelRow, HostTelRow) is correct — checked column-by-column.
• Sort orders (first_seen/last_seen/default) and the default Down totalRequests, urlPath tiebreak match the old SQL ORDER BY clauses exactly.
• No injection concerns — all user-influenced values (pHostM, searchM, sortM) go through #{} bound interpolation; the only raw-SQL-embedded values (startSql, windowStartSql) are server-computed UTCTimes, not user input.