Skip to content

feat: fast and full autocomplete - #2128

Merged
kodiakhq[bot] merged 17 commits into
mainfrom
aaron/feat-autocomplete
May 8, 2026
Merged

kodiakhq[bot] merged 17 commits into
mainfrom
aaron/feat-autocomplete

Conversation

@knudtty

@knudtty knudtty commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds metadataMaterializedViews config to log and trace sources, storing key rollup and KV rollup table references directly on the source instead of hardcoding them by table name.
  • Uses rollups by default in autocomplete field and key fetching methods, allowing for much better autocomplete
  • tcFromSource() populates metadataMVs on TableConnection from the source's metadataMaterializedViews, so all downstream consumers (field discovery, autocomplete, filters) pick it up automatically.
  • Source inference (inferTableSourceConfig) auto-detects rollup tables by checking if {tableName}_key_rollup_15m and {tableName}_kv_rollup_15m exist in ClickHouse before adding the config.
  • Adds a MetadataMaterializedViewsFormSection UI to the log and trace source forms for manual configuration.
  • Updates all default source configs (.env.development, docker-compose.yml, entry.local.base.sh) to include metadataMaterializedViews for the standard OTel log and trace sources.

Screenshots or video

export-1776301077703.mp4

How to test locally or on Vercel

  1. Run this locally. Clear your existing volumes to start from scratch.
  2. Fire it up via yarn dev
  3. Verify logs and traces have the key and kv rollups as part of the sources
  4. Try out autocomplete on either the logs our traces sources

References

Closes HDX-3931

@vercel

vercel Bot commented Apr 16, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment May 8, 2026 8:33pm

Request Review

@knudtty
knudtty requested review from a team and pulpdrew and removed request for a team April 16, 2026 01:36
@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Apr 16, 2026
@github-actions

github-actions Bot commented Apr 16, 2026 •

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.

Why this tier:

  • Critical-path files (4):
    • docker/hyperdx/entry.local.base.sh
    • docker/otel-collector/schema/seed/00006_otel_logs_rollups.sql
    • docker/otel-collector/schema/seed/00007_otel_traces_rollups.sql
    • packages/api/src/routers/external-api/v2/sources.ts
  • Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 25
  • Production lines changed: 1489 (+ 468 in test files, excluded from tier calculation)
  • Branch: aaron/feat-autocomplete
  • Author: knudtty

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@changeset-bot

changeset-bot Bot commented Apr 16, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: da4cc4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@hyperdx/common-utils Minor
@hyperdx/api Minor
@hyperdx/app Minor
@hyperdx/otel-collector Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@knudtty knudtty changed the title Aaron/feat autocomplete feat: fast and full autocomplete Apr 16, 2026
@github-actions

github-actions Bot commented Apr 16, 2026 •

Copy link
Copy Markdown
Contributor

PR Review

  • 🐛 getMapKeys empty rollup result poisons the main-table fallback (packages/common-utils/src/core/metadata.ts:368-426) → The rollup path and the "Original path: scan main table" use the same cacheKey when metadataMVs && alignedDateRange. When the rollup query returns [] (e.g. column not yet rolled up, or rollup query throws and the catch returns []), it's cached at cacheKey, and the trailing cache.getOrFetch(cacheKey, ...) for the main scan hits the cached [] and never runs. Use a distinct cache key for the rollup vs. the main-scan path (e.g. suffix the rollup key with .rollup), or skip caching empty rollup results.

  • ⚠️ Granularity inference in inferTableSourceConfig is effectively dead (packages/app/src/source.ts:354-387) → inferGranularityFromMVSelect(rollupMeta.keyMeta.as_select) is called against the destination tables (${tableName}_key_rollup_15m/_kv_rollup_15m), which are SummingMergeTree tables — not MVs. system.tables.as_select is empty for non-MV tables, so inference always returns undefined and the '15 minute' fallback always wins. Either query the corresponding _mv view's as_select (e.g. otel_logs_attr_kv_rollup_15m_mv), or drop the inference call and just hardcode '15 minute' to make the intent clear.

  • ⚠️ OpenAPI examples show 15m, schema requires 15 minute (packages/api/openapi.json:374,403, packages/api/src/routers/external-api/v2/sources.ts) → MetadataMaterializedViewsSchema.granularity is SQLIntervalSchema (/^\d+ (second|minute|hour|day)$/). mapGranularityToExternalFormat rewrites to 15m on GET, but the docs example reads as if 15m is also a valid input. There's no inverse mapping, and materializedViews.minGranularity already documents the enum. Tighten the metadataMaterializedViews example/description to reflect the same external format and make round-tripping unambiguous (note: v2 sources router is GET-only today, so this is doc-only for now).

  • 🧹 MetadataMaterializedViewsFormSection "Add" seeds invalid empty strings (packages/app/src/components/Sources/SourceForm.tsx:636-650) → Clicking "Add Metadata Materialized Views" sets granularity: '', which fails SQLIntervalSchema until the user picks one. Consider seeding the granularity to a sensible default (e.g. '15 minute') so partial saves don't surface a confusing validation error on an unrelated-looking field.

@knudtty
knudtty removed the request for review from pulpdrew April 16, 2026 01:40
@knudtty
knudtty marked this pull request as draft April 16, 2026 01:41
@github-actions

github-actions Bot commented Apr 16, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 167 passed • 3 skipped • 1083s

Status Count
✅ Passed 167
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@knudtty
knudtty marked this pull request as ready for review April 17, 2026 18:11
@knudtty
knudtty requested review from a team and bot-hyperdx and removed request for a team and bot-hyperdx April 17, 2026 20:52
@@ -0,0 +1,171 @@
-- +goose Up

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general idea:

  1. A row is inserted to otel_logs
  2. Inserting a row to otel_logs triggers otel_logs_kv_rollup_15m_mv, extracting all map keys and map values, as well as column identifiers and column values, and inserts into otel_logs_kv_rollup_15m
  3. Inserting a row to otel_logs_kv_rollup_15m_mv triggers otel_logs_key_rollup_15m_mv extracting the keys and counts, enabling fast and slim key querying

The otel_logs_key_rollup_15m_mv and otel_logs_kv_rollup_15m_mv tables are SummingMergeTree, which will collapse rows that have the same PK and sum numbers in the process. This allows us to build not only a quick list to do DISTINCT queries on, but also get the counts. This will be nice if we want to enable users looking for frequent or infrequent values.

The same idea is applied to both log and trace tables

fi
export DEFAULT_CONNECTIONS
export DEFAULT_SOURCES='[{"from":{"databaseName":"default","tableName":"otel_logs"},"kind":"log","timestampValueExpression":"Timestamp","name":"Logs","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"Body","serviceNameExpression":"ServiceName","bodyExpression":"Body","eventAttributesExpression":"LogAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,SeverityText,Body","severityTextExpression":"SeverityText","traceIdExpression":"TraceId","spanIdExpression":"SpanId","connection":"Local ClickHouse","traceSourceId":"Traces","sessionSourceId":"Sessions","metricSourceId":"Metrics"},{"from":{"databaseName":"default","tableName":"otel_traces"},"kind":"trace","timestampValueExpression":"Timestamp","name":"Traces","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"SpanName","serviceNameExpression":"ServiceName","eventAttributesExpression":"SpanAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,StatusCode,round(Duration/1e6),SpanName","traceIdExpression":"TraceId","spanIdExpression":"SpanId","durationExpression":"Duration","durationPrecision":9,"parentSpanIdExpression":"ParentSpanId","spanNameExpression":"SpanName","spanKindExpression":"SpanKind","statusCodeExpression":"StatusCode","statusMessageExpression":"StatusMessage","connection":"Local ClickHouse","logSourceId":"Logs","sessionSourceId":"Sessions","metricSourceId":"Metrics"},{"from":{"databaseName":"default","tableName":""},"kind":"metric","timestampValueExpression":"TimeUnix","name":"Metrics","resourceAttributesExpression":"ResourceAttributes","metricTables":{"gauge":"otel_metrics_gauge","histogram":"otel_metrics_histogram","sum":"otel_metrics_sum","summary":"otel_metrics_summary","exponentialHistogram":"otel_metrics_exponential_histogram","_id":"682586a8b1f81924e628e808","id":"682586a8b1f81924e628e808"},"connection":"Local ClickHouse","logSourceId":"Logs","traceSourceId":"Traces","sessionSourceId":"Sessions"},{"from":{"databaseName":"default","tableName":"hyperdx_sessions"},"kind":"session","timestampValueExpression":"TimestampTime","name":"Sessions","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"Body","serviceNameExpression":"ServiceName","bodyExpression":"Body","eventAttributesExpression":"LogAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,SeverityText,Body","severityTextExpression":"SeverityText","traceIdExpression":"TraceId","spanIdExpression":"SpanId","connection":"Local ClickHouse","logSourceId":"Logs","traceSourceId":"Traces","metricSourceId":"Metrics"}]'
export DEFAULT_SOURCES='[{"from":{"databaseName":"default","tableName":"otel_logs"},"kind":"log","timestampValueExpression":"Timestamp","name":"Logs","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"Body","serviceNameExpression":"ServiceName","bodyExpression":"Body","eventAttributesExpression":"LogAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,SeverityText,Body","severityTextExpression":"SeverityText","traceIdExpression":"TraceId","spanIdExpression":"SpanId","metadataMaterializedViews":{"keyRollupTable":"otel_logs_key_rollup_15m","kvRollupTable":"otel_logs_kv_rollup_15m","granularity":"15 minute"},"connection":"Local ClickHouse","traceSourceId":"Traces","sessionSourceId":"Sessions","metricSourceId":"Metrics"},{"from":{"databaseName":"default","tableName":"otel_traces"},"kind":"trace","timestampValueExpression":"Timestamp","name":"Traces","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"SpanName","serviceNameExpression":"ServiceName","eventAttributesExpression":"SpanAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,StatusCode,round(Duration/1e6),SpanName","traceIdExpression":"TraceId","spanIdExpression":"SpanId","durationExpression":"Duration","durationPrecision":9,"parentSpanIdExpression":"ParentSpanId","spanNameExpression":"SpanName","spanKindExpression":"SpanKind","statusCodeExpression":"StatusCode","statusMessageExpression":"StatusMessage","metadataMaterializedViews":{"keyRollupTable":"otel_traces_key_rollup_15m","kvRollupTable":"otel_traces_kv_rollup_15m","granularity":"15 minute"},"connection":"Local ClickHouse","logSourceId":"Logs","sessionSourceId":"Sessions","metricSourceId":"Metrics"},{"from":{"databaseName":"default","tableName":""},"kind":"metric","timestampValueExpression":"TimeUnix","name":"Metrics","resourceAttributesExpression":"ResourceAttributes","metricTables":{"gauge":"otel_metrics_gauge","histogram":"otel_metrics_histogram","sum":"otel_metrics_sum","summary":"otel_metrics_summary","exponentialHistogram":"otel_metrics_exponential_histogram","_id":"682586a8b1f81924e628e808","id":"682586a8b1f81924e628e808"},"connection":"Local ClickHouse","logSourceId":"Logs","traceSourceId":"Traces","sessionSourceId":"Sessions"},{"from":{"databaseName":"default","tableName":"hyperdx_sessions"},"kind":"session","timestampValueExpression":"TimestampTime","name":"Sessions","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"Body","serviceNameExpression":"ServiceName","bodyExpression":"Body","eventAttributesExpression":"LogAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,SeverityText,Body","severityTextExpression":"SeverityText","traceIdExpression":"TraceId","spanIdExpression":"SpanId","connection":"Local ClickHouse","logSourceId":"Logs","traceSourceId":"Traces","metricSourceId":"Metrics"}]'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is to add the metadataMaterializedView source by default

