From dbb3fc65b65f40d7d4a9c6f81295178487f48abf Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Mon, 5 Jan 2026 15:14:34 +0100 Subject: [PATCH 1/4] Replace entire identifier when autocompleting inside of it When accepting an autocompletion result within an Identifier node (could be a metric name, function name, keyword, etc.), the inserted completion should replace the entire Identifier node all the way to its last character, not only to the current cursor position. A limitation is that the correct replacement-until-end-of-identifier only works when e.g. a function name is currently incomplete (which is likely anyway when trying to replace it with a different one). This is because otherwise the Identifier node gets replaced with a more specific function node type (like `Rate`, `SumOverTime`, etc.), and handling all those adds more complexity. https://github.com/prometheus/prometheus/issues/15839 Signed-off-by: Julius Volz --- .../src/complete/hybrid.test.ts | 91 ++++++++++++++++++- .../codemirror-promql/src/complete/hybrid.ts | 22 ++++- 2 files changed, 111 insertions(+), 2 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 1f3985af63..8250319681 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { analyzeCompletion, computeStartCompletePosition, ContextKind, durationWithUnitRegexp } from './hybrid'; +import { analyzeCompletion, computeStartCompletePosition, computeEndCompletePosition, ContextKind, durationWithUnitRegexp } from './hybrid'; import { createEditorState, mockedMetricsTerms, mockPrometheusServer } from '../test/utils-test'; import { Completion, CompletionContext } from '@codemirror/autocomplete'; import { @@ -866,6 +866,73 @@ describe('computeStartCompletePosition test', () => { }); }); +describe('computeEndCompletePosition test', () => { + const testCases = [ + { + title: 'cursor at end of metric name', + expr: 'metric_name', + pos: 11, // cursor is at the end + expectedEnd: 11, + }, + { + title: 'cursor in middle of metric name - should extend to end', + expr: 'coredns_cache_hits_total', + pos: 14, // cursor is after 'coredns_cache_' (before 'hits') + expectedEnd: 24, // should extend to end of 'coredns_cache_hits_total' + }, + { + title: 'cursor in middle of metric name inside rate() - should extend to end', + expr: 'rate(coredns_cache_hits_total[2m])', + pos: 19, // cursor is after 'coredns_cache_' (before 'hits') + expectedEnd: 29, // should extend to end of 'coredns_cache_hits_total' + }, + { + title: 'cursor in middle of metric name inside sum(rate()) - should extend to end', + expr: 'sum(rate(coredns_cache_hits_total[2m]))', + pos: 24, // cursor is after 'coredns_cache_' (before 'hits') + expectedEnd: 33, // should extend to end of 'coredns_cache_hits_total' + }, + { + title: 'cursor at beginning of metric name - should extend to end', + expr: 'metric_name', + pos: 1, // cursor after 'm' + expectedEnd: 11, + }, + { + title: 'cursor in middle of incomplete function name - should extend to end', + expr: 'sum_ov', + pos: 4, // cursor after 'sum_' (before 'ov') + expectedEnd: 6, // should extend to end of 'sum_ov' + }, + { + title: 'cursor in middle of incomplete function name within aggregator - should extend to end', + expr: 'sum(sum_ov(foo[5m]))', + pos: 8, // cursor after 'sum_' (before 'ov') + expectedEnd: 10, // should extend to end of 'sum_ov' + }, + { + title: 'empty bracket - returns pos', + expr: '{}', + pos: 1, + expectedEnd: 1, + }, + { + title: 'cursor in label matchers - returns pos', + expr: 'metric_name{label="value"}', + pos: 12, // cursor after '{' + expectedEnd: 12, + }, + ]; + testCases.forEach((value) => { + it(value.title, () => { + const state = createEditorState(value.expr); + const node = syntaxTree(state).resolve(value.pos, -1); + const result = computeEndCompletePosition(state, node, value.pos); + expect(result).toEqual(value.expectedEnd); + }); + }); +}); + describe('autocomplete promQL test', () => { beforeEach(() => { mockPrometheusServer(); @@ -915,6 +982,28 @@ describe('autocomplete promQL test', () => { validFor: /^[a-zA-Z0-9_:]+$/, }, }, + { + title: 'cursor in middle of metric name - to should extend to end (issue #15839)', + expr: 'sum(coredns_cache_hits_total)', + pos: 18, // cursor is after 'coredns_cache_' (before 'hits') + expectedResult: { + options: ([] as Completion[]).concat(functionIdentifierTerms, aggregateOpTerms, snippets), + from: 4, + to: 28, // should extend to end of 'coredns_cache_hits_total' + validFor: /^[a-zA-Z0-9_:]+$/, + }, + }, + { + title: 'cursor in middle of metric name inside rate() - to should extend to end (issue #15839)', + expr: 'rate(coredns_cache_hits_total[2m])', + pos: 19, // cursor is after 'coredns_cache_' (before 'hits') + expectedResult: { + options: ([] as Completion[]).concat(functionIdentifierTerms, aggregateOpTerms, snippets), + from: 5, + to: 29, // should extend to end of 'coredns_cache_hits_total' + validFor: /^[a-zA-Z0-9_:]+$/, + }, + }, { title: 'offline function/aggregation autocompletion in aggregation 3', expr: 'sum(rate())', diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.ts index fc79b6fcd6..d89907699a 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.ts @@ -166,6 +166,20 @@ function arrayToCompletionResult(data: Completion[], from: number, to: number, i } as CompletionResult; } +// computeEndCompletePosition calculates the end position for autocompletion replacement. +// When the cursor is in the middle of an identifier (e.g., metric name), this ensures the entire +// identifier 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(state: EditorState, node: SyntaxNode, pos: number): number { + // For Identifier nodes (metric names), extend the end position to include + // the entire identifier, even if the cursor is in the middle. + if (node.type.id === Identifier) { + return node.to; + } + // Default: use the cursor position as the end position + return pos; +} + // Matches complete PromQL durations, including compound units (e.g., 5m, 1d2h, 1h30m, etc.). // Duration units are a fixed, safe set (no regex metacharacters), so no escaping is needed. export const durationWithUnitRegexp = new RegExp(`^(\\d+(${durationTerms.map((term) => term.label).join('|')}))+$`); @@ -667,7 +681,13 @@ export class HybridComplete implements CompleteStrategy { } } return asyncResult.then((result) => { - return arrayToCompletionResult(result, computeStartCompletePosition(state, tree, pos), pos, completeSnippet, span); + return arrayToCompletionResult( + result, + computeStartCompletePosition(state, tree, pos), + computeEndCompletePosition(state, tree, pos), + completeSnippet, + span + ); }); } From b532eacae88af136f6bb1d67ae6746e7344caa08 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Mon, 5 Jan 2026 20:09:12 +0100 Subject: [PATCH 2/4] Review fixups - also make it work for label names Signed-off-by: Julius Volz --- .../src/complete/hybrid.test.ts | 24 +++++++++++++++++++ .../codemirror-promql/src/complete/hybrid.ts | 11 +++++---- 2 files changed, 30 insertions(+), 5 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 8250319681..5906a692de 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts @@ -922,6 +922,30 @@ describe('computeEndCompletePosition test', () => { pos: 12, // cursor after '{' expectedEnd: 12, }, + { + title: 'cursor in middle of label name in grouping clause - should extend to end', + expr: 'sum by (instance_name)', + pos: 12, // cursor after 'inst' (before 'ance') + expectedEnd: 21, // should extend to end of 'instance_name' + }, + { + title: 'cursor in middle of label name in label matcher - should extend to end', + expr: 'metric{instance_name="value"}', + pos: 11, // cursor after 'inst' (before 'ance') + expectedEnd: 20, // should extend to end of 'instance_name' + }, + { + title: 'cursor in middle of label name in on() modifier - should extend to end', + expr: 'a / on(instance_name) b', + pos: 11, // cursor after 'inst' (before 'ance') + expectedEnd: 20, // should extend to end of 'instance_name' + }, + { + title: 'cursor in middle of label name in ignoring() modifier - should extend to end', + expr: 'a / ignoring(instance_name) b', + pos: 17, // cursor after 'inst' (before 'ance') + expectedEnd: 26, // should extend to end of 'instance_name' + }, ]; testCases.forEach((value) => { it(value.title, () => { diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.ts index d89907699a..23e47ce649 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.ts @@ -167,13 +167,14 @@ function arrayToCompletionResult(data: Completion[], from: number, to: number, i } // computeEndCompletePosition calculates the end position for autocompletion replacement. -// When the cursor is in the middle of an identifier (e.g., metric name), this ensures the entire -// identifier is replaced, not just the portion before the cursor. This fixes issue #15839. +// When the cursor is in the middle of an identifier (e.g., metric name) or label name, 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(state: EditorState, node: SyntaxNode, pos: number): number { - // For Identifier nodes (metric names), extend the end position to include - // the entire identifier, even if the cursor is in the middle. - if (node.type.id === Identifier) { + // For Identifier nodes (metric names) and LabelName nodes (label names in matchers, + // grouping clauses, etc.), extend the end position to include the entire token, + // even if the cursor is in the middle. + if (node.type.id === Identifier || node.type.id === LabelName) { return node.to; } // Default: use the cursor position as the end position From 3fc800410aed73e2fd2143524f3de6a064c246f7 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 6 Jan 2026 10:57:55 +0100 Subject: [PATCH 3/4] Handle autocomplete replacement better for more node types Signed-off-by: Julius Volz --- .../src/complete/hybrid.test.ts | 64 ++++++++++++++++--- .../codemirror-promql/src/complete/hybrid.ts | 33 +++++++--- 2 files changed, 80 insertions(+), 17 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 5906a692de..40c4356cc6 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts @@ -638,7 +638,7 @@ describe('analyzeCompletion test', () => { const state = createEditorState(value.expr); const node = syntaxTree(state).resolve(value.pos, -1); const result = analyzeCompletion(state, node, value.pos); - expect(value.expectedContext).toEqual(result); + expect(result).toEqual(value.expectedContext); }); }); }); @@ -861,7 +861,7 @@ describe('computeStartCompletePosition test', () => { const state = createEditorState(value.expr); const node = syntaxTree(state).resolve(value.pos, -1); const result = computeStartCompletePosition(state, node, value.pos); - expect(value.expectedStart).toEqual(result); + expect(result).toEqual(value.expectedStart); }); }); }); @@ -911,16 +911,16 @@ describe('computeEndCompletePosition test', () => { expectedEnd: 10, // should extend to end of 'sum_ov' }, { - title: 'empty bracket - returns pos', + title: 'empty bracket - ends before the closing bracket', expr: '{}', pos: 1, expectedEnd: 1, }, { - title: 'cursor in label matchers - returns pos', + title: 'cursor in label matchers - ends before the closing bracket', expr: 'metric_name{label="value"}', pos: 12, // cursor after '{' - expectedEnd: 12, + expectedEnd: 25, }, { title: 'cursor in middle of label name in grouping clause - should extend to end', @@ -946,6 +946,54 @@ describe('computeEndCompletePosition test', () => { pos: 17, // cursor after 'inst' (before 'ance') expectedEnd: 26, // should extend to end of 'instance_name' }, + { + title: 'cursor in middle of function name rate - should extend to end', + expr: 'rate(foo[5m])', + pos: 2, // cursor after 'ra' (before 'te') + expectedEnd: 4, // should extend to end of 'rate' + }, + { + title: 'cursor in middle of function name histogram_quantile - should extend to end', + expr: 'histogram_quantile(0.9, rate(foo[5m]))', + pos: 10, // cursor after 'histogram_' (before 'quantile') + expectedEnd: 18, // should extend to end of 'histogram_quantile' + }, + { + title: 'cursor in middle of aggregator sum - should extend to end', + expr: 'sum(rate(foo[5m]))', + pos: 2, // cursor after 'su' (before 'm') + expectedEnd: 3, // should extend to end of 'sum' + }, + { + title: 'cursor in middle of aggregator count_values - should extend to end', + expr: 'count_values("label", foo)', + pos: 6, // cursor after 'count_' (before 'values') + expectedEnd: 12, // should extend to end of 'count_values' + }, + { + title: 'cursor in middle of nested function - should extend to end', + expr: 'sum(rate(foo[5m]))', + pos: 6, // cursor after 'ra' inside rate (before 'te') + expectedEnd: 8, // should extend to end of 'rate' + }, + { + title: 'cursor at beginning of aggregator - should extend to end', + expr: 'avg by (instance) (rate(foo[5m]))', + pos: 1, // cursor after 'a' (before 'vg') + expectedEnd: 3, // should extend to end of 'avg' + }, + { + title: 'cursor in middle of function name with binary op - should extend to end', + expr: 'rate(foo[5m]) / irate(bar[5m])', + pos: 17, // cursor after 'ir' inside irate (before 'ate') + expectedEnd: 21, // should extend to end of 'irate' + }, + { + title: 'error node - returns pos (cursor position)', + expr: 'metric_name !', + pos: 13, // cursor at '!' (error node) + expectedEnd: 13, // error node returns pos + }, ]; testCases.forEach((value) => { it(value.title, () => { @@ -1398,7 +1446,7 @@ describe('autocomplete promQL test', () => { expectedResult: { options: [], from: 10, - to: 10, + to: 11, validFor: /^[a-zA-Z0-9_:]+$/, }, }, @@ -1409,7 +1457,7 @@ describe('autocomplete promQL test', () => { expectedResult: { options: [], from: 10, - to: 10, + to: 12, validFor: /^[a-zA-Z0-9_:]+$/, }, }, @@ -1564,7 +1612,7 @@ describe('autocomplete promQL test', () => { const context = new CompletionContext(state, value.pos, true); const completion = newCompleteStrategy(value.conf); const result = await completion.promQL(context); - expect(value.expectedResult).toEqual(result); + expect(result).toEqual(value.expectedResult); }); }); diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.ts index 23e47ce649..2dd342d305 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.ts @@ -167,18 +167,33 @@ function arrayToCompletionResult(data: Completion[], from: number, to: number, i } // computeEndCompletePosition calculates the end position for autocompletion replacement. -// When the cursor is in the middle of an identifier (e.g., metric name) or label name, this ensures -// the entire token is replaced, not just the portion before the cursor. This fixes issue #15839. +// 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(state: EditorState, node: SyntaxNode, pos: number): number { - // For Identifier nodes (metric names) and LabelName nodes (label names in matchers, - // grouping clauses, etc.), extend the end position to include the entire token, - // even if the cursor is in the middle. - if (node.type.id === Identifier || node.type.id === LabelName) { - return node.to; + // For error nodes, use the cursor position as the end position + if (node.type.id === 0) { + return pos; } - // Default: use the cursor position as the end position - return pos; + + if ( + node.type.id === LabelMatchers || + node.type.id === GroupingLabels || + node.type.id === FunctionCallBody || + node.type.id === MatrixSelector || + node.type.id === SubqueryExpr + ) { + // When we're inside empty brackets, we want to replace up to just before the closing bracket. + return node.to - 1; + } + + if (node.type.id === StringLiteral && (node.parent?.type.id === UnquotedLabelMatcher || node.parent?.type.id === QuotedLabelMatcher)) { + // For label values, we want to replace all content inside the quotes. + return node.parent.to - 1; + } + + // For all other nodes, extend the end position to include the entire token. + return node.to; } // Matches complete PromQL durations, including compound units (e.g., 5m, 1d2h, 1h30m, etc.). From fe76e6c297c58f21d969782ac3ade29cb8306949 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 6 Jan 2026 11:00:01 +0100 Subject: [PATCH 4/4] Remove unneeded state parameter Signed-off-by: Julius Volz --- web/ui/module/codemirror-promql/src/complete/hybrid.test.ts | 2 +- web/ui/module/codemirror-promql/src/complete/hybrid.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 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 40c4356cc6..facda35ac8 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts @@ -999,7 +999,7 @@ describe('computeEndCompletePosition test', () => { it(value.title, () => { const state = createEditorState(value.expr); const node = syntaxTree(state).resolve(value.pos, -1); - const result = computeEndCompletePosition(state, node, value.pos); + const result = computeEndCompletePosition(node, value.pos); expect(result).toEqual(value.expectedEnd); }); }); diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.ts index 2dd342d305..84c101b43c 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.ts @@ -170,7 +170,7 @@ function arrayToCompletionResult(data: Completion[], from: number, to: number, i // 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(state: EditorState, node: SyntaxNode, pos: number): number { +export function computeEndCompletePosition(node: SyntaxNode, pos: number): number { // For error nodes, use the cursor position as the end position if (node.type.id === 0) { return pos; @@ -700,7 +700,7 @@ export class HybridComplete implements CompleteStrategy { return arrayToCompletionResult( result, computeStartCompletePosition(state, tree, pos), - computeEndCompletePosition(state, tree, pos), + computeEndCompletePosition(tree, pos), completeSnippet, span );