diff --git a/crates/splash-ui-l0/src/lib.rs b/crates/splash-ui-l0/src/lib.rs index 7a446bc..1e5e68f 100644 --- a/crates/splash-ui-l0/src/lib.rs +++ b/crates/splash-ui-l0/src/lib.rs @@ -2826,6 +2826,91 @@ fn check_event_batch( /// depended on the host keeping an allowlist that the profile claims to make /// unnecessary. fn validate_sources(card: &Card, sink: &mut Diagnostics) { + // A source name and a card state name must be disjoint. + // + // §5.3 forbids a component from reading card state, and a collision walks + // around the check: `check_path` asks whether the scope KNOWS a path before + // asking whether it names forbidden state, so a source with the same name + // answers the first question and the second is never reached. What the + // component then reads is the SOURCE, which §5.3 permits — so nothing leaks + // — but the card says two things and the runtime silently picks one, which + // is not a defensible way to resolve two namespaces that must not overlap. + for state in &card.states { + if let Some(source) = card.sources.iter().find(|s| s.name == state.path) { + sink.push( + source.line, + 1, + format!( + "{:?} is declared as both a source and card state. The two \ + namespaces must be disjoint: a component may read a source \ + but not card state (profile §5.3), so a shared name makes \ + which one is read depend on lookup order rather than on \ + anything the card says. Rename one", + state.path + ), + ); + } + } + + // A declared source nothing reads is a fetch performed for nothing. + // + // `stock.card` shipped one: `series` went unread the moment `StockPlot` + // began fetching its own data, and no test noticed. On device that is a + // round trip, a parse and a cache entry per refresh, for data that reaches + // no pixel. + let mut read: std::collections::BTreeSet = Default::default(); + let harvest = |element: &Element, read: &mut std::collections::BTreeSet| { + let (mut reads, mut pulls) = (Vec::new(), Vec::new()); + collect_reads(element, &mut reads, &mut pulls); + read.extend(reads.iter().map(root_of)); + read.extend(pulls.iter().map(root_of)); + }; + for view in &card.views { + harvest(&view.body, &mut read); + } + for component in &card.components { + harvest(&component.body, &mut read); + } + // A source read by another source counts: `sys.weather(lat: place.lat)` + // makes `place` needed even though no view names it. + for source in &card.sources { + for (_, arg) in &source.args { + if let SourceArg::Path(path) = arg { + read.insert(root_of(path)); + } + } + } + // So does a state whose initial resolves from one. + for state in &card.states { + if let Some(path) = &state.initial_path { + read.insert(root_of(path)); + } + } + // And a `copy` declaration reads the locale IMPLICITLY: resolving `copy.x` + // looks up `env.locale.lang` to choose which translation to render. No path + // in the card names it, so a purely syntactic scan calls `env.locale` dead + // on every card that has copy but no other locale-dependent binding — which + // is two of the three reference cards. + if !card.copies.is_empty() { + read.insert("env".to_owned()); + read.insert("env.locale".to_owned()); + } + for source in &card.sources { + // A dotted source name (`env.locale`) is read as its own root. + if !read.contains(&source.name) && !read.contains(&root_of(&source.name)) { + sink.push( + source.line, + 2, + format!( + "source {:?} is declared and never read. The host fetches it \ + on every refresh and nothing displays it — either bind it \ + somewhere or remove the declaration", + source.name + ), + ); + } + } + // A source may read card state, other sources and copy — the same names a // view may, minus anything a component introduces. let scope = &Scope::card(card); @@ -3643,6 +3728,34 @@ pub struct UiNode { pub key: String, pub args: Vec<(String, NodeValue)>, pub children: Vec, + /// For each argument whose value came from a `source`, WHICH source and + /// which field of it — alongside the resolved value in `args`, not instead + /// of it. + /// + /// Realization resolves `quote.last` against the data blob and produces a + /// number, at which point nothing downstream can tell that number came from + /// a fetch. A backend that can fetch for itself needs to know, so it can + /// emit a live call rather than bake in whatever the blob happened to hold. + /// Additive on purpose: a consumer that ignores this sees exactly what it + /// saw before. + pub bindings: Vec<(String, SourceBinding)>, +} + +/// Where an argument's value came from, with the source's own arguments already +/// resolved. +/// +/// The arguments are RESOLVED, not paths: `sys.quote(ticker: state.selected)` +/// records `ticker = "NVDA"`, because the state is known at realization and the +/// backend emitting the call has no way to look it up. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SourceBinding { + /// The capability the card named — `sys.quote`. + pub helper: String, + /// Its arguments, resolved to values. + pub args: Vec<(String, String)>, + /// The path WITHIN the source's result: `last` for `quote.last`, empty when + /// the source itself is named. + pub field: String, } /// A resolved argument value. @@ -4022,9 +4135,15 @@ impl Realizer<'_> { key: key.to_string(), args: Vec::new(), children: Vec::new(), + bindings: Vec::new(), }; for arg in &element.args { let value = self.value(&element.name, &arg.name, &arg.value, scope); + if let Operand::Path(path) = &arg.value { + if let Some(binding) = self.source_binding(path, scope) { + node.bindings.push((arg.name.clone(), binding)); + } + } node.args.push((arg.name.clone(), value)); } for (segment, child) in sibling_segments(&element.children) { @@ -4235,6 +4354,51 @@ impl Realizer<'_> { compare(left, cmp, right) } + /// If `path` roots at a declared source, what it would take to fetch. + /// + /// A source's own arguments are resolved here rather than passed through as + /// paths: `sys.quote(ticker: state.selected)` becomes `ticker = "NVDA"`, + /// because the state is known now and a backend emitting the call has no way + /// to look it up. An argument that cannot be resolved drops the binding + /// entirely — a fetch with a hole in it is worse than no fetch, since it + /// would silently request the wrong thing. + fn source_binding(&self, path: &str, scope: &ValueScope) -> Option { + // `env.locale.lang` roots at the source `env.locale`, not at `env`, so + // match the LONGEST declared name that prefixes the path. + let declaration = self + .card + .sources + .iter() + .filter(|s| path == s.name || path.starts_with(&format!("{}.", s.name))) + .max_by_key(|s| s.name.len())?; + + let mut args = Vec::new(); + for (name, arg) in &declaration.args { + let resolved = match arg { + SourceArg::Path(p) => { + // `state.x` in a source argument addresses card state; the + // view scope names it without the prefix. + let looked_up = scope.lookup(p.strip_prefix("state.").unwrap_or(p))?; + json_to_key(&looked_up) + } + SourceArg::Text(t) => t.clone(), + SourceArg::Number(n) => makepad::trim_num(*n), + // A field list is structural, not a value the backend passes on. + SourceArg::List(_) => continue, + }; + args.push((name.clone(), resolved)); + } + Some(SourceBinding { + helper: declaration.helper.clone(), + args, + field: path + .strip_prefix(&declaration.name) + .unwrap_or_default() + .trim_start_matches('.') + .to_string(), + }) + } + fn value( &mut self, ctor: &str, @@ -4299,7 +4463,7 @@ fn json_to_key(value: &serde_json::Value) -> String { /// discipline holding — the card declares questions, and only the *realized /// output* contains answers. pub mod makepad { - use super::{NodeValue, UiNode}; + use super::{NodeValue, SourceBinding, UiNode}; use std::fmt::Write as _; // Matching octos-one's `plan/common.rs`, so a lowered card sits beside the @@ -4356,6 +4520,71 @@ pub mod makepad { /// A value in text position. `Missing` becomes an em dash rather than an /// empty string, so an unresolved binding is visible on screen instead of /// silently rendering a blank field. + /// What to emit for an argument: a LIVE call where the backend can answer + /// the source itself, the realized literal otherwise. + /// + /// This is the difference between a card that shows whatever the host + /// happened to seed and one that shows what is true now. Realization already + /// resolved the value; `bindings` records where it came from, so a backend + /// with its own fetch can go and get it instead. + /// + /// Falling back is not a failure mode to be minimised — `sys.quote` exposes + /// no market cap or P/E, so those two lower to the seeded value and always + /// will. Emitting a call the VM cannot answer would render an em dash where + /// a real number was available. + fn expr_of(node: &UiNode, arg_name: &str) -> String { + if let Some((_, binding)) = node.bindings.iter().find(|(n, _)| n == arg_name) { + if let Some(call) = vm_call(binding) { + return call; + } + } + text_of(arg(node, arg_name)) + } + + /// The VM helper that answers a declared capability, if one does. + /// + /// The names differ because the two vocabularies were designed apart: L0 + /// says `sys.quote(ticker:)` and the VM says `sys.stock(symbol, key)`. This + /// table is the whole of the translation, and a helper or field missing from + /// it means the seeded value is used — never a wrong call. + fn vm_call(binding: &SourceBinding) -> Option { + let arg = |name: &str| { + binding + .args + .iter() + .find(|(n, _)| n == name) + .map(|(_, v)| v.clone()) + }; + match binding.helper.as_str() { + "sys.quote" => { + let symbol = arg("ticker")?; + // What this helper can actually answer, verified against a live + // response rather than against its documentation. + // + // `open` is NOT here even though `sys.stock` accepts it: it + // resolves `regularMarketOpen`, and that key is absent from the + // Yahoo chart response while `regularMarketDayHigh`, `…DayLow`, + // `regularMarketPrice` and `chartPreviousClose` are all present. + // Emitting the call rendered `$—` on device where the seeded blob + // held a real opening price. Market cap and P/E are absent from + // the helper outright. + // + // This is the case the fallback exists for, and it took putting + // it on a phone to find — the DSL was well-formed and the unit + // tests were green. + let key = match binding.field.as_str() { + "last" => "price", + "pct" => "changepct", + "volume" => "vol", + f @ ("name" | "change" | "high" | "low") => f, + _ => return None, + }; + Some(format!("sys.stock({symbol:?}, {key:?})")) + } + _ => None, + } + } + fn text_of(value: Option<&NodeValue>) -> String { match value { Some(NodeValue::Text(s)) => format!("{s:?}"), @@ -4426,28 +4655,72 @@ pub mod makepad { /// A value plus its unit suffix, which is a presentation concern the runtime /// owns — the card never wrote "°" next to a number. fn valued(node: &UiNode) -> String { + // Prefer a live call, where the backend can answer the source itself and + // the decoration survives being concatenated around it. + if let Some(live) = live_valued(node) { + return live; + } + valued_seeded(node) + } + + /// The live form, or `None` when any part of it does not survive. + fn live_valued(node: &UiNode) -> Option { + let decoration = decoration_of(node); + live_value( + node, + &decoration.glyph, + decoration.unit, + &decoration.suffix, + decoration.format.as_deref(), + ) + } + + /// The realized value with its decoration — what is drawn when nothing goes + /// live, and what is MEASURED even when something does. + /// What wraps a value: a leading glyph, a unit, a trailing word, and how the + /// number itself is formatted. Read once and shared, so the live path and + /// the seeded path cannot disagree about what decorates what. + struct Decoration { + glyph: String, + unit: &'static str, + suffix: String, + format: Option, + } + + fn decoration_of(node: &UiNode) -> Decoration { + Decoration { + unit: match arg(node, "unit") { + Some(NodeValue::Token(t)) if t == "c" || t == "f" => "°", + Some(NodeValue::Text(t)) if t == "c" || t == "f" => "°", + Some(NodeValue::Token(t)) if t == "pct" => "%", + _ => "", + }, + glyph: match arg(node, "glyph") { + Some(NodeValue::Text(g)) => g.clone(), + _ => String::new(), + }, + // `suffix` is a declared unit word — "412 pts", not "412". It was in + // the catalog and ignored here, so every score and comment count on + // the news card rendered as a bare number. + suffix: match arg(node, "suffix") { + Some(NodeValue::Text(t)) => format!(" {t}"), + _ => String::new(), + }, + format: match arg(node, "format") { + Some(NodeValue::Token(t)) => Some(t.clone()), + _ => None, + }, + } + } + + fn valued_seeded(node: &UiNode) -> String { let value = arg(node, "value"); - let unit = match arg(node, "unit") { - Some(NodeValue::Token(t)) if t == "c" || t == "f" => "°", - Some(NodeValue::Text(t)) if t == "c" || t == "f" => "°", - Some(NodeValue::Token(t)) if t == "pct" => "%", - _ => "", - }; - let glyph = match arg(node, "glyph") { - Some(NodeValue::Text(g)) => g.clone(), - _ => String::new(), - }; - // `suffix` is a declared unit word — "412 pts", not "412". It was in the - // catalog and ignored here, so every score and comment count on the news - // card rendered as a bare number. - let suffix = match arg(node, "suffix") { - Some(NodeValue::Text(t)) => format!(" {t}"), - _ => String::new(), - }; - let format_kind = match arg(node, "format") { - Some(NodeValue::Token(t)) => Some(t.clone()), - _ => None, - }; + let Decoration { + glyph, + unit, + suffix, + format: format_kind, + } = decoration_of(node); match value { Some(NodeValue::Missing) => "\"—\"".into(), Some(v) => { @@ -4459,8 +4732,88 @@ pub mod makepad { }; format!("{:?}", format!("{glyph}{body}{unit}{suffix}")) } - None => text_of(arg(node, "text")), + None => expr_of(node, "text"), + } + } + + /// What to MEASURE when a layout decision depends on how long a value is. + /// + /// Emitted text and rendered text are the same thing until a value goes + /// live, at which point they diverge completely: `"$" + sys.stock(…)` is 33 + /// characters that draw as six. Any decision made by counting the emitted + /// form is then wrong, and wrong in a way that looks like a layout bug. + /// + /// So measure the realized literal, which is still in `args` and is a sound + /// proxy for the live value's length — a share price is about as long as a + /// share price. When nothing is bound, this IS the emitted text. + fn sizing_text(node: &UiNode, emitted: &str) -> String { + if node + .bindings + .iter() + .any(|(n, _)| n == "value" || n == "text") + { + let literal = valued_literal(node); + if !literal.is_empty() { + return literal; + } + } + emitted.to_owned() + } + + /// The realized value with every decoration applied — exactly what would + /// have been drawn had nothing gone live. + /// + /// This is `valued`'s literal path, reached directly. Reconstructing the + /// decoration here instead would give two formatters to keep in step, and + /// the one used for measuring would drift from the one used for drawing + /// without anything failing. + fn valued_literal(node: &UiNode) -> String { + valued_seeded(node) + } + + /// A `value` argument as a live call plus its decoration, or `None` to use + /// the realized literal. + /// + /// The decoration is the constraint. `glyph`, `unit` and `suffix` are + /// literals that concatenate around a call — the DSL supports + /// `"H:" + sys.weather(…)` and hand-written cards use it. A `format` mostly + /// does not, because it needs the NUMBER: + /// + /// | format | live? | why | + /// |---|---|---| + /// | none | yes | nothing to apply | + /// | `money` | yes | `sys.stock` returns two decimals, so `"$"` prefixes it | + /// | `signed_pct` | yes | the VM already returns `"+0.63%"` — applying it again would double the sign | + /// | `signed_money` | no | the sign goes OUTSIDE the currency symbol; not a prefix | + /// | `compact`, `ratio` | no | need the number to divide or round | + /// + /// Returning `None` is the safe direction: the seeded value is stale but + /// correct, where a wrong concatenation is neither. + fn live_value( + node: &UiNode, + glyph: &str, + unit: &str, + suffix: &str, + format_kind: Option<&str>, + ) -> Option { + let (_, binding) = node.bindings.iter().find(|(n, _)| n == "value")?; + let call = vm_call(binding)?; + let prefix = match format_kind { + None | Some("signed_pct") => String::new(), + Some("money") => "$".to_owned(), + Some(_) => return None, + }; + let mut expr = String::new(); + let head = format!("{glyph}{prefix}"); + if !head.is_empty() { + expr.push_str(&format!("{head:?} + ")); + } + expr.push_str(&call); + let tail = format!("{unit}{suffix}"); + if !tail.is_empty() { + expr.push_str(&format!(" + {tail:?}")); } + Some(expr) } fn pad(depth: usize) -> String { @@ -4630,7 +4983,7 @@ pub mod makepad { let _ = writeln!( out, "{p} TextCaption{{ text: {} draw_text.color: {DIM} }}", - text_of(arg(node, "label")) + expr_of(node, "label") ); let _ = writeln!( out, @@ -4713,7 +5066,7 @@ pub mod makepad { let _ = writeln!( out, "{p} TextCaption{{ text: {} draw_text.color: {ink} }}", - text_of(arg(node, "text")) + expr_of(node, "text") ); let _ = writeln!(out, "{p}}}"); } @@ -4741,14 +5094,22 @@ pub mod makepad { let body = if arg(node, "value").is_some() || arg(node, "glyph").is_some() { valued(node) } else { - text_of(arg(node, "text")) + expr_of(node, "text") }; // A hero is sized for the ONE dominant value. "18°" fits at 62; // "$184.20" clipped off the right edge on device. Scaling to fit // is the runtime's call — the card says "this is the hero", not // "this is 62 points". + // + // Size from the REALIZED value, never from the emitted text. Once + // a value lowers to a live call the emitted text is + // `"$" + sys.stock("NVDA", "price")` — 33 characters of source + // that render as six — so measuring it dropped the hero from 40pt + // to 24pt and shifted the whole card up. The seeded value is a + // sound proxy for the live one's LENGTH, which is all this needs. let size = if role == "TextHero" { - let glyphs = body.trim_matches('"').chars().count(); + let measured = sizing_text(node, &body); + let glyphs = measured.trim_matches('"').chars().count(); let pt = match glyphs { 0..=4 => 62, 5..=6 => 50, @@ -4979,6 +5340,11 @@ pub fn dispatch_with( /// `cycle`, `clear` and `set(.token)` need nothing. Only a transition that /// names a path needs the host's data, and passing Null simply makes that one /// form a no-op instead of writing something wrong. +/// Apply an event's transitions, with the data a path-valued initial resolves +/// against. +/// +/// Returns only whether anything applied. `dispatch_reporting` returns that plus +/// what went stale, and is what a host driving refetch should call. pub fn dispatch_with_data( source: &str, store: &mut InstanceStore, @@ -4987,9 +5353,69 @@ pub fn dispatch_with_data( payload: Option<&serde_json::Value>, data: &serde_json::Value, ) -> bool { + !dispatch_writes(source, store, instance_key, event, payload, data).is_empty() +} + +/// What a dispatch did, and what it obliges the host to do next. +/// +/// `dispatch_with_data` returns a bare `bool`, which is enough to know a tap was +/// not ignored and not enough to act on. §5.9 says a source parameterised by +/// state goes stale when that state changes, and a host holding only `true` has +/// no way to find out which. The pieces to compute it existed — `stale_sources` +/// and `source_plan` — and nothing joined them, so the stock card's detail view +/// worked only because its test data already contained a quote it never asked +/// for. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct DispatchOutcome { + /// Whether any transition committed. False means refused or unknown — see + /// §3, a batch that does not fully resolve does not partially apply. + pub applied: bool, + /// The state paths written, in the order the batch wrote them. + pub changed: Vec, + /// The declared sources those writes invalidate, following the dependency + /// cascade. The host should refetch these before the next realization. + pub stale: Vec, +} + +/// Apply an event and report what it invalidated. +/// +/// This is `dispatch_with_data` plus the answer to "and now what?". Prefer it +/// in a host: the bool-returning form cannot drive a refetch, which is the +/// whole point of letting a source take state as an argument. +pub fn dispatch_reporting( + source: &str, + store: &mut InstanceStore, + instance_key: &str, + event: &str, + payload: Option<&serde_json::Value>, + data: &serde_json::Value, +) -> DispatchOutcome { + let changed = dispatch_writes(source, store, instance_key, event, payload, data); + if changed.is_empty() { + return DispatchOutcome::default(); + } + // `stale_sources` takes the changed names; a write to `selected` is a change + // to `selected` as a source argument reads it. + let names: Vec<&str> = changed.iter().map(String::as_str).collect(); + let stale = stale_sources(source, &names); + DispatchOutcome { + applied: true, + changed, + stale, + } +} + +fn dispatch_writes( + source: &str, + store: &mut InstanceStore, + instance_key: &str, + event: &str, + payload: Option<&serde_json::Value>, + data: &serde_json::Value, +) -> Vec { let mut sink = Diagnostics::default(); let Some(tokens) = lex(source, &mut sink) else { - return false; + return Vec::new(); }; let card = Parser::new(&tokens, &mut sink).parse_card(); @@ -5026,11 +5452,11 @@ pub fn dispatch_with_data( &instance_key[..boundary], Some((component, schema_id(component))), ), - None => return false, + None => return Vec::new(), }, None => match card.events.iter().find(|e| e.name == event) { Some(d) => (d, &card.states, CARD_STATE_KEY, None), - None => return false, + None => return Vec::new(), }, }; @@ -5046,7 +5472,7 @@ pub fn dispatch_with_data( for transition in &declared.transitions { let Some(state) = states.iter().find(|s| s.path == transition.target) else { - return false; + return Vec::new(); }; // The declared initial, path-valued or not. `clear` and a first `cycle` // both fell back to the SHAPE default here, so a card whose initial @@ -5085,10 +5511,10 @@ pub fn dispatch_with_data( // that has to be checked at runtime: every other set form is // decided against the declared shape at check time. Some(v) if value_fits_shape(&state.shape, v) => v.clone(), - Some(_) => return false, + Some(_) => return Vec::new(), // No payload is a no-op rather than a silent clear: falling // back to the initial would look like a deliberate reset. - None => return false, + None => return Vec::new(), }, // A path READS. Treating it as the payload meant // `n: set(config.answer)` wrote whatever the tap carried — or @@ -5097,23 +5523,28 @@ pub fn dispatch_with_data( Some(v) if value_fits_shape(&state.shape, &v) => v, // Unresolvable or ill-shaped: the batch cannot complete, and // §3 says a batch is all or nothing. - _ => return false, + _ => return Vec::new(), }, SetSource::Token(t) | SetSource::Text(t) => serde_json::Value::String(t.clone()), SetSource::Num(n) => serde_json::Value::from(*n), SetSource::Bool(b) => serde_json::Value::Bool(*b), }, - _ => return false, + _ => return Vec::new(), }; staged.push((transition.target.clone(), next)); } - // Commit. - let applied = !staged.is_empty(); + // Commit, and report what was written. The targets are what a host needs + // to know which sources went stale -- discarding them is why §5.9's + // invalidation story had pieces that nothing joined. + let mut written = Vec::new(); for (target, value) in staged { store.set(instance_key, &target, value); + if !written.contains(&target) { + written.push(target); + } } - applied + written } // ───────────────────────────────────────────────────────────────── source plan ── diff --git a/crates/splash-ui-l0/tests/fixtures/stock.card b/crates/splash-ui-l0/tests/fixtures/stock.card index 33b90ec..c9d3d23 100644 --- a/crates/splash-ui-l0/tests/fixtures/stock.card +++ b/crates/splash-ui-l0/tests/fixtures/stock.card @@ -13,8 +13,6 @@ source movers sys.movers(count: 10, fields: [ticker, name, last, change, pc source quote sys.quote(ticker: state.selected, fields: [name, last, change, pct, open, high, low, volume, mktcap, pe]) -source series sys.series(ticker: state.selected, range: state.range, - fields: [points, min, max]) source env.locale sys.locale() state selected { shape: text, initial: "" } # "" ⇒ movers list diff --git a/crates/splash-ui-l0/tests/fixtures/weather.card b/crates/splash-ui-l0/tests/fixtures/weather.card index 01a6c38..59461c1 100644 --- a/crates/splash-ui-l0/tests/fixtures/weather.card +++ b/crates/splash-ui-l0/tests/fixtures/weather.card @@ -21,7 +21,6 @@ source week sys.weather(lat: place.lat, lon: place.lon, days: state.days, # back with the data they describe. source sun sys.daylight(lat: place.lat, lon: place.lon) source moon sys.moonphase() -source aqi sys.airquality(lat: place.lat, lon: place.lon) source scene sys.photo(query: place.name) source env.locale sys.locale() diff --git a/crates/splash-ui-l0/tests/profile.rs b/crates/splash-ui-l0/tests/profile.rs index 36be672..aecc59f 100644 --- a/crates/splash-ui-l0/tests/profile.rs +++ b/crates/splash-ui-l0/tests/profile.rs @@ -682,6 +682,7 @@ fn an_unmapped_constructor_is_visible_rather_than_dropped() { key: "root".into(), args: vec![], children: vec![], + bindings: vec![], }; let dsl = makepad::lower(&node); assert!(dsl.contains("no makepad lowering for Hologram"), "{dsl}"); @@ -897,9 +898,9 @@ fn declared_formats_are_applied_by_the_runtime() { let report = realize(STOCK, &data, RealizeLimits::default()); let dsl = makepad::lower(&report.root.expect("root")); + // A format that needs the NUMBER is applied here, because it cannot be + // applied around a call the backend will make later. for (expect, why) in [ - ("$184.20", "money gains a currency symbol and two places"), - ("+1.7%", "a signed percentage keeps its sign"), ("41.2M", "compact scales volume"), ("4.5T", "compact scales market cap"), ("58.3", "a ratio is plain"), @@ -910,6 +911,77 @@ fn declared_formats_are_applied_by_the_runtime() { !dsl.contains("41200000"), "the raw number should not survive" ); + + // A format that is a literal prefix, or that the backend already applies, + // survives — so these lower to a LIVE call rather than to the seeded value. + // The card then shows the price now, not the price when the blob was made. + assert!( + dsl.contains(r#""$" + sys.stock("NVDA", "price")"#), + "money should prefix a live call:\n{dsl}" + ); + assert!( + dsl.contains(r#"sys.stock("NVDA", "changepct")"#), + "signed_pct is what the VM already returns, so the call stands alone:\n{dsl}" + ); + // And the seeded values they replaced must be GONE, or the card would show + // a stale number beside a live one. + assert!( + !dsl.contains("$184.20") && !dsl.contains("+1.7%"), + "a live call must replace the seeded literal, not sit beside it:\n{dsl}" + ); +} + +/// The fallback is not a failure mode to be minimised — it is what keeps a +/// wrong number off the screen. +/// +/// `sys.stock` exposes no market cap and no P/E, and `compact`/`ratio` need the +/// number itself. Emitting a call for those would render an em dash where a real +/// value was available, so they stay seeded and this asserts they do. +#[test] +fn a_capability_the_backend_cannot_answer_keeps_its_seeded_value() { + let data = serde_json::json!({ + "movers": [], "selected": "NVDA", "range": "m1", "env": {"locale":{}}, + "quote": {"name":"NVIDIA","last":184.2,"change":3.1,"pct":1.7,"open":181.0, + "high":185.6,"low":180.2,"volume":41200000.0, + "mktcap":4520000000000.0,"pe":58.3}, + "copy": {"movers":"Top Movers","open":"Open","high":"High","low":"Low", + "volume":"Volume","mktcap":"Mkt Cap","pe":"P/E"} + }); + let report = realize(STOCK, &data, RealizeLimits::default()); + let dsl = makepad::lower(&report.root.expect("root")); + + assert!( + !dsl.contains("sys.stock(\"NVDA\", \"mktcap\")") && !dsl.contains("\"pe\")"), + "no call may be emitted for a field the helper does not expose:\n{dsl}" + ); + assert!( + dsl.contains("4.5T"), + "market cap keeps its seeded value:\n{dsl}" + ); + assert!(dsl.contains("58.3"), "P/E keeps its seeded value:\n{dsl}"); + + // `open` is the one this test exists for. `sys.stock` ACCEPTS the key, so + // the mapping looked correct and the DSL was well-formed — but it resolves + // `regularMarketOpen`, which the Yahoo chart response does not carry, while + // `regularMarketDayHigh` and `…DayLow` beside it do. On device that rendered + // `$—` where the seeded blob held a real opening price. + // + // A helper accepting a key is not evidence it can answer it, and no unit + // test could have found this: it took a screenshot. + assert!( + !dsl.contains(r#"sys.stock("NVDA", "open")"#), + "open must not be emitted live — the field is absent upstream:\n{dsl}" + ); + assert!( + dsl.contains("$181"), + "open keeps its seeded value instead:\n{dsl}" + ); + // High and low DO resolve, so they must still be live — otherwise the fix + // for `open` could quietly have been "stop emitting calls at all". + assert!( + dsl.contains(r#"sys.stock("NVDA", "high")"#) && dsl.contains(r#"sys.stock("NVDA", "low")"#), + "high and low resolve upstream and must stay live:\n{dsl}" + ); } /// A float hour is a time, not a decimal: 18.9 is 18:54. @@ -1122,7 +1194,10 @@ fn the_plan_orders_dependencies_before_dependents() { let at = |name: &str| plan.requests.iter().position(|r| r.name == name).unwrap(); assert!(at("place") < at("now"), "place must precede now"); assert!(at("place") < at("week"), "place must precede week"); - assert!(at("place") < at("aqi"), "place must precede aqi"); + // `scene`, not `aqi`: the air-quality source went away when `AqiContour` + // began fetching its own field, and a source nothing reads is now refused. + // `scene` reads `place.name`, so it makes the same point. + assert!(at("place") < at("scene"), "place must precede scene"); let now = &plan.requests[at("now")]; assert_eq!(now.depends_on, vec!["place".to_string()]); @@ -3566,13 +3641,14 @@ fn a_loop_key_must_name_the_loop_binder() { fn a_source_argument_path_must_be_declared() { let report = check_ui_l0_named( "leak-arg", - "source photo sys.photo(query: secrets.token)\nview root Rule()", + "source photo sys.photo(query: secrets.token)\nview root Photo(src: photo.url)", ); assert!(!report.valid, "`secrets` is not declared"); // The plan must not carry it either, for a caller that skips checking. - let plan = - splash_ui_l0::source_plan("source photo sys.photo(query: secrets.token)\nview root Rule()"); + let plan = splash_ui_l0::source_plan( + "source photo sys.photo(query: secrets.token)\nview root Photo(src: photo.url)", + ); assert!( !plan.diagnostics.is_empty(), "the plan must report it rather than hand the host an undeclared path" @@ -3582,8 +3658,12 @@ fn a_source_argument_path_must_be_declared() { assert!( check_ui_l0_named( "ok", + // The view reads `place`. A source nothing reads is refused now, and + // this assertion proved a DECLARED argument path is accepted only by + // accident while the source it declared reached no view at all. "state city { shape: text, initial: \"\" }\n\ - source place sys.geocode(name: city)\nview root Rule()" + source place sys.geocode(name: city)\n\ + view root TextRow(text: place.name)" ) .valid ); @@ -4124,3 +4204,151 @@ fn news_fixture() -> serde_json::Value { "article": {}, "selected": "", "env": {"locale": {}} }) } + +// ─── the three defects the state-ownership review found ────────────────────── +// Each was recorded in §8 of the profile as "found and not fixed". These are +// the tests that make them fixable rather than merely known. + +/// §5.3 keeps card state out of a component, and a name collision walks around +/// the check: `check_path` asks `scope.knows(path)` BEFORE asking whether the +/// path names forbidden card state, so a source with the same name satisfies +/// the first question and the second is never reached. +/// +/// The component ends up reading the SOURCE, which §5.3 permits — so this is +/// not a confinement breach and card state does not leak. It is worse in a +/// quieter way: the card says `state selected` and `source selected` and the +/// runtime silently picks one. Two namespaces that must be disjoint are not +/// checked for overlap at all. +#[test] +fn a_source_may_not_share_a_name_with_card_state() { + const CARD: &str = r#" +source selected sys.locale() +state selected { shape: text, initial: "" } +view root TextTitle(text: selected) +"#; + let report = check_ui_l0_named("t", CARD); + assert!( + !report.valid, + "a source and a state sharing a name must be refused:\n{:#?}", + report.diagnostics + ); + assert!( + report + .diagnostics + .iter() + .any(|d| d.message.contains("selected")), + "the diagnostic must name the colliding name:\n{:#?}", + report.diagnostics + ); +} + +/// A declared source nothing reads is a fetch the host performs for nothing. +/// +/// `stock.card` shipped one and no test noticed: `series` became unread the +/// moment `StockPlot` started fetching its own data, and the card kept asking +/// for it. On device that is a network round trip, a parse, and a cache entry +/// per refresh, for data that reaches no pixel. +#[test] +fn a_source_nothing_reads_is_reported() { + const CARD: &str = r#" +source used sys.locale() +source unused sys.movers(count: 1, fields: [ticker]) +view root TextTitle(text: used.lang) +"#; + let report = check_ui_l0_named("t", CARD); + assert!( + report + .diagnostics + .iter() + .any(|d| d.message.contains("unused")), + "an unread source must be reported:\n{:#?}", + report.diagnostics + ); + assert!( + !report + .diagnostics + .iter() + .any(|d| d.message.contains("used.") + || (d.message.contains("used") && !d.message.contains("unused"))), + "and a source that IS read must not be:\n{:#?}", + report.diagnostics + ); +} + +/// §5.9's invalidation story exists in pieces that nothing joins. +/// +/// `stale_sources` computes what a change invalidates and `source_plan` lists +/// what to request, and both are called only from tests. `dispatch_with_data` +/// returns a bare `bool` and discards WHICH fields it wrote — so a host holding +/// the result cannot tell what to refetch, and the stock card's detail view +/// works only because the test data already contains a quote it never asked +/// for. +#[test] +fn a_dispatch_reports_what_it_invalidated() { + let data = serde_json::json!({ + "movers": [{ "ticker": "NVDA", "name": "Nvidia", "last": 1.0, "change": 0.5, "pct": 2.0 }], + "quote": {}, "selected": "", "range": "m1", "env": { "locale": {} }, + "copy": { "movers": "Top Movers" } + }); + let mut store = splash_ui_l0::InstanceStore::default(); + let outcome = splash_ui_l0::dispatch_reporting( + STOCK, + &mut store, + "root", + "open_quote", + Some(&serde_json::Value::String("NVDA".into())), + &data, + ); + assert!(outcome.applied, "the event applies"); + assert!( + outcome.changed.iter().any(|f| f == "selected"), + "it must report WHICH state it wrote: {:?}", + outcome.changed + ); + // `quote` and `series` both take `ticker: state.selected`, so both are now + // stale. This is the fact a host needs and could not previously obtain. + assert!( + outcome.stale.iter().any(|s| s == "quote"), + "and which sources that invalidates: {:?}", + outcome.stale + ); +} + +/// A layout decision must measure what is DRAWN, not what is emitted. +/// +/// The two are the same string until a value goes live, and then they diverge +/// completely: `"$" + sys.stock("NVDA", "price")` is 33 characters of source +/// that render as six. Sizing the hero by counting the emitted form dropped it +/// from 40pt to 24pt and shifted the whole card up — a layout bug with no +/// layout cause, and invisible to every test that only checked the text. +#[test] +fn a_hero_is_sized_by_what_it_draws_not_by_what_it_emits() { + let data = serde_json::json!({ + "movers": [], "selected": "NVDA", "range": "m1", "env": {"locale":{}}, + "quote": {"name":"NVIDIA","last":184.2,"change":3.1,"pct":1.7,"open":181.0, + "high":185.6,"low":180.2,"volume":41200000.0, + "mktcap":4520000000000.0,"pe":58.3}, + "copy": {"movers":"Top Movers","open":"Open","high":"High","low":"Low", + "volume":"Volume","mktcap":"Mkt Cap","pe":"P/E"} + }); + let report = realize(STOCK, &data, RealizeLimits::default()); + let dsl = makepad::lower(&report.root.expect("root")); + + let hero = dsl + .lines() + .find(|l| l.contains("TextHero")) + .unwrap_or_else(|| panic!("no hero:\n{dsl}")); + + // The hero IS live — otherwise this test would pass for the wrong reason, + // since a seeded hero measures its own text correctly by construction. + assert!( + hero.contains("sys.stock"), + "the hero must be live for this test to mean anything:\n{hero}" + ); + // "$184.20" is 7 glyphs, which is the 40pt bucket. The emitted expression is + // 33 characters, which is the 24pt one. + assert!( + hero.contains("font_size: 40"), + "sized by the drawn value (7 glyphs -> 40pt), not the emitted 33:\n{hero}" + ); +} diff --git a/docs/ui-profile-l0.md b/docs/ui-profile-l0.md index 3a2f641..d674a4b 100644 --- a/docs/ui-profile-l0.md +++ b/docs/ui-profile-l0.md @@ -742,6 +742,58 @@ The third is not the first. Reusing its name for instance identity would put per into a global map keyed by whatever the kit chose, which is the collision §5.7 exists to prevent. +### 5.11 Who answers a source + +A `source` declares *what the card needs*, not *who fetches it*. Two answers are +now implemented, and the choice belongs to the backend rather than to the card: + +| | who fetches | when it is chosen | +|---|---|---| +| **Host-answered** | the host walks `source_plan` and fills a data blob | always available; the only option for a backend with no fetch of its own | +| **Backend-answered** | lowering emits a call the backend makes itself | where the backend has a capability that answers the same question | + +The card is identical either way. `source quote sys.quote(ticker: state.selected)` +followed by `quote.last` is one declaration; whether it becomes a seeded number or +a live call is decided below the language. + +**Realization keeps the provenance.** `UiNode.bindings` records, per argument, +which source a value came from and which field of it — alongside the resolved +value, not instead of it. Without that, a backend that *could* fetch has no way +to know it should: by the time lowering runs, `quote.last` is just a number. + +**A backend must fall back rather than guess.** `sys.quote` declares ten fields; +the makepad VM's `sys.stock` answers seven of them. The rest lower to the seeded +value, because a call the backend cannot answer renders an em dash where a real +number was available. Three findings, each from running it on a phone: + +- **`open` accepts and cannot answer.** `sys.stock` takes the key and resolves + `regularMarketOpen`, which is absent from the upstream response while + `regularMarketDayHigh` and `…DayLow` beside it are present. A helper accepting + a key is not evidence it can answer it. +- **Most formats do not survive.** `glyph`, `unit` and `suffix` are literals that + concatenate around a call. A `format` mostly is not: `money` is a `"$"` prefix + and `signed_pct` is what the VM already returns, but `signed_money` puts the + sign outside the currency symbol and `compact` and `ratio` need the number + itself. Those keep the seeded value. +- **Anything that measures text must measure what is DRAWN.** Emitted and drawn + text are the same string until a value goes live, and then they diverge + completely — `"$" + sys.stock("NVDA", "price")` is 33 characters that render as + six. Sizing the hero by the emitted form dropped it from 40pt to 24pt and + shifted the whole card up: a layout bug with no layout cause. + +**A live card cannot be asserted by a whole-screen golden**, and the device +harness now says so per case. Where values are live the comparison is restricted +to a band that is still deterministic, and everything outside it is checked only +for having rendered. That is weaker than the old golden and true, where the old +golden would have been stronger and false — it would have been asserting a share +price. + +**What this costs.** `source_plan` and its dependency ordering have no consumer +on a backend that answers its own sources, and the translation table between L0's +capability names and a backend's helper names lives in that backend's lowering. +Both are the price of reaching live data without a second fetch layer, and both +are recorded here rather than discovered later. + --- ## 6. What makes L0 terminate @@ -867,19 +919,34 @@ An earlier version of this paragraph said `StockPlot` and `AqiContour` "lower wi arrays". They have no data arrays: both name what to plot and fetch it themselves, and the catalog claiming otherwise was the defect. -**Three defects found by reviewing §5.10, not yet fixed.** Each is a checker gap rather than a -language question: - -- **A `source` and a card `state` may share a name**, and nothing rejects the collision. The - component then reads the source — §5.3 is not violated and card state does not leak — but - silent shadowing is not a defensible way to resolve two namespaces that should be disjoint. -- **A declared source that nothing reads is not reported.** `stock.card` shipped one: `series` - became unread when `StockPlot` started fetching its own, and no test noticed. An unread source - is a fetch the host performs for nothing. -- **Nothing connects a state write to a refetch.** `stale_sources` computes what a change - invalidates and `source_plan` lists what to request; both are called only from tests. - `dispatch_with_data` returns a bare `bool` and discards which fields changed. The pieces of - §5.9's invalidation story exist and are individually tested; the loop between them does not. +**Three defects found by reviewing §5.10, now fixed.** Each was a checker gap rather than a +language question, and each is held by a mutation rule: + +- **A `source` and a card `state` sharing a name is refused.** The component would have read the + source — §5.3 was not violated and card state did not leak — but which of the two a name + resolves to depended on lookup order rather than on anything the card said. +- **A declared source that nothing reads is refused.** An unread source is a fetch the host + performs on every refresh for data that reaches no pixel. Two shipped in the reference cards + and no test noticed: `series` and `aqi` both went unread the moment `StockPlot` and + `AqiContour` began fetching their own data. Both are now gone from the cards. + + One implicit read has to be counted or this rejects correct cards: resolving `copy.x` looks up + `env.locale.lang` to choose a translation, and no path in the card names it. A purely syntactic + scan calls `env.locale` dead on every card with copy and no other locale binding — which was + two of the three reference cards. + +- **A state write reports what it invalidated.** `dispatch_reporting` returns a `DispatchOutcome` + — what applied, which state paths were written, and which sources those writes made stale, + following the cascade. The pieces existed (`stale_sources`, `source_plan`) and nothing joined + them, so a host holding `true` could not tell what to refetch. `dispatch_with_data` keeps its + `bool` for callers that do not need the answer. + +**Fixing the second one exposed a test that passed for the wrong reason**, which is the more +useful finding. `a_source_argument_path_must_be_declared` rejected a card carrying an undeclared +path in a source argument — and its fixture ended `view root Rule()`, so the source went unread. +Once unread sources were refused, the card was rejected for *that* instead, and the test passed +with the rule it exists to protect switched off. The mutation harness reported it as unprotected +the moment the new rule landed. Component contracts were question 1 and are now specified in §5.2–§5.8, validated by adding components to two reference cards. That exercise produced amendment #9, which nothing had diff --git a/mutate.py b/mutate.py index fa69a09..665f133 100755 --- a/mutate.py +++ b/mutate.py @@ -62,6 +62,16 @@ class Rule: "None => check_path(path, scope, source.line, 1, sink),", "None => { let _ = (path, scope); }"), + Rule("source-state-collision", "a source and card state may not share a name", + "if let Some(source) = card.sources.iter().find(|s| s.name == state.path) {", + "if let Some(source) = card.sources.iter().find(|s| s.name == state.path).filter(|_| false) {"), + # Mutating to "never report" rather than "always report": the risk is a dead + # fetch going unnoticed, which is how `series` and `aqi` both survived a + # catalog correction that made them unreachable. + Rule("unread-source", "a declared source nothing reads is reported", + "if !read.contains(&source.name) && !read.contains(&root_of(&source.name)) {", + "if false {"), + # ── the no-facts rule (§4) ─────────────────────────────────────────────── Rule("data-laundering", "a literal may not reach a data position through a prop", "if matches!(arg.value, Operand::Str(_) | Operand::Num(_))",