Skip to content

fix: Restore Lucene autocomplete - #2902

Merged
kodiakhq[bot] merged 5 commits into
mainfrom
drew/fix-lucene-autocomplete
Aug 13, 2026
Merged

fix: Restore Lucene autocomplete#2902
kodiakhq[bot] merged 5 commits into
mainfrom
drew/fix-lucene-autocomplete

Conversation

@pulpdrew

@pulpdrew pulpdrew commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR restores lucene auto-complete across the app, which was broken in #2643 for inputs other than the search page WHERE input.

The problem was that in #2643 sourceId became the source of truth for where auto complete suggestions should be queried from, instead of the given tableConnection. However sourceId was not always provided, so when it wasn't no suggestions were queried.

The fix has two parts:

  1. Provide source id (and date range) at all call sites
  2. Where source id is not enough to identify a single table (eg. metrics sources), fallback to the table connection

A better solution in the future would be to make source id required in these input components, but that is a larger refactor saved for a later day.

Screenshots or video

How to test on Vercel preview

Test lucene Auto-complete across the app

References

  • Linear Issue: Closes HDX-5101
  • Related PRs:

@vercel

vercel Bot commented Aug 13, 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 Aug 13, 2026 9:11pm
hyperdx-storybook Ready Ready Preview Aug 13, 2026 9:11pm

Request Review

@changeset-bot

changeset-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cd797a7

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

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

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

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restores Lucene autocomplete by supplying source and date-range context at missing call sites and falling back to explicit table connections when a source does not identify a concrete table.

  • Adds source-aware and table-aware facet discovery.
  • Updates dashboard, session, service, chart-editor, trace, and Kubernetes autocomplete call sites.
  • Adds unit and Playwright coverage for restored autocomplete behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/app/src/hooks/useAutoCompleteOptions.tsx Adds table-connection fallback resolution while retaining source-derived tables as the default.
packages/app/src/components/DBSearchPageFilters/hooks.ts Routes resolved table metadata through facet field and value discovery.
packages/app/src/DBDashboardPage.tsx Builds concrete tile table connections and provides the searched date range to dashboard autocomplete.
packages/app/src/components/KubernetesFilters.tsx Selects the metric gauge table explicitly for Kubernetes filter autocomplete.
packages/app/tests/e2e/utils/lucene-autocomplete.ts Introduces shared Playwright helpers for asserting Lucene field and value suggestions.

Reviews (6): Last reviewed commit: "Merge branch 'main' into drew/fix-lucene..." | Re-trigger Greptile

Comment thread packages/app/src/hooks/useAutoCompleteOptions.tsx
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 289 passed • 1 skipped • 968s

Status Count
✅ Passed 289
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

Comment on lines +363 to +369
// The window the events query itself uses, so autocomplete discovers the
// same fields and values the event list can actually match on.
const eventsDateRange = useMemo(
() => [start, end] as DateRange['dateRange'],
[start, end],
);

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.

Image

tableConnection={tcFromSource(traceTrace)}
// The WHERE runs against the trace source
sourceId={traceTrace?.id}
dateRange={searchedTimeRange}

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.

Image

Comment on lines +360 to +361
sourceId={sourceId}
dateRange={searchedTimeRange}

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.

Image

>
<SearchWhereInput
tableConnection={tableConnection}
sourceId={sourceId}

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.

Image

>
<SearchWhereInput
tableConnections={tableConnections}
dateRange={searchedTimeRange}

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.

Image

Comment on lines +340 to +342
tableConnection={gaugeTableConnection}
sourceId={metricSource.id}
dateRange={dateRange}

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.

Image

<SearchWhereInput
tableConnection={tcFromSource(logTableSource)}
sourceId={logTableSource.id}
dateRange={dateRange}

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.

Image

<SearchWhereInput
tableConnection={tcFromSource(source)}
sourceId={source.id}
dateRange={newDateRange}

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.

