From 8bb69f6226082bbc4bf55eba5d455a7e045effb2 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Thu, 16 Oct 2025 12:57:14 +0100 Subject: [PATCH 01/14] Get suggestions by position, rather than returning every possible suggestion for a query from the typeahead helper which breaks the current plugin behaviour, todo --- lib/cql/src/cqlInput/editor/plugins/cql.ts | 9 +- lib/cql/src/lang/typeahead.spec.ts | 121 ++++----------------- lib/cql/src/lang/typeahead.ts | 25 +++-- lib/cql/src/lang/utils.ts | 64 +++++++++++ 4 files changed, 107 insertions(+), 112 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/plugins/cql.ts b/lib/cql/src/cqlInput/editor/plugins/cql.ts index 0801259..73c2972 100644 --- a/lib/cql/src/cqlInput/editor/plugins/cql.ts +++ b/lib/cql/src/cqlInput/editor/plugins/cql.ts @@ -505,8 +505,15 @@ export const createCqlPlugin = ({ typeaheadPopover?.setIsPending(); } + if (view.state.selection.from !== view.state.selection.to) { + return; + } + try { - const suggestions = await typeahead.getSuggestions(queryAst); + const suggestions = await typeahead.getSuggestions( + queryAst, + mapping.invert().map(view.state.selection.from), + ); const mappedSuggestions = toMappedSuggestions(suggestions, mapping); if (view.hasFocus()) { typeaheadPopover?.updateSuggestions(mappedSuggestions); diff --git a/lib/cql/src/lang/typeahead.spec.ts b/lib/cql/src/lang/typeahead.spec.ts index 354bf32..41b6531 100644 --- a/lib/cql/src/lang/typeahead.spec.ts +++ b/lib/cql/src/lang/typeahead.spec.ts @@ -17,6 +17,7 @@ describe("typeahead", () => { const getSuggestions = async ( query: string, + position: number, _typeahead: Typeahead = typeahead, ) => { const result = createParser()(query); @@ -25,13 +26,13 @@ describe("typeahead", () => { throw result.error; } - return await _typeahead.getSuggestions(result.queryAst); + return await _typeahead.getSuggestions(result.queryAst, position); }; it("should give all options for empty queryFields", async () => { - expect(await getSuggestions("")).toEqual([]); + expect(await getSuggestions("", 0)).toEqual([]); - expect(await getSuggestions("+:")).toEqual([ + expect(await getSuggestions("+:", 1)).toEqual([ { from: 1, to: 1, @@ -66,7 +67,7 @@ describe("typeahead", () => { }); it("should give typeahead suggestions for query meta keys", async () => { - const suggestions = await getSuggestions("+ta:"); + const suggestions = await getSuggestions("+ta:", 1); expect(suggestions).toEqual([ { from: 1, @@ -85,24 +86,10 @@ describe("typeahead", () => { ]); }); - it("should give typeahead suggestions for both query meta keys and values", async () => { - const suggestions = await getSuggestions("+tag:tags-are-magic"); + it("should give typeahead suggestions for values", async () => { + const suggestions = await getSuggestions("+tag:tags-are-magic", 5); expect(suggestions).toEqual([ - { - from: 1, - to: 3, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "Tag", - "tag", - "Search by content tags, e.g. sport/football", - ), - ], - type: "TEXT", - suffix: ":", - }, { from: 4, to: 18, @@ -115,21 +102,7 @@ describe("typeahead", () => { }); it("should give typeahead suggestions in a case insensitive way for synchronous query meta keys across value and label", async () => { - const expectedSuggestions: TypeaheadSuggestion[] = [ - { - from: 1, - position: "chipKey", - suffix: ":", - suggestions: [ - new TextSuggestionOption( - "Sync", - "sync", - "Search synchronous list of tags", - ), - ], - to: 4, - type: "TEXT", - }, + const expectedValue: TypeaheadSuggestion[] = [ { from: 5, to: 8, @@ -146,45 +119,31 @@ describe("typeahead", () => { }, ]; - const suggestionsUppercaseLabelQuery = await getSuggestions("+sync:ABC"); + const suggestionsUppercaseLabelQuery = await getSuggestions("+sync:ABC", 6); - expect(suggestionsUppercaseLabelQuery).toEqual(expectedSuggestions); + expect(suggestionsUppercaseLabelQuery).toEqual(expectedValue); - const suggestionsLowercaseLabelQuery = await getSuggestions("+sync:def"); + const suggestionsLowercaseLabelQuery = await getSuggestions("+sync:def", 6); - expect(suggestionsLowercaseLabelQuery).toEqual(expectedSuggestions); + expect(suggestionsLowercaseLabelQuery).toEqual(expectedValue); - const suggestionsUppercaseValueQuery = await getSuggestions("+sync:JKL"); + const suggestionsUppercaseValueQuery = await getSuggestions("+sync:JKL", 6); - expect(suggestionsUppercaseValueQuery).toEqual(expectedSuggestions); + expect(suggestionsUppercaseValueQuery).toEqual(expectedValue); - const suggestionsLowercaseValueQuery = await getSuggestions("+sync:ghi"); + const suggestionsLowercaseValueQuery = await getSuggestions("+sync:ghi", 6); - expect(suggestionsLowercaseValueQuery).toEqual(expectedSuggestions); + expect(suggestionsLowercaseValueQuery).toEqual(expectedValue); - const suggestionsForNonMatchingQuery = await getSuggestions("+sync:mno"); + const suggestionsForNonMatchingQuery = await getSuggestions("+sync:mno", 6); expect(suggestionsLowercaseValueQuery).not.toEqual( suggestionsForNonMatchingQuery, ); }); it("should give value suggestions for an empty string", async () => { - const suggestions = await getSuggestions("+tag:"); + const suggestions = await getSuggestions("+tag:", 4); expect(suggestions).toEqual([ - { - from: 1, - to: 3, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "Tag", - "tag", - "Search by content tags, e.g. sport/football", - ), - ], - type: "TEXT", - suffix: ":", - }, { from: 4, to: 4, @@ -197,23 +156,9 @@ describe("typeahead", () => { }); it("should give a suggestion of type DATE given e.g. 'from-date'", async () => { - const suggestions = await getSuggestions("+from-date:"); + const suggestions = await getSuggestions("+from-date:", 10); expect(suggestions).toEqual([ - { - from: 1, - to: 9, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "From date", - "from-date", - "The date to search from", - ), - ], - type: "TEXT", - suffix: ":", - }, { from: 10, to: 10, @@ -231,32 +176,10 @@ describe("typeahead", () => { ]); }); - it("should give suggestions for multiple tags", async () => { - const suggestions = await getSuggestions("+tag:a +:"); + it.only("should give suggestions for the correct tag where there is more than one", async () => { + const suggestions = await getSuggestions("+tag:a +:", 8); expect(suggestions).toEqual([ - { - from: 1, - to: 3, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "Tag", - "tag", - "Search by content tags, e.g. sport/football", - ), - ], - type: "TEXT", - suffix: ":", - }, - { - from: 4, - to: 5, - position: "chipValue", - suggestions: testTags, - type: "TEXT", - suffix: " ", - }, { from: 8, to: 8, @@ -296,7 +219,7 @@ describe("typeahead", () => { ); expect( - (await getSuggestions("+g:", typeahead)).flatMap((a) => + (await getSuggestions("+g:", 1, typeahead)).flatMap((a) => a.suggestions.map((s) => s.value), ), ).toEqual(["gnat", "tag", "stage"]); diff --git a/lib/cql/src/lang/typeahead.ts b/lib/cql/src/lang/typeahead.ts index c1d9274..5ab16db 100644 --- a/lib/cql/src/lang/typeahead.ts +++ b/lib/cql/src/lang/typeahead.ts @@ -1,4 +1,4 @@ -import { CqlQuery, CqlField } from "./ast"; +import { CqlQuery } from "./ast"; import { Token } from "./token"; import { DateSuggestionOption, @@ -6,7 +6,7 @@ import { TypeaheadSuggestion, SuggestionType, } from "./types"; -import { getCqlFieldsFromCqlBinary } from "./utils"; +import { getAstNodeAtPos } from "./utils"; type TypeaheadResolver = | ((str: string, signal?: AbortSignal) => Promise) @@ -81,8 +81,9 @@ export class Typeahead { ); } - public getSuggestions( + public async getSuggestions( program: CqlQuery, + position: number, signal?: AbortSignal, ): Promise { return new Promise((resolve, reject) => { @@ -99,13 +100,14 @@ export class Typeahead { reject(new DOMException("Aborted", "AbortError")); }); - const eventuallySuggestions = getCqlFieldsFromCqlBinary( - program.content, - ).flatMap((queryField) => this.suggestCqlField(queryField, signal)); + const { key, value } = getAstNodeAtPos(program.content, position); + console.log({key, value}) + if (!key) { + return resolve([]); + } + - return Promise.all(eventuallySuggestions) - .then((suggestions) => resolve(suggestions.flat())) - .catch(reject); + resolve(this.suggestCqlField(key, value, signal)); }); } @@ -129,10 +131,10 @@ export class Typeahead { } private async suggestCqlField( - q: CqlField, + key: Token, + value?: Token, signal?: AbortSignal, ): Promise { - const { key, value } = q; const keySuggestions = this.getSuggestionsForKeyToken(key); const maybeValueSuggestions = this.suggestFieldValue( @@ -146,7 +148,6 @@ export class Typeahead { } return maybeValueSuggestions.suggestions.then((suggestions) => [ - ...keySuggestions, { from: value ? value.start - 1 : key.end, // Extend backwards into chipKey's ':' to: value ? value.end : key.end, diff --git a/lib/cql/src/lang/utils.ts b/lib/cql/src/lang/utils.ts index 8ad929c..bafb559 100644 --- a/lib/cql/src/lang/utils.ts +++ b/lib/cql/src/lang/utils.ts @@ -1,4 +1,5 @@ import { CqlBinary, CqlExpr, CqlField } from "./ast"; +import { Token } from "./token"; const whitespaceR = /\s/; export const hasWhitespace = (str: string) => !!str.match(whitespaceR); @@ -44,6 +45,69 @@ export function* getPermutations( return permutation.slice(); } +type SuggestionPos = + | undefined + | { key: Token; value: undefined } + | { key: Token; value: Token }; + +export const getAstNodeAtPos = ( + queryBinary: CqlBinary, + position: number, +): SuggestionPos => { + return ( + getAstNodeAtPosExpr(queryBinary.left, position) ?? + (queryBinary.right + ? getAstNodeAtPos(queryBinary.right.binary, position) + : undefined) + ); +}; + +export const getAstNodeAtPosExpr = ( + expr: CqlExpr, + position: number, +): SuggestionPos => { + switch (expr.content.type) { + case "CqlStr": { + const key = isWithinRange(expr.content.token, position); + + return key + ? { + key, + value: undefined, + } + : undefined; + } + case "CqlBinary": + return getAstNodeAtPos(expr.content, position); + case "CqlGroup": + return getAstNodeAtPos(expr.content.content, position); + case "CqlField": { + const key = isWithinRange(expr.content.key, position); + const value = isWithinRange(expr.content.value, position); + + console.log(expr.content, position); + if (!key && !value) { + return undefined; + } + + if (key && !value) { + return { key, value: undefined }; + } + + return { + key: expr.content.key, + value, + }; + } + } +}; + +const isWithinRange = ( + token: Token | undefined, + position: number, +): Token | undefined => + token && position >= token.start && position <= token.end ? token : undefined; + export const getCqlFieldsFromCqlBinary = (queryBinary: CqlBinary): CqlField[] => getCqlFieldsFromQueryExpr(queryBinary.left).concat( queryBinary.right From 00202799c3011e429e611d63cf2c9eb7a39f0207 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Thu, 16 Oct 2025 13:13:41 +0100 Subject: [PATCH 02/14] Ensure one suggestion is returned from the typeahead helper at once, and adjust consumers --- lib/cql/src/cqlInput/editor/utils.ts | 22 +- .../src/cqlInput/popover/TypeaheadPopover.ts | 21 +- lib/cql/src/lang/typeahead.spec.ts | 259 +++++++++--------- lib/cql/src/lang/typeahead.ts | 60 ++-- lib/cql/src/lang/utils.ts | 9 +- 5 files changed, 176 insertions(+), 195 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index 715fc46..8349e01 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -579,18 +579,20 @@ export const toProseMirrorTokens = (tokens: Token[]): ProseMirrorToken[] => })); export const toMappedSuggestions = ( - typeaheadSuggestions: TypeaheadSuggestion[], + typeaheadSuggestion: TypeaheadSuggestion | undefined, mapping: Mapping, -) => - typeaheadSuggestions.map((suggestion) => { - const from = mapping.map(suggestion.from); - const to = mapping.map( - suggestion.to + 1, - suggestion.position === "chipKey" ? -1 : 0, - ); +) => { + if (!typeaheadSuggestion) { + return undefined; + } + const from = mapping.map(typeaheadSuggestion.from); + const to = mapping.map( + typeaheadSuggestion.to + 1, + typeaheadSuggestion.position === "chipKey" ? -1 : 0, + ); - return { ...suggestion, from, to } as MappedTypeaheadSuggestion; - }); + return { ...typeaheadSuggestion, from, to } as MappedTypeaheadSuggestion; +}; const toMappedError = (error: CqlError, mapping: Mapping) => ({ message: error.message, diff --git a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts index 5e89c06..57c0a7f 100644 --- a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts +++ b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts @@ -95,15 +95,16 @@ export class TypeaheadPopover extends Popover { }); } - public isRenderingNavigableMenu = () => this.isVisible && !!this.currentSuggestion?.suggestions.length; + public isRenderingNavigableMenu = () => + this.isVisible && !!this.currentSuggestion?.suggestions.length; public updateSuggestions = ( - typeaheadSuggestions: MappedTypeaheadSuggestion[], + typeaheadSuggestion?: MappedTypeaheadSuggestion, ) => { this.isPending = false; if ( this.view.isDestroyed || - !typeaheadSuggestions.length || + !typeaheadSuggestion || this.view.state.selection.from !== this.view.state.selection.to ) { this.currentSuggestion = undefined; @@ -112,19 +113,7 @@ export class TypeaheadPopover extends Popover { return; } - const { selection: currentSelection } = this.view.state; - const suggestionThatCoversSelection = typeaheadSuggestions.find( - ({ from, to }) => - currentSelection.from >= from && currentSelection.to <= to, - ); - - if (!suggestionThatCoversSelection) { - this.currentSuggestion = undefined; - this.hide(); - return; - } - - this.currentSuggestion = suggestionThatCoversSelection; + this.currentSuggestion = typeaheadSuggestion; this.show(); }; diff --git a/lib/cql/src/lang/typeahead.spec.ts b/lib/cql/src/lang/typeahead.spec.ts index 41b6531..644d3bc 100644 --- a/lib/cql/src/lang/typeahead.spec.ts +++ b/lib/cql/src/lang/typeahead.spec.ts @@ -30,95 +30,86 @@ describe("typeahead", () => { }; it("should give all options for empty queryFields", async () => { - expect(await getSuggestions("", 0)).toEqual([]); - - expect(await getSuggestions("+:", 1)).toEqual([ - { - from: 1, - to: 1, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "Tag", - "tag", - "Search by content tags, e.g. sport/football", - ), - new TextSuggestionOption( - "Sync", - "sync", - "Search synchronous list of tags", - ), - new TextSuggestionOption( - "Section", - "section", - "Search by content sections, e.g. section/news", - ), - new TextSuggestionOption( - "From date", - "from-date", - "The date to search from", - ), - new TextSuggestionOption("ID", "id", "The content ID"), - ], - type: "TEXT", - suffix: ":", - }, - ]); + expect(await getSuggestions("", 0)).toEqual(undefined); + + expect(await getSuggestions("+:", 1)).toEqual({ + from: 1, + to: 1, + position: "chipKey", + suggestions: [ + new TextSuggestionOption( + "Tag", + "tag", + "Search by content tags, e.g. sport/football", + ), + new TextSuggestionOption( + "Sync", + "sync", + "Search synchronous list of tags", + ), + new TextSuggestionOption( + "Section", + "section", + "Search by content sections, e.g. section/news", + ), + new TextSuggestionOption( + "From date", + "from-date", + "The date to search from", + ), + new TextSuggestionOption("ID", "id", "The content ID"), + ], + type: "TEXT", + suffix: ":", + }); }); it("should give typeahead suggestions for query meta keys", async () => { const suggestions = await getSuggestions("+ta:", 1); - expect(suggestions).toEqual([ - { - from: 1, - to: 2, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "Tag", - "tag", - "Search by content tags, e.g. sport/football", - ), - ], - type: "TEXT", - suffix: ":", - }, - ]); + expect(suggestions).toEqual({ + from: 1, + to: 2, + position: "chipKey", + suggestions: [ + new TextSuggestionOption( + "Tag", + "tag", + "Search by content tags, e.g. sport/football", + ), + ], + type: "TEXT", + suffix: ":", + }); }); it("should give typeahead suggestions for values", async () => { const suggestions = await getSuggestions("+tag:tags-are-magic", 5); - expect(suggestions).toEqual([ - { - from: 4, - to: 18, - position: "chipValue", - suggestions: testTags, - type: "TEXT", - suffix: " ", - }, - ]); + expect(suggestions).toEqual({ + from: 4, + to: 18, + position: "chipValue", + suggestions: testTags, + type: "TEXT", + suffix: " ", + }); }); it("should give typeahead suggestions in a case insensitive way for synchronous query meta keys across value and label", async () => { - const expectedValue: TypeaheadSuggestion[] = [ - { - from: 5, - to: 8, - position: "chipValue", - suggestions: [ - new TextSuggestionOption( - "abc DEF", - "GHI jkl", - "A tag with a mix of upper and lowercase strings", - ), - ], - type: "TEXT", - suffix: " ", - }, - ]; - + const expectedValue: TypeaheadSuggestion = { + from: 5, + to: 8, + position: "chipValue", + suggestions: [ + new TextSuggestionOption( + "abc DEF", + "GHI jkl", + "A tag with a mix of upper and lowercase strings", + ), + ], + type: "TEXT", + suffix: " ", + }; const suggestionsUppercaseLabelQuery = await getSuggestions("+sync:ABC", 6); expect(suggestionsUppercaseLabelQuery).toEqual(expectedValue); @@ -143,74 +134,68 @@ describe("typeahead", () => { it("should give value suggestions for an empty string", async () => { const suggestions = await getSuggestions("+tag:", 4); - expect(suggestions).toEqual([ - { - from: 4, - to: 4, - position: "chipValue", - suggestions: testTags, - type: "TEXT", - suffix: " ", - }, - ]); + expect(suggestions).toEqual({ + from: 4, + to: 4, + position: "chipValue", + suggestions: testTags, + type: "TEXT", + suffix: " ", + }); }); it("should give a suggestion of type DATE given e.g. 'from-date'", async () => { const suggestions = await getSuggestions("+from-date:", 10); - expect(suggestions).toEqual([ - { - from: 10, - to: 10, - position: "chipValue", - suggestions: [ - new DateSuggestionOption("1 day ago", "-1d"), - new DateSuggestionOption("7 days ago", "-7d"), - new DateSuggestionOption("14 days ago", "-14d"), - new DateSuggestionOption("30 days ago", "-30d"), - new DateSuggestionOption("1 year ago", "-1y"), - ], - type: "DATE", - suffix: " ", - }, - ]); + expect(suggestions).toEqual({ + from: 10, + to: 10, + position: "chipValue", + suggestions: [ + new DateSuggestionOption("1 day ago", "-1d"), + new DateSuggestionOption("7 days ago", "-7d"), + new DateSuggestionOption("14 days ago", "-14d"), + new DateSuggestionOption("30 days ago", "-30d"), + new DateSuggestionOption("1 year ago", "-1y"), + ], + type: "DATE", + suffix: " ", + }); }); it.only("should give suggestions for the correct tag where there is more than one", async () => { const suggestions = await getSuggestions("+tag:a +:", 8); - expect(suggestions).toEqual([ - { - from: 8, - to: 8, - position: "chipKey", - suggestions: [ - new TextSuggestionOption( - "Tag", - "tag", - "Search by content tags, e.g. sport/football", - ), - new TextSuggestionOption( - "Sync", - "sync", - "Search synchronous list of tags", - ), - new TextSuggestionOption( - "Section", - "section", - "Search by content sections, e.g. section/news", - ), - new TextSuggestionOption( - "From date", - "from-date", - "The date to search from", - ), - new TextSuggestionOption("ID", "id", "The content ID"), - ], - type: "TEXT", - suffix: ":", - }, - ]); + expect(suggestions).toEqual({ + from: 8, + to: 8, + position: "chipKey", + suggestions: [ + new TextSuggestionOption( + "Tag", + "tag", + "Search by content tags, e.g. sport/football", + ), + new TextSuggestionOption( + "Sync", + "sync", + "Search synchronous list of tags", + ), + new TextSuggestionOption( + "Section", + "section", + "Search by content sections, e.g. section/news", + ), + new TextSuggestionOption( + "From date", + "from-date", + "The date to search from", + ), + new TextSuggestionOption("ID", "id", "The content ID"), + ], + type: "TEXT", + suffix: ":", + }); }); it("should suggest keys that start with the search string first", async () => { @@ -219,8 +204,8 @@ describe("typeahead", () => { ); expect( - (await getSuggestions("+g:", 1, typeahead)).flatMap((a) => - a.suggestions.map((s) => s.value), + (await getSuggestions("+g:", 1, typeahead))?.suggestions.map( + (s) => s.value, ), ).toEqual(["gnat", "tag", "stage"]); }); diff --git a/lib/cql/src/lang/typeahead.ts b/lib/cql/src/lang/typeahead.ts index 5ab16db..355d0f7 100644 --- a/lib/cql/src/lang/typeahead.ts +++ b/lib/cql/src/lang/typeahead.ts @@ -85,13 +85,13 @@ export class Typeahead { program: CqlQuery, position: number, signal?: AbortSignal, - ): Promise { + ): Promise { return new Promise((resolve, reject) => { // Abort existing fetch, if it exists this.abortController?.abort(); if (!program.content) { - return resolve([]); + return resolve(undefined); } const abortController = new AbortController(); @@ -100,41 +100,42 @@ export class Typeahead { reject(new DOMException("Aborted", "AbortError")); }); - const { key, value } = getAstNodeAtPos(program.content, position); - console.log({key, value}) - if (!key) { - return resolve([]); + const maybeSuggestionAtPos = getAstNodeAtPos(program.content, position); + + if (!maybeSuggestionAtPos) { + return resolve(undefined); } + const { key, value } = maybeSuggestionAtPos; resolve(this.suggestCqlField(key, value, signal)); }); } - private getSuggestionsForKeyToken(keyToken: Token): TypeaheadSuggestion[] { + private getSuggestionsForKeyToken( + keyToken: Token, + ): TypeaheadSuggestion | undefined { const suggestions = this.suggestFieldKey(keyToken.literal ?? ""); if (!suggestions) { - return []; + return undefined; } - return [ - { - from: keyToken.start, - to: Math.max(keyToken.start, keyToken.end - 1), // Do not include ':' - position: "chipKey", - suggestions, - type: "TEXT", - suffix: ":", - }, - ]; + return { + from: keyToken.start, + to: Math.max(keyToken.start, keyToken.end - 1), // Do not include ':' + position: "chipKey", + suggestions, + type: "TEXT", + suffix: ":", + }; } private async suggestCqlField( key: Token, value?: Token, signal?: AbortSignal, - ): Promise { + ): Promise { const keySuggestions = this.getSuggestionsForKeyToken(key); const maybeValueSuggestions = this.suggestFieldValue( @@ -147,16 +148,17 @@ export class Typeahead { return Promise.resolve(keySuggestions); } - return maybeValueSuggestions.suggestions.then((suggestions) => [ - { - from: value ? value.start - 1 : key.end, // Extend backwards into chipKey's ':' - to: value ? value.end : key.end, - position: "chipValue", - suggestions, - type: maybeValueSuggestions.type, - suffix: " ", - } as TypeaheadSuggestion, - ]); + return maybeValueSuggestions.suggestions.then( + (suggestions) => + ({ + from: value ? value.start - 1 : key.end, // Extend backwards into chipKey's ':' + to: value ? value.end : key.end, + position: "chipValue", + suggestions, + type: maybeValueSuggestions.type, + suffix: " ", + }) as TypeaheadSuggestion, + ); } private suggestFieldKey(str: string): TextSuggestionOption[] | undefined { diff --git a/lib/cql/src/lang/utils.ts b/lib/cql/src/lang/utils.ts index bafb559..64fb9a9 100644 --- a/lib/cql/src/lang/utils.ts +++ b/lib/cql/src/lang/utils.ts @@ -85,7 +85,6 @@ export const getAstNodeAtPosExpr = ( const key = isWithinRange(expr.content.key, position); const value = isWithinRange(expr.content.value, position); - console.log(expr.content, position); if (!key && !value) { return undefined; } @@ -105,9 +104,13 @@ export const getAstNodeAtPosExpr = ( const isWithinRange = ( token: Token | undefined, position: number, -): Token | undefined => - token && position >= token.start && position <= token.end ? token : undefined; +): Token | undefined => { + + const isWithinRange = token && position >= token.start && position <= token.end ? token : undefined; + console.log({type: token?.tokenType, position, isWithinRange}); + return isWithinRange; +} export const getCqlFieldsFromCqlBinary = (queryBinary: CqlBinary): CqlField[] => getCqlFieldsFromQueryExpr(queryBinary.left).concat( queryBinary.right From e349d680a25f4bf84ad59e54be6158d4fb227580 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Thu, 16 Oct 2025 13:25:32 +0100 Subject: [PATCH 03/14] Fix issue where keys would be suggested when values were present --- lib/cql/src/lang/typeahead.ts | 27 ++++++++++++++------------- lib/cql/src/lang/utils.ts | 10 ++++------ lib/cql/src/page.ts | 6 ++---- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/cql/src/lang/typeahead.ts b/lib/cql/src/lang/typeahead.ts index 355d0f7..a76729f 100644 --- a/lib/cql/src/lang/typeahead.ts +++ b/lib/cql/src/lang/typeahead.ts @@ -136,7 +136,9 @@ export class Typeahead { value?: Token, signal?: AbortSignal, ): Promise { - const keySuggestions = this.getSuggestionsForKeyToken(key); + if (!value) { + return this.getSuggestionsForKeyToken(key); + } const maybeValueSuggestions = this.suggestFieldValue( key.literal ?? "", @@ -145,20 +147,19 @@ export class Typeahead { ); if (!maybeValueSuggestions) { - return Promise.resolve(keySuggestions); + return; } - return maybeValueSuggestions.suggestions.then( - (suggestions) => - ({ - from: value ? value.start - 1 : key.end, // Extend backwards into chipKey's ':' - to: value ? value.end : key.end, - position: "chipValue", - suggestions, - type: maybeValueSuggestions.type, - suffix: " ", - }) as TypeaheadSuggestion, - ); + const suggestions = await maybeValueSuggestions.suggestions; + + return { + from: value ? value.start - 1 : key.end, // Extend backwards into chipKey's ':' + to: value ? value.end : key.end, + position: "chipValue", + suggestions, + type: maybeValueSuggestions.type, + suffix: " ", + } as TypeaheadSuggestion; } private suggestFieldKey(str: string): TextSuggestionOption[] | undefined { diff --git a/lib/cql/src/lang/utils.ts b/lib/cql/src/lang/utils.ts index 64fb9a9..84036ae 100644 --- a/lib/cql/src/lang/utils.ts +++ b/lib/cql/src/lang/utils.ts @@ -105,12 +105,10 @@ const isWithinRange = ( token: Token | undefined, position: number, ): Token | undefined => { - - - const isWithinRange = token && position >= token.start && position <= token.end ? token : undefined; - console.log({type: token?.tokenType, position, isWithinRange}); - return isWithinRange; -} + return token && position >= token.start && position <= token.end + ? token + : undefined; +}; export const getCqlFieldsFromCqlBinary = (queryBinary: CqlBinary): CqlField[] => getCqlFieldsFromQueryExpr(queryBinary.left).concat( queryBinary.right diff --git a/lib/cql/src/page.ts b/lib/cql/src/page.ts index 23f7f9f..14fe1cc 100644 --- a/lib/cql/src/page.ts +++ b/lib/cql/src/page.ts @@ -11,6 +11,7 @@ import { Typeahead, TypeaheadField } from "./lang/typeahead.ts"; import { CapiTypeaheadProvider } from "./typeahead/CapiTypeaheadHelpers.ts"; import { toolsSuggestionOptionResolvers } from "./typeahead/tools-index/config"; import { DebugChangeEventDetail, QueryChangeEventDetail } from "./types/dom"; +import { TestTypeaheadHelpers } from "./lang/fixtures/TestTypeaheadHelpers.ts"; const setUrlParam = (key: string, value: string) => { const urlParams = new URLSearchParams(window.location.search); @@ -131,10 +132,7 @@ const params = new URLSearchParams(window.location.search); const endpoint = params.get("endpoint"); const initialEndpointCapi = endpoint || "https://content.guardianapis.com"; -const typeaheadHelpersCapi = new CapiTypeaheadProvider( - initialEndpointCapi, - "test", -); +const typeaheadHelpersCapi = new TestTypeaheadHelpers(); const capiTypeahead = new Typeahead(typeaheadHelpersCapi.typeaheadFields); const CqlInputCapi = createCqlInput(capiTypeahead, { From 9f4dab64b2ab96738feefb9666c477ee68aa23d2 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Mon, 20 Oct 2025 14:42:10 +0100 Subject: [PATCH 04/14] Improve test failure messaging for assertCqlStrPosFromDocPos --- lib/cql/src/lang/typeahead.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cql/src/lang/typeahead.spec.ts b/lib/cql/src/lang/typeahead.spec.ts index 644d3bc..f966d06 100644 --- a/lib/cql/src/lang/typeahead.spec.ts +++ b/lib/cql/src/lang/typeahead.spec.ts @@ -163,7 +163,7 @@ describe("typeahead", () => { }); }); - it.only("should give suggestions for the correct tag where there is more than one", async () => { + it("should give suggestions for the correct tag where there is more than one", async () => { const suggestions = await getSuggestions("+tag:a +:", 8); expect(suggestions).toEqual({ From a96762c44408fa687a3ad2a3a992c7a3ea51b80e Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Fri, 24 Oct 2025 21:52:57 +0100 Subject: [PATCH 05/14] Use ProseMirror positions for typeahead suggestions (which require a selection position either side of a character to perform correctly), and adjust debug visualisation --- lib/cql/index.html | 12 +++++-- lib/cql/src/cqlInput/editor/debug.ts | 44 ++++++++++++++++------- lib/cql/src/cqlInput/editor/utils.spec.ts | 10 ++++-- lib/cql/src/cqlInput/editor/utils.ts | 30 +++++++++------- lib/cql/src/lang/typeahead.spec.ts | 26 +++++++------- lib/cql/src/lang/typeahead.ts | 16 ++++----- lib/cql/src/lang/utils.ts | 40 ++++++++++++++++----- lib/cql/src/page.ts | 3 +- 8 files changed, 120 insertions(+), 61 deletions(-) diff --git a/lib/cql/index.html b/lib/cql/index.html index b0c7b0e..216b163 100644 --- a/lib/cql/index.html +++ b/lib/cql/index.html @@ -162,11 +162,19 @@ flex-direction: column; } - .CqlDebug__queryDiagramToken, - .CqlDebug__queryDiagramNode { + .CqlDebug__queryDiagramToken .CqlDebug__queryDiagramNode { margin-bottom: 6rem; } + .CqlDebug__queryDiagramToken .CqlDebug__queryIndex, + .CqlDebug__queryDiagramToken .CqlDebug__selection { + left: -50%; + } + + .CqlDebug__queryDiagramToken .CqlDebug__queryIndex { + position: relative; + } + .CqlDebug__queryDiagramNode > .CqlDebug__queryDiagramLabel { padding-top: 0rem; } diff --git a/lib/cql/src/cqlInput/editor/debug.ts b/lib/cql/src/cqlInput/editor/debug.ts index c66b60b..28d36ea 100644 --- a/lib/cql/src/cqlInput/editor/debug.ts +++ b/lib/cql/src/cqlInput/editor/debug.ts @@ -11,6 +11,7 @@ import { } from "../../lang/ast"; import { IS_READ_ONLY } from "./schema"; import { Selection } from "prosemirror-state"; +import { toProseMirrorTokens } from "./utils"; // Debugging and visualisation utilities. @@ -32,7 +33,11 @@ export const logNode = (doc: Node) => { }); }; -export const getDebugTokenHTML = (tokens: Token[], selection: Selection, mapping: Mapping) => { +export const getDebugTokenHTML = ( + tokens: Token[], + selection: Selection, + mapping: Mapping, +) => { let html = `
@@ -40,11 +45,14 @@ export const getDebugTokenHTML = (tokens: Token[], selection: Selection, mapping
Literal
`; + const invertedMapping = mapping.invert(); const mappedFrom = invertedMapping.map(selection.from); const mappedTo = invertedMapping.map(selection.to); - tokens.forEach((token, index) => { + const pmTokens = toProseMirrorTokens(tokens); + + pmTokens.forEach((token, index) => { html += `${Array(Math.max(1, token.lexeme.length)) .fill(undefined) .map((_, index) => { @@ -54,7 +62,7 @@ export const getDebugTokenHTML = (tokens: Token[], selection: Selection, mapping ); const literalChar = token.literal?.[index - literalOffset]; - const globalIndex = token.start + index + const globalIndex = token.from + index; return `
${globalIndex}
@@ -64,7 +72,9 @@ export const getDebugTokenHTML = (tokens: Token[], selection: Selection, mapping : "" } ${ - mappedTo === globalIndex ? `
$
` : "" + mappedTo === globalIndex + ? `
$
` + : "" } ${ lexemeChar !== undefined @@ -85,12 +95,18 @@ export const getDebugTokenHTML = (tokens: Token[], selection: Selection, mapping }) .join("")} ${ - tokens[index + 1]?.start > token.end + 1 && - tokens[index + 1]?.tokenType !== "EOF" && - token.tokenType !== "EOF" + pmTokens[index + 1]?.from > token.to && token.tokenType !== "EOF" ? `
${ - token.end + 1 - }
` + token.to + }
${ + mappedFrom === token.to + ? `
^
` + : "" + }${ + mappedTo === token.to + ? `
$
` + : "" + }
` : "" }`; }); @@ -173,10 +189,12 @@ export const getDebugMappingHTML = ( `
${pos}
- ${(queryPosMap[pos] ?? []).map( - ({ char }) => - `
${char}
`, - ).join(" ")} + ${(queryPosMap[pos] ?? []) + .map( + ({ char }) => + `
${char}
`, + ) + .join(" ")} ${ diff --git a/lib/cql/src/cqlInput/editor/utils.spec.ts b/lib/cql/src/cqlInput/editor/utils.spec.ts index 74f61a3..d9bfc2f 100644 --- a/lib/cql/src/cqlInput/editor/utils.spec.ts +++ b/lib/cql/src/cqlInput/editor/utils.spec.ts @@ -290,12 +290,18 @@ describe("utils", () => { expect(text).toEqual(["", "key", "value", "", "key2", "value2", ""]); }); - it("with an incomplete chip", async () => { + it("with an empty chip key", async () => { const text = await getTextFromTokenRanges("+: a b c"); expect(text).toEqual(["", "", "a", "b", "c", ""]); }); + it("with an empty chip value", async () => { + const text = await getTextFromTokenRanges("+a: b"); + + expect(text).toEqual(["", "a", "b", ""]); + }); + it("with chips without prefixes", async () => { const text = await getTextFromTokenRanges("this:that this:that"); @@ -335,7 +341,7 @@ describe("utils", () => { assertCqlStrPosFromDocPos( "+a:", (node) => findNodeAt(0, node, schema.nodes.chipValue) + 1, - 2, + 3, ); }); diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index 8349e01..f5a3122 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -83,19 +83,21 @@ const getFieldKeyRange = ( from: number, to: number, isQuoted: boolean, - isFollowedByEmptyValue: boolean, + isFollowedByChipValue: boolean, ): [number, number, number][] => { const quoteOffset = isQuoted ? 1 : 0; - const emptyValueOffset = isFollowedByEmptyValue ? 1 : 0; return [ // chip begin (-1) // chipKey begin (-1) - [from, 0, 2], + [from, 0, 1], // maybe start quote (+1 to remove from str) [from, quoteOffset, 0], // maybe end quote (+1 to remove from str) // offset `:` into chipValue (+1 to node) - [to - 1, quoteOffset, 1 + emptyValueOffset], + [to - 1, quoteOffset, 1], + ...(!isFollowedByChipValue + ? getFieldValueRanges(to - 1, to - 1, false) + : []), ]; }; @@ -130,6 +132,7 @@ const getQueryStrRanges = ( // This is a slightly obscure solution to this problem - we could also use the // gaps between the token positions and the token literal length to account // for this discrepancy, too. + [to + 1, -1, 0], ...(from === to ? [[to, -1, 0] as [number, number, number]] : []), ]; @@ -190,6 +193,7 @@ export const createProseMirrorTokenToDocumentMap = ( const ranges = compactedTokenRanges.reduce<[number, number, number][]>( (accRanges, { tokenType, from, to, literal }, index, tokens) => { const previousToken = tokens[index - 1] as ProseMirrorToken | undefined; + const nextToken = tokens[index + 1] as ProseMirrorToken | undefined; switch (tokenType) { case TokenType.PLUS: @@ -200,15 +204,13 @@ export const createProseMirrorTokenToDocumentMap = ( ]); } case TokenType.CHIP_KEY: { - const isFollowedByFieldValueToken = - tokens[index + 1]?.tokenType === "CHIP_VALUE"; return accRanges.concat( ...maybeQueryStrRanges(previousToken, index), - getFieldKeyRange( + ...getFieldKeyRange( from, to, shouldQuoteFieldValue(literal ?? ""), - !isFollowedByFieldValueToken, + nextToken?.tokenType === TokenType.CHIP_VALUE, ), ); } @@ -572,11 +574,13 @@ export type ProseMirrorToken = Omit & { * 0 1 2 3 */ export const toProseMirrorTokens = (tokens: Token[]): ProseMirrorToken[] => - tokens.map(({ start, end, ...token }) => ({ - ...token, - from: start, - to: end + 1, - })); + tokens.map(toProseMirrorToken); + +export const toProseMirrorToken = ({ start, end, ...token }: Token) => ({ + ...token, + from: start, + to: end + 1, +}); export const toMappedSuggestions = ( typeaheadSuggestion: TypeaheadSuggestion | undefined, diff --git a/lib/cql/src/lang/typeahead.spec.ts b/lib/cql/src/lang/typeahead.spec.ts index f966d06..d822f79 100644 --- a/lib/cql/src/lang/typeahead.spec.ts +++ b/lib/cql/src/lang/typeahead.spec.ts @@ -34,7 +34,7 @@ describe("typeahead", () => { expect(await getSuggestions("+:", 1)).toEqual({ from: 1, - to: 1, + to: 2, position: "chipKey", suggestions: [ new TextSuggestionOption( @@ -68,7 +68,7 @@ describe("typeahead", () => { const suggestions = await getSuggestions("+ta:", 1); expect(suggestions).toEqual({ from: 1, - to: 2, + to: 4, position: "chipKey", suggestions: [ new TextSuggestionOption( @@ -86,8 +86,8 @@ describe("typeahead", () => { const suggestions = await getSuggestions("+tag:tags-are-magic", 5); expect(suggestions).toEqual({ - from: 4, - to: 18, + from: 5, + to: 19, position: "chipValue", suggestions: testTags, type: "TEXT", @@ -97,8 +97,8 @@ describe("typeahead", () => { it("should give typeahead suggestions in a case insensitive way for synchronous query meta keys across value and label", async () => { const expectedValue: TypeaheadSuggestion = { - from: 5, - to: 8, + from: 6, + to: 9, position: "chipValue", suggestions: [ new TextSuggestionOption( @@ -133,10 +133,10 @@ describe("typeahead", () => { }); it("should give value suggestions for an empty string", async () => { - const suggestions = await getSuggestions("+tag:", 4); + const suggestions = await getSuggestions("+tag:", 5); expect(suggestions).toEqual({ - from: 4, - to: 4, + from: 5, + to: 6, position: "chipValue", suggestions: testTags, type: "TEXT", @@ -145,11 +145,11 @@ describe("typeahead", () => { }); it("should give a suggestion of type DATE given e.g. 'from-date'", async () => { - const suggestions = await getSuggestions("+from-date:", 10); + const suggestions = await getSuggestions("+from-date:", 11); expect(suggestions).toEqual({ - from: 10, - to: 10, + from: 11, + to: 12, position: "chipValue", suggestions: [ new DateSuggestionOption("1 day ago", "-1d"), @@ -168,7 +168,7 @@ describe("typeahead", () => { expect(suggestions).toEqual({ from: 8, - to: 8, + to: 9, position: "chipKey", suggestions: [ new TextSuggestionOption( diff --git a/lib/cql/src/lang/typeahead.ts b/lib/cql/src/lang/typeahead.ts index a76729f..0067835 100644 --- a/lib/cql/src/lang/typeahead.ts +++ b/lib/cql/src/lang/typeahead.ts @@ -1,5 +1,5 @@ +import { ProseMirrorToken } from "../cqlInput/editor/utils"; import { CqlQuery } from "./ast"; -import { Token } from "./token"; import { DateSuggestionOption, TextSuggestionOption, @@ -113,7 +113,7 @@ export class Typeahead { } private getSuggestionsForKeyToken( - keyToken: Token, + keyToken: ProseMirrorToken, ): TypeaheadSuggestion | undefined { const suggestions = this.suggestFieldKey(keyToken.literal ?? ""); @@ -122,8 +122,8 @@ export class Typeahead { } return { - from: keyToken.start, - to: Math.max(keyToken.start, keyToken.end - 1), // Do not include ':' + from: keyToken.from, + to: Math.max(keyToken.to, keyToken.to - 1), // Do not include ':' position: "chipKey", suggestions, type: "TEXT", @@ -132,8 +132,8 @@ export class Typeahead { } private async suggestCqlField( - key: Token, - value?: Token, + key: ProseMirrorToken, + value?: ProseMirrorToken, signal?: AbortSignal, ): Promise { if (!value) { @@ -153,8 +153,8 @@ export class Typeahead { const suggestions = await maybeValueSuggestions.suggestions; return { - from: value ? value.start - 1 : key.end, // Extend backwards into chipKey's ':' - to: value ? value.end : key.end, + from: value ? value.from : key.from, // Extend backwards into chipKey's ':' + to: value ? value.to : key.to, position: "chipValue", suggestions, type: maybeValueSuggestions.type, diff --git a/lib/cql/src/lang/utils.ts b/lib/cql/src/lang/utils.ts index 84036ae..6da84c1 100644 --- a/lib/cql/src/lang/utils.ts +++ b/lib/cql/src/lang/utils.ts @@ -1,5 +1,10 @@ +import { + ProseMirrorToken, + toProseMirrorToken, + toProseMirrorTokens, +} from "../cqlInput/editor/utils"; import { CqlBinary, CqlExpr, CqlField } from "./ast"; -import { Token } from "./token"; +import { Token, TokenType } from "./token"; const whitespaceR = /\s/; export const hasWhitespace = (str: string) => !!str.match(whitespaceR); @@ -47,8 +52,8 @@ export function* getPermutations( type SuggestionPos = | undefined - | { key: Token; value: undefined } - | { key: Token; value: Token }; + | { key: ProseMirrorToken; value: undefined } + | { key: ProseMirrorToken; value: ProseMirrorToken }; export const getAstNodeAtPos = ( queryBinary: CqlBinary, @@ -90,11 +95,25 @@ export const getAstNodeAtPosExpr = ( } if (key && !value) { - return { key, value: undefined }; + return position === key.to + ? { + key, + // Add an empty value here to signal that we are in value position + value: toProseMirrorToken( + new Token( + TokenType.CHIP_VALUE, + "", + undefined, + position, + position, + ), + ), + } + : { key, value: undefined }; } return { - key: expr.content.key, + key: toProseMirrorToken(expr.content.key), value, }; } @@ -104,9 +123,14 @@ export const getAstNodeAtPosExpr = ( const isWithinRange = ( token: Token | undefined, position: number, -): Token | undefined => { - return token && position >= token.start && position <= token.end - ? token +): ProseMirrorToken | undefined => { + if (!token) { + return undefined; + } + + const [pmToken] = toProseMirrorTokens([token]); + return position >= pmToken.from && position <= pmToken.to + ? pmToken : undefined; }; export const getCqlFieldsFromCqlBinary = (queryBinary: CqlBinary): CqlField[] => diff --git a/lib/cql/src/page.ts b/lib/cql/src/page.ts index 14fe1cc..2bfd50f 100644 --- a/lib/cql/src/page.ts +++ b/lib/cql/src/page.ts @@ -8,7 +8,6 @@ import { } from "./cqlInput/editor/debug.ts"; import { createParser } from "./lang/Cql.ts"; import { Typeahead, TypeaheadField } from "./lang/typeahead.ts"; -import { CapiTypeaheadProvider } from "./typeahead/CapiTypeaheadHelpers.ts"; import { toolsSuggestionOptionResolvers } from "./typeahead/tools-index/config"; import { DebugChangeEventDetail, QueryChangeEventDetail } from "./types/dom"; import { TestTypeaheadHelpers } from "./lang/fixtures/TestTypeaheadHelpers.ts"; @@ -64,7 +63,7 @@ const handleDebugChangeEvent = (e: CustomEvent) => { debugMappingContainer.innerHTML = `

Original query:

${getOriginalQueryHTML(queryStr)} -

Tokenises to:

+

Tokenises to (ProseMirror positions):

${getDebugTokenHTML(tokens, selection, mapping)} ${ queryAst From 8890990513dce1b19a806e7981dffd0b70fdbef3 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Sat, 25 Oct 2025 18:09:18 +0100 Subject: [PATCH 06/14] Do not attempt to show the popover if no popover exists --- lib/cql/src/cqlInput/popover/TypeaheadPopover.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts index 57c0a7f..e114db9 100644 --- a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts +++ b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts @@ -130,12 +130,17 @@ export class TypeaheadPopover extends Popover { } }; - protected show = () => { + protected show = async () => { if (!this.currentSuggestion) { return Promise.resolve(); } const { node } = this.view.domAtPos(this.currentSuggestion.from); - return super.show(node as HTMLElement); + + if (node) { + return super.show(node as HTMLElement); + } + + console.warn(`[cql]: Attempted to show popover, but domAtPos did not return an element for ${this.currentSuggestion.from}`) }; public handleAction: ActionHandler = (action) => { From 79acd5dc693dac91f6498d4f2739d1e67ca8805a Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Thu, 30 Oct 2025 19:13:20 +0000 Subject: [PATCH 07/14] Fix remaining tests --- lib/cql/src/cqlInput/editor/plugins/cql.spec.ts | 12 ++++++------ lib/cql/src/cqlInput/editor/plugins/cql.ts | 8 ++++---- lib/cql/src/cqlInput/popover/TypeaheadPopover.ts | 9 ++++++--- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts b/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts index ecc1d56..b6d6b8c 100644 --- a/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts +++ b/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts @@ -432,7 +432,7 @@ describe("cql plugin", () => { const { editor, container, moveCaretToQueryPos } = createCqlEditor("example +tag:"); - await moveCaretToQueryPos(queryStr.length - 1); + await moveCaretToQueryPos(queryStr.length); await editor.insertText("t"); const popoverContainer = await findByTestId( @@ -448,7 +448,7 @@ describe("cql plugin", () => { const { editor, container, waitFor, moveCaretToQueryPos } = createCqlEditor("example +tag:"); - await moveCaretToQueryPos(queryStr.length - 1); + await moveCaretToQueryPos(queryStr.length); await editor.insertText("t"); await selectPopoverOptionWithEnter( editor, @@ -464,7 +464,7 @@ describe("cql plugin", () => { const { editor, container, waitFor, moveCaretToQueryPos } = createCqlEditor("example +tag:"); - await moveCaretToQueryPos(queryStr.length - 1); + await moveCaretToQueryPos(queryStr.length); await editor.insertText("t"); await selectPopoverOptionWithClick( container, @@ -538,7 +538,7 @@ describe("cql plugin", () => { const { editor, container, moveCaretToQueryPos } = createCqlEditor(queryStr); - await moveCaretToQueryPos(queryStr.length - 1); + await moveCaretToQueryPos(queryStr.length); await findByText(container, "1 day ago"); await editor.press("Enter"); @@ -549,7 +549,7 @@ describe("cql plugin", () => { const { editor, waitFor, container, moveCaretToQueryPos } = createCqlEditor(queryStr); - await moveCaretToQueryPos(queryStr.length - 1); + await moveCaretToQueryPos(queryStr.length); const popoverContainer = await findByTestId(container, typeaheadTestId); await findByText(popoverContainer, "1 day ago"); @@ -565,7 +565,7 @@ describe("cql plugin", () => { const { editor, waitFor, container, moveCaretToQueryPos } = createCqlEditor(queryStr); - await moveCaretToQueryPos(queryStr.length - 1); + await moveCaretToQueryPos(queryStr.length); const popoverContainer = await findByTestId(container, typeaheadTestId); await findByText(popoverContainer, "1 day ago"); diff --git a/lib/cql/src/cqlInput/editor/plugins/cql.ts b/lib/cql/src/cqlInput/editor/plugins/cql.ts index 73c2972..0c8cddd 100644 --- a/lib/cql/src/cqlInput/editor/plugins/cql.ts +++ b/lib/cql/src/cqlInput/editor/plugins/cql.ts @@ -135,10 +135,10 @@ export const createCqlPlugin = ({ const queryBeforeParse = docToCqlStr(originalDoc); - const result = parser(queryBeforeParse); - const { tokens, queryAst, error, mapping } = mapResult(result); - - const newDoc = tokensToDoc(tokens); + const newDoc = tokensToDoc(mapResult(parser(queryBeforeParse)).tokens); + const queryAfterParse = docToCqlStr(newDoc); + const result = parser(queryAfterParse); + const { mapping, queryAst, tokens, error } = mapResult(result); const docSelection = new AllSelection(tr.doc); diff --git a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts index e114db9..2ad694d 100644 --- a/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts +++ b/lib/cql/src/cqlInput/popover/TypeaheadPopover.ts @@ -136,11 +136,14 @@ export class TypeaheadPopover extends Popover { } const { node } = this.view.domAtPos(this.currentSuggestion.from); - if (node) { - return super.show(node as HTMLElement); + // The node given from `domAtPos` may be a text node + if (node && node instanceof HTMLElement) { + return super.show(node); } - console.warn(`[cql]: Attempted to show popover, but domAtPos did not return an element for ${this.currentSuggestion.from}`) + console.warn( + `[cql]: Attempted to show popover, but domAtPos did not return an element for ${this.currentSuggestion.from}`, + ); }; public handleAction: ActionHandler = (action) => { From 8aec806a28d067ad6d56018e14a208295aae5363 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Sun, 2 Nov 2025 22:36:29 +0000 Subject: [PATCH 08/14] Add position-based mapping tests, and begin work on smoke test --- lib/cql/src/cqlInput/editor/utils.spec.ts | 333 ++++++++++++++-------- lib/cql/src/cqlInput/editor/utils.ts | 4 +- lib/cql/src/lang/utils.ts | 12 + 3 files changed, 236 insertions(+), 113 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/utils.spec.ts b/lib/cql/src/cqlInput/editor/utils.spec.ts index d9bfc2f..e866602 100644 --- a/lib/cql/src/cqlInput/editor/utils.spec.ts +++ b/lib/cql/src/cqlInput/editor/utils.spec.ts @@ -12,6 +12,7 @@ import { POLARITY, schema } from "./schema"; import { builders } from "prosemirror-test-builder"; import { createParser } from "../../lang/Cql"; import { Node } from "prosemirror-model"; +import { getNPermutations, getPermutations } from "../../lang/utils"; describe("utils", () => { const { chip, chipKey, chipValue, doc, queryStr } = builders(schema); @@ -214,119 +215,89 @@ describe("utils", () => { describe("mapTokens", () => { describe("should map tokens to text positions", () => { - it("with parens", async () => { - const text = await getTextFromTokenRanges("(b OR c)"); - - expect(text).toEqual(["(", "b", "OR", "c", ")", ""]); - }); - - it("with search text and tag", async () => { - const text = await getTextFromTokenRanges("text +key:value text"); - - expect(text).toEqual(["text", "", "key", "value", "text", ""]); - }); - - it("with parens and tags", async () => { - const text = await getTextFromTokenRanges( + const specs: [string, string, string[]][] = [ + ["with parens", "(b OR c)", ["(", "b", "OR", "c", ")", ""]], + [ + "with search text and tag", + "text +key:value text", + ["text", "", "key", "value", "text", ""], + ], + [ + "with parens and tags", "text (b OR c) +key:value text (b OR c)", - ); - - expect(text).toEqual([ - "text", - "(", - "b", - "OR", - "c", - ")", - "", - "key", - "value", - "text", - "(", - "b", - "OR", - "c", - ")", - "", - ]); - }); - - it("with a query field", async () => { - const text = await getTextFromTokenRanges("+tag:test"); - - expect(text).toEqual(["", "tag", "test", ""]); - }); - - it("with a query field with a quoted value and whitespace", async () => { - const text = await getTextFromTokenRanges('+tag:"1 2" a +tag:"3 4" b'); - - expect(text).toEqual([ - "", - "tag", - "1 2", - "a", - "", - "tag", - "3 4", - "b", - "", - ]); - }); - - it("with a query field with a quoted key", async () => { - const text = await getTextFromTokenRanges('+"ta g":"1 2"'); - - expect(text).toEqual(["", "ta g", "1 2", ""]); - }); - - it("with polarity symbols", async () => { - const text = await getTextFromTokenRanges("+example -example"); - - expect(text).toEqual(["+", "example", "-", "example", ""]); - }); - - it("with two queries", async () => { - const text = await getTextFromTokenRanges("+key:value +key2:value2 "); - expect(text).toEqual(["", "key", "value", "", "key2", "value2", ""]); - }); - - it("with an empty chip key", async () => { - const text = await getTextFromTokenRanges("+: a b c"); - - expect(text).toEqual(["", "", "a", "b", "c", ""]); - }); - - it("with an empty chip value", async () => { - const text = await getTextFromTokenRanges("+a: b"); - - expect(text).toEqual(["", "a", "b", ""]); - }); - - it("with chips without prefixes", async () => { - const text = await getTextFromTokenRanges("this:that this:that"); - - expect(text).toEqual(["this", "that", "this", "that", ""]); - }); - - it("with binary queries in the middle of tags", async () => { - const text = await getTextFromTokenRanges( + [ + "text", + "(", + "b", + "OR", + "c", + ")", + "", + "key", + "value", + "text", + "(", + "b", + "OR", + "c", + ")", + "", + ], + ], + ["with a query field", "+tag:test", ["", "tag", "test", ""]], + [ + "with a query field with a quoted value and whitespace", + '+tag:"1 2" a +tag:"3 4" b', + ["", "tag", "1 2", "a", "", "tag", "3 4", "b", ""], + ], + [ + "with a query field with a quoted key", + '+"ta g":"1 2"', + ["", "ta g", "1 2", ""], + ], + [ + "with polarity symbols", + "+example -example", + ["+", "example", "-", "example", ""], + ], + [ + "with two queries", + "+key:value +key2:value2 ", + ["", "key", "value", "", "key2", "value2", ""], + ], + ["with an empty chip key", "+: a b c", ["", "", "a", "b", "c", ""]], + ["with an empty chip value", "+a: b", ["", "a", "b", ""]], + [ + "with chips without prefixes", + "this:that this:that", + ["this", "that", "this", "that", ""], + ], + [ + "with binary queries in the middle of tags", "+key:value (a OR b) +key2:value2", - ); - - expect(text).toEqual([ - "", - "key", - "value", - "(", - "a", - "OR", - "b", - ")", - "", - "key2", - "value2", - "", - ]); + [ + "", + "key", + "value", + "(", + "a", + "OR", + "b", + ")", + "", + "key2", + "value2", + "", + ], + ], + ]; + + specs.forEach(([specName, query, expectedTextFromRanges]) => { + it(specName, async () => { + const text = await getTextFromTokenRanges(query); + + expect(text).toEqual(expectedTextFromRanges); + }); }); it("with selection in an empty chip key", () => { @@ -352,6 +323,146 @@ describe("utils", () => { 4, ); }); + + const getCqlStrPositions = ( + queryWithPos: string, + positions: Record = {}, + ): { query: string; positions: Record } => { + const regex = new RegExp("<(?.*?)>"); + const result = regex.exec(queryWithPos); + if (!result || !result.groups?.position) { + return { query: queryWithPos, positions }; + } + + return getCqlStrPositions(queryWithPos.replace(regex, ""), { + ...positions, + [result.groups?.position]: result.index, + }); + }; + + const mappingSpecs = [ + { + name: "a query string", + queryAndPositions: "a", + doc: doc(queryStr("a")), + }, + { + name: "a chip", + queryAndPositions: "+b:c d", + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey("b"), chipValue("c")), + // Empty queryStr + queryStr("d"), + ), + }, + { + name: "an empty chipKey", + queryAndPositions: "+: ", + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey(""), chipValue()), + // Empty queryStr + queryStr(""), + ), + }, + + { + name: "abutting empty chipKeys", + queryAndPositions: "+: +: ", + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey(""), chipValue()), + // Empty queryStr + queryStr(""), + // Empty chipKey + chip(chipKey(""), chipValue()), + ), + }, + { + name: "an empty chipValue", + queryAndPositions: "+b: d", + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey("b"), chipValue("")), + // Empty queryStr + queryStr("d"), + ), + }, + ]; + + const smokeTestPrimitives: Array< + (key: string, key2: string) => [string, Node] + > = [ + // Empty queryStr + () => ["", queryStr()], + // queryStr w/ content + (key) => [ + `<${key}1>${key}<${key}2> `, + queryStr(`<${key}1>${key}<${key}2>`), + ], + // Chip w/ empty chipKey + (key) => [`+<${key}>: `, chip(chipKey(`<${key}>`), chipValue())], + // Chip w/ empty chipValue + (key) => [ + `+chipKey:<${key}> `, + chip(chipKey(`chipKey`), chipValue(`<${key}>`)), + ], + // Chip + (key, key2) => [ + `+<${key}1>${key}<${key}2>:<${key2}1>${key2}<${key2}2>`, + chip( + chipKey(`<${key}1>${key}<${key}2>`), + chipValue(`<${key2}1>${key2}<${key2}2>`), + ), + ], + ]; + + const smokeTestSpecs = getNPermutations( + [...smokeTestPrimitives, ...smokeTestPrimitives], + 1000, + ).map(primitives => { + + }) + + mappingSpecs.forEach(({ queryAndPositions, doc, name }) => { + it.only(`should map ${name}`, () => { + const { query, positions } = getCqlStrPositions(queryAndPositions); + const tokens = queryToProseMirrorTokens(query); + const mapping = + createProseMirrorTokenToDocumentMapping(tokens).invert(); + + // Sanity check that this equals the text string + expect( + docToCqlStr(doc), + `Expected the doc to match the query spec, minus any position information`, + ).toBe(query); + + const mappedPositions: Record = {}; + const mappedPositionDebugInfo: Record< + string, + { docPos: number; queryPos: number } + > = {}; + Object.entries(doc.tag).forEach(([docKey, docPos]) => { + if (positions[docKey] === undefined) { + throw new Error(`Expected a document position for key ${docKey}`); + } + + const queryPos = mapping.map(docPos); + mappedPositions[docKey] = queryPos; + mappedPositionDebugInfo[docKey] = { docPos, queryPos }; + }); + + expect( + mappedPositions, + `Mapping didn't match - see the diff. The output was: ${JSON.stringify(mappedPositionDebugInfo, null, " ")}`, + ).toEqual(positions); + }); + }); }); }); diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index f5a3122..af3b02b 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -147,12 +147,12 @@ const maybeQueryStrRanges = ( index: number, ) => { // If this field is at the start of the document, or preceded by a - // field value, the editor will add a queryStr node to conform to + // field key/value, the editor will add a queryStr node to conform to // the schema, so we add a queryStr mapping to account for the // additional node. const shouldAddQueryStrMapping = - token?.tokenType === "CHIP_VALUE" || index === 0; + token?.tokenType === "CHIP_KEY" || token?.tokenType === "CHIP_VALUE" || index === 0; const queryStrFrom = token ? token?.to + 1 : 0; return shouldAddQueryStrMapping ? getQueryStrRanges(queryStrFrom, queryStrFrom) diff --git a/lib/cql/src/lang/utils.ts b/lib/cql/src/lang/utils.ts index 6da84c1..798b48e 100644 --- a/lib/cql/src/lang/utils.ts +++ b/lib/cql/src/lang/utils.ts @@ -21,6 +21,18 @@ export const escapeQuotes = (str: string) => str.replaceAll(`"`, `\\"`); export const shouldQuoteFieldValue = (literal: string) => hasWhitespace(literal) || hasReservedChar(literal); +export function getNPermutations(arr: T[], n: number): T[][] { + const generator = getPermutations(arr); + const permutations: T[][] = []; + for (let i = 0; i < n; i++) { + const next = generator.next(); + if (!next.done) { + permutations.push(next.value); + } + } + return permutations +} + export function* getPermutations( permutation: T[], ): Generator { From 6b6c260b2ca859a1dab85d7f17f47ccd6e193984 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Mon, 3 Nov 2025 20:17:50 +0000 Subject: [PATCH 09/14] Add property-based testing to discover edge cases --- lib/cql/src/cqlInput/editor/utils.spec.ts | 176 ++++++++++++++++------ lib/cql/src/utils/test.ts | 9 ++ 2 files changed, 140 insertions(+), 45 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/utils.spec.ts b/lib/cql/src/cqlInput/editor/utils.spec.ts index e866602..b4eec8b 100644 --- a/lib/cql/src/cqlInput/editor/utils.spec.ts +++ b/lib/cql/src/cqlInput/editor/utils.spec.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "bun:test"; +import { describe, expect, it, test } from "bun:test"; import { createProseMirrorTokenToDocumentMap as createProseMirrorTokenToDocumentMapping, docToCqlStr, @@ -12,7 +12,14 @@ import { POLARITY, schema } from "./schema"; import { builders } from "prosemirror-test-builder"; import { createParser } from "../../lang/Cql"; import { Node } from "prosemirror-model"; -import { getNPermutations, getPermutations } from "../../lang/utils"; +import { + escapeQuotes, + getNPermutations, + getPermutations, + shouldQuoteFieldValue, +} from "../../lang/utils"; +import { pseudoRandom } from "../../utils/test"; +import { logNode } from "./debug"; describe("utils", () => { const { chip, chipKey, chipValue, doc, queryStr } = builders(schema); @@ -347,94 +354,170 @@ describe("utils", () => { doc: doc(queryStr("a")), }, { - name: "a chip", - queryAndPositions: "+b:c d", + name: "an empty chipKey surrounded by empty queryStrs", + queryAndPositions: "+: ", doc: doc( queryStr(""), // Empty chipKey - chip(chipKey("b"), chipValue("c")), + chip(chipKey(""), chipValue("")), // Empty queryStr - queryStr("d"), + queryStr(""), ), }, { - name: "an empty chipKey", - queryAndPositions: "+: ", + name: "an empty chipKey surrounded by contentful queryStrs ", + queryAndPositions: "a +: c", doc: doc( - queryStr(""), + queryStr("a"), // Empty chipKey - chip(chipKey(""), chipValue()), + chip(chipKey(""), chipValue("")), // Empty queryStr - queryStr(""), + queryStr("c"), ), }, - { - name: "abutting empty chipKeys", - queryAndPositions: "+: +: ", + name: "an empty chipValue", + queryAndPositions: "+b: d", doc: doc( queryStr(""), // Empty chipKey - chip(chipKey(""), chipValue()), + chip(chipKey("b"), chipValue("")), // Empty queryStr - queryStr(""), - // Empty chipKey - chip(chipKey(""), chipValue()), + queryStr("d"), ), }, { - name: "an empty chipValue", - queryAndPositions: "+b: d", + name: "a chip", + queryAndPositions: "+b:c d", doc: doc( queryStr(""), // Empty chipKey - chip(chipKey("b"), chipValue("")), + chip(chipKey("b"), chipValue("c")), // Empty queryStr queryStr("d"), ), }, + { + name: "abutting empty chipKeys", + queryAndPositions: "+: +: ", + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey(""), chipValue()), + // Empty queryStr + queryStr(""), + // Empty chipKey + chip(chipKey(""), chipValue()), + ), + }, ]; - const smokeTestPrimitives: Array< - (key: string, key2: string) => [string, Node] + const smokeTestLiterals = [ + "u", + // '"', + "unquoted_string", + // "quoted string", + // 'escaped"string', + ]; + + const getSmokeTestString = ( + generator: Generator, + ): string => + smokeTestLiterals[generator.next().value % smokeTestLiterals.length]; + + const getQuotedEscapedStr = (str: string) => + shouldQuoteFieldValue(str) ? `"${escapeQuotes(str)}"` : str; + + const smokeTestQueryStrs: Array< + (key: string, value: string) => [string, Node] > = [ // Empty queryStr () => ["", queryStr()], // queryStr w/ content - (key) => [ - `<${key}1>${key}<${key}2> `, - queryStr(`<${key}1>${key}<${key}2>`), + (key, value) => [ + `<${key}1>${getQuotedEscapedStr(value)}<${key}2> `, + queryStr(`<${key}1>${getQuotedEscapedStr(value)}<${key}2>`), ], + ]; + + const smokeTestChipKeys: Array< + ( + key: string, + value: string, + key2: string, + value2: string, + ) => [string, Node] + > = [ // Chip w/ empty chipKey (key) => [`+<${key}>: `, chip(chipKey(`<${key}>`), chipValue())], // Chip w/ empty chipValue - (key) => [ - `+chipKey:<${key}> `, - chip(chipKey(`chipKey`), chipValue(`<${key}>`)), + (key, value) => [ + `+<${key}1>${getQuotedEscapedStr(value)}<${key}2>:<${key}3> `, + chip(chipKey(`<${key}1>${value}<${key}2>`), chipValue(`<${key}3>`)), ], // Chip - (key, key2) => [ - `+<${key}1>${key}<${key}2>:<${key2}1>${key2}<${key2}2>`, + (key, value, key2, value2) => [ + `+<${key}1>${getQuotedEscapedStr(value)}<${key}2>:<${key2}1>${getQuotedEscapedStr(value2)}<${key2}2> `, chip( - chipKey(`<${key}1>${key}<${key}2>`), - chipValue(`<${key2}1>${key2}<${key2}2>`), + chipKey(`<${key}1>${value}<${key}2>`), + chipValue(`<${key2}1>${value2}<${key2}2>`), ), ], ]; - const smokeTestSpecs = getNPermutations( - [...smokeTestPrimitives, ...smokeTestPrimitives], - 1000, - ).map(primitives => { + const randomGenerator = pseudoRandom(1); + for (let specNo = 0; specNo < 1; specNo++) { + let queryAndPositions = ""; + const docNodes: Node[] = []; + const specDocLength = 1 + (randomGenerator.next().value % 5); + for (let docIndex = 0; docIndex < specDocLength; docIndex++) { + const char = 97 + docIndex * 3; + const queryIndex = + randomGenerator.next().value % smokeTestQueryStrs.length; + const chipKeyIndex = + randomGenerator.next().value % smokeTestChipKeys.length; + + const [queryStrStr, queryStrNode] = smokeTestQueryStrs[queryIndex]( + String.fromCharCode(char), + getSmokeTestString(randomGenerator), + ); + + const [chipStr, chipNode] = smokeTestChipKeys[chipKeyIndex]( + String.fromCharCode(char + 1), + getSmokeTestString(randomGenerator), + String.fromCharCode(char + 2), + getSmokeTestString(randomGenerator), + ); + + queryAndPositions += queryStrStr + chipStr; + docNodes.push(queryStrNode, chipNode); + } + + const docNode = doc(...docNodes, queryStr()); + try { + docNode.check(); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + } catch (_) { + logNode(docNode); + throw new Error( + "Property test created an invalid node — the structure is logged above", + ); + } - }) + mappingSpecs.push({ + name: `property test ${specNo} (\`${queryAndPositions}\`)`, + queryAndPositions, + doc: doc(...docNodes, queryStr()), + }); + } mappingSpecs.forEach(({ queryAndPositions, doc, name }) => { it.only(`should map ${name}`, () => { const { query, positions } = getCqlStrPositions(queryAndPositions); const tokens = queryToProseMirrorTokens(query); - const mapping = - createProseMirrorTokenToDocumentMapping(tokens).invert(); + const queryStrToDocMapping = + createProseMirrorTokenToDocumentMapping(tokens); + const docToQueryStrMapping = queryStrToDocMapping.invert(); // Sanity check that this equals the text string expect( @@ -442,7 +525,7 @@ describe("utils", () => { `Expected the doc to match the query spec, minus any position information`, ).toBe(query); - const mappedPositions: Record = {}; + const positionsMappedFromDocToQueryStr: Record = {}; const mappedPositionDebugInfo: Record< string, { docPos: number; queryPos: number } @@ -452,14 +535,17 @@ describe("utils", () => { throw new Error(`Expected a document position for key ${docKey}`); } - const queryPos = mapping.map(docPos); - mappedPositions[docKey] = queryPos; + const queryPos = docToQueryStrMapping.map(docPos); + + const remappedDocPos = queryStrToDocMapping.map(queryPos, 1); + expect(remappedDocPos, `${queryAndPositions} -> ${queryPos} -> ${docKey}${docPos}`).toBe(docPos); + positionsMappedFromDocToQueryStr[docKey] = queryPos; mappedPositionDebugInfo[docKey] = { docPos, queryPos }; }); expect( - mappedPositions, - `Mapping didn't match - see the diff. The output was: ${JSON.stringify(mappedPositionDebugInfo, null, " ")}`, + positionsMappedFromDocToQueryStr, + `Mapping didn't match - see the diff. The output for the query \`${query}\`, with positions at \`${queryAndPositions}\` was: ${JSON.stringify(mappedPositionDebugInfo, null, " ")}`, ).toEqual(positions); }); }); diff --git a/lib/cql/src/utils/test.ts b/lib/cql/src/utils/test.ts index 1b381d4..e67ec28 100644 --- a/lib/cql/src/utils/test.ts +++ b/lib/cql/src/utils/test.ts @@ -104,3 +104,12 @@ export const docToCqlStrWithSelection = (_state: EditorState) => { ); return docToCqlStr(newState.doc); }; + +export function* pseudoRandom(seed: number): Generator { + let value = seed; + + while(true) { + value = value * 16807 % 2147483647; + yield value; + } +}; From 82cc5ed2f269e8b43427a999b0e60b743a9aadb6 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Mon, 3 Nov 2025 20:19:14 +0000 Subject: [PATCH 10/14] Rewrite mappings using tests as a guide to make them simpler and more comprehensible ... but although the tests pass, they are not reversible, which stretches my mental model a bit ... I need to understand better why this wasn't a problem before --- lib/cql/src/cqlInput/editor/utils.ts | 70 ++++++++++++---------------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index af3b02b..d711210 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -82,40 +82,26 @@ export const joinQueryStrTokens = (tokens: ProseMirrorToken[]) => const getFieldKeyRange = ( from: number, to: number, - isQuoted: boolean, + literalOffsetStart: number, + literalOffsetEnd: number, isFollowedByChipValue: boolean, ): [number, number, number][] => { - const quoteOffset = isQuoted ? 1 : 0; return [ - // chip begin (-1) - // chipKey begin (-1) - [from, 0, 1], - // maybe start quote (+1 to remove from str) - [from, quoteOffset, 0], - // maybe end quote (+1 to remove from str) - // offset `:` into chipValue (+1 to node) - [to - 1, quoteOffset, 1], - ...(!isFollowedByChipValue - ? getFieldValueRanges(to - 1, to - 1, false) - : []), + [from, literalOffsetStart, 2 /* start, start */], + [to - 1, literalOffsetEnd - 1 /* -1 to account for the `:` following the key */, 0], + ...(!isFollowedByChipValue ? getFieldValueRanges(to, to, 0, 0) : []), ]; }; const getFieldValueRanges = ( from: number, to: number, - isQuoted: boolean, + literalOffsetStart: number, + literalOffsetEnd: number, ): [number, number, number][] => { - const quoteOffset = isQuoted ? 1 : 0; - return [ - // chipKey end, chipValue start (-1) - // remove offset from `:` - // maybe start quote - [from, 1 + quoteOffset, 1], - // chipValue end (-1) - // maybe end quote (+1) - [to, quoteOffset, 1], + [from, literalOffsetStart, 1 /* end / start */], + [to, literalOffsetEnd, 1 /* end */], ]; }; @@ -123,22 +109,19 @@ const getQueryStrRanges = ( from: number, to: number, ): [number, number, number][] => [ - [from, -1, 0], // queryStr begin (+1) - // If the queryStr node has content, it will begin with whitespace in the - // query, which pushes subsequent content forward one position. This cancels - // out queryStr end (-1), so only add a mapping for the node end if the - // queryStr node has content. - // - // This is a slightly obscure solution to this problem - we could also use the - // gaps between the token positions and the token literal length to account - // for this discrepancy, too. - [to + 1, -1, 0], - ...(from === to ? [[to, -1, 0] as [number, number, number]] : []), + [from, 0, 1 /* node begin */], + [ + to, + from !== to + ? 1 + : 0 /* whitespace after queryStr when queryStr has content */, + 1 /* node end */, + ], ]; const polarityRanges = (from: number): [number, number, number] => [ - from - 1, - 1, + from, + 1, // Polarity char (`-`|`+`) 0, ]; @@ -152,7 +135,9 @@ const maybeQueryStrRanges = ( // additional node. const shouldAddQueryStrMapping = - token?.tokenType === "CHIP_KEY" || token?.tokenType === "CHIP_VALUE" || index === 0; + token?.tokenType === "CHIP_KEY" || + token?.tokenType === "CHIP_VALUE" || + index === 0; const queryStrFrom = token ? token?.to + 1 : 0; return shouldAddQueryStrMapping ? getQueryStrRanges(queryStrFrom, queryStrFrom) @@ -191,9 +176,12 @@ export const createProseMirrorTokenToDocumentMap = ( const compactedTokenRanges = joinQueryStrTokens(tokens); const ranges = compactedTokenRanges.reduce<[number, number, number][]>( - (accRanges, { tokenType, from, to, literal }, index, tokens) => { + (accRanges, { tokenType, from, to, lexeme, literal }, index, tokens) => { const previousToken = tokens[index - 1] as ProseMirrorToken | undefined; const nextToken = tokens[index + 1] as ProseMirrorToken | undefined; + const literalOffsetStart = literal ? lexeme.indexOf(literal) : 0; + const literalOffsetEnd = + lexeme.length - (literal?.length ?? 0) - literalOffsetStart; switch (tokenType) { case TokenType.PLUS: @@ -209,7 +197,8 @@ export const createProseMirrorTokenToDocumentMap = ( ...getFieldKeyRange( from, to, - shouldQuoteFieldValue(literal ?? ""), + literalOffsetStart, + literalOffsetEnd, nextToken?.tokenType === TokenType.CHIP_VALUE, ), ); @@ -219,7 +208,8 @@ export const createProseMirrorTokenToDocumentMap = ( ...getFieldValueRanges( from, to, - shouldQuoteFieldValue(literal ?? ""), + literalOffsetStart, + literalOffsetEnd, ), ); } From 3b54a4635ab4aea456187a702d10cf783b98dbab Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Mon, 3 Nov 2025 21:16:45 +0000 Subject: [PATCH 11/14] Correct mappings, reversing the specs to test queryStr -> doc first, which makes the behaviour clearer I was testing an inverted mapping, making changes unintuitive D:< Having said that, I'm not quite sure why getFieldValueRanges requires a negative oldSize rather than a positive toSize for its chipValue end, which is something I arrived at via experimentation rather than a complete understanding ... dislike --- lib/cql/src/cqlInput/editor/diff.ts | 2 +- lib/cql/src/cqlInput/editor/editor.spec.ts | 13 ----- .../src/cqlInput/editor/plugins/cql.spec.ts | 4 +- lib/cql/src/cqlInput/editor/utils.spec.ts | 49 +++++++++++++------ lib/cql/src/cqlInput/editor/utils.ts | 4 +- 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/diff.ts b/lib/cql/src/cqlInput/editor/diff.ts index 9913d56..a829da5 100644 --- a/lib/cql/src/cqlInput/editor/diff.ts +++ b/lib/cql/src/cqlInput/editor/diff.ts @@ -1,7 +1,7 @@ import { Fragment } from "prosemirror-model"; /** - * These diff functions are vendored to ignore attributes and markup, as we only care about markup. + * These diff functions are vendored to ignore attributes, as we only care about markup. * Source: https://github.com/ProseMirror/prosemirror-model/blob/c8c7b62645d2a8293fa6b7f52aa2b04a97821f34/src/diff.ts */ diff --git a/lib/cql/src/cqlInput/editor/editor.spec.ts b/lib/cql/src/cqlInput/editor/editor.spec.ts index 67a816f..3d2ca61 100644 --- a/lib/cql/src/cqlInput/editor/editor.spec.ts +++ b/lib/cql/src/cqlInput/editor/editor.spec.ts @@ -127,17 +127,4 @@ describe("updateEditorViewWithQueryStr", () => { "+tag:^$x +tag:y ", ); }); - - it("should preserve the selection state insofar as possible - adding tag", () => { - const initialQuery = "tag:"; - const { editorView, updateEditorView } = - createEditorFromInitialState(initialQuery); - setQueryPosAsSelection(initialQuery.length, editorView); - - expect(docToCqlStrWithSelection(editorView.state)).toEqual("+tag:^$ "); - - updateEditorView("+tag:"); - - expect(docToCqlStrWithSelection(editorView.state)).toEqual("+tag:^$ "); - }); }); diff --git a/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts b/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts index b6d6b8c..4162c96 100644 --- a/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts +++ b/lib/cql/src/cqlInput/editor/plugins/cql.spec.ts @@ -926,7 +926,7 @@ describe("cql plugin", () => { const { editor, waitFor, getPosFromQueryPos } = createCqlEditor(queryStr); - editor.selectText(getPosFromQueryPos(queryStr.indexOf(":"))); + editor.selectText(getPosFromQueryPos(queryStr.indexOf("+"))); await editor.press(key); @@ -938,7 +938,7 @@ describe("cql plugin", () => { const { editor, waitFor, getPosFromQueryPos } = createCqlEditor(queryStr); - editor.selectText(getPosFromQueryPos(queryStr.indexOf(":"))); + editor.selectText(getPosFromQueryPos(queryStr.indexOf(":") + 1)); await editor.press(key); diff --git a/lib/cql/src/cqlInput/editor/utils.spec.ts b/lib/cql/src/cqlInput/editor/utils.spec.ts index b4eec8b..79f52c8 100644 --- a/lib/cql/src/cqlInput/editor/utils.spec.ts +++ b/lib/cql/src/cqlInput/editor/utils.spec.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, test } from "bun:test"; +import { describe, expect, it } from "bun:test"; import { createProseMirrorTokenToDocumentMap as createProseMirrorTokenToDocumentMapping, docToCqlStr, @@ -12,12 +12,7 @@ import { POLARITY, schema } from "./schema"; import { builders } from "prosemirror-test-builder"; import { createParser } from "../../lang/Cql"; import { Node } from "prosemirror-model"; -import { - escapeQuotes, - getNPermutations, - getPermutations, - shouldQuoteFieldValue, -} from "../../lang/utils"; +import { escapeQuotes, shouldQuoteFieldValue } from "../../lang/utils"; import { pseudoRandom } from "../../utils/test"; import { logNode } from "./debug"; @@ -466,7 +461,7 @@ describe("utils", () => { ]; const randomGenerator = pseudoRandom(1); - for (let specNo = 0; specNo < 1; specNo++) { + for (let specNo = 0; specNo < 100; specNo++) { let queryAndPositions = ""; const docNodes: Node[] = []; const specDocLength = 1 + (randomGenerator.next().value % 5); @@ -512,7 +507,7 @@ describe("utils", () => { } mappingSpecs.forEach(({ queryAndPositions, doc, name }) => { - it.only(`should map ${name}`, () => { + it(`should map ${name}`, () => { const { query, positions } = getCqlStrPositions(queryAndPositions); const tokens = queryToProseMirrorTokens(query); const queryStrToDocMapping = @@ -525,8 +520,36 @@ describe("utils", () => { `Expected the doc to match the query spec, minus any position information`, ).toBe(query); + const positionsMappedFromQueryStrToDoc: Record = {}; + const mappedDocPositionDebugInfo: Record< + string, + { docPos: number; queryPos: number } + > = {}; + Object.entries(positions).forEach(([queryKey, queryPos]) => { + if (positions[queryKey] === undefined) { + throw new Error( + `Expected a document position for key ${queryKey}`, + ); + } + + const docPos = queryStrToDocMapping.map(queryPos); + + const remappedDocPos = queryStrToDocMapping.invert().map(docPos); + expect( + remappedDocPos, + `${queryAndPositions} -> ${queryPos} -> ${queryKey}${docPos}`, + ).toBe(queryPos); + positionsMappedFromQueryStrToDoc[queryKey] = docPos; + mappedDocPositionDebugInfo[queryKey] = { docPos, queryPos }; + }); + + expect( + positionsMappedFromQueryStrToDoc, + `Mapping didn't match from queryStr to doc - see the diff. The output for the query \`${query}\`, with positions at \`${queryAndPositions}\` was: ${JSON.stringify(mappedDocPositionDebugInfo, null, " ")}`, + ).toEqual(doc.tag); + const positionsMappedFromDocToQueryStr: Record = {}; - const mappedPositionDebugInfo: Record< + const mappedQueryPositionDebugInfo: Record< string, { docPos: number; queryPos: number } > = {}; @@ -537,15 +560,13 @@ describe("utils", () => { const queryPos = docToQueryStrMapping.map(docPos); - const remappedDocPos = queryStrToDocMapping.map(queryPos, 1); - expect(remappedDocPos, `${queryAndPositions} -> ${queryPos} -> ${docKey}${docPos}`).toBe(docPos); positionsMappedFromDocToQueryStr[docKey] = queryPos; - mappedPositionDebugInfo[docKey] = { docPos, queryPos }; + mappedQueryPositionDebugInfo[docKey] = { docPos, queryPos }; }); expect( positionsMappedFromDocToQueryStr, - `Mapping didn't match - see the diff. The output for the query \`${query}\`, with positions at \`${queryAndPositions}\` was: ${JSON.stringify(mappedPositionDebugInfo, null, " ")}`, + `Mapping didn't match from doc to queryStr - see the diff. The output for the query \`${query}\`, with positions at \`${queryAndPositions}\` was: ${JSON.stringify(mappedQueryPositionDebugInfo, null, " ")}`, ).toEqual(positions); }); }); diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index d711210..f495eba 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -101,7 +101,7 @@ const getFieldValueRanges = ( ): [number, number, number][] => { return [ [from, literalOffsetStart, 1 /* end / start */], - [to, literalOffsetEnd, 1 /* end */], + [to + 1, literalOffsetEnd - 1, 0 /* end */], ]; }; @@ -120,7 +120,7 @@ const getQueryStrRanges = ( ]; const polarityRanges = (from: number): [number, number, number] => [ - from, + from - 1, 1, // Polarity char (`-`|`+`) 0, ]; From f48eb321e392324b3785347c40f61ffe0e17e66b Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Tue, 4 Nov 2025 22:18:49 +0000 Subject: [PATCH 12/14] Add mappings for quoted and escaped chipKeys and chipValues cleaning up the ranges to remove the odd inversion, and expanding the property-based tests to verify they are correct --- lib/cql/src/cqlInput/editor/utils.spec.ts | 36 ++++++++++++++++------- lib/cql/src/cqlInput/editor/utils.ts | 26 +++++++++++----- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/utils.spec.ts b/lib/cql/src/cqlInput/editor/utils.spec.ts index 79f52c8..b934605 100644 --- a/lib/cql/src/cqlInput/editor/utils.spec.ts +++ b/lib/cql/src/cqlInput/editor/utils.spec.ts @@ -405,14 +405,36 @@ describe("utils", () => { chip(chipKey(""), chipValue()), ), }, + { + name: "a chip with quoted key", + queryAndPositions: '+"\\"":c d', + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey('"'), chipValue("c")), + // Empty queryStr + queryStr("d"), + ), + }, + { + name: "a chip with quoted value", + queryAndPositions: '+"\\"":"\\"" d', + doc: doc( + queryStr(""), + // Empty chipKey + chip(chipKey('"'), chipValue("\"")), + // Empty queryStr + queryStr("d"), + ), + }, ]; const smokeTestLiterals = [ "u", - // '"', + '"', "unquoted_string", - // "quoted string", - // 'escaped"string', + "quoted string", + 'escaped"string', ]; const getSmokeTestString = ( @@ -533,12 +555,6 @@ describe("utils", () => { } const docPos = queryStrToDocMapping.map(queryPos); - - const remappedDocPos = queryStrToDocMapping.invert().map(docPos); - expect( - remappedDocPos, - `${queryAndPositions} -> ${queryPos} -> ${queryKey}${docPos}`, - ).toBe(queryPos); positionsMappedFromQueryStrToDoc[queryKey] = docPos; mappedDocPositionDebugInfo[queryKey] = { docPos, queryPos }; }); @@ -566,7 +582,7 @@ describe("utils", () => { expect( positionsMappedFromDocToQueryStr, - `Mapping didn't match from doc to queryStr - see the diff. The output for the query \`${query}\`, with positions at \`${queryAndPositions}\` was: ${JSON.stringify(mappedQueryPositionDebugInfo, null, " ")}`, + `Mapping didn't match from doc to queryStr (inverted mapping) - see the diff. The output for the query \`${query}\`, with positions at \`${queryAndPositions}\` was: ${JSON.stringify(mappedQueryPositionDebugInfo, null, " ")}`, ).toEqual(positions); }); }); diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index f495eba..11305f5 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -87,8 +87,13 @@ const getFieldKeyRange = ( isFollowedByChipValue: boolean, ): [number, number, number][] => { return [ - [from, literalOffsetStart, 2 /* start, start */], - [to - 1, literalOffsetEnd - 1 /* -1 to account for the `:` following the key */, 0], + [from, 0, 2 /* start, start */], + [from, literalOffsetStart, 0], + [ + to - literalOffsetEnd, + literalOffsetEnd - 1, + 0, + ], ...(!isFollowedByChipValue ? getFieldValueRanges(to, to, 0, 0) : []), ]; }; @@ -99,9 +104,12 @@ const getFieldValueRanges = ( literalOffsetStart: number, literalOffsetEnd: number, ): [number, number, number][] => { + return [ - [from, literalOffsetStart, 1 /* end / start */], - [to + 1, literalOffsetEnd - 1, 0 /* end */], + [from, 0, 1 /* end / start */], + [from, literalOffsetStart, 0], + [to - literalOffsetEnd, literalOffsetEnd, 0], + [to, 0, 1 /* end */], ]; }; @@ -179,17 +187,19 @@ export const createProseMirrorTokenToDocumentMap = ( (accRanges, { tokenType, from, to, lexeme, literal }, index, tokens) => { const previousToken = tokens[index - 1] as ProseMirrorToken | undefined; const nextToken = tokens[index + 1] as ProseMirrorToken | undefined; - const literalOffsetStart = literal ? lexeme.indexOf(literal) : 0; + + const unescapedLiteral = unescapeQuotes(literal ?? ""); + const literalOffsetStart = unescapedLiteral ? lexeme.indexOf(unescapedLiteral) : 0; const literalOffsetEnd = - lexeme.length - (literal?.length ?? 0) - literalOffsetStart; + lexeme.length - (unescapedLiteral?.length ?? 0) - literalOffsetStart; switch (tokenType) { case TokenType.PLUS: case TokenType.MINUS: { - return accRanges.concat([ + return accRanges.concat( ...maybeQueryStrRanges(previousToken, index), polarityRanges(from), - ]); + ); } case TokenType.CHIP_KEY: { return accRanges.concat( From 10e1d5f099aa7f688f1aaf99af361a3fef04df61 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Tue, 4 Nov 2025 22:34:46 +0000 Subject: [PATCH 13/14] Tidy up a few things I spotted during the refactor --- lib/cql/src/cqlInput/editor/utils.ts | 10 ++++------ lib/cql/src/lang/typeahead.ts | 6 +++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/cql/src/cqlInput/editor/utils.ts b/lib/cql/src/cqlInput/editor/utils.ts index 11305f5..72c3f6f 100644 --- a/lib/cql/src/cqlInput/editor/utils.ts +++ b/lib/cql/src/cqlInput/editor/utils.ts @@ -89,11 +89,7 @@ const getFieldKeyRange = ( return [ [from, 0, 2 /* start, start */], [from, literalOffsetStart, 0], - [ - to - literalOffsetEnd, - literalOffsetEnd - 1, - 0, - ], + [to - literalOffsetEnd, literalOffsetEnd - 1, 0], ...(!isFollowedByChipValue ? getFieldValueRanges(to, to, 0, 0) : []), ]; }; @@ -189,7 +185,9 @@ export const createProseMirrorTokenToDocumentMap = ( const nextToken = tokens[index + 1] as ProseMirrorToken | undefined; const unescapedLiteral = unescapeQuotes(literal ?? ""); - const literalOffsetStart = unescapedLiteral ? lexeme.indexOf(unescapedLiteral) : 0; + const literalOffsetStart = unescapedLiteral + ? lexeme.indexOf(unescapedLiteral) + : 0; const literalOffsetEnd = lexeme.length - (unescapedLiteral?.length ?? 0) - literalOffsetStart; diff --git a/lib/cql/src/lang/typeahead.ts b/lib/cql/src/lang/typeahead.ts index 0067835..99d4270 100644 --- a/lib/cql/src/lang/typeahead.ts +++ b/lib/cql/src/lang/typeahead.ts @@ -112,7 +112,7 @@ export class Typeahead { }); } - private getSuggestionsForKeyToken( + private getSuggestionsForChipKey( keyToken: ProseMirrorToken, ): TypeaheadSuggestion | undefined { const suggestions = this.suggestFieldKey(keyToken.literal ?? ""); @@ -137,7 +137,7 @@ export class Typeahead { signal?: AbortSignal, ): Promise { if (!value) { - return this.getSuggestionsForKeyToken(key); + return this.getSuggestionsForChipKey(key); } const maybeValueSuggestions = this.suggestFieldValue( @@ -153,7 +153,7 @@ export class Typeahead { const suggestions = await maybeValueSuggestions.suggestions; return { - from: value ? value.from : key.from, // Extend backwards into chipKey's ':' + from: value ? value.from : key.from, to: value ? value.to : key.to, position: "chipValue", suggestions, From 1dc843549cc5dda19f0773a40e8f9fd5dac71441 Mon Sep 17 00:00:00 2001 From: Jonathon Herbert Date: Sat, 15 Nov 2025 14:26:54 +0000 Subject: [PATCH 14/14] Reinstate CAPI typeahead provider for page --- lib/cql/src/page.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/cql/src/page.ts b/lib/cql/src/page.ts index 2bfd50f..668b7d4 100644 --- a/lib/cql/src/page.ts +++ b/lib/cql/src/page.ts @@ -8,9 +8,9 @@ import { } from "./cqlInput/editor/debug.ts"; import { createParser } from "./lang/Cql.ts"; import { Typeahead, TypeaheadField } from "./lang/typeahead.ts"; +import { CapiTypeaheadProvider } from "./typeahead/CapiTypeaheadHelpers.ts"; import { toolsSuggestionOptionResolvers } from "./typeahead/tools-index/config"; import { DebugChangeEventDetail, QueryChangeEventDetail } from "./types/dom"; -import { TestTypeaheadHelpers } from "./lang/fixtures/TestTypeaheadHelpers.ts"; const setUrlParam = (key: string, value: string) => { const urlParams = new URLSearchParams(window.location.search); @@ -131,7 +131,10 @@ const params = new URLSearchParams(window.location.search); const endpoint = params.get("endpoint"); const initialEndpointCapi = endpoint || "https://content.guardianapis.com"; -const typeaheadHelpersCapi = new TestTypeaheadHelpers(); +const typeaheadHelpersCapi = new CapiTypeaheadProvider( + initialEndpointCapi, + "test", +); const capiTypeahead = new Typeahead(typeaheadHelpersCapi.typeaheadFields); const CqlInputCapi = createCqlInput(capiTypeahead, {