[ES|QL] Build function arguments suggestions from hints by sddonne · Pull Request #246495 · elastic/kibana

sddonne · 2025-12-16T08:16:16Z

Summary

For some special function arguments ES is now providing information about how to resolve them. For instance, functions that accepts inference endpoints.

This PR adds support for the inference_endpoint hint, and an easy way to extend it to other hints.

src/platform/packages/shared/kbn-esql-ast/src/commands/definitions/generated/settings.ts

elasticmachine · 2025-12-16T08:20:24Z

Pinging @elastic/kibana-esql (Team:ESQL)

bartoval · 2025-12-16T09:20:30Z

...ql-ast/src/commands/definitions/utils/autocomplete/expressions/positions/empty_expression.ts

+): Promise<ISuggestionItem[]> {
+  // Keep the hints that are unique by entityType + constraints
+  const hints: ParameterHint[] = uniqWith(
+    paramDefinitions.filter((param) => param.hint !== undefined).map((param) => param.hint!),


Null coaleshing should do the work

paramDefinitions.flatMap(({ hint }) => hint ?? []),

Also no need to check the existence of hints

..... const results = await Promise.all( hints.map((hint) => parametersFromHintsMap[hint.entityType]?.(hint, ctx) ?? []) ); return results.flat(); }

Nice! adopted -> 6249586

stratoula

Thanks Seb for working on it. I didnt test as we are aiming for 9.4 but I did a first code review!

stratoula · 2025-12-17T06:16:14Z

src/platform/packages/shared/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts

      hasMinimumLicenseRequired,
      canCreateLookupIndex: callbacks?.canCreateLookupIndex,
      isServerless: callbacks?.isServerless,
+      getInferenceEndpoints: callbacks?.getInferenceEndpoints,


I would prefer the inferences to come on the context

export interface ICommandContext { columns: Map<string, ESQLColumnData>; sources?: ESQLSourceResult[]; joinSources?: IndexAutocompleteItem[]; timeSeriesSources?: IndexAutocompleteItem[]; inferenceEndpoints?: InferenceEndpointAutocompleteItem[]; policies?: Map<string, ESQLPolicy>; editorExtensions?: EditorExtensions; variables?: ESQLControlVariable[]; supportsControls?: boolean; histogramBarTarget?: number; activeProduct?: PricingProduct | undefined; isCursorInSubquery?: boolean; }

We are having them already at the context, it is confusing to have them at the callbacks.

My first idea was using the Command context yes, but did found some drawbacks:

These are suggestions for functions, not commands, in the command context we check if the command is COMPLETION or RERANK and fetch that data.

So the implementation is less straightforward: when creating each command context we would need to walk the command and check the signature of every function within the command to check if some argument has a hint, if it has one do the fetching of the data in case we need it later. This is in theory good for user perceived performance as we have the data before we need it, but adds extra parsing code early on in the code and fetches for data that may not be used, and IMO the difference in speed it's not really noticeable.

In contrast the current implementation makes use of the signature detection that is already been used for other suggestions, so at that point of the flow it's easy to detect if we have hints in it and if we need to fetch for data, and when we do it's because it's needed.

So, putting that on the table, there are some tradeoffs between solutions, I think the current one is simpler, adds less code to maintain and blends with the flow we use for suggesting within functions.
If we go for adding it into the context, it could have better perceived performance and unifies the "resources" available for the command in that object, but that comes with having to do deep analysis early on in the autocomplete routine for each command.

Summarising I think it's a decision about:
1 - Gathering all data we may need for autocomplete early on in the routine.
2 - Or fetching the data as we need it in the subroutines.

Let me know your thoughts with this new information, and if the directrice is to always go with option 1, which is totally fine to me btw :)

It is problematic to have this call in 2 different places. It shows that something goes wrong with the architecture. As long as we have this information on the context, adding this also to the callbacks is wrong.

We should stick with our current one I think (otherwise we need to re-architect this thing). So I think we should go with the more complex one as it aligns with our current architeture.

Can you create another PR with this to see how this will be? If it doesnt look ok then we should think more thoroughly and re-architect!

Will do 👍

@stratoula what do you think about this implementation path -> #246736
(it still need some love before being prod ready, but want to validate we want to go that path before polishing the deatils.)

yes I really like it!! 👏

stratoula · 2025-12-17T06:17:01Z

src/platform/packages/shared/kbn-esql-ast/src/commands/definitions/types.ts

+export const parameterHintEntityTypes = ['inference_endpoint'] as const;
+export type ParameterHintEntityType = (typeof parameterHintEntityTypes)[number];
+export interface ParameterHint {
+  entityType: ParameterHintEntityType;


Let's make it an enum instead

This is a type that is used within the autogenerated files, we will have type errors in them if using enums.

stratoula · 2025-12-17T06:17:50Z

...ql-ast/src/commands/definitions/utils/autocomplete/expressions/positions/empty_expression.ts

+  ctx: ExpressionContext
+): Promise<ISuggestionItem[]> {
+  // Keep the hints that are unique by entityType + constraints
+  const hints: ParameterHint[] = uniqWith(


Why do we need the uniq?

It's to make it more robust, signatures can be redundant with parameters as ES creates permutations with the parameters, so I don't want to create duplicated suggestions if the 'inference_id' param appears in multiple signatures.
At the same time, if the 'inference_id' appears in multiple signatures but with different constraints (different task_types) I DO want to fetch the different endpoints.

We don't have cases now, but we probably will not be notified if they change this signature to these cases, so better be prepared as it's just a line code.

... signature 1... params: [ { name: 'inference_id', type: 'keyword', hint: { entityType: 'inference_endpoint', constraints: { task_type: 'text_embedding', }, }, }, ], ... signature 2... params: [ { name: 'inference_id', type: 'keyword', hint: { entityType: 'inference_endpoint', constraints: { task_type: 'rerank', }, }, }, ],

stratoula · 2025-12-17T06:18:55Z

...ges/shared/kbn-esql-ast/src/commands/definitions/utils/autocomplete/parameters_from_hints.ts

+  ParameterHintEntityType,
+  (hint: ParameterHint, ctx: ExpressionContext) => Promise<ISuggestionItem[]>
+> = {
+  ['inference_endpoint']: inferenceEndpointHandler,


We need better types here otherwise everytime they add a hint we need to change this part.

Oh it was on purpose, it's a way of getting notified if they add a new hint, so we know we might want to add support to it, fixing the type error it's just adding ['new_hint']: () => [],, and we can create an issue to tackle it later.

I can add a Partial<> to it, if you prefer to avoid this.

export const parametersFromHintsMap: Partial< Record< ParameterHintEntityType, (hint: ParameterHint, ctx: ExpressionContext) => Promise<ISuggestionItem[]> > >

Yes it makes it too hardcoded. We need an architecture as we have for functions. So something that we dont need to bother every time they add something. So have that in mind

What do you mean with hardcoded?
Each new hint type will need a dedicated handler, as each hint will be very different from each other right? 🤔

I mean that this is much simpler and better https://github.com/elastic/kibana/pull/246495/files#r2626487947

stratoula · 2025-12-17T06:19:52Z

...ges/shared/kbn-esql-ast/src/commands/definitions/utils/autocomplete/parameters_from_hints.ts

+  if (hint.constraints?.task_type) {
+    const inferenceEnpoints =
+      (
+        await ctx.callbacks?.getInferenceEndpoints?.(


This should follow the other context logic and not having the callback here

stratoula · 2025-12-17T06:20:07Z

...ges/shared/kbn-esql-ast/src/commands/definitions/utils/autocomplete/parameters_from_hints.ts

+        )
+      )?.inferenceEndpoints || [];
+
+    return inferenceEnpoints.map(createInferenceEndpointToCompletionItem).map((item) => {


Do we need the 2 maps?

fixed in new branch 67a221e

stratoula · 2025-12-17T06:20:38Z

...ges/shared/kbn-esql-ast/src/commands/definitions/utils/autocomplete/parameters_from_hints.ts

+    return inferenceEnpoints.map(createInferenceEndpointToCompletionItem).map((item) => {
+      return {
+        ...item,
+        detail: i18n.translate('kbn-esql-ast.esql.definitions.inferenceEndpointInFunction', {


Where is it visible (I didn't test, can you provide a screenshot?)

It's the documentation detail of the suggestion

Ok so this text is wrong but we can discuss after #246495 (comment)

stratoula · 2025-12-17T06:21:31Z

...ackages/shared/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/functions.test.ts

    });
  });
+
+  describe('function parameter built from hint', () => {


Instead of hardcoding them every time they add a hint lets create these tests automatically from the info ES sent to us

Sorry, I don't see it 🤔 💭, hints could vary a lot one from each other, some may need to mock new endpoints, etc. They tell us, hey this param is special, here it's a hint of how to create the suggestions for it, but each hint needs a different implementation from each other, so I'm not seeing how to make a generic test for all u.u

You have the function from the hints, so I am hoping that we can create generic tests that we show suggestions for the hints without every time to have to hard code them (the less work we do for this the better, it means that our architecture can scale). In my mind it should be doable at least 😄

I think I get your idea now.
But it would leave the actual handler untested. So we will still need to create a test for the dedicated hint handler to have good coverage.

I liked this approach more because it test both the handler and the autocomplete routine integration all together, with less code.
I can go with your solution too if you want and create specific tests for the handlers, in the bright side of it, they will be closer. It's all a matter of trade offs.

yes I prefer a generic solution and then yes specific tests for the handlers would be ok 👌

sddonne · 2025-12-19T11:07:16Z

Closed in favour of #246736

sddonne added 6 commits December 15, 2025 15:55

fix rerank not fetching inference endpoints

7d982a6

suggest inference endpoint in TEXT_EMBEDDING function

2e0ce65

add test

f84d936

clean

78e02b4

make hints unique

8a71197

clean

4da15db

sddonne added v9.4.0 Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana t// release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Dec 16, 2025

sddonne commented Dec 16, 2025

View reviewed changes

src/platform/packages/shared/kbn-esql-ast/src/commands/definitions/generated/settings.ts Show resolved Hide resolved

sddonne marked this pull request as ready for review December 16, 2025 08:20

sddonne requested a review from a team as a code owner December 16, 2025 08:20

bartoval reviewed Dec 16, 2025

View reviewed changes

simplify hint resolving

6249586

stratoula reviewed Dec 17, 2025

View reviewed changes

sddonne mentioned this pull request Dec 17, 2025

[ES|QL] Build function arguments suggestions from hints #246736

Merged

sddonne closed this Dec 19, 2025

Conversation

sddonne commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Uh oh!

elasticmachine commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sddonne Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sddonne commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sddonne commented Dec 16, 2025 •

edited

Loading

sddonne Dec 18, 2025 •

edited

Loading