[ES|QL] Improve suggestions based on previous function arguments and date suggestions for `bucket` #190828

qn895 · 2024-08-20T16:29:15Z

This PR improves logic for autocomplete suggestions to suggest new function args based on previous values. For example:

Screen.Recording.2024-08-20.at.14.57.57.mov

It also adds a new snippet 'Add date histogram' for stats ... by <> with BUCKET(<cursor>, 10, ?t_start, ?t_end)'

Screen.Recording.2024-08-30.at.14.11.10.mov

As part of this PR, test cases for eval functions were also updated to cover all possible arguments while avoiding running the same test cases repetitively. For example:

    'st_contains( / )',
    'st_contains(cartesianPointField,  / )',
    'st_contains(cartesianPointField, cartesianPointField,  / )',
    'st_contains(cartesianPointField, cartesianShapeField,  / )',
    'st_contains(cartesianShapeField,  / )',
    'st_contains(cartesianShapeField, cartesianPointField,  / )',
    'st_contains(cartesianShapeField, cartesianShapeField,  / )',
    'st_contains(geoPointField,  / )',
    'st_contains(geoPointField, geoPointField,  / )',
    'st_contains(geoPointField, geoShapeField,  / )',
    'st_contains(geoShapeField,  / )',
    'st_contains(geoShapeField, geoPointField,  / )',
    'st_contains(geoShapeField, geoShapeField,  / )'

    'mv_slice( / )',
    'mv_slice(booleanField,  / )',
    'mv_slice(booleanField, integerField,  / )',
    'mv_slice(booleanField, integerField, integerField,  / )',
    'mv_slice(cartesianPointField,  / )',
    'mv_slice(cartesianPointField, integerField,  / )',
    'mv_slice(cartesianPointField, integerField, integerField,  / )',
    'mv_slice(cartesianShapeField,  / )',
    'mv_slice(cartesianShapeField, integerField,  / )',
    'mv_slice(cartesianShapeField, integerField, integerField,  / )',
    'mv_slice(dateField,  / )',
    'mv_slice(dateField, integerField,  / )',
    'mv_slice(dateField, integerField, integerField,  / )',
    'mv_slice(doubleField,  / )',
    'mv_slice(doubleField, integerField,  / )',
    'mv_slice(doubleField, integerField, integerField,  / )',
    'mv_slice(geoPointField,  / )',
    'mv_slice(geoPointField, integerField,  / )',
    'mv_slice(geoPointField, integerField, integerField,  / )',
    'mv_slice(geoShapeField,  / )',
    'mv_slice(geoShapeField, integerField,  / )',
    'mv_slice(geoShapeField, integerField, integerField,  / )',
    'mv_slice(integerField,  / )',
    'mv_slice(integerField, integerField,  / )',
    'mv_slice(integerField, integerField, integerField,  / )',
    'mv_slice(ipField,  / )',
    'mv_slice(ipField, integerField,  / )',
    'mv_slice(ipField, integerField, integerField,  / )',
    'mv_slice(keywordField,  / )',
    'mv_slice(keywordField, integerField,  / )',
    'mv_slice(keywordField, integerField, integerField,  / )',
    'mv_slice(longField,  / )',
    'mv_slice(longField, integerField,  / )',
    'mv_slice(longField, integerField, integerField,  / )',
    'mv_slice(textField,  / )',
    'mv_slice(textField, integerField,  / )',
    'mv_slice(textField, integerField, integerField,  / )',
    'mv_slice(versionField,  / )',
    'mv_slice(versionField, integerField,  / )',
    'mv_slice(versionField, integerField, integerField,  / )'

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…ignatures-suggestions # Conflicts: # packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts

…ignatures-suggestions

qn895 · 2024-08-20T16:42:40Z

...y/public/components/dataset_quality_details/overview/document_trends/degraded_docs/index.tsx

