Fix brio encoding/decoding

Author: garethj2

src/haystack/encoding/brio.rs

@@ -37,3 +37,6 @@ pub mod decode;
 
 #[cfg(all(test, feature = "brio-encoding", feature = "brio-decoding"))]
 mod haxall_fixtures;
+
+#[cfg(all(test, feature = "brio-encoding", feature = "brio-decoding"))]
+mod json_fixture_tests;

src/haystack/encoding/brio/consts.rs

@@ -17,6 +17,14 @@ use std::sync::OnceLock;
 /// Seconds between the Unix epoch (1970-01-01) and the Fantom epoch (2000-01-01).
 pub const FANTOM_EPOCH_UNIX_SECS: i64 = 946_684_800;
 
+/// Maximum constant code the encoder may emit, matching Haxall's `BrioConsts.maxSafeCode = 945`.
+///
+/// Haxall caps its writer at this value so that older peers (which may not recognise
+/// constants added in post-3.0.17 versions) are not sent an opaque varint they cannot
+/// look up.  Codes 946–1002 (3.0.25 and 3.0.27 additions) are recognised during *decoding*
+/// but intentionally not emitted during *encoding*.
+pub const MAX_SAFE_CONST_CODE: i64 = 945;
+
 /// Canonical string constants, indexed from 0.  Index 0 is the empty string.
 /// Haxall encodes `""` as `varint(0)` — confirmed by `BrioTest.fan`:
 ///   `verifyConsts(cp, "", 0)`
