Fix `{{pick}}` macro seeding inside delayed-resolution macros like `{{if}}` (#4986) - Thread `contextOffset` through `MacroEngine.evaluate()` to preserve global document position - Update `resolve()` in execution context to pass caller's `globalOffset` as `contextOffset` - Add `offsetDelta` option to `resolve()` for additional offset customization - Add regression tests for `{{pick}}` inside `{{if}}` blocks with `delayArgResolution` - Ensure identical picks at different positions produce different results

5c2a02a128218a8460f92b67f39c2a746851bc08

Wolfsblvt <wolfsblvt@gmail.com>

Signed

Parents: ad860f4447c77d8893a19481c789a2f7129bfaa3

public/scripts/macros/engine/MacroEngine.js+6 -2 Viewed

+     *
      * @param {string} input - The input string to evaluate.
      * @param {MacroEnv} env - The environment to pass to the macro handler.
+     * @param {Object} [options={}] - Optional evaluation settings.
+     * @param {number} [options.contextOffset=0] - Base offset from the original top-level document.
+     *        Used when evaluating nested content (via resolve() in handlers) to preserve global
+     *        positioning for macros like {{pick}} that seed on position.
      * @returns {string} The resolved string.
      */
-    evaluate(input, env) {
+    evaluate(input, env, { contextOffset = 0 } = {}) {
         if (!input) {
             return '';
+        }

         try {
             evaluated = MacroCstWalker.evaluateDocument({
                 text: preProcessed,
-                contextOffset: 0,
+                contextOffset,
                 cst,
                 env: safeEnv,
                 resolveMacro: this.#resolveMacro.bind(this),

public/scripts/macros/engine/MacroRegistry.js+7 -2 Viewed

  *           seeding (e.g., in {{pick}}) to ensure identical macros at different positions produce different results.
  * @property {(value: any) => string} normalize - Normalize function to use on unsure macro results to make sure they return strings as expected.
  * @property {(content: string, options?: { trimIndent?: boolean }) => string} trimContent - Trims scoped content with optional indentation dedent. Defaults to trimming indentation.
- * @property {(text: string) => string} resolve - Evaluates macros in the given text using the same environment. Use when delayArgResolution is true.
+ * @property {(text: string, options?: { offsetDelta?: number }) => string} resolve - Evaluates macros in the given text using the same environment.
+ *           Use when delayArgResolution is true. By default, preserves the caller's globalOffset so nested
+ *           macros like {{pick}} maintain deterministic position-based behavior. Pass offsetDelta to add
+ *           an additional offset for uniqueness (e.g., to differentiate between multiple resolve calls).
  */
 /**

             globalOffset: call.globalOffset,
             normalize: MacroEngine.normalizeMacroResult.bind(MacroEngine),
             trimContent: MacroEngine.trimScopedContent.bind(MacroEngine),
-            resolve: (text) => MacroEngine.evaluate(text, call.env),
+            resolve: (text, { offsetDelta = 0 } = {}) => MacroEngine.evaluate(text, call.env, {
+                contextOffset: call.globalOffset + offsetDelta,
+            }),
         };
         const result = def.handler(executionContext);

tests/frontend/MacroEngine.e2e.js+53 -0 Viewed

             // Should be one of the valid options
             expect(['X', 'Y', 'Z'].includes(outputs[0])).toBeTruthy();
         });
+        test('should use different seeds for identical picks inside different if blocks (delayArgResolution)', async ({ page }) => {
+            await registerTestablePick(page);
+            // Key regression test: picks inside {{if}} blocks use resolve() which must preserve globalOffset
+            // This tests the fix for macros with delayArgResolution that call resolve() internally
+            const output = await page.evaluate(async () => {
+                /** @type {import('../../public/scripts/macros/engine/MacroEngine.js')} */
+                const { MacroEngine } = await import('./scripts/macros/engine/MacroEngine.js');
+                /** @type {import('../../public/scripts/macros/engine/MacroEnvBuilder.js')} */
+                const { MacroEnvBuilder } = await import('./scripts/macros/engine/MacroEnvBuilder.js');
+                // Two identical pick macros inside different if blocks
+                // Before the fix, both would get contextOffset=0 when resolve() was called
+                // After the fix, resolve() passes the caller's globalOffset as contextOffset
+                const input = '{{if true}}{{testablePick::A::B::C}}{{/if}}###{{if true}}{{testablePick::A::B::C}}{{/if}}';
+                const env = MacroEnvBuilder.buildFromRawEnv({ content: input });
+                return MacroEngine.evaluate(input, env);
+            });
+            const parts = output.split('###');
+            expect(parts.length).toBe(2);
+            const seed1 = parts[0].match(/seed:([^|]+)/)?.[1];
+            const seed2 = parts[1].match(/seed:([^|]+)/)?.[1];
+            expect(seed1).toBeTruthy();
+            expect(seed2).toBeTruthy();
+            // Seeds must be different because the {{if}} blocks are at different positions
+            expect(seed1).not.toBe(seed2);
+        });
+        test('should maintain stability for picks inside if blocks across evaluations', async ({ page }) => {
+            // Picks inside if blocks should still be deterministic
+            const outputs = await page.evaluate(async () => {
+                /** @type {import('../../public/scripts/macros/engine/MacroEngine.js')} */
+                const { MacroEngine } = await import('./scripts/macros/engine/MacroEngine.js');
+                /** @type {import('../../public/scripts/macros/engine/MacroEnvBuilder.js')} */
+                const { MacroEnvBuilder } = await import('./scripts/macros/engine/MacroEnvBuilder.js');
+                const input = '{{if true}}{{pick::X::Y::Z}}{{/if}}';
+                const env1 = MacroEnvBuilder.buildFromRawEnv({ content: input });
+                const env2 = MacroEnvBuilder.buildFromRawEnv({ content: input });
+                const result1 = MacroEngine.evaluate(input, env1);
+                const result2 = MacroEngine.evaluate(input, env2);
+                return [result1, result2];
+            });
+            // Same input should produce same output (deterministic)
+            expect(outputs[0]).toBe(outputs[1]);
+            // Should be one of the valid options
+            expect(['X', 'Y', 'Z'].includes(outputs[0])).toBeTruthy();
+        });
     });
     test.describe('Dynamic macros', () => {