Skip to content

Conversation

@standujar
Copy link
Collaborator

@standujar standujar commented Dec 30, 2025

Summary

Streaming model calls were not being logged to the database because the logging code was placed after the return statement in the streaming path. This caused LLM calls (TEXT_SMALL, TEXT_LARGE) to not appear in the Model Calls panel.

Changes

  • packages/core/src/runtime.ts

    • Extract model logging logic into logModelCall() helper method to avoid duplication
    • Call helper in both streaming and non-streaming paths (fixes the bug)
  • packages/client/src/hooks/use-query-hooks.ts

    • Filter client logs to show useModel:* and action types
  • packages/client/src/components/agent-action-viewer.tsx

    • Add REASONING type support in model usage categorization
    • Simplify getModelUsageType() function
  • packages/core/src/tests/runtime-streaming.test.ts

    • Add tests for database logging in streaming and non-streaming paths

Test plan

  • Start an agent and have a conversation
  • Verify LLM calls appear in the Model Calls panel with correct type (LLM, TEXT_LARGE, etc.)
  • Verify streaming responses are logged
  • Verify non-streaming responses are still logged
  • Run bun test packages/core/src/__tests__/runtime-streaming.test.ts - 13 tests pass

Screenshots

Before: Only embeddings and events visible, LLM calls missing

After:

Capture d’écran 2025-12-30 à 11 27 18

Greptile Summary

Fixed critical bug where streaming LLM calls (TEXT_SMALL, TEXT_LARGE) were not being logged to the database because the logging code was unreachable after the return statement in the streaming path. The fix extracts logging logic into a reusable logModelCall() helper method that is called in both streaming and non-streaming code paths.

Key changes:

  • Extracted database logging into logModelCall() helper method in packages/core/src/runtime.ts:2172-2222
  • Streaming path now calls logModelCall() after accumulating full text at line 2390
  • Non-streaming path refactored to use the same helper at line 2418
  • Added comprehensive test coverage for both paths
  • Client filtering now correctly displays model calls with useModel:* types
  • Added support for REASONING model type and improved UI categorization

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The fix correctly addresses a real bug with a clean refactoring approach, includes comprehensive test coverage for both code paths, and all changes follow existing patterns in the codebase
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/src/runtime.ts Extracted logging logic into logModelCall() helper and fixed streaming path bug by calling it after stream completes
packages/core/src/tests/runtime-streaming.test.ts Added comprehensive tests for database logging in both streaming and non-streaming paths
packages/client/src/hooks/use-query-hooks.ts Filtered logs to show only useModel:* and action types in Model Calls panel
packages/client/src/components/agent-action-viewer.tsx Added REASONING model type support, simplified categorization logic, and added embedding filter option

Sequence Diagram

sequenceDiagram
    participant Client as Client UI
    participant Runtime as AgentRuntime
    participant Handler as Model Handler
    participant DB as Database Adapter
    
    Note over Client,DB: Streaming Path (Before Fix)
    Client->>Runtime: useModel(TEXT_LARGE, {onStreamChunk, prompt})
    Runtime->>Handler: handler(runtime, params)
    Handler-->>Runtime: TextStreamResult with textStream
    loop For each chunk
        Runtime->>Runtime: Iterate textStream
        Runtime->>Client: onStreamChunk(chunk)
    end
    Runtime->>Runtime: return fullText
    Note over Runtime,DB: ❌ Return happened before logging
    
    Note over Client,DB: Streaming Path (After Fix)
    Client->>Runtime: useModel(TEXT_LARGE, {onStreamChunk, prompt})
    Runtime->>Handler: handler(runtime, params)
    Handler-->>Runtime: TextStreamResult with textStream
    loop For each chunk
        Runtime->>Runtime: Iterate textStream
        Runtime->>Client: onStreamChunk(chunk)
    end
    Runtime->>Runtime: logModelCall(modelType, params, fullText)
    Runtime->>DB: adapter.log({type: 'useModel:TEXT_LARGE', body: {...}})
    Runtime->>Runtime: return fullText
    
    Note over Client,DB: Non-Streaming Path (Unchanged)
    Client->>Runtime: useModel(TEXT_LARGE, {prompt})
    Runtime->>Handler: handler(runtime, params)
    Handler-->>Runtime: response string
    Runtime->>Runtime: logModelCall(modelType, params, response)
    Runtime->>DB: adapter.log({type: 'useModel:TEXT_LARGE', body: {...}})
    Runtime->>Runtime: return response
Loading

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 30, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/streaming-model-calls-logging

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude

This comment was marked as duplicate.

@claude
Copy link
Contributor

claude bot commented Dec 30, 2025

Claude finished @standujar's task —— View job


PR Review Complete ✅

Reviewing PR #6296: fix(core): log streaming LLM calls to database

Code Review Analysis:

GOOD IMPLEMENTATION:

  • Fixes genuine bug where streaming model calls weren't logged due to logging code after return statement
  • Clean refactoring: extracted logModelCall() helper method eliminates code duplication
  • Proper test coverage added for both streaming and non-streaming paths
  • Uses bun:test framework correctly per ElizaOS guidelines
  • TypeScript types are properly defined, no 'any' usage

TESTING:

  • Added comprehensive tests in runtime-streaming.test.ts
  • Tests verify database logging for both streaming/non-streaming calls
  • Mock implementation follows existing patterns
  • 13 tests reported as passing in PR description

ARCHITECTURE:

  • Client changes properly filter logs to show relevant types (useModel:*, action)
  • Component changes add REASONING type support for model categorization
  • Simplified getModelUsageType() function is cleaner

SECURITY & STANDARDS:

  • No hardcoded keys or security issues
  • Proper error handling in place
  • Uses correct imports and follows ElizaOS patterns
  • No breaking changes or migration requirements

Minor Observations:

  • Runtime uses class-based approach (existing pattern, not introduced by this PR)
  • All code changes are focused and minimal for the bug fix
  • Good separation of concerns: core logic in runtime, UI updates in client

Verdict: This is a solid bug fix with proper testing and clean implementation. Approves fix for missing LLM call logging in streaming mode.


@greptile-apps
Copy link

greptile-apps bot commented Dec 30, 2025

Greptile found no issues!

From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

@ChristopherTrimboli ChristopherTrimboli merged commit 2aae5af into develop Dec 30, 2025
19 checks passed
@ChristopherTrimboli ChristopherTrimboli deleted the fix/streaming-model-calls-logging branch December 30, 2025 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants