develop -> main - #808
Merged
Merged
Conversation
…on facts Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…cycle tracker Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Post-review hardening from a third multi-agent review pass: - global_round capped at the PG INTEGER max in both TrainingLog mirrors, so an oversized round (reachable from uploaded app code via flip.send_event) fails validation instead of 500ing at insert. - New AST-level drift guard pins flip.schemas to its flip-api mirrors (TrainingLog, TrainingMetrics, FLLogEvent): the hand-synced definitions now fail the build on drift instead of surfacing as 422s silently swallowed by the best-effort senders. - handle_client_exception docstring no longer claims the hub rejects unattributable error reports outright — this branch made current hubs store them as unattributed fallback rows; only older hubs reject. - resolve_absent_site/forward_replies docstrings say 'never returned a healthy reply' — a synthesised error reply present in the round's replies still counts as absent, which the old 'never answered' hid. - add_log_endpoint docstring documents the log-XOR-event payload shape and the never-drop path for failed free-text rows (stored model-level rather than 400-rejected). Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
… label-helper docstring Review follow-ups (Copilot on #789): log_queue_positions now suppresses add_log's per-row commit (transaction=session) and commits the whole batch once, so a deep queue no longer costs O(N) commits per mutation — with a rollback (itself guarded) on failure. The modelStatusLabelWithQueue doc comment now matches its status-agnostic behaviour. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Review follow-up (Copilot on #789): FLJob has no uniqueness on model_id, so a double initiate can queue the same model twice; a model-keyed last-row lookup lets the newest row suppress one job while the other re-emits on every mutation (oscillation). Tracking the latest prior row per job id matches the function's keyed-by-job-id contract and is pinned by a test. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Copilot review: aria-disabled + a click-capture guard kept the form value safe, but left the Headless UI button focusable and its internal handlers reachable — they never check the disabled prop, and with no v-model bound the component falls back to internal state, so a click or Space on a 'disabled' switch still flipped aria-checked for screen readers. Passing the native disabled attribute through makes the browser deliver no events at all and drops the dead control from the tab order; the greyed-out stays-on-screen presentation is unchanged. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
While a model is queued (INITIATED) the Stop Training button was disabled, so a queued/preparing job could not be cancelled from the UI at all, and a pre-running failure left the net BUSY until manual intervention. flip-ui: the button (and confirm modal) reads 'Abort job' and is enabled from INITIATED; Stop Training copy stays from PREPARED. Download Results remains disabled for stopped models. flip-api: - release_scheduler_for_model frees any BUSY scheduler pinned to a model's jobs (FLScheduler.job_id survives the DELETED flip, so the join still resolves after the dequeue). Wired into abort_model_training's pre-submit, already-terminal and post-abort exits — the stop path frees the net itself instead of waiting for the stale-BUSY watchdog tick. - prepare_and_start_training failure handler now actually releases the net (its log claimed it already did) and returns bool; a JobAbortedError from the new pre-upload/pre-submit gates in start_training means a concurrent abort won: submission is skipped, net released, no ERROR overwrite. - update_model_status ignores late transitions on a STOPPED model (PREPARED/TRAINING_STARTED/ERROR/RESULTS_UPLOADED/RESULTS_UPLOAD_FAILED); only STOPPED->INITIATED (re-initiate) stays open. The internal status endpoint no longer writes a timeline entry for an ignored transition. Integration tests pin the abort->net-free flow on real Postgres. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
… abort The first post-training case still pinned the pre-#787 behaviour (stop button disabled while INITIATED). Update it to assert the new 'Abort job' affordance — enabled while queued, relabelled via title, aborting through the same /fl/stop endpoint — and add a separate case pinning that terminal-state models keep the stop button disabled. Verified green (3 passing) under CI-identical conditions via the dockerized cypress/included image against the stubbed env. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…ings
- Invalidate the session when the emission's failure-path rollback itself
fails: a session whose rollback failed raises PendingRollbackError on every
subsequent use, and handed back to run_jobs_core it would fail the training
start and wedge the net with no researcher-visible error.
- Tiebreak identical created timestamps by FLJob.id in all three queue
orderings (pickup, emission, read rank) so displayed positions can never
disagree with pickup order.
- Exercise log_queue_positions against real Postgres: created-asc/id-tiebreak
ranking, log_date-desc dedup, JSONB details round-trip, the deferred batch
commit, and the feed rendering served by GET /model/{id}/logs.
- Pin the batch-commit contract with an unpatched-add_log unit test and the
invalidate behaviour on the failing-rollback path.
- Fix comment rot found in review: buildModelSteps is no longer purely
status-driven; FLLogEvent now holds a hub-emitted, round-less member that
never passes the ingest validator; per-job (not per-model) emission wording;
duplicate-job semantics on queued_positions_by_model; collateral wording in
private.py, main_models.py, add_log and log_rendering.
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Paint the page background on <body> (bg-body / dark:bg-dark-canvas) so the browser canvas cannot show through as black under color-scheme: dark, and size AuthLayout/SecondaryLayout with h-dvh instead of h-screen so the layout tracks the dynamic mobile viewport (URL bar, keyboard, overscroll). Keep <meta name="theme-color"> in sync with the class-driven app theme via the new useThemeColorMeta composable: static prefers-color-scheme meta variants follow the OS scheme and can disagree with the useDark() toggle. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
iOS 26 Safari no longer honours theme-color and letterboxes any page whose viewport meta lacks viewport-fit=cover inside the safe area, painting the exposed status-bar/search-bar regions itself — so the body-background/h-dvh fix could not reach the black bands at the top and bottom of the screen. Declare viewport-fit=cover so the page paints its own background edge-to-edge behind the Safari chrome, and keep content clear of the notch/home-indicator with env(safe-area-inset-*) guards: AuthLayout's logos and footer links, SecondaryLayout's page padding (with the card capped at max-h-full inside it), and MainLayout's app shell. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…over)" This reverts commit bbdd283. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
On-device testing shows the round-1 body background fixes the bands, but the bleed-cropped corner artwork (anchored top-0/bottom-0) still stops short of the physical screen edges: without viewport-fit=cover iOS confines positioned elements to the safe-area letterbox, so the artwork's cut edges show as floating straight lines. Re-add only the viewport meta (no safe-area content guards yet) to isolate the variable for the next on-device check. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
On-device probing (iOS 26 Safari) shows portrait rendering is hard- confined to a letterbox inset from the physical screen: fixed elements clip at its edges, canvas background-image layers anchor to it, no viewport unit sees the full screen, and safe-area insets report 0 — with or without viewport-fit=cover. Only the flat canvas colour (the body background) reaches the screen edges, tinted under the chrome. The bleed-cropped corner SVGs therefore cannot reach the physical corners on iOS by any positioning, and their straight cut edges show as floating lines against the continuous background. Fade the letterbox-facing edge of each (mask-image, 48px) so the artwork dissolves into the background instead; scoped to iOS via (hover: none) + @supports (-webkit-touch-callout: none) so desktop and Android keep the flush corner rendering. Drop viewport-fit=cover for good: measured inert in portrait and a notch hazard in landscape without env() guards. The index.html guard spec now pins its absence and documents why. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
On narrow screens each footer anchor wraps its label onto multiple lines, and the wrapped text was left-aligned inside its anchor; text-center on the footer keeps the wrapped labels centred. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Screen readers should skip decorative images; AuthLayout's already carry alt="". Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
The staged chip tabs (filled purple pill + connector line) are replaced by the design 04-A ProjectChrome ghost tab nav: no border, no underline — the active tab is ink on a soft warm-gray paper-2 fill, inactive tabs are muted ink-3 text. New ink/paper tokens land in tailwind.config.js from the handoff's tokens.css. Locked/live/done affordances are unchanged; the done tick stays green in both states now that the active fill is light. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…warm body The handoff's paper-2 fill sits within 2% of the app's body background, so the active Prepare/Run tab was nearly invisible. Alpha ink (white in dark mode, slightly lighter there for the same reason) composes on any backdrop; the unused paper token goes, and the temporary design-note references are dropped from the comments. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…ric-plots PR #768 now stacks on #757 (its new PR base). Beyond textual conflicts: - alembic: ee61ab95ef2b re-parented onto 8c4f2b9d31e7 (both revised 23aff57898a0; the chain must stay linear) - metrics response models keep #757's snake_case + serialization_alias convention with #148's float x_value / (label, x_label) plot identity - TrainingMetrics.vue keeps #757's single/grid layout with plots keyed by chartId (yLabel + xLabel pair) and #148's tab disambiguation - get_metrics ordering test fixtures supply x_value (NOT NULL post-migration) Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…ld chart title The FLIP#148 default was 'Global Round' (singular); the chart title it replaces always read 'Global Rounds'. Applied across the constant pair (flip-utils + flip-api mirror), the migration's backfill server_default, the dev mock seed, and every test/docstring that spelled it out. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…callers
Update stale statements in root/service CLAUDE.md and RST docs so they match
current code:
- Root/flip-api CLAUDE.md + AGENTS.md wrongly said the DB layer is "asyncpg
with async context managers". flip-api uses sync SQLModel `Session` on top
of `postgresql+psycopg2://`, with prod auth via RDS Proxy + a per-connection
IAM token minted by a SQLAlchemy `do_connect` hook (FLIP#556), and the
`with Session(...)` context that FLIP#773 made load-bearing on error paths.
- flip-utils/README.md's "Job Types" and "App/Tutorial Compatibility" tables
omitted the two NVFLARE Client API job types (`standard_client_api`,
`evaluation_client_api`) that already ship under `fl-apps/nvflare/` with
matching tutorials under `fl-tutorials/nvflare/`.
- component-fl-nodes.rst still claimed static NVFLARE app files "are being
moved from S3 buckets to the flip package"; FLIP#724 already completed that
move — templates are baked into the flip-api image at `FL_APP_BASE_DIR`
(`fl-apps/`) and the S3 template bucket is gone. The disclaimer also
predated the Client API job types.
- create-flip-app-from-flower.rst said `SUPERNODE_NAME` must match the trust's
central-hub display name; it must actually be the FL kit slot
(`Trust_1` / `Trust_2` / ...), set in the trust compose to `${FL_KIT_SLOT}`.
The hub resolves the slot back to the owning trust in
`resolve_trust_from_fl_client_name`.
- trust/imaging-api/CLAUDE.md and trust/data-access-api/CLAUDE.md said they
"only receive internal requests from trust-api"; imaging-api is also called
by the fl-client via the `flip` package, and data-access-api is also called
by imaging-api for `/cohort/accession-ids`. All internal calls now carry the
per-trust `TRUST_INTERNAL_SERVICE_KEY` header.
Also fix a handful of missing return-type annotations flagged by the sweep
in `init_training.py`/`init_evaluation.py` (NVFLARE `Controller` overrides)
and `user_models.py::Permission.__repr__` — no behaviour change.
Signed-off-by: Claude <noreply@anthropic.com>
Federated training leaves a PyTorch checkpoint, which is a research result rather than something a hospital can run. flip.export turns that checkpoint into a MONAI Bundle -- a TorchScript file carrying inference.json and metadata.json as extra files -- which can then be packaged as a MONAI Application Package that takes DICOM in and emits DICOM out. The exporter consumes the bundle configuration rather than generating it. An inference chain cannot be derived reliably from a free-form training application, so the app author writes inference.json and metadata.json once and they are read from <app-dir>/../export/. That keeps this function's job narrow: load the checkpoint, load it strictly into the architecture the app declares, compile, embed the configs, stamp provenance. Scripting is the default because torch.jit.script needs no example input. Passing --input-shape is still worth it: it enables a numerical equivalence check against eager mode, and a max|delta| of 0.0 is positive evidence that compilation changed nothing. Tracing is available as a fallback. Provenance is embedded rather than attached, so a deployed artefact cannot become separated from the record of the run that produced it. Packaging runs separately from training and does not call the hub, so those values are supplied by the caller. Two constraints shaped the surface. torch lives in the full extra rather than the base install, so the subpackage is not imported from flip/__init__.py and raises a clear message naming the fix if torch is absent. And flip-utils declares no console scripts, so the CLI is python -m flip.export rather than introducing a top-level command. scripts/inspect_checkpoint.py becomes a thin front end over the same checkpoint module, so there is one implementation of the two on-disk checkpoint shapes rather than two. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Packaging a trained model for inference means transcribing its validation transform chain into the bundle's inference.json. That chain cannot be discovered automatically -- transforms.py is optional and free-form, and function names differ legitimately between apps -- so the least a platform can do is make the file predictable. The spleen tutorial and the Flower xray tutorial already declared their transforms in transforms.py. Only the two NVFLARE xray variants buried get_xray_transforms in data_utils.py, so this brings them into line rather than inventing a new convention. The monai.transforms import moves with the function; nothing else in data_utils.py used it. This is a convention, not an enforced contract. transforms.py is in no job type's required-file list and nothing validates it. It buys one predictable place to look, which is worth having when a mismatch between training and inference preprocessing produces a MAP that runs cleanly and is quietly wrong. Verified by re-running both tutorials on the NVFLARE simulator: training and cross-site validation complete and an aggregated model is written, so both trainer.py and validator.py resolve the moved import under NVFLARE's dynamic loading. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
These are the per-model half of packaging: the input specification and the inference chain, written by the person who knows the preprocessing. They live beside the training code whose transforms they must mirror rather than with the MAP templates, because that is the axis along which they vary. The input specification uses MONAI's own network_data_format block in metadata.json -- spatial shape, channel counts and semantics, dtype, value range -- rather than a FLIP-specific schema. It covers exactly this information and is what the MONAI tooling already reads. The spleen preprocessing is transcribed from the tutorial's own get_val_transforms, and deliberately not from the MONAI Model Zoo spleen bundle. The two disagree: the zoo bundle windows CT intensity at a_max=164 while this tutorial trains at a_max=250. The architectures are otherwise identical, so copying the zoo config would have produced a MAP that ran cleanly and segmented slightly worse, with nothing to indicate why. The xray config carries metadata only. Classification drives inference through a bespoke operator rather than the bundle inference operator, so there is no inference.json for the runtime to read; the labels and their activation are recorded for the operator and for provenance. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
A MONAI Application Package wraps an inference pipeline as a container that takes DICOM in and emits DICOM out. These are the reusable half of that: the operator graph, which varies by output type rather than by model. Segmentation is bundle-driven -- MonaiBundleInferenceOperator reads the preprocessing and inferer from the embedded inference.json -- so retargeting it to another segmentation model means changing the bundle, not the graph. Only the segment descriptions and the series-selection rule are app-specific. Derived from the App SDK's spleen example, MONAI Consortium copyright retained. Classification needs its own template because MonaiBundleInferenceOperator maps an image to an image and has no way to emit a label. It drives the scripted network through a bespoke operator and writes a DICOM Structured Report instead of a Segmentation. These live at the top level rather than under each tutorial because a MAP consumes trained weights, not federated-learning configuration: the same segmentation template serves an app trained under either backend. Nesting them under fl-tutorials/<backend>/ would encode a dependency that does not exist, and would duplicate one operator graph across every tutorial. holoscan-source.json pins the packager's base-image manifest locally. The packager otherwise fetches it from GitHub, where most versions now 404, and the pinned image is CUDA 12 on Ubuntu 24.04: CUDA 13 images require NVIDIA driver 580 or newer, and the CLI pins Ubuntu 24.04 Python package versions. The .gitignore rules are deliberately narrow. A blanket *.ts would swallow flip-ui, which is TypeScript, so only the paths where TorchScript bundles appear are ignored. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Covers the step between a finished FLIP training run and something a radiology department can use: obtaining the aggregated checkpoint, checking it still loads into the architecture its app declares, exporting a bundle, packaging a MAP, running it, and verifying the DICOM that comes out. Packaging is documented as a separate step from training rather than a stage inside it. That keeps a packaging failure from ever affecting a training run, avoids the FL runtime carrying deployment dependencies whose versions do not currently agree with its own, and allows a checkpoint to be re-packaged with corrected preprocessing without retraining. Two sections exist because the failure modes are quiet rather than loud. "Finding your inference transforms" gives the transcription procedure, since a mismatch between training and inference preprocessing yields a MAP that runs and is wrong. "From NIfTI training to DICOM deployment" enumerates what the MAP does that training never did -- series selection, volume assembly, frame of reference, coded terminology -- because each is somewhere a valid DICOM object can still be a wrong one. The environment traps are recorded in full. None are documented upstream and several cost real time: App SDK 4.x building CUDA 13 images that need a newer driver, the monai-deploy CLI no longer existing, holoscan-cli 4.3+ dropping the packaging interface, base-image manifests returning 404, a UID mismatch between build and run, and a CUDA runtime shadowing that breaks holoscan. deepcOS has its own section rather than being folded into the walkthrough, because a plain MAP is not a deepcOS submission. What that requires is recorded as consequences for FLIP, with the vendor's access-controlled documentation cited as authoritative rather than reproduced. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…vyjmka docs: fix drift across READMEs, Sphinx docs, and AWS CLAUDE.md
…t-egress fix(security): FLIP-PT-002 gate row-level cohort egress, FLIP-PT-088 replace SQL denylists
…ependence fix(tests): stop alembic's fileConfig disabling the application logger
Auto-regenerated from develop by .github/workflows/regenerate_docs_gifs.yml. Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
docs: add Security and Governance and compliance pages
docs: regenerate documentation GIFs
Resolve the Networking-section conflict in admin-platform-support.rst: develop trimmed the private-subnet/SSM paragraph into the opening one and rewrote the VPN note; keep that structure and re-attach the PT-091 Orthanc basic-auth sentence to it. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
The user guide's "Model Files" section stated a fixed minimum file set (validator.py, trainer.py) for NVFLARE apps and described config.json as optional. Both are wrong: required files are declared per job type in fl-apps/<backend>/<job_type>/required_files.json, config.json is required for every NVFLARE job type, and standard_client_api does not require validator.py while evaluation requires evaluator.py instead of a trainer. The "Training Configuration" subsection also carried a full NVFLARE key reference in a page that is meant to be backend-neutral, with several inaccuracies: the round range is 1-1000 (not "greater than 0, less than 100") and out-of-range values are discarded in favour of the default rather than rejected; AGGREGATION_WEIGHTS keys are FL client site names (the kit slot each Trust occupies, e.g. Trust_1), not Trust codes such as KCH, so the documented example would have had no effect. Rework the user guide to describe the job-type-driven contract and point at where the UI surfaces the applicable list, and move the corrected per-backend reference to the FL nodes component page, which is already the documented home for framework-specific file requirements and job types. The new reference covers the required-files manifests for both backends, the validated NVFLARE config.json keys (including AGGREGATE_ONLY_REGEX and the BEST_MODEL_METRIC pair) and Flower's pyproject.toml/config.toml run configuration. Also correct the FL nodes page's claim that pyproject.toml is a researcher-supplied file for Flower apps: it comes from the platform's base template, and per-run overrides go in config.toml. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Both edited pages described the job type as declared by the job_type key
in config.json without noting that config.json is itself a required file
only for the NVFLARE job types. A Flower app may omit it entirely — the
bundler falls back to job_type=standard when no config.json is present —
so the previous wording ("if that key is absent") covered a missing key
but read as though the file were mandatory for every app.
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Conflict in docs/source/user-guides/user-common.rst: develop (#828) corrected the GLOBAL_ROUNDS/LOCAL_ROUNDS range from '<100' to '1-1000 inclusive' inside the NVFLARE key reference that this branch removes from the user guide. The corrected range is already carried by the replacement reference at _fl-training-configuration in components/component-fl-nodes.rst, so develop's fix is subsumed rather than lost. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
…ning FLIP-PT-091: Orthanc auth hardening — fail-closed image, k8s secret required, CI auth smoke
Harden XNAT configuration on both deployments: fail loud on config API errors (Compose + Kubernetes), drop PACS_DICOM_PORT indirection
Three conflicts, all from PRs that landed on develop after this branch was cut and touch the same XNAT configuration paths: - .github/workflows/docker_build_xnat_web.yml — #444 removed the credential build args; this branch stopped hardcoding XNAT_VERSION and reads it from trust/xnat/.env. Kept both: develop's credential-free BUILD_ARGS list, with XNAT_VERSION still sourced from the .env pin. - deploy/providers/kubernetes/templates/xnat-init-job.yaml and trust/xnat/xnat/config/configure-xnat.sh — #865's fail-loud rework (xnat_curl, bounded readiness waits, ContainerManager role) supersedes this branch's narrower readiness hardening, so develop's versions are taken wholesale and only the 1.10.0-specific deltas re-applied on top: the non-empty siteUrl in the site-activation call (1.10.0 NPEs on a null siteUrl), allowAllUsersToUseDqr=false (#846), the DicomEdit 6.9.x anon-script comment, and the Swarm fresh-install note. Nothing else conflicted. Note that #444 having landed removes this branch's need to bake per-trust datasource passwords into separate images — the password now arrives from the deployment env at start. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Member
Author
|
Description refreshed again — seven more PRs landed on What that added:
Schema migration count is unchanged at seven — none of the new merges add a revision. CI: all 55 checks green at Over to you for the merge. No Copilot review requested on this one — it carries no direct commits and each constituent PR had its own round; a bot pass over a 1001-file release diff would just be noise. |
Review of the new reference sections surfaced six inaccuracies, five of them in prose this branch adds. The invalid-rounds note 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 the documented safety net does not exist for it. Also note the BEST_MODEL_METRIC exception, where a discarded GLOBAL_ROUNDS fails the job rather than defaulting, and that neither key is rewritten in the deployed config.json. 'Any additional file is bundled into the app' omitted two carve-outs: a model file colliding with a base-template name is skipped with only a server-side log line (which for Flower silently drops server_app.py, strategy.py and __init__.py — 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. Drop 'pre-trained checkpoints' as an example of what gets shipped, since it is the case with the carve-out. 'Keys the platform does not recognise are passed through untouched' skipped three keys FLIP's own NVFLARE components do read: SERVER_CHECKPOINT, GLOBAL_ROUNDS_AE/GLOBAL_ROUNDS_DM and models. In the user guide, the same pass-through sentence sat under a both-backends bullet list but is backwards for Flower, where flwr rejects a run-config key the template does not declare instead of ignoring it. Scope it to NVFLARE. Finally, the page's own 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/validator/models/config. Both contradicted the new tables on the same page, so point that bullet at fl-required-files and remove the obsolete Flower one. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
garciadias
previously approved these changes
Aug 5, 2026
…files-docs-atbrtr [Docs]: Correct Model Files / Training Configuration in the user guide
…sert Found live on kind while testing this upgrade: the init job reported success but `flipServiceAccount` could not authenticate, so imaging-api got 401 on every call and the trust's whole imaging path was dead. The chart created the account with an INSERT into `xdat_user` carrying a fixed bcrypt hash. That writes no matching `xhbm_xdat_user_auth` "localdb" record, and that record — not `xdat_user.primary_password` — is what authentication reads. XNAT 1.9.3 masked this by back-filling the record on the password PUT that follows; 1.10.0 does not, and the PUT still answers 200, so nothing in the job noticed. Verified on kind with two releases of this chart side by side on one cluster: `GET /projects/` on imaging-api returns 401 against the SQL-created account and 200 once the account is created over REST. `xhbm_xdat_user_auth` lists `flipServiceAccount` only in the latter. `POST /xapi/users` is the only path that provisions the auth record, and it is what the Compose `configure-xnat.sh` has always used — which is why Compose was never affected. The init job now does the same, check-then-create like the PACS registration, and the SQL container is gone (its role grants are re-applied by the REST calls a few lines below). That also drops a committed bcrypt hash of a shared password from the chart. TROUBLESHOOTING.md 2.3 gains the version-specific root cause, a psql probe for the missing record, and the recovery path for an account already created the old way — a password sync cannot repair it. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
Member
Author
|
I think this one's ready now! |
The worker C-MOVEd studies from Orthanc to XNAT itself, bypassing the DQR plugin, and was written when `PacsNotStorableException` was believed to be a DQR bug. TROUBLESHOOTING 2.1 already records the real cause (the destination AE/port mismatch) and 2.5 already said the worker is "not needed for normal operation" — while values.yaml shipped it `enabled: true`, contradicting its own docs. Two things settle it: It has not run since 2026-06-16. #530 added a container securityContext with `capabilities: drop: [ALL]`, and the worker's command apt-installs dcmtk at startup. apt can neither seteuid to `_apt` (no CAP_SETUID/SETGID) nor bypass DAC on /var/lib/apt/lists/partial, which is owned by `_apt` mode 700 (no CAP_DAC_OVERRIDE), so `apt-get update` exits 100 and the pod crashloops. Every stream is redirected to /dev/null, so the logs are empty and nothing surfaced it. Only the Deployment variant carried a securityContext, which is why the Job variant looked fine. DQR does the job on Kubernetes. Verified on kind against this chart with the in-cluster Orthanc seeded: imaging-api project create -> DQR C-FIND found all 6 accessions -> POST /xapi/dqr/import accepted all 6 (no PacsNotStorableException) -> C-MOVE delivered 6 DICOM files into the XNAT archive -> all 6 sessions archived and `xhbm_direct_archive_session` drained to empty. That is precisely the operation the worker existed to perform. Keeping it would also be wrong on the merits: it bypasses the DQR relabel map, so sessions it imports are not relabelled (Subject UUID / Session=accession) — the labelling fl-client lookups depend on. Removes the Deployment, Job, ConfigMap, the script, and the values block, plus its entries in the Makefile rollout loops, check_status.py expected services, README and TROUBLESHOOTING. The DCMTK diagnostic pod (now 2.5) remains as the manual C-MOVE path. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
DICOM-to-NIfTI conversion now runs end-to-end on Kubernetes.
The Container Service must be told which PVC holds the archive and build
directories. Left unset it falls back to "no PVC mount" and bind-mounts the
scan path as a node hostPath at XNAT's own path (/data/xnat/...). That path
exists only inside the xnat-web pod, so kubelet creates it empty on the node
and dcm2niix exits with "Unable to find any DICOM images in /input" — the
Job pod pulls, starts and fails, which reads like a broken image rather than
a storage misconfiguration. Confirmed on the node: the whole scan directory
tree present, zero files in it.
The single xnat-web data PVC holds both archive/ and build/ at its root
(xnat-web mounts them by subPath), so this is the plugin's "combined" case:
combined-pvc-name + combined-path-translation.
The trailing slash on combined-path-translation is load-bearing. It is the
prefix the plugin strips from XNAT's paths to build the volume's subPath;
without it the subPath keeps a leading "/" and the Kubernetes API rejects
the whole Job with 422 `volumeMounts.subPath: Invalid value: "/archive/...":
must be a relative path`. The launch then fails before any pod exists, so
GET /xapi/containers gains no record and only containers.log shows it.
Verified live on kind: launch -> container status Complete -> the scan gains
a NIFTI resource (input_Chest_HCT_...nii.gz) alongside DICOM. Note the
plugin's Kubernetes informers are interrupted when the backend config
changes, so xnat-web needs a restart after editing it.
Also replaces DCM2NIIX-K8S.md, whose premise ("does not yet run end-to-end")
and recommended fix (RWX storage or nodeAffinity) are both superseded by the
plugin's own PVC settings. What survives from it is the multi-node caveat —
the Job mounts the same PVC as xnat-web, so ReadWriteOnce only works when
the Job lands on the xnat-web node — which moves to the chart README and
TROUBLESHOOTING 2.3b along with the failure modes above.
Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
garciadias
previously approved these changes
Aug 5, 2026
…as5fb3 feat(xnat): upgrade XNAT to 1.10.0 (DQR 3.0.0, Container Service 3.8.1) and fix the Kubernetes imaging path
* fix: clear open Dependabot security alerts across manifests Bump cryptography, aiohttp, pillow, monai, setuptools, and the npm transitives (axios, undici, svgo, postcss, brace-expansion) to their patched versions to close the outstanding Dependabot alerts. No application code changes were required; only lockfiles moved. flask (nvflare pins 3.0.2) and torch/setuptools (cu128 GPU driver pin) in flip-utils remain intentionally held back pending upstream support. Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com> * fix(ci): unbreak docs build after setuptools 83.0.0 bump setuptools 83.0.0 (pulled in by this PR's security sweep) drops the pkg_resources module entirely. pybtex 0.24.0's plugin loader still imported pkg_resources, so sphinxcontrib-bibtex failed to load and the Sphinx build errored out before touching a single page. Bump sphinxcontrib-bibtex to >=2.7.0, which requires pybtex>=0.25 — pybtex switched to importlib.metadata for entry-point discovery in 0.25, removing the pkg_resources dependency (and its transitive six dependency) altogether. Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com> * fix: address PR #874 review feedback - fl-services/flower/fl-base/Dockerfile: pin cryptography>=50.0.0 on the pip install line so the Flower supernode actually picks up the patched version — flwr 1.32.0 pins cryptography<47.0.0, and the plain `pip install` here never read flip-utils/uv.lock's resolved 50.0.0. - flip-utils/pyproject.toml, fl-services/nvflare/fl-api-base/pyproject.toml: raise constraint-dependencies floors (aiohttp, cryptography, pillow, monai) to match the versions this PR actually landed, and re-lock both projects. A dependency scan of the regenerated flip-utils lock also turned up click (CVE-2026-7246) and fonttools, both raised alongside. - PR description: corrected the "no constraint edits" claim, documented the previously-unlisted docs/uv.lock and npm lockfile moves, and added GHSA/CVE references to the known-residual setuptools/torch items. Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com> * fix: raise remaining Flower cryptography and monai floors Two items called out in review as still outstanding after 929afc5: - fl-services/flower/fl-api-flower/pyproject.toml: raise the cryptography override floor from >=48.0.1 to >=50.0.0 to match the version landed everywhere else in this sweep; re-lock uv.lock (resolved version is unchanged at 50.0.0, only the constraint metadata moved). - fl-apps/flower/{standard,evaluation} and the three Flower fl-tutorials templates (xray_classification, 3d_spleen_segmentation, 3d_spleen_segmentation_evaluation): raise monai from >=1.5.2 to >=1.6.0 so the cross-backend MONAI alignment from 1528003 survives Flower's per-run uv sync resolve, rather than depending on newest-wins. fl-api-flower's full test suite (97 tests) passes against the re-locked dependency. Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com> --------- Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
garciadias
approved these changes
Aug 6, 2026
This was referenced Aug 6, 2026
Merged
Merged
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Release of
developintomain— ~108 merged PRs, 1001 files (contents as off4e46391, 2026-08-05; the list below grows if further merges land ondevelopbefore this is merged).Headline features
flip-utils([Bug]: Flower 1.32 runtime dependency install shadows the baked in-repo flip-utils in FL runs #772 / FLIP#767). NVFLARE upgraded to 2.8.0 (Upgrade NVFLARE backend 2.7.2 → 2.8.0 #667).make run-tutorial— app_files not staged into job custom/ (No module named 'models') #691, Head-only broadcast for frozen-backbone cross-site validation (GlobalModelEval) #733, fix(nvflare): Ark+ finetuning platform wiring + PercentilePrivacy stock defaults #740), plus the image-orientation fix — the tutorials were feeding the model sideways radiographs because a transpose was never undone (Ark+ tutorials feed the model sideways radiographs (transpose never undone) #821, with the training-transform half in spike: package a FLIP-trained model as a MONAI Application Package (MAP) #812).add-fl-kitsautomation, so onboarding a trust no longer needs a flip-api task-definition revision or restart ([Feature]: Dynamic FL kit slots — secret-sourced pool, reconcile-on-miss, and add-fl-kits automation #781 / FLIP#780), abort a job before it starts running — pre-run cancel that also frees the net left wedgedBUSY([Feature]: Abort an FL job before it starts running — pre-run cancel that frees the stuck BUSY net #790 / FLIP#787), best-model selection — save the best global model alongside the final one on NVFLARE via stockIntimeModelSelector([Feature]: Save both best and final models — NVFLARE backend #811 / FLIP#673), FL queue position surfaced to users ([Feature]: Show a model's position in the FL scheduler queue (Live activity + Models page) #789), evaluation abort status ([Bug]: Evaluation job reports RESULTS_UPLOADED with empty results when every validate task aborts #756),RUNNINGstatus rename with ingest alias (Rename TRAINING_STARTED → RUNNING and emit it for evaluation jobs (eval runs stuck at INITIATED; e2e false alarm) #783), NVFLARE kit over-provisioning + add-client provisioning (feat(fl): over-provision NVFLARE kits + consolidate FL provisioning into fl-services (#626) #647, Add zero-disruption client addition vianvflare provision --add_client#680).flip.export, per-tutorial bundle configs,map-apps/templates and the deepcOS findings, all docs-and-templates, no deployment procedure (spike: package a FLIP-trained model as a MONAI Application Package (MAP) #812); NVFLARE latent-diffusion tutorial migrated to the Client API (Migrate the NVFLARE latent diffusion model tutorial to the Client API (new multi-stage recipe + sibling tutorial) #840); scripted end-to-end demo video recorder (X-ray + spleen with DICOM-SEG overlay) and demo-data seeding (feat: scripted end-to-end demo video recorder (X-ray + spleen w/ DICOM-SEG overlay) + demo-data seeding #817), with the dcmqi conversion image pinned to v1.5.6 (chore: pin dcmqi conversion image to v1.5.6 #855).curl -sthat exited 0 on an HTTP error is replaced by a 2xx-or-abort helper in the Composeconfigure-xnat.shand the Kubernetesxnat-init-job, so a rejected configuration call aborts the deploy instead of being silently skipped. Porting it surfaced a real chart bug: the admin was never granted XNAT'sContainerManagerrole, so dcm2niix registration was silently 401/403-ing and the Job still exited 0 — a "configured" trust with no DICOM→NIfTI conversion. Also drops thePACS_DICOM_PORTindirection from PACS registration (the container port4242is the only correct value).idle in transactionon error paths ([Bug]: flip-api DB pool exhausted by leaked idle-in-transaction sessions under Flower metrics burst #775 / FLIP#773), lazy engine init (flip-api: lazy-initialize the SQLAlchemy engine in db/database.py #712), XNAT bulk-import livelock fix (fix(xnat): harden trust XNAT against bulk-import event-bus livelock #663), Flowerloss=nantraining guard (Flower xray_classification tutorial: deterministic loss=nan on training start (RTX 5090 / torch 2.11) #766 / FLIP#764), alembic head merge (fix(db): merge the fl_logs and RUNNING-rename alembic heads #806), and the flip-api test suite made deterministic again — Alembic'sfileConfigwas disabling the application logger in-process, turning 24caplogtests permanently red on a full run (fix(tests): stop alembic's fileConfig disabling the application logger #867).Security
uploaded/→scanned/prefixes are now a real quarantine boundary: uploads registerSCANNING, pickle-bearing files get a structural picklescan, and only clean files are promoted (ETag-pinned copies, fail-closed on scan error/timeout,INFECTEDquarantine). Adds an extension whitelist enforced before the presigned POST is minted. Python source is deliberately not analysed — tracked in [Feature]: Gate malicious Python source in uploaded model code (review or static analysis) #844; signature-based AV in FLIP#838./cohortstatistics route always had:/cohort/dataframe(the FL training-data path behindflip.get_dataframe) and/cohort/accession-ids(the pointer set deciding whose studies get pulled into XNAT) refuse below-threshold cohorts with a fixed message, identical for zero rows and threshold-minus-one, so the refusal is not a row-count oracle. The SQL denylist is replaced by a sqlglot parse-and-re-emit with an AST-decided statement allowlist (writable CTEs rejected), anddetail=str(e)leakage is closed on the same handlers.COHORT_QUERY_THRESHOLDis aPositiveIntso a0cannot silently disable all three checks at once.orthanc-registered-userssecretKeyRef wasoptional: true, so a chart install that never set it ran an unauthenticated PACS; unset kit vars degenerated to{"": ""}; and a userless Orthanc auto-registers the well-knownorthanc/orthancdefault. The entrypoint now refuses to start unless at least one non-empty user/password pair is configured,ORTHANC__AUTHENTICATION_ENABLEDis pinned, and a PR-triggered CI smoke asserts the refusal across the whole empty-credential matrix.docker.sockmount (fix(security): isolate XNAT Container Service behind a Docker socket proxy #774), minimal SSM bastion (INFRA: Trim Central Hub EC2 to minimum-viable SSM bastion (post-Fargate cleanup) #698), unused ECR endpoints removed (Remove unused ECR VPC endpoints #696), fl-api path-traversal + zombie hardening (fix(fl-api): harden path-traversal + SSRF + command-injection (#649) #656, fix(fl-api): drop uvicorn --reload in prod + ECS healthcheck (#593 pt.1) #600), kit egress persistence + helm secret ownership (fix(k8s): regenerate FL-server egress block on sync-kit (#593 pt.3) #597, fix(k8s): adopt synced Secret into Helm + patch keys despite hook failure (#595) #598).dockergroup (FLIP-PT-048: drop on-prem trust login user from docker group #441 / FLIP-PT-048).Deploy notes (for whoever merges)
INFECTEDfile status +uploaded_files.updated_atfrom [Feature]: Malware scanning of uploaded model files #841, among others) — flip-api runsalembic upgrade headat boot, fail-fast.GET /fl/quiesceto report no BUSY net, deploy, disable.SCANNED_MODEL_FILES_BUCKETat thescanned/prefix in the stag/prod env files ([Feature]: Malware scanning of uploaded model files #841). Terraform'sscanned_model_files_uriis already repointed, but if the two prefixes still resolve to the same location the promote step degrades to a logged no-op and the quarantine boundary silently disappears./flip/fl_kit_slot_namesSSM parameter before the new flip-api boots ([Feature]: Dynamic FL kit slots — secret-sourced pool, reconcile-on-miss, and add-fl-kits automation #781) — production reads the pool from Parameter Store with no env fallback, so a missing parameter means the kit pool cannot grow. First rollout is self-contained:make -C deploy/providers/AWS apply-fl-kit-slots(renders the parameter and grants the task role the scopedssm:GetParameter).make register-trustwrites them intotrust/.env.<CODE>.<env>; the entrypoint fails fast (with the remedy printed) if they are missing or rejected. An already-initialisedxnat-dbkeeps its old passwords — Postgres applies them only at first initdb — so rotate with theALTER ROLEcommands the entrypoint prints.ORTHANC_USERNAME/ORTHANC_PASSWORD(trust/.env.exampleshipsadmin/admin— Orthanc is the dev mock PACS), but a kit that leaves either empty will now abort instead of running effectively open. Kubernetes deployers must setorthanc-registered-users— the secretKeyRef is no longeroptional, so a chart install that omitted it (previously an unauthenticated PACS) will now fail to start until it is populated.make -C trust up-trust/ the Helm install with the HTTP status and response body, where it used to be skipped in silence — expect deploys that "worked" against a half-configured XNAT to start failing honestly. On Kubernetes the chart now grants admin theContainerManagerrole that Container Service ≥ 3.7.0 requires; without it dcm2niix was silently unregistered and training later died withnum_samples=0.PACS_DICOM_PORTno longer feeds PACS registration — any kit setting it to a non-4242 value was already silently breaking registration.COHORT_QUERY_THRESHOLD(shipped default10, now intrust/.env.exampleand both trust composes) applies to/cohort/dataframeand/cohort/accession-idsas well as/cohort. Trusts whose kit files predate this inherit the default — but note the behaviour change: FL training against a cohort smaller than the threshold now refuses rather than returning rows, and so does the accession-ID pull. Set it per trust to that trust's own disclosure floor.dockergroup (FLIP-PT-048: drop on-prem trust login user from docker group #441) — run docker andmake -C trust up-trustunder sudo (sudo -E env PROD=<env> make -C trust up-trust).--waituses stricter kstatus readiness.sha-<short7>tags via task-definition revisions (Prod/stag hub deploys: immutable SHA image tags + task-definition-revision deploys #752 / FLIP#751) —make deploy-centralhub/make rollback-centralhub.Full PR list (main..develop merges)
#441, #444, #488, #499, #501, #597, #598, #600, #617, #647, #650, #651, #656, #657, #658, #660,
#663, #667, #669, #670, #671, #672, #674, #678, #680, #681, #682, #683, #685, #686, #689, #691,
#696, #697, #698, #700, #709, #710, #712, #713, #714, #717, #720, #721, #722, #723, #725, #728,
#729, #731, #733, #734, #736, #738, #740, #741, #742, #750, #752, #753, #756, #757, #758, #759,
#760, #761, #762, #765, #766, #768, #769, #771, #772, #774, #775, #781, #783, #789, #790, #792,
#795, #797, #799, #800, #801, #804, #806, #810, #811, #812, #817, #821, #825, #828, #839, #840,
#841, #847, #848, #850, #854, #855, #861, #864, #865, #866, #867, #873 — plus the imported
arkplus-appsfork history (#2–#12 from ParhomEsmaeili's fork, merged via #804).Linked Issues
Release PR — issues are linked on the individual PRs above.
Checklist
Type of Change
make -C docs/ docs(per individual PR).Testing
f4e46391— all 55 checks pass: service test suites (flip-api, flip-ui, trust-api, imaging-api, data-access-api, flip-utils), Cypress groups 1–6, kind E2E, CodeQL, Helm lint/template, terraform-validate, lighthouse, secret scanning, and the "Ensure PR to main originates from develop" gate.e2e_smokeon both FL backends).Additional Notes
Written against head
f4e46391(2026-08-05), refreshed from thee96d74d8revision (itself refreshed from the original8a8e324fbaseline). Seven PRs landed since the last refresh: #828, #839, #847, #850, #865, #867, #873. If more merges land ondevelopbefore this is approved, refresh the list (git log origin/main..origin/develop --merges) — the deploy notes above are stable, and notes 3–9 are the ones that need operator action beyond a normal redeploy.