Preserve lexical ordering in streaming template interpolation - #6988
Conversation
fubhy
added
the
audit
🦋 Changeset detectedLatest commit: 46b805a 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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
This PR currently adds only the failing regression test; the implementation fix that would make it pass is still missing. Merging now would leave main with a failing test and broken CI.
Reviewed changes
- Added
packages/effect/test/unstable/http/Template.test.tswith a deterministicTemplate.streamordering regression test. The test correctly fails under the current implementation (abfastcslowvs. expectedaslowbfastc) and will pass once the flattening is made sequential.
⚠️ Implementation fix still needed
The reproduction test is good, but the corresponding fix in packages/effect/src/unstable/http/Template.ts is not included. The current implementation flattens the ordered chunks array with { concurrency: "unbounded" }, which allows later segments to overtake earlier delayed interpolations.
Remove the concurrency option (or use Stream.concatMap) at packages/effect/src/unstable/http/Template.ts:218-223 so each chunk is emitted in lexical template order.
Technical details
# Missing sequential flattening for Template.stream
## Affected sites
- packages/effect/src/unstable/http/Template.ts:218-223 — `Stream.flatMap` with `{ concurrency: "unbounded" }` reorders segments
## Required outcome
- `Template.stream` must emit static and interpolated segments in lexical template order.
- The reproduction test `packages/effect/test/unstable/http/Template.test.ts` must pass.
## Suggested approach
- Remove `{ concurrency: "unbounded" }` from the `Stream.flatMap` call, or replace it with `Stream.concatMap`.@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
| @@ -218,8 +218,7 @@ export function stream<A extends ReadonlyArray<InterpolatedWithStream>>( | |||
| return Stream.flatMap( | |||
There was a problem hiding this comment.
Maybe we can use mapEffect instead to preserve order and concurrency?
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Replaced the original
Stream.flatMap(..., { concurrency: "unbounded" })inpackages/effect/src/unstable/http/Template.tswith a two-stage pipeline:Stream.mapEffect(..., { concurrency: "unbounded" })evaluates effect interpolations concurrently, and a sequentialStream.flatMapflattens the results in lexical template order. - Added a regression test (
"preserves template segment order") that verifies a delayed interpolation no longer gets overtaken by later segments. - Added a second test (
"evaluates effect interpolations concurrently") that confirms the two-stage design still starts consecutive effect interpolations in parallel. - Added a patch changeset
.changeset/tidy-cats-stream.md.
@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.
|
2 participants
Summary
Template.stream allows later static and fast segments to overtake an earlier delayed interpolation, changing a${slow}b${fast}c into abfastcslow rather than aslowbfastc.
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.
Streaming interpolation does not preserve template order
Module:
TemplateAudit ID:
unstable-http-template-stream-orderSeverity / confidence: medium / high
What happens
Template.stream allows later static and fast segments to overtake an earlier delayed interpolation, changing a${slow}b${fast}c into abfastcslow rather than aslowbfastc.
Why it happens
The lexical segment stream is flattened with concurrency set to unbounded, so later segments can emit before an earlier effect or stream interpolation completes.
Expected behavior
A template stream must emit static and interpolated segments in lexical template order.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/unstable/http/Template.ts:218-223View problematic code at
packages/effect/src/unstable/http/Template.ts:218-223View exact lines on GitHub
Reproduction
pnpm test --run packages/effect/test/unstable/http/Template.test.tsObserved failure: Failed deterministically under TestClock with abfastcslow instead of aslowbfastc.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/effect/test/unstable/http/Template.test.tsAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715funstable-http-template-stream-orderCloses EFF-425