Image

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 15
  • Production lines changed: 166 (+ 808 in test files, excluded from tier calculation)
  • Branch: drew/fix-lucene-autocomplete
  • Author: pulpdrew

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

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This PR restores autocomplete by threading sourceId + dateRange to call sites and adding a resolveTableConnection(source, fallback) helper where source stays authoritative and the passed tableConnection is a fallback for the no-source and metric-source cases. The core logic is sound, unit tests cover most resolveTableConnection branches, and new e2e specs guard the regression. Remaining items are advisory.

🟡 P2 -- recommended

  • packages/app/src/components/DBSearchPageFilters/hooks.ts:48 -- For a metric source with no usable fallback, resolveTableConnection returns tcFromSource(source), whose tableName is empty, yielding an unqueryable connection that disables discovery with no error; new call sites passing only sourceId (e.g. the chart editor with a metric source selected) rely on an implicit, unenforced "must pass a fallback" contract.
    • Fix: Narrow the accepted source type or emit a dev warning when a metric source arrives without a usable fallback, and add a unit test for this branch.
    • maintainability, testing
🔵 P3 nitpicks (5)
  • packages/app/src/hooks/useAutoCompleteOptions.tsx:213 -- resolveTableConnection(source, tableConnection) is computed here for chartConfig and again inside useFetchFacets from the same source + fallback, so the two derivations must stay in lockstep or diverge.
    • Fix: Resolve the TableConnection once and thread the resolved value down instead of re-deriving in both places.
  • packages/app/src/components/DBSearchPageFilters/hooks.ts:77 -- The 3-line JSDoc describing the tableConnection fallback param is copy-pasted verbatim on both useFacets and useFetchFacets and can drift.
    • Fix: Reference a single canonical description or a shared typed alias rather than duplicating the prose.
  • packages/app/src/components/KubernetesFilters.tsx:272 -- gaugeTableConnection re-derives databaseName / metricTables?.gauge / connection already computed for chartConfig.from at lines 255-263, so a future change to the metric-table selection could update one site and leave the other pointing at a different table.
    • Fix: Derive gaugeTableConnection from chartConfig (or a shared local) so both read one source of truth.
  • packages/app/src/components/DBSearchPageFilters/hooks.ts:315 -- The same fallback value is named fallback in the helper, tableConnectionFallback in useFacets, and the authoritative-sounding tableConnection at the useFetchFacets / SearchWhereInput / useAutoCompleteOptions hops.
    • Fix: Reuse the fallback vocabulary or document on each public prop that it is a fallback, not the primary connection.
  • packages/app/src/components/DBSearchPageFilters/hooks.ts:55 -- The no-source-without-usable-fallback branch (return tcFromSource(undefined)) is only ever reached indirectly and every existing no-source test passes a fully usable fallback.
    • Fix: Add a test with sourceId null and an omitted/incomplete tableConnection asserting an all-empty connection is returned.

Reviewers (6): correctness, testing, maintainability, kieran-typescript, julik-frontend-races, learnings-researcher. (correctness, kieran-typescript, and julik-frontend-races were dispatched but had not returned findings at synthesis time; the orchestrator's own read of the diff surfaced no additional correctness, type-safety, or async-race defects — the resolveTableConnection aliased-boolean narrowing compiles under TS 4.4+, and the pre-existing unmemoized tableConnection derivation in useAutoCompleteOptions predates this diff.)

Testing gaps:

  • resolveTableConnection's metric-source-without-fallback and no-source-without-fallback branches are not exercised by any test.
  • loadMoreFacetsForKey's new all-path throw guard for an incomplete tableConnection (hooks.ts:243) is untested, and its source?.timestampValueExpression ?? chartConfig.timestampValueExpression ?? '' fallback is only weakly asserted.
  • dateRange threading into the discovery calls is verified only indirectly via e2e specs, not asserted in the unit tests.

@teeohhem

Copy link
Copy Markdown
Contributor

Great catch here. Thank you for adding the additional tests around autocomplete!

@kodiakhq
kodiakhq Bot merged commit 69a89aa into main Aug 13, 2026
27 checks passed
@kodiakhq
kodiakhq Bot deleted the drew/fix-lucene-autocomplete branch August 13, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants