Macros 2.0 [Fix] - Fix macro evaluation to allow nested scoped macros in arguments (#4977) * Fix scoped macro evaluation to allow nested scoped macros in arguments - Refactor `#evaluateArgumentNode` to re-parse argument content for scoped macro pairs - Create shared `#evaluateRawContent` helper method for consistent text evaluation - Update `#evaluateScopedContent` to use the shared helper, reducing duplication - Add 8 comprehensive test cases for nested scoped macros in arguments - Enhance JSDoc documentation for EvaluationContext text parameter * Fix `{{pick}}` macro to use global document offset for deterministic seeding - Replace `range.startOffset` with `globalOffset` in pick macro handler for consistent position-based seeding - Add `contextOffset` to EvaluationContext to track base offset from original document (starts at 0 for top-level) - Calculate `globalOffset` as `contextOffset + range.startOffset` when building MacroCall objects - Thread `contextOffset` through evaluation pipeline: MacroEngine → MacroCstWalker → all evaluation * Fix typo in MacroCstWalker JSDoc comment and thread contextOffset through evaluateDocument - Correct JSDoc typo: "rull macro text" → "full macro text" - Destructure `contextOffset` from options in `evaluateDocument` method - Pass `contextOffset` to EvaluationContext instead of hardcoded 0
Signed| @@ -345,7 +345,7 @@ export function registerCoreMacros() { | |||
| 345 | description: 'Picks a random item from a list, but keeps the choice stable for a given chat and macro position.', | 345 | description: 'Picks a random item from a list, but keeps the choice stable for a given chat and macro position.', |
| 346 | returns: 'Stable randomly selected item from the list.', | 346 | returns: 'Stable randomly selected item from the list.', |
| 347 | exampleUsage: ['{{pick::blonde::brown::red::black::blue}}'], | 347 | exampleUsage: ['{{pick::blonde::brown::red::black::blue}}'], |
| 348 | handler: ({ list, range, env }) => { | 348 | handler: ({ list, globalOffset, env }) => { |
| 349 | // Handle old legacy cases, where we have to split the list manually | 349 | // Handle old legacy cases, where we have to split the list manually |
| 350 | if (list.length === 1) { | 350 | if (list.length === 1) { |
| 351 | list = readSingleArgsRandomList(list[0]); | 351 | list = readSingleArgsRandomList(list[0]); |
| @@ -355,12 +355,19 @@ export function registerCoreMacros() { | |||
| 355 | return ''; | 355 | return ''; |
| 356 | } | 356 | } |
| 357 | 357 | ||
| 358 | // NOTE: | ||
| 359 | // When changing the hashing logic, make sure to update unit test functionality | ||
| 360 | // in registerTestablePick() to be identical. | ||
| 361 | |||
| 358 | const chatIdHash = getChatIdHash(); | 362 | const chatIdHash = getChatIdHash(); |
| 359 | 363 | ||
| 360 | // Use the full original input string for deterministic behavior | 364 | // Use the full original input string for deterministic behavior |
| 361 | const rawContentHash = env.contentHash; | 365 | const rawContentHash = env.contentHash; |
| 362 | 366 | ||
| 363 | const offset = typeof range?.startOffset === 'number' ? range.startOffset : 0; | 367 | // Use globalOffset for deterministic seeding - this ensures identical macros |
| 368 | // at different positions in the document produce different results, even when | ||
| 369 | // nested inside arguments or scoped content | ||
| 370 | const offset = globalOffset; | ||
| 364 | 371 | ||
| 365 | const combinedSeedString = `${chatIdHash}-${rawContentHash}-${offset}`; | 372 | const combinedSeedString = `${chatIdHash}-${rawContentHash}-${offset}`; |
| 366 | const finalSeed = getStringHash(combinedSeedString); | 373 | const finalSeed = getStringHash(combinedSeedString); |
| @@ -18,7 +18,10 @@ import { MacroRegistry } from './MacroRegistry.js'; | |||
| 18 | * @property {string} rawInner | 18 | * @property {string} rawInner |
| 19 | * @property {string} rawWithBraces | 19 | * @property {string} rawWithBraces |
| 20 | * @property {string[]} rawArgs | 20 | * @property {string[]} rawArgs |
| 21 | * @property {{ startOffset: number, endOffset: number }} range | 21 | * @property {{ startOffset: number, endOffset: number }} range - Range relative to the current evaluation context's text. |
| 22 | * @property {number} globalOffset - The offset of this macro in the original top-level document. | ||
| 23 | * This combines the context's base offset with the local range. Use this for deterministic | ||
| 24 | * seeding (e.g., in {{pick}}) to ensure identical macros at different positions produce different results. | ||
| 22 | * @property {CstNode} cstNode | 25 | * @property {CstNode} cstNode |
| 23 | */ | 26 | */ |
| 24 | 27 | ||
| @@ -31,10 +34,21 @@ import { MacroRegistry } from './MacroRegistry.js'; | |||
| 31 | */ | 34 | */ |
| 32 | 35 | ||
| 33 | /** | 36 | /** |
| 37 | * Context passed through the CST evaluation process. | ||
| 38 | * | ||
| 34 | * @typedef {Object} EvaluationContext | 39 | * @typedef {Object} EvaluationContext |
| 35 | * @property {string} text | 40 | * @property {string} text - The text being evaluated at the current level. This is NOT the same as env.content. |
| 36 | * @property {MacroEnv} env | 41 | * At the top level, this is the full document text. When evaluating nested content (arguments or scoped |
| 37 | * @property {(call: MacroCall) => string} resolveMacro | 42 | * content), this is the substring being evaluated. CST node positions are always relative to this text. |
| 43 | * | ||
| 44 | * - Careful, this also means when resolving macros inside macro arguments, this will NOT be the text of | ||
| 45 | * the argument currently being resolved, but the full macro text with identifier and all macros. | ||
| 46 | * @property {number} contextOffset - Base offset from the original top-level document. At the top level this is 0. | ||
| 47 | * When re-parsing nested content (arguments/scoped), this is set to the substring's start position in | ||
| 48 | * the original document. Used to calculate globalOffset for macros that need deterministic positioning. | ||
| 49 | * @property {MacroEnv} env - The macro environment containing context like user/char names, variables, and the | ||
| 50 | * original full content (env.content). This remains constant throughout the evaluation. | ||
| 51 | * @property {(call: MacroCall) => string} resolveMacro - Callback to resolve a macro call to its result string. | ||
| 38 | * @property {(content: string, options?: { trimIndent?: boolean }) => string} trimContent - Shared utility function that trims scoped content with optional indentation dedent. | 52 | * @property {(content: string, options?: { trimIndent?: boolean }) => string} trimContent - Shared utility function that trims scoped content with optional indentation dedent. |
| 39 | */ | 53 | */ |
| 40 | 54 | ||
| @@ -74,7 +88,7 @@ class MacroCstWalker { | |||
| 74 | * @returns {string} | 88 | * @returns {string} |
| 75 | */ | 89 | */ |
| 76 | evaluateDocument(options) { | 90 | evaluateDocument(options) { |
| 77 | const { text, cst, env, resolveMacro, trimContent } = options; | 91 | const { text, cst, contextOffset, env, resolveMacro, trimContent } = options; |
| 78 | 92 | ||
| 79 | if (typeof text !== 'string') { | 93 | if (typeof text !== 'string') { |
| 80 | throw new Error('MacroCstWalker.evaluateDocument: text must be a string'); | 94 | throw new Error('MacroCstWalker.evaluateDocument: text must be a string'); |
| @@ -90,7 +104,7 @@ class MacroCstWalker { | |||
| 90 | } | 104 | } |
| 91 | 105 | ||
| 92 | /** @type {EvaluationContext} */ | 106 | /** @type {EvaluationContext} */ |
| 93 | const context = { text, env, resolveMacro, trimContent }; | 107 | const context = { text, contextOffset, env, resolveMacro, trimContent }; |
| 94 | let items = this.#collectDocumentItems(cst); | 108 | let items = this.#collectDocumentItems(cst); |
| 95 | 109 | ||
| 96 | // Process scoped macros: find opening/closing pairs and merge them | 110 | // Process scoped macros: find opening/closing pairs and merge them |
| @@ -341,7 +355,7 @@ class MacroCstWalker { | |||
| 341 | * @returns {string} | 355 | * @returns {string} |
| 342 | */ | 356 | */ |
| 343 | #evaluateMacroNode(macroNode, context, scopedContent) { | 357 | #evaluateMacroNode(macroNode, context, scopedContent) { |
| 344 | const { text, env, resolveMacro, trimContent } = context; | 358 | const { text, contextOffset, env, resolveMacro, trimContent } = context; |
| 345 | 359 | ||
| 346 | const children = macroNode.children || {}; | 360 | const children = macroNode.children || {}; |
| 347 | 361 | ||
| @@ -467,6 +481,7 @@ class MacroCstWalker { | |||
| 467 | rawWithBraces: text.slice(range.startOffset, range.endOffset + 1), | 481 | rawWithBraces: text.slice(range.startOffset, range.endOffset + 1), |
| 468 | rawArgs, | 482 | rawArgs, |
| 469 | range, | 483 | range, |
| 484 | globalOffset: contextOffset + range.startOffset, | ||
| 470 | cstNode: macroNode, | 485 | cstNode: macroNode, |
| 471 | env, | 486 | env, |
| 472 | }; | 487 | }; |
| @@ -486,7 +501,7 @@ class MacroCstWalker { | |||
| 486 | * @returns {string} | 501 | * @returns {string} |
| 487 | */ | 502 | */ |
| 488 | #evaluateVariableExpr(macroNode, variableExprNode, context) { | 503 | #evaluateVariableExpr(macroNode, variableExprNode, context) { |
| 489 | const { text, env, resolveMacro } = context; | 504 | const { text, contextOffset, env, resolveMacro } = context; |
| 490 | 505 | ||
| 491 | const children = macroNode.children || {}; | 506 | const children = macroNode.children || {}; |
| 492 | const varChildren = variableExprNode.children || {}; | 507 | const varChildren = variableExprNode.children || {}; |
| @@ -560,6 +575,7 @@ class MacroCstWalker { | |||
| 560 | rawWithBraces: text.slice(range.startOffset, range.endOffset + 1), | 575 | rawWithBraces: text.slice(range.startOffset, range.endOffset + 1), |
| 561 | rawArgs: args, | 576 | rawArgs: args, |
| 562 | range, | 577 | range, |
| 578 | globalOffset: contextOffset + range.startOffset, | ||
| 563 | cstNode: macroNode, | 579 | cstNode: macroNode, |
| 564 | env, | 580 | env, |
| 565 | }; | 581 | }; |
| @@ -642,9 +658,13 @@ class MacroCstWalker { | |||
| 642 | * Evaluates a single argument node by resolving nested macros and reconstructing | 658 | * Evaluates a single argument node by resolving nested macros and reconstructing |
| 643 | * the original argument text. | 659 | * the original argument text. |
| 644 | * | 660 | * |
| 645 | * @param {CstNode} argNode | 661 | * This method extracts the argument's raw text and re-parses it to properly |
| 646 | * @param {EvaluationContext} context | 662 | * handle scoped macros (opening/closing tag pairs) that may appear within |
| 647 | * @returns {string} | 663 | * the argument content. |
| 664 | * | ||
| 665 | * @param {CstNode} argNode - The argument CST node to evaluate. | ||
| 666 | * @param {EvaluationContext} context - The evaluation context containing the parent document's text and environment. | ||
| 667 | * @returns {string} The evaluated argument with all nested macros (including scoped ones) resolved. | ||
| 648 | */ | 668 | */ |
| 649 | #evaluateArgumentNode(argNode, context) { | 669 | #evaluateArgumentNode(argNode, context) { |
| 650 | const location = this.#getArgumentLocation(argNode); | 670 | const location = this.#getArgumentLocation(argNode); |
| @@ -652,38 +672,87 @@ class MacroCstWalker { | |||
| 652 | return ''; | 672 | return ''; |
| 653 | } | 673 | } |
| 654 | 674 | ||
| 655 | const { text } = context; | 675 | const { text, contextOffset } = context; |
| 676 | const rawContent = text.slice(location.startOffset, location.endOffset + 1); | ||
| 656 | 677 | ||
| 657 | const nestedMacros = /** @type {CstNode[]} */ ((argNode.children || {}).macro || []); | 678 | // Calculate the new base offset: parent's contextOffset + this argument's start position |
| 679 | const newContextOffset = contextOffset + location.startOffset; | ||
| 658 | 680 | ||
| 659 | // If there are no nested macros, we can just return the original text | 681 | // Use the shared helper to evaluate the content, which handles scoped macros |
| 660 | if (nestedMacros.length === 0) { | 682 | return this.#evaluateRawContent(rawContent, newContextOffset, context); |
| 661 | return text.slice(location.startOffset, location.endOffset + 1); | 683 | } |
| 684 | |||
| 685 | /** | ||
| 686 | * Evaluates a text content string by parsing it and resolving all macros, | ||
| 687 | * including scoped macro pairs (opening/closing tags). | ||
| 688 | * | ||
| 689 | * This is the core helper used by both argument evaluation and scoped content | ||
| 690 | * evaluation to ensure consistent handling of nested and scoped macros. | ||
| 691 | * | ||
| 692 | * @param {string} rawContent - The raw text content to evaluate. | ||
| 693 | * @param {number} newContextOffset - The offset of rawContent's start position in the original top-level document. | ||
| 694 | * @param {EvaluationContext} context - The parent evaluation context (used for env, resolveMacro, trimContent). | ||
| 695 | * @returns {string} The evaluated content with all macros resolved. | ||
| 696 | */ | ||
| 697 | #evaluateRawContent(rawContent, newContextOffset, context) { | ||
| 698 | // If empty, return as-is | ||
| 699 | if (!rawContent) { | ||
| 700 | return ''; | ||
| 662 | } | 701 | } |
| 663 | 702 | ||
| 664 | // If there are macros, evaluate them one by one in appearing order, inside the argument, before we return the resolved argument | 703 | // Re-evaluate the content to find all nested macros including scoped pairs |
| 665 | const nestedWithRange = nestedMacros.map(node => ({ | 704 | // We need to parse and evaluate this content as if it were a standalone document |
| 666 | node, | 705 | const { cst } = MacroParser.parseDocument(rawContent); |
| 667 | range: this.#getMacroRange(node), | ||
| 668 | })); | ||
| 669 | 706 | ||
| 670 | nestedWithRange.sort((a, b) => a.range.startOffset - b.range.startOffset); | 707 | // If parsing fails, return the raw content |
| 708 | if (!cst || typeof cst !== 'object' || !cst.children) { | ||
| 709 | return rawContent; | ||
| 710 | } | ||
| 671 | 711 | ||
| 712 | // Create a new context with the content as the text and updated contextOffset | ||
| 713 | // This is important: positions in the parsed CST are relative to rawContent, | ||
| 714 | // but contextOffset tracks the absolute position in the original document | ||
| 715 | /** @type {EvaluationContext} */ | ||
| 716 | const contentContext = { ...context, text: rawContent, contextOffset: newContextOffset }; | ||
| 717 | |||
| 718 | // Collect items and process scoped macros | ||
| 719 | let items = this.#collectDocumentItems(cst); | ||
| 720 | items = this.#processScopedMacros(items, rawContent); | ||
| 721 | |||
| 722 | // If no items, return raw content | ||
| 723 | if (items.length === 0) { | ||
| 724 | return rawContent; | ||
| 725 | } | ||
| 726 | |||
| 727 | // Evaluate items in order | ||
| 672 | let result = ''; | 728 | let result = ''; |
| 673 | let cursor = location.startOffset; | 729 | let cursor = 0; |
| 674 | 730 | ||
| 675 | for (const entry of nestedWithRange) { | 731 | for (const item of items) { |
| 676 | if (entry.range.startOffset < cursor) { | 732 | if (item.startOffset > cursor) { |
| 677 | continue; | 733 | result += rawContent.slice(cursor, item.startOffset); |
| 678 | } | 734 | } |
| 679 | 735 | ||
| 680 | result += text.slice(cursor, entry.range.startOffset); | 736 | if (item.type === 'plaintext') { |
| 681 | result += this.#evaluateMacroNode(entry.node, context); | 737 | result += rawContent.slice(item.startOffset, item.endOffset + 1); |
| 682 | cursor = entry.range.endOffset + 1; | 738 | cursor = item.endOffset + 1; |
| 739 | } else if (item.keepRaw) { | ||
| 740 | // Unmatched closing macros stay as raw text | ||
| 741 | result += rawContent.slice(item.startOffset, item.endOffset + 1); | ||
| 742 | cursor = item.endOffset + 1; | ||
| 743 | } else { | ||
| 744 | result += this.#evaluateMacroNode(item.node, contentContext, item.scopedContent); | ||
| 745 | // If this macro has scoped content, skip past the closing macro | ||
| 746 | if (item.scopedContent && item.scopedContent.closingEndOffset > item.endOffset) { | ||
| 747 | cursor = item.scopedContent.closingEndOffset + 1; | ||
| 748 | } else { | ||
| 749 | cursor = item.endOffset + 1; | ||
| 750 | } | ||
| 751 | } | ||
| 683 | } | 752 | } |
| 684 | 753 | ||
| 685 | if (cursor <= location.endOffset) { | 754 | if (cursor < rawContent.length) { |
| 686 | result += text.slice(cursor, location.endOffset + 1); | 755 | result += rawContent.slice(cursor); |
| 687 | } | 756 | } |
| 688 | 757 | ||
| 689 | return result; | 758 | return result; |
| @@ -836,76 +905,22 @@ class MacroCstWalker { | |||
| 836 | * This resolves any nested macros within the scoped content. | 905 | * This resolves any nested macros within the scoped content. |
| 837 | * | 906 | * |
| 838 | * @param {{ startOffset: number, endOffset: number }} scopedContent - The range of the scoped content. | 907 | * @param {{ startOffset: number, endOffset: number }} scopedContent - The range of the scoped content. |
| 839 | * @param {EvaluationContext} context - The evaluation context. | 908 | * @param {EvaluationContext} context - The evaluation context. The `text` property contains the parent |
| 909 | * document text, and offsets in scopedContent are relative to that parent text. | ||
| 840 | * @returns {string} - The evaluated scoped content with nested macros resolved. | 910 | * @returns {string} - The evaluated scoped content with nested macros resolved. |
| 841 | */ | 911 | */ |
| 842 | #evaluateScopedContent(scopedContent, context) { | 912 | #evaluateScopedContent(scopedContent, context) { |
| 843 | const { text, env, resolveMacro, trimContent } = context; | 913 | const { text, contextOffset } = context; |
| 844 | const { startOffset, endOffset } = scopedContent; | 914 | const { startOffset, endOffset } = scopedContent; |
| 845 | 915 | ||
| 846 | // Extract the raw content between opening and closing tags | 916 | // Extract the raw content between opening and closing tags |
| 847 | const rawContent = text.slice(startOffset, endOffset + 1); | 917 | const rawContent = text.slice(startOffset, endOffset + 1); |
| 848 | 918 | ||
| 849 | // If empty, return empty string | 919 | // Calculate the new base offset: parent's contextOffset + this scoped content's start position |
| 850 | if (!rawContent) { | 920 | const newContextOffset = contextOffset + startOffset; |
| 851 | return ''; | ||
| 852 | } | ||
| 853 | |||
| 854 | // Re-evaluate the scoped content to resolve any nested macros | ||
| 855 | // We need to parse and evaluate this content as if it were a standalone document | ||
| 856 | const { cst: scopedCst } = MacroParser.parseDocument(rawContent); | ||
| 857 | |||
| 858 | // If parsing fails, return the raw content | ||
| 859 | if (!scopedCst || typeof scopedCst !== 'object' || !scopedCst.children) { | ||
| 860 | return rawContent; | ||
| 861 | } | ||
| 862 | |||
| 863 | // Create a new context with the scoped content text | ||
| 864 | /** @type {EvaluationContext} */ | ||
| 865 | const scopedContext = { text: rawContent, env, resolveMacro, trimContent }; | ||
| 866 | |||
| 867 | // Collect items from the scoped content CST | ||
| 868 | let items = this.#collectDocumentItems(scopedCst); | ||
| 869 | |||
| 870 | // Process any nested scoped macros within this content | ||
| 871 | items = this.#processScopedMacros(items, rawContent); | ||
| 872 | |||
| 873 | // Evaluate the items | ||
| 874 | if (items.length === 0) { | ||
| 875 | return rawContent; | ||
| 876 | } | ||
| 877 | |||
| 878 | let result = ''; | ||
| 879 | let cursor = 0; | ||
| 880 | 921 | ||
| 881 | for (const item of items) { | 922 | // Use the shared helper to evaluate the content |
| 882 | if (item.startOffset > cursor) { | 923 | return this.#evaluateRawContent(rawContent, newContextOffset, context); |
| 883 | result += rawContent.slice(cursor, item.startOffset); | ||
| 884 | } | ||
| 885 | |||
| 886 | if (item.type === 'plaintext') { | ||
| 887 | result += rawContent.slice(item.startOffset, item.endOffset + 1); | ||
| 888 | cursor = item.endOffset + 1; | ||
| 889 | } else if (item.keepRaw) { | ||
| 890 | // Unmatched closing macros stay as raw text | ||
| 891 | result += rawContent.slice(item.startOffset, item.endOffset + 1); | ||
| 892 | cursor = item.endOffset + 1; | ||
| 893 | } else { | ||
| 894 | result += this.#evaluateMacroNode(item.node, scopedContext, item.scopedContent); | ||
| 895 | // If this macro has scoped content, skip past the closing macro | ||
| 896 | if (item.scopedContent && item.scopedContent.closingEndOffset > item.endOffset) { | ||
| 897 | cursor = item.scopedContent.closingEndOffset + 1; | ||
| 898 | } else { | ||
| 899 | cursor = item.endOffset + 1; | ||
| 900 | } | ||
| 901 | } | ||
| 902 | } | ||
| 903 | |||
| 904 | if (cursor < rawContent.length) { | ||
| 905 | result += rawContent.slice(cursor); | ||
| 906 | } | ||
| 907 | |||
| 908 | return result; | ||
| 909 | } | 924 | } |
| 910 | 925 | ||
| 911 | // ======================================================================== | 926 | // ======================================================================== |
| @@ -138,6 +138,7 @@ class MacroEngine { | |||
| 138 | try { | 138 | try { |
| 139 | evaluated = MacroCstWalker.evaluateDocument({ | 139 | evaluated = MacroCstWalker.evaluateDocument({ |
| 140 | text: preProcessed, | 140 | text: preProcessed, |
| 141 | contextOffset: 0, | ||
| 141 | cst, | 142 | cst, |
| 142 | env: safeEnv, | 143 | env: safeEnv, |
| 143 | resolveMacro: this.#resolveMacro.bind(this), | 144 | resolveMacro: this.#resolveMacro.bind(this), |
| @@ -114,8 +114,11 @@ export const MacroValueType = Object.freeze({ | |||
| 114 | * @property {string} rawOriginal - The original full macro text including braces, before any resolution. | 114 | * @property {string} rawOriginal - The original full macro text including braces, before any resolution. |
| 115 | * @property {string[]} rawArgs - The original arguments passed to the macro (always unresolved). | 115 | * @property {string[]} rawArgs - The original arguments passed to the macro (always unresolved). |
| 116 | * @property {MacroEnv} env | 116 | * @property {MacroEnv} env |
| 117 | * @property {CstNode|null} cstNode | 117 | * @property {CstNode} cstNode |
| 118 | * @property {{ startOffset: number, endOffset: number }|null} range | 118 | * @property {{ startOffset: number, endOffset: number }} range - Range relative to the current evaluation context's text. |
| 119 | * @property {number} globalOffset - The offset of this macro in the original top-level document. | ||
| 120 | * This combines the context's base offset with the local range. Use this for deterministic | ||
| 121 | * seeding (e.g., in {{pick}}) to ensure identical macros at different positions produce different results. | ||
| 119 | * @property {(value: any) => string} normalize - Normalize function to use on unsure macro results to make sure they return strings as expected. | 122 | * @property {(value: any) => string} normalize - Normalize function to use on unsure macro results to make sure they return strings as expected. |
| 120 | * @property {(content: string, options?: { trimIndent?: boolean }) => string} trimContent - Trims scoped content with optional indentation dedent. Defaults to trimming indentation. | 123 | * @property {(content: string, options?: { trimIndent?: boolean }) => string} trimContent - Trims scoped content with optional indentation dedent. Defaults to trimming indentation. |
| 121 | * @property {(text: string) => string} resolve - Evaluates macros in the given text using the same environment. Use when delayArgResolution is true. | 124 | * @property {(text: string) => string} resolve - Evaluates macros in the given text using the same environment. Use when delayArgResolution is true. |
| @@ -363,6 +366,7 @@ class MacroRegistry { | |||
| 363 | env: call.env, | 366 | env: call.env, |
| 364 | cstNode: call.cstNode, | 367 | cstNode: call.cstNode, |
| 365 | range: call.range, | 368 | range: call.range, |
| 369 | globalOffset: call.globalOffset, | ||
| 366 | normalize: MacroEngine.normalizeMacroResult.bind(MacroEngine), | 370 | normalize: MacroEngine.normalizeMacroResult.bind(MacroEngine), |
| 367 | trimContent: MacroEngine.trimScopedContent.bind(MacroEngine), | 371 | trimContent: MacroEngine.trimScopedContent.bind(MacroEngine), |
| 368 | resolve: (text) => MacroEngine.evaluate(text, call.env), | 372 | resolve: (text) => MacroEngine.evaluate(text, call.env), |
| @@ -633,28 +633,69 @@ test.describe('MacroEngine', () => { | |||
| 633 | }); | 633 | }); |
| 634 | 634 | ||
| 635 | test.describe('Deterministic pick macro', () => { | 635 | test.describe('Deterministic pick macro', () => { |
| 636 | test('should return stable results for the same chat and content', async ({ page }) => { | 636 | /** Fixed chat ID hash used across all pick tests for deterministic behavior */ |
| 637 | // Simulate a consistent chat id hash | 637 | const TEST_CHAT_ID_HASH = 123456; |
| 638 | let originalHash; | 638 | |
| 639 | await page.evaluate(async ([originalHash]) => { | 639 | /** |
| 640 | * Registers a testable pick macro that returns the seed string instead of the picked value. | ||
| 641 | * This allows tests to verify that different macro positions produce different seeds. | ||
| 642 | * | ||
| 643 | * @param {import('@playwright/test').Page} page | ||
| 644 | */ | ||
| 645 | async function registerTestablePick(page) { | ||
| 646 | await page.evaluate(async () => { | ||
| 647 | /** @type {import('../../public/scripts/macros/engine/MacroRegistry.js')} */ | ||
| 648 | const { MacroRegistry, MacroCategory } = await import('./scripts/macros/engine/MacroRegistry.js'); | ||
| 649 | /** @type {import('../../public/scripts/utils.js')} */ | ||
| 650 | const { getStringHash } = await import('./scripts/utils.js'); | ||
| 651 | /** @type {import('../../public/script.js')} */ | ||
| 652 | const { chat_metadata } = await import('./script.js'); | ||
| 653 | /** @type {import('../../public/lib.js')} */ | ||
| 654 | const { seedrandom } = await import('./lib.js'); | ||
| 655 | |||
| 656 | // Only register once | ||
| 657 | if (MacroRegistry.getMacro('testablePick')) return; | ||
| 658 | |||
| 659 | MacroRegistry.registerMacro('testablePick', { | ||
| 660 | category: MacroCategory.RANDOM, | ||
| 661 | list: true, | ||
| 662 | description: 'Test version of pick that returns the seed string for verification.', | ||
| 663 | handler: ({ list, globalOffset, env }) => { | ||
| 664 | const chatIdHash = chat_metadata.chat_id_hash ?? 0; | ||
| 665 | const rawContentHash = env.contentHash; | ||
| 666 | const offset = globalOffset; | ||
| 667 | const combinedSeedString = `${chatIdHash}-${rawContentHash}-${offset}`; | ||
| 668 | // Return both the seed and what would be picked for validation | ||
| 669 | const finalSeed = getStringHash(combinedSeedString); | ||
| 670 | const rng = seedrandom(String(finalSeed)); | ||
| 671 | const randomIndex = Math.floor(rng() * list.length); | ||
| 672 | return `seed:${combinedSeedString}|pick:${list[randomIndex]}`; | ||
| 673 | }, | ||
| 674 | }); | ||
| 675 | }); | ||
| 676 | } | ||
| 677 | |||
| 678 | test.beforeEach(async ({ page }) => { | ||
| 679 | // Set consistent chat ID hash for all tests | ||
| 680 | await page.evaluate(async (hash) => { | ||
| 640 | /** @type {import('../../public/script.js')} */ | 681 | /** @type {import('../../public/script.js')} */ |
| 641 | const { chat_metadata } = await import('./script.js'); | 682 | const { chat_metadata } = await import('./script.js'); |
| 642 | originalHash = chat_metadata.chat_id_hash; | 683 | chat_metadata.chat_id_hash = hash; |
| 643 | chat_metadata.chat_id_hash = 123456; | 684 | }, TEST_CHAT_ID_HASH); |
| 644 | }, [originalHash]); | 685 | }); |
| 645 | 686 | ||
| 687 | test('should return stable results for the same chat and content', async ({ page }) => { | ||
| 646 | const input = 'Choices: {{pick::red::green::blue}}, {{pick::red::green::blue}}.'; | 688 | const input = 'Choices: {{pick::red::green::blue}}, {{pick::red::green::blue}}.'; |
| 647 | 689 | ||
| 648 | const output1 = await evaluateWithEngine(page, input); | 690 | const output1 = await evaluateWithEngine(page, input); |
| 649 | const output2 = await evaluateWithEngine(page, input); | 691 | const output2 = await evaluateWithEngine(page, input); |
| 650 | 692 | ||
| 651 | // Deterministic: same chat and same content should yield identical output. | 693 | // Deterministic: same chat and same content should yield identical output |
| 652 | expect(output1).toBe(output2); | 694 | expect(output1).toBe(output2); |
| 653 | 695 | ||
| 654 | // Sanity check: both picks should resolve to one of the provided options. | 696 | // Sanity check: both picks should resolve to one of the provided options |
| 655 | const match = output1.match(/Choices: ([^,]+), ([^.]+)\./); | 697 | const match = output1.match(/Choices: ([^,]+), ([^.]+)\./); |
| 656 | expect(match).not.toBeNull(); | 698 | expect(match).not.toBeNull(); |
| 657 | |||
| 658 | if (!match) return; | 699 | if (!match) return; |
| 659 | 700 | ||
| 660 | const first = match[1].trim(); | 701 | const first = match[1].trim(); |
| @@ -663,13 +704,119 @@ test.describe('MacroEngine', () => { | |||
| 663 | 704 | ||
| 664 | expect(options.includes(first)).toBeTruthy(); | 705 | expect(options.includes(first)).toBeTruthy(); |
| 665 | expect(options.includes(second)).toBeTruthy(); | 706 | expect(options.includes(second)).toBeTruthy(); |
| 707 | }); | ||
| 666 | 708 | ||
| 667 | // Restore original hash | 709 | test('should use different seeds for identical picks at different positions', async ({ page }) => { |
| 668 | await page.evaluate(async ([originalHash]) => { | 710 | await registerTestablePick(page); |
| 669 | /** @type {import('../../public/script.js')} */ | 711 | |
| 670 | const { chat_metadata } = await import('./script.js'); | 712 | const output = await page.evaluate(async () => { |
| 671 | chat_metadata.chat_id_hash = originalHash; | 713 | /** @type {import('../../public/scripts/macros/engine/MacroEngine.js')} */ |
| 672 | }, [originalHash]); | 714 | const { MacroEngine } = await import('./scripts/macros/engine/MacroEngine.js'); |
| 715 | /** @type {import('../../public/scripts/macros/engine/MacroEnvBuilder.js')} */ | ||
| 716 | const { MacroEnvBuilder } = await import('./scripts/macros/engine/MacroEnvBuilder.js'); | ||
| 717 | |||
| 718 | const input = '{{testablePick::A::B::C}}###{{testablePick::A::B::C}}'; | ||
| 719 | const env = MacroEnvBuilder.buildFromRawEnv({ content: input }); | ||
| 720 | return MacroEngine.evaluate(input, env); | ||
| 721 | }); | ||
| 722 | |||
| 723 | const parts = output.split('###'); | ||
| 724 | expect(parts.length).toBe(2); | ||
| 725 | |||
| 726 | // Extract seeds from both results | ||
| 727 | const seed1 = parts[0].match(/seed:([^|]+)/)?.[1]; | ||
| 728 | const seed2 = parts[1].match(/seed:([^|]+)/)?.[1]; | ||
| 729 | |||
| 730 | expect(seed1).toBeTruthy(); | ||
| 731 | expect(seed2).toBeTruthy(); | ||
| 732 | // Seeds must be different because the macros are at different positions | ||
| 733 | expect(seed1).not.toBe(seed2); | ||
| 734 | |||
| 735 | // Verify picked values are valid options | ||
| 736 | const pick1 = parts[0].match(/pick:(\w+)/)?.[1]; | ||
| 737 | const pick2 = parts[1].match(/pick:(\w+)/)?.[1]; | ||
| 738 | const options = ['A', 'B', 'C']; | ||
| 739 | expect(options.includes(pick1 ?? '')).toBeTruthy(); | ||
| 740 | expect(options.includes(pick2 ?? '')).toBeTruthy(); | ||
| 741 | }); | ||
| 742 | |||
| 743 | test('should use different seeds for identical picks inside different scoped macros at the same offset', async ({ page }) => { | ||
| 744 | await registerTestablePick(page); | ||
| 745 | |||
| 746 | // Key regression test: picks inside scoped content must use global offsets | ||
| 747 | const output = await page.evaluate(async () => { | ||
| 748 | /** @type {import('../../public/scripts/macros/engine/MacroEngine.js')} */ | ||
| 749 | const { MacroEngine } = await import('./scripts/macros/engine/MacroEngine.js'); | ||
| 750 | /** @type {import('../../public/scripts/macros/engine/MacroEnvBuilder.js')} */ | ||
| 751 | const { MacroEnvBuilder } = await import('./scripts/macros/engine/MacroEnvBuilder.js'); | ||
| 752 | |||
| 753 | // Two identical pick macros inside different setvar scopes | ||
| 754 | // Before the fix, both would get startOffset=0 relative to their argument | ||
| 755 | // After the fix, they get different globalOffset values | ||
| 756 | const input = '{{setvar::first}}{{testablePick::A::B::C}}{{/setvar}}{{setvar::second}}{{testablePick::A::B::C}}{{/setvar}}{{.first}}###{{.second}}'; | ||
| 757 | const env = MacroEnvBuilder.buildFromRawEnv({ content: input }); | ||
| 758 | return MacroEngine.evaluate(input, env); | ||
| 759 | }); | ||
| 760 | |||
| 761 | const parts = output.split('###'); | ||
| 762 | expect(parts.length).toBe(2); | ||
| 763 | |||
| 764 | const seed1 = parts[0].match(/seed:([^|]+)/)?.[1]; | ||
| 765 | const seed2 = parts[1].match(/seed:([^|]+)/)?.[1]; | ||
| 766 | |||
| 767 | expect(seed1).toBeTruthy(); | ||
| 768 | expect(seed2).toBeTruthy(); | ||
| 769 | // Seeds must be different - this is the key assertion for the fix | ||
| 770 | expect(seed1).not.toBe(seed2); | ||
| 771 | }); | ||
| 772 | |||
| 773 | test('should use different seeds for identical picks in inline arguments', async ({ page }) => { | ||
| 774 | await registerTestablePick(page); | ||
| 775 | |||
| 776 | const output = await page.evaluate(async () => { | ||
| 777 | /** @type {import('../../public/scripts/macros/engine/MacroEngine.js')} */ | ||
| 778 | const { MacroEngine } = await import('./scripts/macros/engine/MacroEngine.js'); | ||
| 779 | /** @type {import('../../public/scripts/macros/engine/MacroEnvBuilder.js')} */ | ||
| 780 | const { MacroEnvBuilder } = await import('./scripts/macros/engine/MacroEnvBuilder.js'); | ||
| 781 | |||
| 782 | // Two identical pick macros inside different setvar inline arguments | ||
| 783 | const input = '{{setvar::first::{{testablePick::A::B::C}}}}{{setvar::second::{{testablePick::A::B::C}}}}{{.first}}###{{.second}}'; | ||
| 784 | const env = MacroEnvBuilder.buildFromRawEnv({ content: input }); | ||
| 785 | return MacroEngine.evaluate(input, env); | ||
| 786 | }); | ||
| 787 | |||
| 788 | const parts = output.split('###'); | ||
| 789 | expect(parts.length).toBe(2); | ||
| 790 | |||
| 791 | const seed1 = parts[0].match(/seed:([^|]+)/)?.[1]; | ||
| 792 | const seed2 = parts[1].match(/seed:([^|]+)/)?.[1]; | ||
| 793 | |||
| 794 | expect(seed1).toBeTruthy(); | ||
| 795 | expect(seed2).toBeTruthy(); | ||
| 796 | // Seeds must be different due to different global offsets | ||
| 797 | expect(seed1).not.toBe(seed2); | ||
| 798 | }); | ||
| 799 | |||
| 800 | test('should maintain stability across evaluations for picks in scoped content', async ({ page }) => { | ||
| 801 | // Picks inside scoped content should still be deterministic (same result each time) | ||
| 802 | const outputs = await page.evaluate(async () => { | ||
| 803 | /** @type {import('../../public/scripts/macros/engine/MacroEngine.js')} */ | ||
| 804 | const { MacroEngine } = await import('./scripts/macros/engine/MacroEngine.js'); | ||
| 805 | /** @type {import('../../public/scripts/macros/engine/MacroEnvBuilder.js')} */ | ||
| 806 | const { MacroEnvBuilder } = await import('./scripts/macros/engine/MacroEnvBuilder.js'); | ||
| 807 | |||
| 808 | const input = '{{setvar::val}}{{pick::X::Y::Z}}{{/setvar}}{{.val}}'; | ||
| 809 | const env1 = MacroEnvBuilder.buildFromRawEnv({ content: input }); | ||
| 810 | const env2 = MacroEnvBuilder.buildFromRawEnv({ content: input }); | ||
| 811 | const result1 = MacroEngine.evaluate(input, env1); | ||
| 812 | const result2 = MacroEngine.evaluate(input, env2); | ||
| 813 | return [result1, result2]; | ||
| 814 | }); | ||
| 815 | |||
| 816 | // Same input should produce same output (deterministic) | ||
| 817 | expect(outputs[0]).toBe(outputs[1]); | ||
| 818 | // Should be one of the valid options | ||
| 819 | expect(['X', 'Y', 'Z'].includes(outputs[0])).toBeTruthy(); | ||
| 673 | }); | 820 | }); |
| 674 | }); | 821 | }); |
| 675 | 822 | ||
| @@ -1589,6 +1736,61 @@ test.describe('MacroEngine', () => { | |||
| 1589 | const output = await evaluateWithEngine(page, input); | 1736 | const output = await evaluateWithEngine(page, input); |
| 1590 | expect(output).toBe('middle[first][second]'); | 1737 | expect(output).toBe('middle[first][second]'); |
| 1591 | }); | 1738 | }); |
| 1739 | |||
| 1740 | test.describe('scoped macros nested inside arguments', () => { | ||
| 1741 | test('should resolve scoped macro inside another macro argument', async ({ page }) => { | ||
| 1742 | // {{reverse}}hello{{/reverse}} inside setvar's value argument should resolve first | ||
| 1743 | const input = '{{setvar::testvar::{{reverse}}hello{{/reverse}}}} {{getvar::testvar}}'; | ||
| 1744 | const output = await evaluateWithEngine(page, input); | ||
| 1745 | expect(output).toBe(' olleh'); | ||
| 1746 | }); | ||
| 1747 | |||
| 1748 | test('should resolve scoped if macro inside setvar argument', async ({ page }) => { | ||
| 1749 | // {{if true}}true branch{{/if}} inside setvar should resolve to "true branch" | ||
| 1750 | const input = '{{setvar::testvar::{{if true}}true branch{{/if}}}} {{getvar::testvar}}'; | ||
| 1751 | const output = await evaluateWithEngine(page, input); | ||
| 1752 | expect(output).toBe(' true branch'); | ||
| 1753 | }); | ||
| 1754 | |||
| 1755 | test('should resolve scoped if/else macro inside setvar argument', async ({ page }) => { | ||
| 1756 | const input = '{{setvar::testvar::{{if 0}}wrong{{else}}correct{{/if}}}} {{getvar::testvar}}'; | ||
| 1757 | const output = await evaluateWithEngine(page, input); | ||
| 1758 | expect(output).toBe(' correct'); | ||
| 1759 | }); | ||
| 1760 | |||
| 1761 | test('should resolve multiple scoped macros inside single argument', async ({ page }) => { | ||
| 1762 | // Two scoped macros in the same argument | ||
| 1763 | const input = '{{setvar::testvar::{{reverse}}ab{{/reverse}}-{{reverse}}cd{{/reverse}}}} {{getvar::testvar}}'; | ||
| 1764 | const output = await evaluateWithEngine(page, input); | ||
| 1765 | expect(output).toBe(' ba-dc'); | ||
| 1766 | }); | ||
| 1767 | |||
| 1768 | test('should resolve deeply nested scoped macros in arguments', async ({ page }) => { | ||
| 1769 | // Scoped macro inside scoped macro inside argument | ||
| 1770 | const input = '{{setvar::outer::{{setvar::inner::{{reverse}}xyz{{/reverse}}}}{{getvar::inner}}}} {{getvar::outer}}'; | ||
| 1771 | const output = await evaluateWithEngine(page, input); | ||
| 1772 | expect(output).toBe(' zyx'); | ||
| 1773 | }); | ||
| 1774 | |||
| 1775 | test('should resolve scoped macro with text before and after in argument', async ({ page }) => { | ||
| 1776 | const input = '{{setvar::testvar::before {{reverse}}mid{{/reverse}} after}} {{getvar::testvar}}'; | ||
| 1777 | const output = await evaluateWithEngine(page, input); | ||
| 1778 | expect(output).toBe(' before dim after'); | ||
| 1779 | }); | ||
| 1780 | |||
| 1781 | test('should handle scoped macro inside first argument when macro has multiple args', async ({ page }) => { | ||
| 1782 | // setvar has two args: name and value. Test scoped in value position. | ||
| 1783 | const input = '{{setvar::myvar::prefix-{{reverse}}abc{{/reverse}}-suffix}}{{getvar::myvar}}'; | ||
| 1784 | const output = await evaluateWithEngine(page, input); | ||
| 1785 | expect(output).toBe('prefix-cba-suffix'); | ||
| 1786 | }); | ||
| 1787 | |||
| 1788 | test('should handle multiline scoped content inside argument', async ({ page }) => { | ||
| 1789 | const input = '{{setvar::testvar::{{if true}}\ntrue\nbranch\n{{/if}}}} {{getvar::testvar}}'; | ||
| 1790 | const output = await evaluateWithEngine(page, input); | ||
| 1791 | expect(output).toBe(' true\nbranch'); | ||
| 1792 | }); | ||
| 1793 | }); | ||
| 1592 | }); | 1794 | }); |
| 1593 | 1795 | ||
| 1594 | test.describe('{{if}} conditional macro', () => { | 1796 | test.describe('{{if}} conditional macro', () => { |