Flush events eagerly on TOKEN_COUNT to prevent memory buildup - #1974
Conversation
The publisher's interval-based flush only fires when the next event arrives. Between TOOL_RESULT and the next AI_MESSAGE the LLM is being re-invoked, which typically takes >1s, so pending tool result events sit in memory and subscribers see nothing during the wait. Promote TOOL_RESULT to flush-immediately so subscribers get tool results as soon as they're produced. Signed-off-by: Claude <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
WalkthroughThe event publisher now treats Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:8fb6f745
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:8fb6f745 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:8fb6f745
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:8fb6f745
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:8fb6f745
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:8fb6f745 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:8fb6f745
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:8fb6f745Patch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:8fb6f745 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:8fb6f745Robusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:8fb6f745 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:8fb6f745 |
📂 Previous Runs📜 #4 · Run @ __5bed630__ (#25191649474) — Apr 30, 22:12 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit 5bed630 on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 74 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
📜 #3 · Run @ __0573c65__ (#25177025163) — Apr 30, 16:35 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit 0573c65 on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 74 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
📜 #2 · Run @ __3950bce__ (#25176554505) — Apr 30, 16:25 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit 3950bce on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 74 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
📜 #1 · Run @ __259f00e__ (#25173791298) — Apr 30, 15:28 UTC✅ Results of HolmesGPT evalsAutomatically triggered by commit 259f00e on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 74 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
✅ Results of HolmesGPT evalsAutomatically triggered by commit 5e033ba on branch Results of HolmesGPT evals
Benchmark Comparison DetailsBaseline: latest ci-benchmark experiment on master Status: Success - 73 test/model combinations loaded Benchmark experiment:
No benchmark data available for comparison. Benchmark has no cost, total tokens, cached tokens data. Will appear after the next weekly benchmark run. Comparison indicators:
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" Option 3: Add PR labels to include extra evals (applies to both automatic runs and
Examples: 🏷️ Valid tags
🤖 Valid models
Commands: CLI: |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
call_stream() emits TOKEN_COUNT at two boundaries per loop iteration: right after the LLM response (before tool execution) and right after the last TOOL_RESULT of a parallel batch (before the next LLM call). Both precede a long-running step (>1s tool work or LLM call), so flushing here keeps subscribers up to date without the per-tool write amplification of flushing on every TOOL_RESULT. Signed-off-by: Claude <noreply@anthropic.com>
…gnHo' into claude/flush-events-before-llm-egnHo
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/core/conversations_worker/test_event_publisher.py`:
- Line 324: Replace the ambiguous multiplication glyph in the comment "Second
flush: START_TOOL + TOOL_RESULT × 2 + TOKEN_COUNT" in test_event_publisher (the
test_event_publisher module) with a plain ASCII "x" to satisfy Ruff (RUF003);
i.e., change "×" to "x" so the comment reads "Second flush: START_TOOL +
TOOL_RESULT x 2 + TOKEN_COUNT".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3806a66-a802-4518-b1b0-e30da9c5c813
📒 Files selected for processing (2)
holmes/core/conversations_worker/event_publisher.pytests/core/conversations_worker/test_event_publisher.py
naomi-robusta
changed the title
4 participants
Summary
Modified the event publisher to flush pending events immediately when a
TOOL_RESULTevent is encountered, preventing events from sitting in memory during subsequent LLM calls.Key Changes
StreamEvents.TOOL_RESULTto the_FLUSH_IMMEDIATELY_EVENTSset inevent_publisher.pytest_publisher_flushes_eagerly_on_tool_result()that verifies:Implementation Details
The change treats TOOL_RESULT similarly to terminal events (ANSWER_END, etc.) in terms of triggering immediate flushes, but with different semantics: rather than ending a turn, TOOL_RESULT marks a transition point where the next operation (another LLM call) may take significant time. This prevents a buildup of pending events in memory during that latency window.
https://claude.ai/code/session_01LjFK8k38XDifb4Q3LbuXcu
Summary by CodeRabbit
Bug Fixes
Tests