[Bug]: loop_detector::hash_value deep-clones the entire tool-args JSON tree on every tool call (hot path)

[wangmiao0668000666]

Affected component

crates/zeroclaw-runtime/src/agent/loop_detector.rs — hash_value / canonicalise.

Severity

S2 - degraded behavior (per-tool-call transient allocations, RSS-growth amplifier on long tool-heavy turns, especially when args include large embedded strings or deep trees).

Current behavior

hash_value(&serde_json::Value) is called on the args of every successful tool call from crates/zeroclaw-runtime/src/agent/turn/results_collect.rs:85 (loop_detector.record(&tool_name, args, &outcome.output)). The current implementation:

fn hash_value(value: &serde_json::Value) -> u64 {
    let mut hasher = DefaultHasher::new();
    let canonical = serde_json::to_string(&canonicalise(value)).unwrap_or_default();
    canonical.hash(&mut hasher);
    hasher.finish()
}

fn canonicalise(value: &serde_json::Value) -> serde_json::Value {
    match value {
        serde_json::Value::Object(map) => {
            let mut sorted: Vec<(&String, &serde_json::Value)> = map.iter().collect();
            sorted.sort_by_key(|(k, _)| *k);
            let new_map: serde_json::Map<String, serde_json::Value> = sorted
                .into_iter()
                .map(|(k, v)| (k.clone(), canonicalise(v)))
                .collect();
            serde_json::Value::Object(new_map)
        }
        serde_json::Value::Array(arr) => {
            serde_json::Value::Array(arr.iter().map(canonicalise).collect())
        }
        other => other.clone(),
    }
}

Two deep clones of the entire JSON tree per call:

1. canonicalise returns an owned serde_json::Value — every Object key cloned, every nested Object/Array recursively rebuilt, every leaf cloned.
2. serde_json::to_string then serializes that owned tree into a fresh String, allocating ~the same size again.

The intermediate canonical String and the canonicalised Value are both dropped at the end of the function, but on glibc the freed chunks stay parked in the per-thread arena — so the transient allocations add up to monotonic RSS growth on long tool-heavy turns, similar in shape to the MCP-spec cloning issue split out into #8642.

Expected behavior

Produce the same hash (deterministic over object-key ordering) without materialising either the canonicalised Value or the canonical String. Walk the input &Value recursively, sorting object keys on the fly, and feed a deterministic byte sequence directly to a Hasher. The hasher only needs a stable byte stream keyed by structure, so the canonical tree is never needed as a Rust value.

Steps to reproduce

# Reproducible from code inspection on current upstream state.
# 1. A tool whose args include a large embedded string (e.g. shell with a 100 KB command, file_write with a 100 KB payload) is invoked many times in one turn.
# 2. Each call goes through collect_tool_results → loop_detector.record → hash_value(args).
# 3. hash_value allocates (a) an owned canonicalised Value tree and (b) a canonical String of equivalent size.
# 4. Both are dropped on return; under glibc, the freed chunks remain in the per-thread arena.
# 5. After many tool calls, /proc/self/status Rss is materially higher than the live working set suggests.

Impact

• Hot path: called once per successful tool call inside the agent loop.
• Same RSS-growth family as [Bug]: MCP/tool-schema cloning drives unbounded RSS growth in the agent loop (split from #5542) #8642 (MCP tool spec deep-clone) — but the loop detector is always on, not gated by MCP usage, so it is a baseline contributor.
• Hash output is internal to loop detection, so swapping the implementation does not change any observable cross-process behaviour or persisted state, as long as the hash remains deterministic over the same arg tree.

Acceptance criteria

• A drop-in hash_value that produces the same hash for any two serde_json::Value inputs that canonicalise would consider equal (same keys / same leaf values, regardless of object-key order or array order).
• No owned serde_json::Value allocation; no intermediate String allocation proportional to the input.
• All existing tests in loop_detector (detect_exact_repeat, detect_ping_pong, detect_no_progress, progress, and the hashing regression test if any) keep passing.
• A new test pins the streaming hash to the legacy canonicalise + serde_json::to_string hash for a battery of inputs (nested objects, arrays of objects, large strings, numbers, bools, null, mixed).