Skip to content

Conversation

@bterlson
Copy link
Member

Last week, @joheredi discovered that the name of the merge patch model was the name of the template rather than the mutated name of the type. When investigating this issue, I found that we did not handle updating mutated model properties' model linkage to the mutated model. Doing so led to uncovering a rather fundamental assumption that no longer worked.

Presently, within a subgraph, the only time you can have multiple mutation nodes for a type is when it is a reference mutation, keyed off the model property type that forms the reference. But if we need to have model properties be unique whenever the model is, then those properties need to be an exception too. And then what do we do with the types those model properties reference? Given a situation where A references B references C, do you get unique mutation nodes for A, B, C, A->B, B->C and A->B->C (i.e. the reference mutation is keyed off the possibly cloned member type)? or just A, B, C, A->B, B->C (i.e. the reference mutation is keyed off the source member type)? Something else? There just isn't a good answer here that generalizes.

So the only thing to do was to let the developer decide exactly how their mutation nodes are considered to be unique. Thus, the mutation node graph is changed:

  1. Mutation nodes now have a key and are unique per (type, key) pair.
  2. The notion of a subgraph is removed because they don't serve much purpose when you get the same thing by adding some affix on the key.

With this out of the way, the notion of a subgraph and reference mutation has to be removed from the high-level Mutation nodes. Subgraphs are straight forward - we just use a suffix instead. To replace reference mutation nodes, we need to instead determine the mutation key for a referenced type, which may depend on the member types which reference it. The higher-level Mutations already have a notion similar to the mutation key - MutationOption's cache key. It made sense to align these concepts, so Mutations are also unique for a given (type, key) pair. But options is not sufficient because they are set by the caller, and we really want to be calculating the key for a given Scalar within a Scalar mutation.

So, a new static method is added to Mutations - mutationInfo. This method calculates the mutation key to use for a given type and member type references. This mutation key is then used to determine whether we need to construct a new Mutation or reuse an existing one. Within a Mutation constructor, MutationNodes are created by either using the mutation key directly, or adding a suffix to create multiple mutations. Since calculating the key may require expensive computation that will be useful for the eventually constructed mutation, mutationInfo can return additional info that is passed along to the constructor for the Mutation. So:

  1. Mutations are now unique per (type, key) pair.
  2. Mutations have a static mutationInfo method to determine their mutation key.

There is one additional problem, which is that when we are mutating a type and need to get the mutation for its connected types, we don't actually know what the mutation nodes will end up being. So we have introduced the concept of a HalfEdge to both Mutation and MutationNode. When we recurse to mutate connected types, we establish a half-edge that basically just says how to connect any new or existing nodes to the current node. MutationNodes were updated to handle connecting to already-mutated nodes which allows building up the mutation graph lazily.

Additionally, two bugs are fixed:

  1. Merge patch is handled properly by replacing the template type with the mutated type.
  2. Metadata is properly removed from the wire type.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 11, 2025

All changed packages have been documented.

  • @typespec/http-canonicalization
  • @typespec/mutator-framework
Show changes

@typespec/mutator-framework - breaking ✏️

Fix mutations not handling linkage to parent types (e.g. model property -> model). Remove mutation subgraph, nodes are now unique per (type, key) pair. Remove reference mutations, use a distinct key for references if needed.

@typespec/http-canonicalization - fix ✏️

Fix canonicalization of merge patch models.

@typespec/http-canonicalization - fix ✏️

Remove metadata properties from wire types.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Nov 11, 2025

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

@bterlson bterlson enabled auto-merge November 11, 2025 20:40
import { getEncode, type MemberType, type Program, type Type } from "@typespec/compiler";
import type { Typekit } from "@typespec/compiler/typekit";
import { isHeader } from "@typespec/http";
import type { HttpCanonicalization } from "./http-canonicalization-classes.js";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here are because we need to figure out what the codec is before we construct a canonicalization (because the codec is used to determine the mutation key for a type).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants