Skip to content

[FFL-1694] feat(core): evaluate variant-key flag dependencies - #414

Merged
typotter merged 33 commits into
mainfrom
dependent-flags-variant-key-evaluator
Nov 4, 2026
Merged

typotter merged 33 commits into
mainfrom
dependent-flags-variant-key-evaluator

Conversation

@typotter

@typotter typotter commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

The JavaScript rules evaluator currently resolves targeting conditions only from evaluation-context attributes. UFC can now reference another flag's variant key, so the evaluator needs bounded graph traversal, propagated graph failures, deterministic prerequisite telemetry, and transactional prerequisite exposure handling in the Node provider.

Linear: FFL-1694 — SDK/JS Support Dependent Flags

Protobuf dependency evaluation is included here. Protobuf dependency exposure provenance and publication remains isolated in stacked draft PR #437.

Scope

  • Parse key-only flagEvaluation conditions in JSON and protobuf UFC.
  • Support ONE_OF and NOT_ONE_OF comparisons against prerequisite variant keys.
  • Create a dependency session lazily when evaluation reaches its first dependent condition, including depth-override validation.
  • Memoize each prerequisite once per root evaluation, preserve authored traversal order, and detect active-stack cycles.
  • Enforce an inclusive maximum dependency depth, defaulting to 10 and accepting UFC overrides from 0 through 255.
  • Propagate missing, malformed, cyclic, over-depth, and no-assignment prerequisite failures to the root result.
  • Return an internal evaluation trace containing the root details, a detached prerequisite-evaluation batch, and transactional dependency-exposure candidates.
  • Stage the completed prerequisite evaluation and exposure batches in a shared sidecar keyed to the root result metadata and per-evaluation context.
  • Let the tracer-owned evaluation hook synchronously consume prerequisite evaluations before emitting the root evaluation.
  • Keep evaluation telemetry best-effort: writer failure cannot change assignment or exposure behavior.
  • Buffer JSON prerequisite exposure candidates and retain only candidates that contributed to a successful root result.
  • Integrate the JSON dependency batch with the shared exposure-decision lifecycle on main: prepare cache decisions during resolution, then complete ancestor and root decisions only after all later OpenFeature hooks succeed.
  • Keep protobuf exposure provenance out of this PR: no protobuf candidate buffer, checkpoint hooks, retained exposure traces, or dependency exposure publication.
  • Pin and run the canonical dependent-flags fixture result cases through the shared fixture runner.
flowchart LR
    UFC["JSON or protobuf UFC"] --> Core["Core evaluator"]
    Core --> Session["Per-root dependency session"]
    Session --> Trace["Completed evaluation trace"]
    Trace --> Result["Root result or propagated graph error"]
    Trace --> Sidecar["Root-lifecycle telemetry sidecar"]
    Sidecar --> EvalHook["Tracer evaluation hook"]
    EvalHook --> Parents["Emit reached prerequisites in authored order"]
    Parents --> Root["Emit root evaluation"]
    Sidecar -->|JSON success only| ExposureHook["Provider exposure hook"]
    ExposureHook -->|all hooks succeed| Publish["Complete/cache and publish ancestors, then root"]
    ExposureHook -->|graph or later-hook error| Discard["Discard the whole exposure batch"]
    Core -. "protobuf provenance added by #437" .-> Followup["Successful witness replay"]
Loading

Important behavior

  • Ordinary public evaluator calls continue returning ordinary ResolutionDetails and do not collect trace data.
  • Prerequisite evaluations are collected during traversal and handed to the tracer only after the graph completes. They are append-only and remain available when the graph fails.
  • The tracer evaluation hook consumes the returned batch during the root OpenFeature lifecycle and emits prerequisites before the root event; failed roots and later hook failures use the hook's finally path.
  • Evaluation and exposure halves are consumed independently. Metadata is the normal lookup key; the evaluation context is the fallback when OpenFeature replaces root metadata after a later hook throws.
  • Each reached prerequisite appears once per root evaluation, including failed, missing, cyclic, over-depth, malformed, and no-assignment nodes; the root flag is excluded.
  • Evaluation records carry the evaluation-start timestamp and consent snapshot and contain detached telemetry-safe values and metadata.
  • Exposure telemetry is separate and transactional: graph failure returns no dependency exposure candidates, and later application-hook failure discards the prepared batch.
  • Exposure subscribers control publication only. Successful assignments are still admitted to the exposure cache when no subscriber is present.
  • A missing or failed node fails the whole dependency graph for the root call and returns the caller's coded default with the normalized error.
  • Parallel and re-entrant root evaluations use isolated sessions and sidecar batches.
  • Precomputed assignment evaluation is unchanged.

Validation

Final-head validation completed for 8bc802c4e655d3337924437eb81c1b91d47459c9:

  • Focused Node dependent-flag suite: 13 passed.
  • Full repository test command: 2,476 passed, 6 skipped (core 1,239; Node 627; browser 610).
  • Full typecheck, lint, format, full build, build tests (124 passed), generated declarations, and git diff --check passed.
  • Final-head GitHub CI passed build-and-test, npm compatibility, license validation, Node package installation without OpenFeature, with current OpenFeature, and with the minimum supported OpenFeature version.
  • Final affected-scope review covered graph failure, no-assignment fail-closed behavior, batch ordering, memoization, one-time consumption, parallel and re-entrant isolation, hook-order failure recovery, evaluation-telemetry best effort, and exposure commit/rollback.

Compatibility and risk

  • Existing public evaluation functions retain their return types and behavior; the provider uses a new internal trace entrypoint.
  • DependencyTelemetrySidecar is the internal package-boundary bridge to tracer-owned evaluation and exposure hooks; recursive evaluation no longer publishes telemetry through a diagnostics channel.
  • Evaluation-count collection stays disabled when the tracer does not install the sidecar, preserving the no-telemetry fast path.
  • The highest-risk areas are graph-failure propagation, root-hook ordering, one-time sidecar consumption, post-hook exposure finalization, and transactional JSON exposure rollback; focused evaluator and provider tests cover each.
  • Delivery must not send dependent UFC to SDK versions that do not advertise support.
  • Protobuf exposure parity depends on stacked PR #437.

Checklist

  • Updated UFC parsing and generated protobuf types
  • Added core evaluator coverage
  • Added Node provider integration coverage
  • Added canonical fixture result coverage
  • Replaced recursive evaluation callbacks with a returned prerequisite batch
  • Handed the completed batch to tracer-owned root hooks through a shared sidecar
  • Adapted JSON exposure handling to the shared final-hook lifecycle on main
  • Completed full local final-head validation
  • Tested on staging — deferred until backend producer and capability gating are available

FFE reviewer selection

Reviewers: @hhan2024, @sameerank

Why: Dependent Flags SDK evaluation, failure propagation, telemetry, and provider integration.

@linear-code

linear-code Bot commented Oct 9, 2026

Copy link
Copy Markdown

FFL-1694

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

OpenFeature Browser Provider Bundle Sizes

Measured from the Vite production output after installing packed @datadog/flagging-core and @datadog/openfeature-browser tarballs.

Both scenarios initialize an OpenFeature provider, evaluate a boolean flag, change context, and evaluate again. Telemetry is disabled for DatadogProvider; no tracking hooks are registered for DatadogCoreProvider. DatadogProvider fetches precomputed assignments for each context; DatadogCoreProvider receives rules from fetchRulesConfiguration once and evaluates locally.

Sizes include OpenFeature and the same small scenario harness. Configuration responses are supplied by Playwright and are not bundled. These are complete scenario JS sizes, not configuration payload sizes or isolated provider/Protobuf costs; the difference between rows is not a decoder-only delta.

Scenario HTML JS Assets Raw JS Gzip JS Protobuf Markers
DatadogProvider (precomputed fetching) provider.html 15 107.5 KiB 38.7 KiB no
DatadogCoreProvider + fetchRulesConfiguration core-provider.html 15 205.7 KiB 65.7 KiB yes

Dependency checks passed: no Protobuf markers in default/precomputed scenarios; markers present in rules-based scenarios.

OpenFeature Browser Tracking Hook Bundle Sizes

Synthetic entrypoints import and call tracking hook factories from the packed @datadog/openfeature-browser/rules-based ESM package. Deltas are measured against the no-hook baseline from the same Vite production build.

Scenario HTML JS Assets Raw JS Raw Δ Gzip JS Gzip Δ
tracking hook baseline tracking-baseline.html 5 2.7 KiB 0 B 1.5 KiB 0 B
exposure logging hook tracking-exposure.html 10 71.2 KiB +68.5 KiB 26.4 KiB +24.9 KiB
evaluation logging hook tracking-evaluation.html 11 67.0 KiB +64.3 KiB 25.0 KiB +23.5 KiB
RUM tracking hook tracking-rum.html 8 6.8 KiB +4.1 KiB 3.5 KiB +2.0 KiB
all tracking hooks tracking-all.html 13 76.0 KiB +73.3 KiB 28.6 KiB +27.1 KiB

OpenFeature Node Server Bundle Sizes

Independent Webpack production builds of the installed core and Node SDK tarballs, targeting Node 18 with ESM (module export condition). Node built-ins remain external. OpenFeature, dd-trace, and configuration fixtures are not bundled.

Scenario Raw JS Gzip JS Protobuf Modules
Node provider only 55.0 KiB 16.4 KiB no
SSR helpers only (fetch client rules + serialize) 81.5 KiB 23.6 KiB yes
Node provider + SSR helpers 135.5 KiB 39.0 KiB yes

Adding SSR helpers to the provider adds 80.5 KiB raw / 22.5 KiB gzip in this build. This includes fetching, parsing, and serialization, not just the decoder. SSR helpers fetch rules for the browser, not for configuring that provider (which uses its existing JSON API).

These are bundler sizes, not npm install sizes, unbundled Node require/import costs, payload sizes, or latency. Gzip compares compressed artifacts. The browser scenarios include OpenFeature and a different harness, so their sizes are not directly comparable.

Dependency checks passed: no Protobuf runtime or generated schema modules in the provider-only bundle; present in both SSR bundles (checked using Webpack module statistics).

This report shows current PR artifact sizes only; it does not compare against the base branch.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Depth enforcement, malformed-prerequisite handling, and consent consistency have unresolved correctness issues.

4 open findings
What changed in this PR

Adds variant-key flag dependencies to the core evaluator and connects prerequisite telemetry to the Node provider.

Changes:

  • Adds JSON and protobuf dependency conditions with bounded traversal and memoization.
  • Publishes prerequisite diagnostics and buffers exposures until root success.
  • Updates schemas, declarations, documentation, and focused tests.
File Description
packages/​node-server/​test/​dependent-flags.spec.ts Tests event ordering and deduplication.
packages/​node-server/​src/​provider.ts Publishes prerequisite diagnostics and exposures.
packages/​node-server/​rules-based.d.ts Updates protobuf declarations.
packages/​node-server/​index.d.ts Updates provider and configuration types.
packages/​core/​test/​evaluation/​dependent-flags.spec.ts Tests traversal, failures, and telemetry.
packages/​core/​src/​evaluation/​ufc-v1.ts Adds dependency-depth configuration.
packages/​core/​src/​evaluation/​rules.ts Adds dependency conditions and short-circuiting.
packages/​core/​src/​evaluation/​rules.test.ts Tests dependency-condition validation.
packages/​core/​src/​evaluation/​evaluation.ts Implements dependency sessions and telemetry buffering.
packages/​core/​src/​evaluation/​evaluateProtobufConfiguration.ts Resolves protobuf dependency conditions.
packages/​core/​src/​evaluation/​evaluateForSubject.ts Passes dependency resolution into rules.
packages/​core/​src/​evaluation/​errors.ts Adds dependency graph errors.
packages/​core/​src/​configuration/​generated/​ufc_pb.ts Regenerates protobuf definitions.
packages/​core/​proto/​ufc.proto Defines dependency conditions and depth limits.
packages/​core/​proto/​README.md Updates schema provenance guidance.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@typotter
typotter force-pushed the dependent-flags-variant-key-evaluator branch from 30e94d5 to 9de5533 Compare October 15, 2026 06:37
@typotter
typotter requested a balanced review from Copilot October 15, 2026 06:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Prerequisite diagnostics are deferred until root completion instead of being emitted immediately as specified.

1 open finding
4 resolved since last review

🧠 Review effort: Balanced

@typotter
typotter requested a review from hhan2024 October 15, 2026 06:46
Add bounded dependent-flag traversal for JSON and protobuf UFC with propagated graph failures and transactional prerequisite exposures.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Accept the merged UFC variant_key_string_indexes name and regenerate the TypeScript and bundled declaration surfaces without changing wire tag 2.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Mutable telemetry events can alter dependency resolution, and provider failure-path coverage remains incomplete.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment thread packages/node-server/src/provider.ts Resolved
typotter and others added 2 commits October 21, 2026 20:53
Bring #414 current with main and pin its shared fixture submodule to the verified #45 head containing Leo's latest corpus changes.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Record the attempted missing node before propagating FLAG_NOT_FOUND through its ancestors, while keeping dependency evaluation values optional for error records.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The public dependency-event declaration requires a value that missing-prerequisite diagnostics omit.

2 open issues

Review effort: Balanced

Snapshot callback payloads, cover provider rollback and cache behavior, refresh fixtures, and regenerate public declarations.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-format traversal and transactional telemetry require human validation, and telemetry snapshot failures can still alter evaluation results.

1 open issue
2 resolved since last review

Review effort: Balanced

Skip snapshot work without a handler and contain recursive clone failures inside the best-effort telemetry boundary.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Disabled Node dependency diagnostics still trigger unnecessary deep cloning of JSON prerequisite values on the evaluation hot path.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Avoid cloning dependency values when evaluation has no subscribers

packages/​node-server/​src/​provider.ts:165

All four resolution methods register a dependency-evaluation callback even when dependencyEvaluationChannel is omitted or has no subscribers. Core's emitEvaluation() therefore deep-clones each JSON prerequisite value before handleDependencyEvaluation() discards the event. This adds work and allocations proportional to the value size on every root evaluation, although dependency matching only needs the variant key. Pass undefined for this callback when the channel has no subscribers, in all four resolvers, so core's existing no-handler fast path skips snapshot construction.

1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because hook-order-dependent rollback can discard prerequisite evaluations before the tracer consumes them, potentially losing trace data.

No open issues 🎉

Review effort: Balanced

Keep prerequisite evaluations available through the root finally lifecycle while rolling back dependency exposures transactionally.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

typotter commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the final-head Copilot hook-order concern. The concern was reproducible when dependency telemetry is consumed by a client/global hook and a closer invocation hook throws first: OpenFeature replaces the root error details before finally, so metadata alone cannot recover the staged batch.

The sidecar now correlates a root evaluation by both its metadata and its per-evaluation context, consumes evaluation and exposure batches independently, and rolls back only transactional dependency exposures on a later hook failure. A regression reproduces the exact ordering and proves that the reached prerequisite evaluation is still emitted while no graph exposure is published.

Validation: focused dependent-flag tests 13/13; full repository tests 2,476 passed with 6 skipped; all package typechecks; lint and format; all package builds and generated declarations; 124 build tests; final Node build/declaration regeneration; and git diff --check all passed.

🤖 written by a bot

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The sidecar consumer is outside this checkout and staging is deferred, so prerequisite batch consumption, failed-root hook behavior, and backend capability gating still require real-tracer validation before rollout.

No open issues 🎉

Review effort: Balanced

typotter commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Disposition of Copilot’s final integration caveat: this is not an actionable finding in PR 414. This repository owns the evaluator/provider sidecar boundary; the tracer-owned consumer is intentionally outside this checkout. The final-head regression exercises that boundary as a client-level hook, including failed-root/later-hook ordering, one-time batch recovery, and exposure rollback. Real tracer wiring and protobuf exposure provenance remain in stacked PR #437, while backend capability gating and end-to-end rollout validation remain delivery gates before rollout, as stated in this PR description.

PR 414 is therefore ready for human code review; it is not being represented as independently ready for production rollout.

🤖 written by a bot

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cyclic context attributes can make prerequisite exposure preparation throw, rejecting an otherwise successful resolution and losing evaluation telemetry instead of isolating the failure per candidate.

1 open issue

Review effort: Balanced

Comment thread packages/node-server/src/provider.ts Resolved
Keep uncacheable dependency and root exposures from changing assignments or discarding prerequisite evaluation telemetry.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Production tracer and sidecar consumers are outside this checkout, leaving prerequisite-before-root ordering and finally-path recovery unverified against the real integration across supported OpenFeature versions.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@sameerank sameerank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crucible: reviewing A Crucible review has started; this PR has reviewer attention. mergequeue-status: rejected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants