feat: Add full deterministic support by default

[gpeacock]

Sort map and struct keys deterministically per RFC 8949 §4.2.1

Map, struct, and struct-variant entries are now buffered and written
in bytewise-lexicographic order of their encoded key bytes by default,
as C2PA requires. Arrays keep their existing zero-copy fast path since
element order is never reordered.

Adds Encoder::set_deterministic(false) and to_vec_unordered/
to_writer_unordered to opt out and restore the original unsorted,
unbuffered behavior for callers who need it.

Fix duplicate map keys and Value::Tag encoding for RFC 8949 compliance

Reject duplicate map/struct keys in deterministic mode, since RFC 8949
§4.2.1 forbids them (only enforced when deterministic; unordered mode
is unaffected).

Fix Value::Tag, which previously serialized its inner value and
silently dropped the tag. Replace Tagged's hardcoded whitelist of
~30 tag numbers with a general mechanism (thread-local tag slot + a
single sentinel marker name) that works for any u64 tag, matching the
approach serde_cbor uses. Value::Tag now emits real CBOR tag bytes on
encode, and a new opt-in Decoder::with_capture_tags / Value::
from_tagged_slice reconstructs Value::Tag (including nested tags) on
decode, without changing the default from_slice behavior of
transparently skipping tags for plain types.

[codecov]

Codecov Report

❌ Patch coverage is 81.25000% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.34%. Comparing base (3fd0162) to head (7c6b2fa).

Files with missing lines	Patch %	Lines
src/encoder.rs	73.56%	23 Missing ⚠️
src/value.rs	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #24      +/-   ##
==========================================
+ Coverage   66.58%   68.34%   +1.76%     
==========================================
  Files           6        6              
  Lines        1721     1763      +42     
==========================================
+ Hits         1146     1205      +59     
+ Misses        575      558      -17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

Merging this PR will degrade performance by 42.9%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 7 regressed benchmarks
✅ 23 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	encode_hashmap_100_entries	21.1 µs	94.1 µs	-77.56%
❌	encode_struct	7.6 µs	15.5 µs	-51.17%
❌	encode_nested_3_levels	139 µs	246.6 µs	-43.63%
❌	encode_options_all_some	8.1 µs	12.1 µs	-32.7%
❌	encode_options_all_none	3.3 µs	4.5 µs	-26.33%
❌	roundtrip_struct	19.7 µs	26.6 µs	-25.69%
❌	encode_with_flatten	13.7 µs	15.8 µs	-13.03%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing gpeacock/deterministic_mode (7c6b2fa) with main (3fd0162)}

[tmathern]

Does the deterministic mode has other effects on encoding?
I am thinking maybe of everything float-related, since those can have rounding errors that may be affected by determinism rules precised in the spec (if anything)?

[tmathern]

Doesn't take the set_deterministic value into account?

[tmathern]

Similar to what is in the function fn end(self) -> Result<()> . To make sure they do the same thing, I would refactor and call them in both locations.

[tmathern]

A specific Error for this here might be more helpful for error matching?

[tmathern]

Can't we import it on top? I prefer all imports to be improted rather than crate::something in the middle of the code. So they are also more easily visible/reusable when the time comes.

[tmathern]

Ok to have things in thread_local here? No side-effects to be weary of when used in c2pa-rs? E.g. do we need extra hygiene around cleaning values up?

[scouten-adobe]

Code Review — Deterministic Encoding + CBOR Tag Support

Overview

Two mostly-independent changes, both sound in their core design:

1. Deterministic encoding by default — maps/structs/struct-variants are buffered and written sorted by encoded-key bytes (RFC 8949 §4.2.1), with set_deterministic(false) / to_vec_unordered / to_writer_unordered opt-outs.
2. Real CBOR tag support — replaces the "__cbor_tag_N__" string whitelist with a thread-local tag slot + single sentinel newtype name, so any u64 tag works; Value::Tag now emits real tag bytes and Value::from_tagged_slice reconstructs tags (including nested).

The sorting logic is correct: Vec<u8>'s Ord is bytewise-lexicographic, matching §4.2.1 exactly (verified by the mixed-sign-integer and "b"/"aa" tests). Determinism propagates correctly into nested maps/arrays. The tag thread-local is used synchronously and handles re-entrancy correctly (decode calls take_tag() before recursing, so nested tags don't clobber). All 231 tests pass.

Reading is unaffected: the decode path is unchanged by default (the new capture_tags branch is gated and defaults off), the decoder is order-agnostic, and the on-wire tag format is identical to before — so CBOR written by older versions remains fully readable.

Findings, roughly by severity:

1. Compatibility — default output bytes change (by design, but breaking)

Encoder::new, to_vec, and to_writer now sort keys by default. Any consumer that byte-compares against previously-generated CBOR, or verifies a signature computed over the old (unsorted) encoding, will see mismatches unless the source data was already sorted. This is the intended C2PA fix, but it's the most impactful change in the PR and warrants explicit release notes / a version bump.

2. Compatibility — duplicate keys now hard-error in the default path

Deterministic mode rejects duplicate keys (most realistically from a #[serde(flatten)] map colliding with a sibling field name). Previously these were silently emitted. Callers who unknowingly relied on that will now get a runtime error by default. Correct per RFC, but a behavior change that can surface as new errors. (Nicely covered by the two duplicate_keys_* tests.)

3. Security — nested tags have no recursion-depth guard (pre-existing, but PR adds a second path)

The MAJOR_TAG branch in decoder.rs never calls check_recursion_depth() / increments recursion_depth, unlike MAJOR_ARRAY/MAJOR_MAP. A crafted chain of tag bytes (e.g. 0xc0 0xc0 0xc0 … then a value) recurses without bound → stack-overflow DoS.

This exists on main too, but the new capture_tags mode adds a second unbounded recursion route. Since the array/map DoS is already guarded and tested, the tag asymmetry is a real gap worth closing while this code is being touched — add a depth check/increment around the tag branch, plus a targeted deep-nesting test.

4. Latent bug — to_value drops tags

ValueSerializer::serialize_newtype_struct ignores name and serializes the inner value transparently, so serializing a Tagged<T> or Value::Tag through to_value silently drops the tag. This also means the to_vec indefinite-length fallback would drop tags if it ever triggers on tagged data. Pre-existing, but more surprising now that Value::Tag encoding is fixed on the direct path — the two paths now disagree. (Impact is low since the fallback appears unreachable — see nits — but the inconsistency is real.)

5. Robustness — thread-local tag state cleanup on panic

Tagged::serialize and Value::serialize both do set_tag(Some) … set_tag(None). Correct for the normal and Err cases, but a panic in the inner serialize_newtype_struct (e.g. from a user Serialize impl) leaves a stale Some(tag) in the thread-local, which a later unrelated marker call could consume. The design also assumes strictly synchronous, single-threaded serialization (fine today, fragile if streaming/async ever wraps it). A scope-guard reset would harden it. Low severity.

Minor / nits

• Stale doc comment: the comment on serialize_struct still warns that it writes the declared count and can't handle skip_serializing_if. In the new default (deterministic) path it buffers and writes the actual count, so that footgun only remains in the opt-out unordered path. (Derived structs pass the correct post-skip len anyway, so real-world corruption risk is limited to hand-written Serialize impls on the unordered path.)
• Likely-dead fallback: the msg.contains("indefinite-length") arm in to_vec_with — I couldn't find any encoder path that still emits that string, so it looks like dead code. Not introduced here; flagging since the area was refactored.
• Perf: deterministic mode copies each subtree's bytes once per nesting level (child → parent → grandparent …), i.e. O(depth × size) extra copying plus a Vec allocation per map/struct. Negligible for C2PA-sized manifests, but a regression vs. the old zero-copy fast path for large/deep inputs.
• Consistency: serialize_to_buffer(&key, …) in the struct-variant path vs. serialize_to_buffer(key, …) in the map path — both work, just stylistically inconsistent. sort_by could also be sort_unstable_by since duplicates are rejected.

Summary

Nothing here blocks the core approach — the sorting and tag mechanisms are correct and well-tested. Items 1–2 are intended-but-breaking and belong in release notes; item 3 is the one I'd most want addressed in-PR since the code is already being modified.