Forward experiment: and array-length validation to aggregation metric parameters

Problem

declare_parameter_arguments in app/graphql/types/analytics/aggregation/base_response_type.rb turns an aggregation-engine metric parameter into a GraphQL argument, but it forwards only type, array and description. Two consequences follow, and both affect every engine that declares metric parameters, not any single one.

1. A metric parameter cannot be marked experimental

The GraphQL guidelines say to mark a new argument with experiment: { milestone: 'X.Y' } so there is a rollback path other than a deprecation cycle. Passing experiment: inside a parameter hash today is silently dropped, so every new metric parameter lands as stable public API the moment it merges.

Affected arguments already in the schema:

2. Array parameters have no explicit length validation

The guidelines also ask for validates: { length: { maximum: Types::BaseArgument::MAX_ARRAY_SIZE } } on array arguments, with the limit stated in the description. declare_parameter_arguments has no way to pass validates:.

This is not an unguarded hole. The response type derives from Types::BaseObject, so its argument_class is Types::BaseArgument, whose add_automatic_array_validation! caps every array argument at MAX_ARRAY_SIZE (1000). What is missing is the explicit declaration and the documented limit.

Worth noting alongside this: ParameterizedDefinition#validate_parameters checks membership against the in: allowlist but not list size or uniqueness, so a 1000-element array of the same valid value passes. The SQL is harmless, but instance_key joins the raw values with _, so the generated column alias grows with the input.

3. An empty array silently returns the unfiltered total

Confirmed by running the engine directly against a three-row fixture: totalCount(event: []) returns 3, the grand total, rather than 0.

Count#parameter_conditions calls next if values.blank?, so an empty list contributes no condition and the metric falls back to a plain count(*). instance_parameter does not help, because [] is truthy in Ruby and passes its return unless val guard.

This is the counterpart, on the inclusion side, of the documented c NOT IN () -> 1=1 trap. It matters more here than for a filter, because a UI multi-select that the user clears will render the global total under a per-event label.

Two smaller consequences of the same input:

  • The instance key degrades to a hash. The parameter postfix is "", which fails the /\A\w+\z/ check, so the key becomes total_count_e3b0c. event: [] and event: null produce the same key, so requesting both as aliases in one query trips the duplicate identifier validation.
  • Deduplicating inside a formatter: would not fix the related alias growth, because instance_key is built from the raw configuration values, not the formatter's output. Any size or uniqueness cap has to apply before the formatter runs.

All three engines that declare metric parameters behave this way today.

Proposal

Extend declare_parameter_arguments to forward experiment: when a parameter declares it, and to apply an explicit array-length validation for array: true parameters.

Then decide, per existing argument, whether any can still be marked experimental. Arguments already public must not be marked retroactively, so this is probably only useful for parameters added after the change.

Links

Edited by Chandra Saripaka