diff --git a/docs/LIMITATIONS.md b/docs/LIMITATIONS.md index 6d7f6ef..bfc9f27 100644 --- a/docs/LIMITATIONS.md +++ b/docs/LIMITATIONS.md @@ -87,49 +87,53 @@ conductor's current behavior. **Not yet built:** a FlatBuffers encode/decode path actually wired into `conductor.rs`'s message handling. -## 7. A Single Bridge Update's Equilibrium Check Passes or Fails Arbitrarily - -`message.validate()` (`pchi-schema/src/validation.rs`) rejects a message -outright if its own self-reported `pir_invariants.residual` exceeds -`1e-12` — before the conductor ever applies the state change. The -Resolume bridge (`tools/resolume-pchi-bridge`) computes that residual as -a Prouhet-Thue-Morse-signed sum over its last 1–4 recently-set parameter -values (`create_pchi_message`'s `values = list(self.state.values())[-4:]`). - -Tested live against a real conductor: a single layer's opacity change -(one value, signs `[1]`) always fails — a lone nonzero value can never -sum to zero. Four different layers set to the *same* value in sequence -sometimes passed and sometimes didn't, depending on which arbitrary -1–4-value window each individual message happened to compute against at -send time — not on whether the change was safe or harmonious in any -real sense. A rejected message's state change is **never applied** -(`process_message`'s `message.validate()?` returns before the match -block runs), so this isn't just a logged warning — it's a silently -dropped update from the caller's point of view (the bridge's OSC -handler has no failure feedback path back to Resolume). - -**What this means:** the "Zero-Drift Sync" claim does not hold for -generic Resolume/TouchDesigner/Ableton parameter changes as this bridge -computes equilibrium today — most single-parameter updates will be -dropped, essentially at random, by a check that was designed around -deliberately-paired values (the Kraken tentacle example, where -wobbliness values are constructed in matched pairs specifically so the -signed sum cancels), not around arbitrary incoming state. - -**Not fixed here:** loosening or redesigning the equilibrium check is a -real design decision (what should "harmonious" mean for an arbitrary -single-parameter change?), not a bug fix — it needs an answer from -whoever owns the PIR-invariant design, not a guess. **Fixed in the -Resolume bridge itself:** two independent bugs that meant *no* message — -passing or failing this check — ever reached the conductor before this -change: `message_type`/`payload_type` were serialized under their -Python field names instead of the wire names (`type`/`payloadType`) the -schema's `#[serde(rename = ...)]` actually requires, and messages were -sent OSC-wrapped (`osc_client.send_message`) rather than as raw +## 7. [Fixed] A Single Bridge Update's Equilibrium Check Used to Pass or Fail Arbitrarily + +**Status: fixed.** `message.validate()` (`pchi-schema/src/validation.rs`) +used to reject *every* message outright if its own self-reported +`pir_invariants.residual` exceeded `1e-12` — including `ControlParameter`, +`Heartbeat`, `MusicalContext` and `ArtistTracking` messages, which report +one independent fact and never carried a genuine multi-value invariant to +begin with. The Resolume bridge computed that residual as a +Prouhet-Thue-Morse-signed sum over its last 1–4 recently-set parameter +values — a number with no real meaning for unrelated values — so a +single layer's opacity change (one value, signs `[1]`) always failed, +and matched multi-value updates passed or failed depending purely on +which arbitrary window a given message happened to land on at send +time, not on whether the change was safe. + +**The fix:** `validate_invariants` now only checks residual/coherence-gap +against `SceneUpdate` messages — the one payload shape that can +genuinely carry multiple related object states a sender constructed to +balance, which is what an "equilibrium" claim actually means. Every +other message type is no longer gated on it. This isn't a loosened +check; it's a correctly-scoped one: the conductor already has the real +mechanism for tracking equilibrium across a sequence of independent +updates — it recomputes actual aggregate equilibrium from the real +accumulated scene state after every message +(`SceneState::update_equilibrium_status`), and the shipped default rule +file already acts on that real value (`if equilibrium > 1e-12 then +escalate(...)` in `kraken-tentacle.only-pchi`). The message-level gate +was duplicating that check, badly, over data that was never a valid +invariant claim in the first place. + +Verified live: the exact scenario that used to always fail (a single +layer's opacity change) and the exact scenario that used to pass or fail +essentially at random (11 real control-parameter updates across mixed +parameters) — 25/25 applied, 0 dropped, after the fix, run against the +same real conductor and bridge. `SceneUpdate`'s equilibrium check is +still enforced (regression-tested in +`pchi-schema/src/validation.rs::scene_update_still_rejects_a_real_equilibrium_violation`). + +**Also fixed, previously:** two independent bugs that meant *no* +message — passing or failing this check — ever reached the conductor +before that fix: `message_type`/`payload_type` were serialized under +their Python field names instead of the wire names (`type`/`payloadType`) +the schema's `#[serde(rename = ...)]` actually requires, and messages +were sent OSC-wrapped (`osc_client.send_message`) rather than as raw JSON-over-UDP, which is the only format the conductor's receive loop parses. Heartbeats additionally omitted `pir_invariants` and `payload` -entirely, both required fields with no `#[serde(default)]` — every -heartbeat this bridge sent was rejected as unparseable before this fix. +entirely, both required fields with no `#[serde(default)]`. ## 8. Governance-Event Messages From Other Conductors Are Logged, Not Re-Verified diff --git a/pchi-schema/src/validation.rs b/pchi-schema/src/validation.rs index ca3cdb2..dd4598a 100644 --- a/pchi-schema/src/validation.rs +++ b/pchi-schema/src/validation.rs @@ -99,10 +99,44 @@ fn validate_timestamp(timestamp: f64) -> Result<(), ValidationError> { Ok(()) } +/// Equilibrium is a property of a *scene* — a set of related values that a +/// sender constructed to satisfy an invariant together (the Kraken example: +/// 8 tentacles' wobbliness values, deliberately paired so their +/// Prouhet-Thue-Morse-signed sum cancels). It is not a property of one +/// independent fact. +/// +/// Before this fix, every message type was checked against the same +/// residual/coherence-gap threshold, including ControlParameter, +/// MusicalContext, ArtistTracking and Heartbeat — messages that report a +/// single value or a single unrelated fact and have no multi-value +/// invariant to satisfy in the first place. A real bridge (Resolume) +/// computed its residual as a Prouhet-Thue-Morse-signed sum over its last +/// 1–4 *arbitrary, unrelated* recently-set parameters — a number with no +/// real meaning — and this check rejected the update outright whenever +/// that incidental window didn't happen to cancel. Tested live: a single +/// layer's opacity change (one value) always failed; matched updates +/// sometimes passed depending purely on send-order coincidence. See +/// docs/LIMITATIONS.md item 7 for the full writeup before this fix. +/// +/// Only `SceneUpdate` carries a payload that can genuinely represent +/// multiple related object states a sender constructed to balance — so +/// only `SceneUpdate` is checked here. The conductor already has the +/// correct mechanism for tracking real equilibrium across single +/// incremental updates: it recomputes actual aggregate equilibrium from +/// the real accumulated scene state after every message +/// (`SceneState::update_equilibrium_status`), independent of what any one +/// message claims about itself — and the shipped default rule file already +/// acts on that real value (`if equilibrium > 1e-12 then escalate(...)` +/// in `kraken-tentacle.only-pchi`). This function stops duplicating that +/// check, badly, at the transport layer. fn validate_invariants( message: &PCHIMessage, config: &ValidationConfig, ) -> Result<(), ValidationError> { + if !matches!(message.message_type, crate::MessageType::SceneUpdate) { + return Ok(()); + } + // Check equilibrium if message.pir_invariants.residual.abs() > config.max_residual { return Err(ValidationError::EquilibriumViolation { @@ -110,7 +144,7 @@ fn validate_invariants( threshold: config.max_residual, }); } - + // Check coherence gap if message.pir_invariants.coherence_gap > config.max_coherence_gap { return Err(ValidationError::CoherenceGapViolation { @@ -118,7 +152,7 @@ fn validate_invariants( threshold: config.max_coherence_gap, }); } - + Ok(()) } @@ -231,8 +265,98 @@ mod tests { ); message.version = "1.0.0".to_string(); - + let result = validate_message(&message); assert!(matches!(result, Err(ValidationError::InvalidVersion(_)))); } + + #[test] + fn scene_update_still_rejects_a_real_equilibrium_violation() { + // Regression guard: SceneUpdate is exactly the case this check + // exists for, and must still be enforced. + let mut message = PCHIMessage::new( + MessageType::SceneUpdate, + "test_source".to_string(), + Payload::SceneUpdate(SceneUpdateData { + objects: vec![crate::SceneObject { + id: "obj1".to_string(), + transform: crate::Transform::default(), + custom_data: None, + pir_constraints: None, + }], + }), + ); + message.pir_invariants.residual = 1.0; + let result = validate_message(&message); + assert!(matches!(result, Err(ValidationError::EquilibriumViolation { .. }))); + } + + #[test] + fn control_parameter_is_not_gated_on_residual() { + // This is the actual bug: a single control-parameter change was + // rejected outright whenever its self-reported residual (computed + // by a real bridge as a PTM-signed sum over an arbitrary window of + // unrelated recent values) didn't happen to cancel — see + // docs/LIMITATIONS.md item 7. A single value can never validly + // "cancel" by construction, so this made most real updates fail + // essentially at random. ControlParameter has no multi-value + // invariant to assert in the first place. + let mut message = PCHIMessage::new( + MessageType::ControlParameter, + "resolume_bridge".to_string(), + Payload::ControlParameter(crate::ControlParameterData { + target_id: "resolume_layer_1".to_string(), + parameter: "opacity".to_string(), + value: serde_json::json!(0.85), + }), + ); + message.pir_invariants.residual = 0.85; // exactly what a single-value PTM sum is + let result = validate_message(&message); + assert!(result.is_ok(), "expected Ok, got {:?}", result); + } + + #[test] + fn heartbeat_is_not_gated_on_residual() { + let mut message = PCHIMessage::new( + MessageType::Heartbeat, + "resolume_bridge".to_string(), + Payload::ControlParameter(crate::ControlParameterData { + target_id: "resolume_bridge".to_string(), + parameter: "heartbeat".to_string(), + value: serde_json::json!(1.0), + }), + ); + message.pir_invariants.residual = 0.5; + let result = validate_message(&message); + assert!(result.is_ok(), "expected Ok, got {:?}", result); + } + + #[test] + fn musical_context_and_artist_tracking_are_not_gated_on_residual() { + let mut mc = PCHIMessage::new( + MessageType::MusicalContext, + "ableton_bridge".to_string(), + Payload::MusicalContext(crate::MusicalContextData { + bpm: 128.0, + beat: 1, + kick: true, + snare: false, + section: "drop".to_string(), + }), + ); + mc.pir_invariants.residual = 3.0; + assert!(validate_message(&mc).is_ok()); + + let mut at = PCHIMessage::new( + MessageType::ArtistTracking, + "tracker".to_string(), + Payload::ArtistTracking(crate::ArtistTrackingData { + artist_id: "artist1".to_string(), + transform: crate::Transform::default(), + velocity: None, + }), + ); + at.pir_invariants.residual = 3.0; + assert!(validate_message(&at).is_ok()); + } }