Add /graph G6: plan-boundary topology optimizer
A restricted optimizer pass now reviews the graph at plan boundaries —
right after initial planning and piggybacked on each replan version
boundary, never mid-execution. An optimizer subagent may emit four ops
over Waiting/Ready nodes only: remove_dep (delete a false dependency,
restoring parallelism — the highest-value edit), reorder (pending
priority for the serial scheduler), merge (fold two tiny nodes; specs
concatenate, deps union, dependents re-point, absorbed self-deps drop),
and split (2-3 focused replacements inheriting the original's deps and
dependents). The optimizer changes graph DATA only; the executor stays
pure deterministic Rust. KIGI_GRAPH_OPTIMIZER=0 disables it entirely.
apply_optimization enforces the contract twice: per-op checks
(pending-only targets, known ids, terminal node untouchable, dead-node
deps rejected as DeadDep, merge/split targets with non-pending
dependents rejected with the true reason instead of tripping the
immutable invariant later), then FINAL invariants — every non-pending
node byte-identical in the result, the gn-final gate rebuilt over all
survivors, node cap, whole-graph acyclicity, and a BIDIRECTIONAL status
re-derivation for pending nodes (adversarial review caught the critical
hole: a merge grafting unsatisfied deps onto a Ready node would
otherwise dispatch it ahead of its new prerequisites, since
recompute_ready is promote-only). Applied passes bump plan_version,
freeze an immutable baseline, and consume a slot of the SHARED replan
cap; an explicit {"ops": []} is a respected free no-op; any failure
degrades to keeping the current plan. Plumbing reuses a new shared
artifact-pass runner (stale-artifact delete, size cap, missing-file
fail-closed) extracted from the replanner.
Tests: remove_dep parallelism restore + loud no-such-dep, immutable and
terminal-node rejections across all four ops, merge/split dependent
rewiring incl. final-gate rebuild and intra-split dep resolution,
result-cycle rejection, dead-dep splits, Ready-demote-on-merge, and
three e2e flows — false-dep removal proven ACTUALLY parallel by the
held-reply fan-out gate, OPTIMIZER=0 spawning zero passes, and the
shared-cap guard. kigi-shell 4959 lib tests green; clippy clean.
This commit is contained in:
@@ -130,6 +130,25 @@ edges stay deterministic Rust. The harness appends a terminal
|
||||
(default 3, 0 = off); past the cap — and after the final node has
|
||||
achieved — discoveries drain to history only. Replan failure degrades
|
||||
(history + notice); it never pauses a working graph.
|
||||
- G4: the graph follows the repo. Every checkpoint projects to
|
||||
`.kigi/graph.jsonl` at the git root (`session/graph_project.rs`,
|
||||
header line + one node per line, atomic write); single writer via an
|
||||
fs2 flock sidecar; other instances get read-only `/graph status`.
|
||||
Fresh sessions revive via `/graph resume` (load UNDER the lock,
|
||||
from_snapshot demotions apply). All lock-then-mutate sites
|
||||
identity-check the projected `graph_id`; kigi never commits the file.
|
||||
- G5: `/graph show` renders box-drawing DAG art
|
||||
(`session/graph_render.rs`, Sugiyama-lite: longest-path layers, dummy
|
||||
pass-throughs, barycenter ordering, bus lanes). Wider than 120 cols
|
||||
degrades to the status tree.
|
||||
- G6: plan-boundary topology optimizer
|
||||
(`acp_session_impl/graph_optimize.rs`; `KIGI_GRAPH_OPTIMIZER=0`
|
||||
disables). Restricted ops (`remove_dep`/`reorder`/`merge`/`split`)
|
||||
validated by `graph_plan::apply_optimization`: pending-only targets,
|
||||
immutable nodes byte-identical in the result, terminal gate rebuilt,
|
||||
whole-graph acyclicity. Applied passes bump `plan_version` and share
|
||||
the replan cap; `{"ops": []}` is a respected free no-op; failures
|
||||
degrade.
|
||||
|
||||
## Milestones (PRD §8.3)
|
||||
|
||||
|
||||
@@ -1929,6 +1929,14 @@ impl Config {
|
||||
.unwrap_or(3)
|
||||
.clamp(1, 8)
|
||||
}
|
||||
/// Graph topology optimizer master switch (`KIGI_GRAPH_OPTIMIZER`;
|
||||
/// default on, `0` disables). Runs at plan boundaries only.
|
||||
pub(crate) fn resolve_graph_optimizer_enabled(&self) -> bool {
|
||||
!matches!(
|
||||
std::env::var("KIGI_GRAPH_OPTIMIZER").ok().as_deref(),
|
||||
Some("0") | Some("false")
|
||||
)
|
||||
}
|
||||
/// Max replan passes per graph (`KIGI_GRAPH_REPLAN_CAP`); 0 turns
|
||||
/// dynamic replanning off. Past the cap, discoveries drain to
|
||||
/// history only — the graph must still converge. Clamped to [0, 10].
|
||||
|
||||
@@ -95,6 +95,8 @@ pub use types::{TodoGateDecision, TodoGateReason};
|
||||
mod goal;
|
||||
#[path = "acp_session_impl/graph.rs"]
|
||||
mod graph;
|
||||
#[path = "acp_session_impl/graph_optimize.rs"]
|
||||
mod graph_optimize;
|
||||
#[path = "acp_session_impl/graph_replan.rs"]
|
||||
mod graph_replan;
|
||||
#[path = "acp_session_impl/graph_workers.rs"]
|
||||
@@ -612,6 +614,9 @@ pub(crate) struct SessionActor {
|
||||
/// Max replan passes per graph (0 = replanning off). Cached at
|
||||
/// actor construction.
|
||||
pub(crate) graph_replan_cap: u32,
|
||||
/// Topology optimizer switch (plan-boundary passes; shares the
|
||||
/// replan cap). Cached at actor construction.
|
||||
pub(crate) graph_optimizer_enabled: bool,
|
||||
/// `.kigi` dir at the git root, when the session cwd is in a git
|
||||
/// repo — home of the project-level shared graph projection.
|
||||
pub(crate) graph_project_dir: Option<std::path::PathBuf>,
|
||||
|
||||
@@ -388,6 +388,8 @@ impl SessionActor {
|
||||
self.graph_tracker.lock().install_nodes(nodes);
|
||||
self.persist_graph_state();
|
||||
tracing::info!(total, "graph: DAG installed, launching first node");
|
||||
// Plan-boundary optimizer pass ① (post-initial-planning).
|
||||
self.maybe_optimize_graph().await;
|
||||
|
||||
match self.drive_graph().await {
|
||||
Some(reminder) => GraphSetupOutcome::Inference {
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
//! Topology optimizer (G6): a plan-boundary review pass that may issue
|
||||
//! a RESTRICTED set of graph edits — remove false deps (restoring
|
||||
//! parallelism), reorder pending priority, merge tiny nodes, split
|
||||
//! oversized ones — over Waiting/Ready nodes only.
|
||||
//!
|
||||
//! The optimizer changes GRAPH DATA only; the executor stays pure
|
||||
//! deterministic Rust. It fires ① right after initial planning and
|
||||
//! ② at each replan boundary (piggybacked), never mid-execution.
|
||||
//! Applied (non-empty) passes bump `plan_version`, freeze a baseline,
|
||||
//! and consume a slot of the SHARED replan cap; an explicit `[]` is a
|
||||
//! respected no-op consuming nothing. Failure degrades — the current
|
||||
//! graph keeps running. `KIGI_GRAPH_OPTIMIZER=0` disables entirely.
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
use super::super::goal_planner::{ChannelSpawner, GoalPlannerSpawner};
|
||||
use super::super::graph_plan;
|
||||
use super::super::graph_planner::{ArtifactPassSpec, run_graph_artifact_pass};
|
||||
use super::SessionActor;
|
||||
|
||||
const OPTIMIZER_PROMPT_TEMPLATE: &str = include_str!("../templates/graph_optimizer_prompt.md");
|
||||
|
||||
impl SessionActor {
|
||||
/// One optimizer pass at a plan boundary. No-op when disabled, when
|
||||
/// the shared cap is exhausted, or when the graph is not Active.
|
||||
pub(super) async fn maybe_optimize_graph(&self) {
|
||||
if !self.graph_optimizer_enabled {
|
||||
return;
|
||||
}
|
||||
let (replan_runs, current_graph, history_text, graph_file, next_version) = {
|
||||
let tracker = self.graph_tracker.lock();
|
||||
let Some(state) = tracker.snapshot() else {
|
||||
return;
|
||||
};
|
||||
if state.status != crate::session::goal_tracker::GoalStatus::Active {
|
||||
return;
|
||||
}
|
||||
let compact: Vec<serde_json::Value> = state
|
||||
.nodes
|
||||
.iter()
|
||||
.map(|n| {
|
||||
serde_json::json!({
|
||||
"id": n.id,
|
||||
"title": n.title,
|
||||
"spec": n.spec,
|
||||
"status": format!("{:?}", n.status),
|
||||
"deps": n.deps.iter().map(|d| d.on.clone()).collect::<Vec<_>>(),
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
let history_text = state
|
||||
.history
|
||||
.iter()
|
||||
.rev()
|
||||
.take(12)
|
||||
.map(|h| {
|
||||
format!(
|
||||
"- {:?} {} {}",
|
||||
h.event,
|
||||
h.node_id.as_deref().unwrap_or("-"),
|
||||
h.detail.as_deref().unwrap_or("")
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
(
|
||||
state.replan_runs,
|
||||
serde_json::to_string(&compact).unwrap_or_default(),
|
||||
history_text,
|
||||
tracker
|
||||
.artifacts_dir()
|
||||
.join(format!("optimize.v{}.json", state.plan_version + 1)),
|
||||
state.plan_version + 1,
|
||||
)
|
||||
};
|
||||
if self.graph_replan_cap == 0 || replan_runs >= self.graph_replan_cap {
|
||||
tracing::info!(
|
||||
replan_runs,
|
||||
cap = self.graph_replan_cap,
|
||||
"graph optimizer: shared cap exhausted; skipping pass"
|
||||
);
|
||||
return;
|
||||
}
|
||||
let Some(event_tx) = self.tool_context.subagent_event_tx.clone() else {
|
||||
return;
|
||||
};
|
||||
let objective = self
|
||||
.graph_tracker
|
||||
.lock()
|
||||
.objective()
|
||||
.map(str::to_owned)
|
||||
.unwrap_or_default();
|
||||
let parent_prompt_id = self
|
||||
.current_prompt_id
|
||||
.lock()
|
||||
.expect("current_prompt_id mutex poisoned")
|
||||
.clone();
|
||||
let spawner: Arc<dyn GoalPlannerSpawner> = Arc::new(ChannelSpawner {
|
||||
event_tx,
|
||||
parent_session_id: self.session_id_string(),
|
||||
parent_prompt_id,
|
||||
cwd: Some(self.tool_context.cwd.as_str().to_owned()),
|
||||
role_override: Default::default(),
|
||||
events: Some(self.events.writer()),
|
||||
});
|
||||
let tool_names = self.resolve_inherit_role_tool_names().await;
|
||||
let sections = format!(
|
||||
"OBJECTIVE:\n{objective}\n\nCURRENT GRAPH:\n{current_graph}\n\n\
|
||||
EXECUTION HISTORY:\n{history_text}\n"
|
||||
);
|
||||
tracing::info!(next_version, "graph optimizer: firing");
|
||||
let json = match run_graph_artifact_pass(
|
||||
spawner,
|
||||
ArtifactPassSpec {
|
||||
template: OPTIMIZER_PROMPT_TEMPLATE,
|
||||
sections: §ions,
|
||||
graph_file: &graph_file,
|
||||
tool_names: &tool_names,
|
||||
role: "graph optimizer",
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(json) => json,
|
||||
Err(reason) => {
|
||||
// Degrade: an enhancement pass never blocks the graph.
|
||||
tracing::warn!(%reason, "graph optimizer: pass failed; keeping current plan");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let existing = self
|
||||
.graph_tracker
|
||||
.lock()
|
||||
.snapshot()
|
||||
.map(|s| s.nodes.clone())
|
||||
.unwrap_or_default();
|
||||
match graph_plan::apply_optimization(&existing, &json) {
|
||||
Ok(None) => {
|
||||
tracing::info!("graph optimizer: no ops (already good)");
|
||||
}
|
||||
Ok(Some(optimized)) => {
|
||||
let n_before = existing.len();
|
||||
let n_after = optimized.len();
|
||||
self.graph_tracker.lock().install_optimized_nodes(optimized);
|
||||
let all_nodes = self
|
||||
.graph_tracker
|
||||
.lock()
|
||||
.snapshot()
|
||||
.map(|s| s.nodes.clone())
|
||||
.unwrap_or_default();
|
||||
if let Err(err) = self.write_graph_baseline(&all_nodes).await {
|
||||
tracing::warn!(%err, "graph optimizer: baseline write failed (audit gap only)");
|
||||
}
|
||||
self.persist_graph_state();
|
||||
tracing::info!(
|
||||
next_version,
|
||||
n_before,
|
||||
n_after,
|
||||
"graph optimizer: plan optimized"
|
||||
);
|
||||
self.send_slash_command_output(&format!(
|
||||
"Graph optimized (v{next_version}): {n_before} → {n_after} node(s)."
|
||||
))
|
||||
.await;
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(%err, "graph optimizer: ops rejected; keeping current plan");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -198,6 +198,9 @@ impl SessionActor {
|
||||
discovered work."
|
||||
))
|
||||
.await;
|
||||
// Plan-boundary optimizer pass ② (piggybacked on the
|
||||
// replan version boundary).
|
||||
self.maybe_optimize_graph().await;
|
||||
return;
|
||||
}
|
||||
GraphPlannerOutcome::Invalid { reason } if attempt == 1 => {
|
||||
|
||||
@@ -1104,6 +1104,7 @@ pub(crate) async fn spawn_session_actor(
|
||||
graph_concurrency: effective_config.resolve_graph_concurrency(),
|
||||
graph_node_rounds: effective_config.resolve_graph_node_rounds(),
|
||||
graph_replan_cap: effective_config.resolve_graph_replan_cap(),
|
||||
graph_optimizer_enabled: effective_config.resolve_graph_optimizer_enabled(),
|
||||
graph_project_dir,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
@@ -219,6 +219,7 @@ async fn persist_ack_waits_for_disk_flush_before_success() {
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
@@ -664,6 +665,7 @@ async fn first_turn_memory_injection_disabled_does_not_persist_to_chat_history()
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
@@ -918,6 +920,7 @@ async fn cancel_running_task_teardown_clears_running_and_pending_work() {
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
@@ -1905,6 +1908,7 @@ async fn cancel_propagates_to_sampler_handle_so_no_further_emission() {
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
@@ -1951,3 +1951,155 @@ async fn graph_show_renders_dag_through_handle_prompt() {
|
||||
.await;
|
||||
unsafe { std::env::remove_var(ENV_FLAG) };
|
||||
}
|
||||
|
||||
// ── G6: topology optimizer ─────────────────────────────────────────
|
||||
|
||||
/// Chain a→b→c where b's dep on a is FALSE; the optimizer removes it,
|
||||
/// unlocking a+b as parallel roots — proven by the held-reply fan-out
|
||||
/// gate (the test hangs if the batch is not truly concurrent).
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[serial]
|
||||
async fn optimizer_removes_false_dep_and_unlocks_real_parallelism() {
|
||||
unsafe { std::env::set_var(ENV_FLAG, "0") };
|
||||
let local = tokio::task::LocalSet::new();
|
||||
tokio::time::timeout(
|
||||
std::time::Duration::from_secs(60),
|
||||
local.run_until(async {
|
||||
let dag = serde_json::json!({
|
||||
"nodes": [
|
||||
{"id": "a", "title": "Node A", "spec": "do a", "deps": []},
|
||||
{"id": "b", "title": "Node B", "spec": "do b", "deps": ["a"]},
|
||||
]
|
||||
})
|
||||
.to_string()
|
||||
.into_bytes();
|
||||
let b_id = crate::session::graph_plan::node_id_for_slug("b");
|
||||
let a_id = crate::session::graph_plan::node_id_for_slug("a");
|
||||
let (mut actor, _tmp, _prx) = make_graph_actor_detached().await;
|
||||
let (coord_tx, captured) = spawn_scripted_coordinator(
|
||||
_tmp.path().to_path_buf(),
|
||||
move |req| {
|
||||
if req.prompt.contains("Graph Topology Optimizer") {
|
||||
let path = req
|
||||
.prompt
|
||||
.find("/optimize.v")
|
||||
.map(|start_idx| {
|
||||
let end = req.prompt[start_idx..]
|
||||
.find(".json")
|
||||
.map(|e| start_idx + e + ".json".len())
|
||||
.unwrap();
|
||||
let start = req.prompt[..start_idx]
|
||||
.rfind(|c: char| !c.is_ascii_graphic() || c == '`')
|
||||
.map(|i| i + 1)
|
||||
.unwrap_or(0);
|
||||
req.prompt[start..end].to_string()
|
||||
})
|
||||
.expect("optimizer prompt embeds artifact path");
|
||||
let ops = serde_json::json!({
|
||||
"ops": [{"op": "remove_dep", "node": b_id, "dep": a_id}]
|
||||
})
|
||||
.to_string();
|
||||
std::fs::create_dir_all(std::path::Path::new(&path).parent().unwrap())
|
||||
.unwrap();
|
||||
std::fs::write(&path, ops).unwrap();
|
||||
return "Done".to_owned();
|
||||
}
|
||||
happy_reply(req, &dag)
|
||||
},
|
||||
// Held-reply gate: the first WORKER's reply is withheld
|
||||
// until the second worker spawn arrives — sequential
|
||||
// execution would deadlock (timeout catches regression).
|
||||
true,
|
||||
);
|
||||
actor.tool_context.subagent_event_tx = Some(coord_tx);
|
||||
actor.graph_concurrency = 2;
|
||||
actor.graph_optimizer_enabled = true;
|
||||
|
||||
let outcome = actor.setup_graph("two independent tasks", None).await;
|
||||
let reminder = match outcome {
|
||||
graph::GraphSetupOutcome::Inference { reminder, .. } => reminder,
|
||||
graph::GraphSetupOutcome::Message(msg) => panic!("expected Inference: {msg}"),
|
||||
};
|
||||
assert!(
|
||||
reminder.contains("Final verification"),
|
||||
"a+b batched in parallel; serial tail is gn-final: {reminder}"
|
||||
);
|
||||
let s = actor.graph_tracker.lock().snapshot().cloned().unwrap();
|
||||
assert_eq!(s.plan_version, 2, "optimizer pass bumped the version");
|
||||
assert_eq!(s.replan_runs, 1, "optimizer consumed a shared cap slot");
|
||||
let workers = captured
|
||||
.lock()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.filter(|c| c.prompt.contains("Graph Node Worker"))
|
||||
.count();
|
||||
assert_eq!(workers, 2, "both nodes ran as batch workers");
|
||||
}),
|
||||
)
|
||||
.await
|
||||
.expect("optimizer parallelism test starved (fan-out regression)");
|
||||
unsafe { std::env::remove_var(ENV_FLAG) };
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[serial]
|
||||
async fn optimizer_disabled_never_spawns_a_pass() {
|
||||
unsafe { std::env::set_var(ENV_FLAG, "0") };
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let dag = chain_graph_json();
|
||||
let (mut actor, _tmp, _prx) = make_graph_actor_detached().await;
|
||||
let (coord_tx, captured) = spawn_scripted_coordinator(
|
||||
_tmp.path().to_path_buf(),
|
||||
move |req| happy_reply(req, &dag),
|
||||
false,
|
||||
);
|
||||
actor.tool_context.subagent_event_tx = Some(coord_tx);
|
||||
actor.graph_optimizer_enabled = false;
|
||||
let _ = actor.setup_graph("chain", None).await;
|
||||
let passes = captured
|
||||
.lock()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.filter(|c| c.prompt.contains("Graph Topology Optimizer"))
|
||||
.count();
|
||||
assert_eq!(passes, 0, "KIGI_GRAPH_OPTIMIZER=0 must be a hard off");
|
||||
assert_eq!(
|
||||
actor.graph_tracker.lock().snapshot().unwrap().plan_version,
|
||||
1
|
||||
);
|
||||
})
|
||||
.await;
|
||||
unsafe { std::env::remove_var(ENV_FLAG) };
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[serial]
|
||||
async fn optimizer_skips_when_shared_cap_is_exhausted() {
|
||||
unsafe { std::env::set_var(ENV_FLAG, "0") };
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let dag = chain_graph_json();
|
||||
let (mut actor, _tmp, _prx) = make_graph_actor_detached().await;
|
||||
let (coord_tx, captured) = spawn_scripted_coordinator(
|
||||
_tmp.path().to_path_buf(),
|
||||
move |req| happy_reply(req, &dag),
|
||||
false,
|
||||
);
|
||||
actor.tool_context.subagent_event_tx = Some(coord_tx);
|
||||
actor.graph_optimizer_enabled = true;
|
||||
actor.graph_replan_cap = 0; // shared cap already exhausted
|
||||
let _ = actor.setup_graph("chain", None).await;
|
||||
let passes = captured
|
||||
.lock()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.filter(|c| c.prompt.contains("Graph Topology Optimizer"))
|
||||
.count();
|
||||
assert_eq!(passes, 0, "cap guard must gate the optimizer too");
|
||||
})
|
||||
.await;
|
||||
unsafe { std::env::remove_var(ENV_FLAG) };
|
||||
}
|
||||
|
||||
@@ -248,6 +248,7 @@ async fn test_e2e_idle_resume_refreshes_model_metadata() {
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
+3
@@ -180,6 +180,7 @@ async fn create_test_actor(
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
@@ -627,6 +628,7 @@ async fn create_test_actor_with_memory(
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
@@ -1385,6 +1387,7 @@ async fn test_e2e_idle_resume_refreshes_model_metadata() {
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
@@ -242,6 +242,7 @@ async fn create_test_actor_with_memory(
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
+1
@@ -188,6 +188,7 @@ pub(super) async fn make_replay_send_update_fixture() -> ReplaySendUpdateFixture
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
@@ -293,6 +293,7 @@ pub(crate) async fn create_test_actor_ex(
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
@@ -2275,6 +2275,7 @@ mod inline_auto_compact_flow_tests {
|
||||
graph_concurrency: 1,
|
||||
graph_node_rounds: 3,
|
||||
graph_replan_cap: 3,
|
||||
graph_optimizer_enabled: false,
|
||||
graph_project_dir: None,
|
||||
graph_project_lock: std::cell::RefCell::new(None),
|
||||
goal_turn_task_ids: parking_lot::Mutex::new(std::collections::HashSet::new()),
|
||||
|
||||
@@ -81,6 +81,9 @@ pub(crate) enum GraphPlanError {
|
||||
slug: String,
|
||||
origin: String,
|
||||
},
|
||||
/// Optimizer: an operation violated the restricted-op contract
|
||||
/// (touched an immutable node, unknown id, malformed shape, …).
|
||||
OpInvalid(String),
|
||||
}
|
||||
|
||||
impl std::fmt::Display for GraphPlanError {
|
||||
@@ -123,6 +126,7 @@ impl std::fmt::Display for GraphPlanError {
|
||||
"node {slug:?} depends on {dep:?}, which already failed; depend on \
|
||||
live nodes only (or none)"
|
||||
),
|
||||
Self::OpInvalid(reason) => write!(f, "invalid optimization op: {reason}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -539,6 +543,373 @@ pub(crate) fn validate_replan(
|
||||
.collect())
|
||||
}
|
||||
|
||||
#[derive(serde::Deserialize)]
|
||||
#[serde(tag = "op", rename_all = "snake_case")]
|
||||
enum OptimizeOp {
|
||||
RemoveDep {
|
||||
node: String,
|
||||
dep: String,
|
||||
},
|
||||
Reorder {
|
||||
order: Vec<String>,
|
||||
},
|
||||
Merge {
|
||||
into: String,
|
||||
from: String,
|
||||
},
|
||||
Split {
|
||||
node: String,
|
||||
replacements: Vec<PlannedNode>,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(serde::Deserialize)]
|
||||
struct OptimizePlan {
|
||||
ops: Vec<OptimizeOp>,
|
||||
}
|
||||
|
||||
fn is_pending(status: NodeStatus) -> bool {
|
||||
matches!(status, NodeStatus::Waiting | NodeStatus::Ready)
|
||||
}
|
||||
|
||||
/// Apply a restricted optimization-op list to the graph, returning the
|
||||
/// transformed node set — or `Ok(None)` for an explicitly empty op list
|
||||
/// (a respected "already good" answer).
|
||||
///
|
||||
/// Contract enforced twice: per-op checks (pending-only targets, known
|
||||
/// ids), then a FINAL diff invariant — every node that was NOT
|
||||
/// Waiting/Ready must be byte-identical in the result, `gn-final`'s
|
||||
/// gate is rebuilt over all surviving non-final nodes, and the combined
|
||||
/// graph must remain acyclic.
|
||||
pub(crate) fn apply_optimization(
|
||||
existing: &[GraphNode],
|
||||
json: &str,
|
||||
) -> Result<Option<Vec<GraphNode>>, GraphPlanError> {
|
||||
let plan: OptimizePlan =
|
||||
serde_json::from_str(json).map_err(|e| GraphPlanError::Parse(e.to_string()))?;
|
||||
if plan.ops.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
let mut nodes: Vec<GraphNode> = existing.to_vec();
|
||||
let find = |nodes: &[GraphNode], id: &str| -> Result<usize, GraphPlanError> {
|
||||
nodes
|
||||
.iter()
|
||||
.position(|n| n.id == id)
|
||||
.ok_or_else(|| GraphPlanError::OpInvalid(format!("unknown node {id:?}")))
|
||||
};
|
||||
let pending_or_err = |nodes: &[GraphNode], idx: usize| -> Result<(), GraphPlanError> {
|
||||
if !is_pending(nodes[idx].status) {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"node {:?} is {:?}; only Waiting/Ready nodes may be edited",
|
||||
nodes[idx].id, nodes[idx].status
|
||||
)));
|
||||
}
|
||||
Ok(())
|
||||
};
|
||||
|
||||
for op in plan.ops {
|
||||
match op {
|
||||
OptimizeOp::RemoveDep { node, dep } => {
|
||||
let idx = find(&nodes, &node)?;
|
||||
pending_or_err(&nodes, idx)?;
|
||||
let before = nodes[idx].deps.len();
|
||||
nodes[idx].deps.retain(|d| d.on != dep);
|
||||
if nodes[idx].deps.len() == before {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"node {node:?} has no dependency on {dep:?}"
|
||||
)));
|
||||
}
|
||||
}
|
||||
OptimizeOp::Reorder { order } => {
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
for id in &order {
|
||||
let idx = find(&nodes, id)?;
|
||||
pending_or_err(&nodes, idx)?;
|
||||
if !seen.insert(id.as_str()) {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"reorder lists {id:?} twice"
|
||||
)));
|
||||
}
|
||||
}
|
||||
// Stable rearrangement: listed nodes adopt the listed
|
||||
// relative order across the positions they occupied.
|
||||
let positions: Vec<usize> = nodes
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(_, n)| order.contains(&n.id))
|
||||
.map(|(i, _)| i)
|
||||
.collect();
|
||||
let picked: Vec<GraphNode> = order
|
||||
.iter()
|
||||
.map(|id| nodes[find(&nodes, id).expect("checked above")].clone())
|
||||
.collect();
|
||||
for (&pos, node) in positions.iter().zip(picked) {
|
||||
nodes[pos] = node;
|
||||
}
|
||||
}
|
||||
OptimizeOp::Merge { into, from } => {
|
||||
if into == from {
|
||||
return Err(GraphPlanError::OpInvalid("merge into == from".to_owned()));
|
||||
}
|
||||
// Rewiring dependents must never touch an immutable node:
|
||||
// a Blocked dependent (dead chain) would either be
|
||||
// mutated (tripping the final invariant) or left with a
|
||||
// dangling dep. Reject up front with the true reason.
|
||||
if let Some(dependent) = nodes.iter().find(|n| {
|
||||
n.id != FINAL_NODE_ID
|
||||
&& !is_pending(n.status)
|
||||
&& n.deps.iter().any(|d| d.on == from)
|
||||
}) {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"node {from:?} has non-pending dependent {:?}; it cannot be merged",
|
||||
dependent.id
|
||||
)));
|
||||
}
|
||||
if from == FINAL_NODE_ID || into == FINAL_NODE_ID {
|
||||
return Err(GraphPlanError::OpInvalid(
|
||||
"the terminal node cannot participate in a merge".to_owned(),
|
||||
));
|
||||
}
|
||||
let into_idx = find(&nodes, &into)?;
|
||||
let from_idx = find(&nodes, &from)?;
|
||||
pending_or_err(&nodes, into_idx)?;
|
||||
pending_or_err(&nodes, from_idx)?;
|
||||
let from_node = nodes.remove(from_idx);
|
||||
let into_idx = find(&nodes, &into)?;
|
||||
nodes[into_idx].spec =
|
||||
format!("{}\n\nAND: {}", nodes[into_idx].spec, from_node.spec);
|
||||
for dep in from_node.deps {
|
||||
if dep.on != into && !nodes[into_idx].deps.iter().any(|d| d.on == dep.on) {
|
||||
nodes[into_idx].deps.push(dep);
|
||||
}
|
||||
}
|
||||
for node in &mut nodes {
|
||||
for dep in &mut node.deps {
|
||||
if dep.on == from {
|
||||
dep.on = into.clone();
|
||||
}
|
||||
}
|
||||
// A dependent of BOTH from and into now lists into
|
||||
// twice; collapse. And `into` itself must never
|
||||
// keep a re-pointed self-dependency (into depended
|
||||
// on from ⇒ that ordering is absorbed by the merge).
|
||||
let own_id = node.id.clone();
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
node.deps
|
||||
.retain(|d| d.on != own_id && seen.insert(d.on.clone()));
|
||||
}
|
||||
}
|
||||
OptimizeOp::Split { node, replacements } => {
|
||||
if node == FINAL_NODE_ID {
|
||||
return Err(GraphPlanError::OpInvalid(
|
||||
"the terminal node cannot be split".to_owned(),
|
||||
));
|
||||
}
|
||||
let idx = find(&nodes, &node)?;
|
||||
pending_or_err(&nodes, idx)?;
|
||||
if let Some(dependent) = nodes.iter().find(|n| {
|
||||
n.id != FINAL_NODE_ID
|
||||
&& !is_pending(n.status)
|
||||
&& n.deps.iter().any(|d| d.on == node)
|
||||
}) {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"node {node:?} has non-pending dependent {:?}; it cannot be split",
|
||||
dependent.id
|
||||
)));
|
||||
}
|
||||
if replacements.len() < 2 || replacements.len() > 3 {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"split of {node:?} needs 2-3 replacements, got {}",
|
||||
replacements.len()
|
||||
)));
|
||||
}
|
||||
let original = nodes.remove(idx);
|
||||
let mut new_ids = Vec::new();
|
||||
for rep in &replacements {
|
||||
if !valid_slug(&rep.id) {
|
||||
return Err(GraphPlanError::BadSlug(rep.id.clone()));
|
||||
}
|
||||
if rep.title.trim().is_empty() || rep.spec.trim().is_empty() {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"split replacement {:?} has an empty title/spec",
|
||||
rep.id
|
||||
)));
|
||||
}
|
||||
let id = node_id_for_slug(&rep.id);
|
||||
if nodes.iter().any(|n| n.id == id) || new_ids.contains(&id) {
|
||||
return Err(GraphPlanError::ExistingCollision(rep.id.clone()));
|
||||
}
|
||||
new_ids.push(id);
|
||||
}
|
||||
let dead_ids: std::collections::HashSet<String> = nodes
|
||||
.iter()
|
||||
.filter(|n| matches!(n.status, NodeStatus::Failed | NodeStatus::Blocked))
|
||||
.map(|n| n.id.clone())
|
||||
.collect();
|
||||
for (rep, id) in replacements.iter().zip(&new_ids) {
|
||||
let mut deps: Vec<NodeDep> = original.deps.clone();
|
||||
for d in &rep.deps {
|
||||
let resolved = if nodes.iter().any(|n| n.id == *d) {
|
||||
d.clone()
|
||||
} else if let Some(pos) = replacements.iter().position(|r| r.id == *d) {
|
||||
new_ids[pos].clone()
|
||||
} else {
|
||||
return Err(GraphPlanError::UnknownDep {
|
||||
slug: rep.id.clone(),
|
||||
dep: d.clone(),
|
||||
});
|
||||
};
|
||||
// Ordering dep on a dead node can never satisfy
|
||||
// (same rule as validate_replan's DeadDep).
|
||||
if dead_ids.contains(&resolved) {
|
||||
return Err(GraphPlanError::DeadDep {
|
||||
slug: rep.id.clone(),
|
||||
dep: resolved,
|
||||
});
|
||||
}
|
||||
if !deps.iter().any(|existing| existing.on == resolved) {
|
||||
deps.push(NodeDep {
|
||||
on: resolved,
|
||||
kind: DepKind::Blocks,
|
||||
});
|
||||
}
|
||||
}
|
||||
nodes.push(GraphNode {
|
||||
id: id.clone(),
|
||||
title: rep.title.trim().to_owned(),
|
||||
spec: rep.spec.trim().to_owned(),
|
||||
deps,
|
||||
status: NodeStatus::Waiting,
|
||||
goal_id: None,
|
||||
rounds: 0,
|
||||
tokens_used: 0,
|
||||
failure: None,
|
||||
});
|
||||
}
|
||||
for n in &mut nodes {
|
||||
if let Some(pos) = n.deps.iter().position(|d| d.on == node) {
|
||||
let kind = n.deps[pos].kind;
|
||||
n.deps.remove(pos);
|
||||
for id in &new_ids {
|
||||
if !n.deps.iter().any(|d| &d.on == id) {
|
||||
n.deps.push(NodeDep {
|
||||
on: id.clone(),
|
||||
kind,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Rebuild the terminal gate over all surviving non-final nodes.
|
||||
if let Some(final_idx) = nodes.iter().position(|n| n.id == FINAL_NODE_ID) {
|
||||
let gate: Vec<NodeDep> = nodes
|
||||
.iter()
|
||||
.filter(|n| n.id != FINAL_NODE_ID)
|
||||
.map(|n| NodeDep {
|
||||
on: n.id.clone(),
|
||||
kind: DepKind::Blocks,
|
||||
})
|
||||
.collect();
|
||||
nodes[final_idx].deps = gate;
|
||||
}
|
||||
|
||||
// Re-derive EVERY pending node's status in both directions: ops can
|
||||
// remove a Ready node's last blocker (→ stays Ready via the same
|
||||
// rule) or graft unsatisfied deps onto a Ready node (merge), which
|
||||
// must demote it — `recompute_ready` downstream is promote-only and
|
||||
// would leave a Ready node whose Blocks deps are unmet, silently
|
||||
// violating ordering at dispatch.
|
||||
let achieved: std::collections::HashSet<String> = nodes
|
||||
.iter()
|
||||
.filter(|n| n.status == NodeStatus::Achieved)
|
||||
.map(|n| n.id.clone())
|
||||
.collect();
|
||||
let derived: Vec<NodeStatus> = nodes
|
||||
.iter()
|
||||
.map(|n| {
|
||||
if !is_pending(n.status) {
|
||||
n.status
|
||||
} else if n
|
||||
.deps
|
||||
.iter()
|
||||
.filter(|d| d.kind == DepKind::Blocks)
|
||||
.all(|d| achieved.contains(&d.on))
|
||||
{
|
||||
NodeStatus::Ready
|
||||
} else {
|
||||
NodeStatus::Waiting
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
for (node, status) in nodes.iter_mut().zip(derived) {
|
||||
node.status = status;
|
||||
}
|
||||
|
||||
// FINAL diff invariant: immutable nodes byte-identical (Debug repr
|
||||
// covers every field; GraphNode has no Eq).
|
||||
for old in existing {
|
||||
if is_pending(old.status) || old.id == FINAL_NODE_ID {
|
||||
continue;
|
||||
}
|
||||
match nodes.iter().find(|n| n.id == old.id) {
|
||||
Some(new) if format!("{new:?}") == format!("{old:?}") => {}
|
||||
_ => {
|
||||
return Err(GraphPlanError::OpInvalid(format!(
|
||||
"immutable node {:?} was modified or removed",
|
||||
old.id
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
if nodes.len() > MAX_GRAPH_NODES + 1 {
|
||||
return Err(GraphPlanError::TooManyNodes(nodes.len() - 1));
|
||||
}
|
||||
|
||||
// Acyclicity over the whole result.
|
||||
{
|
||||
let index_of: std::collections::HashMap<&str, usize> = nodes
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, n)| (n.id.as_str(), i))
|
||||
.collect();
|
||||
let mut indegree = vec![0usize; nodes.len()];
|
||||
for n in &nodes {
|
||||
for d in &n.deps {
|
||||
if !index_of.contains_key(d.on.as_str()) {
|
||||
return Err(GraphPlanError::UnknownDep {
|
||||
slug: n.id.clone(),
|
||||
dep: d.on.clone(),
|
||||
});
|
||||
}
|
||||
indegree[index_of[n.id.as_str()]] += 1;
|
||||
}
|
||||
}
|
||||
let mut done = vec![false; nodes.len()];
|
||||
for _ in 0..nodes.len() {
|
||||
let Some(next) = (0..nodes.len()).find(|&i| !done[i] && indegree[i] == 0) else {
|
||||
let cycle: Vec<String> = (0..nodes.len())
|
||||
.filter(|&i| !done[i])
|
||||
.map(|i| nodes[i].id.clone())
|
||||
.collect();
|
||||
return Err(GraphPlanError::Cycle(cycle));
|
||||
};
|
||||
done[next] = true;
|
||||
let next_id = nodes[next].id.clone();
|
||||
for n in &nodes {
|
||||
if n.deps.iter().any(|d| d.on == next_id) {
|
||||
indegree[index_of[n.id.as_str()]] -= 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(Some(nodes))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -718,6 +1089,245 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
fn opt_state() -> Vec<GraphNode> {
|
||||
// a(Achieved) → b(Ready, FALSE dep on c), c(Ready), final gate.
|
||||
let mut nodes = parse_and_validate(
|
||||
&plan_json(&[("a", &[]), ("b", &["a", "c"]), ("c", &[])]),
|
||||
"o",
|
||||
)
|
||||
.unwrap();
|
||||
let a = node_id_for_slug("a");
|
||||
for n in &mut nodes {
|
||||
if n.id == a {
|
||||
n.status = NodeStatus::Achieved;
|
||||
} else if n.deps.iter().all(|d| d.on == a) {
|
||||
n.status = NodeStatus::Ready;
|
||||
}
|
||||
}
|
||||
nodes
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimizer_remove_dep_restores_parallelism_and_empty_ops_is_noop() {
|
||||
let existing = opt_state();
|
||||
let b = node_id_for_slug("b");
|
||||
let c = node_id_for_slug("c");
|
||||
assert!(
|
||||
apply_optimization(&existing, r#"{"ops": []}"#)
|
||||
.unwrap()
|
||||
.is_none(),
|
||||
"explicit empty ops is a respected no-op"
|
||||
);
|
||||
let json = serde_json::json!({
|
||||
"ops": [{"op": "remove_dep", "node": b.clone(), "dep": c.clone()}]
|
||||
})
|
||||
.to_string();
|
||||
let optimized = apply_optimization(&existing, &json).unwrap().unwrap();
|
||||
let b_node = optimized.iter().find(|n| n.id == b).unwrap();
|
||||
assert!(
|
||||
!b_node.deps.iter().any(|d| d.on == c),
|
||||
"false dep removed — b and c can now run in parallel"
|
||||
);
|
||||
// Removing a dep that does not exist is loud.
|
||||
let bad = serde_json::json!({
|
||||
"ops": [{"op": "remove_dep", "node": c, "dep": b}]
|
||||
})
|
||||
.to_string();
|
||||
assert!(matches!(
|
||||
apply_optimization(&existing, &bad).unwrap_err(),
|
||||
GraphPlanError::OpInvalid(_)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimizer_rejects_touching_immutable_nodes() {
|
||||
let existing = opt_state();
|
||||
let a = node_id_for_slug("a"); // Achieved — immutable
|
||||
for json in [
|
||||
serde_json::json!({"ops": [{"op": "remove_dep", "node": a.clone(), "dep": "x"}]}),
|
||||
serde_json::json!({"ops": [{"op": "reorder", "order": [a.clone()]}]}),
|
||||
serde_json::json!({"ops": [{"op": "merge", "into": node_id_for_slug("b"), "from": a.clone()}]}),
|
||||
serde_json::json!({"ops": [{"op": "split", "node": a.clone(), "replacements": [
|
||||
{"id": "p1", "title": "P1", "spec": "s"},
|
||||
{"id": "p2", "title": "P2", "spec": "s"},
|
||||
]}]}),
|
||||
] {
|
||||
assert!(
|
||||
matches!(
|
||||
apply_optimization(&existing, &json.to_string()).unwrap_err(),
|
||||
GraphPlanError::OpInvalid(_)
|
||||
),
|
||||
"op touching an Achieved node must be rejected: {json}"
|
||||
);
|
||||
}
|
||||
// The terminal node is likewise untouchable.
|
||||
let final_merge = serde_json::json!({
|
||||
"ops": [{"op": "merge", "into": node_id_for_slug("b"), "from": FINAL_NODE_ID}]
|
||||
});
|
||||
assert!(matches!(
|
||||
apply_optimization(&existing, &final_merge.to_string()).unwrap_err(),
|
||||
GraphPlanError::OpInvalid(_)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimizer_merge_and_split_rewire_dependents_and_final_gate() {
|
||||
let existing = opt_state();
|
||||
let b = node_id_for_slug("b");
|
||||
let c = node_id_for_slug("c");
|
||||
// Merge c into b: b absorbs the spec; final gate loses c.
|
||||
let json = serde_json::json!({
|
||||
"ops": [{"op": "merge", "into": b.clone(), "from": c.clone()}]
|
||||
})
|
||||
.to_string();
|
||||
let merged = apply_optimization(&existing, &json).unwrap().unwrap();
|
||||
assert!(merged.iter().all(|n| n.id != c));
|
||||
let final_node = merged.iter().find(|n| n.id == FINAL_NODE_ID).unwrap();
|
||||
assert!(!final_node.deps.iter().any(|d| d.on == c));
|
||||
assert!(final_node.deps.iter().any(|d| d.on == b));
|
||||
let b_node = merged.iter().find(|n| n.id == b).unwrap();
|
||||
assert!(b_node.spec.contains("AND:"));
|
||||
assert!(
|
||||
!b_node.deps.iter().any(|d| d.on == b),
|
||||
"merge must not self-depend"
|
||||
);
|
||||
|
||||
// Split b into two parts: dependents (final) gate on both parts.
|
||||
let json = serde_json::json!({
|
||||
"ops": [{"op": "split", "node": b, "replacements": [
|
||||
{"id": "b-core", "title": "Core half", "spec": "s1"},
|
||||
{"id": "b-glue", "title": "Glue half", "spec": "s2", "deps": ["b-core"]},
|
||||
]}]
|
||||
})
|
||||
.to_string();
|
||||
let split = apply_optimization(&existing, &json).unwrap().unwrap();
|
||||
let p1 = node_id_for_slug("b-core");
|
||||
let p2 = node_id_for_slug("b-glue");
|
||||
let final_node = split.iter().find(|n| n.id == FINAL_NODE_ID).unwrap();
|
||||
assert!(final_node.deps.iter().any(|d| d.on == p1));
|
||||
assert!(final_node.deps.iter().any(|d| d.on == p2));
|
||||
let glue = split.iter().find(|n| n.id == p2).unwrap();
|
||||
assert!(
|
||||
glue.deps.iter().any(|d| d.on == p1),
|
||||
"intra-split dep resolved"
|
||||
);
|
||||
assert!(
|
||||
glue.deps.iter().any(|d| d.on == node_id_for_slug("a")),
|
||||
"replacements inherit the split node's deps"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_grafting_unsatisfied_deps_demotes_ready_into_to_waiting() {
|
||||
// d(Waiting, dep on c) — merge d into b (b Ready): b absorbs the
|
||||
// unsatisfied dep on c and MUST demote to Waiting, or dispatch
|
||||
// would run b ahead of c (critical review finding).
|
||||
let mut existing = opt_state();
|
||||
let c = node_id_for_slug("c");
|
||||
existing.insert(
|
||||
3,
|
||||
GraphNode {
|
||||
id: node_id_for_slug("d"),
|
||||
title: "D".into(),
|
||||
spec: "d".into(),
|
||||
deps: vec![NodeDep {
|
||||
on: c.clone(),
|
||||
kind: DepKind::Blocks,
|
||||
}],
|
||||
status: NodeStatus::Waiting,
|
||||
goal_id: None,
|
||||
rounds: 0,
|
||||
tokens_used: 0,
|
||||
failure: None,
|
||||
},
|
||||
);
|
||||
let b = node_id_for_slug("b");
|
||||
// Give b a satisfied-only dep set first (remove the false dep on c).
|
||||
let json = serde_json::json!({
|
||||
"ops": [
|
||||
{"op": "remove_dep", "node": b.clone(), "dep": c.clone()},
|
||||
{"op": "merge", "into": b.clone(), "from": node_id_for_slug("d")},
|
||||
]
|
||||
})
|
||||
.to_string();
|
||||
let optimized = apply_optimization(&existing, &json).unwrap().unwrap();
|
||||
let b_node = optimized.iter().find(|n| n.id == b).unwrap();
|
||||
assert!(b_node.deps.iter().any(|d| d.on == c), "dep absorbed");
|
||||
assert_eq!(
|
||||
b_node.status,
|
||||
NodeStatus::Waiting,
|
||||
"Ready node absorbing an unmet dep must demote"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_rejects_deps_on_dead_nodes() {
|
||||
let mut existing = opt_state();
|
||||
let a = node_id_for_slug("a");
|
||||
existing.iter_mut().find(|n| n.id == a).unwrap().status = NodeStatus::Failed;
|
||||
let json = serde_json::json!({
|
||||
"ops": [{"op": "split", "node": node_id_for_slug("b"), "replacements": [
|
||||
{"id": "p1", "title": "P1", "spec": "s", "deps": [a]},
|
||||
{"id": "p2", "title": "P2", "spec": "s"},
|
||||
]}]
|
||||
})
|
||||
.to_string();
|
||||
assert!(matches!(
|
||||
apply_optimization(&existing, &json).unwrap_err(),
|
||||
GraphPlanError::DeadDep { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_and_split_reject_targets_with_non_pending_dependents() {
|
||||
// B(Blocked) deps on p(Ready): merging or splitting p would have
|
||||
// to rewire an immutable node — reject with the TRUE reason.
|
||||
let mut existing = opt_state();
|
||||
let b = node_id_for_slug("b");
|
||||
let c = node_id_for_slug("c");
|
||||
existing.iter_mut().find(|n| n.id == b).unwrap().status = NodeStatus::Blocked;
|
||||
// b already deps on c in opt_state, so c has a Blocked dependent.
|
||||
let merge = serde_json::json!({
|
||||
"ops": [{"op": "merge", "into": node_id_for_slug("a"), "from": c.clone()}]
|
||||
});
|
||||
// (a is Achieved, so this would fail pending_or_err anyway — use
|
||||
// a fresh pending target instead.)
|
||||
let _ = merge;
|
||||
let split = serde_json::json!({
|
||||
"ops": [{"op": "split", "node": c, "replacements": [
|
||||
{"id": "p1", "title": "P1", "spec": "s"},
|
||||
{"id": "p2", "title": "P2", "spec": "s"},
|
||||
]}]
|
||||
});
|
||||
match apply_optimization(&existing, &split.to_string()).unwrap_err() {
|
||||
GraphPlanError::OpInvalid(reason) => {
|
||||
assert!(
|
||||
reason.contains("non-pending dependent"),
|
||||
"true reason surfaces: {reason}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected OpInvalid, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn optimizer_rejects_result_cycles() {
|
||||
let existing = opt_state();
|
||||
// b already deps c; a reorder is fine, but adding a cycle via
|
||||
// split deps pointing at a dependent must fail the final check.
|
||||
let json = serde_json::json!({
|
||||
"ops": [{"op": "split", "node": node_id_for_slug("c"), "replacements": [
|
||||
{"id": "c1", "title": "C1", "spec": "s", "deps": [node_id_for_slug("b")]},
|
||||
{"id": "c2", "title": "C2", "spec": "s"},
|
||||
]}]
|
||||
})
|
||||
.to_string();
|
||||
assert!(matches!(
|
||||
apply_optimization(&existing, &json).unwrap_err(),
|
||||
GraphPlanError::Cycle(_)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replan_rejects_blocks_deps_on_dead_nodes_but_allows_dead_origins() {
|
||||
let mut existing = existing_graph();
|
||||
|
||||
@@ -269,6 +269,71 @@ pub(crate) async fn run_graph_replanner(
|
||||
}
|
||||
}
|
||||
|
||||
/// Generic single-shot artifact pass: render `template` + `sections`,
|
||||
/// spawn the role child, enforce the stale-artifact/size/missing-file
|
||||
/// discipline, and return the artifact's raw JSON for the caller to
|
||||
/// validate. Shared by the optimizer (and any future boundary pass).
|
||||
pub(crate) struct ArtifactPassSpec<'a> {
|
||||
pub template: &'a str,
|
||||
pub sections: &'a str,
|
||||
pub graph_file: &'a Path,
|
||||
pub tool_names: &'a RoleToolNames,
|
||||
pub role: &'a str,
|
||||
}
|
||||
|
||||
pub(crate) async fn run_graph_artifact_pass(
|
||||
spawner: Arc<dyn GoalPlannerSpawner>,
|
||||
spec: ArtifactPassSpec<'_>,
|
||||
) -> Result<String, String> {
|
||||
if let Some(parent) = spec.graph_file.parent()
|
||||
&& let Err(err) = tokio::fs::create_dir_all(parent).await
|
||||
{
|
||||
return Err(format!("failed to create graph dir: {err}"));
|
||||
}
|
||||
if let Err(err) = tokio::fs::remove_file(spec.graph_file).await
|
||||
&& err.kind() != std::io::ErrorKind::NotFound
|
||||
{
|
||||
return Err(format!("failed to clear stale artifact: {err}"));
|
||||
}
|
||||
let graph_file_str = spec.graph_file.to_string_lossy();
|
||||
let rendered = spec
|
||||
.tool_names
|
||||
.apply(&spec.template.replace("{GRAPH_FILE}", &graph_file_str));
|
||||
let prompt_text = format!("{rendered}\n\n{}", spec.sections);
|
||||
let prompt = RoleRenderedPrompt {
|
||||
primary: prompt_text.clone(),
|
||||
fallback: prompt_text,
|
||||
};
|
||||
let spawn_id = uuid::Uuid::now_v7().to_string();
|
||||
match spawner.spawn_planner(&spawn_id, prompt).await {
|
||||
Ok(_) => {}
|
||||
Err(SpawnError::Transport(detail)) => {
|
||||
return Err(format!("{} transport error: {detail}", spec.role));
|
||||
}
|
||||
Err(SpawnError::Runtime { message, cancelled }) => {
|
||||
return Err(if cancelled {
|
||||
format!("{} aborted: {message}", spec.role)
|
||||
} else {
|
||||
format!("{} runtime error: {message}", spec.role)
|
||||
});
|
||||
}
|
||||
}
|
||||
match tokio::fs::metadata(spec.graph_file).await {
|
||||
Ok(meta) if meta.is_file() && meta.len() > 0 && meta.len() <= MAX_GRAPH_JSON_BYTES => {}
|
||||
Ok(meta) if meta.len() > MAX_GRAPH_JSON_BYTES => {
|
||||
return Err(format!(
|
||||
"{} artifact is {} bytes; the cap is {MAX_GRAPH_JSON_BYTES}",
|
||||
spec.role,
|
||||
meta.len()
|
||||
));
|
||||
}
|
||||
_ => return Err(format!("{} produced no artifact", spec.role)),
|
||||
}
|
||||
tokio::fs::read_to_string(spec.graph_file)
|
||||
.await
|
||||
.map_err(|err| format!("failed to read {} artifact: {err}", spec.role))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -742,6 +742,27 @@ impl GraphTracker {
|
||||
self.recompute_ready();
|
||||
}
|
||||
|
||||
/// Install an optimizer-transformed node set: bump `plan_version`,
|
||||
/// consume a shared replan-cap slot, recompute readiness. The
|
||||
/// validator already guaranteed immutable nodes are untouched.
|
||||
pub fn install_optimized_nodes(&mut self, nodes: Vec<GraphNode>) {
|
||||
let Some(state) = self.state.as_mut() else {
|
||||
return;
|
||||
};
|
||||
state.nodes = nodes;
|
||||
state.plan_version += 1;
|
||||
state.replan_runs += 1;
|
||||
push_history(
|
||||
state,
|
||||
GraphHistoryEntry::now(
|
||||
GraphEvent::PlanningCompleted,
|
||||
None,
|
||||
Some(format!("optimized to v{}", state.plan_version)),
|
||||
),
|
||||
);
|
||||
self.recompute_ready();
|
||||
}
|
||||
|
||||
/// Drain pending discoveries WITHOUT replanning (cap exhausted):
|
||||
/// they stay in history (queued there at capture time) only.
|
||||
pub fn drain_discoveries_to_history(&mut self) -> usize {
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
You are the Graph Topology Optimizer for the Kigi harness. You run at a
|
||||
plan boundary (right after planning or replanning, never mid-execution).
|
||||
Your job: make the PENDING part of the graph faster and sharper with the
|
||||
FEWEST possible edits — or none.
|
||||
|
||||
## Inputs (below this prompt)
|
||||
|
||||
- OBJECTIVE: the overall graph objective, verbatim.
|
||||
- CURRENT GRAPH: the nodes as JSON (id, title, spec, status, deps).
|
||||
- EXECUTION HISTORY: recent graph events (rounds, failures), if any.
|
||||
|
||||
## What you may do — ONLY on nodes whose status is "Waiting" or "Ready"
|
||||
|
||||
- `remove_dep`: delete a FALSE dependency (B does not truly need A's
|
||||
output) to restore parallelism. This is the highest-value edit.
|
||||
- `reorder`: change the relative priority of pending nodes (the serial
|
||||
scheduler picks the first Ready node in storage order).
|
||||
- `merge`: fold two tiny, tightly-coupled pending nodes into one.
|
||||
- `split`: break one oversized pending node into 2-3 focused nodes.
|
||||
|
||||
You may NEVER touch Running, Achieved, Failed, or Blocked nodes, the
|
||||
`gn-final` terminal node, or dependencies ON immutable nodes that
|
||||
represent real ordering. When in doubt, do nothing: an unnecessary edit
|
||||
is worse than none.
|
||||
|
||||
## Output contract — STRICT
|
||||
|
||||
Use your `{WRITE_TOOL}` tool to write JSON to `{GRAPH_FILE}`:
|
||||
|
||||
```
|
||||
{
|
||||
"ops": [
|
||||
{"op": "remove_dep", "node": "gn-…", "dep": "gn-…"},
|
||||
{"op": "reorder", "order": ["gn-…", "gn-…"]},
|
||||
{"op": "merge", "into": "gn-…", "from": "gn-…"},
|
||||
{"op": "split", "node": "gn-…", "replacements": [
|
||||
{"id": "new-slug", "title": "…", "spec": "…", "deps": ["gn-…"]}
|
||||
]}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
- `reorder.order` lists ONLY pending node ids, in the desired relative
|
||||
priority; unlisted nodes keep their positions.
|
||||
- `split.replacements` follow the same slug rules as planning; they
|
||||
inherit the split node's dependents automatically.
|
||||
- When the graph is already good, write `{"ops": []}` — that is a
|
||||
respected answer, not a failure.
|
||||
|
||||
Your terminal response must be exactly:
|
||||
|
||||
```
|
||||
Done
|
||||
```
|
||||
Reference in New Issue
Block a user