Pometry / Pometry/Raphtory

Use GraphQL types directly for query variables instead of serde JSON

Open
#2,720 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
642
Forks
70
Avg merge
2d 13h
Merged PRs (30d)
21

Description

The wire shape of a query variable is defined twice: the client writes it with serde (#[serde(rename)], custom Serialize impls) and the server reads it with async-graphql (#[graphql(name)], Scalar/InputObject derives). Nothing ties the two together, so they are kept in step by hand.

The clearest cost is GqlTimeInput, which needs ~90 lines of hand-written Serialize/Deserialize purely to imitate what its Scalar derive already knows (serde would emit {"Simple": 5}; the server wants a bare 5). Alongside that there are 19 #[serde(rename)] attributes across the model types and 5 golden tests whose only job is catching drift between the two definitions.

This is a maintenance problem rather than a correctness one: the goldens and the e2e/parity suites (which push real filters through a real server) already catch drift today.

What blocks the obvious fix

Rendering variables through the GraphQL machinery would make the two directions inverse by construction, but the render half only exists for scalars:

type kind parse (server) render (client)
scalars (TimeInput, Value, NodeId) ScalarValue::from_value ScalarValue::to_value — exists
input objects / oneOf (the filter types) FromValue::from_value none — dynamic-graphql has no ToValue derive

So the work is: add a ToValue derive (locally, or upstream in dynamic-graphql) mirroring the field metadata the InputObject/OneOfInput derives already carry, then migrate the variable path off serde.

Landmine already defused

GqlTimeInput::to_value used to render Indexed(t, event_id) as a bare t, silently dropping the locked event id — the same loss fixed in 2c264835f. It is now the true inverse of from_value and pinned by round-trip tests, so a future migration to to_value will not reintroduce it. The other scalars' to_value impls should get the same round-trip check before being relied on.

Raised in review: https://github.com/Pometry/Raphtory/pull/2675#discussion_r3812162014

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the query-variable serialization path and reading the existing ScalarValue::to_value and FromValue implementations, including GqlTimeInput and its round-trip tests. Then inspect the metadata produced by the InputObject and OneOfInput derives. Done means adding ToValue support for input objects and oneOf types, migrating variables off serde, and keeping the golden, e2e/parity, and scalar round-trip tests passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, rust
Domain
api, backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.