Skip to content

Conversation

@aaronvg
Copy link
Contributor

@aaronvg aaronvg commented Sep 27, 2025

  • checkpoint
  • Fix streaming not running the parser, vscode image rendering issue, openai responses bug, and vertex anthropic streaming

Note

Refactors streaming to parse/deliver partials reliably, corrects OpenAI Responses message/media formatting, enables Anthropic-on-Vertex streaming, and improves playground cURL preview/copy behavior.

  • Runtime/Streaming:
    • Decouple SSE consumption from parsing with a throttled parser loop and shared ParserState; emit deduped partials via watch snapshots.
    • Improve final fallback/error (ErrorCode::Other(2), message) and cancellation handling.
  • Providers:
    • OpenAI Responses: Normalize content parts (input_text, input_image with detail: auto, input_file with filename), disallow assistant media, and add unit test for mixed text+file.
    • Vertex (Anthropic on Vertex): Route streaming via ResponseType::Anthropic when anthropic_version is set and set stream: true in request body.
    • Streaming request: Raise SSE event log level to debug.
  • Tracing:
    • Downgrade BAML source upload check failures from error to warn.
  • Playground UI:
    • Stabilize cURL preview: export curlAtom, memoize highlighter, retain last cURL during loading, and support copying cURL when cURL tab is active.
    • VSCode API wrapper: minor API cleanups for media file loading.
  • Tests:
    • Update/expand integration tests for Responses API (image detail, PDF filename), Anthropic/Vertex streaming, and timing/format tweaks.

Written by Cursor Bugbot for commit 2181a28. This will update automatically on new commits. Configure here.

@vercel
Copy link

vercel bot commented Sep 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
promptfiddle Skipped Skipped Sep 27, 2025 10:24pm

@aaronvg aaronvg temporarily deployed to boundary-tools-dev September 27, 2025 21:03 — with GitHub Actions Inactive
@aaronvg aaronvg temporarily deployed to boundary-tools-dev September 27, 2025 21:03 — with GitHub Actions Inactive
@aaronvg aaronvg enabled auto-merge September 27, 2025 21:04
@github-actions
Copy link

@entelligence-ai-pr-reviews
Copy link

🔒 Entelligence AI Vulnerability Scanner

No security vulnerabilities found!

Your code passed our comprehensive security analysis.

📊 Files Analyzed: 3 files


@codecov
Copy link

codecov bot commented Sep 27, 2025

@entelligence-ai-pr-reviews
Copy link

Review Summary

🏷️ Draft Comments (11)

Skipped posting 11 draft comments that were valid but scored below your review threshold (>=10/15). Feel free to update them here.

engine/baml-runtime/src/internal/llm_client/orchestrator/stream.rs (1)

244-266: If the SSE stream ends before any LLMResponse::Success is received, no error event is emitted to downstream listeners, violating the contract for error propagation.

📊 Impact Scores:

  • Production Impact: 3/5
  • Fix Specificity: 2/5
  • Urgency Impact: 3/5
  • Total Score: 8/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/baml-runtime/src/internal/llm_client/orchestrator/stream.rs, lines 244-266, if the SSE stream ends before any LLMResponse::Success is received, no error event is emitted to downstream listeners, violating the error propagation contract. Update the SSE consumer future to emit a final error event if no success events were received before stream end.

engine/baml-runtime/src/internal/llm_client/primitive/openai/openai_client.rs (1)

141-285: The deeply nested closure in ProviderStrategy::ResponsesApi's build_body (lines 141-285) is highly complex, making it difficult to maintain and extend, increasing risk of bugs and performance regressions as message part types grow.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 3/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Refactor the closure in ProviderStrategy::ResponsesApi's build_body (engine/baml-runtime/src/internal/llm_client/primitive/openai/openai_client.rs, lines 141-285) to extract the message part-to-JSON logic into a dedicated helper function. This will reduce cyclomatic complexity, improve maintainability, and make it easier to add new media types or validation logic. Ensure the new helper preserves all current logic and error handling.

