[kylebernhardy]

Honor nested relationship filters, sorting and pagination consistently across collection and single-record GET requests.

⊙ Problem

REST nested relationship selections silently ignore filters and limit accepts nested constraints but returns every related record with HTTP 200. Collection and single-record GET need the same per-parent execution and authorization behavior.

❓ Your call: Kris selected v5.4 following the patch-scope discussion; core and the documentation companion now both target v5.4. The earlier v5.3 patch-target framing is superseded. Nested filtering remains the intended supported behavior, without claiming proof of a historical public guarantee.

💡 Solution

Apply nested predicates, ordering, offset and limit to each parent's existing relationship candidates. select(id,children[state=open&limit(1)&select(id,state)]) returns at most one open child per parent and retains parents whose child selection is empty. Eligible unsorted pagination stops before reading the unused child tail.

⚖️ Alternatives

❓ Your call: Reliability planning verdict resolved by adoption: evaluate parity once in a dedicated job, with the token only on that comparison step. Retain live peer revision resolution to preserve the required peer-only drift gate; a fixed peer SHA would weaken it. Retry only bounded transient read failures; permanent errors and mismatches still fail.

Framing-Verdict: better-alternative-exists

Gemini’s initial planning review returned chosen-approach-sound for the runtime approach. The follow-up reliability planning review recommended moving parity out of the runtime matrix; that smaller execution boundary is adopted. Live companion-head comparison is retained because a fixed SHA would allow later peer-only fixture drift to pass against an obsolete snapshot.

❓ Your call: The repair planning review recommended a smaller approach, which is adopted: compile pure record predicates, fence the upstream iterator, and reuse the existing post-materialization row-filter hook. A new projection-mask engine and refresh-before-filter/order pipeline were rejected because they would refresh and write rejected source candidates and change the documented top-level cache-query policy.

Nested predicates qualify the immediately selected children. To narrow grandchildren, place constraints on that descendant's own subselect. Existing outer-join candidate restrictions remain inherited. This removes the accidental behavior where the first evaluated OR term determined which grandchildren were projected.

❓ Your call: Cache candidates and sort keys keep Harper's existing local-entry semantics. A locally eligible child that refreshes to a nonmatch is excluded without consuming offset or limit; locally rejected rows are not fetched from source. Fresh-source ordering and discovery of matches absent from the qualifying cache set would require a broader query-policy change.

🔧 Changes

Shared execution and bounded work

How does each parent receive its own constrained child page?

From relationship candidates to a page

Both GET forms use one prepared selection; inherited candidates remain owned by the original query.

flowchart TD
  A[Collection GET] --> C[Prepared relationship selection]
  B[Single-record GET] --> C
  C --> D[Existing join candidates or lazy resolver]
  D --> E[Local predicate and ordering]
  E --> F[Materialize and recheck predicate]
  F --> G[Offset then limit]
  G --> H[Child projection]

Loading

Table.ts condition preparation shares validation and coercion with top-level search, checks child filter/sort permissions, preserves constraint-only selections during authorization, and adds per-call lazy resolver mode while retaining ordinary accessor and comparison contracts. Sorting or unmatched predicates can still scan all candidates. Unsorted FK pagination preserves missing slots; sorting drops them.

Cancellation and projection compatibility

How do nested selections preserve lifecycle and projection contracts?

Table.ts selection execution owns nested iterators until successful closure, observes asynchronous cleanup failures, and fences both pending projection resumptions and raw upstream reads. Returning the parent or abandoning its transaction at a candidate yield stops additional child reads. Offset guards recheck refreshed rows without computing output fields for skipped rows; rejected refreshed rows and source deletions do not consume the page.

Resource.ts validates unsupported nested options recursively, shares table preparation with single-record GET, resolves asynchronous object/positional arrays including constrained grandchildren, and confines the ambient-context fallback to the new relationship path. Projection-only JSON bracket syntax remains supported. Unsupported non-relationship query constraints and non-record/full-text predicates fail explicitly.

ResourceInterface.ts types the existing nested condition/order/page fields and makes the projection optional. resources/DESIGN.md records candidate ownership, immediate-child predicate scope, cache behavior, missing-value semantics, and the source-iterator cancellation invariant.

Executable documentation contract

