Simplify Context: inline HashMap in fabro-core, extension trait in fabro-workflows

Remove the ContextStore trait, InMemoryStore, and Context::with_store() from
fabro-core — put the HashMap directly in Context. Replace the duplicate
fabro-workflows Context struct with a re-export of fabro_core::Context, and
move domain accessors (fidelity, run_id, preamble, thread_id) to a
WorkflowContext extension trait. Eliminate the bridge layer (WfContextStore,
bridge_context, WorkflowContextExt) entirely since there is now one Context
type. Rename clone_context() to fork() for clarity.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-24 16:26:05 -04:00
parent e9ff887ace
commit 685ba9fdbc
No known key found for this signature in database
15 changed files with 79 additions and 447 deletions

View file

@ -3,80 +3,28 @@ use std::sync::{Arc, RwLock};
use serde_json::Value;
pub trait ContextStore: Send + Sync {
fn set(&self, key: String, value: Value);
fn get(&self, key: &str) -> Option<Value>;
fn snapshot(&self) -> HashMap<String, Value>;
fn fork(&self) -> Arc<dyn ContextStore>;
}
pub struct InMemoryStore {
data: RwLock<HashMap<String, Value>>,
}
impl InMemoryStore {
pub fn new() -> Self {
Self {
data: RwLock::new(HashMap::new()),
}
}
}
impl Default for InMemoryStore {
fn default() -> Self {
Self::new()
}
}
impl ContextStore for InMemoryStore {
fn set(&self, key: String, value: Value) {
self.data.write().unwrap().insert(key, value);
}
fn get(&self, key: &str) -> Option<Value> {
self.data.read().unwrap().get(key).cloned()
}
fn snapshot(&self) -> HashMap<String, Value> {
self.data.read().unwrap().clone()
}
fn fork(&self) -> Arc<dyn ContextStore> {
let cloned = self.data.read().unwrap().clone();
Arc::new(InMemoryStore {
data: RwLock::new(cloned),
})
}
}
#[derive(Clone)]
#[derive(Clone, Default)]
pub struct Context {
store: Arc<dyn ContextStore>,
}
impl Default for Context {
fn default() -> Self {
Self::new()
}
values: Arc<RwLock<HashMap<String, Value>>>,
}
impl Context {
pub fn new() -> Self {
Self::default()
}
pub fn from_values(values: HashMap<String, Value>) -> Self {
Self {
store: Arc::new(InMemoryStore::new()),
values: Arc::new(RwLock::new(values)),
}
}
pub fn with_store(store: Arc<dyn ContextStore>) -> Self {
Self { store }
}
pub fn set(&self, key: impl Into<String>, value: Value) {
self.store.set(key.into(), value);
self.values.write().unwrap().insert(key.into(), value);
}
pub fn get(&self, key: &str) -> Option<Value> {
self.store.get(key)
self.values.read().unwrap().get(key).cloned()
}
pub fn get_string(&self, key: &str, default: &str) -> String {
@ -86,18 +34,21 @@ impl Context {
}
pub fn apply_updates(&self, updates: &HashMap<String, Value>) {
let mut values = self.values.write().unwrap();
for (k, v) in updates {
self.store.set(k.clone(), v.clone());
values.insert(k.clone(), v.clone());
}
}
pub fn snapshot(&self) -> HashMap<String, Value> {
self.store.snapshot()
self.values.read().unwrap().clone()
}
pub fn clone_context(&self) -> Self {
/// Deep copy for parallel branch isolation.
/// `.clone()` shares state (Arc clone); `.fork()` creates an independent copy.
pub fn fork(&self) -> Self {
Self {
store: self.store.fork(),
values: Arc::new(RwLock::new(self.snapshot())),
}
}
@ -117,36 +68,6 @@ impl Context {
mod tests {
use super::*;
use serde_json::json;
use std::sync::atomic::{AtomicUsize, Ordering};
#[test]
fn in_memory_store_set_and_get() {
let store = InMemoryStore::new();
store.set("k".into(), json!("v"));
assert_eq!(store.get("k"), Some(json!("v")));
assert_eq!(store.get("missing"), None);
}
#[test]
fn in_memory_store_snapshot_is_independent() {
let store = InMemoryStore::new();
store.set("a".into(), json!(1));
let snap = store.snapshot();
store.set("b".into(), json!(2));
assert!(!snap.contains_key("b"));
assert_eq!(snap.len(), 1);
}
#[test]
fn in_memory_store_fork() {
let store = InMemoryStore::new();
store.set("x".into(), json!(10));
let forked = store.fork();
forked.set("y".into(), json!(20));
assert!(store.get("y").is_none());
assert_eq!(forked.get("x"), Some(json!(10)));
assert_eq!(forked.get("y"), Some(json!(20)));
}
#[test]
fn context_set_and_get() {
@ -180,59 +101,24 @@ mod tests {
assert_eq!(ctx.get("b"), Some(json!(2)));
}
#[test]
fn context_clone_is_independent() {
let ctx = Context::new();
ctx.set("x", json!(1));
let cloned = ctx.clone_context();
cloned.set("x", json!(2));
assert_eq!(ctx.get("x"), Some(json!(1)));
assert_eq!(cloned.get("x"), Some(json!(2)));
}
#[test]
fn context_with_custom_store() {
struct CountingStore {
inner: InMemoryStore,
set_count: AtomicUsize,
}
impl ContextStore for CountingStore {
fn set(&self, key: String, value: Value) {
self.set_count.fetch_add(1, Ordering::Relaxed);
self.inner.set(key, value);
}
fn get(&self, key: &str) -> Option<Value> {
self.inner.get(key)
}
fn snapshot(&self) -> HashMap<String, Value> {
self.inner.snapshot()
}
fn fork(&self) -> Arc<dyn ContextStore> {
self.inner.fork()
}
}
let store = Arc::new(CountingStore {
inner: InMemoryStore::new(),
set_count: AtomicUsize::new(0),
});
let ctx = Context::with_store(store.clone());
ctx.set("k", json!(1));
ctx.set("k2", json!(2));
assert_eq!(store.set_count.load(Ordering::Relaxed), 2);
assert_eq!(ctx.get("k"), Some(json!(1)));
}
#[test]
fn context_fork_is_independent() {
let ctx = Context::new();
ctx.set("shared", json!("original"));
let forked = ctx.clone_context();
let forked = ctx.fork();
forked.set("shared", json!("modified"));
assert_eq!(ctx.get("shared"), Some(json!("original")));
assert_eq!(forked.get("shared"), Some(json!("modified")));
}
#[test]
fn context_from_values() {
let mut vals = HashMap::new();
vals.insert("k".into(), json!("v"));
let ctx = Context::from_values(vals);
assert_eq!(ctx.get("k"), Some(json!("v")));
}
#[test]
fn context_current_node_id() {
let ctx = Context::new();

View file

@ -12,7 +12,7 @@ pub mod state;
#[cfg(test)]
pub mod test_fixtures;
pub use context::{Context, ContextStore, InMemoryStore};
pub use context::Context;
pub use error::{CoreError, HandlerErrorDetail, Result};
pub use executor::{Executor, ExecutorBuilder, ExecutorSettings};
pub use graph::{EdgeSelection, EdgeSpec, Graph, NodeSpec};

View file

@ -12,7 +12,7 @@ use fabro_llm::client::Client;
use fabro_model::FallbackTarget;
use fabro_model::Provider;
use crate::context::Context;
use crate::context::{Context, WorkflowContext};
use crate::cost::compute_stage_cost;
use crate::error::FabroError;
use crate::event::WorkflowRunEvent;

View file

@ -1,148 +1,42 @@
pub mod keys;
use std::collections::HashMap;
use std::sync::{Arc, RwLock};
pub use fabro_core::Context;
use serde_json::Value;
use fabro_graphviz::Fidelity;
/// Thread-safe key-value context shared across pipeline stages.
#[derive(Debug, Clone)]
pub struct Context {
values: Arc<RwLock<HashMap<String, Value>>>,
/// Domain-specific typed accessors for workflow context values.
pub trait WorkflowContext {
fn fidelity(&self) -> Fidelity;
fn thread_id(&self) -> Option<String>;
fn preamble(&self) -> String;
fn run_id(&self) -> String;
}
impl Default for Context {
fn default() -> Self {
Self::new()
}
}
impl Context {
#[must_use]
pub fn new() -> Self {
Self {
values: Arc::new(RwLock::new(HashMap::new())),
}
}
/// Set a key-value pair in the context.
///
/// # Panics
///
/// Panics if the internal lock is poisoned.
pub fn set(&self, key: impl Into<String>, value: Value) {
self.values
.write()
.expect("context lock poisoned")
.insert(key.into(), value);
}
/// Get a value by key, returning None if not present.
///
/// # Panics
///
/// Panics if the internal lock is poisoned.
#[must_use]
pub fn get(&self, key: &str) -> Option<Value> {
self.values
.read()
.expect("context lock poisoned")
.get(key)
.cloned()
}
/// Get a value as a string, returning the default if not present or not a string.
#[must_use]
pub fn get_string(&self, key: &str, default: &str) -> String {
self.get(key)
.and_then(|v| v.as_str().map(String::from))
.unwrap_or_else(|| default.to_string())
}
/// Return a snapshot (clone) of all current context values.
///
/// # Panics
///
/// Panics if the internal lock is poisoned.
#[must_use]
pub fn snapshot(&self) -> HashMap<String, Value> {
self.values.read().expect("context lock poisoned").clone()
}
/// Deep copy for parallel branch isolation.
#[must_use]
pub fn clone_context(&self) -> Self {
let values = self.snapshot();
Self {
values: Arc::new(RwLock::new(values)),
}
}
/// Merge a map of updates into the context.
///
/// # Panics
///
/// Panics if the internal lock is poisoned.
pub fn apply_updates(&self, updates: &HashMap<String, Value>) {
let mut values = self.values.write().expect("context lock poisoned");
for (key, value) in updates {
values.insert(key.clone(), value.clone());
}
}
// --- Internal accessors for bridge code ---
pub(crate) fn from_values(values: HashMap<String, Value>) -> Self {
Self {
values: Arc::new(RwLock::new(values)),
}
}
pub(crate) fn values_arc(&self) -> Arc<RwLock<HashMap<String, Value>>> {
self.values.clone()
}
// --- Typed accessors ---
#[must_use]
pub fn run_id(&self) -> String {
self.get_string(keys::INTERNAL_RUN_ID, "unknown")
}
#[must_use]
pub fn fidelity(&self) -> keys::Fidelity {
impl WorkflowContext for Context {
fn fidelity(&self) -> Fidelity {
self.get_string(keys::INTERNAL_FIDELITY, "")
.parse()
.unwrap_or_default()
}
#[must_use]
pub fn preamble(&self) -> String {
self.get_string(keys::CURRENT_PREAMBLE, "")
}
#[must_use]
pub fn thread_id(&self) -> Option<String> {
fn thread_id(&self) -> Option<String> {
self.get(keys::INTERNAL_THREAD_ID)
.and_then(|v| v.as_str().map(String::from))
}
#[must_use]
pub fn node_visit_count(&self) -> usize {
self.get(keys::INTERNAL_NODE_VISIT_COUNT)
.and_then(|v| v.as_u64())
.unwrap_or(1) as usize
fn preamble(&self) -> String {
self.get_string(keys::CURRENT_PREAMBLE, "")
}
#[must_use]
pub fn current_node_id(&self) -> String {
self.get_string(keys::CURRENT_NODE, "")
fn run_id(&self) -> String {
self.get_string(keys::INTERNAL_RUN_ID, "unknown")
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
#[test]
fn new_context_is_empty() {
@ -189,24 +83,20 @@ mod tests {
ctx.set("a", serde_json::json!(1));
let snap = ctx.snapshot();
ctx.set("b", serde_json::json!(2));
// snapshot should not contain "b"
assert!(snap.contains_key("a"));
assert!(!snap.contains_key("b"));
}
#[test]
fn clone_context_is_independent() {
fn fork_is_independent() {
let ctx = Context::new();
ctx.set("shared", serde_json::json!("original"));
let cloned = ctx.clone_context();
cloned.set("shared", serde_json::json!("modified"));
let forked = ctx.fork();
forked.set("shared", serde_json::json!("modified"));
// original should be unchanged
assert_eq!(ctx.get("shared"), Some(serde_json::json!("original")));
// cloned has the modification
assert_eq!(cloned.get("shared"), Some(serde_json::json!("modified")));
assert_eq!(forked.get("shared"), Some(serde_json::json!("modified")));
}
#[test]
@ -291,7 +181,9 @@ mod tests {
#[test]
fn node_visit_count_default() {
let ctx = Context::new();
assert_eq!(ctx.node_visit_count(), 1);
// fabro-core returns 0 for missing; workflow code expects 1 as default
// when used in workflow context. The raw core accessor returns 0.
assert_eq!(ctx.node_visit_count(), 0);
}
#[test]

View file

@ -1,137 +0,0 @@
use std::collections::HashMap;
use std::sync::{Arc, RwLock};
use fabro_core::context::{Context as CoreContext, ContextStore};
use serde_json::Value;
use crate::context::keys;
use crate::context::Context as WfContext;
/// A ContextStore implementation that delegates to a wf::Context's internal values map.
struct WfContextStore {
values: Arc<RwLock<HashMap<String, Value>>>,
}
impl ContextStore for WfContextStore {
fn set(&self, key: String, value: Value) {
self.values
.write()
.expect("context lock poisoned")
.insert(key, value);
}
fn get(&self, key: &str) -> Option<Value> {
self.values
.read()
.expect("context lock poisoned")
.get(key)
.cloned()
}
fn snapshot(&self) -> HashMap<String, Value> {
self.values.read().expect("context lock poisoned").clone()
}
fn fork(&self) -> Arc<dyn ContextStore> {
let cloned = self.values.read().expect("context lock poisoned").clone();
Arc::new(WfContextStore {
values: Arc::new(RwLock::new(cloned)),
})
}
}
/// Create a fabro_core::Context that shares the same underlying values
/// as the given wf::Context. Writes through either are visible to both.
pub fn bridge_context(wf_ctx: &WfContext) -> CoreContext {
let store = Arc::new(WfContextStore {
values: wf_ctx.values_arc(),
});
CoreContext::with_store(store)
}
/// Extension trait providing typed domain accessors on a fabro_core::Context.
pub trait WorkflowContextExt {
fn run_id(&self) -> String;
fn fidelity(&self) -> keys::Fidelity;
fn preamble(&self) -> String;
fn thread_id(&self) -> Option<String>;
}
impl WorkflowContextExt for CoreContext {
fn run_id(&self) -> String {
self.get_string(keys::INTERNAL_RUN_ID, "unknown")
}
fn fidelity(&self) -> keys::Fidelity {
self.get_string(keys::INTERNAL_FIDELITY, "")
.parse()
.unwrap_or_default()
}
fn preamble(&self) -> String {
self.get_string(keys::CURRENT_PREAMBLE, "")
}
fn thread_id(&self) -> Option<String> {
self.get(keys::INTERNAL_THREAD_ID)
.and_then(|v| v.as_str().map(String::from))
}
}
#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;
#[test]
fn bridge_shares_values() {
let wf = WfContext::new();
let core = bridge_context(&wf);
// Set via wf, read via core
wf.set("key1", json!("from_wf"));
assert_eq!(core.get("key1"), Some(json!("from_wf")));
// Set via core, read via wf
core.set("key2", json!("from_core"));
assert_eq!(wf.get("key2"), Some(json!("from_core")));
}
#[test]
fn bridge_fork_is_independent() {
let wf = WfContext::new();
wf.set("shared", json!("original"));
let core = bridge_context(&wf);
let forked = core.clone_context();
// Write to fork should not affect original
forked.set("shared", json!("modified"));
assert_eq!(wf.get("shared"), Some(json!("original")));
assert_eq!(core.get("shared"), Some(json!("original")));
assert_eq!(forked.get("shared"), Some(json!("modified")));
}
#[test]
fn workflow_context_ext_accessors() {
let wf = WfContext::new();
wf.set(keys::INTERNAL_RUN_ID, json!("run-42"));
wf.set(keys::INTERNAL_FIDELITY, json!("full"));
wf.set(keys::CURRENT_PREAMBLE, json!("You are a helpful assistant"));
wf.set(keys::INTERNAL_THREAD_ID, json!("thread-1"));
let core = bridge_context(&wf);
assert_eq!(core.run_id(), "run-42");
assert_eq!(core.fidelity(), keys::Fidelity::Full);
assert_eq!(core.preamble(), "You are a helpful assistant");
assert_eq!(core.thread_id(), Some("thread-1".to_string()));
}
#[test]
fn workflow_context_ext_defaults() {
let core = CoreContext::new();
assert_eq!(core.run_id(), "unknown");
assert_eq!(core.fidelity(), keys::Fidelity::Compact);
assert_eq!(core.preamble(), "");
assert_eq!(core.thread_id(), None);
}
}

View file

@ -1,11 +1,11 @@
use std::collections::HashMap;
use std::sync::Arc;
use fabro_core::context::Context as CoreContext;
use fabro_core::error::{CoreError, Result as CoreResult};
use fabro_core::graph::{EdgeSelection, EdgeSpec, Graph, NodeSpec};
use fabro_graphviz::graph::types::{Edge as GvEdge, Graph as GvGraph, Node as GvNode};
use crate::context::Context;
use crate::engine;
use crate::outcome::{Outcome, StageUsage};
@ -101,16 +101,12 @@ impl Graph for WorkflowGraph {
&self,
node: &Self::Node,
outcome: &Outcome,
context: &CoreContext,
context: &Context,
) -> Option<EdgeSelection<Self>> {
// Build a wf Context from the core context snapshot so edge conditions
// that read context values (e.g. `context.failure_class=budget_exhausted`)
// evaluate correctly.
let wf_context = crate::context::Context::from_values(context.snapshot());
let selection = engine::select_edge(
node.inner(),
outcome,
&wf_context,
context,
self.inner(),
node.inner().selection(),
);

View file

@ -5,12 +5,13 @@ use std::sync::Arc;
use async_trait::async_trait;
use futures::FutureExt;
use fabro_core::context::Context as CoreContext;
use fabro_core::error::{CoreError, HandlerErrorDetail, Result as CoreResult};
use fabro_core::handler::NodeHandler;
use fabro_core::outcome::FailureCategory;
use fabro_core::retry::RetryPolicy as CoreRetryPolicy;
use crate::context::Context;
use super::graph::WorkflowGraph;
use super::WorkflowNode;
use crate::engine;
@ -20,8 +21,8 @@ use crate::outcome::{Outcome, StageStatus};
/// Production node handler that bridges fabro-core's NodeHandler to the
/// existing fabro-workflows Handler trait via EngineServices.
///
/// On each `execute()` call, snapshots the CoreContext into a WfContext,
/// runs the handler, then diffs and applies changes back.
/// On each `execute()` call, forks the context, runs the handler,
/// then diffs and applies changes back.
pub struct WorkflowNodeHandler {
pub services: Arc<EngineServices>,
pub run_dir: PathBuf,
@ -33,16 +34,15 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
async fn execute(
&self,
node: &WorkflowNode,
context: &CoreContext,
context: &Context,
_graph: &WorkflowGraph,
) -> CoreResult<Outcome> {
let gv_node = node.inner();
let handler = self.services.registry.resolve(gv_node);
// Per-call snapshot/apply context bridge:
// 1. Snapshot the CoreContext into a WfContext
// Fork the context so handler writes don't leak back unless we diff+apply.
let snapshot = context.snapshot();
let wf_context = crate::context::Context::from_values(snapshot.clone());
let wf_context = context.fork();
// Timeout from the node
let node_timeout = gv_node.timeout();
@ -75,8 +75,8 @@ impl NodeHandler<WorkflowGraph> for WorkflowNodeHandler {
panic_safe.await
};
// 2. After handler returns, diff the WfContext against the snapshot
// and apply changes back to the CoreContext
// 2. After handler returns, diff the forked context against the snapshot
// and apply changes back to the original context
let new_values = wf_context.snapshot();
for (k, v) in &new_values {
if snapshot.get(k) != Some(v) {
@ -154,7 +154,7 @@ mod tests {
async fn execute(
&self,
_node: &WorkflowNode,
_context: &CoreContext,
_context: &Context,
_graph: &WorkflowGraph,
) -> CoreResult<Outcome> {
Ok(Outcome::success())

View file

@ -87,16 +87,13 @@ impl RunLifecycle<WorkflowGraph> for FidelityLifecycle {
);
// 4. Preamble building: if Full, empty preamble; otherwise build from context
let preamble = {
let wf_context = crate::context::Context::from_values(state.context.snapshot());
build_preamble(
fidelity,
&wf_context,
&self.graph,
&state.completed_nodes,
&state.node_outcomes,
)
};
let preamble = build_preamble(
fidelity,
&state.context,
&self.graph,
&state.completed_nodes,
&state.node_outcomes,
);
state
.context
.set(keys::CURRENT_PREAMBLE, serde_json::json!(preamble));

View file

@ -1,9 +1,7 @@
pub mod context;
pub mod graph;
pub mod handler;
pub mod lifecycle;
pub use context::{bridge_context, WorkflowContextExt};
pub use graph::{WorkflowEdge, WorkflowGraph, WorkflowNode};
pub use handler::WorkflowNodeHandler;
pub use lifecycle::WorkflowLifecycle;

View file

@ -1664,7 +1664,7 @@ impl WorkflowRunEngine {
match result {
Ok((core_outcome, final_state)) => {
// Extract the executor's final context so callers see all state
let ctx = Context::from_values(final_state.context.snapshot());
let ctx = final_state.context.clone();
Ok((core_outcome, ctx))
}
Err(fabro_core::CoreError::StallTimeout { node_id }) => {

View file

@ -6,7 +6,7 @@ use async_trait::async_trait;
use fabro_agent::Sandbox;
use crate::context::keys;
use crate::context::Context;
use crate::context::{Context, WorkflowContext};
use crate::error::FabroError;
use crate::event::EventEmitter;
use crate::outcome::{Outcome, OutcomeExt, StageUsage};

View file

@ -8,7 +8,7 @@ use async_trait::async_trait;
use crate::condition::evaluate_condition;
use crate::context::keys;
use crate::context::Context;
use crate::context::{Context, WorkflowContext};
use crate::engine::{RunConfig, WorkflowRunEngine};
use crate::error::FabroError;
use crate::outcome::{Outcome, OutcomeExt, StageStatus};
@ -161,7 +161,7 @@ impl Handler for SubWorkflowHandler {
};
// Clone parent context for child; inject parent preamble
let child_context = context.clone_context();
let child_context = context.fork();
let parent_preamble = context.preamble();
if !parent_preamble.is_empty() {
child_context.set(

View file

@ -7,7 +7,7 @@ use fabro_agent::{Sandbox, WorktreeConfig, WorktreeSandbox};
use tokio::sync::Semaphore;
use crate::context::keys;
use crate::context::Context;
use crate::context::{Context, WorkflowContext};
use crate::engine::set_hook_node;
use crate::error::FabroError;
use crate::event::WorkflowRunEvent;
@ -73,7 +73,7 @@ impl Handler for ParallelHandler {
let target_id = &edge.to;
if let Some(target_node) = graph.nodes.get(target_id) {
let handler = services.registry.resolve(target_node);
let branch_context = context.clone_context();
let branch_context = context.fork();
let outcome = super::dispatch_handler(
handler,
target_node,
@ -196,7 +196,7 @@ impl Handler for ParallelHandler {
let mut branch_setups: Vec<BranchSetup> = Vec::new();
for (branch_index, edge) in branches.iter().enumerate() {
let target_id = edge.to.clone();
let branch_context = context.clone_context();
let branch_context = context.fork();
let (branch_sandbox, worktree_path): (Arc<dyn Sandbox>, Option<PathBuf>) = if let (
Some(ref gs),

View file

@ -5,7 +5,7 @@ use async_trait::async_trait;
use fabro_model::Provider;
use crate::context::keys;
use crate::context::Context;
use crate::context::{Context, WorkflowContext};
use crate::error::FabroError;
use crate::outcome::Outcome;
use fabro_graphviz::graph::{Graph, Node};

View file

@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet};
use crate::artifact::{artifact_path, format_artifact_reference};
use crate::context::keys;
use crate::context::Context;
use crate::context::{Context, WorkflowContext};
use crate::outcome::Outcome;
use crate::outcome::OutcomeExt;
use fabro_graphviz::graph::{is_llm_handler_type, Graph, Node};