Skip to content

feat(superposition): Add get resolved config explanation proxy support#13132

Open
NISHANTH1221 wants to merge 43 commits into
mainfrom
add_explain_resolved_config_proxy_support
Open

feat(superposition): Add get resolved config explanation proxy support#13132
NISHANTH1221 wants to merge 43 commits into
mainfrom
add_explain_resolved_config_proxy_support

Conversation

@NISHANTH1221

@NISHANTH1221 NISHANTH1221 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Add GetResolvedConfigExplanation superposition proxy support in hyperswitch.

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Follow steps in #12203

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

AdityaKumaar21 and others added 30 commits May 8, 2026 04:18
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
@NISHANTH1221 NISHANTH1221 self-assigned this Jul 2, 2026
@NISHANTH1221 NISHANTH1221 requested review from a team as code owners July 2, 2026 07:43
@semanticdiff-com

semanticdiff-com Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/routes/superposition_proxy.rs  54% smaller
  crates/router/src/routes/lock_utils.rs  25% smaller
  crates/router/src/core/superposition_proxy.rs  18% smaller
  crates/router/src/routes/app.rs  15% smaller
  crates/external_services/src/superposition.rs  1% smaller
  crates/api_models/src/superposition_proxy.rs  0% smaller
  crates/router_env/src/logger/types.rs  0% smaller

@NISHANTH1221 NISHANTH1221 added the S-test-ready Status: This PR is ready for cypress-tests label Jul 2, 2026
@NISHANTH1221 NISHANTH1221 force-pushed the add_explain_resolved_config_proxy_support branch from 9121f11 to 60640dc Compare July 2, 2026 07:50
@NISHANTH1221 NISHANTH1221 removed request for a team July 2, 2026 07:50
@XyneSpaces

Copy link
Copy Markdown

🔍 [nit] New SuperpositionResolveConfigExplanation route handler uses httpclient directly without connection pooling configuration shown in the diff.

Verify the ExternalHttpClient used for superposition calls has appropriate timeout and retry settings to prevent cascading failures if superposition service degrades.

@XyneSpaces

Copy link
Copy Markdown

Code Review: Superposition Config Explanation Proxy

Findings

[should-fix] In crates/router/src/core/superposition_proxy.rs, the new handle_superposition_proxy_flow function (renamed from handle_proxy_flow) should have #[instrument(skip_all, fields(...))] attribute added since it's an async function in a hot path. The existing calls don't appear to have tracing instrumentation.

[blocking] In crates/router/src/routes/superposition_proxy.rs, the resolve_config_explanation handler at line ~245 creates a closure that captures request by cloning it. Since ResolveConfigExplanationRequest contains potentially sensitive context data (configuration dimensions), ensure this doesn't get logged inadvertently via #[instrument] on the route handler.

[nit] The ResolveExplanationEntry struct in api_models/src/superposition_proxy.rs uses serde_json::Value for condition, value_before, and value_after. This is acceptable for a proxy response type, but document that these are opaque Superposition-specific values.

[should-fix] In resolve_config_explanation_to_response function, consider adding error context if doc_map_to_json or document_to_value conversions fail — currently the error propagation isn't visible in this diff snippet.

@XyneSpaces

Copy link
Copy Markdown

[should-fix] Missing #[derive(serde::Deserialize)] on ResolveExplanationEntry.

The struct derives Serialize but not Deserialize, preventing round-trip deserialization of the explanation response. Add serde::Deserialize if this type needs to be parsed from JSON elsewhere.

#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
pub struct ResolveExplanationEntry {

@XyneSpaces

Copy link
Copy Markdown

[nit] Renaming handle_proxy_flowhandle_superposition_proxy_flow is a breaking change for any internal callers.

The PR updates all existing call sites, which is good. However, the rename itself is purely cosmetic and increases diff noise. Consider whether the disambiguation is worth the churn, or if the original name with module-level clarity (superposition_proxy::handle_flow) suffices.

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

Labels

S-test-ready Status: This PR is ready for cypress-tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(superposition): Add get resolved config explanation proxy support

3 participants