Fix SQL resolvers executing empty batches - #7120
Conversation
🦋 Changeset detectedLatest commit: d55e535 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Important
This PR adds a correct reproduction test, but the implementation fix it claims to make is not present in the current commit. CI is expected to fail until the fix lands.
Reviewed changes
Reviewed the single added regression test in packages/effect/test/unstable/sql/SqlResolver.test.ts and the unchanged implementation in packages/effect/src/unstable/sql/SqlResolver.ts that it exercises.
- Added a focused reproduction test showing
SqlResolver.orderedcallsexecutewith an empty batch when every request fails encoding.
The test fails as expected against the current source (executions is 1, expected 0), confirming the bug. Before this can merge, packages/effect/src/unstable/sql/SqlResolver.ts needs to guard every options.execute(inputs) call so it is only invoked when inputs is non-empty. The PR body calls out both ordered and findById; the current implementation also calls execute unconditionally in grouped and void, so those should be covered for consistency.
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the new commit 6246060 against the prior Pullfrog review at abf4f5e.
- Added
Arr.isArrayNonEmptyguards beforeoptions.execute(inputs)inordered,grouped,findById, andvoid_so empty encoded batches never reach a callback typed asArr.NonEmptyArray. - Removed the
as anycasts oninputsnow that the guard narrows the type. - Added focused regression tests for
grouped,findById,ordered, andvoidthat assert noexecutecall and a failed exit when every request fails encoding. - Added a changeset describing the
effectpatch.
The prior Pullfrog thread requesting an analogous findById regression test is addressed and resolved.
Validation run:
pnpm test --run packages/effect/test/unstable/sql/SqlResolver.test.tspasses (6 tests).pnpm --filter effect checkpasses.pnpm lint-fixon the changed files produces no changes.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the new commit 9520a88 against the prior Pullfrog review at 6246060.
- Refactored
partitionRequestsByIdinpackages/effect/src/unstable/sql/SqlResolver.tsto build the ID deduplication map before encoding any request, then encode only the unique IDs in a second pass. This prevents duplicate requests for an invalid ID from being left uncompleted when encoding fails and the resolver returns early on an empty batch. - Updated the duplicate-merging
completeUnsafeclosure to cascade completion through the deduplication chain. - Added a focused regression test in
packages/effect/test/unstable/sql/SqlResolver.test.tsverifying that two concurrentfindByIdrequests for the same invalid ID both complete withSchemaError. - Tightened existing empty-batch regression assertions from
Exit.isFailuretoerror._tag === "SchemaError"viaEffect.flip.
Validation run:
pnpm test --run packages/effect/test/unstable/sql/SqlResolver.test.tspasses (7 tests).pnpm --filter effect checkpasses.pnpm lint-fixon the changed files produces no changes.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
9520a88 to
5e7137f
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the new commit 5e7137f ("Simplify SQL resolver request encoding") against the prior Pullfrog review at 9520a88. The PR branch was rebased onto newer main (b9875ea), so the earlier commits now carry different SHAs but the same content as already reviewed.
- Refactored
partitionRequestsByIdinpackages/effect/src/unstable/sql/SqlResolver.tsto encode each deduplicated entry with a per-iterationmatchCauseEagerclosure andyield*, replacing the previous shared-mutable-entry+handleclosure +yield (...)pattern. This is behavior-preserving and removes the mutable-capture hazard that originally caused duplicate invalid requests to be left uncompleted.
Validation run:
pnpm test --run packages/effect/test/unstable/sql/SqlResolver.test.tspasses (7 tests).pnpm --filter effect checkpasses.
No new issues introduced by this refactor; all four resolver Arr.isArrayNonEmpty guards remain in place.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Summary
SQL resolver constructors invoked
options.execute(inputs)after request encoding even when every request failed encoding andinputswas empty. This violated the publicArr.NonEmptyArraycallback contract.This PR now guards the
ordered,grouped,findById, andvoidresolvers against empty encoded batches and adds focused regression coverage for every path.SQL resolvers invoke NonEmpty callbacks with an empty batch
Module:
packages/effect/src/unstable/sql/SqlResolver.tsAudit ID:
relsem-sql-empty-encoded-batchSeverity / confidence: medium / high
What happens
Each resolver partitions requests by whether encoding succeeds. Previously,
executewas invoked unconditionally after partitioning, so an all-invalid batch reached a callback typed to accept onlyArr.NonEmptyArray.Expected behavior
The resolver
executecallbacks receive only non-empty encoded input batches. Per-request encoding failures complete those requests withSchemaErrorand do not trigger an impossible empty execution.Implementation
Each resolver now narrows the encoded inputs with
Arr.isArrayNonEmptybefore invokingexecute. The existingSqlResolver.test.tscovers all-invalid batches forordered,grouped,findById, andvoid.Validation
Audit provenance
b206fa5d7655c1634c9993410a9203f6616a5ca2b206fa5d7655c1634c9993410a9203f6616a5ca2relsem-sql-empty-encoded-batchCloses EFF-555