engine/baml-runtime/src/internal/llm_client/primitive/stream_request.rs (2)

189-191: scan closure returns None if accumulated is Err, causing the stream to end silently and skip emitting a final error if the parser fails mid-stream.

📊 Impact Scores:

  • Production Impact: 4/5
  • Fix Specificity: 2/5
  • Urgency Impact: 3/5
  • Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/baml-runtime/src/internal/llm_client/primitive/stream_request.rs, lines 189-191, the scan closure currently returns None if `accumulated` is Err, which causes the stream to end without emitting a final error. Change the Err branch to return `Some(LLMResponse::LLMFailure(e.clone()))` instead of None, so that a final error is emitted downstream.

74-95: The .take_while closure (lines 74-93) processes every SSE event, including potentially high-frequency, high-volume streams, and logs every event at debug level (line 95), which can cause substantial I/O and CPU overhead in production environments with large or frequent streams.

📊 Impact Scores:

  • Production Impact: 3/5
  • Fix Specificity: 4/5
  • Urgency Impact: 2/5
  • Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/baml-runtime/src/internal/llm_client/primitive/stream_request.rs, lines 74-95, the code logs every SSE event at debug level, which can cause significant I/O and CPU overhead for high-frequency or large streams. Update the code so that debug logging only occurs if debug logging is enabled (using log::log_enabled!), to avoid unnecessary string formatting and log I/O in production. Do not remove the logging entirely, but guard it efficiently.

engine/baml-runtime/src/tracingv2/publisher/publisher.rs (1)

326-402: TracePublisher::run (lines 326-402) is an extremely long and complex function with deeply nested logic, making it difficult to understand and maintain.

📊 Impact Scores:

  • Production Impact: 1/5
  • Fix Specificity: 4/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

Refactor the function `TracePublisher::run` in engine/baml-runtime/src/tracingv2/publisher/publisher.rs (lines 326-402). The function is excessively long and deeply nested, which significantly impairs readability and maintainability. Extract the main message and tick handling logic into separate helper async methods (e.g., `handle_publisher_message` and `handle_tick`) to reduce complexity and improve structure, while preserving all original logic and indentation.

typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-preview-curl.tsx (3)

82-224: SyntaxHighlightedCurl re-creates and re-initializes the Shiki highlighter for every instance, causing redundant async work and memory usage, especially if multiple cURL previews are rendered (e.g., in a list or with rapid tab switching).

📊 Impact Scores:

  • Production Impact: 3/5
  • Fix Specificity: 4/5
  • Urgency Impact: 2/5
  • Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-preview-curl.tsx, lines 82-224, refactor `SyntaxHighlightedCurl` so that the Shiki highlighter is created only once and shared across all component instances (e.g., via a module-level singleton or promise). This will prevent redundant async work and memory usage when multiple cURL previews are rendered. Replace the current per-instance highlighter state/effect with a global highlighter promise and update the highlighting logic accordingly.

29-42: Unused variables files (line 29) and hasApiKeys (line 37) are declared but never used, introducing dead code that reduces maintainability and increases cognitive load.

📊 Impact Scores:

  • Production Impact: 1/5
  • Fix Specificity: 4/5
  • Urgency Impact: 2/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-preview-curl.tsx, lines 29-42, remove the unused variables `files` and `hasApiKeys` as they are declared but never used. This will eliminate dead code and improve maintainability.

84-84: Use of any type for highlighter state (line 84) undermines type safety and maintainability, making the codebase more error-prone and harder to refactor.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 2/5
  • Urgency Impact: 2/5
  • Total Score: 6/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-preview-curl.tsx, line 84, replace the use of `any` in the `highlighter` state with a precise type based on the return type of `createHighlighterCore` from 'shiki/core' to improve type safety and maintainability.

typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-render-wrapper.tsx (2)

23-23: isSidebarOpen is assigned but never used in FunctionMetadata, introducing dead code and reducing maintainability.

📊 Impact Scores:

  • Production Impact: 1/5
  • Fix Specificity: 5/5
  • Urgency Impact: 1/5
  • Total Score: 7/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-render-wrapper.tsx, line 23: Remove the unused variable assignment `const { open: isSidebarOpen } = useSidebar();` from the `FunctionMetadata` component to eliminate dead code and improve maintainability.

132-132: Use of any in onValueChange={(v) => setActiveTab(v as any)} undermines type safety and violates TypeScript best practices.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 5/5
  • Urgency Impact: 2/5
  • Total Score: 9/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In typescript/packages/playground-common/src/shared/baml-project-panel/playground-panel/prompt-preview/prompt-render-wrapper.tsx, line 132: Replace the use of `any` in `setActiveTab(v as any)` with an explicit union type cast to `'preview' | 'curl' | 'client-graph' | 'mermaid-graph'` to maintain type safety and adhere to TypeScript best practices.

typescript/packages/playground-common/src/shared/baml-project-panel/vscode.ts (1)

29-30, 64-64, 442-442: Use of explicit any types in function signatures (e.g., acquireVsCodeApi(): any, private listenForRpcResponses(event: any)) undermines type safety and maintainability in a TypeScript codebase.

📊 Impact Scores:

  • Production Impact: 2/5
  • Fix Specificity: 4/5
  • Urgency Impact: 2/5
  • Total Score: 8/15

🤖 AI Agent Prompt (Copy & Paste Ready):

In typescript/packages/playground-common/src/shared/baml-project-panel/vscode.ts, on lines 29-30, 64, and 442, replace all explicit `any` types in function signatures and class properties with more specific types. For `acquireVsCodeApi`, define a minimal interface for the returned object (e.g., with `postMessage`, `getState`, `setState`). For `vsCodeApi`, use this interface instead of `any`. For `listenForRpcResponses`, use `MessageEvent` instead of `any` for the event parameter. This will improve type safety and maintainability.

🔍 Comments beyond diff scope (3)
engine/baml-runtime/src/internal/llm_client/primitive/vertex/response_handler.rs (1)

83-83: usage_metadata is unwrapped without checking for None, causing a panic if missing in the response and crashing the application.
Category: correctness


typescript/packages/playground-common/src/shared/baml-project-panel/vscode.ts (2)

378-383: loadMediaFile in non-VSCode environments does not check if response.uri is present before using it, which can cause a runtime error if the server returns a malformed response.
Category: correctness


403-439: rpc uses new Promise(async ...) which creates an async executor; this causes unhandled promise rejections and memory leaks under load.
Category: performance


Ok(response) => response,
Err(e) => {
tracing::error!("Failed to check BAML source upload status: {}", e);
tracing::warn!("Failed to check BAML source upload status: {}", e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness: process_baml_src_upload_impl downgrades all BAML source upload check failures to warn, which may hide critical errors (e.g., authentication failures) that should abort publishing and not be silently ignored.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In engine/baml-runtime/src/tracingv2/publisher/publisher.rs, line 550, the code downgrades all errors from the BAML source upload check to a warning. This can hide critical issues (like authentication failures) that should abort publishing. Change the log level from warn back to error to ensure critical failures are not silently ignored.
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
tracing::warn!("Failed to check BAML source upload status: {}", e);
tracing::error!("Failed to check BAML source upload status: {}", e);

@github-actions
Copy link

@github-actions
Copy link

@github-actions
Copy link

@aaronvg aaronvg added this pull request to the merge queue Sep 27, 2025
Merged via the queue into canary with commit 4bb2f33 Sep 27, 2025
25 of 26 checks passed
@aaronvg aaronvg deleted the streamfix branch September 27, 2025 22:44
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.

2 participants