feat(agents): typed AgentSpec + explore-first onboarding chain (FP-161)#94
Open
aleckhoury wants to merge 16 commits into
Open
feat(agents): typed AgentSpec + explore-first onboarding chain (FP-161)#94aleckhoury wants to merge 16 commits into
aleckhoury wants to merge 16 commits into
Conversation
6 tasks
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements a canonical agent specification system: Pydantic schema with job/categories validation; markdown render/parse for bidirectional conversion; convention-based spec location helpers; and three integrated skill docs (nemo-spec, nemo-explore, nemo-build-agent) coordinating spec lifecycle, codebase exploration, and agent deployment with hard preconditions and Fileset-as-canonical storage. ChangesAgent Spec System
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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
`@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.md`:
- Around line 47-50: The pre-flight check strips the workspace by using
${SPEC_FILE_REF#*/}, so the command nemo files filesets get only checks the
fileset name against the wrong workspace; fix it by preserving and passing the
full reference instead of just the part after the slash—either pass
"${SPEC_FILE_REF}" directly to nemo files filesets get or explicitly split into
workspace="${SPEC_FILE_REF%%/*}" and fileset="${SPEC_FILE_REF#*/}" and call nemo
files filesets get "${workspace}/${fileset}" so the check validates the intended
workspace and fileset together.
In
`@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.md`:
- Around line 55-57: The template contains free-form guidance lines inside
strict list sections which breaks parse_spec()—move any explanatory sentences
out of the "Constraints", "Open Questions", and "Allowed Changes" sections so
those sections contain only the allowed formats: for "Constraints" and "Open
Questions" only bullet items or exactly `_(none)_`, and for "Allowed Changes"
only labeled bullets; update the agent-spec.md template by removing or
relocating the guidance lines currently inside those headings (lines referenced
around the "Constraints", "Open Questions", and "Allowed Changes" sections) into
a preceding or trailing paragraph so parse_spec() can parse those sections
cleanly.
In `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.md`:
- Line 3: The file uses two different canonical filenames (AGENTSpec.md and
AGENT_SPEC.md); choose AGENT_SPEC.md as the canonical name and replace all
occurrences of AGENTSpec.md, AGENTSpec, and any variants (e.g.,
agents/<name>.spec.md mentions that imply the other form) with AGENT_SPEC.md so
the description, the upload/verification sections, and the examples/usage blocks
(the parts that reference the AgentSpec schema, upload, and fileset steps)
consistently reference AGENT_SPEC.md.
- Around line 128-135: The SPEC_FILE_REF assignment always hardcodes "default"
and ignores any overridden workspace; update the logic that assigns
SPEC_FILE_REF (currently set to "default/${NAME}-spec") to use the actual
workspace variable (e.g., ${WORKSPACE} or the variable used elsewhere in this
file) so it becomes "${WORKSPACE}/${NAME}-spec" (falling back to "default" only
if workspace is unset), and ensure subsequent uses of SPEC_FILE_REF still
reference that variable.
In `@plugins/nemo-agents/src/nemo_agents_plugin/spec.py`:
- Line 185: Replace the Unicode EN DASH (U+2013) with an ASCII hyphen in the
description string so it reads "3-6 task buckets the agent handles."; update the
description value in plugins/nemo-agents/src/nemo_agents_plugin/spec.py where
the description parameter is defined to eliminate the en-dash and use a plain
hyphen.
- Around line 231-240: The _validate_job method currently strips whitespace but
does not re-check length, letting whitespace-padded short strings bypass
_MIN_JOB_LENGTH; update nemo_agents_plugin.spec._validate_job to, after
computing stripped = value.strip(), verify len(stripped) >= _MIN_JOB_LENGTH (or
use StringConstraints(strip_whitespace=True, min_length=_MIN_JOB_LENGTH) on the
job field) and raise a ValueError with a clear message (e.g., "'job' is too
short after trimming") if it fails; keep the existing vague-phrase check against
_VAGUE_JOB_PHRASES and return stripped when valid.
In `@plugins/nemo-agents/tests/unit/test_spec_render.py`:
- Line 151: The pytest assertion using pytest.raises(SpecRenderError,
match="missing section.*Audience") uses a normal string with regex
metacharacters; change the match argument to a raw string (e.g., match=r"missing
section.*Audience") to avoid RUF043 and ensure the regex is interpreted
correctly in the pytest.raises call that references SpecRenderError.
🪄 Autofix (Beta)
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: 75f72587-011d-4b89-a782-12a3dc39c53c
⛔ Files ignored due to path filters (4)
sdk/python/nemo-platform/src/nemo_platform/skills/nemo-build-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-explore/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/references/templates/agent-spec.mdis excluded by!sdk/**
📒 Files selected for processing (15)
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-explore/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.mdplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.pyplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/entities.pyplugins/nemo-agents/src/nemo_agents_plugin/schema.pyplugins/nemo-agents/src/nemo_agents_plugin/spec.pyplugins/nemo-agents/src/nemo_agents_plugin/spec_render.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_entities.pyplugins/nemo-agents/tests/unit/test_spec.pyplugins/nemo-agents/tests/unit/test_spec_render.py
Contributor
|
nicot
reviewed
May 28, 2026
nicot
reviewed
May 28, 2026
aleckhoury
added a commit
that referenced
this pull request
Jun 1, 2026
Address PR #94 review feedback. The spec_file_ref field on Agent (and the matching --spec-file-ref CLI flag, API request field, and SPEC_FILE_REF shell ceremony in the skills) is redundant under the project's stated constraints: spec and agent always live in the same workspace, and the spec filename is the industry-standard AGENTSpec.md. The location is fully derivable from (workspace, agent_name). - Remove spec_file_ref from Agent entity, CreateAgentRequest schema, POST /agents handler, --spec-file-ref CLI option, and openapi.yaml. - Add canonical helpers in nemo_agents_plugin.entities: AGENT_SPEC_FILENAME, agent_spec_fileset_name, agent_spec_file_ref. Downstream consumers (analyst, Studio, optimization loop) call these rather than reconstructing the path inline; layout changes are then a one-function migration. - Canonicalize the filename to AGENTSpec.md across nemo-spec and nemo-build-agent skills and template; drop the SPEC_FILE_REF shell variable and the --spec-file-ref flag from skill bash blocks. - Mirror skill changes to the vendored sdk/python copies. Also picks up CodeRabbit review fixes: - spec.py: re-enforce _MIN_JOB_LENGTH after strip (Pydantic min_length runs before @field_validator), with vague-phrase check first so the more specific diagnosis wins. Replace EN DASH with hyphen for RUF001. - test_spec_render.py: raw regex string for RUF043. - agent-spec.md template: move parse-rule guidance out of the strict Constraints / Allowed Changes / Open Questions sections into the top-level prose block. Test suite trimmed from 80 to 62 tests by removing assertions that re-state Field declarations or test Pydantic itself. Remaining tests lock validators, the markdown round-trip, parser invariants, and the new convention helpers.
aleckhoury
added a commit
that referenced
this pull request
Jun 1, 2026
Address PR #94 review feedback. The spec_file_ref field on Agent (and the matching --spec-file-ref CLI flag, API request field, and SPEC_FILE_REF shell ceremony in the skills) is redundant under the project's stated constraints: spec and agent always live in the same workspace, and the spec filename is the industry-standard AGENTSpec.md. The location is fully derivable from (workspace, agent_name). - Remove spec_file_ref from Agent entity, CreateAgentRequest schema, POST /agents handler, --spec-file-ref CLI option, and openapi.yaml. - Add canonical helpers in nemo_agents_plugin.entities: AGENT_SPEC_FILENAME, agent_spec_fileset_name, agent_spec_file_ref. Downstream consumers (analyst, Studio, optimization loop) call these rather than reconstructing the path inline; layout changes are then a one-function migration. - Canonicalize the filename to AGENTSpec.md across nemo-spec and nemo-build-agent skills and template; drop the SPEC_FILE_REF shell variable and the --spec-file-ref flag from skill bash blocks. - Mirror skill changes to the vendored sdk/python copies. Also picks up CodeRabbit review fixes: - spec.py: re-enforce _MIN_JOB_LENGTH after strip (Pydantic min_length runs before @field_validator), with vague-phrase check first so the more specific diagnosis wins. Replace EN DASH with hyphen for RUF001. - test_spec_render.py: raw regex string for RUF043. - agent-spec.md template: move parse-rule guidance out of the strict Constraints / Allowed Changes / Open Questions sections into the top-level prose block. Test suite trimmed from 80 to 62 tests by removing assertions that re-state Field declarations or test Pydantic itself. Remaining tests lock validators, the markdown round-trip, parser invariants, and the new convention helpers.
aleckhoury
added a commit
that referenced
this pull request
Jun 1, 2026
Address PR #94 review feedback. The spec_file_ref field on Agent (and the matching --spec-file-ref CLI flag, API request field, and SPEC_FILE_REF shell ceremony in the skills) is redundant under the project's stated constraints: spec and agent always live in the same workspace, and the spec filename is the industry-standard AGENTSpec.md. The location is fully derivable from (workspace, agent_name). - Remove spec_file_ref from Agent entity, CreateAgentRequest schema, POST /agents handler, --spec-file-ref CLI option, and openapi.yaml. - Add canonical helpers in nemo_agents_plugin.entities: AGENT_SPEC_FILENAME, agent_spec_fileset_name, agent_spec_file_ref. Downstream consumers (analyst, Studio, optimization loop) call these rather than reconstructing the path inline; layout changes are then a one-function migration. - Canonicalize the filename to AGENTSpec.md across nemo-spec and nemo-build-agent skills and template; drop the SPEC_FILE_REF shell variable and the --spec-file-ref flag from skill bash blocks. - Mirror skill changes to the vendored sdk/python copies. Also picks up CodeRabbit review fixes: - spec.py: re-enforce _MIN_JOB_LENGTH after strip (Pydantic min_length runs before @field_validator), with vague-phrase check first so the more specific diagnosis wins. Replace EN DASH with hyphen for RUF001. - test_spec_render.py: raw regex string for RUF043. - agent-spec.md template: move parse-rule guidance out of the strict Constraints / Allowed Changes / Open Questions sections into the top-level prose block. Test suite trimmed from 80 to 62 tests by removing assertions that re-state Field declarations or test Pydantic itself. Remaining tests lock validators, the markdown round-trip, parser invariants, and the new convention helpers.
aleckhoury
force-pushed
the
fp-161-spec-schema-pydantic/akhoury
branch
from
c91baf3 to
4cfda64
Compare
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
`@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.md`:
- Around line 21-27: The rules list in agent-spec.md omits parser-enforced
bullet-list sections; update the Section-rules block to include `Categories` and
`Success Criteria` alongside `Constraints` and `Open Questions` so the parser
enforces list-only behavior for those sections; ensure you match the existing
phrasing/format (e.g., "- **Bullet sections** (`Constraints`, `Open Questions`,
`Categories`, `Success Criteria`): list items only. If the list is empty, write
`_(none)_`") so generated specs won't fail parsing.
🪄 Autofix (Beta)
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: e5de852a-03c4-49ce-aaf5-b0e136dee2cf
⛔ Files ignored due to path filters (3)
sdk/python/nemo-platform/src/nemo_platform/skills/nemo-build-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/references/templates/agent-spec.mdis excluded by!sdk/**
📒 Files selected for processing (10)
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.mdplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/entities.pyplugins/nemo-agents/src/nemo_agents_plugin/spec.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_entities.pyplugins/nemo-agents/tests/unit/test_spec.pyplugins/nemo-agents/tests/unit/test_spec_render.py
💤 Files with no reviewable changes (1)
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/nemo-agents/src/nemo_agents_plugin/spec.py
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.md (1)
21-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSection-rules list still incomplete.
CategoriesandSuccess Criteriaare also bullet-only sections but not listed here. Add them to prevent parse failures.Suggested fix
-- **Bullet sections** (`Constraints`, `Open Questions`): list items only. +- **Bullet sections** (`Categories`, `Constraints`, `Success Criteria`, `Open Questions`): list items only.🤖 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 `@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.md` around lines 21 - 22, The template's list of "bullet-only" sections is missing "Categories" and "Success Criteria", causing parse failures; update the rule block that currently mentions "Constraints" and "Open Questions" to also include "Categories" and "Success Criteria" as bullet-only sections and ensure the same empty-list convention (use `_(none)_`) is documented for them; look for the section rules block in agent-spec.md that describes "Bullet sections" and add those two section names so the parser enforces bullet-only content consistently.
🤖 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.
Duplicate comments:
In
`@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.md`:
- Around line 21-22: The template's list of "bullet-only" sections is missing
"Categories" and "Success Criteria", causing parse failures; update the rule
block that currently mentions "Constraints" and "Open Questions" to also include
"Categories" and "Success Criteria" as bullet-only sections and ensure the same
empty-list convention (use `_(none)_`) is documented for them; look for the
section rules block in agent-spec.md that describes "Bullet sections" and add
those two section names so the parser enforces bullet-only content consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aac63af3-21ee-4e7b-9cfd-9068e4c6b03d
⛔ Files ignored due to path filters (3)
sdk/python/nemo-platform/src/nemo_platform/skills/nemo-build-agent/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/SKILL.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/skills/nemo-spec/references/templates/agent-spec.mdis excluded by!sdk/**
📒 Files selected for processing (10)
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.mdpackages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/references/templates/agent-spec.mdplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/entities.pyplugins/nemo-agents/src/nemo_agents_plugin/spec.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_entities.pyplugins/nemo-agents/tests/unit/test_spec.pyplugins/nemo-agents/tests/unit/test_spec_render.py
💤 Files with no reviewable changes (1)
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-build-agent/SKILL.md
- plugins/nemo-agents/tests/unit/test_spec_render.py
- plugins/nemo-agents/tests/unit/test_entities.py
- plugins/nemo-agents/tests/unit/test_spec.py
- packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-spec/SKILL.md
- plugins/nemo-agents/src/nemo_agents_plugin/spec.py
- plugins/nemo-agents/src/nemo_agents_plugin/entities.py
aleckhoury
added a commit
that referenced
this pull request
Jun 1, 2026
Address PR #94 review feedback. The spec_file_ref field on Agent (and the matching --spec-file-ref CLI flag, API request field, and SPEC_FILE_REF shell ceremony in the skills) is redundant under the project's stated constraints: spec and agent always live in the same workspace, and the spec filename is the industry-standard AGENTSpec.md. The location is fully derivable from (workspace, agent_name). - Remove spec_file_ref from Agent entity, CreateAgentRequest schema, POST /agents handler, --spec-file-ref CLI option, and openapi.yaml. - Add canonical helpers in nemo_agents_plugin.entities: AGENT_SPEC_FILENAME, agent_spec_fileset_name, agent_spec_file_ref. Downstream consumers (analyst, Studio, optimization loop) call these rather than reconstructing the path inline; layout changes are then a one-function migration. - Canonicalize the filename to AGENTSpec.md across nemo-spec and nemo-build-agent skills and template; drop the SPEC_FILE_REF shell variable and the --spec-file-ref flag from skill bash blocks. - Mirror skill changes to the vendored sdk/python copies. Also picks up CodeRabbit review fixes: - spec.py: re-enforce _MIN_JOB_LENGTH after strip (Pydantic min_length runs before @field_validator), with vague-phrase check first so the more specific diagnosis wins. Replace EN DASH with hyphen for RUF001. - test_spec_render.py: raw regex string for RUF043. - agent-spec.md template: move parse-rule guidance out of the strict Constraints / Allowed Changes / Open Questions sections into the top-level prose block. Test suite trimmed from 80 to 62 tests by removing assertions that re-state Field declarations or test Pydantic itself. Remaining tests lock validators, the markdown round-trip, parser invariants, and the new convention helpers. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
aleckhoury
force-pushed
the
fp-161-spec-schema-pydantic/akhoury
branch
from
c367370 to
870a16f
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Documentation preview is readyPreview: https://nvidia-nemo.github.io/nemo-platform/pr-preview/pr-94/pr-94/ Built from This preview is deployed from this PR branch, updates when docs changes are pushed, and will be removed when the PR closes. |
Make the agent spec a typed, server-side-visible artifact instead of an unstructured local markdown file, and rebuild the onboarding skill chain around it. What changes - New AgentSpec Pydantic model (nemo_agents_plugin.spec) with hard validators on Job (vague-phrase rejection, min length) and Framework (LangGraph + NAT vs. needs-wrapper enum). 12 body sections + 2 front-matter fields, sourced from the POR's existing + proposed sections table. - New spec_render module with bidirectional AgentSpec <-> markdown round-trip. Strict parser: rejects unknown sections, duplicate sections, and malformed labeled bullets before the value reaches the Pydantic model. - spec_file_ref: FilesetRef | None added to the Agent entity and CreateAgentRequest, with a --spec-file-ref flag on `nemo agents create`. Lets `nemo agents list` answer "which agents have an onboarded spec?" with a single field check. - nemo-explore SKILL.md rewritten to be explore-first and schema-driven: scans the codebase, fills every inferable field, presents the full spec in a single review block, and only asks the user for gaps. Replaces the hardcoded 12-question Q&A loop. - nemo-spec SKILL.md rewritten to validate against AgentSpec, refuse to write when Job or Framework is unresolved, upload the canonical copy to a per-agent Filesets fileset (Fileset is canonical, local file is a write-through cache), and hand SPEC_FILE_REF to nemo-build-agent. - nemo-build-agent SKILL.md updated to thread SPEC_FILE_REF through all three `nemo agents create` invocations (initial deploy, RAG redeploy, guardrails redeploy). - Spec template (references/templates/agent-spec.md) rewritten to match the new schema + renderer format. Drops the Known Issues section (handled by the Insights plugin's first-class entity instead). - Plugin OpenAPI regenerated; SDK skill copies re-vendored. Tests - 45 new unit tests across test_spec.py and test_spec_render.py covering the schema, validators, JSON-schema export, render output shape, round-trip identity, and structural parse errors. - test_entities.py / test_cli.py extended for spec_file_ref on Agent, CreateAgentRequest, and the new CLI flag. - 102 tests pass across the touched files. ruff + ruff format + ty clean on changed files (two ty diagnostics pre-existing in setup.py, unrelated). Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Address PR #94 review feedback. The spec_file_ref field on Agent (and the matching --spec-file-ref CLI flag, API request field, and SPEC_FILE_REF shell ceremony in the skills) is redundant under the project's stated constraints: spec and agent always live in the same workspace, and the spec filename is the industry-standard AGENTSpec.md. The location is fully derivable from (workspace, agent_name). - Remove spec_file_ref from Agent entity, CreateAgentRequest schema, POST /agents handler, --spec-file-ref CLI option, and openapi.yaml. - Add canonical helpers in nemo_agents_plugin.entities: AGENT_SPEC_FILENAME, agent_spec_fileset_name, agent_spec_file_ref. Downstream consumers (analyst, Studio, optimization loop) call these rather than reconstructing the path inline; layout changes are then a one-function migration. - Canonicalize the filename to AGENTSpec.md across nemo-spec and nemo-build-agent skills and template; drop the SPEC_FILE_REF shell variable and the --spec-file-ref flag from skill bash blocks. - Mirror skill changes to the vendored sdk/python copies. Also picks up CodeRabbit review fixes: - spec.py: re-enforce _MIN_JOB_LENGTH after strip (Pydantic min_length runs before @field_validator), with vague-phrase check first so the more specific diagnosis wins. Replace EN DASH with hyphen for RUF001. - test_spec_render.py: raw regex string for RUF043. - agent-spec.md template: move parse-rule guidance out of the strict Constraints / Allowed Changes / Open Questions sections into the top-level prose block. Test suite trimmed from 80 to 62 tests by removing assertions that re-state Field declarations or test Pydantic itself. Remaining tests lock validators, the markdown round-trip, parser invariants, and the new convention helpers. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Second-pass audit of the spec/entity test additions, looking for tests that lock Pydantic, Field declarations, or constants against themselves rather than behavior we wrote. Cuts: - test_no_spec_file_ref_field, test_spec_file_ref_field_rejected: assert pydantic silently drops unknown keys without ``extra="forbid"``. - test_canonical_filename: ``assert AGENT_SPEC_FILENAME == "AGENTSpec.md"`` locks a constant against itself. - test_file_ref_uses_active_workspace: redundant with test_file_ref_combines_workspace_fileset_and_filename; both exercise the same one-line helper. - test_missing_framework_rejected: tests pydantic's required-field handling — framework being required is in the type signature. - test_category_count_out_of_range_rejected: tests ``Field(min_length=3, max_length=6)`` against itself. - test_validation_error_propagates: tests that parse_spec calls AgentSpec(...), which is the next line in the function. 53 tests remain; all genuinely lock validator behavior, the markdown round-trip contract, parser invariants, or the convention helpers. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
The CLI's create handler builds its payload from a fixed dict literal with no source for spec_file_ref — asserting the key is absent locks nothing the code could plausibly violate. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
The lsof + curl /v1/models block in nemo-build-agent's pre-flight was a byte-for-byte duplicate of nemo-status's Step 1. Replace it with a one-line reference so platform-health probing has a single owner and future probe changes (new component, port change) land in one place. Service-specific endpoint checks (Files API, Agents API) stay local to the skills that need them — nemo-status doesn't probe those today, and expanding it would change its character from user-facing dashboard to programmatic health-check service. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
…and Notes Final-sweep cleanups before merge: - cli.py: revert the leftover ``payload: dict[str, Any]`` annotation. It was needed when ``payload`` had a conditional ``spec_file_ref`` update that widened the inferred type; with that gone the annotation is pure diff noise. ``cli.py`` is now byte-identical to ``main``. - Rename the spec section ``## Eval Command`` to ``## Eval Command Notes`` to match the underlying ``eval_command_notes`` field name. A human reading the markdown could reasonably put the actual command under the ``Eval Command`` header, but the runnable command lives in the ``eval_command`` front matter; this section is for notes about eval state. Updated in the renderer, template, both skills, and the SDK vendored copies. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
The manual yaml edit in 4cfda64 removed the spec_file_ref field but did not pick up the new Agent class-docstring sentence pointing at agent_spec_file_ref. Regenerated via make refresh-openapi. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
aleckhoury
force-pushed
the
fp-161-spec-schema-pydantic/akhoury
branch
from
acdbad5 to
695a420
Compare
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Signed-off-by: Alec Khoury <akhoury@nvidia.com>
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.
Summary
Make the agent spec a typed, server-side-visible artifact instead of an unstructured local markdown file, and rebuild the onboarding skill chain around it.
AgentSpecPydantic model (nemo_agents_plugin.spec) with hard validators onJob(vague-phrase rejection, min length) andFramework(langgraph-natvsneeds-wrapperenum). 12 body sections + 2 front-matter fields, sourced from the POR's existing + proposed sections table.spec_rendermodule with bidirectionalAgentSpec↔ markdown round-trip. Strict parser: rejects unknown sections, duplicate sections, and malformed labeled bullets before the value reaches the Pydantic model.spec_file_ref: FilesetRef | Noneon theAgententity andCreateAgentRequest, with a--spec-file-refflag onnemo agents create. Letsnemo agents listanswer "which agents have an onboarded spec?" with a single field check.nemo-exploreSKILL.md rewritten to be explore-first and schema-driven: scans the codebase, fills every inferable field, presents the full spec in a single review block, and only asks for gaps. Replaces the hardcoded 12-question Q&A loop.nemo-specSKILL.md rewritten to validate againstAgentSpec, refuse to write whenJoborFrameworkis unresolved, upload the canonical copy to a per-agent Filesets fileset (Fileset is canonical, local file is a write-through cache), and handSPEC_FILE_REFtonemo-build-agent.nemo-build-agentSKILL.md updated to threadSPEC_FILE_REFthrough all threenemo agents createinvocations.Known Issuessection (handled by the Insights plugin's first-class entity instead).Test plan
test_spec.pyandtest_spec_render.pycovering schema, validators, JSON-schema export, render output shape, round-trip identity, and structural parse errorstest_entities.py/test_cli.pyextended forspec_file_refonAgent,CreateAgentRequest, and the new CLI flagpytest plugins/nemo-agents/tests/unit/test_{spec,spec_render,entities,cli,agents_api}.py)ruff check+ruff format --checkcleanty checkclean on changed files (two pre-existing diagnostics insetup.pyunrelated, verified via stash)pre-commit run -apasses all hooks exceptstudio-lint-staged(pre-existing local node-version mismatch, only fires under-a)nemo-explorecodebase scan → single review pass →nemo-specvalidate + Filesets upload →nemo-build-agentcreate with--spec-file-refpopulatedNotes for reviewers
nemo-explore/SKILL.md, not in the schema docstrings, so the skill stays the single source of truth for interview prompts and the schema stays terse.Frameworkis marked planned-deprecation in JSON schema (x-planned-deprecation) — expected to relax as NAT supports more frameworks; flagged so future-us can delete the field cleanly.Known Issuessection was deliberately dropped from the schema per the design discussion: known issues / recurring failure patterns are first-classInsightentities owned by the insights plugin, not duplicated into markdown.Summary by CodeRabbit
New Features
Documentation
Refactor