feat: typed AI configs, defaults, and root-level tools#9
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ 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 1536ec7. Configure here.
| return 1 | ||
| } | ||
| return c.version | ||
| } |
There was a problem hiding this comment.
Version() conflates explicit zero with unset version
Medium Severity
Version() checks c.version == 0 to return 1, but the old code checked c.c.Meta.Version == nil. In evaluateConfig, a wire version of 0 (*int pointing to 0) is stored as version = 0, making it indistinguishable from "version not set." The old code correctly returned 0 for an explicit wire version of 0. This changes behavior for tracking events where version is emitted via the Tracker's trackData.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1536ec7. Configure here.
| // returnDefault sets a tracker factory on a copy of def (so CreateTracker always works) and | ||
| // returns the resulting Config. Used for all error-path returns in evaluateConfig. | ||
| func (c *Client) returnDefault(key string, context ldcontext.Context, def Config) Config { | ||
| def.key = key |
There was a problem hiding this comment.
returnDefault doesn't ensure non-nil evaluator
Low Severity
returnDefault sets key and trackerFactory on the default but does not set evaluator. The Evaluator() accessor documents "Never returns nil," but if a caller passes a directly constructed AICompletionConfig{} (without using Disabled() or Build()), the returned config will have a nil evaluator, violating the contract and risking nil-pointer panics downstream.
Reviewed by Cursor Bugbot for commit 1536ec7. Configure here.


Summary
Introduces typed config types (
AICompletionConfig,AIAgentConfig,AIJudgeConfig) backed by a sharedaiConfigBase, along with typed defaults,ToolConfigfor root-level tool definitions, and a noopEvaluatorstub. The existingConfigtype is preserved as a deprecated alias for backward compatibility.Shared base via
aiConfigBaseAll three typed configs embed
aiConfigBase, which holds common fields and exposes:Two new public types —
ModelConfig(Name, Parameters, Custom) andProviderConfig(Name) — replace the previous pattern of accessor methods likeModelName(),ProviderName(),ModelParam().Typed config types
AICompletionConfig— embedsaiConfigBase, addsMessages(),JudgeConfiguration(),AsLdValue(). Retains deprecated accessors (VariationKey(),Version(),ModelName(),ProviderName(),ModelParam(),CustomModelParam(),Mode(),EvaluationMetricKey(),EvaluationMetricKeys()) for backward compatibility.AIAgentConfig— embedsaiConfigBase, addsInstructions(),JudgeConfiguration().AIJudgeConfig— embedsaiConfigBase, addsMessages(),EvaluationMetricKey().Backward compatibility
ConfigBuilderis preserved with deprecation markers. ItsBuild()now constructs anAICompletionConfiginternally. All existing callers ofCompletionConfig()continue to work unchanged.Typed defaults with immutable-copy builder
Three default types (
AICompletionConfigDefault,AIAgentConfigDefault,AIJudgeConfigDefault) use value-receiverWith*methods that return copies:All three defaults have
WithModelParamandWithCustomModelParam.AIAgentConfigDefaultadditionally hasWithToolandWithJudgeConfiguration. Each providesDisabled()andAsLdValue().ToolConfig— root-level tool definitionsNew
ToolConfigtype withName(),Description(),Type(),Parameters(),CustomParameters()— all map accessors return defensive copies viamaps.Clone. Backed by a newdatamodel.Toolwire type withtoolsandinstructionsfields added todatamodel.Config.model.parameters.tools[](the LLM-passable array) is preserved verbatim — the rawdatamodel.Configis kept onAICompletionConfigforAsLdValue()marshaling.evaluateConfigrewriteclient.go'sevaluateConfigno longer usesConfigBuilder. It constructsAICompletionConfigdirectly, parsing root-level tools viatoolConfigFromWire, cloning model params/custom viamaps.Clone, and settingkeyon both the happy path andreturnDefault.Noop evaluator stub
Evaluatoris a struct stub inevaluator.go.newNoopEvaluator()returns a non-nil instance so thatEvaluator()on all typed configs is safe to call. Full evaluator implementation deferred to Task 05.Test plan
go test -race ./...passesAICompletionConfig: messages, model, provider, tools, key, enabled, tracker, evaluator all accessibleAIAgentConfig: instructions, judgeConfiguration accessibleAIJudgeConfig: messages, evaluationMetricKey accessibleConfigalias: existingCompletionConfig()call site works unchangedConfigBuilder.Build()produces a validAICompletionConfigToolConfigaccessors return correct values; mutating returned maps does not affect configmodel.parameters.tools[]preserved verbatim inAsLdValue()outputDisabled()producesenabled=false;AsLdValue()marshals correctlyWithModelParam/WithCustomModelParampresent and functional on all three defaultsWithTool/WithJudgeConfigurationfunctional onAIAgentConfigDefaultKey()returns the flag key on configs from bothevaluateConfigandreturnDefaultEvaluator()returns non-nil noop evaluator on all typed configsNote
Medium Risk
Public API surface and config evaluation paths change substantially, though backward compatibility is preserved via aliases and deprecated accessors; incorrect handling of tools vs model.parameters.tools could affect LLM payloads.
Overview
Replaces the single
Configtype with mode-specificAICompletionConfig,AIAgentConfig, andAIJudgeConfig, all embedding sharedaiConfigBase(Key,Model/Providerstructs, rootTools(),CreateTracker, non-nilEvaluator()).Configremains a deprecated type alias toAICompletionConfig;ConfigBuilder.Build()now producesAICompletionConfigwith a retainedrawwire payload forAsLdValue().Adds immutable
With*default types for completion, agent, and judge fallbacks, plusToolConfigbacked by new wiretoolsandinstructionsondatamodel.Config.evaluateConfigno longer usesConfigBuilder—it buildsAICompletionConfigdirectly, parses root tools viatoolConfigFromWire, setskeyon defaults, and keepsmodel.parameters.tools[]untouched inraw. A noopEvaluatorstub is wired everywhere;judge’sConfiginterface assertion targetsAICompletionConfig.Reviewed by Cursor Bugbot for commit 1536ec7. Bugbot is set up for automated code reviews on this repo. Configure here.