feat(skills): add nemo-analyst workflow - #1116
Merged
Merged
Conversation
Contributor
|
callingmedic911
force-pushed
the
aditya/ase-745-analyst-skill-b080
branch
from
August 6, 2026 22:06
519f9f1 to
e463b0b
Compare
callingmedic911
added a commit
that referenced
this pull request
Aug 7, 2026
Review feedback on #1116. #1159 moves the optimization agents onto the models chosen in `nemo setup` and drops `INFERENCE_API_KEY` as an agent credential, so naming that variable, the gateway, and Claude Opus 4.8 would go stale the day it lands. State the requirement instead — the Analyst needs a model to run on — and let `analyst doctor` be the authority on whether it is satisfied, which is true before and after that change. Also from review: - Frame the skill as analyzing behavior and producing Insights rather than "finding what an agent keeps getting wrong". - Describe `--agent-spec` by what it gives the Analyst: the intent behind the agent, which is not recoverable from code or traces. Per the AGENT-SPEC design doc, that is its whole job. (The proposed ETHOS.md rename has not landed, so the filename stays.) - Drop `--base-url` from the run example; it defaults to `NMP_BASE_URL`. The wider `NMP_*` rename is ASE-815 — 414 files reference those variables, so it is not a skill-PR change. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
Add the platform-bundled nemo-analyst skill so coding agents have a reliable route to `nemo agents analyst run` and the minimum correct operating knowledge to use it. The body stays lean and defers depth to docs/agents/insight-driven-optimization.mdx rather than restating it. It covers the Insight shape, the credential distinction from the Experimentalist (the Analyst reads only INFERENCE_API_KEY), the doctor pre-flight, the run, where Insights are stored, and a verification step. Persistence is documented as it behaves after #1054: Insights always go to the platform, and --insights-file-output mirrors what was stored. Routing tests follow the explicit/implicit/negative-control convention already used by nemo-explore. Verified that all six analyst-intent prompts resolve to nemo-analyst, that the skill introduces no new failures elsewhere in the catalog, and that it wins no ties by sort order. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Aditya Pandey <aditya@autospace.co> Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
Drove the skill end to end in a container against a real LangGraph agent's telemetry. It carried the workflow, so this sharpens what it left the driver to work out rather than adding to it — net zero lines, paid for by cutting redundancy from the storage and verify sections. - `analyst doctor` takes no `--agent` flag, so it always reports a red profile line. Say that passing `--agent` makes that line noise instead of leaving the reader to reason past a red mark, or to write an `optimizer.yaml` it does not need. - `agent_name` is carried on agent-level spans, not on their model and tool children, so the old "matches the `agent_name` on the spans" pointed at a query that undercounts. - Every span came back `status=success` while the agent was badly broken, so state that the Analyst judges behavior rather than status or scores. - Telemetry that captures only the shape of a run is a third cause of an empty result, alongside scoping and volume. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
Review feedback on #1116. #1159 moves the optimization agents onto the models chosen in `nemo setup` and drops `INFERENCE_API_KEY` as an agent credential, so naming that variable, the gateway, and Claude Opus 4.8 would go stale the day it lands. State the requirement instead — the Analyst needs a model to run on — and let `analyst doctor` be the authority on whether it is satisfied, which is true before and after that change. Also from review: - Frame the skill as analyzing behavior and producing Insights rather than "finding what an agent keeps getting wrong". - Describe `--agent-spec` by what it gives the Analyst: the intent behind the agent, which is not recoverable from code or traces. Per the AGENT-SPEC design doc, that is its whole job. (The proposed ETHOS.md rename has not landed, so the filename stays.) - Drop `--base-url` from the run example; it defaults to `NMP_BASE_URL`. The wider `NMP_*` rename is ASE-815 — 414 files reference those variables, so it is not a skill-PR change. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
The previous commit wrote the credential story as #1159 will leave it, but that PR has not landed: the analyst still reads INFERENCE_API_KEY directly today, so "rather than any credential of its own" was false in the world this ships into. Say only that the model configuration is in flux and that pre-flight names what is missing — accurate before and after. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
`lint-sdk-vendored` and `lint-cli` both fail on this branch because every skill is mirrored into `sdk/python/nemo-platform` and this one never was — it was the only skill missing from the vendored tree. Generated with `make vendor`; as with every other skill, `tests.json` is not vendored. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
Follow the pattern #1088 settled on. That PR started with the Experimentalist skill in the platform package and deliberately moved it into the plugin; this one was written against the earlier shape. The skill documents `nemo agents analyst`, which only exists when nemo-insights is installed, so the platform package should not ship it. Moving it also removes it from the vendored SDK tree — plugin-owned skills are discovered through the `nemo.skills` entry point instead, which is what made lint-sdk-vendored care about it in the first place. Mirrors the Experimentalist layout: skills.py exposing skills_dir, a skills README, and an entry-point test. `nemo skills list` now reports the skill with source nemo-insights-plugin. Also adds the nemo-analyst row to nemo-skill-selection, as #1088 did for the Experimentalist. The catalog already told agents to use the Experimentalist "after `agents analyst` has created an Insight" while having no row that routed there. The description said "Produces the Insight that nemo-experimentalist consumes". Now that both skills are in the catalog together, that literal token made this skill win the Experimentalist's own explicit routing test — scoring treats a hyphenated name as one token. Reworded to "the Experimentalist", which keeps the loop context without the collision. Catalog-wide: 47 failures both before and after, 12 more passes, and nemo-analyst involved in no failure. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
callingmedic911
force-pushed
the
aditya/ase-745-analyst-skill-b080
branch
from
August 7, 2026 19:56
e133e68 to
e9e96ef
Compare
Completes the #1088 parallel: that PR appended the Experimentalist to the plugin-owned skills sentence in both files once its skill moved into the plugin. The Analyst now lives there too. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
Member
Author
|
/nvskills-ci |
callingmedic911
marked this pull request as ready for review
August 7, 2026 20:13
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds the ChangesAnalyst skill integration
Sequence Diagram(s)sequenceDiagram
participant NemoSkillSelection
participant InsightsEntryPoint
participant SkillsDirectory
participant NemoAnalystSkill
NemoSkillSelection->>InsightsEntryPoint: Discover the insights nemo.skills entry point
InsightsEntryPoint->>SkillsDirectory: Call skills_dir()
SkillsDirectory-->>NemoSkillSelection: Return the packaged skills Path
NemoSkillSelection->>NemoAnalystSkill: Load nemo-analyst/SKILL.md
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 3
🧹 Nitpick comments (1)
plugins/nemo-insights/tests/test_skills_entry_point.py (1)
20-30: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the loaded entry point's target.
The test only proves that
insightsloads a callable that returns some directory. It does not prove that the callable isnemo_insights_plugin.skills.skills_diror that the loaded directory contains the new skill. Compareloadedandresultwithskills_dir()and assert thatSKILL.mdexists.Suggested assertion
from importlib.metadata import entry_points + from nemo_insights_plugin.skills import skills_dir eps = [ep for ep in entry_points(group="nemo.skills") if ep.name == "insights"] assert len(eps) == 1, f"Expected exactly one 'insights' entry-point, got {eps}" loaded = eps[0].load() + assert loaded is skills_dir assert callable(loaded), f"Entry-point did not resolve to a callable: {loaded!r}" result = loaded() + assert result == skills_dir() assert isinstance(result, Path), f"skills_dir() returned {result!r} (not Path)" assert result.is_dir(), f"skills_dir() returned {result!r} which is not a directory" + assert (result / "nemo-analyst" / "SKILL.md").is_file()🤖 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 `@plugins/nemo-insights/tests/test_skills_entry_point.py` around lines 20 - 30, Strengthen test_entry_point_loads_skills_dir by importing the canonical skills_dir symbol, asserting the loaded entry-point callable matches it, and comparing the returned result with skills_dir(). Also assert that the returned directory contains the new skill’s SKILL.md file while preserving the existing entry-point and directory validations.
🤖 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 `@plugins/nemo-insights/src/nemo_insights_plugin/skills/nemo-analyst/SKILL.md`:
- Line 18: Update the routing trigger in SKILL.md by replacing the incomplete
phrase “my agent keeps getting wrong” with “my agent keeps getting things
wrong,” or remove the trigger entirely if it is not needed.
- Around line 127-129: Update the curl example in the nemo analyst skill to
avoid expanding the bearer token in command arguments: obtain authentication
through a protected header file or stdin-based mechanism, and keep the
unauthenticated request command separate from token handling while preserving
the existing insights endpoint and query parameters.
- Around line 132-136: Update the analyst-run validation to inspect the current
AnalystResult or its operation-specific create/update marker, rather than
relying on the stored ?agent=... Insight listing. Treat “no insights created or
updated” as a valid successful run, while still validating any newly created or
updated Insights for clear titles, actionable descriptions, non-empty
trace_refs, and Studio optimizer visibility.
---
Nitpick comments:
In `@plugins/nemo-insights/tests/test_skills_entry_point.py`:
- Around line 20-30: Strengthen test_entry_point_loads_skills_dir by importing
the canonical skills_dir symbol, asserting the loaded entry-point callable
matches it, and comparing the returned result with skills_dir(). Also assert
that the returned directory contains the new skill’s SKILL.md file while
preserving the existing entry-point and directory validations.
🪄 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: 3ec17f55-a813-44fc-bacb-5925e763f5a9
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/skills/nemo-skill-selection/SKILL.mdis excluded by!sdk/**
📒 Files selected for processing (9)
AGENTS.mdCLAUDE.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-skill-selection/SKILL.mdplugins/nemo-insights/pyproject.tomlplugins/nemo-insights/src/nemo_insights_plugin/skills.pyplugins/nemo-insights/src/nemo_insights_plugin/skills/README.mdplugins/nemo-insights/src/nemo_insights_plugin/skills/nemo-analyst/SKILL.mdplugins/nemo-insights/src/nemo_insights_plugin/skills/nemo-analyst/tests.jsonplugins/nemo-insights/tests/test_skills_entry_point.py
CodeRabbit review on #1116. The Verify section contradicted the skill's own premise: it said a successful run leaves at least one Insight, while "What it produces" says filing nothing is a valid outcome. It also verified the wrong thing — listing by `?agent=` returns every Insight for the agent, including earlier runs', so it attests the store rather than this run. Verify against what the run reports instead. `analyst_backend` emits a line per operation (`- created: <title> [<id>]`, `- updated: <id>`, or `- no insights created or updated`), so read those ids back individually via `GET .../insights/<insight-id>`, and treat "no insights created or updated" as the success it is. Two smaller fixes from the same review: - Keep the bearer token out of `curl`'s argv, where any process on the host can read it, by passing it through curl's config on stdin. Verified the header still arrives. - The trigger `my agent keeps getting wrong` was missing its object; now `my agent keeps getting things wrong`. Routing is unchanged against a clean main baseline — 47 failures both sides, and nemo-analyst still fails none and steals none. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
aleckhoury
approved these changes
Aug 7, 2026
aleckhoury
left a comment
aleckhoury
left a comment
Contributor
There was a problem hiding this comment.
Just a couple questions, not blocking
Review feedback on #1116. Neither file was pulling its weight. skills/README.md is never read — the registry only looks at subdirectories containing a SKILL.md — and with a single skill in the directory it restated that skill's own frontmatter. The Experimentalist's equivalent earns its place by disambiguating a user-facing skill from an internal one; this one had nothing to disambiguate. test_skills_entry_point.py guarded the `nemo.skills` declaration in pyproject.toml. Without that entry the skill simply never appears in `nemo skills list`, so the failure announces itself the first time anyone uses the feature and does not need a regression test of its own. Verified by hand instead: the entry point resolves and `nemo skills list` reports nemo-analyst with source nemo-insights-plugin. Signed-off-by: Aditya Pandey <aditypandey@nvidia.com>
Member
Author
|
/nvskills-ci |
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.
Adds the platform-bundled
nemo-analystskill (ASE-745), so coding agents have a reliable route tonemo agents analyst runplus the minimum correct operating knowledge to use it.Closes ASE-745.
Summary by CodeRabbit
New Features
Tests