feat: add ignore-prefix filter for resolve api#1115
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds optional ChangesIgnore-prefix configuration flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SDKClient
participant ResolvedConfigEndpoint
participant apply_resolution_filters
participant Config
SDKClient->>ResolvedConfigEndpoint: Send prefix and ignore_prefix query values
ResolvedConfigEndpoint->>apply_resolution_filters: Build resolution query
apply_resolution_filters->>Config: Apply prefix then ignored-prefix filters
Config-->>ResolvedConfigEndpoint: Return filtered configuration
ResolvedConfigEndpoint-->>SDKClient: Return resolved configuration
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds an ignore_prefix query parameter to the “resolve config” APIs so callers can exclude configuration keys that start with specific prefixes, and propagates the new parameter through server-side filtering, Smithy/OpenAPI docs, and generated SDKs.
Changes:
- Add
ignore_prefixquery parameter toGetResolvedConfigandGetDetailedResolvedConfig(Smithy + OpenAPI/docs). - Implement ignored-prefix filtering in
Configand apply it in the resolve API handler. - Regenerate/extend multi-language SDKs (Rust, Python, JS, Java, Haskell) to support
ignore_prefix.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| smithy/models/config.smithy | Adds ignore_prefix to operations |
| docs/docs/api/Superposition.openapi.json | Documents new query parameter |
| docs/docs/api/get-resolved-config.ParamsDetails.json | Params detail includes ignore_prefix |
| docs/docs/api/get-resolved-config.api.mdx | Regenerated API doc payload |
| docs/docs/api/get-detailed-resolved-config.ParamsDetails.json | Params detail includes ignore_prefix |
| docs/docs/api/get-detailed-resolved-config.api.mdx | Regenerated API doc payload |
| crates/superposition_types/src/overridden.rs | Adds ignored-prefix map filter helper |
| crates/superposition_types/src/config/tests.rs | Adds tests for ignored-prefix filtering |
| crates/superposition_types/src/config.rs | Adds Config::filter_by_ignored_prefix |
| crates/superposition_types/src/api/config.rs | Adds ignore_prefix to query struct |
| crates/superposition_sdk/src/operation/get_resolved_config/builders.rs | Rust SDK fluent builder support |
| crates/superposition_sdk/src/operation/get_resolved_config/_get_resolved_config_input.rs | Rust SDK input adds field + builder |
| crates/superposition_sdk/src/operation/get_resolved_config.rs | Rust SDK serializes ignore_prefix |
| crates/superposition_sdk/src/operation/get_detailed_resolved_config/builders.rs | Rust SDK fluent builder support |
| crates/superposition_sdk/src/operation/get_detailed_resolved_config/_get_detailed_resolved_config_input.rs | Rust SDK input adds field + builder |
| crates/superposition_sdk/src/operation/get_detailed_resolved_config.rs | Rust SDK serializes ignore_prefix |
| crates/superposition_sdk/src/client/get_resolved_config.rs | Rust SDK client docs mention new param |
| crates/superposition_sdk/src/client/get_detailed_resolved_config.rs | Rust SDK client docs mention new param |
| crates/context_aware_config/src/api/config/helpers.rs | Server applies ignored-prefix filter |
| clients/python/sdk/superposition_sdk/serialize.py | Python SDK serializes ignore_prefix |
| clients/python/sdk/superposition_sdk/models.py | Python models include ignore_prefix |
| clients/python/sdk/superposition_sdk/_private/schemas.py | Python schema indices updated |
| clients/javascript/sdk/src/protocols/Aws_restJson1.ts | JS SDK serializes ignore_prefix |
| clients/javascript/sdk/src/models/models_0.ts | JS models include ignore_prefix |
| clients/javascript/sdk/src/commands/GetResolvedConfigCommand.ts | JS command docs include field |
| clients/javascript/sdk/src/commands/GetDetailedResolvedConfigCommand.ts | JS command docs include field |
| clients/java/sdk/src/main/java/io/juspay/superposition/model/GetResolvedConfigInput.java | Java model adds ignorePrefix |
| clients/java/sdk/src/main/java/io/juspay/superposition/model/GetDetailedResolvedConfigInput.java | Java model adds ignorePrefix |
| clients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionClientImpl.java | Generated ordering change |
| clients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionClient.java | Generated ordering change |
| clients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionAsyncClientImpl.java | Generated ordering change |
| clients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionAsyncClient.java | Generated ordering change |
| clients/haskell/sdk/Io/Superposition/Model/GetResolvedConfigInput.hs | Haskell model adds ignore_prefix |
| clients/haskell/sdk/Io/Superposition/Model/GetDetailedResolvedConfigInput.hs | Haskell model adds ignore_prefix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Some(ignore_prefix) = &query_filters.ignore_prefix { | ||
| *config = config | ||
| .clone() | ||
| .filter_by_ignored_prefix(&ignore_prefix.iter().cloned().collect()); | ||
| } |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
smithy/models/config.smithy (1)
503-505: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
ignore_prefixtoGetResolvedConfigWithIdentifier.The
GetResolvedConfigWithIdentifieroperation shares the same underlying resolution logic (ResolveConfigQueryin the backend) asGetResolvedConfig. Consider adding theignore_prefixparameter here as well to ensure consistent filtering capabilities across all resolve endpoints and to expose the option to generated SDK clients.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@smithy/models/config.smithy` around lines 503 - 505, Add the ignore_prefix query parameter to GetResolvedConfigWithIdentifier alongside the existing prefix parameter, matching the parameter definition and annotations used by GetResolvedConfig. Ensure it is included in the operation’s generated SDK interface and passed through the shared ResolveConfigQuery resolution flow.
🧹 Nitpick comments (1)
crates/context_aware_config/src/api/config/helpers.rs (1)
257-263: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid an unnecessary clone of the
Configobject.Currently, the
Configobject is cloned twice ifignore_prefixis present. You can avoid the second costly clone by threading the ownership of the filtered configuration directly.♻️ Proposed refactor to eliminate the extra clone
- *config = apply_prefix_filter_to_config(&query_filters.prefix, config.clone())?; - if let Some(ignore_prefix) = &query_filters.ignore_prefix { - *config = config - .clone() - .filter_by_ignored_prefix(&ignore_prefix.iter().cloned().collect()); - } - apply_context_id_filter(config, &query_filters.context_id) + let mut temp_config = apply_prefix_filter_to_config(&query_filters.prefix, config.clone())?; + if let Some(ignore_prefix) = &query_filters.ignore_prefix { + let prefixes = ignore_prefix.iter().cloned().collect(); + temp_config = temp_config.filter_by_ignored_prefix(&prefixes); + } + apply_context_id_filter(&mut temp_config, &query_filters.context_id)?; + *config = temp_config; + Ok(())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/context_aware_config/src/api/config/helpers.rs` around lines 257 - 263, Update the ignore_prefix branch in apply_context_id_filter’s surrounding configuration-filtering flow to consume the already-owned filtered Config rather than cloning it again before filter_by_ignored_prefix. Preserve the existing ignore-prefix behavior and pass the resulting Config through apply_context_id_filter unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@smithy/models/config.smithy`:
- Around line 503-505: Add the ignore_prefix query parameter to
GetResolvedConfigWithIdentifier alongside the existing prefix parameter,
matching the parameter definition and annotations used by GetResolvedConfig.
Ensure it is included in the operation’s generated SDK interface and passed
through the shared ResolveConfigQuery resolution flow.
---
Nitpick comments:
In `@crates/context_aware_config/src/api/config/helpers.rs`:
- Around line 257-263: Update the ignore_prefix branch in
apply_context_id_filter’s surrounding configuration-filtering flow to consume
the already-owned filtered Config rather than cloning it again before
filter_by_ignored_prefix. Preserve the existing ignore-prefix behavior and pass
the resulting Config through apply_context_id_filter unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc9cfd0d-88fa-4b69-b2ed-50b00019b405
📒 Files selected for processing (34)
clients/haskell/sdk/Io/Superposition/Model/GetDetailedResolvedConfigInput.hsclients/haskell/sdk/Io/Superposition/Model/GetResolvedConfigInput.hsclients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionAsyncClient.javaclients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionAsyncClientImpl.javaclients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionClient.javaclients/java/sdk/src/main/java/io/juspay/superposition/client/SuperpositionClientImpl.javaclients/java/sdk/src/main/java/io/juspay/superposition/model/GetDetailedResolvedConfigInput.javaclients/java/sdk/src/main/java/io/juspay/superposition/model/GetResolvedConfigInput.javaclients/javascript/sdk/src/commands/GetDetailedResolvedConfigCommand.tsclients/javascript/sdk/src/commands/GetResolvedConfigCommand.tsclients/javascript/sdk/src/models/models_0.tsclients/javascript/sdk/src/protocols/Aws_restJson1.tsclients/python/sdk/superposition_sdk/_private/schemas.pyclients/python/sdk/superposition_sdk/models.pyclients/python/sdk/superposition_sdk/serialize.pycrates/context_aware_config/src/api/config/helpers.rscrates/superposition_sdk/src/client/get_detailed_resolved_config.rscrates/superposition_sdk/src/client/get_resolved_config.rscrates/superposition_sdk/src/operation/get_detailed_resolved_config.rscrates/superposition_sdk/src/operation/get_detailed_resolved_config/_get_detailed_resolved_config_input.rscrates/superposition_sdk/src/operation/get_detailed_resolved_config/builders.rscrates/superposition_sdk/src/operation/get_resolved_config.rscrates/superposition_sdk/src/operation/get_resolved_config/_get_resolved_config_input.rscrates/superposition_sdk/src/operation/get_resolved_config/builders.rscrates/superposition_types/src/api/config.rscrates/superposition_types/src/config.rscrates/superposition_types/src/config/tests.rscrates/superposition_types/src/overridden.rsdocs/docs/api/Superposition.openapi.jsondocs/docs/api/get-detailed-resolved-config.ParamsDetails.jsondocs/docs/api/get-detailed-resolved-config.api.mdxdocs/docs/api/get-resolved-config.ParamsDetails.jsondocs/docs/api/get-resolved-config.api.mdxsmithy/models/config.smithy
582b53d to
aa08fd4
Compare
| @documentation("Excludes configuration keys that start with any of the supplied prefixes. When combined with prefix, exclusion is applied to the allow-listed keys.") | ||
| @httpQuery("ignore_prefix") | ||
| @notProperty | ||
| ignore_prefix: StringList |
There was a problem hiding this comment.
if we are adding something like this, then it's an incomplete feature if we dont expand it the plain get config, list experiments, get experiment config, list default configs, list contexts
features on key level filtering specially like prefix should expand to all the usecases
There was a problem hiding this comment.
And also to providers , i was thinking to do that in the second phase.
I will close the api coverage in this PR , we can do the provider changes in the next PR.
Didn't want to make this PR big.
aa08fd4 to
62a260d
Compare
Problem
The existing
prefixfilter acts only as an allow-list. Callers cannot retrieve resolved configuration while excluding keys belonging to specific prefixes.This affects both the Resolve API and the Detailed Resolve API.
Solution
Added an optional, repeatable
ignore_prefixquery parameter.prefix, all resolved keys are returned except those matchingignore_prefix.prefixallow-list minus keys matchingignore_prefix.ignore_prefixis absent.The Smithy model, OpenAPI documentation, and generated Rust, TypeScript, Python, Java, and Haskell SDKs have also been updated.
Environment variable changes
None.
Pre-deployment activity
None.
Post-deployment activity
None.
Consumers wanting to use this feature should upgrade to an SDK version containing the new
ignore_prefixrequest field.API changes
/config/resolveignore_prefixquery parameter/config/resolve/detailedignore_prefixquery parameterExamples
Exclude multiple prefixes:
Combine it with the existing allow-list:
This returns keys beginning with
payments., excluding keys beginning withpayments.private..The request body and response schemas remain unchanged.
Possible issues in the future
ignore_prefix=, matches every key and can produce an empty result.Validation