From 4269caf8b79d0ca6eca3b8bc5eebfd1482b18227 Mon Sep 17 00:00:00 2001
From: mjmahone <mahoney.mattj@gmail.com>
Date: Mon, 2 Jan 2023 17:45:26 -0500
Subject: [PATCH 01/30] RFC: Fragment Arguments

---
 spec/Appendix B -- Grammar Summary.md |   6 +-
 spec/Section 2 -- Language.md         |  73 ++++++++-
 spec/Section 5 -- Validation.md       | 227 ++++++++++++++++++++++----
 spec/Section 6 -- Execution.md        |  53 +++++-
 4 files changed, 307 insertions(+), 52 deletions(-)

diff --git a/spec/Appendix B -- Grammar Summary.md b/spec/Appendix B -- Grammar Summary.md
index 92f222cb3..22c30704b 100644
--- a/spec/Appendix B -- Grammar Summary.md	
+++ b/spec/Appendix B -- Grammar Summary.md	
@@ -170,12 +170,12 @@ Arguments[Const] : ( Argument[?Const]+ )
 
 Argument[Const] : Name : Value[?Const]
 
-FragmentSpread : ... FragmentName Directives?
+FragmentSpread : ... FragmentName Arguments? Directives?
 
 InlineFragment : ... TypeCondition? Directives? SelectionSet
 
-FragmentDefinition : fragment FragmentName TypeCondition Directives?
-SelectionSet
+FragmentDefinition : fragment FragmentName VariablesDefinition? TypeCondition
+Directives? SelectionSet
 
 FragmentName : Name but not `on`
 
diff --git a/spec/Section 2 -- Language.md b/spec/Section 2 -- Language.md
index 2fe3a5a31..c3c38f9d0 100644
--- a/spec/Section 2 -- Language.md	
+++ b/spec/Section 2 -- Language.md	
@@ -520,10 +520,10 @@ which returns the result:
 
 ## Fragments
 
-FragmentSpread : ... FragmentName Directives?
+FragmentSpread : ... FragmentName Arguments? Directives?
 
-FragmentDefinition : fragment FragmentName TypeCondition Directives?
-SelectionSet
+FragmentDefinition : fragment FragmentName VariablesDefinition? TypeCondition
+Directives? SelectionSet
 
 FragmentName : Name but not `on`
 
@@ -1219,13 +1219,70 @@ size `60`:
 
 **Variable Use Within Fragments**
 
-Variables can be used within fragments. Variables have global scope with a given
-operation, so a variable used within a fragment must be declared in any
-top-level operation that transitively consumes that fragment. If a variable is
-referenced in a fragment and is included by an operation that does not define
-that variable, that operation is invalid (see
+Variables can be used within fragments. Operation-defined variables have global
+scope with a given operation, so a variable used within a fragment must either
+be declared in any top-level operation that transitively consumes that fragment,
+or by that same fragment as a fragment variable definition. If a variable is
+referenced in a fragment is included by an operation where neither the fragment
+nor the operation defines that variable, that operation is invalid (see
 [All Variable Uses Defined](#sec-All-Variable-Uses-Defined)).
 
+## Fragment Variable Definitions
+
+Fragments may define locally scoped variables. This allows fragments to be
+reused while enabling the caller to specify the fragment's behavior.
+
+For example, the profile picture may need to be a different size depending on
+the parent context:
+
+```graphql example
+query withFragmentArguments {
+  user(id: 4) {
+    ...dynamicProfilePic(size: 100)
+    friends(first: 10) {
+      id
+      name
+      ...dynamicProfilePic
+    }
+  }
+}
+
+fragment dynamicProfilePic($size: Int! = 50) on User {
+  profilePic(size: $size)
+}
+```
+
+In this case the `user` will have a larger `profilePic` than those found in the
+list of `friends`.
+
+A fragment-defined variable is scoped to the fragment that defines it.
+Fragment-defined variables are allowed to shadow operation-defined variables.
+
+```graphql example
+query withShadowedVariables($size: Int) {
+  user(id: 4) {
+    ...variableProfilePic
+  }
+  secondUser: user(id: 5) {
+    ...dynamicProfilePic(size: 10)
+  }
+}
+
+fragment variableProfilePic on User {
+  ...dynamicProfilePic(size: $size)
+}
+
+fragment dynamicProfilePic($size: Int!) on User {
+  profilePic(size: $size)
+}
+```
+
+The profilePic for `user` will be determined by the variables set by the
+operation, while `secondUser` will always have a profilePic of size 10. In this
+case, the fragment `variableProfilePic` uses the operation-defined variable,
+while `dynamicProfilePic` uses the value passed in via the fragment spread's
+argument `size`.
+
 ## Type References
 
 Type :
diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 473cf5457..a5cdeaf94 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -418,8 +418,15 @@ fragment directFieldSelectionOnUnion on CatOrDog {
 
 FieldsInSetCanMerge(set):
 
+- Let {visitedSelections} be the selections in {set} including visiting
+  fragments and inline fragments and applying any supplied fragment spread
+  arguments.
+- Let {spreadsForName} be the set of fragment spreads with a given name in
+  {visitedSelections}.
+- Given each pair of members {spreadA} and {spreadB} in {spreadsForName}:
+  - {spreadA} and {spreadB} must have identical sets of arguments.
 - Let {fieldsForName} be the set of selections with a given response name in
-  {set} including visiting fragments and inline fragments.
+  {visitedSelections}.
 - Given each pair of members {fieldA} and {fieldB} in {fieldsForName}:
   - {SameResponseShape(fieldA, fieldB)} must be true.
   - If the parent types of {fieldA} and {fieldB} are equal or if either is not
@@ -570,6 +577,50 @@ fragment conflictingDifferingResponses on Pet {
 }
 ```
 
+Fragment spread arguments can also cause fields to fail to merge.
+
+While the following is valid:
+
+```graphql example
+fragment commandFragment($command: DogCommand!) on Dog {
+  doesKnowCommand(dogCommand: $command)
+}
+
+fragment potentiallyConflictingArguments(
+  $commandOne: DogCommand!
+  $commandTwo: DogCommand!
+) on Dog {
+  ...commandFragment(command: $commandOne)
+  ...commandFragment(command: $commandTwo)
+}
+
+fragment safeFragmentArguments on Dog {
+  ...potentiallyConflictingArguments(commandOne: SIT, commandTwo: SIT)
+}
+```
+
+it is only valid because `safeFragmentArguments` uses
+`potentiallyConflictingArguments` with the same value for the fragment-defined
+variables `commandOne` and `commandTwo`. Therefore `commandFragment` resolves
+`doesKnowCommand`'s `dogCommand` argument value to `SIT` in both cases.
+
+However, by changing the fragment spread argument values:
+
+```graphql counter-example
+fragment conflictingFragmentArguments on Dog {
+  ...potentiallyConflictingArguments(commandOne: SIT, commandTwo: DOWN)
+}
+```
+
+the response will have two conflicting versions of the `doesKnowCommand`
+fragment that cannot merge.
+
+If two fragment spreads with the same name supply different argument values,
+their fields will not be able to merge. In this case, validation fails because
+the fragment spread `...commandFragment(command: SIT)` and
+`...commandFragment(command: DOWN)` are part of the visited selections that will
+be merged.
+
 ### Leaf Field Selections
 
 **Formal Specification**
@@ -647,8 +698,8 @@ query directQueryOnObjectWithSubFields {
 
 ## Arguments
 
-Arguments are provided to both fields and directives. The following validation
-rules apply in both cases.
+Arguments are provided to fields, fragment spreads and directives. The following
+validation rules apply in each case.
 
 ### Argument Names
 
@@ -656,14 +707,16 @@ rules apply in both cases.
 
 - For each {argument} in the document:
   - Let {argumentName} be the Name of {argument}.
-  - Let {argumentDefinition} be the argument definition provided by the parent
-    field or definition named {argumentName}.
+  - If the parent is a field or directive:
+  - Let {argumentDefinition} be the argument or variable definition named
+    {argumentName} provided by the parent field definition, directive definition
+    or fragment definition.
   - {argumentDefinition} must exist.
 
 **Explanatory Text**
 
-Every argument provided to a field or directive must be defined in the set of
-possible arguments of that field or directive.
+Every argument provided to a field or directive or fragment spread must be
+defined in the set of possible arguments of that field, directive or fragment.
 
 For example the following are valid:
 
@@ -675,9 +728,18 @@ fragment argOnRequiredArg on Dog {
 fragment argOnOptional on Dog {
   isHouseTrained(atOtherHomes: true) @include(if: true)
 }
+
+fragment withFragmentArg($command: DogCommand) on Dog {
+  doesKnowCommand(dogCommand: $command)
+}
+
+fragment usesFragmentArg on Dog {
+  ...withFragmentArg(command: DOWN)
+}
 ```
 
-the following is invalid since `command` is not defined on `DogCommand`.
+The following is invalid since `command` is not defined on
+`Dog.doesKnowCommand`.
 
 ```graphql counter-example
 fragment invalidArgName on Dog {
@@ -685,6 +747,15 @@ fragment invalidArgName on Dog {
 }
 ```
 
+and this is also invalid as the variable `dogCommand` is not defined on fragment
+`withFragmentArg`.
+
+```graphql counter-example
+fragment invalidFragmentArgName on Dog {
+  ...withFragmentArg(dogCommand: SIT)
+}
+```
+
 and this is also invalid as `unless` is not defined on `@include`.
 
 ```graphql counter-example
@@ -727,9 +798,9 @@ fragment multipleArgsReverseOrder on Arguments {
 
 ### Argument Uniqueness
 
-Fields and directives treat arguments as a mapping of argument name to value.
-More than one argument with the same name in an argument set is ambiguous and
-invalid.
+Fields, fragment spreads and directives treat arguments as a mapping of argument
+name to value. More than one argument with the same name in an argument set is
+ambiguous and invalid.
 
 **Formal Specification**
 
@@ -741,10 +812,11 @@ invalid.
 
 ### Required Arguments
 
-- For each Field or Directive in the document:
-  - Let {arguments} be the arguments provided by the Field or Directive.
+- For each Field, Fragment Spread or Directive in the document:
+  - Let {arguments} be the arguments provided by the Field, Directive or
+    Fragment Spread.
   - Let {argumentDefinitions} be the set of argument definitions of that Field
-    or Directive.
+    or Directive, or the variable definitions of that Fragment.
   - For each {argumentDefinition} in {argumentDefinitions}:
     - Let {type} be the expected type of {argumentDefinition}.
     - Let {defaultValue} be the default value of {argumentDefinition}.
@@ -1523,18 +1595,19 @@ query ($foo: Boolean = true, $bar: Boolean = false) {
 
 **Formal Specification**
 
-- For every {operation} in the document:
-  - For every {variable} defined on {operation}:
+- For every {operation} and {fragment} in the document:
+  - Let {operationOrFragment} be that {operation} or {fragment}.
+  - For every {variable} defined on {operationOrFragment}:
     - Let {variableName} be the name of {variable}.
     - Let {variables} be the set of all variables named {variableName} on
-      {operation}.
+      {operationOrFragment}.
     - {variables} must be a set of one.
 
 **Explanatory Text**
 
-If any operation defines more than one variable with the same name, it is
-ambiguous and invalid. It is invalid even if the type of the duplicate variable
-is the same.
+If any operation or fragment defines more than one variable with the same name,
+it is ambiguous and invalid. It is invalid even if the type of the duplicate
+variable is the same.
 
 ```graphql counter-example
 query houseTrainedQuery($atOtherHomes: Boolean, $atOtherHomes: Boolean) {
@@ -1563,12 +1636,36 @@ fragment HouseTrainedFragment on Query {
 }
 ```
 
+Likewise, it is valid for both an operation and a fragment to define a variable
+with the same name:
+
+```graphql example
+query C($atOtherHomes: Boolean) {
+  ...HouseTrainedFragment
+  aDog: dog {
+    ...HouseTrainedDog
+  }
+}
+
+fragment HouseTrainedDog($atOtherHomes: Boolean) on Dog {
+  isHouseTrained(atOtherHomes: $atOtherHomes)
+}
+```
+
+Fragment-defined variables are scoped locally to the fragment that defines them,
+and override any operation-defined variable values, so there is never ambiguity
+about which value to use. In this case, the value of the argument `atOtherHomes`
+within `HouseTrainedFragment` will be the operation-set value, and within
+`HouseTrainedDog` will resolve to `null`, as the argument is not set by the
+fragment spread in the query `C`.
+
 ### Variables Are Input Types
 
 **Formal Specification**
 
-- For every {operation} in a {document}:
-  - For every {variable} on each {operation}:
+- For every {operation} and {fragment} in a {document}:
+  - Let {operationOrFragment} be that {operation} or {fragment}.
+  - For every {variable} defined on {operationOrFragment}:
     - Let {variableType} be the type of {variable}.
     - {IsInputType(variableType)} must be {true}.
 
@@ -1636,13 +1733,14 @@ query takesCatOrDog($catOrDog: CatOrDog) {
     transitively.
   - For each {fragment} in {fragments}:
     - For each {variableUsage} in scope of {fragment}, variable must be in
-      {operation}'s variable list.
+      {fragment}'s or {operation}'s variable list.
 
 **Explanatory Text**
 
-Variables are scoped on a per-operation basis. That means that any variable used
-within the context of an operation must be defined at the top level of that
-operation
+Operation-defined Variables are scoped on a per-operation basis, while
+Fragment-defined Variables are scoped locally to the fragment. That means that
+any variable used within the context of an operation must either be defined at
+the top level of that operation or on the fragment that uses that variable.
 
 For example:
 
@@ -1669,9 +1767,10 @@ query variableIsNotDefined {
 ${atOtherHomes} is not defined by the operation.
 
 Fragments complicate this rule. Any fragment transitively included by an
-operation has access to the variables defined by that operation. Fragments can
-appear within multiple operations and therefore variable usages must correspond
-to variable definitions in all of those operations.
+operation has access to the variables defined by that operation and defined on
+the fragment. Fragments can appear within multiple operations and therefore
+variable usages not defined on the fragment must correspond to variable
+definitions in all of those operations.
 
 For example the following is valid:
 
@@ -1768,7 +1867,7 @@ This is because {houseTrainedQueryTwoNotDefined} does not define a variable
 ${atOtherHomes} but that variable is used by {isHouseTrainedFragment} which is
 included in that operation.
 
-### All Variables Used
+### All Operation Variables Used
 
 **Formal Specification**
 
@@ -1776,7 +1875,7 @@ included in that operation.
   - Let {variables} be the variables defined by that {operation}.
   - Each {variable} in {variables} must be used at least once in either the
     operation scope itself or any fragment transitively referenced by that
-    operation.
+    operation, excluding fragments that define the same name as an argument.
 
 **Explanatory Text**
 
@@ -1828,6 +1927,29 @@ fragment isHouseTrainedWithoutVariableFragment on Dog {
 }
 ```
 
+Fragment arguments can shadow operation variables: fragments that use an
+argument are not using the operation-defined variable of the same name.
+
+Likewise, it would be invalid if the variable was shadowed by a fragment
+argument:
+
+```graphql counter-example
+query variableNotUsedWithinFragment($atOtherHomes: Boolean) {
+  dog {
+    ...shadowedVariableFragment
+  }
+}
+
+fragment shadowedVariableFragment($atOtherHomes: Boolean) on Dog {
+  isHouseTrained(atOtherHomes: $atOtherHomes)
+}
+```
+
+because
+{$atOtherHomes} is only referenced in a fragment that defines it as a
+locally scoped argument, the operation-defined {$atOtherHomes}
+variable is never used.
+
 All operations in a document must use all of their variables.
 
 As a result, the following document does not validate.
@@ -1853,6 +1975,41 @@ fragment isHouseTrainedFragment on Dog {
 This document is not valid because {queryWithExtraVar} defines an extraneous
 variable.
 
+### All Fragment Variables Used
+
+**Formal Specification**
+
+- For every {fragment} in the document:
+  - Let {variables} be the variables defined by that {fragment}.
+  - Each {variable} in {variables} must be used at least once in the fragment's
+    scope.
+
+**Explanatory Text**
+
+All variables defined by a fragment must be used in that same fragment. Because
+fragment-defined variables are scoped to the fragment they are defined on, if
+the fragment does not use the variable, then the variable definition is
+superfluous.
+
+For example, the following is invalid:
+
+```graphql counter-example
+query queryWithFragmentArgUnused($atOtherHomes: Boolean) {
+  dog {
+    ...fragmentArgUnused(atOtherHomes: $atOtherHomes)
+  }
+}
+
+fragment fragmentArgUnused($atOtherHomes: Boolean) on Dog {
+  isHouseTrained
+}
+```
+
+This document is invalid: even though `fragmentArgUnused` is spread with the
+argument `atOtherHomes` and `$atOtherHomes` is defined as an operation variable,
+there is never a variable `$atOtherHomes` used within the scope of
+`fragmentArgUnused`.
+
 ### All Variable Usages Are Allowed
 
 **Formal Specification**
@@ -1861,8 +2018,12 @@ variable.
   - Let {variableUsages} be all usages transitively included in the {operation}.
   - For each {variableUsage} in {variableUsages}:
     - Let {variableName} be the name of {variableUsage}.
-    - Let {variableDefinition} be the {VariableDefinition} named {variableName}
-      defined within {operation}.
+    - If the usage is within a {fragment} that defines a {variableDefinition}
+      for {variableName}:
+      - Let {variableDefinition} be the {VariableDefinition} named
+        {variableName} defined within {fragment}.
+    - Otherwise, let {variableDefinition} be the {VariableDefinition} named
+      {variableName} defined within {operation}.
     - {IsVariableUsageAllowed(variableDefinition, variableUsage)} must be
       {true}.
 
diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 8184f95bb..2a849ec0f 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -515,17 +515,23 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments):
     - Append {selection} to the {groupForResponseKey}.
   - If {selection} is a {FragmentSpread}:
     - Let {fragmentSpreadName} be the name of {selection}.
-    - If {fragmentSpreadName} is in {visitedFragments}, continue with the next
-      {selection} in {selectionSet}.
-    - Add {fragmentSpreadName} to {visitedFragments}.
     - Let {fragment} be the Fragment in the current Document whose name is
       {fragmentSpreadName}.
     - If no such {fragment} exists, continue with the next {selection} in
       {selectionSet}.
+    - Let {arguments} be the set of arguments on {selection}.
+    - Let {fragmentSpreadKey} be a unique key of {fragmentSpreadName} and
+      {arguments}.
+    - If {fragmentSpreadKey} is in {visitedFragments}, continue with the next
+      {selection} in {selectionSet}.
+    - Add {fragmentSpreadKey} to {visitedFragments}.
     - Let {fragmentType} be the type condition on {fragment}.
     - If {DoesFragmentTypeApply(objectType, fragmentType)} is {false}, continue
       with the next {selection} in {selectionSet}.
-    - Let {fragmentSelectionSet} be the top-level selection set of {fragment}.
+    - Let {fragmentWithArgumentSubstitutions} be the result of calling
+      {SubstituteFragmentArguments(fragment, arguments)}.
+    - Let {fragmentSelectionSet} be the top-level selection set of
+      {fragmentWithArgumentSubstitutions}.
     - Let {fragmentGroupedFieldSet} be the result of calling
       {CollectFields(objectType, fragmentSelectionSet, variableValues,
       visitedFragments)}.
@@ -564,9 +570,36 @@ DoesFragmentTypeApply(objectType, fragmentType):
   - If {objectType} is a possible type of {fragmentType}, return {true}
     otherwise return {false}.
 
+SubstituteFragmentArguments(fragment, arguments):
+
+- Let {variablesToSubstitute} be initialized to an empty map.
+- For each {variableDefinition} in {fragment}:
+  - Let {variableName} be the name of {variableDefinition}.
+  - If {variableName} is a key in {arguments}:
+    - Let {argumentValue} be the value of {variableName} in {arguments}.
+    - Add {argumentValue} to {variablesToSubstitute} at key {variableName}.
+  - Otherwise if {variableDefinition} has a default value {defaultValue}:
+    - Add {defaultValue} to {variablesToSubstitute} at key {variableName}.
+  - Otherwise:
+    - Set the key {variableName} in {variableToSubstitute} to a value indicating
+      the variable is unset.
+- Let {substitutedFragment} be a copy of {fragment} where:
+  - For each {variable} in the selection set of {fragment}:
+    - Let {variableUsageName} be the name of {variable}.
+    - If {variableUsageName} is in {variablesToSubstitute}:
+      - Replace {variable} with the value of {variableUsageName} in
+        {variablesToSubstitute}.
+- Return {substitutedFragment}.
+
 Note: The steps in {CollectFields()} evaluating the `@skip` and `@include`
 directives may be applied in either order since they apply commutatively.
 
+Note: The unset value used to replace unset fragment-defined arguments in
+{SubstituteFragmentArguments()} must not be a variable defined by any operation
+that includes the fragment. An example would be to use a variable with a
+reserved prefix, like `$__UNSET`, to replace all unset fragment-defined
+variables.
+
 ## Executing Fields
 
 Each field requested in the grouped field set that is defined on the selected
@@ -579,8 +612,8 @@ ExecuteField(objectType, objectValue, fieldType, fields, variableValues):
 
 - Let {field} be the first entry in {fields}.
 - Let {fieldName} be the field name of {field}.
-- Let {argumentValues} be the result of {CoerceArgumentValues(objectType, field,
-  variableValues)}.
+- Let {argumentValues} be the result of {CoerceFieldArgumentValues(objectType,
+  field, variableValues)}.
 - Let {resolvedValue} be {ResolveFieldValue(objectType, objectValue, fieldName,
   argumentValues)}.
 - Return the result of {CompleteValue(fieldType, fields, resolvedValue,
@@ -595,13 +628,17 @@ the type system to have a specific input type.
 At each argument position in an operation may be a literal {Value}, or a
 {Variable} to be provided at runtime.
 
-CoerceArgumentValues(objectType, field, variableValues):
+CoerceFieldArgumentValues(objectType, field, variableValues):
 
-- Let {coercedValues} be an empty unordered Map.
 - Let {argumentValues} be the argument values provided in {field}.
 - Let {fieldName} be the name of {field}.
 - Let {argumentDefinitions} be the arguments defined by {objectType} for the
   field named {fieldName}.
+- Return {CoerceArgumentValues(argumentDefinitions, argumentValues,
+  variableValues)}
+
+CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues):
+
 - For each {argumentDefinition} in {argumentDefinitions}:
   - Let {argumentName} be the name of {argumentDefinition}.
   - Let {argumentType} be the expected type of {argumentDefinition}.

From 6e91f98474e1acf6b495fcc50118d6212566f69a Mon Sep 17 00:00:00 2001
From: jdecroock <decroockjovi@gmail.com>
Date: Wed, 7 Feb 2024 10:46:28 +0100
Subject: [PATCH 02/30] address
 https://github.com/graphql/graphql-js/pull/3835#discussion_r1101825832

---
 spec/Section 6 -- Execution.md | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 2a849ec0f..0ce8efe29 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -519,12 +519,9 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments):
       {fragmentSpreadName}.
     - If no such {fragment} exists, continue with the next {selection} in
       {selectionSet}.
-    - Let {arguments} be the set of arguments on {selection}.
-    - Let {fragmentSpreadKey} be a unique key of {fragmentSpreadName} and
-      {arguments}.
-    - If {fragmentSpreadKey} is in {visitedFragments}, continue with the next
+    - If {fragmentSpreadName} is in {visitedFragments}, continue with the next
       {selection} in {selectionSet}.
-    - Add {fragmentSpreadKey} to {visitedFragments}.
+    - Add {fragmentSpreadName} to {visitedFragments}.
     - Let {fragmentType} be the type condition on {fragment}.
     - If {DoesFragmentTypeApply(objectType, fragmentType)} is {false}, continue
       with the next {selection} in {selectionSet}.

From a868d0411b8bdfc54a85f845f310d467d20b3ffe Mon Sep 17 00:00:00 2001
From: jdecroock <decroockjovi@gmail.com>
Date: Wed, 7 Feb 2024 10:56:41 +0100
Subject: [PATCH 03/30] wip

---
 spec/Section 6 -- Execution.md | 66 +++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 0ce8efe29..5759af00e 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -340,11 +340,12 @@ ExecuteSelectionSet(selectionSet, objectType, objectValue, variableValues):
 - For each {groupedFieldSet} as {responseKey} and {fields}:
   - Let {fieldName} be the name of the first entry in {fields}. Note: This value
     is unaffected if an alias is used.
+  - Let {fragmentVariableValues} be the fragment-variables value of the first entry in {fields}.
   - Let {fieldType} be the return type defined for the field {fieldName} of
     {objectType}.
   - If {fieldType} is defined:
     - Let {responseValue} be {ExecuteField(objectType, objectValue, fieldType,
-      fields, variableValues)}.
+      fields, variableValues, fragmentVariableValues)}.
     - Set {responseValue} as the value for {responseKey} in {resultMap}.
 - Return {resultMap}.
 
@@ -492,7 +493,7 @@ The depth-first-search order of the field groups produced by {CollectFields()}
 is maintained through execution, ensuring that fields appear in the executed
 response in a stable and predictable order.
 
-CollectFields(objectType, selectionSet, variableValues, visitedFragments):
+CollectFields(objectType, selectionSet, variableValues, visitedFragments, localVariableValues):
 
 - If {visitedFragments} is not provided, initialize it to the empty set.
 - Initialize {groupedFields} to an empty ordered map of lists.
@@ -500,19 +501,19 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments):
   - If {selection} provides the directive `@skip`, let {skipDirective} be that
     directive.
     - If {skipDirective}'s {if} argument is {true} or is a variable in
-      {variableValues} with the value {true}, continue with the next {selection}
+      {localVariableValues} or {variableValues} with the value {true}, continue with the next {selection}
       in {selectionSet}.
   - If {selection} provides the directive `@include`, let {includeDirective} be
     that directive.
     - If {includeDirective}'s {if} argument is not {true} and is not a variable
-      in {variableValues} with the value {true}, continue with the next
+      in {localVariableValues} or {variableValues} with the value {true}, continue with the next
       {selection} in {selectionSet}.
   - If {selection} is a {Field}:
     - Let {responseKey} be the response key of {selection} (the alias if
       defined, otherwise the field name).
     - Let {groupForResponseKey} be the list in {groupedFields} for
       {responseKey}; if no such list exists, create it as an empty list.
-    - Append {selection} to the {groupForResponseKey}.
+    - Append {selection} and {localVariableValues} to the {groupForResponseKey}.
   - If {selection} is a {FragmentSpread}:
     - Let {fragmentSpreadName} be the name of {selection}.
     - Let {fragment} be the Fragment in the current Document whose name is
@@ -525,10 +526,8 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments):
     - Let {fragmentType} be the type condition on {fragment}.
     - If {DoesFragmentTypeApply(objectType, fragmentType)} is {false}, continue
       with the next {selection} in {selectionSet}.
-    - Let {fragmentWithArgumentSubstitutions} be the result of calling
-      {SubstituteFragmentArguments(fragment, arguments)}.
-    - Let {fragmentSelectionSet} be the top-level selection set of
-      {fragmentWithArgumentSubstitutions}.
+    - Let {localVariableValues} be the result of calling
+      {getArgumentValuesFromSpread(fragmentSpread, schema, fragmentDefinition.variableDefinitions, variableValues, localVariableValues)}.
     - Let {fragmentGroupedFieldSet} be the result of calling
       {CollectFields(objectType, fragmentSelectionSet, variableValues,
       visitedFragments)}.
@@ -567,26 +566,25 @@ DoesFragmentTypeApply(objectType, fragmentType):
   - If {objectType} is a possible type of {fragmentType}, return {true}
     otherwise return {false}.
 
-SubstituteFragmentArguments(fragment, arguments):
+getArgumentValuesFromSpread(fragmentSpread, schema, fragmentDefinitionVariableDefinitions, variableValues, fragmentArgumentValues):
 
-- Let {variablesToSubstitute} be initialized to an empty map.
-- For each {variableDefinition} in {fragment}:
+- Let {coercedValues} be an empty unordered Map.
+- For each {variableDefinition} in {fragmentDefinitionVariableDefinitions}:
   - Let {variableName} be the name of {variableDefinition}.
-  - If {variableName} is a key in {arguments}:
-    - Let {argumentValue} be the value of {variableName} in {arguments}.
-    - Add {argumentValue} to {variablesToSubstitute} at key {variableName}.
-  - Otherwise if {variableDefinition} has a default value {defaultValue}:
-    - Add {defaultValue} to {variablesToSubstitute} at key {variableName}.
-  - Otherwise:
-    - Set the key {variableName} in {variableToSubstitute} to a value indicating
-      the variable is unset.
-- Let {substitutedFragment} be a copy of {fragment} where:
-  - For each {variable} in the selection set of {fragment}:
-    - Let {variableUsageName} be the name of {variable}.
-    - If {variableUsageName} is in {variablesToSubstitute}:
-      - Replace {variable} with the value of {variableUsageName} in
-        {variablesToSubstitute}.
-- Return {substitutedFragment}.
+  - Let {variableType} be the type of {variableDefinition}.
+  - Let {defaultValue} be the default value for {variableDefinition}.
+  - Let {argumentNode} be the node provided in the fragment-spread for {variableName}
+  - If {argumentNode} isn't present or is null
+    - If {defaultValue} exists
+      - Add an entry to {coercedValues} named {argumentName} with the value
+        {defaultValue}.
+    - If {variableType} is not-nullable raise a field-error
+  - Let {hasValue} be {true} if {fragmentArgumentValues} or {variableValues} provides a   value for the name
+    {variableName}.
+  - If {variableType} is not-nullable and {hasValue} is {false} raise a field-error
+  - Add an entry to {coercedValues} named {argumentName} with the value
+    found in {variableValues} or {fragmentArgumentValues}.
+- Return {coercedValues}.
 
 Note: The steps in {CollectFields()} evaluating the `@skip` and `@include`
 directives may be applied in either order since they apply commutatively.
@@ -605,12 +603,12 @@ coerces any provided argument values, then resolves a value for the field, and
 finally completes that value either by recursively executing another selection
 set or coercing a scalar value.
 
-ExecuteField(objectType, objectValue, fieldType, fields, variableValues):
+ExecuteField(objectType, objectValue, fieldType, fields, variableValues, fragmentVariableValues):
 
 - Let {field} be the first entry in {fields}.
 - Let {fieldName} be the field name of {field}.
 - Let {argumentValues} be the result of {CoerceFieldArgumentValues(objectType,
-  field, variableValues)}.
+  field, variableValues, fragmentVariableValues)}
 - Let {resolvedValue} be {ResolveFieldValue(objectType, objectValue, fieldName,
   argumentValues)}.
 - Return the result of {CompleteValue(fieldType, fields, resolvedValue,
@@ -625,16 +623,16 @@ the type system to have a specific input type.
 At each argument position in an operation may be a literal {Value}, or a
 {Variable} to be provided at runtime.
 
-CoerceFieldArgumentValues(objectType, field, variableValues):
+CoerceFieldArgumentValues(objectType, field, variableValues, fragmentVariableValues):
 
 - Let {argumentValues} be the argument values provided in {field}.
 - Let {fieldName} be the name of {field}.
 - Let {argumentDefinitions} be the arguments defined by {objectType} for the
   field named {fieldName}.
 - Return {CoerceArgumentValues(argumentDefinitions, argumentValues,
-  variableValues)}
+  variableValues, fragmentVariableValues)}
 
-CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues):
+CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues, fragmentVariableValues):
 
 - For each {argumentDefinition} in {argumentDefinitions}:
   - Let {argumentName} be the name of {argumentDefinition}.
@@ -646,6 +644,10 @@ CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues):
     {argumentName}.
   - If {argumentValue} is a {Variable}:
     - Let {variableName} be the name of {argumentValue}.
+    - Let {hasValue} be {true} if {fragmentVariableValues} provides a value for the name
+      {variableName}.
+    - Let {value} be the value provided in {fragmentVariableValues} for the name
+      {variableName}.
     - Let {hasValue} be {true} if {variableValues} provides a value for the name
       {variableName}.
     - Let {value} be the value provided in {variableValues} for the name

From 548163a53f9a561f8873917f2e11904a2605fe09 Mon Sep 17 00:00:00 2001
From: jdecroock <decroockjovi@gmail.com>
Date: Mon, 12 Feb 2024 10:14:39 +0100
Subject: [PATCH 04/30] wording

---
 spec/Section 6 -- Execution.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 5759af00e..977a493ec 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -578,10 +578,10 @@ getArgumentValuesFromSpread(fragmentSpread, schema, fragmentDefinitionVariableDe
     - If {defaultValue} exists
       - Add an entry to {coercedValues} named {argumentName} with the value
         {defaultValue}.
-    - If {variableType} is not-nullable raise a field-error
+    - If {variableType} is non-nullable raise a field-error
   - Let {hasValue} be {true} if {fragmentArgumentValues} or {variableValues} provides a   value for the name
     {variableName}.
-  - If {variableType} is not-nullable and {hasValue} is {false} raise a field-error
+  - If {variableType} is non-nullable and {hasValue} is {false} raise a field-error
   - Add an entry to {coercedValues} named {argumentName} with the value
     found in {variableValues} or {fragmentArgumentValues}.
 - Return {coercedValues}.

From 210a814d412beccb81aefef2b21b330fdfe362eb Mon Sep 17 00:00:00 2001
From: jdecroock <decroockjovi@gmail.com>
Date: Fri, 16 Feb 2024 08:35:18 +0100
Subject: [PATCH 05/30] corrections

---
 spec/Section 6 -- Execution.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 977a493ec..22dfb57f2 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -527,7 +527,7 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments, localV
     - If {DoesFragmentTypeApply(objectType, fragmentType)} is {false}, continue
       with the next {selection} in {selectionSet}.
     - Let {localVariableValues} be the result of calling
-      {getArgumentValuesFromSpread(fragmentSpread, schema, fragmentDefinition.variableDefinitions, variableValues, localVariableValues)}.
+      {getArgumentValuesFromSpread(selection, fragmentDefinition, variableValues, localVariableValues)}.
     - Let {fragmentGroupedFieldSet} be the result of calling
       {CollectFields(objectType, fragmentSelectionSet, variableValues,
       visitedFragments)}.
@@ -566,10 +566,10 @@ DoesFragmentTypeApply(objectType, fragmentType):
   - If {objectType} is a possible type of {fragmentType}, return {true}
     otherwise return {false}.
 
-getArgumentValuesFromSpread(fragmentSpread, schema, fragmentDefinitionVariableDefinitions, variableValues, fragmentArgumentValues):
+getArgumentValuesFromSpread(fragmentSpread, fragmentDefinition, variableValues, fragmentArgumentValues):
 
 - Let {coercedValues} be an empty unordered Map.
-- For each {variableDefinition} in {fragmentDefinitionVariableDefinitions}:
+- For each {variableDefinition} in {fragmentDefinition}:
   - Let {variableName} be the name of {variableDefinition}.
   - Let {variableType} be the type of {variableDefinition}.
   - Let {defaultValue} be the default value for {variableDefinition}.

From 562edd83f9f7d381a663ce77ffec4639603d03ae Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Tue, 27 Feb 2024 21:00:51 +0100
Subject: [PATCH 06/30] Update spec/Section 2 -- Language.md

Co-authored-by: Matt Mahoney <mahoney.mattj@gmail.com>
---
 spec/Section 2 -- Language.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/Section 2 -- Language.md b/spec/Section 2 -- Language.md
index c3c38f9d0..36d632fe1 100644
--- a/spec/Section 2 -- Language.md	
+++ b/spec/Section 2 -- Language.md	
@@ -1222,7 +1222,7 @@ size `60`:
 Variables can be used within fragments. Operation-defined variables have global
 scope with a given operation, so a variable used within a fragment must either
 be declared in any top-level operation that transitively consumes that fragment,
-or by that same fragment as a fragment variable definition. If a variable is
+or by that same fragment as a fragment variable definition. If a variable
 referenced in a fragment is included by an operation where neither the fragment
 nor the operation defines that variable, that operation is invalid (see
 [All Variable Uses Defined](#sec-All-Variable-Uses-Defined)).

From c46e706aad2da7cba203de75bd589dfadd3c6ab8 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Fri, 8 Mar 2024 07:59:31 +0100
Subject: [PATCH 07/30] address validation comments

---
 spec/Section 5 -- Validation.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index a5cdeaf94..44183b4ef 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -424,7 +424,7 @@ FieldsInSetCanMerge(set):
 - Let {spreadsForName} be the set of fragment spreads with a given name in
   {visitedSelections}.
 - Given each pair of members {spreadA} and {spreadB} in {spreadsForName}:
-  - {spreadA} and {spreadB} must have identical sets of arguments.
+  - {spreadA} and {spreadB} must have identical sets of arguments, values and directives.
 - Let {fieldsForName} be the set of selections with a given response name in
   {visitedSelections}.
 - Given each pair of members {fieldA} and {fieldB} in {fieldsForName}:
@@ -1636,8 +1636,8 @@ fragment HouseTrainedFragment on Query {
 }
 ```
 
-Likewise, it is valid for both an operation and a fragment to define a variable
-with the same name:
+Likewise, it is valid for a fragment to define a variable with a name that
+is also defined on an operation:
 
 ```graphql example
 query C($atOtherHomes: Boolean) {

From 2117038955c2369416edd7d97e5af62e0f65590d Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Fri, 8 Mar 2024 08:03:06 +0100
Subject: [PATCH 08/30] address language comments

---
 spec/Section 2 -- Language.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/Section 2 -- Language.md b/spec/Section 2 -- Language.md
index 36d632fe1..a423a999e 100644
--- a/spec/Section 2 -- Language.md	
+++ b/spec/Section 2 -- Language.md	
@@ -1220,7 +1220,8 @@ size `60`:
 **Variable Use Within Fragments**
 
 Variables can be used within fragments. Operation-defined variables have global
-scope with a given operation, so a variable used within a fragment must either
+scope with a given operation. Fragment-defined variables have local scope within the
+fragment definition they are defined in. A variable used within a fragment must either
 be declared in any top-level operation that transitively consumes that fragment,
 or by that same fragment as a fragment variable definition. If a variable
 referenced in a fragment is included by an operation where neither the fragment

From e027a4f78ae5e34fd3d6096591f1d14fe2619007 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Tue, 26 Mar 2024 21:15:23 +0100
Subject: [PATCH 09/30] Remove unused `$__UNSET`

---
 spec/Section 6 -- Execution.md | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 22dfb57f2..9f2a37270 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -589,12 +589,6 @@ getArgumentValuesFromSpread(fragmentSpread, fragmentDefinition, variableValues,
 Note: The steps in {CollectFields()} evaluating the `@skip` and `@include`
 directives may be applied in either order since they apply commutatively.
 
-Note: The unset value used to replace unset fragment-defined arguments in
-{SubstituteFragmentArguments()} must not be a variable defined by any operation
-that includes the fragment. An example would be to use a variable with a
-reserved prefix, like `$__UNSET`, to replace all unset fragment-defined
-variables.
-
 ## Executing Fields
 
 Each field requested in the grouped field set that is defined on the selected

From 2e541bb27172a0782e6d08c4403175fe64505215 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Wed, 27 Mar 2024 13:17:38 +0100
Subject: [PATCH 10/30] Apply Benjie's suggestions

Co-authored-by: Benjie <benjie@jemjie.com>
---
 spec/Section 2 -- Language.md | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/spec/Section 2 -- Language.md b/spec/Section 2 -- Language.md
index a423a999e..de563f1cb 100644
--- a/spec/Section 2 -- Language.md	
+++ b/spec/Section 2 -- Language.md	
@@ -1220,9 +1220,9 @@ size `60`:
 **Variable Use Within Fragments**
 
 Variables can be used within fragments. Operation-defined variables have global
-scope with a given operation. Fragment-defined variables have local scope within the
-fragment definition they are defined in. A variable used within a fragment must either
-be declared in any top-level operation that transitively consumes that fragment,
+scope within a given operation. Fragment-defined variables have local scope within the
+fragment definition in which they are defined. A variable used within a fragment must either
+be declared in each top-level operation that transitively consumes that fragment,
 or by that same fragment as a fragment variable definition. If a variable
 referenced in a fragment is included by an operation where neither the fragment
 nor the operation defines that variable, that operation is invalid (see
@@ -1237,7 +1237,7 @@ For example, the profile picture may need to be a different size depending on
 the parent context:
 
 ```graphql example
-query withFragmentArguments {
+query userAndFriends {
   user(id: 4) {
     ...dynamicProfilePic(size: 100)
     friends(first: 10) {
@@ -1260,7 +1260,7 @@ A fragment-defined variable is scoped to the fragment that defines it.
 Fragment-defined variables are allowed to shadow operation-defined variables.
 
 ```graphql example
-query withShadowedVariables($size: Int) {
+query withShadowedVariables($size: Int!) {
   user(id: 4) {
     ...variableProfilePic
   }
@@ -1279,10 +1279,10 @@ fragment dynamicProfilePic($size: Int!) on User {
 ```
 
 The profilePic for `user` will be determined by the variables set by the
-operation, while `secondUser` will always have a profilePic of size 10. In this
+operation, while `secondUser` will always have a `profilePic` of size `10`. In this
 case, the fragment `variableProfilePic` uses the operation-defined variable,
 while `dynamicProfilePic` uses the value passed in via the fragment spread's
-argument `size`.
+`size` argument.
 
 ## Type References
 

From 16e8986e6497eb1e110c4fddb2ac84de43073cbe Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Wed, 27 Mar 2024 14:38:26 +0100
Subject: [PATCH 11/30] conciser validation

---
 spec/Section 5 -- Validation.md | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 44183b4ef..6c1c3e4b7 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -579,8 +579,6 @@ fragment conflictingDifferingResponses on Pet {
 
 Fragment spread arguments can also cause fields to fail to merge.
 
-While the following is valid:
-
 ```graphql example
 fragment commandFragment($command: DogCommand!) on Dog {
   doesKnowCommand(dogCommand: $command)
@@ -588,39 +586,25 @@ fragment commandFragment($command: DogCommand!) on Dog {
 
 fragment potentiallyConflictingArguments(
   $commandOne: DogCommand!
-  $commandTwo: DogCommand!
 ) on Dog {
   ...commandFragment(command: $commandOne)
   ...commandFragment(command: $commandTwo)
 }
 
 fragment safeFragmentArguments on Dog {
-  ...potentiallyConflictingArguments(commandOne: SIT, commandTwo: SIT)
-}
-```
-
-it is only valid because `safeFragmentArguments` uses
-`potentiallyConflictingArguments` with the same value for the fragment-defined
-variables `commandOne` and `commandTwo`. Therefore `commandFragment` resolves
-`doesKnowCommand`'s `dogCommand` argument value to `SIT` in both cases.
-
-However, by changing the fragment spread argument values:
-
-```graphql counter-example
-fragment conflictingFragmentArguments on Dog {
   ...potentiallyConflictingArguments(commandOne: SIT, commandTwo: DOWN)
 }
 ```
 
-the response will have two conflicting versions of the `doesKnowCommand`
-fragment that cannot merge.
-
 If two fragment spreads with the same name supply different argument values,
 their fields will not be able to merge. In this case, validation fails because
 the fragment spread `...commandFragment(command: SIT)` and
 `...commandFragment(command: DOWN)` are part of the visited selections that will
 be merged.
 
+If both of these spreads would have `$commandOne` or `$commandTwo` as the argument-value,
+it would be allowed as we can be sure that we'd resolve identical fields.
+
 ### Leaf Field Selections
 
 **Formal Specification**

From 8be056175b7f4050169ace16582aca8187d51b38 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Fri, 29 Mar 2024 11:25:44 +0100
Subject: [PATCH 12/30] Apply suggestions from code review

Co-authored-by: Benjie <benjie@jemjie.com>
---
 spec/Section 5 -- Validation.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 6c1c3e4b7..01e2124d9 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -423,8 +423,8 @@ FieldsInSetCanMerge(set):
   arguments.
 - Let {spreadsForName} be the set of fragment spreads with a given name in
   {visitedSelections}.
-- Given each pair of members {spreadA} and {spreadB} in {spreadsForName}:
-  - {spreadA} and {spreadB} must have identical sets of arguments, values and directives.
+- For each {spreadsForName} as {name} and {spreads}:
+  - Each entry in {spreads} must have identical sets of arguments to each other entry in {spreads}.
 - Let {fieldsForName} be the set of selections with a given response name in
   {visitedSelections}.
 - Given each pair of members {fieldA} and {fieldB} in {fieldsForName}:

From 492c556e56b830675fe2f3bfccf9db39d890c9d0 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Fri, 29 Mar 2024 11:27:23 +0100
Subject: [PATCH 13/30] Apply suggestions from code review

Co-authored-by: Benjie <benjie@jemjie.com>
---
 spec/Section 5 -- Validation.md | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 01e2124d9..18421899e 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -425,7 +425,7 @@ FieldsInSetCanMerge(set):
   {visitedSelections}.
 - For each {spreadsForName} as {name} and {spreads}:
   - Each entry in {spreads} must have identical sets of arguments to each other entry in {spreads}.
-- Let {fieldsForName} be the set of selections with a given response name in
+- Let {fieldsForName} be the set of field selections with a given response name in
   {visitedSelections}.
 - Given each pair of members {fieldA} and {fieldB} in {fieldsForName}:
   - {SameResponseShape(fieldA, fieldB)} must be true.
@@ -731,7 +731,7 @@ fragment invalidArgName on Dog {
 }
 ```
 
-and this is also invalid as the variable `dogCommand` is not defined on fragment
+and this is also invalid as the argument `dogCommand` is not defined on fragment
 `withFragmentArg`.
 
 ```graphql counter-example
@@ -1989,10 +1989,7 @@ fragment fragmentArgUnused($atOtherHomes: Boolean) on Dog {
 }
 ```
 
-This document is invalid: even though `fragmentArgUnused` is spread with the
-argument `atOtherHomes` and `$atOtherHomes` is defined as an operation variable,
-there is never a variable `$atOtherHomes` used within the scope of
-`fragmentArgUnused`.
+This document is invalid: fragment `fragmentArgUnused` defines a fragment variable `$atOtherHomes`, but this variable is not used within this fragment.
 
 ### All Variable Usages Are Allowed
 

From b4f3c5d3068ceb577656bd41213fcb7644beedcf Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sat, 30 Mar 2024 08:28:21 +0100
Subject: [PATCH 14/30] formatting and expand examples

---
 spec/Section 2 -- Language.md   | 21 ++++++++-------
 spec/Section 5 -- Validation.md | 33 ++++++++++++++++-------
 spec/Section 6 -- Execution.md  | 47 ++++++++++++++++++++-------------
 3 files changed, 62 insertions(+), 39 deletions(-)

diff --git a/spec/Section 2 -- Language.md b/spec/Section 2 -- Language.md
index de563f1cb..633a99d29 100644
--- a/spec/Section 2 -- Language.md	
+++ b/spec/Section 2 -- Language.md	
@@ -1220,12 +1220,13 @@ size `60`:
 **Variable Use Within Fragments**
 
 Variables can be used within fragments. Operation-defined variables have global
-scope within a given operation. Fragment-defined variables have local scope within the
-fragment definition in which they are defined. A variable used within a fragment must either
-be declared in each top-level operation that transitively consumes that fragment,
-or by that same fragment as a fragment variable definition. If a variable
-referenced in a fragment is included by an operation where neither the fragment
-nor the operation defines that variable, that operation is invalid (see
+scope within a given operation. Fragment-defined variables have local scope
+within the fragment definition in which they are defined. A variable used within
+a fragment must either be declared in each top-level operation that transitively
+consumes that fragment, or by that same fragment as a fragment variable
+definition. If a variable referenced in a fragment is included by an operation
+where neither the fragment nor the operation defines that variable, that
+operation is invalid (see
 [All Variable Uses Defined](#sec-All-Variable-Uses-Defined)).
 
 ## Fragment Variable Definitions
@@ -1279,10 +1280,10 @@ fragment dynamicProfilePic($size: Int!) on User {
 ```
 
 The profilePic for `user` will be determined by the variables set by the
-operation, while `secondUser` will always have a `profilePic` of size `10`. In this
-case, the fragment `variableProfilePic` uses the operation-defined variable,
-while `dynamicProfilePic` uses the value passed in via the fragment spread's
-`size` argument.
+operation, while `secondUser` will always have a `profilePic` of size `10`. In
+this case, the fragment `variableProfilePic` uses the operation-defined
+variable, while `dynamicProfilePic` uses the value passed in via the fragment
+spread's `size` argument.
 
 ## Type References
 
diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 18421899e..074ffcf63 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -424,9 +424,10 @@ FieldsInSetCanMerge(set):
 - Let {spreadsForName} be the set of fragment spreads with a given name in
   {visitedSelections}.
 - For each {spreadsForName} as {name} and {spreads}:
-  - Each entry in {spreads} must have identical sets of arguments to each other entry in {spreads}.
-- Let {fieldsForName} be the set of field selections with a given response name in
-  {visitedSelections}.
+  - Each entry in {spreads} must have identical sets of arguments to each other
+    entry in {spreads}.
+- Let {fieldsForName} be the set of field selections with a given response name
+  in {visitedSelections}.
 - Given each pair of members {fieldA} and {fieldB} in {fieldsForName}:
   - {SameResponseShape(fieldA, fieldB)} must be true.
   - If the parent types of {fieldA} and {fieldB} are equal or if either is not
@@ -602,8 +603,9 @@ the fragment spread `...commandFragment(command: SIT)` and
 `...commandFragment(command: DOWN)` are part of the visited selections that will
 be merged.
 
-If both of these spreads would have `$commandOne` or `$commandTwo` as the argument-value,
-it would be allowed as we can be sure that we'd resolve identical fields.
+If both of these spreads would have `$commandOne` or `$commandTwo` as the
+argument-value, it would be allowed as we can be sure that we'd resolve
+identical fields.
 
 ### Leaf Field Selections
 
@@ -699,8 +701,8 @@ validation rules apply in each case.
 
 **Explanatory Text**
 
-Every argument provided to a field or directive or fragment spread must be
-defined in the set of possible arguments of that field, directive or fragment.
+Every argument provided to a field or directive must be defined in the set of
+possible arguments of that field or directive.
 
 For example the following are valid:
 
@@ -712,7 +714,13 @@ fragment argOnRequiredArg on Dog {
 fragment argOnOptional on Dog {
   isHouseTrained(atOtherHomes: true) @include(if: true)
 }
+```
 
+The above is also applicable to fragment-definitions and fragment-spreads, each
+variable must be defined by the fragment-definition before it can be inserted as
+an argument by the fragment-spread.
+
+```graphql example
 fragment withFragmentArg($command: DogCommand) on Dog {
   doesKnowCommand(dogCommand: $command)
 }
@@ -738,6 +746,10 @@ and this is also invalid as the argument `dogCommand` is not defined on fragment
 fragment invalidFragmentArgName on Dog {
   ...withFragmentArg(dogCommand: SIT)
 }
+
+fragment withFragmentArg($command: DogCommand) on Dog {
+  doesKnowCommand(dogCommand: $command)
+}
 ```
 
 and this is also invalid as `unless` is not defined on `@include`.
@@ -1620,8 +1632,8 @@ fragment HouseTrainedFragment on Query {
 }
 ```
 
-Likewise, it is valid for a fragment to define a variable with a name that
-is also defined on an operation:
+Likewise, it is valid for a fragment to define a variable with a name that is
+also defined on an operation:
 
 ```graphql example
 query C($atOtherHomes: Boolean) {
@@ -1989,7 +2001,8 @@ fragment fragmentArgUnused($atOtherHomes: Boolean) on Dog {
 }
 ```
 
-This document is invalid: fragment `fragmentArgUnused` defines a fragment variable `$atOtherHomes`, but this variable is not used within this fragment.
+This document is invalid: fragment `fragmentArgUnused` defines a fragment
+variable `$atOtherHomes`, but this variable is not used within this fragment.
 
 ### All Variable Usages Are Allowed
 
diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index 9f2a37270..1c05c5520 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -340,7 +340,8 @@ ExecuteSelectionSet(selectionSet, objectType, objectValue, variableValues):
 - For each {groupedFieldSet} as {responseKey} and {fields}:
   - Let {fieldName} be the name of the first entry in {fields}. Note: This value
     is unaffected if an alias is used.
-  - Let {fragmentVariableValues} be the fragment-variables value of the first entry in {fields}.
+  - Let {fragmentVariableValues} be the fragment-variables value of the first
+    entry in {fields}.
   - Let {fieldType} be the return type defined for the field {fieldName} of
     {objectType}.
   - If {fieldType} is defined:
@@ -493,7 +494,8 @@ The depth-first-search order of the field groups produced by {CollectFields()}
 is maintained through execution, ensuring that fields appear in the executed
 response in a stable and predictable order.
 
-CollectFields(objectType, selectionSet, variableValues, visitedFragments, localVariableValues):
+CollectFields(objectType, selectionSet, variableValues, visitedFragments,
+localVariableValues):
 
 - If {visitedFragments} is not provided, initialize it to the empty set.
 - Initialize {groupedFields} to an empty ordered map of lists.
@@ -501,13 +503,13 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments, localV
   - If {selection} provides the directive `@skip`, let {skipDirective} be that
     directive.
     - If {skipDirective}'s {if} argument is {true} or is a variable in
-      {localVariableValues} or {variableValues} with the value {true}, continue with the next {selection}
-      in {selectionSet}.
+      {localVariableValues} or {variableValues} with the value {true}, continue
+      with the next {selection} in {selectionSet}.
   - If {selection} provides the directive `@include`, let {includeDirective} be
     that directive.
     - If {includeDirective}'s {if} argument is not {true} and is not a variable
-      in {localVariableValues} or {variableValues} with the value {true}, continue with the next
-      {selection} in {selectionSet}.
+      in {localVariableValues} or {variableValues} with the value {true},
+      continue with the next {selection} in {selectionSet}.
   - If {selection} is a {Field}:
     - Let {responseKey} be the response key of {selection} (the alias if
       defined, otherwise the field name).
@@ -527,7 +529,8 @@ CollectFields(objectType, selectionSet, variableValues, visitedFragments, localV
     - If {DoesFragmentTypeApply(objectType, fragmentType)} is {false}, continue
       with the next {selection} in {selectionSet}.
     - Let {localVariableValues} be the result of calling
-      {getArgumentValuesFromSpread(selection, fragmentDefinition, variableValues, localVariableValues)}.
+      {getArgumentValuesFromSpread(selection, fragmentDefinition,
+      variableValues, localVariableValues)}.
     - Let {fragmentGroupedFieldSet} be the result of calling
       {CollectFields(objectType, fragmentSelectionSet, variableValues,
       visitedFragments)}.
@@ -566,24 +569,27 @@ DoesFragmentTypeApply(objectType, fragmentType):
   - If {objectType} is a possible type of {fragmentType}, return {true}
     otherwise return {false}.
 
-getArgumentValuesFromSpread(fragmentSpread, fragmentDefinition, variableValues, fragmentArgumentValues):
+getArgumentValuesFromSpread(fragmentSpread, fragmentDefinition, variableValues,
+fragmentArgumentValues):
 
 - Let {coercedValues} be an empty unordered Map.
 - For each {variableDefinition} in {fragmentDefinition}:
   - Let {variableName} be the name of {variableDefinition}.
   - Let {variableType} be the type of {variableDefinition}.
   - Let {defaultValue} be the default value for {variableDefinition}.
-  - Let {argumentNode} be the node provided in the fragment-spread for {variableName}
+  - Let {argumentNode} be the node provided in the fragment-spread for
+    {variableName}
   - If {argumentNode} isn't present or is null
     - If {defaultValue} exists
       - Add an entry to {coercedValues} named {argumentName} with the value
         {defaultValue}.
     - If {variableType} is non-nullable raise a field-error
-  - Let {hasValue} be {true} if {fragmentArgumentValues} or {variableValues} provides a   value for the name
-    {variableName}.
-  - If {variableType} is non-nullable and {hasValue} is {false} raise a field-error
-  - Add an entry to {coercedValues} named {argumentName} with the value
-    found in {variableValues} or {fragmentArgumentValues}.
+  - Let {hasValue} be {true} if {fragmentArgumentValues} or {variableValues}
+    provides a value for the name {variableName}.
+  - If {variableType} is non-nullable and {hasValue} is {false} raise a
+    field-error
+  - Add an entry to {coercedValues} named {argumentName} with the value found in
+    {variableValues} or {fragmentArgumentValues}.
 - Return {coercedValues}.
 
 Note: The steps in {CollectFields()} evaluating the `@skip` and `@include`
@@ -597,7 +603,8 @@ coerces any provided argument values, then resolves a value for the field, and
 finally completes that value either by recursively executing another selection
 set or coercing a scalar value.
 
-ExecuteField(objectType, objectValue, fieldType, fields, variableValues, fragmentVariableValues):
+ExecuteField(objectType, objectValue, fieldType, fields, variableValues,
+fragmentVariableValues):
 
 - Let {field} be the first entry in {fields}.
 - Let {fieldName} be the field name of {field}.
@@ -617,7 +624,8 @@ the type system to have a specific input type.
 At each argument position in an operation may be a literal {Value}, or a
 {Variable} to be provided at runtime.
 
-CoerceFieldArgumentValues(objectType, field, variableValues, fragmentVariableValues):
+CoerceFieldArgumentValues(objectType, field, variableValues,
+fragmentVariableValues):
 
 - Let {argumentValues} be the argument values provided in {field}.
 - Let {fieldName} be the name of {field}.
@@ -626,7 +634,8 @@ CoerceFieldArgumentValues(objectType, field, variableValues, fragmentVariableVal
 - Return {CoerceArgumentValues(argumentDefinitions, argumentValues,
   variableValues, fragmentVariableValues)}
 
-CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues, fragmentVariableValues):
+CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues,
+fragmentVariableValues):
 
 - For each {argumentDefinition} in {argumentDefinitions}:
   - Let {argumentName} be the name of {argumentDefinition}.
@@ -638,8 +647,8 @@ CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues, fragme
     {argumentName}.
   - If {argumentValue} is a {Variable}:
     - Let {variableName} be the name of {argumentValue}.
-    - Let {hasValue} be {true} if {fragmentVariableValues} provides a value for the name
-      {variableName}.
+    - Let {hasValue} be {true} if {fragmentVariableValues} provides a value for
+      the name {variableName}.
     - Let {value} be the value provided in {fragmentVariableValues} for the name
       {variableName}.
     - Let {hasValue} be {true} if {variableValues} provides a value for the name

From f5b30614940d2c410ce9dceadda9539e807eda5a Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sat, 30 Mar 2024 08:30:46 +0100
Subject: [PATCH 15/30] add in undefined fragment

---
 spec/Section 5 -- Validation.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 074ffcf63..81a31d7d8 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -1643,6 +1643,12 @@ query C($atOtherHomes: Boolean) {
   }
 }
 
+fragment HouseTrainedFragment on Query {
+  dog {
+    isHouseTrained(atOtherHomes: $atOtherHomes)
+  }
+}
+
 fragment HouseTrainedDog($atOtherHomes: Boolean) on Dog {
   isHouseTrained(atOtherHomes: $atOtherHomes)
 }

From df272196a24c5497a85ee7760c06f7523ba4fe20 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sat, 30 Mar 2024 08:32:18 +0100
Subject: [PATCH 16/30] unset instead of null

---
 spec/Section 5 -- Validation.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 81a31d7d8..8252d9d86 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -1658,8 +1658,8 @@ Fragment-defined variables are scoped locally to the fragment that defines them,
 and override any operation-defined variable values, so there is never ambiguity
 about which value to use. In this case, the value of the argument `atOtherHomes`
 within `HouseTrainedFragment` will be the operation-set value, and within
-`HouseTrainedDog` will resolve to `null`, as the argument is not set by the
-fragment spread in the query `C`.
+`HouseTrainedDog` will default to being unset (unless a default-value applies),
+as the argument is not set by the fragment spread in the query `C`.
 
 ### Variables Are Input Types
 

From 44aea8b8a6f11f1428a20c63d7bc5638472cadb6 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sat, 30 Mar 2024 08:34:00 +0100
Subject: [PATCH 17/30] shorten

---
 spec/Section 5 -- Validation.md | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 8252d9d86..9bb7f0077 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -419,8 +419,7 @@ fragment directFieldSelectionOnUnion on CatOrDog {
 FieldsInSetCanMerge(set):
 
 - Let {visitedSelections} be the selections in {set} including visiting
-  fragments and inline fragments and applying any supplied fragment spread
-  arguments.
+  fields, fragment-spreads and inline fragments.
 - Let {spreadsForName} be the set of fragment spreads with a given name in
   {visitedSelections}.
 - For each {spreadsForName} as {name} and {spreads}:

From 61d2117a62961ae6360b28c02df3eeec2c0419aa Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 16 May 2024 08:41:18 +0200
Subject: [PATCH 18/30] Apply suggestions from code review

Co-authored-by: Benjie <benjie@jemjie.com>
---
 spec/Section 5 -- Validation.md | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 9bb7f0077..1ebc27e86 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -1768,7 +1768,7 @@ query variableIsNotDefined {
 ${atOtherHomes} is not defined by the operation.
 
 Fragments complicate this rule. Any fragment transitively included by an
-operation has access to the variables defined by that operation and defined on
+operation has access to the variables defined by that operation and also those defined on
 the fragment. Fragments can appear within multiple operations and therefore
 variable usages not defined on the fragment must correspond to variable
 definitions in all of those operations.
@@ -1931,8 +1931,7 @@ fragment isHouseTrainedWithoutVariableFragment on Dog {
 Fragment arguments can shadow operation variables: fragments that use an
 argument are not using the operation-defined variable of the same name.
 
-Likewise, it would be invalid if the variable was shadowed by a fragment
-argument:
+As such, it would be invalid if the operation defined a variable and variables of that name were used exclusively inside fragments that define a variable with the same name:
 
 ```graphql counter-example
 query variableNotUsedWithinFragment($atOtherHomes: Boolean) {
@@ -1982,8 +1981,7 @@ variable.
 
 - For every {fragment} in the document:
   - Let {variables} be the variables defined by that {fragment}.
-  - Each {variable} in {variables} must be used at least once in the fragment's
-    scope.
+  - Each {variable} in {variables} must be used at least once transitively within the fragment's selection set excluding traversal of named fragment spreads.
 
 **Explanatory Text**
 

From 815952a61567a3572ff26a835bb5b31190186890 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 16 May 2024 08:42:50 +0200
Subject: [PATCH 19/30] remove non standard hyphens

---
 spec/Section 5 -- Validation.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 1ebc27e86..bde55d64e 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -715,9 +715,9 @@ fragment argOnOptional on Dog {
 }
 ```
 
-The above is also applicable to fragment-definitions and fragment-spreads, each
-variable must be defined by the fragment-definition before it can be inserted as
-an argument by the fragment-spread.
+The above is also applicable to fragment definitions and fragment spreads, each
+variable must be defined by the fragment definition before it can be inserted as
+an argument by the fragment spread.
 
 ```graphql example
 fragment withFragmentArg($command: DogCommand) on Dog {

From 21e388177b6b80b5a5db3fc7404899b698f5c185 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 16 May 2024 19:00:00 +0200
Subject: [PATCH 20/30] address redundant if

---
 spec/Section 5 -- Validation.md | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index bde55d64e..bc1307417 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -692,10 +692,9 @@ validation rules apply in each case.
 
 - For each {argument} in the document:
   - Let {argumentName} be the Name of {argument}.
-  - If the parent is a field or directive:
   - Let {argumentDefinition} be the argument or variable definition named
     {argumentName} provided by the parent field definition, directive definition
-    or fragment definition.
+    or fragment spread.
   - {argumentDefinition} must exist.
 
 **Explanatory Text**

From b60feace9f6161c4bacd62f74d1b4078767c1c5d Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 16 May 2024 19:02:02 +0200
Subject: [PATCH 21/30] address logical or

---
 spec/Section 5 -- Validation.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index bc1307417..3170c55ab 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -1732,8 +1732,8 @@ query takesCatOrDog($catOrDog: CatOrDog) {
   - Let {fragments} be every fragment referenced by that {operation}
     transitively.
   - For each {fragment} in {fragments}:
-    - For each {variableUsage} in scope of {fragment}, variable must be in
-      {fragment}'s or {operation}'s variable list.
+    - For each {variableUsage} in scope of {fragment}, variable must be in either
+      {fragment}'s or {operation}'s variable list or both.
 
 **Explanatory Text**
 

From 3516ca153c6b6da74ab2c48dde7098f993953175 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 16 May 2024 19:06:03 +0200
Subject: [PATCH 22/30] clarify example

---
 spec/Section 5 -- Validation.md | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 3170c55ab..718924814 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -591,15 +591,17 @@ fragment potentiallyConflictingArguments(
   ...commandFragment(command: $commandTwo)
 }
 
-fragment safeFragmentArguments on Dog {
-  ...potentiallyConflictingArguments(commandOne: SIT, commandTwo: DOWN)
+query {
+  pet {
+    ...potentiallyConflictingArguments(commandOne: SIT, commandTwo: DOWN)
+  }
 }
 ```
 
-If two fragment spreads with the same name supply different argument values,
-their fields will not be able to merge. In this case, validation fails because
-the fragment spread `...commandFragment(command: SIT)` and
-`...commandFragment(command: DOWN)` are part of the visited selections that will
+If two fragment spreads with the same name, and hence the same selection,
+supply different argument values, their fields will not be able to merge.
+In this case, validation fails because the fragment spread `...commandFragment(command: SIT)`
+and `...commandFragment(command: DOWN)` are part of the visited selections that will
 be merged.
 
 If both of these spreads would have `$commandOne` or `$commandTwo` as the

From f306736273e1fee0be1841f8c3aa14770d5679f4 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Fri, 17 May 2024 09:36:28 +0200
Subject: [PATCH 23/30] Ensure variables are defined

---
 spec/Section 5 -- Validation.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 718924814..17c703987 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -586,6 +586,7 @@ fragment commandFragment($command: DogCommand!) on Dog {
 
 fragment potentiallyConflictingArguments(
   $commandOne: DogCommand!
+  $commandTwo: DogCommand!
 ) on Dog {
   ...commandFragment(command: $commandOne)
   ...commandFragment(command: $commandTwo)

From 94a00276763055e8902f1915b7cdba38d8d41213 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 23 May 2024 18:46:08 +0200
Subject: [PATCH 24/30] fix formatting

---
 spec/Section 5 -- Validation.md | 34 ++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 17c703987..9a4a66310 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -418,8 +418,8 @@ fragment directFieldSelectionOnUnion on CatOrDog {
 
 FieldsInSetCanMerge(set):
 
-- Let {visitedSelections} be the selections in {set} including visiting
-  fields, fragment-spreads and inline fragments.
+- Let {visitedSelections} be the selections in {set} including visiting fields,
+  fragment-spreads and inline fragments.
 - Let {spreadsForName} be the set of fragment spreads with a given name in
   {visitedSelections}.
 - For each {spreadsForName} as {name} and {spreads}:
@@ -599,11 +599,11 @@ query {
 }
 ```
 
-If two fragment spreads with the same name, and hence the same selection,
-supply different argument values, their fields will not be able to merge.
-In this case, validation fails because the fragment spread `...commandFragment(command: SIT)`
-and `...commandFragment(command: DOWN)` are part of the visited selections that will
-be merged.
+If two fragment spreads with the same name, and hence the same selection, supply
+different argument values, their fields will not be able to merge. In this case,
+validation fails because the fragment spread `...commandFragment(command: SIT)`
+and `...commandFragment(command: DOWN)` are part of the visited selections that
+will be merged.
 
 If both of these spreads would have `$commandOne` or `$commandTwo` as the
 argument-value, it would be allowed as we can be sure that we'd resolve
@@ -1735,8 +1735,8 @@ query takesCatOrDog($catOrDog: CatOrDog) {
   - Let {fragments} be every fragment referenced by that {operation}
     transitively.
   - For each {fragment} in {fragments}:
-    - For each {variableUsage} in scope of {fragment}, variable must be in either
-      {fragment}'s or {operation}'s variable list or both.
+    - For each {variableUsage} in scope of {fragment}, variable must be in
+      either {fragment}'s or {operation}'s variable list or both.
 
 **Explanatory Text**
 
@@ -1770,10 +1770,10 @@ query variableIsNotDefined {
 ${atOtherHomes} is not defined by the operation.
 
 Fragments complicate this rule. Any fragment transitively included by an
-operation has access to the variables defined by that operation and also those defined on
-the fragment. Fragments can appear within multiple operations and therefore
-variable usages not defined on the fragment must correspond to variable
-definitions in all of those operations.
+operation has access to the variables defined by that operation and also those
+defined on the fragment. Fragments can appear within multiple operations and
+therefore variable usages not defined on the fragment must correspond to
+variable definitions in all of those operations.
 
 For example the following is valid:
 
@@ -1933,7 +1933,9 @@ fragment isHouseTrainedWithoutVariableFragment on Dog {
 Fragment arguments can shadow operation variables: fragments that use an
 argument are not using the operation-defined variable of the same name.
 
-As such, it would be invalid if the operation defined a variable and variables of that name were used exclusively inside fragments that define a variable with the same name:
+As such, it would be invalid if the operation defined a variable and variables
+of that name were used exclusively inside fragments that define a variable with
+the same name:
 
 ```graphql counter-example
 query variableNotUsedWithinFragment($atOtherHomes: Boolean) {
@@ -1983,7 +1985,9 @@ variable.
 
 - For every {fragment} in the document:
   - Let {variables} be the variables defined by that {fragment}.
-  - Each {variable} in {variables} must be used at least once transitively within the fragment's selection set excluding traversal of named fragment spreads.
+  - Each {variable} in {variables} must be used at least once transitively
+    within the fragment's selection set excluding traversal of named fragment
+    spreads.
 
 **Explanatory Text**
 

From 0361664c0f34348ef0b597abedf1e87a8f82ad94 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Tue, 4 Jun 2024 13:49:20 +0200
Subject: [PATCH 25/30] Update spec/Section 5 -- Validation.md

Co-authored-by: Benjie <benjie@jemjie.com>
---
 spec/Section 5 -- Validation.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 9a4a66310..e7558b452 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -579,7 +579,7 @@ fragment conflictingDifferingResponses on Pet {
 
 Fragment spread arguments can also cause fields to fail to merge.
 
-```graphql example
+```graphql counter-example
 fragment commandFragment($command: DogCommand!) on Dog {
   doesKnowCommand(dogCommand: $command)
 }

From cbf20d04fd28a894741cfd97480f32ffc782a3b6 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 6 Jun 2024 19:01:56 +0200
Subject: [PATCH 26/30] Update spec/Section 5 -- Validation.md

Co-authored-by: Benjie <benjie@jemjie.com>
---
 spec/Section 5 -- Validation.md | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index e7558b452..b5efa0c17 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -605,9 +605,8 @@ validation fails because the fragment spread `...commandFragment(command: SIT)`
 and `...commandFragment(command: DOWN)` are part of the visited selections that
 will be merged.
 
-If both of these spreads would have `$commandOne` or `$commandTwo` as the
-argument-value, it would be allowed as we can be sure that we'd resolve
-identical fields.
+If both of these spreads had used `$commandOne` as the argument value, it would
+be allowed as we can be sure that we'd resolve identical fields.
 
 ### Leaf Field Selections
 

From b0ac79907d5e8edfdc5cd8c9cb9c55f0de1a81e1 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sat, 8 Jun 2024 07:57:46 +0200
Subject: [PATCH 27/30] wording

---
 spec/Section 5 -- Validation.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index b5efa0c17..f81a3cec9 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -605,7 +605,7 @@ validation fails because the fragment spread `...commandFragment(command: SIT)`
 and `...commandFragment(command: DOWN)` are part of the visited selections that
 will be merged.
 
-If both of these spreads had used `$commandOne` as the argument value, it would
+If both of these spreads had used the same variable for the argument value, it would
 be allowed as we can be sure that we'd resolve identical fields.
 
 ### Leaf Field Selections

From 4edb48144332b4c35a47a240af5943d56739a259 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sun, 9 Jun 2024 07:52:34 +0200
Subject: [PATCH 28/30] formatting

---
 spec/Section 5 -- Validation.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 0986c94e4..5ba39bc7c 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -609,8 +609,8 @@ validation fails because the fragment spread `...commandFragment(command: SIT)`
 and `...commandFragment(command: DOWN)` are part of the visited selections that
 will be merged.
 
-If both of these spreads had used the same variable for the argument value, it would
-be allowed as we can be sure that we'd resolve identical fields.
+If both of these spreads had used the same variable for the argument value, it
+would be allowed as we can be sure that we'd resolve identical fields.
 
 ### Leaf Field Selections
 

From 641e3d964068c91ba06a1776f9db3bed34c60aaf Mon Sep 17 00:00:00 2001
From: jdecroock <decroockjovi@gmail.com>
Date: Wed, 4 Sep 2024 18:25:39 +0200
Subject: [PATCH 29/30] update validation

---
 spec/Section 5 -- Validation.md | 52 ++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index 5ba39bc7c..d345cc273 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -609,8 +609,29 @@ validation fails because the fragment spread `...commandFragment(command: SIT)`
 and `...commandFragment(command: DOWN)` are part of the visited selections that
 will be merged.
 
-If both of these spreads had used the same variable for the argument value, it
-would be allowed as we can be sure that we'd resolve identical fields.
+If both of these spreads had used the same value for the argument value, it
+would be allowed as we can be sure that we would resolve identical fields.
+Spreads that use different variables that would always resolve to the same value
+are also valid. For example, the following is valid:
+
+```graphql example
+fragment commandFragment($command: DogCommand!) on Dog {
+  doesKnowCommand(dogCommand: $command)
+}
+
+fragment noConflictWhenPassedOperationCommand(
+  $fragmentCommand: DogCommand!
+) on Dog {
+  ...commandFragment(command: $operationCommand)
+  ...commandFragment(command: $fragmentCommand)
+}
+
+query($operationCommand: DogCommand!) {
+  pet {
+    ...noConflictWhenPassedOperationCommand(fragmentCommand: $operationCommand)
+  }
+}
+```
 
 ### Leaf Field Selections
 
@@ -1873,7 +1894,7 @@ This is because {houseTrainedQueryTwoNotDefined} does not define a variable
 ${atOtherHomes} but that variable is used by {isHouseTrainedFragment} which is
 included in that operation.
 
-### All Operation Variables Used
+### All Variables Used
 
 **Formal Specification**
 
@@ -1882,6 +1903,11 @@ included in that operation.
   - Each {variable} in {variables} must be used at least once in either the
     operation scope itself or any fragment transitively referenced by that
     operation, excluding fragments that define the same name as an argument.
+- For every {fragment} in the document:
+  - Let {variables} be the variables defined by that {fragment}.
+  - Each {variable} in {variables} must be used at least once transitively
+    within the fragment's selection set excluding traversal of named fragment
+    spreads.
 
 **Explanatory Text**
 
@@ -1982,24 +2008,8 @@ fragment isHouseTrainedFragment on Dog {
 This document is not valid because {queryWithExtraVar} defines an extraneous
 variable.
 
-### All Fragment Variables Used
-
-**Formal Specification**
-
-- For every {fragment} in the document:
-  - Let {variables} be the variables defined by that {fragment}.
-  - Each {variable} in {variables} must be used at least once transitively
-    within the fragment's selection set excluding traversal of named fragment
-    spreads.
-
-**Explanatory Text**
-
-All variables defined by a fragment must be used in that same fragment. Because
-fragment-defined variables are scoped to the fragment they are defined on, if
-the fragment does not use the variable, then the variable definition is
-superfluous.
-
-For example, the following is invalid:
+Fragment variables must also be used within their definitions. For example, the
+following is invalid:
 
 ```graphql counter-example
 query queryWithFragmentArgUnused($atOtherHomes: Boolean) {

From 9ef0002347029034595726bf7f42c8d36e9177c2 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Sat, 15 Feb 2025 11:32:24 +0100
Subject: [PATCH 30/30] Partial update, rest depends on
 https://github.com/graphql/graphql-spec/pull/1039

---
 spec/Section 6 -- Execution.md | 114 ++++++++++++++++++---------------
 1 file changed, 61 insertions(+), 53 deletions(-)

diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index fd3aa7caf..96e857866 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -86,10 +86,8 @@ CoerceVariableValues(schema, operation, variableValues):
 - Let {coercedValues} be an empty unordered Map.
 - Let {variablesDefinition} be the variables defined by {operation}.
 - For each {variableDefinition} in {variablesDefinition}:
-  - Let {variableName} be the name of {variableDefinition}.
-  - Let {variableType} be the expected type of {variableDefinition}.
-  - Assert: {IsInputType(variableType)} must be {true}.
-  - Let {defaultValue} be the default value for {variableDefinition}.
+  - Let {variableName}, {variableType}, and {defaultValue} be the result of
+    {GetVariableSignature(variableDefinition)}.
   - Let {hasValue} be {true} if {variableValues} provides a value for the name
     {variableName}.
   - Let {value} be the value provided in {variableValues} for the name
@@ -114,6 +112,14 @@ CoerceVariableValues(schema, operation, variableValues):
 
 Note: This algorithm is very similar to {CoerceArgumentValues()}.
 
+GetVariableSignature(variableDefinition):
+
+- Let {variableName} be the name of {variableDefinition}.
+- Let {variableType} be the expected type of {variableDefinition}.
+- Assert: {IsInputType(variableType)} must be {true}.
+- Let {defaultValue} be the default value for {variableDefinition}.
+- Return {variableName}, {variableType}, and {defaultValue}.
+
 ## Executing Operations
 
 The type system, as described in the "Type System" section of the spec, must
@@ -498,45 +504,54 @@ is maintained through execution, ensuring that fields appear in the executed
 response in a stable and predictable order.
 
 CollectFields(objectType, selectionSet, variableValues, visitedFragments,
-localVariableValues):
+fragmentVariables):
 
 - If {visitedFragments} is not provided, initialize it to the empty set.
 - Initialize {groupedFields} to an empty ordered map of lists.
 - For each {selection} in {selectionSet}:
   - If {selection} provides the directive `@skip`, let {skipDirective} be that
     directive.
-    - If {skipDirective}'s {if} argument is {true} or is a variable in
-      {localVariableValues} or {variableValues} with the value {true}, continue
-      with the next {selection} in {selectionSet}.
+    - Let {directiveValues} be the result of {GetDirectiveValues(skipDirective,
+      variableValues, fragmentVariables)}.
+    - If the entry for the {if} argument within {directiveValues} is {true},
+      continue with the next {selection} in {selectionSet}.
   - If {selection} provides the directive `@include`, let {includeDirective} be
     that directive.
-    - If {includeDirective}'s {if} argument is not {true} and is not a variable
-      in {localVariableValues} or {variableValues} with the value {true},
-      continue with the next {selection} in {selectionSet}.
+    - Let {directiveValues} be the result of
+      {GetDirectiveValues(includeDirective, variableValues, fragmentVariables)}.
+    - If the entry for the {if} argument within {directiveValues} is not {true},
   - If {selection} is a {Field}:
     - Let {responseKey} be the response key of {selection} (the alias if
       defined, otherwise the field name).
     - Let {groupForResponseKey} be the list in {groupedFields} for
       {responseKey}; if no such list exists, create it as an empty list.
-    - Append {selection} and {localVariableValues} to the {groupForResponseKey}.
+    - Let {fieldDetails} be a new unordered map containing {fragmentVariables}.
+    - Set the entry for {field} on {fieldDetails} to {selection}.
+    - Append {fieldDetails} to the {groupForResponseKey}.
   - If {selection} is a {FragmentSpread}:
     - Let {fragmentSpreadName} be the name of {selection}.
+    - If {fragmentSpreadName} is in {visitedFragments}, continue with the next
+      {selection} in {selectionSet}.
+    - Add {fragmentSpreadName} to {visitedFragments}.
     - Let {fragment} be the Fragment in the current Document whose name is
       {fragmentSpreadName}.
     - If no such {fragment} exists, continue with the next {selection} in
       {selectionSet}.
-    - If {fragmentSpreadName} is in {visitedFragments}, continue with the next
-      {selection} in {selectionSet}.
-    - Add {fragmentSpreadName} to {visitedFragments}.
     - Let {fragmentType} be the type condition on {fragment}.
     - If {DoesFragmentTypeApply(objectType, fragmentType)} is {false}, continue
       with the next {selection} in {selectionSet}.
-    - Let {localVariableValues} be the result of calling
-      {getArgumentValuesFromSpread(selection, fragmentDefinition,
-      variableValues, localVariableValues)}.
+    - Let {variableDefinitions} be the variable definitions for {fragment}.
+    - Initialize {signatures} to an empty list.
+    - For each {variableDefinition} of {variableDefinitions}:
+      - Append the result of {GetVariableSignature(variableDefinition)} to
+        {signatures}.
+    - Let {values} be the result of {CoerceArgumentValues(fragment,
+      argumentDefinitions, variableValues, fragmentVariables)}.
+    - Let {newFragmentVariables} be an unordered map containing {signatures} and
+      {values}.
     - Let {fragmentGroupedFieldSet} be the result of calling
       {CollectFields(objectType, fragmentSelectionSet, variableValues,
-      visitedFragments)}.
+      visitedFragments, newFragmentVariables)}.
     - For each {fragmentGroup} in {fragmentGroupedFieldSet}:
       - Let {responseKey} be the response key shared by all fields in
         {fragmentGroup}.
@@ -551,7 +566,7 @@ localVariableValues):
     - Let {fragmentSelectionSet} be the top-level selection set of {selection}.
     - Let {fragmentGroupedFieldSet} be the result of calling
       {CollectFields(objectType, fragmentSelectionSet, variableValues,
-      visitedFragments)}.
+      visitedFragments, fragmentVariables)}.
     - For each {fragmentGroup} in {fragmentGroupedFieldSet}:
       - Let {responseKey} be the response key shared by all fields in
         {fragmentGroup}.
@@ -572,32 +587,19 @@ DoesFragmentTypeApply(objectType, fragmentType):
   - If {objectType} is a possible type of {fragmentType}, return {true}
     otherwise return {false}.
 
-getArgumentValuesFromSpread(fragmentSpread, fragmentDefinition, variableValues,
-fragmentArgumentValues):
-
-- Let {coercedValues} be an empty unordered Map.
-- For each {variableDefinition} in {fragmentDefinition}:
-  - Let {variableName} be the name of {variableDefinition}.
-  - Let {variableType} be the type of {variableDefinition}.
-  - Let {defaultValue} be the default value for {variableDefinition}.
-  - Let {argumentNode} be the node provided in the fragment-spread for
-    {variableName}
-  - If {argumentNode} isn't present or is null
-    - If {defaultValue} exists
-      - Add an entry to {coercedValues} named {argumentName} with the value
-        {defaultValue}.
-    - If {variableType} is non-nullable raise a field-error
-  - Let {hasValue} be {true} if {fragmentArgumentValues} or {variableValues}
-    provides a value for the name {variableName}.
-  - If {variableType} is non-nullable and {hasValue} is {false} raise a
-    field-error
-  - Add an entry to {coercedValues} named {argumentName} with the value found in
-    {variableValues} or {fragmentArgumentValues}.
-- Return {coercedValues}.
-
 Note: The steps in {CollectFields()} evaluating the `@skip` and `@include`
 directives may be applied in either order since they apply commutatively.
 
+GetDirectiveValues(directive, variableValues, fragmentVariables):
+
+- Let {directiveName} be the name of {directive}.
+- Let {directiveDefinition} be the definition for {directiveName} within the
+  schema.
+- Assert {directiveDefinition} is defined.
+- Let {argumentDefinitions} be the arguments defined by {directiveDefinition}.
+- Return the result of {CoerceArgumentValues(directiveDefinition,
+  argumentDefinitions, variableValues, fragmentVariables)}.
+
 ## Executing Fields
 
 Each field requested in the grouped field set that is defined on the selected
@@ -606,6 +608,8 @@ coerces any provided argument values, then resolves a value for the field, and
 finally completes that value either by recursively executing another selection
 set or coercing a scalar value.
 
+### TODO: this needs updating
+
 ExecuteField(objectType, objectValue, fieldType, fields, variableValues,
 fragmentVariableValues):
 
@@ -618,11 +622,13 @@ fragmentVariableValues):
 - Return the result of {CompleteValue(fieldType, fields, resolvedValue,
   variableValues)}.
 
-### Coercing Field Arguments
+### Coercing Arguments
 
-Fields may include arguments which are provided to the underlying runtime in
-order to correctly produce a value. These arguments are defined by the field in
-the type system to have a specific input type.
+Fields, directives, and fragment spreads may include arguments which are
+provided to the underlying runtime in order to correctly produce a value. For
+fields and directives, these arguments are defined by the field or directive in
+the type system to have a specific input type; for fragment spreads, the
+fragment definition within the document specifies the input type.
 
 At each argument position in an operation may be a literal {Value}, or a
 {Variable} to be provided at runtime.
@@ -637,7 +643,7 @@ fragmentVariableValues):
 - Return {CoerceArgumentValues(argumentDefinitions, argumentValues,
   variableValues, fragmentVariableValues)}
 
-CoerceArgumentValues(argumentDefinitions, argumentValues, variableValues,
+CoerceArgumentValues(node, argumentDefinitions, argumentValues, variableValues,
 fragmentVariableValues):
 
 - For each {argumentDefinition} in {argumentDefinitions}:
@@ -650,12 +656,12 @@ fragmentVariableValues):
     {argumentName}.
   - If {argumentValue} is a {Variable}:
     - Let {variableName} be the name of {argumentValue}.
-    - Let {hasValue} be {true} if {fragmentVariableValues} provides a value for
+    - Let {signatures} and {values} be the corresponding entries on
+      {fragmentVariables}.
+    - Let {scopedVariableValues} be {values} if an entry in {signatures} exists
+      for {variableName}; otherwise, let it be {variableValues}.
+    - Let {hasValue} be {true} if {scopedVariableValues} provides a value for
       the name {variableName}.
-    - Let {value} be the value provided in {fragmentVariableValues} for the name
-      {variableName}.
-    - Let {hasValue} be {true} if {variableValues} provides a value for the name
-      {variableName}.
     - Let {value} be the value provided in {variableValues} for the name
       {variableName}.
   - Otherwise, let {value} be {argumentValue}.
@@ -713,6 +719,8 @@ After resolving the value for a field, it is completed by ensuring it adheres to
 the expected return type. If the return type is another Object type, then the
 field execution process continues recursively.
 
+### TODO: needs updating with fieldDetails
+
 CompleteValue(fieldType, fields, result, variableValues):
 
 - If the {fieldType} is a Non-Null type: