feat: Implement optimization code paths and functionality for initial release#140
feat: Implement optimization code paths and functionality for initial release#140andrewklatzke wants to merge 40 commits intomainfrom
Conversation
…ype, remove required context_choices argument and default to anon
|
|
||
| # variation() returns the raw JSON before chevron.render(), so instructions | ||
| # still contain {{placeholder}} tokens rather than empty strings. | ||
| raw_variation = self._ldClient._client.variation(agent_key, context, {}) |
There was a problem hiding this comment.
Not a blocker but I would be careful relying on the hidden property. Maybe this is something we need to expose in the public api?
There was a problem hiding this comment.
It's not really "hidden" in the sense that the user shouldn't be able to access it if they're determined. It's just a property that's only really used internally. We're using this to re-derive the variables that were present in the initial variation so that the LLM has a stable reference when trying to replace them
| Errors are caught and logged rather than raised so that persistence | ||
| failures never abort an in-progress optimization run. | ||
|
|
There was a problem hiding this comment.
Will this handle things like backof/throttle/retry in the event of 429? Maybe aborting an in-progress optimization is best in cases of permanent errors (eg 401, indicates an invalid token), or even sporadic errors like 403, where certain requests fail, and others succeed, but it would lead to incorrect results?
There was a problem hiding this comment.
The api key gets checked up-front with the fetch for model configs etc. so shouldn't error unless it's revoked midway through. Given that, I'd say it makes sense to fail on any 401s here.
I'll add some retry logic for other status codes up to a max and then fail the optimization if we hit max retries
| "current_parameters": { | ||
| "type": "object", | ||
| "description": "The improved agent parameters (e.g., temperature, max_tokens, etc.)", | ||
| "additionalProperties": True, |
There was a problem hiding this comment.
What does this do? Does it allow the LLM to emit arbitrary keys, which then get persisted on agent_optimization_result.parameters and, on auto_commit, pushed as the new AI Config variation's parameters? It looks like we only expect certain values here?
Maybe after extract_json_from_response, filter current_parameters to a known allow-list of LLM call parameters — e.g., {temperature, top_p, max_tokens, presence_penalty, frequency_penalty, stop} — and drop unknown keys with a warning log. This also belongs in the server-side validation on POST /ai-configs/{config_key}/variations.
There was a problem hiding this comment.
Let me look into this, it may only exist for posterity at this point (moved away from tool-based validation on these since some providers didn't play nicely with it).
In the case we need to keep it, we unfortunately cannot know what is valid in this parameters object given that the user provides their LLM call. Each provider's parameters differ so we could only allow-list if we knew up-front which provider they were using
| "Failed to extract JSON from response. " | ||
| "Response length: %d, response: %s", | ||
| len(response_str), | ||
| response_str, |
There was a problem hiding this comment.
This is not really safe to log, as it likely contains sensitive info
| ] | ||
| dependencies = [ | ||
| "launchdarkly-server-sdk-ai>=0.16.0", | ||
| "coolname>=2.0.0", |
There was a problem hiding this comment.
Do we need this? Why such an old version? Is having cool names for variation keys worth the risk of this dependency? Would the 'cool' names even be useful/meaningful, vs a default date-based name?
There was a problem hiding this comment.
Discussed in review meeting; will just hand roll this as its pretty simple to implement
| Attempts direct JSON parsing first, then progressively falls back to | ||
| extracting JSON from markdown code blocks and balanced-brace scanning. |
There was a problem hiding this comment.
the flexibility here is neat, but it increases the risks of adversarial JSON smuggling.
Would it be possible to get metrics from how many times the fallback methods are used?
Maybe if a fallback method was used, we should check to see if there are unexpected keys in the resulting object, and WARN-log it, to hopefully alert the customer that something is amiss?
There was a problem hiding this comment.
Yeah this is basically just fallback parsing if the LLM decides to respond with something like:
Here is your JSON:
{...}
Unfortunately since we don't know which provider the user is using for this we can't enforce any structured output (for those that even support it).
I think it's worth emitting a warn on
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes 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 7468372. Configure here.
|
New commits added with security review results + cursor feedback on those changes: this includes:
Includes:
Includes:
|
|
Includes:
|
| @@ -1,7 +1,7 @@ | |||
| [project] | |||
| name = "launchdarkly-server-sdk-ai-optimization" | |||
| name = "ldai_optimizer" | |||
There was a problem hiding this comment.
I won't block on this but all other launchdarkly python packages have the launchdarkly-* name.

Requirements
Related issues
This PR encapsulates all previous changes in the chain of optimization PRs that were broken up into smaller pieces. Consolidating here so that we can have a single commit/release of the package. The PRs were independently reviewed and approved.
Describe the solution you've provided
See:
#116
#117
#119
#122
#127
#128
#130
#135
#139
Note
High Risk
Large new implementation that introduces iterative prompt-optimization logic plus authenticated LaunchDarkly REST API calls (config fetch, result persistence, variation creation), so regressions could affect correctness and API-side state.
Overview
Implements the initial end-to-end
ldai_optimizerpackage: a newOptimizationClientruns iterative agent variation generation, judge-based evaluation, optional validation sampling, and supports both option-driven runs and config-driven runs via the LaunchDarkly REST API.Adds supporting modules for typed option/context dataclasses, prompt construction, slug generation, JSON extraction/validation, token/duration tracking, logging redaction, and an internal REST client with retries; also renames packaging/build targets and updates docs/metadata from the old placeholder
ldai_optimizationscaffold to the publishableldai_optimizerdistribution.Reviewed by Cursor Bugbot for commit e8c6692. Bugbot is set up for automated code reviews on this repo. Configure here.