From ede26bb231a2bb07a8504295c043fa825c477eee Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 26 Jul 2026 23:43:23 +0200 Subject: [PATCH] =?UTF-8?q?fix(aura-cli,=20docs):=20fieldtest=20bugs=20?= =?UTF-8?q?=E2=80=94=20a=20refusal=20that=20misdirects,=20a=20handle=20den?= =?UTF-8?q?ied,=20a=20stale=20signature?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RED-first fixes for the three bugs the source-blind field test caught. Two are the same failure the whole cycle keeps circling: prose that tells a caller something the code does not do. - The chart not-found refusal prescribed `aura exec --tap =`. The selector takes a declared tap NAME, so following the refusal verbatim earns a second refusal ("the tap plan names 'sub.value', but the blueprint declares no such tap"), and the flag is not needed at all — a plain run records every declared tap. This sentence predates the cycle and was carried over verbatim while the rest of the refusal was corrected; the field test ran it as written, which is exactly why it ran it. - `aura measure ic ` answered "no recorded run '…' under runs/traces" for a handle that IS recorded — the value the campaign run printed, that `chart` renders, whose directory exists. A family holds members rather than one series, so IC is measured over a member. The refusal now says that and lists the member keys, which also closes the friction that nothing on any text surface prints them: reaching a member previously meant the very `ls runs/traces/` this cycle set out to retire. - C28's #297 note still gave the entry points' signature as the retired `(_, Vec)` pair in the present tense, contradicting C27 one contract over. The close commit claimed lockstep; this makes it true. Both code fixes have tests that fail without them. The field test also found four friction items and two spec gaps, filed rather than fixed here. refs #309 --- crates/aura-cli/src/main.rs | 28 ++++++++++++-- crates/aura-cli/tests/cli_run.rs | 27 +++++++++++++ crates/aura-cli/tests/measure_ic.rs | 42 +++++++++++++++++++++ docs/design/contracts/c28-stratification.md | 6 ++- 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/crates/aura-cli/src/main.rs b/crates/aura-cli/src/main.rs index e8601bc..bc26336 100644 --- a/crates/aura-cli/src/main.rs +++ b/crates/aura-cli/src/main.rs @@ -464,9 +464,11 @@ fn render_chart_by_kind( "aura: no recorded run or family '{name}' under runs/traces \ (a single run prints its handle as `trace_name` on stdout; a campaign \ run prints it as `campaign_run.trace_name`, one per run — check that \ - value for a typo. A trace is produced by \ - `aura exec --tap =` on a blueprint \ - or a campaign's `presentation.persist_taps` section, not by naming a handle here)" + value for a typo. A trace is produced by running a blueprint that \ + declares taps: `aura exec ` records its declared taps by \ + default, and `--tap =` selects among them by name; a \ + campaign produces one through its `presentation.persist_taps` \ + section. Naming a handle here produces nothing)" ); } } @@ -1692,7 +1694,25 @@ fn dispatch_measure(cmd: MeasureCmd, env: &aura_runner::project::Env) { } fn dispatch_measure_ic(a: MeasureIcCmd, env: &aura_runner::project::Env) { - let traces = env.trace_store().read(&a.run).unwrap_or_else(|e| { + let store = env.trace_store(); + // #309 fieldtest: a campaign's trace handle is what the run printed and + // what `chart` accepts, so reporting it as "no recorded run" is false — + // it is recorded, as a family of members, and IC is measured over one + // member's series. Naming the members here is also the only text route to + // them: nothing else prints a member key. + if matches!(store.name_kind(&a.run), aura_registry::NameKind::Family) { + let members: Vec = + store.read_family(&a.run).map(|ms| ms.into_iter().map(|m| m.key).collect()).unwrap_or_default(); + eprintln!( + "aura: '{}' names a recorded family, not a single run — measure one of its \ + members instead ({}/). Members: {}", + a.run, + a.run, + if members.is_empty() { "(none readable)".to_string() } else { members.join(", ") } + ); + std::process::exit(1); + } + let traces = store.read(&a.run).unwrap_or_else(|e| { eprintln!("aura: reading run '{}' traces failed: {e}", a.run); std::process::exit(1); }); diff --git a/crates/aura-cli/tests/cli_run.rs b/crates/aura-cli/tests/cli_run.rs index dd8c31d..3da133a 100644 --- a/crates/aura-cli/tests/cli_run.rs +++ b/crates/aura-cli/tests/cli_run.rs @@ -4607,6 +4607,33 @@ fn chart_family_id_suggests_the_single_recorded_handle() { let _ = std::fs::remove_dir_all(&cwd); } +/// #309 fieldtest: the not-found remedy prescribed +/// `aura exec --tap =`, but the selector takes a declared +/// tap NAME, and a plain `aura exec` records every declared tap anyway. A +/// refusal that sends the caller into a second refusal is a defect even when +/// the path it guards is right. +#[test] +fn chart_unknown_name_refusal_does_not_prescribe_a_node_field_tap_selector() { + let cwd = temp_cwd("chart-unknown-tap-grammar"); + + let out = Command::new(BIN) + .args(["chart", "ghost"]) + .current_dir(&cwd) + .output() + .expect("spawn chart ghost"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("NODE.FIELD"), + "the selector takes a declared tap name, not a node.field address: {stderr}" + ); + assert!( + stderr.contains("records its declared taps"), + "the refusal must say a plain run already records, so the flag is not required: {stderr}" + ); + + let _ = std::fs::remove_dir_all(&cwd); +} + /// #309: an index-less directory is NOT a chartable handle — an interrupted /// run leaves one behind, and suggesting it would send the caller straight /// back into the refusal they came from. The store lists it; the refusal must diff --git a/crates/aura-cli/tests/measure_ic.rs b/crates/aura-cli/tests/measure_ic.rs index 59e5bf3..1022ba4 100644 --- a/crates/aura-cli/tests/measure_ic.rs +++ b/crates/aura-cli/tests/measure_ic.rs @@ -123,3 +123,45 @@ fn measure_ic_missing_tap_errors() { String::from_utf8_lossy(&out.stderr) ); } + +/// #309 fieldtest: a campaign's trace handle is exactly what the run printed +/// and what `chart` accepts, so denying that it exists at all is false. A +/// family holds members, not one series — say so, and name the members, since +/// no other text surface lists them. +#[test] +fn measure_ic_on_a_family_handle_names_its_members_instead_of_denying_the_handle() { + let cwd = temp_cwd("measure-ic-family"); + for member in ["c0/m0", "c0/m1"] { + let dir = cwd.join("runs/traces/fam1234-0").join(member); + std::fs::create_dir_all(&dir).expect("fabricate a family fan-out"); + // A minimally valid member index: the manifest shape the store reads + // back, with no taps (the refusal needs the member KEY, not series). + std::fs::write( + dir.join("index.json"), + r#"{"manifest":{"commit":"fieldtest","params":[],"defaults":[],"window":[0,0],"seed":0,"broker":"sim"},"taps":[]}"#, + ) + .expect("write member index"); + } + + let out = std::process::Command::new(BIN) + .args(["measure", "ic", "fam1234-0", "--signal", "signal", "--price", "price"]) + .current_dir(&cwd) + .output() + .expect("spawn measure ic"); + + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("no recorded run 'fam1234-0'"), + "the handle exists as a family; denying it outright is false: {stderr}" + ); + assert!( + stderr.contains("family"), + "the refusal must say the handle names a family: {stderr}" + ); + assert!( + stderr.contains("c0/m0") && stderr.contains("c0/m1"), + "the refusal must name the member keys, the only text route to them: {stderr}" + ); + + let _ = std::fs::remove_dir_all(&cwd); +} diff --git a/docs/design/contracts/c28-stratification.md b/docs/design/contracts/c28-stratification.md index b0b0347..1323f16 100644 --- a/docs/design/contracts/c28-stratification.md +++ b/docs/design/contracts/c28-stratification.md @@ -173,8 +173,10 @@ the single-run verb paths (`member.rs` ×8, `measure.rs` ×5, `translate.rs` ×1 — the "~24" this paragraph used to cite predates the #319 dual-grammar retirement and never updated in lockstep) are now `RunnerError { exit_code, message }` constructors threaded through a fallible chain; the two entry -points, `run_signal_r` and `run_measurement`, return -`Result<(_, Vec), RunnerError>`, and the CLI's `exec_blueprint_leg` +points, `run_signal_r` and `run_measurement`, return a fallible outcome +carrying the report beside the per-invocation values the shell renders +(`Result, RunnerError>` since #309 widened the original +`(_, Vec)` pair), and the CLI's `exec_blueprint_leg` remaps via one shared `exit_on_runner_error` helper (the `dispatch_reproduce` pattern). The one hole in "a sweep worker never process-exits" — `cost_knob`, reachable from `run_blueprint_member` — is closed the same cycle: the