feat(server-ai): stamp modelKey and modelVersion on AI usage events (…#308
Open
atornsii wants to merge 4 commits into
Open
feat(server-ai): stamp modelKey and modelVersion on AI usage events (…#308atornsii wants to merge 4 commits into
atornsii wants to merge 4 commits into
Conversation
…AIC-2856) Read modelKey and modelVersion from the AI Config variation payload and stamp them on all LdAiConfigTracker metric event payloads alongside existing modelName/providerName fields. - Add ModelKey/ModelVersion to ModelConfig - Parse new fields from variation payload in ConfigFactory - Include modelVersion unconditionally (default 1) and modelKey when non-empty in track data; exclude both from resumption token - Additive/backward compatible — older payloads without the new fields continue to work Part of AIC-2849. Depends on AIC-2876 (backend payload). Co-authored-by: Anthony Torns II <atornsii@users.noreply.github.com>
Contributor
|
Holding based on our conversation today and we will follow up on slack before reviewing. |
atornsii
marked this pull request as ready for review
July 9, 2026 20:32
atornsii
marked this pull request as draft
July 15, 2026 21:36
Gonfalon moved these two fields from the payload's model object to _ldMeta (launchdarkly/gonfalon#67230) to avoid modelVersion reading as the underlying LLM's own version. Field names are unchanged; only the JSON location moves. ModelConfig's public properties and the tracker's stamped event fields are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
atornsii
marked this pull request as ready for review
July 16, 2026 16:55
jsonbailey
reviewed
Jul 16, 2026
| var parameters = LdValueObjectToDictionary(modelValue.Get("parameters")); | ||
| var custom = LdValueObjectToDictionary(modelValue.Get("custom")); | ||
| return new LdAiConfigTypes.ModelConfig(name, parameters, custom); | ||
| var meta = value.Get("_ldMeta"); |
Contributor
There was a problem hiding this comment.
I don't think this should be part of the parse model method. We already have a parse meta method. Maybe that needs to be refactored to return an object instead of the individual parameters, but it shouldn't be part of the model object like we do here.
jsonbailey
requested changes
Jul 16, 2026
Per review: ParseModel reaching into _ldMeta mixed concerns — it should only parse the model object. ParseMeta now returns a Meta record struct (replacing the 4-field tuple) that also carries ModelKey/ModelVersion, and callers pass those into ParseModel as plain parameters instead of letting it fetch _ldMeta itself. No behavior change; 242/242 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per follow-up discussion: modelKey/modelVersion shouldn't be exposed on the model object a client sees at all — they'd read as properties of the model itself (e.g. a version like "5.4") rather than LaunchDarkly tracking metadata. They now live only where variationKey/version already live: as internal-only fields on LdAiConfig, passed straight into the tracker factory. ModelConfig, ParseModel, and the *ConfigDefault constructors go back to carrying only name/parameters/custom. Tests that asserted Model.ModelKey/ModelVersion are replaced with tracker-level assertions (mirroring the existing modelKey-present test); the "absent from payload" case now verifies via the tracker too. Co-Authored-By: Claude Fable 5 <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
3 participants
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.
…AIC-2856)
Read modelKey and modelVersion from the AI Config variation payload and stamp them on all LdAiConfigTracker metric event payloads alongside existing modelName/providerName fields.
Part of AIC-2849. Depends on AIC-2876 (backend payload).
Requirements
Related issues
Provide links to any issues in this repository or elsewhere relating to this pull request.
Describe the solution you've provided
Provide a clear and concise description of what you expect to happen.
Describe alternatives you've considered
Provide a clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context about the pull request here.
Note
Low Risk
Additive telemetry on existing track payloads with defaults for legacy flags and resumption tokens; no public API surface change beyond internal config fields.
Overview
Adds LaunchDarkly model identity (
modelKey,modelVersion) from AI Config flag_ldMetaand threads it through completion, agent, and judge configs intoLdAiConfigTrackermetric payloads.Parsing & config:
ParseMetareadsmodelKey/modelVersionfrom_ldMeta(not themodelobject somodelVersionisn’t confused with the LLM name/version). MissingmodelVersiondefaults to 1; caller defaults usemodelKey: nullandmodelVersion: 1. New internal fields live onLdAiConfig, notModelConfig.Tracking: All tracker events include
modelVersion;modelKeyis added only when non-empty. Resumption tokens are unchanged—neither field is encoded; resumed trackers still getmodelVersion1 and nomodelKey.Tests cover flag parsing →
TrackSuccesspayloads and resumption defaults.Reviewed by Cursor Bugbot for commit 86ad0d3. Bugbot is set up for automated code reviews on this repo. Configure here.