Commit 69418c8
authored
chore: Fix ldctx silent override and Add Tools to completion config (3 of 6) (#281)
BEGIN_COMMIT_OVERRIDE
feat: Add Tools property to LdAiCompletionConfig — parses the same tools
block agents use, exposes IReadOnlyDictionary<string, ToolConfig> (empty
when absent)
fix: Silently override 'ldctx' in user-supplied template variables
instead of warning and discarding it — the SDK context always wins,
matches cross-SDK behavior
END_COMMIT_OVERRIDE
## Summary
Two tightly-scoped changes to the **Completion** path:
### 1. `Tools` on `LdAiCompletionConfig`
`LdAiCompletionConfig` now exposes `Tools` (an
`IReadOnlyDictionary<string, ToolConfig>`), populated from the `tools`
block on the variation JSON. `ConfigFactory.BuildCompletionConfig` and
`BuildFromDefault` both pass it through; on the default path it's an
empty dictionary.
`tools` is a model-level field rather than an agent-only one, so it now
lives on both completion and agent configs.
### 2. Silent `ldctx` override
`ConfigFactory.MergeVariables` previously warned-and-dropped when a
caller passed an `ldctx` key in their template variables. That diverged
from the other SDKs (python, js-core, ruby) where `ldctx` is silently
overwritten by the LaunchDarkly context. The behavior is now:
- User-supplied variables are seeded first.
- `ldctx` is added last, silently overriding any caller-supplied value.
- No warning is logged. The caller's `ldctx` value never reaches the
Mustache template.
The reserved-key warning is gone; the comment explaining the override is
moved to the line that does it. Practical effect: callers who try to set
`ldctx` explicitly no longer get a confusing "this was ignored" log
line, and the template always sees the real LD context's attributes.
## Migration
`Tools` is additive — no migration needed; existing callers that ignore
it see no change.
The `ldctx` warning change is a **silent behavior change** for any
caller who was previously relying on the warning to spot the
reserved-key collision. If a caller was depending on the warning to flag
an unintended override, they will no longer see it. In practice this
matches what all other SDKs already do.
## Test plan
- [ ] `dotnet build` succeeds across `netstandard2.0`, `net462`,
`net8.0`
- [ ] `dotnet test --framework net8.0` passes
- [ ] `LdAiClientTest.LdCtxOverrideIsSilent` — verifies the SDK
context's `key` wins over a user-supplied `ldctx`, with **no** warning
emitted
- [ ] `LdAiClientTest.ToolsPopulatedOnCompletionConfig` — covers a
`tools` block on a completion config flag, including `customParameters`
- [ ] `LdAiClientTest.ToolsEmptyWhenAbsentFromJson` — covers the
missing-`tools` case, asserting an empty (non-null) dictionary
- [ ] `LdAiConfigTrackerTest` updated to pass `tools` through the new
constructor signature
- [ ] Reviewer confirms the silent-override semantics match python /
js-core / ruby
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Additive public API and template-variable behavior aligned with other
SDKs; only callers who relied on the removed `ldctx` warning are
affected.
>
> **Overview**
> **Completion configs** now surface **`Tools`** (parsed from variation
JSON via shared `ParseTools`, empty when missing) and
**`JudgeConfiguration`** (parsed when present, carried through caller
defaults and `LdAiCompletionConfigDefault` builder/`ToLdValue`
roundtrips).
>
> **Template variables:** `MergeVariables` no longer logs a warning or
drops a caller-supplied **`ldctx`** key—it seeds user vars first, then
always sets **`ldctx`** from the LaunchDarkly context so Mustache
templates match other SDKs.
>
> Tests cover silent `ldctx` override, tools presence/absence, judge
config parsing and default fallbacks, and updated `LdAiCompletionConfig`
constructor usage in tracker tests.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
fc3487e. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent a744d60 commit 69418c8
5 files changed
Lines changed: 319 additions & 13 deletions
File tree
- pkgs/sdk/server-ai
- src/Config
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
62 | 64 | | |
63 | 65 | | |
64 | 66 | | |
65 | 67 | | |
66 | 68 | | |
67 | 69 | | |
68 | 70 | | |
| 71 | + | |
| 72 | + | |
69 | 73 | | |
70 | 74 | | |
71 | 75 | | |
| |||
86 | 90 | | |
87 | 91 | | |
88 | 92 | | |
| 93 | + | |
| 94 | + | |
89 | 95 | | |
90 | 96 | | |
91 | 97 | | |
| |||
399 | 405 | | |
400 | 406 | | |
401 | 407 | | |
402 | | - | |
| 408 | + | |
403 | 409 | | |
404 | 410 | | |
405 | | - | |
406 | | - | |
407 | 411 | | |
408 | 412 | | |
409 | 413 | | |
410 | 414 | | |
411 | 415 | | |
412 | | - | |
413 | | - | |
414 | | - | |
415 | | - | |
416 | | - | |
417 | | - | |
418 | 416 | | |
419 | 417 | | |
420 | 418 | | |
| 419 | + | |
421 | 420 | | |
422 | 421 | | |
423 | 422 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
3 | 4 | | |
4 | 5 | | |
5 | 6 | | |
| |||
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
30 | 41 | | |
31 | | - | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
32 | 45 | | |
33 | 46 | | |
34 | 47 | | |
35 | 48 | | |
| 49 | + | |
| 50 | + | |
36 | 51 | | |
37 | 52 | | |
Lines changed: 39 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| 35 | + | |
34 | 36 | | |
35 | 37 | | |
36 | 38 | | |
| |||
118 | 120 | | |
119 | 121 | | |
120 | 122 | | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
121 | 134 | | |
122 | 135 | | |
123 | 136 | | |
| |||
129 | 142 | | |
130 | 143 | | |
131 | 144 | | |
132 | | - | |
| 145 | + | |
133 | 146 | | |
134 | 147 | | |
135 | 148 | | |
| |||
138 | 151 | | |
139 | 152 | | |
140 | 153 | | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
141 | 159 | | |
| 160 | + | |
142 | 161 | | |
143 | 162 | | |
144 | 163 | | |
145 | 164 | | |
| 165 | + | |
146 | 166 | | |
147 | 167 | | |
148 | 168 | | |
| |||
153 | 173 | | |
154 | 174 | | |
155 | 175 | | |
156 | | - | |
| 176 | + | |
157 | 177 | | |
158 | 178 | | |
159 | 179 | | |
| |||
171 | 191 | | |
172 | 192 | | |
173 | 193 | | |
174 | | - | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
175 | 211 | | |
176 | 212 | | |
177 | 213 | | |
| |||
0 commit comments