feat: Validation for ai-configs and observability slices#428
Open
kinyoklion wants to merge 3 commits into
Open
feat: Validation for ai-configs and observability slices#428kinyoklion wants to merge 3 commits into
kinyoklion wants to merge 3 commits into
Conversation
- ai-configs (5 SDKs × 5 snippets = 25): bind context, implementation,
import, initialize bodies to per-SDK syntax-only scaffolds; bind
install bodies to the shell-install validator. Extend
csharp-syntax-only with the AI namespaces and stub aiClient field
so dotnet bodies compile against the LaunchDarkly.ServerSdk.Ai
package.
- observability (6 SDKs): bind every install body (npm/yarn/pip) to
shell-install. Bind node-server and python-server import bodies to
their syntax-only scaffolds (parsing-only is sufficient — node
--check and ast.parse don't try to resolve import paths).
- shell-install: support multi-line install bodies (the previous
last_pkg awk emitted one package per line, breaking the
post-install assertion when a body installs the SDK and an addon
in two commands). Add gem support so ruby-server-sdk installs bind
cleanly; the per-run GEM_HOME keeps installs root-free and
collision-free.
- Frontend observability/import (js, react, vue, react-native) and
every observability/initialize body documented as Bucket C in
`_aiconfigs-observability-port-notes.md` along with the dotnet
ai-configs install (PowerShell `Install-Package` cmdlets, same
shape as the sdk-info install-csharp.txt that's already
documented in `_sdk-info-port-notes.md`).
- Fixed two real defects in the dotnet-server-sdk ai-configs bodies
found during validation: `Context.Builder().LastName().Email().Groups()`
used non-existent ContextBuilder methods (replaced with `.Set("lastName",
...)` etc.); `Role.system` used the wrong case (renamed to
`Role.System`).
Coverage delta:
- ai-configs: 0/25 → 24/25 bound (1 Bucket C: dotnet PowerShell install).
- observability: 0/23 → 13/23 bound (10 Bucket C: 4 frontend imports + 6
end-to-end initializes).
kinyoklion
changed the title
This was referenced May 8, 2026
Finishes the observability slice (was 13/23 in the previous commit)
by binding the 4 frontend `observability/import` fragments and all 6
`observability/initialize` fragments to per-SDK validation harnesses.
Coverage delta: 13/23 → 23/23 observability snippets bound; 0 Bucket C
remaining for observability.
Frontend imports (js, react, vue, react-native):
- Pre-bake `@launchdarkly/observability` + `@launchdarkly/session-replay`
(or `@launchdarkly/observability-react-native` for RN) in each
validator's Dockerfile so the bundler can resolve the imports.
- Extend the js-client and react-native syntax-only scaffolds with
`//IMPORT_LIFT_TARGET` / `//BODY_BEGIN` / `//BODY_END` markers and
add the matching awk pre-step in each harness, mirroring the
existing react-client + flutter pattern. ESM forbids `import`
inside a function body, so the install-time imports lift to
module scope.
- Rewrite react-native-syntax-only to emit App.tsx (was App.js) so
the harness can find it; add a `SNIPPET_MODE=syntax-only`
dispatch in the harness that runs Babel parse instead of jest.
- Vue scaffold gains a queueMicrotask fallback so import-only bodies
that don't themselves mount still surface the EXAM-HELLO line.
Initialize runners (all 6 SDKs):
- New `init-runner-observability` companion scaffold per SDK.
- Each scaffold supplies the matching `observability/import`
snippet's symbols at module scope (init/createApp/createClient/
withLDProvider/ReactNativeLDClient + Observability/SessionReplay).
- Wrappee body's `'SDK_KEY'` literal substituted via
`validation.placeholders` to LAUNCHDARKLY_SDK_KEY (server),
LAUNCHDARKLY_CLIENT_SIDE_ID (browser), or LAUNCHDARKLY_MOBILE_KEY
(RN).
- Scaffold awaits the SDK's waitForInitialization() (or polls
is_initialized() for Python, watches the LDProvider-rendered DOM
for React, watches `app.mount()` for Vue) and emits the
EXAM-HELLO line on success. Per the brief, success = SDK starts
cleanly with the o11y plugin attached; we don't assert that
traces / events land in LD.
js-client validator Dockerfile changes:
- Add launchdarkly-js-client-sdk (legacy v3) alongside
@launchdarkly/js-client-sdk; the observability snippet bodies use
the v3 namespace shape.
- Switch tsdown to `format: 'iife'` + `entryFileNames: 'app.js'`
+ `noExternal: [/.*/]` so the script-tag-loaded bundle resolves
all imports strictly and produces non-ESM output (the previous
config left o11y packages as external imports the file:// page
couldn't resolve).
Defects fixed inline:
- js-client-sdk/observability/import: body had
`import { LDClient } from 'launchdarkly-js-client-sdk'` (named
import of a type-only interface). The companion initialize body
then calls `LDClient.initialize(...)` at runtime, which fails
because LDClient was never bound to a value. Switched to the
default import so LDClient resolves to the v3 LaunchDarkly
namespace object.
- js-client-sdk/observability/initialize: body called
`LDClient.initialize('SDK_KEY', { plugins: [...] })`, missing
the required context argument. Added an EXAMPLE_CONTEXT_KEY
user context.
- js-client-sdk/sdk-docs/use-promises-...-v4-x-typescript: pre-
existing snippet that surfaced once the bundler resolved imports
strictly. `import { Observability }` and `import { SessionReplay }`
were named imports of default exports; `import { initialize }`
from `@launchdarkly/js-client-sdk` v4 doesn't exist (v4 renamed
to `createClient`). Switched the o11y imports to default-import
shape, replaced `initialize` with `createClient`, added the
missing context argument.
vue-client init-runner: leading semicolon before the IIFE so ASI
doesn't try to call the result of the body's final
`app.mount('#app')` (the body has no trailing semicolon). Same fix
applied to the new init-runner-observability scaffold.
Update _aiconfigs-observability-port-notes.md to drop the two
resolved Bucket C entries; only the dotnet ai-configs install
fragment remains documented.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ingle-line imports
The previous round's IMPORT_LIFT awk pre-step in
validators/languages/react-client/harness/run.sh only recognized
single-line imports terminated by `;`. Imports without a trailing
semicolon (`import Observability, { LDObserve } from
'@launchdarkly/observability'`) entered `in_multi_import` mode, never
found a `;`, and were silently dropped from the lifted output.
Validation passed because the wrappee body's `if (false) { … }` block
parsed fine, but two of three observability imports in
react-client-sdk/observability/import were never actually exercised.
The js-client and react-native-client harnesses already checked
`/;[ \t]*$/ || /[\x27"][ \t]*$/` for the single-line case;
react-client wasn't updated. Bring it in line.
Multi-line continuation handling stays `;`-only — a genuine multi-line
import `import {\n foo,\n} from 'pkg'` still needs the terminator,
which is the conventional shape.
3 tasks
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default mode 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 125835f. Configure here.
| MODE="${SNIPPET_MODE:-init}" | ||
| if [ "$MODE" = "init" ]; then | ||
| require_env LAUNCHDARKLY_MOBILE_KEY | ||
| fi |
There was a problem hiding this comment.
Flag key required unnecessarily in syntax-only mode
Low Severity
require_env LAUNCHDARKLY_FLAG_KEY runs unconditionally on line 24, but this variable is only used by the jest test path (init mode). The developer correctly moved LAUNCHDARKLY_MOBILE_KEY into the if [ "$MODE" = "init" ] conditional but forgot to do the same for LAUNCHDARKLY_FLAG_KEY. In syntax-only mode, validation will fail with a misleading "LAUNCHDARKLY_FLAG_KEY not set" error even though that variable is completely unused by the Babel parse path.
Reviewed by Cursor Bugbot for commit 125835f. Configure here.
| } | ||
| ' "$ENTRY_FILE" > /tmp/lifted.ts | ||
| mv /tmp/lifted.ts "$ENTRY_FILE" | ||
| fi |
There was a problem hiding this comment.
IMPORT_LIFT awk script duplicated across three harnesses
Low Severity
The ~60-line IMPORT_LIFT awk script is now identically duplicated across three harnesses (js-client, react-native-client, and the existing react-client). A shared lib.sh mechanism already exists at validators/shared/lib.sh and is mounted into each container as /harness-shared/lib.sh. Extracting this awk logic into a shared helper would eliminate the need to synchronize bug fixes across three copies.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 125835f. Configure here.
kinyoklion
added a commit
that referenced
this pull request
May 8, 2026
Adds 12 canonical .snippet.md files under
snippets/sdks/<sdk>/snippets/sdk-docs/ for the install/manifest
CodeBlocks in ld-docs's top-level reference pages
(`fern/topics/sdk/{server-side,client-side}/<sdk>/index.mdx`) that
the original auto-port (sdk-meta PR #414) skipped per the
shell/bash/cmd skip rule (SDK-2300).
These snippets unblock ld-docs-private PR #7592, which can insert
markers pointing at them in a follow-up commit once this lands and
a new `snippets/v*` is cut.
Per-SDK file count:
| SDK | snippet IDs |
|---|---|
| dotnet-server-sdk | install-the-sdk-shell |
| java-server-sdk | install-the-sdk-gradle |
| node-server-sdk | install-the-sdk-shell |
| php-server-sdk | install-the-sdk-shell |
| ruby-server-sdk | install-the-sdk-shell |
| rust-server-sdk | install-the-sdk-shell |
| node-client-sdk | install-the-sdk-installing-with-npm, install-the-sdk-installing-with-yarn |
| react-native-client-sdk | install-the-sdk-installing-react-native-sdk-v10, install-the-sdk-adding-the-async-storage-dependency |
| vue-client-sdk | install-the-sdk-installing-with-npm, install-the-sdk-installing-with-yarn |
`go-server-sdk` is intentionally not in the list: the "Shell install"
mention in PR #7592's per-SDK table refers to a prose backtick quote
of `go get …`, not an MDX CodeBlock — there's nothing to canonicalize.
Bodies are byte-verbatim copies of what's in ld-docs today; verified
by running `snippets render --target=ld-docs` after a synthetic
marker insertion and confirming a zero-body-diff.
No `validation:` block: these are install/manifest fragments, not
runnable units. Adding shell-install or build-system-harness
validation for them is a separate workstream (PR #428's territory).
Replaces the originally-opened-as-9-separate-PRs sequence
(#430-#438) with a single PR per the user's
intent.
kinyoklion
added a commit
that referenced
this pull request
May 8, 2026
Adds 12 canonical .snippet.md files under
snippets/sdks/<sdk>/snippets/sdk-docs/ for the install/manifest
CodeBlocks in ld-docs's top-level reference pages
(`fern/topics/sdk/{server-side,client-side}/<sdk>/index.mdx`) that
the original auto-port (sdk-meta PR #414) skipped per the
shell/bash/cmd skip rule (SDK-2300).
These snippets unblock ld-docs-private PR #7592, which can insert
markers pointing at them in a follow-up commit once this lands and
a new `snippets/v*` is cut.
Per-SDK file count:
| SDK | snippet IDs |
|---|---|
| dotnet-server-sdk | install-the-sdk-shell |
| java-server-sdk | install-the-sdk-gradle |
| node-server-sdk | install-the-sdk-shell |
| php-server-sdk | install-the-sdk-shell |
| ruby-server-sdk | install-the-sdk-shell |
| rust-server-sdk | install-the-sdk-shell |
| node-client-sdk | install-the-sdk-installing-with-npm, install-the-sdk-installing-with-yarn |
| react-native-client-sdk | install-the-sdk-installing-react-native-sdk-v10, install-the-sdk-adding-the-async-storage-dependency |
| vue-client-sdk | install-the-sdk-installing-with-npm, install-the-sdk-installing-with-yarn |
`go-server-sdk` is intentionally not in the list: the "Shell install"
mention in PR #7592's per-SDK table refers to a prose backtick quote
of `go get …`, not an MDX CodeBlock — there's nothing to canonicalize.
Bodies are byte-verbatim copies of what's in ld-docs today; verified
by running `snippets render --target=ld-docs` after a synthetic
marker insertion and confirming a zero-body-diff.
No `validation:` block: these are install/manifest fragments, not
runnable units. Adding shell-install or build-system-harness
validation for them is a separate workstream (PR #428's territory).
Replaces the originally-opened-as-9-separate-PRs sequence
(#430-#438) with a single PR per the user's
intent.
apucacao
approved these changes
May 13, 2026
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
2 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.
Summary
Wires
validation:blocks for the 25 ai-configs and 23 observability snippets that PRs #424 and #425 ported from gonfalon. Coverage delta:Single PR against
main. The first commit (a2253af) wired ai-configs end-to-end and the install + node/python observability imports (13/23 observability). The follow-up commit on this PR wires the remaining 10 observability snippets (4 frontend imports + 6 init-runners).Per-SDK before / after
ai-configs
Install-Packagecmdlets)observability
Strategy
First commit (a2253af) — already merged into branch
shell-installvalidator. Multi-line installs (e.g.npm install <sdk>+npm install <addon>) needed a fix to thelast_pkgawk extractor — the old form emitted one package per line, breaking the post-install assertion. Now folds across lines and asserts on the last package the body installs.*-syntax-onlyscaffold. Compilation/parse-only matches the user brief ("compilation for now"). The dotnetcsharp-syntax-onlyscaffold gainedusing LaunchDarkly.Sdk.Server.Ai*imports +LaunchDarkly.ServerSdk.Airequirement + adynamic aiClientstub field so cross-snippet references resolve.node --checkandast.parseare purely syntactic and don't need the o11y package on disk.Follow-up commit — finishes Bucket C observability
observability/import(js-client, react-client, react-native, vue-client): each frontend validator's Dockerfile now pre-bakes@launchdarkly/observability+@launchdarkly/session-replay(or@launchdarkly/observability-react-nativefor RN). The js-client and react-native syntax-only scaffolds gained//IMPORT_LIFT_TARGET///BODY_BEGIN///BODY_ENDmarkers and the matching awk pre-step in their harnesses (mirroring the existing react-client + flutter pattern), so install-timeimportdirectives lift out of the never-invoked wrappee function up to module scope where ESM allows them. The vue scaffold (already at module scope) gained aqueueMicrotaskfallback that surfaces the EXAM-HELLO line for import-only bodies that don't themselves mount.App.jswhile the harness expectedApp.tsx; this commit rewrites the scaffold to emitApp.tsxand adds aSNIPPET_MODE=syntax-onlydispatch in the harness that bypasses jest and runs Babel parse instead.observability/initialize: each SDK gains a companioninit-runner-observabilityscaffold that:observability/importsnippet's symbols at module scope.'SDK_KEY'literal viavalidation.placeholdersto the appropriate env var (LAUNCHDARKLY_SDK_KEYfor server,LAUNCHDARKLY_CLIENT_SIDE_IDfor browser,LAUNCHDARKLY_MOBILE_KEYfor RN).waitForInitialization()(or the equivalent — Python usesis_initialized()polling, React usesLDProvider-rendered DOM, Vue usesapp.mount('#app')returning) and emits the EXAM-HELLO line on success.Validator changes
shell-install (first commit)
Dockerfile: layerruby + ruby-dev + build-essential + zlib1g-dev + libssl-dev + libyaml-dev + libffi-devsogem installworks for the ruby-server-sdk ai-configs install snippet.harness/run.sh: add agem)case that routes installs into a per-runGEM_HOME(no root needed) and asserts via<GEM_HOME>/gems/<pkg>-<version>directory existence. Fixlast_pkgto fold across multi-line bodies via anENDblock.Follow-up commit
validators/languages/js-client/Dockerfile: addlaunchdarkly-js-client-sdk(legacy v3, used by the observability/import body),@launchdarkly/observability, and@launchdarkly/session-replayto the pre-baked dep tree. Switch tsdown output toformat: 'iife'withentryFileNames: 'app.js'andnoExternal: [/.*/]so the bundle resolves the o11y packages strictly and produces a script-tag-loadable IIFE (the previous ESM output choked the file://<script>tag once the bundle started exporting symbols from the bundled o11y deps).validators/languages/js-client/harness/run.sh: add the IMPORT_LIFT awk pre-step (mirror of react-client) so wrappees with body-internalimportlines lift them to module scope.validators/languages/react-client/Dockerfile: addlaunchdarkly-react-client-sdk(legacy v3),@launchdarkly/observability,@launchdarkly/session-replay.validators/languages/vue-client/Dockerfile: add@launchdarkly/observability,@launchdarkly/session-replay.validators/languages/react-native-client/Dockerfile: add@launchdarkly/observability-react-native.validators/languages/react-native-client/harness/run.sh: addSNIPPET_MODE=syntax-onlydispatch — runs Babel parse on the staged App.tsx and exits before the jest end-to-end run. Used for parse-only doc fragments (the newobservability/import) that aren't standalone-runnable.Defects fixed inline
First commit (a2253af) — dotnet ai-configs
dotnet-server-sdk/ai-configs/context— body called.LastName(...),.Email(...),.Groups([...])onContextBuilder. None of those methods exist on the .NET SDK'sContextBuildertype — the v7+ surface uses.Set(name, value)for custom attributes, withLdValue.ArrayOf(...)for array values. Rewritten to use the canonicalSetAPI.dotnet-server-sdk/ai-configs/implementation— body called.AddMessage("", Role.system). The .NET enum's member isRole.System(PascalCase, per .NET conventions);Role.systemdoesn't compile. Renamed.Follow-up commit — js-client observability + adjacent snippets
js-client-sdk/observability/import— body hadimport { LDClient } from 'launchdarkly-js-client-sdk', a named import of theLDClienttype-only interface. The companionobservability/initializebody then callsLDClient.initialize(...)at runtime, which fails becauseLDClientwas never bound to a value. Switched to the default import (import LDClient from 'launchdarkly-js-client-sdk') soLDClient.initializeresolves to the SDK'sLaunchDarklydefault-export object at runtime.js-client-sdk/observability/initialize— body calledLDClient.initialize('SDK_KEY', { plugins: [...] }), missing the requiredcontextargument (the SDK'sinitialize(envKey, context, options)signature has been positional-context since v3). Addedconst context = { kind: 'user', key: 'EXAMPLE_CONTEXT_KEY' };and passed it through.js-client-sdk/sdk-docs/use-promises-to-determine-when-the-client-is-ready-javascript-sdk-v4-x-typescript(pre-existing, surfaced by the new bundler-strict mode) — body hadimport { Observability }/import { SessionReplay }(both default exports of their packages, not named) andimport { initialize, LDClient }from@launchdarkly/js-client-sdk(v4 dropped theinitializenamed export in favor ofcreateClient(envKey, context, options)). Switched both o11y imports to default-import shape, replacedinitializewithcreateClient, and added an explicit context argument. The snippet was passing previously only because tsdown was configured to leave o11y packages as external (with no node_modules entry); the new strict-bundling config caught the latent bug.Bucket C entries (deferred)
Documented in
snippets/sdks/_aiconfigs-observability-port-notes.md:dotnet-server-sdk/ai-configs/install(NuGet PowerShell cmdlets — same shape as the sdk-infoinstall-csharpsnippet already documented in_sdk-info-port-notes.md).All other observability snippets are now bound.
Test plan
go test ./...passesobservability/initializesnippets stage + build through the validator end-to-end (final auth fails with the local fake-key fixture, expected behavior — CI runs with real keys)observability/importsnippets pass parse-only validationNote
Medium Risk
Medium risk because it updates multiple language validator Docker images and harness scripts (import-lifting, bundling, new toolchains), which can break snippet validation across many SDKs if assumptions change.
Overview
Adds validation coverage for the newly ported
ai-configs/andobservability/snippet sets by wiringvalidation:blocks to existing syntax-only scaffolds and to a broadenedshell-installvalidator.Introduces new init-runner scaffolds for observability across JS client, Node, Python, React, React Native, and Vue to run SDK initialization with observability plugins using env-driven placeholders, plus updates JS/React/React Native/Vue validators (Docker deps, bundling settings, and
import-lifting preprocessors) so import-only and ESM snippets compile/parse correctly.Extends
shell-installto support multi-line install bodies andgem install, updates the .NET C# syntax scaffold to include AI Config dependencies/stubs, fixes a few snippet bodies to match real SDK APIs (notably .NET context builder and JS client observability initialization), and documents the one remaining unvalidated snippet in_aiconfigs-observability-port-notes.md.Reviewed by Cursor Bugbot for commit 125835f. Bugbot is set up for automated code reviews on this repo. Configure here.