Core integration CI executes the documented schema, dataset, six REST requests and two programmatic snippets against authenticated Harper. The docs companion checks its Markdown against the same fixtures through its required Lint Check. Both repos compare fixture bytes against the companion PR head until merge, then main, so editing only the docs or only the core contract fails CI. Parity now runs once in a dedicated dependency-free job, with the read-only token restricted to its comparison step. A shared 30-second deadline and bounded transient retries retain fail-closed drift detection without putting GitHub reads in the native runtime matrix. These additions change tests, CI and guidance; the qualified runtime implementation is unchanged.

Fix REST and operations API select projection defects overlaps these projection surfaces and proposes rejecting nested filters. Its unrelated projection fixes are not superseded by this implementation.

✅ Verification

Merge with current main

Merged main abf67cd59 in 115e3828c. The count cleanup retains projection closure and main’s generation-bound read-reference release, with release in a finally so a close error cannot strand that reference. Three new real-table cases exercise exact/estimated counts with bounded children and projection failure; they check returned data, iterator ownership, and read-reference restoration on both engines. No new API or query policy is introduced.

The outside review identified a pending relationship-provider callback that could resume after cancellation. A four-line guard closes that gap; the real-table regression fails against the merge build before the guard and passes afterward.

Local verification	Result
Build, full lint, full formatting	Pass
Relationship/count/read-reference suites	99 passing / 1 pending on RocksDB; 81 passing / 19 pending on LMDB
Native documentation and relationship/status HTTP suites	37 passing on each engine after the final guard
Full resource gate	RocksDB: 4,976 passing / 66 pending / 2 failures; LMDB: 3,786 passing / 848 pending / 1 failure
Full integration gate	2,703 passing / 31 failing / 14 cancelled / 18 skipped / 2 todo
Main unit gate	Interrupted after a detached-worker fixture stalled; a second run excluding that fixture was stopped during slow activation recovery. Clean main also reports the portable-bundle failure, but its interrupted log does not establish the same stall

The WebSocket fixture failure and dedicated-worker integration failures reproduce on clean main at the merged revision. Subscription replay also failed in a standalone merged-branch control, while its clean-main control passed; that race remains unresolved. The remaining broad integration failures were not all isolated. These results are not an all-green qualification claim; fresh CI is running at the pushed head. npm 11 was used for the broad main/integration comparisons because this machine’s npm 12 rejects the existing remote dependency/lifecycle path.

❓ Your call: Claude and Composer re-read the merged change, with domain adjudication. They agree the conflict resolution preserves generation-bound release. Remaining review concerns include async custom-iterator cleanup ordering, per-parent allocation/read-reference cost, inherited metadata-name collisions, and malformed structured sort input. No built-in async-close failure was reproduced; these are retained review questions, not an all-clear. Framing decision: keep the existing per-parent query approach and the narrow cancellation guard; broader iterator redesign and cache-query policy changes remain separate. Post-review delta: 22 additions in two reviewed files, not re-reviewed because it directly fences the pending callback identified by that review; the failing-before/passing-after regression and focused suites were rerun. No additional delta review was started after the framing checkpoint.

Related overlap: Apply related-table role permissions to @relationship reads in REST, GraphQL and NoSQL operations shares the nested authorization boundary; Search select() returns stored nested JSON instead of running the outer table’s resolvers on it shares projection paths; Fix REST query parsing of list values, (...) groups, range values and chained conditions shares condition preparation; Stop transaction abort() from spinning forever on read references taken after commit is already integrated by this merge. None is claimed superseded. The sibling scan is partial: all 82 open PRs and the most recent 50 of 116 merged PRs were scanned; one large PR was truncated.

Kris review follow-up

• The existing _readTxnForContext declaration now also carries @internal, completing Kris’s sibling-helper request in 76fb61278. This one-line annotation was self-reviewed without another outside review. Build, full lint, and full formatting checks pass; the emitted .d.ts preserves the annotation and emitted Table.js is byte-for-byte unchanged. Runtime suites were not rerun for that historical comment-only follow-up; the current merge verification is recorded above.

Core 8b2f0aa56 adds @internal to the three helper declarations and returns HTTP 400 for invalid nested operators while preserving top-level validation behavior. Three new regressions fail before the repair and pass afterward; build, focused lint/format, and relationship suites pass (76 RocksDB; 63 LMDB with 13 existing engine-specific skips). The documentation repair calls out unsupported nested constraints on custom non-table resources. These narrow repairs were self-reviewed; the earlier full outside reviews were not rerun. Main-PR CI passed at that head. Both fixture-coupling discussions were subsequently replied to and closed with the intentional tradeoffs retained following Kris’s approval. This follow-up supersedes earlier statements below that no runtime source changed or no significant concerns remain.

Fixture reliability and OR follow-up

❓ Your call: The full review re-raised async child-cursor close versus native read-transaction release ordering in unchanged runtime code. The supported ExtendedIterable map return is synchronous, and five cancellation/abandonment regressions pass; no supported built-in failure sequence was reproduced. Broader asynchronous custom-iterator cleanup and internal join-metadata name collisions remain review questions for Kris, not fixes claimed by this tooling/docs follow-up.

• .github/workflows/integration-tests.yml runs network reliability tests and companion parity once, independently of the native runtime matrix. Only the focused comparison step receives the existing read-only GitHub token; no new repository permissions or native-process credentials are added.
• .github/scripts/nested-relationship-fixture-parity.test.mjs invokes the comparison with a 35-second test timeout. integrationTests/resources/nested-relationship-docs/github-fixture-fetch.mjs authenticates only GitHub API metadata, leaves raw downloads unauthenticated, permits at most three transient read attempts under a shared 30-second deadline, honors short Retry-After values, and fails permanent errors, long waits, closed-unmerged companions and mismatched bytes. It is the fourth paired fixture.
• .github/scripts/github-fixture-fetch.test.mjs adds nine real-HTTP/header tests covering retry recovery and exhaustion, permanent errors, credential boundaries, companion revisions and cancellation. No fetch stubs are used. Removing authentication/retry behavior makes four checks fail; missing CI token wiring fails explicitly before reading GitHub. A real authenticated metadata read through the helper passes.
• The native suite now executes all six exact REST requests/responses and both programmatic snippets: 14 passing on each of RocksDB and LMDB. The sixth case groups OR conditions, sorts children by descending price with an id tie-breaker, and pages each parent independently; additional controls exercise both OR branches and AND-empty behavior. Temporary tie-test rows are deleted in finally blocks. Nine network checks plus one standalone offline parity check pass; docs has 14 passing local checks. Full docs build/format/workflow checks and core lint/changed-file formatting pass. New CI is reported separately after push; prior runtime release qualification remains valid because runtime code is unchanged.

❓ Your call: The final full Claude/Composer reviews cover the reliability helper and OR example. Post-review delta: 50 additions / 15 removals across four reviewed files, not re-reviewed because these are narrow responses to the findings: real-row cleanup in the tie tests, preserving status/retry handling when a discarded body has errored (with a real Response/ReadableStream regression), a direct three-parent exclusive-end guard, and disabling unused package-manager caching in the standalone job. These narrow test/helper repairs are self-reviewed; 14 native checks rerun on both engines and nine reliability tests pass. Hypothetical future formatter divergence and the already-refuted per-depth Map allegation do not justify weakening parity or changing runtime semantics.

❓ Your call: Review-depth checkpoint: previous full rounds found the peer-fixture drift gap, which is now enforced in both repos. We ran one final full pass on the final test artifact and stopped after resolving scoped concerns; remaining minor infrastructure/naming suggestions are author-triaged. No runtime source changed in this follow-up.

