Strip the whole escape_str when unescaping

[youdie006]

escape_str has three consumers and one of them ignores it.

• _symbol_map is built as escape_str + symbol.label (jsondiff/__init__.py:795)
• _escape prepends self.options.escape_str (:1059, :1061)
• _unescape strips with x[1:] (:1034)

The module's own rule at the top of the file describes the escaping side:

keys and strings which start with $ (or specified escape_str) are escaped to $$ (or
escape_str * 2)

So _escape matches the stated rule and _unescape is the odd one out. It has been that way
since #43 ("Optionally allow different escape_str than '$'", merged 2022-01-24), which threaded
escape_str through _escape and _symbol_map and left the x[1:] behind.

unmarshal therefore does not invert marshal:

{'__a': 1}      -marshal->  {'____a': 1}      -unmarshal->  {'___a': 1}     # lost one underscore
{'____b': 2}    -marshal->  {'______b': 2}    -unmarshal->  {'_____b': 2}

Control, with the default escape_str='$' - correct, because len('$') == 1:

{'$a': 1}   ->  {'$$a': 1}   ->  {'$a': 1}
{'$$b': 2}  ->  {'$$$b': 2}  ->  {'$$b': 2}

End to end through the public API it corrupts a document rather than raising:

a = {'__x': 1}
b = {'__x': 2}
d = diff(a, b, marshal=True, escape_str='__')   # {'____x': 2}
patch(a, d, marshal=True, escape_str='__')      # {'__x': 1, '___x': 2}

The patch writes a mangled key and leaves the original value untouched, so the result is
wrong in two places and nothing reports it.

The change

One line - strip len(escape_str) instead of one character.

No observable change for anyone on the default escape_str='$', since the two expressions
are identical when the escape string is a single character. That is also why the existing suite
never caught it: every test constructs the differ with the default.

Tests

Two, next to the existing test_marshal, one for each half:
test_marshal_multi_char_escape_str is test_marshal with escape_str='__', and
test_patch_multi_char_escape_str is the end-to-end case above.

Reverting only the one-line change:

FAILED tests/test_jsondiff.py::JsonDiffTests::test_marshal_multi_char_escape_str
FAILED tests/test_jsondiff.py::JsonDiffTests::test_patch_multi_char_escape_str
E   AssertionError: {'__x': 2} != {'__x': 1, '___x': 2}

pytest is 28 passed with the change.

---

Disclosure: found and prepared with AI assistance (Claude). Every figure above is from a run on
this branch.

[corytodd]

@fzumstein what's the policy on AI-assisted contributions?

The fix is valid and tests are sufficient. I'm okay merging this from a technical perspective.

[youdie006]

Happy to answer the part I can, and then leave the policy call to you both.

Yes, AI-assisted - it is in the PR body and I put it on every PR I open, so it is consistent rather
than disclosed only where someone asks. Concretely what that means here: I use Claude to find
candidates and draft, and I run and verify everything myself before it goes out. For this one that
was the three-way comparison (_escape / _symbol_map / _unescape), reading PR #43 to confirm
_unescape was the site that got left behind when escape_str was threaded through, and running
the end-to-end patch case to check it corrupts rather than raises.

If the answer is that the project would rather not take AI-assisted contributions, that is a
completely reasonable position and I will not argue it - close it and I will stop opening PRs here.
If it would help instead to have the finding without the patch, I am glad to convert it to an issue
with the repro so someone else can take it.

One thing that might be relevant either way: the bug predates any of this. _unescape has stripped
exactly one character since #43 in 2022, and the reader half has always accepted the escaped form,
so the fix is making two halves of the crate agree rather than changing a design decision.

[corytodd]

I definitely appreciate the fix and have no objection to merging this. I also appreciate your usage of an agent and keeping a human in the loop. This is responsible use of AI as far as I'm concerned.

I need to get buy in from the repo owner on AI though. thanks for your understanding.

[fzumstein]

@fzumstein what's the policy on AI-assisted contributions?

The fix is valid and tests are sufficient. I'm okay merging this from a technical perspective.

Whatever the active maintainer feels like ;) Sure fine by me!

[corytodd]

Awesome, I'll add a note to the readme then.

[corytodd]

Thanks for the contribution, @youdie006 !