From 41e88e3c40e4d5e16ba97cf7489c8699e317e668 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 26 Oct 2026 10:12:22 +0900 Subject: [PATCH] fix: separate playground values from object descriptions --- site/browser/src/manual/playground_tests.rs | 117 +++++++++++++---- site/src/scripts/playground-tree.ts | 12 +- site/worldspace/src/lib.rs | 22 ++-- site/worldspace/tests/playground.rs | 131 ++++++++++++-------- 4 files changed, 190 insertions(+), 92 deletions(-) diff --git a/site/browser/src/manual/playground_tests.rs b/site/browser/src/manual/playground_tests.rs index cd8045a..20d0cfa 100644 --- a/site/browser/src/manual/playground_tests.rs +++ b/site/browser/src/manual/playground_tests.rs @@ -28,6 +28,22 @@ fn collection(s: &mut BrowserSession, operation: &str, name: &str) -> Result Vec<&str> { observed["observation"]["children"] .as_array() @@ -38,14 +54,14 @@ fn names(observed: &Json) -> Vec<&str> { } #[test] -fn explicit_observe_refreshes_counter_and_parent_but_does_not_retry_or_fill_interfaces() { +fn explicit_observe_preserves_counter_metadata_and_does_not_call_retry_or_fill_interfaces() { let mut s = session(); s.manual_observe("/book", 1.0).unwrap(); let initial = s.manual_observe("/playground", 1.0).unwrap(); assert_eq!(names(&initial), ["apple", "orange"]); assert_eq!( initial["observation"]["children"][0]["object"]["description"], - "3" + "An in-memory counter." ); assert!( increment(&mut s, 4).is_err(), @@ -53,6 +69,7 @@ fn explicit_observe_refreshes_counter_and_parent_but_does_not_retry_or_fill_inte ); assert_eq!(s.calls, 0); s.manual_interface("/", "counter").unwrap(); + assert_value(&mut s, "apple", 3); let stats = s.manual_stats(); let result = increment(&mut s, 4).unwrap(); assert_eq!(result["result"], json!({"kind":"integer","value":"7"})); @@ -60,7 +77,8 @@ fn explicit_observe_refreshes_counter_and_parent_but_does_not_retry_or_fill_inte assert!(result.get("textView").is_none()); assert_eq!(s.observes, stats["observes"].as_u64().unwrap() as u32); assert_eq!(s.interfaces, stats["interfaces"].as_u64().unwrap() as u32); - // A result did not silently fabricate a refreshed Client Object. + assert_value(&mut s, "apple", 7); + // Counter metadata is descriptive, not a cached counter value. assert_eq!( s.client .object(&s.session, "/playground/apple") @@ -69,9 +87,12 @@ fn explicit_observe_refreshes_counter_and_parent_but_does_not_retry_or_fill_inte .unwrap() .description .as_deref(), - Some("3") + Some("An in-memory counter.") ); + let calls = s.calls; let parent = s.manual_observe("/playground", 1.0).unwrap(); + assert_eq!(s.calls, calls, "Observe must not implicitly call get"); + assert_eq!(parent["observation"], initial["observation"]); assert_eq!(parent["cached"], false); assert_eq!( parent["observation"]["children"][0]["object"]["name"], @@ -79,18 +100,22 @@ fn explicit_observe_refreshes_counter_and_parent_but_does_not_retry_or_fill_inte ); assert_eq!( parent["observation"]["children"][0]["object"]["description"], - "7" + "An in-memory counter." ); let selected = s.manual_observe("/playground/apple", 1.0).unwrap(); assert_eq!(selected["cached"], false); - assert_eq!(selected["observation"]["object"]["description"], "7"); + assert_eq!( + selected["observation"]["object"], + initial["observation"]["children"][0]["object"] + ); + assert_eq!(s.calls, calls, "Observe must not implicitly call get"); assert_eq!( selected["textView"], wip_text_view::render_object(&s.fresh_object("/playground/apple").unwrap()).unwrap() ); assert_eq!(s.manual_observe("/book", 1.0).unwrap()["cached"], true); assert_eq!(s.manual_interface("/", "counter").unwrap()["cached"], true); - assert_eq!(s.calls, 1); + assert_eq!(s.calls, 3, "two explicit gets and one increment"); } #[test] @@ -122,19 +147,36 @@ fn collection_refresh_removes_stale_children_and_recovers_from_retained_not_foun collection(&mut s, "create", "apple").unwrap(); // Explicit fresh refresh recovers even from a retained Client NotFound. let selected = s.manual_observe("/playground/apple", 1.0).unwrap(); - assert_eq!(selected["observation"]["object"]["description"], "0"); + assert_eq!( + selected["observation"]["object"]["description"], + "An in-memory counter." + ); + assert_value(&mut s, "apple", 0); assert_eq!( names(&s.manual_observe("/playground", 1.0).unwrap()), ["apple", "orange", "pear"] ); assert_eq!(increment(&mut s, 2).unwrap()["result"]["value"], "2"); - s.manual_observe("/playground", 1.0).unwrap(); - s.manual_call("/playground/apple", "/", "counter", "reset", "{}") - .unwrap(); + assert_value(&mut s, "apple", 2); + let calls = s.calls; + let incremented = s.manual_observe("/playground/apple", 1.0).unwrap(); + assert_eq!(incremented["observation"], selected["observation"]); + let parent = s.manual_observe("/playground", 1.0).unwrap(); + assert_eq!(s.calls, calls, "Observe must not implicitly call get"); assert_eq!( - s.manual_observe("/playground", 1.0).unwrap()["observation"]["children"][0]["object"]["description"], - "0" + s.manual_call("/playground/apple", "/", "counter", "reset", "{}") + .unwrap()["result"], + json!({"kind":"integer","value":"0"}) ); + assert_value(&mut s, "apple", 0); + assert_eq!(s.calls, calls + 2, "one reset and one explicit get"); + let reset = s.manual_observe("/playground/apple", 1.0).unwrap(); + assert_eq!(reset["observation"], selected["observation"]); + assert_eq!( + s.manual_observe("/playground", 1.0).unwrap()["observation"], + parent["observation"] + ); + assert_eq!(s.calls, calls + 2, "Observe must not implicitly call get"); } #[test] @@ -143,9 +185,14 @@ fn failures_are_atomic_and_each_explicit_call_dispatches_at_most_once() { s.manual_observe("/playground", 2.0).unwrap(); s.manual_interface("/", "counter").unwrap(); s.manual_interface("/", "collection").unwrap(); - increment(&mut s, wip_protocol::MAX_SAFE_INTEGER - 3).unwrap(); - let maximum = s.manual_observe("/playground/apple", 1.0).unwrap(); + assert_eq!( + increment(&mut s, wip_protocol::MAX_SAFE_INTEGER - 3).unwrap()["result"], + json!({"kind":"integer","value":wip_protocol::MAX_SAFE_INTEGER.to_string()}) + ); + assert_value(&mut s, "apple", wip_protocol::MAX_SAFE_INTEGER); let calls = s.calls; + let maximum = s.manual_observe("/playground/apple", 1.0).unwrap(); + assert_eq!(s.calls, calls, "Observe must not implicitly call get"); let failure = increment(&mut s, 1).unwrap_err(); assert_eq!(failure.kind, "protocol"); assert_eq!(failure.code.as_deref(), Some("Internal")); @@ -154,15 +201,31 @@ fn failures_are_atomic_and_each_explicit_call_dispatches_at_most_once() { s.manual_observe("/playground/apple", 1.0).unwrap()["observation"], maximum["observation"] ); + assert_eq!(s.calls, calls + 1, "Observe must not implicitly call get"); + assert_value(&mut s, "apple", wip_protocol::MAX_SAFE_INTEGER); + assert_eq!( + s.calls, + calls + 2, + "one failed increment and one explicit get" + ); for name in ["apple", "bad/name", "", ""] { - let before = s.manual_observe("/playground", 1.0).unwrap(); let calls = s.calls; + let before = s.manual_observe("/playground", 1.0).unwrap(); + assert_eq!(s.calls, calls, "Observe must not implicitly call get"); assert!(collection(&mut s, "create", name).is_err()); assert_eq!(s.calls, calls + 1); assert_eq!( s.manual_observe("/playground", 1.0).unwrap()["observation"], before["observation"] ); + assert_eq!(s.calls, calls + 1, "Observe must not implicitly call get"); + assert_value(&mut s, "apple", wip_protocol::MAX_SAFE_INTEGER); + assert_value(&mut s, "orange", 0); + assert_eq!( + s.calls, + calls + 3, + "one failed create and two explicit gets" + ); } let calls = s.calls; assert!(increment(&mut s, i64::MAX).is_err()); @@ -170,6 +233,8 @@ fn failures_are_atomic_and_each_explicit_call_dispatches_at_most_once() { s.calls, calls, "out-of-range typed arguments rejected before Host dispatch" ); + assert_value(&mut s, "apple", wip_protocol::MAX_SAFE_INTEGER); + assert_eq!(s.calls, calls + 1, "only the explicit get dispatches"); } #[test] @@ -189,29 +254,39 @@ fn normal_tree_and_document_reads_ignore_dynamic_children_and_reload_resets_stat let book = s.read("/book").unwrap(); s.manual_interface("/", "counter").unwrap(); s.manual_interface("/", "collection").unwrap(); - increment(&mut s, 10).unwrap(); + assert_eq!(increment(&mut s, 10).unwrap()["result"]["value"], "13"); + assert_value(&mut s, "apple", 13); collection(&mut s, "create", "pear").unwrap(); collection(&mut s, "remove", "orange").unwrap(); s.manual_observe("/playground", 1.0).unwrap(); assert_eq!(s.tree().unwrap(), tree); assert_eq!(s.read("/playground").unwrap(), document); assert_eq!(s.read("/book").unwrap(), book); + let calls = s.calls; assert_eq!( s.manual_observe("/playground/apple", 1.0).unwrap()["observation"]["object"]["description"], - "13", - "navigation does not reset state" + "An in-memory counter." ); + assert_eq!(s.calls, calls, "Observe must not implicitly call get"); + // Navigation does not reset state; verify through an explicit operation. + assert_value(&mut s, "apple", 13); let mut reloaded = session(); let initial = reloaded.manual_observe("/playground", 1.0).unwrap(); assert_eq!(names(&initial), ["apple", "orange"]); assert_eq!( initial["observation"]["children"][0]["object"]["description"], - "3" + "An in-memory counter." ); assert_eq!( initial["observation"]["children"][1]["object"]["description"], - "0" + "An in-memory counter." ); + assert_eq!(reloaded.calls, 0, "Observe must not implicitly call get"); + reloaded.manual_interface("/", "counter").unwrap(); + assert_value(&mut reloaded, "apple", 3); + assert_value(&mut reloaded, "orange", 0); + assert_eq!(reloaded.calls, 2, "two explicit gets after reload"); + assert_value(&mut s, "apple", 13); } #[test] diff --git a/site/src/scripts/playground-tree.ts b/site/src/scripts/playground-tree.ts index 0cd0eb2..c9a2741 100644 --- a/site/src/scripts/playground-tree.ts +++ b/site/src/scripts/playground-tree.ts @@ -2,19 +2,18 @@ import type { TreeNode } from '../lib/landing-tree'; export interface PlaygroundChild { object: { name: string; description?: string | null }; } -/** Only Observe data supplies names and values. Never project an Operation result. */ +/** Only Observe data supplies tree membership and names, never counter values. */ export function playgroundNodes(children: readonly PlaygroundChild[], href: string): TreeNode[] { const names = new Set(); if (children.length > 32) throw new Error('Playground child limit exceeded'); return children.map(({ object }) => { - if (!/^[A-Za-z0-9_-]{1,32}$/.test(object.name) || names.has(object.name) - || !/^-?\d+$/.test(object.description ?? '')) throw new Error('Invalid Playground observation'); + if (!/^[A-Za-z0-9_-]{1,32}$/.test(object.name) || names.has(object.name)) throw new Error('Invalid Playground observation'); names.add(object.name); const path = `/playground/${object.name}`; const url = new URL(href, location.href); url.searchParams.set('mode', 'protocol'); url.searchParams.set('object', path); - return { name: object.name, title: object.description!, path, parent: '/playground', depth: 3, href: `${url.pathname}${url.search}` }; + return { name: object.name, path, parent: '/playground', depth: 3, href: `${url.pathname}${url.search}` }; }); } @@ -28,9 +27,8 @@ export function playgroundRow(node: TreeNode, prefixTemplate: HTMLElement, index const anchor = document.createElement('a'); anchor.className = 'demo-node'; anchor.href = node.href!; anchor.dataset.path = node.path; anchor.dataset.wipPath = node.path; anchor.setAttribute('role', 'treeitem'); anchor.setAttribute('aria-level', '3'); anchor.tabIndex = -1; - anchor.setAttribute('aria-label', `${node.name}, ${node.title}`); + anchor.setAttribute('aria-label', node.name); const label = document.createElement('span'); label.className = 'node-label'; label.textContent = node.name; - const value = document.createElement('span'); value.className = 'node-value'; value.textContent = node.title!; - anchor.append(label, value); row.append(prefix, anchor); + anchor.append(label); row.append(prefix, anchor); return row; } diff --git a/site/worldspace/src/lib.rs b/site/worldspace/src/lib.rs index 6082e90..110600a 100644 --- a/site/worldspace/src/lib.rs +++ b/site/worldspace/src/lib.rs @@ -68,11 +68,7 @@ pub trait Document { /// Host-private implementation; no body lookup bypass is exposed to clients. pub enum SiteObject { Document(Arc, Option>), - Counter { - name: String, - description: String, - state: Rc, - }, + Counter { name: String, state: Rc }, } impl WipObject for SiteObject { @@ -86,7 +82,7 @@ impl WipObject for SiteObject { fn description(&self) -> Option<&str> { Some(match self { Self::Document(node, _) => &node.title, - Self::Counter { description, .. } => description, + Self::Counter { .. } => "An in-memory counter.", }) } } @@ -130,7 +126,7 @@ fn protocol_error(code: ProtocolErrorCode, message: impl Into) -> Protoc } /// Immutable documents and isolated mutable playground state. This synchronous, -/// single-threaded provider publishes a fresh counter snapshot on every lookup. +/// single-threaded provider publishes stable counter metadata; values are read by calls. pub struct Worldspace { registry: InterfaceRegistry, objects: BTreeMap>, @@ -327,16 +323,14 @@ impl ObjectProvider for Worldspace { &self.counter, path.strip_prefix("/playground/"), ) { - let value = state.counters.borrow().get(name).copied(); - let Some(value) = value else { + if !state.counters.borrow().contains_key(name) { return ProviderResult::NotFound; - }; - // No shared mutable description: each callback receives its own current - // snapshot. Rc/RefCell makes the synchronous lookup/dispatch boundary - // single-threaded, with no external writer between resolution and call. + } + // Resolve existence only; counter values are exposed by typed operations. + // Rc/RefCell keeps lookup/dispatch synchronous and single-threaded, + // with no external writer between resolution and call. let publication = PublishedObject::new(SiteObject::Counter { name: name.into(), - description: value.to_string(), state: Rc::clone(state), }) .bind_scope(self.current_scope.clone()) diff --git a/site/worldspace/tests/playground.rs b/site/worldspace/tests/playground.rs index 0464b09..d4dd0f7 100644 --- a/site/worldspace/tests/playground.rs +++ b/site/worldspace/tests/playground.rs @@ -65,6 +65,29 @@ fn increment(world: &Worldspace, amount: i64) -> Result Value { + call( + world, + &format!("/playground/{name}"), + "counter", + "get", + BTreeMap::new(), + ) + .unwrap() + .result +} + +fn assert_initial_values(world: &Worldspace, observation: &ObjectObservation) { + for child in observation.children.as_ref().unwrap() { + let name = &child.object.name; + assert_eq!( + get(world, name), + Value::Integer(if name == "apple" { 3 } else { 0 }), + "counter {name} must remain unchanged" + ); + } +} + fn observe(world: &Worldspace, path: &str, depth: u32) -> ObjectObservation { world .observe(ObserveRequest { @@ -137,25 +160,18 @@ fn typed_descriptors_root_scope_and_initial_snapshots() { .iter() .map(|c| (c.object.name.as_str(), c.object.description.as_deref())) .collect::>(), - [("apple", Some("3")), ("orange", Some("0"))] + [ + ("apple", Some("An in-memory counter.")), + ("orange", Some("An in-memory counter.")), + ] ); assert!( children .iter() .all(|c| c.object.interfaces == [reference("counter")] && c.children == Some(vec![])) ); - assert_eq!( - call( - &world, - "/playground/apple", - "counter", - "get", - BTreeMap::new() - ) - .unwrap() - .result, - Value::Integer(3) - ); + assert_eq!(get(&world, "apple"), Value::Integer(3)); + assert_eq!(get(&world, "orange"), Value::Integer(0)); assert_eq!( call( &world, @@ -168,18 +184,21 @@ fn typed_descriptors_root_scope_and_initial_snapshots() { .code, ProtocolErrorCode::InterfaceMismatch ); + assert_eq!(get(&world, "apple"), Value::Integer(3)); assert_eq!( call(&world, "/book", "collection", "create", BTreeMap::new()) .unwrap_err() .code, ProtocolErrorCode::InterfaceMismatch ); + assert_eq!(get(&world, "apple"), Value::Integer(3)); } #[test] -fn mutations_publish_new_values_and_children_without_mutating_documents() { +fn mutations_preserve_metadata_and_publish_values_and_children_without_mutating_documents() { let world = world(); let before = observe(&world, "/playground/apple", 1); + let parent_before = observe(&world, "/playground", 2); let documents: Vec<_> = ["/", "/book", "/playground"] .into_iter() .map(|path| { @@ -192,19 +211,11 @@ fn mutations_publish_new_values_and_children_without_mutating_documents() { }) .collect(); assert_eq!(increment(&world, 7).unwrap().result, Value::Integer(10)); - assert_eq!( - observe(&world, "/playground/apple", 1) - .object - .description - .as_deref(), - Some("10") - ); - assert_eq!( - before.object.description.as_deref(), - Some("3"), - "old snapshots stay immutable" - ); + assert_eq!(get(&world, "apple"), Value::Integer(10)); + assert_eq!(observe(&world, "/playground/apple", 1), before); + assert_eq!(observe(&world, "/playground", 2), parent_before); assert_eq!(increment(&world, -12).unwrap().result, Value::Integer(-2)); + assert_eq!(get(&world, "apple"), Value::Integer(-2)); assert_eq!( call( &world, @@ -217,7 +228,9 @@ fn mutations_publish_new_values_and_children_without_mutating_documents() { .result, Value::Integer(0) ); - assert_eq!(observe(&world, "/playground/apple", 1).object.name, "apple"); + assert_eq!(get(&world, "apple"), Value::Integer(0)); + assert_eq!(observe(&world, "/playground/apple", 1), before); + assert_eq!(observe(&world, "/playground", 2), parent_before); assert_eq!( collection(&world, "create", "New_counter-2") .unwrap() @@ -229,8 +242,9 @@ fn mutations_publish_new_values_and_children_without_mutating_documents() { .object .description .as_deref(), - Some("0") + Some("An in-memory counter.") ); + assert_eq!(get(&world, "New_counter-2"), Value::Integer(0)); collection(&world, "remove", "apple").unwrap(); assert_eq!( call( @@ -252,13 +266,8 @@ fn mutations_publish_new_values_and_children_without_mutating_documents() { .any(|child| child.object.name == "apple") ); collection(&world, "create", "apple").unwrap(); - assert_eq!( - observe(&world, "/playground/apple", 1) - .object - .description - .as_deref(), - Some("0") - ); + assert_eq!(observe(&world, "/playground/apple", 1), before); + assert_eq!(get(&world, "apple"), Value::Integer(0)); for (path, expected) in documents { assert_eq!( call(&world, path, "document", "read", BTreeMap::new()) @@ -273,6 +282,7 @@ fn mutations_publish_new_values_and_children_without_mutating_documents() { fn invalid_names_duplicates_missing_and_capacity_fail_without_mutation() { let world = world(); let before = observe(&world, "/", 3); + let counters_before = observe(&world, "/playground", 1); for name in [ "", ".", @@ -291,11 +301,15 @@ fn invalid_names_duplicates_missing_and_capacity_fail_without_mutation() { "{operation} {name:?}" ); assert_eq!(observe(&world, "/", 3), before); + assert_initial_values(&world, &counters_before); } } assert!(collection(&world, "create", "apple").is_err()); + assert_eq!(observe(&world, "/", 3), before); + assert_initial_values(&world, &counters_before); assert!(collection(&world, "remove", "missing").is_err()); assert_eq!(observe(&world, "/", 3), before); + assert_initial_values(&world, &counters_before); collection(&world, "create", &"a".repeat(32)).unwrap(); for index in 3..MAX_COUNTERS { collection(&world, "create", &format!("counter-{index}")).unwrap(); @@ -307,6 +321,7 @@ fn invalid_names_duplicates_missing_and_capacity_fail_without_mutation() { ProtocolErrorCode::ResourceLimitExceeded ); assert_eq!(observe(&world, "/playground", 1), full); + assert_initial_values(&world, &full); collection(&world, "remove", "orange").unwrap(); collection(&world, "create", "over-limit").unwrap(); } @@ -318,22 +333,36 @@ fn integer_and_typed_argument_failures_are_atomic() { increment(&world, MAX_SAFE_INTEGER - 3).unwrap().result, Value::Integer(MAX_SAFE_INTEGER) ); + assert_eq!(get(&world, "apple"), Value::Integer(MAX_SAFE_INTEGER)); let maximum = observe(&world, "/playground/apple", 1); for amount in [1, MAX_SAFE_INTEGER, i64::MAX, i64::MIN] { assert!(increment(&world, amount).is_err()); assert_eq!(observe(&world, "/playground/apple", 1), maximum); + assert_eq!(get(&world, "apple"), Value::Integer(MAX_SAFE_INTEGER)); } - call( - &world, - "/playground/apple", - "counter", - "reset", - BTreeMap::new(), - ) - .unwrap(); - increment(&world, MIN_SAFE_INTEGER).unwrap(); + assert_eq!( + call( + &world, + "/playground/apple", + "counter", + "reset", + BTreeMap::new(), + ) + .unwrap() + .result, + Value::Integer(0) + ); + assert_eq!(get(&world, "apple"), Value::Integer(0)); + assert_eq!(observe(&world, "/playground/apple", 1), maximum); + assert_eq!( + increment(&world, MIN_SAFE_INTEGER).unwrap().result, + Value::Integer(MIN_SAFE_INTEGER) + ); + assert_eq!(get(&world, "apple"), Value::Integer(MIN_SAFE_INTEGER)); let minimum = observe(&world, "/playground/apple", 1); + assert_eq!(minimum, maximum); assert!(increment(&world, -1).is_err()); + assert_eq!(get(&world, "apple"), Value::Integer(MIN_SAFE_INTEGER)); for args in [ BTreeMap::new(), BTreeMap::from([("amount".into(), Value::String("1".into()))]), @@ -349,23 +378,25 @@ fn integer_and_typed_argument_failures_are_atomic() { ProtocolErrorCode::InvalidArguments ); assert_eq!(observe(&world, "/playground/apple", 1), minimum); + assert_eq!(get(&world, "apple"), Value::Integer(MIN_SAFE_INTEGER)); } } #[test] fn new_worldspace_resets_state_and_manifest_documents_cannot_be_shadowed() { let first = world(); - increment(&first, 12).unwrap(); + assert_eq!(increment(&first, 12).unwrap().result, Value::Integer(15)); + assert_eq!(get(&first, "apple"), Value::Integer(15)); collection(&first, "remove", "orange").unwrap(); collection(&first, "create", "pear").unwrap(); let fresh = world(); assert_eq!( - observe(&fresh, "/playground/apple", 1) - .object - .description - .as_deref(), - Some("3") + observe(&fresh, "/playground/apple", 1), + observe(&first, "/playground/apple", 1) ); + assert_eq!(get(&fresh, "apple"), Value::Integer(3)); + assert_eq!(get(&fresh, "orange"), Value::Integer(0)); + assert_eq!(get(&first, "apple"), Value::Integer(15)); assert_eq!( observe(&fresh, "/playground", 1) .children