Implement per-site configuration for FedAvg and FedEval recipes

[nvkevlu]

Enable set_per_site_config() as the canonical per-site configuration API for the built-in FedAvg, FedEval, and XGBoost recipes.

Description

• Validates per-site configuration when set_per_site_config() is called and stores a stable snapshot of each site’s settings.
• Preserves referenced objects such as data loaders while preventing later caller mutations from changing deferred configuration.
• Defers client-app creation until client customization, export, or run, then materializes the topology exactly once.
• Creates named client apps when per-site configuration is supplied; otherwise creates the default @ALL client app.
• Keeps recipe-specific validation and construction within each concrete recipe, including XGBoost data loaders and client-rank resolution.
• Preserves the NumPy runner exchange format and applies later client additions, including CSE components, through the selected topology.
• Retains constructor-level per_site_config as a deprecated compatibility path that delegates to the canonical setter and emits a FutureWarning.
• Updates documentation and examples and adds focused coverage for configuration snapshots, deferred topology preparation, compatibility paths, and regressions.

Validation

• Final focused tests: 127 passed.
• Broader affected recipe suite: 552 passed, 23 optional-dependency skips.
• ./runtest.sh -s: Black, isort, flake8, and agent-skill lint passed.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Quick tests passed locally by running ./runtest.sh.
• In-line docstrings updated.
• Documentation updated.

[greptile-apps]

Greptile Summary

This PR makes per-site recipe configuration use set_per_site_config() across the built-in recipes. The main changes are:

• Deferred client app creation until customization, export, or run.
• Per-site config validation and snapshot storage.
• FedAvg, FedEval, and XGBoost recipe updates for per-site topology.
• Deprecated constructor-level per_site_config compatibility.
• Updated docs, examples, and focused tests.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
nvflare/recipe/spec.py	Adds the shared deferred preparation flow for per-site recipe configuration.
nvflare/recipe/fedavg.py	Moves FedAvg client runner creation into the new per-site preparation path.
nvflare/app_opt/pt/recipes/fedeval.py	Moves FedEval client runner creation into the new per-site preparation path.
nvflare/app_opt/xgboost/recipes/histogram.py	Applies deferred per-site component creation for horizontal XGBoost.
nvflare/app_opt/xgboost/recipes/bagging.py	Applies deferred per-site component creation for bagging and cyclic XGBoost.
nvflare/app_opt/xgboost/recipes/vertical.py	Defers vertical XGBoost controller and client component creation until per-site config is applied.

_{Reviews (12): Last reviewed commit: "Validate per-site target names eagerly" | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR adds recipe-specific support for applying set_per_site_config after recipe construction to the unified FedAvg family and PyTorch FedEvalRecipe, enabling conversion from a single @ALL client app into per-site client apps while preserving client-side Recipe API additions (configs, files, filters, components) and supporting safe reapplication/rollback. It also clarifies that XGBoost recipes must continue to receive per_site_config during construction and explicitly reject the helper.

Changes:

• Implement runner-topology replacement and rollback logic in the base Recipe class, and wire it into unified FedAvg and PyTorch FedEval recipes.
• Add targeted unit tests covering export shape, preservation of client additions, reapply semantics, and XGBoost helper rejection.
• Update docs and examples to document supported per-site fields and the required lifecycle (call helper before export/execute).

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/unit_test/recipe/fedavg_recipe_test.py	Adds coverage for helper-driven per-site app creation, export validation, preservation of client additions, and reapply/reject behaviors.
tests/unit_test/app_opt/xgboost/xgboost_recipe_test.py	Ensures XGBoost recipes reject post-construction set_per_site_config and preserve configured sites.
tests/unit_test/app_opt/sklearn/sklearn_recipe_test.py	Verifies the sklearn FedAvg recipe family inherits the helper behavior and produces per-site client apps.
tests/unit_test/app_opt/pt/recipes/fed_eval_recipe_test.py	Adds helper-driven per-site rebuild tests for FedEval and validates reserved-target rejection.
nvflare/recipe/utils.py	Updates helper documentation to clarify which recipe families apply the helper post-construction and which reject it.
nvflare/recipe/spec.py	Adds core per-site runner replacement, cloning/preservation, and rollback mechanisms to support helper-driven topology conversion.
nvflare/recipe/fedavg.py	Refactors runner creation and implements _apply_per_site_config to rebuild per-site client apps via the new base helper logic.
nvflare/app_opt/xgboost/recipes/vertical.py	Documents constructor-only per-site config and rejects the helper via _apply_per_site_config.
nvflare/app_opt/xgboost/recipes/histogram.py	Documents constructor-only per-site config and rejects the helper via _apply_per_site_config.
nvflare/app_opt/xgboost/recipes/bagging.py	Documents constructor-only per-site config and rejects the helper via _apply_per_site_config.
nvflare/app_opt/tf/recipes/fedavg.py	Updates docs to reflect that per-site config may be applied via helper after construction.
nvflare/app_opt/sklearn/recipes/svm.py	Updates docs to reflect helper-based per-site config application.
nvflare/app_opt/sklearn/recipes/kmeans.py	Updates docs to reflect helper-based per-site config application.
nvflare/app_opt/sklearn/recipes/fedavg.py	Updates docs and example usage to show set_per_site_config application after construction.
nvflare/app_opt/pt/recipes/fedeval.py	Implements helper-based per-site rebuild support and validates reserved targets.
nvflare/app_opt/pt/recipes/fedavg.py	Updates docs to reflect helper-based per-site config application.
nvflare/app_common/np/recipes/fedavg.py	Updates docs to reflect helper-based per-site config application.
examples/advanced/sklearn-linear/README.md	Updates guidance/examples to use set_per_site_config for site-specific train_args.
examples/advanced/sklearn-linear/job.py	Switches example to apply per-site config via helper and derives simulation client list from configured_sites().
examples/advanced/experiment-tracking/mlflow/hello-pt-mlflow-client/README.md	Updates example to use set_per_site_config prior to targeted clients=[...] placement.
docs/user_guide/data_scientist_guide/recipe_api.rst	Documents helper lifecycle constraints and adds a support matrix for per-site fields by recipe family.
docs/user_guide/data_scientist_guide/job_recipe.rst	Updates narrative to reflect helper-driven rebuilding for supported recipes and constructor-only requirement for XGBoost.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.56604% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.81%. Comparing base (3aa02e4) to head (0c8c3a7).

Files with missing lines	Patch %	Lines
nvflare/app_opt/xgboost/recipes/vertical.py	68.51%	17 Missing ⚠️
nvflare/app_opt/xgboost/recipes/bagging.py	95.45%	1 Missing ⚠️
nvflare/app_opt/xgboost/recipes/histogram.py	95.83%	1 Missing ⚠️
nvflare/recipe/utils.py	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4902      +/-   ##
==========================================
+ Coverage   61.79%   61.81%   +0.01%     
==========================================
  Files         985      985              
  Lines       95776    95860      +84     
==========================================
+ Hits        59189    59256      +67     
- Misses      36587    36604      +17     

Flag	Coverage Δ
unit-tests	61.81% <90.56%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[YuanTingHsieh]

The intended direction of set_per_site_config() is to make it the canonical per-site configuration API and eventually deprecate constructor-level per_site_config. This change currently adds a second configuration path for FedAvg/FedEval while keeping XGBoost permanently constructor-only.

Could we consolidate each recipe onto one implementation path? The legacy constructor argument can remain temporarily for compatibility, emit a deprecation warning, and delegate to set_per_site_config(). XGBoost can support this by moving its site validation, rank derivation, and site-dependent job construction into _apply_per_site_config().

I also do not think ScriptRunner-specific removal/cloning logic belongs in the generic Recipe specification or should directly mutate private FedJob state. Details are included inline.

[nvkevlu]

@YuanTingHsieh thanks for the guidance, this should now be reworked properly

[YuanTingHsieh]

Thanks for the rework. The earlier XGBoost/FedEval concerns are substantially addressed.

I still have one broader lifecycle concern. Given the direction in #4903 that the generated FedJob is private Recipe state, do we need to preserve eager client-app construction here?

FedAvg/FedEval currently create an @ALL client app during construction, then set_per_site_config() creates named apps and removes @ALL. This create-then-replace lifecycle is why this PR needs exact job.clients assertions, rollback, remove_client_app(), and _configure_per_site_clients(); it also makes the topology fragile if a later operation calls job.to_clients().

Could client topology instead be materialized once, internally, after the recipe configuration is known? set_per_site_config() could validate/store the configuration, and an internal one-time preparation hook could create either the named apps or the default @ALL app. The hook would need to run before client-targeted Recipe customization and from both export() and run(), since run() currently calls env.deploy(_job) directly. This would not add a public configure() step.

The two inline comments below are concrete correctness blockers in the current implementation.

[YuanTingHsieh]

The latest rework addresses the previous lifecycle and NumPy blockers. One remaining configuration-state issue is noted inline. Also, please update the PR description: it still says the helper converts an eagerly-created all-client app, while the implementation now defers client topology materialization.

[YuanTingHsieh]

Two follow-up comments on the spec.py/utils.py boundary: one deferred target-validation issue and one non-blocking validation deduplication.