Skip to content

fix(ollama): yield during dense stream processing#86633

Closed
udaymanish6 wants to merge 1 commit into
openclaw:mainfrom
udaymanish6:fix/86599-local-provider-event-loop
Closed

fix(ollama): yield during dense stream processing#86633
udaymanish6 wants to merge 1 commit into
openclaw:mainfrom
udaymanish6:fix/86599-local-provider-event-loop

Conversation

@udaymanish6

@udaymanish6 udaymanish6 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add cooperative yields while processing dense Ollama NDJSON stream chunks
  • preserve abort checks inside the stream loop
  • add a regression test covering event-loop yielding under a dense chunk burst

Real behavior proof

Behavior or issue addressed: Ollama stream chunk processing was monopolizing the event loop on dense native streams.
Real environment tested: local OpenClaw checkout in /Users/miya/Desktop/openclaw-issue-86599.
Exact steps or command run after this patch: pnpm test extensions/ollama/src/stream.test.ts
Evidence after fix:

Test Files  1 passed (1)
Tests  9 passed (9)
Start at  17:03:09
Duration  2.02s

Observed result after fix: the dense-stream regression test passed after the cooperative-yield change.
What was not tested: I did not reproduce the Windows beta gateway loop on a live Windows beta machine in this environment.

Verification

  • pnpm test extensions/ollama/src/stream.test.ts

@openclaw-barnacle openclaw-barnacle Bot added extensions: ollama size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the useful Ollama dense-stream mitigation has been ported onto a current maintainer branch with stronger proof, while this contributor branch is now dirty and no longer the best landing candidate.

Canonical path: Close this dirty branch and let #87818 carry the same narrow Ollama mitigation; keep the broader gateway starvation work tracked separately.

So I’m closing this here and keeping the remaining discussion on #87818.

Review details

Best possible solution:

Close this dirty branch and let #87818 carry the same narrow Ollama mitigation; keep the broader gateway starvation work tracked separately.

Do we have a high-confidence way to reproduce the issue?

Yes for the narrow source path: current main drains dense native Ollama chunks without a cooperative scheduler, and the patch adds a timer-progress regression test. The full Windows beta gateway starvation report remains broader than this PR.

Is this the best way to solve the issue?

Yes for the narrow mitigation, but not via this branch. The better solution is the rebased maintainer PR because it ports the same fix onto current main and carries stronger real-behavior proof.

Security review:

Security review cleared: The diff only changes Ollama stream scheduling and a regression test; it adds no dependency, workflow, credential, download, install, or package-resolution surface.

AGENTS.md: found and applied where relevant.

What I checked:

  • Repository policy read: Root AGENTS.md and the scoped extensions guide were read fully; the applicable guidance is to review bundled plugin runtime changes against the plugin boundary and require real behavior proof for bug-fix PRs. (AGENTS.md:1, 1e48ca4e3251)
  • Current main stream behavior: Current main still processes parsed Ollama chunks in the native stream loop without the cooperative scheduler added by this PR, so this is not implemented on main yet. (extensions/ollama/src/stream.ts:1291, 1e48ca4e3251)
  • This PR diff: The contributor branch adds a local scheduler, abort checks, and awaits the scheduler after non-final Ollama stream chunks; it also adds the dense native stream timer regression test. (extensions/ollama/src/stream.ts:50, e40e6a473100)
  • Superseding PR diff: The replacement branch at fix(ollama): yield during dense stream processing #87818 applies the same scheduler/test mitigation onto the newer Ollama stream layout, with only line offsets and current-main function names differing. (extensions/ollama/src/stream.ts:51, c0f574bf4f1e)
  • Live replacement state: Live GitHub API shows this PR is open but dirty, while fix(ollama): yield during dense stream processing #87818 is open, maintainer-owned, labeled proof:sufficient, and ready for maintainer look. (c0f574bf4f1e)
  • Maintainer proof and scope comments: Maintainer comments scope this as a native-Ollama partial mitigation, confirm AWS Crabbox dense-stream test proof plus live Ollama Cloud smoke, and keep the broader Windows/local-provider starvation issue separate.

Likely related people:

  • vincentkoc: Opened the current maintainer branch that ports the same mitigation, carries the proof-sufficient label, and records Crabbox plus Ollama Cloud proof. (role: canonical replacement branch owner; confidence: high; commits: c0f574bf4f1e; files: extensions/ollama/src/stream.ts, extensions/ollama/src/stream.test.ts)
  • osolmaz: Commented that this PR is a partial native-Ollama mitigation and opened the broader dense-stream gateway follow-up. (role: related reviewer and broader follow-up owner; confidence: medium; commits: 14013f7b57d7; files: extensions/ollama/src/stream.ts, src/agents/embedded-agent-subscribe.handlers.messages.test.ts)
  • steipete: Local blame in this checkout attributes the current Ollama stream loop and parser file state to a recent main commit by Peter Steinberger. (role: recent area contributor; confidence: medium; commits: f2dfb67f2c5b; files: extensions/ollama/src/stream.ts, extensions/ollama/src/stream.test.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 1e48ca4e3251.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@osolmaz

osolmaz commented May 28, 2026

Copy link
Copy Markdown
Member

Related: #86599.

This PR addresses one contributor from that issue: native Ollama can deliver dense stream chunks, and the Ollama parser can process many of them before the gateway gets a chance to handle other work.

I would scope this as a related but partial mitigation. It may improve the native-Ollama dense-stream path, but #86599 is broader unless we also prove the gateway stays responsive during the original repro and address the repeated full-partial stream work discussed there.

@vincentkoc

Copy link
Copy Markdown
Member

Maintainer verification for the native Ollama mitigation, tested on the PR patch rebased onto current main because the original branch now conflicts with newer Ollama tests.

Behavior addressed: native Ollama stream processing should yield during dense stream bursts, and the patched branch should still pass a real Ollama provider smoke.

Real environment tested: AWS Crabbox Linux, provider=aws, lease=cbx_e26f0fab127c, run=run_4597e73408f8 for the dense-stream test and run=run_4c1504086f15 for live Ollama Cloud.

Exact steps or command run after this patch:

OPENCLAW_VITEST_MAX_WORKERS=1 timeout 240s corepack pnpm test extensions/ollama/src/stream.test.ts -- --reporter=verbose

OPENCLAW_LIVE_TEST=1 \
OPENCLAW_LIVE_OLLAMA=1 \
OPENCLAW_LIVE_OLLAMA_BASE_URL=https://ollama.com \
OPENCLAW_LIVE_OLLAMA_MODEL=gemma3:4b \
OPENCLAW_LIVE_OLLAMA_WEB_SEARCH=0 \
OPENCLAW_LIVE_OLLAMA_EMBEDDINGS=0 \
OPENCLAW_VITEST_MAX_WORKERS=1 \
timeout 360s corepack pnpm test:live -- extensions/ollama/ollama.live.test.ts

Evidence after fix:

extensions/ollama/src/stream.test.ts: 11 passed, including "yields to the event loop while processing dense native stream chunks"
extensions/ollama/ollama.live.test.ts: 2 passed, 2 skipped by config
live model: gemma3:4b via https://ollama.com

Observed result after fix: the dense NDJSON parser regression passes, and the patched Ollama runtime can complete both the CLI local transport live smoke and native chat live smoke against a real Ollama Cloud model.

What was not tested: this is still not a full Windows local-daemon reproduction of #86599. I would still scope this PR as a native-Ollama mitigation, not the complete fix for the broader full-partial/repeated-work issue.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: ollama P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants