You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
useStreamingText documents cancel as: "Stop consuming the current source; no later chunk from it reaches
state. Idempotent, safe post-unmount." (packages/loopover-ui-kit/src/hooks/use-streaming-text.ts:22). Its
implementation is gated only on the per-effect cancelled flag:
cancelled is only ever set by cancel() itself and by the effect's cleanup (line 76-78, i.e. a new source or
unmount). It is not set when the stream settles. So after the worker reaches a terminal state, cancelled
is still false and a later cancel() call is accepted and overwrites the status:
Stream completed. The loop drains and setStatus("done") runs (line 67). A subsequent cancel() sets
status to "cancelled". A consumer rendering "done" UI flips back to a cancelled state for a stream that
finished successfully.
Stream failed.setStatus("error") runs (line 71) with error populated. A subsequent cancel() sets
status to "cancelled" while error stays non-null — an inconsistent pair
(status === "cancelled" && error !== null) that no branch of StreamingStatus describes, and which hides
a real failure from any consumer switching on status.
No source at all. With source === null the worker takes the if (!source) { setStatus("idle"); return; }
path (lines 57-60) and never touches cancelled. cancel() then moves an idle hook — one that never
started a stream — to "cancelled".
The last case is the clearest: const { status, cancel } = useStreamingText(null) starts at "idle", and
calling cancel() reports "cancelled" even though nothing was ever consumed.
packages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts covers cancel-mid-stream, source swap, unmount
and mid-stream error, but never calls cancel() after a stream settles or on a null source, so none of this
is pinned.
Requirements
cancel() must be a no-op when the current effect run has already settled — that is, when the worker has
already reached idle, done or error for the current source. In those cases status, text and error must all be left exactly as they are.
cancel() must keep working unchanged while the stream is in flight: it must set status to "cancelled",
stop consumption, and prevent any later chunk from that source from reaching state.
cancel() must remain idempotent and must remain safe to call after unmount (no state write, no throw).
Settling must be tracked per effect run, in the same closure that owns cancelled — a useRef shared across
runs would let a settled previous stream suppress a cancel on the new one. Starting a new source must
restore cancellability.
The StreamingTextState shape, the ChunkSource type, the cancelled-guarded state-write discipline (every
write inside the async worker, each guarded), and the [source] dependency array must NOT change.
The JSDoc on cancel (line 22) must be updated to state that it is a no-op once the stream has settled.
⚠️ Required pattern: add a second per-effect closure flag alongside let cancelled = false (line 41) and set
it on each of the three terminal transitions (lines 58, 67, 71), then gate cancelRef.current on it — the
file's existing "per-effect flag, fresh closure each run" discipline, spelled out in the comment at lines
39-40. What does NOT satisfy this issue: (a) reading the status state variable inside cancelRef.current,
which closes over a stale value from the render in which the effect ran; (b) hoisting the flag into a useRef shared across effect runs, which breaks the source-swap case; (c) adding a new exported status such
as "settled", which changes the public StreamingStatus union; (d) making cancel() unconditionally
no-op, which removes the feature; (e) a test-only PR.
Deliverables
renderHook(() => useStreamingText(null)) then act(() => result.current.cancel()) leaves result.current.status as "idle", asserted in packages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts.
A source that is driven to completion (status "done", text accumulated) then cancel()ed leaves status === "done" and text unchanged, asserted in the same file.
A source that throws mid-stream (status "error", error.message set) then cancel()ed leaves status === "error" and error non-null, asserted in the same file.
The existing "cancel() stops the stream and no later chunk reaches state" test still passes unmodified,
proving in-flight cancellation is unchanged.
A test in the same file asserting that after a completed stream is cancel()ed, swapping in a NEW
source starts it (status reaches "streaming") and that source can then be cancelled — pinning that
settling is per-effect-run, not sticky.
A regression test at packages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts named for this bug.
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that fixes the done case but leaves cancel() on a null source reporting "cancelled", or that omits the
new-source-after-settle test so a sticky flag would pass — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, packages/loopover-engine/src/**/*.ts, packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts, packages/loopover-contract/src/**/*.ts and packages/discovery-index/src/**/*.ts. It does not cover packages/loopover-ui-kit/**, and that package's own packages/loopover-ui-kit/vitest.config.ts deliberately
declares no coverage block. Codecov does not gate the patch on this change.
The tests are still mandatory and are named above: they go in the existing packages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts and must be run with npm --workspace @loopover/ui-kit run test before pushing (that suite is not currently reached by npm run test:ci, so a local run is the only way to see it pass).
Both arms of every branch the change touches need a test in that file: the new settled-guard (cancel while
in-flight -> "cancelled"; cancel after settle -> unchanged) for each of the three terminal transitions
(idle via a null source, done, error); the existing if (!cancelled) guard inside cancelRef.current
(first call vs second, idempotence); the if (cancelled) return guard inside the chunk loop; and the
post-unmount path (cleanup sets cancelled, so cancel() writes nothing).
Expected Outcome
cancel() means "stop the stream that is running", not "overwrite whatever the last status was": a completed
stream stays done, a failed stream stays error with its Error intact, and a hook with no source stays idle — while cancelling an in-flight stream behaves exactly as it does today.
Links & Resources
packages/loopover-ui-kit/src/hooks/use-streaming-text.ts:22 — the cancel contract in JSDoc
packages/loopover-ui-kit/src/hooks/use-streaming-text.ts:41-47 — cancelRef.current and the cancelled flag
packages/loopover-ui-kit/src/hooks/use-streaming-text.ts:57-60,67,71 — the three terminal transitions
packages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts — the existing suite
Context
useStreamingTextdocumentscancelas: "Stop consuming the current source; no later chunk from it reachesstate. Idempotent, safe post-unmount." (
packages/loopover-ui-kit/src/hooks/use-streaming-text.ts:22). Itsimplementation is gated only on the per-effect
cancelledflag:cancelledis only ever set bycancel()itself and by the effect's cleanup (line 76-78, i.e. a new source orunmount). It is not set when the stream settles. So after the worker reaches a terminal state,
cancelledis still
falseand a latercancel()call is accepted and overwrites the status:setStatus("done")runs (line 67). A subsequentcancel()setsstatus to
"cancelled". A consumer rendering "done" UI flips back to a cancelled state for a stream thatfinished successfully.
setStatus("error")runs (line 71) witherrorpopulated. A subsequentcancel()setsstatus to
"cancelled"whileerrorstays non-null — an inconsistent pair(
status === "cancelled" && error !== null) that no branch ofStreamingStatusdescribes, and which hidesa real failure from any consumer switching on
status.source === nullthe worker takes theif (!source) { setStatus("idle"); return; }path (lines 57-60) and never touches
cancelled.cancel()then moves an idle hook — one that neverstarted a stream — to
"cancelled".The last case is the clearest:
const { status, cancel } = useStreamingText(null)starts at"idle", andcalling
cancel()reports"cancelled"even though nothing was ever consumed.packages/loopover-ui-kit/src/hooks/use-streaming-text.test.tscovers cancel-mid-stream, source swap, unmountand mid-stream error, but never calls
cancel()after a stream settles or on a null source, so none of thisis pinned.
Requirements
cancel()must be a no-op when the current effect run has already settled — that is, when the worker hasalready reached
idle,doneorerrorfor the currentsource. In those casesstatus,textanderrormust all be left exactly as they are.cancel()must keep working unchanged while the stream is in flight: it must setstatusto"cancelled",stop consumption, and prevent any later chunk from that source from reaching state.
cancel()must remain idempotent and must remain safe to call after unmount (no state write, no throw).cancelled— auseRefshared acrossruns would let a settled previous stream suppress a cancel on the new one. Starting a new
sourcemustrestore cancellability.
StreamingTextStateshape, theChunkSourcetype, thecancelled-guarded state-write discipline (everywrite inside the async worker, each guarded), and the
[source]dependency array must NOT change.cancel(line 22) must be updated to state that it is a no-op once the stream has settled.Deliverables
renderHook(() => useStreamingText(null))thenact(() => result.current.cancel())leavesresult.current.statusas"idle", asserted inpackages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts."done", text accumulated) thencancel()ed leavesstatus === "done"andtextunchanged, asserted in the same file."error",error.messageset) thencancel()ed leavesstatus === "error"anderrornon-null, asserted in the same file.proving in-flight cancellation is unchanged.
cancel()ed, swapping in a NEWsource starts it (
statusreaches"streaming") and that source can then be cancelled — pinning thatsettling is per-effect-run, not sticky.
packages/loopover-ui-kit/src/hooks/use-streaming-text.test.tsnamed for this bug.All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that fixes the
donecase but leavescancel()on a null source reporting"cancelled", or that omits thenew-source-after-settle test so a sticky flag would pass — does not resolve this issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted.
vitest.config.ts'scoverage.includecovers
src/**/*.ts,packages/loopover-engine/src/**/*.ts,packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts,packages/loopover-contract/src/**/*.tsandpackages/discovery-index/src/**/*.ts. It does not coverpackages/loopover-ui-kit/**, and that package's ownpackages/loopover-ui-kit/vitest.config.tsdeliberatelydeclares no
coverageblock. Codecov does not gate the patch on this change.The tests are still mandatory and are named above: they go in the existing
packages/loopover-ui-kit/src/hooks/use-streaming-text.test.tsand must be run withnpm --workspace @loopover/ui-kit run testbefore pushing (that suite is not currently reached bynpm run test:ci, so a local run is the only way to see it pass).Both arms of every branch the change touches need a test in that file: the new settled-guard (cancel while
in-flight ->
"cancelled"; cancel after settle -> unchanged) for each of the three terminal transitions(
idlevia a null source,done,error); the existingif (!cancelled)guard insidecancelRef.current(first call vs second, idempotence); the
if (cancelled) returnguard inside the chunk loop; and thepost-unmount path (cleanup sets
cancelled, socancel()writes nothing).Expected Outcome
cancel()means "stop the stream that is running", not "overwrite whatever the last status was": a completedstream stays
done, a failed stream stayserrorwith itsErrorintact, and a hook with no source staysidle— while cancelling an in-flight stream behaves exactly as it does today.Links & Resources
packages/loopover-ui-kit/src/hooks/use-streaming-text.ts:22— thecancelcontract in JSDocpackages/loopover-ui-kit/src/hooks/use-streaming-text.ts:41-47—cancelRef.currentand thecancelledflagpackages/loopover-ui-kit/src/hooks/use-streaming-text.ts:57-60,67,71— the three terminal transitionspackages/loopover-ui-kit/src/hooks/use-streaming-text.test.ts— the existing suite