@@ -125,7 +126,10 @@ export default function DegradedDocs({ lastReloadTime }: { lastReloadTime: numbe
            <EuiButtonIcon
              display="base"
              iconType="discoverApp"
-              aria-label="Discover"
+              aria-label={i18n.translate(


Linting is causing this change automatically 🤔

qn895 · 2024-08-20T17:34:10Z

/ci

qn895 · 2024-08-20T20:04:48Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts

+      }
+    } catch (error) {
+      // eslint-disable-next-line no-console
+      console.error(`Failed query\n-------------\n${query}`);


Added this one liner so if a test fails, it will print out which case specifically failed. I made this because it was getting hard to find the needle in the haystack to figure out which case inside the test suite failed.

qn895 · 2024-08-20T20:06:48Z

/ci

qn895 · 2024-08-22T22:05:01Z

@elasticmachine merge upstream

qn895 · 2024-08-23T18:47:44Z

/ci

qn895 · 2024-08-27T15:47:08Z

@elasticmachine merge upstream

elasticmachine · 2024-08-27T15:49:28Z

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

drewdaemon

Looking good! I did notice that after I select my first date parameter with the date picker, I don't get suggestions for the second date parameter:

Screen.Recording.2024-08-28.at.11.01.13.AM.mov

drewdaemon · 2024-08-28T16:18:23Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts

@@ -429,459 +409,7 @@ describe('autocomplete', () => {
    }
  });

-  describe('eval', () => {


Was this a clean copy, or did we add cases?

I modified the tests actually to test all possible arguments

drewdaemon · 2024-08-28T16:21:59Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts

@@ -93,6 +102,59 @@ export function getSupportedTypesForBinaryOperators(
    : [previousType];
 }

+export function getValidFunctionSignaturesForPreviousArgs(


This is great. This will come in handy with #180518 as well

drewdaemon · 2024-08-28T16:24:15Z

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

+  if (a === 'time_literal' || a === 'time_duration') return b === 'timeInterval';
+  if (b === 'time_literal' || b === 'time_duration') return a === 'timeInterval';
+  if (a === 'time_literal') return b === 'time_duration';
+  if (b === 'time_literal') return a === 'time_duration';


Could we pretend that a new developer is looking at this and add a comment here about the date type weirdness?

Updated here a0a67d8 (#190828)

drewdaemon · 2024-08-28T16:24:41Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts

+}
+
+/**
+ *


Missing explanation

Updated here a0a67d8 (#190828)

drewdaemon · 2024-08-28T16:26:31Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts

+  );
+  const compatibleTypesToSuggestForArg = uniqBy(
+    relevantFuncSignatures.map((f) => f.params[argIndex]).filter((d) => d),
+    (o) => `${o.type}-${o.constantOnly}`


drewdaemon · 2024-08-28T16:27:01Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts

@@ -57,6 +59,13 @@ export function getParamAtPosition(
  return params.length > position ? params[position] : minParams ? params[params.length - 1] : null;
 }

+export function strictlyGetParamAtPosition(


I think a function comment about what "strictly" means in this context would be helpful.

Updated here a0a67d8 (#190828)

drewdaemon · 2024-08-28T16:36:56Z

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

@@ -446,7 +448,7 @@ function areCurrentArgsValid(
  return true;
 }

-function extractFinalTypeFromArg(
+export function extractFinalTypeFromArg(


I know you didn't name this function. But, does "Final" mean anything to you? I'm wondering if this should just be called extractTypeFromASTArg or something.

Updated here a0a67d8 (#190828)

drewdaemon · 2024-08-28T16:38:26Z

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

+  };
+
+  const enrichedArgs = node.args.map((nodeArg) => {
+    let esType = extractFinalTypeFromArg(nodeArg, references);


nit: any reason not to call this dataType?

Updated here a0a67d8 (#190828)

…ignatures-suggestions

qn895 · 2024-08-29T13:54:51Z

@elasticmachine merge upstream

qn895 · 2024-08-30T17:37:10Z

@drewdaemon As discussed in office hours, I've added the 'Add date histogram' snippet to this PR which pre-populates with BUCKET($0, 20, ?start, ?end). Also made some modification on how comma/advance suggestion is added so no longer add comma if it detects there's already a comma right after the cursor.

drewdaemon

Looking great! This is a major improvement to BUCKET 👏

Just make sure we get those parameter names changed to t_start and t_end before merging.

drewdaemon · 2024-08-30T18:03:30Z

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

+                  defaultMessage: 'Add date histogram using bucket()',
+                }
+              ),
+              sortText: '1A',


I think we should open the suggestions menu when the user accepts the date histogram snippet

Suggested change

sortText: '1A',

sortText: '1A',

command: TRIGGER_SUGGESTIONS_COMMAND,

Updated here ca2d8e7 (#190828)

drewdaemon · 2024-08-30T18:07:27Z

packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts

@@ -30,6 +30,7 @@ const allFunctions = statsAggregationFunctionDefinitions
  .concat(groupingFunctionDefinitions);

 export const TIME_SYSTEM_PARAMS = ['?t_start', '?t_end'];
+export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?start, ?end)';


Suggested change

export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?start, ?end)';

export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?t_start, ?t_end)';

Also, I have no idea if 20 is the right number here. Not necessarily something we have to block this PR on, but could you make sure we discuss this as a team?

…ignatures-suggestions

kibana-ci · 2024-08-30T20:01:07Z

💚 Build Succeeded

Buildkite Build
Commit: f252c14

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`kbnUiSharedDeps-srcJs`	3.3MB	3.3MB	+1020.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`@kbn/esql-validation-autocomplete`	1	2	+1

Total ESLint disabled count

id	before	after	diff
`@kbn/esql-validation-autocomplete`	1	2	+1

History

💔 Build #231199 failed f8efe7aac68c95c58ea7ca4d52e3d99337307a14
💔 Build #230937 failed 4e3aa27
💛 Build #230810 was flaky 3f5fa25
💛 Build #230612 was flaky d0378f0
💛 Build #230173 was flaky f22827f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @qn895

qn895 added 9 commits August 9, 2024 12:48

Improve function arg suggestions

d95562a

TODo: Improve suggestions

767e253

Merge remote-tracking branch 'upstream/main' into esql-improve-func-s…

b8e2089

…ignatures-suggestions # Conflicts: # packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts

Add new test file

de8435f

Fix tests

47e7160

Update logic

496952c

Update logic

7a160b5

Update

f8ccdec

Merge remote-tracking branch 'upstream/main' into esql-improve-func-s…

01a8fb1

…ignatures-suggestions

qn895 commented Aug 20, 2024

View reviewed changes

Revert cookieAuth change

2780de7

qn895 added 2 commits August 20, 2024 14:04

Add better suggestion for named parameters

928f8aa

Update tests

b291d1a

qn895 changed the title ~~[ES|QL] Improve suggestions based on previous function args~~ [ES|QL] Improve suggestions based on previous function arguments and date suggestions for bucket Aug 20, 2024

qn895 commented Aug 20, 2024

View reviewed changes

Move around imports

347ab09

elasticmachine and others added 3 commits August 22, 2024 23:05

Merge branch 'main' into esql-improve-func-signatures-suggestions

4c31d01

Merge upstream/main into branch

4918185

Update tests

8a27505

Merge branch 'main' into esql-improve-func-signatures-suggestions

f22827f

qn895 marked this pull request as ready for review August 27, 2024 15:47

qn895 requested review from a team as code owners August 27, 2024 15:47

qn895 self-assigned this Aug 27, 2024

qn895 added the Feature:ES|QL ES|QL related features in Kibana label Aug 27, 2024

qn895 added Team:ESQL ES|QL related features in Kibana v8.16.0 labels Aug 27, 2024

qn895 added the release_note:enhancement label Aug 27, 2024

drewdaemon mentioned this pull request Aug 28, 2024

[ES|QL] Autocomplete suggestions improvements #176033

Open

1 task

drewdaemon reviewed Aug 28, 2024

View reviewed changes

qn895 added 3 commits August 28, 2024 15:25

Fix date picker string

abe8c3e

Update types, comments, add clarity

a0a67d8

Merge remote-tracking branch 'upstream/main' into esql-improve-func-s…

d0378f0

…ignatures-suggestions

stratoula removed the request for review from a team August 29, 2024 05:46

elasticmachine and others added 4 commits August 29, 2024 14:54

Merge branch 'main' into esql-improve-func-signatures-suggestions

3f5fa25

Add date histogram

4e3aa27

Update tests

cfa7363

Make it so it doesn't add comma when completing an arg from a snippet

abcaab4

qn895 force-pushed the esql-improve-func-signatures-suggestions branch from f8efe7a to abcaab4 Compare August 30, 2024 17:07

Fix not opening suggestions for snippets

558cb4e

drewdaemon approved these changes Aug 30, 2024

View reviewed changes

qn895 added 2 commits August 30, 2024 13:28

Update tests and bucket number

ca2d8e7

Merge remote-tracking branch 'upstream/main' into esql-improve-func-s…

f252c14

…ignatures-suggestions

qn895 enabled auto-merge (squash) August 30, 2024 19:02

qn895 merged commit 70a33f4 into elastic:main Aug 30, 2024
20 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Aug 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ES|QL] Improve suggestions based on previous function arguments and date suggestions for `bucket` #190828

[ES|QL] Improve suggestions based on previous function arguments and date suggestions for `bucket` #190828

qn895 commented Aug 20, 2024 •

edited

Loading

qn895 Aug 20, 2024

qn895 commented Aug 20, 2024

qn895 Aug 20, 2024

qn895 commented Aug 20, 2024

qn895 commented Aug 22, 2024

qn895 commented Aug 23, 2024

qn895 commented Aug 27, 2024

elasticmachine commented Aug 27, 2024

drewdaemon left a comment

drewdaemon Aug 28, 2024

qn895 Aug 28, 2024

drewdaemon Aug 28, 2024

drewdaemon Aug 28, 2024

qn895 Aug 30, 2024

drewdaemon Aug 28, 2024

qn895 Aug 30, 2024

drewdaemon Aug 28, 2024

drewdaemon Aug 28, 2024

qn895 Aug 30, 2024

drewdaemon Aug 28, 2024

qn895 Aug 28, 2024

drewdaemon Aug 28, 2024

qn895 Aug 28, 2024

qn895 commented Aug 29, 2024

qn895 commented Aug 30, 2024

drewdaemon left a comment

drewdaemon Aug 30, 2024

qn895 Aug 30, 2024

drewdaemon Aug 30, 2024

kibana-ci commented Aug 30, 2024

ESLint disabled line counts

Total ESLint disabled count

+              }
+              /**
+               *

	sortText: '1A',
	sortText: '1A',
	command: TRIGGER_SUGGESTIONS_COMMAND,

	export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?start, ?end)';
	export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?t_start, ?t_end)';

[ES|QL] Improve suggestions based on previous function arguments and date suggestions for bucket #190828

[ES|QL] Improve suggestions based on previous function arguments and date suggestions for bucket #190828

Conversation

qn895 commented Aug 20, 2024 • edited Loading

Checklist

Risk Matrix

For maintainers

Choose a reason for hiding this comment

qn895 commented Aug 20, 2024

Choose a reason for hiding this comment

qn895 commented Aug 20, 2024

qn895 commented Aug 22, 2024

qn895 commented Aug 23, 2024

qn895 commented Aug 27, 2024

elasticmachine commented Aug 27, 2024

drewdaemon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qn895 commented Aug 29, 2024

qn895 commented Aug 30, 2024

drewdaemon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Aug 30, 2024

💚 Build Succeeded

Metrics [docs]

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

[ES|QL] Improve suggestions based on previous function arguments and date suggestions for `bucket` #190828

[ES|QL] Improve suggestions based on previous function arguments and date suggestions for `bucket` #190828

qn895 commented Aug 20, 2024 •

edited

Loading