Skip to content

Conversation

@dkorittki
Copy link
Contributor

@dkorittki dkorittki commented Feb 2, 2026

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Provide ability to disable field arg mapping
  • Wait for feat: improve memory management & request deduplication #1336 to be merged first, then basing this branch onto those changes

Context

This is WIP.

Extends the engines variable normalizer to map field arguments as a side product of it's actual work and return the results alongside the file upload mapping.

This adds a useful feature which the router uses, called field argument mapping. It's described in detail in the corresponding router pull request.

The principle of field argument mapping is to allow Cosmo Streams hook users to be able to access values of field arguments to filter subscription events based on this. The idea is to give access the values via operation variables.
Values of any field arguments are available via variables, either because the users already use variables or because they have been extracted from literal values via variable extraction. The map that is being build holds the path to the field argument as a dot notated string and the variable name where the argument value is hold. Later on, when hook users want to access the value, the variable name can be resolved from the argument path and we can provide the variable value.

Here's an example:

query GetUser($userId: ID!) {
    user(id: $userId) {
        name
        orders(limit:12) {
            id
        }
    }
}`,

The map thats being build is:

map[argumentPath]variableName
"query.user.id", "userId"
"query.user.orders.limit", "a"

If a hook user later accesses query.user.orders.limit we resolve the value of variable a to get 12.
If the hook accesses query.user.id we can resolve userId, even if the engine performed variable remapping (userId being renamed to a or b, etc.)
This works because the variables which later are accessed are the ones from the operation context from the router and those are unaffected of variable remapping. In other words: The router config option engine.disable_variables_remapping does not have an effect on this. It works regardless.

The first implementation of this feature was done solely on the router. It worked without engine changes. A newly created walker would walk the operation AST and collect and map all field arguments. While this worked the problem was the introduction of a new walker during the request processing right in the hot path and it wasn't cachable either.

With this pull request the actual walking is done by an already existing visitor as a side product: variablesExtractionVisitor. This visitor is part of the variable normalizer and already walks field arguments. I just placed the new functionality into the visitors EnterArgument hook.

The variable normalizer does it's work by invoking the NormalizeOperation method. This method, in it's current form, returns the file upload mapping, which is cached by the router. This method has been modified to extend the return type by the field arg mapping results. Since the router already cached the result we get caching field arg mapping for free.

Field Argument Mapping can be enabled or disabled when the variable normalizer is instanciated.

func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer

This option is then passed down to the variablesExtractionVisitor, where the field argument mapping is happening.
The same visitor exists for the operation normalizer. There, however, we disable field argument mapping because it's a useful feature only on the variable normalizer.

The router makes sure to only pass normalized operations
to the variable normalizer. This means in practice that both,
named and inlined fragments, have already been inlined into
the query and the variable normalizer visitor never sees any
fragments. Therefore we don't need to explicitely support
them for field argument mapping. I removed it because
it would be expensive (if used), adds code complexity and
in practice is dead code.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 2, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dominik/eng-8826-add-field-argument-mapping-support-to-engine

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Adds the ability to enable or disable field arg mapping
when the variable normalizer is created. Since this
option is only useful on the variable normalizer it is
disabled in code for the variable extraction on the
operation normalizer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants