Skip to content

feat(cli): document ng_init_resources_server generated config inline (#1205 friction #7) - #1597

Merged
wprazuch merged 3 commits into
mainfrom
wprazuch/generated-config-comments
Jun 16, 2026
Merged

feat(cli): document ng_init_resources_server generated config inline (#1205 friction #7)#1597
wprazuch merged 3 commits into
mainfrom
wprazuch/generated-config-comments

Conversation

@wprazuch

@wprazuch wprazuch commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Friction point #7 — "No inline documentation in generated configs"

ng_init_resources_server generated a config with zero comments, so a new author had no way
to tell what domain: other should be, what type: resources_servers meant, that policy_model
is resolved elsewhere, or which fields are required — without reading external docs. This is
friction point 7 in #1205 (also relates to #191).

Change

Add inline # comments to every field in the generated template — server type, the full list of
valid domain values, the cross-reference structs, dataset splits, and the license requirement.
The domain comment lists the values factually ("change 'other' to the closest fit") and
deliberately does not editorialize the taxonomy (that's the separate #600 discussion).

Generated config, after

# Resources server: owns this environment's task verification (verify()) and reward.
my_server_resources_server:          # instance name — how agents/CLI refer to this server
  resources_servers:                 # server type: resources_servers | responses_api_agents | responses_api_models
    my_server:                       # implementation directory under resources_servers/
      entrypoint: app.py             # server entry module
      domain: other                  # capability under test; one of: math, coding, agent, knowledge,
                                     #   instruction_following, long_context, safety, games, translation,
                                     #   e2e, rlhf, other. Change 'other' to the closest fit.
      verified: false                # set true once the benchmark has been baselined and reviewed
...

Why verified: false is now in the template

The add-verified-flag pre-commit hook (scripts/add_verified_flag.py) rewrites a resources-server
config via yaml.safe_load + yaml.dump whenever verified is missing — which strips all
comments. Since generated configs previously had no verified field, that hook would have nuked
these new comments on the author's first commit. Emitting verified: false in the template makes
the hook a no-op, so the comments survive. No hook change, no new dependency.

Testing

test_init_resources_server_includes_domain extended to assert: the generated config still parses
and validates, verified: false is present, inline comments exist, and — crucially — running
ensure_verified_flag on the generated file returns False and leaves it byte-for-byte
unchanged (comments preserved). pytest tests/unit_tests/test_cli.py 14/14, ruff clean.

Pure additive change — only affects what new servers get generated; existing configs and runtime
behavior are untouched.

@copy-pr-bot

copy-pr-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cmunley1

cmunley1 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

/ok to test 192a7f3

cmunley1
cmunley1 reviewed Jun 15, 2026
View reviewed changes
Comment thread nemo_gym/cli.py Resolved
cmunley1
cmunley1 reviewed Jun 15, 2026
View reviewed changes
Comment thread nemo_gym/cli.py Resolved
cmunley1
cmunley1 reviewed Jun 15, 2026
View reviewed changes
Comment thread nemo_gym/cli.py Resolved
cmunley1
cmunley1 reviewed Jun 15, 2026
View reviewed changes
Comment thread nemo_gym/cli.py Resolved
@wprazuch

wprazuch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test adb1c3c

@wprazuch
wprazuch force-pushed the wprazuch/generated-config-comments branch from adb1c3c to 512a4d0 Compare June 15, 2026 13:59
@wprazuch

wprazuch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 512a4d0

wprazuch added 3 commits June 15, 2026 16:04
@wprazuch
feat(cli): document ng_init_resources_server generated config inline
d861b6f 
Generated server configs shipped with zero comments — new authors couldn't tell
what each field meant or which values were valid without consulting external
docs (#1205 friction point #7). Add inline comments to every field in the
generated template (server type, domain values, cross-references, dataset
splits, license).

Also emit 'verified: false' in the generated resources_servers block so the
add-verified-flag pre-commit hook is a no-op: its yaml.safe_load + yaml.dump
round-trip would otherwise rewrite the file and strip the new comments on the
author's first commit. Test asserts the hook leaves the generated config byte
-for-byte unchanged.

Addresses #1205 friction point #7.

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
docs(cli): tighten generated-config comment accuracy
e6a0f7a 
Fact-checked the generated comments against the code:
- simple_agent isn't single-turn (it runs a tool-calling loop bounded by
  max_steps) — drop the 'single-turn' wording.
- num_repeats repeats each example N times (pass@k/mean@k), not 'rollouts per
  task'.
- note huggingface_identifier as an alternative to gitlab_identifier (both are
  first-class).

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
docs(cli): address review on generated-config comments
c720c42 
- reword resources_server comment (tools, state and verification)
- rephrase agent server config header
- drop 'capability under test' framing on domain (eval-only connotation)
- remove gitlab_identifier from generated train/validation splits;
  note remote source as opt-in (gitlab/huggingface are Optional)

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@wprazuch
wprazuch force-pushed the wprazuch/generated-config-comments branch from 512a4d0 to c720c42 Compare June 15, 2026 14:04
@wprazuch

wprazuch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test c720c42

1 similar comment
@wprazuch

wprazuch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test c720c42

@wprazuch
wprazuch enabled auto-merge (squash) June 15, 2026 14:13
@wprazuch

wprazuch commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test c720c42

1 similar comment
@wprazuch

wprazuch commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test c720c42

@marta-sd

marta-sd commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

/ok to test c720c42

@wprazuch
wprazuch merged commit fd1c91c into main Jun 16, 2026
16 checks passed
@wprazuch
wprazuch deleted the wprazuch/generated-config-comments branch June 16, 2026 11:25
@wprazuch wprazuch mentioned this pull request Jun 17, 2026
Merged
wprazuch added a commit that referenced this pull request Jun 22, 2026
@wprazuch
feat(cli): document ng_init_resources_server generated config inline (#…
3516269 
…1205 friction #7) (#1597)

## Friction point #7 — "No inline documentation in generated configs"

`ng_init_resources_server` generated a config with **zero comments**, so
a new author had no way
to tell what `domain: other` should be, what `type: resources_servers`
meant, that `policy_model`
is resolved elsewhere, or which fields are required — without reading
external docs. This is
friction point 7 in #1205 (also relates to #191).

## Change

Add inline `#` comments to every field in the generated template —
server type, the full list of
valid `domain` values, the cross-reference structs, dataset splits, and
the license requirement.
The `domain` comment lists the values factually ("change 'other' to the
closest fit") and
deliberately does **not** editorialize the taxonomy (that's the separate
#600 discussion).

### Generated config, after
```yaml
# Resources server: owns this environment's task verification (verify()) and reward.
my_server_resources_server:          # instance name — how agents/CLI refer to this server
  resources_servers:                 # server type: resources_servers | responses_api_agents | responses_api_models
    my_server:                       # implementation directory under resources_servers/
      entrypoint: app.py             # server entry module
      domain: other                  # capability under test; one of: math, coding, agent, knowledge,
                                     #   instruction_following, long_context, safety, games, translation,
                                     #   e2e, rlhf, other. Change 'other' to the closest fit.
      verified: false                # set true once the benchmark has been baselined and reviewed
...
```

## Why `verified: false` is now in the template

The `add-verified-flag` pre-commit hook (`scripts/add_verified_flag.py`)
rewrites a resources-server
config via `yaml.safe_load` + `yaml.dump` **whenever `verified` is
missing** — which strips all
comments. Since generated configs previously had no `verified` field,
that hook would have nuked
these new comments on the author's first commit. Emitting `verified:
false` in the template makes
the hook a **no-op**, so the comments survive. No hook change, no new
dependency.

## Testing

`test_init_resources_server_includes_domain` extended to assert: the
generated config still parses
and validates, `verified: false` is present, inline comments exist, and
— crucially — running
`ensure_verified_flag` on the generated file returns `False` and leaves
it **byte-for-byte
unchanged** (comments preserved). `pytest tests/unit_tests/test_cli.py`
14/14, ruff clean.

Pure additive change — only affects what new servers get generated;
existing configs and runtime
behavior are untouched.

---------

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
wprazuch added a commit that referenced this pull request Jun 24, 2026
@wprazuch
docs(config): document the Domain enum (#1205 friction #9 / FEP-1023) (…
f378fee 
…#1633)

## What

Adds a docstring to the `Domain` enum in `config_types.py` — a one-line
description per value with a recognizable example benchmark, plus
guidance to **pick the most specific domain** and use `other` only as a
catch-all (addresses the
[#600](#600) 'agent is too
broad / no guidance' concern **without** redefining the taxonomy).

```
math   — mathematical problem solving (e.g. AIME, MATH, GSM8K).
coding — code generation, repair, or execution (e.g. SWE-bench, LiveCodeBench).
agent  — multi-step, tool-using tasks (e.g. tau2, workplace_assistant). Prefer a more specific value when it fits.
... (knowledge, instruction_following, long_context, safety, games, translation, e2e, rlhf, other)
```

## Why

Epic [#1205](#1205) friction 9
(naming confusion). Surfaces on IDE hover and when reading
`config_types`; complements the inline `domain` comment added in #1597.

## Tests

`test_domain_enum_documents_every_value` asserts every `Domain` value
appears in the docstring, so a newly-added domain can't ship
undocumented.

---------

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
ritaneves pushed a commit that referenced this pull request Jun 25, 2026
@wprazuch @ritaneves
docs(config): document the Domain enum (#1205 friction #9 / FEP-1023) (…
2da9142 
…#1633)

## What

Adds a docstring to the `Domain` enum in `config_types.py` — a one-line
description per value with a recognizable example benchmark, plus
guidance to **pick the most specific domain** and use `other` only as a
catch-all (addresses the
[#600](#600) 'agent is too
broad / no guidance' concern **without** redefining the taxonomy).

```
math   — mathematical problem solving (e.g. AIME, MATH, GSM8K).
coding — code generation, repair, or execution (e.g. SWE-bench, LiveCodeBench).
agent  — multi-step, tool-using tasks (e.g. tau2, workplace_assistant). Prefer a more specific value when it fits.
... (knowledge, instruction_following, long_context, safety, games, translation, e2e, rlhf, other)
```

## Why

Epic [#1205](#1205) friction 9
(naming confusion). Surfaces on IDE hover and when reading
`config_types`; complements the inline `domain` comment added in #1597.

## Tests

`test_domain_enum_documents_every_value` asserts every `Domain` value
appears in the docstring, so a newly-added domain can't ship
undocumented.

---------

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
Signed-off-by: Rita Fernandes Neves <rfernandesne@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmunley1 cmunley1 cmunley1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wprazuch @cmunley1 @marta-sd