feat(studio): make the example agent a Fabric one - #1229
Closed
marcusds wants to merge 1 commit into
Closed
Conversation
Create Example Agent offered one sample, and it was NAT. nemo-agents-spec-v1 is the format nemo-build-agent produces by default, so the one agent a user could create from Studio was in the legacy format, and no Studio path exercised the current one. Point the sample at the email-phishing agent the Fabric example ships. The loader already branches on config_format and writes models.default.model for nemo-agents-spec-v1, so only the registry entry and the static asset were missing — the unused configFormat field on SampleAgent was left for exactly this. The asset is a byte-identical copy of the plugin example rather than an edited variant, and a test asserts that, so the two cannot drift silently. It keeps the example's telemetry block, which posts ATIF to 127.0.0.1:8080; that is correct for a local platform and wrong for a remote one, and is the reason to keep the copy honest rather than quietly patch it here. The email-security-analyst directory stays: EVAL_CONFIG_SAMPLES serves both eval configs, their READMEs, and the dataset from it. Only the now-unreferenced agent.yml is removed. The invariant comment gains the Fabric case. A NAT entry needs its custom _type installed in the deploy venv; a Fabric entry resolves its stdio MCP url as a console script on PATH, which email-phishing-iocs provides as a workspace member. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
studio-fabric-example-agent/mschwab
branch
from
August 11, 2026 00:21
f292a59 to
ddf44c7
Compare
Contributor
|
marcusds
marked this pull request as ready for review
August 11, 2026 00:25
Contributor
📝 WalkthroughWalkthroughChangesThe Studio sample-agent catalog now replaces the NAT email security analyst with a Fabric-based email phishing analyzer. The new asset defines DeepAgents orchestration, optional IOC extraction, YAML verdict output, and relay telemetry. Tests validate the asset and registry metadata. Email phishing agent migration
Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant PhishingAnalyzer
participant ExtractIOCs
participant RelayTelemetry
Orchestrator->>PhishingAnalyzer: delegate email verdict generation
PhishingAnalyzer->>ExtractIOCs: optionally extract IOCs
PhishingAnalyzer-->>Orchestrator: return YAML verdict
Orchestrator->>RelayTelemetry: emit ATIF/ATOF output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/constants/sampleAgents.test.ts`:
- Around line 21-27: Update the test around
getSampleAgent('email_phishing_agent') to assert that sample.key equals
'email_phishing_agent' before validating the Fabric configuration, ensuring the
test does not pass through the unknown-key fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d8bd3803-3c86-4fff-b3c4-bf62f443d473
📒 Files selected for processing (4)
web/packages/studio/public/sample-agents/email-phishing-agent/agent.yamlweb/packages/studio/public/sample-agents/email-security-analyst/agent.ymlweb/packages/studio/src/constants/sampleAgents.test.tsweb/packages/studio/src/constants/sampleAgents.ts
💤 Files with no reviewable changes (1)
- web/packages/studio/public/sample-agents/email-security-analyst/agent.yml
Comment on lines
+21
to
+27
| it('the Fabric sample exposes the model slot loadSampleAgentConfig writes to', () => { | ||
| const sample = getSampleAgent('email_phishing_agent'); | ||
| const text = readFileSync(join(PUBLIC_DIR, sample.agentConfigPath), 'utf8'); | ||
| const config = YAML.parse(text) as { models?: { default?: { model?: string } } }; | ||
|
|
||
| expect(sample.configFormat).toBe('nemo-agents-spec-v1'); | ||
| expect(config.models?.default).toBeDefined(); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the selected sample key.
getSampleAgent falls back to SAMPLE_AGENTS[0] for an unknown key. Assert sample.key === 'email_phishing_agent' before validating the Fabric configuration.
Proposed fix
const sample = getSampleAgent('email_phishing_agent');
+ expect(sample.key).toBe('email_phishing_agent');
const text = readFileSync(join(PUBLIC_DIR, sample.agentConfigPath), 'utf8');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('the Fabric sample exposes the model slot loadSampleAgentConfig writes to', () => { | |
| const sample = getSampleAgent('email_phishing_agent'); | |
| const text = readFileSync(join(PUBLIC_DIR, sample.agentConfigPath), 'utf8'); | |
| const config = YAML.parse(text) as { models?: { default?: { model?: string } } }; | |
| expect(sample.configFormat).toBe('nemo-agents-spec-v1'); | |
| expect(config.models?.default).toBeDefined(); | |
| it('the Fabric sample exposes the model slot loadSampleAgentConfig writes to', () => { | |
| const sample = getSampleAgent('email_phishing_agent'); | |
| expect(sample.key).toBe('email_phishing_agent'); | |
| const text = readFileSync(join(PUBLIC_DIR, sample.agentConfigPath), 'utf8'); | |
| const config = YAML.parse(text) as { models?: { default?: { model?: string } } }; | |
| expect(sample.configFormat).toBe('nemo-agents-spec-v1'); | |
| expect(config.models?.default).toBeDefined(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/packages/studio/src/constants/sampleAgents.test.ts` around lines 21 - 27,
Update the test around getSampleAgent('email_phishing_agent') to assert that
sample.key equals 'email_phishing_agent' before validating the Fabric
configuration, ensuring the test does not pass through the unknown-key fallback.
This was referenced Aug 11, 2026
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
1 participant
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.
Summary
Create Example Agent offered one sample and it was NAT.
nemo-agents-spec-v1is the formatnemo-build-agentproduces by default, so the only agent a user could create from Studio was in the legacy format and no Studio path exercised the current one. This points the sample at the Fabric email-phishing agent added in #1117.The loader already branches on
config_formatand writesmodels.default.modelfornemo-agents-spec-v1, so only the registry entry and the static asset were missing — the unusedconfigFormatfield onSampleAgentwas left for exactly this.Related Issue
No tracking issue. Depends on #1219 — see Limitations.
Changes
SAMPLE_AGENTSnow holds the Fabric email-phishing agent (configFormat: 'nemo-agents-spec-v1') in place of the NAT email-security-analyst entry.public/sample-agents/email-phishing-agent/agent.yamlas a byte-identical copy ofplugins/nemo-agents/examples/nemo-agent-config/email-phishing-agent/agent.yaml(verified: same sha256), with a test asserting the two stay identical so they cannot drift silently.public/sample-agents/email-security-analyst/agent.yml. The directory itself stays:EVAL_CONFIG_SAMPLESserves both eval configs, their READMEs, anddataset.jsonlfrom it._typeinstalled in the deploy venv; a Fabric entry resolves its stdio MCPurlas a console script on PATH, whichemail-phishing-iocsprovides as a workspace member.config_formatthe entry claims, the Fabric sample exposes themodels.defaultslot the loader writes to, and generated sample names are still recognised byisSampleAgentName.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
pnpm --filter nemo-studio-ui typecheck— passed.pnpm --filter nemo-studio-ui test— 315 files / 2857 tests, with intermittent 10s hook timeouts traced to machine load (load average 14 with an antivirus daemon saturating a core), not to this change. Every affected file passes on retry; the failures land in the global MSWbeforeAllinvitest.setup.tsxor in a lazy-loadedCodeEditor, neither touched here.npx eslint --fix --max-warnings 0 src/constants— clean.uv run pre-commit run -a— every hook passed exceptRun uv lock with platform uv, which fails on a local toolchain mismatch:script/uv-lock.shrequires uv exactly 0.9.14 and this machine has 0.9.30. This PR changes no Python and no dependency metadata, the separateCheck for uv.lock drifthook passed, and the run left the worktree unmodified. Not marked as passing above.Limitations
Ordering. This should not merge before #1219. On its own it makes Studio's only example agent a Fabric one, and a Fabric agent cannot be evaluated from Studio until #1219 lands — the eval target is hardcoded to the NAT
/generate, which the Fabric server does not serve. Merged first, this turns a latent bug into the default experience.Not verified end to end. The sample has not been created through Studio and deployed. Its stdio MCP
urlresolvesemail-phishing-iocson PATH, whichuv sync --all-packagesprovides in a dev checkout as a workspace member, but a deployed run was not exercised.Telemetry endpoint. The asset keeps the example's
telemetry.atif.storageblock, which posts ATIF tohttp://127.0.0.1:8080/apis/intake/.... That is correct for the defaultsubprocessdeployment, where the agent runs as a child process on the platform host, and wrong for thedocker/k8smodes the deployment modal also offers, where127.0.0.1is the container. The port is likewise assumed. Left as-is deliberately: patching the Studio copy is what would make it drift from the plugin copy. The durable fix is to rewrite host-less ATIF endpoints at deploy time the wayinject_fabric_gateway_urlalready rewrites modelbase_urls, which belongs on the backend rather than here.Summary by CodeRabbit
New Features
Changes