From ae9c54e133e1cb2bfa6487fd3130f0a395d71534 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Mon, 3 Aug 2026 13:58:08 -0700 Subject: [PATCH 1/6] feat(action-grammar): treat ?/*/+ as special quantifier chars - Postfix ?/*/+ are quantifiers only after ")" or ">" - Bare ?/*/+ are parse errors; literals require \? / \* / \+ - ?/*/+ parse as optional/repeat (equiv. to grouped form) - Writer/prettier prefers ()? over bare ? - Colon stays unescaped (no ambiguity outside $()) - Quotes remain literal match chars (not string syntax) - Corpus + sample.agr + fuzz/generator escapes updated - Tests cover CurtisM proposal cases and match semantics --- ts/extensions/agr-language/sample.agr | 9 +- ts/packages/actionGrammar/README.md | 4 + .../src/builtInGrammarCategories.ts | 3 +- .../src/fuzz/grammarGenerator.ts | 12 +- .../src/generation/scenarioBasedGenerator.ts | 4 +- .../generation/schemaToGrammarGenerator.ts | 14 +- .../actionGrammar/src/grammarCompiler.ts | 51 +- .../actionGrammar/src/grammarRuleParser.ts | 71 ++- .../actionGrammar/src/grammarRuleWriter.ts | 21 +- .../test/grammarOptimizerDispatch.spec.ts | 8 +- .../test/quantifierSpecialChars.spec.ts | 461 ++++++++++++++++++ .../test/calendar-extended.agr | 6 +- .../agentSdkWrapper/test/calendar-new.agr | 6 +- .../agents/code/src/vscode/debugSchema.agr | 22 +- .../agents/code/src/vscode/displaySchema.agr | 30 +- .../agents/code/src/vscode/editorSchema.agr | 2 +- .../agents/code/src/vscode/generalSchema.agr | 8 +- .../src/osNotificationsSchema.agr | 2 +- ts/packages/agents/photo/src/photoSchema.agr | 2 +- 19 files changed, 656 insertions(+), 80 deletions(-) create mode 100644 ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts diff --git a/ts/extensions/agr-language/sample.agr b/ts/extensions/agr-language/sample.agr index b559b5d640..512c44e172 100644 --- a/ts/extensions/agr-language/sample.agr +++ b/ts/extensions/agr-language/sample.agr @@ -48,7 +48,7 @@ import { Ordinal, CalendarDate }; artists: [artist] } } - | pause (the)? music? -> { actionName: "pause" }; + | pause (the)? (music)? -> { actionName: "pause" }; // ─── Exported rule ──────────────────────────────────────────────────────────── @@ -160,15 +160,18 @@ export = hello | goodbye; // ─── Operators and grouping ────────────────────────────────────────────────── = - one? two* three+ four // optional, zero-or-more, one-or-more + (one)? (two)* (three)+ four // optional, zero-or-more, one-or-more | (first | second | third) // alternation with grouping | (item)+ // one-or-more group | (prefix)* suffix; // zero-or-more group + // Literal "?" must be escaped (bare ?/*/+ are quantifiers after ) or > only): + // what is the time\? + // who sings song \? // ─── Optional capture quantifiers ──────────────────────────────────────────── = - add $(item:word)+ to $(list:string) -> { + add ($(item:word))+ to $(list:string) -> { actionName: "addItems", parameters: { items: [item], listName: list } }; diff --git a/ts/packages/actionGrammar/README.md b/ts/packages/actionGrammar/README.md index a576a8478f..9f53a6dd45 100644 --- a/ts/packages/actionGrammar/README.md +++ b/ts/packages/actionGrammar/README.md @@ -42,6 +42,10 @@ import { Helper } from "./other.agr"; // Grammar imports = (skip | next) (track | song)? // Optionals, alternation -> { actionName: "skip" }; = $(item:string) (, $(item:string))*; // Repetition (Kleene star) +// Quantifiers ? * + are special: valid only after ")" or ">" (e.g. ()?, ?). +// Bare ? elsewhere is a parse error — escape literals as \? +// what is the time\? // literal trailing ? +// who sings song \? // required Song + literal ? ``` ## Exports diff --git a/ts/packages/actionGrammar/src/builtInGrammarCategories.ts b/ts/packages/actionGrammar/src/builtInGrammarCategories.ts index 1fc6fa629e..e4aa6a7d54 100644 --- a/ts/packages/actionGrammar/src/builtInGrammarCategories.ts +++ b/ts/packages/actionGrammar/src/builtInGrammarCategories.ts @@ -10,7 +10,8 @@ * the stored grammar is fully self-contained. * * Naming convention for prompt use: - * Usage in patterns: ()? (note: ()? not ? — bare optional not yet supported) + * Usage in patterns: ()? or bare ? + * (writer/prettier always emits the grouped form) */ export interface BuiltInGrammarCategory { /** AGR rule name — used as in patterns */ diff --git a/ts/packages/actionGrammar/src/fuzz/grammarGenerator.ts b/ts/packages/actionGrammar/src/fuzz/grammarGenerator.ts index fe3279b1f4..b98fe40014 100644 --- a/ts/packages/actionGrammar/src/fuzz/grammarGenerator.ts +++ b/ts/packages/actionGrammar/src/fuzz/grammarGenerator.ts @@ -861,17 +861,17 @@ function maybeEscapeWord( * space character as part of the token, not a separator. * * Grammar-special chars (`|`, `(`, `)`, `<`, `>`, `$`, `-`, `;`, - * `{`, `}`, `[`, `]`, `\`) and comment starters (`/`) are - * excluded - they require backslash escapes that the parser handles - * via the broader `escapeProb` knob, not via embedded-separator - * semantics. + * `{`, `}`, `[`, `]`, `?`, `*`, `+`, `\`) and comment starters (`/`) are + * excluded from bare embedding — `?`/`*`/`+` require escapes in source + * (they are postfix quantifiers). Entries below that are special use the + * escaped source form. */ const SEPARATOR_LITERAL_CHARS: ReadonlyArray = [ [",", ","], [".", "."], [":", ":"], ["!", "!"], - ["?", "?"], + ["\\?", "?"], ["=", "="], ["@", "@"], ["#", "#"], @@ -879,7 +879,7 @@ const SEPARATOR_LITERAL_CHARS: ReadonlyArray = [ ["&", "&"], ["'", "'"], ['"', '"'], - ["+", "+"], + ["\\+", "+"], // Escaped space: source `\ ` decodes to a single space char that // is treated as part of the literal (not a flex-space). ["\\ ", " "], diff --git a/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts b/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts index e66f56154d..e23525e1a9 100644 --- a/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts +++ b/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts @@ -856,13 +856,13 @@ export class ScenarioBasedGrammarGenerator { /** * Escape special characters in quoted string literals - * Special chars: \, @, |, (, ), <, >, $, -, {, }, [, ], ' + * Special chars: \, @, |, (, ), <, >, $, -, {, }, [, ], ?, *, +, ' * Backslashes must be escaped first to avoid double-escaping */ private escapeSpecialChars(text: string): string { return text .replace(/\\/g, "\\\\") - .replace(/[@|()\[\]<>$\-{}']/g, "\\$&"); + .replace(/[@|()\[\]<>$\-{}?*+']/g, "\\$&"); } /** diff --git a/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts b/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts index 263896017f..6e845dc24c 100644 --- a/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts +++ b/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts @@ -115,7 +115,15 @@ CRITICAL SYNTAX RULES: CORRECT: // This is a comment WRONG: # This is a comment -10. Hyphenated and apostrophe string literals: +10. Quantifiers ? * + are SPECIAL characters in patterns: + - Valid ONLY immediately after ")" or ">" : ()?, ?, $(x)?, (a|b)* + - Bare "?" after a word is a PARSE ERROR. Escape literals: what is the time\? + - Required name + question mark: who sings song \? + - Optional name + question mark: who sings song ()?\? + - Do NOT write ? intending a literal "?"; that makes Polite optional. + - The writer/prettier prefers the grouped form ()? over bare ?. + +11. Hyphenated and apostrophe string literals: a) Apostrophes/contractions: "don't" "it's" "let's" (NOT 'don\'t' or 'it\'s' — use double quotes) b) Hyphenated words like 'auto-reload' or "auto-generate" CANNOT appear in any quoted string (hyphens are special characters even inside double-quoted strings). @@ -124,13 +132,13 @@ CRITICAL SYNTAX RULES: CORRECT: ('auto' 'generate' | 'autogenerate')? WRONG: 'auto-generate' or "auto-generate" (both cause parse errors!) -11. Action body values must be SIMPLE variable names only — no dot notation, array access, or expressions: +12. Action body values must be SIMPLE variable names only — no dot notation, array access, or expressions: CORRECT: -> { actionName: "create", parameters: { name: name, language: language } } WRONG: -> { actionName: "create", parameters: { declaration: details.declaration, body: details.body } } If a TypeScript schema parameter has nested fields, just capture it as a single string wildcard. Grammar rules capture flat key/value pairs; don't model nested object structures. -12. When using a CUSTOM SUB-RULE (not a built-in entity type) as a wildcard type, wrap the rule name in angle brackets: +13. When using a CUSTOM SUB-RULE (not a built-in entity type) as a wildcard type, wrap the rule name in angle brackets: CORRECT: $(location:) — rule reference in wildcard (angle brackets required) CORRECT: $(days:)? — optional rule-typed capture WRONG: $(location:LocationSpec) — rule name without angle brackets (will cause "Undefined type" error) diff --git a/ts/packages/actionGrammar/src/grammarCompiler.ts b/ts/packages/actionGrammar/src/grammarCompiler.ts index 7ede554fd9..960a4e332b 100644 --- a/ts/packages/actionGrammar/src/grammarCompiler.ts +++ b/ts/packages/actionGrammar/src/grammarCompiler.ts @@ -1318,6 +1318,7 @@ function createGrammarRule( // match time — no rule definition needed, no NFA state expansion. // BUT: only use the phrase-set if the rule is NOT defined locally // or via import (preserves grammars that define their own etc.) + const { optional, repeat } = expr; const isLocallyDefined = context.ruleDefMap.has(expr.refName.name) || context.importedRuleMap.has(expr.refName.name); @@ -1325,22 +1326,38 @@ function createGrammarRule( !isLocallyDefined && globalPhraseSetRegistry.isPhraseSetName(expr.refName.name) ) { - parts.push( - createPhraseSetPart( - expr.refName.name, - undefined, - allocPartId( - context, - expr.pos, - `<${expr.refName.name}>`, - ), + const phrasePart = createPhraseSetPart( + expr.refName.name, + undefined, + allocPartId( + context, + expr.pos, + `<${expr.refName.name}>`, ), ); + // PhraseSetPart cannot carry optional/repeat; wrap so bare + // ? / * / + match the grouped form. + if (optional || repeat) { + const q = repeat ? (optional ? "*" : "+") : "?"; + parts.push( + createRulesPart([{ parts: [phrasePart] }], { + optional, + repeat, + partId: allocPartId( + context, + expr.pos, + `<${expr.refName.name}>${q}`, + ), + }), + ); + } else { + parts.push(phrasePart); + } // Phrase sets don't produce a captured value on their own. // Use defaultValue=true so single-part rules using a phrase set // don't trip the "Start rule does not produce a value" check. defaultValue = true; - consumedInput(); // phrase sets always consume input + if (!optional) consumedInput(); // required / + still consume break; } const record = createNamedGrammarRules( @@ -1355,6 +1372,8 @@ function createGrammarRule( parts.push( createRulesPart(record.grammarRules, { name: expr.refName.name, + optional, + repeat, partId: allocPartId( context, expr.pos, @@ -1362,14 +1381,16 @@ function createGrammarRule( ), }), ); - // RuleRefExpr has no optional modifier; it is always non-optional. + // Optional / * rule refs can be skipped — do not force non-null. // === false: only clear when *definitely* non-nullable (same // asymmetry as the variable ruleRef case above). - if (record.nullable === false) { - currentEpr = new Set(); + if (!optional) { + if (record.nullable === false) { + currentEpr = new Set(); + } + // ?? false: treat undefined (back-ref) as non-nullable. + ruleNullable = ruleNullable && (record.nullable ?? false); } - // ?? false: treat undefined (back-ref) as non-nullable. - ruleNullable = ruleNullable && (record.nullable ?? false); break; } case "rules": { diff --git a/ts/packages/actionGrammar/src/grammarRuleParser.ts b/ts/packages/actionGrammar/src/grammarRuleParser.ts index 247281946e..cc56e2a866 100644 --- a/ts/packages/actionGrammar/src/grammarRuleParser.ts +++ b/ts/packages/actionGrammar/src/grammarRuleParser.ts @@ -61,10 +61,17 @@ const debugParse = registerDebug("typeagent:grammar:parse"); * // Per-alternate annotations override the definition-level setting. * // Omitting the annotation is equivalent to [spacing=auto]. * // - * // is any character except special chars (| ( ) < > $ - ; { } [ ]) and backslash, - * // and not the start of a comment sequence ("//" or "/*"). Escape a special character - * // or comment opener to make it literal (e.g. \|, \//, or \/*). + * // is any character except special chars (| ( ) < > $ - ; { } [ ] ? * +) and + * // backslash, and not the start of a comment sequence ("//" or "/*"). Escape a special + * // character or comment opener to make it literal (e.g. \|, \?, \//, or \/*). * // An escaped space (e.g. "\ ") is treated as a literal character, not a flex space. + * // + * // Quantifiers ? * + are special characters. They are postfix operators only when + * // immediately after ")" or ">" (group, capture, or rule-ref). Anywhere else in a + * // pattern expression they are a parse error — use \? / \* / \+ for literals. + * // Colon ":" is intentionally NOT special: it only appears inside $() type specs, so + * // there is no ambiguity with pattern text (CurtisM). Quotes are also not string + * // syntax in patterns — " and ' are ordinary literal characters. * ::= ( | | )+ * ::= "\\" * ::= "0" // null character \0 @@ -87,7 +94,9 @@ const debugParse = registerDebug("typeagent:grammar:parse"); * * ::= (":" ( | ))? * - * ::= + * ::= ( "?" | "*" | "+" )? + * // Bare ? / * / + are equivalent to ()? / ()* / ()+. + * // The writer/prettier always prefers the grouped form for clarity. * ::= "(" ( ")" | ")?" | ")*" | ")+" ) * * // ── Value (basic mode: enableValueExpressions=false) ────────────────────────── @@ -232,6 +241,10 @@ export type CommentedName = { export type RuleRefExpr = { type: "ruleReference"; refName: CommentedName; + /** True when postfix `?` or `*` follows the rule name (`?` / `*`). */ + optional?: boolean | undefined; + /** True when postfix `*` or `+` follows the rule name (`*` / `+`). */ + repeat?: boolean | undefined; pos?: number | undefined; leadingComments?: Comment[] | undefined; }; @@ -403,7 +416,7 @@ export function isIdContinue(char: string) { } // Even some of these are not used yet, include them for future use. export const expressionsSpecialChar = [ - // Must escape + // Must escape in pattern expressions "|", "(", ")", @@ -417,6 +430,13 @@ export const expressionsSpecialChar = [ "}", "[", "]", + // Postfix quantifiers: only valid immediately after ")" or ">". + // Elsewhere they are a parse error; escape as \? / \* / \+ for literals. + // Colon ":" is intentionally omitted — it only appears inside $() type + // positions, so there is no ambiguity with ordinary pattern text. + "?", + "*", + "+", ]; export function isExpressionSpecialChar(char: string) { @@ -689,6 +709,29 @@ class GrammarRuleParser implements ValueExprParserContext { } } + + /** + * Apply a postfix quantifier immediately after a rule reference (`>`). + * Sets optional/repeat on `target` and advances past the quantifier char. + * No-op when the next char is not one of: ? * + + */ + private applyPostfixQuantifier(target: { + optional?: boolean | undefined; + repeat?: boolean | undefined; + }): void { + if (this.isAt("?")) { + target.optional = true; + this.skipWhitespace(1); + } else if (this.isAt("*")) { + target.optional = true; + target.repeat = true; + this.skipWhitespace(1); + } else if (this.isAt("+")) { + target.repeat = true; + this.skipWhitespace(1); + } + } + // INVARIANT EXCEPTION: does not skip trailing whitespace — whitespace is // semantically significant here (flex-space boundaries). The caller // (parseExpression) manages whitespace skipping in its loop. @@ -803,6 +846,9 @@ class GrammarRuleParser implements ValueExprParserContext { refName: this.parseRuleName(), pos, }; + // Postfix quantifiers after ">": ?, *, + + // (equivalent to ()?, ()*, ()+). + this.applyPostfixQuantifier(node); attach(node); expNodes.push(node); continue; @@ -812,6 +858,9 @@ class GrammarRuleParser implements ValueExprParserContext { const v = this.parseVariableSpecifier(); attach(v); expNodes.push(v); + // Captures only support optional today ($(x)?). Use ($(x))* / ($(x))+ + // for repetition (group form). Bare )* / )+ after $() is rejected below + // if someone writes $(x)* without grouping — variables lack repeat. if (this.isAt(")?")) { v.optional = true; this.skipWhitespace(2); @@ -843,6 +892,18 @@ class GrammarRuleParser implements ValueExprParserContext { continue; } + // Bare quantifier not attached to ")" or ">" — always a parse error. + // CurtisM: this must be a hard error, not a lint warning. + if (this.isAt("?") || this.isAt("*") || this.isAt("+")) { + const q = this.content[this.curr]; + this.throwError( + `Unexpected quantifier '${q}'. ` + + `Postfix quantifiers (?, *, +) are only valid immediately after ')' or '>' ` + + `(e.g. ()?, ?, $(x)?). ` + + `Escape as \\${q} for a literal '${q}' in the pattern.`, + ); + } + const s = this.parseStrExpr(); if (s === undefined) { // end of expression diff --git a/ts/packages/actionGrammar/src/grammarRuleWriter.ts b/ts/packages/actionGrammar/src/grammarRuleWriter.ts index 52719e091e..0b8ce48141 100644 --- a/ts/packages/actionGrammar/src/grammarRuleWriter.ts +++ b/ts/packages/actionGrammar/src/grammarRuleWriter.ts @@ -923,9 +923,26 @@ function writeSingleExpr( } break; } - case "ruleReference": - writeBracketedName(result, expr.refName); + case "ruleReference": { + // CurtisM: prettier always prefers the grouped form so authors see + // the quantifier attachment explicitly. ? and ()? are + // equivalent at parse time. + const quant = expr.repeat + ? expr.optional + ? "*" + : "+" + : expr.optional + ? "?" + : undefined; + if (quant) { + result.write("("); + writeBracketedName(result, expr.refName); + result.write(`)${quant}`); + } else { + writeBracketedName(result, expr.refName); + } break; + } case "rules": { result.write("("); // brokenCol = -1: in broken mode, | aligns with ( which is one column diff --git a/ts/packages/actionGrammar/test/grammarOptimizerDispatch.spec.ts b/ts/packages/actionGrammar/test/grammarOptimizerDispatch.spec.ts index 96ab1e92c2..da1bfa9b72 100644 --- a/ts/packages/actionGrammar/test/grammarOptimizerDispatch.spec.ts +++ b/ts/packages/actionGrammar/test/grammarOptimizerDispatch.spec.ts @@ -1078,7 +1078,7 @@ describe("Grammar Optimizer - non-canonical DispatchPart shapes", () => { // or `@`, dropping the match entirely. describe("regression: separator char inside required-mode literal", () => { it("matches a required-mode alternation whose first token embeds '?'", () => { - const text = ` [spacing=required] = d? b@ a% -> "first" + const text = ` [spacing=required] = d\\? b@ a% -> "first" | b d d d@ -> "second";`; const baseline = loadGrammarRules("t.grammar", text); const optimized = loadGrammarRules("t.grammar", text, { @@ -1113,12 +1113,12 @@ describe("Grammar Optimizer - non-canonical DispatchPart shapes", () => { | x. a -> "dot" | x: a -> "colon" | x! a -> "bang" - | x? a -> "qmark" + | x\\? a -> "qmark" | x@ a -> "at" | x# a -> "hash" | x% a -> "pct" | x& a -> "amp" - | x+ a -> "plus" + | x\\+ a -> "plus" | x= a -> "eq" | x' a -> "apos" | x" a -> "quot";`; @@ -1156,7 +1156,7 @@ describe("Grammar Optimizer - non-canonical DispatchPart shapes", () => { // dispatch can't bucket this rule (peek would never // return a key matching `?x`), so it must land in the // fallback subset and still match correctly. - const text = ` [spacing=required] = ?x -> "lead-sep" + const text = ` [spacing=required] = \\?x -> "lead-sep" | yy -> "normal";`; const baseline = loadGrammarRules("t.grammar", text); const optimized = loadGrammarRules("t.grammar", text, { diff --git a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts new file mode 100644 index 0000000000..40a5ce8b8b --- /dev/null +++ b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts @@ -0,0 +1,461 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { parseGrammarRules } from "../src/grammarRuleParser.js"; +import { writeGrammarRules } from "../src/grammarRuleWriter.js"; +import { loadGrammarRules } from "../src/grammarLoader.js"; +import { describeForEachMatcher } from "./testUtils.js"; + +/** + * Proposal (CurtisM): postfix ? / * / + are quantifiers only after ")" or ">". + * Bare quantifiers are parse errors. Literal ? * + require escaping. + * Writer/prettier prefers the grouped form ()? over bare ?. + */ + +function parse(src: string, file = "test.agr") { + return parseGrammarRules(file, src, false); +} + +function expectParseError(src: string, match: RegExp | string) { + expect(() => parse(src)).toThrow(match); +} + +describe("Quantifier special chars (? * +)", () => { + describe("expressionsSpecialChar + bare quantifier errors", () => { + it("errors on bare '?' not after ')' or '>'", () => { + expectParseError( + ` = what is the time? -> "q";`, + /Unexpected quantifier '\?'/, + ); + }); + + it("errors on bare '*' and '+' after words", () => { + expectParseError(` = one* two -> "x";`, /Unexpected quantifier '\*'/); + expectParseError(` = one+ two -> "x";`, /Unexpected quantifier '\+'/); + }); + + it("errors on bare '?' after a string group close that already consumed )?", () => { + // After (the)? the next bare ? is illegal. + expectParseError( + ` = (the)? ? -> "x";`, + /Unexpected quantifier '\?'/, + ); + }); + + it("errors on \"please\"? (quotes are literal chars; ? is bare)", () => { + // Quotes have no special meaning in patterns — "please" is chars + // including the quote glyphs, then bare ? is illegal. + expectParseError( + ` = "please"? -> "x";`, + /Unexpected quantifier '\?'/, + ); + }); + + it("errors on 'really?' (? inside unescaped quoted-looking text is still special)", () => { + // parseStrExpr stops at special '?', leaving bare ? → error + expectParseError( + ` = 'really?' -> "x";`, + /Unexpected quantifier '\?'/, + ); + }); + + it("errors on standalone quantifier token", () => { + expectParseError(` = ? -> "x";`, /Unexpected quantifier '\?'/); + expectParseError(` = * -> "x";`, /Unexpected quantifier '\*'/); + expectParseError(` = + -> "x";`, /Unexpected quantifier '\+'/); + }); + }); + + describe("escaped literals", () => { + it("accepts \\? as a literal question mark in the pattern", () => { + const ast = parse(` = what is the time\\? -> "q";`); + const expr = ast.definitions[0].rules[0].expressions[0] as { + type: string; + value: string[]; + }; + expect(expr.type).toBe("string"); + // "time?" is one token (escaped ? is part of the word) + expect(expr.value).toEqual(["what", "is", "the", "time?"]); + }); + + it("accepts \\* and \\+ as literals", () => { + const ast = parse(` = star\\* plus\\+ -> "x";`); + const expr = ast.definitions[0].rules[0].expressions[0] as { + type: string; + value: string[]; + }; + expect(expr.value).toEqual(["star*", "plus+"]); + }); + + it('accepts "really\\?" (quotes are match chars; escaped ? is literal)', () => { + const ast = parse(` = "really\\?" -> "q";`); + const expr = ast.definitions[0].rules[0].expressions[0] as { + type: string; + value: string[]; + }; + // Leading/trailing " are ordinary pattern characters + expect(expr.value).toEqual(['"really?"']); + }); + }); + + describe("postfix after '>' (rule refs)", () => { + it("parses ? / * / + as optional/repeat", () => { + const ast = parse(` + = alice | bob; + = show ? files -> "opt"; + = show * files -> "star"; + = show + files -> "plus"; + `); + const getRef = (name: string) => { + const def = ast.definitions.find( + (d) => d.definitionName.name === name, + )!; + return def.rules[0].expressions[1] as { + type: string; + optional?: boolean; + repeat?: boolean; + }; + }; + expect(getRef("Opt")).toMatchObject({ + type: "ruleReference", + optional: true, + }); + expect(getRef("Star")).toMatchObject({ + type: "ruleReference", + optional: true, + repeat: true, + }); + expect(getRef("Plus")).toMatchObject({ + type: "ruleReference", + repeat: true, + }); + expect(getRef("Plus").optional).toBeFalsy(); + }); + + it("parses required + escaped question: who sings song \\?", () => { + const ast = parse(` + = hello | goodbye; + = who sings song \\? -> "q"; + `); + const exprs = ast.definitions.find( + (d) => d.definitionName.name === "Start", + )!.rules[0].expressions; + // string "who sings song", ruleRef Song (required), string "?" + expect(exprs[0]).toMatchObject({ + type: "string", + value: ["who", "sings", "song"], + }); + expect(exprs[1]).toMatchObject({ + type: "ruleReference", + }); + expect( + (exprs[1] as { optional?: boolean }).optional, + ).toBeFalsy(); + expect(exprs[2]).toMatchObject({ + type: "string", + value: ["?"], + }); + }); + }); + + describe("postfix after ')' (groups + captures) unchanged", () => { + it("keeps (the | a)? / ()? / $(units:)?", () => { + const ast = parse(` + = metric | imperial; + = please | kindly; + = + (the | a)? item -> "det" + | ()? open -> "polite" + | measure $(units:)? -> "cap" + ; + `); + expect(ast.definitions.length).toBe(3); + const start = ast.definitions.find( + (d) => d.definitionName.name === "Start", + )!; + const det = start.rules[0].expressions[0] as { + type: string; + optional?: boolean; + }; + expect(det).toMatchObject({ type: "rules", optional: true }); + const polite = start.rules[1].expressions[0] as { + type: string; + optional?: boolean; + }; + expect(polite).toMatchObject({ type: "rules", optional: true }); + const cap = start.rules[2].expressions[1] as { + type: string; + optional?: boolean; + }; + expect(cap).toMatchObject({ type: "variable", optional: true }); + }); + + it("keeps ()* and ()+", () => { + const ast = parse(` + = + tag (bug | feature)* -> "star" + | need (reviewer)+ -> "plus" + ; + `); + const start = ast.definitions[0]; + expect(start.rules[0].expressions[1]).toMatchObject({ + type: "rules", + optional: true, + repeat: true, + }); + expect(start.rules[1].expressions[1]).toMatchObject({ + type: "rules", + repeat: true, + }); + }); + }); + + describe("writer/prettier prefers grouped form", () => { + it("rewrites bare ?/*/+ to grouped () form", () => { + const src = ` + = alice | bob; + = show ? files -> "opt"; + = show * files -> "star"; + = show + files -> "plus"; + `; + const written = writeGrammarRules(parse(src)); + expect(written).toContain("()?"); + expect(written).toContain("()*"); + expect(written).toContain("()+"); + // bare form should not remain on those quantified sites + expect(written).not.toMatch(/\?/); + expect(written).not.toMatch(/\*/); + expect(written).not.toMatch(/\+/); + + // reparse still has optional/repeat (via group) + const reparsed = parse(written, "roundtrip.agr"); + const getExpr = (name: string) => { + const def = reparsed.definitions.find( + (d) => d.definitionName.name === name, + )!; + return def.rules[0].expressions[1] as { + type: string; + optional?: boolean; + repeat?: boolean; + }; + }; + // After prettier, these are groups not bare rule refs + expect(getExpr("Opt")).toMatchObject({ + type: "rules", + optional: true, + }); + expect(getExpr("Star")).toMatchObject({ + type: "rules", + optional: true, + repeat: true, + }); + expect(getExpr("Plus")).toMatchObject({ + type: "rules", + repeat: true, + }); + }); + + it("escapes literal ? on write-back", () => { + const src = ` = what is the time\\? -> "q";`; + const written = writeGrammarRules(parse(src)); + expect(written).toMatch(/time\\\?/); + // must still parse + expect(() => parse(written, "rt.agr")).not.toThrow(); + }); + }); + + describe("colon stays unescaped (no ambiguity inside $())", () => { + it("still parses $(h:number) : $(m:number) with spacing annotation", () => { + // ":" in type position and as a literal separator between captures + // (existing builtInEntities pattern). Colon is not special. + const ast = parse(` + [spacing=optional] = $(h:number) : $(m:number) -> { h, m }; + `); + const exprs = ast.definitions[0].rules[0].expressions; + expect(exprs[0]).toMatchObject({ type: "variable" }); + expect(exprs[1]).toMatchObject({ + type: "string", + value: [":"], + }); + expect(exprs[2]).toMatchObject({ type: "variable" }); + }); + }); + + describe("value expressions after -> still use ternary ?", () => { + it("does not treat value-side ? as a pattern quantifier", () => { + const ast = parse( + ` = $(h:number) pm -> { hours: h < 12 ? h + 12 : h };`, + ); + expect(ast.definitions[0].rules[0].value).toBeDefined(); + }); + }); +}); + +describeForEachMatcher( + "Quantifier special chars — match semantics", + (testMatchGrammar) => { + it("bare ? matches with or without owner (same as ()?)", () => { + const bare = loadGrammarRules( + "bare.agr", + ` + = alice | bob; + = show ? files -> "bare"; + `, + ); + const grouped = loadGrammarRules( + "grouped.agr", + ` + = alice | bob; + = show ()? files -> "grouped"; + `, + ); + for (const input of [ + "show files", + "show alice files", + "show bob files", + ]) { + expect(testMatchGrammar(bare, input).length).toBe( + testMatchGrammar(grouped, input).length, + ); + expect(testMatchGrammar(bare, input)).toStrictEqual(["bare"]); + } + expect(testMatchGrammar(bare, "show charlie files")).toStrictEqual( + [], + ); + }); + + it("required Song + escaped ? matches question utterances", () => { + const g = loadGrammarRules( + "q.agr", + ` + = hello | goodbye; + = who sings song \\? -> "hit"; + `, + ); + expect(testMatchGrammar(g, "who sings song hello?")).toStrictEqual([ + "hit", + ]); + expect(testMatchGrammar(g, "who sings song goodbye?")).toStrictEqual([ + "hit", + ]); + // missing song + expect(testMatchGrammar(g, "who sings song?")).toStrictEqual([]); + // missing ? + expect(testMatchGrammar(g, "who sings song hello")).toStrictEqual( + [], + ); + }); + + it("optional Song + escaped ?: who sings song ()?\\?", () => { + const g = loadGrammarRules( + "opt.agr", + ` + = hello | goodbye; + = who sings song ()?\\? -> "hit"; + `, + ); + expect(testMatchGrammar(g, "who sings song?")).toStrictEqual([ + "hit", + ]); + expect(testMatchGrammar(g, "who sings song hello?")).toStrictEqual([ + "hit", + ]); + }); + + it("pitfall: who sings song ? is OPTIONAL song (no literal ? in pattern)", () => { + const g = loadGrammarRules( + "pitfall.agr", + ` + = hello | goodbye; + = who sings song ? -> "hit"; + `, + ); + // optional song — pattern has no literal "?" + expect(testMatchGrammar(g, "who sings song")).toStrictEqual([ + "hit", + ]); + expect(testMatchGrammar(g, "who sings song hello")).toStrictEqual([ + "hit", + ]); + // Matcher treats trailing utterance "?" as flex-space punctuation, so + // these still match. The pitfall is author intent (optional Song), not + // a hard reject of "?" in the request. + expect(testMatchGrammar(g, "who sings song hello?")).toStrictEqual([ + "hit", + ]); + // "who sings song?" — song name missing; trailing punct alone is not a Song + expect(testMatchGrammar(g, "who sings song?")).toStrictEqual([ + "hit", + ]); + }); + + it("literal time\\? matches trailing question mark", () => { + const g = loadGrammarRules( + "time.agr", + ` = what is the time\\? -> "q";`, + ); + expect(testMatchGrammar(g, "what is the time?")).toStrictEqual([ + "q", + ]); + expect(testMatchGrammar(g, "what is the time")).toStrictEqual([]); + }); + + it("? open app — optional polite prefix", () => { + const g = loadGrammarRules( + "polite.agr", + ` + = please | can you; + = ? open outlook -> "ok"; + `, + ); + expect(testMatchGrammar(g, "open outlook")).toStrictEqual(["ok"]); + expect(testMatchGrammar(g, "please open outlook")).toStrictEqual([ + "ok", + ]); + expect(testMatchGrammar(g, "can you open outlook")).toStrictEqual([ + "ok", + ]); + expect(testMatchGrammar(g, "open notepad")).toStrictEqual([]); + }); + + it("bare * / + match zero-or-more / one-or-more", () => { + const star = loadGrammarRules( + "star.agr", + ` + = alice | bob; + = show * files -> "star"; + `, + ); + const plus = loadGrammarRules( + "plus.agr", + ` + = alice | bob; + = show + files -> "plus"; + `, + ); + expect(testMatchGrammar(star, "show files")).toStrictEqual([ + "star", + ]); + expect(testMatchGrammar(star, "show alice bob files")).toStrictEqual( + ["star"], + ); + expect(testMatchGrammar(plus, "show files")).toStrictEqual([]); + expect(testMatchGrammar(plus, "show alice files")).toStrictEqual([ + "plus", + ]); + }); + + it("is Tesla the best car(\\?)? — optional literal ?", () => { + const g = loadGrammarRules( + "tesla.agr", + ` = is Tesla the best car(\\?)? -> "ok";`, + ); + expect(testMatchGrammar(g, "is Tesla the best car")).toContain("ok"); + // May yield multiple matches (optional group taken vs skipped with + // trailing punct as flex-space) — both are successful hits. + const withQ = testMatchGrammar(g, "is Tesla the best car?"); + expect(withQ.length).toBeGreaterThanOrEqual(1); + expect(withQ.every((v) => v === "ok")).toBe(true); + }); + }, +); diff --git a/ts/packages/agentSdkWrapper/test/calendar-extended.agr b/ts/packages/agentSdkWrapper/test/calendar-extended.agr index f860fa6b9f..de900ce1ce 100644 --- a/ts/packages/agentSdkWrapper/test/calendar-extended.agr +++ b/ts/packages/agentSdkWrapper/test/calendar-extended.agr @@ -11,9 +11,9 @@ = ('add' | 'invite' | 'include' | 'bring in') ('to' | 'in' | 'on') -> { actionName: "addParticipant", parameters: { description, participant } }; - = ('show me' | 'find' | 'get' | 'what is' | 'what\'s' | 'tell me' | 'can you tell me') ('on my calendar'? 'today' | 'today\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')? | ('what I have' | 'what\'s scheduled') ('for'? 'today')) -> { actionName: "findTodaysEvents", parameters: {} }; + = ('show me' | 'find' | 'get' | 'what is' | 'what\'s' | 'tell me' | 'can you tell me') (('on my calendar')? 'today' | 'today\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')? | ('what I have' | 'what\'s scheduled') (('for')? 'today')) -> { actionName: "findTodaysEvents", parameters: {} }; - = ('show me' | 'find' | 'get' | 'what is' | 'what\'s' | 'tell me') ('on my calendar'? ('this week' | 'for this week') | 'this week\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')?) -> { actionName: "findThisWeeksEvents", parameters: {} }; + = ('show me' | 'find' | 'get' | 'what is' | 'what\'s' | 'tell me') (('on my calendar')? ('this week' | 'for this week') | 'this week\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')?) -> { actionName: "findThisWeeksEvents", parameters: {} }; // Shared sub-rules = ('can you' | 'please' | 'would you' | 'could you')?; @@ -32,7 +32,7 @@ = ('in' | 'at' | 'location') ($(location:string) | 'TBD'); - = ('please'? ('invite' | 'include' | 'bring in') | 'with') ; + = (('please')? ('invite' | 'include' | 'bring in') | 'with') ; = $(participant:string) ((',' | 'and') $(participant:string))*; diff --git a/ts/packages/agentSdkWrapper/test/calendar-new.agr b/ts/packages/agentSdkWrapper/test/calendar-new.agr index 460ecfe472..f3e4d477f3 100644 --- a/ts/packages/agentSdkWrapper/test/calendar-new.agr +++ b/ts/packages/agentSdkWrapper/test/calendar-new.agr @@ -11,9 +11,9 @@ = ('add' | 'invite' | 'include') ('to' | 'in') -> { actionName: "addParticipant", parameters: { description, participant } }; - = ('show me' | 'find' | 'get' | 'what is' | 'what\'s') ('on my calendar'? 'today' | 'today\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')?) -> { actionName: "findTodaysEvents", parameters: {} }; + = ('show me' | 'find' | 'get' | 'what is' | 'what\'s') (('on my calendar')? 'today' | 'today\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')?) -> { actionName: "findTodaysEvents", parameters: {} }; - = ('show me' | 'find' | 'get' | 'what is' | 'what\'s') ('on my calendar'? ('this week' | 'for this week') | 'this week\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')?) -> { actionName: "findThisWeeksEvents", parameters: {} }; + = ('show me' | 'find' | 'get' | 'what is' | 'what\'s') (('on my calendar')? ('this week' | 'for this week') | 'this week\'s' ('events' | 'schedule' | 'calendar' | 'meetings' | 'appointments')?) -> { actionName: "findThisWeeksEvents", parameters: {} }; // Shared sub-rules = ('can you' | 'please' | 'would you')?; @@ -32,7 +32,7 @@ = ('in' | 'at') $(location:string); - = ('please'? ('invite' | 'include') | 'with') ; + = (('please')? ('invite' | 'include') | 'with') ; = $(participant:string) ((',' | 'and') $(participant:string))*; diff --git a/ts/packages/agents/code/src/vscode/debugSchema.agr b/ts/packages/agents/code/src/vscode/debugSchema.agr index 3b73894a24..a1eb754380 100644 --- a/ts/packages/agents/code/src/vscode/debugSchema.agr +++ b/ts/packages/agents/code/src/vscode/debugSchema.agr @@ -11,8 +11,8 @@ import { CodeDebugActions } from "./debugActionsSchema.ts"; = ('show' | 'open' | 'display') ('debug' | 'debugging') ('panel' | 'window' | 'view') -> { actionName: "showDebugPanel", parameters: {} }; - = () ('with' | 'using') ('the'?) $(configurationName:string) ('configuration' | 'config' | 'setup') $(noDebug:) -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: noDebug } } - | () ('with' | 'using') ('the'?) $(configurationName:string) ('configuration' | 'config' | 'setup') -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: "false" } } + = () ('with' | 'using') ('the')? $(configurationName:string) ('configuration' | 'config' | 'setup') $(noDebug:) -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: noDebug } } + | () ('with' | 'using') ('the')? $(configurationName:string) ('configuration' | 'config' | 'setup') -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: "false" } } | () $(configurationName:string) ('config' | 'configuration') $(noDebug:) -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: noDebug } } | () $(configurationName:string) ('config' | 'configuration') -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: "false" } } | ('debug' | 'launch') $(configurationName:string) $(noDebug:) -> { actionName: "startDebugging", parameters: { configurationName: configurationName, noDebug: noDebug } } @@ -25,17 +25,17 @@ import { CodeDebugActions } from "./debugActionsSchema.ts"; = ('show' | 'display') ('hover' | 'hover info' | 'hover information') -> { actionName: "showHover", parameters: {} }; - = ('toggle' | 'add or remove' | 'set') ('a'?) ('breakpoint') ('on' | 'at') ('line')? $(line:string) ('in' | 'of') $(fileName:string) ('in' | 'within') ('the')? $(folderName:string) ('folder' | 'directory')? -> { actionName: "toggleBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } - | ('toggle' | 'add or remove' | 'set') ('a'?) ('breakpoint') ('line') $(line:string) ('file' | 'in') $(fileName:string) ('folder' | 'directory') $(folderName:string) -> { actionName: "toggleBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } - | ('toggle' | 'add or remove' | 'set') ('a'?) ('breakpoint') ('line') $(line:string) (',' | 'in') ('file')? $(fileName:string) (',' | 'in') $(folderName:string) ('folder')? -> { actionName: "toggleBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } }; + = ('toggle' | 'add or remove' | 'set') ('a')? ('breakpoint') ('on' | 'at') ('line')? $(line:string) ('in' | 'of') $(fileName:string) ('in' | 'within') ('the')? $(folderName:string) ('folder' | 'directory')? -> { actionName: "toggleBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } + | ('toggle' | 'add or remove' | 'set') ('a')? ('breakpoint') ('line') $(line:string) ('file' | 'in') $(fileName:string) ('folder' | 'directory') $(folderName:string) -> { actionName: "toggleBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } + | ('toggle' | 'add or remove' | 'set') ('a')? ('breakpoint') ('line') $(line:string) (',' | 'in') ('file')? $(fileName:string) (',' | 'in') $(folderName:string) ('folder')? -> { actionName: "toggleBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } }; - = ('set' | 'add' | 'place') ('a'?) ('breakpoint') ('on' | 'at') ('line')? $(line:string) ('in' | 'of') $(fileName:string) ('in' | 'within') ('the')? $(folderName:string) ('folder' | 'directory')? -> { actionName: "setBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } - | ('set' | 'add' | 'place') ('a'?) ('breakpoint') ('line') $(line:string) ('file' | 'in') $(fileName:string) ('folder' | 'directory') $(folderName:string) -> { actionName: "setBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } - | ('set' | 'add' | 'place') ('a'?) ('breakpoint') ('line') $(line:string) (',' | 'in') ('file')? $(fileName:string) (',' | 'in') $(folderName:string) ('folder')? -> { actionName: "setBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } }; + = ('set' | 'add' | 'place') ('a')? ('breakpoint') ('on' | 'at') ('line')? $(line:string) ('in' | 'of') $(fileName:string) ('in' | 'within') ('the')? $(folderName:string) ('folder' | 'directory')? -> { actionName: "setBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } + | ('set' | 'add' | 'place') ('a')? ('breakpoint') ('line') $(line:string) ('file' | 'in') $(fileName:string) ('folder' | 'directory') $(folderName:string) -> { actionName: "setBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } + | ('set' | 'add' | 'place') ('a')? ('breakpoint') ('line') $(line:string) (',' | 'in') ('file')? $(fileName:string) (',' | 'in') $(folderName:string) ('folder')? -> { actionName: "setBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } }; - = ('remove' | 'delete' | 'clear') ('the'?) ('breakpoint') ('on' | 'at') ('line')? $(line:string) ('in' | 'of') $(fileName:string) ('in' | 'within') ('the')? $(folderName:string) ('folder' | 'directory')? -> { actionName: "removeBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } - | ('remove' | 'delete' | 'clear') ('the'?) ('breakpoint') ('line') $(line:string) ('file' | 'in') $(fileName:string) ('folder' | 'directory') $(folderName:string) -> { actionName: "removeBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } - | ('remove' | 'delete' | 'clear') ('the'?) ('breakpoint') ('line') $(line:string) (',' | 'in') ('file')? $(fileName:string) (',' | 'in') $(folderName:string) ('folder')? -> { actionName: "removeBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } }; + = ('remove' | 'delete' | 'clear') ('the')? ('breakpoint') ('on' | 'at') ('line')? $(line:string) ('in' | 'of') $(fileName:string) ('in' | 'within') ('the')? $(folderName:string) ('folder' | 'directory')? -> { actionName: "removeBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } + | ('remove' | 'delete' | 'clear') ('the')? ('breakpoint') ('line') $(line:string) ('file' | 'in') $(fileName:string) ('folder' | 'directory') $(folderName:string) -> { actionName: "removeBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } } + | ('remove' | 'delete' | 'clear') ('the')? ('breakpoint') ('line') $(line:string) (',' | 'in') ('file')? $(fileName:string) (',' | 'in') $(folderName:string) ('folder')? -> { actionName: "removeBreakpoint", parameters: { line: line, fileName: fileName, folderName: folderName } }; = ('remove' | 'delete' | 'clear') ('all') ('breakpoints' | 'the breakpoints') -> { actionName: "removeAllBreakpoints", parameters: {} }; diff --git a/ts/packages/agents/code/src/vscode/displaySchema.agr b/ts/packages/agents/code/src/vscode/displaySchema.agr index 256ce43f49..5f9ee85a23 100644 --- a/ts/packages/agents/code/src/vscode/displaySchema.agr +++ b/ts/packages/agents/code/src/vscode/displaySchema.agr @@ -20,31 +20,31 @@ import { CodeDisplayActions } from "./displayActionsSchema.ts"; // Zoom actions = (('zoom' ) | ) -> { actionName: "zoomIn", parameters: {} } - | 'see this closer' ('zoom in'?)? -> { actionName: "zoomIn", parameters: {} } + | 'see this closer' ('zoom in')? -> { actionName: "zoomIn", parameters: {} } | ('so I can see better' | 'please')? -> { actionName: "zoomIn", parameters: {} }; = 'zoom out' ('a bit' | 'please' | 'so I can see more')? -> { actionName: "zoomOut", parameters: {} } | ('so I can see more' | 'so I can get a better overview' | 'for a better view')? -> { actionName: "zoomOut", parameters: {} } - | ('see the bigger picture' | 'see more of the page') ('zoom out'?)? -> { actionName: "zoomOut", parameters: {} }; + | ('see the bigger picture' | 'see more of the page') ('zoom out')? -> { actionName: "zoomOut", parameters: {} }; = ('reset' | 'restore') ('font' | 'text')? ('zoom' | 'size') ('to default' | 'to normal')? -> { actionName: "fontZoomReset", parameters: {} } | ('default' | 'normal' | 'original') ('font' | 'text') ('size' | 'zoom') -> { actionName: "fontZoomReset", parameters: {} }; // Panel and view actions - = ('the'? ('file' | 'project')? ('explorer' | 'tree' | 'browser') ?) -> { actionName: "showExplorer", parameters: {} } - | 'see the project files' ('show explorer'?)? -> { actionName: "showExplorer", parameters: {} } + = (('the')? ('file' | 'project')? ('explorer' | 'tree' | 'browser') ?) -> { actionName: "showExplorer", parameters: {} } + | 'see the project files' ('show explorer')? -> { actionName: "showExplorer", parameters: {} } | 'explorer' -> { actionName: "showExplorer", parameters: {} }; - = ('the'? 'search' ('for me' | | 'interface')?) -> { actionName: "showSearch", parameters: {} } - | ('search for something' | 'find something') ('bring up the search interface'?)? -> { actionName: "showSearch", parameters: {} } + = (('the')? 'search' ('for me' | | 'interface')?) -> { actionName: "showSearch", parameters: {} } + | ('search for something' | 'find something') ('bring up the search interface')? -> { actionName: "showSearch", parameters: {} } | 'search' -> { actionName: "showSearch", parameters: {} }; - = ('the'? 'source control' ?) -> { actionName: "showSourceControl", parameters: {} } - | ('see the git status' | 'check my changes' | 'see source control') ('show source control please'?)? -> { actionName: "showSourceControl", parameters: {} } + = (('the')? 'source control' ?) -> { actionName: "showSourceControl", parameters: {} } + | ('see the git status' | 'check my changes' | 'see source control') ('show source control please')? -> { actionName: "showSourceControl", parameters: {} } | ('git' | 'version control' | 'scm') ? -> { actionName: "showSourceControl", parameters: {} }; - = ('the'? 'output' ?) -> { actionName: "showOutputPanel", parameters: {} } - | 'see the output' ('show output panel'?)? -> { actionName: "showOutputPanel", parameters: {} } + = (('the')? 'output' ?) -> { actionName: "showOutputPanel", parameters: {} } + | 'see the output' ('show output panel')? -> { actionName: "showOutputPanel", parameters: {} } | 'output' -> { actionName: "showOutputPanel", parameters: {} }; // Search and replace actions @@ -56,7 +56,7 @@ import { CodeDisplayActions } from "./displayActionsSchema.ts"; | ('replace text across files' | 'do a global find and replace') -> { actionName: "replaceInFiles", parameters: {} }; // Markdown preview actions - = ('the'? 'markdown preview' | 'preview' ('of this markdown' | 'for this file')?) -> { actionName: "openMarkdownPreview", parameters: {} } + = (('the')? 'markdown preview' | 'preview' ('of this markdown' | 'for this file')?) -> { actionName: "openMarkdownPreview", parameters: {} } | 'preview this markdown' ('file' | 'document')? -> { actionName: "openMarkdownPreview", parameters: {} }; = ('markdown preview' | 'preview') -> { actionName: "openMarkdownPreviewToSide", parameters: {} } @@ -65,12 +65,12 @@ import { CodeDisplayActions } from "./displayActionsSchema.ts"; // Mode and editor actions = ('enter' | 'enable' | 'turn on')? 'zen mode' -> { actionName: "zenMode", parameters: {} } - | ('focus' | 'concentrate') ('enter zen mode'?)? -> { actionName: "zenMode", parameters: {} } + | ('focus' | 'concentrate') ('enter zen mode')? -> { actionName: "zenMode", parameters: {} } | 'zen mode' -> { actionName: "zenMode", parameters: {} }; - = ('the'? ('current'? ('editor' | 'tab' | 'file') | 'this' ('file' | 'tab'))) -> { actionName: "closeEditor", parameters: {} } + = (('the')? (('current')? ('editor' | 'tab' | 'file') | 'this' ('file' | 'tab'))) -> { actionName: "closeEditor", parameters: {} } | 'close this' ('file' | 'tab' | 'editor')? -> { actionName: "closeEditor", parameters: {} }; - = ('the'? ('settings' | 'preferences' | 'configuration' | 'options')) -> { actionName: "openSettings", parameters: {} } - | ('change settings' | 'configure something') ('open settings'?)? -> { actionName: "openSettings", parameters: {} } + = (('the')? ('settings' | 'preferences' | 'configuration' | 'options')) -> { actionName: "openSettings", parameters: {} } + | ('change settings' | 'configure something') ('open settings')? -> { actionName: "openSettings", parameters: {} } | ('settings' | 'preferences') -> { actionName: "openSettings", parameters: {} }; \ No newline at end of file diff --git a/ts/packages/agents/code/src/vscode/editorSchema.agr b/ts/packages/agents/code/src/vscode/editorSchema.agr index 0e169ec8fd..86c851f26b 100644 --- a/ts/packages/agents/code/src/vscode/editorSchema.agr +++ b/ts/packages/agents/code/src/vscode/editorSchema.agr @@ -10,7 +10,7 @@ import { EditorCodeActions } from "./editorCodeActionsSchema.ts"; : EditorCodeActions = | | | | | | | | | ; // Common patterns and sub-rules - = ('can you'? | 'please'? | 'could you'?); + = (('can you')? | ('please')? | ('could you')?); = ('quick' | 'quickly' | 'fast')?; = ('in' | 'using' | 'as') $(language:string) -> language; = ('in' | 'to' | 'at') $(file:string) ('file' | 'document')? -> file; diff --git a/ts/packages/agents/code/src/vscode/generalSchema.agr b/ts/packages/agents/code/src/vscode/generalSchema.agr index e8be693f75..29f6d12b40 100644 --- a/ts/packages/agents/code/src/vscode/generalSchema.agr +++ b/ts/packages/agents/code/src/vscode/generalSchema.agr @@ -10,15 +10,15 @@ import { CodeGeneralActions } from "./generalActionsSchema.ts"; : CodeGeneralActions = | | | ; // Action rules - = ('open' | 'show' | 'bring up' | 'display') 'the'? 'command palette' -> { actionName: "showCommandPalette", parameters: {} }; + = ('open' | 'show' | 'bring up' | 'display') ('the')? 'command palette' -> { actionName: "showCommandPalette", parameters: {} }; = $(goto:string) ('in' | 'from') $(ref:string) -> { actionName: "gotoFileOrLineOrSymbol", parameters: { goto: goto, ref: ref } } | $(goto:string) -> { actionName: "gotoFileOrLineOrSymbol", parameters: { goto: goto, ref: goto } } - | 'to' 'the'? $(goto:string) ('function' | 'method' | 'component' | 'class')? -> { actionName: "gotoFileOrLineOrSymbol", parameters: { goto: goto, ref: goto } }; + | 'to' ('the')? $(goto:string) ('function' | 'method' | 'component' | 'class')? -> { actionName: "gotoFileOrLineOrSymbol", parameters: { goto: goto, ref: goto } }; - = ('open' | 'show' | 'display') ('user'? 'settings' | 'preferences' | 'configuration') -> { actionName: "showUserSettings", parameters: {} }; + = ('open' | 'show' | 'display') (('user')? 'settings' | 'preferences' | 'configuration') -> { actionName: "showUserSettings", parameters: {} }; - = ('show' | 'display' | 'open') ('keyboard'? 'shortcuts' | 'keybindings' | 'hotkeys') -> { actionName: "showKeyboardShortcuts", parameters: {} }; + = ('show' | 'display' | 'open') (('keyboard')? 'shortcuts' | 'keybindings' | 'hotkeys') -> { actionName: "showKeyboardShortcuts", parameters: {} }; // Shared sub-rules = ('can you' | 'please' | 'would you')?; diff --git a/ts/packages/agents/osNotifications/src/osNotificationsSchema.agr b/ts/packages/agents/osNotifications/src/osNotificationsSchema.agr index 2eb2719a9d..3200416c6e 100644 --- a/ts/packages/agents/osNotifications/src/osNotificationsSchema.agr +++ b/ts/packages/agents/osNotifications/src/osNotificationsSchema.agr @@ -11,6 +11,6 @@ import { OsNotificationsActions } from "./osNotificationsSchema.ts"; = ("can you" | "please" | "would you")?; - = ('sync' | 'pull' | 'show me' | 'replay') ('the' | 'my' | 'all')? ('os' | 'system' | 'windows')? 'notification' 's'? -> { actionName: "syncOsNotifications", parameters: {} }; + = ('sync' | 'pull' | 'show me' | 'replay') ('the' | 'my' | 'all')? ('os' | 'system' | 'windows')? 'notification' ('s')? -> { actionName: "syncOsNotifications", parameters: {} }; = ('test' | 'send' ('a' | 'me' 'a')?) ('os' | 'system')? 'notification' ('saying' | 'with' 'message' | 'that' 'says')? $(message:string) -> { actionName: "testOsNotification", parameters: { message: message } }; diff --git a/ts/packages/agents/photo/src/photoSchema.agr b/ts/packages/agents/photo/src/photoSchema.agr index abcaa9978b..37d8a0284e 100644 --- a/ts/packages/agents/photo/src/photoSchema.agr +++ b/ts/packages/agents/photo/src/photoSchema.agr @@ -16,4 +16,4 @@ import { PhotoAction } from "./photoSchema.ts"; = ('take' ('a' ('photo' | 'picture' | 'pic' | 'shot'))?) | 'snap' ('a' ('picture' | 'pic' | 'shot'))? | 'photograph' | 'photo'; - = 'of'?; \ No newline at end of file + = ('of')?; \ No newline at end of file From 7ad1da9ba889dc3ad56a7778f49bbce627cd2504 Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Mon, 3 Aug 2026 21:01:54 +0000 Subject: [PATCH 2/6] style: apply prettier formatting and policy fixes --- .../actionGrammar/src/grammarRuleParser.ts | 1 - .../test/quantifierSpecialChars.spec.ts | 47 ++++++++++++------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/ts/packages/actionGrammar/src/grammarRuleParser.ts b/ts/packages/actionGrammar/src/grammarRuleParser.ts index cc56e2a866..58c1d10fb9 100644 --- a/ts/packages/actionGrammar/src/grammarRuleParser.ts +++ b/ts/packages/actionGrammar/src/grammarRuleParser.ts @@ -709,7 +709,6 @@ class GrammarRuleParser implements ValueExprParserContext { } } - /** * Apply a postfix quantifier immediately after a rule reference (`>`). * Sets optional/repeat on `target` and advances past the quantifier char. diff --git a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts index 40a5ce8b8b..e949622527 100644 --- a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts +++ b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts @@ -30,8 +30,14 @@ describe("Quantifier special chars (? * +)", () => { }); it("errors on bare '*' and '+' after words", () => { - expectParseError(` = one* two -> "x";`, /Unexpected quantifier '\*'/); - expectParseError(` = one+ two -> "x";`, /Unexpected quantifier '\+'/); + expectParseError( + ` = one* two -> "x";`, + /Unexpected quantifier '\*'/, + ); + expectParseError( + ` = one+ two -> "x";`, + /Unexpected quantifier '\+'/, + ); }); it("errors on bare '?' after a string group close that already consumed )?", () => { @@ -42,7 +48,7 @@ describe("Quantifier special chars (? * +)", () => { ); }); - it("errors on \"please\"? (quotes are literal chars; ? is bare)", () => { + it('errors on "please"? (quotes are literal chars; ? is bare)', () => { // Quotes have no special meaning in patterns — "please" is chars // including the quote glyphs, then bare ? is illegal. expectParseError( @@ -60,9 +66,18 @@ describe("Quantifier special chars (? * +)", () => { }); it("errors on standalone quantifier token", () => { - expectParseError(` = ? -> "x";`, /Unexpected quantifier '\?'/); - expectParseError(` = * -> "x";`, /Unexpected quantifier '\*'/); - expectParseError(` = + -> "x";`, /Unexpected quantifier '\+'/); + expectParseError( + ` = ? -> "x";`, + /Unexpected quantifier '\?'/, + ); + expectParseError( + ` = * -> "x";`, + /Unexpected quantifier '\*'/, + ); + expectParseError( + ` = + -> "x";`, + /Unexpected quantifier '\+'/, + ); }); }); @@ -148,9 +163,7 @@ describe("Quantifier special chars (? * +)", () => { expect(exprs[1]).toMatchObject({ type: "ruleReference", }); - expect( - (exprs[1] as { optional?: boolean }).optional, - ).toBeFalsy(); + expect((exprs[1] as { optional?: boolean }).optional).toBeFalsy(); expect(exprs[2]).toMatchObject({ type: "string", value: ["?"], @@ -335,9 +348,9 @@ describeForEachMatcher( expect(testMatchGrammar(g, "who sings song hello?")).toStrictEqual([ "hit", ]); - expect(testMatchGrammar(g, "who sings song goodbye?")).toStrictEqual([ - "hit", - ]); + expect( + testMatchGrammar(g, "who sings song goodbye?"), + ).toStrictEqual(["hit"]); // missing song expect(testMatchGrammar(g, "who sings song?")).toStrictEqual([]); // missing ? @@ -436,9 +449,9 @@ describeForEachMatcher( expect(testMatchGrammar(star, "show files")).toStrictEqual([ "star", ]); - expect(testMatchGrammar(star, "show alice bob files")).toStrictEqual( - ["star"], - ); + expect( + testMatchGrammar(star, "show alice bob files"), + ).toStrictEqual(["star"]); expect(testMatchGrammar(plus, "show files")).toStrictEqual([]); expect(testMatchGrammar(plus, "show alice files")).toStrictEqual([ "plus", @@ -450,7 +463,9 @@ describeForEachMatcher( "tesla.agr", ` = is Tesla the best car(\\?)? -> "ok";`, ); - expect(testMatchGrammar(g, "is Tesla the best car")).toContain("ok"); + expect(testMatchGrammar(g, "is Tesla the best car")).toContain( + "ok", + ); // May yield multiple matches (optional group taken vs skipped with // trailing punct as flex-space) — both are successful hits. const withQ = testMatchGrammar(g, "is Tesla the best car?"); From 9a9c202c6edd9b0e81e7fef8677158cff008f296 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Mon, 3 Aug 2026 14:58:31 -0700 Subject: [PATCH 3/6] fix(action-grammar): harden quantifier specials after adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Prettier: lock bare ?/*/+ → ()?/*/+ rewrite (writer + grammar-tools format) - Generator prompts: fix \\? so runtime teaches real escapes (not bare ?) - Sync agentSdkWrapper schema→grammar prompt with quantifier rules - Capture $(x)*/$(x)+: actionable error pointing at ($(x))+ form - Phrase-set wrap inherits parent spacingMode (bare ≡ grouped lowering) - Docs/tests: silent ? pitfall, import *, value ?. / ??, CORRECT-line guards --- ts/extensions/agr-language/sample.agr | 7 +- ts/packages/actionGrammar/README.md | 3 + .../generation/schemaToGrammarGenerator.ts | 25 ++- .../actionGrammar/src/grammarCompiler.ts | 24 ++- .../actionGrammar/src/grammarRuleParser.ts | 12 +- .../test/grammarRuleWriter.spec.ts | 25 +++ .../test/quantifierSpecialChars.spec.ts | 193 ++++++++++++++++++ .../src/schemaToGrammarGenerator.ts | 29 ++- .../grammarTools/core/test/format.spec.ts | 58 ++++++ 9 files changed, 348 insertions(+), 28 deletions(-) create mode 100644 ts/packages/grammarTools/core/test/format.spec.ts diff --git a/ts/extensions/agr-language/sample.agr b/ts/extensions/agr-language/sample.agr index 512c44e172..ccf85c933b 100644 --- a/ts/extensions/agr-language/sample.agr +++ b/ts/extensions/agr-language/sample.agr @@ -165,8 +165,11 @@ export = hello | goodbye; | (item)+ // one-or-more group | (prefix)* suffix; // zero-or-more group // Literal "?" must be escaped (bare ?/*/+ are quantifiers after ) or > only): - // what is the time\? - // who sings song \? + // what is the time\? // literal trailing ? + // who sings song \? // required Song + literal ? + // who sings song ? // OPTIONAL Song, NO literal "?" (silent pitfall) + // who sings song ()?\? // optional Song + literal ? + // Writer prefers ()? over bare ?. // ─── Optional capture quantifiers ──────────────────────────────────────────── diff --git a/ts/packages/actionGrammar/README.md b/ts/packages/actionGrammar/README.md index 9f53a6dd45..470f7e13d6 100644 --- a/ts/packages/actionGrammar/README.md +++ b/ts/packages/actionGrammar/README.md @@ -46,6 +46,9 @@ import { Helper } from "./other.agr"; // Grammar imports // Bare ? elsewhere is a parse error — escape literals as \? // what is the time\? // literal trailing ? // who sings song \? // required Song + literal ? +// who sings song ? // OPTIONAL Song, NO literal "?" (pitfall) +// who sings song ()?\? // optional Song + literal "?" +// Writer/prettier always emits the grouped form: ()? not bare ?. ``` ## Exports diff --git a/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts b/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts index 6e845dc24c..99baa7e66e 100644 --- a/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts +++ b/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts @@ -47,9 +47,9 @@ The Action Grammar format uses: - Rule definitions: = pattern; - Literal text: "play" or 'play' - Wildcards with types: $(name:Type) - captures any text and assigns it to 'name' with validation type 'Type' -- Optional elements: element? -- Zero or more: element* -- One or more: element+ +- Optional: (element)? or ? (quantifiers ? * + ONLY valid immediately after ")" or ">") +- Zero or more: (element)* or * +- One or more: (element)+ or + - Alternation: pattern1 | pattern2 - Grouping: (expression) - groups expressions for operators - Rule references: @@ -62,8 +62,9 @@ FULL EXAMPLE showing captures and action body: CRITICAL SYNTAX RULES: 1. ALWAYS use parentheses around alternatives when combined with operators - CORRECT: ('can you'? 'add' | 'include') - WRONG: 'can you'? 'add' | 'include' + CORRECT: (((can you)? add) | include) + WRONG: can you? add | include (bare ? after a word is a PARSE ERROR) + WRONG: 'can you'? 'add' | 'include' (bare ? after a string is also a PARSE ERROR) 2. ALWAYS use parentheses around groups that should be treated as a unit CORRECT: ('on' | 'for') $(date:CalendarDate) @@ -117,9 +118,9 @@ CRITICAL SYNTAX RULES: 10. Quantifiers ? * + are SPECIAL characters in patterns: - Valid ONLY immediately after ")" or ">" : ()?, ?, $(x)?, (a|b)* - - Bare "?" after a word is a PARSE ERROR. Escape literals: what is the time\? - - Required name + question mark: who sings song \? - - Optional name + question mark: who sings song ()?\? + - Bare "?" after a word is a PARSE ERROR. Escape literals: what is the time\\? + - Required name + question mark: who sings song \\? + - Optional name + question mark: who sings song ()?\\? - Do NOT write ? intending a literal "?"; that makes Polite optional. - The writer/prettier prefers the grouped form ()? over bare ?. @@ -153,7 +154,8 @@ EFFICIENCY GUIDELINES: Example: If multiple actions use date expressions, create = ('on' | 'for') $(date:CalendarDate); 2. Create shared vocabulary rules for common phrases - Example: = 'can you'? | 'please'? | 'would you'?; + Example: = can you | please | would you; + Then make the whole rule optional at use sites: ()? open outlook 3. Reuse entity type rules across actions Example: If multiple actions need participant names, reference the same wildcard pattern @@ -647,6 +649,11 @@ Remember the CRITICAL SYNTAX RULES: CORRECT: { name: name, language: language } WRONG: { declaration: details.declaration } (dot notation not valid) Capture each parameter as its own $(var:type) wildcard. +12. Quantifiers ? * + are SPECIAL — valid ONLY immediately after ")" or ">": + CORRECT: ()?, ?, $(x)?, (a|b)*, (please)?, (can you)? + WRONG: please? can you? 'can you'? "please"? word* (bare quantifier = PARSE ERROR) + Literal ? * + require backslash escapes: what is the time\\? + Prefer grouped form ()? over bare ?. Return the complete corrected grammar, starting with the copyright header.`; diff --git a/ts/packages/actionGrammar/src/grammarCompiler.ts b/ts/packages/actionGrammar/src/grammarCompiler.ts index 960a4e332b..bc1bd59d71 100644 --- a/ts/packages/actionGrammar/src/grammarCompiler.ts +++ b/ts/packages/actionGrammar/src/grammarCompiler.ts @@ -1337,18 +1337,24 @@ function createGrammarRule( ); // PhraseSetPart cannot carry optional/repeat; wrap so bare // ? / * / + match the grouped form. + // Inherit parent spacingMode on the synthetic rule so bare + // and grouped ()? lower to the same boundary mode + // (grouped goes through createGrammarRules(..., spacingMode)). if (optional || repeat) { const q = repeat ? (optional ? "*" : "+") : "?"; parts.push( - createRulesPart([{ parts: [phrasePart] }], { - optional, - repeat, - partId: allocPartId( - context, - expr.pos, - `<${expr.refName.name}>${q}`, - ), - }), + createRulesPart( + [{ parts: [phrasePart], spacingMode }], + { + optional, + repeat, + partId: allocPartId( + context, + expr.pos, + `<${expr.refName.name}>${q}`, + ), + }, + ), ); } else { parts.push(phrasePart); diff --git a/ts/packages/actionGrammar/src/grammarRuleParser.ts b/ts/packages/actionGrammar/src/grammarRuleParser.ts index 58c1d10fb9..c4e411e1fc 100644 --- a/ts/packages/actionGrammar/src/grammarRuleParser.ts +++ b/ts/packages/actionGrammar/src/grammarRuleParser.ts @@ -858,13 +858,21 @@ class GrammarRuleParser implements ValueExprParserContext { attach(v); expNodes.push(v); // Captures only support optional today ($(x)?). Use ($(x))* / ($(x))+ - // for repetition (group form). Bare )* / )+ after $() is rejected below - // if someone writes $(x)* without grouping — variables lack repeat. + // for repetition (group form). if (this.isAt(")?")) { v.optional = true; this.skipWhitespace(2); continue; } + // $(x)* / $(x)+ look like "after )" but captures have no repeat flag — + // reject with an actionable message before the generic bare-quantifier error. + if (this.isAt(")*") || this.isAt(")+")) { + const q = this.content[this.curr + 1]; + this.throwError( + `Capture $(...) only supports optional via )? . ` + + `Use ($(...))${q} for repetition (group form).`, + ); + } this.consume(")", "at end of variable"); continue; } diff --git a/ts/packages/actionGrammar/test/grammarRuleWriter.spec.ts b/ts/packages/actionGrammar/test/grammarRuleWriter.spec.ts index 799f72293c..dce73afce3 100644 --- a/ts/packages/actionGrammar/test/grammarRuleWriter.spec.ts +++ b/ts/packages/actionGrammar/test/grammarRuleWriter.spec.ts @@ -287,6 +287,31 @@ describe("Grammar Rule Writer", () => { it("kleene star with alternates", () => { validateRoundTrip(` = hello (world | earth)* end;`); }); + // CurtisM: prettier always prefers the grouped form. Bare ?/*/+ and + // ()?/*/+ are equivalent at parse time; writer emits the grouped form. + it("rewrites bare quantified rule refs to grouped form", () => { + const written = fmt(` + = one | two; + = hello ? world; + = hello * world; + = hello + world; + `); + expect(written).toContain("()?"); + expect(written).toContain("()*"); + expect(written).toContain("()+"); + expect(written).not.toMatch(/\?/); + expect(written).not.toMatch(/\*/); + expect(written).not.toMatch(/\+/); + // Grouped output still round-trips + validateRoundTrip(written); + }); + it("escapes literal quantifier chars on write-back", () => { + const written = fmt(` = what is the time\\? star\\* plus\\+;`); + expect(written).toMatch(/time\\\?/); + expect(written).toMatch(/star\\\*/); + expect(written).toMatch(/plus\\\+/); + validateRoundTrip(written); + }); it("spaces in expressions", () => { validateRoundTrip( ` = ${spaces}${escapedSpaces}${spaces}${escapedSpaces}${spaces};`, diff --git a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts index e949622527..0d72d2a1c2 100644 --- a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts +++ b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts @@ -1,11 +1,16 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; import { parseGrammarRules } from "../src/grammarRuleParser.js"; import { writeGrammarRules } from "../src/grammarRuleWriter.js"; import { loadGrammarRules } from "../src/grammarLoader.js"; import { describeForEachMatcher } from "./testUtils.js"; +const testDir = dirname(fileURLToPath(import.meta.url)); + /** * Proposal (CurtisM): postfix ? / * / + are quantifiers only after ")" or ">". * Bare quantifiers are parse errors. Literal ? * + require escaping. @@ -40,6 +45,38 @@ describe("Quantifier special chars (? * +)", () => { ); }); + it("errors on $(x)* / $(x)+ with actionable group-form message", () => { + expectParseError( + ` = measure $(u:word)* -> "x";`, + /Capture \$\(\.\.\.\) only supports optional via \)\?/, + ); + expectParseError( + ` = measure $(u:word)+ -> "x";`, + /Use \(\$\(\.\.\.\)\)\+/, + ); + // Group form still works + expect(() => + parse(` = measure ($(u:word))+ -> "x";`), + ).not.toThrow(); + }); + + it("errors on bare the?/one?/music? (word + quantifier, no group)", () => { + // Historical silent-wrong forms that used to mean "optional word". + // Now they are hard parse errors; use (the)? / (one)? / (music)?. + expectParseError( + ` = pause the? music -> "x";`, + /Unexpected quantifier '\?'/, + ); + expectParseError( + ` = one? two -> "x";`, + /Unexpected quantifier '\?'/, + ); + expectParseError( + ` = pause (the)? music? -> "x";`, + /Unexpected quantifier '\?'/, + ); + }); + it("errors on bare '?' after a string group close that already consumed )?", () => { // After (the)? the next bare ? is illegal. expectParseError( @@ -301,6 +338,162 @@ describe("Quantifier special chars (? * +)", () => { ); expect(ast.definitions[0].rules[0].value).toBeDefined(); }); + + it("keeps optional chaining ?. and nullish coalescing in values", () => { + const ast = parse( + ` = lookup $(obj:word) -> obj.value?.name;`, + ); + expect(ast.definitions[0].rules[0].value).toBeDefined(); + const ast2 = parse( + ` = get $(x:word) -> x ?? "default";`, + ); + expect(ast2.definitions[0].rules[0].value).toBeDefined(); + }); + }); + + describe("import * and $(x)? still work", () => { + it("parses wildcard import * from without treating * as quantifier", () => { + const ast = parse( + `import * from "other.agr";\n = hi -> "x";`, + ); + expect(ast.imports).toHaveLength(1); + expect(ast.imports[0].names).toBe("*"); + expect(ast.imports[0].source).toBe("other.agr"); + expect(ast.definitions[0].definitionName.name).toBe("Start"); + }); + + it("keeps $(x)? optional capture", () => { + const ast = parse(` = measure $(units:word)? -> "cap";`); + const cap = ast.definitions[0].rules[0].expressions[1] as { + type: string; + optional?: boolean; + }; + expect(cap).toMatchObject({ type: "variable", optional: true }); + }); + }); + + describe("pitfall docs: bare ? is optional, no literal ?", () => { + it("parses who sings song ? as optional Song (no trailing ? string)", () => { + const ast = parse(` + = hello | goodbye; + = who sings song ? -> "hit"; + `); + const exprs = ast.definitions.find( + (d) => d.definitionName.name === "Start", + )!.rules[0].expressions; + expect(exprs).toHaveLength(2); + expect(exprs[0]).toMatchObject({ + type: "string", + value: ["who", "sings", "song"], + }); + expect(exprs[1]).toMatchObject({ + type: "ruleReference", + optional: true, + }); + // No third expression for a literal "?" + }); + }); + + describe("schema→grammar generator prompts use legal quantifiers", () => { + // LLM prompts that ship illegal bare ?/*/+ CORRECT examples cause the + // generator to emit unparseable .agr. Guard both package copies. + // Tests execute from dist/test/*.js — climb to package root / sibling package. + const promptSources = [ + join( + testDir, + "../../src/generation/schemaToGrammarGenerator.ts", + ), + join( + testDir, + "../../../agentSdkWrapper/src/schemaToGrammarGenerator.ts", + ), + ]; + + function loadPromptSource(path: string): string { + return readFileSync(path, "utf8"); + } + + /** Lines that teach CORRECT syntax (not WRONG counter-examples). */ + function correctLines(src: string): string[] { + return src + .split("\n") + .filter((l) => /^\s*CORRECT:/i.test(l) || /^\s*Example:/i.test(l)); + } + + it("CORRECT/Example lines never use bare quantifier after a word or string", () => { + // Illegal: word? 'str'? "str"? (quantifier not after ) or >) + const bareAfterAtom = + /(?:^|[^)>\s\\])(['"][^'"]*['"]|[A-Za-z_][\w-]*)\s*[?*+]/; + for (const path of promptSources) { + const src = loadPromptSource(path); + for (const line of correctLines(src)) { + expect(line).not.toMatch(bareAfterAtom); + } + } + }); + + it("both generators document quantifier special-char rules", () => { + for (const path of promptSources) { + const src = loadPromptSource(path); + expect(src).toMatch(/Quantifiers \? \* \+ are SPECIAL/i); + expect(src).toMatch(/immediately after "\)" or ">"/i); + expect(src).toMatch(/PARSE ERROR/); + expect(src).toMatch(/\(\)\?/); + // Shared Polite vocab must not use bare optional words + expect(src).toMatch( + /\s*=\s*can you\s*\|\s*please\s*\|\s*would you\s*;/, + ); + expect(src).toMatch(/\(\)\? open outlook/); + // Old illegal Polite forms must not appear outside WRONG lines + const nonWrong = src + .split("\n") + .filter((l) => !/\bWRONG\b/i.test(l)) + .join("\n"); + expect(nonWrong).not.toMatch(/'can you'\?/); + expect(nonWrong).not.toMatch(/'please'\?/); + expect(nonWrong).not.toMatch(/"please"\?/); + expect(nonWrong).not.toMatch(/Optional elements: element\?/); + } + }); + + it("literal-escape examples keep a real backslash in the runtime prompt", () => { + // Prompt bodies are TS template literals. A single \? in source + // collapses to bare "?" at runtime and teaches the silent pitfall + // (who sings song ? = optional Song). Source must use \\? + // so the model sees a real backslash-question sequence. + for (const path of promptSources) { + const src = loadPromptSource(path); + // File text must contain time\\? and \\? (two backslashes) + expect(src).toMatch(/time\\\\\?/); + expect(src).toMatch(/\\\\\?/); + expect(src).toMatch(/\)\?\\\\\?/); // ()?\? + // Simulate template evaluation of those escape sequences + expect(Function("return `time\\\\?`")()).toBe("time\\?"); + expect(Function("return `time\\?`")()).toBe("time?"); // the bug + } + }); + + it("prompt CORRECT fragments actually parse", () => { + // Fragments taught as CORRECT in both generators. + const fragments = [ + ` = (((can you)? add) | include) -> "x";`, + ` = (please)? open -> "x";`, + ` = (can you)? open -> "x";`, + ` = can you | please | would you;\n` + + ` = ()? open outlook -> "ok";`, + ` = hello;\n` + + ` = who sings song ? -> "opt";`, + ` = hello;\n` + + ` = who sings song \\? -> "req";`, + ` = hello;\n` + + ` = who sings song ()?\\? -> "both";`, + ` = (a|b)* c -> "x";`, + ` = measure $(x:word)? -> "cap";`, + ]; + for (const frag of fragments) { + expect(() => parse(frag)).not.toThrow(); + } + }); }); }); diff --git a/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts b/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts index a36190f83a..942c14782a 100644 --- a/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts +++ b/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts @@ -47,9 +47,9 @@ The Action Grammar format uses: - Rule definitions: = pattern; - Literal text: "play" or 'play' - Wildcards with types: $(name:Type) - captures any text and assigns it to 'name' with validation type 'Type' -- Optional elements: element? -- Zero or more: element* -- One or more: element+ +- Optional: (element)? or ? (quantifiers ? * + ONLY valid immediately after ")" or ">") +- Zero or more: (element)* or * +- One or more: (element)+ or + - Alternation: pattern1 | pattern2 - Grouping: (expression) - groups expressions for operators - Rule references: @@ -57,8 +57,9 @@ The Action Grammar format uses: CRITICAL SYNTAX RULES: 1. ALWAYS use parentheses around alternatives when combined with operators - CORRECT: ('can you'? 'add' | 'include') - WRONG: 'can you'? 'add' | 'include' + CORRECT: (((can you)? add) | include) + WRONG: can you? add | include (bare ? after a word is a PARSE ERROR) + WRONG: 'can you'? 'add' | 'include' (bare ? after a string is also a PARSE ERROR) 2. ALWAYS use parentheses around groups that should be treated as a unit CORRECT: ('on' | 'for') $(date:CalendarDate) @@ -81,12 +82,23 @@ CRITICAL SYNTAX RULES: WRONG: = ... ; for action "scheduleEvent" This enables easy targeting of specific actions when extending grammars incrementally. +6. Quantifiers ? * + are SPECIAL characters in patterns: + - Valid ONLY immediately after ")" or ">" : ()?, ?, $(x)?, (a|b)* + - Bare "?" after a word/string is a PARSE ERROR. Escape literals: what is the time\\? + - Required name + question mark: who sings song \\? + - Optional name + question mark: who sings song ()?\\? + - Do NOT write ? intending a literal "?"; that makes Polite optional. + - The writer/prettier prefers the grouped form ()? over bare ?. + CORRECT: (please)? (can you)? ()? ? $(x)? + WRONG: please? can you? 'can you'? "please"? word* + EFFICIENCY GUIDELINES: 1. Identify common patterns across actions and extract them as sub-rules Example: If multiple actions use date expressions, create = ('on' | 'for') $(date:CalendarDate); 2. Create shared vocabulary rules for common phrases - Example: = 'can you'? | 'please'? | 'would you'?; + Example: = can you | please | would you; + Then make the whole rule optional at use sites: ()? open outlook 3. Reuse entity type rules across actions Example: If multiple actions need participant names, reference the same wildcard pattern @@ -530,6 +542,11 @@ Remember the CRITICAL SYNTAX RULES: 1. ALWAYS use parentheses around alternatives when combined with operators 2. ALWAYS use parentheses around groups that should be treated as a unit 3. Optional groups must have parentheses +4. Quantifiers ? * + are SPECIAL — valid ONLY immediately after ")" or ">": + CORRECT: ()?, ?, $(x)?, (a|b)*, (please)?, (can you)? + WRONG: please? can you? 'can you'? "please"? word* (bare quantifier = PARSE ERROR) + Literal ? * + require backslash escapes: what is the time\\? + Prefer grouped form ()? over bare ?. Return the complete corrected grammar, starting with the copyright header.`; diff --git a/ts/packages/grammarTools/core/test/format.spec.ts b/ts/packages/grammarTools/core/test/format.spec.ts new file mode 100644 index 0000000000..ffbe75dd52 --- /dev/null +++ b/ts/packages/grammarTools/core/test/format.spec.ts @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { format } from "../src/format.js"; + +/** + * grammar-tools-core `format` is the AGR prettier used by the LSP and CLI. + * It parse→writeGrammarRules; quantified bare rule refs must become grouped. + */ +describe("format (AGR prettier)", () => { + it("returns unparseable input unchanged", () => { + const bad = `this is not valid <<<`; + expect(format(bad)).toBe(bad); + }); + + it("rewrites bare ?/*/+ to grouped ()?/*/+", () => { + const src = ` + = alice | bob; + = + show ? files -> "opt" + | tag * items -> "star" + | need + here -> "plus" + ; + `; + const out = format(src); + expect(out).toContain("()?"); + expect(out).toContain("()*"); + expect(out).toContain("()+"); + expect(out).not.toMatch(/\?/); + expect(out).not.toMatch(/\*/); + expect(out).not.toMatch(/\+/); + }); + + it("escapes literal ? * + on write-back", () => { + const out = format(` = what is the time\\? -> "q";`); + expect(out).toMatch(/time\\\?/); + // Re-format is stable + expect(format(out)).toBe(out); + }); + + it("keeps already-grouped quantifiers stable", () => { + const src = ` = (please)? open ()+ -> "ok";\n`; + // May re-indent; must retain grouped quantifiers and not invent bare ones. + const out = format(src); + expect(out).toContain(")?"); + expect(out).toContain(")+"); + expect(format(out)).toBe(out); + }); + + it("does not treat value-side ternary ? as a pattern quantifier", () => { + const src = ` = $(h:number) pm -> { hours: h < 12 ? h + 12 : h };`; + const out = format(src); + expect(out).toContain("?"); + expect(out).toContain("h + 12"); + // Must still parse (format didn't break value expr) + expect(format(out)).toBe(out); + }); +}); From 626fa0092bebb8538dc56d15c1ffbe8542a1c834 Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Mon, 3 Aug 2026 22:01:51 +0000 Subject: [PATCH 4/6] style: apply prettier formatting and policy fixes --- .../test/quantifierSpecialChars.spec.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts index 0d72d2a1c2..e41ec0ec42 100644 --- a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts +++ b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts @@ -344,9 +344,7 @@ describe("Quantifier special chars (? * +)", () => { ` = lookup $(obj:word) -> obj.value?.name;`, ); expect(ast.definitions[0].rules[0].value).toBeDefined(); - const ast2 = parse( - ` = get $(x:word) -> x ?? "default";`, - ); + const ast2 = parse(` = get $(x:word) -> x ?? "default";`); expect(ast2.definitions[0].rules[0].value).toBeDefined(); }); }); @@ -399,10 +397,7 @@ describe("Quantifier special chars (? * +)", () => { // generator to emit unparseable .agr. Guard both package copies. // Tests execute from dist/test/*.js — climb to package root / sibling package. const promptSources = [ - join( - testDir, - "../../src/generation/schemaToGrammarGenerator.ts", - ), + join(testDir, "../../src/generation/schemaToGrammarGenerator.ts"), join( testDir, "../../../agentSdkWrapper/src/schemaToGrammarGenerator.ts", @@ -417,7 +412,9 @@ describe("Quantifier special chars (? * +)", () => { function correctLines(src: string): string[] { return src .split("\n") - .filter((l) => /^\s*CORRECT:/i.test(l) || /^\s*Example:/i.test(l)); + .filter( + (l) => /^\s*CORRECT:/i.test(l) || /^\s*Example:/i.test(l), + ); } it("CORRECT/Example lines never use bare quantifier after a word or string", () => { From ad0c91072614ceec1907399a759d3428272e0b39 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Mon, 3 Aug 2026 16:16:56 -0700 Subject: [PATCH 5/6] fix(action-grammar): finish adversarial quantifier fix loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Match standalone \? on NFA/DFA by peeling trailing sentence punct - Grammar matcher: expand PhraseSetPart so bare ? works - Wire grammarStore DFA path with request context for punct peel - Extension schema→grammar prompts teach quantifier specials - scenarioBasedGenerator: escape ?/*/+ in shared verb categories - Expand quantifierSpecialChars NFA/DFA + prompt runtime guards --- ts/packages/actionGrammar/src/dfaMatcher.ts | 68 +++- .../src/generation/scenarioBasedGenerator.ts | 7 +- .../generation/schemaToGrammarGenerator.ts | 15 +- .../actionGrammar/src/grammarMatcher.ts | 115 ++++++ ts/packages/actionGrammar/src/index.ts | 1 + ts/packages/actionGrammar/src/nfaMatcher.ts | 158 ++++++++- .../nfaDfaStringPhraseSetVariable.spec.ts | 23 +- .../test/quantifierSpecialChars.spec.ts | 328 ++++++++++++++++-- ts/packages/actionGrammar/test/testUtils.ts | 2 + .../src/schemaToGrammarGenerator.ts | 15 +- ts/packages/cache/src/cache/grammarStore.ts | 8 +- 11 files changed, 695 insertions(+), 45 deletions(-) diff --git a/ts/packages/actionGrammar/src/dfaMatcher.ts b/ts/packages/actionGrammar/src/dfaMatcher.ts index 9cb85596b8..af52a99200 100644 --- a/ts/packages/actionGrammar/src/dfaMatcher.ts +++ b/ts/packages/actionGrammar/src/dfaMatcher.ts @@ -14,6 +14,7 @@ import { globalPhraseSetRegistry } from "./builtInPhraseMatchers.js"; import { normalizeToken, parseNumberToken, + tokenizeRequestKeepingTrailingPunct, tokenizeRequestWithOffsets, } from "./nfaMatcher.js"; import type { GrammarCompletionResult } from "./grammarCompletion.js"; @@ -720,12 +721,17 @@ function compareDFAMatchPriority(a: DFAMatchResult, b: DFAMatchResult): number { } /** - * Match tokens against a DFA, performing a two-pass split-candidate strategy - * when the DFA has split candidates (for spacing=optional/auto grammars). + * Match tokens against a DFA, performing a multi-pass strategy: * - * Pass 1 — original whitespace tokens. + * Pass 1 — original whitespace tokens (trailing sentence punct stripped). * Pass 2 — pre-split tokens using dfa.splitCandidates (e.g. "Swift's" → ["Swift", "'s"]). - * The higher-priority result is returned. + * Pass 3 — when `spacingContext.request` is set and earlier passes miss, retry + * with trailing sentence punctuation peeled into its own tokens so + * grammars with a standalone literal `\?` match natural questions + * like `"who sings song hello?"` (mirrors matchGrammarWithNFA). + * + * The higher-priority result across passes is returned. The strip pass is + * preferred when it matches (trailing punct as flex-space). */ export function matchDFAWithSplitting( dfa: DFA, @@ -739,8 +745,47 @@ export function matchDFAWithSplitting( * original request the matcher cannot distinguish " helloworld" from * "helloworld". Callers that have the request and grammar should * pass them; legacy callers can omit. + * + * When `request` is provided, a failed strip-tokenized match is retried + * with trailing punctuation kept as separate tokens (see Pass 3 above). */ - spacingContext?: { request: string; grammar: Grammar }, + spacingContext?: { request: string; grammar?: Grammar }, +): DFAMatchResult { + const best = matchDFAWithSplittingCore(dfa, tokens, debugMode); + + // Pass 3: peel glued trailing sentence punctuation (needs original request). + if (!best.matched && spacingContext?.request) { + const punctTokens = tokenizeRequestKeepingTrailingPunct( + spacingContext.request, + ); + if ( + punctTokens.length > 0 && + (punctTokens.length !== tokens.length || + punctTokens.some((t, i) => t !== tokens[i])) + ) { + const punctBest = matchDFAWithSplittingCore( + dfa, + punctTokens, + debugMode, + ); + if (punctBest.matched) { + return applySpacingNoneRejection( + punctBest, + punctTokens, + spacingContext, + ); + } + } + } + + return applySpacingNoneRejection(best, tokens, spacingContext); +} + +/** Split-candidate passes only (no trailing-punct retry, no spacing=none). */ +function matchDFAWithSplittingCore( + dfa: DFA, + tokens: string[], + debugMode: boolean, ): DFAMatchResult { // O(1) first-token pre-filter if (dfaFirstTokenRejects(dfa, tokens)) { @@ -776,10 +821,21 @@ export function matchDFAWithSplitting( } } } + return best; +} +function applySpacingNoneRejection( + best: DFAMatchResult, + tokens: string[], + spacingContext?: { request: string; grammar?: Grammar }, +): DFAMatchResult { // spacing=none rejection of leading/trailing whitespace. Mirrors the // check in matchGrammarWithNFA (nfaMatcher.ts). - if (best.matched && spacingContext && best.ruleIndex !== undefined) { + if ( + best.matched && + spacingContext?.grammar && + best.ruleIndex !== undefined + ) { const { request, grammar } = spacingContext; const hasOuterWhitespace = request.length !== request.trim().length && diff --git a/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts b/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts index e23525e1a9..eab996d8c5 100644 --- a/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts +++ b/ts/packages/actionGrammar/src/generation/scenarioBasedGenerator.ts @@ -589,10 +589,9 @@ export class ScenarioBasedGrammarGenerator { output += `<${categoryName}> =\n`; verbsList.forEach((verb, index) => { const separator = index < verbsList.length - 1 ? " |" : ""; - // Escape backslashes first, then single quotes in verb phrases - const escapedVerb = verb - .replace(/\\/g, "\\\\") - .replace(/'/g, "\\'"); + // Same escapes as generatePrefixSuffixRule — quotes are + // ordinary match chars, so bare ?/*/+ inside would parse-error. + const escapedVerb = this.escapeSpecialChars(verb); output += ` '${escapedVerb}'${separator}\n`; }); output += `\n`; diff --git a/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts b/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts index 99baa7e66e..711e5b799a 100644 --- a/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts +++ b/ts/packages/actionGrammar/src/generation/schemaToGrammarGenerator.ts @@ -211,6 +211,19 @@ IMPROVEMENT INSTRUCTIONS: AVAILABLE ENTITY TYPES AND CONVERTERS: {entityTypes} +CRITICAL SYNTAX RULES (must follow when extending): +1. Quantifiers ? * + are SPECIAL characters in patterns: + - Valid ONLY immediately after ")" or ">" : ()?, ?, $(x)?, (a|b)* + - Bare "?" after a word is a PARSE ERROR. Escape literals: what is the time\\? + - Required name + question mark: who sings song \\? + - Optional name + question mark: who sings song ()?\\? + - Do NOT write ? intending a literal "?"; that makes Polite optional. + - The writer/prettier prefers the grouped form ()? over bare ?. + CORRECT: (please)? (can you)? ()? ? $(x)? + WRONG: please? can you? 'can you'? "please"? word* +2. Comments use // not # +3. Action rule names MUST match the exact action name (not capitalized) + Your task: 1. Analyze the existing grammar and identify areas for improvement 2. Incorporate the new examples by extending or refining existing rules @@ -218,7 +231,7 @@ Your task: 4. Maintain consistency with existing patterns and style 5. Ensure all actions in the schema are covered 6. Keep shared sub-rules and don't duplicate patterns -7. Follow all AGR syntax rules (see above) +7. Follow all AGR syntax rules above (especially quantifier special-char rules) 8. IMPORTANT: Use exact action names for action rules (e.g., = ... ;, not = ... ;) This enables easy targeting of specific actions when extending grammars incrementally diff --git a/ts/packages/actionGrammar/src/grammarMatcher.ts b/ts/packages/actionGrammar/src/grammarMatcher.ts index 06f99324a9..c19d93af81 100644 --- a/ts/packages/actionGrammar/src/grammarMatcher.ts +++ b/ts/packages/actionGrammar/src/grammarMatcher.ts @@ -10,6 +10,7 @@ import { Grammar, GrammarPart, GrammarRule, + PhraseSetPart, RulesPart, StringPart, StringPartRegExpEntry, @@ -17,6 +18,7 @@ import { VarStringPart, DispatchModeBucket, createRulesPart, + createStringPart, } from "./grammarTypes.js"; import { wordBoundaryScriptRe } from "./spacingScripts.js"; import { @@ -24,6 +26,7 @@ import { getDispatchMergedSingle, getDispatchMergedMulti, } from "./dispatchHelpers.js"; +import { globalPhraseSetRegistry } from "./builtInPhraseMatchers.js"; import type { TraceCallback } from "./traceEvents.js"; // Separator mode for completion results. Structurally identical to @@ -2752,6 +2755,67 @@ function matchVarStringPart(state: MatchState, part: VarStringPart) { return true; } +/** + * Expand a PhraseSetPart into a RulesPart whose alternatives are fixed + * string phrases from the registry. Cached on the part so repeated + * match attempts (and multi-request reuse of a compiled grammar) do not + * rebuild the alternation. Phrase-set registry growth via addPhrase() + * invalidates the cache by phrase count. + * + * Nested rules inherit the surrounding spacingMode so flex-space / + * boundary behavior matches a hand-written alternation of the same + * tokens. Capture (`part.variable`) is carried on the RulesPart so + * finalizeNestedRule binds the matched phrase the same way NFA does. + */ +type PhraseSetExpandCache = { + rulesPart: RulesPart; + phraseCount: number; + spacingMode: CompiledSpacingMode; + variable: string | undefined; +}; +const phraseSetExpandCache = new WeakMap(); + +function getPhraseSetRulesPart( + part: PhraseSetPart, + spacingMode: CompiledSpacingMode, +): RulesPart | undefined { + const matcher = globalPhraseSetRegistry.getMatcher(part.matcherName); + if (matcher === undefined || matcher.phrases.length === 0) { + return undefined; + } + const cached = phraseSetExpandCache.get(part); + if ( + cached !== undefined && + cached.phraseCount === matcher.phrases.length && + cached.spacingMode === spacingMode && + cached.variable === part.variable + ) { + return cached.rulesPart; + } + const rulesPart = createRulesPart( + matcher.phrases.map((phrase) => ({ + // Slice so later registry mutations cannot alias into the + // compiled string part's immutable token array. + parts: [createStringPart(phrase.slice())], + spacingMode, + })), + { + variable: part.variable, + partId: part.partId, + name: part.matcherName, + // Phrase-set expansions are pure string alternations — memo is + // fine and helps when the same appears in many rules. + }, + ); + phraseSetExpandCache.set(part, { + rulesPart, + phraseCount: matcher.phrases.length, + spacingMode, + variable: part.variable, + }); + return rulesPart; +} + // Enter a tail `RulesPart` (true tail call). No parent frame is // pushed - `state.parent` keeps pointing at whatever ancestor frame // was already current. When the selected member finishes, finalize @@ -3053,6 +3117,57 @@ export function matchState(state: MatchState, request: string) { // continue the loop (without incrementing partIndex) continue; } + case "phraseSet": { + // Built-in phrase sets (, , …) compile to + // PhraseSetPart. Expand to a string-phrase alternation and + // reuse the RulesPart entry path (backtrack + capture). + const rulesPart = getPhraseSetRulesPart( + part, + state.spacingMode, + ); + if (rulesPart === undefined) { + if (trace !== undefined) { + trace({ + seq: state.traceSeq++, + inputPos: state.index, + kind: "partFailed", + rule: state.name, + part: part.partId ?? partIndex, + }); + } + return false; + } + if (debugEnabled) { + debugMatch( + state, + `expanding phraseSet <${part.matcherName}> (${rulesPart.alternatives.length} phrases)`, + ); + } + const namePrefix = state.trackNames + ? `<${part.matcherName}>` + : ""; + if ( + !enterRulesAlternation( + state, + rulesPart, + rulesPart.alternatives, + namePrefix, + ) + ) { + if (trace !== undefined) { + trace({ + seq: state.traceSeq++, + inputPos: state.index, + kind: "partFailed", + rule: state.name, + part: part.partId ?? partIndex, + }); + } + return false; + } + // continue the loop (without incrementing partIndex) + continue; + } } if (trace !== undefined) { // Wildcard parts defer value capture (the extent isn't diff --git a/ts/packages/actionGrammar/src/index.ts b/ts/packages/actionGrammar/src/index.ts index df6180cfd1..dacbe55913 100644 --- a/ts/packages/actionGrammar/src/index.ts +++ b/ts/packages/actionGrammar/src/index.ts @@ -174,6 +174,7 @@ export { export { matchGrammarWithNFA, tokenizeRequest, + tokenizeRequestKeepingTrailingPunct, normalizeToken, type NFAGrammarMatchResult, } from "./nfaMatcher.js"; diff --git a/ts/packages/actionGrammar/src/nfaMatcher.ts b/ts/packages/actionGrammar/src/nfaMatcher.ts index 17cfee68af..36e4f9b35a 100644 --- a/ts/packages/actionGrammar/src/nfaMatcher.ts +++ b/ts/packages/actionGrammar/src/nfaMatcher.ts @@ -44,6 +44,24 @@ export interface NFAGrammarMatchResult { entityWildcardPropertyNames: string[]; } +/** + * Sentence-level trailing punctuation peeled/stripped from word tokens. + * Kept as a shared constant so strip and peel stay in lockstep. + */ +const TRAILING_PUNCTUATION = "?!.,;:"; + +/** + * Find the index where trailing sentence punctuation begins on a token. + * Returns `token.length` when there is none; `0` when the token is all punct. + */ +function trailingPunctuationStart(token: string): number { + let end = token.length; + while (end > 0 && TRAILING_PUNCTUATION.includes(token[end - 1])) { + end--; + } + return end; +} + /** * Strip trailing punctuation from a token (linear time). * @@ -54,11 +72,7 @@ export interface NFAGrammarMatchResult { * normalize to just `done`. */ function stripTrailingPunctuation(token: string): string { - const punctuation = "?!.,;:"; - let end = token.length; - while (end > 0 && punctuation.includes(token[end - 1])) { - end--; - } + const end = trailingPunctuationStart(token); if (end === 0) { // Token is all punctuation — return as-is. return token; @@ -114,6 +128,13 @@ export function parseNumberToken(token: string): number | undefined { * original case so that wildcard captures retain the user's casing. * Normalization (lowercasing) for fixed-token comparisons is done * separately at match time via normalizeToken(). + * + * Trailing sentence punctuation on word tokens is discarded here so that + * natural utterances like `"pause?"` still match a grammar written without + * an explicit `?` (canonical treats leftover punct as flex-space). When the + * grammar *requires* a standalone literal `?` / `!` / etc. (e.g. + * `who sings song \?`), {@link matchGrammarWithNFA} retries with + * {@link tokenizeRequestKeepingTrailingPunct}. */ export function tokenizeRequest(request: string): string[] { return request @@ -123,6 +144,27 @@ export function tokenizeRequest(request: string): string[] { .filter((token) => token.length > 0); } +/** + * Tokenize like {@link tokenizeRequest}, but peel trailing sentence + * punctuation off word-bearing tokens into their own tokens instead of + * discarding it. + * + * Examples: + * `"hello?"` → `["hello", "?"]` + * `"time!?"` → `["time", "!?"]` + * `"?"` → `["?"]` (all-punct tokens stay intact) + * + * Used as a fallback pass so grammars with a standalone escaped literal + * (`\?`, `\!`, …) after a word/rule can match natural glued questions + * (`"who sings song hello?"`) the same way the char-based canonical matcher + * does. Callers that already have a successful strip-tokenized match should + * prefer that result (trailing punct as flex-space). + */ +export function tokenizeRequestKeepingTrailingPunct(request: string): string[] { + const { tokens } = tokenizeRequestWithOffsetsKeepingTrailingPunct(request); + return tokens; +} + /** * Tokenize a request string while also recording each token's character * offsets in the original (untrimmed) input. Used by completion to compute @@ -158,6 +200,55 @@ export function tokenizeRequestWithOffsets(request: string): { return { tokens, starts, ends }; } +/** + * Offset-aware variant of {@link tokenizeRequestKeepingTrailingPunct}. + * Peeled punctuation becomes its own token with offsets covering only the + * punct run; the base word's `ends` stops before the punct. + */ +export function tokenizeRequestWithOffsetsKeepingTrailingPunct(request: string): { + tokens: string[]; + starts: number[]; + ends: number[]; +} { + const tokens: string[] = []; + const starts: number[] = []; + const ends: number[] = []; + const re = /\S+/g; + for (const m of request.matchAll(re)) { + const raw = m[0]; + const start = m.index ?? 0; + const punctAt = trailingPunctuationStart(raw); + if (punctAt === 0) { + // All punctuation — keep as a single token. + tokens.push(raw); + starts.push(start); + ends.push(start + raw.length); + } else if (punctAt === raw.length) { + tokens.push(raw); + starts.push(start); + ends.push(start + raw.length); + } else { + // Word + trailing punct → two tokens. + tokens.push(raw.slice(0, punctAt)); + starts.push(start); + ends.push(start + punctAt); + tokens.push(raw.slice(punctAt)); + starts.push(start + punctAt); + ends.push(start + raw.length); + } + } + return { tokens, starts, ends }; +} + +/** True when two token arrays are identical (same length and elements). */ +function tokenArraysEqual(a: string[], b: string[]): boolean { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) return false; + } + return true; +} + // --------------------------------------------------------------------------- // Token pre-splitting for optional/auto spacing mode // --------------------------------------------------------------------------- @@ -298,6 +389,63 @@ export function matchGrammarWithNFA( } } + // Pass 3: peel glued trailing sentence punctuation into its own tokens. + // Default tokenization strips `"hello?"` → `"hello"` so incidental + // trailing punct acts like flex-space (canonical finalizeState). That + // discards a standalone grammar literal `\?` after a word/rule, so + // `"who sings song hello?"` fails against `who sings song \?`. + // Retry with peeled tokens only when earlier passes missed — prefer the + // strip pass when both would match (trailing punct as flex-space). + if (!bestResult.matched) { + const punctOffsets = + tokenizeRequestWithOffsetsKeepingTrailingPunct(request); + const punctTokens = punctOffsets.tokens; + if ( + punctTokens.length > 0 && + !tokenArraysEqual(punctTokens, tokens) + ) { + debug( + `Trailing-punct tokens: [${punctTokens.join(", ")}] (${punctTokens.length} tokens)`, + ); + const punctCtx = { + request, + starts: punctOffsets.starts, + ends: punctOffsets.ends, + }; + let punctBest = matchNFAWithIndex( + nfa, + index, + punctTokens, + false, + punctCtx, + ); + const punctSplit = applySplitToTokens( + punctTokens, + splitCandidates, + ); + if (punctSplit !== null) { + const punctSplitResult = matchNFAWithIndex( + nfa, + index, + punctSplit, + ); + if (punctSplitResult.matched) { + if (!punctBest.matched) { + punctBest = punctSplitResult; + } else { + [punctBest] = sortNFAMatches([ + punctBest, + punctSplitResult, + ]); + } + } + } + if (punctBest.matched) { + bestResult = punctBest; + } + } + } + if (!bestResult.matched) { debug(`Match result: NO MATCH`); return []; diff --git a/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts b/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts index 8bc1847517..b1ef46edf5 100644 --- a/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts +++ b/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts @@ -10,14 +10,11 @@ * code emits these forms. * * Coverage: - * - matchGrammar (interpreter): StringPart capture (single + multi-token) + * - matchGrammar (interpreter): StringPart + PhraseSetPart capture * - matchGrammarWithNFA: StringPart and PhraseSetPart capture * - matchDFAWithSplitting: StringPart and PhraseSetPart capture (delegates * to matchNFA via dfa.sourceNFA) * - JSON round-trip: variable preserved through serialize/deserialize - * - * grammarMatcher.ts has no PhraseSetPart match path — phraseSet capture is - * exercised only against the NFA / DFA matchers. */ import { matchGrammar } from "../src/grammarMatcher.js"; @@ -187,6 +184,24 @@ describe("PhraseSetPart variable capture", () => { ], }; + function bestGrammarActionValue( + g: Grammar, + request: string, + ): unknown { + const results = matchGrammar(g, request); + if (results.length === 0) return undefined; + return results[0].match; + } + + it("grammar matcher: PhraseSetPart binds matched phrase tokens joined", () => { + expect(bestGrammarActionValue(grammar, "please go")).toStrictEqual({ + opener: "please", + }); + expect(bestGrammarActionValue(grammar, "could you go")).toStrictEqual({ + opener: "could you", + }); + }); + it("NFA: PhraseSetPart binds matched phrase tokens joined", () => { // "Polite" set includes "please", "could you", "would you", etc. expect(bestNfaActionValue(grammar, "please go")).toStrictEqual({ diff --git a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts index e41ec0ec42..ab9760d6a7 100644 --- a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts +++ b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts @@ -7,6 +7,14 @@ import { fileURLToPath } from "node:url"; import { parseGrammarRules } from "../src/grammarRuleParser.js"; import { writeGrammarRules } from "../src/grammarRuleWriter.js"; import { loadGrammarRules } from "../src/grammarLoader.js"; +import { compileGrammarToNFA } from "../src/nfaCompiler.js"; +import { compileNFAToDFA } from "../src/dfaCompiler.js"; +import { + matchGrammarWithNFA, + tokenizeRequest, + tokenizeRequestKeepingTrailingPunct, +} from "../src/nfaMatcher.js"; +import { matchDFAWithSplitting } from "../src/dfaMatcher.js"; import { describeForEachMatcher } from "./testUtils.js"; const testDir = dirname(fileURLToPath(import.meta.url)); @@ -408,6 +416,42 @@ describe("Quantifier special chars (? * +)", () => { return readFileSync(path, "utf8"); } + /** + * Evaluate schema→grammar prompt template literals the way Node does + * at runtime. Source text checks alone miss `\?` → `?` + * NonEscapeCharacter collapse. Includes EXTENSION (used when + * generateGrammar({ existingGrammar }) is selected alone) and FIX. + */ + function loadRuntimePrompts(path: string): string[] { + const src = loadPromptSource(path); + const prompts: string[] = []; + const re = + /const\s+(SCHEMA_GRAMMAR_PROMPT|SCHEMA_GRAMMAR_EXTENSION_PROMPT|FIX_GRAMMAR_PROMPT)\s*=\s*(`[\s\S]*?`);/g; + let m: RegExpExecArray | null; + while ((m = re.exec(src)) !== null) { + prompts.push(Function(`return ${m[2]}`)() as string); + } + if (prompts.length === 0) { + throw new Error(`No prompt templates found in ${path}`); + } + return prompts; + } + + function loadRuntimePromptByName( + path: string, + name: string, + ): string { + const src = loadPromptSource(path); + const re = new RegExp( + `const\\s+${name}\\s*=\\s*(\`[\\s\\S]*?\`);`, + ); + const m = re.exec(src); + if (!m) { + throw new Error(`${name} not found in ${path}`); + } + return Function(`return ${m[1]}`)() as string; + } + /** Lines that teach CORRECT syntax (not WRONG counter-examples). */ function correctLines(src: string): string[] { return src @@ -419,37 +463,68 @@ describe("Quantifier special chars (? * +)", () => { it("CORRECT/Example lines never use bare quantifier after a word or string", () => { // Illegal: word? 'str'? "str"? (quantifier not after ) or >) + // Check RUNTIME prompt text (not just source) so a single-backslash + // template bug that collapses \\? → ? is caught. const bareAfterAtom = /(?:^|[^)>\s\\])(['"][^'"]*['"]|[A-Za-z_][\w-]*)\s*[?*+]/; for (const path of promptSources) { - const src = loadPromptSource(path); - for (const line of correctLines(src)) { - expect(line).not.toMatch(bareAfterAtom); + for (const prompt of loadRuntimePrompts(path)) { + for (const line of correctLines(prompt)) { + expect(line).not.toMatch(bareAfterAtom); + } } } }); it("both generators document quantifier special-char rules", () => { for (const path of promptSources) { + for (const prompt of loadRuntimePrompts(path)) { + expect(prompt).toMatch(/Quantifiers \? \* \+ are SPECIAL/i); + expect(prompt).toMatch(/immediately after "\)" or ">"/i); + expect(prompt).toMatch(/PARSE ERROR/); + expect(prompt).toMatch(/\(\)\?/); + // Old illegal Polite forms must not appear outside WRONG lines + const nonWrong = prompt + .split("\n") + .filter((l) => !/\bWRONG\b/i.test(l)) + .join("\n"); + expect(nonWrong).not.toMatch(/'can you'\?/); + expect(nonWrong).not.toMatch(/'please'\?/); + expect(nonWrong).not.toMatch(/"please"\?/); + expect(nonWrong).not.toMatch(/Optional elements: element\?/); + } + // Main SCHEMA prompt also teaches shared Polite vocab + const main = loadRuntimePromptByName( + path, + "SCHEMA_GRAMMAR_PROMPT", + ); + expect(main).toMatch( + /\s*=\s*can you\s*\|\s*please\s*\|\s*would you\s*;/, + ); + expect(main).toMatch(/\(\)\? open outlook/); + } + }); + + it("EXTENSION prompt documents quantifier rules (existingGrammar path)", () => { + // generateGrammar({ existingGrammar }) uses EXTENSION alone — + // it must not rely on "see above" for quantifier specials. + for (const path of promptSources) { + const ext = loadRuntimePromptByName( + path, + "SCHEMA_GRAMMAR_EXTENSION_PROMPT", + ); + expect(ext).toMatch(/Quantifiers \? \* \+ are SPECIAL/i); + expect(ext).toMatch(/immediately after "\)" or ">"/i); + expect(ext).toMatch(/PARSE ERROR/); + expect(ext).toContain("time\\?"); + expect(ext).toContain("\\?"); + expect(ext).toContain(")?\\?"); + expect(ext).not.toContain("()??"); + // Source must use double-backslash so runtime keeps \ const src = loadPromptSource(path); - expect(src).toMatch(/Quantifiers \? \* \+ are SPECIAL/i); - expect(src).toMatch(/immediately after "\)" or ">"/i); - expect(src).toMatch(/PARSE ERROR/); - expect(src).toMatch(/\(\)\?/); - // Shared Polite vocab must not use bare optional words expect(src).toMatch( - /\s*=\s*can you\s*\|\s*please\s*\|\s*would you\s*;/, + /SCHEMA_GRAMMAR_EXTENSION_PROMPT[\s\S]*?time\\\\\?/, ); - expect(src).toMatch(/\(\)\? open outlook/); - // Old illegal Polite forms must not appear outside WRONG lines - const nonWrong = src - .split("\n") - .filter((l) => !/\bWRONG\b/i.test(l)) - .join("\n"); - expect(nonWrong).not.toMatch(/'can you'\?/); - expect(nonWrong).not.toMatch(/'please'\?/); - expect(nonWrong).not.toMatch(/"please"\?/); - expect(nonWrong).not.toMatch(/Optional elements: element\?/); } }); @@ -464,9 +539,16 @@ describe("Quantifier special chars (? * +)", () => { expect(src).toMatch(/time\\\\\?/); expect(src).toMatch(/\\\\\?/); expect(src).toMatch(/\)\?\\\\\?/); // ()?\? - // Simulate template evaluation of those escape sequences - expect(Function("return `time\\\\?`")()).toBe("time\\?"); - expect(Function("return `time\\?`")()).toBe("time?"); // the bug + // Runtime prompt must still contain a real backslash before ? + for (const prompt of loadRuntimePrompts(path)) { + expect(prompt).toContain("time\\?"); + expect(prompt).toContain("\\?"); + expect(prompt).toContain(")?\\?"); + // Collapsed pitfall forms must NOT appear as the taught escape + expect(prompt).not.toMatch(/time\?(?!\\)/); + // ()?? would be the collapsed form of ()?\? + expect(prompt).not.toContain("()??"); + } } }); @@ -486,6 +568,8 @@ describe("Quantifier special chars (? * +)", () => { ` = who sings song ()?\\? -> "both";`, ` = (a|b)* c -> "x";`, ` = measure $(x:word)? -> "cap";`, + // Group-form repetition of captures (bare $(x)* is rejected) + ` = add ($(item:word))+ to list -> "x";`, ]; for (const frag of fragments) { expect(() => parse(frag)).not.toThrow(); @@ -603,7 +687,7 @@ describeForEachMatcher( expect(testMatchGrammar(g, "what is the time")).toStrictEqual([]); }); - it("? open app — optional polite prefix", () => { + it("? open app — optional polite prefix (local rule)", () => { const g = loadGrammarRules( "polite.agr", ` @@ -621,6 +705,44 @@ describeForEachMatcher( expect(testMatchGrammar(g, "open notepad")).toStrictEqual([]); }); + it("built-in phrase-set ? (no local rule) consumes polite phrases", () => { + // Unbound compiles to PhraseSetPart. Canonical matcher + // must consume the phrase (not skip the part) — otherwise + // "please open outlook" fails while bare "open outlook" works. + const g = loadGrammarRules( + "builtin-polite.agr", + ` = ? open outlook -> "ok";`, + ); + expect(testMatchGrammar(g, "open outlook")).toStrictEqual(["ok"]); + expect(testMatchGrammar(g, "please open outlook")).toStrictEqual([ + "ok", + ]); + expect(testMatchGrammar(g, "can you open outlook")).toStrictEqual([ + "ok", + ]); + expect( + testMatchGrammar(g, "could you open outlook"), + ).toStrictEqual(["ok"]); + expect( + testMatchGrammar(g, "would you please open outlook"), + ).toStrictEqual(["ok"]); + expect(testMatchGrammar(g, "open notepad")).toStrictEqual([]); + }); + + it("grouped ($(item:word))+ accepts repeated captures", () => { + const g = loadGrammarRules( + "cap-plus.agr", + ` = add ($(item:word))+ to list -> "x";`, + ); + expect(testMatchGrammar(g, "add milk to list")).toStrictEqual([ + "x", + ]); + expect( + testMatchGrammar(g, "add milk eggs to list"), + ).toStrictEqual(["x"]); + expect(testMatchGrammar(g, "add to list")).toStrictEqual([]); + }); + it("bare * / + match zero-or-more / one-or-more", () => { const star = loadGrammarRules( "star.agr", @@ -664,3 +786,163 @@ describeForEachMatcher( }); }, ); + +/** + * NFA/DFA always-on guards for trailing literal `?`. + * describeForEachMatcher defaults to grammar-only in CI; these lock the + * token peel fallback that lets standalone `\?` match glued questions. + */ +describe("Quantifier special chars — NFA/DFA trailing literal ?", () => { + const songQSrc = ` + = hello | goodbye; + = who sings song \\? -> "hit"; + `; + const optSongQSrc = ` + = hello | goodbye; + = who sings song ()?\\? -> "hit"; + `; + const timeSepSrc = ` = what is the time \\? -> "q";`; + const timeGluedSrc = ` = what is the time\\? -> "q";`; + + it("tokenizeRequest strips trailing ?; KeepingTrailingPunct peels it", () => { + expect(tokenizeRequest("who sings song hello?")).toEqual([ + "who", + "sings", + "song", + "hello", + ]); + expect( + tokenizeRequestKeepingTrailingPunct("who sings song hello?"), + ).toEqual(["who", "sings", "song", "hello", "?"]); + // All-punct tokens stay intact under both modes + expect(tokenizeRequest("?")).toEqual(["?"]); + expect(tokenizeRequestKeepingTrailingPunct("?")).toEqual(["?"]); + }); + + it("NFA matches required Song + \\? on glued hello?", () => { + const g = loadGrammarRules("nfa-q.agr", songQSrc); + const nfa = compileGrammarToNFA(g); + expect( + matchGrammarWithNFA(g, nfa, "who sings song hello?").map( + (m) => m.match, + ), + ).toEqual(["hit"]); + expect( + matchGrammarWithNFA(g, nfa, "who sings song goodbye?").map( + (m) => m.match, + ), + ).toEqual(["hit"]); + expect( + matchGrammarWithNFA(g, nfa, "who sings song hello").map( + (m) => m.match, + ), + ).toEqual([]); + expect( + matchGrammarWithNFA(g, nfa, "who sings song?").map((m) => m.match), + ).toEqual([]); + }); + + it("DFA matches required Song + \\? on glued hello?", () => { + const g = loadGrammarRules("dfa-q.agr", songQSrc); + const nfa = compileGrammarToNFA(g); + const dfa = compileNFAToDFA(nfa); + const tokens = tokenizeRequest("who sings song hello?"); + const hit = matchDFAWithSplitting(dfa, tokens, false, { + request: "who sings song hello?", + grammar: g, + }); + expect(hit.matched).toBe(true); + expect(hit.actionValue).toBe("hit"); + + const miss = matchDFAWithSplitting( + dfa, + tokenizeRequest("who sings song hello"), + false, + { request: "who sings song hello", grammar: g }, + ); + expect(miss.matched).toBe(false); + + // Without request context, strip-only tokens cannot see the glued ? + const noCtx = matchDFAWithSplitting(dfa, tokens, false); + expect(noCtx.matched).toBe(false); + }); + + it("NFA/DFA optional Song + \\? matches who sings song? and hello?", () => { + const g = loadGrammarRules("opt-q.agr", optSongQSrc); + const nfa = compileGrammarToNFA(g); + const dfa = compileNFAToDFA(nfa); + for (const req of [ + "who sings song?", + "who sings song hello?", + "who sings song ?", + ]) { + expect( + matchGrammarWithNFA(g, nfa, req).map((m) => m.match), + ).toEqual(["hit"]); + const dfaHit = matchDFAWithSplitting( + dfa, + tokenizeRequest(req), + false, + { request: req, grammar: g }, + ); + expect(dfaHit.matched).toBe(true); + expect(dfaHit.actionValue).toBe("hit"); + } + }); + + it("NFA matches separate time \\? on glued time?", () => { + const g = loadGrammarRules("time-sep.agr", timeSepSrc); + const nfa = compileGrammarToNFA(g); + expect( + matchGrammarWithNFA(g, nfa, "what is the time?").map( + (m) => m.match, + ), + ).toEqual(["q"]); + expect( + matchGrammarWithNFA(g, nfa, "what is the time ?").map( + (m) => m.match, + ), + ).toEqual(["q"]); + expect( + matchGrammarWithNFA(g, nfa, "what is the time").map( + (m) => m.match, + ), + ).toEqual([]); + }); + + it("NFA still matches glued time\\? (display-punct path, strip pass)", () => { + const g = loadGrammarRules("time-glued.agr", timeGluedSrc); + const nfa = compileGrammarToNFA(g); + expect( + matchGrammarWithNFA(g, nfa, "what is the time?").map( + (m) => m.match, + ), + ).toEqual(["q"]); + // Space-separated ? is not the glued form + expect( + matchGrammarWithNFA(g, nfa, "what is the time ?").map( + (m) => m.match, + ), + ).toEqual([]); + }); + + it("NFA strip pass still treats trailing ? as flex-space when grammar has no literal ?", () => { + const g = loadGrammarRules( + "flex.agr", + ` + = hello | goodbye; + = who sings song ? -> "hit"; + `, + ); + const nfa = compileGrammarToNFA(g); + // optional Song, no literal ? — "hello?" strips to hello and matches + expect( + matchGrammarWithNFA(g, nfa, "who sings song hello?").map( + (m) => m.match, + ), + ).toEqual(["hit"]); + expect( + matchGrammarWithNFA(g, nfa, "who sings song").map((m) => m.match), + ).toEqual(["hit"]); + }); +}); diff --git a/ts/packages/actionGrammar/test/testUtils.ts b/ts/packages/actionGrammar/test/testUtils.ts index f055d4fbb9..243405f533 100644 --- a/ts/packages/actionGrammar/test/testUtils.ts +++ b/ts/packages/actionGrammar/test/testUtils.ts @@ -52,6 +52,8 @@ function testMatchGrammarDFA(grammar: Grammar, request: string): unknown[] { const nfa = compileGrammarToNFA(grammar, "test.grammar"); const dfa = compileNFAToDFA(nfa, "test.grammar"); const tokens = tokenizeRequest(request); + // spacingContext.request also enables the trailing-punct peel retry so + // standalone grammar `\?` matches natural glued questions (hello?). const result = matchDFAWithSplitting(dfa, tokens, false, { request, grammar, diff --git a/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts b/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts index 942c14782a..0e6267c975 100644 --- a/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts +++ b/ts/packages/agentSdkWrapper/src/schemaToGrammarGenerator.ts @@ -154,6 +154,19 @@ IMPROVEMENT INSTRUCTIONS: AVAILABLE ENTITY TYPES AND CONVERTERS: {entityTypes} +CRITICAL SYNTAX RULES (must follow when extending): +1. Quantifiers ? * + are SPECIAL characters in patterns: + - Valid ONLY immediately after ")" or ">" : ()?, ?, $(x)?, (a|b)* + - Bare "?" after a word is a PARSE ERROR. Escape literals: what is the time\\? + - Required name + question mark: who sings song \\? + - Optional name + question mark: who sings song ()?\\? + - Do NOT write ? intending a literal "?"; that makes Polite optional. + - The writer/prettier prefers the grouped form ()? over bare ?. + CORRECT: (please)? (can you)? ()? ? $(x)? + WRONG: please? can you? 'can you'? "please"? word* +2. Comments use // not # +3. Action rule names MUST match the exact action name (not capitalized) + Your task: 1. Analyze the existing grammar and identify areas for improvement 2. Incorporate the new examples by extending or refining existing rules @@ -161,7 +174,7 @@ Your task: 4. Maintain consistency with existing patterns and style 5. Ensure all actions in the schema are covered 6. Keep shared sub-rules and don't duplicate patterns -7. Follow all AGR syntax rules (see above) +7. Follow all AGR syntax rules above (especially quantifier special-char rules) 8. IMPORTANT: Use exact action names for action rules (e.g., = ... ;, not = ... ;) This enables easy targeting of specific actions when extending grammars incrementally diff --git a/ts/packages/cache/src/cache/grammarStore.ts b/ts/packages/cache/src/cache/grammarStore.ts index ae81f4508e..83b80c850f 100644 --- a/ts/packages/cache/src/cache/grammarStore.ts +++ b/ts/packages/cache/src/cache/grammarStore.ts @@ -210,7 +210,13 @@ export class GrammarStoreImpl implements GrammarStore { let grammarMatches; if (this.useDFA && entry.dfa) { const tokens = tokenizeRequest(request); - const dfaResult = matchDFAWithSplitting(entry.dfa, tokens); + // Pass request so matchDFAWithSplitting can retry with trailing + // sentence punctuation peeled into its own tokens (e.g. hello? + // against a grammar ending in standalone \?). + const dfaResult = matchDFAWithSplitting(entry.dfa, tokens, false, { + request, + grammar: entry.grammar, + }); grammarMatches = dfaResult.matched ? [ { From 37a4d5ba99356c682c69161afda4ee96a6432b5f Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Mon, 3 Aug 2026 23:20:24 +0000 Subject: [PATCH 6/6] style: apply prettier formatting and policy fixes --- ts/packages/actionGrammar/src/nfaMatcher.ts | 14 ++++----- .../nfaDfaStringPhraseSetVariable.spec.ts | 5 +--- .../test/quantifierSpecialChars.spec.ts | 29 ++++++++----------- ts/packages/cache/src/cache/grammarStore.ts | 13 ++++++--- 4 files changed, 27 insertions(+), 34 deletions(-) diff --git a/ts/packages/actionGrammar/src/nfaMatcher.ts b/ts/packages/actionGrammar/src/nfaMatcher.ts index 36e4f9b35a..23576c5d81 100644 --- a/ts/packages/actionGrammar/src/nfaMatcher.ts +++ b/ts/packages/actionGrammar/src/nfaMatcher.ts @@ -205,7 +205,9 @@ export function tokenizeRequestWithOffsets(request: string): { * Peeled punctuation becomes its own token with offsets covering only the * punct run; the base word's `ends` stops before the punct. */ -export function tokenizeRequestWithOffsetsKeepingTrailingPunct(request: string): { +export function tokenizeRequestWithOffsetsKeepingTrailingPunct( + request: string, +): { tokens: string[]; starts: number[]; ends: number[]; @@ -400,10 +402,7 @@ export function matchGrammarWithNFA( const punctOffsets = tokenizeRequestWithOffsetsKeepingTrailingPunct(request); const punctTokens = punctOffsets.tokens; - if ( - punctTokens.length > 0 && - !tokenArraysEqual(punctTokens, tokens) - ) { + if (punctTokens.length > 0 && !tokenArraysEqual(punctTokens, tokens)) { debug( `Trailing-punct tokens: [${punctTokens.join(", ")}] (${punctTokens.length} tokens)`, ); @@ -419,10 +418,7 @@ export function matchGrammarWithNFA( false, punctCtx, ); - const punctSplit = applySplitToTokens( - punctTokens, - splitCandidates, - ); + const punctSplit = applySplitToTokens(punctTokens, splitCandidates); if (punctSplit !== null) { const punctSplitResult = matchNFAWithIndex( nfa, diff --git a/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts b/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts index b1ef46edf5..3cec25cfde 100644 --- a/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts +++ b/ts/packages/actionGrammar/test/nfaDfaStringPhraseSetVariable.spec.ts @@ -184,10 +184,7 @@ describe("PhraseSetPart variable capture", () => { ], }; - function bestGrammarActionValue( - g: Grammar, - request: string, - ): unknown { + function bestGrammarActionValue(g: Grammar, request: string): unknown { const results = matchGrammar(g, request); if (results.length === 0) return undefined; return results[0].match; diff --git a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts index ab9760d6a7..9ace02a8eb 100644 --- a/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts +++ b/ts/packages/actionGrammar/test/quantifierSpecialChars.spec.ts @@ -437,14 +437,9 @@ describe("Quantifier special chars (? * +)", () => { return prompts; } - function loadRuntimePromptByName( - path: string, - name: string, - ): string { + function loadRuntimePromptByName(path: string, name: string): string { const src = loadPromptSource(path); - const re = new RegExp( - `const\\s+${name}\\s*=\\s*(\`[\\s\\S]*?\`);`, - ); + const re = new RegExp(`const\\s+${name}\\s*=\\s*(\`[\\s\\S]*?\`);`); const m = re.exec(src); if (!m) { throw new Error(`${name} not found in ${path}`); @@ -491,7 +486,9 @@ describe("Quantifier special chars (? * +)", () => { expect(nonWrong).not.toMatch(/'can you'\?/); expect(nonWrong).not.toMatch(/'please'\?/); expect(nonWrong).not.toMatch(/"please"\?/); - expect(nonWrong).not.toMatch(/Optional elements: element\?/); + expect(nonWrong).not.toMatch( + /Optional elements: element\?/, + ); } // Main SCHEMA prompt also teaches shared Polite vocab const main = loadRuntimePromptByName( @@ -720,9 +717,9 @@ describeForEachMatcher( expect(testMatchGrammar(g, "can you open outlook")).toStrictEqual([ "ok", ]); - expect( - testMatchGrammar(g, "could you open outlook"), - ).toStrictEqual(["ok"]); + expect(testMatchGrammar(g, "could you open outlook")).toStrictEqual( + ["ok"], + ); expect( testMatchGrammar(g, "would you please open outlook"), ).toStrictEqual(["ok"]); @@ -737,9 +734,9 @@ describeForEachMatcher( expect(testMatchGrammar(g, "add milk to list")).toStrictEqual([ "x", ]); - expect( - testMatchGrammar(g, "add milk eggs to list"), - ).toStrictEqual(["x"]); + expect(testMatchGrammar(g, "add milk eggs to list")).toStrictEqual([ + "x", + ]); expect(testMatchGrammar(g, "add to list")).toStrictEqual([]); }); @@ -904,9 +901,7 @@ describe("Quantifier special chars — NFA/DFA trailing literal ?", () => { ), ).toEqual(["q"]); expect( - matchGrammarWithNFA(g, nfa, "what is the time").map( - (m) => m.match, - ), + matchGrammarWithNFA(g, nfa, "what is the time").map((m) => m.match), ).toEqual([]); }); diff --git a/ts/packages/cache/src/cache/grammarStore.ts b/ts/packages/cache/src/cache/grammarStore.ts index 83b80c850f..db5489d16f 100644 --- a/ts/packages/cache/src/cache/grammarStore.ts +++ b/ts/packages/cache/src/cache/grammarStore.ts @@ -213,10 +213,15 @@ export class GrammarStoreImpl implements GrammarStore { // Pass request so matchDFAWithSplitting can retry with trailing // sentence punctuation peeled into its own tokens (e.g. hello? // against a grammar ending in standalone \?). - const dfaResult = matchDFAWithSplitting(entry.dfa, tokens, false, { - request, - grammar: entry.grammar, - }); + const dfaResult = matchDFAWithSplitting( + entry.dfa, + tokens, + false, + { + request, + grammar: entry.grammar, + }, + ); grammarMatches = dfaResult.matched ? [ {