Introduce QueryContext to replace magic GraphQL context keys - #1341
Merged
Conversation
myronmarston
requested review from
BrianSigafoos-SQ,
bsorbo,
ellisandrews-toast,
jwils,
jwondrusch,
marcdaniels-toast and
rossroberts-toast
as code owners
August 12, 2026 01:52
…1339 Replaces untyped context hash keys (:elastic_graph_schema, :datastore_search_router, :datastore_query_cache, :elastic_graph_query_tracker, :monotonic_clock_deadline, :elastic_graph_client, :http_request) with typed accessors on a QueryContext < GraphQL::Query::Context. - QueryContext.new_class closes an anonymous subclass over elastic_graph_schema and datastore_search_router, registered as the schema's context_class. - monotonic_clock_deadline/elastic_graph_client/http_request are read-only attrs, set once via QueryContext#register_elastic_graph_values (defaults: elastic_graph_client: Client::ANONYMOUS, others nil) called by each query-building call site right after building the query. - :visibility_profile stays in the context hash (needed at schema-boot time, before a QueryContext exists). - No dedicated unit spec: existing seam-level specs already give 100% coverage, and a direct unit test would pin internal APIs unnecessarily.
myronmarston
force-pushed
the
worktree-gh-1339
branch
from
August 12, 2026 02:26
76b9c7b to
64fa156
Compare
ellisandrews-toast
approved these changes
Aug 13, 2026
PR-1341 followup (review comment from Ellis): raise a helpful ConfigError instead of silently returning nil if custom resolver code still reads a key that moved to a typed accessor.
myronmarston
enabled auto-merge
August 14, 2026 02:28
myronmarston
added a commit
that referenced
this pull request
Aug 18, 2026
Closes #1340. Stacked on #1341. A resolver that builds its datastore query from a lookahead can discover that a *descendant* field is invalid — `approximatePercentile(percentile: 150)`, say. It can't raise `GraphQL::ExecutionError` there: that fails the recording field's entire subtree instead of the one bad leaf. So it records a **lookahead error**, and ElasticGraph fails the flagged field when execution reaches it. ## The API ```ruby # From any resolver, while it is being resolved: context.record_lookahead_error(node, message, extensions: nil) # For a resolver flagging *itself*, to report it the same way a descendant would be reported: context.matching_lookahead_error ``` That's the whole surface. `Resolvers::GraphQLAdapterBuilder`'s resolver lambdas check for a matching record before resolving, and return a `GraphQL::ExecutionError` built from the record instead of running the registered resolver — halting just that field or subtree, leaving siblings and other list elements alone. ## How a record is matched Each record is keyed by the flagged node's **absolute, index-free response path**, found by walking down from `context.query.lookahead` to the node's AST nodes. A field matches when its `context.current_path`, with list indices dropped, equals that path. `docs/adr/0001-lookahead-error-record-key.md` records the rationale and the alternatives considered. Two consequences worth calling out: - **One record can match many response positions** — every bucket of an aggregation, for instance — each producing its own error at its own path. That's what the GraphQL spec asks for, and it's what lets a client tell an errored `null` from a real one. Nothing is lost: a lookahead error is a property of the query, not of the data. - **Datastore query memoization needs no involvement.** A cache hit implies identical `lookahead.ast_nodes` (which hash by identity), hence the same document position, hence the same index-free path as the record made on the miss. ## Misuse is caught at the call site `record_lookahead_error` raises `Errors::ConfigError` if the node isn't selected at or beneath the field being resolved. That covers a node from an unrelated part of the query and a typo'd `Lookahead#selection` name (which returns a null object) alike, so there's no after-the-fact bug detection to maintain. ## Replaces the `approximatePercentile` workaround `Aggregation::QueryAdapter#computation_for` records the error rather than silently omitting the computation, and `Aggregation::Resolvers::AggregatedValues#resolve` no longer needs its own parallel validation and error-path logic. ## Testing - `spec/unit/elastic_graph/graphql/lookahead_error_recording_spec.rb` exercises the mechanism end-to-end through `register_graphql_resolver` + `graphql_query_executor.execute` against static data (no datastore), covering: multi-alias leaf halting, object-subtree halting, self-flagging, a record execution never reaches, and both misuse cases. - `spec/acceptance/aggregations_spec.rb` covers the real `approximatePercentile` path against a live datastore, grouped so that multiple buckets each get their own error. - `script/run_specs`, `script/type_check`, `script/lint`, `script/spellcheck` all clean; 100% line and branch coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
:elastic_graph_schema,:datastore_search_router,:datastore_query_cache,:elastic_graph_query_tracker,:monotonic_clock_deadline,:elastic_graph_client,:http_request) with typed accessors on a newElasticGraph::GraphQL::QueryContext < GraphQL::Query::Context.QueryContext.new_classcloses an anonymous subclass overelastic_graph_schema/datastore_search_router, registered as the schema'scontext_class.monotonic_clock_deadline/elastic_graph_client/http_requestare read-only attrs set once viaQueryContext#register_elastic_graph_values(defaults:elastic_graph_client: Client::ANONYMOUS, othersnil), called by each query-building call site immediately after building the query.:visibility_profilestays in the context hash since it's needed at schema-boot time, before aQueryContextexists.Closes #1339
Test plan
script/lintscript/type_checkscript/run_gem_specs elasticgraph-graphql(100% coverage)script/run_gem_specs elasticgraph-apollo(100% coverage)script/run_gem_specs elasticgraph-query_registry(100% coverage)script/run_gem_specs elasticgraph-query_interceptor(100% coverage)script/quick_build