[Docs]: Correct Model Files / Training Configuration in the user guide

[atriaybagur]

Description

Follow-up to #811. The Model Files section of the user guide (and its Training Configuration subsection) is factually wrong in several places and carries backend-specific detail that does not belong on a page documenting functions common to all users.

What was wrong

Required files. The page stated a fixed minimum set — validator.py and trainer.py — for NVFLARE apps, and described config.json as an optional extra. Required files are actually declared per job type in fl-apps/<backend>/<job_type>/required_files.json:

• config.json is required by every NVFLARE job type (it carries job_type), so it is not optional.
• models.py is required by every NVFLARE job type except evaluation, and was not mentioned at all.
• standard_client_api does not require validator.py.
• evaluation / evaluation_client_api require evaluator.py and no trainer at all.
• For Flower, the page named pyproject.toml as a required user-supplied file. It is not — it comes from the platform's base template, and the required set is client_app.py + models.py.

Training configuration. The subsection carried a full NVFLARE key reference, correctly noted as FLARE-only but still sitting in a backend-neutral page. It also had its own inaccuracies:

• Rounds were documented as "greater than 0, less than 100"; the accepted range is 1–1000 inclusive, and an out-of-range or non-numeric value is discarded in favour of the default of 1 rather than rejected — worth calling out, since a job silently running one round reads like a bug.
• AGGREGATION_WEIGHTS was documented as keyed on Trust abbreviations (KCH, UCLH). The FL server knows clients by their FL kit slot name (Trust_1, Trust_2, …), so weights keyed on a Trust code match no participating client and have no effect. The documented example would have silently done nothing.
• AGGREGATE_ONLY_REGEX and the BEST_MODEL_METRIC / BEST_MODEL_METRIC_MINIMIZE pair added in [Feature]: Save both best and final models — NVFLARE backend #811 were absent.

What this PR does

• docs/source/user-guides/user-common.rst — rewrites Model Files around the job-type contract and points at where the UI surfaces the applicable list (the Model Files panel shows the detected job type; the Training panel lists required and missing files). Trims Training Configuration to a short backend-neutral explanation of where configuration lives per backend, keeping the training-configuration label so inbound references still resolve.
• docs/source/components/component-fl-nodes.rst — becomes the canonical reference it was already being pointed at as. Adds _fl-required-files (per-job-type tables for both backends, with a note that the UI/manifests are the source of truth) and _fl-training-configuration (the validated NVFLARE config.json keys with accepted values and defaults, plus Flower's pyproject.toml / config.toml run configuration). Also corrects the page's own claim that pyproject.toml is researcher-supplied.
• docs/source/working-with-flip-apps/create-flip-app-from-flower.rst — repoints its "canonical list" reference at the new anchor instead of back at the user guide.

Corrections from a self-review round (bfca0385, after the sections above were written). Six
further inaccuracies surfaced when every claim was re-checked against the source — five of them in
prose this PR itself adds:

• The invalid-rounds note was wrong for LOCAL_ROUNDS. It claimed a discarded GLOBAL_ROUNDS
  or LOCAL_ROUNDS is replaced by the default of 1. That holds only for GLOBAL_ROUNDS, which
  the platform reads and writes into the server's job configuration. LOCAL_ROUNDS is read by the
  app's own trainer straight out of config.json, and configure_config fills in the default only
  when the key is absent — a key that is present but invalid is left as written and reaches the
  trainer verbatim, so "LOCAL_ROUNDS": 5000 really does run 5000 local iterations. The note now
  splits the two keys, and adds the BEST_MODEL_METRIC exception (a discarded GLOBAL_ROUNDS fails
  the job outright rather than defaulting) and the fact that neither key is rewritten in the
  deployed config.json.
• "Any additional file is bundled into the app" had two carve-outs. A model file colliding with a
  base-template name is skipped with only a server-side log line — for Flower that silently drops
  server_app.py, strategy.py and __init__.py, which is exactly what the Flower tutorials ship.
  And a checkpoint named by SERVER_CHECKPOINT or an evaluation job's models entries is staged
  server-side and never reaches a client. pre-trained checkpoints has been dropped as an example
  of what does get shipped, since it is the case with the exception.
• "Keys the platform does not recognise are passed through untouched" skipped three that are read
  by FLIP's own NVFLARE components: SERVER_CHECKPOINT, GLOBAL_ROUNDS_AE / GLOBAL_ROUNDS_DM and
  models.
• The same sentence in the user guide is backwards for Flower, where it sat under a
  both-backends bullet list: flwr rejects a run-config key the template does not declare rather
  than ignoring it, failing the run at submission. Now scoped to NVFLARE, with Flower's behaviour
  stated.
• The page contradicted itself. Its pre-existing "Disclaimer" section still told Flower users to
  upload server_app.py (the template supplies it, and an uploaded copy is dropped) and listed
  evaluation among the job types taking trainer.py / validator.py / models.py / config.json.
  Both contradicted the new tables on the same page, so that bullet now points at
  fl-required-files and the obsolete Flower one is gone.

Linked Issues

Follow-up to #811. No issue filed yet — happy to open one if you'd prefer this tracked.

Checklist

• Follows the project's coding conventions and style guide
• Updates documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Type of Change

• 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.
• In-line docstrings updated.
• Documentation updated, tested make -C docs/ docs.

Testing

Documentation-only change — no source files touched.

• make -C docs/ docs after make clean: build succeeds with zero warnings. (When this PR was opened there were 8, all pre-existing autoapi-generated ones; docs: add Security and Governance and compliance pages #850 has since fixed them on develop.)
• Every claim in the new reference checked against the code rather than carried over: the required-files tables against fl-apps/<backend>/required_files.json; the config keys, their ranges and defaults against validate_config / upload.py in fl-services/nvflare/fl-api-base; the client site naming against the kit-slot lookup in fl_scheduler_service (confirmed by [Feature]: Save both best and final models — NVFLARE backend #811's own live e2e log, which shows the server reporting Trust_1 / Trust_2); the Flower run-config path against fl-api-flower's --run-config handling and the base-template bundling in fl_service.py; and the UI behaviour against ModelUpload.vue / Training.vue.
• Cross-references resolve: no new Sphinx reference warnings for the two new labels.
• Re-verified after the corrections above: docs still build with zero warnings, all five inbound fl-required-files references resolve, and the rewritten note renders as a proper admonition. Full CI green on bfca0385.

Additional Notes

The open question for review is scope, not correctness. This sat as a draft while the claims were
being verified; that work is done and every assertion is now traced to a source file. What remains is
a judgement call that is yours:

The split I've applied assumes the user guide should answer "what do I have to do?" and the FL nodes component page should answer "what exactly does the platform accept?". If you'd rather the detail moved out of ReadTheDocs entirely and lived next to the code (fl-apps/ READMEs, with ReadTheDocs linking to it), that is a smaller edit from here — the two new sections are self-contained and can be lifted wholesale.

One thing I deliberately did not do: reconcile this with the "What FLIP actually validates in config.json" section of working-with-flip-apps/package-model-as-map.rst. That section says FLIP reads only job_type and that everything else is unvalidated, which is true of the Central Hub API but not of the FL API, which does validate the keys documented here. It is accurate in its own frame but reads as contradictory next to the new reference. Worth a follow-up, and worth a second opinion on the right framing before I touch it.

[Copilot]

Pull request overview

This PR corrects and restructures the ReadTheDocs documentation around Model Files and Training Configuration, removing misleading backend-specific guidance from the common user guide and moving canonical backend/job-type specifics to the FL nodes component reference page.

Changes:

• Rewrites the user guide’s Model Files section to describe required files as a per-job-type contract surfaced by the UI, rather than a fixed list.
• Replaces the user guide’s detailed NVFLARE-only training config key reference with a short backend-neutral explanation, keeping the existing training-configuration anchor.
• Expands the FL nodes component page with new canonical anchors for required files and training configuration, and repoints the Flower app guide to the new reference anchor.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
docs/source/working-with-flip-apps/create-flip-app-from-flower.rst	Updates the Flower app submission guidance to link to the new canonical required-files anchor.
docs/source/user-guides/user-common.rst	Rewrites common guidance on model files and training configuration to be job-type/back-end aware without embedding backend-specific key listings.
docs/source/components/component-fl-nodes.rst	Adds canonical per-job-type required-files tables and backend-specific training-configuration reference sections with stable anchors.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[atriaybagur]

Ready for review @garciadias.

What this is. The Model Files / Training Configuration section of the user guide was factually wrong in several places. Two of the errors were the kind that waste someone's afternoon: the required-file list omitted models.py and described config.json as optional (it is required by every NVFLARE job type), and the AGGREGATION_WEIGHTS example was keyed on Trust codes (KCH, UCLH). The FL server only ever knows a client by its FL kit slot name — get_net_status.py carries the comment "the FL net never sees Trust.name" — so the documented example matched no client and silently did nothing.

Every claim is traced to a source file. The required-files tables against the per-template and aggregate required_files.json manifests (all nine job types); the config keys, ranges and defaults against validate_config and upload.py; the client naming against the kit-slot lookup in fl_scheduler_service; the Flower run-config path against fl-api-flower and the base-template bundling in fl_service.py; the UI behaviour against ModelUpload.vue / Training.vue.

A self-review round then found six more inaccuracies (bfca0385), five in prose this PR itself adds — they are listed in the description. The one worth your attention is the rounds note. It claimed an out-of-range GLOBAL_ROUNDS or LOCAL_ROUNDS is discarded in favour of the default of 1. True for GLOBAL_ROUNDS, false for LOCAL_ROUNDS: the platform never consumes that key, the default is filled in only when it is absent, and a present-but-invalid value reaches the app's trainer verbatim. "LOCAL_ROUNDS": 5000 runs 5000 local iterations. No FLIP doc described that trap before this PR.

The question for you is scope, not correctness. The split applied here assumes the user guide answers "what do I have to do?" and the FL nodes component page answers "what exactly does the platform accept?". If you would rather the detailed reference lived next to the code (fl-apps/ READMEs, with ReadTheDocs linking to it), that is a smaller edit from here — the two new sections are self-contained and can be lifted wholesale.

Two follow-ups I deliberately did not fold in:

1. working-with-flip-apps/package-model-as-map.rst states "FLIP reads exactly one key out of config.json: job_type" and "everything else is unvalidated", naming LOCAL_ROUNDS. That is true of the Central Hub API but not of the FL API, which validates the keys documented here — so it now reads as a direct contradiction. It needs a framing decision (scope it to the hub bundler, or rewrite it), and I would rather have your view first. Its three code citations are also stale.
2. Drift-proofing: the required-files tables duplicate the manifests. They could become .. literalinclude:: of the two required_files.json aggregates, which are already linguist-generated and regenerated by the fl-apps-required-files pre-commit hook — making the page correct by construction rather than by vigilance.

Docs build with zero warnings, full CI green, no conflicts with develop.

[Copilot]

Pull request overview

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

Suppressed comments (1)

docs/source/components/component-fl-nodes.rst:221

• In this note, “discarded” is a bit misleading because an out-of-range/non-numeric value is ignored by the FL API validator (it isn’t rejected), and the original value can still remain in the deployed config.json. Rephrasing to “ignored by the platform validator” (and using the same term consistently) will better match the actual behavior and avoid implying the key is removed from the file.

   .. note::

      A value that is not a number, or that falls outside the accepted range, is discarded rather
      than reported. What happens next differs between the two keys, because the platform consumes
      them differently.