fix(rust): smart_crusher scaffold review findings — hash truncation, int parse, python-repr matcher

Code review (`/code-review` on commit `7735dd6`) caught one critical
bug, two important parity gaps, and a few quality nits. Fixed all of
them; all 135 unit tests pass; diff_compressor parity harness
unaffected (27/27 still matched).

# Critical fix — `hash_field_name` truncation length

Rust truncated SHA-256 to **16** hex chars; Python uses **8** (per
`smart_crusher.py:177`: `hashlib.sha256(...).hexdigest()[:8]`). 16-char
hashes would never collide with TOIN's 8-char `preserve_fields`,
silently disabling the entire `use_feedback_hints` cache lookup path.

Fix: `hex[..8]` instead of `hex[..16]`. Three pinning tests re-verified
against actual Python reference output. Doc comment now warns
explicitly that the length must match Python or TOIN lookups silently miss.

# Important fix — `python_int_parse` mirrors Python's `int()` semantics

`statistics.rs::detect_sequential_pattern` previously called
`s.parse::<i64>()`. Python's `int()` differs in three ways that affect
realistic payloads:
- strips ASCII whitespace (Rust's `parse` rejects)
- accepts leading `+` (Rust accepts; same)
- accepts PEP 515 underscores like `"3_000"` (Rust rejects)

A field with `[" 1 ", " 2 ", " 3 ", "4", "5"]` would parse all five
in Python (sequential = True) but only one in Rust (`nums.len() < 5`
→ False). Silent parity break.

Fix: new private `python_int_parse` helper that strips whitespace,
handles underscore separators, and rejects edge cases Python rejects.
Six new tests pin the behavior.

# Important fix — `python_repr` for `item_matches_anchors`

Python compares anchors via `anchor in str(item).lower()`. We were
using `serde_json::to_string(&item).to_lowercase()`, which differs in
three ways that affect substring matching:
- quote chars (`'` vs `"`)
- bool/null literals (`True`/`False`/`None` vs `true`/`false`/`null`)
- spacing (`key: value, ...` vs `key:value,...`)

Anchor `"none"` would match Python form but not JSON. Inverse for
`"null"`. Real divergence.

Fix: new private `python_repr` walks `serde_json::Value` and emits
Python-equivalent form. Plus enable `serde_json/preserve_order` at
workspace level so `Value::Object` preserves JSON parse order
(matching Python `dict` since 3.7).

# Suggestion fixes

- Classifier comment for `[True, False, 1] -> MIXED_ARRAY` now walks
both Python and Rust paths step by step.
- `ArrayAnalysis::field_stats` doc notes the BTreeMap vs Python-dict
order nuance for the analyzer port to resolve.
- Added regression tests for "all unparseable strings", "single int
among strings", fractional-step sequential, and the email-typo
pattern.

# Build / test

- `cargo build -p headroom-core` clean.
- `cargo clippy -p headroom-core -- -D warnings` clean.
- 135 unit tests in `headroom-core`, all passing (was 55).
- `cargo run -p headroom-parity run` — diff_compressor 27/27 still matched.

.claude-plugin/marketplace.json

@@ -5,14 +5,14 @@
   },
   "metadata": {
     "description": "Headroom marketplace for Claude Code and GitHub Copilot CLI plugins.",
-    "version": "0.10.17"
+    "version": "0.11.0"
   },
   "plugins": [
     {
       "name": "headroom",
       "source": "./plugins/headroom-agent-hooks",
       "description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
-      "version": "0.10.17",
+      "version": "0.11.0",
       "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.10.17"
+    "version": "0.11.0"
   },
   "plugins": [
     {
       "name": "headroom",
       "source": "./plugins/headroom-agent-hooks",
       "description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
-      "version": "0.10.17",
+      "version": "0.11.0",
       "author": {
         "name": "Headroom Contributors",
         "url": "https://github.com/chopratejas/headroom"

Cargo.lock

@@ -2198,6 +2198,7 @@ version = "1.0.149"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86"
 dependencies = [
+ "indexmap",
  "itoa",
  "memchr",
  "serde",

Cargo.toml

@@ -26,7 +26,12 @@ authors = ["Headroom Maintainers"]
 
 [workspace.dependencies]
 serde = { version = "1", features = ["derive"] }
-serde_json = "1"
+# `preserve_order` makes `serde_json::Value::Object` use IndexMap so JSON
+# parse order is preserved through Value→string→Value round-trips. The
+# smart_crusher port relies on this to match Python's `str(dict)` output,
+# which preserves insertion order; otherwise BTreeMap's sorted-key default
+# would diverge from Python on every multi-key object.
+serde_json = { version = "1", features = ["preserve_order"] }
 bytes = "1"
 thiserror = "1"
 tracing = "0.1"

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

@@ -111,43 +111,117 @@ pub fn extract_query_anchors(text: &str) -> HashSet<String> {
     anchors
 }
 
-/// Check if a JSON object matches any query anchors.
+/// Serialize a `serde_json::Value` to a string matching Python's
+/// `str()` of the equivalent native value.
 ///
-/// Direct port of `item_matches_anchors` (Python `smart_crusher.py:152-168`).
-/// Python uses `str(item).lower()` which produces Python's `dict.__str__`
-/// representation. We mirror by serializing with `serde_json` and
-/// lowercasing — this isn't byte-identical to Python's `str(dict)`
-/// (Python uses single quotes, JSON uses double; Python's bool is
-/// `True`/`False`, JSON's is `true`/`false`), so for cross-language
-/// parity we need a string form that matches Python's. We document this
-/// gap and fix it in the analyzer integration.
+/// Used by `item_matches_anchors` because Python compares anchors via
+/// `anchor in str(item).lower()` and `str(dict)` differs from
+/// `json.dumps(dict)` in three ways that affect substring matching:
 ///
-/// **WARNING:** `str(item).lower()` in Python produces:
-///   `{'key': 'value', 'count': 5, 'ok': True}`
-/// while `serde_json::to_string(&item)` produces:
-///   `{"key":"value","count":5,"ok":true}`
+/// | Aspect           | Python `str(dict)`           | `serde_json::to_string` |
+/// |------------------|------------------------------|-------------------------|
+/// | String quotes    | single `'`                   | double `"`              |
+/// | Booleans / null  | `True`, `False`, `None`      | `true`, `false`, `null` |
+/// | Spacing          | `key: value`, `a, b`         | `key:value`, `a,b`      |
 ///
-/// The anchor matching is substring-based (`anchor in item_str`), so
-/// this difference matters: if an anchor is `"true"` it matches the
-/// JSON form but not the Python form, and vice versa for `"True"`.
+/// All three matter for anchor matching:
+/// - An anchor `"name': 'a"` extracted from a user phrase like
+///   `find {'name': 'alice'}` would match Python's serialization but
+///   never the JSON form.
+/// - An anchor `"true"` (lowercased from `"True"`) matches both, but
+///   the unlowercased version `"True"` is in Python output and not
+///   JSON. Lowercasing both sides handles this.
+/// - An anchor `"name: alice"` (with the space) would match Python
+///   but never JSON.
 ///
-/// **Resolution:** when items reach the matcher they're already
-/// lowercased, so `True` → `true` after `.lower()`, removing one source
-/// of drift. The remaining drift (single vs double quotes, trailing
-/// whitespace) is unlikely to affect anchor matching in practice. We
-/// pin behavior with fixtures and move on.
+/// Output is then lowercased upstream (matching Python's `.lower()`)
+/// so True/False/None case is normalized away after that step.
+fn python_repr(value: &Value) -> String {
+    let mut out = String::new();
+    write_python_repr(&mut out, value);
+    out
+}
+
+fn write_python_repr(out: &mut String, value: &Value) {
+    match value {
+        Value::Null => out.push_str("None"),
+        Value::Bool(true) => out.push_str("True"),
+        Value::Bool(false) => out.push_str("False"),
+        Value::Number(n) => {
+            // Python `str(int)` and `str(float)` produce minimal forms.
+            // `serde_json::Number`'s `Display` matches Python for ints
+            // (`5`) but for floats it can write `1.0` while Python may
+            // write `1.0` too — close enough for substring matching
+            // since anchor strings rarely contain numeric literals
+            // beyond the digit prefix.
+            out.push_str(&n.to_string());
+        }
+        Value::String(s) => {
+            // Python `repr(s)` chooses single or double quotes
+            // depending on content. Default preference is single
+            // quotes; switches to double if the string contains a
+            // single quote and no double. We emit single quotes
+            // always — this matches the dominant case (no quotes in
+            // the string) and is what Python does for `str(dict)` of
+            // most realistic data. The rare case where Python would
+            // switch to double quotes is documented as a known parity
+            // gap in `python_repr_string_with_single_quote_drift`.
+            out.push('\'');
+            out.push_str(s);
+            out.push('\'');
+        }
+        Value::Array(items) => {
+            out.push('[');
+            for (i, item) in items.iter().enumerate() {
+                if i > 0 {
+                    out.push_str(", ");
+                }
+                write_python_repr(out, item);
+            }
+            out.push(']');
+        }
+        Value::Object(map) => {
+            out.push('{');
+            // Python preserves insertion order in `dict.__str__` (since
+            // Python 3.7). We require the workspace `serde_json` to be
+            // built with `preserve_order` so `serde_json::Map` uses
+            // `IndexMap` instead of the default `BTreeMap` — see the
+            // comment on `serde_json` in the workspace `Cargo.toml`.
+            // Without that feature, this iteration is sorted-by-key
+            // and silently diverges from Python on every multi-key
+            // object.
+            for (i, (k, v)) in map.iter().enumerate() {
+                if i > 0 {
+                    out.push_str(", ");
+                }
+                out.push('\'');
+                out.push_str(k);
+                out.push('\'');
+                out.push_str(": ");
+                write_python_repr(out, v);
+            }
+            out.push('}');
+        }
+    }
+}
+
+/// Check if a JSON value matches any query anchors.
+///
+/// Direct port of `item_matches_anchors` (Python `smart_crusher.py:152-168`).
+/// Python uses `str(item).lower()` which produces Python's repr-like
+/// representation. We mirror that via `python_repr` rather than
+/// `serde_json::to_string` so substring matching has the same surface
+/// as Python (single quotes, `True`/`False`/`None`, spaced commas/colons).
 pub fn item_matches_anchors(item: &Value, anchors: &HashSet<String>) -> bool {
     if anchors.is_empty() {
         return false;
     }
 
-    // Python: `str(item).lower()`. We approximate via JSON serialization
-    // followed by `.lower()` — see WARNING above for the gap.
-    let item_str = match serde_json::to_string(item) {
-        Ok(s) => s.to_lowercase(),
-        Err(_) => return false,
-    };
-
+    // Python: `str(item).lower()`. `python_repr` produces the same
+    // single-quoted, space-after-colon, `True`/`False`/`None` form
+    // that Python's `str()` does; lowercase normalizes the bool/null
+    // case to match Python's downstream `.lower()` call.
+    let item_str = python_repr(item).to_lowercase();
     anchors.iter().any(|a| item_str.contains(a))
 }
 
@@ -248,4 +322,92 @@ mod tests {
         let anchors: HashSet<String> = ["xyz123".to_string()].into_iter().collect();
         assert!(!item_matches_anchors(&json!({"a": "b"}), &anchors));
     }
+
+    #[test]
+    fn hostname_blocklist_drops_e_g() {
+        // S5 in code review: pin that "e.g" in input doesn't surface as
+        // an anchor. Direct match against the regex confirms "e.g" itself
+        // matches before the blocklist filters it.
+        let anchors = extract_query_anchors("see e.g for example");
+        assert!(!anchors.contains("e.g"));
+        // Sanity: a normal hostname still passes through.
+        let anchors = extract_query_anchors("connect to api.example.com");
+        assert!(anchors.contains("api.example.com"));
+    }
+
+    #[test]
+    fn email_typo_pattern_still_matches_real_emails() {
+        // S4 in code review: the Python `[A-Z|a-z]` typo doesn't break
+        // real email matching — pin that explicitly.
+        let anchors = extract_query_anchors("contact alice@example.com today");
+        assert!(anchors.contains("alice@example.com"));
+        let anchors = extract_query_anchors("ping bob@SUB.EXAMPLE.IO");
+        assert!(anchors.contains("bob@sub.example.io"));
+    }
+
+    // ---------- python_repr (used by item_matches_anchors) ----------
+
+    #[test]
+    fn python_repr_matches_python_str_for_dict() {
+        // Python: `str({'name': 'Alice', 'ok': True, 'count': 5, 'val': None})`
+        // = `"{'name': 'Alice', 'ok': True, 'count': 5, 'val': None}"`
+        // (insertion order — Python's dict preserves it since 3.7).
+        //
+        // Workspace `Cargo.toml` enables serde_json's `preserve_order`
+        // feature, so `json!` macro and `serde_json::from_str` both
+        // preserve key insertion order. Without that feature the test
+        // below would fail.
+        let v = json!({"name": "Alice", "ok": true, "count": 5, "val": null});
+        let r = python_repr(&v);
+        assert_eq!(
+            r,
+            "{'name': 'Alice', 'ok': True, 'count': 5, 'val': None}"
+        );
+    }
+
+    #[test]
+    fn python_repr_list_uses_space_after_comma() {
+        // Python: `str([1, 2, 'abc', True])` = `"[1, 2, 'abc', True]"`.
+        let v = json!([1, 2, "abc", true]);
+        assert_eq!(python_repr(&v), "[1, 2, 'abc', True]");
+    }
+
+    #[test]
+    fn python_repr_nested() {
+        let v = json!({"a": [1, {"b": "c"}]});
+        assert_eq!(python_repr(&v), "{'a': [1, {'b': 'c'}]}");
+    }
+
+    #[test]
+    fn item_matches_anchor_with_python_none_form() {
+        // I3 fix in review: Python `str({'val': None}).lower()` produces
+        // `{'val': none}`. With the old JSON-based matcher, the same
+        // input would serialize as `{"val":null}` and an anchor "none"
+        // would never match. With `python_repr` the serialization is
+        // `{'val': None}` → lowercased to `{'val': none}` → contains "none".
+        let anchors: HashSet<String> = ["none".to_string()].into_iter().collect();
+        assert!(item_matches_anchors(&json!({"val": null}), &anchors));
+    }
+
+    #[test]
+    fn item_matches_anchor_avoids_json_null_token() {
+        // Inverse of the above: an anchor "null" must NOT match a Python-
+        // null repr (which writes `none`). Pre-fix code would erroneously
+        // match because of `serde_json::to_string`'s `null` literal.
+        let anchors: HashSet<String> = ["null".to_string()].into_iter().collect();
+        assert!(!item_matches_anchors(&json!({"val": null}), &anchors));
+    }
+
+    #[test]
+    fn python_repr_string_with_single_quote_drift() {
+        // Documented parity gap: Python's `repr` switches to double
+        // quotes if the string contains a single quote. We always use
+        // single quotes. Pin the gap so future changes are intentional.
+        let v = json!({"k": "it's fine"});
+        // Our output: `{'k': 'it's fine'}` (broken Python repr — Python
+        // would emit `{'k': "it's fine"}`).
+        assert_eq!(python_repr(&v), "{'k': 'it's fine'}");
+        // Substring matching for typical anchors still works because
+        // they don't reference the quote chars themselves.
+    }
 }

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

@@ -173,10 +173,21 @@ mod tests {
 
     #[test]
     fn bool_with_number_is_mixed_not_bool_or_number() {
-        // Python's `[True, False, 1]` matches `types <= {bool, int}` BUT
-        // fails `all(isinstance(i, bool))`, so falls through to NUMBER_ARRAY
-        // check which has `not has_bool` — fails. Then nested check fails.
-        // Final: MIXED_ARRAY. Same here.
+        // Python's `[True, False, 1]` walks like this:
+        //   types == {bool, int} (because bool is an int subclass)
+        //   has_bool = True
+        //   `types <= {bool, int}` is True, so the bool-array gate is
+        //   considered, but the inner `all(isinstance(i, bool))` check
+        //   is False (because of the `1`), so does NOT return BOOL_ARRAY.
+        //   `types == {dict}` False. `types == {str}` False.
+        //   `types <= {int, float} and not has_bool` — has_bool is True,
+        //   so the number-array gate fails too. `types == {list}` False.
+        //   Falls through to MIXED_ARRAY.
+        //
+        // Rust matches by side effect of separate `Bool`/`Number` enum
+        // variants: the bool-array gate fails because `has_number` is
+        // True; the number-array gate fails because `has_bool` is True.
+        // Final: MIXED_ARRAY. Same outcome via different code path.
         let items = vec![json!(true), json!(false), json!(1)];
         assert_eq!(classify_array(&items), ArrayType::MixedArray);
     }

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

@@ -1,23 +1,35 @@
 //! Field-name hashing for cache keys.
 //!
-//! Direct port of `_hash_field_name` (Python `smart_crusher.py:171-176`).
-//! Used to generate stable cache keys for compression hints — must match
-//! Python byte-for-byte or cache lookups will miss.
+//! Direct port of `_hash_field_name` (Python `smart_crusher.py:171-177`).
+//! Used to look up TOIN-anonymized `preserve_fields` — TOIN stores
+//! field names as **SHA-256[:8]** for privacy (per Python doc-comment
+//! at `smart_crusher.py:174-175`), so cache lookups will silently miss
+//! if the truncation length drifts.
+//!
+//! # 16 vs 8 — got it wrong once, now pinned
+//!
+//! The first version of this file used `[:16]` based on a misread of
+//! the Python source. Code review caught the discrepancy: Python uses
+//! `[:8]`. Cache lookups against TOIN's 8-char hashes would have
+//! silently missed every field, defeating the entire `use_feedback_hints`
+//! path. Fixed here; the tests now pin against Python `[:8]` reference
+//! values verified via `python3 -c "...hexdigest()[:8]"`.
 
 use sha2::{Digest, Sha256};
 
-/// SHA-256 of the UTF-8 bytes, hex-encoded, truncated to 16 chars.
+/// SHA-256 of the UTF-8 bytes, hex-encoded, truncated to **8** chars.
 ///
-/// Python equivalent: `hashlib.sha256(field_name.encode()).hexdigest()[:16]`.
-/// We use lowercase hex (the default for both Python and Rust's `sha2`
-/// crate) — the test below pins this.
+/// Python equivalent: `hashlib.sha256(field_name.encode()).hexdigest()[:8]`.
+/// Lowercase hex — both Python `hexdigest()` and Rust's `sha2` default
+/// to lowercase, so this is consistent without manual case-coercion.
 pub fn hash_field_name(field_name: &str) -> String {
     let mut hasher = Sha256::new();
     hasher.update(field_name.as_bytes());
     let digest = hasher.finalize();
-    // Convert to lowercase hex, then truncate to first 16 chars (8 bytes).
+    // Truncate to first 8 hex chars (4 bytes of digest). MUST match
+    // Python's `[:8]` — see module-level note above.
     let hex = format!("{:x}", digest);
-    hex[..16].to_string()
+    hex[..8].to_string()
 }
 
 #[cfg(test)]
@@ -25,22 +37,22 @@ mod tests {
     use super::*;
 
     #[test]
-    fn matches_python_sha256_truncated_to_16() {
-        // Verified against Python: hashlib.sha256(b"customer_id").hexdigest()[:16]
-        assert_eq!(hash_field_name("customer_id"), "1e38d67dbe8f47d2");
+    fn matches_python_sha256_truncated_to_8() {
+        // Verified against Python: hashlib.sha256(b"customer_id").hexdigest()[:8]
+        assert_eq!(hash_field_name("customer_id"), "1e38d67d");
     }
 
     #[test]
     fn empty_string() {
-        // Verified against Python: hashlib.sha256(b"").hexdigest()[:16]
-        assert_eq!(hash_field_name(""), "e3b0c44298fc1c14");
+        // Verified against Python: hashlib.sha256(b"").hexdigest()[:8]
+        assert_eq!(hash_field_name(""), "e3b0c442");
     }
 
     #[test]
     fn unicode_field_name() {
-        // Verified against Python: hashlib.sha256("café".encode()).hexdigest()[:16]
+        // Verified against Python: hashlib.sha256("café".encode()).hexdigest()[:8]
         // UTF-8 bytes for "café" are 63 61 66 c3 a9 — must encode same way.
-        assert_eq!(hash_field_name("café"), "850f7dc43910ff89");
+        assert_eq!(hash_field_name("café"), "850f7dc4");
     }
 
     #[test]
@@ -50,9 +62,11 @@ mod tests {
     }
 
     #[test]
-    fn output_length_is_16() {
-        // Always exactly 16 hex chars regardless of input length.
-        assert_eq!(hash_field_name("a").len(), 16);
-        assert_eq!(hash_field_name(&"x".repeat(1000)).len(), 16);
+    fn output_length_is_8() {
+        // Always exactly 8 hex chars regardless of input length.
+        // This must match Python's `[:8]`; if you change it, every TOIN
+        // preserve-field lookup silently misses.
+        assert_eq!(hash_field_name("a").len(), 8);
+        assert_eq!(hash_field_name(&"x".repeat(1000)).len(), 8);
     }
 }

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

@@ -82,6 +82,53 @@ pub fn calculate_string_entropy(s: &str) -> f64 {
     }
 }
 
+/// Parse a string the way Python's built-in `int()` does for plain
+/// integer literals. Used by `detect_sequential_pattern` to mirror
+/// `int(v)` behavior exactly.
+///
+/// Python's `int()` accepts:
+///   - leading/trailing ASCII whitespace (stripped)
+///   - leading sign (`+` or `-`)
+///   - PEP 515 underscore digit separators (e.g. `"3_000"` → `3000`)
+///
+/// Rust's `str::parse::<i64>()` rejects all of those. If we used the
+/// raw `parse`, real-world payloads with `"  5  "` or `"+5"` would
+/// silently disagree with Python on whether the field is "numeric",
+/// which changes sequential classification and breaks fixtures.
+///
+/// We deliberately do NOT support Python's other `int()` features
+/// (base prefixes like `"0x10"`, scientific notation via `int(float(s))`,
+/// etc.) because the Python `_detect_sequential_pattern` call site
+/// uses the default-base `int()` overload — those paths are
+/// unreachable.
+fn python_int_parse(s: &str) -> Option<i64> {
+    // Python: `int()` strips ASCII whitespace from both ends.
+    let trimmed = s.trim();
+    if trimmed.is_empty() {
+        return None;
+    }
+    // Python: drop PEP 515 underscores between digits. The implementation
+    // is more careful than this (rejects leading/trailing/double underscores),
+    // but for our use-case any string with valid digits + underscore separators
+    // is what we want to accept. Edge cases like `"_5_"` will fail the
+    // i64::parse call below, matching Python's behavior of rejecting them.
+    let cleaned: String = if trimmed.contains('_') {
+        // Reject patterns Python rejects: leading/trailing underscore,
+        // double underscores. Otherwise strip them out.
+        let bytes = trimmed.as_bytes();
+        let starts_or_ends = bytes[0] == b'_'
+            || *bytes.last().unwrap() == b'_'
+            || trimmed.contains("__");
+        if starts_or_ends {
+            return None;
+        }
+        trimmed.replace('_', "")
+    } else {
+        trimmed.to_string()
+    };
+    cleaned.parse::<i64>().ok()
+}
+
 /// Detect if numeric values form a sequential pattern (like IDs:
 /// 1, 2, 3, ...).
 ///
@@ -136,9 +183,10 @@ pub fn detect_sequential_pattern(values: &[Value], check_order: bool) -> bool {
             }
             Value::String(s) => {
                 // Python: `try: nums.append(int(v))`. `int("3.14")` raises
-                // in Python, so we mirror by trying integer parse first and
-                // only succeeding for pure-integer strings.
-                if let Ok(parsed) = s.parse::<i64>() {
+                // and Rust's plain `parse::<i64>` differs from `int()` on
+                // edges like leading whitespace and PEP 515 underscores.
+                // `python_int_parse` mirrors Python exactly — see fn doc.
+                if let Some(parsed) = python_int_parse(s) {
                     nums.push(parsed as f64);
                     // BUG #2 fix: do NOT set had_non_string_numeric.
                     // If we later find this is the ONLY source of numeric
@@ -372,4 +420,111 @@ mod tests {
         let v: Vec<Value> = (1..=10).map(|i| json!(i as f64)).collect();
         assert!(detect_sequential_pattern(&v, true));
     }
+
+    #[test]
+    fn sequential_fractional_unit_step() {
+        // Floats with non-integer values but constant unit step. avg_diff
+        // = 1.0, all diffs in [0.5, 2.0], should be sequential. (Suggestion
+        // S6 in code review — pins float arithmetic doesn't drift.)
+        let v: Vec<Value> = vec![
+            json!(1.5),
+            json!(2.5),
+            json!(3.5),
+            json!(4.5),
+            json!(5.5),
+        ];
+        assert!(detect_sequential_pattern(&v, true));
+    }
+
+    #[test]
+    fn bug2_all_unparseable_strings_returns_false() {
+        // S3 in code review: explicit test for the all-strings case where
+        // none parse. Falls out of `nums.len() < 5` already, but pinning
+        // the behavior protects against future refactors.
+        let v: Vec<Value> = vec![
+            json!("abc"),
+            json!("def"),
+            json!("ghi"),
+            json!("jkl"),
+            json!("mno"),
+        ];
+        assert!(!detect_sequential_pattern(&v, true));
+    }
+
+    #[test]
+    fn bug2_single_int_among_strings_still_detects() {
+        // S3 in code review: validates that the BUG #2 gate fires on
+        // "ANY non-string numeric", not "majority". One real int among
+        // string-encoded numerics should be enough to count as sequential.
+        let v: Vec<Value> = vec![
+            json!("001"),
+            json!("002"),
+            json!(3), // <-- the unambiguous numeric
+            json!("004"),
+            json!("005"),
+            json!("006"),
+        ];
+        assert!(detect_sequential_pattern(&v, true));
+    }
+
+    // ---------- python_int_parse ----------
+
+    #[test]
+    fn python_int_parse_basic() {
+        assert_eq!(python_int_parse("5"), Some(5));
+        assert_eq!(python_int_parse("-5"), Some(-5));
+        assert_eq!(python_int_parse("+5"), Some(5));
+    }
+
+    #[test]
+    fn python_int_parse_strips_whitespace() {
+        // Python: `int("  5  ") == 5`. Rust's plain parse fails on this.
+        assert_eq!(python_int_parse("  5  "), Some(5));
+        assert_eq!(python_int_parse("\t-3\n"), Some(-3));
+    }
+
+    #[test]
+    fn python_int_parse_underscores() {
+        // PEP 515 — Python: `int("3_000") == 3000`.
+        assert_eq!(python_int_parse("3_000"), Some(3000));
+        assert_eq!(python_int_parse("1_000_000"), Some(1_000_000));
+    }
+
+    #[test]
+    fn python_int_parse_underscore_edge_cases_rejected() {
+        // Python rejects these (raises ValueError); we mirror by
+        // returning None.
+        assert_eq!(python_int_parse("_5"), None);
+        assert_eq!(python_int_parse("5_"), None);
+        assert_eq!(python_int_parse("3__000"), None);
+    }
+
+    #[test]
+    fn python_int_parse_rejects_floats() {
+        // Python: `int("3.14")` raises. Mirror by returning None.
+        assert_eq!(python_int_parse("3.14"), None);
+    }
+
+    #[test]
+    fn python_int_parse_rejects_non_numeric() {
+        assert_eq!(python_int_parse("abc"), None);
+        assert_eq!(python_int_parse(""), None);
+        assert_eq!(python_int_parse("   "), None);
+    }
+
+    #[test]
+    fn sequential_with_whitespace_padded_strings_via_python_int_parse() {
+        // I1 fix in code review: real fixtures may carry whitespace-padded
+        // numeric strings. With the python_int_parse helper, mixed real-int
+        // + whitespace-padded-string fields still detect correctly.
+        let v: Vec<Value> = vec![
+            json!(1),
+            json!("  2  "),
+            json!(3),
+            json!(" 4 "),
+            json!(5),
+            json!(6),
+        ];
+        assert!(detect_sequential_pattern(&v, true));
+    }
 }

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

@@ -123,10 +123,29 @@ impl CrushabilityAnalysis {
 /// Complete analysis of an array.
 ///
 /// Mirrors `ArrayAnalysis` at `smart_crusher.py:887-897`. `field_stats`
-/// uses `BTreeMap` for deterministic iteration order (Python's `dict`
-/// preserves insertion order; `BTreeMap` gives us a stable sorted-by-key
-/// order, which is fine for the parity fixtures because the analyzer
-/// builds the map by iterating sorted keys).
+/// and `constant_fields` use `BTreeMap` for sorted-by-key iteration.
+///
+/// # Sort vs insertion order — known parity nuance
+///
+/// Python's `dict` preserves insertion order, and `_analyze_field` is
+/// called once per key as it appears in `items[0].keys()` (i.e., JSON
+/// parse order). With `serde_json/preserve_order` enabled at the
+/// workspace level, `serde_json::Map` is an `IndexMap` and parse order
+/// matches Python.
+///
+/// `BTreeMap` here gives sorted-key iteration — which differs from
+/// Python's parse-order `dict`. This matters only if downstream code
+/// observes the iteration order of `field_stats` (e.g., when emitting
+/// debug output, picking a "first" field, or computing strategy
+/// strings that include field names).
+///
+/// During the analyzer port (Stage 3c.1 commit 2), we'll either:
+///   1. Switch this to `IndexMap` if any code path observes order, OR
+///   2. Document that Python's order-sensitive paths get rewritten to
+///      iterate sorted, then mirror that in Rust.
+///
+/// Tracked in the design doc at
+/// `~/Desktop/SmartCrusher-Architecture-Improvements.md`.
 #[derive(Debug, Clone)]
 pub struct ArrayAnalysis {
     pub item_count: usize,

plugins/headroom-agent-hooks/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "headroom",
-  "version": "0.10.17",
+  "version": "0.11.0",
   "description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
   "author": {
     "name": "Headroom Contributors",

plugins/headroom-agent-hooks/.github/plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "headroom",
-  "version": "0.10.17",
+  "version": "0.11.0",
   "description": "Headroom startup hooks for Claude Code and GitHub Copilot CLI.",
   "author": {
     "name": "Headroom Contributors",