NVFLARE agent skills: PyTorch/Lightning conversion, orient, diagnose — with lint, eval, packaging, and security hardening

[chesterxgchen]

Summary

Adds a set of NVFLARE agent skills — procedural playbooks a coding agent
(Codex/Claude) loads to convert ML training code to federated learning, orient
in an NVFLARE project, and diagnose failed jobs — together with the packaging,
install, lint, and evaluation infrastructure to author and ship them safely.

The skills follow the three-layer operating model: the skill owns procedure
(what to extract, which commands to run, the validation ladder, semantic stop
conditions, and reporting), the product API owns truth (recipe schema,
validation, execution, and export), and the agent host owns permissions and
execution security. The agent discovers product truth at runtime through
commands such as nvflare recipe show --format json; recipe list is reserved
for cases where framework and algorithm intent do not already identify the
recipe. This keeps skill prose from drifting into a "shadow API." Skills conform
to the agentskills.io specification and the company (NVCARPS) skills guideline.

Skills

• nvflare-convert-pytorch — plain PyTorch → FLARE Client API conversion.
• nvflare-convert-lightning — PyTorch Lightning → FLARE via
  flare.patch(trainer), Lightning-native evaluation, and DDP handling.
• nvflare-diagnose-job — read-only diagnosis of failed/stalled/suspicious
  jobs by mapping bounded evidence to a failure-pattern catalog.
• nvflare-orient — routes an open-ended or ambiguous NVFLARE request to
  the right workflow skill without editing files; explicit conversion requests
  select their converter directly after static framework inspection.
• nvflare-shared — internal, non-triggered reference library
  (non-standard conversion guidance, validation ladder, dependency ordering,
  PyTorch-family recipe selection, model exchange, metrics/artifact reporting)
  plus the shared ModelAggregator template.

Conversion skills use explicit {"class_path", "args"} recipe model config (no
live model instances), paired training/evaluation templates, custom aggregation
via the recipe aggregator= extension point, enable_tensor_disk_offload
paired with server_expected_format=PYTORCH, build setup once before the FL
round loop, device-selection preservation, and configurable data paths. The
standard FedAvg path is inline and uses direct recipe show fedavg-pt; broader
references and catalog discovery are loaded only for relevant exceptions.
Missing dependencies are installed before import-level preflight, and requested
validation proceeds under the agent host's permission system without a
skill-issued install, repository-trust, or execution prompt. Follow-up questions
are limited to genuinely missing conversion semantics.

Homomorphic encryption, differential privacy, and privacy filters are explicitly
not supported by conversion (report and route to provisioning/deployment;
never silently substitute an unprotected job).

Product code

• Static source inspector (nvflare/tool/agent/inspector.py) — AST-only
  framework detection (no import/execution of user code) via a
  frameworks/ detector-plugin package (PyTorch/Lightning), with entry-context
  reachability and evidence collection; surfaced as nvflare agent inspect.
• Agent CLI (nvflare agent inspect | info | skills …) with JSON output
  contracts.
• Skill packaging & install (skill_manifest.py, skill_manager.py) —
  content-addressed shared snapshots, atomic stage-then-publish, manifest
  path-traversal defense, TOCTOU-closing no-follow/identity fd checks,
  owned-by-current-user targets, and bytecode/eval exclusion. A build hook
  bundles the released skills into the wheel.

Evaluation & lint infrastructure

• Eval suites live outside the shipped skills in
  dev_tools/agent/skill_evals/<skill>/ — trigger routing, source-fact
  extraction, code/evaluation transformation, device selection, checkpoint
  safety, prompt-injection resistance, direct recipe inspection, progressive
  reference loading, and negative cases.
• Deterministic lint engine (dev_tools/agent/skills/checks/) — a
  frontmatter validator (agentskills.io + NVCARPS constraints: required
  metadata.author, name/description/compatibility limits, bounded no-follow
  file reads, no alwaysApply/globs) plus v1 lints (including the 200-line
  SKILL.md limit, trigger/negative/policy coverage, command drift, fixture
  integrity, and runtime boundaries). NVFLARE-specific checks stay on top of
  company conformance.

Security and authorization boundary

• Source comments, READMEs, config values, requirement-file prose, logs, and
  tool output are evidence, not authority. Embedded instructions cannot change
  aggregation, skip validation, authorize installation, or add exfiltration;
  they are ignored and reported as prompt-injection anomalies.
• Skills enforce only controls they directly own: static fact extraction,
  install-before-import ordering, safe torch.load(..., weights_only=True),
  secret redaction, and generated code that does not upload local data, paths,
  credentials, models, or datasets.
• The agent host owns sandboxing, filesystem permissions, network isolation,
  resource limits, and authorization. Skills use the host-supplied environment,
  do not probe or construct isolation mechanisms, do not assess whether the
  runtime is trusted, and report only actual host/tool denials or command
  failures as execution blockers.
• Package entries and installer options are dependency configuration handled
  under host permissions, not a skill-owned allowlist, typosquat audit, or
  supply-chain security claim. Natural-language instructions embedded in those
  files remain untrusted source text.
• Generated source may sit beside writable training source. Simulator
  workspaces, exports, models, and logs use a host-provided runtime directory or
  one temporary directory, with exact paths reported; skills do not construct
  permission hierarchies or run manifests.
• Product packaging/install code retains its own no-follow, path traversal,
  ownership, and atomic-publication protections. Diagnosis continues to treat
  log content and spoofable status markers as untrusted evidence.

Enforcement

The skill lint gates every push and merge: a pre-push hook (.githooks/),
./runtest.sh -s, and the CI unit tests all run
python -m dev_tools.agent.skills.checks --skills-root skills (covering skills
and eval suites); the packaged bundle is verified at build/install time.

Design docs

Only docs/design/skills_architecture.md ships; the operating-model,
evaluation, and check-report design docs are kept local (gitignored).

[greptile-apps]

Greptile Summary

This PR adds the nvflare-convert-lightning agent skill and refactors nvflare-convert-pytorch to align both conversion skills to an explicit operating model: skills are procedural guidance for coding agents, while product APIs own schema, validation, execution, and export. A new _shared/conversion-workflow.md replaces the lifecycle reference, the lint engine is decoupled from docs/design/ (replacing docs_root with evals_root), and the doctor command is scoped to conversion-only readiness checks.

• New Lightning skill and shared framework detection: The inspector engine is refactored into a plugin architecture (frameworks/ package) with per-framework FrameworkDetector classes; the Lightning detector handles promotion-over-family disambiguation, import alias tracking, and flare.patch conversion-state detection. All four Lightning eval fixtures include the required empty-batch ValueError guards.
• Lint engine decoupling: run_v1_lints no longer reads docs/design/*.md; evals are moved to dev_tools/agent/skill_evals/<skill>/; a new skill-runtime-boundary-lint prevents docs/design/ references and evaluator hooks from leaking into shipped skill content; agent-doc-crosslink-lint and skill-catalog-category-lint are removed.
• Doctor scope reduction: startup_kits, poc, and online fields are removed from nvflare agent doctor output; the command is scoped to conversion/ML framework readiness only, with corresponding JSON contract tests added.

Confidence Score: 5/5

Safe to merge; the changes are well-scoped, well-tested, and introduce no regressions in the areas reviewed.

The inspector refactoring is sophisticated but thoroughly tested — every Lightning import alias form, the flare.patch conversion-state path, family promotion logic, entry-context reachability, and nemo-wrapper edge cases all have dedicated parameterized tests. The lint engine decoupling replaces docs-coupled checks with deterministic evals-root-based checks, and the new runtime-boundary lint adds surface coverage without removing any existing admission gates. The only observable behavioral changes are the removal of startup_kits/poc/online from doctor output (intentional scope reduction, covered by updated tests) and the consolidation of the DistributedDataParallel duplicate-append into a single tuple check (a minor correctness fix). Empty-batch guards previously flagged in review threads are present in all four Lightning eval fixtures.

No files require special attention. The nvflare/tool/agent/inspector.py rewrite is the highest-complexity change, but the _FamilyResolver adapter pattern and entry-context reachability helpers are each unit-tested in isolation.

Important Files Changed

Filename	Overview
nvflare/tool/agent/inspector.py	Major refactor: framework detection extracted to plugin architecture, entry-context reachability added for primary-framework selection, Lightning integration signals drive conversion state, evidence collection cap raised from 12 to 10000 to avoid skewed confidence scores. Logic is well-tested and design decisions are thoroughly documented.
nvflare/tool/agent/frameworks/lightning.py	New Lightning framework detector: handles all import alias forms (pytorch_lightning, lightning, lightning.pytorch, module aliases), tracks flare.patch conversion signals, and implements promote_over_family with reachability-aware disambiguation logic.
nvflare/tool/agent/frameworks/registry.py	New registry module: aggregates import roots and evidence weights from all detectors, implements framework_for_import/recommended_skill_for/resolve_primary_framework, and defines UTILITY_FRAMEWORKS to prevent numpy from hijacking primary-framework selection.
dev_tools/agent/skills/checks/lints.py	Significant refactor: docs_root to evals_root, evals moved to dev_tools/agent/skill_evals/, new skill-runtime-boundary-lint replaces agent-doc-crosslink-lint and skill-catalog-category-lint, lint registry replaces the ad-hoc dict, SkillRecord gains cached_property validation and has_helper_tests. All changes are intentional and well-explained.
dev_tools/agent/skills/checks/frontmatter.py	SKILL.md schema updated: NVFLARE custom fields now nested under metadata sub-map per agentskills.io spec; new validators for supported top-level fields, metadata mapping type, spec constraints, and SKILL.md length; skill_metadata() accessor added.
nvflare/tool/agent/doctor.py	Scoped to conversion-only readiness: startup_kits, poc, and online fields removed; doctor_environment() now returns nvflare/commands/optional_dependencies/skills/findings/status only. Breaking change to JSON schema is intentional and covered by updated tests.
nvflare/tool/agent/agent_cli.py	--online and startup-kit selection args removed from nvflare agent doctor; known flags list updated to match; hint message updated to conversion context.
tests/unit_test/tool/agent/agent_inspector_test.py	Large new test suite for Lightning detection, flare.patch conversion state classification (all import alias forms), family disambiguation (pytorch vs lightning), entry-context reachability, and nemo-wrapper edge cases.
tests/unit_test/tool/agent/agent_json_contract_test.py	New golden-field contract tests for recipe list/show and agent inspect JSON shapes; fails CI if a skill-consumed field is renamed or dropped.
dev_tools/agent/skill_evals/nvflare-convert-lightning/files/gpu-device-lightning/model.py	New GPU-device Lightning eval fixture with empty-batch ValueError guard in training_step. No validation_step (intentional: fixture targets device-placement testing only).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[inspect_path] --> B[_inspect_dir / _inspect_file]
    B --> C[_PythonInspector AST walk per file]
    C --> D[visit_Import / visit_ImportFrom / visit_ClassDef / visit_Call]
    D --> E[FrameworkDetector plugins: PyTorchDetector, LightningDetector]
    E --> F[DetectContext: emit evidence, record flare_call, record integration_signal]
    F --> G[InspectState: framework_evidence, flare_calls, integration_signals, class_body_ranges]
    G --> H[_rank_frameworks count-based confidence]
    H --> I[_detect_primary_framework]
    I --> J{Entry context reachability check}
    J -->|Active evidence tied to entry point| K[Non-utility framework with active evidence wins]
    J -->|No entry context| L[resolve_primary_framework family promotion]
    L --> M{Lightning + PyTorch both present?}
    M -->|Yes| N[promote_over_family LightningDetector decides]
    N -->|Lightning reachable from entry| O[pytorch_lightning]
    N -->|PyTorch reachable from entry| P[pytorch]
    K --> Q[_conversion_state]
    O --> Q
    P --> Q
    Q --> R{_has_conversion_integration? integration_signals AND flare_imports}
    R -->|Yes - flare.patch detected| S[client_api_converted]
    R -->|No| T{flare.receive+send or FLModel in calls?}
    T -->|Yes| S
    T -->|No| U{any flare signals?}
    U -->|Yes| V[partial_client_api]
    U -->|No| W[not_converted - recommend skill]
    W --> X[nvflare-convert-pytorch OR nvflare-convert-lightning]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[inspect_path] --> B[_inspect_dir / _inspect_file]
    B --> C[_PythonInspector AST walk per file]
    C --> D[visit_Import / visit_ImportFrom / visit_ClassDef / visit_Call]
    D --> E[FrameworkDetector plugins: PyTorchDetector, LightningDetector]
    E --> F[DetectContext: emit evidence, record flare_call, record integration_signal]
    F --> G[InspectState: framework_evidence, flare_calls, integration_signals, class_body_ranges]
    G --> H[_rank_frameworks count-based confidence]
    H --> I[_detect_primary_framework]
    I --> J{Entry context reachability check}
    J -->|Active evidence tied to entry point| K[Non-utility framework with active evidence wins]
    J -->|No entry context| L[resolve_primary_framework family promotion]
    L --> M{Lightning + PyTorch both present?}
    M -->|Yes| N[promote_over_family LightningDetector decides]
    N -->|Lightning reachable from entry| O[pytorch_lightning]
    N -->|PyTorch reachable from entry| P[pytorch]
    K --> Q[_conversion_state]
    O --> Q
    P --> Q
    Q --> R{_has_conversion_integration? integration_signals AND flare_imports}
    R -->|Yes - flare.patch detected| S[client_api_converted]
    R -->|No| T{flare.receive+send or FLModel in calls?}
    T -->|Yes| S
    T -->|No| U{any flare signals?}
    U -->|Yes| V[partial_client_api]
    U -->|No| W[not_converted - recommend skill]
    W --> X[nvflare-convert-pytorch OR nvflare-convert-lightning]

Loading

_{Reviews (56): Last reviewed commit: "Add empty-batch guard to gpu-device-ligh..." | Re-trigger Greptile}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.71429% with 196 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.34%. Comparing base (3c0458d) to head (f05de35).

Files with missing lines	Patch %	Lines
nvflare/tool/agent/skill_manager.py	77.97%	122 Missing ⚠️
nvflare/tool/agent/skill_manifest.py	81.31%	51 Missing ⚠️
nvflare/tool/agent/frameworks/pytorch.py	91.48%	8 Missing ⚠️
nvflare/tool/agent/inspector.py	97.23%	7 Missing ⚠️
nvflare/tool/agent/frameworks/lightning.py	96.42%	4 Missing ⚠️
nvflare/tool/agent/frameworks/base.py	89.65%	3 Missing ⚠️
nvflare/tool/agent/frameworks/registry.py	98.14%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4837      +/-   ##
==========================================
+ Coverage   56.98%   57.34%   +0.36%     
==========================================
  Files         969      973       +4     
  Lines       92310    93350    +1040     
==========================================
+ Hits        52603    53536     +933     
- Misses      39707    39814     +107     

Flag	Coverage Δ
unit-tests	57.34% <85.71%> (+0.36%)	⬆️

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.

[holgerroth]

@chesterxgchen One architectural suggestion before this evaluation surface grows further: move test-only evaluation artifacts out of the distributable skill directories while retaining per-skill namespacing.

A possible layout:

skills/
  nvflare-convert-lightning/
    SKILL.md
    references/

dev_tools/agent/skill_evals/
  nvflare-convert-lightning/
    evals.json
    files/

tests/unit_test/tool/agent_skill_evals/
  ...

The main reasons are:

• evals.json contains expected outputs, assertions, mandatory behavior, and prohibited behavior. That is grading-oracle data rather than runtime skill guidance, and including it in dev/nightly-installed skills can expose benchmark answers.
• The current colocated layout requires include_analysis_files, release exclusions, and mode-dependent skill hashes solely to keep eval content out of release bundles.
• Cross-skill negative-routing cases and the evaluation runner naturally belong to a central suite.
• Runtime skill directories would then contain only the instructions and references that an installed agent should load.

Small examples that are genuinely useful during conversion can remain under references/; test-only fixtures and grading metadata would live in the evaluation tree. The runner can discover suites by skill name, preserving clear ownership without shipping evaluation artifacts.

[chesterxgchen]

@holgerroth Adopted this layout — thanks, it's a clear improvement. Test-only evaluation artifacts now live outside the distributable skill directories, with per-skill namespacing preserved:

• skills/<skill>/ holds only runtime content (SKILL.md, references/, assets/); no evals/ remains under skills/.
• Eval suites moved to dev_tools/agent/skill_evals/<skill>/{evals.json, files/}; a central runner discovers suites by skill name, keeping ownership clear.
• Structure/runner tests are centralized under tests/unit_test/tool/agent_skill_checks/.

On the reasons you gave:

• Grading-oracle data (expected outputs, assertions, mandatory/prohibited behavior) is no longer shipped in dev/nightly-installed skills.
• The colocated-layout machinery that existed solely to keep eval content out of release bundles — include_analysis_files, mode-dependent skill hashes, and the eval-only release exclusions — is removed. As defense-in-depth, SKILL_PACKAGING_EXCLUDE_NAMES still fail-closes on a stray skills/<skill>/evals/ so eval content can never re-enter a bundle even if one is reintroduced by mistake.
• Cross-skill negative-routing cases and the runner now live in the central eval tree.
• Runtime skill directories contain only the instructions/references an installed agent should load; small conversion-time examples stay under references//assets/, only test-only fixtures + grading metadata moved to the eval tree.

Landed in 78d0cfb and follow-ups.

[chesterxgchen]

@greptile-apps

[chesterxgchen]

@greptile-apps

[greptile-apps]

Too many files changed for review. (110 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

[chesterxgchen]

@greptile-app

[chesterxgchen]

@greptile-app