Merge pull request #306 from chopratejas/rust-stage-3d-audit-finalize

fix(smart_crusher): re-land orphaned audit close-out — CCR knob + scorer fail-loud

Author: chopratejas

.claude-plugin/marketplace.json

@@ -5,14 +5,14 @@
   },
   "metadata": {
     "description": "Headroom marketplace for Claude Code and GitHub Copilot CLI plugins.",
-    "version": "0.13.4"
+    "version": "0.14.2"
   },
   "plugins": [
     {
       "name": "headroom",
       "source": "./plugins/headroom-agent-hooks",
       "description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
-      "version": "0.13.4",
+      "version": "0.14.2",
       "author": {
         "name": "Headroom Contributors",
         "url": "https://github.com/chopratejas/headroom"

.github/plugin/marketplace.json

@@ -5,14 +5,14 @@
   },
   "metadata": {
     "description": "Headroom marketplace for Claude Code and GitHub Copilot CLI plugins.",
-    "version": "0.13.4"
+    "version": "0.14.2"
   },
   "plugins": [
     {
       "name": "headroom",
       "source": "./plugins/headroom-agent-hooks",
       "description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
-      "version": "0.13.4",
+      "version": "0.14.2",
       "author": {
         "name": "Headroom Contributors",
         "url": "https://github.com/chopratejas/headroom"

RUST_DEV.md

@@ -172,8 +172,8 @@ so they don't regress further or get forgotten.
 | Subsystem | State | Tracked by |
 |---|---|---|
 | TOIN learning loop | **Re-attached 2026-04-28.** Shim's `crush()` and `_smart_crush_content()` now call `toin.record_compression()` after a real compression. Filtered on `strategy != "passthrough"` to ignore JSON re-canonicalization. Best-effort: TOIN failures are logged at debug level and don't break compression. | `tests/test_smart_crusher_toin_attachment.py` |
-| CCR marker emission knob | **Open gap.** `ccr_config.inject_retrieval_marker=False` is not honored — the Rust port emits `<<ccr:HASH N_rows_offloaded>>` markers as part of `dropped_summary` unconditionally. Shim now logs a WARNING when callers pass `False`. **Fix needed:** add a `enable_ccr_marker: bool` gate in `crates/headroom-core/src/transforms/smart_crusher/crusher.rs::crush_array`, plumb through `SmartCrusherConfig` and the PyO3 bridge. | This file + warning at `headroom/transforms/smart_crusher.py` |
-| Custom relevance scorer | **Open gap.** `relevance_config` and `scorer` constructor args are accepted for source compatibility but the Rust default `HybridScorer` always runs. Shim now logs WARNING (previously debug). **Fix needed:** expose a Python-bridged scorer constructor surface from `crates/headroom-core/src/relevance/`. | Warning at `headroom/transforms/smart_crusher.py` |
+| CCR marker emission knob | **Honored end-to-end 2026-04-29.** New `enable_ccr_marker: bool` field on Rust `SmartCrusherConfig`; `crush_array` checks it before emitting the `<<ccr:HASH>>` marker text and the CCR store write. Python shim flips it from `ccr_config.enabled and ccr_config.inject_retrieval_marker` — both flags collapse to the same Rust gate, since storing payloads under either off-switch makes no sense. Scope: gates only the row-drop sentinel path; Stage-3c.2 opaque-string CCR substitutions still emit always (no Python equivalent, no production caller asks for suppression). | `tests/test_smart_crusher_toin_attachment.py` + `crates/headroom-core/.../crusher.rs::tests::enable_ccr_marker_*` |
+| Custom relevance scorer | **Closed (fail-loud) 2026-04-29.** `relevance_config` and `scorer` constructor args remain in the signature for source compat, but the shim raises `NotImplementedError` when either is non-None — silently dropping a user-supplied scorer is a textbook silent-fallback bug. Full plumbing waits on Stage-3c.2's relevance-crate Python bridge. | `tests/test_smart_crusher_toin_attachment.py::test_custom_*_arg_raises_not_implemented` |
 | Per-tool TOIN learning hook | **Re-attached partially.** `_smart_crush_content` accepts `tool_name` and now threads it into the TOIN record. The hook is best-effort — it improves `query_context` aggregation but doesn't drive per-tool overrides yet. | `tests/test_smart_crusher_toin_attachment.py::test_smart_crush_content_records_to_toin` |
 
 ### DiffCompressor
@@ -185,7 +185,7 @@ so they don't regress further or get forgotten.
 
 ### Watch list (potential regressions, not yet audited)
 
-- `CCRConfig.enabled=False` end-to-end behavior. Currently the Rust port has a CCR store that's controlled by builder selection, not by a config flag. Sometimes-disabled paths haven't been audited.
+- `CCRConfig.enabled=False` end-to-end — **closed 2026-04-29**. Both `enabled=False` and `inject_retrieval_marker=False` collapse to the same Rust `enable_ccr_marker=False` gate (no marker, no store write). See the SmartCrusher table above.
 - `SmartCrusherConfig.use_feedback_hints=False` — config field is forwarded to Rust but its honoring inside the Rust crusher hasn't been verified against a parity fixture for the disabled path.
 
 When any item above changes, update both this section and the test file. The shim's docstring also references this section — keep them aligned.

crates/headroom-core/src/transforms/smart_crusher/config.rs

@@ -62,6 +62,21 @@ pub struct SmartCrusherConfig {
     /// lossless when available; set to `1.0` to effectively disable
     /// the lossless path (lossy + CCR always).
     pub lossless_min_savings_ratio: f64,
+    /// Master gate for CCR-Dropped row-drop sentinels. When `false`,
+    /// the lossy `crush_array` path skips both the `<<ccr:HASH>>`
+    /// marker text AND the CCR-store write (no point storing a
+    /// payload nothing in the prompt can reference).
+    ///
+    /// The Python shim flips this from
+    /// `ccr_config.enabled and ccr_config.inject_retrieval_marker`,
+    /// so either off-switch on the Python side disables the gate.
+    /// Default `true` — preserves prior behavior.
+    ///
+    /// Scope: gates only the `crush_array` row-drop path. Stage-3c.2
+    /// opaque-string CCR substitutions (in `walker::process_value`)
+    /// still emit always; they have no Python equivalent and no
+    /// production caller has asked for them to be suppressed.
+    pub enable_ccr_marker: bool,
 }
 
 impl Default for SmartCrusherConfig {
@@ -87,6 +102,7 @@ impl Default for SmartCrusherConfig {
             last_fraction: 0.15,
             relevance_threshold: 0.3,
             lossless_min_savings_ratio: 0.30,
+            enable_ccr_marker: true,
         }
     }
 }
@@ -118,5 +134,6 @@ mod tests {
         assert_eq!(c.last_fraction, 0.15);
         assert_eq!(c.relevance_threshold, 0.3);
         assert_eq!(c.lossless_min_savings_ratio, 0.30);
+        assert!(c.enable_ccr_marker);
     }
 }

crates/headroom-core/src/transforms/smart_crusher/crusher.rs

@@ -660,13 +660,20 @@ impl SmartCrusher {
         );
         let result = self.execute_plan(&plan, items);
 
-        // Emit CCR-Dropped marker iff rows were actually dropped.
-        // **This is the cornerstone of CCR's no-data-loss guarantee:**
-        // we hash the full original, stash it in the configured store,
-        // and emit a marker pointing at that hash. The runtime later
-        // serves the original back via retrieval tool calls.
+        // Emit CCR-Dropped marker iff rows were actually dropped AND
+        // the marker gate is on. **The marker is the cornerstone of
+        // CCR's no-data-loss guarantee:** we hash the full original,
+        // stash it in the configured store, and emit a marker pointing
+        // at that hash. The runtime later serves the original back via
+        // retrieval tool calls.
+        //
+        // When `enable_ccr_marker` is false (Python shim's path for
+        // `ccr_config.enabled=False` or `inject_retrieval_marker=False`)
+        // we keep the row drops (compression is still requested) but
+        // skip the marker text and the store write — there's no point
+        // storing a payload that nothing in the prompt can reference.
         let dropped_count = items.len().saturating_sub(result.len());
-        let (ccr_hash, dropped_summary) = if dropped_count > 0 {
+        let (ccr_hash, dropped_summary) = if dropped_count > 0 && self.config.enable_ccr_marker {
             // Serialize the original array exactly ONCE. The hash is
             // taken over those bytes, and (if a store is configured) the
             // same bytes get stored — eliminating a redundant tree clone
@@ -1439,4 +1446,84 @@ mod tests {
         assert!(try_parse_json_container("not json").is_none());
         assert!(try_parse_json_container("{malformed").is_none());
     }
+
+    // ---------- enable_ccr_marker gate (PR #301 re-land) ----------
+
+    #[test]
+    fn enable_ccr_marker_false_suppresses_marker_and_store() {
+        // The Rust-side gate. Compression still runs (rows drop) but
+        // the result carries no marker text, no hash, and the CCR
+        // store does NOT grow — there's no point storing what nothing
+        // in the prompt can reference.
+        use crate::ccr::InMemoryCcrStore;
+        use crate::transforms::smart_crusher::SmartCrusherBuilder;
+        use std::sync::Arc;
+
+        let store: Arc<dyn CcrStore> = Arc::new(InMemoryCcrStore::new());
+        let cfg = SmartCrusherConfig {
+            lossless_min_savings_ratio: 0.99, // force lossy path
+            enable_ccr_marker: false,
+            ..SmartCrusherConfig::default()
+        };
+        let c = SmartCrusherBuilder::new(cfg)
+            .with_ccr_store(Arc::clone(&store))
+            .build();
+        let items: Vec<Value> = (0..50).map(|_| json!({"status": "ok"})).collect();
+
+        let store_len_before = store.len();
+        let result = c.crush_array(&items, "", 1.0);
+        let store_len_after = store.len();
+
+        // Rows were dropped (we built 50, kept fewer).
+        assert!(result.items.len() < items.len(), "lossy path didn't fire");
+        // Gate held: no marker, no hash.
+        assert!(result.ccr_hash.is_none(), "ccr_hash should be None");
+        assert!(
+            result.dropped_summary.is_empty(),
+            "dropped_summary should be empty, got: {:?}",
+            result.dropped_summary
+        );
+        // Store did NOT grow.
+        assert_eq!(
+            store_len_after, store_len_before,
+            "ccr_store grew despite enable_ccr_marker=false"
+        );
+    }
+
+    #[test]
+    fn enable_ccr_marker_true_is_default_behavior() {
+        // Default config still emits markers + stores when rows drop.
+        // Sanity: the gate is opt-out, not opt-in.
+        use crate::ccr::InMemoryCcrStore;
+        use crate::transforms::smart_crusher::SmartCrusherBuilder;
+        use std::sync::Arc;
+
+        let store: Arc<dyn CcrStore> = Arc::new(InMemoryCcrStore::new());
+        let cfg = SmartCrusherConfig {
+            lossless_min_savings_ratio: 0.99, // force lossy path
+            ..SmartCrusherConfig::default()
+        };
+        // Default: enable_ccr_marker = true.
+        assert!(cfg.enable_ccr_marker);
+        let c = SmartCrusherBuilder::new(cfg)
+            .with_ccr_store(Arc::clone(&store))
+            .build();
+        let items: Vec<Value> = (0..50).map(|_| json!({"status": "ok"})).collect();
+
+        let store_len_before = store.len();
+        let result = c.crush_array(&items, "", 1.0);
+        let store_len_after = store.len();
+
+        assert!(result.items.len() < items.len(), "lossy path didn't fire");
+        assert!(result.ccr_hash.is_some(), "default should produce a hash");
+        assert!(
+            result.dropped_summary.contains("<<ccr:"),
+            "default should produce a marker: {:?}",
+            result.dropped_summary
+        );
+        assert!(
+            store_len_after > store_len_before,
+            "default should write to ccr_store"
+        );
+    }
 }

crates/headroom-parity/src/lib.rs

@@ -397,6 +397,13 @@ impl TransformComparator for SmartCrusherComparator {
                 .get("lossless_min_savings_ratio")
                 .and_then(|v| v.as_f64())
                 .unwrap_or(defaults.lossless_min_savings_ratio),
+            // Rust-only audit-fix knob — fixtures don't carry this; use
+            // default (true). Recorded fixtures predate the gate and
+            // their expected outputs assume markers fire as before.
+            enable_ccr_marker: config
+                .get("enable_ccr_marker")
+                .and_then(|v| v.as_bool())
+                .unwrap_or(defaults.enable_ccr_marker),
         };
 
         // Use without_compaction so the legacy fixtures (recorded

crates/headroom-py/src/lib.rs

@@ -456,6 +456,7 @@ impl PySmartCrusherConfig {
         last_fraction = 0.15,
         relevance_threshold = 0.3,
         lossless_min_savings_ratio = 0.30,
+        enable_ccr_marker = true,
     ))]
     #[allow(clippy::too_many_arguments)]
     fn new(
@@ -476,6 +477,7 @@ impl PySmartCrusherConfig {
         last_fraction: f64,
         relevance_threshold: f64,
         lossless_min_savings_ratio: f64,
+        enable_ccr_marker: bool,
     ) -> Self {
         Self {
             inner: RustSmartCrusherConfig {
@@ -496,6 +498,7 @@ impl PySmartCrusherConfig {
                 last_fraction,
                 relevance_threshold,
                 lossless_min_savings_ratio,
+                enable_ccr_marker,
             },
         }
     }
@@ -564,6 +567,10 @@ impl PySmartCrusherConfig {
     fn relevance_threshold(&self) -> f64 {
         self.inner.relevance_threshold
     }
+    #[getter]
+    fn enable_ccr_marker(&self) -> bool {
+        self.inner.enable_ccr_marker
+    }
 
     fn __repr__(&self) -> String {
         format!(

headroom/transforms/smart_crusher.py

@@ -18,23 +18,27 @@
 fallback. Build it locally with `scripts/build_rust_extension.sh`
 (wraps `maturin develop`) or install a prebuilt wheel.
 
-# Functionality state (post-audit, 2026-04-28)
+# Functionality state (post-audit, 2026-04-29)
 
 - **TOIN learning** — re-attached. `crush()` and `_smart_crush_content`
   call `toin.record_compression()` after a real compression (filtered on
   `strategy != "passthrough"` to ignore JSON re-canonicalization).
   The retired Python class did this inline; the bridge keeps the
   highest-traffic strategy fueling the learning loop.
-- **CCR marker emission** — partial gap. The Rust port emits
-  `<<ccr:HASH N_rows_offloaded>>` markers unconditionally as part of
-  `dropped_summary`; the `ccr_config.inject_retrieval_marker=False`
-  knob is therefore not honored end-to-end. The shim now logs a
-  WARNING when callers pass `False`. Suppression needs a Rust-side
-  gate; see RUST_DEV.md.
-- **Custom relevance scorer / scorer override** — still not wired.
-  `relevance_config` and `scorer` constructor args are accepted for
-  source compat but the Rust default `HybridScorer` is always used.
-  The shim now logs a WARNING (was: debug) when these are passed.
+- **CCR marker emission** — honored end-to-end. Both
+  `ccr_config.enabled=False` and
+  `ccr_config.inject_retrieval_marker=False` flip the Rust crusher's
+  `enable_ccr_marker` field; the lossy row-drop path then skips both
+  the marker text and the CCR store write. Scope: gates only the
+  row-drop sentinel path. Stage-3c.2 opaque-string CCR substitutions
+  still emit always — they have no Python equivalent.
+- **Custom relevance scorer / scorer override** — fails loud.
+  `relevance_config` and `scorer` constructor args remain in the
+  signature for source compat, but the shim raises
+  `NotImplementedError` when either is non-None. Silently dropping a
+  user-supplied scorer is a silent-fallback bug we explicitly refuse
+  to ship; full plumbing lands with Stage-3c.2's relevance-crate
+  Python bridge.
 """
 
 from __future__ import annotations
@@ -176,37 +180,48 @@ def __init__(
         self._observer = observer
 
         # CCR config is preserved on `self` for callers that read it
-        # back (`headroom.proxy.server` does). Storage-side semantics
-        # (CCR cache lookups) are honored via the Rust crusher's own
-        # CCR store. *Marker emission* is the gap: the Rust port emits
-        # `<<ccr:HASH N_rows_offloaded>>` markers unconditionally as
-        # part of `dropped_summary`, so `inject_retrieval_marker=False`
-        # has no effect on the prompt today. We log a WARNING (not
-        # debug) so it's visible. Suppression of marker emission needs
-        # a Rust-side gate; tracked in RUST_DEV.md.
+        # back (`headroom.proxy.server` does). Both `enabled=False` and
+        # `inject_retrieval_marker=False` collapse to the Rust crusher's
+        # `enable_ccr_marker=False` gate — when either is off, the
+        # lossy row-drop path skips marker emission AND the CCR store
+        # write (no point storing a payload nothing in the prompt can
+        # reference; storing it under `enabled=False` would also be a
+        # surprise side effect the user explicitly disabled).
+        #
+        # Default falls through to `CCRConfig()` so direct callers
+        # (the proxy and tests that don't pass an explicit config) get
+        # the documented dataclass defaults (`enabled=True,
+        # inject_retrieval_marker=True`). The previous override here
+        # set `inject_retrieval_marker=False` as a no-op-intent hack
+        # back when the Rust port silently ignored the flag; now that
+        # the flag is honored, that override would actively suppress
+        # markers + store writes for every caller.
+        #
+        # Scope: gates ONLY the row-drop sentinel path. Stage-3c.2
+        # opaque-string CCR substitutions still emit always — they have
+        # no Python equivalent and no production caller has asked for
+        # them to be suppressed.
         if ccr_config is None:
-            self._ccr_config = CCRConfig(enabled=True, inject_retrieval_marker=False)
+            self._ccr_config = CCRConfig()
         else:
             self._ccr_config = ccr_config
-        if not self._ccr_config.inject_retrieval_marker:
-            logger.warning(
-                "SmartCrusher: ccr_config.inject_retrieval_marker=False "
-                "is currently NOT honored by the Rust port — CCR row-drop "
-                "markers (`<<ccr:HASH N_rows_offloaded>>`) will still appear "
-                "in compressed output. Tracked as a known regression in "
-                "RUST_DEV.md until a Rust-side gate lands."
-            )
 
-        # `relevance_config` and `scorer` are accepted for source
-        # compatibility but currently dropped — Stage 3c.1 ships with
-        # the Rust default `HybridScorer`. Custom scorers re-attach in
-        # Stage 3c.2 when the relevance crate gains a Python-bridged
-        # constructor surface.
+        # `relevance_config` and `scorer` remain in the signature for
+        # source compatibility, but the Rust port doesn't support
+        # overrides yet (it always uses `HybridScorer` from the
+        # relevance crate; the Python-bridged constructor surface
+        # arrives in Stage 3c.2). Silently dropping a user-supplied
+        # scorer would be a textbook silent fallback — if a caller
+        # depends on a custom scoring function and we ignore it, the
+        # compression they get back is wrong in a way they cannot see.
+        # Fail loud instead. See `feedback_no_silent_fallbacks.md`.
         if relevance_config is not None or scorer is not None:
-            logger.warning(
-                "SmartCrusher: custom relevance_config/scorer args are "
-                "currently ignored (Rust port uses default HybridScorer). "
-                "Tracked as a known regression in RUST_DEV.md."
+            raise NotImplementedError(
+                "SmartCrusher: custom `relevance_config` / `scorer` "
+                "overrides are not yet supported by the Rust-backed "
+                "implementation. Pass `None` to use the default "
+                "HybridScorer. Tracked in RUST_DEV.md; full support "
+                "lands with Stage 3c.2's relevance-crate Python bridge."
             )
 
         # Lazy TOIN handle. Loaded on first compression that has items
@@ -236,6 +251,9 @@ def __init__(
             first_fraction=cfg.first_fraction,
             last_fraction=cfg.last_fraction,
             relevance_threshold=0.3,
+            enable_ccr_marker=(
+                self._ccr_config.enabled and self._ccr_config.inject_retrieval_marker
+            ),
         )
         # Default: lossless-first compaction (PR4). Lossless wins for
         # cleanly tabular input where it saves ≥ 30% bytes; otherwise