Repository navigation
Conversation
Advertise optional UFC extensions only when the native evaluator binding reports support. Keep dependent flags unadvertised until evaluation and exposure support are implemented. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
hhan2024
added
the
changelog/no-changelog
ChatGPT Codex Connector
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 0768653 | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-11-05 18:07:15 Comparing candidate commit 0768653 in PR branch Found 2 performance improvements and 0 performance regressions! Performance is the same for 591 metrics, 10 unstable metrics, 1 flaky benchmarks without significant changes.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
FFE reviewer selectionReviewers: @typotter, @sameerank Why: Dependent Flags — The PR prepares the Python SDK for dependent (prerequisite) flags. Reviewers come from the matched pod. 🤖 Automated selection. Selection detailsInitiative: project / dependent-flags Why this match: The PR prepares the Python SDK for dependent (prerequisite) flags. It adds a native evaluator capability reporter (ffe.get_evaluator_capabilities in Rust, which returns nothing for now because prerequisite evaluation isn't supported yet) and a Python wrapper with a DEPENDENT_FLAG_VARIANT_KEY_V1 token. The agentless poller sends the X-DD-FEATURE-FLAGS-CAPABILITIES: dependent-flag-variant-key-v1 header only when the evaluator reports that capability. The agentless request path is just where the negotiation happens: the change does not alter configuration delivery itself, so it does not fit deployment-modes. No openfeature-architecture review is needed. The new capability token is a dependent-flags contract, not a provider-lifecycle or evaluation-metadata change, and today it returns an empty list, so runtime behavior is unchanged.
The PR author was excluded. Existing reviewers and teams were preserved. Debug or fix a problemWorkflow run · Bits Code session
Evidence and run details✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head. Generated asynchronously by the FFE reviewer-routing workflow. Changed paths: Evaluated head: Registry revision: Pinned skill source: Instructions: workflow-injected snapshot (no plugin loaded) Instruction snapshot SHA-256: |
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Capability advertisement requires explicit native support. The current evaluator reports no capabilities, and older extensions fall back to advertising none, preserving existing request behavior.
🤖 Bits Code Review · Commit 290b9fb
sameerank
added
the
crucible: reviewing
| @staticmethod | ||
| def get_evaluator_capabilities() -> list[str]: ... | ||
|
|
||
| """ | ||
| Native Feature Flags and Experimentation module. | ||
| """ |
There was a problem hiding this comment.
🤖 Comment from Claude working with Sameeran Kunche:
[P3] Put the class ffe docstring back above the new get_evaluator_capabilities stub.
A string is a class docstring only when it is the first statement in the class body. With the stub above it, the string on lines 1014-1016 is a bare expression. So the stub has no class docstring for ffe, which can affect the description that tools such as IDEs show. Runtime behavior does not change.
The sibling stubs logger and config put the docstring first. #21346 also moves it back. Fixing it here keeps that hunk out of the feature PR.
Suggested patch
diff --git a/ddtrace/internal/native/_native.pyi b/ddtrace/internal/native/_native.pyi
index c5cb0bee5c..f91e80610b 100644
--- a/ddtrace/internal/native/_native.pyi
+++ b/ddtrace/internal/native/_native.pyi
@@ -1008,13 +1008,11 @@ class SerializationError(Exception):
...
class ffe:
+ """Native Feature Flags and Experimentation module."""
+
@staticmethod
def get_evaluator_capabilities() -> list[str]: ...
- """
- Native Feature Flags and Experimentation module.
- """
-
class FlagType(Enum):
String = ...
Integer = ...This uses the same one-line text as #21346. A local merge of this change with the #21346 head (9d2a8a5) has no conflict.
How I checked
I parsed the stub with Python's ast module. ast.get_docstring() for class ffe returns 'Native Feature Flags and Experimentation module.' at the base (3001869) and None at this head (290b9fb). The stub has 58 top-level classes, and 40 of them carry a description string. At the base, all 40 put it first. At this head, ffe is the only exception. With the patch, ffe has its docstring again, and scripts/lint fmt passes.
| [ | ||
| ((), None), | ||
| (("unknown-feature-v1",), None), | ||
| (("dependent-flag-variant-key-v1",), "dependent-flag-variant-key-v1"), |
There was a problem hiding this comment.
🤖 Comment from Claude working with Sameeran Kunche:
[P3] Add a test row where the reporter returns the dependent-flag token together with a token Python does not know.
_headers() sends only DEPENDENT_FLAG_VARIANT_KEY_V1, even when the reporter returns other tokens. The ("unknown-feature-v1",) row checks that an unknown token alone is not sent. No row checks the same filter when the dependent-flag token is also present.
So a later change that forwards every reported token would pass the current tests. For example, headers["X-DD-FEATURE-FLAGS-CAPABILITIES"] = ",".join(get_evaluator_capabilities()), with the guard on _agentless_source.py:320 kept. Only the dependent-only row reaches that assignment, and its join is the same single token.
The new row uses a list, which is also what the real binding returns. The current rows use tuples.
Suggested patch
diff --git a/tests/openfeature/test_agentless_poller.py b/tests/openfeature/test_agentless_poller.py
index a8c3523710..23f8a346df 100644
--- a/tests/openfeature/test_agentless_poller.py
+++ b/tests/openfeature/test_agentless_poller.py
@@ -234,6 +234,7 @@ def test_api_key_header_present_and_absent(harness):
((), None),
(("unknown-feature-v1",), None),
(("dependent-flag-variant-key-v1",), "dependent-flag-variant-key-v1"),
+ (["unknown-feature-v1", "dependent-flag-variant-key-v1"], "dependent-flag-variant-key-v1"),
],
)
def test_evaluator_capability_header_on_initial_and_conditional_requests(harness, monkeypatch, capabilities, expected):What I checked, and what I could not run
I applied the row to this head in a separate worktree, and scripts/lint fmt passes. I could not run the tests: scripts/run-tests needs Docker, and none was available. That the forwarding change passes the current rows and fails the new row comes from tracing the current test inputs by hand, not from a run.
sameerank
left a comment
sameerank
left a comment
There was a problem hiding this comment.
Both comments are non-blocking/nits!
This review also pointed to a couple suggestions in your other draft PR
Restore the native ffe stub docstring and cover filtering mixed evaluator capabilities on initial and conditional requests. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
…eter Use the evaluator capability report to select the dependent-flag URL variant, preserve unrelated query encoding, and cover conditional requests and duplicate parameters. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Description
Prepare agentless feature-flag requests to send
cap_dependent-flags-variant-key=v1in the query string only when the native evaluator binding explicitly reportsdependent-flag-variant-key-v1. The optional HTTP capability header is no longer sent. Existing capability parameters in custom endpoints are normalized to one supported value or removed for unsupported evaluators; unrelated query values and their encoding are preserved.Add a shared capability reporter for configuration transports; older native extensions without the reporter advertise no optional features. The reporter deliberately returns no capabilities today: the linked evaluator does not support prerequisite evaluation. Activation requires native evaluation, error propagation, and exposure-reporting support plus corresponding integration tests.
Testing
scripts/run-tests --venv 616dbb5f2c38 -- -q -k test_agentless_poller: 85 tests passed on Python 3.11.scripts/lint checks, scoped typing, and type-comment checks pass.Risks
A future positive capability declaration must cover both evaluation and exposure semantics; request transport support alone is insufficient. Capability selection is fixed when the source is constructed so initial and conditional polls use the same URL variant.
Additional Notes
No release note: preparatory internal plumbing does not enable a customer-facing feature.
Benchmark discovery finds the exposure aggregation scenario, which does not exercise the capability reporter or HTTP poller; performance coverage for that path remains a gap.