Fix sQL findById resolvers invoke NonEmpty callbacks with an empty batch - #7119
Conversation
🦋 Changeset detectedLatest commit: 0e9a296 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 for findById, but the implementation fix is still pending and the ordered resolver (which the description says is also affected) lacks a matching regression test.
Reviewed changes
- Added a focused regression test in
packages/effect/test/unstable/sql/SqlResolver.test.tsthat verifiesSqlResolver.findByIddoes not callexecutewhen every id fails schema encoding.
⚠️ Implementation fix still pending
The PR description states this branch intentionally starts with failing reproduction tests and that the implementation fix should be added to the same branch. The current commit only adds the test; the unconditional options.execute(inputs) calls in SqlResolver.ordered, SqlResolver.findById, SqlResolver.grouped, and SqlResolver.void_ still pass an empty array when all requests fail encoding.
⚠️ Missing regression test for ordered
The description explicitly identifies ordered as one of the two affected resolvers, but the diff only adds a regression test for findById. Add an equivalent test under describe("ordered") that verifies execute is not invoked when every request fails encoding.
@v0 or keep the SHA fresh with Dependabot | Fix it ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior review at 6db67574, the author added the implementation fix and the missing regression test.
- Added empty-batch guards in
SqlResolver.tsbefore every unconditionaloptions.execute(inputs)call inordered,grouped,findById, andvoid_. - Added the missing
orderedregression test that verifiesexecuteis not invoked when every request fails encoding. - Added a changeset marking
effectas a patch release for the fix.
@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.
|

Summary
Both resolvers invoke options.execute(inputs) unconditionally after partitioning. When all requests fail encoding, inputs is [], despite the public callback type being NonEmptyArray.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
SQL findById resolvers invoke NonEmpty callbacks with an empty batch
Module:
packages/effect/src/unstable/sql/SqlResolver.tsAudit ID:
relsem-sql-empty-id-batchSeverity / confidence: medium / high
What happens
Both resolvers invoke options.execute(inputs) unconditionally after partitioning. When all requests fail encoding, inputs is [], despite the public callback type being NonEmptyArray.
Why it happens
The owning implementation diverges from relation unstable-services-018.
Expected behavior
ordered.execute and findById.execute receive Arr.NonEmptyArray encoded inputs. Per-request encoding failures complete those requests with SchemaError and must not create an impossible empty execute batch.
Relevant implementation
These links and excerpts are pinned to audit base
b206fa5d7655c1634c9993410a9203f6616a5ca2.packages/effect/src/unstable/sql/SqlResolver.ts:101View problematic code at
packages/effect/src/unstable/sql/SqlResolver.ts:101View exact lines on GitHub
Reproduction
Observed failure: Independently rerun; failed at the intended semantic assertion.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
Audit provenance
b206fa5d7655c1634c9993410a9203f6616a5ca2b206fa5d7655c1634c9993410a9203f6616a5ca2relsem-sql-empty-id-batchCloses EFF-554