Repository navigation
fix(rehydration): restore Anthropic tool calls, and escape values on the JSON path - #33
Conversation
…the JSON path 1.6.2 closed the reversible-redaction gap for OpenAI-shaped tool calls and left it open for Anthropic-native ones, which is half a guarantee. Three gaps, one per path that a restored value can travel. Anthropic non-streaming `tool_use` blocks were never rehydrated. `_rehydrate_json_response` walked `content[]` looking for a `text` key, and a tool_use block has none, so the block was skipped whole and the caller received a placeholder as a tool argument. Its `input` is a decoded object rather than JSON text, so `_rehydrate_decoded_leaves` restores the string leaves in place, bounded by depth, and the serializer escapes them on the way out. Anthropic streamed tool input was never rehydrated. `grep input_json_delta` matched nothing in the tree. Anthropic carries tool arguments as `partial_json` fragments on a content block -- the shape OpenAI carries as `function.arguments` -- so the stream translation now opens the call from `content_block_start` (carrying its id and name once, as an OpenAI client needs), restores each fragment through that block's own window, and drains the window BEFORE `content_block_stop`. That ordering is the Anthropic counterpart of the `finish_reason` rule: the stop event is where the call finishes growing, so a tail held past it is parsed against a truncated document. Anthropic numbers tool blocks in the same space as text blocks, so a reply whose first block is prose puts its tool call at index 1. Emitting that verbatim would open a call at index 1 with no index 0 in front of it, so ordinals are handed out densely from 0 in order of first sight. Restored values on the non-streaming OpenAI path were not escaped into `arguments`. 1.6.2 fixed this for the stream and left the same reply behaving differently depending on whether it was streamed. `json_escaped_vault` is now exported from the streaming module and `_rehydrate_json_response` builds one per response, so both paths agree. Ten new tests. Nine fail against 1.6.2; the tenth is a no-regression guard that a value needing no escaping still round-trips byte-identical, and passes either way on purpose.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| Bounded rather than trusting the payload's own nesting to terminate. | ||
| """ | ||
| if depth > _MAX_TOOL_INPUT_DEPTH: |
There was a problem hiding this comment.
For a valid Anthropic tool input with a string leaf at depth 9 or deeper, this cutoff returns the nested node without visiting that leaf. If the value contains a vault token repeated from the redacted prompt, the caller receives the placeholder instead of the original value and may invoke the tool with an incorrect argument.
… truncating at 8 Two findings from review on the previous commit, both real. A flushed Anthropic tool tail was emitted with a single newline, so it shared the upstream's blank line with the `content_block_stop` that followed it. That is one SSE event carrying two newline-joined JSON documents: no compliant client can parse it, and the restored tail is lost -- the failure this change exists to prevent, reintroduced one layer down in the framing. It now terminates itself. The existing tests could not see it. They read the stream line by line, which is exactly the reading that makes a framing bug invisible, so the new one splits on blank lines the way a client does and asserts every event carries at most one `data:` line and that each payload parses on its own. `_MAX_TOOL_INPUT_DEPTH` was 8, which sits inside the range a legitimate tool input can occupy. Past it the walk returned the node untouched, so a string leaf at depth 9 kept its placeholder and the caller had no way to tell that from a value -- a quiet instance of the bug the walk exists to fix. Raised to 40, matching the ceiling `NonStreamingRehydrator` already applies to decoded payloads, and reaching it now logs. The log names no node content; invariant 4 holds in log records too. Two tests, both verified to fail against the previous commit.
|
Both findings fixed in Malformed SSE event boundary — correct, and worse than a formatting nit: the flush shared the upstream's blank line with the stop event, so the two arrived as one event carrying two newline-joined JSON documents and the restored tail was lost. That is the failure this PR exists to prevent, reintroduced one layer down in the framing. The pre-line now terminates itself. Worth noting why my tests missed it: they read the stream line by line, which is exactly the reading that makes a framing bug invisible. The new test splits on blank lines the way a client does, asserts every event carries at most one Deep tool inputs remain placeholders — also correct. 8 sits inside the range a legitimate tool input can occupy, and past it the walk returned the node untouched, so a leaf at depth 9 kept its placeholder with no signal. Raised to 40, matching the ceiling |
|
Re-reviewed Tip: You can customize Greptile's behavior for this repo with |
|
The remaining finding describes the previous commit.
The residual truth in the finding is that any bound leaves something unrestored below it, and that is intentional here: it stops a crafted reply becoming an unbounded walk, it matches the ceiling |
|
You're right—the previous finding was based on the stale depth-8 implementation. With |
1 participant
Why
1.6.2 (#32) closed the reversible-redaction gap for OpenAI-shaped tool calls and left it open for Anthropic-native ones. That is half a guarantee: an Anthropic tool call still reached the caller carrying
[EMAIL_1], on every request, not as an edge case.This closes all three cells.
1. Anthropic non-streaming
tool_use.inputwas skipped whole_rehydrate_json_responsewalkedcontent[]looking for atextkey. Atool_useblock has none, so it was skipped entirely and the caller invoked its tool with a placeholder.inputis a decoded object, not JSON text, so_rehydrate_decoded_leavesrestores the string leaves in place (depth-bounded) and the serializer escapes them on the way out — no escaping dance needed here, unlike the OpenAIargumentsstring.2. Anthropic streamed tool input was never handled
grep input_json_deltamatched nothing in the tree. Anthropic carries tool arguments aspartial_jsonfragments on a content block — the shape OpenAI carries asfunction.arguments.The stream translation now:
content_block_start, carrying itsidandnameonce, as an OpenAI-shaped client needscontent_block_stop, never afterThat last point is the Anthropic counterpart of the
finish_reasonrule: the stop event is where the call finishes growing, so a tail held past it is parsed against a truncated document.Index translation. Anthropic numbers tool blocks in the same space as text blocks, so a reply whose first block is prose puts its tool call at index 1. Emitting that verbatim would open a tool call at index 1 with no index 0 in front of it, which clients concatenating by index handle badly. Ordinals are handed out densely from 0 in order of first sight.
3. Non-streaming OpenAI
argumentswas not escaped1.6.2 fixed this for the stream and left the same reply behaving differently depending on whether it was streamed — an inconsistency that change introduced.
json_escaped_vaultis now exported from the streaming module, and_rehydrate_json_responsebuilds one per response so both paths agree.Testing
10 new tests. 9 fail against 1.6.2 — verified by reverting
llm_shield_proxy/and re-running with the tests in place. The tenth is a deliberate no-regression guard (a value needing no escaping round-trips byte-identical) and passes either way on purpose.tests/test_response_rehydration.pyis new:_rehydrate_json_responsehad no test coverage at all before this.Full suite: 979 passed. The two conformance failures in that run are pre-existing on clean
main, fail identically without this patch, and pass in isolation — order-dependent flakes, unrelated.ruff checkclean.Still open after this
GenericGuardrailAPIResponsehas no field to return them. Upstream fix proposed in fix(generic_guardrail_api): return tool_calls on the guardrail response BerriAI/litellm#41013.JSON-RPC traffic was never affected — it routes to the stateless v3 engine, whose lexer is field-agnostic.