feat!: remove feature gates — adaptive/trust/placement always compiled#20
feat!: remove feature gates — adaptive/trust/placement always compiled#20
Conversation
BREAKING: Remove adaptive-ml, trust-routing, geographic, sybil-detection, collusion-detection, skademlia, placement, experimental, and full features. All adaptive networking, trust-weighted routing, placement orchestration, and geographic modules are now always compiled. This eliminates 21 silently skipped integration tests in default CI, fixes benchmark compilation, and removes unnecessary complexity from no-op fallback implementations. Only the `metrics` feature (optional dep on prometheus) remains gated. Features retained: default, metrics, mocks, h2_greedy, test-utils. Changes: - Cargo.toml: remove 9 features, bump 0.10.5 → 0.11.0 - 7 src/ files: remove all #[cfg(feature = "...")] gates + delete fallback implementations in core_engine.rs and dht_network_manager.rs - 20 test files: remove #![cfg(feature = "adaptive-ml")] - 3 CI workflows: remove adaptive-ml from --features flags - New test: adaptive_always_available_test.rs (6 compile+runtime checks) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Removes feature-gating around adaptive/trust/geographic/placement functionality so these modules always compile, and updates CI/tests/docs config accordingly.
Changes:
- Deleted
cfg(feature = "...")gates from core modules and integration tests to ensure modules are always available. - Updated CI workflows to stop enabling the removed feature flags when building/running integration tests.
- Added a new integration test ensuring adaptive/placement/geographic APIs remain available under default features.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Removes deprecated feature definitions; bumps version; updates docs.rs and example requirements. |
| src/lib.rs | Makes adaptive/geographic/placement modules and exports unconditional. |
| src/prelude.rs | Makes placement + adaptive prelude exports unconditional. |
| src/network.rs | Makes trust/EigenTrust integration always compiled and removes feature-gated APIs/docs. |
| src/error.rs | Makes AdaptiveNetworkError → P2PError conversion always compiled. |
| src/dht/mod.rs | Makes trust peer selector module + re-exports unconditional. |
| src/dht/core_engine.rs | Makes trust-weighted peer selection always compiled; removes non-adaptive fallback implementations. |
| src/dht_network_manager.rs | Removes feature-gated no-op stubs and always records trust outcomes. |
| tests/adaptive_always_available_test.rs | New test asserting core modules/types are available under default features. |
| tests/* | Removes #![cfg(feature = "adaptive-ml")] gating so tests run in default CI. |
| .github/workflows/integration-core.yml | Removes adaptive-ml from CI feature lists when building/running tests. |
| .github/workflows/integration-identity.yml | Removes adaptive-ml from CI feature lists when building/running tests. |
| .github/workflows/integration-network.yml | Removes adaptive-ml from CI feature lists when building/running tests. |
| # adaptive-ml is required for these tests. | ||
| - name: Build tests | ||
| run: cargo build --tests --features "default,adaptive-ml,mocks,h2_greedy,test-utils" | ||
| run: cargo build --tests --features "default,mocks,h2_greedy,test-utils" |
There was a problem hiding this comment.
default is not an enable-able Cargo feature name (it’s a special key), so --features "default,..." can cause Cargo to error with “feature default not found”. Drop default, from these lists (since default features are enabled implicitly), or if you intended to be explicit, use --no-default-features --features "metrics,mocks,h2_greedy,test-utils".
| run: cargo nextest run --test adaptive_components_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast | ||
|
|
||
| - name: Adaptive Integration | ||
| run: cargo nextest run --test adaptive_integration_tests --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test adaptive_integration_tests --features "default,mocks,h2_greedy,test-utils" --no-fail-fast | ||
|
|
||
| - name: Multi-Armed Bandit | ||
| run: cargo nextest run --test multi_armed_bandit_integration_test --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test multi_armed_bandit_integration_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast | ||
|
|
||
| - name: Q-Learning Cache | ||
| run: cargo nextest run --test q_learning_cache_integration_test --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test q_learning_cache_integration_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast | ||
|
|
||
| - name: Hyperbolic Routing | ||
| run: cargo nextest run --test hyperbolic_routing_test --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test hyperbolic_routing_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast |
There was a problem hiding this comment.
default is not an enable-able Cargo feature name (it’s a special key), so --features "default,..." can cause Cargo to error with “feature default not found”. Drop default, from these lists (since default features are enabled implicitly), or if you intended to be explicit, use --no-default-features --features "metrics,mocks,h2_greedy,test-utils".
| run: cargo build --tests --features "default,mocks,h2_greedy,test-utils" -j2 | ||
|
|
||
| - name: EigenTrust | ||
| run: cargo nextest run --test eigentrust_integration_test --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test eigentrust_integration_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast |
There was a problem hiding this comment.
Same issue as other workflows: default is not a Cargo feature you can list under --features. Remove default, (or switch to --no-default-features --features "metrics,..." if you intended an explicit set).
| run: cargo build --tests --features "default,mocks,h2_greedy,test-utils" | ||
|
|
||
| - name: Gossipsub | ||
| run: cargo nextest run --test gossipsub_integration_test --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test gossipsub_integration_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast | ||
|
|
||
| - name: Coordinator Integration | ||
| run: cargo nextest run --test coordinator_integration_test --features "default,adaptive-ml,mocks,h2_greedy,test-utils" --no-fail-fast | ||
| run: cargo nextest run --test coordinator_integration_test --features "default,mocks,h2_greedy,test-utils" --no-fail-fast |
There was a problem hiding this comment.
--features "default,... risks failing because default isn’t a normal feature. Remove it from the comma-separated feature list (or use --no-default-features --features "metrics,..." if you need explicitness).
| [package.metadata.docs.rs] | ||
| features = [] | ||
| no-default-features = true | ||
| features = ["metrics"] |
There was a problem hiding this comment.
This docs.rs configuration is internally redundant: with no-default-features = false, docs.rs will already build with default = ["metrics"], and specifying features = ["metrics"] again doesn’t change anything. Consider either removing the features = ["metrics"] line (keep defaults), or setting no-default-features = true if the intent is “metrics only” regardless of future default changes.
| features = ["metrics"] |
| let _ = std::any::type_name::<GeographicNetworkConfig>(); | ||
| assert!(!std::any::type_name::<GeographicNetworkConfig>().is_empty()); |
There was a problem hiding this comment.
This calls type_name::<GeographicNetworkConfig>() twice; it’s a minor duplication. Consider storing it in a local variable once (or keeping just the assert!), which makes the intent clearer and avoids repeated calls.
| let _ = std::any::type_name::<GeographicNetworkConfig>(); | |
| assert!(!std::any::type_name::<GeographicNetworkConfig>().is_empty()); | |
| let type_name = std::any::type_name::<GeographicNetworkConfig>(); | |
| assert!(!type_name.is_empty()); |
Greptile OverviewGreptile SummaryThis PR removes all feature gate complexity from the adaptive networking, trust routing, and placement systems by making these modules always available at compile time. The change eliminates 9 feature flags ( Key Changes:
Impact:
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| Cargo.toml | Removed 9 feature flags (adaptive-ml, trust-routing, geographic, sybil-detection, collusion-detection, skademlia, placement, experimental, full) and bumped version to 0.11.0; removed feature requirements from examples |
| src/dht/core_engine.rs | Removed #[cfg(feature = "adaptive-ml")] gates from trust-weighted peer selection methods and imports; deleted fallback no-op implementation |
| src/dht_network_manager.rs | Removed feature gates from record_peer_success and record_peer_failure methods; deleted no-op fallback implementations |
| src/network.rs | Removed feature gates from EigenTrust engine initialization and trust API methods; trust engine now always available |
| src/lib.rs | Updated module visibility - adaptive, placement, and geographic modules now unconditionally exported without feature gates |
| tests/adaptive_always_available_test.rs | New test file with 6 compile-time and runtime checks proving adaptive/placement/geographic modules are always available under default features |
| .github/workflows/integration-core.yml | Removed adaptive-ml from feature flags in build and test commands; tests now run with default features |
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant Cargo as Cargo.toml
participant Src as Source Files
participant Tests as Test Files
participant CI as CI Workflows
participant Build as Build System
Dev->>Cargo: Remove 9 feature flags
Note over Cargo: adaptive-ml, trust-routing,<br/>geographic, sybil-detection,<br/>collusion-detection, skademlia,<br/>placement, experimental, full
Dev->>Cargo: Bump version 0.10.5 → 0.11.0
Dev->>Cargo: Remove example feature requirements
Dev->>Src: Remove #[cfg(feature)] gates
Note over Src: src/dht/core_engine.rs<br/>src/dht_network_manager.rs<br/>src/network.rs<br/>src/lib.rs<br/>src/prelude.rs
Dev->>Src: Delete fallback no-op implementations
Note over Src: record_peer_success/failure<br/>trust selection methods
Dev->>Tests: Remove #![cfg(feature)] from 20 test files
Note over Tests: All adaptive/trust/placement<br/>tests now run by default
Dev->>Tests: Add adaptive_always_available_test.rs
Note over Tests: 6 compile-time + runtime checks
Dev->>CI: Update workflow feature flags
Note over CI: Remove adaptive-ml from<br/>all CI test commands
Build->>Src: Compile adaptive modules
Note over Build: Always compiled,<br/>no feature flag required
Build->>Tests: Run all 21 integration tests
Note over Tests: Previously skipped tests<br/>now execute in CI
Tests-->>Build: All tests pass
Build-->>Dev: Build successful
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
- Remove invalid `default` from --features lists in CI workflows (default features are enabled implicitly by Cargo) - Remove redundant `features = ["metrics"]` from docs.rs config (already included via no-default-features = false) - Store type_name result in local variable to avoid duplicate call Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After rebasing onto main (which includes PR #20's feature gate removal), our new code still had #[cfg(feature = "adaptive-ml")] guards that referenced the now-removed feature, causing compilation failures in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
adaptive-ml,trust-routing,geographic,sybil-detection,collusion-detection,skademlia,placement,experimental,full0.10.5→0.11.0(breaking change)saorsa_core::adaptivenow always available)core_engine.rsanddht_network_manager.rsadaptive_always_available_test.rs— 6 tests proving all modules compile under default featuresRetained features
default = ["metrics"],metrics,mocks,h2_greedy,test-utilsFiles changed (32)
Cargo.toml— feature definitions + examplessrc/files — cfg gate removal + fallback deletiontests/files — crate-level cfg removal.github/workflows/— CI flag cleanupTest plan
cargo check(default features)cargo check --no-default-featurescargo clippy -- -D warningscargo fmt --all -- --checkcargo build --example security_examplecargo build --example adaptive_network_monitorcargo doc --no-depscargo test --test adaptive_always_available_test(6/6 pass)src/andtests/🤖 Generated with Claude Code