diff --git a/crates/ogar-loco/Cargo.toml b/crates/ogar-loco/Cargo.toml index f34237a..26c748c 100644 --- a/crates/ogar-loco/Cargo.toml +++ b/crates/ogar-loco/Cargo.toml @@ -14,3 +14,13 @@ serde = ["dep:serde"] [dependencies] serde = { workspace = true, optional = true } + +# The 34 NARS reasoning recipes (`recipes::RECIPES`) — the rung-3 runbooks this +# crate's `nars` vocabulary addresses as domain bytes. `lance-graph-contract` is +# itself ZERO-DEP ("a trait-only crate ... MUST stay dependency-free even of +# optional path deps"), so this costs nothing in dependency weight. EIGHT sibling +# OGAR crates already pull it on this exact coordinate — ogar-auth, +# ogar-class-view, ogar-doc-ir, ogar-from-ruff, ogar-r2il, ogar-rbac, +# ogar-render-askama, ogar-render-typst — so `branch = "main"` here is this +# repo's own established convention, measured rather than assumed. +lance-graph-contract = { git = "https://github.com/AdaWorldAPI/lance-graph", branch = "main" } diff --git a/crates/ogar-loco/src/interpret.rs b/crates/ogar-loco/src/interpret.rs index 57b6169..8e0dc6f 100644 --- a/crates/ogar-loco/src/interpret.rs +++ b/crates/ogar-loco/src/interpret.rs @@ -41,6 +41,7 @@ //! truthy condition, and would pass any test whose loop runs zero or one //! times. +use crate::inventory::{FnAddr, Inventory}; use crate::vocabulary::conformance::CheckedVocabulary; use crate::{Call, FnIndex, FunctionBody, Program, Vocabulary}; @@ -101,13 +102,25 @@ pub enum RunError { /// The call whose arity is unknown. call: FnIndex, }, - /// A branch named a function index the program does not contain. + /// A branch named a function index that did not RESOLVE to a body. + /// + /// Not merely "out of range": the bound is whether the backing answers + /// with a body, so a sparse [`Inventory`] with a hole at a legal address + /// lands here too. See [`Interpreter::branch`] for why the two cannot be + /// separate rules. UnresolvedBody { /// The branching call. call: FnIndex, /// The index it named. target: u8, }, + /// [`Interpreter::run`] found no body at the entry address. + /// + /// Distinct from [`RunError::UnresolvedBody`] because there is no + /// branching call to name: nothing asked for this body, the run simply + /// had nowhere to start. Reported rather than returning `Ok(())`, which + /// is what an empty program and an unresolvable entry used to share. + MissingEntryBody, /// A loop ran past [`Interpreter::iteration_cap`]. IterationCap { /// The loop call. @@ -115,6 +128,18 @@ pub enum RunError { /// The cap it exceeded. cap: u32, }, + /// A branch recursed past [`Interpreter::recursion_depth`]. + /// + /// Distinct from [`RunError::IterationCap`] on purpose: a loop that runs + /// too long and a cycle of bodies calling each other are different + /// defects in the program, and collapsing them would send an author + /// looking at the wrong one. + RecursionDepth { + /// The branching call that would have exceeded the depth. + call: FnIndex, + /// The ceiling it hit. + depth: u32, + }, /// A control-flow call outside the five this engine executes. Refused /// rather than approximated — see the module doc. UnhandledControlFlow { @@ -123,13 +148,67 @@ pub enum RunError { }, } +/// The shared core's control band: `0x01..=0x1F`. +/// +/// Every byte in it is the ENGINE's to execute or refuse — never the +/// dialect's. Below it sits `NOP`; above it start the value families +/// (logic `0x20`, compare `0x30`, arithmetic `0x40`, variables `0x80`), and +/// past [`DOMAIN_FLOOR`] a dialect owns its own bytes outright. +const CONTROL_BAND: core::ops::RangeInclusive = 0x01..=0x1F; + +/// Is `f` the engine's to handle? +/// +/// # Why not `Vocabulary::branches` +/// +/// Because that answers a DIFFERENT QUESTION, and its own doc says so: +/// *"this predicate answers 'does lowering this call require emitting another +/// function?', which is the only question a cast asks"* — a CODEGEN question. +/// It is `body_refs(f) > 0`, and `BREAK` / `CONTINUE` / `STOP` / `RETURN` / +/// `WAIT` reference no body, so they are `false` under it. +/// +/// Used as an EXECUTION predicate it silently handed every one of them to +/// `Dialect::call`, where a dialect could invent a meaning for `BREAK` and +/// let the run report success — while this module's contract promised +/// [`RunError::UnhandledControlFlow`]. Two questions, one name, and the +/// vocabulary had already written the warning. +/// +/// The band is the right discriminator: it is a property of the shared core's +/// own layout, it needs no per-byte list to drift, and a domain vocabulary +/// cannot forge it (`DOMAIN_FLOOR` is `0x90`). +fn is_engine_control(f: FnIndex) -> bool { + CONTROL_BAND.contains(&f.0) +} + +/// How deep [`Interpreter::branch`] may recurse before refusing. +/// +/// Not a style limit — a safety one. `Program::references_are_resolvable` +/// rejects only target `0` and out-of-range targets; it does NOT reject +/// cycles, so two bodies that branch to each other pass validation and then +/// recurse until the native stack is gone. A stack overflow aborts the +/// process, which is the one failure an orchestration engine must never turn +/// a bad program into. +/// +/// 64 is chosen against the substrate rather than taste: a continuation stack +/// is 90 quad slots in one node, so a depth past that could not be reified +/// anyway. +pub const DEFAULT_RECURSION_DEPTH: u32 = 64; + /// A running program: the engine's own state, plus the dialect's. pub struct Interpreter<'a, V: Vocabulary, D: Dialect> { vocab: &'a CheckedVocabulary, program: &'a Program, + /// Where bodies come from, when a caller supplies one. + /// + /// `None` resolves against `program.functions`, which is what every + /// pre-`Inventory` caller does and what keeps this change behaviour-neutral + /// for them: a `VecInventory` built from a program's own `functions` has + /// registration order as its address, so the two agree row for row. + inventory: Option<&'a dyn Inventory>, dialect: D, stack: Vec, iteration_cap: u32, + recursion_depth: u32, + depth: u32, } impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { @@ -138,12 +217,54 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { Self { vocab, program, + inventory: None, dialect, stack: Vec::new(), iteration_cap: DEFAULT_ITERATION_CAP, + recursion_depth: DEFAULT_RECURSION_DEPTH, + depth: 0, + } + } + + /// Resolve bodies through an [`Inventory`] instead of the program's own + /// `functions` list. + /// + /// This is the consumer half of functions-as-objects: a node store, a Lance + /// scan or a cache implements [`Inventory`] and the interpreter branches + /// into bodies it has never seen inside a `Program`. + /// + /// ⊘ Landed after review flagged, correctly, that `inventory.rs` shipped a + /// trait no execution path could reach — `Interpreter::new` took only a + /// `Program` and both resolution sites went through `program.functions`, so + /// the advertised behaviour was unavailable to any caller. Two built ends + /// that did not meet, which is the exact shape this session kept naming + /// elsewhere. + #[must_use] + pub fn with_inventory(mut self, inventory: &'a dyn Inventory) -> Self { + self.inventory = Some(inventory); + self + } + + /// The body at `index`, from whichever backing this interpreter resolves + /// against. One place, so the two call sites cannot disagree. + fn body_at(&self, index: usize) -> Option<&'a FunctionBody> { + match self.inventory { + Some(inv) => u16::try_from(index).ok().and_then(|i| inv.body(FnAddr(i))), + None => self.program.functions.get(index), } } + /// Replace the recursion ceiling. + pub fn with_recursion_depth(mut self, depth: u32) -> Self { + self.recursion_depth = depth; + self + } + + /// The depth a branch may not exceed. + pub fn recursion_depth(&self) -> u32 { + self.recursion_depth + } + /// Replace the per-loop iteration ceiling. pub fn with_iteration_cap(mut self, cap: u32) -> Self { self.iteration_cap = cap; @@ -166,19 +287,29 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { } /// Run the program's entry function. + /// + /// Errors with [`RunError::MissingEntryBody`] when the entry address does + /// not resolve. It used to return `Ok(())`, which made "there was nothing + /// to run" indistinguishable from "it ran and did nothing" — and under an + /// [`Inventory`] backing, a body the store could not produce then read as + /// a completed run. pub fn run(&mut self) -> Result<(), RunError> { - self.run_function(0) + match self.body_at(0) { + Some(body) => self.run_body(body), + None => Err(RunError::MissingEntryBody), + } } - /// Run one function body to completion. - fn run_function(&mut self, index: usize) -> Result<(), RunError> { - let Some(body) = self.program.functions.get(index) else { - return Ok(()); - }; + /// Run one already-resolved body to completion. + /// + /// Takes the body rather than an address on purpose: resolution is the + /// caller's to do and to report on, so there is no path through here that + /// can fail to find one and silently succeed. + fn run_body(&mut self, body: &'a FunctionBody) -> Result<(), RunError> { let mut pc = 0usize; while let Some(call) = body.call(pc) { let f = call.function; - if !self.vocab.table().branches(f) { + if !is_engine_control(f) { self.dialect .call(f, call.values, &mut self.stack) .map_err(RunError::Dialect)?; @@ -199,12 +330,11 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { call: Call, ) -> Result<(), RunError> { let f = call.function; - let arity = self - .vocab - .table() - .stack_arity(f) - .ok_or(RunError::UncoveredArity { call: f })?; - + // The arity is resolved INSIDE the loop arms, not here: it is needed + // only by the condition-span walk, and computing it up front made + // `RETURN` — whose arity the core deliberately leaves uncovered — + // report `UncoveredArity`, which points an author at the vocabulary + // when the true answer is that this ENGINE does not execute the byte. match f { FnIndex::IF => { let cond = self.pop(f)?; @@ -223,13 +353,29 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { } FnIndex::REPEAT => { let n = self.pop(f)?; - let count = self.dialect.repeat_count(&n).min(self.iteration_cap); + let count = self.dialect.repeat_count(&n); + // REFUSE, never clamp. `min` ran the capped prefix and + // returned `Ok`, so a 200_000-iteration repeat reported + // success having run 100_000 times — a wrong answer presented + // as a right one, and inconsistent with WHILE, which reports + // the cap in the same situation. + if count > self.iteration_cap { + return Err(RunError::IterationCap { + call: f, + cap: self.iteration_cap, + }); + } for _ in 0..count { self.branch(f, call.values[0])?; } } FnIndex::WHILE | FnIndex::REPEAT_UNTIL => { let until = f == FnIndex::REPEAT_UNTIL; + let arity = self + .vocab + .table() + .stack_arity(f) + .ok_or(RunError::UncoveredArity { call: f })?; // Where the condition's own calls begin — re-running THEM is // what makes the next test a new test. let cond_start = self.operand_span_start(body, pc, arity)?; @@ -240,14 +386,19 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { if truthy == until { break; } - self.branch(f, call.values[0])?; - iters += 1; + // The cap is checked when ANOTHER iteration is requested, + // never after the last one ran. Checked afterwards it + // rejected a loop that terminates in exactly `cap` + // iterations — inside the advertised ceiling — and at + // `cap = 0` it ran the body once before refusing. if iters >= self.iteration_cap { return Err(RunError::IterationCap { call: f, cap: self.iteration_cap, }); } + self.branch(f, call.values[0])?; + iters += 1; for i in cond_start..pc { let c = body.call(i).expect("in bounds: span already walked"); if self.vocab.table().branches(c.function) { @@ -265,12 +416,40 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { } /// Recurse into the body a branch names. + /// + /// The bound is **whether a body comes back**, never a length. + /// [`Inventory::len`]'s own doc says so — *"used only for diagnostics; + /// `body` returning `None` is the real bound"* — and this call site used + /// to check the length anyway, which is a contract a backing can satisfy + /// while still having a hole: an [`Inventory`] over a node store or a + /// cache answers `len()` from its address space and `body()` from what it + /// can actually produce. A miss then passed the bound, resolved to + /// nothing, and the run reported success. + /// + /// Address 0 stays refused separately: it is the ENTRY body, and a branch + /// into it is a re-entry the recursion ceiling cannot tell from a + /// legitimate cycle. fn branch(&mut self, call: FnIndex, target: u8) -> Result<(), RunError> { let idx = usize::from(target); - if idx == 0 || idx >= self.program.functions.len() { + if idx == 0 { return Err(RunError::UnresolvedBody { call, target }); } - self.run_function(idx) + let Some(body) = self.body_at(idx) else { + return Err(RunError::UnresolvedBody { call, target }); + }; + if self.depth >= self.recursion_depth { + return Err(RunError::RecursionDepth { + call, + depth: self.recursion_depth, + }); + } + self.depth += 1; + let r = self.run_body(body); + // Restored on the error path too: an interpreter a caller inspects + // after a failure would otherwise report a depth that never unwound, + // and one reused after a caught error would refuse legal programs. + self.depth -= 1; + r } fn pop(&mut self, call: FnIndex) -> Result> { @@ -324,6 +503,7 @@ impl<'a, V: Vocabulary, D: Dialect> Interpreter<'a, V, D> { #[cfg(test)] mod tests { use super::*; + use crate::inventory::VecInventory; use crate::vocabulary::conformance::validate; use crate::{Call, LaneShape}; @@ -433,6 +613,193 @@ mod tests { Ok(interp.dialect) } + /// Run a program whose bodies come from an [`Inventory`] instead of from + /// the program's own `functions` list. + fn run_with_inventory( + program: &Program, + inv: &dyn Inventory, + ) -> Result> { + let vocab = validate(CoreOnly).expect("core-only vocabulary conforms"); + let mut interp = + Interpreter::new(&vocab, program, I64Dialect::default()).with_inventory(inv); + interp.run()?; + Ok(interp.dialect) + } + + /// `total = 0; REPEAT 3 -> body 1`, where body 1 adds `step` to `total`. + /// The entry is identical in both backings; only body 1 differs, so the + /// result says which backing was actually read. + fn step_program(step: u8) -> Program { + let entry = FunctionBody::from_calls( + LaneShape::Pairs, + &[ + Call::with_value(FnIndex::NUMBER, 0), + Call::with_value(FnIndex::VAR_SET, 0), + Call::with_value(FnIndex::NUMBER, 3), + Call::with_value(FnIndex::REPEAT, 1), + ], + ) + .unwrap(); + let body = FunctionBody::from_calls( + LaneShape::Pairs, + &[ + Call::with_value(FnIndex::VAR_GET, 0), + Call::with_value(FnIndex::NUMBER, step), + Call::new(FnIndex::ADD), + Call::with_value(FnIndex::VAR_SET, 0), + ], + ) + .unwrap(); + Program { + functions: vec![entry, body], + } + } + + /// FAILS IF: `with_inventory` does not actually redirect body resolution — + /// if `run_function` or `branch` still reads `program.functions`, the run + /// returns the PROGRAM's 3 rather than the INVENTORY's 21. + /// + /// Two-sided on purpose: the same program run WITHOUT an inventory must + /// still return 3, so this cannot pass by an implementation that ignores + /// the program entirely. + #[test] + fn with_inventory_resolves_bodies_the_program_does_not_carry() { + let program = step_program(1); + let without = run(&program).expect("runs"); + assert_eq!( + without.vars[0], 3, + "the program's own body adds 1, three times" + ); + + // Same entry, a DIFFERENT body 1 — reachable only through the trait. + let other = step_program(7); + let inv: VecInventory = other.functions.iter().copied().collect(); + let with = run_with_inventory(&program, &inv).expect("runs"); + assert_eq!(with.vars[0], 21, "the inventory's body adds 7, three times"); + assert_ne!( + without.vars[0], with.vars[0], + "if these agree the inventory was never consulted" + ); + } + + /// FAILS IF: `branch`'s bound reads the program's length while bodies come + /// from the inventory. A one-body program with a two-body inventory must + /// reach address 1; the old `self.program.functions.len()` refuses it. + #[test] + fn the_branch_bound_follows_the_inventory_not_the_program() { + let full = step_program(7); + let entry_only = Program { + functions: vec![full.functions[0]], + }; + let inv: VecInventory = full.functions.iter().copied().collect(); + let d = run_with_inventory(&entry_only, &inv).expect("runs"); + assert_eq!( + d.vars[0], 21, + "address 1 exists in the inventory, not the program" + ); + } + + /// An inventory with a HOLE: it reports a length that COVERS an address + /// and answers `None` for that address anyway. + /// + /// Not a contrived shape. It is what a node store, a Lance scan or a + /// cache does whenever a row is absent, evicted, or not yet materialized + /// — the length comes from the address space, the body from what the + /// backing can actually produce. [`Inventory::len`]'s own doc says the + /// two are different questions; this is the test double that makes the + /// difference observable. + struct HolePunchedInventory { + /// One slot per address; `None` is a hole inside the length. + bodies: Vec>, + } + + impl Inventory for HolePunchedInventory { + fn body(&self, addr: FnAddr) -> Option<&FunctionBody> { + self.bodies.get(addr.0 as usize)?.as_ref() + } + + fn len(&self) -> usize { + self.bodies.len() + } + } + + /// FAILS IF: `branch` bounds on a LENGTH instead of on whether a body came + /// back. Address 1 is inside `len()` and resolves to nothing, so the old + /// `idx >= self.body_count()` check passed it, `run_function` found no + /// body and returned `Ok(())`, and the run reported success having + /// executed nothing — three times over, once per `REPEAT` iteration. + /// + /// Two-sided: filling the same hole must run to 21, so an implementation + /// that simply refuses every inventory branch cannot pass. + #[test] + fn a_branch_into_a_hole_is_reported_not_silently_completed() { + let full = step_program(7); + + let holed = HolePunchedInventory { + bodies: vec![Some(full.functions[0]), None], + }; + // The premise, asserted rather than assumed: the hole is INSIDE the + // reported length. Without this the test could pass for an ordinary + // out-of-range address and prove nothing about sparseness. + assert_eq!(holed.len(), 2, "the hole lies inside the reported length"); + assert!(holed.body(FnAddr(1)).is_none(), "and address 1 is the hole"); + + match run_with_inventory(&full, &holed) { + Err(RunError::UnresolvedBody { target: 1, .. }) => {} + Err(other) => panic!("expected UnresolvedBody at address 1, got {other:?}"), + Ok(d) => panic!( + "a branch that resolved to nothing was read as a completed run \ + (var0 = {}, the loop iterated and did nothing)", + d.vars[0] + ), + } + + let filled = HolePunchedInventory { + bodies: vec![Some(full.functions[0]), Some(full.functions[1])], + }; + let d = run_with_inventory(&full, &filled) + .expect("the same shape with the hole filled still runs"); + assert_eq!(d.vars[0], 21, "so the refusal is the hole, not the backing"); + } + + /// FAILS IF: an entry address that does not resolve is read as a finished + /// run. `run` used to be `run_function(0)`, which returned `Ok(())` on a + /// missing body — so "there was nothing to run" and "it ran and did + /// nothing" were the same answer. + /// + /// Both backings are pinned, because the silent `Ok(())` was shared: the + /// inventory path is the one a store can hit in production, and the empty + /// program is the behaviour change a pre-`Inventory` caller would see. + #[test] + fn an_unresolvable_entry_is_reported_not_read_as_a_finished_run() { + let program = step_program(7); + + let no_entry = HolePunchedInventory { + bodies: vec![None, Some(program.functions[1])], + }; + assert_eq!(no_entry.len(), 2, "the entry address is inside the length"); + match run_with_inventory(&program, &no_entry) { + Err(RunError::MissingEntryBody) => {} + Err(other) => panic!("expected MissingEntryBody, got {other:?}"), + Ok(d) => panic!( + "an entry that resolved to nothing was read as a finished run \ + (var0 = {})", + d.vars[0] + ), + } + + let empty = Program { functions: vec![] }; + assert!( + matches!(run(&empty), Err(RunError::MissingEntryBody)), + "a program with no bodies reports it rather than succeeding" + ); + + // The silent half: an entry that DOES resolve still runs, so this + // cannot pass for an implementation that refuses every run. + let d = run(&program).expect("a program with an entry body runs"); + assert_eq!(d.vars[0], 21); + } + /// `sum 1..=n` with `REPEAT`: var0 = total, var1 = counter. fn sum_program(n: u8) -> Program { let entry = FunctionBody::from_calls( @@ -869,4 +1236,225 @@ mod tests { interp.dialect.counter_reads ); } + + /// Records every call the dialect is handed, and succeeds on all of them. + /// + /// The point is the recording: a refusal the engine owes is only proven + /// if the byte never reached a dialect that would have accepted it. + #[derive(Default)] + struct Permissive { + seen: Vec, + } + impl Dialect for Permissive { + type Value = i64; + type Error = (); + fn truthy(&self, v: &i64) -> bool { + *v != 0 + } + fn repeat_count(&self, v: &i64) -> u32 { + u32::try_from(*v).unwrap_or(0) + } + fn call(&mut self, f: FnIndex, values: [u8; 3], stack: &mut Vec) -> Result<(), ()> { + self.seen.push(f.0); + if f == FnIndex::NUMBER { + stack.push(i64::from(values[0])); + } + Ok(()) + } + } + + /// FAILS IF: the engine decides what to execute with `Vocabulary::branches`. + /// + /// That predicate is `body_refs(f) > 0` and answers a CODEGEN question — + /// its own doc says so. `BREAK`, `CONTINUE`, `STOP`, `RETURN` and `WAIT` + /// reference no body, so under it they are not control flow, and every one + /// was handed to `Dialect::call`. A permissive dialect then accepts them + /// and the run reports success, while this module's contract promises + /// `UnhandledControlFlow`. + /// + /// The existing `an_unproven_control_flow_call_is_refused_not_approximated` + /// could not see this: it uses `FOR_EACH`, which HAS a body reference, so + /// it took the branching path and reached the refusal. The fixture's SHAPE + /// was the coverage gap, not its content. + #[test] + fn a_body_less_control_byte_is_refused_by_the_engine_not_offered_to_the_dialect() { + for byte in [ + FnIndex::BREAK, + FnIndex::CONTINUE, + FnIndex::STOP, + FnIndex::RETURN, + FnIndex::WAIT, + ] { + // Anti-vacuity: each really is body-less, so the old predicate + // really did call it a non-branch. Without this the test could + // pass by accidentally picking branching bytes. + let vocab = validate(CoreOnly).expect("conforms"); + assert_eq!( + vocab.table().body_refs(byte), + 0, + "{byte:?} must be body-less or this row proves nothing" + ); + + let entry = FunctionBody::from_calls( + LaneShape::Pairs, + &[Call::with_value(FnIndex::NUMBER, 1), Call::new(byte)], + ) + .unwrap(); + let p = Program { + functions: vec![entry], + }; + let mut interp = Interpreter::new(&vocab, &p, Permissive::default()); + assert_eq!( + interp.run(), + Err(RunError::UnhandledControlFlow { call: byte }), + "{byte:?} must be refused by the engine" + ); + assert!( + !interp.dialect.seen.contains(&byte.0), + "{byte:?} was offered to the dialect: {:?}", + interp.dialect.seen + ); + } + } + + /// FAILS IF: `REPEAT` clamps an over-cap count instead of refusing it. + /// + /// `min(count, cap)` ran the capped prefix and returned `Ok`, so a + /// 200_000-iteration repeat reported SUCCESS having run 100_000 times — a + /// wrong answer presented as a right one, and inconsistent with `WHILE`, + /// which reports the cap in exactly this situation. + #[test] + fn a_repeat_count_above_the_cap_is_refused_rather_than_truncated() { + let entry = FunctionBody::from_calls( + LaneShape::Pairs, + &[ + Call::with_value(FnIndex::NUMBER, 10), + Call::with_value(FnIndex::REPEAT, 1), + ], + ) + .unwrap(); + let body = + FunctionBody::from_calls(LaneShape::Pairs, &[Call::with_value(FnIndex::NUMBER, 1)]) + .unwrap(); + let p = Program { + functions: vec![entry, body], + }; + let vocab = validate(CoreOnly).expect("conforms"); + let mut interp = Interpreter::new(&vocab, &p, Permissive::default()).with_iteration_cap(4); + assert_eq!( + interp.run(), + Err(RunError::IterationCap { + call: FnIndex::REPEAT, + cap: 4 + }) + ); + // The second half, and the one the clamp would fail: refusing means + // running NOTHING. A version that ran the capped prefix and then + // errored would satisfy the assertion above and still be wrong. + assert!( + !interp.dialect.seen.contains(&FnIndex::NUMBER.0) || interp.dialect.seen.len() == 1, + "the body must not have run: {:?}", + interp.dialect.seen + ); + } + + /// FAILS IF: a cycle of bodies recurses until the native stack is gone. + /// + /// `Program::references_are_resolvable` rejects only target `0` and + /// out-of-range targets — it does NOT reject cycles, so two bodies that + /// branch to each other pass validation. Unbounded, that is a stack + /// overflow, which ABORTS THE PROCESS: the one failure an orchestration + /// engine must never turn a bad program into, and one no `RunError` can + /// report because there is no stack left to return on. + #[test] + fn two_bodies_that_branch_to_each_other_are_refused_not_overflowed() { + // 1 -> 2 -> 1 -> ... , each hop guarded by an always-true IF. + let hop = |target: u8| { + FunctionBody::from_calls( + LaneShape::Pairs, + &[ + Call::with_value(FnIndex::NUMBER, 1), + Call::with_value(FnIndex::IF, target), + ], + ) + .unwrap() + }; + let p = Program { + functions: vec![hop(1), hop(2), hop(1)], + }; + let vocab = validate(CoreOnly).expect("conforms"); + // Anti-vacuity: the cycle really does pass the program's own check, + // so this is a defect in the ENGINE and not something validation + // was already catching. + assert!( + p.references_are_resolvable(&vocab), + "the fixture must be a program validation accepts, or it proves nothing" + ); + let mut interp = Interpreter::new(&vocab, &p, Permissive::default()); + assert_eq!( + interp.run(), + Err(RunError::RecursionDepth { + call: FnIndex::IF, + depth: DEFAULT_RECURSION_DEPTH + }) + ); + } + + /// FAILS IF: the iteration cap is charged after the last body instead of + /// before the next one. + /// + /// Checked afterwards, a loop that terminates in EXACTLY `cap` iterations + /// — inside the advertised ceiling — was rejected, because the cap fired + /// before the condition could be re-tested one final time. At `cap = 0` + /// the same ordering ran the body once before refusing, which is a cap of + /// zero that executes. + #[test] + fn a_loop_terminating_in_exactly_cap_iterations_is_accepted() { + // counter = 3; while counter > 0 { counter -= 1 } — exactly 3 bodies. + let entry = FunctionBody::from_calls( + LaneShape::Pairs, + &[ + Call::with_value(FnIndex::NUMBER, 3), + Call::with_value(FnIndex::VAR_SET, 0), + Call::with_value(FnIndex::VAR_GET, 0), + Call::with_value(FnIndex::NUMBER, 0), + Call::new(FnIndex::GT), + Call::with_value(FnIndex::WHILE, 1), + ], + ) + .unwrap(); + let body = FunctionBody::from_calls( + LaneShape::Pairs, + &[ + Call::with_value(FnIndex::VAR_GET, 0), + Call::with_value(FnIndex::NUMBER, 1), + Call::new(FnIndex::SUB), + Call::with_value(FnIndex::VAR_SET, 0), + ], + ) + .unwrap(); + let p = Program { + functions: vec![entry, body], + }; + let vocab = validate(CoreOnly).expect("conforms"); + let mut interp = Interpreter::new(&vocab, &p, I64Dialect::default()).with_iteration_cap(3); + interp + .run() + .expect("3 iterations under a cap of 3 must run"); + assert_eq!(interp.dialect.vars[0], 0, "the loop ran to completion"); + + // The paired half: a cap of ZERO must refuse before running anything. + let mut zero = Interpreter::new(&vocab, &p, I64Dialect::default()).with_iteration_cap(0); + assert_eq!( + zero.run(), + Err(RunError::IterationCap { + call: FnIndex::WHILE, + cap: 0 + }) + ); + assert_eq!( + zero.dialect.vars[0], 3, + "a cap of zero must not execute a body" + ); + } } diff --git a/crates/ogar-loco/src/inventory.rs b/crates/ogar-loco/src/inventory.rs new file mode 100644 index 0000000..32938a6 --- /dev/null +++ b/crates/ogar-loco/src/inventory.rs @@ -0,0 +1,311 @@ +//! **Functions as objects** — a call resolves an ADDRESS, not a `Vec` index. +//! +//! # What was missing, and where it already said so +//! +//! [`LocoConcept::Inventory`](crate::LocoConcept::Inventory) — `0x1702`, minted +//! — already carries the design in its own doc: +//! +//! > the **inventory** row: the function registry entry (which functions exist, +//! > **addressed by identity**). A registry read never touches a body. +//! +//! That is functions-as-objects, and nothing implemented it. +//! [`Program`](crate::Program)'s `Vec` stood in for it, so a +//! branch target was a byte index into one program's private list. A function +//! could not be passed, stored, shared between programs, or named from +//! anywhere but the body that happened to contain it. +//! +//! A function AT REST was already an object: +//! [`FunctionNode`](crate::FunctionNode) is 512 bytes with a 16-byte key in +//! slot 0, and `node.rs` keeps that key deliberately opaque so the substrate +//! mints it. The identity existed; the runtime did not use it. +//! +//! # What an address is +//! +//! [`FnAddr`] is a `u16` — 65,536 functions per inventory, the same ceiling +//! the substrate's other `u16` index spaces use. It is an **index into an +//! inventory**, NOT a GUID: the 16-byte key is the canon's, minting is the +//! substrate's, and an interpreter that embedded a key in every branch would +//! be carrying 16 bytes where a call has one or two. +//! +//! The inventory is what maps between them ([`Inventory::key_of`]), so a +//! consumer that stores by key and executes by address needs no second table. +//! +//! # Why a trait +//! +//! Because where the bodies live is not this crate's business. A test holds +//! them in a `Vec`; a consumer resolves them out of a node store, a Lance +//! scan, or a cache. The interpreter only ever asks "give me the body at this +//! address", which is the one question every such backing can answer. + +use crate::FunctionBody; + +/// A function's address within one [`Inventory`]. +/// +/// Deliberately NOT the 16-byte key: an address is what a CALL carries, and a +/// call's whole payload is 1-3 bytes. The key is what a STORE carries. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[repr(transparent)] +pub struct FnAddr(pub u16); + +/// How many addresses a [`VecInventory`] can name: 65 536, the `u16` space. +/// +/// ONE spelling, read by both [`VecInventory::push`] and the `FromIterator` +/// impl, so the two cannot enforce different bounds. +pub const MAX_ADDRESSES: usize = u16::MAX as usize + 1; + +impl FnAddr { + /// The entry address. Reserved: a body-reference byte of `0` means + /// "unset", so no call may branch to it — the same rule + /// `Program::references_are_resolvable` already enforces. + pub const ENTRY: FnAddr = FnAddr(0); +} + +/// Where the interpreter gets a body from. +/// +/// One question, because that is all the interpreter asks. `key_of` is the +/// second half of functions-as-objects — without it an address is a private +/// index again, and a consumer could not tell two inventories apart. +pub trait Inventory { + /// The body at `addr`, or `None` if nothing is registered there. + fn body(&self, addr: FnAddr) -> Option<&FunctionBody>; + + /// The canonical 16-byte key for `addr`, when the backing knows one. + /// + /// `None` is honest rather than a zero key: a test inventory built from + /// bare bodies has no minted identity, and answering `[0u8; 16]` would be + /// a GUID that collides with every other unminted function. + fn key_of(&self, _addr: FnAddr) -> Option<[u8; 16]> { + None + } + + /// How many addresses this inventory can answer. Used only for + /// diagnostics; `body` returning `None` is the real bound. + fn len(&self) -> usize; + + /// Whether the inventory holds nothing. + fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +/// The obvious in-memory inventory: bodies in registration order. +/// +/// Registration order IS the address, which makes a `VecInventory` built from +/// a [`Program`](crate::Program)'s own `functions` behave exactly as the old +/// index did — that equivalence is what lets the change land without +/// re-pinning a single existing program. +#[derive(Debug, Clone, Default)] +pub struct VecInventory { + bodies: Vec, + keys: Vec>, +} + +impl VecInventory { + /// An empty inventory. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Append a body, returning the address it landed at. + /// Panics past [`MAX_ADDRESSES`] — and does so BEFORE mutating. + /// + /// A `u16` address space means the 65 537th body has nowhere to live. + /// Saturating would silently alias it onto the last legal address, so the + /// conversion is checked and the overflow is a panic at BUILD time rather + /// than a wrong branch at run time. + /// + /// ⊘ The order here is the fix, not the check. The first version pushed to + /// both vectors and converted afterwards, which review caught on + /// OGAR #304: a caller that catches the panic is then holding an inventory + /// with 65 537 entries whose last one no `FnAddr` can name, and `len()` + /// counts it. Converting FIRST makes the failure leave nothing behind — + /// the conversion IS the check, so there is no second rule to keep in + /// step with it. + pub fn push(&mut self, body: FunctionBody) -> FnAddr { + let Ok(addr) = u16::try_from(self.bodies.len()) else { + panic!( + "inventory exceeds the u16 address space: {} bodies, max {MAX_ADDRESSES}", + self.bodies.len() + ); + }; + self.bodies.push(body); + self.keys.push(None); + FnAddr(addr) + } + + /// Append a body with its minted key. + pub fn push_keyed(&mut self, key: [u8; 16], body: FunctionBody) -> FnAddr { + let a = self.push(body); + self.keys[a.0 as usize] = Some(key); + a + } + + /// The address holding `key`, if any. Linear — a consumer with many + /// functions brings its own index; this one is for tests and small sets. + #[must_use] + pub fn addr_of(&self, key: &[u8; 16]) -> Option { + self.keys + .iter() + .position(|k| k.as_ref() == Some(key)) + .and_then(|i| u16::try_from(i).ok()) + .map(FnAddr) + } +} + +impl FromIterator for VecInventory { + /// Panics on more than [`MAX_ADDRESSES`] bodies, exactly as [`push`] does. + /// + /// ⊘ The first version collected straight into the `Vec` with no check, + /// which review flagged: it bypassed the bound `push` enforces, so a + /// 65 537-body iterator produced an inventory whose tail no `FnAddr` can + /// name while `len()` still counted it. A silently unaddressable entry is + /// worse than a panic — the bound is the address space, not a policy. + /// + /// [`push`]: VecInventory::push + fn from_iter>(iter: I) -> Self { + let bodies: Vec = iter.into_iter().collect(); + assert!( + bodies.len() <= MAX_ADDRESSES, + "inventory exceeds the u16 address space: {} bodies, max {MAX_ADDRESSES}", + bodies.len() + ); + let keys = vec![None; bodies.len()]; + Self { bodies, keys } + } +} + +impl Inventory for VecInventory { + /// A plain index, because registration order IS the address here. `None` + /// past the end rather than a wrap. + fn body(&self, addr: FnAddr) -> Option<&FunctionBody> { + self.bodies.get(addr.0 as usize) + } + + /// `None` for an entry pushed without a key — this backing never mints + /// one, so an unkeyed entry stays unkeyed rather than acquiring a zero key + /// that would collide with every other unminted function. + fn key_of(&self, addr: FnAddr) -> Option<[u8; 16]> { + self.keys.get(addr.0 as usize).copied().flatten() + } + + /// How many addresses are registered. + /// + /// **Diagnostic only**, as the trait says — [`Inventory::body`] returning + /// `None` is the real bound. The two happen to agree here only because a + /// `Vec` is dense; a backing over a node store, a Lance scan or a cache + /// answers this from its address space and `body` from what it can + /// actually produce, and a consumer that bounds on this number instead of + /// on a resolved body reads a hole as a success (OGAR #304). + fn len(&self) -> usize { + self.bodies.len() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Call, FnIndex, LaneShape}; + + fn body(v: u8) -> FunctionBody { + FunctionBody::from_calls(LaneShape::Pairs, &[Call::with_value(FnIndex::NUMBER, v)]) + .expect("a one-call body is well formed") + } + + /// FAILS IF: an address stops being registration order, or `key_of` + /// invents a key for an unkeyed entry. The `None` is the load-bearing + /// half — answering `[0u8; 16]` would be a GUID colliding with every + /// other unminted function. + #[test] + fn an_address_is_registration_order_and_an_unkeyed_entry_has_no_key() { + let mut inv = VecInventory::new(); + let a = inv.push(body(1)); + let b = inv.push(body(2)); + assert_eq!((a, b), (FnAddr(0), FnAddr(1))); + assert_eq!(inv.len(), 2); + assert_eq!(inv.body(a), Some(&body(1))); + assert_eq!(inv.body(b), Some(&body(2))); + assert_eq!(inv.key_of(a), None, "an unkeyed entry must not invent one"); + assert_eq!( + inv.body(FnAddr(2)), + None, + "past the end is None, not a wrap" + ); + } + + /// FAILS IF: `push_keyed` does not bind the key to the address it + /// returned, or `addr_of` matches a key it was never given. Two entries, + /// so a "return the only key" implementation cannot pass. + #[test] + fn a_minted_key_round_trips_to_its_own_address() { + let mut inv = VecInventory::new(); + let k1 = [1u8; 16]; + let k2 = [2u8; 16]; + let a = inv.push_keyed(k1, body(1)); + let b = inv.push_keyed(k2, body(2)); + assert_ne!(a, b); + assert_eq!(inv.key_of(a), Some(k1)); + assert_eq!(inv.key_of(b), Some(k2)); + assert_eq!(inv.addr_of(&k1), Some(a)); + assert_eq!(inv.addr_of(&k2), Some(b)); + assert_eq!( + inv.addr_of(&[9u8; 16]), + None, + "an absent key has no address" + ); + } + + /// FAILS IF: `FromIterator` collects without the bound `push` enforces. + /// + /// Review flagged exactly this on OGAR #304: the 65 537th body has no + /// `FnAddr` that can name it, so a silent collect produces an inventory + /// whose tail is unreachable while `len()` still counts it. The bound is + /// the address space, not a policy, so it panics rather than truncates. + #[test] + #[should_panic(expected = "exceeds the u16 address space")] + fn from_iter_refuses_more_bodies_than_the_address_space_can_name() { + let _: VecInventory = std::iter::repeat_n(body(1), MAX_ADDRESSES + 1).collect(); + } + + /// FAILS IF: `push` mutates before it validates. A caught panic must leave + /// the inventory EXACTLY as it was — `len()` unchanged and every address + /// still resolvable — because a caller that recovers is otherwise holding + /// an entry no `FnAddr` can name. + /// + /// The `len()` check is the load-bearing assertion: the old order pushed + /// to both vectors first, so this would read `MAX_ADDRESSES + 1`. + #[test] + fn a_refused_push_leaves_the_inventory_untouched() { + let mut inv: VecInventory = std::iter::repeat_n(body(1), MAX_ADDRESSES).collect(); + assert_eq!(inv.len(), MAX_ADDRESSES, "full to the last address"); + + let refused = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + inv.push(body(2)); + })); + assert!(refused.is_err(), "the 65 537th push must be refused"); + assert_eq!( + inv.len(), + MAX_ADDRESSES, + "a refused push must not have grown the inventory" + ); + assert!( + inv.body(FnAddr(u16::MAX)).is_some(), + "the last legal address still resolves" + ); + } + + /// The paired silent half: EXACTLY the address space is legal, and every + /// one of its addresses resolves. Without this the test above would pass + /// for an implementation that rejects any non-trivial iterator. + #[test] + fn from_iter_accepts_exactly_the_address_space() { + let inv: VecInventory = std::iter::repeat_n(body(1), MAX_ADDRESSES).collect(); + assert_eq!(inv.len(), MAX_ADDRESSES); + assert!(inv.body(FnAddr(0)).is_some(), "the first address resolves"); + assert!( + inv.body(FnAddr(u16::MAX)).is_some(), + "the LAST address resolves — this is the one an off-by-one loses" + ); + } +} diff --git a/crates/ogar-loco/src/lib.rs b/crates/ogar-loco/src/lib.rs index 56b9176..2b9fed9 100644 --- a/crates/ogar-loco/src/lib.rs +++ b/crates/ogar-loco/src/lib.rs @@ -121,6 +121,8 @@ use serde::{Deserialize, Serialize}; pub mod basin; pub mod interpret; +pub mod inventory; +pub mod nars; pub mod node; pub mod pool; pub mod program; @@ -130,6 +132,7 @@ pub mod telemetry; pub mod vocabulary; pub use interpret::{Dialect, Interpreter, RunError}; +pub use inventory::{FnAddr, Inventory, VecInventory}; pub use node::FunctionNode; pub use pool::{Constant, ConstantPool, PoolError}; pub use program::{Program, branches_of}; diff --git a/crates/ogar-loco/src/nars.rs b/crates/ogar-loco/src/nars.rs new file mode 100644 index 0000000..178b4e4 --- /dev/null +++ b/crates/ogar-loco/src/nars.rs @@ -0,0 +1,171 @@ +//! The 34 NARS reasoning recipes as a loco vocabulary — and the column that +//! says **which calls are masking ops**. +//! +//! # The mapping was already data +//! +//! Each [`Recipe`] in `lance_graph_contract::recipes::RECIPES` carries a +//! `bucket`, and the three buckets ARE the three execution tiers. This is not +//! an assignment invented here; it is a column that has been sitting in the +//! catalogue: +//! +//! | `Bucket` | the catalogue's own words | who executes it | +//! |---|---|---| +//! | `Datapath` | *"uniform, branch-free, every-cycle SIMD"* | **the masking ops** | +//! | `Control` | *"branchy decision at a control point"* | **loco's interpreter** | +//! | `Gate` | *"a cheap marker that gates whether deeper work fires"* | a predicate, before either | +//! +//! Measured over the real catalogue: **9 Datapath, 19 Control, 6 Gate**. The +//! Datapath nine are literally mask/VSA primitives — #25 `HPM` is *"the +//! substrate: fingerprint cosine/Hamming sweep (SIMD)"*, #19 `ARE` is +//! `A⊗B⊗B=A`, #27 `MPC` is *"bundle = majority-vote-per-bit"*. For those, +//! **a `(function : value)` call IS a masking op**, and [`tier_of`] is how a +//! consumer asks. +//! +//! # Why depending on the contract costs nothing +//! +//! `lance-graph-contract` is itself zero-dep — "a trait-only crate" that "MUST +//! stay dependency-free even of optional path deps". EIGHT OGAR crates already +//! pull it (`ogar-auth`, `ogar-class-view`, `ogar-doc-ir`, `ogar-from-ruff`, +//! `ogar-r2il`, `ogar-rbac`, `ogar-render-askama`, `ogar-render-typst` — +//! counted, not recalled; an earlier draft of this paragraph said four), +//! and `ogar-from-ruff`'s manifest says so in as many words: *"Pulls the +//! ZERO-DEP `lance-graph-contract` only."* This crate's "Zero-dep" description +//! was never an argument against carrying the catalogue. +//! +//! # Byte allocation +//! +//! The 34 occupy `0x90..=0xB1` — `DOMAIN_FLOOR` plus 34. That they overlap +//! `ogar-r2il`'s range is not a collision: a vocabulary is selected by the +//! node's **classid**, so every dialect starts its own bytes at the floor. +//! That is what the registry is for. +//! +//! # What is DERIVED and what is POLICY +//! +//! Derived (read from the catalogue, never chosen here): the code, the name, +//! the bucket, the substrate string, the id→byte mapping. +//! +//! **Policy (chosen here, and re-pinnable):** the stack arities below. The +//! catalogue records what realizes a tactic, not how many operands its call +//! pops, so the arities are a stated default per bucket rather than a +//! measurement — say so rather than let a future session inherit them as +//! fact. Each is the narrowest shape its bucket implies: +//! +//! - `Datapath` → **2**: the VSA/mask primitives here are binary at their +//! core (`bind(A,B)`, bundle-of-two, a cosine sweep of query against +//! corpus). A ternary form like `ARE`'s `A⊗B⊗B` composes two binds. +//! - `Gate` → **1**: a marker reads one thing and answers. +//! - `Control` → **1**: an orchestration call takes the subject it +//! orchestrates. +//! +//! # The seam this does NOT close +//! +//! `body_refs` is **0 for every recipe**, including Control. A Control recipe +//! genuinely orchestrates a body, so it wants a body reference — but the +//! engine's control band is `0x01..=0x1F` and these bytes sit above +//! `DOMAIN_FLOOR`, so a body reference here would branch through a path +//! nothing has executed. Declaring `0` keeps the catalogue honest about what +//! runs today; the orchestration seam lands with its own falsifier, not as an +//! unexercised field. + +use crate::{DOMAIN_FLOOR, FnIndex, Vocabulary}; +use lance_graph_contract::recipes::{Bucket, RECIPES, Recipe}; + +/// First byte this vocabulary owns — [`DOMAIN_FLOOR`]. +pub const NARS_BASE: u8 = DOMAIN_FLOOR; + +/// How many bytes it owns: one per recipe. +pub const NARS_COUNT: u8 = RECIPES.len() as u8; + +/// Last byte this vocabulary owns, inclusive. +pub const NARS_LAST: u8 = NARS_BASE + NARS_COUNT - 1; + +const _: () = assert!( + NARS_LAST < 0xFF, + "the 34 recipes must fit above the domain floor" +); + +/// The recipe a byte names, or `None` outside `NARS_BASE..=NARS_LAST`. +/// +/// Recipe ids are `1..=34` and the catalogue is id-ascending, so the byte is +/// `NARS_BASE + (id - 1)`. Resolved by INDEX rather than by searching for a +/// matching `id`, and the debug assert below is what keeps those two readings +/// from drifting if the catalogue's order ever stops matching its ids. +#[must_use] +pub fn recipe_at(f: FnIndex) -> Option<&'static Recipe> { + let i = f.0.checked_sub(NARS_BASE)? as usize; + let r = RECIPES.get(i)?; + debug_assert_eq!( + r.id as usize, + i + 1, + "RECIPES is documented as id-ascending; byte↔id resolution relies on it" + ); + Some(r) +} + +/// Which tier executes this byte — the question "is this call a masking op?" +/// +/// `Some(Bucket::Datapath)` is a yes. +#[must_use] +pub fn tier_of(f: FnIndex) -> Option { + recipe_at(f).map(|r| r.bucket) +} + +/// Is this call a masking op — i.e. does the Datapath tier execute it? +#[must_use] +pub fn is_mask_op(f: FnIndex) -> bool { + matches!(tier_of(f), Some(Bucket::Datapath)) +} + +/// The byte a recipe id (`1..=34`) is addressed by, or `None` out of range. +#[must_use] +pub fn byte_of_id(id: u8) -> Option { + (1..=NARS_COUNT) + .contains(&id) + .then(|| FnIndex(NARS_BASE + id - 1)) +} + +/// The 34 recipes as a plug-in vocabulary. +/// +/// Below [`DOMAIN_FLOOR`] it is transparent — the shared core answers, as the +/// conformance rule requires. Above it, every byte's shape comes from its +/// recipe's bucket. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct NarsVocabulary; + +impl Vocabulary for NarsVocabulary { + /// Two operands for a [`Bucket::Datapath`] byte, one for [`Bucket::Gate`] + /// and [`Bucket::Control`]. + /// + /// POLICY, not a measurement — see the module doc. The catalogue records + /// each recipe's bucket, not its arity, so these numbers are this + /// vocabulary's reading of the bucket rather than a fact it carries. + fn domain_stack_arity(&self, f: FnIndex) -> Option { + tier_of(f).map(|b| match b { + Bucket::Datapath => 2, + Bucket::Gate | Bucket::Control => 1, + }) + } + + /// Zero for every recipe, [`Bucket::Control`] included. + /// + /// See "the seam this does NOT close" in the module doc: a body reference + /// here would branch through a path nothing has executed. + fn domain_body_refs(&self, _f: FnIndex) -> u8 { + 0 + } + + /// Every tier answers with something — `Datapath` a mask, `Gate` a + /// marker, `Control` a verdict — so all three push. + /// + /// Declared rather than left unknown so bodies using these bytes are + /// statement-segmentable instead of refused. + fn domain_pushes_result(&self, f: FnIndex) -> Option { + tier_of(f).map(|_| true) + } + + /// The recipe's own `code` from the shared catalogue, so a legend renders + /// the canonical name rather than a byte this crate invented. + fn domain_name(&self, f: FnIndex) -> Option<&'static str> { + recipe_at(f).map(|r| r.code) + } +} diff --git a/docs/LOCO-ORCHESTRATION-GAP.md b/docs/LOCO-ORCHESTRATION-GAP.md new file mode 100644 index 0000000..dfdb91b --- /dev/null +++ b/docs/LOCO-ORCHESTRATION-GAP.md @@ -0,0 +1,288 @@ +# What `ogar-loco` needs to carry `rs-graph-llm`'s orchestration + +> Design pass, 2026-09-14. Reference read: `AdaWorldAPI/rs-graph-llm` +> `graph-flow` at `59f9315` (3,723 LOC across 11 modules). Nothing here is +> built; this is the gap list and the shape each closure must take. + +## ⊘ CORRECTED — the answer is ONE thing, and this doc buried it (operator, 2026-09-14) + +Operator, on the version below: *"All I need from ogar-loco to know functions +as objects. It can't be so hard."* + +Right, and this crate already says so. `LocoConcept::Inventory` — **`0x1702`, +already minted** — carries this doc in `lib.rs`: + +> the **inventory** row: the function registry entry (which functions exist, +> **addressed by identity**). A registry read never touches a body. + +That IS functions-as-objects. It is already the design, already has a concept +id, and **nothing implements it**. `Program { functions: Vec }` +is a placeholder standing in for it, and the interpreter resolves a branch as +`self.program.functions.get(idx)` — a Vec index, not an address. + +So the whole change is one indirection: + +``` +branch(target) → inventory.get(address) // not functions[byte] +``` + +A function AT REST is already an object: `FunctionNode` is 512 bytes with a +16-byte key in slot 0, and `node.rs` keeps that key deliberately opaque so the +substrate mints it. The identity exists; the runtime does not use it. + +**Once a branch target is an address rather than a local index, the four +"gaps" below stop being gaps and become consequences.** A function can be a +VALUE (its address fits an immediate byte through the constant pool, or the +facet's 12 bytes directly); a continuation is a function; graph orchestration +is functions referencing functions. No async, no `Context` map, no +`SessionStorage` — those are graph-flow's answers to *not having this*. + +**The error in what follows:** it maps graph-flow's architecture onto loco +feature-by-feature, instead of asking what loco lacks. Everything below is +kept as the reading record of `graph-flow` at `59f9315` — the decision "no +async" still stands, and the four consequences are real — but `Inventory` plus +a `branch` that resolves through it is the answer, and the rest is downstream +of it. + +## The headline: loco must NOT become async + +`graph-flow` gets human-in-the-loop by being async all the way down — +`#[async_trait] Task`, tokio, a `SessionStorage` trait with four backends, and +a `FlowRunner` that loads a session, steps it, and saves it. Porting that +shape into `ogar-loco` would pull `tokio` + `serde_json` + `async-trait` into +a zero-dep crate whose whole value is being a 512-byte call ABI. That is not a +trade worth making, and it is not necessary. + +**Suspension replaces async.** A node that needs to await an LLM call returns +`Suspend`; the HOST awaits; the host resumes. That is strictly more general +than an async runtime, for one reason worth stating plainly: + +> A suspended loco program is **bytes** — persistable, inspectable, +> diffable, learnable. A pending Rust future is none of those. + +Everything below follows from that one decision. + +## Feature-by-feature + +| `graph-flow` | `ogar-loco` today | verdict | +|---|---|---| +| `Task` — async node, `run(Context) -> TaskResult` | `Call` / `FunctionBody` | **have it**, and finer-grained: a node is 180 calls, not one closure | +| `Edge { from, to, condition: Arcbool> }` | `IF` / `IF_ELSE` + the condition span | **have it, and better** — see below | +| `NextAction::{Continue, ContinueAndExecute}` | the pc walk | have it | +| `NextAction::End` | body exhaustion | have it | +| `NextAction::GoTo(String)` | — | **G1** | +| `NextAction::WaitForInput` + session save/load | — | **G2 — the keystone** | +| `Context` — `HashMap` behind a lock | dialect-private state + `VAR_GET`/`VAR_SET`'s 256 slots | **G3** | +| `FanOutTask` + tokio | — | **G4** | +| `SessionStorage` × 4 backends | — | **nothing in loco** — see G2 | + +### The conditional edge is already the stronger form + +`graph-flow` attaches `Arc bool>` to an edge. loco attaches +a **program** — the calls preceding the branch, re-run each iteration +(`Interpreter::operand_span_start`). A closure cannot be serialized, cannot be +inspected, cannot be compiled to a different backend, and cannot be learned. A +condition span is all four. This is not a gap to close; it is a reason the +port goes in this direction rather than the other. + +--- + +## G1 — a jump byte + +`NextAction::GoTo(String)` is dynamic routing by name. loco's equivalent is a +call whose value byte is a **body reference** — the `body_refs` machinery the +vocabulary already declares per byte, and which `IF`/`REPEAT` already use. + +The shared core reserves `STOP` / `RETURN` / `BREAK` / `CONTINUE` and the +engine currently REFUSES all four (`RunError::UnhandledControlFlow`), which is +the right posture: they were never validated by a probe. `GOTO` joins that +list, and lands the same way — with a falsifier, not with a guess. + +Cost: one arm in `run_branching`, plus a loop-safety question `IF`/`REPEAT` do +not have (a `GOTO` can build a cycle the structured ops cannot). + +**And "the iteration cap already covers it" is wrong — that sentence stood here +and review was right to reject it.** `iteration_cap` is a PER-LOOP ceiling: +`REPEAT`, `WHILE` and `REPEAT_UNTIL` each check their own count against it. A +`GOTO` cycle contains no loop construct, so it checks nothing and runs forever +inside a cap that is never consulted. A per-loop ceiling cannot bound a +control-flow graph; only a shared budget can. + +So `GOTO` lands with a second, distinct quantity, and the spec is the part that +has to exist before the arm does: + +- **`step_budget`** — one counter for the whole run, decremented **once per + call executed**, whatever executed it. A structured loop body, a `GOTO` + target, a branch into another function, and a resumed run after a suspension + all consume it identically, because they are all "a call ran". +- **`iteration_cap` stays, and stays per-loop.** It is a different guarantee: + it bounds ONE construct's repetitions so a runaway loop is attributable to + that loop. The budget bounds the RUN. Neither subsumes the other — a program + can exhaust the budget with no loop at all, and a single loop can hit its cap + while the budget is barely touched. +- **Exhaustion is a refusal, not a truncation:** `RunError::StepBudget`, + carrying the call that spent the last step, so a caller can see where. +- **A resumed run does NOT get a fresh budget.** The remaining count is part of + what `snapshot` persists (see G3 below) — otherwise suspension is an + unbounded-execution loophole: suspend, resume, repeat. + +Until that exists, `GOTO` stays refused alongside `STOP` / `RETURN` / `BREAK` / +`CONTINUE`, which is the correct posture and not a gap. + +## G2 — an explicit frame stack (the keystone) + +**Today** `Interpreter::branch` RECURSES into `run_body`. The interpreter's +state therefore lives in the Rust call stack, where it cannot be paused, +persisted, or examined. + +(It was `run_function(index)` until #304 split resolution out of execution — +`run_body` takes an already-resolved body so no path through it can fail to +find one and silently succeed. The recursion this gap is about is unchanged; +only where the address is resolved moved.) + +**Needed:** `frames: Vec`, walked iteratively. +Then the whole interpreter state is `(frames, dialect_state, stack)`. + +The loco-native part, and the reason this is cheap rather than a rewrite tax: + +> A `Frame` is `(u16, u16)` = 4 bytes = exactly one `u8:u8:u8:u8` slot in +> `LaneShape::Quads`. **A continuation IS a `FunctionBody`.** 90 quads = 90 +> frames of depth, in one 512-byte node, in the format everything else already +> speaks. + +So `run()` becomes: + +``` +enum Run { Done(V), Suspended(Continuation) } +``` + +and `resume(k, injected_value)` continues. `SessionStorage` needs no analog in +loco at all: a session is a `FunctionBody` plus a dialect blob, and what a +consumer does with bytes is the consumer's business. The zero-dep posture +survives intact. + +Two things this changes that must be re-pinned, not absorbed: + +- **The `WHILE` condition-span re-run becomes frame state.** Today it is an + inner `for` loop inside one `run_branching` call; under an explicit stack it + must be a resumable position. That is the single subtlest part of the + rewrite, and it is exactly the machinery + `while_reruns_its_condition_span_and_computes_gcd` already falsifies — so + the test that guards it exists before the change does. +- **`step_budget` persists across resumptions; `iteration_cap` stays per-loop.** + A resumed run continues spending the same budget rather than receiving a + fresh one — which is what makes step limits fall out for free, and what + stops suspend-resume-repeat from being an unbounded-execution loophole. + + ⊘ This bullet used to read *"the iteration cap becomes a BUDGET spendable + across resumptions"*, which contradicted G1's spec four sections above — a + spec whose own text rejects exactly that reading (*"'the iteration cap + already covers it' is wrong"*). So the correction was written in one section + and the error left standing in another. **A second section is a second place + to be wrong**, and the one that summarizes is the one a reader reaches + first. + +## G3 — a dialect snapshot seam + +`graph-flow`'s `Context` is a `HashMap` behind a +lock. loco must not grow one: the V3 12-byte facet register keyed by classid +is the stack's own state model, and it is zero-copy where a JSON map is not. + +What is genuinely missing is narrower: **the dialect's store is opaque to the +engine**, so the engine cannot persist it across a suspension. The fix is a +byte seam, and loco must define the *seam* and never the *format*: + +``` +trait Dialect { + fn snapshot(&self, out: &mut Vec); // or a &mut [u8] sink + fn restore(&mut self, bytes: &[u8]) -> Result<(), Self::Error>; +} +``` + +A dialect whose state is already facet rows writes them directly; one holding +`[i64; 256]` writes 2 KiB; one holding masks writes mask words. The engine +never looks inside. `VAR_GET`/`VAR_SET`'s 256 slots stay exactly what they +are — the named half of the state model, already addressable. + +### Where those bytes are allowed to live — the boundary, stated + +Review asked this and the doc did not answer it, which is a real omission +rather than a nit: a byte seam whose destination is unspecified is one review +away from becoming a serialization channel. + +**The snapshot is HOST-LOCAL and OUTSIDE the hot path. It does not cross a +mailbox.** A suspended run's bytes belong to whoever is holding that run — a +scheduler slot, a local arena, a durable store the host owns — and they are +read back by the same host on resume. That is what makes `snapshot` cheap +enough to be worth having. + +This is not a preference; it is what the repo's own non-negotiables already +require: + +- **ADR-022 / ADR-023 — the Firewall.** No serialization in the hot path; the + IR is wire truth. An opaque dialect blob is by construction NOT the IR, so it + is exactly the thing that must not be on a wire. Writing it host-locally is + not a crossing; handing it to another mailbox would be. +- **Therefore:** if a suspension ever has to move between owners, it does not + travel as a snapshot blob. It travels as the IR — the program, its address, + and the facet rows that are already the state model — and the receiving host + rebuilds. `snapshot` is a resume aid for one host, never a transport. + +The falsifier, so this cannot quietly erode: **no snapshot byte may appear in +any type that crosses an owner boundary.** If a future `Baton`, envelope, or +mailbox row grows a field carrying `Dialect::snapshot` output, that is the +violation, and it is greppable rather than a matter of judgement. + +## G4 — fan-out is a MASK op, not a task pool + +`FanOutTask` runs N child tasks concurrently on tokio and merges their writes +into one shared `Context`. That is the right shape for an async LLM +orchestrator and the wrong one here. + +In loco's world, N parallel branches over one population is **one mask program**: +N `MaskOp::Pred` into N slots, then a combine. No threads, no shared mutable +context, no merge policy to get wrong — and it is the same substrate the query +side uses. Fan-out therefore lowers DOWN into `lance-graph-mask-risc`, not out +into a runtime. + +This is where the operator's line lands: *the masking ops were necessary to +make the 34 NARS reasoning cheap*. A tactic that costs a task spawn cannot be +orchestration; a tactic that costs a ternlog can. + +--- + +## The ladder this sits in + +``` +ndarray::simd masking ops ← primitives +mask-risc Program ← primitives composed into a method +loco opcode (0x90.. per classid) ← the method becomes an opcode +loco Interpreter ← orchestration: branch, loop, suspend, resume +``` + +`ogar-r2il` already occupies `0x90..=0xE1` **under its own classid** — 82 +machine opcodes. A query dialect, a NARS-tactic dialect and a Blockly palette +each get their own classid and their own `0x90..` range, routed by +`VocabularyRegistry` (concept id → validated `VocabularyTable`, hi-u16 +canon-high). That is what "loco speaks all dialects" means mechanically: +**anything else just needs a classid.** + +The basic vocabulary is **144 + 34 + 36**: the 144 verb atoms (rung 2), the 34 +NARS tactic recipes (`lance_graph_contract::recipes::RECIPES: [Recipe; 34]`, +rung 3 — the runbooks), and the 36 `ThinkingStyle`s (rung 4). The 34 exist +today as ~30 bespoke Rust functions in `ndarray/src/hpc/styles/`; as loco +programs they become data, and data is what can be revised, learned and +persisted. + +## Open, and deliberately not decided here + +- **The core's 144 slots currently hold the Blockly palette.** The core is + exactly `0x00..=0x8F` = 144, and the verb table is exactly 12×12 = 144. + Whether the palette moves above `DOMAIN_FLOOR` so the core can hold the 144 + verb atoms is an operator call, not one to make inside a design doc. +- **`256:256` as function-value syntax.** A `u8:u8` lane read as + `(function, value)` is one call; read as `palette256:palette256` it is a + centroid pair. Both readings live in the same 12 bytes and the ClassView + picks. What that buys the ORCHESTRATION layer specifically — a call whose + "function" is itself a centroid, i.e. a soft dispatch — is unexplored and + should be probed before it is designed.