@@ -1055,9 +1063,17 @@ fn consts_map() -> &'static HashMap<&'static str, i64> {
 
 /// Look up `s` in the constant table.
 ///
-/// Returns `Some(index)` (1-based) if found, `None` otherwise.
+/// Returns `Some(index)` if found **and** the index is within [`MAX_SAFE_CONST_CODE`],
+/// otherwise `None` (triggering inline encoding).  This mirrors Haxall's
+/// `BrioConsts.encode(val, maxStrCode)` cap so that libhaystack does not emit opaque
+/// constant codes that older Haxall peers cannot resolve.
 pub fn lookup_const(s: &str) -> Option<i64> {
-    consts_map().get(s).copied()
+    let idx = consts_map().get(s).copied()?;
+    if idx <= MAX_SAFE_CONST_CODE {
+        Some(idx)
+    } else {
+        None
+    }
 }
 
 /// Retrieve the constant string at `idx` (0-based; 0 = `""`).
@@ -1160,13 +1176,28 @@ mod tests {
 
     #[test]
     fn test_lookup_roundtrip() {
-        // Every entry including index 0 ("") should round-trip.
+        // Entries 0..=MAX_SAFE_CONST_CODE should round-trip via lookup_const.
+        // Entries above MAX_SAFE_CONST_CODE are decode-only (get_const still works,
+        // but lookup_const returns None so they are encoded as inline strings).
         for (i, &s) in CONSTS.iter().enumerate() {
-            let idx =
-                lookup_const(s).unwrap_or_else(|| panic!("Missing const at index {i}: {s:?}"));
-            assert_eq!(idx, i as i64);
-            let got = get_const(idx).unwrap();
-            assert_eq!(got, s);
+            if i as i64 <= MAX_SAFE_CONST_CODE {
+                let idx =
+                    lookup_const(s).unwrap_or_else(|| panic!("Missing const at index {i}: {s:?}"));
+                assert_eq!(idx, i as i64);
+                let got = get_const(idx).unwrap();
+                assert_eq!(got, s);
+            } else {
+                // Above the safe cap: should NOT be returned by lookup_const.
+                assert!(
+                    lookup_const(s).is_none(),
+                    "Expected None for index {i} ({s:?}) above MAX_SAFE_CONST_CODE"
+                );
+                // get_const should still decode them (for streams from Haxall).
+                assert!(
+                    get_const(i as i64).is_some(),
+                    "get_const({i}) should still work"
+                );
+            }
         }
     }
 }

src/haystack/encoding/brio/decode.rs

@@ -13,10 +13,10 @@ use crate::haystack::val::{
 
 use super::consts::{FANTOM_EPOCH_UNIX_SECS, get_const};
 use super::encode::{
-    CTRL_COORD, CTRL_DATE, CTRL_DATETIME_I4, CTRL_DATETIME_I8, CTRL_DICT, CTRL_DICT_EMPTY,
-    CTRL_FALSE, CTRL_GRID, CTRL_LIST, CTRL_LIST_EMPTY, CTRL_MARKER, CTRL_NA, CTRL_NULL,
-    CTRL_NUMBER_F8, CTRL_NUMBER_I2, CTRL_NUMBER_I4, CTRL_REF_I8, CTRL_REF_STR, CTRL_REMOVE,
-    CTRL_STR, CTRL_SYMBOL, CTRL_TIME, CTRL_TRUE, CTRL_URI, CTRL_XSTR,
+    CTRL_BUF, CTRL_COORD, CTRL_DATE, CTRL_DATETIME_I4, CTRL_DATETIME_I8, CTRL_DICT,
+    CTRL_DICT_EMPTY, CTRL_FALSE, CTRL_GRID, CTRL_LIST, CTRL_LIST_EMPTY, CTRL_MARKER, CTRL_NA,
+    CTRL_NULL, CTRL_NUMBER_F8, CTRL_NUMBER_I2, CTRL_NUMBER_I4, CTRL_REF_I8, CTRL_REF_STR,
+    CTRL_REMOVE, CTRL_STR, CTRL_SYMBOL, CTRL_TIME, CTRL_TRUE, CTRL_URI, CTRL_XSTR,
 };
 
 // ---------------------------------------------------------------------------
@@ -243,6 +243,24 @@ fn datetime_from_nanos(fantom_nanos: i64, tz: &str) -> Result<DateTime> {
 // Private payload decoders (ctrl byte already consumed by caller)
 // ---------------------------------------------------------------------------
 
+/// Decode a Haxall binary buffer (`CTRL_BUF`/0x13) payload: `varint(size)` + raw bytes.
+///
+/// libhaystack has no `Bin` value kind, so the buffer is surfaced as
+/// `XStr("Bin", "<lowercase hex>")`, preserving all bytes without data loss and
+/// remaining compatible with Haystack's XStr encoding convention for binary data.
+fn decode_buf_as_xstr<R: Read>(reader: &mut R) -> Result<XStr> {
+    let size = decode_varint(reader)?;
+    if size < 0 {
+        return Err(Error::Message("Negative Buf size".into()));
+    }
+    let mut bytes = vec![0u8; size as usize];
+    reader
+        .read_exact(&mut bytes)
+        .map_err(|e| Error::Message(e.to_string()))?;
+    let hex: String = bytes.iter().map(|b| format!("{b:02x}")).collect();
+    Ok(XStr::make("Bin", &hex))
+}
+
 /// Decode a non-empty Dict payload: `'{' varint(count) (key value)* '}'`
 fn decode_dict_payload<R: Read>(reader: &mut R) -> Result<Dict> {
     let marker = read_u8(reader)?;
@@ -474,6 +492,7 @@ impl FromBrio for Value {
                 &decode_str(reader)?,
             ))),
             CTRL_SYMBOL => Ok(Value::from(Symbol::make(&decode_str(reader)?))),
+            CTRL_BUF => decode_buf_as_xstr(reader).map(Value::from),
             CTRL_DICT_EMPTY => Ok(Value::from(Dict::default())),
             CTRL_DICT => decode_dict_payload(reader).map(Value::from),
             CTRL_LIST_EMPTY => Ok(Value::from(List::default())),
@@ -501,8 +520,23 @@ pub fn from_brio<R: Read>(reader: &mut R) -> Result<Value> {
 
 fn make_number(v: f64, unit: &str) -> Number {
     if unit.is_empty() {
+        return Number::make(v);
+    }
+    // Haxall `BrioReader.consumeUnit` calls `Number.loadUnit(str, checked:false)` which
+    // returns `null` for unrecognised unit strings (the number becomes unitless).  Match
+    // that semantic when units-db is available.  Without units-db, fall back to the
+    // DEFAULT_UNIT sentinel so unit information is at least structurally preserved.
+    #[cfg(feature = "units-db")]
+    {
+        use crate::units::get_unit;
+        if let Some(u) = get_unit(unit) {
+            return Number::make_with_unit(v, u);
+        }
+        // Unit string present but not in database → unitless (matches Haxall null).
         Number::make(v)
-    } else {
+    }
+    #[cfg(not(feature = "units-db"))]
+    {
         use crate::units::get_unit_or_default;
         Number::make_with_unit(v, get_unit_or_default(unit))
     }

src/haystack/encoding/brio/encode.rs

@@ -35,6 +35,10 @@ pub const CTRL_DATETIME_I4: u8 = 0x0f;
 pub const CTRL_DATETIME_I8: u8 = 0x10;
 pub const CTRL_COORD: u8 = 0x11;
 pub const CTRL_XSTR: u8 = 0x12;
+/// Binary buffer control byte.  Emitted by Haxall's `BrioWriter.writeBuf`; libhaystack
+/// does not encode this type (the `Value` model has no Bin variant) but the decoder must
+/// consume it gracefully to avoid stream corruption when reading Haxall-produced data.
+pub const CTRL_BUF: u8 = 0x13;
 pub const CTRL_DICT_EMPTY: u8 = 0x14;
 pub const CTRL_DICT: u8 = 0x15;
 pub const CTRL_LIST_EMPTY: u8 = 0x16;
@@ -389,12 +393,21 @@ impl ToBrio for XStr {
 
 impl ToBrio for Dict {
     fn to_brio<W: Write>(&self, writer: &mut W) -> Result<()> {
-        if self.is_empty() {
+        // Haxall BrioWriter.fan skips null-valued tags: they must not appear in
+        // the count or in the encoded tag/value pairs.
+        let non_null_count = self
+            .iter()
+            .filter(|(_, v)| !matches!(v, Value::Null))
+            .count();
+        if non_null_count == 0 {
             writer.write_all(&[CTRL_DICT_EMPTY])?;
         } else {
             writer.write_all(&[CTRL_DICT, b'{'])?;
-            encode_varint(writer, self.len() as i64)?;
+            encode_varint(writer, non_null_count as i64)?;
             for (key, val) in self.iter() {
+                if matches!(val, Value::Null) {
+                    continue;
+                }
                 encode_str(writer, key)?;
                 val.to_brio(writer)?;
             }