Repository navigation
fix(repair): recover relaxed-JSON object args and drop null optionals - #59
Conversation
Object-typed properties given a non-strict JSON string now go through the same lenient recovery ladder as whole argument blobs, so captures with trailing commas, bare keys, fences, or leaked template markers still coerce. A null for an optional property whose type rejects null is dropped, since the model meant "not given", while required properties are kept so the validator still names them. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Tiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedNo supported behavioral explanation was produced. Features
Tests
Findings
Resolved this pass
Before mergeNone. How this fits togetherflowchart LR
n0["coerce_to_schema<br/>changed<br/>2 findings"]:::flagged
n1["coerce_value<br/>changed<br/>2 findings"]:::flagged
n2["from_call_object"]:::impacted
n3["coerce_items"]:::impacted
n4["decode"]:::impacted
n5["schema_type"]:::impacted
n6["scalars_are_coerced_to_the_declared_type"]:::impacted
n7["read_call"]:::impacted
n0 -->|calls| n1
n1 -->|calls| n0
n1 -->|calls| n3
n1 -->|calls| n5
n2 -->|calls| n4
n3 -->|calls| n1
n3 -->|calls| n5
n6 -->|calls| n0
n6 -->|tests| n0
n7 -->|calls| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50. Or wait 24 minutes for your next included review. View limit details
Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0056 · 57,255 in / 12,190 out · 5,860 cached (10%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0017 · 14,012 in / 2,206 out · 2,116 cached (15%) · gpt-5.6-luna
security: $0.0024 · 20,716 in / 3,296 out · 3,744 cached (18%) · gpt-5.6-luna
tests: $0.0007 · 12,860 in / 3,340 out · 0 cached (0%) · glm-5.3-flash
description: $0.0004 · 5,379 in / 1,315 out · 0 cached (0%) · glm-5.3-flash
|
|
||
| /// Whether `schema` declares a `type` that does not include `null`. An | ||
| /// untyped property accepts null already, so it is never rewritten. | ||
| fn rejects_null(schema: &Value) -> bool { |
There was a problem hiding this comment.
Honor nullable schemas expressed with composition keywords
This helper only recognizes nullability from the direct type field. A valid schema such as { "anyOf": [{"type":"string"}, {"type":"null"}] } or an equivalent oneOf schema has no direct type, so this helper currently leaves the value intact; however, schemas that combine a non-null direct type with a nullable branch can be incorrectly classified and have explicit nulls dropped. Determine whether the schema accepts null through anyOf, oneOf, enum, or const before removing the property, so normalization does not turn a valid nullable value into an omitted property.
[RULE] schema-nullability ·
| let mut out = Map::with_capacity(map.len()); | ||
| for (key, value) in map { | ||
| let coerced = match properties.get(&key) { | ||
| Some(property) if value.is_null() && !is_required(&key) && rejects_null(property) => { |
There was a problem hiding this comment.
Preserve explicit nulls for schema validation
An explicit null is not equivalent to an omitted optional property: the schema deliberately rejects null for this property, and downstream tools may distinguish a missing argument from an explicit null or apply a default when the key is absent. Dropping attacker- or model-controlled nulls silently turns an invalid tool call into a different, valid call and bypasses the validator's error path. Preserve the value for validation, or introduce an explicit, tool-level policy for treating null as omission rather than applying this transformation generically.
[RULE] schema-validation-bypass ·
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0061 · 96,533 in / 8,534 out · 12,400 cached (13%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0047 · 60,245 in / 4,496 out · 8,464 cached (14%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0012 · 14,102 in / 1,204 out · 3,744 cached (27%) · gpt-5.6-luna
tests: $0.0001 · 6,052 in / 599 out · 64 cached (1%) · glm-5.3-flash
description: $0.0001 · 5,718 in / 457 out · 64 cached (1%) · glm-5.3-flash
| let mut out = Map::with_capacity(map.len()); | ||
| for (key, value) in map { | ||
| let coerced = match properties.get(&key) { | ||
| Some(property) if value.is_null() && !is_required(&key) && rejects_null(property) => { |
There was a problem hiding this comment.
Preserve explicit nulls for schema validation
This drops an explicitly supplied null for every optional property whose simple type rejects null. That changes the caller's input into a different argument object and contradicts the existing contract that values which do not convert are left in place so the schema validator can report them. For example, { "tool": null } with an optional { "tool": { "type": "string" } } becomes {}, allowing the call to proceed as if tool was omitted instead of reporting the invalid explicit null. Keep the value and let schema validation distinguish omission from an explicit null.
[RULE] preserve-input-semantics ·
|
|
||
| /// Whether `schema` declares a `type` that does not include `null`. An | ||
| /// untyped property accepts null already, so it is never rewritten. | ||
| fn rejects_null(schema: &Value) -> bool { |
There was a problem hiding this comment.
Honor nullable schemas expressed with composition keywords
This only detects nullability from the top-level type keyword. An optional property such as { "anyOf": [{ "type": "string" }, { "type": "null" }] } returns false here, so an explicit null is retained instead of being treated consistently with a nullable type array. Extend the nullability check to the supported composition keywords (anyOf/oneOf, and any other schema forms accepted by the validator) so a schema that admits null is not treated as rejecting it.
[RULE] schema-nullability ·
1 participant
Summary
Production Langfuse traces (OpenHuman, Oct 3–10) show
use_skillfailing schema validation in about 20% of its errors. Models send the nestedargsobject as a string that is not strict JSON, or sendnullfor an optional property whose schema doesn't allow null. The argument repair pass only tried strict JSON for a string in an object-typed slot and kept the null, so the call was rejected.coerce_value: for an object-typed property given a string, fall back torecover_object(strip_code_fence(..))when strict parsing fails.coerce_to_schema: dropnullfor an optional property whose declared type has nonull. A null in a required property is left for the validator to report.Tests
an_object_typed_string_that_is_relaxed_json_is_recovered(failed before the fix)null_for_an_optional_non_nullable_property_is_dropped(failed before the fix)null_for_a_required_property_is_keptcargo test -p tinytools-agent: 395/395 pass. clippy-D warningsand fmt are clean.Consumed by tinyhumansai/tinyagents#365 and tinyhumansai/openhuman (tool-call error fixes).