Add annotation provider

[blazingzephyr]

Added TypeAnnotationProvider and TypeAnnotationOptions for additional json metadata not linked to T.

Reasoning:

1. Adding metadata to types from other packages.
2. Support comptime-generated types from @Struct and @Union, where embedding annotations by declaring fields is currently impossible / undesirable behavior.
3. Allow to overwrite default annotations for specific files when needed.

Usage example:

fn ComponentUnion(comptime container: []const u8, comptime specs: anytype) type {
    var field_names: [specs.len][]const u8 = undefined;
    var field_types: [specs.len]type = undefined;
    var field_attrs: [specs.len]FieldAttributes = undefined;
    ...
    const Tag = ComponentEnum(container, specs);
    return @Union(.auto, Tag, &field_names, &field_types, &field_attrs);
}

const _Query: json.TypeAnnotationProvider(Query) = .{
    .json_tag = "$type",
    .json_rename = &.{ .{ .from = "AlwaysMatchesQuery", .to = "AlwaysMatches" } },
};
pub const Query = ComponentUnion("Queries", .{
    .{
        "AdjacentLaneQuery",
        struct {
            Side: Side,
            OriginEntityType: OriginEntityType,
        },
    },
    .{ "AlwaysMatches", struct {} },
    .{ "AttackComparisonQuery", struct { ComparisonOperator: ComparisonOperator, AttackValue: u8 } },
    ...
});
pub const ComponentUnionRegistry = json.TypeAnnotationOptions(.{ _Query, ... });

[blazingzephyr]

Originally intended to open a second pull request for json_payload, but decided to push it into the metadata branch because this also might fix some of the other issues introduced in my previous commits.

[blazingzephyr]

Here's how the annotations currently work for my types.

const _CardDescriptor: json.TypeAnnotationProvider(card_descriptor.CardDescriptor) = .{
    .json_rename = &.{
        .{ .to = "se", .from = "special_abilities" },
        .{ .to = "t", .from = "tags" },
    },
    .json_skip = &[_][]const u8{ "_isGravestone", "_isSurprise" },
};
const _EffectEntityComponent: json.TypeAnnotationProvider(EffectEntityComponent) = .{
    .json_tag = "$type",
    .json_payload = "$data",
};
const _Query: json.TypeAnnotationProvider(Query) = .{
    .json_tag = "$type",
    .json_payload = "$data",
    .json_rename = &.{
        .{
            .from = "PvZCards.Engine.Queries.AlwaysMatchesQuery, EngineLib, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null",
            .to = "PvZCards.Engine.Queries.TestRename, EngineLib, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null",
        },
    },
};
const _Component: json.TypeAnnotationProvider(Component) = .{
    .json_tag = "$type",
    .json_payload = "$data",
};
pub const ComponentUnionRegistry = json.TypeAnnotationOptions(.{ _Component, _EffectEntityComponent, _Query, _CardDescriptor });

[sakakibara]

Thanks for putting this together. This is a useful mechanism. Supporting annotations for external and comptime-generated types fills a real gap, and rejecting invalid entries at comptime is the right approach.

There are a few API and correctness details to address before merging. I left inline comments on source compatibility, annotation precedence, and void json_payload encoding.

Please also:

• Add provider-path tests for json_rename, json_skip, json_flatten, json_tag, json_payload, fromJson, and toJson.
• Cover provider precedence and sorted and unsorted encode/decode round trips.
• Add an Unreleased entry to CHANGELOG.md for the new public API.
• Remove the extra space in the README snippet: cfg, annotations, arena.

[sakakibara]

The annotation registry should not be a new positional parameter. This makes the public typed-codec API source-incompatible even though the feature is opt-in.

Please put it in the existing options structs as a defaulted field, for example annotations: type = DefaultTypes. Existing calls such as parseInto(T, arena, src, .{}) would remain valid, while callers could opt in with .{ .annotations = MyAnnotations }. The lowercase field name also follows the style of dialect and ignore_unknown_fields.

[sakakibara]

The override behavior is inconsistent here. When a provider exists, json_skip = null suppresses the type-local T.json_skip; json_flatten behaves the same way. In contrast, json_rename = null, or a provider rename list without this field, falls back to T.json_rename.

Please define one precedence rule and apply it consistently across every annotation. My recommendation is that each non-null provider property overrides its type-local counterpart, while null falls back to the type-local declaration. Please cover both override and fallback behavior in tests.

[sakakibara]

This emits invalid JSON for a void variant: writeTypedMember writes the payload key and colon, but no value is written when union_field.type == void. The sorted path also attempts to render the void value.

Please define the intended representation for a void payload, handle it consistently in sorted and unsorted encoding, and add a round-trip regression test.

[blazingzephyr]

Alright, thank you for feedback! I will look into this shortly.

[blazingzephyr]

Ah, right, I guess I should have mentioned it in the original pull request. The reason why I opted for a function argument instead of an options struct field is because annotations have to be resolved comptime, which breaks compatibility with existing options.

[sakakibara]

You're right about the comptime constraint. I verified it with a runtime-valued options call: adding a type field makes the entire options struct comptime-only. My earlier options-field suggestion was incorrect; sorry about that.

Please preserve the existing typed-codec entry points and expose custom annotations through a configured codec namespace instead:

const Codec = json.TypedCodec(.{ query_annotations, component_annotations });

const value = try Codec.parseInto(T, arena, src, .{});
try Codec.encode(&writer, value, arena, .{});

TypedCodec(entries) should return a type exposing parseInto, parseIntoReader, decode, and encode; Codec.encode is the configured counterpart of the global encodeTyped, not the dynamic-tree encode. The existing global functions can delegate internally to a private TypedCodec(.{}), so current callers remain source-compatible and runtime-valued ParseOptions and EncodeOptions continue to work.

I also ran the full suite and completed a broader audit. Please treat this as a correction and addendum to my original review.

API naming

• Use Annotations(T) for one type's external annotation record, AnnotationRename for a rename entry, and TypedCodec(entries) for the configured codec namespace.
• Use directional field names such as json_name and zig_name instead of from and to.
• Keep the empty codec private; callers using the existing global functions do not need a public DefaultTypes or default-annotation symbol.

Correctness

• The current head fails zig build test. Sorted tagged-union encoding instantiates the typed encoder for a void payload, including with an empty annotation set.
• External annotation hooks currently do not override type-local hooks, despite the encoder documentation saying they take priority. Null fallback is also inconsistent across rename, skip, flatten, tag, payload, and hooks. A non-null external annotation property should override the corresponding type-local property; null should fall back to it.
• Reject duplicate configured annotations for the same Zig type, ambiguous rename mappings, inapplicable properties, and tag/payload name collisions at comptime. The missing-entry path should also use @typeName(T); it currently produces Zig's expected indexable; found 'type' error instead of the intended diagnostic.
• For json_payload, decode the nested payload value directly and validate the wrapper object separately. The current code removes a legitimate nested payload field when it has the same name as the discriminator, and it silently accepts unknown fields beside the payload in strict mode. I reproduced both cases with focused tests.
• I recommend allowing json_payload to wrap any otherwise-supported payload type and omitting it for void variants. Without json_payload, non-void union payloads should be struct-only and fail with a direct comptime diagnostic otherwise. Payload type errors should report the payload value's kind, not the outer object's kind.

Tests and documentation

• Add focused external-annotation tests for local fallback, external override, duplicate/invalid configurations, hooks, tree/stream equivalence, sorted/unsorted encoding, and struct/scalar/void tagged-union payloads.
• Correct the README and API table signatures, including the required EncodeOptions argument. Document precedence and payload semantics with a small self-contained example, and include every exported annotation type.
• Reconcile the source and generated documentation with the implemented contract: hook-priority claims must match the code, json_payload needs a public doc comment, and implementation-rationale narration should be replaced by a concise current API contract.
• Add the Unreleased changelog entry requested in the original review.
• When updating the branch, rebase it onto the current main so final verification includes the latest test suite.

I will re-review the revised API and run the full suite again once the branch is updated.

[blazingzephyr]

Hello! Just wanted to report that I've just recently come back to working on this and the progress has since been steady, currently finishing the tests and then only need to adjust documentation.

[sakakibara]

Thank you for the revision. TypedCodec / Annotations(T) is the right shape, the global entry points are source-compatible again, and three things from last round are fixed: void json_payload encoding, a nested payload field colliding with the discriminator, and strict-mode unknown fields beside the payload. The changelog entry is there too.

At bf08e4a all gates pass, and a differential corpus against main shows no runtime change for callers not using a codec.

I have audited this fairly closely and found a number of things, but most of them are mine to fix rather than yours, so the list below is short on purpose.

What I would like from you

1. Fix sort_keys with json_payload (src/encoder.zig:300). It emits two concatenated objects today, which is invalid JSON. There is a committable suggestion on the inline comment, which I applied and verified against the full suite. Please add a regression test alongside it, since that branch has no coverage.
2. Decide the precedence API (src/codec.zig:60). I would like precedence moved into TypedCodec construction with the per-call parameter dropped, and four modes reduced to two. This is the only item that becomes published API, so it is the only one that gets meaningfully more expensive after merge. Reasoning is on the inline comment.
3. Add tests for Codec.decode and Codec.parseIntoReader, which have no test uses at all, and extend the tree-versus-stream test to cover a codec-supplied rename rather than only json_skip.
4. Rebase onto current main. The conflicts in CHANGELOG.md and src/decode.zig are mechanical, caused by the upstream commits arriving as a squash rather than a merge.

Once those are in I intend to merge.

What I will handle myself after merge

Several of these are things I asked you for in earlier rounds, including the comptime validations and the precedence rule. I am taking them back. They are small, they sit in code I know well, and none of them touch published API, so none get more expensive by landing after merge. Recorded here so none of it is a surprise later, but please do not spend time on any of it.

• json.Value union payloads (src/decode.zig:624). Last round I asked for non-void payloads to be struct-only without json_payload, and you implemented that correctly. I had not considered json.Value, which decodes on main and no longer compiles. My instruction was too strict, so the fix is mine.
• fromJson hooks returning T (src/decode.zig:332). Taking the hook as a function pointer stops a hook shape that compiles on main from compiling. The lost arity checks on fromJson and toJson belong with it.
• The getJsonTag diagnostic (src/decode.zig:219). orelse unreachable replaced a compile error that named the type. Diagnostic quality only.
• json_skip and json_flatten resolving by union rather than override (src/decode.zig:152), which makes two of the four modes identical for those properties.
• The comptime validation gaps (src/codec.zig:105 and :145): ambiguous renames, inapplicable properties, the missing-entry diagnostic, and type-local or cross-source json_tag / json_payload collisions.
• The json_payload diagnostics (src/decode.zig:680): error precedence, the missing suggestion, and the dotted path.
• The README and API tables, the source documentation, the leftover src/annotation.zig, and the example. I asked for the documentation work in an earlier round and I am withdrawing that request.

Happy to re-review as soon as the four above are done.

[sakakibara]

This emits invalid JSON. The payload goes straight to w while every other member is buffered into members and emitted afterwards, so it lands before the discriminator:

const C = union(enum) {
    pub const json_tag = "kind";
    pub const json_payload = "value";
    version: struct { major: u8, minor: u8 },
    none: void,
};
try json.encodeTyped(&aw.writer, C{ .version = .{ .major = 1, .minor = 0 } }, a, .{ .sort_keys = true });
// {"value":{"major":1,"minor":0}}{"kind":"version"}

This reproduces through plain json.encodeTyped with type-local annotations, and the output fails this library's own parser. Every non-void payload shape is affected; a null optional payload additionally collapses to {}{"kind":"maybe"}, and indent comes out a level off.

Appending the rendered payload as a member fixes all of it, and drops the need for the synthesized @Struct:

Suggested change

	const names = [_][]const u8{json_payload};
	const types = [_]type{union_field.type};
	const atts = [_]std.builtin.Type.StructField.Attributes{.{}};
	const Payload = @Struct(.auto, null, &names, &types, &atts);
	var payload: Payload = undefined;
	@field(payload, json_payload) = @field(value, union_field.name);
	try writeTypedValue(Payload, TCodec, codec_priority, payload, w, arena, options, depth + 1);
	try members.append(arena, .{
	.key = json_payload,
	.json = try renderTypedValue(union_field.type, TCodec, codec_priority, @field(value, union_field.name), arena, options, depth + 1),
	});

I applied exactly that and ran it: zig build test and zig fmt --check both pass, and sorted output becomes {"kind":"version","value":{"major":1,"minor":0}}, {"kind":"maybe","value":null}, and {"kind":"none"}, all of which re-decode.

This got through because the one sort_keys union test at src/encoder.zig:1320 uses a union without json_payload, leaving this branch uncovered. Please add a sorted-encoding test for json_payload on both the type-local and codec paths.

This is the same sorted-path gap I raised in the first round, when I asked for the void payload to be handled consistently in sorted and unsorted encoding. Void is fixed; the non-void sorted path still needs it. Everything else I found is on the review body, and it is all mine to fix.

[sakakibara]

This is the one design decision I would like settled before merge, since it is the only part that becomes published API.

Proposal: move precedence into TypedCodec construction, drop the per-call parameter, and reduce these four modes to two.

const Codec = json.TypedCodec(.{ query_annotations, component_annotations }, .codec_then_local);

const value = try Codec.parseInto(T, arena, src, .{});
try Codec.encode(&writer, value, arena, .{});

Two reasons are defects rather than taste.

Nothing binds decode and encode to the same mode. Your Euclidean tests differ only in name, mode, and expectation, giving 503 under .codec_then_local against 53 under .local_then_codec, so encoding with one and decoding with the other turns 5, 3 into 50, 3 with no error.

And the mode applies to the whole type graph rather than the annotated root, so .codec_only silently discards a nested type's annotations, including json_skip:

const Inner = struct {
    pub const json_rename = .{ .listen_addr = "listen-addr" };
    pub const json_skip = .{"secret"};
    listen_addr: []const u8,
    secret: u8 = 7,
};
// Outer is in the codec, Inner is not
// .codec_then_local => {"n":"x","inner":{"listen-addr":"0.0.0.0"}}
// .codec_only       => {"n":"x","inner":{"listen_addr":"0.0.0.0","secret":7}}

A field the type asked to omit gets emitted. Binding the mode to the codec narrows the round-trip hazard without removing it, since one codec with toJson set and fromJson null is still asymmetric, but the asymmetry then has to be written into an entry rather than arising from two call sites drifting apart.

On which two to keep:

• Drop .local_only. TypedCodec(.{}) under either mixed mode is byte-identical to it across all seven properties, including nested and recursive types, and the global json.parseInto / encodeTyped already serve the ignore-the-codec case.
• Consider dropping .local_then_codec. For json_skip and json_flatten the two mixed modes are the same function, so nothing is lost. For json_tag, json_payload, fromJson, and toJson, leaving the codec field null demotes it. json_rename needs the most care: you would omit every tuple whose zig_name also has a local rename.
• Keep .codec_only. For json_skip, json_flatten, json_payload, and the two hooks it is the only way to suppress a type's own annotation, since skip and flatten have no un-set and json_payload cannot express "none". It is not needed for json_rename, where an identity tuple under .codec_then_local gives identical bytes, nor for json_tag, which a codec tag simply overrides.

One related gap to decide on, since it bears on the external-type use case: json_skip requires the skipped field to have a default, which you cannot add to a type you do not own. const Foreign = struct { name: []const u8, internal: u32 }; with a codec json_skip for internal fails with json_skip field 'internal' on Foreign has no default value. Assigning undefined, or documenting the limit, would both be reasonable.

[blazingzephyr]

Interesting, I will look into this, thanks!