fix: Don't use opgroup in pytket barrier encoding

tket/src/serialize/pytket/circuit.rs

@@ -376,11 +376,7 @@ impl<Node: HugrNode> EncodedCircuit<Node> {
     /// [`OpaqueSubgraphPayload::External`][super::opaque::OpaqueSubgraphPayload::External]
     /// payloads in opaque barriers with inline payloads.
     ///
-    /// # Errors
-    ///
-    /// Returns an error if a barrier operation with the
-    /// [`OPGROUP_OPAQUE_HUGR`][super::opaque::OPGROUP_OPAQUE_HUGR]
-    /// opgroup has an invalid payload.
+    /// Barrier operation with unrecognised payloads will be ignored.
     pub fn ensure_standalone(
         &mut self,
         hugr: &impl HugrView<Node = Node>,

tket/src/serialize/pytket/encoder.rs

@@ -33,9 +33,7 @@ use crate::circuit::Circuit;
 use crate::serialize::pytket::circuit::{AdditionalNodesAndWires, EncodedCircuitInfo};
 use crate::serialize::pytket::config::PytketEncoderConfig;
 use crate::serialize::pytket::extension::RegisterCount;
-use crate::serialize::pytket::opaque::{
-    OpaqueSubgraph, OpaqueSubgraphPayload, OPGROUP_OPAQUE_HUGR,
-};
+use crate::serialize::pytket::opaque::{OpaqueSubgraph, OpaqueSubgraphPayload};
 
 /// The state of an in-progress [`SerialCircuit`] being built from a [`Circuit`].
 #[derive(derive_more::Debug)]
@@ -672,8 +670,7 @@ impl<H: HugrView> PytketEncoderContext<H> {
         let mut pytket_op = make_tk1_operation(tket_json_rs::OpType::Barrier, args);
         pytket_op.data = Some(serde_json::to_string(&payload).unwrap());
 
-        let opgroup = Some(OPGROUP_OPAQUE_HUGR.to_string());
-        self.emit_command(pytket_op, &op_values.qubits, &op_values.bits, opgroup);
+        self.emit_command(pytket_op, &op_values.qubits, &op_values.bits, None);
         Ok(())
     }
 
@@ -703,7 +700,9 @@ impl<H: HugrView> PytketEncoderContext<H> {
     ///   Normally obtained from a HUGR node's inputs using
     ///   [`PytketEncoderContext::get_input_values`] or allocated via
     ///   [`ValueTracker::new_bit`].
-    /// - `opgroup`: A tket1 operation group identifier, if any.
+    /// - `opgroup`: A tket1 [operation group
+    ///   identifier](https://docs.quantinuum.com/tket/user-guide/manual/manual_circuit.html#modifying-operations-within-circuits),
+    ///   if any.
     pub fn emit_command(
         &mut self,
         pytket_op: circuit_json::Operation,

tket/src/serialize/pytket/extension/core.rs

@@ -10,7 +10,7 @@ use crate::serialize::pytket::decoder::{
     DecodeStatus, LoadedParameter, PytketDecoderContext, TrackedBit, TrackedQubit,
 };
 use crate::serialize::pytket::extension::PytketDecoder;
-use crate::serialize::pytket::opaque::{OpaqueSubgraphPayload, OPGROUP_OPAQUE_HUGR};
+use crate::serialize::pytket::opaque::OpaqueSubgraphPayload;
 use crate::serialize::pytket::{DecodeInsertionTarget, DecodeOptions, PytketDecodeError};
 use hugr::builder::Container;
 use hugr::extension::prelude::{bool_t, qb_t};
@@ -36,19 +36,18 @@ impl PytketDecoder for CoreDecoder {
         qubits: &[TrackedQubit],
         bits: &[TrackedBit],
         params: &[LoadedParameter],
-        opgroup: Option<&str>,
+        _opgroup: Option<&str>,
         decoder: &mut PytketDecoderContext<'h>,
     ) -> Result<DecodeStatus, PytketDecodeError> {
         match &op {
             PytketOperation {
                 op_type: PytketOptype::Barrier,
                 data: Some(payload),
                 ..
-            } if opgroup == Some(OPGROUP_OPAQUE_HUGR) => {
-                let payload =
-                    OpaqueSubgraphPayload::load_str(payload, decoder.extension_registry())?;
-                decoder.insert_subgraph_from_payload(qubits, bits, params, &payload)
-            }
+            } => match OpaqueSubgraphPayload::load_str(payload, decoder.extension_registry()) {
+                Ok(payload) => decoder.insert_subgraph_from_payload(qubits, bits, params, &payload),
+                _ => Ok(DecodeStatus::Unsupported),
+            },
             PytketOperation {
                 op_type: PytketOptype::CircBox,
                 op_box:

tket/src/serialize/pytket/extension/prelude.rs

@@ -7,6 +7,7 @@ use crate::serialize::pytket::decoder::{
 };
 use crate::serialize::pytket::encoder::{EmitCommandOptions, EncodeStatus, PytketEncoderContext};
 use crate::serialize::pytket::extension::{PytketDecoder, PytketTypeTranslator, RegisterCount};
+use crate::serialize::pytket::opaque::OpaqueSubgraphPayload;
 use crate::serialize::pytket::{PytketDecodeError, PytketEncodeError};
 use crate::Circuit;
 use hugr::extension::prelude::{bool_t, qb_t, BarrierDef, Noop, TupleOpDef, PRELUDE_ID};
@@ -129,21 +130,26 @@ impl PytketDecoder for PreludeEmitter {
         qubits: &[TrackedQubit],
         bits: &[TrackedBit],
         params: &[LoadedParameter],
-        opgroup: Option<&str>,
+        _opgroup: Option<&str>,
         decoder: &mut PytketDecoderContext<'h>,
     ) -> Result<DecodeStatus, PytketDecodeError> {
         let op: OpType = match op.op_type {
             PytketOptype::noop => Noop::new(qb_t()).into(),
             PytketOptype::Barrier => {
-                // We use pytket barriers in the pytket encoder framework to store
-                // HUGRs that cannot be represented in pytket.
+                // We use tket1 barriers as part of the encoder/decoder to
+                // represent regions of the hugr that could not be encoded.
                 //
-                // We take care here and detect when that happens.
-                // TODO: For now, we just say the conversion is unsupported instead of extracting the Hugr.
-                if opgroup == Some("UNSUPPORTED_HUGR") {
+                // Those are handled in in the `core.rs` decoder, so we should
+                // ignore them here.
+                if op
+                    .data
+                    .as_ref()
+                    .is_some_and(|payload| OpaqueSubgraphPayload::is_valid_payload(payload))
+                {
                     return Ok(DecodeStatus::Unsupported);
                 }
 
+                // For all other barrier commands, we emit a hugr Barrier.
                 let types = [vec![qb_t(); qubits.len()], vec![bool_t(); bits.len()]].concat();
                 hugr::extension::prelude::Barrier::new(types).into()
             }

tket/src/serialize/pytket/opaque.rs

@@ -6,7 +6,10 @@ mod subgraph;
 
 pub use subgraph::OpaqueSubgraph;
 
-pub use payload::{EncodedEdgeID, OpaqueSubgraphPayload, OPGROUP_OPAQUE_HUGR};
+pub use payload::{EncodedEdgeID, OpaqueSubgraphPayload};
+
+#[expect(deprecated)]
+pub use payload::OPGROUP_OPAQUE_HUGR;
 
 use std::collections::BTreeMap;
 use std::ops::Index;
@@ -117,31 +120,25 @@ impl<N: HugrNode> OpaqueSubgraphs<N> {
     /// If the pytket command is a barrier operation encoding an opaque subgraph, replace its [`OpaqueSubgraphPayload::External`] pointer
     /// if present with a [`OpaqueSubgraphPayload::Inline`] payload.
     ///
-    /// # Errors
-    ///
-    /// Returns an error if a barrier operation with the [`OPGROUP_OPAQUE_HUGR`] opgroup has an invalid payload.
+    /// Ignores barriers whose data payload cannot be decoded into an [`OpaqueSubgraphPayload`].
     pub(super) fn inline_if_payload(
         &self,
         command: &mut tket_json_rs::circuit_json::Command,
         hugr: &impl HugrView<Node = N>,
     ) -> Result<(), PytketEncodeError<N>> {
-        if command.op.op_type != tket_json_rs::OpType::Barrier
-            || command.opgroup.as_deref() != Some(OPGROUP_OPAQUE_HUGR)
-        {
+        if command.op.op_type != tket_json_rs::OpType::Barrier {
             return Ok(());
         }
         let Some(payload) = command.op.data.take() else {
-            return Err(PytketEncodeError::custom(format!(
-                "Barrier operation with opgroup {OPGROUP_OPAQUE_HUGR} has no data payload."
-            )));
+            return Ok(());
         };
 
-        let Some(subgraph_id) = parse_external_payload_id(&payload)? else {
-            // Inline payload, nothing to do.
+        let Some(subgraph_id) = OpaqueSubgraphPayload::parse_external_id(&payload) else {
+            // Not an External Payload, nothing to do.
             return Ok(());
         };
         if !self.contains(subgraph_id) {
-            return Err(PytketEncodeError::custom(format!("Barrier operation with opgroup {OPGROUP_OPAQUE_HUGR} points to an unknown subgraph: {subgraph_id}")));
+            return Err(PytketEncodeError::custom(format!("Barrier operation with external subgraph payload points to an unknown subgraph: {subgraph_id}")));
         }
 
         let payload = OpaqueSubgraphPayload::new_inline(&self[subgraph_id], hugr)?;
@@ -169,38 +166,3 @@ impl<N> Default for OpaqueSubgraphs<N> {
         }
     }
 }
-
-/// Parse an external payload from a string payload.
-///
-/// Returns `None` if the payload is inline.
-///
-/// # Errors
-///
-/// Returns an error if the payload is invalid.
-fn parse_external_payload_id<N: HugrNode>(
-    payload: &str,
-) -> Result<Option<SubgraphId>, PytketEncodeError<N>> {
-    // Check if the payload is inline, without fully copying it to memory.
-    #[derive(serde::Deserialize)]
-    struct PartialPayload {
-        pub typ: String,
-        pub id: Option<SubgraphId>,
-    }
-
-    // Don't do the full deserialization of the payload to avoid allocating a new String for the
-    // encoded envelope.
-    let partial_payload: PartialPayload =
-        serde_json::from_str(payload).map_err(|e: serde_json::Error| {
-            PytketEncodeError::custom(format!(
-            "Barrier operation with opgroup {OPGROUP_OPAQUE_HUGR} has corrupt data payload: {e}"
-        ))
-        })?;
-
-    match (partial_payload.typ.as_str(), partial_payload.id) {
-        ("Inline", None) => Ok(None),
-        ("External", Some(id)) => Ok(Some(id)),
-        _ => Err(PytketEncodeError::custom(format!(
-            "Barrier operation with opgroup {OPGROUP_OPAQUE_HUGR} has invalid data payload: {payload:?}"
-        ))),
-    }
-}

tket/src/serialize/pytket/opaque/payload.rs

@@ -19,6 +19,10 @@ use super::SubgraphId;
 /// Pytket opgroup used to identify opaque barrier operations that encode opaque HUGR subgraphs.
 ///
 /// See [`OpaqueSubgraphPayload`].
+#[deprecated(
+    note = "Opaque barriers do not set an opgroup anymore",
+    since = "0.16.1"
+)]
 pub const OPGROUP_OPAQUE_HUGR: &str = "OPAQUE_HUGR";
 
 /// Identifier for a wire in the Hugr, encoded as a 64-bit hash that is
@@ -77,11 +81,13 @@ pub enum OpaqueSubgraphPayload {
     /// A reference to a subgraph tracked by an `OpaqueSubgraphs` registry
     /// in an [`EncodedCircuit`][super::super::circuit::EncodedCircuit]
     /// structure.
+    #[serde(rename = "HugrExternal")]
     External {
         /// The ID of the subgraph in the `OpaqueSubgraphs` registry.
         id: SubgraphId,
     },
     /// An inline payload, carrying the encoded envelope for the HUGR subgraph.
+    #[serde(rename = "HugrInline")]
     Inline {
         /// A string envelope containing the encoded HUGR subgraph.
         hugr_envelope: String,
@@ -208,4 +214,43 @@ impl OpaqueSubgraphPayload {
     pub fn is_external(&self) -> bool {
         matches!(self, Self::External { .. })
     }
+
+    /// Parse the contents of an [`OpaqueSubgraphPayload::External`] from a string payload.
+    ///
+    /// Returns `None` if the payload is [`OpaqueSubgraphPayload::Inline`] or not an
+    /// [`OpaqueSubgraphPayload`].
+    ///
+    /// This method is more efficient than calling [Self::load_str], as it
+    /// requires no allocations.
+    pub fn parse_external_id(payload: &str) -> Option<SubgraphId> {
+        #[derive(serde::Deserialize)]
+        #[serde(rename = "HugrExternal")]
+        #[serde(tag = "typ")]
+        struct PartialPayload {
+            pub id: SubgraphId,
+        }
+
+        // Deserialize the payload if it is External, avoiding a full copy to memory
+        // for the other variant.
+        serde_json::from_str::<PartialPayload>(payload)
+            .ok()
+            .map(|payload| payload.id)
+    }
+
+    /// Returns `true` if a string encodes an [`OpaqueSubgraphPayload`].
+    ///
+    /// This method is more efficient than calling [Self::load_str], as it
+    /// requires no allocations.
+    pub fn is_valid_payload(payload: &str) -> bool {
+        #[derive(serde::Deserialize)]
+        #[serde(tag = "typ")]
+        enum PartialPayload {
+            HugrExternal {},
+            HugrInline {},
+        }
+
+        // Deserialize the payload if it is External, avoiding a full copy to memory
+        // for the other variant.
+        serde_json::from_str::<PartialPayload>(payload).is_ok()
+    }
 }

tket/src/serialize/pytket/tests.rs

@@ -108,7 +108,7 @@ const BARRIER: &str = r#"{
         "qubits": [["q", [0]], ["q", [1]], ["q", [2]]],
         "commands": [
             {"args": [["q", [0]], ["q", [1]]], "op": {"type": "CX"}},
-            {"args": [["q", [1]], ["q", [2]]], "op": {"type": "Barrier"}},
+            {"args": [["q", [1]], ["q", [2]]], "op": {"type": "Barrier", "data": "Something invalid"}},
             {"args": [["q", [2]], ["c", [1]]], "op": {"type": "Measure"}}
         ],
         "created_qubits": [],