From 70610a6506fab9fbe6d8ec927dcb814f4343114a Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 9 Jun 2026 10:01:55 +0000 Subject: [PATCH] codemirror: fix autocomplete edgecase for functions We want to stop suggesting metric names on closing function brackets. Signed-off-by: Michael Hoffmann --- .../src/complete/hybrid.test.ts | 96 ++++++++++++++++++- .../codemirror-promql/src/complete/hybrid.ts | 24 ++++- 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts index 6b33c2e527..04f313ecb0 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts @@ -74,6 +74,31 @@ describe('analyzeCompletion test', () => { { kind: ContextKind.Aggregation }, ], }, + { + title: 'autocomplete AggregateOpModifier or BinOp after closing aggregation', + expr: 'sum()', + pos: 5, // cursor is after the closing bracket + expectedContext: [{ kind: ContextKind.AggregateOpModifier }, { kind: ContextKind.BinOp }], + }, + { + title: 'metric/function/aggregation autocompletion in incomplete function', + expr: 'sum(', + pos: 4, + expectedContext: [ + { + kind: ContextKind.MetricName, + metricName: '', + }, + { kind: ContextKind.Function }, + { kind: ContextKind.Aggregation }, + ], + }, + { + title: 'autocomplete binOp after closing function', + expr: 'rate(foo[5m])', + pos: 13, // cursor is after the closing bracket + expectedContext: [{ kind: ContextKind.BinOp }], + }, { title: 'metric/function/aggregation autocompletion 2', expr: 'sum(rat)', @@ -100,6 +125,18 @@ describe('analyzeCompletion test', () => { { kind: ContextKind.Aggregation }, ], }, + { + title: 'autocomplete AggregateOpModifier or BinOp after closing nested aggregation', + expr: 'sum(rate(foo[5m]))', + pos: 18, // cursor is after the closing bracket + expectedContext: [{ kind: ContextKind.AggregateOpModifier }, { kind: ContextKind.BinOp }], + }, + { + title: 'autocomplete binOp after closing aggregation with existing modifier', + expr: 'sum by(job)(rate(foo[5m]))', + pos: 26, // cursor is after the closing bracket + expectedContext: [{ kind: ContextKind.BinOp }], + }, { title: 'metric/function/aggregation autocompletion 4', expr: 'sum(rate(my_))', @@ -717,6 +754,18 @@ describe('computeStartCompletePosition test', () => { pos: 9, // cursor is between the bracket expectedStart: 9, }, + { + title: 'start should be equal to the pos after closing function', + expr: 'rate(foo[5m])', + pos: 13, // cursor is after the closing bracket + expectedStart: 13, + }, + { + title: 'start should be equal to the pos after closing aggregation', + expr: 'sum(rate(foo[5m]))', + pos: 18, // cursor is after the closing bracket + expectedStart: 18, + }, { title: 'bracket containing a substring', expr: '{myL}', @@ -994,12 +1043,24 @@ describe('computeEndCompletePosition test', () => { pos: 13, // cursor at '!' (error node) expectedEnd: 13, // error node returns pos }, + { + title: 'end should be equal to the pos after closing function', + expr: 'rate(foo[5m])', + pos: 13, // cursor is after the closing bracket + expectedEnd: 13, + }, + { + title: 'end should be equal to the pos after closing aggregation', + expr: 'sum(rate(foo[5m]))', + pos: 18, // cursor is after the closing bracket + expectedEnd: 18, + }, ]; testCases.forEach((value) => { it(value.title, () => { const state = createEditorState(value.expr); const node = syntaxTree(state).resolve(value.pos, -1); - const result = computeEndCompletePosition(node, value.pos); + const result = computeEndCompletePosition(state, node, value.pos); expect(result).toEqual(value.expectedEnd); }); }); @@ -1043,6 +1104,28 @@ describe('autocomplete promQL test', () => { validFor: /^[a-zA-Z0-9_:]+$/, }, }, + { + title: 'offline autocomplete aggregate operation modifier or binary operator after closing aggregation', + expr: 'sum()', + pos: 5, // cursor is after the closing bracket + expectedResult: { + options: ([] as Completion[]).concat(aggregateOpModifierTerms, binOpTerms), + from: 5, + to: 5, + validFor: /^[a-zA-Z0-9_:]+$/, + }, + }, + { + title: 'offline autocomplete binary operator after closing function', + expr: 'rate(foo[5m])', + pos: 13, // cursor is after the closing bracket + expectedResult: { + options: binOpTerms, + from: 13, + to: 13, + validFor: /^[a-zA-Z0-9_:]+$/, + }, + }, { title: 'offline function/aggregation autocompletion in aggregation 2', expr: 'sum(ra)', @@ -1087,6 +1170,17 @@ describe('autocomplete promQL test', () => { validFor: /^[a-zA-Z0-9_:]+$/, }, }, + { + title: 'offline autocomplete aggregate operation modifier or binary operator after closing nested aggregation', + expr: 'sum(rate(foo[5m]))', + pos: 18, // cursor is after the closing bracket + expectedResult: { + options: ([] as Completion[]).concat(aggregateOpModifierTerms, binOpTerms), + from: 18, + to: 18, + validFor: /^[a-zA-Z0-9_:]+$/, + }, + }, { title: 'offline function/aggregation autocompletion in aggregation 4', expr: 'sum by (instance, job) ( sum_over(scrape_series_added[1h])) / sum by (instance, job) (sum_over_time(scrape_samples_scraped[1h])) > 0.1 and sum by(instance, job) (scrape_samples_scraped{) > 100', diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.ts index 58dfd7c15d..ce7b31b538 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.ts @@ -17,6 +17,7 @@ import { PrometheusClient } from '../client'; import { Add, AggregateExpr, + AggregateModifier, And, BinaryExpr, BoolModifier, @@ -175,16 +176,24 @@ function escapePromQLString(str: string): string { return str.replace(/([\\"])/g, '\\$1'); } +function isAfterClosedFunctionCallBody(state: EditorState, node: SyntaxNode, pos: number): boolean { + return node.type.id === FunctionCallBody && pos >= node.to && node.from < node.to && state.sliceDoc(node.to - 1, node.to) === ')'; +} + // computeEndCompletePosition calculates the end position for autocompletion replacement. // When the cursor is in the middle of a token, this ensures the entire token is replaced, // not just the portion before the cursor. This fixes issue #15839. // Note: this method is exported only for testing purpose. -export function computeEndCompletePosition(node: SyntaxNode, pos: number): number { +export function computeEndCompletePosition(state: EditorState, node: SyntaxNode, pos: number): number { // For error nodes, use the cursor position as the end position if (node.type.id === 0) { return pos; } + if (isAfterClosedFunctionCallBody(state, node, pos)) { + return pos; + } + if ( node.type.id === LabelMatchers || node.type.id === GroupingLabels || @@ -240,7 +249,9 @@ function computeStartCompleteLabelPositionInLabelMatcherOrInGroupingLabel(node: export function computeStartCompletePosition(state: EditorState, node: SyntaxNode, pos: number): number { const currentText = state.doc.slice(node.from, pos).toString(); let start = node.from; - if (node.type.id === LabelMatchers || node.type.id === GroupingLabels) { + if (isAfterClosedFunctionCallBody(state, node, pos)) { + start = pos; + } else if (node.type.id === LabelMatchers || node.type.id === GroupingLabels) { start = computeStartCompleteLabelPositionInLabelMatcherOrInGroupingLabel(node, pos); } else if ( (node.type.id === FunctionCallBody && node.firstChild === null) || @@ -545,6 +556,13 @@ export function analyzeCompletion(state: EditorState, node: SyntaxNode, pos: num result.push({ kind: ContextKind.Duration }); break; case FunctionCallBody: + if (isAfterClosedFunctionCallBody(state, node, pos)) { + if (node.parent?.type.id === AggregateExpr && !containsAtLeastOneChild(node.parent, AggregateModifier)) { + result.push({ kind: ContextKind.AggregateOpModifier }); + } + result.push({ kind: ContextKind.BinOp }); + break; + } // For aggregation function such as Topk, the first parameter is a number. // The second one is an expression. // When moving to the second parameter, the node is an error node. @@ -711,7 +729,7 @@ export class HybridComplete implements CompleteStrategy { return arrayToCompletionResult( result, computeStartCompletePosition(state, tree, pos), - computeEndCompletePosition(tree, pos), + computeEndCompletePosition(state, tree, pos), completeSnippet, span );