fix(ambient): handle string or u32 for Claude tool parameters (#133)#175
Open
quangdang46 wants to merge 1 commit into
Open
fix(ambient): handle string or u32 for Claude tool parameters (#133)#175quangdang46 wants to merge 1 commit into
quangdang46 wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Every ambient cycle (
end_ambient_cycle,schedule_ambient,schedule_tool, andnext_schedule.wake_in_minutespayloads) wasfailing at the JSON deserialization layer with:
Claude tool calls occasionally serialize numeric arguments as strings
(e.g.
{"compactions": "0"}) rather than JSON numbers. Ouru32fields rejected those calls before any logic ran, breaking ambient
mode entirely.
This addresses issue #133: #133
Changes
Deserializerfrom serde.deserialize_string_or_u32— accepts JSON number or numericstring, returns
u32.deserialize_string_or_option_u32— same but returnsOption<u32>and preserves missing/null.#[serde(deserialize_with = ...)]on:EndCycleInput::memories_modifiedEndCycleInput::compactionsNextScheduleInput::wake_in_minutesScheduleInput::wake_in_minutesScheduleToolInput::wake_in_minutestest_end_cycle_input_accepts_stringified_u32_fieldstest_end_cycle_input_still_accepts_native_u32_fieldstest_schedule_input_accepts_stringified_wake_in_minutestest_schedule_tool_input_accepts_stringified_wake_in_minutestest_end_cycle_input_rejects_non_numeric_string(defensive: anon-numeric string must still fail loudly, not silently coerce
to 0)
Tests
All pre-existing ambient deserialization tests still pass (numbers
flow through the new
visit_u64branch unchanged).Acceptance criteria
{"compactions": "0"}style payloads nolonger fail at deserialization —
src/tool/ambient.rs:122-204EndCycleInput,NextScheduleInput,ScheduleInput,ScheduleToolInput) accept both formstest_end_cycle_input_still_accepts_native_u32_fieldstest_end_cycle_input_rejects_non_numeric_stringNotes
src/tool/ambient.rsportion of upstream PRFix ambient serde bug: handle string or u32 for Claude tool parameters 1jehuang/jcode#173 by @zanni098.
docs/WINDOWS_SETUP.mdhunk that upstream PR also bundles isunrelated content from upstream PR [Upstream #235] jcode fails to execute in Termux due to missing dynamic linker and LD_PRELOAD conflict #172 (= our issue [Upstream PR #172] Add Windows Setup and Troubleshooting Guide #132) and is
intentionally NOT ported here. That doc will be ported separately
if/when issue [Upstream PR #172] Add Windows Setup and Troubleshooting Guide #132 is picked up.
clippy::question_markwarning incrates/jcode-tui-mermaidis unrelated and reproducible onmaster.