feat(core-loop): scheduler decision drives harness/weight/both branch (#90)

src/closed_loop.rs

@@ -23,16 +23,28 @@
 //!    ([`crate::weights::extract_training_examples`]), runs the CPU reference
 //!    LoRA ([`crate::weights::LoraReferenceUpdater`]), and writes
 //!    `<gen_dir>/weight_update.json` with the before/after loss.
+//! 3. [`record_acted_decision`] (issue #90) annotates
+//!    `<gen_dir>/scheduler_decision.json` with the [`ActedDecision`] the
+//!    orchestrator actually executed and the weight-update summary, so the loop
+//!    surfaces what it *did*, not just what it recommended.
 //!
-//! # Hard invariant: purely additive
+//! # Closing the loop (issue #90)
 //!
-//! Nothing here mutates any pre-existing deterministic output (prompts,
-//! `context.md`, feedback context, `results.json`, `improvement.md`). Every
-//! function is panic-free and degrades to `None` / a no-op when inputs are
-//! missing, so the orchestrator's existing tests (which drive
-//! `run_generation_with` with mock fns and no telemetry/trajectory) are
-//! unaffected. The only observable additions are the two new JSON files and a
-//! console log line.
+//! Through #84 this module was **observational** — it only wrote artifacts and a
+//! log line. As of #90 the orchestrator *acts* on the decision via
+//! [`action_for_decision`]: a `weight` decision runs the weight update and
+//! short-circuits the feedback (harness) step for that generation, `both` runs
+//! both, and `harness` keeps today's behavior.
+//!
+//! The control-flow change is gated on a decision actually being produced.
+//! [`record_scheduler_decision`] still degrades to `None` when inputs are missing
+//! (no score yet, no telemetry/trajectory), and the orchestrator maps a missing
+//! decision to the harness path — so its existing tests (which drive
+//! `run_generation_with` with mock fns and no `results.json`) are unaffected and
+//! the default path stays byte-for-byte as before. Every function here remains
+//! panic-free and never mutates a pre-existing deterministic output
+//! (`context.md`, the parity-checked feedback context, `results.json`); the only
+//! additions are the JSON artifacts and the acted-decision keys.
 
 use std::path::Path;
 
@@ -391,6 +403,117 @@ pub fn maybe_run_weight_update(
     Some(outcome)
 }
 
+// --------------------------------------------------------------------------- //
+// 3. Acting on the decision (issue #90)
+// --------------------------------------------------------------------------- //
+
+/// What the orchestrator actually *did* this generation in response to the
+/// scheduler decision — issue #90 closes the loop so the recommendation drives
+/// real control flow instead of only being recorded.
+///
+/// `harness` runs the meta/feedback harness update (today's behavior); `weight`
+/// runs the weight update and **skips** the harness/feedback step; `both` runs
+/// both. The variant is derived from the scheduler decision by
+/// [`action_for_decision`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ActedDecision {
+    /// Run the harness (meta/feedback) update only.
+    Harness,
+    /// Run the weight update only; skip the harness/feedback step.
+    Weight,
+    /// Run both the weight update and the harness update.
+    Both,
+}
+
+impl ActedDecision {
+    /// Stable lower-case label for the artifact / logs.
+    pub fn as_str(self) -> &'static str {
+        match self {
+            ActedDecision::Harness => "harness",
+            ActedDecision::Weight => "weight",
+            ActedDecision::Both => "both",
+        }
+    }
+
+    /// Whether the harness (meta/feedback) update should run for this decision.
+    pub fn runs_harness(self) -> bool {
+        matches!(self, ActedDecision::Harness | ActedDecision::Both)
+    }
+
+    /// Whether the weight update should run for this decision.
+    pub fn runs_weight(self) -> bool {
+        matches!(self, ActedDecision::Weight | ActedDecision::Both)
+    }
+}
+
+/// Map a scheduler `decision` string to the action the loop takes.
+///
+/// `"weight"` -> [`ActedDecision::Weight`], `"both"` -> [`ActedDecision::Both`],
+/// and **anything else** (including `"harness"`, an unknown spelling, or an empty
+/// string) -> [`ActedDecision::Harness`]. Defaulting unknown decisions to harness
+/// keeps the safe, cheap lever as the fallback and preserves today's behavior
+/// when no usable decision is present.
+pub fn action_for_decision(decision: &str) -> ActedDecision {
+    match decision {
+        "weight" => ActedDecision::Weight,
+        "both" => ActedDecision::Both,
+        _ => ActedDecision::Harness,
+    }
+}
+
+/// Annotate this generation's `scheduler_decision.json` with what the loop
+/// **acted** on (issue #90), so SIA Studio and `improvement.md` can show the
+/// action taken — not just the recommendation.
+///
+/// Adds keys to the existing artifact (leaving the parity-checked recommendation
+/// fields untouched):
+///
+/// * `acted` — `"harness" | "weight" | "both"`, the lever actually pulled.
+/// * `harness_ran` / `weight_ran` — booleans for the two levers.
+/// * `weight_update` — a compact `{ updated, num_examples, loss_before,
+///   loss_after }` summary when a weight update ran, else `null`.
+///
+/// Best-effort: if the artifact is missing/unparseable it writes a fresh object
+/// carrying just the acted fields; any IO error is swallowed. Never panics.
+pub fn record_acted_decision(
+    layout: &RunLayout,
+    current_gen: i64,
+    acted: ActedDecision,
+    weight_outcome: Option<&WeightUpdateOutcome>,
+) {
+    if current_gen < 0 {
+        return;
+    }
+    let gen_dir = layout.gen_dir(current_gen);
+    let out_path = Path::new(&gen_dir).join(SCHEDULER_DECISION_JSON);
+
+    let mut artifact = match read_json(&out_path) {
+        Some(v @ Value::Object(_)) => v,
+        _ => json!({ "generation": current_gen }),
+    };
+
+    let weight_summary = match weight_outcome {
+        Some(o) => json!({
+            "updated": o.updated,
+            "num_examples": o.num_examples,
+            "loss_before": o.loss_before,
+            "loss_after": o.loss_after,
+        }),
+        None => Value::Null,
+    };
+
+    if let Some(obj) = artifact.as_object_mut() {
+        obj.insert("acted".to_string(), Value::from(acted.as_str()));
+        obj.insert("harness_ran".to_string(), Value::from(acted.runs_harness()));
+        obj.insert("weight_ran".to_string(), Value::from(acted.runs_weight()));
+        obj.insert("weight_update".to_string(), weight_summary);
+    }
+
+    if let Ok(text) = serde_json::to_string_pretty(&artifact) {
+        let _ = std::fs::write(&out_path, text);
+    }
+}
+
 /// Load a single trajectory for the generation: prefer the single
 /// `agent_execution.json`, else the first `execution_q*.json` in the
 /// `agent_execution/` directory. Returns `None` if neither parses.
