From 71a5c81240823e8ba2fdeddfef8dd979b7749950 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Tue, 22 Sep 2026 12:42:43 +0200 Subject: [PATCH] Add the batch decoder and fix the actor vocabulary (PA.f54, PA.f55, PA.f58) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PA.f54, ruling: in a round, each maximal run of consecutive authoring calls is one patch, committed atomically under one command id, with draft handles shared inside the run. Adds `decode_authoring_calls`, which decodes a whole run into one shared `WorldPatch` plus a `site_calls` map from each declaration/operation site back to the call index that produced it, so a later call's reference to an earlier call's draft handle resolves within the run and a decode error names its own call index. `decode_authoring_call` stays as the one-call entry point with its own unprefixed error text; both now read the same `decode_one_authoring_call`, so there is one decoder rather than two copies. PA.f55: `actor_tools` offered `record_need` and `finish_without_proposal` through `catalog_tools`, but those end a turn — the controller lane's own vocabulary — and `decode_actor_call` has no arm for either, so the play agent could be handed a tool call it would always refuse. Splits `catalog_tools` into `affordance_tools` (one tool per granted entry, no turn-enders) plus the two pushes `catalog_tools` still makes for its own callers; `actor_tools` now reads `affordance_tools`. `decode_actor_call` also gains a `prefix` parameter and strips it from the incoming name before matching, so a name `actor_tools` emits (`{prefix}{kind}`) decodes as emitted through the one decoder rather than a caller stripping the prefix itself first. PA.f58: `decode_actor_call` silently dropped a non-string `display` (`and_then(Value::as_str)` swallows the wrong type) and the shared `decode_catalog_call` silently dropped a `text` field on an entry that carries no speech. Both are now refused. Also widens the `table_view_never_reaches_persona_or_projector_code` pin from grepping only `controllers.rs` to scanning every library source file except `table.rs` for a `table_view(` call. Dungeon's `play.rs` is adjusted only for the new `decode_actor_call` signature: its one dispatch call site and one test call site pass `""` for the new `prefix` parameter, since Dungeon already strips the `__` prefix itself before calling. Behaviour is unchanged; Cut 8b is the one that wires Dungeon's own dispatch through the table's prefix handling properly. Co-Authored-By: Claude Sonnet 5 --- crates/ghostlight-dungeon/src/play.rs | 8 +- crates/ghostlight/src/controllers.rs | 49 +++-- crates/ghostlight/src/lib.rs | 5 +- crates/ghostlight/src/table.rs | 283 ++++++++++++++++++++++++-- 4 files changed, 312 insertions(+), 33 deletions(-) diff --git a/crates/ghostlight-dungeon/src/play.rs b/crates/ghostlight-dungeon/src/play.rs index 6d45740..a78e773 100644 --- a/crates/ghostlight-dungeon/src/play.rs +++ b/crates/ghostlight-dungeon/src/play.rs @@ -833,7 +833,11 @@ impl PlayTable { Some(RecordedCall::PersonaAct(_, opportunity, invocation)) => (opportunity, invocation), _ => { let (opportunity, invocation) = - match decode_actor_call(snapshot, actor.subject, kind, arguments) { + // `kind` is already stripped of its `__` prefix + // by the `split_once` above, so `""` here decodes it + // unchanged; Cut 8b is the one that wires this dispatch + // through the table's own prefix-stripping properly. + match decode_actor_call(snapshot, actor.subject, "", kind, arguments) { Ok(decoded) => decoded, Err(detail) => { return Ok((RoundOutcome::Continue, format!("refused: {detail}"))); @@ -2244,7 +2248,7 @@ mod tests { let snapshot = fixture.world.snapshot().await.unwrap(); let player = player_id(&snapshot); let (opportunity, invocation) = - decode_actor_call(&snapshot, player, "speak", &serde_json::json!({"text": "hold fast"}).to_string()) + decode_actor_call(&snapshot, player, "", "speak", &serde_json::json!({"text": "hold fast"}).to_string()) .unwrap(); let command_id = derived_command_id(&turn_id, 0, 0); // The crash window: the kernel already committed the exercise, but diff --git a/crates/ghostlight/src/controllers.rs b/crates/ghostlight/src/controllers.rs index 75659d3..552d0fe 100644 --- a/crates/ghostlight/src/controllers.rs +++ b/crates/ghostlight/src/controllers.rs @@ -6499,7 +6499,35 @@ pub(super) fn catalog_tools( prefix: &str, granted: &[AffordanceSnapshot], ) -> Vec { - let mut tools: Vec = granted + let mut tools = affordance_tools(prefix, granted); + tools.push(tool_schema::tool( + &format!("{prefix}{RECORD_NEED_TOOL}"), + "Record information or capability the agent would need but does not currently have.", + tool_schema::object(vec![( + "detail".into(), + tool_schema::canonical_string("what the agent would need and does not have"), + )]), + )); + tools.push(tool_schema::tool( + &format!("{prefix}{FINISH_WITHOUT_PROPOSAL_TOOL}"), + "Finish this opportunity without proposing an action.", + tool_schema::empty_schema(), + )); + tools +} + +/// `catalog_tools` minus the two turn-enders: exactly one tool per granted +/// affordance entry, so `decode_catalog_call` — which only ever matches a +/// name against `granted`'s own entries — refuses nothing this vocabulary +/// offers. `record_need` and `finish_without_proposal` end a turn; they are +/// the controller lane's own vocabulary (`decode_catalog_call` has no arm +/// for either), not part of what one entry-granted actor may invoke, so the +/// play table's `actor_tools` reads this and not `catalog_tools`. +pub(super) fn affordance_tools( + prefix: &str, + granted: &[AffordanceSnapshot], +) -> Vec { + granted .iter() .map(|entry| { let mut properties: Vec<(String, Value)> = Vec::new(); @@ -6539,21 +6567,7 @@ pub(super) fn catalog_tools( tool_schema::object(properties), ) }) - .collect(); - tools.push(tool_schema::tool( - &format!("{prefix}{RECORD_NEED_TOOL}"), - "Record information or capability the agent would need but does not currently have.", - tool_schema::object(vec![( - "detail".into(), - tool_schema::canonical_string("what the agent would need and does not have"), - )]), - )); - tools.push(tool_schema::tool( - &format!("{prefix}{FINISH_WITHOUT_PROPOSAL_TOOL}"), - "Finish this opportunity without proposing an action.", - tool_schema::empty_schema(), - )); - tools + .collect() } /// The same iteration rendered as one prose line, so the prompt's tool list and @@ -6693,6 +6707,9 @@ pub(super) fn decode_catalog_call( .ok_or_else(|| "`text` is missing or not a string".to_owned())?; Some(Statement::new(text).ok_or_else(|| "`text` is not canonical".to_owned())?) } else { + if fields.contains_key("text") { + return Err("this entry carries no speech but `text` was proposed".to_owned()); + } None }; Ok(DecisionInvocation { diff --git a/crates/ghostlight/src/lib.rs b/crates/ghostlight/src/lib.rs index 4efce63..2afb697 100644 --- a/crates/ghostlight/src/lib.rs +++ b/crates/ghostlight/src/lib.rs @@ -120,7 +120,10 @@ pub(crate) use patch::{ EffectSlot, OutcomeBand, Precondition, RoleSpec, }; pub use sdk_inference::{DEFAULT_SDK_MODEL_PREFIX, SdkBinding}; -pub use table::{TableError, actor_tools, authoring_tools, decode_actor_call, decode_authoring_call, describe_refusal, table_view}; +pub use table::{ + DecodedBatch, PatchSite, TableError, actor_tools, authoring_tools, decode_actor_call, + decode_authoring_call, decode_authoring_calls, describe_refusal, table_view, +}; pub use vault::{VaultError, VaultEvidenceSource}; use patch::{EdgeRecord, EntityRecord, LedgerDelta, ResolvedOp, ResolvedPatch}; diff --git a/crates/ghostlight/src/table.rs b/crates/ghostlight/src/table.rs index 511b4c2..3fe0bf3 100644 --- a/crates/ghostlight/src/table.rs +++ b/crates/ghostlight/src/table.rs @@ -59,11 +59,70 @@ pub fn authoring_tools(names: &[&str]) -> Result, Table } /// One authoring tool call, decoded to the one-item patch it names: exactly -/// one declaration or one operation, plus any evidence it cites. This is +/// one declaration or one operation, plus any evidence it cites. The one-call +/// case of the same code `decode_authoring_calls` shares +/// (`decode_one_authoring_call`), kept as its own entry point with its own +/// unprefixed error text: `apply_tool_call`'s `Declare`/`Operate` arms and +/// the tests that check one tool's own example have no call index to name. +pub fn decode_authoring_call(name: &str, arguments: &str) -> Result { + decode_one_authoring_call(name, arguments) +} + +/// Where one item in a decoded batch's `patch` came from: a declaration at +/// this index in `patch.declarations`, or an operation at this index in +/// `patch.operations`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum PatchSite { + Declaration(usize), + Operation(usize), +} + +/// The result of decoding one maximal run of consecutive authoring calls: +/// the whole run's `WorldPatch`, plus which call produced each site in it. +/// Dungeon commits the run atomically under one command id; draft handles +/// declared by an earlier call in the run resolve against later calls in the +/// same run because every call's items land in the one shared `patch`. +#[derive(Debug)] +pub struct DecodedBatch { + pub patch: WorldPatch, + /// `site_calls[&site]` is the index into `calls` (the slice + /// `decode_authoring_calls` was given) that produced that site. A + /// refusal names its call the same way, through this same index. + pub site_calls: std::collections::BTreeMap, +} + +/// One run of authoring calls, decoded to one shared patch (PA.f54): each +/// call still decodes to exactly one declaration or one operation — the same +/// one-item rule `decode_authoring_call` documents — but every call's item +/// lands in the same `WorldPatch`, so a later call's reference to an earlier +/// call's draft handle resolves within the run. A decode error names the +/// call's index in `calls`, not merely its tool name, because the same name +/// can appear more than once in a run. +pub fn decode_authoring_calls(calls: &[(&str, &str)]) -> Result { + let mut patch = WorldPatch::default(); + let mut site_calls = std::collections::BTreeMap::new(); + for (call_index, (name, arguments)) in calls.iter().enumerate() { + let item = decode_one_authoring_call(name, arguments) + .map_err(|error| format!("call #{call_index} (`{name}`): {error}"))?; + for declaration in item.declarations { + site_calls.insert(PatchSite::Declaration(patch.declarations.len()), call_index); + patch.declarations.push(declaration); + } + for operation in item.operations { + site_calls.insert(PatchSite::Operation(patch.operations.len()), call_index); + patch.operations.push(operation); + } + patch.evidence.extend(item.evidence); + } + Ok(DecodedBatch { patch, site_calls }) +} + +/// The one decode arm both `decode_authoring_call` and `decode_authoring_calls` +/// read: one authoring tool call to the one-item patch it names. Also /// `apply_tool_call`'s own decode arm, factored out so it has one owner; /// `apply_tool_call` calls it for every `Declare` or `Operate` shape rather /// than carrying a second copy of the same match. -pub fn decode_authoring_call(name: &str, arguments: &str) -> Result { +fn decode_one_authoring_call(name: &str, arguments: &str) -> Result { let Some(entry) = PATCH_TOOLS.iter().find(|entry| entry.name == name) else { return Err(format!("`{name}` is not a tool of this patch")); }; @@ -140,13 +199,17 @@ fn subject_opportunity<'a>( Some((opportunity, granted)) } -/// The tool catalog for one subject's own granted affordances: `catalog_tools` -/// over the entries its current opportunity actually grants. An unknown +/// The tool catalog for one subject's own granted affordances: an unknown /// subject or one with no live opportunity gets an empty catalog rather than -/// an error — there is nothing for it to call. +/// an error — there is nothing for it to call. The table's own actor +/// vocabulary is the subject's granted entries only — `affordance_tools`, +/// never `catalog_tools`. `record_need` and `finish_without_proposal` end a +/// turn; that is the controller lane's own vocabulary, and +/// `decode_actor_call` has no arm for either, so offering them here would +/// hand the play agent a tool `decode_actor_call` always refuses (PA.f55). pub fn actor_tools(prefix: &str, snapshot: &WorldSnapshot, subject: SubjectId) -> Vec { match subject_opportunity(snapshot, subject) { - Some((_, granted)) => controllers::catalog_tools(prefix, &granted), + Some((_, granted)) => controllers::affordance_tools(prefix, &granted), None => Vec::new(), } } @@ -158,12 +221,20 @@ pub fn actor_tools(prefix: &str, snapshot: &WorldSnapshot, subject: SubjectId) - /// one, decoded through the same canonical-text rule speech already uses). /// The opportunity returned is the snapshot's own, never a value the caller /// supplied, so Dungeon can commit it without ever having constructed one. +/// +/// `prefix` is the same prefix `actor_tools` was called with: a name +/// `actor_tools` emits (`{prefix}{kind}`) decodes here as emitted, one +/// decoder rather than a caller stripping the prefix itself before this is +/// reached (PA.f55). An empty prefix strips nothing, so a bare `kind` still +/// decodes unchanged. pub fn decode_actor_call( snapshot: &WorldSnapshot, subject: SubjectId, + prefix: &str, kind: &str, arguments: &str, ) -> Result<(DecisionOpportunity, DecisionInvocation), String> { + let kind = kind.strip_prefix(prefix).unwrap_or(kind); let (opportunity, granted) = subject_opportunity(snapshot, subject) .ok_or_else(|| "this subject holds no exact opportunity".to_owned())?; let entry = granted @@ -172,8 +243,11 @@ pub fn decode_actor_call( .ok_or_else(|| format!("`{kind}` is not granted by this subject's opportunity"))?; let mut invocation = controllers::decode_catalog_call(entry, arguments)?; if let Ok(Value::Object(fields)) = serde_json::from_str::(arguments) - && let Some(display) = fields.get("display").and_then(Value::as_str) + && let Some(display) = fields.get("display") { + let display = display + .as_str() + .ok_or_else(|| "`display` is not a string".to_owned())?; invocation.display = Some(Statement::new(display).ok_or_else(|| "`display` is not canonical text".to_owned())?); } @@ -1280,6 +1354,71 @@ mod tests { } } + /// Rule (PA.f54): a later call in one run resolves a draft handle an + /// earlier call in the same run declared, because every call's item lands + /// in the run's one shared `WorldPatch`. `declare_place` has no field a + /// `relocate` call takes (it takes a subject and a route, not a place), + /// so `declare_route`'s `from` is the real field this catalog offers that + /// takes a place reference; it stands for the same claim PA.f54 names. + #[test] + fn decode_authoring_calls_shares_draft_handles_across_one_run() { + let place = serde_json::json!({ + "handle": "yard", + "label": "The Cavity Yard", + "container": null, + }) + .to_string(); + let route = serde_json::json!({ + "handle": "ramp", + "label": "The Yard Ramp", + "from": {"ref": "draft", "value": "yard"}, + "to": {"ref": "existing", "value": "11111111-1111-4111-8111-111111111111"}, + "access": {"access": "public"}, + "cost": 1, + }) + .to_string(); + let batch = decode_authoring_calls(&[("declare_place", &place), ("declare_route", &route)]) + .expect("both calls in the run decode together"); + assert_eq!(batch.patch.declarations.len(), 2); + assert_eq!(batch.patch.operations.len(), 0); + let Declaration::Route(declared) = &batch.patch.declarations[1] else { + panic!("declare_route decoded to something other than a route declaration"); + }; + assert_eq!(declared.from, patch::Ref::Draft(patch::DraftHandle::new("yard"))); + + // The site map: each declaration's index names the call index that + // produced it. + assert_eq!( + batch.site_calls.get(&PatchSite::Declaration(0)), + Some(&0), + "declare_place's declaration must be attributed to call #0" + ); + assert_eq!( + batch.site_calls.get(&PatchSite::Declaration(1)), + Some(&1), + "declare_route's declaration must be attributed to call #1" + ); + } + + /// Rule (PA.f54): a decode error names the failing call's index, not only + /// its tool name — the same name can appear more than once in one run. + #[test] + fn decode_authoring_calls_names_the_failing_call_index() { + let good = serde_json::json!({ + "handle": "yard", + "label": "The Cavity Yard", + "container": null, + }) + .to_string(); + let error = decode_authoring_calls(&[ + ("declare_place", &good), + ("declare_place", "not json at all"), + ]) + .unwrap_err(); + assert!(error.contains("call #1"), "{error}"); + assert!(error.contains("declare_place"), "{error}"); + } + /// Rule: the elaborator's own decode and the table's decode agree, because /// the elaborator's arm now calls the table's function rather than /// carrying a second copy. @@ -1332,7 +1471,7 @@ mod tests { }) .to_string(); let (opportunity, invocation) = - decode_actor_call(&active, speech.speaker, "whisper", &arguments) + decode_actor_call(&active, speech.speaker, "", "whisper", &arguments) .expect("whisper decodes against the speaker's own opportunity"); assert_eq!(opportunity, expected); assert_eq!( @@ -1341,6 +1480,98 @@ mod tests { ); } + /// Rule (PA.f55): `actor_tools` never offers `record_need` or + /// `finish_without_proposal` — those end a turn, which is the controller + /// lane's own vocabulary, and `decode_actor_call` has no arm for either. + /// Every name it does emit decodes as emitted once the same prefix is + /// handed to `decode_actor_call`. + #[test] + fn actor_tools_offers_only_names_decode_actor_call_accepts() { + let directory = tempfile::tempdir().expect("a temp dir"); + let mut kernel = crate::WorldKernel::create( + directory.path().join("world.cc"), + creation(super::CommandId::new(), "PlayActorVocabulary"), + &auth_principal(owner()), + ) + .expect("a created world") + .0; + let (speech, active) = speech_world(&mut kernel); + let prefix = "c0__"; + let tools = actor_tools(prefix, &active, speech.speaker); + assert!(!tools.is_empty(), "the speaker holds granted affordances"); + for tool in &tools { + assert!( + !tool.name.ends_with("record_need") && !tool.name.ends_with("finish_without_proposal"), + "actor_tools must not offer the controller lane's turn-enders: {}", + tool.name + ); + } + let arguments = serde_json::json!({ + "target": id_text(speech.listener), + "text": "a whispered warning", + }) + .to_string(); + let (_, invocation) = + decode_actor_call(&active, speech.speaker, prefix, &format!("{prefix}whisper"), &arguments) + .expect("a name actor_tools emits decodes as emitted, prefix included"); + assert_eq!( + invocation.speech.as_ref().map(Statement::as_str), + Some("a whispered warning") + ); + } + + /// Rule (PA.f58): a non-string `display` is refused, not silently + /// dropped. + #[test] + fn decode_actor_call_refuses_a_non_string_display() { + let directory = tempfile::tempdir().expect("a temp dir"); + let mut kernel = crate::WorldKernel::create( + directory.path().join("world.cc"), + creation(super::CommandId::new(), "PlayActorDisplay"), + &auth_principal(owner()), + ) + .expect("a created world") + .0; + let (speech, active) = speech_world(&mut kernel); + let arguments = serde_json::json!({ + "target": id_text(speech.listener), + "text": "a whispered warning", + "display": 7, + }) + .to_string(); + let error = decode_actor_call(&active, speech.speaker, "", "whisper", &arguments).unwrap_err(); + assert!(error.contains("display"), "{error}"); + } + + /// Rule (PA.f58): `text` on an entry that carries no speech is refused, + /// not silently dropped. + #[test] + fn decode_actor_call_refuses_text_on_a_silent_entry() { + let fixture = play_fixture(); + let holder = fixture + .snapshot + .subjects + .iter() + .find(|subject| subject.id == fixture.custody.holder) + .expect("the holder is in the snapshot"); + let carry = fixture + .snapshot + .affordances + .iter() + .find(|entry| holder.affordances.contains(&entry.id) && entry.entry.kind.0 == "carry") + .expect("the holder holds the silent `carry` affordance"); + let mut object = serde_json::Map::new(); + for role in &carry.entry.roles { + object.insert(role.role.0.clone(), Value::String(id_text(fixture.custody.holder))); + } + object.insert("slot_0_qty".into(), Value::from(1)); + object.insert("text".into(), Value::String("this affordance carries no speech".into())); + let arguments = Value::Object(object).to_string(); + let error = decode_actor_call(&fixture.snapshot, fixture.custody.holder, "", "carry", &arguments) + .unwrap_err(); + assert!(error.contains("text"), "{error}"); + } + /// Mutation M7.1: if `decode_actor_call` used the snapshot's first /// opportunity instead of the subject's own, this would no longer hold — /// `speech.listener` holds a different, later-ordered opportunity. @@ -1366,7 +1597,7 @@ mod tests { "text": "a returned whisper", }) .to_string(); - let (opportunity, _) = decode_actor_call(&active, speech.listener, "whisper", &arguments) + let (opportunity, _) = decode_actor_call(&active, speech.listener, "", "whisper", &arguments) .expect("whisper decodes against the listener's own opportunity"); assert_eq!(opportunity, expected); assert_ne!( @@ -1589,10 +1820,34 @@ mod tests { /// Persona prompt. #[test] fn table_view_never_reaches_persona_or_projector_code() { - let source = include_str!("controllers.rs"); - assert!( - !source.contains("table_view"), - "controllers.rs must not call table::table_view" - ); + // Every library source file except this one: the structural guard is + // that no other module — not `controllers.rs` alone, which is only + // where `PersonaLane` and the Projector happen to live today — ever + // calls `table_view(`. `PersonaLane`'s own signature takes no text + // that could carry it in, so a call anywhere outside `table.rs` + // would have to be a fresh, wrong wire-up. + const SOURCES: &[(&str, &str)] = &[ + ("action.rs", include_str!("action.rs")), + ("clock.rs", include_str!("clock.rs")), + ("consumer.rs", include_str!("consumer.rs")), + ("controllers.rs", include_str!("controllers.rs")), + ("cover.rs", include_str!("cover.rs")), + ("elaboration.rs", include_str!("elaboration.rs")), + ("journal.rs", include_str!("journal.rs")), + ("lens.rs", include_str!("lens.rs")), + ("lib.rs", include_str!("lib.rs")), + ("local_inference.rs", include_str!("local_inference.rs")), + ("mailbox.rs", include_str!("mailbox.rs")), + ("patch.rs", include_str!("patch.rs")), + ("sdk_inference.rs", include_str!("sdk_inference.rs")), + ("tool_schema.rs", include_str!("tool_schema.rs")), + ("vault.rs", include_str!("vault.rs")), + ]; + for (name, source) in SOURCES { + assert!( + !source.contains("table_view("), + "{name} must not call table::table_view" + ); + } } }