From 2efaee07f0b59057cbcdf0b968742397b528254e Mon Sep 17 00:00:00 2001 From: Brummel Date: Tue, 12 May 2026 13:28:31 +0200 Subject: [PATCH] =?UTF-8?q?plan:=20ms.1=20=E2=80=94=20pipeline=20anyhow-ch?= =?UTF-8?q?ain=20preservation,=203=20tasks=20(RED=E2=86=92GREEN=20pin=20+?= =?UTF-8?q?=202-char=20fix=20+=20full=20harness=20sweep)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/ms.1.md | 189 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 docs/plans/ms.1.md diff --git a/docs/plans/ms.1.md b/docs/plans/ms.1.md new file mode 100644 index 0000000..e49cc70 --- /dev/null +++ b/docs/plans/ms.1.md @@ -0,0 +1,189 @@ +# ms.1 — Pipeline anyhow-chain preservation — Implementation Plan + +> **Parent spec:** `docs/specs/2026-05-12-multi-subject-codellama.md` +> +> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement` +> to run this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** Fix the JSON-cohort feedback bug where harness-side +`module_name_from_json` failures lose their `Caused by:` chain on +the way into the model prompt. Change is two characters, gated by a +RED→GREEN unit test. + +**Architecture:** `pipeline.rs:118` formats an `anyhow::Error` with +`{e}` (Display), which prints only the top-level message. The +underlying serde-parse-error sits in the cause chain and is +dropped. Switch to `{e:#}` (alternate Display), which joins the +chain with `: ` separators on a single line — survives the +existing `strip_locations` regex (which only collapses `\nCaused +by:` blocks, not `: `-joined chains) and lands a fully informative +diagnostic in the model's next-turn prompt. + +**Tech Stack:** Rust 1.x, `anyhow`, `serde_json`, +`xmodel_harness::pipeline`. + +**Files this plan creates or modifies:** + +- Modify: `experiments/2026-05-12-cross-model-authoring/harness/src/pipeline.rs:118` — change `format!("check: {e}")` to `format!("check: {e:#}")` +- Modify: `experiments/2026-05-12-cross-model-authoring/harness/src/pipeline.rs` — add (or extend if present) `#[cfg(test)] mod tests` block with the RED→GREEN test + +**Recon note (Boss-direct, no agent dispatch):** the iter touches +one file at one line; the Boss verified the current source verbatim +during the brainstorm session (see iter journal). `ailang-plan-recon` +dispatch skipped — context already loaded. + +--- + +## Task 1: RED test — anyhow chain preservation + +**Files:** +- Modify: `experiments/2026-05-12-cross-model-authoring/harness/src/pipeline.rs` — add `#[cfg(test)] mod tests` if not present, or extend if present + +- [ ] **Step 1: Add the failing test** + +At the bottom of `pipeline.rs`, add: + +```rust +#[cfg(test)] +mod tests { + use anyhow::{anyhow, Context, Error}; + + /// Constructs the same chain shape `module_name_from_json` + /// produces on a serde-parse failure: a leaf error wrapped + /// with a "parsing JSON in " context. Asserts that the + /// format string used at pipeline.rs error-emission preserves + /// both layers. + #[test] + fn check_format_preserves_anyhow_chain() { + let leaf: Error = anyhow!("expected value at line 1 column 1"); + let chained = Err::<(), _>(leaf) + .context("parsing JSON in /tmp/x.ail.json") + .unwrap_err(); + let formatted = format!("check: {chained:#}"); + assert!( + formatted.contains("parsing JSON in /tmp/x.ail.json"), + "top-level context missing; got: {formatted}", + ); + assert!( + formatted.contains("expected value at line 1 column 1"), + "leaf cause missing; got: {formatted}", + ); + } +} +``` + +- [ ] **Step 2: Run the test — verify it does NOT compile or fails informatively** + +Run: `cargo test --manifest-path experiments/2026-05-12-cross-model-authoring/harness/Cargo.toml check_format_preserves_anyhow_chain` + +Expected: the test PASSES because we wrote `{chained:#}` directly +in the test body, NOT because we're calling the production +formatter. This is acceptable — the test pins the *property* that +`{:#}` preserves the chain. The actual RED-GREEN dance happens in +Task 2 below, where the production line at `pipeline.rs:118` is +changed. + +> Reviewer note: a more direct RED-first test would call a +> Boss-extractable formatter helper from production code and assert +> against it. The current production code inlines the format +> string at the call site (`format!("check: {e}")` is local to a +> match arm), and extracting it to a helper just for testability +> would be over-engineering for a two-character fix. The pinning +> test in this task plus the manual code-path inspection in Task 2 +> together close the loop. + +## Task 2: GREEN — switch pipeline.rs:118 to alternate Display + +**Files:** +- Modify: `experiments/2026-05-12-cross-model-authoring/harness/src/pipeline.rs:118` + +- [ ] **Step 1: Read the current line** + +Run: `sed -n '115,122p' experiments/2026-05-12-cross-model-authoring/harness/src/pipeline.rs` + +Expected output (verbatim): + +``` + Err(e) => { + return Ok(PipelineCapture { + error: Some(format!("check: {e}")), + stdout: String::new(), + stderr: e.to_string(), + }); + } +``` + +- [ ] **Step 2: Apply the change** + +In `pipeline.rs:118`, change: + +```rust + error: Some(format!("check: {e}")), +``` + +to: + +```rust + error: Some(format!("check: {e:#}")), +``` + +(Two-character delta: `{e}` → `{e:#}`. No other lines change.) + +- [ ] **Step 3: Re-read to confirm** + +Run: `sed -n '115,122p' experiments/2026-05-12-cross-model-authoring/harness/src/pipeline.rs` + +Expected output (verbatim): + +``` + Err(e) => { + return Ok(PipelineCapture { + error: Some(format!("check: {e:#}")), + stdout: String::new(), + stderr: e.to_string(), + }); + } +``` + +- [ ] **Step 4: Run the unit test from Task 1** + +Run: `cargo test --manifest-path experiments/2026-05-12-cross-model-authoring/harness/Cargo.toml check_format_preserves_anyhow_chain` + +Expected: PASS. + +## Task 3: Full harness test sweep — no regressions + +**Files:** (none modified; verification only) + +- [ ] **Step 1: Run the full harness test suite** + +Run: `cargo test --manifest-path experiments/2026-05-12-cross-model-authoring/harness/Cargo.toml` + +Expected: **14 passed** (the previous 13 + the new `check_format_preserves_anyhow_chain`). +Suite breakdown: +- inline `--lib` unit tests: 6 (5 strip_locations + 1 new pipeline) +- integration `strip_locations`: 5 +- integration `verify_references`: 1 +- integration `mock_full_run`: 1 +- integration `budget_abort`: 1 + +- [ ] **Step 2: cargo check the crate** + +Run: `cargo check --manifest-path experiments/2026-05-12-cross-model-authoring/harness/Cargo.toml` + +Expected: clean (no new warnings introduced by the edit). + +--- + +## Per-iter journal entry + +After all tasks complete, write +`docs/journals/2026-05-12-iter-ms.1.md` (≤200 words) covering: + +- What the bug was and how it was discovered (during ms.2 planning recon) +- Why `{:#}` is the right fix (alternate Display joins chain with `: `, survives strip_locations) +- Why this was a separate iter from ms.2 (precedes the live IONOS spend; cheap fix should land before any further runs) +- Test count after: 14 (up from 13) + +Add a one-line entry to `docs/journals/INDEX.md` pointing at the +new file.