Modularize the agent inspector and pin routing behavior - #4974
Draft
chesterxgchen wants to merge 58 commits into
Draft
Modularize the agent inspector and pin routing behavior#4974chesterxgchen wants to merge 58 commits into
chesterxgchen wants to merge 58 commits into
Conversation
The agentskills.io spec allows six top-level SKILL.md fields and defines `metadata` as a map of string keys to string values. `skills/README.md` documents the same six, but eight skills declared `version` at the top level and used YAML lists for `tags`/`languages`/`frameworks`, so the published frontmatter did not match the convention the repo documents. `nvflare-convert-huggingface` was already conformant; the rest now match it. Nothing enforces this: the lint permits top-level `version` as an explicit NVIDIA-skills-catalog carve-out and never checks metadata value types, so the two forms could coexist indefinitely. `min_flare_version` moves to 2.9.0 across all skills because 2.9.0 is the first FLARE release that ships agent skills; the 2.8.0 values predate that decision. The string form also matters for the 200-line SKILL.md limit: list-form metadata costs a dozen lines in files already sitting at exactly 200.
Static routing analysis fails silently: a wrong answer is a plausible recommendation, not a crash. A file-level simplification of the lexical identity model looked correct and regressed 35 routing cases, so pin the behavior before any future consolidation is attempted. Adds 35 unconverted source cases covering baseline ownership, mixed Lightning/Hugging Face projects, trainer identity across global, nonlocal, and closure scopes, assignment and unpacking provenance, every flare.patch call form, nine shadowing cases that must not yield an owner, and evaluation-only, unused-trainer, and inference-only suppression. The contract compares the full normalized envelope against a golden generated from the parent commit, and pins the normalizer's own digest and exclusion list so the gate cannot be weakened by narrowing what it inspects. Golden generation strips editable-install finders and verifies the resolved package, so it cannot be silently produced from the working tree. Also deduplicates the redaction helpers the module split had forked into two copies, inlines a single-use scoring pass-through, and drops an unreachable branch in installed-skill discovery.
chesterxgchen
force-pushed
the
agent-inspector-modularization
branch
from
July 28, 2026 05:58
1363de9 to
8340803
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4974 +/- ##
==========================================
+ Coverage 64.28% 64.53% +0.25%
==========================================
Files 1030 1037 +7
Lines 103733 104450 +717
==========================================
+ Hits 66681 67411 +730
+ Misses 37052 37039 -13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 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
Modularize the agent inspector that selects conversion skills, and add an
executable characterization suite that pins its routing behavior.
Depends on #4967. That PR adds the Hugging Face conversion skill and the
framework detection this refactor reorganizes. Until it merges, the diff here
also shows its commits.
Production code is a net reduction. No routing behavior changes: inspector
output is byte-identical to the pre-refactor commit across the whole
characterization corpus.
What Changed
Inspector modularization
routing decision so the two outputs cannot silently disagree.
inspection facts and narrower project/routing projections.
inspection.
nvflare/tool/agent/inspector.pyfrom 2,543 lines to a 198-lineorchestrator, moving cohesive responsibilities into seven modules under
nvflare/tool/agent/inspection/(models, scanner, python_scanner, project,routing, skill_discovery).
for project analysis, routing, scanning, and skill discovery.
copies, inline a single-use scoring pass-through, and drop an unreachable
branch in installed-skill discovery.
Behavior contract suite
Static routing analysis fails silently: a wrong answer is a plausible-looking
recommendation, not a crash. This adds an executable gate so future
consolidation can be attempted safely.
tests/unit_test/tool/agent/fixtures/inspection_behavior/— 35 unconvertedsource cases covering baseline ownership, mixed Lightning/Hugging Face
projects, trainer identity across
global/nonlocal/closure scopes,assignment and unpacking provenance, every
flare.patchcall form, nineshadowing cases that must not produce an owner, and evaluation-only,
unused-trainer, and inference-only suppression cases.
cases.jsondeclares the expected framework, conversion state, ownership,and recommended skills per case.
expected.jsonis a normalized full-envelope golden generated from theparent commit and treated as immutable.
inspection_behavior_contract_test.pycompares the full envelope, pins thenormalizer's exclusion list, and asserts the normalizer's own digest so the
gate cannot be weakened by narrowing what it inspects.
inspection_behavior_normalizer.py --source-rootstrips editable-installfinders and verifies the resolved package, so the golden cannot be silently
generated from the working tree instead of the reference.
lexical_scope_bindings_test.pycoverscandidate_scopes(),binding_scope(), enclosing-scope resolution, class-scope skipping,global/nonlocal, and identity invalidation after rebinding.Why The Lexical Analysis Stays
Per-file import matching is not sufficient to choose a converter. A project can
import
transformersfor a model while Lightning owns the training loop, rebinda trainer name after construction, or build the trainer in a
global,nonlocal, or closure scope. Replacing lexical identity resolution,source-order handling, and assignment RHS provenance with file-level matching
changes observable output in 35 covered cases: converted jobs report
partial_client_apiinstead ofclient_api_converted, Hugging Facerecommendations are lost or redirected to
nvflare-orient, andPyTorch/Lightning evidence disappears from class bodies, definition-time
expressions, and unpacking targets. The corpus above pins each of those.
Non-Goals
ownership result is unchanged.
They contain similar-looking import, alias, pending-call, and finalization
machinery, but
patch()has exactly two entry points in the codebase(
app_opt/lightning/api.py,app_opt/hf/api.py) and no third can join —manual PyTorch has no Trainer to wrap. Consolidating two closed members adds
indirection without a payoff.
Validation
python -m pytest tests/unit_test/tool/agent tests/unit_test/tool/agent_skill_checks -qexpected.jsonregenerated from the pre-refactor commit in adetached worktree matches the checked-in golden byte for byte.
adding a field to the normalizer's exclusion list fails both the semantic and
golden tests.
./runtest.sh -sand./runtest.sh -l