Skip to content

feat(typed): generic, type-safe client and query builder#17

Open
mlwelles wants to merge 8 commits into
matthewmcneely:mainfrom
mlwelles:feature/typed-client
Open

feat(typed): generic, type-safe client and query builder#17
mlwelles wants to merge 8 commits into
matthewmcneely:mainfrom
mlwelles:feature/typed-client

Conversation

@mlwelles

@mlwelles mlwelles commented Jun 4, 2026

Copy link
Copy Markdown

What this adds

A generic typed layer over modusgraph.Client that binds a Go type to the
otherwise any-typed client, giving you compile-time-typed CRUD and a fluent
query builder with no per-entity code generation. It is the handwritten
substrate modusgraph-gen's generated clients compose over, and it stands on
its own wherever you want types over modusgraph.

The whole diff lives under typed/. It builds and tests green against the
current client with no changes to the root package.

The problem it solves

modusgraph.Client is value-oriented: its methods take and return any, and a
query is assembled by hand from dgman primitives and decoded into a slice you
declare at the call site. Every call site repeats the same shape — declare the
destination, build the query, decode, re-assert the type. The typed layer lifts
that shape into the type system once.

Before — untyped client

// "First person named Alice" — the type is repeated on every line,
// and the single-result case is hand-rolled.
var out []Person
q := client.Query(ctx, &Person{}).
    Filter("eq(name, $1)", "Alice").
    First(1)
if err := q.Nodes(&out); err != nil {
    return nil, err
}
var person *Person
if len(out) > 0 {
    person = &out[0]
}

After — typed client

people := typed.NewClient[Person](client) // declare the type once

person, err := people.Query(ctx).
    Filter("eq(name, $1)", "Alice").
    First() // returns *Person; nil when nothing matched

The same lift applies across the API: Nodes() returns []Person,
IterNodes() yields *Person, Get returns *Person.

Query builder

Query[T] is fluent: builder methods chain, terminals (Nodes, First,
NodesAndCount, IterNodes) execute and decode typed results.

  • Filters accumulate and AND together, each fragment parenthesized so a
    fragment containing OR keeps its precedence.
  • OrGroup ORs several sub-scopes into one parenthesized group:
    // age >= 18 AND (name == "Alice" OR name == "Bob")
    people.Query(ctx).
        Filter("ge(age, $1)", 18).
        OrGroup(
            typed.NewDetachedQuery[Person]().Filter(`eq(name, "Alice")`),
            typed.NewDetachedQuery[Person]().Filter(`eq(name, "Bob")`),
        ).Nodes()
  • WhereEdge constrains T by a scalar of a neighbour reached over an edge
    (a root filter cannot express this); resolved by a pre-pass and intersected
    with any root you set.
  • IterNodes streams arbitrarily large result sets a page at a time over a
    single read-only snapshot.
  • MultiQuery batches N same-type blocks into one round-trip.

On reusing dgman vs. reinventing it

The Or/OrGroup support builds on dgman, it does not reimplement it.
dgman exposes a single string-based Query.Filter(filter string, params ...any)
with $N positional markers and no AND/OR combinators and no root-intersection
helpers
. So any builder offering Where/Or composition must assemble the
filter string itself and hand it to dgman's Filter. This layer does exactly
that and no more:

  • the actual filtering, parameter binding, and injection-safe $N substitution
    stay in dgman (Query.Filter, parseQueryWithParams);
  • shiftPlaceholders only renumbers $N to match dgman's own convention
    when independently-written fragments are concatenated — it does not re-parse
    or re-escape values;
  • the layer adds only what dgman lacks: fragment composition (AND/OR grouping
    with correct precedence) and the type binding.

Review round — changes since first push

Addresses the automated review (cubic). Highlights:

  • Filter precedence (P1): combineAnd now parenthesizes each fragment, so
    a raw OR fragment no longer binds incorrectly when ANDed.
  • WhereEdge roots (P1): the pre-pass no longer discards a caller-set
    UID/RootFunc; it intersects via a uid() @filter instead.
  • MultiQuery reuse (P1): Add rejects the same *Query[T] under two names
    (Execute names the underlying query in place).
  • NodesAndCount tracing: now opens a span like the other terminals.
  • Tests: added a precedence regression, a WhereEdge+root intersection
    test, and a duplicate-Query guard; the filter-sequencing test now asserts
    the exact expression; the laziness check is delta-based.

Documentation

  • Package doc.go with the before/after narrative and the design rationale.
  • Runnable, compile-checked examples for the client, query builder, OrGroup,
    WhereEdge, IterNodes, and MultiQuery.
  • A verified-output example for the filter builder.

Add a generic typed layer over modusgraph.Client: typed.Client[T] with
CRUD and iterators; a fluent Query[T] builder (filters, ordering, paging,
edge traversal, IterNodes); MultiQuery for N homogeneous blocks in one
round-trip; functional options; a filter DSL (typed/filter); and ordered
result merging (typed/search).

A small no-op-by-default Tracer seam (typed.SetTracer) lets a host plug in
tracing without the typed package depending on any telemetry library.

