chore: rust code cleanup for FFI - #1127
Conversation
|
Review changes with Changed Files
|
|
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:
WalkthroughThe core crate now provides filtered JSON/TOML configuration parsing and provider-cache FFI operations. Separate JSON/TOML parser exports were removed from Rust, Kotlin, and Python bindings. ChangesFiltered configuration and provider-cache FFI
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant LegacyFFI
participant ProviderCache
participant ConfigParser
Caller->>LegacyFFI: submit filtered config or cache request
LegacyFFI->>ConfigParser: parse file with filters
LegacyFFI->>ProviderCache: filter cache or resolve variants
ConfigParser-->>LegacyFFI: return filtered Config
ProviderCache-->>LegacyFFI: return filtered result
LegacyFFI-->>Caller: return JSON or error
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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
This PR refactors superposition_core’s Rust-side FFI surface to reduce duplicated logic across UniFFI and the legacy C ABI, while adding legacy C ABI entry points to filter cached configs/experiments and compute applicable variants.
Changes:
- Centralized parse+filter behavior into
format::parse_config_file_with_filtersand updated UniFFI/C-ABI adapters to call it. - Added shared non-FFI
ProviderCache::*_innermethods and exposed new legacy C ABI functions for cache filtering and variant lookup. - Removed legacy UniFFI
ffi_parse_json_config/ffi_parse_toml_configand updated generated Python/Java bindings accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/superposition_core/src/lib.rs | Updates public re-exports to reflect the new shared parse/filter API and legacy C-ABI exports. |
| crates/superposition_core/src/format.rs | Introduces shared parse_config_file_with_filters used by both UniFFI and C-ABI layers. |
| crates/superposition_core/src/ffi.rs | Removes duplicated parsing logic and factors cache operations into shared *_inner methods. |
| crates/superposition_core/src/ffi_legacy.rs | Adds new C-ABI functions for cache filtering/variants and replaces per-format parse entry points with a unified parse+filter function. |
| crates/superposition_core/src/experiment.rs | Adds Serialize/Deserialize derives needed for JSON serialization in C-ABI returns. |
| clients/python/bindings/superposition_bindings/superposition_client.py | Removes bindings/checksums for deleted UniFFI parse functions. |
| clients/java/bindings/src/main/kotlin/uniffi/superposition_client/superposition_client.kt | Removes bindings/checksums for deleted UniFFI parse functions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let file_content = match c_str_to_string(file_content) { | ||
| Ok(s) => s, | ||
| Err(e) => { | ||
| copy_string(ebuf, format!("Invalid UTF-8 in toml_content: {}", e)); | ||
| copy_string(ebuf, format!("Invalid UTF-8 in file_content: {}", e)); | ||
| return ptr::null_mut(); | ||
| } | ||
| }; | ||
|
|
||
| // Parse TOML | ||
| let parsed = match TomlFormat::parse_config(&toml_str) { | ||
| Ok(p) => p, | ||
| let format = match c_str_to_string(format) { | ||
| Ok(s) => s, | ||
| Err(e) => { | ||
| copy_string(ebuf, e.to_string()); | ||
| copy_string(ebuf, format!("Invalid UTF-8 in format: {}", e)); | ||
| return ptr::null_mut(); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@crates/superposition_core/src/lib.rs`:
- Around line 24-32: Remove or migrate all references to core_parse_toml_config
and core_parse_json_config in the Haskell FFI, JavaScript native resolver,
related Python tests, and documentation before deleting their exported symbols
from the core bindings. Use the supported parse_config_file_with_filters and
format-based API where migration is appropriate, and keep tests and docs aligned
with the resulting public entrypoints.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bfb007b0-e103-4229-bba9-3c961f167667
📒 Files selected for processing (7)
clients/java/bindings/src/main/kotlin/uniffi/superposition_client/superposition_client.ktclients/python/bindings/superposition_bindings/superposition_client.pycrates/superposition_core/src/experiment.rscrates/superposition_core/src/ffi.rscrates/superposition_core/src/ffi_legacy.rscrates/superposition_core/src/format.rscrates/superposition_core/src/lib.rs
💤 Files with no reviewable changes (2)
- clients/python/bindings/superposition_bindings/superposition_client.py
- clients/java/bindings/src/main/kotlin/uniffi/superposition_client/superposition_client.kt
| core_parse_config_file_with_filters, core_provider_cache_filter_config, | ||
| core_provider_cache_filter_experiment, core_provider_cache_get_applicable_variants, | ||
| }; | ||
|
|
||
| // Re-export format module and types | ||
| pub use format::{json::JsonFormat, toml::TomlFormat, ConfigFormat, FormatError}; | ||
| pub use format::{ | ||
| json::JsonFormat, parse_config_file_with_filters, toml::TomlFormat, ConfigFormat, | ||
| FormatError, | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find stale references to the removed JSON/TOML parser entrypoints.
rg -n --iglob '!**/target/**' \
'core_parse_toml_config|core_parse_json_config|ffi_parse_toml_config|ffi_parse_json_config' \
. | head -100Repository: juspay/superposition
Length of output: 3451
Remove stale parser references from client bindings and tests.
The Haskell FFI and JavaScript native resolver still reference core_parse_toml_config and core_parse_json_config. Remove or migrate these references before removing the exported symbols. Update the related Python tests and documentation if these entrypoints are no longer supported.
🤖 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/superposition_core/src/lib.rs` around lines 24 - 32, Remove or migrate
all references to core_parse_toml_config and core_parse_json_config in the
Haskell FFI, JavaScript native resolver, related Python tests, and documentation
before deleting their exported symbols from the core bindings. Use the supported
parse_config_file_with_filters and format-based API where migration is
appropriate, and keep tests and docs aligned with the resulting public
entrypoints.
9b15fe8 to
036c032
Compare
036c032 to
a2ac0de
Compare
Change log
Rust side changes in
superposition_coreneeded for FFI languagesSummary by CodeRabbit
New Features
Breaking Changes