size?: 'xs' | 'sm' | 'lg';
autocompleteOptions?: { value: string; label: string }[];
isLoadingValues?: boolean;
tokenInfo?: TokenInfo;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were multiple rerenders occuring due to cascading debouncing of values, so the logic for extracting a token tokenInfo is now centralized. Additionally, a token does not have to be the last search term, but can be any throughout the lucene query.

Comment on lines +22 to 29
const path = parseKeyPath(cleanKey);

// Match patterns like: ResourceAttributes['some.property'], SpanAttributes['key'], or json_column.key
const mapPattern = /^([^[]+)\[['"]([^'"]+)['"]\]$/;
const match = cleanKey.match(mapPattern);

if (match) {
if (path.length >= 2) {
return {
baseName: match[1],
propertyPath: match[2],
baseName: path[0],
propertyPath: path.slice(1).join('.'),
};
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted functionality to common function parseKeyPath

@knudtty
knudtty marked this pull request as ready for review May 6, 2026 01:02
@knudtty
knudtty requested review from karl-power and removed request for dhable May 6, 2026 14:52
Comment thread packages/app/src/hooks/useMetadata.tsx Resolved
@github-actions

github-actions Bot commented May 7, 2026 •

Copy link
Copy Markdown
Contributor

Deep Review

🔴 P0/P1 -- must fix

  • packages/common-utils/src/core/metadata.ts:381-488 -- getMapKeys and getAllKeyValues reuse the same cacheKey for both the rollup query and the source-table fallback, so an empty rollup result (or a transient ClickHouse error caught into []) is cached and silently blocks the fallback for the lifetime of the in-process MetadataCache, leaving autocomplete keys permanently empty.

    • Fix: Use distinct cache keys for the rollup vs source-table paths, or skip caching when the rollup returns [] / throws so the fallback closure can still run.
    • correctness, reliability, adversarial, performance
  • packages/app/src/hooks/useAutoCompleteOptions.tsx:237 -- metadataMVs = tcs[0]?.metadataMVs and mode = metadataMVs ? 'all' : 'exact' are derived from the first table connection only, so SearchInputV2 / SearchWhereInput callers passing tableConnections lose value suggestions from every connection past index 0 — a regression from the prior per-source mapping.

    • Fix: Iterate tcs and union the per-connection results, or assert that tableConnections callers pass a single tableConnection and remove the array path.
    • correctness, adversarial
  • packages/app/src/hooks/useAutoCompleteOptions.tsx:194-199 -- tokenInfo reads inputRef.current.selectionStart from the live DOM but tokenizes the 300ms-debounced value, so when the user accepts a suggestion mid-typing, AutocompleteInput.onAcceptSuggestion rebuilds the textarea from a stale token list and silently drops any characters typed after the last debounce flush.

    • Fix: Capture selectionStart synchronously in an onKeyUp / onSelect handler and pass a primitive cursorPos to the memo, then replay any not-yet-debounced suffix into the new value before calling onChange.
    • adversarial, julik-frontend-races, kieran-typescript, maintainability
  • packages/common-utils/src/core/metadata.ts:39-83 -- MetadataCache is a process-lifetime Map with no TTL or LRU, and the new rollup paths key entries by connectionId.db.table.column.<alignedStart>.<alignedEnd>.keys, so every distinct date-window plus column combination accumulates forever (~96 entries/day per column at 15-minute granularity).

    • Fix: Add an LRU cap or TTL inside MetadataCache.set, or shard by aligned bucket and evict windows older than a fixed retention.
  • packages/common-utils/src/core/metadata.ts:425, 1293 -- The if (rollupKeys.length > 0) return rollupKeys short-circuit (and its getAllKeyValues twin) treats any non-empty rollup result as authoritative, so an MV that is behind on ingestion or that omits a column the source has data for silently truncates autocomplete with no health check and no UI signal.

    • Fix: Compare the rollup's earliest Timestamp against the requested range (or the source's min(Timestamp)) and union with the source-table results when the rollup does not cover the window.

🟡 P2 -- recommended

  • packages/api/src/routers/external-api/v2/sources.ts -- metadataMaterializedViews is exposed via GET /api/v2/sources but the v2 router has no POST / PUT / PATCH, so agents and external integrations can read the new field but cannot configure it.

    • Fix: Add v2 write endpoints validating against SourceSchemaNoId / SourceSchema and document the request body in openapi.json.
  • packages/api/openapi.json:2568-2588, 2766-2808 -- The metadataMaterializedViews object schemas omit a required array even though the runtime MetadataMaterializedViewsSchema requires keyRollupTable, kvRollupTable, and granularity (each .min(1)); spec readers will believe partial objects are valid.

    • Fix: Add "required": ["keyRollupTable", "kvRollupTable", "granularity"] to both the LogSource and TraceSource schemas.
  • packages/api/src/routers/external-api/v2/sources.ts:12-29 -- granularity is mapped outbound via mapGranularityToExternalFormat ("15 minute" → "15m") but no inverse mapper exists, so a future write endpoint that accepts a value read back from the v2 API will fail SQLIntervalSchema validation.

    • Fix: Add a reverse mapper applied to inbound granularity fields, or accept both forms in the schema.
  • packages/app/src/source.ts:354-387 -- inferTableSourceConfig blindly trusts that any <tableName>_key_rollup_15m and <tableName>_kv_rollup_15m it finds are HyperDX rollups; an unrelated table with the same name is auto-attached, then inferGranularityFromMVSelect falls back to '15 minute' and queries either fail or surface unrelated data.

    • Fix: Validate the detected table's engine (SummingMergeTree) and required columns (Timestamp, ColumnIdentifier, Key, count, plus Value for kv) before populating metadataMaterializedViews.
  • packages/app/src/source.ts:357-373 -- The rollup-detection Promise.all([metadata.getTableMetadata(...), metadata.getTableMetadata(...)]) is neither wrapped in try/catch nor given a max_execution_time, so a slow or failing system.tables query blocks the entire source-inference flow instead of degrading to "no rollup".

    • Fix: Pass clickhouse_settings: { timeout_overflow_mode: 'break', max_execution_time: 5 }, switch to Promise.allSettled, and treat any failure as metadataMaterializedViews = undefined.
    • reliability, correctness
  • packages/common-utils/src/core/materializedViews.ts:153-167 -- inferGranularityFromMVSelect returns the first toStartOf* it encounters, so an MV whose as_select references a non-bucketing toStartOf first (e.g. toStartOfMinute(received_at) before toStartOfFifteenMinutes(Timestamp)) bakes the wrong granularity into both the WHERE predicate and the cache key.

    • Fix: Anchor detection on the Timestamp argument (or the alias used by downstream queries) instead of source order.
  • packages/common-utils/src/core/metadata.ts:1522 -- The same config object travels under three names — metadataMaterializedViews (Zod / Mongoose), metadataMVs (TableConnection), and metadataMVsOverride (hook destructure) — so a typo on tc.metadataMaterializedViews is silently undefined and grep misses two of three call sites.

    • Fix: Rename TableConnection.metadataMVs to metadataMaterializedViews and drop the destructure alias.
    • api-contract, kieran-typescript, maintainability
  • packages/app/src/components/Sources/SourceForm.tsx:638 -- The "Add Metadata Materialized Views" button initializes granularity: '', which fails SQLIntervalSchema (^\d+ (second|minute|hour|day)$) on submit and surfaces as a confusing Zod error rather than disabling Save until the field is filled.

    • Fix: Initialize granularity to a valid default (e.g. '15 minute') or leave it undefined and gate Save until the field is populated.
  • packages/app/src/hooks/useAutoCompleteOptions.tsx:159-167 -- The default dateRange uses NOW = Date.now() evaluated once at module init in packages/app/src/config.ts:60, so long-lived tabs hit autocomplete with a 24h window ending at page-load time, fetching aged-out / empty rollup buckets that then interact with the cache-poisoning bug above.

    • Fix: Replace NOW with Date.now() evaluated at hook-call time inside useAutoCompleteOptions.
  • packages/app/src/hooks/useAutoCompleteOptions.tsx:255 -- The .flatMap callback annotates v: string | Record<string, string> but the enclosing useQuery<{ key: string; value: string[] }[]> types v as string; the wider annotation keeps dead object-handling code alive and silently masks future drift.

    • Fix: Drop the annotation and let inference narrow to string, or widen the query return type if both shapes are intentionally supported.
  • packages/app/src/hooks/__tests__/useAutoCompleteOptions.test.tsx:37-41 -- Every test uses a mockTableConnection without metadataMVs, so the new mode === 'all' dispatch in useMultipleGetKeyValues — the most consequential code path in this PR — is never exercised.

    • Fix: Add a test case with tableConnection.metadataMVs populated and assert useGetKeyValues is called with mode: 'all' and the expected metadataMVs.
    • kieran-typescript, testing
  • packages/common-utils/src/core/metadata.ts:1545-1554 -- tcFromSource is the load-bearing propagation point that turns source.metadataMaterializedViews into a TableConnection.metadataMVs, but it has no test; a silent regression here would disable the entire rollup path with no observable failure.

    • Fix: Add unit tests covering log-with-MVs, trace-with-MVs, metric / session (no MVs), and source === undefined.
  • packages/app/src/source.ts:342-420 -- The new ~40-line inferTableSourceConfig rollup-detection branch has zero tests covering its three outcomes (both tables exist, one missing, granularity unrecognized) plus the throw path.

    • Fix: Add tests for each outcome with a mocked metadata.getTableMetadata.
🔵 P3 nitpicks (16)
  • packages/common-utils/src/core/metadata.ts:380-435, 1199-1315 -- getMapKeys and getAllKeyValues rollup paths have no unit tests for any branch (rollup hits, empty, throws, fallback).

    • Fix: Add tests with a mocked clickhouseClient covering all four branches per method.
  • packages/api/src/routers/external-api/v2/sources.ts:32-55 -- mapSourceToExternalSource has no test for sources carrying metadataMaterializedViews.

    • Fix: Add a unit test asserting outbound granularity is mapped to external format and that absence of the field doesn't crash.
  • packages/common-utils/src/core/materializedViews.ts:186-199 -- renderStartOfBucketExpr and getNamedBucketFunction are exported but untested, including the UNSAFE_RAW_SQL named-bucket branch.

    • Fix: Add tests for a granularity that maps to a named function ('1 hour' → toStartOfHour) and one that does not ('7 minute' → toStartOfInterval).
  • packages/common-utils/src/core/materializedViews.ts:178 -- getNamedBucketFunction is exported despite having only one in-file caller.

    • Fix: Drop the export keyword to keep the named-bucket lookup table internal.
  • packages/common-utils/src/core/metadata.ts:385, 1247 -- The four-line block aligning the date range and assembling the rollup timeFilter is duplicated verbatim in getMapKeys and getAllKeyValues.

    • Fix: Extract a private buildRollupTimeFilter(granularity, dateRange) helper and call it from both methods.
  • packages/app/src/hooks/useMetadata.tsx:221-333 -- useMultipleGetKeyValues forks completely on mode: 'all' | 'exact', with limit / disableRowLimit silently ignored in 'all' mode.

    • Fix: Split into useGetAllKeyValues (rollup path) and keep the existing useGetKeyValues for the exact path; pick the right hook at the call site.
  • packages/app/src/hooks/useMetadata.tsx:271 -- The (isLogSource(s) || isTraceSource(s)) ? s.metadataMaterializedViews : undefined ternary is duplicated in useMultipleGetKeyValues and tcFromSource.

    • Fix: Extract a getMetadataMVsFromSource(source) helper next to MetadataMaterializedViewsSchema.
  • packages/common-utils/src/core/metadata.ts:1210 -- The JSDoc on getAllKeyValues claims "Only filters by date range -- no WHERE conditions" but the rollup branch applies ColumnIdentifier and Key equality filters.

    • Fix: Rewrite the comment to describe the actual behavior or delete it.
  • packages/common-utils/src/core/metadata.ts:1516 -- The // Describes a table and potentially related views comment on TableConnection restates the type name without explaining when metadataMVs is populated.

    • Fix: Replace with a comment describing the log/trace-only invariant, or remove it.
  • packages/app/src/hooks/useMetadata.tsx:240 -- options?: Omit<UseQueryOptions<any, Error>, 'queryKey'> lets callers pass select / placeholderData transformers with mismatched return types.

    • Fix: Replace any with { key: string; value: string[] }[] so option types stay aligned with the actual query shape.
  • packages/common-utils/src/core/materializedViews.ts:158 -- parseIntervalLiteral returns `${num} ${unit}` as SQLInterval; the cast bypasses any future tightening of SQLIntervalSchema.

    • Fix: Replace the cast with SQLIntervalSchema.parse(...) so the runtime validator enforces the contract.
  • packages/common-utils/src/core/materializedViews.ts:166 -- NAMED_BUCKET_FUNCTIONS is typed Record<string, SQLInterval>, so the index access is well-typed only because noUncheckedIndexedAccess is off.

    • Fix: Declare the map as const satisfies Record<string, SQLInterval> so the fn in narrowing drives index access correctly.
  • packages/app/src/components/Sources/SourceForm.tsx:1975 -- setValue(key as any, value) in the auto-infer effect disables RHF path checking, so a typo from inferTableSourceConfig would silently set an unknown form field.

    • Fix: Cast to the specific union of valid form paths (e.g. as 'metadataMaterializedViews') instead of as any.
  • docker/otel-collector/schema/seed/00006_otel_logs_rollups.sql:28-36 -- The key-rollup tables ORDER BY (ColumnIdentifier, Key, Timestamp), but getMapKeys filters on ColumnIdentifier plus Timestamp range, so the primary index can't skip granules on Timestamp because Key sits between them; same applies to 00007_otel_traces_rollups.sql.

    • Fix: Reorder the key tables to ORDER BY (ColumnIdentifier, Timestamp, Key) so the time filter prunes after the column-identifier prefix match.
  • packages/app/src/hooks/useMetadata.tsx:121-196 -- useMultipleAllFields has no staleTime, so every component mount triggers an immediate background refetch — now multiplied by N map columns issuing parallel rollup queries.

    • Fix: Add staleTime: 1000 * 60 * 5 to match useMultipleGetKeyValues.
    • julik-frontend-races, performance
  • packages/app/src/hooks/useMetadata.tsx:145-168 -- useMultipleAllFields's queryFn ignores the signal from React Query, so in-flight field fetches cannot be aborted when the user switches sources rapidly.

    • Fix: Destructure signal from the queryFn context and thread it into each metadata.getAllFields call.

Reviewers (13): correctness, testing, maintainability, project-standards, agent-native, learnings, security, performance, api-contract, reliability, adversarial, kieran-typescript, julik-frontend-races.

Testing gaps:

  • No coverage for getMapKeys / getAllKeyValues rollup-empty-fallback or rollup-throws paths.
  • No test asserts tcFromSource propagates metadataMVs from a log/trace source.
  • No test for useAutoCompleteOptions with tableConnection.metadataMVs populated — the entire mode: 'all' branch is unexercised.
  • inferTableSourceConfig rollup-detection has zero tests across its three outcomes (both tables exist, one missing, granularity unrecognized).
  • No test for mapSourceToExternalSource with metadataMaterializedViews (granularity outbound transform untested).
  • No test for the metadataMVsOverride-in-query-key contract that the in-PR fix commit 1c97c059 introduced.

Note: docs/solutions/ does not yet exist in this repo, so no past learnings could be matched; the React-Query cache-key discipline lesson from the in-PR metadataMVsOverride fix is a candidate first entry.

@kodiakhq
kodiakhq Bot merged commit 7d7269a into main May 8, 2026
19 checks passed
@kodiakhq
kodiakhq Bot deleted the aaron/feat-autocomplete branch May 8, 2026 20:38

This branch was successfully deployed

1 active deployment
Preview — da4cc4d1 Deployed May 8, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants