Skip to content

MagicAST restructure: Phase 2 — JSON polymorphic converter via attribute scan #3

Description

@Spelkington

Parent

Part of the MagicAST restructure milestone. Phase 2 of 5. Builds on the attribute-discovery pattern established in Phase 1.

This is the #1 merge-conflict hotspot in the codebase today — every new ability/effect kind currently requires editing one of two heavily-shared base files. Eliminating this is the highest-leverage improvement for parallel-agent productivity.

What to build

Today, libs/magic-ast/AST/Abilities/Ability.cs (around lines 9–15) lists all derived ability types via [JsonPolymorphic] + [JsonDerivedType] attributes on the base class:

[JsonPolymorphic(TypeDiscriminatorPropertyName = "kind")]
[JsonDerivedType(typeof(SpellAbility), "spell")]
[JsonDerivedType(typeof(ActivatedAbility), "activated")]
// ... etc
public abstract record Ability { ... }

The same pattern is on libs/magic-ast/AST/Effects/Effect.cs with 30+ derived types in one file. Every new concrete type requires editing this base file.

Replace this with a runtime-discovered polymorphic JsonConverter for each polymorphic AST base (at minimum: Ability and Effect; audit for others). The new pattern:

// Base AST classes carry zero JSON polymorphism attributes.
public abstract record Ability { /* properties only */ }

// Concrete types carry a domain-flavored discriminator attribute.
[OracleAbility("triggered")]
public sealed record TriggeredAbility : Ability { ... }

// A custom JsonConverter<Ability> scans the assembly at construction time,
// builds the discriminator <-> Type map, and handles read/write polymorphically.

The converter is registered through MagicASTJsonOptions.Strict so all consumers automatically pick it up. Assembly scan happens once per converter instance and the result is cached (static or instance-cached — implementer's choice, but no re-scan per Read/Write call).

Strict-mode behavior must be preserved end-to-end:

  • Unknown discriminator value → JsonException
  • Unmapped JSON property on a known type → JsonException
  • Duplicate JSON properties → JsonException

(All three are guarantees the existing JsonSerializerOptions.Strict preset provides for the current [JsonPolymorphic] setup; the new converter must not weaken them.)

Acceptance criteria

  • libs/magic-ast/AST/Abilities/Ability.cs contains zero [JsonPolymorphic] / [JsonDerivedType] attributes.
  • libs/magic-ast/AST/Effects/Effect.cs contains zero [JsonPolymorphic] / [JsonDerivedType] attributes.
  • All currently-derived ability types are decorated with the new [OracleAbility(\"...\")] attribute, using the same discriminator strings as the existing setup (no breaking change to on-disk JSON).
  • All currently-derived effect types are decorated with the new [OracleEffect(\"...\")] attribute, same discriminator preservation.
  • Custom JsonConverter(s) discover types via reflection at construction time and cache the result for the converter's lifetime.
  • Strict-mode round-trip behavior preserved: unknown discriminators throw JsonException; unmapped properties throw JsonException.
  • Full ratchet test suite remains green or improves — no regressions in test-baseline.json.
  • Adding a new ability/effect kind is a single new file with one attribute; no edits to the base record or any other existing file.
  • All hand-parsed JSON test fixtures in tools/test/magic-ast/Data/HandParsedCards/ continue to deserialize identically to before this change.

Blocked by

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-agentFully specified, ready for an AFK agentscope:magic-astAffects libs/magic-ast/type:refactorRefactor — no behavior change expected

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions