Skip to content

Conversation

@kevingosse
Copy link
Contributor

  • Convert APIs to async to remove sync over async
  • Enforce one type per file where it makes sense
  • Move factory methods to non-generic static classes to take advantage of type inference
  • Rename interfaces with the I prefix
  • Refactor config overrides
  • Replace cursors with IAsyncEnumerable
  • Use IParsable<T> in BaseConfig to make code simpler and cleaner

Add I prefix to interface names.
Simplifies scorer instantiation by removing the IScorer.Of factory method and making FunctionScorer public. Updates the IScorer interface to use a Name property instead of GetName(), and modernizes code to use newer C# features. These changes improve API clarity and usability for defining custom scorers.
Refactors environment override handling in BraintrustConfig to use tuple-based syntax and nullable values, improving type safety and clarity. Updates constructors and tests to support explicit null overrides and removes legacy key-value array logic.
Replaces the custom ICursor interface with C# async streams for dataset case iteration. This modernizes the API, improves scalability, and simplifies usage by leveraging await foreach and IAsyncEnumerable throughout the codebase.
This helps discoverability (it is not expected to find static methods on interfaces) and allows users to take advantage of type inference.
Copilot AI review requested due to automatic review settings January 12, 2026 08:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the Braintrust SDK API to modernize the codebase with async/await patterns, improved type safety, and better organization. The refactoring eliminates sync-over-async anti-patterns, consolidates types into separate files following one-type-per-file convention, and replaces cursor-based iteration with IAsyncEnumerable.

Changes:

  • Converted all API methods to async, removing sync-over-async patterns throughout IBraintrustApiClient and BraintrustApiClient
  • Refactored config API from variadic string pairs to tuple-based overrides for better type safety and inference
  • Replaced cursor-based dataset iteration with IAsyncEnumerable pattern for cleaner async enumeration
  • Renamed interfaces with I prefix (Task → ITask, Scorer → IScorer, Dataset → IDataset) for .NET conventions
  • Extracted types into separate files (ApiException, InstrumentedChatClient, FunctionScorer, MockBraintrustApiClient)
  • Converted Score, TaskResult, and EvalResult to readonly structs for better performance
  • Updated BaseConfig to use IParsable constraint, simplifying type conversion logic

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/Braintrust.Sdk/Api/BraintrustApiClient.cs Converted all methods to async, added ConfigureAwait patterns
src/Braintrust.Sdk/Api/IBraintrustApiClient.cs Updated interface to async Task-returning methods
src/Braintrust.Sdk/Api/ApiException.cs Extracted to separate file following one-type-per-file
src/Braintrust.Sdk/Config/BaseConfig.cs Refactored to use IParsable constraint, changed to abstract class
src/Braintrust.Sdk/Config/BraintrustConfig.cs Updated to tuple-based config overrides API
src/Braintrust.Sdk/Eval/Eval.cs Converted Build to BuildAsync, RunAsync uses IAsyncEnumerable
src/Braintrust.Sdk/Eval/IDataset.cs Replaced ICursor with IAsyncEnumerable
src/Braintrust.Sdk/Eval/DatasetInMemoryImpl.cs Implemented GetCasesAsync with iterator pattern
src/Braintrust.Sdk/Eval/IScorer.cs Extracted interface, renamed from Scorer
src/Braintrust.Sdk/Eval/FunctionScorer.cs Made public, extracted to separate file
src/Braintrust.Sdk/Eval/Score.cs Converted to readonly record struct
src/Braintrust.Sdk/Eval/TaskResult.cs Converted to readonly record struct
src/Braintrust.Sdk/Eval/EvalResult.cs Converted to readonly struct with ToString override
src/Braintrust.Sdk/Braintrust.cs Changed ProjectUri to async GetProjectUriAsync
src/Braintrust.Sdk/Instrumentation/OpenAI/InstrumentedChatClient.cs Extracted to separate file
tests/Braintrust.Sdk.Tests/Eval/MockBraintrustApiClient.cs Extracted mock to separate file with async methods
examples/*/Program.cs Updated all examples to use async Main and await async APIs

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

public override ClientResult<ChatCompletion> CompleteChat(IEnumerable<ChatMessage> messages, ChatCompletionOptions? options = null, CancellationToken cancellationToken = default)
{
// Start a span for the chat completion
var activity = _activitySource.StartActivity("Chat Completion", ActivityKind.Client);
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

This variable is manually disposed in a finally block - consider a C# using statement as a preferable resource management technique.

Copilot uses AI. Check for mistakes.
public override async Task<ClientResult<ChatCompletion>> CompleteChatAsync(IEnumerable<ChatMessage> messages, ChatCompletionOptions? options = null, CancellationToken cancellationToken = default)
{
// Start a span for the chat completion
var activity = _activitySource.StartActivity("Chat Completion", ActivityKind.Client);
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

This variable is manually disposed in a finally block - consider a C# using statement as a preferable resource management technique.

Copilot uses AI. Check for mistakes.
Comment on lines +219 to +231
OrganizationAndProjectInfo? orgAndProject;

if (_projectId == null)
{
orgAndProject = await _apiClient.GetProjectAndOrgInfo().ConfigureAwait(false)
?? throw new InvalidOperationException("Unable to retrieve project and org info");
}
else
{
orgAndProject = await _apiClient.GetProjectAndOrgInfo(_projectId).ConfigureAwait(false)
?? throw new InvalidOperationException($"Invalid project id: {_projectId}");
}

Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Suggested change
OrganizationAndProjectInfo? orgAndProject;
if (_projectId == null)
{
orgAndProject = await _apiClient.GetProjectAndOrgInfo().ConfigureAwait(false)
?? throw new InvalidOperationException("Unable to retrieve project and org info");
}
else
{
orgAndProject = await _apiClient.GetProjectAndOrgInfo(_projectId).ConfigureAwait(false)
?? throw new InvalidOperationException($"Invalid project id: {_projectId}");
}
OrganizationAndProjectInfo? orgAndProject = _projectId == null
? await _apiClient.GetProjectAndOrgInfo().ConfigureAwait(false)
?? throw new InvalidOperationException("Unable to retrieve project and org info")
: await _apiClient.GetProjectAndOrgInfo(_projectId).ConfigureAwait(false)
?? throw new InvalidOperationException($"Invalid project id: {_projectId}");

Copilot uses AI. Check for mistakes.
@realark realark merged commit f79d677 into braintrustdata:main Jan 15, 2026
1 check failed
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