Skip to content

feat: Distribute exact-match lucene variable references - #2987

Open
pulpdrew wants to merge 2 commits into
mainfrom
drew/lucene-variable-distribute
Open

feat: Distribute exact-match lucene variable references#2987
pulpdrew wants to merge 2 commits into
mainfrom
drew/lucene-variable-distribute

Conversation

@pulpdrew

@pulpdrew pulpdrew commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR enhances support for variables in lucene by rewriting Field:"$var" to (Field:"A" OR Field"B") (when A and B are selected for $var. This preserves exact-match semantics. The basic expansion Field:("A" OR "B") is a substring condition `Field ILIKE "%A%" OR Field ILIKE "%B%".

The quoted form was chosen for exact-match semantics because Field:"A" is exact match. Field:$var continues using the basic expansion, matching Field:A substring semantics.

How

The rewrite is done through the following process:

  1. Tokenize the lucene input string using the existing macro/variable tokenizer. This turns the text into a stream of Variable/Macro/Text tokens.
  2. Replace all Variable type tokens with unique placeholders __hdx_sentinel_N in a sentinelString, and track the locations of the placeholders within the sentinelString. We do this to ensure that the sentinelString can be parsed as valid lucene (${var} reference are not valid single lucene terms).
  3. Parse the sentinelString as lucene
  4. Rewrite the terms representing Field:"Value" terms (from the AST), where Value is a tracked placeholder string in the sentinelString.

The transform is a lucene --> lucene transformation, so the resulting lucene still goes through the existing lucene --> SQL transpiler and inherits all of its optimizations.

Screenshots or video

Some examples:

Screenshot 2026-08-24 at 3 02 01 PM Screenshot 2026-08-24 at 3 03 07 PM Screenshot 2026-08-24 at 3 02 52 PM Screenshot 2026-08-24 at 3 02 34 PM Screenshot 2026-08-24 at 3 02 15 PM

How to test on Vercel preview

  • Create a dashboard and add some variables via the Edit Filters and Variables button
  • Create a chart and reference lucene variables. Inspect the SQL generated for various conditions

References

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

@changeset-bot

changeset-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 39963bf

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

This PR includes changesets to release 4 packages
Name Type
@hyperdx/app Patch
@hyperdx/common-utils 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

@vercel

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

Request Review

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds exact-match expansion for quoted Lucene variable references while preserving substring semantics for unquoted references.

  • Parses variable-bearing Lucene through temporary sentinels and distributes quoted field references across selected values.
  • Encodes parser-sensitive URL, port, backslash, and escaped-colon syntax before rewriting, then restores its source spelling.
  • Updates SQL editor previews, shared substitution context, and parser/substitution regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/common-utils/src/variables.ts Adds context-aware Lucene substitution and AST-based distribution of quoted field references, including the prior-thread special-syntax fix.
packages/common-utils/src/queryParser.ts Centralizes parser-sensitive token encoding and adds source-preserving decoding used by variable rewriting.
packages/app/src/components/SQLEditor/variableCompletions.tsx Uses language-aware substitution for previews and documents quoted exact-match behavior.
packages/common-utils/src/tests/variables.test.ts Covers exact-match distribution, escaping, negation, empty selections, and the URL and escaped-colon cases from the previous finding.
packages/common-utils/src/tests/queryParser.test.ts Verifies special-token source round-tripping and the SQL semantics of distributed exact-match terms.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Input[Lucene template] --> Tokenize[Tokenize variables and macros]
  Tokenize --> Encode[Encode parser-sensitive text]
  Encode --> Sentinel[Replace references with sentinels]
  Sentinel --> Parse[Parse Lucene AST]
  Parse --> Rewrite{Quoted field reference?}
  Rewrite -->|Yes| Exact[Distribute field exact matches]
  Rewrite -->|No| Standard[Apply standard variable expansion]
  Exact --> Restore[Restore source spelling]
  Standard --> Restore
  Restore --> Output[Expanded Lucene query]
Loading

Reviews (2): Last reviewed commit: "fix: Encode special values before lucene..." | Re-trigger Greptile

Comment thread packages/common-utils/src/variables.ts
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 308 passed • 1 skipped • 1206s

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

Tests ran across 4 shards in parallel.

View full report →

@pulpdrew
pulpdrew marked this pull request as ready for review August 24, 2026 19:39
@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Diff size: 409 production lines changed (Tier 2 max: < 250)
  • Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 4
  • Production lines changed: 409 (+ 440 in test files, excluded from tier calculation)
  • Branch: drew/lucene-variable-distribute
  • Author: pulpdrew

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

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This PR rewrites quoted Lucene variable references (Field:"$var"(Field:"A" OR Field:"B")) via sentinel replacement, special-token encoding, lucene.parse, and AST-offset span rewriting. The lucene→SQL transpiler is unchanged. Key correctness questions were verified as safe: variable values reach SQL only as escaped Lucene terms bound through SqlString parameterization (no injection); the exported-API rename (substituteVariablessubstituteWithContext) is internal to a private package with every caller migrated in this PR; and the prior Greptile P1 (URL/escaped-colon syntax breaking the fallback) is fully resolved by the encodeSpecialTokens pass and pinned by a broad it.each table.

🟡 P2 -- recommended

  • packages/common-utils/src/__tests__/variables.test.ts:516 -- the new distribution path lacks tests for variable values carrying special content (http://, raw colons, $ref-like, or literal __hdx_sentinel_N strings) and for 10+ references where __hdx_sentinel_1 could be confused with __hdx_sentinel_10.
    • Fix: Add distribution cases with special-content and sentinel-like values plus an ≥11-reference expression, asserting each field distributes to the correct value.
    • testing, maintainability
🔵 P3 nitpicks (5)
  • packages/app/src/components/SQLEditor/variableCompletions.tsx:183 -- the Lucene suggestion help text renders (Field:"value1" OR Field:"value1"), repeating value1 where the second term should be a distinct value.

    • Fix: Change the second value1 to value2 in the description string.
  • packages/common-utils/src/queryParser.ts:63 -- in SPECIAL_TOKEN_ENCODINGS the localhost entry stores source/value as localhost:$1 (a String.replace backreference), while every other entry is a plain literal, and the doc comment implies literals; a future entry with a literal $ would be silently mis-substituted.

    • Fix: Document that source/value are replacement strings, or escape $ in literal entries.
  • packages/common-utils/src/variables.ts:730 -- substituteTokensWithLuceneRewrites records offsets in encoded space and relies on encodeSpecialTokens/decodeSpecialTokensToSource being exact inverses and on placeholders always parsing as Lucene terms; edits to SPECIAL_TOKEN_ENCODINGS could corrupt offsets in this module with no local signal.

    • Fix: Add a round-trip invariant test and/or expose a single encode-and-parse helper from queryParser so the offset assumptions live in one place.
  • .changeset/lucene-variable-field-distribution.md:2 -- a feat that also renames an exported @hyperdx/common-utils function and makes inputLanguage a required field is declared as a patch bump.

    • Fix: Consider a minor bump, or confirm the repo treats internal 0.x breaking changes as patch.
  • packages/common-utils/src/variables.ts:696 -- a negated quoted reference with an empty selection compiles to NOT (1=1) (matches nothing), the opposite of the empty-state no-op; this is pre-existing and consistent with the unquoted -Field:$var path and is deliberately characterized in the tests.

    • Fix: Track as a known product-semantics question rather than a blocker, since the diff does not regress it.

Reviewers (7): correctness (independent trace), security, testing, maintainability, api-contract, project-standards, previous-comments, performance.

Testing gaps:

  • No ≥10-reference case proving __hdx_sentinel_1 vs __hdx_sentinel_10 disambiguation.
  • No distributed-path coverage for values containing http://, raw colons, or sentinel-like strings.
  • encodeSpecialTokens/decodeSpecialTokensToSource round-trip is tested only on curated inputs, not placeholder-collision cases (e.g. text literally containing HDX_COLON).

Note: the adversarial and TypeScript reviewer sub-agents did not return before synthesis; their lanes (break-input construction and AST type-safety) were partially covered by the correctness trace and security review.

Comment on lines +93 to 99
export function decodeSpecialTokensToSource(query: string): string {
return SPECIAL_TOKEN_ENCODINGS.reduce(
(decoded, { decodePattern, source }) =>
decoded.replace(decodePattern, source),
query,
);
}

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 changes here were introduced so that a lossless encode/decoding could be used to (a) encode before parsing lucene in variables.ts and (b) decode back to the exact original input instead of what decodeSpecialTokens returns, which is slightly lossy in some cases because it returns unescaped values.

@pulpdrew
pulpdrew requested a review from wrn14897 August 24, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant