Skip to content

fix(ambient): handle string or u32 for Claude tool parameters (#133)#175

Merged
quangdang46 merged 1 commit into
masterfrom
fix/ambient-serde-string-or-u32
May 22, 2026
Merged

fix(ambient): handle string or u32 for Claude tool parameters (#133)#175
quangdang46 merged 1 commit into
masterfrom
fix/ambient-serde-string-or-u32

Conversation

@quangdang46
Copy link
Copy Markdown
Owner

What

Every ambient cycle (end_ambient_cycle, schedule_ambient,
schedule_tool, and next_schedule.wake_in_minutes payloads) was
failing at the JSON deserialization layer with:

invalid type: string "0", expected u32

Claude tool calls occasionally serialize numeric arguments as strings
(e.g. {"compactions": "0"}) rather than JSON numbers. Our u32
fields rejected those calls before any logic ran, breaking ambient
mode entirely.

This addresses issue #133: #133

Changes

  • src/tool/ambient.rs:
    • Imported Deserializer from serde.
    • Added two custom serde visitor helpers:
      • deserialize_string_or_u32 — accepts JSON number or numeric
        string, returns u32.
      • deserialize_string_or_option_u32 — same but returns
        Option<u32> and preserves missing/null.
    • Wired #[serde(deserialize_with = ...)] on:
      • EndCycleInput::memories_modified
      • EndCycleInput::compactions
      • NextScheduleInput::wake_in_minutes
      • ScheduleInput::wake_in_minutes
      • ScheduleToolInput::wake_in_minutes
  • src/tool/ambient/tests.rs: 5 new regression tests:
    • test_end_cycle_input_accepts_stringified_u32_fields
    • test_end_cycle_input_still_accepts_native_u32_fields
    • test_schedule_input_accepts_stringified_wake_in_minutes
    • test_schedule_tool_input_accepts_stringified_wake_in_minutes
    • test_end_cycle_input_rejects_non_numeric_string (defensive: a
      non-numeric string must still fail loudly, not silently coerce
      to 0)

Tests

$ cargo test -p jcode --lib tool::ambient::tests::
test result: ok. 27 passed; 0 failed; 0 ignored

All pre-existing ambient deserialization tests still pass (numbers
flow through the new visit_u64 branch unchanged).

Acceptance criteria

  • Claude tool calls with {"compactions": "0"} style payloads no
    longer fail at deserialization — src/tool/ambient.rs:122-204
  • All four affected structs (EndCycleInput, NextScheduleInput,
    ScheduleInput, ScheduleToolInput) accept both forms
  • Existing JSON-number form still works — covered by
    test_end_cycle_input_still_accepts_native_u32_fields
  • Non-numeric strings still rejected — covered by
    test_end_cycle_input_rejects_non_numeric_string

Notes

Claude tool calls occasionally serialize numeric arguments as strings
(e.g. {"compactions": "0"}) instead of JSON numbers. Strict u32
deserialization rejected those calls with:
    invalid type: string "0", expected u32
which made every ambient cycle (end_ambient_cycle, schedule_ambient,
schedule_tool, next_schedule.wake_in_minutes) fail at the JSON layer
before it ever reached business logic.

Add two custom serde visitor helpers in src/tool/ambient.rs:
- deserialize_string_or_u32         — for required u32 fields
- deserialize_string_or_option_u32  — for Option<u32> fields
Both visitors accept a JSON number OR a numeric string and forward to
the appropriate parser. Non-numeric strings still fail loudly.

Apply via #[serde(deserialize_with = ...)] to:
- EndCycleInput::memories_modified
- EndCycleInput::compactions
- NextScheduleInput::wake_in_minutes
- ScheduleInput::wake_in_minutes
- ScheduleToolInput::wake_in_minutes

Add 5 regression tests in src/tool/ambient/tests.rs covering both the
new stringified form and the existing native-number form, plus a
defensive test that rejects non-numeric strings.

Ports the src/tool/ambient.rs portion of upstream PR
1jehuang#173 by @zanni098. The
docs/WINDOWS_SETUP.md hunk that upstream PR also carries belongs to a
different feature (upstream PR #172 / our issue #132) and is skipped
here.

Closes #133
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant