mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
a64716d5d1
Code review (`/code-review` on commit `d219bee`) 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.
44 lines
1.6 KiB
TOML
44 lines
1.6 KiB
TOML
[workspace]
|
|
resolver = "2"
|
|
members = [
|
|
"crates/headroom-core",
|
|
"crates/headroom-proxy",
|
|
"crates/headroom-py",
|
|
"crates/headroom-parity",
|
|
]
|
|
# headroom-py is a Python extension module — it must be built via maturin, not
|
|
# plain cargo (the "extension-module" feature tells pyo3 not to link libpython,
|
|
# which is required for `import` to work). `cargo build --workspace` without
|
|
# explicit members skips it; `cargo test --workspace` still runs its tests
|
|
# because pyo3 can dynamically link here for the cdylib used by tests.
|
|
default-members = [
|
|
"crates/headroom-core",
|
|
"crates/headroom-proxy",
|
|
"crates/headroom-parity",
|
|
]
|
|
|
|
[workspace.package]
|
|
edition = "2021"
|
|
rust-version = "1.80"
|
|
license = "Apache-2.0"
|
|
repository = "https://github.com/chopratejas/headroom"
|
|
authors = ["Headroom Maintainers"]
|
|
|
|
[workspace.dependencies]
|
|
serde = { version = "1", features = ["derive"] }
|
|
# `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"
|
|
anyhow = "1"
|
|
clap = { version = "4", features = ["derive"] }
|
|
tokio = { version = "1", features = ["macros", "rt-multi-thread", "signal"] }
|
|
axum = "0.7"
|
|
tower = "0.5"
|
|
reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }
|
|
pyo3 = "0.22"
|