feat!: Split LdAiCompletionConfig from LdAiCompletionConfigDefault#277
Merged
jsonbailey merged 6 commits intoJun 3, 2026
Merged
Conversation
Separate the unified LdAiConfig into two public types that mirror the cross-SDK design: - LdAiCompletionConfig: the type returned by LdAiClient.CompletionConfig. Constructed only by the SDK, has a non-null CreateTracker() factory, and exposes the prompt/model/provider data. - LdAiCompletionConfigDefault: the user-input type passed as the defaultValue fallback. Has the public Builder, a Disabled factory, and a New() entry point. No tracker. Tracker construction is now driven through LdAiCompletionConfig.CreateTracker, which delegates to a required factory wired up at evaluation time. Tests were updated to reflect the new shape and to construct LdAiCompletionConfig through the SDK's internal constructor via InternalsVisibleTo. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Apply the design decisions from the multi-agent review of PR #277: - Option B inheritance: introduce two abstract record bases (LdAiConfigDefaultBase, LdAiConfigBase) in the Config namespace. The public default and live records each extend their own base; there is no IS-A relationship between Default and Live, removing the record equality / serialization / shadowing footguns of the alternative shape. The bases are marked EditorBrowsableState.Never so they stay out of IDE autocomplete. - Pull out top-level data types LdAiMessage, LdAiModel, LdAiProvider into LaunchDarkly.Sdk.Server.Ai.Config. The "ModelConfiguration" suffix is dropped now that the type lives at the top level. - LdAiCompletionConfigDefault no longer carries VariationKey or Version -- those are SDK-output metadata. The Default's ToLdValue() omits the variationKey/version entries from _ldMeta; LdAiClient synthesizes an empty Meta in the fallback path. - LdAiCompletionConfig is now sealed. - LdAiCompletionConfig gains a Key field (the AI Config key passed to CompletionConfig), shared via LdAiConfigBase. - Add IsExternalInit polyfill so `init`-only properties compile on netstandard2.0 and net462. Tests: - Add reflection-based test asserting LdAiCompletionConfig has no public constructors (locks in the SDK-only-construction invariant). - Add tests asserting CreateTracker() returns a working tracker on both fallback paths (parse failure and interpolation failure). - Fix the misleading "default enabled" comment in LdAiClientTest. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Introduce internal ConfigFactory that owns LdValue->Config translation, variable merging, message interpolation, and tracker factory wiring. LdAiClient becomes a thin facade over JsonVariation. - Replace ParseConfig + CompletionConfigFromDefault dual-fallback with a single tolerant parse. Missing or wrong-typed fields degrade to typed defaults; a malformed template degrades only that one message (raw content preserved). - Mode-mismatch handling: if _ldMeta.mode does not match the expected mode, log a warning and return a disabled config with a working tracker factory. Default _ldMeta.mode is "completion" when missing. - LdAiConfigTracker takes LdAiConfigBase (mode-agnostic); CreateTracker is concrete on the base; configs no longer need to override. - Convert config types (LdAiConfigBase, LdAiConfigDefaultBase, LdAiCompletionConfig, LdAiCompletionConfigDefault) from record to class. Removes the latent record-equality landmine on the tracker factory delegate. Pure data types (Message, ModelConfig, ProviderConfig) stay as records. - Rename data types to match the AISDK spec and drop redundant prefixes: LdAiMessage -> Message, LdAiModel -> ModelConfig, LdAiProvider -> ProviderConfig. - LdAiCompletionConfigDefault.Enabled defaults to true per AISDK spec Requirement 1.3.2. _ldMeta.mode = "completion" is baked into ToLdValue() so the default-swap path always round-trips cleanly. - Drop tracker.Config public property (caller already has the config). Restore ArgumentNullException on client/config in tracker ctor. Drop EditorBrowsable.Never on bases (cosmetic; abstract + private protected ctor enforce the real protection). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mattrmc1
approved these changes
Jun 2, 2026
Contributor
mattrmc1
left a comment
There was a problem hiding this comment.
A super minor nit but looks great. Pulled down and tested locally 👍
kinyoklion
reviewed
Jun 2, 2026
kinyoklion
approved these changes
Jun 2, 2026
…scing, extract Mode const
- Convert all eleven `{ get; init; }` accessors on the four config types
to `{ get; }`. The init accessor required a System.Runtime.CompilerServices.
IsExternalInit polyfill on netstandard2.0 / net462, which is no longer
needed since records were converted to classes (and the prior PR-level
pattern on main used `{ get; }` too).
- Delete src/Properties/IsExternalInit.cs.
- Move ownership of the shared fields (Key, Enabled, VariationKey,
Version, Model, Provider, plus the tracker factory) into LdAiConfigBase
and LdAiConfigDefaultBase ctors. Derived ctors forward via : base(...)
and only assign their per-mode Messages property.
- Centralize null-coalescing for Model and Provider on
LdAiConfigDefaultBase (mirroring LdAiConfigBase). Builder.Build()
constructs ModelConfig / ProviderConfig from its accumulated primitives
and passes them to the simpler internal ctor.
- Extract the magic string "completion" as
LdAiCompletionConfig.Mode (internal const). Three call sites updated:
ConfigFactory.ParseMeta default, ConfigFactory.BuildCompletionConfig
mismatch check, and LdAiCompletionConfigDefault.ToLdValue(). Future
agent / judge config types will own their own Mode constants.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cb7230d. Configure here.
Spec Req 1.2.3.4 step 2 (non-object variation) was previously silently degrading to per-field defaults; now we explicitly log an error and return the caller's default. Mode mismatch previously returned a synthetic disabled config; per sdk-specs#229 we now return the caller's default. Removes the AiConfig/Meta/Model/Provider/Message DTOs (unused since the factory switched to tolerant LdValue parsing) and moves the Role enum from LaunchDarkly.Sdk.Server.Ai.DataModel to LaunchDarkly.Sdk.Server.Ai.Config so it lives alongside Message. The DataModel namespace is removed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two findings from multi-agent review round 2: 1. LdAiCompletionConfigDefault.Builder.Build() passed its mutable model parameter dictionaries to ModelConfig by reference, so a caller mutating the Builder after Build() could observe through both the Default and the live config returned on the fallback path. Wrap them in new Dictionary(...) copies in Build() so the returned Default is structurally immutable. 2. ConfigFactory's mode-mismatch warning and non-object error logs interpolated server-controlled values directly into the format string passed to _logger.Warn / _logger.Error. The underlying LaunchDarkly logging facade calls String.Format on that string, so braces in _ldMeta.mode would throw FormatException and propagate up through CompletionConfig. Pass key, ldValue.Type, and mode as positional args so they're substituted as data rather than format syntax. Tests updated to verify the format string and args separately. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

BEGIN_COMMIT_OVERRIDE
feat!: Split unified LdAiConfig into LdAiCompletionConfig (SDK output) and LdAiCompletionConfigDefault (user input) — Builder, New(), and Disabled move to the Default type; introduce abstract LdAiConfig and LdAiConfigDefault base types for future agent/judge modes
feat!: Change CompletionConfig to return LdAiCompletionConfig instead of ILdAiConfigTracker — obtain a tracker via config.CreateTracker()
feat!: Default LdAiCompletionConfigDefault.Enabled to true per AISDK spec (was false on the LdAiConfig builder in 0.9.x)
feat!: Remove ILdAiConfigTracker.Config property — read config fields from the LdAiCompletionConfig the caller already holds
feat!: Make LdAiConfigTracker SDK-constructed only — the public constructor is removed; obtain trackers via config.CreateTracker() or ILdAiClient.CreateTracker(resumptionToken, context)
feat!: Convert LdAiCompletionConfig and LdAiCompletionConfigDefault from records to classes — equality is reference-based instead of structural
feat!: Remove LaunchDarkly.Sdk.Server.Ai.DataModel namespace — delete unused AiConfig, Meta, Model, Provider, and Message JSON DTO classes
feat: Tolerant LdValue parsing — missing or wrong-typed fields degrade to typed defaults instead of discarding the whole config
feat: Per-message interpolation fallback — a malformed Mustache template keeps raw content for that message rather than discarding the entire config
feat: Mode-mismatch detection — log a warning and return the caller's default when the flag's _ldMeta.mode does not match the requested mode (per sdk-specs#229)
feat: Non-object variation handling — log an error and return the caller's default when the variation result is not an object
END_COMMIT_OVERRIDE
Summary
This change splits the unified
LdAiConfigfrom 0.9.x into two public sealed types so that the data a user supplies as a fallback is clearly separated from the data the SDK returns:LdAiCompletionConfigis whatLdAiClient.CompletionConfigreturns. It is constructed only by the SDK, exposes the evaluated config data, and provides aCreateTracker()method that always returns a non-null tracker. There is no public constructor, no public Builder, and noDisabledfactory.LdAiCompletionConfigDefaultis the user-input type passed as thedefaultValue:parameter. It carries the prompt/model/provider data but has no tracker. Construction uses the publicBuilder,New(), andDisabledfactory.Tracker construction is driven by a required factory wired up at evaluation time inside
LdAiClient.LdAiCompletionConfig.CreateTracker()delegates to that factory, so callers no longer have to deal with a nullable tracker -- even on fallback paths (parse failure, interpolation failure).Type shape (Option B: parallel hierarchies)
Two abstract bases factor out the shared structure, but the Default and Live types do not inherit from each other:
Users should reference the concrete sealed types (
LdAiCompletionConfig,LdAiCompletionConfigDefault). Shared payload types (Message,Role,ModelConfig,ProviderConfig) live under staticLdAiConfigTypesin theConfignamespace (see #284).This matches the cross-SDK pattern (parallel hierarchies in python and ruby; live-extends-omit-default in js-core). Future
LdAiAgentConfig/LdAiJudgeConfigslot in symmetrically -- each Default extendsLdAiConfigDefault, each Live extendsLdAiConfig.Why this lands separately from #249
PR #249 contains concurrency fixes for the AI SDK. It rebased on top of this once this merged so that both reviews stay focused.
Migration
Before (0.9.x):
After (0.10.0):
The obsolete
Config(...)method is preserved with the same[Obsolete]attribute; only its parameter and return types are updated to match.Test plan
dotnet buildsucceeds acrossnetstandard2.0,net462,net8.0dotnet test --framework net8.0passes (53/53)LdAiCompletionConfighas no public constructors (locks in the SDK-only-construction invariant)CreateTracker()returns a working tracker on both the parse-failure and interpolation-failure fallback pathsNote
High Risk
Large breaking public API surface (types, return shapes, tracker construction) plus new evaluation/fallback semantics that integrators must migrate to; behavior changes on parse/interpolation errors affect production prompts and telemetry wiring.
Overview
Breaking API redesign for the server AI SDK: unified
LdAiConfigis replaced byLdAiCompletionConfig(SDK evaluation result) andLdAiCompletionConfigDefault(caller fallback withNew/Builder/Disabled).CompletionConfignow returns the config object; usage tracking isconfig.CreateTracker()instead of receiving a tracker directly.ILdAiConfigTracker.Configis removed.Evaluation logic moves into internal
ConfigFactory, which parses flag JSON asLdValue(theDataModelJSON DTOs are deleted).LdAiClientis slimmed down toJsonVariation+ factory build. Fallback behavior is more tolerant: wrong type or_ldMeta.modemismatch uses the caller’s default (with logging); bad Mustache templates fail per message instead of dropping the whole config. Shared shapes live onLdAiConfigBase/LdAiConfigDefaultBasewith top-levelMessage,ModelConfig,ProviderConfig,Role. Default builderEnableddefaults to true; main config types are classes (reference equality).LdAiConfigTrackeris internal and keyed offconfig.Key.Reviewed by Cursor Bugbot for commit 86ff9c8. Bugbot is set up for automated code reviews on this repo. Configure here.