@@ -651,4 +774,76 @@ mod tests {
             record_scheduler_decision(&layout, 2, &SchedulerConfig::default()).expect("decision");
         assert_eq!(v["decision"], json!("harness"));
     }
+
+    // -- Acting on the decision (issue #90) ------------------------------------
+
+    #[test]
+    fn action_for_decision_maps_each_lever_and_defaults_to_harness() {
+        assert_eq!(action_for_decision("weight"), ActedDecision::Weight);
+        assert_eq!(action_for_decision("both"), ActedDecision::Both);
+        assert_eq!(action_for_decision("harness"), ActedDecision::Harness);
+        // Unknown / empty spellings fall back to the cheap, safe harness lever.
+        assert_eq!(action_for_decision("nonsense"), ActedDecision::Harness);
+        assert_eq!(action_for_decision(""), ActedDecision::Harness);
+    }
+
+    #[test]
+    fn acted_decision_lever_flags() {
+        assert!(ActedDecision::Harness.runs_harness());
+        assert!(!ActedDecision::Harness.runs_weight());
+        assert!(!ActedDecision::Weight.runs_harness());
+        assert!(ActedDecision::Weight.runs_weight());
+        assert!(ActedDecision::Both.runs_harness());
+        assert!(ActedDecision::Both.runs_weight());
+    }
+
+    #[test]
+    fn record_acted_decision_annotates_existing_artifact() {
+        // Seed an existing recommendation artifact, then record what we acted on.
+        let (_d, layout) = make_run(&[0.5]);
+        let v =
+            record_scheduler_decision(&layout, 0, &SchedulerConfig::default()).expect("decision");
+        // Pretend we ran a `both` action with a weight outcome.
+        let outcome = WeightUpdateOutcome {
+            num_examples: 2,
+            loss_before: 0.5,
+            loss_after: 0.25,
+            updated: true,
+            details: "test".to_string(),
+        };
+        record_acted_decision(&layout, 0, ActedDecision::Both, Some(&outcome));
+
+        let path = Path::new(&layout.gen_dir(0)).join(SCHEDULER_DECISION_JSON);
+        let on_disk = read_json(&path).unwrap();
+        // Original recommendation fields are preserved.
+        assert_eq!(on_disk["decision"], v["decision"]);
+        // Acted fields are added.
+        assert_eq!(on_disk["acted"], json!("both"));
+        assert_eq!(on_disk["harness_ran"], json!(true));
+        assert_eq!(on_disk["weight_ran"], json!(true));
+        assert_eq!(on_disk["weight_update"]["updated"], json!(true));
+        assert_eq!(on_disk["weight_update"]["num_examples"], json!(2));
+        assert_eq!(on_disk["weight_update"]["loss_after"], json!(0.25));
+    }
+
+    #[test]
+    fn record_acted_decision_without_artifact_writes_fresh_object() {
+        // No prior scheduler_decision.json (e.g. harness with no weight outcome).
+        let (_d, layout) = make_run(&[0.4]);
+        record_acted_decision(&layout, 0, ActedDecision::Harness, None);
+        let path = Path::new(&layout.gen_dir(0)).join(SCHEDULER_DECISION_JSON);
+        let on_disk = read_json(&path).unwrap();
+        assert_eq!(on_disk["acted"], json!("harness"));
+        assert_eq!(on_disk["harness_ran"], json!(true));
+        assert_eq!(on_disk["weight_ran"], json!(false));
+        assert!(on_disk["weight_update"].is_null());
+    }
+
+    #[test]
+    fn record_acted_decision_negative_gen_is_noop() {
+        let d = tempfile::tempdir().unwrap();
+        let layout = RunLayout::new(d.path().join("run_x").to_string_lossy().into_owned());
+        // Must not panic and must not create anything.
+        record_acted_decision(&layout, -1, ActedDecision::Weight, None);
+    }
 }

src/orchestrator.rs

@@ -775,29 +775,66 @@ pub fn run_generation_with(
     // Run evaluation (if evaluate.py exists).
     let _ = run_evaluation(&gen_dir, dataset_dir, &run_setup.venv_dir, env_config);
 
-    // Closed-loop step (#84): record the adaptive scheduler's harness-vs-weight
-    // decision for this generation and, when it recommends weights, run an
-    // observable CPU-reference weight update. Purely additive and best-effort —
-    // it only writes NEW artifacts + a log line, guards every failure, and never
-    // affects this function's control flow or return value.
-    {
-        let decision = crate::closed_loop::record_scheduler_decision(
+    // Closed-loop step (#84, made to ACT on the decision in #90): record the
+    // adaptive scheduler's harness-vs-weight decision for this generation, then
+    // branch the per-generation action on it:
+    //
+    //   * `harness` -> run the meta/feedback harness update (today's behavior),
+    //   * `weight`  -> run the weight update and SKIP the feedback step, and
+    //   * `both`    -> do both.
+    //
+    // Backward-compat is preserved by deriving the action from the decision and
+    // defaulting to harness whenever there is no usable decision (the default in
+    // every existing test, where no `results.json` is produced so
+    // `record_scheduler_decision` returns `None`): in that case the harness path
+    // runs and the feedback agent is invoked exactly as before. The acted
+    // decision + weight outcome are recorded back into `scheduler_decision.json`
+    // (read by SIA Studio / `web::runs`) and threaded into the feedback context
+    // for the next generation.
+    let decision = crate::closed_loop::record_scheduler_decision(
+        &layout,
+        current_gen,
+        &crate::scheduler::SchedulerConfig::default(),
+    );
+    // No decision (or an unreadable one) -> default to the harness path so the
+    // behavior is identical to before #90.
+    let decision_kind = decision
+        .as_ref()
+        .and_then(|d| d.get("decision").and_then(|v| v.as_str()))
+        .map(str::to_string);
+    let acted =
+        crate::closed_loop::action_for_decision(decision_kind.as_deref().unwrap_or("harness"));
+
+    let weight_outcome = if acted.runs_weight() {
+        crate::closed_loop::maybe_run_weight_update(
+            &layout,
+            current_gen,
+            acted.as_str(),
+            &crate::weights::WeightUpdateConfig::default(),
+        )
+    } else {
+        None
+    };
+
+    // Persist what we actually acted on (additive keys on the existing artifact)
+    // and surface it. Only when a decision exists, so the no-scheduler default
+    // path writes nothing new and stays byte-for-byte as before.
+    if decision.is_some() {
+        crate::closed_loop::record_acted_decision(
             &layout,
             current_gen,
-            &crate::scheduler::SchedulerConfig::default(),
+            acted,
+            weight_outcome.as_ref(),
+        );
+        let rationale = decision
+            .as_ref()
+            .and_then(|d| d.get("rationale").and_then(|v| v.as_str()))
+            .unwrap_or("");
+        println!(
+            "[scheduler] gen {current_gen}: decision={} acted={} — {rationale}",
+            decision_kind.as_deref().unwrap_or("harness"),
+            acted.as_str(),
         );
-        if let Some(d) = &decision {
-            if let Some(kind) = d.get("decision").and_then(|v| v.as_str()) {
-                let _ = crate::closed_loop::maybe_run_weight_update(
-                    &layout,
-                    current_gen,
-                    kind,
-                    &crate::weights::WeightUpdateConfig::default(),
-                );
-                let rationale = d.get("rationale").and_then(|v| v.as_str()).unwrap_or("");
-                println!("[scheduler] gen {current_gen}: decision={kind} — {rationale}");
-            }
-        }
     }
 
     // Add generation to context.
@@ -825,8 +862,11 @@ pub fn run_generation_with(
         },
     );
 
-    if current_gen < max_gen {
-        let (execution_status, execution_section) = build_feedback_context(
+    // The harness (feedback) step runs unless the scheduler's acted decision was
+    // a pure `weight` update (issue #90): a `weight` generation short-circuits
+    // the feedback agent for this generation; `harness` and `both` still run it.
+    if current_gen < max_gen && acted.runs_harness() {
+        let (execution_status, mut execution_section) = build_feedback_context(
             current_gen,
             &gen_dir,
             dataset_dir,
@@ -838,6 +878,17 @@ pub fn run_generation_with(
             task_files,
             env_config,
         );
+        // Thread the acted decision + weight-update outcome into the feedback
+        // context for the next generation. Appended ONLY when a decision exists,
+        // so `build_feedback_context`'s parity-checked output is untouched on the
+        // default/no-scheduler path.
+        if decision.is_some() {
+            execution_section.push_str(&format_scheduler_feedback_section(
+                acted,
+                weight_outcome.as_ref(),
+                decision.as_ref(),
+            ));
+        }
         let next_gen = current_gen + 1;
         let next_gen_directory = layout.gen_dir(next_gen);
         feedback_fn(&FeedbackArgs {
@@ -853,6 +904,41 @@ pub fn run_generation_with(
     Ok(())
 }
 
+/// Render a short feedback-context addendum describing the scheduler decision the
+/// loop **acted** on this generation and any weight-update outcome (issue #90).
+///
+/// This is only ever appended to the feedback `execution_section` when a
+/// scheduler decision was produced, so it never perturbs the parity-checked
+/// [`build_feedback_context`] output on the default/no-scheduler path.
+fn format_scheduler_feedback_section(
+    acted: crate::closed_loop::ActedDecision,
+    weight_outcome: Option<&crate::weights::WeightUpdateOutcome>,
+    decision: Option<&Value>,
+) -> String {
+    let rationale = decision
+        .and_then(|d| d.get("rationale").and_then(Value::as_str))
+        .unwrap_or("");
+    let recommended = decision
+        .and_then(|d| d.get("recommended_next").and_then(Value::as_str))
+        .unwrap_or(acted.as_str());
+
+    let weight_line = match weight_outcome {
+        Some(o) => format!(
+            "- Weight update: {} example(s), loss {:.6} -> {:.6} (updated: {}).\n",
+            o.num_examples, o.loss_before, o.loss_after, o.updated,
+        ),
+        None => "- Weight update: not run this generation.\n".to_string(),
+    };
+
+    format!(
+        "\n\n**ADAPTIVE SCHEDULER DECISION**:\n\n\
+- Acted on: {acted} (recommended next: {recommended}).\n\
+{weight_line}\
+- Rationale: {rationale}\n",
+        acted = acted.as_str(),
+    )
+}
+
 /// Run the feedback agent to create an improved target agent (real wiring).
 #[allow(clippy::too_many_arguments)]
 pub fn run_feedback_agent(