diff --git a/crates/headroom-core/src/transforms/smart_crusher/compaction/classifier.rs b/crates/headroom-core/src/transforms/smart_crusher/compaction/classifier.rs index 7a5d88652..7cb6c7556 100644 --- a/crates/headroom-core/src/transforms/smart_crusher/compaction/classifier.rs +++ b/crates/headroom-core/src/transforms/smart_crusher/compaction/classifier.rs @@ -57,6 +57,11 @@ pub struct ClassifyConfig { /// `<` count above which a long string is considered HTML-ish. /// Default: 3. pub html_min_open_brackets: usize, + /// When false, long strings are NOT classified as opaque — they stay + /// `Scalar` and render verbatim, so output is marker-free and + /// guaranteed-lossless. Mirrors the row-drop path's `enable_ccr_marker` + /// gate (see `crusher.rs`). Default: true. + pub emit_opaque_markers: bool, } impl Default for ClassifyConfig { @@ -65,6 +70,7 @@ impl Default for ClassifyConfig { opaque_min_bytes: 256, base64_alphabet_ratio: 0.95, html_min_open_brackets: 3, + emit_opaque_markers: true, } } } @@ -94,8 +100,10 @@ fn classify_string(s: &str, cfg: &ClassifyConfig) -> CellClass { } } - // Opaque-blob check — only for strings above the byte threshold. - if s.len() <= cfg.opaque_min_bytes { + // Opaque-blob check — only for strings above the byte threshold, and + // only when opaque markers are enabled. With markers off, keep the full + // string verbatim (Scalar) so the output stays lossless and marker-free. + if s.len() <= cfg.opaque_min_bytes || !cfg.emit_opaque_markers { return CellClass::Scalar; } @@ -234,6 +242,22 @@ mod tests { } } + #[test] + fn long_string_stays_scalar_when_opaque_markers_disabled() { + // #1091: with opaque markers disabled, a long string must NOT be + // classified Opaque (which would emit a `<>` marker); it stays + // Scalar and renders verbatim, so the output is lossless. + let v = Value::String("x".repeat(512)); + // Default config classifies it Opaque. + assert!(matches!(classify_cell(&v, &cfg()), CellClass::Opaque(_))); + // Markers disabled → Scalar (verbatim). + let no_markers = ClassifyConfig { + emit_opaque_markers: false, + ..ClassifyConfig::default() + }; + assert_eq!(classify_cell(&v, &no_markers), CellClass::Scalar); + } + #[test] fn base64_blob_detected() { let s = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/==".repeat(5); diff --git a/crates/headroom-core/src/transforms/smart_crusher/crusher.rs b/crates/headroom-core/src/transforms/smart_crusher/crusher.rs index 456271b67..98d986780 100644 --- a/crates/headroom-core/src/transforms/smart_crusher/crusher.rs +++ b/crates/headroom-core/src/transforms/smart_crusher/crusher.rs @@ -163,6 +163,13 @@ impl SmartCrusher { max_flatten_inner_keys: config.compaction_max_flatten_inner_keys, min_buckets: config.compaction_min_buckets, max_buckets: config.compaction_max_buckets, + // Honor the CCR marker gate for opaque-blob cells too (not just + // the row-drop path), so `enable_ccr_marker=false` yields + // marker-free, lossless output. Fixes #1091. + classify: ClassifyConfig { + emit_opaque_markers: config.enable_ccr_marker, + ..ClassifyConfig::default() + }, ..CompactConfig::default() }; SmartCrusherBuilder::new(config) @@ -595,7 +602,12 @@ impl SmartCrusher { // 2. Opaque blob: substitute with CCR marker AND stash the // original in the store (PR8) so retrieval works. Hash + format // identical to walker.rs via the shared helper — zero drift. - let cfg = ClassifyConfig::default(); + // Gated by `enable_ccr_marker` so disabling markers stays lossless + // here too (#1091). + let cfg = ClassifyConfig { + emit_opaque_markers: self.config.enable_ccr_marker, + ..ClassifyConfig::default() + }; if let CellClass::Opaque(kind) = classify_cell(&Value::String(s.to_string()), &cfg) { let marker = emit_opaque_ccr_marker(s, &kind, self.ccr_store.as_ref()); let kind_label = opaque_kind_label(&kind); diff --git a/tests/test_smart_crusher_toin_attachment.py b/tests/test_smart_crusher_toin_attachment.py index 95d2ede18..c4713787d 100644 --- a/tests/test_smart_crusher_toin_attachment.py +++ b/tests/test_smart_crusher_toin_attachment.py @@ -194,6 +194,41 @@ def test_ccr_inject_marker_false_suppresses_markers_in_output(fresh_toin): assert "_ccr_dropped" not in result.compressed +def test_ccr_inject_marker_false_suppresses_opaque_blob_markers(fresh_toin): + """#1091: `inject_retrieval_marker=False` must also suppress the + *opaque-blob* CCR markers, not just the row-drop path. + + A long string cell (> opaque_min_bytes) used to be substituted with a + `<>` marker unconditionally — so no config produced + guaranteed-lossless output. This test pins both directions: with markers + ON the opaque blob IS replaced by a marker (proving the input genuinely + triggers the opaque path), and with markers OFF the blob survives verbatim + with no marker.""" + import json + + from headroom.config import CCRConfig + + # Distinct >256-byte string cells trigger the opaque-blob path. + payload = json.dumps( + [{"id": i, "name": f"row{i}", "blob": f"sentinel{i}_" + "x" * 400} for i in range(60)] + ) + + on = SmartCrusher( + SmartCrusherConfig(), + ccr_config=CCRConfig(enabled=True, inject_retrieval_marker=True), + ).crush(payload, query="", bias=1.0) + # Sanity: the input really does exercise the opaque-blob path. + assert "<