Skip to content

Move typeahead per-position suggestion logic from ProseMirror-based TypeaheadPopover to CQL-based Typeahead #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jonathonherbert wants to merge 16 commits into
base: jsh/correct-mappings-like-really-correct
Choose a base branch
from
Open

Move typeahead per-position suggestion logic from ProseMirror-based TypeaheadPopover to CQL-based Typeahead #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8bb69f6
Get suggestions by position, rather than returning every possible sug…
jonathonherbert Oct 16, 2025
0020279
Ensure one suggestion is returned from the typeahead helper at once, …
jonathonherbert Oct 16, 2025
e349d68
Fix issue where keys would be suggested when values were present
jonathonherbert Oct 16, 2025
9f4dab6
Improve test failure messaging for assertCqlStrPosFromDocPos
jonathonherbert Oct 20, 2025
a96762c
Use ProseMirror positions for typeahead suggestions (which require a …
jonathonherbert Oct 24, 2025
8890990
Do not attempt to show the popover if no popover exists
jonathonherbert Oct 25, 2025
79acd5d
Fix remaining tests
jonathonherbert Oct 30, 2025
8aec806
Add position-based mapping tests, and begin work on smoke test
jonathonherbert Nov 2, 2025
6b6c260
Add property-based testing to discover edge cases
jonathonherbert Nov 3, 2025
82cc5ed
Rewrite mappings using tests as a guide
jonathonherbert Nov 3, 2025
3b54a46
Correct mappings, reversing the specs to test queryStr -> doc first, …
jonathonherbert Nov 3, 2025
f48eb32
Add mappings for quoted and escaped chipKeys and chipValues
jonathonherbert Nov 4, 2025
10e1d5f
Tidy up a few things I spotted during the refactor
jonathonherbert Nov 4, 2025
1dc8435
Reinstate CAPI typeahead provider for page
jonathonherbert Nov 15, 2025
660b169
Merge branch 'jsh/correct-mappings-like-really-correct' into jsh/pos-…
jonathonherbert Nov 22, 2025
70008ed
Merge branch 'jsh/correct-mappings-like-really-correct' into jsh/pos-…
jonathonherbert Nov 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions lib/cql/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
44 changes: 31 additions & 13 deletions lib/cql/src/cqlInput/editor/debug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -32,19 +33,26 @@ 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 = `
<div class="CqlDebug__queryDiagram CqlDebug__queryDiagramToken">
<div class="CqlDebug__queryDiagramLabel">
<div>Lexeme</div>
<div>Literal</div>
</div>
<div class="CqlDebug__queryDiagramContent">`;

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) => {
Expand All @@ -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 `
<div class="CqlDebug__queryBox">
<div class="CqlDebug__queryIndex">${globalIndex}</div>
Expand All @@ -64,7 +72,9 @@ export const getDebugTokenHTML = (tokens: Token[], selection: Selection, mapping
: ""
}
${
mappedTo === globalIndex ? `<div class="CqlDebug__selection">$</div>` : ""
mappedTo === globalIndex
? `<div class="CqlDebug__selection">$</div>`
: ""
}
${
lexemeChar !== undefined
Expand All @@ -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"
? `<div class="CqlDebug__queryBox"><div class="CqlDebug__queryIndex">${
token.end + 1
}</div></div>`
token.to
}</div>${
mappedFrom === token.to
? `<div class="CqlDebug__selection">^</div>`
: ""
}${
mappedTo === token.to
? `<div class="CqlDebug__selection">$</div>`
: ""
}</div>`
: ""
}`;
});
Expand Down Expand Up @@ -173,10 +189,12 @@ export const getDebugMappingHTML = (
`
<div class="CqlDebug__queryBox CqlDebug__queryBox--offset" data-pos="${pos}">
<div class="CqlDebug__queryIndex">${pos}</div>
${(queryPosMap[pos] ?? []).map(
({ char }) =>
`<div class="CqlDebug__originalChar">${char}</div>`,
).join(" ")}
${(queryPosMap[pos] ?? [])
.map(
({ char }) =>
`<div class="CqlDebug__originalChar">${char}</div>`,
)
.join(" ")}


${
Expand Down
6 changes: 3 additions & 3 deletions lib/cql/src/cqlInput/editor/plugins/cql.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand Down
17 changes: 12 additions & 5 deletions lib/cql/src/cqlInput/editor/plugins/cql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Collaborator Author

@jonathonherbert jonathonherbert Nov 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing twice here eliminates an issue that came up in the tests where the cql query derived by a document would not be exactly the same as the query that produced it. For example, the following could occur:

k:v -> <chip><chipKey>k</chipKey><chipValue>v</chipValue></chip> -> +k:v

Re-parsing the string derived from the new document, which may differ from the original, ensures that we get the correct tokens and mapping.


const docSelection = new AllSelection(tr.doc);

Expand Down Expand Up @@ -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);
Expand Down
34 changes: 19 additions & 15 deletions lib/cql/src/cqlInput/editor/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,25 +572,29 @@ export type ProseMirrorToken = Omit<Token, "start" | "end"> & {
* 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 = (
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,
Expand Down
33 changes: 15 additions & 18 deletions lib/cql/src/cqlInput/popover/TypeaheadPopover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Copy link
Collaborator Author

@jonathonherbert jonathonherbert Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic to find suggestions here has been moved to the Typeahead class

this.show();
};

Expand All @@ -141,12 +130,20 @@ 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);

// 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}`,
);
};

public handleAction: ActionHandler = (action) => {
Expand Down
Loading