From aae107a5ff1ce86df2a8ef3c2b8f8bd8c7697780 Mon Sep 17 00:00:00 2001
From: MechaCat02 <fabian@diekaulbachs.de>
Date: Sun, 7 Jun 2026 20:13:28 +0200
Subject: [PATCH] fix(executor-core): F-P-004 cache AST across invoke()
 re-entry (per-Engine cache)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two paths bypassed the AST cache: LocalExecutorClient::execute (tests +
fallback) and the synchronous invoke() re-entry in the executor SDK.
The latter is the hot one — composed workflows multiplied parse cost
by depth, so a 4-deep invoke chain on a 200-line script paid the parse
budget × 4 per call.

Add a per-Engine HashMap<ScriptId, (updated_at, Arc<AST>)> + a
`compile_for_identity(script_id, updated_at, source)` helper that
behaves like LocalExecutorClient::get_or_compile but lives on the
Engine. Update the SDK invoke synchronous re-entry to:
  resolved → compile_for_identity → execute_ast

The orchestrator-core LocalExecutorClient cache (HTTP-path dispatch)
is left untouched — it caches a different access pattern at a
different boundary.

AUDIT.md anchor: F-P-004.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 crates/executor-core/src/engine.rs     | 48 ++++++++++++++++++++++++--
 crates/executor-core/src/sdk/invoke.rs | 17 +++++++--
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/crates/executor-core/src/engine.rs b/crates/executor-core/src/engine.rs
index 5d1575c..dffa141 100644
--- a/crates/executor-core/src/engine.rs
+++ b/crates/executor-core/src/engine.rs
@@ -1,10 +1,10 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap};
 use std::sync::{Arc, Mutex, OnceLock, Weak};
 use std::time::Instant;
 
-use chrono::Utc;
+use chrono::{DateTime, Utc};
 use picloud_shared::{
-    ScriptValidator, SdkCallCx, Services, TriggerEvent, ValidatedScript, ValidationError,
+    ScriptId, ScriptValidator, SdkCallCx, Services, TriggerEvent, ValidatedScript, ValidationError,
     SDK_VERSION,
 };
 use rhai::{Dynamic, Engine as RhaiEngine, EvalAltResult, Map, Module, Scope, AST};
@@ -52,6 +52,13 @@ pub struct Engine {
     /// error if invoke is called without the back-reference being set
     /// (which only happens in tests that don't wire it).
     self_weak: OnceLock<Weak<Engine>>,
+    /// F-P-004: per-Engine AST cache keyed on `(script_id, updated_at)`.
+    /// Populated by `compile_for_identity`; consumed by the SDK invoke
+    /// bridge so the synchronous re-entry path doesn't re-parse the
+    /// callee on every invoke. Independent of the orchestrator-core
+    /// `LocalExecutorClient` AST cache — that one caches HTTP-path
+    /// dispatch; this one caches function-call dispatch.
+    invoke_ast_cache: Mutex<HashMap<ScriptId, (DateTime<Utc>, Arc<AST>)>>,
 }
 
 impl Engine {
@@ -76,9 +83,44 @@ impl Engine {
             services,
             module_cache: new_module_cache(module_cache_capacity),
             self_weak: OnceLock::new(),
+            invoke_ast_cache: Mutex::new(HashMap::new()),
         }
     }
 
+    /// F-P-004: synchronous-invoke fast path. Returns a cached AST when
+    /// (script_id, updated_at) matches; otherwise compiles, inserts,
+    /// returns. Used by the `invoke` SDK bridge to skip the per-call
+    /// parse when one script calls another.
+    ///
+    /// # Errors
+    ///
+    /// Propagates `ExecError::Parse` from the inner compile step.
+    pub fn compile_for_identity(
+        &self,
+        script_id: ScriptId,
+        updated_at: DateTime<Utc>,
+        source: &str,
+    ) -> Result<Arc<AST>, ExecError> {
+        {
+            let cache = self
+                .invoke_ast_cache
+                .lock()
+                .expect("invoke ast cache poisoned");
+            if let Some((ts, ast)) = cache.get(&script_id) {
+                if *ts == updated_at {
+                    return Ok(ast.clone());
+                }
+            }
+        }
+        let ast = self.compile(source)?;
+        let mut cache = self
+            .invoke_ast_cache
+            .lock()
+            .expect("invoke ast cache poisoned");
+        cache.insert(script_id, (updated_at, ast.clone()));
+        Ok(ast)
+    }
+
     /// v1.1.9: install the back-reference used by the `invoke` SDK
     /// bridge for synchronous re-entry. Idempotent (subsequent calls
     /// are no-ops). The picloud binary calls this right after
diff --git a/crates/executor-core/src/sdk/invoke.rs b/crates/executor-core/src/sdk/invoke.rs
index 04696c6..2031bd0 100644
--- a/crates/executor-core/src/sdk/invoke.rs
+++ b/crates/executor-core/src/sdk/invoke.rs
@@ -187,10 +187,21 @@ fn invoke_blocking(
         event: None,
     };
 
-    // Synchronous re-entry — same engine instance, same Services,
-    // fresh SdkCallCx built inside Engine::execute_ast.
+    // F-P-004: synchronous re-entry — route through the per-Engine
+    // AST cache so each callee parses once per (script_id, updated_at),
+    // not once per invoke. Composed workflows multiply parse cost by
+    // depth; the cache cuts that to constant compile + N executions.
+    let ast = self_engine
+        .compile_for_identity(resolved.script_id, resolved.updated_at, &resolved.source)
+        .map_err(|e| -> Box<EvalAltResult> {
+            EvalAltResult::ErrorRuntime(
+                format!("invoke({target_label}): {e}").into(),
+                rhai::Position::NONE,
+            )
+            .into()
+        })?;
     let resp = self_engine
-        .execute(&resolved.source, req)
+        .execute_ast(&ast, req)
         .map_err(|e| -> Box<EvalAltResult> {
             EvalAltResult::ErrorRuntime(
                 format!("invoke({target_label}): {e}").into(),