• integrationTests/resources/nested-relationship-docs.test.ts adds 14 native authenticated regression tests: the six exact documented REST request/response pairs, both exact programmatic snippets, repeated-filter equivalence, independently bounded parents and explicit ascending/descending tie breakers for parent names and child prices. The 14 native tests pass on both RocksDB and LMDB, including grouped OR versus AND and multirow descending-order controls. Parity now runs separately once per integration workflow. An unindexed child sort and a three-candidate window also discriminate nested sort/index and exclusive-end pagination semantics. The existing test:integration:all glob includes this file in CI; no new runtime code is changed.
• Fixture integrationTests/resources/nested-relationship-docs/config.yaml enables REST/schema/custom resources; integrationTests/resources/nested-relationship-docs/schema.graphql matches the documented schema; integrationTests/resources/nested-relationship-docs/examples.json pins the dataset, URLs and full responses; integrationTests/resources/nested-relationship-docs/resources.js executes both programmatic examples through a native resource. The three paired docs fixture files match byte-for-byte. In CI, the peer check reads the docs companion PR head until merge and docs main afterward; API/raw read failures or mismatches fail the check.
• nestedRelationshipConstraints.test.js has 73 real-module cases covering production REST dispatch, structured GET/search, ordinary-role authorization, boolean groups/coercion, missing FK slots, async/positional/deep projection, caller immutability, inherited joins, OR commutation, source refresh/deletion, pagination refill, and cancellation. The RocksDB counter proves two limited child reads versus seven unlimited; cancellation at 2,048 candidate reads permits zero further reads. Thirteen new cases fail on pre-repair production code at 43ec51cd3 and pass after the repair.
• Alongside query, missing-value, permission and count suites: 259 passing on RocksDB; 246 passing / 13 skipped on LMDB. Existing caching tests additionally expose two blob-reclamation failures that reproduce on 43ec51cd3.
• relationship-and-status-contract.test.ts adds an authenticated native REST filter/sort/limit case across collection and single GET; all 23 tests pass per engine. Expanded with cached-computed integration: 34 passing on RocksDB; 33 passing / 1 failing on LMDB. The LMDB cached-computed 999-versus-60 failure also reproduces on 43ec51cd3.
• The v5.3 replay of runtime head 96fb3b8 applied its eight commits cleanly after an import-only history correction. Its build and dedicated suite pass: 73 on RocksDB; 60 passing / 13 skipped on LMDB. Production behavior is unchanged from the reviewed artifact.
• Build, TypeScript, lint, formatting and design-doc checks pass. Canonical CI at the preceding runtime head 96fb3b8 passes Node 22/24/26 and Windows units, including resource and LMDB gates. Node 24 reports 7,189 main, 3,745 core-resource, 661 index-resource and 3,361 LMDB-resource tests passing. Main and v5.3 integration also pass; that revision completed with 53 successful checks / 3 skipped. Earlier local runs remain recorded separately: 4,385 resource passes / 54 pending / 2 native full-text capability failures reproduced on base; blob-reclamation and cached-computed failures also have base controls. Other earlier broad local failures are not all root-caused and are not claimed repaired; they were not observed in canonical CI.
• Claude and Cursor Composer reviewed through the standard pre-push runner across this branch. Composer's final reported join-map blocker was checked with its exact URL and a constrained counterpart; both pass. Nested maps remain Maps inside the per-depth dictionary. Review findings were author-triaged; no separate domain adjudicator ran on the final pass. That historical test-only artifact also received full Claude and Composer review. The current merge review and remaining concerns are recorded above. Earlier live CI-mode reciprocal peer checks passed; the updated authenticated gates are revalidated after this follow-up is pushed.

Companion documentation: Document nested relationship filters and pagination. REST and Resource API reference pages and the v5.4 release note explain per-parent constraints, parent retention, missing FK behavior, inherited joins, immediate-child scope, cache semantics, scan costs and rejected inputs. The docs build and full formatting checks pass; Claude and Cursor Composer reviewed them, and all six REST examples and both programmatic examples are now committed native regressions; docs CI checks their Markdown against paired offline fixtures. The docs PR depends on this implementation and marks availability as targeted for v5.4.0 and not released yet.

Additional qualification on the actual v5.3 replay (5ab1dd904f45):

• Upgrade and rollback against the latest published harper@5.3.2: 13 passing / 0 skipped, including cold restart, writes from both versions, rollback and forward round trip. CI separately exercises 5.3.1. Command: HARPER_PREVIOUS_MINOR_PATH=<isolated harper@5.3.2 root> npm run test:integration -- integrationTests/upgrade/minor-upgrade.test.ts integrationTests/upgrade/minor-downgrade.test.ts.
• Local HTTP YCSB C/E measurement: 1,000 records, 5,000 operations per workload, three repetitions, concurrency 2, zero load/workload errors on both published 5.3.2 and candidate. Median-repetition read p95: 0.097 ms versus 0.091 ms; scan p95: 3.41 ms versus 3.47 ms. This is low-concurrency smoke evidence, not a scale/soak or agreed-tolerance sign-off. The runner was copied to scratch only to select the binary; its stock git metadata identifies the runner checkout, so binary identity was recorded separately.
• Established Pro v5.3 risk-query smoke with candidate core: 2 passing; 100 records replicate across two nodes, then 150 HTTP reads at 10 RPS for 15 seconds yield zero failures and p95 1.99 ms (existing ceiling 1,000 ms). The established redirector canary also passes: 2 tests, rule replication, 301 HTTP checks at 20 RPS for 15 seconds, zero failures and p95 3.27 ms. No Pro source change is proposed.

These checks qualify the PR surfaces exercised; they do not cut a release or replace the release owner's final sign-off.

Fixes REST nested relationship selections silently ignore filters and limit.

Comment generated by kAIle (GPT-6)

Release target: v5.4 — selected by Kris for the nested relationship correction
Related PRs: #3221 overlaps, #3316 overlaps, #3415 overlaps, #3456 overlaps, #3337 overlaps, #3443 overlaps, 18 others independent
Complexity: complicated

_{Review-Coverage: authored=codex; ran=claude,cursor-composer; adjudicated=domain; declined=gemini,cursor-grok,cursor-kimi,cursor-muse; rounds=6; full=6 @ 7b2778f}

_{Review-Attention: deep ~90m (critical: Table.ts; sensitive: integration-tests.yml; hot: Resource.ts; decisions: do-less-alternative, descendant-filter-semantics, missing-fk-ordering, cross-repo-parity, internal-static-surface, release-target-vs-evidence) @ 7b2778f}

[github-actions]

Release cherry-pick v5.3: cancelled

Cherry-pick branch cherry-pick/v5.3/pr-3279 was deleted — this PR no longer targets v5.3 (milestone is now v5.4).

[gemini-code-assist]

Code Review

This pull request introduces support for nested relationship query constraints (conditions, sort, offset, and limit) across search and single-record GET operations. It refactors condition preparation, implements nested permission checks, and adds comprehensive unit and integration tests. The review feedback suggests hoisting static selection and sorting configuration logic out of the per-record relationshipSelection closure in Table.ts to avoid unnecessary allocations on the hot path.

[kriszyp]

I think this generally looks good, but I don't see a justification for it being a patch (and 600 lines of production code probably deserve a solid justification).

_{🤖 Reviewed with Claude}

[kylebernhardy]

Thanks for reviewing this. I agree the patch-release case needs a customer-need justification; the fact that this corrects accepted-but-ignored syntax does not by itself justify the compatibility changes or size of the backport. I have asked Kyle whether there is a v5.3 customer/release commitment or whether we should target the next minor. I have left the milestone unchanged pending that decision.

Follow-up repairs are pushed: core 8b2f0aa makes malformed nested operators client errors and marks all three helper declarations @internal; docs 71d0f8021c73077d4375c70c22df7f20aa2e35ff explicitly calls out the non-table 400 change. The core regression cases failed before the fix and pass afterward; the full focused suite passes on RocksDB (76) and LMDB (63, 13 existing engine-specific skips). Build, focused lint/format, and docs 14 tests pass. New-head CI is pending.

These narrow follow-up deltas (core +14/-3 in two previously reviewed files; docs +2/-2 in two previously reviewed pages) were self-reviewed and were not sent through another outside-model round. Two existing PR-body anchors drifted after the code edit; Kyle's monitoring constraint requires author authorization before changing that body, which I have requested. The fixture-coupling tradeoff remains open in the inline replies.

Comment generated by kAIle (GPT-6)

[kylebernhardy]

Thanks, Kris. Kyle agrees with proposing the next minor release for this change rather than continuing to justify a v5.3 patch. Would you be comfortable targeting the next minor for this implementation and Document nested relationship filters and pagination? The existing v5.3 milestones are unchanged pending your release-target decision.

The description now records this proposal, refreshes the two shifted code links, and discloses the narrow follow-up repairs and their verification. The two fixture-coupling discussions remain open for your decision.

Comment generated by kAIle (GPT-6)

[kriszyp]

This is pretty cool!

I have asked Kyle

Who is driving this?!? Hopefully the real Kyle!

_{🤖 Reviewed with Claude}