refactor(loop): unify tool-call dedup signature (canonical_json) [dirge-ark9]

.beads/issues.jsonl

@@ -78,7 +78,7 @@
 {"_type":"issue","id":"dirge-9f1","title":"Chat history ignores 120-col content_width cap","description":"max_line_width and wrap_line use raw content_cols, so on wide terminals scrollback overflows the centered band into divider/panel margin.","status":"closed","priority":1,"issue_type":"bug","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-21T22:17:33Z","created_by":"Yogthos","updated_at":"2026-05-21T22:26:36Z","started_at":"2026-05-21T22:17:42Z","closed_at":"2026-05-21T22:26:36Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-woq","title":"R1: fix 3 critical plugin bugs (FFI panic, dialog deadlock, init hang)","description":"From the plugin subsystem audit: (1) wrap JanetCFunctions in catch_unwind so Rust panics don't unwind across the C-FFI boundary into Janet; (2) cancel send_dialog's reply_rx.recv() on worker shutdown so the worker thread doesn't block forever when the UI exits mid-dialog; (3) add timeout to the init handshake so a worker panic before init_tx.send() doesn't hang the main thread. Also: (4) bounds-assert wrap_string's i32 cast for the unlikely \u003e2GB case, (5) make take_string_slot atomic to close the race window, (6) don't eat unrelated user events in the dialog arm.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-20T14:59:57Z","created_by":"Yogthos","updated_at":"2026-05-20T15:30:28Z","started_at":"2026-05-20T15:00:10Z","closed_at":"2026-05-20T15:30:28Z","dependency_count":0,"dependent_count":1,"comment_count":0}
 {"_type":"issue","id":"dirge-vcsn","title":"Unify the mid-loop interjection layer (5+ inject sites)","description":"Steering, context_depth, verifier, critic, todo-nudge, and reflexion each have their own trigger + channel + integration point, firing at three different places in run.rs (inner poll ~:69, finalize gates :1327-1377, storm guard :1002) with ad-hoc precedence. Notably verifier silently blocks critic via 'if follow_up.is_empty()' so the critic verdict never surfaces on a turn that also has a red build, and MID_TURN_STEER_WRAPPER is applied at two independent sites. Design + build one interjection abstraction with explicit priority and a single wrap-once point. MED — top consolidation target for a smoother loop.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:42Z","created_by":"Yogthos","updated_at":"2026-06-03T17:53:38Z","started_at":"2026-06-03T17:41:09Z","closed_at":"2026-06-03T17:53:38Z","close_reason":"Merged in #366 — poll_finalization_follow_up single precedence authority","dependency_count":0,"dependent_count":0,"comment_count":0}
-{"_type":"issue","id":"dirge-w5iy","title":"Single context-budget authority (estimate+thresholds+action)","description":"Three modules independently decide 'context is filling': compression.rs (pre-send per-result caps + should_compress), context_manager.rs (post-response fold/exit tiers), context_depth.rs (ambient reminder). Pre-send uses a chars/4 ESTIMATE while post-response uses the API's ACTUAL prompt_tokens — two disagreeing numbers — plus a snip-feedback override living only in run.rs that's invisible to the decision engine. Consolidate into one ContextBudget owning the estimate, all tier thresholds, and the chosen action. MED.","status":"open","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:42Z","created_by":"Yogthos","updated_at":"2026-06-03T17:02:42Z","dependency_count":0,"dependent_count":0,"comment_count":0}
+{"_type":"issue","id":"dirge-w5iy","title":"Single context-budget authority (estimate+thresholds+action)","description":"Three modules independently decide 'context is filling': compression.rs (pre-send per-result caps + should_compress), context_manager.rs (post-response fold/exit tiers), context_depth.rs (ambient reminder). Pre-send uses a chars/4 ESTIMATE while post-response uses the API's ACTUAL prompt_tokens — two disagreeing numbers — plus a snip-feedback override living only in run.rs that's invisible to the decision engine. Consolidate into one ContextBudget owning the estimate, all tier thresholds, and the chosen action. MED.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:42Z","created_by":"Yogthos","updated_at":"2026-06-03T18:13:22Z","started_at":"2026-06-03T17:53:39Z","closed_at":"2026-06-03T18:13:22Z","close_reason":"Merged in #367 — canonical budget-ladder doc; concrete dup already resolved (single estimator; 75% in #365); decision/mechanism split preserved","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-p99h","title":"Extract built-in tool collision filter (written 3×)","description":"The same BUILTIN_TOOL_NAMES.contains(name){warn;skip} block is copy-pasted at builder/loop_tools.rs:41, :482, :520 (MCP / plugin / semantic registration). Extract one generic filter_by_builtin_names() and call once per source. MED.","status":"closed","priority":2,"issue_type":"task","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:40Z","created_by":"Yogthos","updated_at":"2026-06-03T17:34:06Z","started_at":"2026-06-03T17:27:12Z","closed_at":"2026-06-03T17:34:06Z","close_reason":"Merged (#363, #364)","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-ftmo","title":"Round A: find_callees_in_range + ext_of + spawn helpers + short_id","description":"#2 hoist the byte-identical parser+query+sort/dedup envelope of find_callees_in_range across 10 semantic adapters into common::run_callee_query(lang,query,src,range) (~280 LOC). #5 tools::ext_of(path) for the 8 .extension().and_then(...).unwrap_or('') sites. #9 spawn.rs private tool_defs_for()/model_name_opt() helpers. #10 short_id(id) for the 9 id.chars().take(8) sites (2 already-dup local fns). All S-effort, near-zero risk, test-pinned.","status":"in_progress","priority":2,"issue_type":"task","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-06-02T21:10:18Z","created_by":"Yogthos","updated_at":"2026-06-02T21:16:49Z","started_at":"2026-06-02T21:16:49Z","dependencies":[{"issue_id":"dirge-ftmo","depends_on_id":"dirge-9l12","type":"blocks","created_at":"2026-06-02T17:10:20Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":1,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-9l12","title":"Codebase-wide duplication consolidation (post-vix audit)","description":"wavescope overview + 4 parallel dup-hunt subagents surfaced genuine consolidation candidates across semantic adapters, UI text processing, tools, and provider. Execute in rounds A-D, each its own PR. Several suspected dups were correctly dismissed (fork-spawn family already consolidated; only one fenced-block parser; rig::Tool boilerplate not abstractable).","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-06-02T21:10:16Z","created_by":"Yogthos","updated_at":"2026-06-02T22:27:10Z","closed_at":"2026-06-02T22:27:10Z","close_reason":"Consolidation epic complete: A find_callees+helpers (#353,#354), B ANSI+perm merges (#355), C now_unix+lock_ignore_poison (#356), D text helpers (#357). Several proposed merges honestly declined after feasibility review (ext_of overcount, transcript unification, head/tail truncators).","dependency_count":0,"dependent_count":4,"comment_count":0}
@@ -280,7 +280,7 @@
 {"_type":"issue","id":"dirge-ny0","title":"Phase 3: right-side info panel (cwd, MCP, LSP, todos, modified files)","description":"Carve right ~32 cols (auto-hide when terminal narrower than ~100). Sources: cwd from env, MCP from McpClientManager.handles, LSP via new public accessor on LspManager, todos from TODO_LIST mutex, modified files via new shared Arc\u003cMutex\u003cIndexSet\u003cPathBuf\u003e\u003e\u003e populated by Write/Edit/ApplyPatch tools. New /panel on|off toggle. Default on when wide enough.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-20T03:40:08Z","created_by":"Yogthos","updated_at":"2026-05-20T04:21:53Z","started_at":"2026-05-20T04:11:20Z","closed_at":"2026-05-20T04:21:53Z","close_reason":"Closed","dependency_count":0,"dependent_count":1,"comment_count":0}
 {"_type":"issue","id":"dirge-r2u","title":"Phase 2A: queue user input while agent is running","description":"Remove 'agent is busy' guard at src/ui/mod.rs:663 and :720 for plain text. Push to VecDeque\u003cString\u003e interjection_queue. Show queue count + dim preview above input. Esc/Ctrl-X drops most recent. Ctrl-C still aborts. Slash commands stay gated to current allow-list. On AgentEvent::Done, drain queue and run as next turn.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-20T03:40:07Z","created_by":"Yogthos","updated_at":"2026-05-20T04:04:50Z","started_at":"2026-05-20T03:58:39Z","closed_at":"2026-05-20T04:04:50Z","close_reason":"Closed","dependency_count":0,"dependent_count":1,"comment_count":0}
 {"_type":"issue","id":"dirge-sxt","title":"Phase 1: soft-wrap input box instead of horizontal scroll","description":"Replace horizontal scroll logic in src/ui/renderer.rs::draw_bottom (lines ~500-660) with display-column wrap. Drop input_scroll_offset; compute visual rows from logical lines wrapped to visible_width; keep MAX_INPUT_VISIBLE_LINES cap with vertical scroll keeping cursor visible. Move or guard the token counter so it doesn't collide with wrapped text. Add a unit test for cursor (logical -\u003e visual) mapping.","status":"closed","priority":2,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-20T03:40:06Z","created_by":"Yogthos","updated_at":"2026-05-20T03:58:35Z","started_at":"2026-05-20T03:52:14Z","closed_at":"2026-05-20T03:58:35Z","close_reason":"Closed","dependency_count":0,"dependent_count":1,"comment_count":0}
-{"_type":"issue","id":"dirge-ark9","title":"Unify JSON signature canonicalization for dedup","description":"storm.rs dedups tool calls via raw serde_json::to_string (insertion order, no normalization) while run.rs uses canonical_json (sorted keys, normalized numbers) and scavenge.rs does no normalization. A 1 vs 1.0 or reordered-keys duplicate can slip past one but not another. Route all three through one canonical signature fn. LOW.","status":"open","priority":3,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:44Z","created_by":"Yogthos","updated_at":"2026-06-03T17:02:44Z","dependency_count":0,"dependent_count":0,"comment_count":0}
+{"_type":"issue","id":"dirge-ark9","title":"Unify JSON signature canonicalization for dedup","description":"storm.rs dedups tool calls via raw serde_json::to_string (insertion order, no normalization) while run.rs uses canonical_json (sorted keys, normalized numbers) and scavenge.rs does no normalization. A 1 vs 1.0 or reordered-keys duplicate can slip past one but not another. Route all three through one canonical signature fn. LOW.","status":"in_progress","priority":3,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:44Z","created_by":"Yogthos","updated_at":"2026-06-03T20:41:54Z","started_at":"2026-06-03T20:41:54Z","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-mrbh","title":"Disambiguate the four 'plan/task/work' concepts","description":"plan_enter/exit (read-only mode lock), phased /plan workflow (agent/plan/* + ActivePlan), write_todo_list (in-session checklist), and task/task_status (background subagents) overlap in mental model. tools/plan.rs already carries a comment warning it's 'distinct from plan-mode, similar name'. No documented answer to 'what happens if in plan-mode AND run /plan'. Disambiguate naming + document interaction; consider deriving the todo list from ActivePlan on kickoff. MED (naming/docs).","status":"open","priority":3,"issue_type":"task","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:43Z","created_by":"Yogthos","updated_at":"2026-06-03T17:02:43Z","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-95gl","title":"De-magic the 0.75 fold threshold (constant vs inline literal)","description":"context_manager.rs:30 defines HISTORY_FOLD_THRESHOLD=0.75; compression.rs:143 should_compress hardcodes 0.75 again. Same number, two modules, no cross-reference — silent drift risk if one is updated. Make should_compress reference the constant. LOW.","status":"closed","priority":3,"issue_type":"task","owner":"yogthos@gmail.com","created_at":"2026-06-03T17:02:41Z","created_by":"Yogthos","updated_at":"2026-06-03T17:39:56Z","started_at":"2026-06-03T17:34:08Z","closed_at":"2026-06-03T17:39:56Z","close_reason":"Merged in #365 — should_compress shares HISTORY_FOLD_THRESHOLD","dependency_count":0,"dependent_count":0,"comment_count":0}
 {"_type":"issue","id":"dirge-wk7m","title":"DECSET-2026 sync-update brackets go to redirected fd 1 (dead anti-flicker)","description":"renderer.rs ~696 executes BeginSynchronizedUpdate/EndSynchronizedUpdate on std::io::stdout() (fd 1, dup2'd to the log for the TUI), and gates on IsTerminal(stdout). So the ?2026 begin/end never wrap the actual terminal.draw (which writes to a separate /dev/tty fd) and the per-frame flicker mitigation doesn't run. Fix: write the begin/end through the same /dev/tty writer ratatui uses; base the sync decision on tty availability.","status":"open","priority":3,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-06-03T14:47:00Z","created_by":"Yogthos","updated_at":"2026-06-03T14:47:00Z","dependency_count":0,"dependent_count":0,"comment_count":0}

src/agent/agent_loop/message.rs

@@ -312,6 +312,62 @@ pub fn loop_message_to_value(msg: &LoopMessage) -> Value {
     }
 }
 
+/// Canonical string form of a JSON value, for stable tool-call dedup
+/// signatures. Sorts object keys and normalizes integers-stored-as-floats
+/// (`1.0` ≡ `1`) so two encodings of the same logical call hash equal,
+/// regardless of key order or numeric representation.
+///
+/// Single source of truth for tool-call signatures — used by the scavenge
+/// dedup (run.rs) and the storm repeat-loop detector (storm.rs), which
+/// previously diverged: storm relied on `serde_json::to_string` (sorted only
+/// while `serde_json`'s `preserve_order` feature stays off, and not normalizing
+/// `1` vs `1.0`) [dirge-ark9].
+pub fn canonical_json(v: &Value) -> String {
+    match v {
+        Value::Object(m) => {
+            let mut keys: Vec<&String> = m.keys().collect();
+            keys.sort();
+            let mut s = String::from("{");
+            for (i, k) in keys.iter().enumerate() {
+                if i > 0 {
+                    s.push(',');
+                }
+                s.push_str(&serde_json::to_string(k).unwrap_or_default());
+                s.push(':');
+                s.push_str(&canonical_json(&m[*k]));
+            }
+            s.push('}');
+            s
+        }
+        Value::Array(a) => {
+            let mut s = String::from("[");
+            for (i, e) in a.iter().enumerate() {
+                if i > 0 {
+                    s.push(',');
+                }
+                s.push_str(&canonical_json(e));
+            }
+            s.push(']');
+            s
+        }
+        Value::Number(n) => {
+            // Normalize integers-stored-as-floats (`1.0` ≡ `1`) so reps match.
+            if let Some(i) = n.as_i64() {
+                i.to_string()
+            } else if let Some(f) = n.as_f64() {
+                if f.fract() == 0.0 && f.is_finite() {
+                    (f as i64).to_string()
+                } else {
+                    f.to_string()
+                }
+            } else {
+                n.to_string()
+            }
+        }
+        other => serde_json::to_string(other).unwrap_or_default(),
+    }
+}
+
 /// Pi's `AgentEvent` is plain JSON-serializable in TypeScript;
 /// here we keep `LoopEvent` as a Rust-only enum. The fields hold
 /// in-memory `AssistantMessage` instances, not their JSON form,
@@ -578,6 +634,31 @@ mod tests {
         );
     }
 
+    /// `canonical_json` produces the same signature regardless of object key
+    /// order and numeric representation — the property storm + scavenge dedup
+    /// both rely on (dirge-ark9).
+    #[test]
+    fn canonical_json_is_order_and_number_stable() {
+        // Key order doesn't matter.
+        let a = serde_json::json!({"a": 1, "b": 2});
+        let b = serde_json::json!({"b": 2, "a": 1});
+        assert_eq!(canonical_json(&a), canonical_json(&b));
+        assert_eq!(canonical_json(&a), "{\"a\":1,\"b\":2}");
+
+        // `1` and `1.0` collapse to the same signature (the storm/run
+        // divergence this unifies).
+        let int = serde_json::json!({"limit": 1});
+        let float = serde_json::json!({"limit": 1.0});
+        assert_eq!(canonical_json(&int), canonical_json(&float));
+
+        // Genuine fractionals are preserved; nested objects/arrays recurse.
+        let nested = serde_json::json!({"z": [{"y": 2.5, "x": 1}], "a": "s"});
+        assert_eq!(
+            canonical_json(&nested),
+            "{\"a\":\"s\",\"z\":[{\"x\":1,\"y\":2.5}]}"
+        );
+    }
+
     /// `StopReason` round-trips at pi's exact wire format.
     /// `ToolUse` is camelCase (one word in wire form). Caught
     /// here so a future enum reshuffle can't break ACP / external