Skip to content

Conversation

aaronvg
Copy link
Contributor

@aaronvg aaronvg commented Sep 20, 2025

Done mostly by codex. seems to pass the failing imports in typescript-esm now. But need to run the react code still

Copy link

vercel bot commented Sep 20, 2025

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

Project Deployment Preview Comments Updated (UTC)
promptfiddle Ready Ready Preview Comment Sep 21, 2025 8:06pm

Copy link

Copy link

cursor[bot]

This comment was marked as outdated.

Copy link

codecov bot commented Sep 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link

🔒 Entelligence AI Vulnerability Scanner

❌ Security analysis failed: Security analysis failed: Command exited with code 1 and error:
sudo: unknown user sonarqube
sudo: error initializing audit plugin sudoers_audit

Copy link

Review Summary

🏷️ Draft Comments (8)

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

engine/language_client_typescript/async_context_vars.js (1)

73-76,94-97: args.reduce((acc, arg, i) => ({ ...acc, [arg${i}]: arg })) in traceFnSync and traceFnAsync creates a new object on every iteration, causing O(n²) time and memory usage for large argument lists.

📊 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 engine/language_client_typescript/async_context_vars.js, lines 73-76 and 94-97, replace the use of `args.reduce` with object spread (which is O(n²)) for building `params` with a simple for-loop that assigns each argument to the params object. This will reduce time and memory complexity from O(n²) to O(n) for large argument lists.

engine/language_client_typescript/audio.js (1)

34-44: fromBlob reads the entire Blob into memory and base64-encodes it, which can cause high memory usage and slow performance for large audio files.

📊 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/language_client_typescript/audio.js, lines 34-44, the `fromBlob` method reads the entire Blob into memory and base64-encodes it, which is inefficient for large audio files. Update this method to use `URL.createObjectURL` for blobs larger than 5MB (or another reasonable threshold), returning a 'url' BamlAudio instead of base64. For smaller blobs, keep the current base64 logic. Ensure the code preserves original formatting and functionality.

engine/language_client_typescript/errors.js (1)

133-133: BamlAbortError.from always sets reason to undefined and detailed_message to empty string, losing original error details and causing incorrect error reconstruction.

📊 Impact Scores:

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

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/language_client_typescript/errors.js, line 133, the static from() method of BamlAbortError always sets reason to undefined and detailed_message to an empty string, which loses important error details. Update this line to extract reason and detailed_message from the error object if present, so that the reconstructed error preserves the original information.

engine/language_client_typescript/pdf.js (1)

34-44: fromBlob reads the entire Blob into memory as a base64 string, which can cause significant memory and performance issues for large PDF files in browsers.

📊 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/language_client_typescript/pdf.js, lines 34-44, the `fromBlob` method reads the entire Blob into memory as a base64 string, which can cause significant memory and performance issues for large PDF files. Update this method to check the Blob size and throw an error or warn if the file is too large (e.g., >10MB), to prevent browser memory exhaustion. If possible, suggest alternative handling for large files.

engine/language_client_typescript/stream.js (2)

9-9: eventQueue is implemented as an array with shift(), causing O(n) per dequeue; this will degrade performance with large or high-frequency streams.

📊 Impact Scores:

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

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/language_client_typescript/stream.js, lines 9-9, the `eventQueue` is implemented as a standard array and uses `shift()` for dequeueing, which is O(n) and will cause significant performance degradation for large or high-frequency streams. Refactor `eventQueue` to use a queue structure with O(1) enqueue and dequeue operations (e.g., a ring buffer or an object with head/tail pointers). Update all code that pushes to or shifts from `eventQueue` accordingly.

71-72: The async iterator uses setTimeout polling (100ms) to wait for events, causing unnecessary CPU wakeups and latency under load.

📊 Impact Scores:

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

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/language_client_typescript/stream.js, lines 71-72, the async iterator waits for new events by polling with `setTimeout`, which is inefficient and can cause unnecessary CPU usage and increased latency, especially under high concurrency. Refactor this logic to use a more efficient signaling mechanism (such as a Promise that resolves when a new event is pushed to the queue) to avoid polling.

engine/language_client_typescript/typescript_src/async_context_vars.ts (1)

84-90: args.reduce in both traceFnSync and traceFnAsync creates a new object per argument, causing O(n) object spreads and memory churn for large arg lists; this is inefficient for high-frequency or large-argument calls.

📊 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 engine/language_client_typescript/typescript_src/async_context_vars.ts, lines 84-90, replace the use of `args.reduce` with repeated object spreads in `traceFnSync` with a single for-loop that builds the params object. This avoids O(n) object allocations and improves performance for large or frequent argument lists.

engine/language_client_typescript/typescript_src/stream.ts (1)

90-95: eventQueue is polled with a fixed 100ms delay in the async iterator, causing unnecessary latency and CPU wakeups under high concurrency or large-scale streaming.

📊 Impact Scores:

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

🤖 AI Agent Prompt (Copy & Paste Ready):

In engine/language_client_typescript/typescript_src/stream.ts, lines 90-95, the async iterator polls `eventQueue` with a fixed 100ms delay, which can cause unnecessary latency and CPU wakeups at scale. Refactor this section to use a promise-based approach that resolves as soon as an event is available, reducing idle waiting and improving responsiveness. Replace the polling loop with a function that checks the queue and waits with a short delay (e.g., 10ms) only if the queue is empty, resolving immediately when data arrives.

🔍 Comments beyond diff scope (4)
engine/language_client_typescript/async_context_vars.js (1)

29-30: upsertTags will throw if this.ctx.getStore() returns undefined, as manager will be undefined and manager.upsertTags(tags) will cause a runtime error.
Category: correctness


engine/language_client_typescript/image.js (1)

51-51: fromBlob returns a Promise resolving to a BamlImage, but fromUrlAsync does not await it, returning a Promise<Promise> instead of Promise.
Category: correctness


engine/language_client_typescript/type_builder.js (1)

114-115: ClassViewer.property(name) and ClassBuilder.property(name) throw if name is not in this.properties, but this.properties is not updated if properties are added/removed via the underlying builder, causing false negatives.
Category: correctness


engine/language_client_typescript/video.js (1)

51-51: fromBlob returns a Promise resolving to a BamlVideo, but fromUrlAsync does not await it, returning a Promise<Promise> instead of Promise.
Category: correctness


Comment on lines +173 to 175
const props = this.rawProperties()
return Object.keys(props).map((name) => [name, new ClassPropertyViewer()])
}

Choose a reason for hiding this comment

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

correctness: ClassViewer.listProperties() and ClassBuilder.listProperties() return all properties from the underlying class, not just those in the properties set, which can cause contract violations and unexpected results.

🤖 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/language_client_typescript/typescript_src/type_builder.ts, lines 173-175, the implementation of ClassViewer.listProperties() returns all properties from the underlying class, not just those tracked in the `properties` set. This can violate the class contract and produce unexpected results. Update the method so it only returns properties present in `this.properties` and that exist in the raw properties object.
📝 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
const props = this.rawProperties()
return Object.keys(props).map((name) => [name, new ClassPropertyViewer()])
}
const props = this.rawProperties()
return Array.from(this.properties).filter((name) => name in props).map((name) => [name, new ClassPropertyViewer()])

@aaronvg aaronvg temporarily deployed to boundary-tools-dev September 21, 2025 19:49 — with GitHub Actions Inactive
@aaronvg aaronvg temporarily deployed to boundary-tools-dev September 21, 2025 19:49 — with GitHub Actions Inactive
@aaronvg aaronvg temporarily deployed to boundary-tools-dev September 21, 2025 19:49 — with GitHub Actions Inactive
Copy link

Copy link

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.

1 participant