fix(editor): finish the PromQL list — metrics on arrival, and no catalog in a label
Two reports from external review, both reproduced before being believed and both
mine.
METRICS NEVER REACHED THE SEEDED LIST. I seeded the catalog so Ctrl+Space would
work before the first keystroke, and stopped there. Metrics arrive later, from a
watcher on the stream results, and updateMetricKeywords only filled its own ref
-- nothing rebuilt what the editor was bound to. So a freshly opened PromQL
editor offered 113 catalog entries and not one metric name until the user edited
the query: the same complaint I thought I had fixed, half fixed. It rebuilds on
arrival now, and deliberately does NOT open the widget while doing it -- that is
a refresh, not an invitation.
AN EMPTY LABEL RESULT SHOWED THE WHOLE LANGUAGE. `[]` meant two different things
to updatePromqlKeywords: "no context, give me the catalog" and "the label lookup
matched nothing". A lookup returning [] therefore put 97 function names inside
`up{instance="`, where not one of them can be typed. Measured, not guessed: the
probe reported 97. This predates the catalog -- it used to leak the 7 hardcoded
functions -- so my change did not cause it, it made it 14x louder. The label
path now says so explicitly (`{ contextual: true }`), an empty contextual result
stays empty, and the widget closes rather than hanging open over the query.
The two are one mechanism: a flag for whether what is showing belongs to a label
or value position. It also answers the question the first fix raises -- metrics
arriving mid-edit must not replace a label list, which is its own test.
Failure-first, as usual. Both new behavioural tests fail against the unfixed
composable (metric absent; 97 functions where [] belongs). The third, that a
metric refresh cannot clobber a contextual list, passes either way against the
OLD code -- it guards the new rebuild rather than reproducing a bug -- so I
checked it the only way that means anything: removing the `if
(!contextualSuggestions)` guard turns it red.
Verified in the running app: 115 entries with two metrics sorting above every
function, popup never opened; and no function leaks into a label position.
Full suite: 40419 total, 39988 passed, 23 failed -- 14 synthetics journey specs
and the 9 load-flaky CodeQueryEditor ones, all pre-existing. type-check, eslint,
prettier clean.
This commit is contained in:
parent
6b4df354ad
commit
219e2be518
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { flushPromises } from "@vue/test-utils";
|
||||
|
||||
// Mock services
|
||||
vi.mock("@/services/search", () => ({
|
||||
|
|
@ -672,6 +673,67 @@ describe("PromQL catalog reaches the editor", () => {
|
|||
expect((fresh.autoCompletePromqlKeywords.value as any[]).length).toBeGreaterThan(90);
|
||||
});
|
||||
|
||||
it("offers metric names the moment they arrive, with no keystroke", async () => {
|
||||
// The catalog is seeded at construction, but METRICS arrive later, from a
|
||||
// watcher on the stream results. Nothing rebuilt the offered list when they
|
||||
// did, so a freshly opened PromQL editor showed 113 catalog entries and not
|
||||
// one metric name until the user edited the query — the same "Ctrl+Space
|
||||
// shows nothing useful" complaint, half fixed.
|
||||
const fresh = usePromqlSuggestions();
|
||||
const popupOpen = vi.fn();
|
||||
fresh.autoCompleteData.value.popup.open = popupOpen;
|
||||
|
||||
fresh.updateMetricKeywords([{ label: "http_requests_total", type: "counter" }]);
|
||||
|
||||
const labels = (fresh.autoCompletePromqlKeywords.value as any[]).map((k: any) => k.label);
|
||||
expect(labels.some((l: string) => l.startsWith("http_requests_total"))).toBe(true);
|
||||
// Rebuilding the list is not a reason to open the widget over whatever the
|
||||
// user is doing.
|
||||
expect(popupOpen, "arriving metrics popped the suggest widget open").not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not let arriving metrics clobber a contextual list", async () => {
|
||||
// Label suggestions are showing; a metric refresh landing at that moment
|
||||
// must not replace them with the catalog.
|
||||
const fresh = usePromqlSuggestions();
|
||||
const labelList = [{ label: "instance", kind: "Variable", insertText: "instance=" }];
|
||||
await fresh.updatePromqlKeywords(labelList, { contextual: true });
|
||||
fresh.updateMetricKeywords([{ label: "http_requests_total", type: "counter" }]);
|
||||
expect(fresh.autoCompletePromqlKeywords.value).toEqual(labelList);
|
||||
});
|
||||
|
||||
it("keeps an empty label result empty instead of showing every function", async () => {
|
||||
// A label lookup that matched nothing is not the same as "no context" — but
|
||||
// both arrived as `[]`, so the catalog took over and offered 97 functions
|
||||
// inside `up{instance="`, where none of them can be typed.
|
||||
vi.mocked(searchService.get_promql_series).mockResolvedValue({ data: { data: [] } } as any);
|
||||
const fresh = usePromqlSuggestions();
|
||||
fresh.autoCompleteData.value.query = 'up{instance="';
|
||||
fresh.autoCompleteData.value.position.cursorIndex = 12;
|
||||
|
||||
await fresh.getSuggestions();
|
||||
await flushPromises();
|
||||
|
||||
const rows = fresh.autoCompletePromqlKeywords.value as any[];
|
||||
expect(rows.filter((k: any) => k.kind === "Function")).toEqual([]);
|
||||
});
|
||||
|
||||
it("still shows the label suggestions when the lookup finds some", async () => {
|
||||
vi.mocked(searchService.get_promql_series).mockResolvedValue({
|
||||
data: { data: [{ instance: "server-1", job: "api" }] },
|
||||
} as any);
|
||||
const fresh = usePromqlSuggestions();
|
||||
fresh.autoCompleteData.value.query = "up{";
|
||||
fresh.autoCompleteData.value.position.cursorIndex = 2;
|
||||
|
||||
await fresh.getSuggestions();
|
||||
await flushPromises();
|
||||
|
||||
const labels = (fresh.autoCompletePromqlKeywords.value as any[]).map((k: any) => k.label);
|
||||
expect(labels).toContain("instance");
|
||||
expect(labels).not.toContain("rate");
|
||||
});
|
||||
|
||||
it("has the catalog ready before the first keystroke", async () => {
|
||||
// getSuggestions is what fills this list today, and getSuggestions only
|
||||
// runs on a query update — so a freshly opened PromQL editor has an EMPTY
|
||||
|
|
|
|||
|
|
@ -25,6 +25,11 @@ const usePromqlSuggestions = () => {
|
|||
// which runs on a query update — so Ctrl+Space on a freshly opened PromQL
|
||||
// editor offered nothing at all until the user typed a character.
|
||||
const autoCompletePromqlKeywords: any = ref([...PROMQL_CATALOG]);
|
||||
|
||||
// True while the list belongs to a label or value position. Metrics arriving
|
||||
// in the background must not replace such a list, and an empty result in one
|
||||
// means "nothing matches here" — not "show me the language".
|
||||
let contextualSuggestions = false;
|
||||
const metricKeywords: any = ref([]);
|
||||
|
||||
const parsePromQlQuery = (query: string) => {
|
||||
|
|
@ -225,10 +230,14 @@ const usePromqlSuggestions = () => {
|
|||
);
|
||||
})
|
||||
.finally(() => {
|
||||
if (labelSuggestions) updatePromqlKeywords(labelSuggestions);
|
||||
else {
|
||||
// Back to the catalog rather than to nothing — the labels are
|
||||
// unavailable, the language is not.
|
||||
if (labelSuggestions) {
|
||||
updatePromqlKeywords(labelSuggestions, { contextual: true });
|
||||
// Nothing matched: leave the position empty and take the widget
|
||||
// away, rather than leaving an empty box open over the query.
|
||||
if (!labelSuggestions.length) autoCompleteData.value.popup.close("");
|
||||
} else {
|
||||
// The request itself failed. The labels are unavailable; the
|
||||
// language is not, so fall back to the catalog.
|
||||
updatePromqlKeywords([]);
|
||||
autoCompleteData.value.popup.close("");
|
||||
}
|
||||
|
|
@ -265,13 +274,29 @@ const usePromqlSuggestions = () => {
|
|||
return keywords;
|
||||
};
|
||||
|
||||
const updatePromqlKeywords = async (data: any[]) => {
|
||||
// A caller with something contextual to show — label names, label values —
|
||||
// replaces the list outright. Otherwise: the catalog, plus this org's
|
||||
// metrics, which sort above it.
|
||||
autoCompletePromqlKeywords.value = data.length
|
||||
? [...data]
|
||||
: [...PROMQL_CATALOG, ...metricKeywords.value];
|
||||
/** The default list: the language, plus this org's metrics above it. */
|
||||
const rebuildBaseSuggestions = () => {
|
||||
autoCompletePromqlKeywords.value = [...PROMQL_CATALOG, ...metricKeywords.value];
|
||||
contextualSuggestions = false;
|
||||
};
|
||||
|
||||
const updatePromqlKeywords = async (
|
||||
data: any[],
|
||||
{ contextual = false }: { contextual?: boolean } = {},
|
||||
) => {
|
||||
if (contextual) {
|
||||
// Verbatim, INCLUDING an empty list. A label lookup that matched nothing
|
||||
// and a plain request for the catalog both arrive as [], and treating
|
||||
// them the same put 97 function names inside `up{instance="`, where not
|
||||
// one of them can be typed.
|
||||
autoCompletePromqlKeywords.value = [...data];
|
||||
contextualSuggestions = true;
|
||||
} else if (data.length) {
|
||||
autoCompletePromqlKeywords.value = [...data];
|
||||
contextualSuggestions = true;
|
||||
} else {
|
||||
rebuildBaseSuggestions();
|
||||
}
|
||||
|
||||
await nextTick();
|
||||
autoCompleteData.value.popup.open("");
|
||||
|
|
@ -289,6 +314,13 @@ const usePromqlSuggestions = () => {
|
|||
sortText: SORT_LANE.field + metric.label,
|
||||
});
|
||||
});
|
||||
|
||||
// Rebuild what is on offer, because these arrive from a watcher AFTER the
|
||||
// list was seeded — without this a freshly opened editor showed the whole
|
||||
// catalog and not one metric name until the user edited the query. Not
|
||||
// while a label or value list is showing, and never by opening the widget:
|
||||
// this is a refresh, not an invitation.
|
||||
if (!contextualSuggestions) rebuildBaseSuggestions();
|
||||
};
|
||||
|
||||
return {
|
||||
|
|
|
|||
Loading…
Reference in New Issue