diff --git a/crates/aura-cli/src/graph_construct.rs b/crates/aura-cli/src/graph_construct.rs index 6a43d95..7ef9fc3 100644 --- a/crates/aura-cli/src/graph_construct.rs +++ b/crates/aura-cli/src/graph_construct.rs @@ -415,7 +415,28 @@ fn resolve_use_op( /// replay (#317): a resolution/doc-gate fault is attributed exactly like any /// other per-op construction fault (same `op N (kind): cause` shape, exit 1). fn composite_from_str(doc: &str, env: &aura_runner::project::Env) -> Result { - let docs: Vec = serde_json::from_str(doc).map_err(|e| format!("invalid op-list: {e}"))?; + // #336: deserialize element-by-element off the untyped `Value` array, not in + // one `Vec` shot. For an internally-tagged enum, serde_json's + // `deny_unknown_fields` fault fires only once the parser has consumed the + // WHOLE offending object — its reported line/column lands on the start of + // the FOLLOWING element, misattributing the fault in longer op-lists. The + // op-list's own index is immune to that token-position quirk, so every + // per-element parse fault is wrapped with it directly. + let raw: Vec = + serde_json::from_str(doc).map_err(|e| format!("invalid op-list: {e}"))?; + let mut docs: Vec = Vec::with_capacity(raw.len()); + for (idx, v) in raw.into_iter().enumerate() { + // Read the op-kind tag straight off the still-untyped value: a parse + // fault (e.g. an unknown key) means `OpDoc` never materializes, so + // `OpDoc::kind_label` isn't available yet, but the raw "op" string is + // the same lowercase label it would produce for every kind but `use`. + let kind = v.get("op").and_then(serde_json::Value::as_str).map(str::to_string); + let parsed: OpDoc = serde_json::from_value(v).map_err(|e| match &kind { + Some(k) => format!("op {idx} ({k}): {e}"), + None => format!("op {idx}: {e}"), + })?; + docs.push(parsed); + } let labels: Vec = docs.iter().map(OpDoc::kind_label).collect(); let mut cache: std::collections::HashMap = std::collections::HashMap::new(); let mut ops: Vec = Vec::with_capacity(docs.len()); @@ -733,7 +754,11 @@ pub(crate) fn blueprint_load_prose(e: &LoadError) -> String { match e { LoadError::Json(err) => format!("blueprint document is not valid JSON: {err}"), LoadError::UnsupportedVersion { found, supported } => { - format!("blueprint format_version {found} is unsupported (this build reads {supported})") + // #341 item 2: name the full accepted range, not just the ceiling — the + // loader's own floor is the fixed `1` of its `1..=BLUEPRINT_FORMAT_VERSION` + // check (`blueprint_serde::blueprint_from_json`), never carried in the + // error itself. + format!("blueprint format_version {found} is unsupported (this build reads versions 1..={supported})") } LoadError::UnknownNodeType(t) => format!("unknown node type {t:?}"), LoadError::Gang(e) => format!("gang section invalid: {}", gang_fault_prose(e)), @@ -774,28 +799,38 @@ fn gang_fault_prose(e: &CompileError) -> String { } } -/// (#185/#241/#244) When an unresolved type id LOOKS project-namespaced -/// (`::`), the likeliest causes are environmental, tiered: outside any -/// project the hint points at the missing `Aura.toml`; inside a data-only -/// project it points at the missing node crate. Bare std ids (no `::`) and -/// invocations with a loaded node crate get no hint (the vocabulary error -/// carries the message). Shared core: both the blueprint LOAD path -/// (`unresolved_namespace_hint`, over `LoadError`) and the op-script `graph -/// build` path (`composite_from_str`, over `OpError`) call this same helper, -/// so the tier texts cannot drift between the two error families. +/// (#185/#241/#244/#341 item 5) When an unresolved type id LOOKS project- +/// namespaced (`::`), the likeliest causes are environmental, tiered: outside +/// any project the hint points at the missing `Aura.toml`; inside a data-only +/// project it points at the missing node crate. Outside any project, a BARE id +/// still gets no hint — at least as likely a plain typo of a std name as an +/// escalation miss, and "no Aura.toml" would mislead a std-typo author. Inside +/// a data-only project, though, a bare id gets the SAME escalation pointer as +/// a namespaced one (#341 item 5): the consumer who wrote a bare name is the +/// one least likely to know yet that new logic needs a namespace at all, so +/// withholding the hint from exactly them was backwards. A project with a +/// loaded node crate gets no hint either way (the vocabulary error carries the +/// message; an unresolved type there is a genuine miss, not an escalation +/// gap). Shared core: both the blueprint LOAD path (`unresolved_namespace_ +/// hint`, over `LoadError`) and the op-script `graph build` path +/// (`composite_from_str`, over `OpError`) call this same helper, so the tier +/// texts cannot drift between the two error families. fn tier_hint_for_type_id(type_id: &str, env: &aura_runner::project::Env) -> Option { - if !type_id.contains("::") { - return None; - } + let namespaced = type_id.contains("::"); match env.provenance() { - None => Some( + None if namespaced => Some( "type id looks project-namespaced but no Aura.toml was found — run inside a project directory" .to_string(), ), - Some(p) if p.namespace.is_none() => Some( + None => None, + Some(p) if p.namespace.is_none() && namespaced => Some( "type id looks project-namespaced but this project binds no node crate — attach one with `aura nodes new `" .to_string(), ), + Some(p) if p.namespace.is_none() => Some( + "new logic is authored in Rust — this project binds no node crate yet; attach one with `aura nodes new `" + .to_string(), + ), Some(_) => None, } } @@ -1088,6 +1123,18 @@ mod tests { assert_eq!(err, "op 1 (add): unknown node type \"Nope\""); } + /// Property (#341 item 2): a v3 document's `UnsupportedVersion` refusal names + /// the FULL supported range, not just the ceiling — the pre-fix prose named + /// only `supported` ("this build reads 2"), leaving a reader to guess whether + /// 0/1 are also refused. + #[test] + fn blueprint_load_prose_unsupported_version_names_the_full_range() { + let e = LoadError::UnsupportedVersion { found: 3, supported: 2 }; + let msg = super::blueprint_load_prose(&e); + assert!(msg.contains("1..=2"), "names the full supported range 1..=2: {msg}"); + assert!(msg.contains('3'), "still names the found version: {msg}"); + } + #[test] fn build_from_str_reports_unconnected_slot_at_finalize() { // sub.rhs left unwired -> the holistic 0-arm fires at the finalize step. diff --git a/crates/aura-cli/tests/graph_construct.rs b/crates/aura-cli/tests/graph_construct.rs index a4ed787..b1fdebf 100644 --- a/crates/aura-cli/tests/graph_construct.rs +++ b/crates/aura-cli/tests/graph_construct.rs @@ -482,6 +482,26 @@ fn graph_build_rejects_an_unknown_op_field() { assert!(stderr.contains("params"), "names the unknown key: {stderr}"); } +/// Property (#336): the unknown-key refusal cites the OFFENDING element's own op +/// index, not serde's `deny_unknown_fields` token position, which (for an +/// internally-tagged enum) surfaces only once the parser has consumed the whole +/// object and lands on the START OF THE FOLLOWING element. With the typo'd key at +/// index 1 (of 3), a line/column-based message would mislead toward index 2; +/// this pins the op-list's own coordinate instead. +#[test] +fn graph_build_unknown_op_field_names_the_offending_op_index() { + let doc = r#"[ + {"op":"source","role":"price","kind":"F64"}, + {"op":"add","type":"Const","params":{"value":{"F64":1.0}}}, + {"op":"expose","from":"const.value","as":"bias"} + ]"#; + let (stderr, code) = run_code(&["graph", "build"], doc); + assert_eq!(code, Some(1), "unknown op field is a content fault -> exit 1; stderr: {stderr}"); + assert!(stderr.contains("op 1"), "names the offending element's own op index (1): {stderr}"); + assert!(!stderr.contains("op 2"), "must not attribute the fault to the following element: {stderr}"); + assert!(stderr.contains("params"), "still names the unknown key: {stderr}"); +} + /// Property (exit-code partition, #175 iteration 2): within the SAME `graph /// introspect` subcommand, an invalid `--node ` flag VALUE is a USAGE error (a /// command-line fault the user must fix in argv) — exit 2 — distinct from bad stdin @@ -1898,6 +1918,25 @@ fn graph_build_hints_attach_for_namespaced_ids_in_a_data_only_project() { ); } +/// Property (#341 item 5): the BARE counterpart of the test above — the +/// consumer least likely to know yet that new logic is Rust (and therefore +/// least likely to namespace their type) is the one who most needs the +/// escalation pointer, not the one least likely to get it. Same data-only +/// project, same unresolved-type refusal, no `::` in the type id this time. +#[test] +fn graph_build_hints_attach_for_a_bare_unresolved_type_in_a_data_only_project() { + let dir = temp_cwd("graph-build-dataonly-bare-hint"); + std::fs::write(dir.join("Aura.toml"), "").expect("write bare Aura.toml"); + let ops = r#"[{"op":"add","type":"ThirdCandle","name":"n"}]"#; + let (stdout, stderr, code) = run_in_stdin(&dir, &["graph", "build"], ops); + assert_ne!(code, Some(0), "an unresolvable bare type refuses: {stderr}"); + assert!(stdout.is_empty(), "no blueprint emitted on a fault: {stdout}"); + assert!( + stderr.contains("aura nodes new"), + "the bare-form refusal now carries the same escalation pointer as the namespaced one: {stderr}" + ); +} + /// Property (#284): the `tap` op-script op, driven through the real `aura /// graph build` process (not an in-crate call to `build_from_str`), resolves /// a name-addressed interior wire (`"fast.value"`, session node 0 / output