Self-contained: builds and tests against the current client with no other
changes.
@mlwelles mlwelles requested a review from matthewmcneely as a code owner June 4, 2026 17:07

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 16 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread typed/multi_query.go
Comment thread typed/query.go Outdated
Comment thread typed/query.go Outdated
Comment thread typed/filter/filter_test.go Outdated
Comment thread typed/query_test.go
Addresses review feedback on the typed query builder:

- combineAnd now parenthesizes each accumulated @filter fragment. Without
  this, a fragment containing OR ANDed with another fragment rendered as
  "a OR b AND c", which dgraph parses as "a OR (b AND c)" — silently
  widening results. dgman exposes only a string Filter(), so the builder
  must compose the expression itself; this makes that composition correct.
- WhereEdge's pre-pass no longer discards a caller-set UID/RootFunc root.
  When a custom root is present, the matched UIDs are intersected via a
  uid() @filter instead of overwriting the root.
- MultiQuery.Add rejects the same *Query[T] under two names; Execute names
  the underlying query in place, so reuse would corrupt block composition.
- NodesAndCount now opens a tracing span, matching the other terminals.

Tests: add a precedence regression, a WhereEdge+UID intersection test, and
a duplicate-Query guard test; strengthen the filter-sequencing test to
assert the exact expression; make the IterNodes laziness check delta-based.

Docs: add package doc.go with a before/after narrative, runnable examples
for the client/query builder/MultiQuery, and a verified filter example.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 9 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread typed/doc.go
Wrap a >120-char error string in MultiQuery.Execute and a long test helper
signature, and pre-allocate two test result slices. Resolves the Trunk
golangci-lint findings; no behavior change.

@matthewmcneely matthewmcneely left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Six subpackage files are missing the SPDX header that every other file in the repo carries: filter/filter.go, filter/fulltext.go, filter/filter_test.go, filter/fulltext_test.go, search/merge.go, search/merge_test.go.

Comment thread typed/query.go
Comment thread typed/multi_query.go Outdated
Comment thread typed/query.go Outdated
Comment thread typed/multi_query.go Outdated
@matthewmcneely

Copy link
Copy Markdown
Owner

@mlwelles Also, can you update the README with the major feature additions of this PR?

mlwelles added 3 commits June 24, 2026 17:43
…n MultiQuery

MultiQuery.Execute remapped only top-level predicate keys, so a renamed
predicate (dgraph predicate= diverging from json=) on a nested edge struct
silently decoded as empty. Make the remap type-driven and recursive, mirroring
dgman's QueryBlock.Scan, so nested edges decode at every depth.

Also propagate the top-level remap error instead of swallowing it, so a
malformed block surfaces its root cause rather than a generic downstream decode
error.

Addresses review feedback on multi_query.go (nested-predicate remap gap and
swallowed remap error).
…e tracer

WhereEdge ran an eager client-side pre-pass that pulled every matching root UID
into the client and inlined them as uid(0x1, ..., 0xN) in the main query —
unbounded in memory and DQL size on a high-cardinality match, and eager even
under IterNodes. Replace it with a single multi-block request: a var block binds
the matched roots server-side (mgMatched as var(func: ...) @cascade {...}) and
the data block roots at uid(mgMatched). NodesAndCount adds a count block over the
same var; the var block roots at the caller's UID/RootFunc when present, so a
custom root intersects the match rather than being overwritten. The matched UIDs
never leave the server, so memory and query size stay bounded, and IterNodes
re-resolves the var per page. This mirrors the var-block shape dgman's own
NodesAndCount uses internally.

Also make the process-wide typed tracer race-safe: hold it in an atomic.Pointer
so SetTracer and the per-operation reads in currentTracer no longer race.

Addresses review feedback on query.go (unbounded WhereEdge pre-pass) and the
unsynchronized global tracer.
…de var block

The WhereEdge doc comment referenced
docs/specs/2026-05-21-query-edge-filter-design.md, but the file was never
committed on this branch. Add it, and revise its execution narrative from the
originally-drafted client-side two-step to the server-side var block that ships:
the matched UIDs stay off the client, and the QueryRaw two-block path the draft
rejected is in fact viable because dgman renders the data block's projection
(reverse-edge-aware), so nothing is hand-written.

Addresses review feedback on the missing design doc.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 9 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread typed/query.go Outdated
mlwelles added 2 commits June 24, 2026 17:50
trunk fmt (prettier) reflow, code-block tabs to spaces (MD010), and de-nested
backticks in the DQL-example caption (MD038). trunk check is clean.
The WhereEdge path (runEdge) renders its own multi-block request and ran it via
QueryRaw with a nil variable map, so a query built with .Vars(...) lost its
GraphQL named variables -- a regression from the pre-var-block path, which ran
qb.q.Nodes() and forwarded them via QueryWithVars. Store the Vars funcDef and
map on the typed Query and forward them to the QueryBlock (so the
"query <funcDef>" declaration renders) and to QueryRaw (so they bind).

Tests: TestQuery_WhereEdgeForwardsVars (Vars + WhereEdge, verified to fail with
"Variable not defined $n" without the fix); TestRemapPredicateKeys_SurfacesMalformedBlock
(guards the earlier swallowed-remap-error fix in MultiQuery.Execute).

Addresses cubic review feedback (P1: WhereEdge drops Vars).
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